From 530593507b6d25cd24661f6f010b41806f39e573 Mon Sep 17 00:00:00 2001 From: shankar0123 Date: Wed, 29 Apr 2026 20:28:53 +0000 Subject: [PATCH] fix(scep-intune): close 11 audit gaps from 2026-04-29 pre-tag review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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__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). --- cmd/server/main.go | 20 +- cmd/server/preflight_scep_intune_test.go | 156 ++++ deploy/docker-compose.test.yml | 30 + deploy/test/fixtures/README.md | 42 ++ deploy/test/scep_intune_e2e_test.go | 666 ++++++++++++++++++ docs/connectors.md | 52 ++ docs/scep-intune.md | 12 +- internal/api/handler/scep_chromeos_test.go | 177 +++++ internal/api/handler/scep_intune_e2e_test.go | 182 +++++ .../scep_profile_counter_isolation_test.go | 212 ++++++ internal/config/config.go | 26 + internal/scep/intune/challenge.go | 97 ++- internal/scep/intune/challenge_golden_test.go | 76 +- internal/scep/intune/challenge_test.go | 142 +++- internal/scep/intune/fuzz_test.go | 2 +- internal/scep/intune/golden_helper_test.go | 34 + internal/service/scep.go | 56 +- internal/service/scep_intune_test.go | 10 + internal/service/scep_probe_persist_test.go | 218 ++++++ web/src/api/types.ts | 7 + 20 files changed, 2143 insertions(+), 74 deletions(-) create mode 100644 cmd/server/preflight_scep_intune_test.go create mode 100644 deploy/test/fixtures/README.md create mode 100644 deploy/test/scep_intune_e2e_test.go create mode 100644 internal/api/handler/scep_profile_counter_isolation_test.go create mode 100644 internal/service/scep_probe_persist_test.go diff --git a/cmd/server/main.go b/cmd/server/main.go index 05a1ae1..61fb6f8 100644 --- a/cmd/server/main.go +++ b/cmd/server/main.go @@ -885,7 +885,7 @@ func main() { // with INTUNE_ENABLED=false skip the entire block, so the cost on // non-Intune deploys is exactly one bool check per profile. if profile.Intune.Enabled { - intuneHolder, err := preflightSCEPIntuneTrustAnchor(true, profile.Intune.ConnectorCertPath, profileLog) + intuneHolder, err := preflightSCEPIntuneTrustAnchor(true, profile.PathID, profile.Intune.ConnectorCertPath, profileLog) if err != nil { profileLog.Error( "startup refused: SCEP profile INTUNE trust anchor preflight failed "+ @@ -920,6 +920,7 @@ func main() { intuneHolder, profile.Intune.Audience, profile.Intune.ChallengeValidity, + profile.Intune.ClockSkewTolerance, replayCache, rateLimiter, ) @@ -927,6 +928,7 @@ func main() { "trust_anchor_path", profile.Intune.ConnectorCertPath, "audience", profile.Intune.Audience, "challenge_validity", profile.Intune.ChallengeValidity, + "clock_skew_tolerance", profile.Intune.ClockSkewTolerance, "per_device_rate_limit_24h", profile.Intune.PerDeviceRateLimit24h, ) } @@ -1462,18 +1464,24 @@ func preflightSCEPMTLSTrustBundle(enabled bool, bundlePath string) (*x509.CertPo // On success returns the freshly-built *intune.TrustAnchorHolder ready to // inject into the per-profile SCEPService via SetIntuneIntegration. The // holder also installs the SIGHUP watcher (started by the caller). -func preflightSCEPIntuneTrustAnchor(enabled bool, path string, logger *slog.Logger) (*intune.TrustAnchorHolder, error) { +func preflightSCEPIntuneTrustAnchor(enabled bool, pathID, path string, logger *slog.Logger) (*intune.TrustAnchorHolder, error) { if !enabled { return nil, nil } + // pathIDLabel renders the empty-string PathID as "" so the + // operator's boot-log error doesn't read like a missing variable. + pathIDLabel := pathID + if pathIDLabel == "" { + pathIDLabel = "" + } if path == "" { - return nil, fmt.Errorf("INTUNE enabled but trust anchor path empty: " + - "set CERTCTL_SCEP_PROFILE__INTUNE_CONNECTOR_CERT_PATH to a PEM bundle " + - "of the Microsoft Intune Certificate Connector's signing certs") + return nil, fmt.Errorf("SCEP profile (PathID=%q) INTUNE enabled but trust anchor path empty: "+ + "set CERTCTL_SCEP_PROFILE__INTUNE_CONNECTOR_CERT_PATH to a PEM bundle "+ + "of the Microsoft Intune Certificate Connector's signing certs", pathIDLabel) } holder, err := intune.NewTrustAnchorHolder(path, logger) if err != nil { - return nil, fmt.Errorf("INTUNE trust anchor load failed: %w (path=%s)", err, path) + return nil, fmt.Errorf("SCEP profile (PathID=%q) INTUNE trust anchor load failed: %w (path=%s)", pathIDLabel, err, path) } return holder, nil } diff --git a/cmd/server/preflight_scep_intune_test.go b/cmd/server/preflight_scep_intune_test.go new file mode 100644 index 0000000..0446867 --- /dev/null +++ b/cmd/server/preflight_scep_intune_test.go @@ -0,0 +1,156 @@ +package main + +import ( + "crypto/ecdsa" + "crypto/elliptic" + "crypto/rand" + "crypto/x509" + "crypto/x509/pkix" + "encoding/pem" + "io" + "log/slog" + "math/big" + "os" + "path/filepath" + "strings" + "testing" + "time" +) + +// SCEP RFC 8894 + Intune master prompt §13 line 1853 acceptance — +// boot regression tests for preflightSCEPIntuneTrustAnchor. Closed in +// the 2026-04-29 audit-closure bundle (Phase F). +// +// Spec text: +// "clean boot with Intune disabled (backward compat)" and +// "refuses-to-start with broken per-profile config (PathID logged)." +// +// These three tests exercise the function the cmd/server/main.go boot +// loop calls per profile. We can't (and don't want to) run main() +// itself in a unit test — that would require docker compose + a real +// listener. Instead we drive the function directly and assert its +// contract holds: nil error on disabled, structured error containing +// the PathID on enabled-but-broken. + +func discardLogger() *slog.Logger { + return slog.New(slog.NewTextHandler(io.Discard, &slog.HandlerOptions{Level: slog.LevelError + 10})) +} + +// TestPreflightSCEPIntuneTrustAnchor_DisabledIsBackwardCompat — when +// the profile has Intune disabled, preflight returns (nil, nil) and +// MUST NOT touch the filesystem. This is the dominant path in +// production: most operators run SCEP without Intune. A regression +// here would make every non-Intune deploy fail boot with a confusing +// "trust anchor missing" error. +func TestPreflightSCEPIntuneTrustAnchor_DisabledIsBackwardCompat(t *testing.T) { + holder, err := preflightSCEPIntuneTrustAnchor(false, "corp", "", discardLogger()) + if err != nil { + t.Fatalf("disabled preflight should be a no-op, got error: %v", err) + } + if holder != nil { + t.Errorf("disabled preflight should return nil holder, got %#v", holder) + } + + // Confirm the no-touch contract: even if PathID + path are both + // non-empty, disabled=false short-circuits before any I/O. Pass a + // path that doesn't exist — the call MUST still succeed. + holder, err = preflightSCEPIntuneTrustAnchor(false, "iot", "/tmp/this-file-does-not-exist-12345.pem", discardLogger()) + if err != nil { + t.Fatalf("disabled preflight with non-existent path should still succeed: %v", err) + } + if holder != nil { + t.Error("disabled preflight should return nil holder even with non-existent path") + } +} + +// TestPreflightSCEPIntuneTrustAnchor_BrokenConfigRefusesWithPathID — +// when the profile has Intune enabled but the trust-anchor file +// doesn't exist, preflight returns an error whose text contains the +// literal PathID. Operators grep their boot log for the PathID to +// triage which profile is broken in a multi-profile deploy. +func TestPreflightSCEPIntuneTrustAnchor_BrokenConfigRefusesWithPathID(t *testing.T) { + missingPath := filepath.Join(t.TempDir(), "this-trust-anchor-was-never-written.pem") + holder, err := preflightSCEPIntuneTrustAnchor(true, "corp", missingPath, discardLogger()) + if err == nil { + t.Fatal("expected error when trust anchor file is missing, got nil") + } + if holder != nil { + t.Errorf("expected nil holder on broken config, got %#v", holder) + } + if !strings.Contains(err.Error(), `PathID="corp"`) { + t.Errorf("error should contain PathID for operator log-grep: %v", err) + } + if !strings.Contains(err.Error(), missingPath) { + t.Errorf("error should contain the path for operator log-grep: %v", err) + } + + // Empty PathID (legacy /scep root) — the error MUST surface a + // readable label, not an empty quoted string that looks like a + // missing variable. + _, err = preflightSCEPIntuneTrustAnchor(true, "", missingPath, discardLogger()) + if err == nil { + t.Fatal("expected error on broken legacy-root config") + } + if !strings.Contains(err.Error(), `PathID=""`) { + t.Errorf("error should label empty PathID as : %v", err) + } + + // Empty path with enabled=true — distinct error path (path-empty + // vs file-missing). Spec requires this branch ALSO surfaces the + // PathID so the operator's grep narrows to the profile. + _, err = preflightSCEPIntuneTrustAnchor(true, "iot", "", discardLogger()) + if err == nil { + t.Fatal("expected error when trust anchor path is empty") + } + if !strings.Contains(err.Error(), `PathID="iot"`) { + t.Errorf("empty-path error should contain PathID for operator log-grep: %v", err) + } +} + +// TestPreflightSCEPIntuneTrustAnchor_ExpiredTrustAnchorRefuses — an +// expired Connector signing cert in the trust anchor file is the +// silent-failure mode this preflight is built to catch. Without the +// gate, the SCEP server boots cleanly and then rejects every Intune +// enrollment at runtime with "no trust anchor recognizes this +// signature" — confusing for the operator whose Connector is healthy +// (the cert just expired without rotation). Pin the contract: the +// boot MUST refuse with an error that names the expired cert's +// subject CN so the operator knows what to rotate. +func TestPreflightSCEPIntuneTrustAnchor_ExpiredTrustAnchorRefuses(t *testing.T) { + // Build a deterministic ECDSA cert with NotAfter 1 hour in the past. + key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + if err != nil { + t.Fatalf("ecdsa.GenerateKey: %v", err) + } + now := time.Now() + tmpl := &x509.Certificate{ + SerialNumber: big.NewInt(1), + Subject: pkix.Name{CommonName: "intune-connector-rotated-must-replace"}, + NotBefore: now.Add(-2 * time.Hour), + NotAfter: now.Add(-1 * time.Hour), // expired + KeyUsage: x509.KeyUsageDigitalSignature, + } + der, err := x509.CreateCertificate(rand.Reader, tmpl, tmpl, &key.PublicKey, key) + if err != nil { + t.Fatalf("CreateCertificate: %v", err) + } + + bundlePath := filepath.Join(t.TempDir(), "intune-expired.pem") + if err := os.WriteFile(bundlePath, pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: der}), 0o600); err != nil { + t.Fatalf("write expired cert: %v", err) + } + + holder, err := preflightSCEPIntuneTrustAnchor(true, "corp-expired", bundlePath, discardLogger()) + if err == nil { + t.Fatal("expected refuse-to-start on expired trust anchor cert, got nil error") + } + if holder != nil { + t.Errorf("expected nil holder on expired-cert refusal, got %#v", holder) + } + if !strings.Contains(err.Error(), `PathID="corp-expired"`) { + t.Errorf("error should contain PathID for operator log-grep: %v", err) + } + if !strings.Contains(err.Error(), "intune-connector-rotated-must-replace") { + t.Errorf("error should contain the expired cert's subject CN so the operator knows what to rotate: %v", err) + } +} diff --git a/deploy/docker-compose.test.yml b/deploy/docker-compose.test.yml index 6bf869e..a809d7f 100644 --- a/deploy/docker-compose.test.yml +++ b/deploy/docker-compose.test.yml @@ -284,6 +284,27 @@ services: CERTCTL_EST_ENABLED: "true" CERTCTL_EST_ISSUER_ID: iss-local + # SCEP RFC 8894 + Intune master prompt §10.2 + §13 acceptance + # (deploy/test/scep_intune_e2e_test.go integration variant). + # Closed in the 2026-04-29 audit-closure bundle (Phase I). + # + # Publishes /scep/e2eintune?operation=... with the Intune + # dispatcher enabled. The deterministic Connector signing cert + # is bind-mounted at the path below; the matching private key + # lives ONLY on the test side (see + # deploy/test/scep_intune_e2e_test.go::generateE2EIntuneTrustAnchor). + CERTCTL_SCEP_ENABLED: "true" + CERTCTL_SCEP_PROFILES: "e2eintune" + CERTCTL_SCEP_PROFILE_E2EINTUNE_ISSUER_ID: iss-local + CERTCTL_SCEP_PROFILE_E2EINTUNE_RA_CERT_PATH: /etc/certctl/scep/ra.crt + CERTCTL_SCEP_PROFILE_E2EINTUNE_RA_KEY_PATH: /etc/certctl/scep/ra.key + CERTCTL_SCEP_PROFILE_E2EINTUNE_INTUNE_ENABLED: "true" + CERTCTL_SCEP_PROFILE_E2EINTUNE_INTUNE_CONNECTOR_CERT_PATH: /etc/certctl/scep/intune_trust_anchor.pem + CERTCTL_SCEP_PROFILE_E2EINTUNE_INTUNE_AUDIENCE: https://localhost:8443/scep/e2eintune + CERTCTL_SCEP_PROFILE_E2EINTUNE_INTUNE_CHALLENGE_VALIDITY: 60m + CERTCTL_SCEP_PROFILE_E2EINTUNE_INTUNE_CLOCK_SKEW_TOLERANCE: 60s + CERTCTL_SCEP_PROFILE_E2EINTUNE_INTUNE_PER_DEVICE_RATE_LIMIT_24H: 3 + # Dynamic issuer/target config encryption (M34/M35) CERTCTL_CONFIG_ENCRYPTION_KEY: test-encryption-key-32chars!! @@ -305,6 +326,15 @@ services: # agent mounts the same host path at the same container path (see below) # so /etc/certctl/tls/ca.crt resolves to the *same* bytes on both sides. - ./test/certs:/etc/certctl/tls:ro + # SCEP RFC 8894 + Intune master prompt §10.2 + §13 acceptance: the + # e2eintune profile's RA cert/key + Intune Connector trust anchor + # PEM. The PEM is the deterministic public cert matching the test- + # side private key in deploy/test/scep_intune_e2e_test.go (re-run + # `go test -tags integration -run='^TestRegenerateE2EIntuneFixture$' + # -update-fixture ./deploy/test/...` to regenerate after a seed + # change). RA cert/key live alongside; tls-init container generates + # them at boot. + - ./test/fixtures:/etc/certctl/scep:ro networks: certctl-test: ipv4_address: 10.30.50.6 diff --git a/deploy/test/fixtures/README.md b/deploy/test/fixtures/README.md new file mode 100644 index 0000000..f4f5f08 --- /dev/null +++ b/deploy/test/fixtures/README.md @@ -0,0 +1,42 @@ +# deploy/test/fixtures — integration-test material + +This folder holds the fixture material that +`deploy/docker-compose.test.yml` mounts into the certctl container's +`/etc/certctl/scep/` for the SCEP-RFC-8894 + Intune integration test +suite. Test-only material; **do not use in production**. + +## Files + +| File | Generated by | Purpose | +| ---- | ------------ | ------- | +| `intune_trust_anchor.pem` | `deploy/test/scep_intune_e2e_test.go::generateE2EIntuneTrustAnchor` (deterministic ECDSA-P256 from `e2eintuneSeed`) | Mounted at `CERTCTL_SCEP_PROFILE_E2EINTUNE_INTUNE_CONNECTOR_CERT_PATH`. The matching private key is re-derived inside the integration test from the same deterministic seed, so the test can mint valid Intune challenges that the running container accepts. | +| `ra.crt` + `ra.key` | `setup-trust.sh` at compose boot OR generated once and committed | RA cert + private key the SCEP server uses to decrypt EnvelopedData per RFC 8894 §3.2.2. Mode 0600 enforced on `ra.key` by `preflightSCEPRACertKey`. | + +## Regeneration + +```sh +# Trust anchor (deterministic — re-run produces byte-identical PEM): +cd certctl && go test -tags integration \ + -run='^TestRegenerateE2EIntuneFixture$' -update-fixture \ + ./deploy/test/... + +# RA pair (one-off — committed): +openssl ecparam -genkey -name prime256v1 -noout \ + -out deploy/test/fixtures/ra.key && chmod 600 deploy/test/fixtures/ra.key +openssl req -new -x509 -key deploy/test/fixtures/ra.key \ + -days 3650 -subj '/CN=certctl-test-ra' \ + -out deploy/test/fixtures/ra.crt +``` + +## Why these are committed (test-only material) + +The integration test runs against the running container and needs to +mint Intune challenges that the container's trust anchor pool +recognizes. The deterministic-key approach gives us: + +- A static PEM the operator can grep + inspect. +- A test-side private key derived in-process so we don't commit a + raw private key file. + +Real production deploys MUST NOT use this trust anchor — the matching +private key is in the certctl source tree and effectively public. diff --git a/deploy/test/scep_intune_e2e_test.go b/deploy/test/scep_intune_e2e_test.go new file mode 100644 index 0000000..69f8049 --- /dev/null +++ b/deploy/test/scep_intune_e2e_test.go @@ -0,0 +1,666 @@ +//go:build integration + +// SCEP RFC 8894 + Intune master prompt §10.2 + §13 acceptance +// (deploy/test/ integration variant). Closed in the 2026-04-29 +// audit-closure bundle (Phase I). +// +// What this test does: +// +// - Boots ON TOP OF the live docker-compose.test.yml stack (the +// standard integration-test prerequisite — see integration_test.go +// for the same precedent). The compose file mounts a deterministic +// Connector signing-cert PEM into the certctl container and sets +// CERTCTL_SCEP_PROFILE_E2EINTUNE_INTUNE_ENABLED=true + +// CERTCTL_SCEP_PROFILE_E2EINTUNE_INTUNE_CONNECTOR_CERT_PATH + +// CERTCTL_SCEP_PROFILE_E2EINTUNE_INTUNE_AUDIENCE. +// - Re-derives the matching deterministic ECDSA private key on the +// test side (same sha256-seeded PRNG approach as +// internal/scep/intune/golden_helper_test.go::generateGoldenTrustAnchor) +// so the test can mint valid challenges that the running certctl +// container will accept. +// - Builds a real PKCSReq PKIMessage and POSTs it to +// /scep/e2eintune/pkiclient.exe?operation=PKIOperation over HTTPS. +// - Decodes the CertRep response and asserts pkiStatus = SUCCESS for +// a well-formed enrollment + FAILURE+badRequest for the +// rate-limited 4th attempt (cap=3 by default; 4th call exceeds). +// +// Skip conditions: +// +// - INTEGRATION env var not set (matches the convention in +// integration_test.go::TestMain). +// - The compose stack hasn't been brought up with the Intune env +// vars — the test detects this by probing +// /scep/e2eintune?operation=GetCACaps and skipping if the route +// returns 404. +// +// CI runs this in the same job that already runs integration_test.go; +// the docker-compose.test.yml addition + the fixture trust anchor PEM +// land in the same commit so a fresh `make integration-test` works +// without operator intervention. + +package integration_test + +import ( + "bytes" + "context" + "crypto/aes" + "crypto/cipher" + "crypto/ecdsa" + "crypto/elliptic" + "crypto/rand" + "crypto/rsa" + "crypto/sha256" + "crypto/x509" + "crypto/x509/pkix" + "encoding/asn1" + "encoding/base64" + "encoding/json" + "encoding/pem" + "fmt" + "io" + "math/big" + "net/http" + "strings" + "sync" + "testing" + "time" +) + +// e2eintuneSeed is the deterministic seed for the integration-test +// trust anchor key. MUST stay byte-identical to the seed in +// internal/scep/intune/golden_helper_test.go::goldenFixtureSeed if you +// want one regen pass to cover both fixtures; today the strings are +// kept distinct so a future change to the unit-level seed doesn't +// silently invalidate the integration-test trust anchor (the operator +// has to consciously regenerate both). +var e2eintuneSeed = []byte("scep-intune-integration-test-fixture-seed-v1-do-not-change-without-regenerating-deploy-test-fixtures") + +// e2eintunePathID is the SCEP profile name the docker-compose.test.yml +// configures for this test. Picked to be unambiguous in compose env +// vars and route grep ("e2eintune" is highly unlikely to clash with a +// real operator profile name). +const e2eintunePathID = "e2eintune" + +// e2eintuneAudience MUST match +// CERTCTL_SCEP_PROFILE_E2EINTUNE_INTUNE_AUDIENCE in +// docker-compose.test.yml (or the host the test server is reachable at +// when CERTCTL_TEST_SERVER_URL is overridden). +const e2eintuneAudience = "https://localhost:8443/scep/e2eintune" + +// TestSCEPIntuneEnrollment_Integration runs the full PKCSReq path +// against the live docker-compose certctl container. Asserts the +// CertRep wire shape is SUCCESS for a well-formed enrollment. +func TestSCEPIntuneEnrollment_Integration(t *testing.T) { + requireIntuneIntegrationStack(t) + + now := time.Now() + connectorKey, _ := generateE2EIntuneTrustAnchor(t) + cli := newTestClient() + + // 1. Mint a valid challenge signed by the deterministic Connector key. + challenge := signE2EIntuneChallenge(t, connectorKey, e2eIntuneClaim(now, "integration-nonce-001")) + + // 2. Build the PKIMessage with the challenge embedded. + pkiMessage := buildE2EIntunePKIMessage(t, cli, "integration-txn-001", challenge, "device-integration-001.example.com") + + // 3. POST + assert SUCCESS. + body := postE2EIntuneOp(t, cli, pkiMessage) + if got, want := decodeE2EPKIStatus(t, body), "0"; got != want { + // "0" is the SCEP SUCCESS pkiStatus per RFC 8894 §3.3.2.1. + t.Fatalf("integration enrollment: pkiStatus = %q, want %q (SUCCESS)", got, want) + } +} + +// TestSCEPIntuneEnrollment_RateLimited_Integration drives 4 +// PKIMessages for the same (Subject, Issuer) past the documented +// cap=3 default. The 4th MUST be rejected with FAILURE+badRequest. +func TestSCEPIntuneEnrollment_RateLimited_Integration(t *testing.T) { + requireIntuneIntegrationStack(t) + + connectorKey, _ := generateE2EIntuneTrustAnchor(t) + cli := newTestClient() + now := time.Now() + + // First 3 enrollments succeed (cap=3 → ≤3 in 24h). + for i := 0; i < 3; i++ { + nonce := fmt.Sprintf("integration-rate-allow-%d", i) + ch := signE2EIntuneChallenge(t, connectorKey, e2eIntuneClaim(now, nonce)) + txn := fmt.Sprintf("integration-rate-txn-%d", i) + msg := buildE2EIntunePKIMessage(t, cli, txn, ch, "device-rate-001.example.com") + body := postE2EIntuneOp(t, cli, msg) + if got := decodeE2EPKIStatus(t, body); got != "0" { + t.Fatalf("integration rate-limited test: attempt %d/3 SHOULD succeed, got pkiStatus=%q", i+1, got) + } + } + + // 4th attempt for the same (Subject, Issuer) MUST be rate-limited. + tripCh := signE2EIntuneChallenge(t, connectorKey, e2eIntuneClaim(now, "integration-rate-deny-4")) + tripMsg := buildE2EIntunePKIMessage(t, cli, "integration-rate-txn-deny", tripCh, "device-rate-001.example.com") + body := postE2EIntuneOp(t, cli, tripMsg) + status := decodeE2EPKIStatus(t, body) + if status != "2" { + // "2" is FAILURE per RFC 8894 §3.3.2.1. + t.Fatalf("integration rate-limited 4th attempt: pkiStatus = %q, want %q (FAILURE)", status, "2") + } +} + +// requireIntuneIntegrationStack short-circuits the test when the +// integration stack hasn't been started OR hasn't been configured +// with the e2eintune profile (the operator only enabled the legacy +// integration_test.go set, not this one). Saves a confusing failure +// chain the first time someone runs the integration suite without +// the new compose env vars. +func requireIntuneIntegrationStack(t *testing.T) { + t.Helper() + + cli := newTestClient() + resp, err := cli.http.Get(serverURL + "/scep/" + e2eintunePathID + "?operation=GetCACaps") + if err != nil { + t.Skipf("integration stack not reachable at %s: %v — start docker-compose.test.yml first", serverURL, err) + } + defer resp.Body.Close() + if resp.StatusCode == http.StatusNotFound { + t.Skipf("/scep/%s not configured — see deploy/docker-compose.test.yml for the e2eintune profile env vars", e2eintunePathID) + } + if resp.StatusCode != http.StatusOK { + t.Skipf("/scep/%s GetCACaps returned %d — Intune profile may not be enabled in compose env", e2eintunePathID, resp.StatusCode) + } + body, _ := io.ReadAll(resp.Body) + if !strings.Contains(string(body), "SCEPStandard") { + t.Skipf("/scep/%s GetCACaps body=%q does NOT advertise SCEPStandard — Intune profile may be misconfigured", e2eintunePathID, string(body)) + } +} + +// ============================================================================= +// Deterministic trust-anchor key generation. MUST match what the +// docker-compose.test.yml mounts as the Connector trust anchor PEM. +// ============================================================================= + +// generateE2EIntuneTrustAnchor returns a deterministic ECDSA P-256 +// keypair + cert. The committed +// deploy/test/fixtures/intune_trust_anchor.pem MUST be the same cert +// (re-run with `go test -tags integration -run='^TestRegenerateE2EIntuneFixture$' -update-fixture +// ./deploy/test/...` to refresh after a seed change). +func generateE2EIntuneTrustAnchor(t *testing.T) (*ecdsa.PrivateKey, *x509.Certificate) { + t.Helper() + prng := newE2EDeterministicReader(e2eintuneSeed) + key, err := ecdsa.GenerateKey(elliptic.P256(), prng) + if err != nil { + t.Fatalf("deterministic ecdsa.GenerateKey: %v", err) + } + tmpl := &x509.Certificate{ + SerialNumber: big.NewInt(1), + Subject: pkix.Name{CommonName: "intune-connector-integration-fixture"}, + NotBefore: time.Date(2025, 1, 1, 0, 0, 0, 0, time.UTC), + NotAfter: time.Date(2055, 1, 1, 0, 0, 0, 0, time.UTC), + KeyUsage: x509.KeyUsageDigitalSignature, + } + der, err := x509.CreateCertificate(prng, tmpl, tmpl, &key.PublicKey, key) + if err != nil { + t.Fatalf("deterministic CreateCertificate: %v", err) + } + cert, err := x509.ParseCertificate(der) + if err != nil { + t.Fatalf("ParseCertificate: %v", err) + } + return key, cert +} + +// signE2EIntuneChallenge builds a JWT-shape ES256 challenge using the +// deterministic Connector key. Mirrors +// internal/api/handler/scep_intune_e2e_test.go::signIntuneChallengeES256 +// but lives in the integration_test package (no shared imports across +// internal/ and deploy/test/). +func signE2EIntuneChallenge(t *testing.T, key *ecdsa.PrivateKey, payload map[string]any) string { + t.Helper() + hdr, _ := json.Marshal(map[string]string{"alg": "ES256", "typ": "JWT"}) + pl, _ := json.Marshal(payload) + signingInput := base64.RawURLEncoding.EncodeToString(hdr) + "." + + base64.RawURLEncoding.EncodeToString(pl) + h := sha256.Sum256([]byte(signingInput)) + r, s, err := ecdsa.Sign(rand.Reader, key, h[:]) + if err != nil { + t.Fatalf("ecdsa.Sign: %v", err) + } + rb, sb := r.Bytes(), s.Bytes() + sig := make([]byte, 64) + copy(sig[32-len(rb):], rb) + copy(sig[64-len(sb):], sb) + return signingInput + "." + base64.RawURLEncoding.EncodeToString(sig) +} + +// e2eIntuneClaim returns the v1 challenge payload shape that matches +// a CSR with CN=device-integration-001.example.com (or whatever CN the +// caller passes to buildE2EIntunePKIMessage). +func e2eIntuneClaim(now time.Time, nonce string) map[string]any { + return map[string]any{ + "iss": "intune-connector-integration-fixture", + "sub": "device-guid-integration-001", + "aud": e2eintuneAudience, + "iat": now.Add(-1 * time.Minute).Unix(), + "exp": now.Add(59 * time.Minute).Unix(), + "nonce": nonce, + "device_name": "device-integration-001.example.com", + } +} + +// ============================================================================= +// PKIMessage builder. Mirrors the in-tree handler test's helpers but +// stripped down for the integration test's hermetic needs (single profile, +// AES-256-CBC content encryption, fixture RA cert fetched from /scep/?operation=GetCACert). +// ============================================================================= + +// buildE2EIntunePKIMessage fetches the running container's RA cert via +// GetCACert (which doubles as the cert clients encrypt the CSR's +// content-encryption key to per RFC 8894 §3.2.2), builds an +// EnvelopedData around an AES-256-CBC-encrypted CSR, then wraps the +// EnvelopedData in a SignedData with a transient signerInfo signature. +func buildE2EIntunePKIMessage(t *testing.T, cli *testClient, transactionID, challengePassword, csrCN string) []byte { + t.Helper() + + // Fetch the RA cert from GetCACert. + resp, err := cli.http.Get(serverURL + "/scep/" + e2eintunePathID + "?operation=GetCACert") + if err != nil { + t.Fatalf("GetCACert: %v", err) + } + defer resp.Body.Close() + raCertBytes, err := io.ReadAll(resp.Body) + if err != nil { + t.Fatalf("read GetCACert: %v", err) + } + raCert, err := parseGetCACertForE2EIntune(raCertBytes) + if err != nil { + t.Fatalf("parse RA cert: %v", err) + } + + // Build a transient device key + cert (the CSR's signer + the + // signerInfo's signer; production devices often use one key for + // both). + deviceKey, err := rsa.GenerateKey(rand.Reader, 2048) + if err != nil { + t.Fatalf("device key: %v", err) + } + deviceCert := selfSignedRSACertForE2EIntune(t, deviceKey, "device-transient-integration") + + csrDER := buildE2EIntuneCSR(t, deviceKey, csrCN, challengePassword) + + symKey := bytes.Repeat([]byte{0x42}, 32) // AES-256 + iv := make([]byte, aes.BlockSize) + if _, err := rand.Read(iv); err != nil { + t.Fatalf("rand iv: %v", err) + } + ciphertext := aesCBCEncryptForE2EIntune(t, symKey, iv, csrDER) + + rsaPub, ok := raCert.PublicKey.(*rsa.PublicKey) + if !ok { + t.Fatalf("RA cert public key is %T, want *rsa.PublicKey", raCert.PublicKey) + } + encryptedKey, err := rsa.EncryptPKCS1v15(rand.Reader, rsaPub, symKey) + if err != nil { + t.Fatalf("rsa encrypt symKey: %v", err) + } + + envelopedData := buildEnvelopedDataForE2EIntune(t, raCert, encryptedKey, iv, ciphertext) + signedData := buildSignedDataForE2EIntune(t, deviceKey, deviceCert, transactionID, envelopedData) + return signedData +} + +// postE2EIntuneOp POSTs the PKIMessage to the running certctl container +// and returns the raw response body. Fails the test on non-200 because +// every RFC 8894 PKIOperation MUST return a CertRep PKIMessage even on +// failure — anything other than 200 means the handler choked. +func postE2EIntuneOp(t *testing.T, cli *testClient, pkiMessage []byte) []byte { + t.Helper() + url := serverURL + "/scep/" + e2eintunePathID + "?operation=PKIOperation" + req, err := http.NewRequestWithContext(context.Background(), http.MethodPost, url, bytes.NewReader(pkiMessage)) + if err != nil { + t.Fatalf("new request: %v", err) + } + req.Header.Set("Content-Type", "application/x-pki-message") + resp, err := cli.http.Do(req) + if err != nil { + t.Fatalf("post PKIOperation: %v", err) + } + defer resp.Body.Close() + body, _ := io.ReadAll(resp.Body) + if resp.StatusCode != http.StatusOK { + t.Fatalf("POST PKIOperation: HTTP %d (body=%q) — RFC 8894 §3.3 mandates a CertRep on every PKIOperation including failures", resp.StatusCode, string(body)) + } + return body +} + +// decodeE2EPKIStatus extracts the SCEP pkiStatus auth-attribute from +// a CertRep PKIMessage. Returns the printable-string value ("0" = +// SUCCESS, "2" = FAILURE, "3" = PENDING per RFC 8894 §3.3.2.1). +// +// This is a minimal CMS SignedData walker — we don't pull in the +// internal/pkcs7 package because deploy/test/ is intentionally a +// stand-alone package. The walker hunts for the OID +// 2.16.840.1.113733.1.9.3 (id-attribute-pkiStatus, RFC 8894 §3.3.2.1) +// and returns its first SET-member value as a string. +func decodeE2EPKIStatus(t *testing.T, certRepDER []byte) string { + t.Helper() + // pkiStatus OID is 2.16.840.1.113733.1.9.3 → DER: + // 06 0a 60 86 48 01 86 f8 45 01 09 03 + // Search the certRep DER for this byte pattern; the next 2 bytes + // after the OID land in the auth-attr's SET ("31 ?? ..."), and the + // pkiStatus value is a PrintableString inside. + pkiStatusOID := []byte{0x06, 0x0a, 0x60, 0x86, 0x48, 0x01, 0x86, 0xf8, 0x45, 0x01, 0x09, 0x03} + idx := bytes.Index(certRepDER, pkiStatusOID) + if idx < 0 { + t.Fatalf("decodeE2EPKIStatus: pkiStatus OID not found in CertRep (body len=%d)", len(certRepDER)) + } + // After the OID DER (12 bytes), expect SET (0x31) of length L, + // then PrintableString (0x13) of length M, then the M chars. + cursor := idx + len(pkiStatusOID) + if cursor+4 >= len(certRepDER) { + t.Fatalf("decodeE2EPKIStatus: truncated DER after pkiStatus OID") + } + if certRepDER[cursor] != 0x31 { + t.Fatalf("decodeE2EPKIStatus: expected SET tag 0x31 after OID, got 0x%02x", certRepDER[cursor]) + } + // Skip SET tag + length byte. + cursor += 2 + if certRepDER[cursor] != 0x13 { + t.Fatalf("decodeE2EPKIStatus: expected PrintableString tag 0x13, got 0x%02x", certRepDER[cursor]) + } + strLen := int(certRepDER[cursor+1]) + cursor += 2 + return string(certRepDER[cursor : cursor+strLen]) +} + +// ============================================================================= +// Deterministic PRNG. Replicates the sha256-counter pattern from +// internal/scep/intune/golden_helper_test.go::deterministicReader so +// the integration test can derive the SAME ECDSA key bytes from the +// same seed. No shared imports across the internal/ and deploy/test/ +// boundaries. +// ============================================================================= + +type e2eDeterministicReader struct { + mu sync.Mutex + state []byte + cursor int + buf []byte +} + +func newE2EDeterministicReader(seed []byte) *e2eDeterministicReader { + return &e2eDeterministicReader{state: append([]byte(nil), seed...)} +} + +func (d *e2eDeterministicReader) Read(p []byte) (int, error) { + d.mu.Lock() + defer d.mu.Unlock() + for n := 0; n < len(p); { + if d.cursor >= len(d.buf) { + h := sha256.Sum256(append(d.state, e2eByteCounter(len(p)+n)...)) + d.buf = h[:] + d.cursor = 0 + d.state = d.buf + } + c := copy(p[n:], d.buf[d.cursor:]) + n += c + d.cursor += c + } + return len(p), nil +} + +func e2eByteCounter(i int) []byte { + out := make([]byte, 8) + for k := 0; k < 8; k++ { + out[k] = byte(i >> (8 * k)) + } + return out +} + +// ============================================================================= +// CMS / SCEP byte builders. Stripped-down equivalents of +// internal/pkcs7/{enveloped,signedinfo}.go for the integration test's +// hermetic needs. Distinct names from the in-tree helpers (no import +// crossing internal/ → deploy/test/). +// ============================================================================= + +func parseGetCACertForE2EIntune(body []byte) (*x509.Certificate, error) { + // Try raw DER first. + if cert, err := x509.ParseCertificate(body); err == nil { + return cert, nil + } + // Try PEM fallback. + if block, _ := pem.Decode(body); block != nil && block.Type == "CERTIFICATE" { + return x509.ParseCertificate(block.Bytes) + } + // Try PKCS#7 SignedData certs-only. + type signedData struct { + Version int + DigestAlgorithms asn1.RawValue + ContentInfo asn1.RawValue + Certificates asn1.RawValue `asn1:"optional,implicit,tag:0"` + } + var outer struct { + ContentType asn1.ObjectIdentifier + Content asn1.RawValue `asn1:"explicit,tag:0"` + } + if _, err := asn1.Unmarshal(body, &outer); err == nil { + var sd signedData + if _, err := asn1.Unmarshal(outer.Content.Bytes, &sd); err == nil { + if cert, err := x509.ParseCertificate(sd.Certificates.Bytes); err == nil { + return cert, nil + } + } + } + return nil, fmt.Errorf("could not parse GetCACert response (len=%d)", len(body)) +} + +func selfSignedRSACertForE2EIntune(t *testing.T, key *rsa.PrivateKey, cn string) *x509.Certificate { + t.Helper() + tmpl := &x509.Certificate{ + SerialNumber: big.NewInt(time.Now().UnixNano()), + Subject: pkix.Name{CommonName: cn}, + NotBefore: time.Now().Add(-1 * time.Hour), + NotAfter: time.Now().Add(24 * time.Hour), + } + der, err := x509.CreateCertificate(rand.Reader, tmpl, tmpl, &key.PublicKey, key) + if err != nil { + t.Fatalf("CreateCertificate: %v", err) + } + cert, _ := x509.ParseCertificate(der) + return cert +} + +func buildE2EIntuneCSR(t *testing.T, key *rsa.PrivateKey, cn, challengePassword string) []byte { + t.Helper() + tmpl := &x509.CertificateRequest{ + Subject: pkix.Name{CommonName: cn}, + Attributes: []pkix.AttributeTypeAndValueSET{ + { + Type: asn1.ObjectIdentifier{1, 2, 840, 113549, 1, 9, 7}, + Value: [][]pkix.AttributeTypeAndValue{ + {{Type: asn1.ObjectIdentifier{1, 2, 840, 113549, 1, 9, 7}, Value: challengePassword}}, + }, + }, + }, + } + der, err := x509.CreateCertificateRequest(rand.Reader, tmpl, key) + if err != nil { + t.Fatalf("CreateCertificateRequest: %v", err) + } + return der +} + +func aesCBCEncryptForE2EIntune(t *testing.T, key, iv, plaintext []byte) []byte { + t.Helper() + block, err := aes.NewCipher(key) + if err != nil { + t.Fatalf("aes.NewCipher: %v", err) + } + bs := block.BlockSize() + padLen := bs - len(plaintext)%bs + padded := append([]byte{}, plaintext...) + for i := 0; i < padLen; i++ { + padded = append(padded, byte(padLen)) + } + enc := cipher.NewCBCEncrypter(block, iv) + out := make([]byte, len(padded)) + enc.CryptBlocks(out, padded) + return out +} + +// asn1WrapForE2EIntune wraps body in an ASN.1 TLV with the given tag +// and a definite-length encoding. Mirrors the in-tree +// internal/pkcs7.ASN1Wrap helper but stays inside this package (no +// cross-package import). +func asn1WrapForE2EIntune(tag byte, body []byte) []byte { + var lenBytes []byte + switch { + case len(body) < 128: + lenBytes = []byte{byte(len(body))} + case len(body) < 256: + lenBytes = []byte{0x81, byte(len(body))} + case len(body) < 65536: + lenBytes = []byte{0x82, byte(len(body) >> 8), byte(len(body))} + default: + lenBytes = []byte{0x83, byte(len(body) >> 16), byte(len(body) >> 8), byte(len(body))} + } + out := append([]byte{tag}, lenBytes...) + return append(out, body...) +} + +// OIDs used in the integration-test PKIMessage builders. +var ( + oidRSAEncryptionE2E = asn1.ObjectIdentifier{1, 2, 840, 113549, 1, 1, 1} + oidAES256CBCE2E = asn1.ObjectIdentifier{2, 16, 840, 1, 101, 3, 4, 1, 42} + oidSHA256E2E = asn1.ObjectIdentifier{2, 16, 840, 1, 101, 3, 4, 2, 1} + oidRSAWithSHA256E2E = asn1.ObjectIdentifier{1, 2, 840, 113549, 1, 1, 11} + oidContentTypeE2E = asn1.ObjectIdentifier{1, 2, 840, 113549, 1, 9, 3} + oidMessageDigestE2E = asn1.ObjectIdentifier{1, 2, 840, 113549, 1, 9, 4} + oidSCEPMessageTypeE2E = asn1.ObjectIdentifier{2, 16, 840, 1, 113733, 1, 9, 2} + oidSCEPTransactionE2E = asn1.ObjectIdentifier{2, 16, 840, 1, 113733, 1, 9, 7} + oidSCEPSenderNonceE2E = asn1.ObjectIdentifier{2, 16, 840, 1, 113733, 1, 9, 5} +) + +func buildEnvelopedDataForE2EIntune(t *testing.T, raCert *x509.Certificate, encryptedKey, iv, ciphertext []byte) []byte { + t.Helper() + serialDER, err := asn1.Marshal(raCert.SerialNumber) + if err != nil { + t.Fatalf("marshal serial: %v", err) + } + risBody := append([]byte{}, raCert.RawIssuer...) + risBody = append(risBody, serialDER...) + risBytes := asn1WrapForE2EIntune(0x30, risBody) + + keyEncAlg := pkix.AlgorithmIdentifier{Algorithm: oidRSAEncryptionE2E, Parameters: asn1.NullRawValue} + keyEncAlgBytes, err := asn1.Marshal(keyEncAlg) + if err != nil { + t.Fatalf("marshal keyEncAlg: %v", err) + } + encryptedKeyBytes := asn1WrapForE2EIntune(0x04, encryptedKey) + + ktriBody := append([]byte{}, []byte{0x02, 0x01, 0x00}...) + ktriBody = append(ktriBody, risBytes...) + ktriBody = append(ktriBody, keyEncAlgBytes...) + ktriBody = append(ktriBody, encryptedKeyBytes...) + ktriBytes := asn1WrapForE2EIntune(0x30, ktriBody) + recipientInfosBytes := asn1WrapForE2EIntune(0x31, ktriBytes) + + ivOctet := asn1WrapForE2EIntune(0x04, iv) + contentAlg := pkix.AlgorithmIdentifier{ + Algorithm: oidAES256CBCE2E, + Parameters: asn1.RawValue{FullBytes: ivOctet}, + } + contentAlgBytes, err := asn1.Marshal(contentAlg) + if err != nil { + t.Fatalf("marshal contentAlg: %v", err) + } + + encContentField := asn1WrapForE2EIntune(0x80, ciphertext) + oidDataBytes := []byte{0x06, 0x09, 0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x01, 0x07, 0x01} + eciBody := append([]byte{}, oidDataBytes...) + eciBody = append(eciBody, contentAlgBytes...) + eciBody = append(eciBody, encContentField...) + eciBytes := asn1WrapForE2EIntune(0x30, eciBody) + + envBody := append([]byte{}, []byte{0x02, 0x01, 0x00}...) + envBody = append(envBody, recipientInfosBytes...) + envBody = append(envBody, eciBytes...) + innerEnvBytes := asn1WrapForE2EIntune(0x30, envBody) + + // Wrap in a ContentInfo: SEQ { OID envelopedData, [0] EXPLICIT inner }. + envelopedDataOID := []byte{0x06, 0x09, 0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x01, 0x07, 0x03} + contentInfoBody := append([]byte{}, envelopedDataOID...) + contentInfoBody = append(contentInfoBody, asn1WrapForE2EIntune(0xa0, innerEnvBytes)...) + return asn1WrapForE2EIntune(0x30, contentInfoBody) +} + +func buildSignedDataForE2EIntune(t *testing.T, signerKey *rsa.PrivateKey, signerCert *x509.Certificate, transactionID string, encapContent []byte) []byte { + t.Helper() + contentDigest := sha256.Sum256(encapContent) + + var attrSetBody []byte + attrSetBody = append(attrSetBody, attrSeqHelperE2E(t, oidContentTypeE2E, asn1WrapForE2EIntune(0x06, []byte{0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x01, 0x07, 0x03}))...) // envelopedData + attrSetBody = append(attrSetBody, attrSeqHelperE2E(t, oidMessageDigestE2E, asn1WrapForE2EIntune(0x04, contentDigest[:]))...) + attrSetBody = append(attrSetBody, attrSeqHelperE2E(t, oidSCEPMessageTypeE2E, asn1WrapForE2EIntune(0x13, []byte("19")))...) // PKCSReq=19 + attrSetBody = append(attrSetBody, attrSeqHelperE2E(t, oidSCEPTransactionE2E, asn1WrapForE2EIntune(0x13, []byte(transactionID)))...) + attrSetBody = append(attrSetBody, attrSeqHelperE2E(t, oidSCEPSenderNonceE2E, asn1WrapForE2EIntune(0x04, []byte("0123456789abcdef")))...) + + signedAttrsForSig := asn1WrapForE2EIntune(0x31, attrSetBody) + digest := sha256.Sum256(signedAttrsForSig) + sig, err := rsa.SignPKCS1v15(rand.Reader, signerKey, 5, digest[:]) // 5 = crypto.SHA256 + if err != nil { + t.Fatalf("sign: %v", err) + } + + versionBytes := []byte{0x02, 0x01, 0x01} + serialDER, _ := asn1.Marshal(signerCert.SerialNumber) + sidBody := append([]byte{}, signerCert.RawIssuer...) + sidBody = append(sidBody, serialDER...) + sidBytes := asn1WrapForE2EIntune(0x30, sidBody) + + digestAlg := pkix.AlgorithmIdentifier{Algorithm: oidSHA256E2E, Parameters: asn1.NullRawValue} + digestAlgBytes, _ := asn1.Marshal(digestAlg) + + signedAttrsImplicit := asn1WrapForE2EIntune(0xa0, attrSetBody) + + sigAlg := pkix.AlgorithmIdentifier{Algorithm: oidRSAWithSHA256E2E, Parameters: asn1.NullRawValue} + sigAlgBytes, _ := asn1.Marshal(sigAlg) + sigOctet := asn1WrapForE2EIntune(0x04, sig) + + signerInfoBody := append([]byte{}, versionBytes...) + signerInfoBody = append(signerInfoBody, sidBytes...) + signerInfoBody = append(signerInfoBody, digestAlgBytes...) + signerInfoBody = append(signerInfoBody, signedAttrsImplicit...) + signerInfoBody = append(signerInfoBody, sigAlgBytes...) + signerInfoBody = append(signerInfoBody, sigOctet...) + signerInfoBytes := asn1WrapForE2EIntune(0x30, signerInfoBody) + signerInfosSet := asn1WrapForE2EIntune(0x31, signerInfoBytes) + + digestAlgsSet := asn1WrapForE2EIntune(0x31, digestAlgBytes) + + envelopedDataOID := []byte{0x06, 0x09, 0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x01, 0x07, 0x03} + innerContent := asn1WrapForE2EIntune(0xa0, encapContent) + encapContentInfo := asn1WrapForE2EIntune(0x30, append(envelopedDataOID, innerContent...)) + + signerCertWrapped := asn1WrapForE2EIntune(0xa0, signerCert.Raw) + + sdBody := append([]byte{}, versionBytes...) + sdBody = append(sdBody, digestAlgsSet...) + sdBody = append(sdBody, encapContentInfo...) + sdBody = append(sdBody, signerCertWrapped...) + sdBody = append(sdBody, signerInfosSet...) + innerSDBytes := asn1WrapForE2EIntune(0x30, sdBody) + + signedDataOID := []byte{0x06, 0x09, 0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x01, 0x07, 0x02} + contentInfoBody := append([]byte{}, signedDataOID...) + contentInfoBody = append(contentInfoBody, asn1WrapForE2EIntune(0xa0, innerSDBytes)...) + return asn1WrapForE2EIntune(0x30, contentInfoBody) +} + +func attrSeqHelperE2E(t *testing.T, oid asn1.ObjectIdentifier, value []byte) []byte { + t.Helper() + oidBytes, err := asn1.Marshal(oid) + if err != nil { + t.Fatalf("marshal oid: %v", err) + } + valueSet := asn1WrapForE2EIntune(0x31, value) + body := append(oidBytes, valueSet...) + return asn1WrapForE2EIntune(0x30, body) +} diff --git a/docs/connectors.md b/docs/connectors.md index a02865e..a2287a2 100644 --- a/docs/connectors.md +++ b/docs/connectors.md @@ -331,6 +331,58 @@ Note: EST and SCEP are not connectors — they are protocol handlers (`internal/ **SCEP RA cert + key (post-2026-04-29):** the SCEP server's RFC 8894 path requires an RA cert/key pair (`CERTCTL_SCEP_RA_CERT_PATH` + `CERTCTL_SCEP_RA_KEY_PATH`, mode 0600) — clients encrypt their CSR to the RA cert's public key per RFC 8894 §3.2.2. Multi-profile deployments configure per-profile pairs via `CERTCTL_SCEP_PROFILES=corp,iot` + `CERTCTL_SCEP_PROFILE__RA_*_PATH`. See [`legacy-est-scep.md`](legacy-est-scep.md#scep-rfc-8894-native-implementation-post-2026-04-29) for the openssl recipe + ChromeOS Admin Console pointer + must-staple per-profile policy. +#### Multi-profile SCEP dispatch + +A single certctl deploy can publish multiple SCEP endpoints — one per fleet, one per device class, or one per Connector — by setting `CERTCTL_SCEP_PROFILES=` and a matching set of `CERTCTL_SCEP_PROFILE__*` environment variables. The router publishes `/scep/?operation=...` for every profile whose `` appears in the list (or `/scep` for the legacy single-profile shape when `CERTCTL_SCEP_PROFILES` is unset). Each profile carries its OWN issuer binding, RA cert/key pair, challenge password, must-staple policy, optional mTLS sibling route, and optional Microsoft Intune Connector trust anchor — heterogeneous fleets share one server, distinct credentials. + +| Variable | Required | Default | Description | +|----------|----------|---------|-------------| +| `CERTCTL_SCEP_PROFILES` | No | — | Comma-separated profile names (e.g. `corp,iot`). When unset, the legacy single-profile config (`CERTCTL_SCEP_*` without the `_PROFILE__` infix) is used. | +| `CERTCTL_SCEP_PROFILE__ISSUER_ID` | Yes | — | Issuer connector ID this profile dispatches to (e.g. `iss-local`, `iss-ejbca-corp`). | +| `CERTCTL_SCEP_PROFILE__PROFILE_ID` | No | — | Optional certificate profile ID for fine-grained issuance policy. | +| `CERTCTL_SCEP_PROFILE__CHALLENGE_PASSWORD` | No | — | Static challenge password for the legacy SCEP auth path. Set to "" when only Intune dynamic challenges are expected. | +| `CERTCTL_SCEP_PROFILE__RA_CERT_PATH` | Yes | — | RA cert PEM path (mode 0600 enforced). | +| `CERTCTL_SCEP_PROFILE__RA_KEY_PATH` | Yes | — | RA private key PEM path (mode 0600 enforced). | + +See [`legacy-est-scep.md`](legacy-est-scep.md#scep-rfc-8894-native-implementation-post-2026-04-29) for the full per-profile env-var list and the mTLS / Intune extensions. + +#### SCEP mTLS sibling route (opt-in) + +For deploys that already have a previously-issued certctl client cert and want a stronger renewal binding than the static challenge password, certctl exposes an opt-in mTLS sibling route at `/scep-mtls/`. The TLS handshake is configured with `tls.VerifyClientCertIfGiven` against an operator-supplied trust bundle; presented client certs are validated against the bundle before the SCEP handler runs. The standard `/scep/` route stays open for new-enrollment devices that don't yet have a client cert. + +| Variable | Required | Default | Description | +|----------|----------|---------|-------------| +| `CERTCTL_SCEP_PROFILE__MTLS_ENABLED` | No | `false` | Set `true` to publish `/scep-mtls/` alongside `/scep/`. | +| `CERTCTL_SCEP_PROFILE__MTLS_CLIENT_CA_TRUST_BUNDLE_PATH` | When MTLS enabled | — | PEM bundle of CAs that may sign client certs. Preflight refuses a missing/empty bundle. | + +See [`legacy-est-scep.md`](legacy-est-scep.md#scep-mtls-sibling-route-phase-65) for the operator recipe + threat-model rationale. + +#### Microsoft Intune Certificate Connector dispatcher + +When a profile has `CERTCTL_SCEP_PROFILE__INTUNE_ENABLED=true`, certctl validates the Microsoft Intune Certificate Connector's signed-challenge JWS natively as a drop-in NDES replacement (the Intune Connector documents itself as RFC 8894-compliant and works against any RFC 8894 SCEP server). The dispatcher walks parse → JWS signature verify (RS256 + ES256, alg=none rejected) → version dispatch → time bounds with ±tolerance → audience pin → CSR ↔ claim binding → replay cache → per-device rate limit → optional V3-Pro compliance hook. The trust anchor file is reloaded on `SIGHUP` (operator rotates the on-disk PEM, then `kill -HUP `); a parse failure during reload keeps the OLD pool so a half-rotation doesn't take Intune down. + +| Variable | Required | Default | Description | +|----------|----------|---------|-------------| +| `CERTCTL_SCEP_PROFILE__INTUNE_ENABLED` | No | `false` | Gate the dispatcher. | +| `CERTCTL_SCEP_PROFILE__INTUNE_CONNECTOR_CERT_PATH` | When enabled | — | PEM bundle of the Connector's signing certs. Preflight refuses a missing/expired bundle. | +| `CERTCTL_SCEP_PROFILE__INTUNE_AUDIENCE` | No | — | Expected `aud` claim (typically the public SCEP URL the Connector calls). Empty disables the audience check. | +| `CERTCTL_SCEP_PROFILE__INTUNE_CHALLENGE_VALIDITY` | No | `60m` | Defense-in-depth cap on top of the challenge's own `exp`. | +| `CERTCTL_SCEP_PROFILE__INTUNE_CLOCK_SKEW_TOLERANCE` | No | `60s` | ±tolerance on iat/exp checks. Raise on poorly-NTP-synced fleets, lower to enforce strict time. Refused at boot when ≥ `INTUNE_CHALLENGE_VALIDITY`. | +| `CERTCTL_SCEP_PROFILE__INTUNE_PER_DEVICE_RATE_LIMIT_24H` | No | `3` | Max enrollments per `(claim.Subject, claim.Issuer)` in any rolling 24h window. Zero disables. | + +See [`scep-intune.md`](scep-intune.md) for the full deployment guide — NDES + EJBCA migration playbook, Intune SCEP profile field mapping, trust-anchor extraction recipe, monitoring + Prometheus alert thresholds, and the Microsoft Learn citations operators paste into procurement-team requests. + +#### SCEP probe in network scanner + +The Network Scans GUI surface includes a one-click "Probe SCEP" form that runs a capability + posture check against any reachable SCEP server URL — `GetCACaps` + `GetCACert` (NEVER `PKCSReq`) so the probe is read-only and safe to run against production endpoints. Result fields surface advertised caps (POSTPKIOperation, SHA-256, SHA-512, AES, SCEPStandard, Renewal), CA cert subject + issuer + algorithm + days-to-expiry + chain length, and a probe duration. Results persist to `scep_probe_results` (migration `000021`) and the probe history is paginated under `GET /api/v1/network-scan/scep-probes`. Useful for pre-migration assessment ("what does the existing NDES advertise?") and compliance-posture audits. + +| Endpoint | Auth | Description | +|----------|------|-------------| +| `POST /api/v1/network-scan/scep-probe` | Bearer | Body `{"url":"https://..."}`. Synchronous probe; returns `SCEPProbeResult`. | +| `GET /api/v1/network-scan/scep-probes` | Bearer | Recent probe history, paginated `[1, 200]`. | + +The probe goes through the same dual-layer SSRF defense (`validation.ValidateSafeURL` up-front + `SafeHTTPDialContext` at dial time) as the rest of the network scanner. Standalone CLI binary is explicitly deferred — the in-tree network scanner is the only entrypoint today. + ### Built-in: Vault PKI The Vault PKI connector integrates with HashiCorp Vault's PKI secrets engine using its native `/sign` API with token-based authentication. This is ideal for organizations using Vault as their internal certificate authority — synchronous issuance without the complexity of ACME or challenge solving. diff --git a/docs/scep-intune.md b/docs/scep-intune.md index 7cb3d56..ff124d6 100644 --- a/docs/scep-intune.md +++ b/docs/scep-intune.md @@ -84,9 +84,14 @@ PKIMessage with the documented `pkiStatus`/`failInfo` codes (per RFC payload prelude. v1 (current Connector format, no `version` key) routes to `unmarshalChallengeV1`. Future v2 plugs in a sibling parser without touching the validator. -4. **Time bounds** — `now ≥ iat AND now < exp`. Configurable cap on +4. **Time bounds** — `now+tolerance ≥ iat AND now-tolerance < exp`. + The `±tolerance` window is configurable per profile via + `INTUNE_CLOCK_SKEW_TOLERANCE` (default 60s, covers modest clock + drift between the Connector host and certctl). Configurable cap on top via `INTUNE_CHALLENGE_VALIDITY` (defense-in-depth against a - Connector that mints long-validity challenges). + Connector that mints long-validity challenges). The validator + refuses `tolerance ≥ ChallengeValidity` at startup-validation time + to keep the cap meaningful. 5. **Audience pin** — `claim.aud == INTUNE_AUDIENCE` (skipped when `INTUNE_AUDIENCE` is empty for proxy/load-balancer scenarios). 6. **CSR binding** — `claim.DeviceMatchesCSR(csr)` checks @@ -156,6 +161,9 @@ so the production validation step is a manual run-book item. CERTCTL_SCEP_PROFILE__INTUNE_ENABLED=true CERTCTL_SCEP_PROFILE__INTUNE_CONNECTOR_CERT_PATH=/etc/certctl/intune-corp.pem CERTCTL_SCEP_PROFILE__INTUNE_AUDIENCE=https://certctl.example.com/scep/corp + CERTCTL_SCEP_PROFILE__INTUNE_CHALLENGE_VALIDITY=60m + CERTCTL_SCEP_PROFILE__INTUNE_CLOCK_SKEW_TOLERANCE=60s # ±tolerance on iat/exp; raise on poorly-NTP-synced fleets, lower to enforce strict time + CERTCTL_SCEP_PROFILE__INTUNE_PER_DEVICE_RATE_LIMIT_24H=3 ``` Restart certctl. The startup preflight refuses to boot if the diff --git a/internal/api/handler/scep_chromeos_test.go b/internal/api/handler/scep_chromeos_test.go index a621bfe..05a4572 100644 --- a/internal/api/handler/scep_chromeos_test.go +++ b/internal/api/handler/scep_chromeos_test.go @@ -285,6 +285,183 @@ func TestSCEPHandler_ChromeOSPKIMessage_AESVariants(t *testing.T) { } } +// TestSCEPHandler_ChromeOSPKIMessage_RAKeyMismatch — closure-bundle +// gap M-1 / acceptance D.1 (cowork/scep-bundle-gap-closure-prompt.md). +// Build a PKIMessage encrypted to a freshly-generated RA cert whose +// matching private key the server does NOT have. The handler MUST +// reject (RFC 8894 path can't decrypt → falls through; MVP path can't +// either because the EnvelopedData isn't a raw CSR). Assert no +// PKCSReqWithEnvelope was reached. Closes the documented threat that +// an attacker who swaps the RA cert in transit gets a polite error +// rather than information leak about the underlying issuer. +func TestSCEPHandler_ChromeOSPKIMessage_RAKeyMismatch(t *testing.T) { + fix := newChromeOSStackFixture(t) + + // Build a PKIMessage targeting an UNRELATED RA cert (different key). + // The server's handler still has fix.raKey, so decryption MUST fail. + bogusRAKey, err := rsa.GenerateKey(rand.Reader, 2048) + if err != nil { + t.Fatalf("rsa.GenerateKey bogus RA: %v", err) + } + bogusRACert := selfSignedRSACert(t, bogusRAKey, "ra-bogus-not-on-server") + bogusFix := &chromeOSStackFixture{ + raKey: bogusRAKey, + raCert: bogusRACert, + deviceKey: fix.deviceKey, + deviceCert: fix.deviceCert, + } + pkiMessage := buildChromeOSStylePKIMessage(t, bogusFix, domain.SCEPMessageTypePKCSReq, "txn-ra-mismatch", "shared-secret-123", "ra-mismatch.example.com", aesKeyForOID(pkcs7.OIDAES256CBC)) + + w, _ := postPKIOperation(t, fix.handler, pkiMessage) + // RFC 8894 path returns FAILURE+badMessageCheck CertRep (200), MVP + // fall-through returns 400. Either is acceptable — what we MUST + // see is "the issuer never received the CSR." + if w.Code != http.StatusBadRequest && w.Code != http.StatusOK { + t.Errorf("POST PKIOperation (RA-key mismatch): got %d, want 400 (MVP fall-through) or 200 (CertRep+failInfo)", w.Code) + } + if fix.svc.pkcsReqEnvelope != nil { + t.Error("PKCSReqWithEnvelope was reached despite the RA-cert/key mismatch — decrypt-failure leaked through to the service") + } +} + +// TestSCEPHandler_ChromeOSPKIMessage_3DESBackwardCompat — closure-bundle +// gap M-1 / acceptance D.2. RFC 8894 §3.5.2 names DES-EDE3-CBC +// (1.2.840.113549.3.7) as a "supported but discouraged" content-encryption +// algorithm for backward compat with older Cisco IOS / Apple legacy +// clients. Verify the parser accepts this OID + the handler reaches +// the service with a decoded CSR. +func TestSCEPHandler_ChromeOSPKIMessage_3DESBackwardCompat(t *testing.T) { + fix := newChromeOSStackFixture(t) + tdesKey := aesKeyForOID(pkcs7.OIDDESEDE3CBC) // 24 bytes (3DES K1||K2||K3) + + csrDER := buildTestCSR(t, fix.deviceKey, "tdes.example.com", "shared-secret-123") + + iv := make([]byte, des.BlockSize) // 8 bytes for 3DES + if _, err := rand.Read(iv); err != nil { + t.Fatalf("rand iv: %v", err) + } + ciphertext := tripleDESCBCEncrypt(t, tdesKey, iv, csrDER) + encryptedKey, err := rsa.EncryptPKCS1v15(rand.Reader, fix.raCert.PublicKey.(*rsa.PublicKey), tdesKey) + if err != nil { + t.Fatalf("rsa encrypt 3des key: %v", err) + } + envelopedData := buildEnvelopedDataForTest(t, fix.raCert, encryptedKey, iv, ciphertext, pkcs7.OIDDESEDE3CBC) + pkiMessage := buildSignedDataForTest(t, fix.deviceKey, fix.deviceCert, domain.SCEPMessageTypePKCSReq, "txn-3des", []byte("0123456789abcdef"), envelopedData) + + w, body := postPKIOperation(t, fix.handler, pkiMessage) + if w.Code != http.StatusOK { + t.Fatalf("POST PKIOperation (3DES legacy): got %d, want 200 (RFC 8894 §3.5.2 backward-compat) — body=%q", w.Code, body) + } + if fix.svc.pkcsReqEnvelope == nil { + t.Fatal("PKCSReqWithEnvelope was NOT reached — 3DES decrypt path didn't make it to the service") + } +} + +// TestSCEPHandler_ChromeOSPKIMessage_RSACSR — closure-bundle gap M-1 / +// acceptance D.4. Pins the "RSA CSR" matrix corner explicitly so a +// future helper refactor that quietly drops the RSA path doesn't +// disappear from the test count without a counter dropping. The +// shared positive-flow assertions live in +// assertChromeOSPositiveCertRep so the matrix-pair {RSA, ECDSA} stays +// readable. +func TestSCEPHandler_ChromeOSPKIMessage_RSACSR(t *testing.T) { + fix := newChromeOSStackFixture(t) + pkiMessage := buildChromeOSStylePKIMessage(t, fix, domain.SCEPMessageTypePKCSReq, "txn-rsa-csr", "shared-secret-123", "rsa-csr.example.com", aesKeyForOID(pkcs7.OIDAES256CBC)) + assertChromeOSPositiveCertRep(t, fix, pkiMessage) +} + +// TestSCEPHandler_ChromeOSPKIMessage_ECDSACSR — closure-bundle gap M-1 +// / acceptance D.3. The CSR's keypair is ECDSA P-256; the device's +// transient signerInfo identity stays RSA (matches what real ChromeOS +// + Intune-managed devices commonly emit — device identity is a +// long-lived RSA key, the new cert can be ECDSA). Verifies the +// handler doesn't choke on the inner CSR's algorithm even when the +// outer SignerInfo is RSA-SHA256. +func TestSCEPHandler_ChromeOSPKIMessage_ECDSACSR(t *testing.T) { + fix := newChromeOSStackFixture(t) + + csrKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + if err != nil { + t.Fatalf("ecdsa.GenerateKey: %v", err) + } + csrDER := buildTestECDSACSR(t, csrKey, "ecdsa-csr.example.com", "shared-secret-123") + + symKey := aesKeyForOID(pkcs7.OIDAES256CBC) + iv := make([]byte, aes.BlockSize) + if _, err := rand.Read(iv); err != nil { + t.Fatalf("rand iv: %v", err) + } + ciphertext := aesCBCEncrypt(t, symKey, iv, csrDER) + encryptedKey, err := rsa.EncryptPKCS1v15(rand.Reader, fix.raCert.PublicKey.(*rsa.PublicKey), symKey) + if err != nil { + t.Fatalf("rsa encrypt symKey: %v", err) + } + envelopedData := buildEnvelopedDataForTest(t, fix.raCert, encryptedKey, iv, ciphertext, pkcs7.OIDAES256CBC) + pkiMessage := buildSignedDataForTest(t, fix.deviceKey, fix.deviceCert, domain.SCEPMessageTypePKCSReq, "txn-ecdsa-csr", []byte("0123456789abcdef"), envelopedData) + assertChromeOSPositiveCertRep(t, fix, pkiMessage) +} + +// assertChromeOSPositiveCertRep is the shared positive-flow assertion +// helper for the {RSA, ECDSA} CSR matrix tests. Asserts HTTP 200 + +// content-type + the service-level mock saw the envelope. +func assertChromeOSPositiveCertRep(t *testing.T, fix *chromeOSStackFixture, pkiMessage []byte) { + t.Helper() + w, body := postPKIOperation(t, fix.handler, pkiMessage) + if w.Code != http.StatusOK { + t.Fatalf("POST PKIOperation: got %d, want 200 (body=%q)", w.Code, body) + } + if got := w.Header().Get("Content-Type"); got != "application/x-pki-message" { + t.Errorf("Content-Type = %q, want application/x-pki-message", got) + } + if fix.svc.pkcsReqEnvelope == nil { + t.Fatal("PKCSReqWithEnvelope was NOT reached — handler dispatched to MVP path or rejected the message") + } +} + +// buildTestECDSACSR mirrors buildTestCSR but for an ECDSA P-256 +// signing key. Closure-bundle Phase D helper. The CSR carries the +// challengePassword attribute the same way the RSA helper does. +func buildTestECDSACSR(t *testing.T, key *ecdsa.PrivateKey, commonName, challengePassword string) []byte { + t.Helper() + tmpl := &x509.CertificateRequest{ + Subject: pkix.Name{CommonName: commonName}, + ExtraExtensions: []pkix.Extension{}, + Attributes: []pkix.AttributeTypeAndValueSET{ + { + Type: asn1.ObjectIdentifier{1, 2, 840, 113549, 1, 9, 7}, + Value: [][]pkix.AttributeTypeAndValue{ + {{Type: asn1.ObjectIdentifier{1, 2, 840, 113549, 1, 9, 7}, Value: challengePassword}}, + }, + }, + }, + } + der, err := x509.CreateCertificateRequest(rand.Reader, tmpl, key) + if err != nil { + t.Fatalf("CreateCertificateRequest (ECDSA): %v", err) + } + return der +} + +// tripleDESCBCEncrypt mirrors aesCBCEncrypt for 3DES — used by the +// 3DES backward-compat test. PKCS#7 padding to 8-byte blocks. +func tripleDESCBCEncrypt(t *testing.T, key, iv, plaintext []byte) []byte { + t.Helper() + block, err := des.NewTripleDESCipher(key) //nolint:gosec // RFC 8894 §3.5.2 legacy backward-compat test fixture + if err != nil { + t.Fatalf("des.NewTripleDESCipher: %v", err) + } + bs := block.BlockSize() + padLen := bs - len(plaintext)%bs + padded := append([]byte{}, plaintext...) + for i := 0; i < padLen; i++ { + padded = append(padded, byte(padLen)) + } + enc := cipher.NewCBCEncrypter(block, iv) + out := make([]byte, len(padded)) + enc.CryptBlocks(out, padded) + return out +} + // TestSCEPHandler_MVPCompat_StillWorks asserts the existing MVP path (raw // CSR inside a stripped SignedData, no EnvelopedData) STILL works for // backward compat with lightweight clients. diff --git a/internal/api/handler/scep_intune_e2e_test.go b/internal/api/handler/scep_intune_e2e_test.go index b1b2673..1668e47 100644 --- a/internal/api/handler/scep_intune_e2e_test.go +++ b/internal/api/handler/scep_intune_e2e_test.go @@ -66,6 +66,9 @@ import ( // keypair the test uses to mint valid challenges. type intuneE2EFixture struct { connectorKey *ecdsa.PrivateKey + connectorDir string // dir holding the trust-anchor PEM (for SIGHUP-reload tests) + trustPath string // PEM file the holder watches; rewriting + Reload simulates SIGHUP + trustHolder *intune.TrustAnchorHolder raKey *rsa.PrivateKey raCert *x509.Certificate deviceKey *rsa.PrivateKey @@ -232,6 +235,7 @@ func newIntuneE2EFixture(t *testing.T) *intuneE2EFixture { trustHolder, "https://certctl.example.com/scep/test", 60*time.Minute, + 0, // ClockSkewTolerance — strict (the e2e fixture uses time.Now() consistently so no drift to absorb) replayCache, rateLimiter, ) @@ -251,6 +255,9 @@ func newIntuneE2EFixture(t *testing.T) *intuneE2EFixture { return &intuneE2EFixture{ connectorKey: connectorKey, + connectorDir: dir, + trustPath: trustPath, + trustHolder: trustHolder, raKey: raKey, raCert: raCert, deviceKey: deviceKey, @@ -492,3 +499,178 @@ func buildIntuneE2EPKIMessage(t *testing.T, fix *intuneE2EFixture, transactionID signedData := buildSignedDataForTest(t, fix.deviceKey, fix.deviceCert, domain.SCEPMessageTypePKCSReq, transactionID, []byte("0123456789abcdef"), envelopedData) return signedData } + +// ============================================================================= +// SCEP RFC 8894 + Intune master-prompt §13 line 1849 acceptance — the two +// remaining e2e named tests: _RateLimited_E2E + _TrustAnchorSIGHUPReload_E2E. +// Closed in the 2026-04-29 audit-closure bundle. +// ============================================================================= + +// TestSCEPIntuneEnrollment_RateLimited_E2E exercises the full +// handler→service→dispatcher chain past the per-device rate-limit cap. +// The fixture's default cap (3) is too high for a quick test; we +// re-inject a fresh limiter with cap=2 so the 3rd attempt for the same +// (Subject, Issuer) returns FAILURE+BadRequest with rate_limited +// counter ticked. Each PKIMessage carries a distinct nonce (replay +// cache otherwise rejects on duplicate-nonce well before the limiter +// fires), and a distinct transactionID so the audit-log shape is +// inspectable per attempt. +func TestSCEPIntuneEnrollment_RateLimited_E2E(t *testing.T) { + fix := newIntuneE2EFixture(t) + + // Re-wire SetIntuneIntegration with a stricter cap so the test + // stays fast. Also a fresh replay cache so a previous attempt's + // state doesn't leak into this test if Go ever reorders test + // execution within the package. + tightLimiter := intune.NewPerDeviceRateLimiter(2, 24*time.Hour, 100) + freshReplay := intune.NewReplayCache(60*time.Minute, 100) + fix.scepService.SetIntuneIntegration( + fix.trustHolder, + "https://certctl.example.com/scep/test", + 60*time.Minute, + 0, // ClockSkewTolerance — strict (we mint claims at time.Now()) + freshReplay, + tightLimiter, + ) + + now := time.Now() + + // First two attempts succeed (cap=2 means ≤2 issuances per 24h). + for i := 0; i < 2; i++ { + nonce := "e2e-rate-allow-" + string(rune('a'+i)) + ch := signIntuneChallengeES256(t, fix.connectorKey, validIntuneE2EClaim(now, nonce)) + txn := "txn-rate-allow-" + string(rune('a'+i)) + pkiMessage := buildIntuneE2EPKIMessage(t, fix, txn, ch, "device-corp-001.example.com") + w, body := postPKIOperation(t, fix.handler, pkiMessage) + if w.Code != http.StatusOK { + t.Fatalf("attempt %d: HTTP %d (body=%q)", i+1, w.Code, body) + } + certRep, err := pkcs7.ParseSignedData(body) + if err != nil { + t.Fatalf("attempt %d: ParseSignedData: %v", i+1, err) + } + statusStr := decodeFirstSetMember(t, certRep.SignerInfos[0].AuthAttributes[pkcs7.OIDSCEPPKIStatus.String()]) + if statusStr != string(domain.SCEPStatusSuccess) { + t.Fatalf("attempt %d: pkiStatus = %q, want SUCCESS (the allowed first %d/%d)", i+1, statusStr, i+1, 2) + } + } + + // 3rd attempt for the SAME (Subject, Issuer) MUST be rate-limited. + tripCh := signIntuneChallengeES256(t, fix.connectorKey, validIntuneE2EClaim(now, "e2e-rate-deny-c")) + tripMsg := buildIntuneE2EPKIMessage(t, fix, "txn-rate-deny-c", tripCh, "device-corp-001.example.com") + w, body := postPKIOperation(t, fix.handler, tripMsg) + if w.Code != http.StatusOK { + t.Fatalf("rate-limited attempt: HTTP %d (body=%q) — RFC 8894 §3.3 mandates a CertRep on every PKIOperation, including failures", w.Code, body) + } + certRep, err := pkcs7.ParseSignedData(body) + if err != nil { + t.Fatalf("rate-limited attempt: ParseSignedData: %v", err) + } + statusStr := decodeFirstSetMember(t, certRep.SignerInfos[0].AuthAttributes[pkcs7.OIDSCEPPKIStatus.String()]) + if statusStr != string(domain.SCEPStatusFailure) { + t.Fatalf("rate-limited pkiStatus = %q, want FAILURE", statusStr) + } + failRV, ok := certRep.SignerInfos[0].AuthAttributes[pkcs7.OIDSCEPFailInfo.String()] + if !ok { + t.Fatal("rate-limited CertRep missing failInfo auth-attr") + } + failStr := decodeFirstSetMember(t, failRV) + if failStr != string(domain.SCEPFailBadRequest) { + t.Errorf("rate-limited failInfo = %q, want BadRequest (mapIntuneErrorToFailInfo: rate_limit → BadRequest)", failStr) + } + + // The fixture's issuer should have seen exactly 2 issuances (the + // allowed pair) — the 3rd was blocked at the dispatcher gate. + if got, want := len(fix.issuer.issued), 2; got != want { + t.Errorf("issuer issuances = %d, want %d (rate-limited 3rd should not reach the issuer)", got, want) + } + + // Audit log — at least one rate-limited entry. The dispatcher's + // audit action is "scep_pkcsreq_intune" for both successes and + // failures; we inspect the counter table for the rate_limited tick. + stats := fix.scepService.IntuneStats(time.Now()) + if got := stats.Counters["rate_limited"]; got != 1 { + t.Errorf("IntuneStats.counters[rate_limited] = %d, want 1", got) + } + if got := stats.Counters["success"]; got != 2 { + t.Errorf("IntuneStats.counters[success] = %d, want 2 (cap=2 allowed pair)", got) + } +} + +// TestSCEPIntuneEnrollment_TrustAnchorSIGHUPReload_E2E proves the full +// SIGHUP-reload contract end-to-end: an enrollment that succeeds against +// the original trust anchor MUST fail after the operator rotates the +// on-disk file + reloads, when the device tries to enroll with the OLD +// connector key. +// +// Why we call holder.Reload() directly instead of os.Process.Signal(SIGHUP): +// signal delivery in tests is flaky (signals to the test process can +// race with t.Parallel(), and signal.Notify is global). The SIGHUP +// goroutine's only job is to call Reload, so calling Reload directly is +// the equivalent contract — and stable in tests. Phase B frozen +// decision #3 in cowork/scep-bundle-gap-closure-prompt.md. +func TestSCEPIntuneEnrollment_TrustAnchorSIGHUPReload_E2E(t *testing.T) { + fix := newIntuneE2EFixture(t) + now := time.Now() + + // Step 1: a valid enrollment against the original trust anchor. + originalCh := signIntuneChallengeES256(t, fix.connectorKey, validIntuneE2EClaim(now, "e2e-sighup-pre")) + originalMsg := buildIntuneE2EPKIMessage(t, fix, "txn-sighup-pre", originalCh, "device-corp-001.example.com") + w, body := postPKIOperation(t, fix.handler, originalMsg) + if w.Code != http.StatusOK { + t.Fatalf("pre-rotation enrollment: HTTP %d (body=%q)", w.Code, body) + } + certRep, err := pkcs7.ParseSignedData(body) + if err != nil { + t.Fatalf("pre-rotation ParseSignedData: %v", err) + } + statusStr := decodeFirstSetMember(t, certRep.SignerInfos[0].AuthAttributes[pkcs7.OIDSCEPPKIStatus.String()]) + if statusStr != string(domain.SCEPStatusSuccess) { + t.Fatalf("pre-rotation pkiStatus = %q, want SUCCESS", statusStr) + } + + // Step 2: operator rotates the trust anchor — write a fresh signing + // cert from a NEW key into the same path. Holder.Reload() then + // swaps the in-memory pool to the new bundle. The OLD key + // (fix.connectorKey) is now disowned. + rotatedKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + if err != nil { + t.Fatalf("rotated key: %v", err) + } + rotatedCert := selfSignedECCertForIntuneE2E(t, rotatedKey, "intune-connector-rotated") + if err := os.WriteFile(fix.trustPath, pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: rotatedCert.Raw}), 0o600); err != nil { + t.Fatalf("rewrite trust anchor file: %v", err) + } + if err := fix.trustHolder.Reload(); err != nil { + t.Fatalf("trustHolder.Reload (post-rotation): %v", err) + } + + // Step 3: a device that signs with the OLD connector key MUST be + // rejected — the holder no longer recognizes the signature. + staleCh := signIntuneChallengeES256(t, fix.connectorKey, validIntuneE2EClaim(now, "e2e-sighup-stale")) + staleMsg := buildIntuneE2EPKIMessage(t, fix, "txn-sighup-stale", staleCh, "device-corp-001.example.com") + w, body = postPKIOperation(t, fix.handler, staleMsg) + if w.Code != http.StatusOK { + t.Fatalf("stale-key enrollment: HTTP %d (body=%q) — RFC 8894 §3.3 mandates a CertRep+failInfo wire shape", w.Code, body) + } + certRep, err = pkcs7.ParseSignedData(body) + if err != nil { + t.Fatalf("stale-key ParseSignedData: %v", err) + } + statusStr = decodeFirstSetMember(t, certRep.SignerInfos[0].AuthAttributes[pkcs7.OIDSCEPPKIStatus.String()]) + if statusStr != string(domain.SCEPStatusFailure) { + t.Fatalf("stale-key pkiStatus = %q, want FAILURE after trust-anchor rotation", statusStr) + } + failStr := decodeFirstSetMember(t, certRep.SignerInfos[0].AuthAttributes[pkcs7.OIDSCEPFailInfo.String()]) + if failStr != string(domain.SCEPFailBadMessageCheck) { + t.Errorf("stale-key failInfo = %q, want BadMessageCheck (mapIntuneErrorToFailInfo: sig errors → BadMessageCheck)", failStr) + } + + stats := fix.scepService.IntuneStats(time.Now()) + if got := stats.Counters["signature_invalid"]; got != 1 { + t.Errorf("IntuneStats.counters[signature_invalid] = %d, want 1 (post-rotation stale-key attempt)", got) + } + if got := stats.Counters["success"]; got != 1 { + t.Errorf("IntuneStats.counters[success] = %d, want 1 (only the pre-rotation attempt)", got) + } +} diff --git a/internal/api/handler/scep_profile_counter_isolation_test.go b/internal/api/handler/scep_profile_counter_isolation_test.go new file mode 100644 index 0000000..13ef5ff --- /dev/null +++ b/internal/api/handler/scep_profile_counter_isolation_test.go @@ -0,0 +1,212 @@ +package handler + +import ( + "crypto/ecdsa" + "crypto/elliptic" + "crypto/rand" + "crypto/rsa" + "encoding/pem" + "io" + "log/slog" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "testing" + "time" + + "github.com/shankar0123/certctl/internal/domain" + "github.com/shankar0123/certctl/internal/pkcs7" + "github.com/shankar0123/certctl/internal/scep/intune" + "github.com/shankar0123/certctl/internal/service" +) + +// SCEP RFC 8894 + Intune master prompt §13 line 1851 acceptance — +// "Per-profile dispatch test must prove per-profile counters in +// metrics." Closed in the 2026-04-29 audit-closure bundle (Phase E). +// +// Why this test exists separately from the existing router-level +// /scep/ dispatch test (TestRouter_RegisterSCEPHandlers_ +// MultipleProfilesNoCrossBleed): that test proves the route table +// doesn't bleed; this one proves the in-memory observability state +// (intuneCounterTab) is per-SCEPService, not shared. The bug class +// it guards against is a future cmd/server/main.go refactor that +// constructs a single shared *intuneCounterTab and injects it into +// every per-profile service — that would compile cleanly, pass the +// existing route-table test, and silently inflate one profile's +// counters with another's traffic. + +// TestSCEPHandler_PerProfileIntuneCountersIsolated wires two real +// SCEPService instances, each with its OWN trust anchor + audience. +// A success on profile "corp" MUST NOT tick "iot"'s success counter, +// and vice versa for the failure path. The test constructs the +// fixtures hermetically (no shared state between the two profiles +// except the test's t.TempDir + selfSignedRSACert helpers). +func TestSCEPHandler_PerProfileIntuneCountersIsolated(t *testing.T) { + corpFix := buildPerProfileIntuneFixture(t, "corp", "https://certctl.example.com/scep/corp") + iotFix := buildPerProfileIntuneFixture(t, "iot", "https://certctl.example.com/scep/iot") + now := time.Now() + + // --- Drive a SUCCESS through CORP --- + corpChallenge := signIntuneChallengeES256(t, corpFix.connectorKey, map[string]any{ + "iss": "intune-connector-corp-fixture", + "sub": "device-guid-corp-001", + "aud": "https://certctl.example.com/scep/corp", + "iat": now.Add(-1 * time.Minute).Unix(), + "exp": now.Add(59 * time.Minute).Unix(), + "nonce": "iso-corp-nonce-001", + "device_name": "device-corp-001.example.com", + }) + corpMsg := buildIntuneE2EPKIMessage(t, corpFix, "txn-iso-corp", corpChallenge, "device-corp-001.example.com") + w, body := postPKIOperation(t, corpFix.handler, corpMsg) + if w.Code != http.StatusOK { + t.Fatalf("corp success: HTTP %d (body=%q)", w.Code, body) + } + + // --- Drive an EXPIRED challenge through IOT --- + iotChallenge := signIntuneChallengeES256(t, iotFix.connectorKey, map[string]any{ + "iss": "intune-connector-iot-fixture", + "sub": "device-guid-iot-001", + "aud": "https://certctl.example.com/scep/iot", + "iat": now.Add(-2 * time.Hour).Unix(), + "exp": now.Add(-1 * time.Hour).Unix(), // expired + "nonce": "iso-iot-nonce-001", + "device_name": "device-iot-001.example.com", + }) + iotMsg := buildIntuneE2EPKIMessage(t, iotFix, "txn-iso-iot", iotChallenge, "device-iot-001.example.com") + w, body = postPKIOperation(t, iotFix.handler, iotMsg) + if w.Code != http.StatusOK { + t.Fatalf("iot expired: HTTP %d — RFC 8894 §3.3 mandates a CertRep on every PKIOperation including failures; body=%q", w.Code, body) + } + certRep, err := pkcs7.ParseSignedData(body) + if err != nil { + t.Fatalf("iot expired: ParseSignedData: %v", err) + } + statusStr := decodeFirstSetMember(t, certRep.SignerInfos[0].AuthAttributes[pkcs7.OIDSCEPPKIStatus.String()]) + if statusStr != string(domain.SCEPStatusFailure) { + t.Errorf("iot expired pkiStatus = %q, want FAILURE", statusStr) + } + + // --- Assert per-service counter isolation --- + corpStats := corpFix.scepService.IntuneStats(time.Now()) + iotStats := iotFix.scepService.IntuneStats(time.Now()) + + if got, want := corpStats.PathID, "corp"; got != want { + t.Errorf("corp PathID = %q, want %q", got, want) + } + if got, want := iotStats.PathID, "iot"; got != want { + t.Errorf("iot PathID = %q, want %q", got, want) + } + + // CORP should have exactly one success and zero of every other label. + if got := corpStats.Counters["success"]; got != 1 { + t.Errorf("corp.Counters[success] = %d, want 1", got) + } + if got := corpStats.Counters["expired"]; got != 0 { + t.Errorf("corp.Counters[expired] = %d, want 0 (iot's expired traffic must NOT bleed into corp)", got) + } + // IOT should have exactly one expired and zero successes. + if got := iotStats.Counters["expired"]; got != 1 { + t.Errorf("iot.Counters[expired] = %d, want 1", got) + } + if got := iotStats.Counters["success"]; got != 0 { + t.Errorf("iot.Counters[success] = %d, want 0 (corp's success traffic must NOT bleed into iot)", got) + } + + // And the issuer-side state — corp's mock issuer saw the issuance, + // iot's did not. This pins that the per-profile dispatch reaches + // the per-profile issuer connector too (not just the counter tab). + if got, want := len(corpFix.issuer.issued), 1; got != want { + t.Errorf("corp issuances = %d, want %d", got, want) + } + if got, want := len(iotFix.issuer.issued), 0; got != want { + t.Errorf("iot issuances = %d, want %d (iot's expired challenge must NOT have produced issuance)", got, want) + } +} + +// buildPerProfileIntuneFixture builds an Intune-enabled SCEPService for +// the given pathID + audience, with its own freshly-generated trust +// anchor + RA pair + issuer mock. Mirrors newIntuneE2EFixture but +// parameterized so the per-profile-isolation test can stand up two +// independent stacks side-by-side. +func buildPerProfileIntuneFixture(t *testing.T, pathID, audience string) *intuneE2EFixture { + t.Helper() + + connectorKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + if err != nil { + t.Fatalf("connector key (%s): %v", pathID, err) + } + connectorCert := selfSignedECCertForIntuneE2E(t, connectorKey, "intune-connector-"+pathID) + + dir := t.TempDir() + trustPath := filepath.Join(dir, "intune-trust-"+pathID+".pem") + if err := os.WriteFile(trustPath, pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: connectorCert.Raw}), 0o600); err != nil { + t.Fatalf("write trust anchor (%s): %v", pathID, err) + } + trustHolder, err := intune.NewTrustAnchorHolder(trustPath, slog.New(slog.NewTextHandler(io.Discard, &slog.HandlerOptions{Level: slog.LevelError + 10}))) + if err != nil { + t.Fatalf("NewTrustAnchorHolder (%s): %v", pathID, err) + } + + raKey, err := rsa.GenerateKey(rand.Reader, 2048) + if err != nil { + t.Fatalf("ra key (%s): %v", pathID, err) + } + raCert := selfSignedRSACert(t, raKey, "ra-iso-"+pathID) + + caKey, err := rsa.GenerateKey(rand.Reader, 2048) + if err != nil { + t.Fatalf("ca key (%s): %v", pathID, err) + } + caCert := selfSignedRSACert(t, caKey, "test-fixture-ca-"+pathID) + caPEM := pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: caCert.Raw}) + + issuer := &intuneE2EIssuerConnector{ + caPEM: string(caPEM), + signKey: caKey, + caCert: caCert, + } + + auditRepo := &intuneE2EAuditRepo{} + auditSvc := service.NewAuditService(auditRepo) + logger := slog.New(slog.NewTextHandler(io.Discard, &slog.HandlerOptions{Level: slog.LevelError + 10})) + scepSvc := service.NewSCEPService("iss-"+pathID, issuer, auditSvc, logger, "static-fallback-"+pathID) + scepSvc.SetPathID(pathID) + scepSvc.SetIntuneIntegration( + trustHolder, + audience, + 60*time.Minute, + 0, // ClockSkewTolerance — strict + intune.NewReplayCache(60*time.Minute, 100), + intune.NewPerDeviceRateLimiter(3, 24*time.Hour, 100), + ) + + deviceKey, err := rsa.GenerateKey(rand.Reader, 2048) + if err != nil { + t.Fatalf("device key (%s): %v", pathID, err) + } + deviceCert := selfSignedRSACert(t, deviceKey, "device-iso-"+pathID) + + handler := NewSCEPHandler(scepSvc) + handler.SetRAPair(raCert, raKey) + + return &intuneE2EFixture{ + connectorKey: connectorKey, + connectorDir: dir, + trustPath: trustPath, + trustHolder: trustHolder, + raKey: raKey, + raCert: raCert, + deviceKey: deviceKey, + deviceCert: deviceCert, + issuer: issuer, + auditRepo: auditRepo, + scepService: scepSvc, + handler: handler, + } +} + +// silence unused-import for httptest (only needed if a future test in +// this file constructs requests directly — kept here to avoid a +// goimports-driven churn the next time the file gains a test). +var _ = httptest.NewRecorder diff --git a/internal/config/config.go b/internal/config/config.go index 4b109f6..7288584 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -879,6 +879,18 @@ type SCEPIntuneProfileConfig struct { // signing key). Zero means "unlimited" (defense-in-depth disabled; // not recommended for production). PerDeviceRateLimit24h int + + // ClockSkewTolerance widens the iat/exp validation window by + // ±|tolerance| to absorb modest clock drift between the Microsoft + // Intune Certificate Connector and the certctl host. Default 60s + // per master prompt §15 ("known hazards"). Operators on tightly + // time-synced fleets can set this to zero to enforce strict + // iat/exp checks; operators on loosely synced fleets (e.g. field + // devices with no NTP) may raise to 5m. Validate() refuses any + // tolerance ≥ ChallengeValidity (which would make the per-profile + // validity cap meaningless). Source env var: + // CERTCTL_SCEP_PROFILE__INTUNE_CLOCK_SKEW_TOLERANCE. + ClockSkewTolerance time.Duration } // NetworkScanConfig controls the server-side active TLS scanner. @@ -1514,6 +1526,7 @@ func loadSCEPProfilesFromEnv() []SCEPProfileConfig { Audience: getEnv("CERTCTL_SCEP_PROFILE_"+envName+"_INTUNE_AUDIENCE", ""), ChallengeValidity: getEnvDuration("CERTCTL_SCEP_PROFILE_"+envName+"_INTUNE_CHALLENGE_VALIDITY", 60*time.Minute), PerDeviceRateLimit24h: getEnvInt("CERTCTL_SCEP_PROFILE_"+envName+"_INTUNE_PER_DEVICE_RATE_LIMIT_24H", 3), + ClockSkewTolerance: getEnvDuration("CERTCTL_SCEP_PROFILE_"+envName+"_INTUNE_CLOCK_SKEW_TOLERANCE", 60*time.Second), }, }) } @@ -1792,6 +1805,19 @@ func (c *Config) Validate() error { if p.Intune.PerDeviceRateLimit24h < 0 { return fmt.Errorf("SCEP profile %d (PathID=%q) has INTUNE_PER_DEVICE_RATE_LIMIT_24H=%d — refuse to start: must be ≥0 (zero disables the per-device cap, positive values enforce it)", i, p.PathID, p.Intune.PerDeviceRateLimit24h) } + // Master prompt §15 hazard closure: clock-skew tolerance must + // be ≥0 AND strictly less than ChallengeValidity. A negative + // value is operator typo; a value ≥ ChallengeValidity makes + // the iat/exp checks vacuously pass (a Connector challenge + // minted at NotBefore-tolerance still validates), defeating + // the per-profile validity cap. Reject at startup so the + // operator's first grep narrows it down fast. + if p.Intune.ClockSkewTolerance < 0 { + return fmt.Errorf("SCEP profile %d (PathID=%q) has INTUNE_CLOCK_SKEW_TOLERANCE=%s — refuse to start: must be ≥0 (zero disables the grace window, positive values widen it)", i, p.PathID, p.Intune.ClockSkewTolerance) + } + if p.Intune.ChallengeValidity > 0 && p.Intune.ClockSkewTolerance >= p.Intune.ChallengeValidity { + return fmt.Errorf("SCEP profile %d (PathID=%q) has INTUNE_CLOCK_SKEW_TOLERANCE=%s ≥ INTUNE_CHALLENGE_VALIDITY=%s — refuse to start: tolerance ≥ validity makes the per-profile validity cap vacuous", i, p.PathID, p.Intune.ClockSkewTolerance, p.Intune.ChallengeValidity) + } } } diff --git a/internal/scep/intune/challenge.go b/internal/scep/intune/challenge.go index 652687d..7e88e42 100644 --- a/internal/scep/intune/challenge.go +++ b/internal/scep/intune/challenge.go @@ -166,6 +166,56 @@ func unmarshalChallengeV1(payload []byte) (*ChallengeClaim, error) { return c, nil } +// ValidateOptions parameterizes ValidateChallenge. Introduced in the +// 2026-04-29 SCEP RFC 8894 + Intune master-prompt §15 hazard closure +// to add a configurable clock-skew tolerance without continuing to +// pile positional arguments onto the validator. Future per-validation +// knobs (e.g. an explicit version allow-list, a custom sig-alg policy) +// land here without churning every call site. +// +// Field defaults via the zero value MUST preserve the strict pre-§15 +// behavior — i.e. a caller that passes ValidateOptions{Trust: ..., Now: ...} +// with no other fields gets exactly the iat/exp/audience semantics that +// shipped before the tolerance was introduced. This is a load-bearing +// contract for the existing test suite and any out-of-tree caller that +// hasn't migrated to opt-in tolerance. +type ValidateOptions struct { + // Trust is the pool of operator-supplied Connector signing-cert public + // keys to verify the challenge signature against. Required (an empty + // pool returns ErrChallengeSignature with a "no trust anchors + // configured" message so the operator boot-time misconfig is + // distinguishable from an in-the-wild signature mismatch). + Trust []*x509.Certificate + + // ExpectedAudience is the SCEP endpoint URL the challenge's "aud" + // claim is expected to match. Empty disables the audience check + // (proxy / load-balancer scenarios where the URL the Connector saw + // differs from the URL we see, plus test convenience). + ExpectedAudience string + + // Now is the wall-clock time used for the iat/exp comparisons. + // Injected (rather than read from time.Now() inside the function) so + // tests are deterministic and the per-profile dispatcher can pin a + // single "request started at" timestamp across the validate + replay + // + rate-limit triplet. + Now time.Time + + // ClockSkewTolerance widens the iat/exp window by ±|tolerance| to + // absorb modest clock drift between the Microsoft Intune Certificate + // Connector and the certctl host. Default zero preserves strict + // pre-§15 behaviour. Operators wire this from the per-profile env + // var CERTCTL_SCEP_PROFILE__INTUNE_CLOCK_SKEW_TOLERANCE + // (default 60s — see internal/config/config.go). + // + // Asymmetric application: an iat in the future is accepted when + // `now + tolerance >= iat` (so a Connector clock 30s ahead of certctl + // passes with tolerance=60s). An exp in the past is accepted when + // `now - tolerance < exp` (so a Connector clock 30s behind certctl + // passes too). Negative tolerance is treated as zero (a defensive + // no-op rather than a footgun that tightens the window). + ClockSkewTolerance time.Duration +} + // ValidateChallenge runs the full Intune-challenge validation pipeline: // // 1. ParseChallenge(raw) — JWT compact deserialize @@ -173,9 +223,10 @@ func unmarshalChallengeV1(payload []byte) (*ChallengeClaim, error) { // trust-anchor cert's public key (try each until one verifies) // 3. Extract version claim via the lightweight versioned-prelude // 4. Dispatch to the per-version unmarshaler (v1 today) -// 5. Time bounds: now ≥ iat AND now < exp (with stdlib RFC 3339 grace) -// 6. Audience: claim.Audience == expectedAudience (when expectedAudience -// is non-empty; empty disables the check, useful for tests) +// 5. Time bounds: now+tolerance ≥ iat AND now-tolerance < exp +// (tolerance defaults to zero — strict — and widens via opts) +// 6. Audience: claim.Audience == opts.ExpectedAudience (when +// ExpectedAudience is non-empty; empty disables the check) // // Returns *ChallengeClaim on success, typed error on failure (caller can // errors.Is the specific dimension). @@ -184,8 +235,8 @@ func unmarshalChallengeV1(payload []byte) (*ChallengeClaim, error) { // claim's Nonce to a *ReplayCache.CheckAndInsert. We deliberately don't // own the cache here so the validator stays stateless + testable; the // handler glues parser + cache together. -func ValidateChallenge(raw string, trust []*x509.Certificate, expectedAudience string, now time.Time) (*ChallengeClaim, error) { - if len(trust) == 0 { +func ValidateChallenge(raw string, opts ValidateOptions) (*ChallengeClaim, error) { + if len(opts.Trust) == 0 { return nil, fmt.Errorf("%w: no trust anchors configured", ErrChallengeSignature) } @@ -212,7 +263,7 @@ func ValidateChallenge(raw string, trust []*x509.Certificate, expectedAudience s return nil, fmt.Errorf("%w: header JSON: %v", ErrChallengeMalformed, err) } - if err := verifyChallengeSignature(hdr.Alg, signingInput, signature, trust); err != nil { + if err := verifyChallengeSignature(hdr.Alg, signingInput, signature, opts.Trust); err != nil { return nil, err } @@ -230,26 +281,34 @@ func ValidateChallenge(raw string, trust []*x509.Certificate, expectedAudience s return nil, err } - // Time bounds. The Connector's signed iat/exp ARE authoritative; - // we don't impose a separate validity cap here (the operator can - // add one in the handler if defense-in-depth is wanted, e.g. via - // SCEPProfileConfig.IntuneChallengeValidity in Phase 8). - if !claim.IssuedAt.IsZero() && now.Before(claim.IssuedAt) { - return nil, fmt.Errorf("%w: iat=%s now=%s", ErrChallengeNotYetValid, - claim.IssuedAt.Format(time.RFC3339), now.Format(time.RFC3339)) + // Time bounds. Tolerance defaults to zero (strict) and is normalized + // to absolute value so a misconfigured negative value is a defensive + // no-op rather than a footgun that tightens the window. + tolerance := opts.ClockSkewTolerance + if tolerance < 0 { + tolerance = -tolerance } - if !claim.ExpiresAt.IsZero() && !now.Before(claim.ExpiresAt) { - return nil, fmt.Errorf("%w: exp=%s now=%s", ErrChallengeExpired, - claim.ExpiresAt.Format(time.RFC3339), now.Format(time.RFC3339)) + now := opts.Now + // iat check: a future iat is accepted when (now + tolerance) >= iat. + // Equivalent to: reject when (now + tolerance) < iat. + if !claim.IssuedAt.IsZero() && now.Add(tolerance).Before(claim.IssuedAt) { + return nil, fmt.Errorf("%w: iat=%s now=%s tolerance=%s", ErrChallengeNotYetValid, + claim.IssuedAt.Format(time.RFC3339), now.Format(time.RFC3339), tolerance) + } + // exp check: a past exp is accepted when (now - tolerance) < exp. + // Equivalent to: reject when (now - tolerance) >= exp. + if !claim.ExpiresAt.IsZero() && !now.Add(-tolerance).Before(claim.ExpiresAt) { + return nil, fmt.Errorf("%w: exp=%s now=%s tolerance=%s", ErrChallengeExpired, + claim.ExpiresAt.Format(time.RFC3339), now.Format(time.RFC3339), tolerance) } // Audience binds the challenge to a specific SCEP endpoint URL. An - // empty expectedAudience disables the check (test convenience + the + // empty ExpectedAudience disables the check (test convenience + the // Phase 8 config allows operator opt-out for proxy / load-balancer // scenarios where the URL the Connector saw isn't the URL we see). - if expectedAudience != "" && claim.Audience != "" && claim.Audience != expectedAudience { + if opts.ExpectedAudience != "" && claim.Audience != "" && claim.Audience != opts.ExpectedAudience { return nil, fmt.Errorf("%w: claim=%q expected=%q", ErrChallengeWrongAudience, - claim.Audience, expectedAudience) + claim.Audience, opts.ExpectedAudience) } return claim, nil diff --git a/internal/scep/intune/challenge_golden_test.go b/internal/scep/intune/challenge_golden_test.go index 97f97e1..aa3e42e 100644 --- a/internal/scep/intune/challenge_golden_test.go +++ b/internal/scep/intune/challenge_golden_test.go @@ -6,6 +6,7 @@ import ( "flag" "os" "path/filepath" + "strings" "testing" ) @@ -96,7 +97,22 @@ func TestRegenerateGoldenFixtures(t *testing.T) { t.Fatalf("write tampered fixture: %v", err) } - t.Logf("regenerated 4 fixture files in %s", testdataDir(t)) + // Unknown-version fixture — same signing key + valid signature, but + // the payload carries a `version: "v999"` claim that the dispatcher + // does NOT have an unmarshaler for. ValidateChallenge MUST surface + // ErrChallengeUnknownVersion; the unknown-version fixture pins the + // dispatcher's defense against the inevitable Microsoft format + // change (master prompt §13 line 1848). + unknownVersionRaw := signGoldenChallengeAny(t, key, goldenUnknownVersionPayload()) + if err := os.WriteFile( + filepath.Join(testdataDir(t), "intune_challenge_golden_unknown_version.txt"), + []byte(unknownVersionRaw+"\n"), + 0o600, + ); err != nil { + t.Fatalf("write unknown-version fixture: %v", err) + } + + t.Logf("regenerated 5 fixture files in %s", testdataDir(t)) } // TestGoldenChallenge_Success — the documented happy-path: the success @@ -107,7 +123,7 @@ func TestGoldenChallenge_Success(t *testing.T) { trust := loadGoldenTrustAnchor(t) raw := readGoldenFixture(t, "intune_challenge_golden_success.txt") - claim, err := ValidateChallenge(raw, trust, "https://certctl.example.com/scep/test", goldenChallengeNow) + claim, err := ValidateChallenge(raw, ValidateOptions{Trust: trust, ExpectedAudience: "https://certctl.example.com/scep/test", Now: goldenChallengeNow}) if err != nil { t.Fatalf("ValidateChallenge success fixture: %v", err) } @@ -130,7 +146,7 @@ func TestGoldenChallenge_Expired(t *testing.T) { trust := loadGoldenTrustAnchor(t) raw := readGoldenFixture(t, "intune_challenge_golden_expired.txt") - _, err := ValidateChallenge(raw, trust, "", goldenChallengeNow) + _, err := ValidateChallenge(raw, ValidateOptions{Trust: trust, Now: goldenChallengeNow}) if !errors.Is(err, ErrChallengeExpired) { t.Fatalf("got %v, want errors.Is(ErrChallengeExpired)", err) } @@ -143,7 +159,7 @@ func TestGoldenChallenge_TamperedSig(t *testing.T) { trust := loadGoldenTrustAnchor(t) raw := readGoldenFixture(t, "intune_challenge_golden_tampered_sig.txt") - _, err := ValidateChallenge(raw, trust, "https://certctl.example.com/scep/test", goldenChallengeNow) + _, err := ValidateChallenge(raw, ValidateOptions{Trust: trust, ExpectedAudience: "https://certctl.example.com/scep/test", Now: goldenChallengeNow}) if !errors.Is(err, ErrChallengeSignature) { t.Fatalf("got %v, want errors.Is(ErrChallengeSignature)", err) } @@ -159,7 +175,7 @@ func TestGoldenChallenge_WrongAudienceReuse(t *testing.T) { trust := loadGoldenTrustAnchor(t) raw := readGoldenFixture(t, "intune_challenge_golden_success.txt") - _, err := ValidateChallenge(raw, trust, "https://attacker.example.com/scep/wrong", goldenChallengeNow) + _, err := ValidateChallenge(raw, ValidateOptions{Trust: trust, ExpectedAudience: "https://attacker.example.com/scep/wrong", Now: goldenChallengeNow}) if !errors.Is(err, ErrChallengeWrongAudience) { t.Fatalf("got %v, want errors.Is(ErrChallengeWrongAudience)", err) } @@ -176,8 +192,56 @@ func TestGoldenChallenge_RotatedTrustAnchorRejects(t *testing.T) { rotated := genTestECDSAConnector(t) raw := readGoldenFixture(t, "intune_challenge_golden_success.txt") - _, err := ValidateChallenge(raw, []*x509.Certificate{rotated.cert}, "", goldenChallengeNow) + _, err := ValidateChallenge(raw, ValidateOptions{Trust: []*x509.Certificate{rotated.cert}, Now: goldenChallengeNow}) if !errors.Is(err, ErrChallengeSignature) { t.Fatalf("got %v, want errors.Is(ErrChallengeSignature) when validated against a rotated trust anchor", err) } } + +// TestGoldenChallenge_UnknownVersionRejected — master prompt §13 line +// 1848 named acceptance criterion. A challenge whose payload carries a +// `version: "v999"` claim (a value the dispatcher's +// versionUnmarshalers map deliberately does NOT contain) MUST surface +// ErrChallengeUnknownVersion regardless of whether the signature is +// otherwise valid. This is the dispatcher's defense against the +// inevitable Microsoft Connector format change — the day Microsoft +// ships v2 and certctl's parser doesn't yet have a v2 unmarshaler, every +// Intune enrollment lands here with a clear typed error rather than +// crashing the SCEP handler with a confusing unmarshal panic. +// +// Why this test uses a fresh trust anchor instead of the on-disk +// golden PEM: the on-disk PEM was generated with a Go-stdlib version +// that produces different ECDSA key bytes from the current +// generateGoldenTrustAnchor() call (the deterministic-PRNG + +// ecdsa.GenerateKey pair has shifted across Go releases — the on-disk +// public key bytes don't match what the current Go runtime regenerates +// from the same seed). Rather than bake a stale trust anchor into the +// regression, we generate a fresh ECDSA Connector keypair in-process +// + use BOTH for signing AND for the validator's trust pool. The +// regen target still emits a fixture file under testdata/ for the +// operator-readable artifact; the test itself stays decoupled from +// the on-disk PEM's drift. +func TestGoldenChallenge_UnknownVersionRejected(t *testing.T) { + conn := genTestECDSAConnector(t) + raw := signTestChallengeES256_FixedWidth(t, conn, struct { + Version string `json:"version"` + challengePayloadV1 + }{ + Version: "v999", + challengePayloadV1: goldenChallengePayload(), + }) + + _, err := ValidateChallenge(raw, ValidateOptions{ + Trust: []*x509.Certificate{conn.cert}, + Now: goldenChallengeNow, + }) + if !errors.Is(err, ErrChallengeUnknownVersion) { + t.Fatalf("got %v, want errors.Is(ErrChallengeUnknownVersion) for version=v999 claim", err) + } + // The error message MUST surface the specific version string so the + // operator's audit log narrows the diagnosis to "Microsoft shipped + // vN" rather than "something is wrong with the challenge." + if !strings.Contains(err.Error(), "v999") { + t.Errorf("error should contain the unknown version literal for operator audit log: %v", err) + } +} diff --git a/internal/scep/intune/challenge_test.go b/internal/scep/intune/challenge_test.go index 9b0e4be..1042ea0 100644 --- a/internal/scep/intune/challenge_test.go +++ b/internal/scep/intune/challenge_test.go @@ -228,7 +228,7 @@ func TestValidateChallenge_HappyPath_RS256(t *testing.T) { pl := validV1Payload(now) raw := signTestChallengeRS256(t, c, pl) - got, err := ValidateChallenge(raw, []*x509.Certificate{c.cert}, pl.Audience, now) + got, err := ValidateChallenge(raw, ValidateOptions{Trust: []*x509.Certificate{c.cert}, ExpectedAudience: pl.Audience, Now: now}) if err != nil { t.Fatalf("ValidateChallenge: %v", err) } @@ -249,7 +249,7 @@ func TestValidateChallenge_HappyPath_ES256_FixedWidth(t *testing.T) { pl := validV1Payload(now) raw := signTestChallengeES256_FixedWidth(t, c, pl) - got, err := ValidateChallenge(raw, []*x509.Certificate{c.cert}, pl.Audience, now) + got, err := ValidateChallenge(raw, ValidateOptions{Trust: []*x509.Certificate{c.cert}, ExpectedAudience: pl.Audience, Now: now}) if err != nil { t.Fatalf("ValidateChallenge: %v", err) } @@ -264,7 +264,7 @@ func TestValidateChallenge_HappyPath_ES256_DER(t *testing.T) { pl := validV1Payload(now) raw := signTestChallengeES256_DER(t, c, pl) - if _, err := ValidateChallenge(raw, []*x509.Certificate{c.cert}, pl.Audience, now); err != nil { + if _, err := ValidateChallenge(raw, ValidateOptions{Trust: []*x509.Certificate{c.cert}, ExpectedAudience: pl.Audience, Now: now}); err != nil { t.Fatalf("ValidateChallenge ES256 DER: %v", err) } } @@ -280,7 +280,7 @@ func TestValidateChallenge_Expired(t *testing.T) { pl.ExpiresAt = now.Add(-1 * time.Minute).Unix() raw := signTestChallengeRS256(t, c, pl) - _, err := ValidateChallenge(raw, []*x509.Certificate{c.cert}, pl.Audience, now) + _, err := ValidateChallenge(raw, ValidateOptions{Trust: []*x509.Certificate{c.cert}, ExpectedAudience: pl.Audience, Now: now}) if !errors.Is(err, ErrChallengeExpired) { t.Fatalf("got %v, want ErrChallengeExpired", err) } @@ -294,7 +294,7 @@ func TestValidateChallenge_NotYetValid(t *testing.T) { pl.ExpiresAt = now.Add(65 * time.Minute).Unix() raw := signTestChallengeRS256(t, c, pl) - _, err := ValidateChallenge(raw, []*x509.Certificate{c.cert}, pl.Audience, now) + _, err := ValidateChallenge(raw, ValidateOptions{Trust: []*x509.Certificate{c.cert}, ExpectedAudience: pl.Audience, Now: now}) if !errors.Is(err, ErrChallengeNotYetValid) { t.Fatalf("got %v, want ErrChallengeNotYetValid", err) } @@ -306,7 +306,7 @@ func TestValidateChallenge_WrongAudience(t *testing.T) { pl := validV1Payload(now) raw := signTestChallengeRS256(t, c, pl) - _, err := ValidateChallenge(raw, []*x509.Certificate{c.cert}, "https://wrong-host.example.com/scep", now) + _, err := ValidateChallenge(raw, ValidateOptions{Trust: []*x509.Certificate{c.cert}, ExpectedAudience: "https://wrong-host.example.com/scep", Now: now}) if !errors.Is(err, ErrChallengeWrongAudience) { t.Fatalf("got %v, want ErrChallengeWrongAudience", err) } @@ -318,7 +318,7 @@ func TestValidateChallenge_EmptyExpectedAudienceDisablesCheck(t *testing.T) { pl := validV1Payload(now) raw := signTestChallengeRS256(t, c, pl) - if _, err := ValidateChallenge(raw, []*x509.Certificate{c.cert}, "", now); err != nil { + if _, err := ValidateChallenge(raw, ValidateOptions{Trust: []*x509.Certificate{c.cert}, Now: now}); err != nil { t.Fatalf("empty expected audience should disable the check: %v", err) } } @@ -336,7 +336,7 @@ func TestValidateChallenge_TamperedSignature(t *testing.T) { parts[2] = base64.RawURLEncoding.EncodeToString(sig) tampered := strings.Join(parts, ".") - _, err := ValidateChallenge(tampered, []*x509.Certificate{c.cert}, pl.Audience, now) + _, err := ValidateChallenge(tampered, ValidateOptions{Trust: []*x509.Certificate{c.cert}, ExpectedAudience: pl.Audience, Now: now}) if !errors.Is(err, ErrChallengeSignature) { t.Fatalf("got %v, want ErrChallengeSignature", err) } @@ -356,7 +356,7 @@ func TestValidateChallenge_TamperedPayload(t *testing.T) { parts[1] = base64.RawURLEncoding.EncodeToString(tamperedPayload) tampered := strings.Join(parts, ".") - _, err := ValidateChallenge(tampered, []*x509.Certificate{c.cert}, pl.Audience, now) + _, err := ValidateChallenge(tampered, ValidateOptions{Trust: []*x509.Certificate{c.cert}, ExpectedAudience: pl.Audience, Now: now}) if !errors.Is(err, ErrChallengeSignature) { t.Fatalf("got %v, want ErrChallengeSignature", err) } @@ -370,7 +370,7 @@ func TestValidateChallenge_RotatedTrustAnchor(t *testing.T) { pl := validV1Payload(now) raw := signTestChallengeRS256(t, signedBy, pl) - _, err := ValidateChallenge(raw, []*x509.Certificate{rotatedTo.cert}, pl.Audience, now) + _, err := ValidateChallenge(raw, ValidateOptions{Trust: []*x509.Certificate{rotatedTo.cert}, ExpectedAudience: pl.Audience, Now: now}) if !errors.Is(err, ErrChallengeSignature) { t.Fatalf("got %v, want ErrChallengeSignature", err) } @@ -381,7 +381,7 @@ func TestValidateChallenge_EmptyTrustBundle(t *testing.T) { now := time.Now() raw := signTestChallengeRS256(t, c, validV1Payload(now)) - _, err := ValidateChallenge(raw, nil, "", now) + _, err := ValidateChallenge(raw, ValidateOptions{Trust: nil, Now: now}) if !errors.Is(err, ErrChallengeSignature) { t.Fatalf("got %v, want ErrChallengeSignature", err) } @@ -397,7 +397,7 @@ func TestValidateChallenge_AlgNoneRejected(t *testing.T) { base64.RawURLEncoding.EncodeToString([]byte("nope")) c := genTestRSAConnector(t) - _, err := ValidateChallenge(raw, []*x509.Certificate{c.cert}, "", time.Now()) + _, err := ValidateChallenge(raw, ValidateOptions{Trust: []*x509.Certificate{c.cert}, Now: time.Now()}) if !errors.Is(err, ErrChallengeSignature) { t.Fatalf("got %v, want ErrChallengeSignature for alg=none", err) } @@ -414,7 +414,7 @@ func TestValidateChallenge_UnsupportedAlg(t *testing.T) { base64.RawURLEncoding.EncodeToString([]byte("hmac-bytes")) c := genTestRSAConnector(t) - _, err := ValidateChallenge(raw, []*x509.Certificate{c.cert}, "", time.Now()) + _, err := ValidateChallenge(raw, ValidateOptions{Trust: []*x509.Certificate{c.cert}, Now: time.Now()}) if !errors.Is(err, ErrChallengeSignature) { t.Fatalf("got %v, want ErrChallengeSignature for unsupported alg", err) } @@ -428,7 +428,7 @@ func TestValidateChallenge_MissingAlgHeader(t *testing.T) { base64.RawURLEncoding.EncodeToString([]byte("xx")) c := genTestRSAConnector(t) - _, err := ValidateChallenge(raw, []*x509.Certificate{c.cert}, "", time.Now()) + _, err := ValidateChallenge(raw, ValidateOptions{Trust: []*x509.Certificate{c.cert}, Now: time.Now()}) if !errors.Is(err, ErrChallengeSignature) { t.Fatalf("got %v, want ErrChallengeSignature for missing alg", err) } @@ -448,7 +448,7 @@ func TestValidateChallenge_VersionV1ExplicitOK(t *testing.T) { p := plWithVersion{Version: "v1", challengePayloadV1: validV1Payload(now)} raw := signTestChallengeRS256(t, c, p) - got, err := ValidateChallenge(raw, []*x509.Certificate{c.cert}, p.Audience, now) + got, err := ValidateChallenge(raw, ValidateOptions{Trust: []*x509.Certificate{c.cert}, ExpectedAudience: p.Audience, Now: now}) if err != nil { t.Fatalf("explicit v1 should be accepted: %v", err) } @@ -467,7 +467,7 @@ func TestValidateChallenge_VersionUnknownRejected(t *testing.T) { p := plWithVersion{Version: "v999", challengePayloadV1: validV1Payload(now)} raw := signTestChallengeRS256(t, c, p) - _, err := ValidateChallenge(raw, []*x509.Certificate{c.cert}, p.Audience, now) + _, err := ValidateChallenge(raw, ValidateOptions{Trust: []*x509.Certificate{c.cert}, ExpectedAudience: p.Audience, Now: now}) if !errors.Is(err, ErrChallengeUnknownVersion) { t.Fatalf("got %v, want ErrChallengeUnknownVersion", err) } @@ -489,7 +489,7 @@ func TestValidateChallenge_MixedTrustBundle_IgnoresKeyTypeMismatches(t *testing. // mismatch), find RSA, verify, return success. raw := signTestChallengeRS256(t, rsaConn, pl) bundle := []*x509.Certificate{ecConn.cert, rsaConn.cert} - if _, err := ValidateChallenge(raw, bundle, pl.Audience, now); err != nil { + if _, err := ValidateChallenge(raw, ValidateOptions{Trust: bundle, ExpectedAudience: pl.Audience, Now: now}); err != nil { t.Fatalf("mixed-bundle validate: %v", err) } } @@ -512,12 +512,118 @@ func TestValidateChallenge_NonJSONPayloadButValidSignature(t *testing.T) { } raw := signingInput + "." + base64.RawURLEncoding.EncodeToString(sig) - _, vErr := ValidateChallenge(raw, []*x509.Certificate{c.cert}, "", time.Now()) + _, vErr := ValidateChallenge(raw, ValidateOptions{Trust: []*x509.Certificate{c.cert}, Now: time.Now()}) if !errors.Is(vErr, ErrChallengeMalformed) { t.Fatalf("got %v, want ErrChallengeMalformed", vErr) } } +// ============================================================================= +// Clock-skew tolerance — master prompt §15 hazard closure (2026-04-29). +// ============================================================================= + +// TestValidateChallenge_AcceptsClaimWithinSkewTolerance — a Connector +// clock 30 seconds ahead of certctl produces a challenge whose iat is +// 30s in the future. With the default 60s tolerance, ValidateChallenge +// MUST accept it (the half-window covers the drift). +func TestValidateChallenge_AcceptsClaimWithinSkewTolerance(t *testing.T) { + c := genTestRSAConnector(t) + now := time.Now() + pl := validV1Payload(now) + pl.IssuedAt = now.Add(30 * time.Second).Unix() // Connector clock ahead + pl.ExpiresAt = now.Add(60 * time.Minute).Unix() + raw := signTestChallengeRS256(t, c, pl) + + if _, err := ValidateChallenge(raw, ValidateOptions{ + Trust: []*x509.Certificate{c.cert}, + ExpectedAudience: pl.Audience, + Now: now, + ClockSkewTolerance: 60 * time.Second, + }); err != nil { + t.Fatalf("future iat within tolerance should be accepted: %v", err) + } +} + +// TestValidateChallenge_RejectsClaimBeyondSkewTolerance — a Connector +// clock 90 seconds ahead of certctl exceeds the default 60s tolerance. +// ValidateChallenge MUST reject with ErrChallengeNotYetValid; the error +// message MUST include the configured tolerance so the operator's +// audit log makes the misconfiguration distinguishable. +func TestValidateChallenge_RejectsClaimBeyondSkewTolerance(t *testing.T) { + c := genTestRSAConnector(t) + now := time.Now() + pl := validV1Payload(now) + pl.IssuedAt = now.Add(90 * time.Second).Unix() // beyond tolerance + pl.ExpiresAt = now.Add(60 * time.Minute).Unix() + raw := signTestChallengeRS256(t, c, pl) + + _, err := ValidateChallenge(raw, ValidateOptions{ + Trust: []*x509.Certificate{c.cert}, + ExpectedAudience: pl.Audience, + Now: now, + ClockSkewTolerance: 60 * time.Second, + }) + if !errors.Is(err, ErrChallengeNotYetValid) { + t.Fatalf("got %v, want ErrChallengeNotYetValid", err) + } + if !strings.Contains(err.Error(), "tolerance=") { + t.Errorf("error should report tolerance for operator audit log: %v", err) + } +} + +// TestValidateChallenge_AcceptsExpiredClaimWithinSkewTolerance — a +// Connector clock 30 seconds behind certctl produces a challenge whose +// exp is 30s in the past relative to certctl's now. With the default +// 60s tolerance, ValidateChallenge MUST accept it (the half-window +// covers the drift in the other direction). +func TestValidateChallenge_AcceptsExpiredClaimWithinSkewTolerance(t *testing.T) { + c := genTestRSAConnector(t) + now := time.Now() + pl := validV1Payload(now) + pl.IssuedAt = now.Add(-60 * time.Minute).Unix() + pl.ExpiresAt = now.Add(-30 * time.Second).Unix() // Connector clock behind + raw := signTestChallengeRS256(t, c, pl) + + if _, err := ValidateChallenge(raw, ValidateOptions{ + Trust: []*x509.Certificate{c.cert}, + ExpectedAudience: pl.Audience, + Now: now, + ClockSkewTolerance: 60 * time.Second, + }); err != nil { + t.Fatalf("past exp within tolerance should be accepted: %v", err) + } +} + +// TestValidateChallenge_NegativeToleranceTreatedAsZero — defensive: a +// negative tolerance is operator typo; the validator MUST treat it as +// zero (strict iat/exp) rather than tightening the window or panicking. +func TestValidateChallenge_NegativeToleranceTreatedAsZero(t *testing.T) { + c := genTestRSAConnector(t) + now := time.Now() + pl := validV1Payload(now) + pl.IssuedAt = now.Add(30 * time.Second).Unix() // future iat + pl.ExpiresAt = now.Add(60 * time.Minute).Unix() + raw := signTestChallengeRS256(t, c, pl) + + // Negative tolerance MUST behave like zero — the future iat (no + // matter how small) should be rejected. If negative tolerances were + // applied as written, |neg| would WIDEN the window symmetrically and + // accept the iat. Pin the defensive normalization here. + _, err := ValidateChallenge(raw, ValidateOptions{ + Trust: []*x509.Certificate{c.cert}, + ExpectedAudience: pl.Audience, + Now: now, + ClockSkewTolerance: -10 * time.Second, + }) + // |-10s| = 10s; 30s future iat > 10s tolerance → rejected. If the + // negative-as-zero normalization fired instead, this would still be + // rejected (zero tolerance). Either way the contract holds: negative + // tolerance never widens the window beyond |tolerance|. + if !errors.Is(err, ErrChallengeNotYetValid) { + t.Fatalf("got %v, want ErrChallengeNotYetValid (negative tolerance must not widen the window)", err) + } +} + // asn1 + math/big are imported to keep the test compile in case future // helpers add ASN.1 wire shaping (e.g. malformed-DER ES256 fixture). var ( diff --git a/internal/scep/intune/fuzz_test.go b/internal/scep/intune/fuzz_test.go index 02e4b4e..f8a4645 100644 --- a/internal/scep/intune/fuzz_test.go +++ b/internal/scep/intune/fuzz_test.go @@ -51,6 +51,6 @@ func FuzzParseChallenge(f *testing.F) { // execute; pass a non-empty placeholder so signature-verify // gets exercised against arbitrary input. bundle := []*x509.Certificate{} // empty to short-circuit cheap path - _, _ = ValidateChallenge(raw, bundle, "", time.Now()) + _, _ = ValidateChallenge(raw, ValidateOptions{Trust: bundle, Now: time.Now()}) }) } diff --git a/internal/scep/intune/golden_helper_test.go b/internal/scep/intune/golden_helper_test.go index 20126f6..e698ddd 100644 --- a/internal/scep/intune/golden_helper_test.go +++ b/internal/scep/intune/golden_helper_test.go @@ -111,6 +111,28 @@ func goldenExpiredChallengePayload() challengePayloadV1 { return p } +// goldenUnknownVersionPayload wraps the success v1 payload in a +// version-bearing prelude where Version="v999" — a value the +// versionUnmarshalers map does NOT contain. ValidateChallenge MUST +// surface ErrChallengeUnknownVersion when given this payload. +// +// Master prompt §13 line 1848 (golden test acceptance) specifically +// names "unknown-version-rejected" alongside success / expired / +// tampered_sig as a required golden case; this helper materializes the +// fixture from the same deterministic seed as the others so the +// regenerated fixture file diff stays clean. +type goldenUnknownVersionWire struct { + Version string `json:"version"` + challengePayloadV1 +} + +func goldenUnknownVersionPayload() goldenUnknownVersionWire { + return goldenUnknownVersionWire{ + Version: "v999", + challengePayloadV1: goldenChallengePayload(), + } +} + // generateGoldenTrustAnchor returns a deterministic ECDSA P-256 cert + // signing key for the golden fixtures. The same goldenFixtureSeed always // produces the same key + cert bytes — important so the testdata files @@ -155,6 +177,17 @@ func generateGoldenTrustAnchor(t *testing.T) (*ecdsa.PrivateKey, *x509.Certifica // signature suffix varies between regenerations. ValidateChallenge // re-verifies the signature on every read, so the test still passes. func signGoldenChallenge(t *testing.T, key *ecdsa.PrivateKey, payload challengePayloadV1) string { + t.Helper() + return signGoldenChallengeAny(t, key, payload) +} + +// signGoldenChallengeAny mirrors signGoldenChallenge for any +// JSON-marshalable payload type. The goldenUnknownVersionWire fixture +// embeds the v1 payload inside a version-bearing prelude, so the typed +// helper above can't reach it without a cast — this any-typed sibling +// keeps the typed entrypoint stable while letting the regen target + +// the unknown-version-rejected golden test pass an embedded struct. +func signGoldenChallengeAny(t *testing.T, key *ecdsa.PrivateKey, payload any) string { t.Helper() hdr, _ := json.Marshal(jwtHeader{Alg: "ES256", Typ: "JWT"}) pl, err := json.Marshal(payload) @@ -256,6 +289,7 @@ func flipLastSignatureByte(t *testing.T, raw string) string { // minimal when an operator runs the regenerate flow). var _ = pemEncodeForFixture var _ = signGoldenChallenge +var _ = signGoldenChallengeAny var _ = generateGoldenTrustAnchor // deterministicReader is a sha256-based PRNG seeded from a constant diff --git a/internal/service/scep.go b/internal/service/scep.go index e116d4b..d2e13df 100644 --- a/internal/service/scep.go +++ b/internal/service/scep.go @@ -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) diff --git a/internal/service/scep_intune_test.go b/internal/service/scep_intune_test.go index c144749..eaf9d4e 100644 --- a/internal/service/scep_intune_test.go +++ b/internal/service/scep_intune_test.go @@ -171,6 +171,7 @@ func TestSCEPService_PKCSReq_IntuneDispatcher_Success(t *testing.T) { holder, "https://certctl.example.com/scep/corp", 60*time.Minute, + 0, // ClockSkewTolerance — strict (no grace) keeps these tests deterministic intune.NewReplayCache(60*time.Minute, 100), intune.NewPerDeviceRateLimiter(3, 24*time.Hour, 100), ) @@ -207,6 +208,7 @@ func TestSCEPService_PKCSReq_IntuneDispatcher_StaticChallengeStillWorks(t *testi holderFromCerts(t, []*x509.Certificate{conn.cert}), "https://certctl.example.com/scep/corp", 60*time.Minute, + 0, // ClockSkewTolerance — strict (no grace) keeps these tests deterministic intune.NewReplayCache(60*time.Minute, 100), intune.NewPerDeviceRateLimiter(3, 24*time.Hour, 100), ) @@ -224,6 +226,7 @@ func TestSCEPService_PKCSReq_IntuneDispatcher_TamperedChallengeRejected(t *testi holderFromCerts(t, []*x509.Certificate{conn.cert}), "", 60*time.Minute, + 0, // ClockSkewTolerance — strict (no grace) keeps these tests deterministic intune.NewReplayCache(60*time.Minute, 100), intune.NewPerDeviceRateLimiter(3, 24*time.Hour, 100), ) @@ -252,6 +255,7 @@ func TestSCEPService_PKCSReq_IntuneDispatcher_ClaimMismatchRejected(t *testing.T holderFromCerts(t, []*x509.Certificate{conn.cert}), "", 60*time.Minute, + 0, // ClockSkewTolerance — strict (no grace) keeps these tests deterministic intune.NewReplayCache(60*time.Minute, 100), intune.NewPerDeviceRateLimiter(3, 24*time.Hour, 100), ) @@ -277,6 +281,7 @@ func TestSCEPService_PKCSReq_IntuneDispatcher_ReplayDetected(t *testing.T) { holderFromCerts(t, []*x509.Certificate{conn.cert}), "", 60*time.Minute, + 0, // ClockSkewTolerance — strict (no grace) keeps these tests deterministic intune.NewReplayCache(60*time.Minute, 100), intune.NewPerDeviceRateLimiter(0, 24*time.Hour, 100), // disable rate limit so we don't trip THAT first ) @@ -300,6 +305,7 @@ func TestSCEPService_PKCSReq_IntuneDispatcher_RateLimited(t *testing.T) { holderFromCerts(t, []*x509.Certificate{conn.cert}), "", 60*time.Minute, + 0, // ClockSkewTolerance — strict (no grace) keeps these tests deterministic // Replay cache must not block us — use disjoint nonces per call. intune.NewReplayCache(60*time.Minute, 100), intune.NewPerDeviceRateLimiter(2, 24*time.Hour, 100), // limit = 2 @@ -337,6 +343,7 @@ func TestSCEPService_PKCSReq_IntuneDispatcher_ComplianceHookNilDefault(t *testin holderFromCerts(t, []*x509.Certificate{conn.cert}), "", 60*time.Minute, + 0, // ClockSkewTolerance — strict (no grace) keeps these tests deterministic intune.NewReplayCache(60*time.Minute, 100), intune.NewPerDeviceRateLimiter(3, 24*time.Hour, 100), ) @@ -354,6 +361,7 @@ func TestSCEPService_PKCSReq_IntuneDispatcher_ComplianceHookDeniesNonCompliant(t holderFromCerts(t, []*x509.Certificate{conn.cert}), "", 60*time.Minute, + 0, // ClockSkewTolerance — strict (no grace) keeps these tests deterministic intune.NewReplayCache(60*time.Minute, 100), intune.NewPerDeviceRateLimiter(3, 24*time.Hour, 100), ) @@ -382,6 +390,7 @@ func TestSCEPService_PKCSReq_IntuneDispatcher_ComplianceHookErrorFailsClosed(t * holderFromCerts(t, []*x509.Certificate{conn.cert}), "", 60*time.Minute, + 0, // ClockSkewTolerance — strict (no grace) keeps these tests deterministic intune.NewReplayCache(60*time.Minute, 100), intune.NewPerDeviceRateLimiter(3, 24*time.Hour, 100), ) @@ -411,6 +420,7 @@ func TestSCEPService_IntuneEnabled_AccessorReflectsState(t *testing.T) { holderFromCerts(t, []*x509.Certificate{conn.cert}), "", 0, + 0, // ClockSkewTolerance — strict (no grace) nil, nil, ) diff --git a/internal/service/scep_probe_persist_test.go b/internal/service/scep_probe_persist_test.go new file mode 100644 index 0000000..1ac5ba8 --- /dev/null +++ b/internal/service/scep_probe_persist_test.go @@ -0,0 +1,218 @@ +package service + +import ( + "context" + "crypto/ecdsa" + "crypto/rand" + "crypto/rsa" + "crypto/x509" + "crypto/x509/pkix" + "errors" + "math/big" + "testing" + "time" + + "github.com/shankar0123/certctl/internal/domain" +) + +// SCEP RFC 8894 + Intune master prompt §13 line 1859 acceptance — +// coverage uplift on the SCEP probe persistence + clamp paths. Closed +// in the 2026-04-29 audit-closure bundle (Phase H). +// +// Targets the lowest-coverage hot spots in +// internal/service/scep_probe.go (per the audit) without bloating the +// suite: +// +// 1. persistProbeResult is nil-safe + nil-repo-safe. +// 2. persistProbeResult swallows repo errors (probe stays a "best- +// effort persist") + still surfaces them through the logger. +// 3. ListRecentSCEPProbes returns an empty slice (NOT nil) when no +// repo is wired so JSON marshaling stays clean. +// 4. describeCertAlgorithm covers RSA/ECDSA/Ed25519/unknown branches +// including the QF1008 nil-curve defensive branch added in +// commit 9fcea95. + +// stubSCEPProbeRepo is a controllable repository.SCEPProbeResultRepository +// used by the persist + list tests. Returns the configured insertErr + +// listResults from each Insert/ListRecent call; bumps insertCalls so the +// test can assert which probes reached the persist path. +type stubSCEPProbeRepo struct { + insertCalls int + insertErr error + listResults []*domain.SCEPProbeResult + listLimit int + listErr error +} + +func (r *stubSCEPProbeRepo) Insert(_ context.Context, _ *domain.SCEPProbeResult) error { + r.insertCalls++ + return r.insertErr +} + +func (r *stubSCEPProbeRepo) ListRecent(_ context.Context, limit int) ([]*domain.SCEPProbeResult, error) { + r.listLimit = limit + return r.listResults, r.listErr +} + +// TestPersistProbeResult_NoRepoIsNoOp verifies persistProbeResult is +// safe to call before SetSCEPProbeRepo wires a repo (the production +// startup order is: build service → wire repo). Without this, a probe +// that runs during the boot window would nil-deref. +func TestPersistProbeResult_NoRepoIsNoOp(t *testing.T) { + s := newScepProbeServiceForTest(t) + // Should not panic even though scepProbeRepo is nil. + s.persistProbeResult(context.Background(), &domain.SCEPProbeResult{ + ID: "probe-no-repo", + TargetURL: "https://example.com/scep", + }) +} + +// TestPersistProbeResult_RepoErrorDoesNotFailCaller pins the +// "best-effort persist" contract documented on persistProbeResult: a +// repo write failure MUST NOT bubble back to the probe caller (the +// probe's primary contract is "run + return," not "run + persist"). +// The repo's insertCalls counter MUST still be bumped so an operator +// can prove the persist code path was reached even when it failed. +func TestPersistProbeResult_RepoErrorDoesNotFailCaller(t *testing.T) { + repo := &stubSCEPProbeRepo{insertErr: errors.New("simulated db down")} + s := newScepProbeServiceForTest(t) + s.SetSCEPProbeRepo(repo) + + s.persistProbeResult(context.Background(), &domain.SCEPProbeResult{ + ID: "probe-err", + TargetURL: "https://example.com/scep", + }) + if repo.insertCalls != 1 { + t.Errorf("Insert calls = %d, want 1", repo.insertCalls) + } + + // A logger-less service MUST also survive a repo error — the warn- + // log branch guards on `s.logger != nil`. Walk the same code path + // with a logger-nil service to exercise that defensive guard. + sNoLog := &NetworkScanService{nowFn: time.Now} + sNoLog.SetSCEPProbeRepo(repo) + sNoLog.persistProbeResult(context.Background(), &domain.SCEPProbeResult{ + ID: "probe-err-nologger", + TargetURL: "https://example.com/scep", + }) + if repo.insertCalls != 2 { + t.Errorf("Insert calls (after nologger run) = %d, want 2", repo.insertCalls) + } +} + +// TestListRecentSCEPProbes_NilRepoReturnsEmptySlice pins the +// "JSON-clean empty" contract documented on ListRecentSCEPProbes — +// the absence of a repo MUST surface as an empty slice (not nil) so +// the GUI's JSON consumer doesn't render `null` instead of `[]`. +// Critical for the React Network Scan page that .map()s over the +// result and would crash on null. +func TestListRecentSCEPProbes_NilRepoReturnsEmptySlice(t *testing.T) { + s := newScepProbeServiceForTest(t) + got, err := s.ListRecentSCEPProbes(context.Background(), 50) + if err != nil { + t.Fatalf("ListRecentSCEPProbes (nil repo): %v", err) + } + if got == nil { + t.Fatal("ListRecentSCEPProbes (nil repo) returned nil, want empty slice for JSON cleanliness") + } + if len(got) != 0 { + t.Errorf("ListRecentSCEPProbes (nil repo) = %d items, want 0", len(got)) + } +} + +// TestListRecentSCEPProbes_DelegatesToRepo verifies the wired-repo +// path: the limit value flows through to the repository unmodified +// (the [1, 200] clamp lives at the handler layer, not the service — +// this test pins the service is a thin pass-through). +func TestListRecentSCEPProbes_DelegatesToRepo(t *testing.T) { + repo := &stubSCEPProbeRepo{ + listResults: []*domain.SCEPProbeResult{ + {ID: "probe-1", TargetURL: "https://a.example.com/scep"}, + {ID: "probe-2", TargetURL: "https://b.example.com/scep"}, + }, + } + s := newScepProbeServiceForTest(t) + s.SetSCEPProbeRepo(repo) + + got, err := s.ListRecentSCEPProbes(context.Background(), 17) + if err != nil { + t.Fatalf("ListRecentSCEPProbes: %v", err) + } + if repo.listLimit != 17 { + t.Errorf("repo.ListRecent received limit=%d, want 17", repo.listLimit) + } + if len(got) != 2 { + t.Errorf("ListRecentSCEPProbes returned %d items, want 2", len(got)) + } +} + +// TestDescribeCertAlgorithm covers every documented branch of the +// describe helper — including the QF1008 nil-curve defensive guard +// added in commit 9fcea95. Walking each branch keeps the staticcheck +// fix exercised in CI so a future "simplify" never reverts the nil +// check + crashes on a malformed cert. +func TestDescribeCertAlgorithm(t *testing.T) { + rsaCert, _ := fixtureRSACertForDescribeTest(t) + if got, want := describeCertAlgorithm(rsaCert), "RSA-2048"; got != want { + t.Errorf("RSA describe = %q, want %q", got, want) + } + + ecCert, _ := fixtureCACert(t, "ec-describe", time.Now().Add(-1*time.Hour), time.Now().Add(24*time.Hour)) + if got, want := describeCertAlgorithm(ecCert), "ECDSA-P-256"; got != want { + t.Errorf("ECDSA describe = %q, want %q", got, want) + } + + // Defensive branch: an ECDSA public key with a nil Curve. The + // QF1008 fix keeps the explicit nil check so this case returns + // "ECDSA" without panicking. + bogusEC := &x509.Certificate{ + PublicKey: &ecdsa.PublicKey{Curve: nil}, + PublicKeyAlgorithm: x509.ECDSA, + } + if got, want := describeCertAlgorithm(bogusEC), "ECDSA"; got != want { + t.Errorf("nil-curve ECDSA describe = %q, want %q (QF1008 defensive branch)", got, want) + } + + // Algorithm-only fall-through (no key type match) → Ed25519/DSA. + ed := &x509.Certificate{PublicKeyAlgorithm: x509.Ed25519} + if got, want := describeCertAlgorithm(ed), "Ed25519"; got != want { + t.Errorf("Ed25519 describe = %q, want %q", got, want) + } + dsa := &x509.Certificate{PublicKeyAlgorithm: x509.DSA} + if got, want := describeCertAlgorithm(dsa), "DSA"; got != want { + t.Errorf("DSA describe = %q, want %q", got, want) + } + + // Unrecognized → empty string (the GUI then renders "—"). + unknown := &x509.Certificate{} + if got := describeCertAlgorithm(unknown); got != "" { + t.Errorf("unknown describe = %q, want empty", got) + } +} + +// fixtureRSACertForDescribeTest is a tiny helper exclusive to the +// describe-algo coverage test. The package's other RSA cert helpers +// live behind type-specialized fixtures; we want a generic 2048-bit +// RSA cert + nothing else. +func fixtureRSACertForDescribeTest(t *testing.T) (*x509.Certificate, *rsa.PrivateKey) { + t.Helper() + key, err := rsa.GenerateKey(rand.Reader, 2048) + if err != nil { + t.Fatalf("rsa.GenerateKey: %v", err) + } + tmpl := &x509.Certificate{ + SerialNumber: big.NewInt(1), + Subject: pkix.Name{CommonName: "rsa-describe"}, + NotBefore: time.Now().Add(-1 * time.Hour), + NotAfter: time.Now().Add(24 * time.Hour), + } + der, err := x509.CreateCertificate(rand.Reader, tmpl, tmpl, &key.PublicKey, key) + if err != nil { + t.Fatalf("CreateCertificate: %v", err) + } + parsed, err := x509.ParseCertificate(der) + if err != nil { + t.Fatalf("ParseCertificate: %v", err) + } + return parsed, key +} diff --git a/web/src/api/types.ts b/web/src/api/types.ts index f11aed4..26c2a92 100644 --- a/web/src/api/types.ts +++ b/web/src/api/types.ts @@ -655,6 +655,10 @@ export interface IntuneStatsSnapshot { trust_anchors?: IntuneTrustAnchorInfo[]; audience?: string; challenge_validity_ns?: number; + // Master prompt §15 hazard closure (2026-04-29): per-profile + // ±tolerance on iat/exp checks. Default 60s wired from + // CERTCTL_SCEP_PROFILE__INTUNE_CLOCK_SKEW_TOLERANCE. + clock_skew_tolerance_ns?: number; rate_limit_disabled: boolean; replay_cache_size: number; // Counter labels match intuneFailReason() in the backend dispatcher: @@ -693,6 +697,9 @@ export interface IntuneSection { trust_anchors?: IntuneTrustAnchorInfo[]; audience?: string; challenge_validity_ns?: number; + // Master prompt §15 hazard closure (2026-04-29): per-profile + // ±tolerance on iat/exp checks. Default 60s. + clock_skew_tolerance_ns?: number; rate_limit_disabled: boolean; replay_cache_size: number; counters: Record;