2 Commits

Author	SHA1	Message	Date
shankar0123	530593507b	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).	2026-04-29 20:28:53 +00:00
shankar0123	7e4d423561	feat(scep-intune): parser + validator for Microsoft Intune Connector challenge format ... Phase 7 of the SCEP RFC 8894 + Intune master bundle. Adds the internal/scep/intune package that validates Microsoft Intune Certificate Connector signed challenges embedded in SCEP CSR challengePassword attributes. This is the parsing/validation foundation; Phase 8 wires it into the SCEP service dispatcher. What's included: * doc.go — package architecture (Intune cloud → Connector → certctl SCEP server) + 'what this package is NOT' guard rails. We do NOT implement full JOSE: no JKU / kid / x5c trust, no JWKS fetch. Trust anchor is operator-supplied at startup and pinned. The package does NOT call Microsoft's API directly — the Connector already did that; we validate its signed attestation. * trust_anchor.go — LoadTrustAnchor(path) reads a PEM bundle of Intune Connector signing certs. Skips non-CERTIFICATE PEM blocks (operators sometimes paste chains with the priv key by mistake). Rejects empty bundles + expired certs at startup with an operator-actionable message including the cert subject. SIGHUP reload lands in Phase 8.5; today it's load-once-at-boot. * claim.go — ChallengeClaim struct + DeviceMatchesCSR helper. Set-equality semantics for SAN-DNS/SAN-RFC822/SAN-UPN: the CSR must carry EXACTLY the claim's elements, no extras and no missing. Empty claim slice = no constraint on that dimension. Per-dimension typed errors (ErrClaimCNMismatch / ErrClaimSANDNSMismatch / ErrClaimSANRFC822Mismatch / ErrClaimSANUPNMismatch) so audit logs surface the failure dimension without string-matching. extractUPNSans is stubbed to return nil with documented fail-closed behavior — non-empty UPN claims fail the equalSets check (correct behavior; the rare deploy that pins UPN SANs hot-fixes the ASN.1 walker per the inline comment). * replay.go — ReplayCache: bounded in-memory cache of seen nonces with TTL. Sized for 100,000 entries (60-min Connector validity × 25 RPS Intune fleet steady-state ≈ 90,000 challenges/hour with headroom). sync.Map for concurrent read/write; janitor goroutine wakes every TTL/4 to evict expired entries; at-cap O(N) oldest-eviction (rarely fires; janitor keeps the cache below cap). Redis-backed variant deferred to V3-Pro. * challenge.go — the load-bearing piece: - ParseChallenge(raw) splits the JWT-like compact serialization into header/payload/signature and base64url-decodes each. Tolerates both padded + unpadded encodings (some Connector builds emit padded; RFC 7515 §2 says unpadded; we accept both). Validates the header parses as JSON before returning so the malformed-signal lands earlier in the pipeline. - ValidateChallenge(raw, trust, expectedAudience, now): 1. ParseChallenge 2. JWS signature verify over (segment0 || '.' || segment1) — re-derived from the raw on-wire bytes, NOT re-base64-encoded, per RFC 7515 §3.1 (re-encoding could produce a byte-different input than what was signed) 3. Signature alg dispatch: RS256: rsa.VerifyPKCS1v15(SHA-256) ES256: tries fixed-width r||s (JOSE-canonical) first, falls back to ASN.1 DER (older Connectors) alg=none: explicit reject with audit-log-friendly message (RFC 7515 §3.6 attack vector) HS*/PS*: rejected as 'unsupported alg' (no shared secret in our threat model) 4. Version-detection prelude (versionedChallenge struct + versionUnmarshalers map). Today's format is v1 (no explicit version field; absence IS the v1 signal). Adding v2 = adding a parser + a registration line; v1 path stays untouched. Defends against the inevitable Microsoft format change at ~30 LoC + 2 tests cost vs. a P0 incident. 5. Time bounds (iat / exp); audience pin (skipped when expectedAudience == ""). Replay protection is the CALLER's job (handler glues parser + cache; validator stays stateless + testable). * Typed errors: ErrChallengeMalformed / ErrChallengeSignature / ErrChallengeExpired / ErrChallengeNotYetValid / ErrChallengeWrongAudience / ErrChallengeReplay / ErrChallengeUnknownVersion. errors.Is-friendly so the handler can audit failure dimension. Tests (94.8% coverage): * challenge_test.go (18 tests): happy-path RS256 + ES256 fixed-width + ES256 DER; TamperedSignature; TamperedPayload; Expired; NotYetValid; WrongAudience; EmptyExpectedAudience disables check; RotatedTrustAnchor; EmptyTrustBundle; AlgNoneRejected; UnsupportedAlg (HS256); MissingAlg; VersionV1ExplicitOK; VersionUnknownRejected; MixedTrustBundle iter (skip key-type mismatches without surfacing as Signature err); NonJSONPayloadButValidSignature; Malformed cases (empty, missing dots, bad base64, non-JSON header — 9 sub-cases); PaddedBase64Tolerated. * claim_test.go (13 tests): per-dimension matching across CN + SAN-DNS + SAN-RFC822 + SAN-UPN; nil guards; case-insensitive DNS (RFC 4343); dedupe set-equality; empty claim = no constraint; UPN stub canary; normaliseSet edge cases; equalSets length mismatch. * replay_test.go (11 tests): first-fresh; duplicate-rejected; past-TTL-fresh; Sweep-evicts-expired; empty-nonce short-circuits; at-cap LRU eviction; default-cap=100k; Close-idempotent; TTL=0 disables janitor; concurrent-race-free (50 goroutines × 200 inserts); empty-nonce twice is fresh both times (we don't cache empties). * trust_anchor_test.go: HappyPath single + multi cert; SkipsNonCertBlocks (priv key + cert mix); EmptyBundleRejected; OnlyKeyBlocksRejected; ExpiredCertRejected (with subject CN in error); MalformedCertRejected; LoadTrustAnchor disk + EmptyPath + MissingFile. * fuzz_test.go: FuzzParseChallenge with seed corpus covering both the well-formed and the obvious-malformed shapes. Survived 187k execs in 21s without panic on the local burst; CI runs 5 min. Verification: * gofmt -l ./internal/scep/intune: clean * go vet ./internal/scep/intune/...: clean * staticcheck ./internal/scep/intune/...: clean * go test -count=1 -cover ./internal/scep/intune/...: 94.8% (target was ≥85%) * go vet ./internal/... ./cmd/...: clean (no rest-of-repo regressions) * No new CERTCTL_* env vars (those land in Phase 8 with the config gate); G-3 docs-drift CI guard not triggered. * No new HTTP routes; openapi-parity guard not triggered. Phase 8 will: - Add SCEPProfileConfig.Intune* env vars + preflight gate - Wire the validator into the SCEP service dispatcher (Intune-shaped challenges → validator; static → existing path) - Trust-anchor SIGHUP reload mirroring cmd/server/tls.go::watchSIGHUP - Per-claim rate limit + audit metrics Refs: cowork/scep-rfc8894-intune-master-prompt.md::Phase 7 cowork/scep-rfc8894-intune/progress.md	2026-04-29 14:38:35 +00:00