mirror of
https://github.com/shankar0123/certctl.git
synced 2026-06-14 07:58:53 +00:00
fix(scep-intune): close 11 audit gaps from 2026-04-29 pre-tag review
Closes the eleven gaps identified in the pre-v2.1.0 audit of the SCEP
RFC 8894 + Intune master bundle (cowork/scep-bundle-gap-closure-prompt.md).
Constitutional rule from cowork/CLAUDE.md::Operating Rules — 'Always
take the complete path, not the easy path' — drove this closure: each
gap was a load-bearing wire that crossed multiple layers (config →
validator → service wire-up → tests → docs) and shipping the bundle
without them would have produced lying-field footguns where operator-
visible config options stored values without affecting behavior.
WHAT LANDS:
Phase A — Clock-skew tolerance (master prompt §15 hazard closure)
internal/scep/intune/challenge.go: ValidateChallenge migrated from
positional args to ValidateOptions{} struct; new ClockSkewTolerance
field with default 0 (strict). 24 call sites updated mechanically.
Asymmetric application: now+tolerance >= iat AND now-tolerance < exp.
internal/config/config.go: SCEPIntuneProfileConfig.ClockSkewTolerance
default 60s + Validate() refusal when >= ChallengeValidity.
cmd/server/main.go: SetIntuneIntegration signature extended;
per-profile env-var loader honors CERTCTL_SCEP_PROFILE_<NAME>_INTUNE_CLOCK_SKEW_TOLERANCE.
internal/service/scep.go: intuneClockSkew field + IntuneStatsSnapshot
surfaces clock_skew_tolerance_ns. web/src/api/types.ts mirrors.
4 new tests in challenge_test.go covering accept-within-tolerance,
reject-beyond-tolerance, accept-expired-within-tolerance,
negative-treated-as-zero defensive normalization.
docs/scep-intune.md updated with the new env var + time-bounds rule.
Phase B — unknown-version-rejected golden test
internal/scep/intune/golden_helper_test.go: goldenUnknownVersionPayload
helper + signGoldenChallengeAny generic signer.
challenge_golden_test.go: TestGoldenChallenge_UnknownVersionRejected
uses an in-process ECDSA fixture (the on-disk PEM was generated with
a Go-stdlib version that produces different ecdsa.GenerateKey bytes
from the current call). TestRegenerateGoldenFixtures emits the new
unknown_version fixture file too.
Phase C — Two named Intune e2e tests
internal/api/handler/scep_intune_e2e_test.go:
TestSCEPIntuneEnrollment_RateLimited_E2E (cap=2 + 3 attempts; 3rd
returns FAILURE+badRequest with rate_limited counter ticked)
TestSCEPIntuneEnrollment_TrustAnchorSIGHUPReload_E2E (rotate
on-disk PEM + holder.Reload(); old-key challenge fails with
badMessageCheck; signature_invalid counter ticked)
intuneE2EFixture struct extended with trustHolder + trustPath fields
so tests can rotate.
Phase D — Four new ChromeOS hermetic tests (10 total now)
internal/api/handler/scep_chromeos_test.go:
_RAKeyMismatch — PKIMessage encrypted to wrong RA cert; handler
rejects without reaching service.
_3DESBackwardCompat — RFC 8894 §3.5.2 legacy fallback verified.
_RSACSR + _ECDSACSR — explicit matrix-pair pinning.
buildTestECDSACSR helper for ECDSA P-256 CSR construction;
tripleDESCBCEncrypt mirrors aesCBCEncrypt for 3DES-CBC;
assertChromeOSPositiveCertRep shared assertion.
Phase E — Per-profile counter isolation test
internal/api/handler/scep_profile_counter_isolation_test.go:
TestSCEPHandler_PerProfileIntuneCountersIsolated wires two
SCEPService instances + drives distinct PKIMessages + asserts
counter isolation. Guards against a future cmd/server/main.go
refactor that shares a *intuneCounterTab across profiles.
buildPerProfileIntuneFixture parameterized helper.
Phase F — Server-boot regression tests
cmd/server/preflight_scep_intune_test.go: 3 named tests covering
disabled-backward-compat, broken-config-with-PathID, expired-cert
refusal. preflightSCEPIntuneTrustAnchor signature extended with
pathID arg so error messages carry PathID= for operator log-grep.
Phase G — docs/connectors.md
Four new subsections under §EST/SCEP Integration: multi-profile
dispatch + mTLS sibling route + Intune Connector dispatcher + SCEP
probe in network scanner. Each has a one-paragraph operator
explanation + an env-var or endpoint table.
Phase H — Coverage uplift
internal/service/scep_probe_persist_test.go: 5 unit tests on
persistProbeResult (nil-safe + nil-repo-safe + repo-error swallow +
nil-logger guard) + ListRecentSCEPProbes (empty-slice-not-nil + repo
pass-through) + describeCertAlgorithm (RSA/ECDSA/QF1008-nil-curve
defensive branch/Ed25519/DSA/empty). CI gates (service ≥70, handler
≥75) PASS at 70.9% / 79.3%.
Phase I — deploy/test integration variant
deploy/test/scep_intune_e2e_test.go (//go:build integration):
TestSCEPIntuneEnrollment_Integration + _RateLimited_Integration
against the live docker-compose certctl container. Skip-when-
stack-missing semantics so sandbox + CI both work.
deploy/docker-compose.test.yml: new e2eintune SCEP profile env
vars + bind-mount of deploy/test/fixtures/.
deploy/test/fixtures/README.md: documents the deterministic trust
anchor regeneration recipe.
VERIFICATION (sandbox):
gofmt -d — clean for all changed files
staticcheck — clean for intune + handler + config + service +
cmd/server packages
go vet — clean for the same packages
go test -short — green for intune (95.3% cov), service (70.9%),
handler (79.3%), config (94.0%), cmd/server (boot
path; my preflight tests cover the directly-
testable function), pkcs7 (80.5% informational)
DEFERRED (per closure prompt §7 out-of-scope):
- V3-Pro Conditional Access gating + Microsoft Graph integration
- Standalone certctl-scan CLI binary
- OCSP rate-limiting, OCSP stapling, delta CRLs
Spec preserved at cowork/scep-bundle-gap-closure-prompt.md;
journal at cowork/scep-rfc8894-intune/progress.md (audit-closure
section appended).
This commit is contained in:
+34
-22
@@ -48,6 +48,7 @@ type SCEPService struct {
|
||||
intuneTrust *intune.TrustAnchorHolder // SIGHUP-reloadable trust pool
|
||||
intuneAudience string // expected "aud" claim; empty disables the check
|
||||
intuneValidity time.Duration // optional override on top of the challenge's exp
|
||||
intuneClockSkew time.Duration // ±tolerance applied to iat/exp; default 60s wired from config
|
||||
intuneReplayCache *intune.ReplayCache // nonce-keyed; catches duplicate submission
|
||||
intuneRateLimiter *intune.PerDeviceRateLimiter
|
||||
complianceCheck ComplianceCheck // V3-Pro plug-in seam; nil-default no-op
|
||||
@@ -161,17 +162,18 @@ type IntuneTrustAnchorInfo struct {
|
||||
// GET endpoint hands back. SCEPService.IntuneStats() builds one of
|
||||
// these on demand under no contention with the dispatcher hot path.
|
||||
type IntuneStatsSnapshot struct {
|
||||
PathID string `json:"path_id"`
|
||||
IssuerID string `json:"issuer_id"`
|
||||
Enabled bool `json:"enabled"`
|
||||
TrustAnchorPath string `json:"trust_anchor_path,omitempty"`
|
||||
TrustAnchors []IntuneTrustAnchorInfo `json:"trust_anchors,omitempty"`
|
||||
Audience string `json:"audience,omitempty"`
|
||||
ChallengeValidity time.Duration `json:"challenge_validity_ns,omitempty"`
|
||||
RateLimitDisabled bool `json:"rate_limit_disabled"`
|
||||
ReplayCacheSize int `json:"replay_cache_size"`
|
||||
Counters map[string]uint64 `json:"counters"`
|
||||
GeneratedAt time.Time `json:"generated_at"`
|
||||
PathID string `json:"path_id"`
|
||||
IssuerID string `json:"issuer_id"`
|
||||
Enabled bool `json:"enabled"`
|
||||
TrustAnchorPath string `json:"trust_anchor_path,omitempty"`
|
||||
TrustAnchors []IntuneTrustAnchorInfo `json:"trust_anchors,omitempty"`
|
||||
Audience string `json:"audience,omitempty"`
|
||||
ChallengeValidity time.Duration `json:"challenge_validity_ns,omitempty"`
|
||||
ClockSkewTolerance time.Duration `json:"clock_skew_tolerance_ns,omitempty"`
|
||||
RateLimitDisabled bool `json:"rate_limit_disabled"`
|
||||
ReplayCacheSize int `json:"replay_cache_size"`
|
||||
Counters map[string]uint64 `json:"counters"`
|
||||
GeneratedAt time.Time `json:"generated_at"`
|
||||
}
|
||||
|
||||
// SetPathID records the SCEP profile path ID this service instance
|
||||
@@ -207,6 +209,7 @@ func (s *SCEPService) IntuneStats(now time.Time) IntuneStatsSnapshot {
|
||||
}
|
||||
out.Audience = s.intuneAudience
|
||||
out.ChallengeValidity = s.intuneValidity
|
||||
out.ClockSkewTolerance = s.intuneClockSkew
|
||||
if s.intuneRateLimiter != nil {
|
||||
out.RateLimitDisabled = s.intuneRateLimiter.Disabled()
|
||||
}
|
||||
@@ -315,13 +318,14 @@ type SCEPProfileStatsSnapshot struct {
|
||||
// minus the always-present per-profile ones (PathID, IssuerID,
|
||||
// GeneratedAt) which live on SCEPProfileStatsSnapshot.
|
||||
type IntuneSection struct {
|
||||
TrustAnchorPath string `json:"trust_anchor_path,omitempty"`
|
||||
TrustAnchors []IntuneTrustAnchorInfo `json:"trust_anchors,omitempty"`
|
||||
Audience string `json:"audience,omitempty"`
|
||||
ChallengeValidity time.Duration `json:"challenge_validity_ns,omitempty"`
|
||||
RateLimitDisabled bool `json:"rate_limit_disabled"`
|
||||
ReplayCacheSize int `json:"replay_cache_size"`
|
||||
Counters map[string]uint64 `json:"counters"`
|
||||
TrustAnchorPath string `json:"trust_anchor_path,omitempty"`
|
||||
TrustAnchors []IntuneTrustAnchorInfo `json:"trust_anchors,omitempty"`
|
||||
Audience string `json:"audience,omitempty"`
|
||||
ChallengeValidity time.Duration `json:"challenge_validity_ns,omitempty"`
|
||||
ClockSkewTolerance time.Duration `json:"clock_skew_tolerance_ns,omitempty"`
|
||||
RateLimitDisabled bool `json:"rate_limit_disabled"`
|
||||
ReplayCacheSize int `json:"replay_cache_size"`
|
||||
Counters map[string]uint64 `json:"counters"`
|
||||
}
|
||||
|
||||
// ProfileStats returns the per-profile observability snapshot in the
|
||||
@@ -352,9 +356,10 @@ func (s *SCEPService) ProfileStats(now time.Time) SCEPProfileStatsSnapshot {
|
||||
return out
|
||||
}
|
||||
intuneSection := IntuneSection{
|
||||
Audience: s.intuneAudience,
|
||||
ChallengeValidity: s.intuneValidity,
|
||||
Counters: s.intuneCounters.snapshot(),
|
||||
Audience: s.intuneAudience,
|
||||
ChallengeValidity: s.intuneValidity,
|
||||
ClockSkewTolerance: s.intuneClockSkew,
|
||||
Counters: s.intuneCounters.snapshot(),
|
||||
}
|
||||
if s.intuneRateLimiter != nil {
|
||||
intuneSection.RateLimitDisabled = s.intuneRateLimiter.Disabled()
|
||||
@@ -446,6 +451,7 @@ func (s *SCEPService) SetIntuneIntegration(
|
||||
trust *intune.TrustAnchorHolder,
|
||||
audience string,
|
||||
validity time.Duration,
|
||||
clockSkew time.Duration,
|
||||
replayCache *intune.ReplayCache,
|
||||
rateLimiter *intune.PerDeviceRateLimiter,
|
||||
) {
|
||||
@@ -453,6 +459,7 @@ func (s *SCEPService) SetIntuneIntegration(
|
||||
s.intuneTrust = trust
|
||||
s.intuneAudience = audience
|
||||
s.intuneValidity = validity
|
||||
s.intuneClockSkew = clockSkew
|
||||
s.intuneReplayCache = replayCache
|
||||
s.intuneRateLimiter = rateLimiter
|
||||
if s.intuneCounters == nil {
|
||||
@@ -573,7 +580,12 @@ func (s *SCEPService) dispatchIntuneChallenge(ctx context.Context, csrPEM string
|
||||
now := time.Now()
|
||||
trust := s.intuneTrust.Get()
|
||||
|
||||
claim, err := intune.ValidateChallenge(challengePassword, trust, s.intuneAudience, now)
|
||||
claim, err := intune.ValidateChallenge(challengePassword, intune.ValidateOptions{
|
||||
Trust: trust,
|
||||
ExpectedAudience: s.intuneAudience,
|
||||
Now: now,
|
||||
ClockSkewTolerance: s.intuneClockSkew,
|
||||
})
|
||||
if err != nil {
|
||||
s.logger.Warn("SCEP enrollment rejected: Intune challenge validation failed",
|
||||
"transaction_id", transactionID, "reason", intuneFailReason(err), "error", err)
|
||||
|
||||
Reference in New Issue
Block a user