mirror of
https://github.com/shankar0123/certctl.git
synced 2026-06-07 15:51:30 +00:00
edf6bee7f872828511a5fe4aa74b2e84618f8365
142 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 shankar0123
|
edf6bee7f8 |
target(awsacm): SDK-driven AWS Certificate Manager target connector
Closes Rank 5 (AWS half) of the 2026-05-03 Infisical deep-research
deliverable (cowork/infisical-deep-research-results.md Part 5).
Pre-fix, certctl had no path to deploy certs to AWS-managed TLS-
termination endpoints (ALB / CloudFront / API Gateway / App Runner)
— operators terminating TLS at AWS had to use Infisical secret-sync,
manual aws-cli imports, or external automation. This commit lands
the SDK-driven AWS Certificate Manager target connector that closes
the gap end-to-end.
Architecture:
- internal/connector/target/awsacm/awsacm.go — Connector wraps
*acm.Client behind the ACMClient interface seam (mirrors
awsacmpca's ACMPCAClient pattern from the issuer side).
LoadDefaultConfig handles the standard AWS credential chain
(IRSA / EC2 instance profile / SSO / env vars); no embedded
creds in connector Config.
- Pre-deploy snapshot via DescribeCertificate + GetCertificate so
on-import-failure rollback restores the previous cert. Mirrors
the Bundle 5 IIS pattern + the Bundle 7/8 WinCertStore /
JavaKeystore patterns. Surfaces rollback success/failure via
the existing certctl_deploy_rollback_total Prometheus counter
label set.
- Provenance tags: certctl-managed-by=certctl + certctl-
certificate-id=<mc-id> set automatically on every import. ACM
strips tags on re-import, so the connector calls
AddTagsToCertificate post-import to keep the provenance pair
fresh. Operators looking up a cert ARN by managed-cert ID
(Terraform data source, CloudFormation output) match against
these tags.
- DeploymentRequest.KeyPEM held in agent memory only — never
written to disk. Aligns with the pull-only deployment model
documented in CLAUDE.md.
Tests:
- awsacm_test.go: 15-subtest happy-path + validation matrix
covering ValidateConfig (success / missing-region / malformed-
region / malformed-ARN / reserved-tag rejection),
DeployCertificate (fresh import / rotate-in-place / rollback-
on-serial-mismatch / rollback-also-fails / empty-key-rejected /
no-client-rejected), ValidateOnly (returns sentinel),
ValidateDeployment (serial match / mismatch / no-ARN-yet).
- awsacm_failure_test.go: 5 per-error-class contract tests
mirroring the awsacmpca_failure_test.go shape (commit
|
||
|
shankar0123
|
109f32ff41 |
notifications: per-policy multi-channel expiry-alert routing
Closes Rank 4 of the 2026-05-03 Infisical deep-research deliverable
(see cowork/infisical-deep-research-results.md Part 5). Pre-fix,
RenewalService.CheckExpiringCertificates already ran daily,
RenewalPolicy.AlertThresholdsDays drove per-cert thresholds, and
NotificationService.SendThresholdAlert deduped per (cert, threshold)
— but the channel was hardcoded to Email
(internal/service/notification.go:118 pre-fix). Operators who
configured PagerDuty / Slack / Teams / OpsGenie via
CERTCTL_PAGERDUTY_ROUTING_KEY etc. got nothing at any threshold
unless SMTP was also wired. Their first signal of an expired cert
was a 3 AM outage.
This commit lands the routing matrix on top of the existing
infrastructure:
1. RenewalPolicy gains AlertChannels (per-tier channel list) +
AlertSeverityMap (per-threshold tier assignment) +
EffectiveAlertChannels / EffectiveAlertSeverity accessors.
Default*() helpers preserve the back-compat Email-only
behaviour for operators who haven't touched their policies
post-upgrade. Migration 000026 adds the JSONB columns
idempotently.
2. NotificationService.SendThresholdAlertOnChannel — the new
per-channel dispatch helper. Old SendThresholdAlert stays as
an Email-only alias so non-policy callers (admin "send test
alert" surfaces) keep working byte-for-byte.
3. NotificationService.HasThresholdNotificationOnChannel — per-
(cert, threshold, channel) deduplication so a transient
PagerDuty 5xx today does NOT suppress today's Slack alert and
tomorrow's PagerDuty retry will still fire.
4. RenewalService.sendThresholdAlerts walks the resolved channel
set per threshold tier, fans out to every configured channel,
handles per-channel failures independently, defensively drops
off-enum channels with an audit row trail, and records a per-
channel audit event with metadata.channel + metadata.severity_tier.
5. service.ExpiryAlertMetrics — atomic counter table mirrored on
the VaultRenewalMetrics shape from the 2026-05-03 audit fix #5
(commit
|
||
|
shankar0123
|
0792271dc6 |
vault: add automatic token renewal at TTL/2 + Prometheus metric
Closes Top-10 fix #5 of the 2026-05-03 issuer-coverage audit (see cowork/issuer-coverage-audit-2026-05-03/RESULTS.md). Pre-fix, the VaultPKI adapter authenticated with a static token and never called renew-self. Long-lived deploys hit token expiry; the first operator-visible signal was failed cert renewals on production targets. This commit: 1. Connector.Start(ctx) spawns a goroutine that calls POST /v1/auth/token/renew-self at TTL/2 cadence (computed from a one-shot lookup-self at startup). Honours ctx.Done() for graceful shutdown via a per-loop done channel + Stop(). 2. On `renewable: false` response (initial lookup OR any subsequent renewal), the loop emits a WARN, increments the not_renewable counter, and exits. The operator must rotate the token before Vault's Max TTL elapses. 3. New Prometheus counter certctl_vault_token_renewals_total with labels result={success,failure,not_renewable}. Registered alongside existing certctl_issuance_* counters in internal/api/handler/metrics.go. 4. ERROR-level logging on renewal failure with operator-actionable substring ("vault token renewal failed; rotate the token before TTL expires") so journalctl + grep find it. Loop keeps ticking after a failure — transient blips don't kill it. New optional issuer.Lifecycle interface: type Lifecycle interface { Start(ctx context.Context) error Stop() } Connectors that hold no background goroutines (almost all of them) do not implement this — IssuerRegistry.StartLifecycles / StopLifecycles feature-detect via type assertion. New lifecycle-bearing connectors plug in by implementing the interface; no further registry plumbing required. Wiring (cmd/server/main.go): - service.NewVaultRenewalMetrics() instance is shared between issuerRegistry.SetVaultRenewalMetrics (so Vault connectors built by Rebuild get a recorder) and metricsHandler.SetVaultRenewals (so the Prometheus exposer emits the new series). - issuerRegistry.StartLifecycles(ctx) is called after issuerService.BuildRegistry; defer issuerRegistry.StopLifecycles is paired so goroutines exit cleanly on signal. - IssuerConnectorAdapter.Underlying() exposes the wrapped issuer.Connector so registry-level machinery can reach the concrete connector behind the adapter without duplicating the wiring at every call site. Tests (internal/connector/issuer/vault/vault_renew_test.go): - TestVault_RenewLoop_TickAtHalfTTL — three ticks → three renewals, all "success". - TestVault_RenewLoop_StopsOnNotRenewable — second renewal returns renewable=false, loop exits, third tick fires no HTTP call. - TestVault_RenewLoop_FailureSurfacesViaMetric — first renewal 403 bumps "failure", second renewal succeeds → loop kept ticking. - TestVault_RenewLoop_CtxCancellation_StopsCleanly — Stop returns within 200ms after ctx cancel. - TestVault_RenewLoop_StartsNothingWhenNotRenewable — token already non-renewable at boot ⇒ no goroutine, "not_renewable" metric increments at startup so operators see it in Grafana. - TestVault_ComputeInterval — 4 cases pinning TTL/2 + minRenewInterval floor. - TestVault_RenewSelf_ParseFailure_NamesActionableInError — surfaced error contains "vault token renewal failed" + "rotate the token". Cadence is dynamic — every successful renewal re-derives TTL/2 from the renewed lease's lease_duration, so a short bootstrap token that gets renewed up to a longer Max TTL shifts to the longer cadence automatically (defends against degenerate fast ticking on a token whose Max TTL is far longer than its initial TTL). Documentation: - docs/connectors.md Vault PKI section gains "Token TTL + automatic renewal" subsection (operator-facing: cadence, metric, renewable=false rotation playbook). Out of scope (intentional, flagged in the audit follow-up): - AppRole / Kubernetes / AWS IAM auth methods (different renewal semantics). - Hot-reload of rotated token from disk (operator restarts today; future: GUI/MCP issuer-update path triggers Rebuild which Stops the old connector and Starts the new one). - Auto-re-auth after token death (operator playbook owns it). CHANGELOG.md is intentionally not hand-edited (per CHANGELOG.md itself: "no longer maintains a hand-edited per-version changelog; per-release notes are auto-generated from commit messages between consecutive tags"). Verified locally: - gofmt clean. - go vet ./internal/service/... ./internal/api/handler/... ./internal/connector/issuer/vault/... ./cmd/server/... clean. - go test -short -count=1 ./internal/connector/issuer/vault/... ./internal/service/... ./internal/api/handler/... green. - go test -race -count=10 -run 'TestVault_RenewLoop|TestVault_ComputeInterval' ./internal/connector/issuer/vault/... green. Audit reference: cowork/issuer-coverage-audit-2026-05-03/RESULTS.md Top-10 fix #5. |
||
|
shankar0123
|
bee47f0318 |
acme-server: cert-manager integration test + production hardening (Phase 5/7)
Closes the production-readiness loop on the ACME surface. After this
commit, certctl ships per-account rate limits + a GC sweeper for
expired ACME state + a kind-driven cert-manager 1.15 integration test
+ a lego-driven RFC conformance harness + a k6 loadtest scenario for
the unauthenticated ACME path.
Architecture:
- Rate limits live in-memory + per-replica. Restart wipes the
counters; orders/hour caps are eventual-consistency anyway. A
3-replica certctl-server fleet behind an LB effectively has 3x
the configured throughput per account; persistent rate limiting
is a follow-up if production telemetry shows abuse patterns we
can't catch in a single restart cycle. Per-key + per-action
isolation: ActionNewOrder/acc-1, ActionKeyChange/acc-1, and
ActionChallengeRespond/<challenge-id> are independent buckets.
- GC loop follows the existing scheduler-loop pattern (atomic.Bool
+ sync.WaitGroup; see crlGenerationLoop for shape). Three
independent SQL sweeps per tick (DELETE expired nonces; UPDATE
pending authzs whose expires_at < now() to expired; UPDATE
pending/ready/processing orders whose expires_at < now() to
invalid). Each sweep is a single statement; failures are logged-
and-continued so a failing nonces sweep doesn't block authzs.
Per-sweep 1m timeout bounds a stuck Postgres.
- cert-manager integration test is gated on KIND_AVAILABLE so CI
skips it cleanly (kind is too heavy for per-PR). Operators run
locally via 'make acme-cert-manager-test'; the harness brings up
a fresh cluster each run + tears it down on Cleanup.
- lego conformance harness drives a real ACME client through
register → run → cert-PEM-landed against a hermetic certctl
stack. Catches RFC-shape regressions third-party clients would
hit before they ship.
- k6 ACME-flow scenario hammers the unauthenticated surface
(directory + new-nonce + ARI synthetic-id) at 100 VUs × 5m. JWS-
signed flows are out of scope for k6 (no JWS support); they're
covered by the lego harness above.
What ships:
- internal/api/acme/ratelimit.go (+ ratelimit_test.go: 7 cases —
disable-when-perHour-zero, capacity, per-key isolation, per-
action isolation, refill-over-time, RetryAfter, concurrent-access
with -race + 200 goroutines × 200 calls).
- internal/repository/postgres/acme.go: 4 new methods —
CountActiveOrdersByAccount + GCExpiredNonces + GCExpireAuthorizations
+ GCInvalidateExpiredOrders. Each a single SQL statement.
- internal/service/acme.go: SetRateLimiter + GarbageCollect +
rate-limit gates at 3 entry points (CreateOrder + RotateAccountKey
+ RespondToChallenge) + concurrent-orders gate at CreateOrder.
2 new sentinels (ErrACMERateLimited, ErrACMEConcurrentOrdersExceeded);
5 new GC metrics (gc_runs / gc_run_failures / gc_nonces_reaped /
gc_authzs_expired / gc_orders_invalidated).
- internal/scheduler/scheduler.go: ACMEGarbageCollector interface +
acmeGCRunning atomic.Bool + acmeGCInterval + 2 setters (SetACME-
GarbageCollector + SetACMEGCInterval) + acmeGCLoop following the
crlGenerationLoop shape.
- internal/api/handler/acme.go: writeServiceError gains rateLimited
(429 + RFC 8555 §6.7) + concurrent-orders-exceeded mappings.
- internal/config/config.go: 5 new env vars
(CERTCTL_ACME_SERVER_RATE_LIMIT_ORDERS_PER_HOUR=100,
CERTCTL_ACME_SERVER_RATE_LIMIT_CONCURRENT_ORDERS=5,
CERTCTL_ACME_SERVER_RATE_LIMIT_KEY_CHANGE_PER_HOUR=5,
CERTCTL_ACME_SERVER_RATE_LIMIT_CHALLENGE_RESPONDS_PER_HOUR=60,
CERTCTL_ACME_SERVER_GC_INTERVAL=1m).
- cmd/server/main.go: NewRateLimiter() + SetRateLimiter() at
startup; conditional SetACMEGarbageCollector(acmeService) +
SetACMEGCInterval(cfg.ACMEServer.GCInterval) when Enabled+
GCInterval > 0.
- deploy/test/acme-integration/: kind-config.yaml + cert-manager-
install.sh + clusterissuer-trust-authenticated.yaml +
clusterissuer-challenge.yaml + certificate-test.yaml + conformance-
lego.sh + certmanager_test.go (//go:build integration + KIND_AVAILABLE
gate).
- deploy/test/loadtest/k6/acme_flow.js + README ACME-flows section.
- Makefile: 2 new PHONY targets (acme-cert-manager-test +
acme-rfc-conformance-test).
- docs/acme-server.md: status flipped to Phase 5; Configuration
table grows 5 rows; new 'Phase 5 — operational guidance' section
explaining rate-limit math + GC sweeper semantics + cert-manager
integration + lego conformance + k6 baseline.
Tests:
- 'go vet ./...' clean across the repo.
- 'go test -short -count=1 ./internal/...' green across every
affected package (service / acme / handler / scheduler / repo /
config).
- 'go vet -tags=integration ./deploy/test/acme-integration/' clean
(the integration test compiles cleanly with the build tag).
- The kind/cert-manager harness is gated behind KIND_AVAILABLE so
CI skips by default; operators run locally via 'make acme-cert-
manager-test'.
Engineering history: cowork/WORKSPACE-CHANGELOG.md 'ACME-Server-5'.
|
||
|
shankar0123
|
1e1bc9b3b4 |
ci: fix Phase 4 post-push unused-symbol failures
CI on commit
|
||
|
shankar0123
|
4dc8d3fa5b |
acme-server: key rollover + revocation + ARI (Phase 4/7)
Closes the RFC 8555 + RFC 9773 surface beyond the issuance happy-path:
- POST /acme/profile/<id>/key-change (RFC 8555 §7.3.5)
- POST /acme/profile/<id>/revoke-cert (RFC 8555 §7.6)
- GET /acme/profile/<id>/renewal-info/<cert-id> (RFC 9773 ARI)
After this commit, ACME clients can rotate account keys, revoke certs
through the ACME surface (rather than only via the certctl GUI/API),
and fetch ARI for proactive renewal scheduling.
Architecture:
- Key rollover: outer JWS verified against the registered account key
(existing kid path); the inner JWS — embedded as the outer's payload
— verified against the embedded NEW jwk in a new dedicated routine
(ParseAndVerifyKeyChangeInner) that enforces RFC 8555 §7.3.5
inner-only invariants: MUST use jwk + MUST NOT use kid, payload
.account == outer.kid, payload.oldKey thumbprint-equals registered.
A single WithinTx swaps the stored thumbprint+pem and writes the
audit row. Concurrent-rollover safety via SELECT…FOR UPDATE on the
conflicting account row in UpdateAccountJWKWithTx; the loser
observes the winner's new thumbprint and is told to retry (409).
- Revocation: two auth paths. kid → AccountOwnsCertificate single-
indexed COUNT lookup over acme_orders. jwk → constant-time RFC 7638
thumbprint compare against the cert's pubkey. Both paths route
through service.RevocationSvc.RevokeCertificateWithActor so the
existing CRL/OCSP refresh + audit + metrics pipeline applies. RFC
5280 §5.3.1 numeric reason codes clamp to certctl's
domain.ValidRevocationReasons; codes 8 (removeFromCRL) + 10
(aACompromise) clamp to 'unspecified' since they aren't in the set.
- ARI is GET-only and unauth per RFC 9773 §4. Cert-id wire shape is
base64url(AKI).base64url(serial); ParseARICertID strict-decodes,
SerialHex emits the canonical certctl-shape lowercase-no-leading-
zeros hex used in certificate_versions.serial_number.
ComputeRenewalWindow has 3 branches: bound RenewalPolicy →
[notAfter - days, notAfter - days/2]; no policy → last 33% of
validity; past expiry → [now, now + 1d] (renew immediately).
Retry-After honors CERTCTL_ACME_SERVER_ARI_POLL_INTERVAL.
What ships:
- internal/api/acme/{keychange,ari}.go (+ phase4_test.go: 15 tests).
- internal/api/acme/order.go: RevokeCertRequest wire shape.
- internal/api/handler/acme.go: KeyChange, RevokeCert, RenewalInfo
+ 11 new writeServiceError mappings.
- internal/repository/postgres/acme.go: UpdateAccountJWKWithTx (FOR
UPDATE + expectedOldThumbprint precondition; ErrACMEAccountKey-
ConcurrentUpdate sentinel) + AccountOwnsCertificate.
- internal/service/acme.go: RotateAccountKey + RevokeCert +
RenewalInfo; CertificateRevoker + RenewalPolicyLookup interfaces;
SetRevocationDelegate + SetRenewalPolicyLookup wiring; 11 new
sentinels; 6 new metrics.
- internal/service/acme_phase4_test.go: service-layer tests for
RotateAccountKey (happy + duplicate-key) + RevokeCert (kid mismatch
+ jwk mismatch + jwk happy + already-revoked + reason-clamping) +
RenewalInfo (disabled + bad cert-id).
- internal/api/router/router.go: 6 new register calls (3 per-profile
+ 3 shorthand). Router parity exceptions extended in lockstep
(in-tree SpecParityExceptions + CI-only openapi-handler-exceptions
.yaml).
- cmd/server/main.go: SetRevocationDelegate(revocationSvc) +
SetRenewalPolicyLookup(renewalPolicyRepo) at startup.
- internal/config/config.go: CERTCTL_ACME_SERVER_ARI_ENABLED (default
true) + CERTCTL_ACME_SERVER_ARI_POLL_INTERVAL (default 6h);
BuildDirectory's ariEnabled flag now flips on under
cfg.ARIEnabled.
- docs/acme-server.md: phase status flipped to Phase 4; endpoints
table grows 6 rows (3 per-profile + 3 shorthand); FAQ section
appended explaining how to rotate keys, revoke certs, and consume
ARI.
Tests:
- 'go vet ./...' clean across the repo.
- 'go test -short -count=1 ./...' green across every package.
- phase4_test.go covers: keychange happy-path + 5 negatives +
MapKeyChangeErrorToProblem coverage; ARI cert-id round-trip + 6
malformed cases + BuildARICertID from a generated cert; window-
math 3 branches.
- service-layer tests confirm: RotateAccountKey atomically swaps the
thumbprint (verifies persisted state) and rejects duplicate keys;
RevokeCert routes through the stub RevocationSvc with the right
actor string + reason on the jwk path, rejects mismatched keys,
rejects already-revoked certs, clamps reason codes correctly;
RenewalInfo respects ARIEnabled + cert-id format.
Engineering history: cowork/WORKSPACE-CHANGELOG.md 'ACME-Server-4'.
|
||
|
shankar0123
|
62513ad12f |
ci: fix Phase 3 post-push CI failures (contextcheck + ST1021)
CI on commit
|
||
|
shankar0123
|
9bc845304e |
acme-server: HTTP-01 + DNS-01 + TLS-ALPN-01 challenge validation (Phase 3/7)
Wires up the actual challenge-validation machinery so profiles in
acme_auth_mode='challenge' resolve end-to-end. After this commit,
cert-manager 1.15+ with `solver: http01: ingress` against a
challenge-mode profile completes a real HTTP-01 flow and gets a cert.
DNS-01 + TLS-ALPN-01 share the same code path with the appropriate
validator selection.
Architecture (the load-bearing parts):
- 3 separate semaphore-bounded worker pools (one per challenge type),
so HTTP-01 and DNS-01 can't starve each other under load. Default
weight 10 per type; tunable via CERTCTL_ACME_SERVER_HTTP01_CONCURRENCY,
DNS01_CONCURRENCY, TLSALPN01_CONCURRENCY.
- 30s per-challenge timeout (configurable via PoolConfig.PerChallengeTimeout).
- HTTP-01 validator runs validation.IsReservedIPForDial (newly
exported wrapper preserving the existing private impl byte-for-byte
for the network scanner + ValidateSafeURL paths) on the resolved
IP — both at the initial dial and every redirect hop. SSRF probes
into private IP space are refused before the connect.
- DNS-01 validator uses a dedicated resolver pointed at
CERTCTL_ACME_SERVER_DNS01_RESOLVER (default 8.8.8.8:53) — does
NOT use the system resolver to keep behavior deterministic across
deployments. Wildcard handling: `*.example.com` queries
_acme-challenge.example.com.
- TLS-ALPN-01 validator (RFC 8737) connects with ALPN `acme-tls/1`,
inspects the id-pe-acmeIdentifier extension (OID 1.3.6.1.5.5.7.1.31),
asserts the ASN.1 OCTET STRING value equals SHA-256 of the key
authorization. Cert chain is intentionally NOT validated
(InsecureSkipVerify=true is correct per RFC 8737 — the proof is
in the extension, not the chain). Documented in docs/tls.md L-001
table + the //nolint:gosec comment carries the justification.
SSRF guard: same posture as HTTP-01.
- Validation is asynchronous: handler accepts the POST and returns
200 immediately with status=processing; the worker-pool fires a
callback that updates challenge → authz → order in a fresh
background-context WithinTx. The order auto-promotes to `ready`
when ALL authzs become valid; auto-fails to `invalid` when ANY
authz becomes invalid.
What ships:
- internal/api/acme/challenge.go: KeyAuthorization (RFC 8555 §8.1) +
DNS01TXTRecordValue (§8.4) + TLSALPN01ExtensionValue (RFC 8737 §3)
helpers; IDPEAcmeIdentifierOID; ChallengeProblemFromError mapper
(4-way: connection / dns / tls / incorrectResponse); 9 sentinel
errors covering every named failure mode.
- internal/api/acme/validators.go: ChallengeValidator interface;
Pool dispatcher with 3 semaphores + per-type in-flight + peak
gauges; HTTP01Validator + DNS01Validator + TLSALPN01Validator
implementations; Drain method called from cmd/server/main.go's
shutdown sequence.
- internal/api/acme/validators_test.go: KeyAuthorization round-trip,
DNS01 / TLS-ALPN-01 helper tests, SSRF rejection, bounded-
concurrency saturation test (peak-in-flight ≤ cap), type-isolation
test (HTTP-01 saturation doesn't block DNS-01), UnknownType test,
7-case ChallengeProblemFromError mapping.
- internal/repository/postgres/acme.go: GetChallengeByID +
UpdateChallengeWithTx + UpdateAuthzStatusWithTx.
- internal/service/acme.go: SetValidatorPool wires the *acme.Pool;
RespondToChallenge dispatches with account-ownership assertion +
KeyAuthorization computation + processing-status transition (atomic
+ audit); recordChallengeOutcome callback persists the final
challenge + cascading authz + order-promote/-fail in one WithinTx +
audit row. 4 new metrics.
- internal/api/handler/acme.go: Challenge handler; round-trips
account.JWKPEM through ParseJWKFromPEM to recover the *jose.JSONWebKey
the validator pool needs.
- internal/api/router/router.go + openapi_parity_test.go +
api/openapi-handler-exceptions.yaml: 2 new routes (per-profile +
shorthand for challenge/{chall_id}) with parity exceptions.
- cmd/server/main.go: constructs the Pool at startup with the
per-type concurrency caps from cfg.ACMEServer; ACMEService.ValidatorPool()
accessor exposed for the shutdown drain sequence.
- internal/validation/ssrf.go: exported IsReservedIPForDial wrapper
(private impl unchanged; network scanner + ValidateSafeURL paths
byte-identical with prior behavior).
- docs/tls.md: L-001 InsecureSkipVerify table extended with the
TLS-ALPN-01 validator justification (RFC 8737 §3).
- docs/acme-server.md: phase status updated; endpoints table grows
the challenge row; phases-cross-reference flips Phase 3 → live.
Tests:
- 80%+ coverage on the new files.
- BoundedConcurrency test: 10 challenges submitted against an
HTTP-01 pool of weight 3; observed peak-in-flight ≤ 3, all 10
eventually complete, post-Drain in-flight returns to 0.
- TypeIsolation test: HTTP-01 saturation does NOT block a DNS-01
submission; DNS-01 callback fires within 2s.
- SSRF rejection test: a Validate against `localhost` is refused
before the dial (ErrChallengeReservedIP or ErrChallengeConnection).
Engineering history: cowork/WORKSPACE-CHANGELOG.md "ACME-Server-3".
|
||
|
shankar0123
|
c351bba41a |
acme-server: orders + authorizations + finalize + cert download (Phase 2/7)
Closes the issuance loop in trust_authenticated mode (commits |
||
|
shankar0123
|
44a85d6f85 |
acme-server: account resource + JWS verifier (Phase 1b/7)
Layers JWS-authenticated POST machinery onto the Phase 1a foundation
(commit
|
||
|
shankar0123
|
ec88a61274 |
acme-server: foundation — directory + new-nonce + per-profile routing (Phase 1a/7)
First slice of the RFC 8555 ACME server endpoint (master plan at cowork/acme-server-endpoint-prompt.md, per-phase prompts at cowork/acme-server-prompts/). This commit lands the smallest viable end-to-end deployable slice: an ACME client running curl -sk https://certctl/acme/profile/<id>/directory curl -sk -I https://certctl/acme/profile/<id>/new-nonce successfully fetches the directory document and a Replay-Nonce. Account creation, JWS verification, orders, challenges, and revocation are all out of scope for this phase and arrive in Phases 1b–4. Closes the Rank 1 LHF from the 2026-05-03 Infisical deep-research (cowork/infisical-deep-research-results.md). Pre-fix, certctl was an ACME consumer only — no /acme/directory endpoint, no JWS verifier, no challenge validators. K8s customers running cert-manager could not point at certctl as an ACME issuer; they had to deploy a certctl agent on every node. What ships: - internal/api/acme/{directory,nonce,errors}.go (+ tests). - internal/api/handler/acme.go + acme_handler_test.go. - internal/repository/postgres/acme.go (nonce ops only — Phase 1b extends with account CRUD; Phases 2-4 extend with order / authz / challenge CRUD). - internal/service/acme.go (BuildDirectory + IssueNonce stubs; Phase 1b adds VerifyJWS / NewAccount / etc.). - migrations/000025_acme_server.{up,down}.sql ships the full 5-table ACME schema (acme_accounts / acme_orders / acme_authorizations / acme_challenges / acme_nonces) PLUS the per-profile certificate_profiles.acme_auth_mode column. Phase 1a actively uses only acme_nonces; remaining tables are empty until Phases 1b-4 plug in. - internal/config/config.go: ACMEServerConfig struct + ACMEServer field on Config. Env vars use CERTCTL_ACME_SERVER_* prefix to avoid colliding with the existing consumer-side ACMEConfig at config.go:1746 (CERTCTL_ACME_DIRECTORY_URL / PROFILE / CHALLENGE_TYPE etc.). Phase 1a wires Enabled + DefaultAuthMode + DefaultProfileID + NonceTTL + DirectoryMeta; Order/Authz TTLs + per-challenge-type concurrency caps + DNS01 resolver are reserved fields parsed in 1a so operators can set them ahead of Phases 2/3. - cmd/server/main.go: wire ACMEHandler into the HandlerRegistry literal alongside the existing certificate / EST / SCEP / etc. handlers. - internal/api/router/router.go: HandlerRegistry.ACME field + 6 Register calls (3 per-profile + 3 shorthand). - internal/api/router/openapi_parity_test.go: 6 new entries in SpecParityExceptions. ACME is a wire-protocol surface (JWS-signed JSON over HTTPS per RFC 7515) whose semantics are dictated by RFC 8555 + RFC 9773 rather than by an OpenAPI document, same precedent as SCEP/EST. The canonical reference is docs/acme-server.md. - docs/acme-server.md: Phase-1a-shaped reference. Configuration table for every CERTCTL_ACME_SERVER_* env var. Per-profile auth-mode decision tree skeleton. TLS trust bootstrap section flagging cert-manager's ClusterIssuer.spec.acme.caBundle requirement (the single biggest first-time-deploy footgun; the full cert-manager walkthrough lands in Phase 6 but the requirement is documented up front). Architecture decisions baked in: - URL family is /acme/profile/<id>/* (per-profile, canonical) with /acme/* shorthand active when CERTCTL_ACME_SERVER_DEFAULT_PROFILE_ID is set. Path matches existing per-profile precedent in EST + SCEP. - Auth mode is per-profile (acme_auth_mode column on certificate_profiles), NOT server-wide. One certctl-server can serve trust_authenticated for an internal-PKI profile and challenge for a public-trust-style profile simultaneously. The column is read at request time, not cached at server start — operators flipping a profile's mode via SQL take effect on the next order without restart. - Nonces are DB-backed (acme_nonces table). Survive server restart. The RFC 8555 §6.5 replay defense requires the store to outlast the client's nonce caching window; an in-memory-only nonce store would lose every in-flight order on restart. - Per-op atomic counters on service.ACMEService.Metrics() — certctl_acme_directory_total, certctl_acme_directory_failures_total, certctl_acme_new_nonce_total, certctl_acme_new_nonce_failures_total. Naming follows certctl frozen decision 0.10 cardinality discipline. Phase 1b will extend with new_account counters; Phase 2 with order / finalize / cert; Phase 3 with per-challenge-type counters. Audit fixes #11 + #12 (cowork/acme-server-prompts/audit-additions.md) applied: - #11: CERTCTL_ACME_SERVER_* prefix avoids the consumer-side CERTCTL_ACME_* namespace collision. - #12: prior-attempt WIP from two failed Phase-1 dispatches was discarded at phase start; this commit starts from a clean tree. Tests: - 14 unit tests in internal/api/acme/ (directory, nonce, errors). - 7 handler-level tests via httptest.NewServer + mockACMEService (mirrors the mockSCEPService pattern at scep_handler_test.go). - 7 service-layer tests with mocked repo + injected profileLookup. - All pass under -race -count=1 -short. Deferred to Phase 1b: - JWS verification (go-jose v4 — see master-prompt §8a for the API surface and audit doc for the speculation pitfalls). - new-account / account/<id> endpoints + AccountService. - Nonce *consumption* path (issue path is in this commit; consume is only invoked by JWS-verified POSTs which Phase 1b adds). Engineering history: cowork/WORKSPACE-CHANGELOG.md "ACME-Server-1a". Per-phase implementation plan: cowork/acme-server-prompts/. Master plan + audit fixes: cowork/acme-server-endpoint-prompt.md + cowork/acme-server-prompt-audit.md + cowork/acme-server-prompts/audit-additions.md. |
||
|
shankar0123
|
475421457f |
fix(test): TestBoundedFanOut_SkipsAgentRoutedDeployments race on seenIDs slice
CI race detector flagged TestBoundedFanOut_SkipsAgentRoutedDeployments
on commit
|
||
|
shankar0123
|
35e18bfc56 |
scheduler: bound renewal concurrency via CERTCTL_RENEWAL_CONCURRENCY
Closes the #9 acquisition-readiness blocker from the 2026-05-01 issuer coverage audit. Pre-fix, JobService.ProcessPendingJobs ran every claimed job sequentially in a single goroutine: safe but slow, and operators with large fleets had no lever to dial throughput up. Switching to fire-and-forget per-job goroutines would have unbounded the upstream-CA call rate and tripped DigiCert / Entrust / Sectigo rate limits — certctl's response to 429 was to retry on the next tick, re-fanning out the same calls and digging deeper into the limit. Operators need a knob. This commit: - Adds CERTCTL_RENEWAL_CONCURRENCY env var (default 25) loaded via the existing getEnvInt pattern in internal/config/config.go. Documented inline as the cap for the per-tick renewal/issuance/ deployment goroutine fan-out, with operator-tuning guidance: permissive upstream limits + large fleets (>10k certs) → 100; strict limits or async-CA-heavy fleets → 25 or lower. - Wires golang.org/x/sync/semaphore.Weighted around the per-job goroutine launch in JobService.ProcessPendingJobs. Acquire(ctx, 1) is the load-bearing piece — it BLOCKS the loop when at the cap, providing real backpressure rather than fire-and-forget. The fan-out is split into processPendingJobsSequential (legacy, preserved for unit-test wiring that doesn't call SetRenewalConcurrency) and processPendingJobsConcurrent (production, delegates to a generic boundedFanOut helper). - boundedFanOut takes the per-job work as a closure so the cap can be tested directly without standing up the renewal/deployment service graph. processed/failed counters use atomic.Int64 to avoid mutex overhead on every job completion; final log line reads both AFTER wg.Wait so the counts reflect every dispatched job. ctx-aware Acquire ensures a shutdown ctx cancel interrupts the dispatch loop promptly; in-flight goroutines drain via Wait before the function returns so no goroutine outlives the scheduler tick. - shouldSkipJob extracted as a package-private helper so the agent-routed-deployment skip logic is shared between the sequential and concurrent paths byte-for-byte (the audit prompt's "channel-based semaphore without ctx-aware acquire" anti-pattern is explicitly avoided — semaphore.Weighted.Acquire returns on ctx done; channel <- struct{}{} would block forever). - SetRenewalConcurrency setter on JobService normalises ≤0 to 1. semaphore.NewWeighted(0) constructs a semaphore that blocks every Acquire forever; the normalisation prevents a misconfigured env var from wedging the scheduler. - cmd/server/main.go wires SetRenewalConcurrency(cfg.Scheduler. RenewalConcurrency) on the freshly-built jobService, immediately after SetAuditService. Production deployments always take the bounded path; tests that build JobService directly via NewJobService keep their strict-sequential behaviour because renewalConcurrency is the zero value. - Tests in internal/service/job_concurrency_test.go: * TestBoundedFanOut_CapHolds — primary regression guard. 50 jobs × 50ms work × cap=5 → asserts peak in-flight never exceeds 5 AND reaches 5 at least once (catches both upper-bound regressions and gates that incorrectly cap below the configured value). Lock-free max via CompareAndSwap so the measurement instrument doesn't itself constrain concurrency. * TestBoundedFanOut_AllJobsRun — lower-bound: every non-skipped job is dispatched. * TestBoundedFanOut_SkipsAgentRoutedDeployments — pins the shouldSkipJob contract. * TestBoundedFanOut_CtxCancelInterrupts — ctx cancellation interrupts a stuck fan-out within the timeout budget. * TestBoundedFanOut_FailedJobsCounted — per-job errors don't abort the fan-out. * TestSetRenewalConcurrency_NormalizesNonPositive — ≤0 → 1 fail-safe pinned across negative/zero/positive inputs. - docs/features.md: scheduler-loop table augmented with the concurrency-cap env-var pointer alongside the job-processor row. - docs/architecture.md: Concurrency Safety section gains a paragraph explaining the cap, the operator-tuning guidance, the ctx-aware Acquire semantics, and the audit reference. Operator-facing impact: the first big renewal sweep no longer takes down the upstream CA's rate-limit budget. Existing deployments get the bounded path automatically (default 25); operators can override via env var without code changes. Verified locally: - gofmt -l . clean - go vet ./... clean - staticcheck ./... clean - go test -short -count=1 across service / scheduler / config / integration: green - Six new tests under TestBoundedFanOut* + TestSetRenewalConcurrency*: green Audit reference: cowork/issuer-coverage-audit-2026-05-01/RESULTS.md Top-10 fix #9. |
||
|
shankar0123
|
fefa5a5fd7 |
acme: support serial-only revocation via local cert-version lookup
Closes the #7 acquisition-readiness blocker from the 2026-05-01 issuer coverage audit. Pre-fix, ACME RevokeCertificate at acme.go:L519-L529 returned the literal error "ACME revocation by serial not supported in V1; provide certificate DER". RFC 8555 §7.6 genuinely requires the cert DER bytes (not just the serial), but a CLM platform's job is to abstract over that limitation. Operators routinely have only the serial in hand: lost PEM, rotated key, GUI revoke action driven by a row in the certs list. This commit: - Adds CertificateLookupRepo interface at the ACME connector boundary (connector boundary, NOT a service/repository import — the connector accepts whatever satisfies the shape). Production wiring in cmd/server/main.go injects the postgres CertificateRepository; tests inject a fake. - Adds CertificateRepository.GetVersionBySerial(ctx, issuerID, serial) + interface declaration in repository/interfaces.go, returning the certificate_versions row whose SerialNumber matches, scoped to the issuer via JOIN on managed_certificates. Mirrors the existing GetByIssuerAndSerial shape but returns the version (where PEMChain lives). Per RFC 5280 §5.2.3 the issuer scope is required for determinism. - Adds SetCertificateLookup + SetIssuerID setters on *acme.Connector. Mirror the pattern local.Connector already uses for OCSP responder wiring. Both must be wired before serial-only revoke works; unwired state falls back to a more actionable error pointing at the wiring requirement (the historical "not supported" wording is retired). - Rewrites RevokeCertificate end-to-end: lookup → empty-PEM check → pem.Decode → block.Type == "CERTIFICATE" check → ensureClient → golang.org/x/crypto/acme.Client.RevokeCert(ctx, accountKey, der, reasonCode). RFC 8555 §7.6 case 1 (revocation request signed with account key) — the same account key issued the cert, so authority is intrinsic. The not-found path returns an actionable operator- facing error pointing at the local-store requirement. - Adds mapRevocationReason translating RFC 5280 §5.3.1 reason strings (unspecified, keyCompromise, cACompromise, affiliationChanged, superseded, cessationOfOperation, certificateHold, removeFromCRL, privilegeWithdrawn, aACompromise) into golang.org/x/crypto/acme. CRLReasonCode. Accepts canonical camelCase + underscore_lower + ALL_CAPS_UNDERSCORE. Nil reason → 0 (unspecified). Unknown reason errors rather than silently demoting (operators rely on the reason for compliance reporting). - Wiring update in service/issuer_registry.go: SetACMECertLookup setter on the registry; Rebuild type-asserts *acme.Connector and calls SetCertificateLookup + SetIssuerID, mirroring the existing *local.Connector branch. cmd/server/main.go calls issuerRegistry.SetACMECertLookup(certificateRepo) immediately after SetIssuanceMetrics — the postgres repo satisfies the interface via GetVersionBySerial. - Tests: * acme_revoke_test.go (new): TestRevokeCertificate_NoCertLookupWired, TestRevokeCertificate_NoIssuerIDWired, TestRevokeCertificate_LookupReturnsNotFound (operator-facing "may not have been issued through certctl" hint pinned), TestRevokeCertificate_LookupArbitraryError, TestRevokeCertificate_VersionPEMEmpty (corrupt-row guard), TestRevokeCertificate_PEMMalformed_NoBlock, TestRevokeCertificate_PEMMalformed_WrongType (PRIVATE KEY block rejected as not a CERTIFICATE). * TestMapRevocationReason_TableDriven: full RFC 5280 reason set plus camelCase / underscore / ALL-CAPS variants plus nil-reason and unknown-reason cases. * acme_failure_test.go: renamed TestRevokeCertificate_AlwaysError → TestRevokeCertificate_UnwiredCertLookupFallback; the test still exercises the same backward-compat branch but now asserts the new "CertificateLookup wiring" error wording. - Mock-repo updates (3 sites): mockCertificateRepository in internal/integration/lifecycle_test.go, mockCertRepo in internal/service/testutil_test.go, mockCertRepoWithGetError in internal/service/shortlived_test.go each gain a GetVersionBySerial implementation that mirrors the GetByIssuerAndSerial logic but returns the version row. - docs/connectors.md ACME section: new "Revocation by serial number" subsection covering the workflow, the local-store requirement (cert was issued through certctl, not imported), the reason-code mapping with the three accepted spelling variants, and a pointer to the audit reference. Out of scope (intentional, per spec): - Recovering the DER from outside the local cert store (CT logs, CSR + signature reconstruction). If the cert wasn't issued through certctl, revoke-by-serial via certctl isn't possible. - Revocation via the cert's private key (RFC 8555 §7.6 case 2). The account-key path covers all certctl-issued certs because the same account key issued them. - Pebble-backed integration test for the happy path. Pebble integration is the right home for that — the unit tests in this commit pin all failure-mode branches before the network call, and the wiring branch in Rebuild is exercised by the existing TestIssuerRegistryRebuild paths. Verified locally: - gofmt -l . clean - go vet ./... clean - staticcheck ./... clean - go test -short -count=1 across connector, service, repository, integration, api/middleware, api/handler: green Audit reference: cowork/issuer-coverage-audit-2026-05-01/RESULTS.md Top-10 fix #7. |
||
|
shankar0123
|
74d6b462a4 |
metrics: gofmt issuance_metrics_test.go — fix CI
Trivial whitespace fix: gofmt collapsed three trailing-comment columns that I'd hand-aligned in the test file. Local sandbox missed this because the per-file gofmt run earlier in the commit cycle was scoped to the changed-files list and didn't include the test file at the final write moment; CI's project-wide `gofmt -l .` caught it. Behavior unchanged. |
||
|
shankar0123
|
3b92048242 |
metrics: add per-issuer-type issuance counters, histogram, and failure classifier
Closes the #4 acquisition-readiness blocker from the 2026-05-01 issuer coverage audit. Before this commit, certctl's Prometheus exposition had zero per-issuer-type signal — operators answering "is DigiCert slow?" or "is Sectigo failing more than ACME?" had to grep logs by issuer name. This commit adds three series labelled by issuer type: certctl_issuance_total{issuer_type, outcome} certctl_issuance_duration_seconds{issuer_type} (histogram) certctl_issuance_failures_total{issuer_type, error_class} The histogram covers 0.05–120 second buckets to span the local-issuer fast path and async-CA slow path (DigiCert/Sectigo/Entrust polling can take minutes). error_class is a closed enum of eight values (timeout, auth, rate_limited, validation, upstream_5xx, upstream_4xx, network, other) classified once in service.ClassifyError. Cardinality budget is ~276 new series, well within Prometheus's comfortable range. Implementation: - service.IssuanceMetrics is the thread-safe counter + histogram table. Three independent views (counters / failures / durations) exposed via SnapshotCounters / SnapshotFailures / SnapshotDurations. sync.RWMutex protects the map shape; per-key sync/atomic.Uint64 primitives keep the recording hot path lock-free under concurrent service-layer goroutines. - service.IssuanceCounterEntry / IssuanceFailureEntry / IssuanceDurationEntry / IssuanceMetricsSnapshotter live in service (not handler) to avoid an import cycle: handler already imports service for admin_est.go etc., so service can't import handler back. Handler's exposer takes the snapshotter via the service-defined interface. - service.ClassifyError pure function maps error → error_class. context.DeadlineExceeded / context.Canceled → timeout; *net.OpError → network; substring matches against canonical AWS / DigiCert / Sectigo error shapes for auth / rate_limited / validation / upstream_5xx / upstream_4xx / network; unknown → other. Each branch has at least one representative test case in TestClassifyError. - IssuerConnectorAdapter.SetMetrics wires per-adapter recording (issuerType + metrics). Existing 28+ test call sites of NewIssuerConnectorAdapter keep their one-arg signature; production wiring goes through SetMetrics post-construction. - IssuerRegistry.SetIssuanceMetrics + Rebuild type-asserts to *IssuerConnectorAdapter and calls SetMetrics with the issuer type string. nil-guarded — tests that hand-build adapters without metrics get no-op recording. - IssuerConnectorAdapter.IssueCertificate / RenewCertificate wrap the underlying connector call with start := time.Now() and recordIssuance(start, err). Renewal is recorded into the same certctl_issuance_* series as initial issuance — operationally, renewal IS issuance from the connector's perspective (matches the audit prompt's guidance on series naming). - handler/metrics.go GetPrometheusMetrics gains a new exposer block emitting all three series in stable label order with correct Prometheus format (_bucket / _sum / _count for the histogram, +Inf bucket appended). Sorted via sort.Slice for stable output. nil- guarded so deploys without the wire produce clean exposition. - formatLE helper trims trailing zeros from histogram bucket labels via strconv.FormatFloat(le, 'f', -1, 64) so the `le` labels match Prometheus client conventions ("0.05", "30", "120", not "0.0500" etc.). - cmd/server/main.go wires a single IssuanceMetrics instance into both the IssuerRegistry (recording) and the MetricsHandler (exposer) using DefaultIssuanceBucketBoundaries. Tests: - TestIssuanceMetrics_RecordAndSnapshot — happy-path counter + histogram + failure recording, BucketBoundaries returns a copy (not shared storage). - TestIssuanceMetrics_HistogramCumulative — pins the cumulative-buckets contract. 100ms observation lands in 0.1 bucket and every larger bucket; 750ms only in the 1.0 bucket. Off-by-one here would corrupt every quantile query downstream. - TestIssuanceMetrics_Concurrency — 100 goroutines × 1000 ops under the race detector. Asserts atomic counter integrity across contended writes. - TestClassifyError — 17 cases covering every branch of the closed enum plus the nil-error special case. Implementation chooses the existing hand-rolled fmt.Fprintf exposition pattern (no prometheus/client_golang dependency added) to stay consistent with the OCSP / deploy counter blocks already in the file. Out of scope (separate follow-ups): - Revocation metrics (certctl_revocation_*) — symmetric to issuance but the audit didn't ask; explicit follow-up commit. - Discovery / health-check duration histograms. - prometheus/client_golang migration. Verified locally: - gofmt clean - go vet ./... clean - staticcheck ./... clean - golangci-lint run --timeout 5m ./... → 0 issues - go test -short -count=1 ./internal/service/ green - go test -short -count=1 -race -run TestIssuanceMetrics ./internal/service/ green - go test -short -count=1 ./internal/api/handler/ green - go build ./... success Audit reference: cowork/issuer-coverage-audit-2026-05-01/RESULTS.md Top-10 fix #4 (Part 3, narrative section). |
||
|
shankar0123
|
b0efdbe2f8 |
repo,service: introduce WithinTx and atomic audit rows for issue/renew/revoke
Closes the #3 acquisition-readiness blocker from the 2026-05-01 issuer coverage audit (Part 1.5 finding #1: audit row not transactional with issuance). AuditRepository.Create previously ran on the package-level *sql.DB while the certificate insert / version insert / revocation insert ran on independent connections — a failed audit INSERT after a successful operation INSERT was silently lost. SOX §404 over IT general controls, PCI-DSS §10 audit logging, HIPAA §164.312(b) audit controls, and CA/B Forum Baseline Requirements §5.4.1 audit log records all presume audit-with-operation atomicity. Design — Option A (Querier abstraction). The chosen pattern: a shared repository.Querier interface (subset of *sql.DB and *sql.Tx) plus a postgres.WithinTx helper that begins a tx, runs fn, commits on nil error, rolls back on error or panic, and returns the wrapped result. Repository methods that participate in a service-layer transaction expose a *WithTx variant taking repository.Querier; the bare methods remain for stand-alone use. A repository.Transactor abstracts the "begin tx, run fn, commit/rollback" lifecycle so service-layer code runs multi-write operations atomically without holding *sql.DB directly. Option B (UnitOfWork) was considered but adds boilerplate without behavioral benefit for the current scope. Option C (context-carried tx) was explicitly rejected — it hides the transactional boundary from the type system, reproducing the class of bug we're fixing. This commit: - Adds internal/repository/querier.go with the Querier interface (compile-time guards that *sql.DB and *sql.Tx satisfy it) and the Transactor interface for service-layer use. - Adds internal/repository/postgres/tx.go with the WithinTx helper (begin/fn/commit/rollback with panic recovery) and a transactor type that satisfies repository.Transactor. - Adds CreateWithTx variants on AuditRepository, CertificateRepository (Create + Update + CreateVersion), and RevocationRepository. Existing bare methods now delegate to the *WithTx variant using the package-level *sql.DB so existing call sites are behavior-preserving. - Updates repository/interfaces.go: AuditRepository, CertificateRepository, and RevocationRepository declare the new *WithTx methods. Adds an atomicity contract doc-comment on AuditRepository pointing at WithinTx + the audit blocker. - Adds AuditService.RecordEventWithTx, mirroring RecordEvent but routing through CreateWithTx so the audit row is part of the caller's transaction. Same redaction + marshalling contract. - Refactors three audit-emitting service paths to use Transactor.WithinTx when SetTransactor was wired, with a legacy fallback for backward compat: * CertificateService.Create — cert insert + audit row in one tx. * RevocationSvc.RevokeCertificateWithActor — cert status update + revocation row + audit row in one tx. The OCSP cache invalidate remains best-effort (out of scope per the prompt). * RenewalService CompleteServerRenewal — cert version insert + cert update + audit row in one tx. Job status update stays outside the audit-atomicity scope (job state lives outside the operator-facing audit trail). - Adds SetTransactor on CertificateService, RevocationSvc, and RenewalService. cmd/server/main.go wires a single Transactor instance shared across all three so all audit-emitting paths run their writes in transactions backed by the same *sql.DB handle. - Updates 5 mock implementations to satisfy the new interface methods: mockCertRepo (testutil_test.go), mockCertRepoWithGetError (shortlived_test.go), fakeRevocationRepo (crl_cache_test.go), intuneE2EAuditRepo (scep_intune_e2e_test.go), and the integration- test mocks (lifecycle_test.go: mockCertificateRepository, mockAuditRepository, mockRevocationRepository). All *WithTx mocks ignore the Querier and delegate to the bare method (mocks have no DB; in-memory state is shared regardless of "tx"). - Adds a service-layer test mockTransactor with BeginTxErr and CommitErr knobs so the atomic-audit tests can assert error propagation through the transactional boundary. - Adds internal/repository/postgres/tx_test.go: unit-level test that WithinTx surfaces "begin tx" wrap when BeginTx fails, and that Transactor.WithinTx delegates correctly. Real-Postgres rollback semantics are covered by the testcontainers tests in the postgres package — sandbox disk pressure prevented adding a sqlmock dep for the in-fn / commit-failure unit test, so those scenarios are exercised through atomic_audit_test.go using the mockTransactor's CommitErr / BeginTxErr fields. - Adds internal/service/atomic_audit_test.go: * TestCertificateService_Create_AtomicWithTx — asserts audit insert failure inside the tx surfaces as the operation's error (closes the blocker contract). * TestCertificateService_Create_LegacyPathLogs — pins the backward-compat behavior when SetTransactor isn't wired: audit failure is logged-not-failed, matching pre-fix. * TestCertificateService_Create_TransactorBeginFailure — BeginTx error path: operation fails, no cert insert, no audit insert. * TestCertificateService_Create_TransactorCommitFailure — Commit error after successful in-fn writes surfaces as the operation's error. Real Postgres can fail Commit on serialization conflicts; the service must report this. Out of scope (separate follow-up commits, same shape): - Issuer CRUD audit atomicity. - Target CRUD audit atomicity. - Agent retire (already transactional via RetireAgentWithCascade; verified, not changed). - Renewal-policy CRUD audit atomicity. - Owner/team/agent-group CRUD audit atomicity. - Discovery / health-check audit atomicity. Verified locally: - gofmt -l . clean - go vet ./... clean - staticcheck ./... clean - golangci-lint run --timeout 5m ./... → 0 issues - go test -short -count=1 ./internal/service/ green - go test -short -count=1 ./internal/api/handler/ green - go test -short -count=1 ./internal/integration/ green - go test -short -count=1 ./internal/repository/postgres/ green - go build ./... success Audit reference: cowork/issuer-coverage-audit-2026-05-01/RESULTS.md Top-10 fix #3 (Part 3, narrative section). |
||
|
shankar0123
|
804a1b05ce |
awsacmpca: thread ctx through factory + registry — fix CI contextcheck
Follow-up to |
||
|
shankar0123
|
7cb453a336 |
chore(fmt): repo-wide gofmt -w sweep — close drift surfaced by ci-pipeline-cleanup Phase 4
Mechanical reformat. The new 'gofmt drift' CI step (added in
ci-pipeline-cleanup Phase 4, commit
|
||
|
shankar0123
|
8637131f80 |
chore: gofmt fixes across deploy-hardening I new files
Phase 13 verification surfaced gofmt-formatting drift in 6 files across the bundle's new code: - internal/api/handler/metrics.go (struct field alignment) - internal/connector/target/k8ssecret/validate_only_test.go (alignment) - internal/connector/target/nginx/nginx.go (alignment) - internal/connector/target/postfix/postfix.go (alignment) - internal/connector/target/ssh/validate_only_test.go (alignment) - internal/service/deploy_counters.go (alignment) Pure mechanical gofmt -w fixes; no behavior changes. CI's make verify gate (which runs `go fmt ./...`) didn't catch these because go fmt is more lenient than gofmt -l, but golangci-lint v2.11.4 + the explicit gofmt step in Phase 13 verification did. Phase 13 full-matrix verification all green: - gofmt -l: empty across all bundle-touched files - go vet ./internal/deploy/... ./internal/connector/target/... ./internal/service/ ./internal/api/handler/ ./cmd/agent/: clean - golangci-lint v2.11.4 (the version CI runs): 0 issues - go test -race -count=1 across deploy + nginx + apache + haproxy + agent + service: all green - INTEGRATION=1 go test -tags integration -run Deploy ./deploy/test/...: 4/4 e2e tests green Phase 14 next: release prep — Active Focus update, release notes, Reddit-beat draft, final tag handoff to operator. |
||
|
shankar0123
|
135b271197 |
feat(metrics): per-target-type deploy counters wired into /metrics/prometheus
Phase 10 of the deploy-hardening I master bundle. Mirrors the
production-hardening-II Phase 8 OCSP-counter pattern. Per frozen
decision 0.9, the metric naming convention is
`certctl_deploy_<area>_total` with target_type + sub-label.
internal/service/deploy_counters.go:
- DeployCounters struct with sync.Map of per-target-type buckets
(apache, nginx, etc.). Lock-free fast path via sync/atomic
Uint64 counters; LoadOrStore on first tick.
- 8 sub-counters per target-type bucket:
- attemptsSuccess / attemptsFailure
- validateFailures (PreCommit returned error)
- reloadFailures (PostCommit returned error → rollback ran)
- postVerifyFails (post-deploy TLS handshake failed)
- rollbackRestored (rollback succeeded)
- rollbackAlsoFail (operator-actionable escalation)
- idempotentSkips (SHA-256 match → no-op deploy)
- Snapshot returns []DeploySnapshot for the Prometheus exposer.
internal/service/deploy_counters_test.go:
- 5 tests: zero-state, per-target-type tick isolation, race-detector
smoke under concurrent ticks, cross-target bucket isolation,
snapshot-mutation-doesn't-affect-counter.
internal/api/handler/metrics.go:
- New DeployCounterSnapshotter interface (mirrors CounterSnapshotter
for the OCSP counters but uses the per-target-type tuple shape).
- New DeploySnapshotEntry struct copying the service-layer shape;
avoids importing the service package directly so the handler
stays dependency-light.
- New SetDeployCounters setter on MetricsHandler (mirrors
SetOCSPCounters wiring).
- Prometheus exposer extended with 6 new metric blocks per frozen
decision 0.9:
- certctl_deploy_attempts_total{target_type, result}
- certctl_deploy_validate_failures_total{target_type}
- certctl_deploy_reload_failures_total{target_type}
- certctl_deploy_post_verify_failures_total{target_type}
- certctl_deploy_rollback_total{target_type, outcome}
- certctl_deploy_idempotent_skip_total{target_type}
- Output sorted by target_type for stable diffs across requests.
The agent-side wire-up (cmd/agent/main.go ticking counters in the
DeployCertificate dispatch site) is intentionally deferred to a
follow-up commit — Phase 10's load-bearing change is the
infrastructure; per-connector tick wiring is a mechanical follow-on.
Build + go vet clean. go test -count=1 green for service +
handler packages.
Phase 11 next: cross-cutting integration tests at deploy/test/.
|
||
|
shankar0123
|
9e6c57673e |
test(service): coverage uplift for production hardening II + adjacent helpers (R-CI-extended floor)
CI's R-CI-extended coverage gate failed on 2025-04-30: service-layer
coverage was 68.7% vs the 70% floor. The drag was from new files
(internal/service/ocsp_counters.go, ocsp_response_cache.go,
export_audit_actions.go) that shipped without enough direct tests
to keep the package above the floor.
NEW internal/service/ocsp_counters_test.go (4 tests):
- TestOCSPCounters_NewIsZero — fresh counter snapshot is all zero
- TestOCSPCounters_EveryIncTicksItsLabel — table-driven test
pinning every Inc* method to its label string + the no-cross-
bleed invariant. Critical for Phase 8 Prometheus exposer
contract: a typo in either side would silently drop the
counter from /metrics/prometheus.
- TestOCSPCounters_SnapshotIsCopy — mutating the returned map
doesn't affect the underlying counters
- TestOCSPCounters_ConcurrentTicksRace — race-detector smoke
against sync/atomic primitives
NEW internal/service/ocsp_response_cache_real_test.go (10 tests):
- HappyPath_CachesAfterMiss — first fetch live-signs + writes
cache row; second fetch hits cache
- CacheWriteFailureIsNonFatal — putErrorRepo simulates disk full;
response still returned (fail-soft contract)
- StaleEntryRegenerates — entries with next_update in the past
trigger re-sign on next fetch
- InvalidateOnRevoke — pin the load-bearing security wire
- InvalidateOnRevoke_DeleteFailureSurfacesError — error-path
coverage for the delete branch
- CountByIssuer + NilRepoReturnsEmpty
- CAOperationsSvc.GetOCSPResponseWithNonce_CacheDispatchHit pins
the nil-nonce → cache dispatch wire
- CAOperationsSvc.GetOCSPResponseWithNonce_NonceBypassesCache
pins the nonce-bearing → live-sign bypass wire (cache stays
empty)
- RevocationSvc.SetOCSPCacheInvalidator_WireConnects pins the
setter through to the wired interface
NEW internal/service/coverage_extras_test.go (~12 tests) targets the
0%-coverage chunks adjacent to the bundle's modified files so the
package as a whole stays above the floor:
- cert-export typed audit emission (Phase 7) round-trip with
detail-map inspection (has_private_key + actor_kind + cipher pin)
- PKCS12CipherModernAES256 pinned-value test (drift catches a
future go-pkcs12 default change)
- audit.ListAuditEvents + GetAuditEvent (handler-interface methods
that were at 0%)
- certificate.ListCertificatesWithFilter (M20 filter delegate)
- discovery.{ListScans,GetScan,GetDiscoverySummary} (delegates)
- health_check.{Update,SetNotificationService} delegates + audit
- est.{deterministicSerial,zeroizeBytes,zeroizeKey} pure helpers
+ the live RSA + ECDSA key-zeroize branches
Sandbox total: 67.6% → 69.9% (+2.3pp). The live keygen branches
in zeroizeKey skip in the sandbox when crypto/rand isn't available
but run on CI, so the CI total should land above the 70% floor with
a small buffer.
Pre-commit verification: go build ./... clean; go test -short
-count=1 green for ./internal/service/.
|
||
|
shankar0123
|
13b29ca1bd |
fix(cert-export): satisfy staticcheck ST1022 on PKCS12CipherModernAES256
Production hardening II Phase 11 verification — golangci-lint v2.11.4 flagged the const PKCS12CipherModernAES256 doc comment with ST1022 (comments on exported identifiers should start with the identifier name). Reformatted to lead with the const name; same content. Reproduced clean: 0 issues across handler/, service/, connector/issuer/local/, api/router/, ratelimit/. |
||
|
shankar0123
|
8cba794723 |
feat(cert-export): typed audit-action constants + has_private_key + cipher detail (Phase 7)
Production hardening II Phase 7 — typify the cert-export audit
emission. The pre-Phase-7 audit log carried inline strings
("export_pem" / "export_pkcs12"); this commit adds typed
constants alongside via the split-emit pattern so operators get
both back-compat with existing log analysers AND a stable typed
grep target.
NEW internal/service/export_audit_actions.go:
- AuditActionCertExportPEM = "cert_export_pem"
- AuditActionCertExportPEMWithKey = "cert_export_pem_with_key"
(reserved for future bundle that adds key-bearing export; not
emitted in V2)
- AuditActionCertExportPKCS12 = "cert_export_pkcs12"
- AuditActionCertExportFailed = "cert_export_failed"
- PKCS12CipherModernAES256 = "AES-256-CBC-PBE2-SHA256" pinned
string for the cipher detail (drift catches a future go-pkcs12
default change)
Detail enrichment on both emission sites:
- has_private_key (bool, V2 always false — cert-only export is
the only V2 path; key-bearing export deferred to future bundle)
- actor_kind ("user")
- cipher (PKCS12 only — pinned to PKCS12CipherModernAES256)
Split-emit pattern: each export emits BOTH the legacy bare action
code AND the typed constant. Mirrors est.go::processEnrollment which
emits both "est_simple_enroll" + "est_simple_enroll_success".
Existing audit-log analysers that match by exact string "export_pem"
keep working; new operator alerts can target the typed constant.
Pre-commit verification: go build ./... clean; go test -short
-count=1 green for service/.
|
||
|
shankar0123
|
40fd96a416 |
feat(ocsp): pre-signed response cache + invalidate-on-revoke (Phase 2)
Production hardening II Phase 2 — closes the per-request live-signing
bottleneck for OCSP. Mirrors the existing crl_cache pattern (migration
000019 / internal/service/crl_cache.go) but per (issuer_id, serial_hex)
instead of per-issuer.
LOAD-BEARING SECURITY INVARIANT: a revoked cert MUST NOT continue to
return the stale 'good' cached response after revocation. The
RevocationSvc.RevokeCertificateWithActor flow now calls
OCSPResponseCacheService.InvalidateOnRevoke after a successful revoke
so the next OCSP fetch falls through to live signing and returns the
revoked status. Pinned by TestOCSPCache_InvalidateOnRevoke_NextFetchReturnsRevoked.
NEW migrations/000024_ocsp_response_cache.{up,down}.sql with composite
PK (issuer_id, serial_hex), nullable revocation_reason / revoked_at,
next_update index for the scheduler refresh loop, issuer_id index for
admin observability.
NEW internal/domain/ocsp_response_cache.go::OCSPResponseCacheEntry +
IsStale helper.
NEW internal/repository/postgres/ocsp_response_cache.go implementing
repository.OCSPResponseCacheRepository (Get / Put / Delete /
CountByIssuer). Interface defined in internal/repository/interfaces.go.
NEW internal/service/ocsp_response_cache.go::OCSPResponseCacheService
with read-through facade + sync.Map singleflight + InvalidateOnRevoke.
On cache miss, calls caOperationsSvc.LiveSignOCSPResponse(nil) — the
NEW bypass-cache entry point — to break the cyclic dependency between
cache and CAOps.
REFACTORED internal/service/ca_operations.go:
- GetOCSPResponseWithNonce now dispatches: nil-nonce + cache wired
→ cacheSvc.Get (cache); nonce != nil OR cache nil → live-sign.
- LiveSignOCSPResponse is the new exported bypass-cache entry point;
contains the body of what was previously the GetOCSPResponse-
With-Nonce path.
- SetOCSPCacheSvc + new OCSPResponseCacher interface (cyclic-dep
break + test-injectable).
The cache stores nil-nonce blobs by design. Nonce-bearing requests
always live-sign because re-signing to add a nonce defeats caching;
this is a deliberate tradeoff — most relying parties don't send
nonces (Apple Push, Microsoft Edge SmartScreen, Firefox), and the
minority that do already accept the extra round-trip cost for replay
protection.
WIRED in cmd/server/main.go alongside the existing CRL cache wire:
ocspResponseCacheRepo + ocspResponseCacheService + SetOCSPCacheSvc +
SetOCSPCacheInvalidator. Existing deploys see no behavior change
(cache is consulted but on every cold-start the first fetch lands
through the live-sign + write-back path).
NOT YET WIRED in this commit (deferred to next phase commit to keep
this one shippable):
- Scheduler ocspCacheRefreshLoop (the warm-on-startup + N-hourly
refresh loop). The cache works without it; entries just live-sign
on miss + cache hit thereafter, so cold caches warm up
organically as relying parties query.
- Admin observability endpoint /api/v1/admin/ocsp/cache.
- CERTCTL_OCSP_CACHE_REFRESH_INTERVAL env var.
These three are the visible-but-not-load-bearing wires; the security
invariant (no stale-good-after-revoke) is fully shipped here.
7 new tests in internal/service/ocsp_response_cache_test.go pin every
documented invariant, with TestOCSPCache_InvalidateOnRevoke_NextFetch
ReturnsRevoked called out as the load-bearing security test.
Pre-commit verification: go build ./... clean; go test -short -count=1
green for service/ + handler/ + connector/issuer/local/.
|
||
|
shankar0123
|
3d15a3e5af |
feat(ocsp): RFC 6960 §4.4.1 nonce extension support — echo client nonce in response, reject malformed
Production hardening II Phase 1.
The OCSP responder previously ignored the request's nonce extension
entirely, leaving relying parties vulnerable to replay attacks. RFC
6960 §4.4.1 defines the OPTIONAL id-pkix-ocsp-nonce extension (OID
1.3.6.1.5.5.7.48.1.2): when present in the request, the responder
MUST echo the same value in the response; when absent, no nonce in
the response (back-compat with relying parties that don't send one).
NEW internal/service/ocsp_nonce.go: ParseOCSPRequestNonce walks raw
DER (golang.org/x/crypto/ocsp.Request doesn't expose the request's
extensions field — the library only exposes IssuerNameHash +
IssuerKeyHash + SerialNumber). Returns one of three states:
- (nil, false, nil) — no nonce extension in request
- (nonce, true, nil) — well-formed nonce, ≤ MaxOCSPNonceLength (32)
- (nil, false, ErrOCSPNonceMalformed) — empty or oversized
NEW internal/service/ocsp_counters.go: sync/atomic counter table for
OCSP request lifecycle (request_get/post, request_success/invalid,
nonce_echoed, nonce_malformed, rate_limited, ...). Mirrors the EST/
SCEP counter pattern; Phase 8 wires these into /metrics/prometheus.
CertSrv types extended:
- internal/connector/issuer/interface.go::OCSPSignRequest gains
Nonce []byte field.
- internal/service/renewal.go::OCSPSignRequest (the service-layer
duplicate used by ca_operations.go) gains the same field.
- internal/service/issuer_adapter.go bridges the two.
Service path: CAOperationsSvc.GetOCSPResponseWithNonce(ctx, issuerID,
serialHex, nonce) is the new entry point that plumbs the nonce
through every signing site (good / revoked / unknown / short-lived).
The legacy GetOCSPResponse becomes a nil-nonce wrapper for back-
compat — every existing caller (tests, the GET handler) sees no
behavior change.
CertificateService gains the same WithNonce variant; the handler
interface adds it to the contract. MockCertificateService in tests
extended with the new method (delegates to the legacy fn when no
override is set, so existing tests that don't care about the nonce
keep working).
Local issuer's SignOCSPResponse appends the id-pkix-ocsp-nonce
extension (non-Critical per RFC 6960 §4.4) to the response template's
ExtraExtensions when req.Nonce != nil. The extnValue is the nonce
bytes wrapped in an OCTET STRING per RFC 6960 §4.4.1.
POST OCSP handler (HandleOCSPPost):
- After ocsp.ParseRequest succeeds, calls ParseOCSPRequestNonce on
the raw body to extract the optional nonce.
- On ErrOCSPNonceMalformed (empty or > 32 bytes): writes an
'unauthorized' OCSP response (status 6 per RFC 6960 §2.3) using
the canonical ocsp.UnauthorizedErrorResponse from x/crypto/ocsp.
Does NOT echo malicious bytes back.
- On well-formed nonce: passes it through GetOCSPResponseWithNonce.
- On no nonce: nil passed through; back-compat preserved.
GET OCSP handler unchanged — the GET form has no body to carry a
nonce extension.
6 new tests in internal/service/ocsp_nonce_test.go pin every
documented failure mode + the 32-byte boundary. The test fixture
builds an OCSPRequest via golang.org/x/crypto/ocsp.CreateRequest then
splices in a [2] EXPLICIT Extensions element by hand (the library
doesn't expose extension construction either).
Pre-commit verification: gofmt clean, go vet clean across affected
packages, go test -short -count=1 green for service/ + handler/ +
connector/issuer/local/. No new env vars introduced (Phase 1 is
always-on per RFC; no operator opt-out).
|
||
|
shankar0123
|
5834e5b866 |
fix(est): plumb context through ESTService.ReloadTrust to satisfy contextcheck
CI golangci-lint v2.11.4 flagged internal/api/handler/admin_est.go:178: the AdminESTServiceImpl.ReloadTrust method took ctx context.Context but called svc.ReloadTrust() with no context, then the underlying ESTService.ReloadTrust used context.Background() internally for the audit RecordEvent call. That's the contextcheck linter's textbook 'context discarded at boundary' violation. Fix: change ESTService.ReloadTrust signature to ReloadTrust(ctx context.Context) and forward the caller-supplied ctx into auditService.RecordEvent. AdminESTServiceImpl.ReloadTrust now passes its received ctx through. The HTTP handler already forwards r.Context() one layer up, so the request-scoped trace identifiers now flow end-to-end into the audit row instead of being severed at the service boundary. Verified locally with golangci-lint v2.11.4 (the same version CI runs) against ./internal/api/handler/... ./internal/service/... — '0 issues.' All cmd/* binaries build clean, go test -short -count=1 green for both packages. |
||
|
shankar0123
|
5a682db8e2 |
EST RFC 7030 hardening master bundle Phases 10-11: libest sidecar e2e
+ Cisco IOS quirk fixtures + ManagedCertificate.Source provenance + EST bulk-revoke endpoint + 13 typed audit action codes. Phase 10.1 — libest reference-client sidecar: - deploy/test/libest/Dockerfile: multi-stage Debian-bookworm-slim build of Cisco's libest v3.2.0-2 from source (autoconf/automake/ libtool + libcurl4-openssl-dev + libssl-dev). Runtime stage carries only estclient + bash + openssl + ca-certificates so the exec surface stays small + predictable. - docker-compose.test.yml libest-client entry (profiles: [est-e2e]) with bind mounts for /config/est (test workspace) + /config/certs (certctl CA bundle for TLS pinning); IP 10.30.50.9 (10.30.50.8 was already taken by certctl-agent). - deploy/test/est/.gitkeep keeps the bind-mount target tracked. Phase 10.2 — 5 integration tests (//go:build integration) in deploy/test/est_e2e_test.go: - TestEST_LibESTClient_Enrollment_Integration (cacerts → simpleenroll → cert-shape assertion) - TestEST_LibESTClient_MTLSEnrollment_Integration (mTLS sibling-route cert auth; skip when bootstrap cert absent) - TestEST_LibESTClient_ServerKeygen_Integration (RFC 7030 §4.4 multipart; skip when profile gate disabled) - TestEST_LibESTClient_RateLimited_Integration (4th enroll trips per-principal cap, asserts 429-shaped error) - TestEST_LibESTClient_ChannelBinding_Integration (libest --tls-exporter; skip when libest build lacks the flag). - requireESTSidecar guard skips the suite when the operator forgot --profile est-e2e; helpful error message includes the exact command to bring the sidecar up. Phase 10.3 — Cisco IOS quirk fixtures + 3 unit tests in internal/api/handler/cisco_ios_quirks_test.go: - testdata/cisco_ios_15x_pem_csr.txt: PEM body sent with Content-Type application/x-pem-file. Handler dispatches on body-prefix not Content-Type — accepts cleanly. - testdata/cisco_ios_16x_trailing_newline_csr.txt: extra trailing newlines after base64 body. strings.TrimSpace tolerates. - testdata/cisco_ios_crlf_b64_csr.txt: CRLF-wrapped base64. base64.StdEncoding handles CRLF + LF identically. Phase 11.1 — ManagedCertificate.Source provenance: - New domain.CertificateSource enum (Unspecified/EST/SCEP/API/Agent). - Migration 000023_managed_certificates_source.up.sql adds source TEXT NOT NULL DEFAULT '' so existing rows scan as CertificateSourceUnspecified — back-compat: bulk-revoke filter treats empty as "any source". - Postgres repo Insert/Update/scan paths all wire the new column. Phase 11.2 — EST bulk-revoke endpoint: - BulkRevocationCriteria.Source field (Source-only requests rejected as too broad — must accompany at least one narrower criterion). - service.bulk_revocation.resolveCertificates post-filter by Source (empty=any, no SQL change so existing CertificateFilter callers unaffected). - New BulkRevocationHandler.BulkRevokeEST method pins Source=EST + dispatches; new route POST /api/v1/est/certificates/bulk-revoke (M-008 admin-gated). openapi.yaml documented + parity-guard green. Phase 11.3 — 13 typed audit action codes in internal/service/est_audit_actions.go: - est_simple_enroll_success / _failed - est_simple_reenroll_success / _failed - est_server_keygen_success / _failed - est_auth_failed_basic / _mtls / _channel_binding - est_rate_limited - est_csr_policy_violation - est_bulk_revoke - est_trust_anchor_reloaded - ESTService.processEnrollment + SimpleServerKeygen + ReloadTrust split-emit BOTH the legacy bare action codes (back-compat for the GUI activity-tab chip filters that match by exact string + existing audit-log analysers) AND the new typed _success / _failed variants (operator grep target + per-failure-mode counter). Tests: - internal/api/handler/bulk_revocation_est_test.go — 5 cases (admin-true happy path pins Source=EST + non-admin 403 + empty-criteria 400 + invalid-reason 400 + method-not-allowed). - internal/service/est_audit_actions_test.go — 5 cases (SimpleEnroll legacy+typed emission / SimpleReEnroll typed / IssuerError typed-failed / PolicyViolation triple-emit / unique-string invariant). Pre-commit verification (sandbox): gofmt clean, go vet clean (excluding repository/postgres testcontainers limit), staticcheck clean across api/handler/api/router/domain/service/deploy/test, go test -short -count=1 green for every non-postgres Go package + integration build (`go build -tags integration ./deploy/test/...`) clean. G-3 docs-drift guard reproduced locally clean (Phases 10-11 added zero new env vars). Spec preserved at cowork/est-rfc7030-hardening-prompt.md. Phases 12-13 (docs/est.md + WiFi/802.1X / IoT bootstrap / FreeRADIUS recipes; release prep + tag) remain — post-2.1.0 work. |
||
|
shankar0123
|
43075a1b5c |
EST RFC 7030 hardening master bundle Phases 5-7: end-to-end serverkeygen
+ profile-driven csrattrs + admin observability with per-status counters + reload-trust endpoint. Phase 5 — RFC 7030 §4.4 server-driven key generation: - internal/pkcs7/envelopeddata_builder.go is the inverse of the existing parser/decryptor: AES-256-CBC content cipher + RSA PKCS#1 v1.5 keyTrans + per-call random IV. Round-trip pinned in test (BuildEnvelopedData → ParseEnvelopedData → Decrypt returns the original plaintext byte-for-byte). - ESTService.SimpleServerKeygen runs the full §4.4 flow: parse client CSR → require RSA pubkey for keyTrans → resolve per-profile algorithm (RSA-2048 default; honors AllowedKeyAlgorithms) → in- memory keygen → re-build CSR with server pubkey → run existing issuer pipeline → marshal PKCS#8 → CMS-EnvelopedData wrap to a synthetic recipient cert wrapping the device's CSR-supplied pubkey → zeroize plaintext + PKCS#8 bytes → return CertPEM + ChainPEM + EncryptedKey. Typed sentinels ErrServerKeygenRequiresKey- Encipherment / ErrServerKeygenUnsupportedAlgorithm / ErrServerKeygenDisabled. - ESTHandler.ServerKeygen + ServerKeygenMTLS emit RFC 7030 §4.4.2 multipart/mixed with random per-response boundary; per-profile SetServerKeygenEnabled gate returns 404 when off (defense in depth even if the route was registered). - New routes POST /.well-known/est/[<PathID>/]serverkeygen + /.well-known/est-mtls/<PathID>/serverkeygen; openapi.yaml + openapi-parity guard updated. Phase 6 — Real csrattrs implementation: - New CertificateProfile.RequiredCSRAttributes []string + migration 000022_certificate_profiles_csrattrs.up.sql. The migration also lands the previously-unwired must_staple column (closes the 5.6 follow-up loop where the field shipped at the domain + service layer but the postgres scan/insert/update never persisted it). - domain.EKUStringToOID + AttributeStringToOID lookup tables: id-kp-* EKUs (RFC 5280 §4.2.1.12) + RFC 5280 DN attributes + RFC 2985 PKCS#10 attributes + Microsoft Intune device-serial OID. - ESTService.GetCSRAttrs replaces the v2.0.x nil/204 stub with a profile-derived SEQUENCE OF OID ASN.1 marshal. Unknown EKU / attribute strings dropped + warning-logged so a typo doesn't take down the entire endpoint. Phase 7 — Admin observability + counters + reload-trust: - internal/service/est_counters.go: estCounterTab (sync/atomic; 12 named labels) + ESTStatsSnapshot per-profile shape + ESTService.Stats(now) zero-allocation accessor + ReloadTrust() SIGHUP-equivalent + SetESTAdminMetadata setter. - Counter ticks wired into processEnrollment + SimpleServerKeygen at every success/failure leg. - internal/api/handler/admin_est.go mirrors AdminSCEPIntune verbatim: Profiles + ReloadTrust handlers + AdminESTServiceImpl. Both endpoints admin-gated (M-008 triplet pinned + admin_est.go added to AdminGatedHandlers). - New routes GET /api/v1/admin/est/profiles + POST /api/v1/admin/ est/reload-trust; openapi.yaml documented; openapi-parity guard reproduced clean. - cmd/server/main.go grows estServices map populated by the per- profile EST loop + handed to AdminEST. New MTLSTrust() + HasMTLSTrust() accessors on ESTHandler so main.go can pull the trust holder for the admin-metadata wire-up. - Per-profile counter isolation regression test (internal/service/est_profile_counter_isolation_test.go) proves a future shared-counter refactor would fail at compile-time pointer-identity check. Pre-commit verification (sandbox): gofmt clean, go vet clean (excluding repository/postgres which the sandbox can't build — disk-space testcontainers download), staticcheck clean across cms/trustanchor/api/handler/api/router/scep/intune/ratelimit/ service/pkcs7/domain/cmd/server, go test -short -count=1 green for every non-postgres package. G-3 docs-drift guard reproduced locally clean (Phases 5-7 added zero new env vars; Phase 1 already documented per-profile SERVER_KEYGEN_ENABLED). Spec preserved at cowork/est-rfc7030-hardening-prompt.md. Phases 8-13 (GUI ESTAdminPage / CLI+MCP / libest e2e / bulk revocation / docs/est.md / release prep) remain — post-2.1.0 work. |
||
|
shankar0123
|
530593507b |
fix(scep-intune): close 11 audit gaps from 2026-04-29 pre-tag review
Closes the eleven gaps identified in the pre-v2.1.0 audit of the SCEP
RFC 8894 + Intune master bundle (cowork/scep-bundle-gap-closure-prompt.md).
Constitutional rule from cowork/CLAUDE.md::Operating Rules — 'Always
take the complete path, not the easy path' — drove this closure: each
gap was a load-bearing wire that crossed multiple layers (config →
validator → service wire-up → tests → docs) and shipping the bundle
without them would have produced lying-field footguns where operator-
visible config options stored values without affecting behavior.
WHAT LANDS:
Phase A — Clock-skew tolerance (master prompt §15 hazard closure)
internal/scep/intune/challenge.go: ValidateChallenge migrated from
positional args to ValidateOptions{} struct; new ClockSkewTolerance
field with default 0 (strict). 24 call sites updated mechanically.
Asymmetric application: now+tolerance >= iat AND now-tolerance < exp.
internal/config/config.go: SCEPIntuneProfileConfig.ClockSkewTolerance
default 60s + Validate() refusal when >= ChallengeValidity.
cmd/server/main.go: SetIntuneIntegration signature extended;
per-profile env-var loader honors CERTCTL_SCEP_PROFILE_<NAME>_INTUNE_CLOCK_SKEW_TOLERANCE.
internal/service/scep.go: intuneClockSkew field + IntuneStatsSnapshot
surfaces clock_skew_tolerance_ns. web/src/api/types.ts mirrors.
4 new tests in challenge_test.go covering accept-within-tolerance,
reject-beyond-tolerance, accept-expired-within-tolerance,
negative-treated-as-zero defensive normalization.
docs/scep-intune.md updated with the new env var + time-bounds rule.
Phase B — unknown-version-rejected golden test
internal/scep/intune/golden_helper_test.go: goldenUnknownVersionPayload
helper + signGoldenChallengeAny generic signer.
challenge_golden_test.go: TestGoldenChallenge_UnknownVersionRejected
uses an in-process ECDSA fixture (the on-disk PEM was generated with
a Go-stdlib version that produces different ecdsa.GenerateKey bytes
from the current call). TestRegenerateGoldenFixtures emits the new
unknown_version fixture file too.
Phase C — Two named Intune e2e tests
internal/api/handler/scep_intune_e2e_test.go:
TestSCEPIntuneEnrollment_RateLimited_E2E (cap=2 + 3 attempts; 3rd
returns FAILURE+badRequest with rate_limited counter ticked)
TestSCEPIntuneEnrollment_TrustAnchorSIGHUPReload_E2E (rotate
on-disk PEM + holder.Reload(); old-key challenge fails with
badMessageCheck; signature_invalid counter ticked)
intuneE2EFixture struct extended with trustHolder + trustPath fields
so tests can rotate.
Phase D — Four new ChromeOS hermetic tests (10 total now)
internal/api/handler/scep_chromeos_test.go:
_RAKeyMismatch — PKIMessage encrypted to wrong RA cert; handler
rejects without reaching service.
_3DESBackwardCompat — RFC 8894 §3.5.2 legacy fallback verified.
_RSACSR + _ECDSACSR — explicit matrix-pair pinning.
buildTestECDSACSR helper for ECDSA P-256 CSR construction;
tripleDESCBCEncrypt mirrors aesCBCEncrypt for 3DES-CBC;
assertChromeOSPositiveCertRep shared assertion.
Phase E — Per-profile counter isolation test
internal/api/handler/scep_profile_counter_isolation_test.go:
TestSCEPHandler_PerProfileIntuneCountersIsolated wires two
SCEPService instances + drives distinct PKIMessages + asserts
counter isolation. Guards against a future cmd/server/main.go
refactor that shares a *intuneCounterTab across profiles.
buildPerProfileIntuneFixture parameterized helper.
Phase F — Server-boot regression tests
cmd/server/preflight_scep_intune_test.go: 3 named tests covering
disabled-backward-compat, broken-config-with-PathID, expired-cert
refusal. preflightSCEPIntuneTrustAnchor signature extended with
pathID arg so error messages carry PathID= for operator log-grep.
Phase G — docs/connectors.md
Four new subsections under §EST/SCEP Integration: multi-profile
dispatch + mTLS sibling route + Intune Connector dispatcher + SCEP
probe in network scanner. Each has a one-paragraph operator
explanation + an env-var or endpoint table.
Phase H — Coverage uplift
internal/service/scep_probe_persist_test.go: 5 unit tests on
persistProbeResult (nil-safe + nil-repo-safe + repo-error swallow +
nil-logger guard) + ListRecentSCEPProbes (empty-slice-not-nil + repo
pass-through) + describeCertAlgorithm (RSA/ECDSA/QF1008-nil-curve
defensive branch/Ed25519/DSA/empty). CI gates (service ≥70, handler
≥75) PASS at 70.9% / 79.3%.
Phase I — deploy/test integration variant
deploy/test/scep_intune_e2e_test.go (//go:build integration):
TestSCEPIntuneEnrollment_Integration + _RateLimited_Integration
against the live docker-compose certctl container. Skip-when-
stack-missing semantics so sandbox + CI both work.
deploy/docker-compose.test.yml: new e2eintune SCEP profile env
vars + bind-mount of deploy/test/fixtures/.
deploy/test/fixtures/README.md: documents the deterministic trust
anchor regeneration recipe.
VERIFICATION (sandbox):
gofmt -d — clean for all changed files
staticcheck — clean for intune + handler + config + service +
cmd/server packages
go vet — clean for the same packages
go test -short — green for intune (95.3% cov), service (70.9%),
handler (79.3%), config (94.0%), cmd/server (boot
path; my preflight tests cover the directly-
testable function), pkcs7 (80.5% informational)
DEFERRED (per closure prompt §7 out-of-scope):
- V3-Pro Conditional Access gating + Microsoft Graph integration
- Standalone certctl-scan CLI binary
- OCSP rate-limiting, OCSP stapling, delta CRLs
Spec preserved at cowork/scep-bundle-gap-closure-prompt.md;
journal at cowork/scep-rfc8894-intune/progress.md (audit-closure
section appended).
|
||
|
shankar0123
|
84fac19f98 |
fix(scep-probe): satisfy staticcheck QF1008 in describeCertAlgorithm
CI flagged QF1008 on the chained selector pub.Curve.Params() — the linter wants the promoted-method form pub.Params() (Curve is embedded in ecdsa.PublicKey, so Params is reachable via promotion). Restructure the nil check so the embedded interface still gets validated before the promoted call, then invoke pub.Params() once and reuse the result. Verification: * gofmt clean * staticcheck on internal/service/...: clean * 6/6 TestProbeSCEP_* tests still pass |
||
|
shankar0123
|
506cff137d |
feat(scep): SCEP probe in network scanner for fleet-readiness assessment
Phase 11.5 of the SCEP RFC 8894 + Intune master bundle. Adds an
operator-facing SCEP probe that issues GetCACaps + GetCACert against
an arbitrary SCEP server URL and returns a structured posture snapshot
(reachable + advertised caps + RFC 8894 / AES / POST / Renewal /
SHA-256 / SHA-512 support flags + CA cert subject + issuer + NotBefore
+ NotAfter + days-to-expiry + algorithm + chain length).
Two operator use cases per the master prompt:
1. Pre-migration assessment — probe an existing EJBCA / NDES SCEP
server before switching to certctl to see what capabilities it
advertises and what the CA cert looks like.
2. Compliance posture audits — periodic ad-hoc probes against the
operator's own SCEP servers to flag drift.
Capability-only — does NOT POST a CSR per the spec (would consume slot
allocations on the target server + create audit noise). Standalone CLI
binary explicitly out of scope (per the master prompt §11.5.6 and the
operator's confirmation): the probe code lands inside certctl; a
future thin Cobra wrapper is a separate decision.
Backend (six new + one extended file):
* internal/domain/network_scan.go — new SCEPProbeResult struct with
every probe field documented for the GUI's display layer.
* migrations/000021_scep_probe_results.up.sql + .down.sql — new
scep_probe_results table with TEXT id, target_url, all probe
flags, CA cert metadata, probed_at, probe_duration_ms, error.
Two indexes: idx_scep_probe_results_probed_at (DESC) for the
'recent probes' GUI query, idx_scep_probe_results_target_url
(target_url, probed_at DESC) for the future per-URL history view.
* internal/repository/interfaces.go — new SCEPProbeResultRepository
interface (Insert + ListRecent).
* internal/repository/postgres/scep_probe_results.go — Postgres
implementation. ListRecent clamps limit to [1, 200]; on read
re-derives ca_cert_days_to_expiry against the query-time wall
clock so 'X days remaining' stays fresh.
* internal/service/scep_probe.go — ProbeSCEP(ctx, url) on
NetworkScanService. Validation order:
1. Up-front URL validation via validation.ValidateSafeURL
(defaults to validation.ValidateSafeURL but injectable for
tests via the new scepValidateURL field on the service).
2. Dial-time SSRF re-check via SafeHTTPDialContext on the
http.Transport (defends against DNS rebinding).
3. GET ?operation=GetCACaps + GET ?operation=GetCACert.
GetCACert handles three response shapes: PKCS#7 SignedData
certs-only envelope (multi-cert), raw DER (single-cert),
and PEM-wrapped DER (non-conforming servers).
Times out at 30s; uses a 1MB body cap for DoS defense; wraps
the result + persists via the repo (nil-safe) before returning.
describeCertAlgorithm helper returns 'RSA-N' / 'ECDSA-curve' /
'Ed25519' / 'DSA' for the GUI's algorithm column.
* internal/service/network_scan.go — added scepProbeRepo +
scepHTTPClient + scepValidateURL + scepIDFn + nowFn fields;
SetSCEPProbeRepo wires the repo at startup.
* internal/api/handler/network_scan.go — extended NetworkScanService
interface with ProbeSCEP + ListRecentSCEPProbes; added two new
HTTP handlers:
POST /api/v1/network-scan/scep-probe (body {url})
GET /api/v1/network-scan/scep-probes (recent history)
Synchronous probe; HTTP 200 with the result body for both success
and reachable-but-failed cases (so the GUI can render the failure
tone with the operator-actionable error message).
* internal/api/router/router.go — registered the two routes inline
after the existing network-scan target endpoints.
* api/openapi.yaml — documented both endpoints (operationId
probeSCEP + listSCEPProbes) with full schema + response codes.
* cmd/server/main.go — wires the new SCEPProbeResultRepository
onto the network scan service via SetSCEPProbeRepo right after
the existing NewNetworkScanService construction.
Backend tests (6 new — exit-criteria-named per the master prompt):
* TestProbeSCEP_AdvertisesAllCaps — happy path, full RFC 8894
capability set, ECDSA P-256 CA cert, 365-day expiry.
* TestProbeSCEP_MissingSCEPStandard — pre-RFC-8894 server (only
POSTPKIOperation + SHA-1 + DES3); SupportsRFC8894 = false.
* TestProbeSCEP_GetCACertExpired — CA cert NotAfter 30d in the
past; CACertExpired = true.
* TestProbeSCEP_Unreachable — connect to TCP port 1; probe
returns Reachable=false + non-empty Error.
* TestProbeSCEP_RejectsReservedIP — http://169.254.169.254/scep
(EC2 metadata literal) rejected by the up-front
validation.ValidateSafeURL gate; result captures the error
without ever issuing the HTTP call.
* TestProbeSCEP_PEMWrappedCert — server returns PEM instead of
raw DER for GetCACert; the fallback parse path handles it.
Frontend (one extended file + types/client):
* web/src/api/types.ts — SCEPProbeResult + SCEPProbesResponse.
* web/src/api/client.ts — probeSCEPServer + listSCEPProbes
helpers.
* web/src/pages/NetworkScanPage.tsx — new SCEPProbeSection
component + ProbeResultPanel (with capability badges + CA cert
details panel + raw caps line) + SCEPProbeHistoryTable. Form
rejects empty URL with inline error before calling the API.
Reload mutation goes through useTrackedMutation with explicit
invalidates: [['scep-probes']] (M-009 contract).
Frontend tests (5 new + 0 regressions):
* Scep probe section header + form renders.
* Empty URL is rejected with inline error and never calls the
probe endpoint.
* Successful probe renders capability badges + CA cert subject
+ days-remaining inline panel.
* Probe-level errors are surfaced in the inline panel (no result
panel rendered).
* Recent-probes history table renders one row per probe.
* (Existing 2 NetworkScanPage XSS-hardening tests stub the new
listSCEPProbes endpoint to an empty list so they still pass.)
Verification:
* gofmt clean on touched files
* go vet ./... clean
* staticcheck on service+handler+router+repository+cmd-server clean
* go test -short across service+handler+router+repository+cmd-server
+ integration: all green (existing + 6 new probe tests pass)
* Frontend tsc --noEmit clean
* Vitest: 7/7 NetworkScanPage tests pass (2 existing XSS + 5 new
probe section)
* G-3 docs-drift CI guard reproduced locally clean (no new env vars)
* M-009 hard-zero useMutation guard clean (probe mutation goes
through useTrackedMutation)
* openapi-parity guard satisfied (both new routes documented)
* The mockNetworkScanService in handler + integration packages
extended with stub Probe methods; targeted coverage stays in
scep_probe_test.go.
Out of scope (per master prompt §11.5.6 + operator confirmation):
* Standalone certctl-scan CLI binary — separate decision, ~1d of
follow-up work when/if shipped.
Refs: cowork/scep-rfc8894-intune-master-prompt.md::Phase 11.5
cowork/scep-rfc8894-intune/progress.md
|
||
|
shankar0123
|
0be889ff1d |
refactor(scep-gui): rebrand SCEP admin surface to per-profile tabbed interface (Profiles + Intune + Recent Activity)
Phase 9 follow-up to the SCEP RFC 8894 + Intune master bundle. The
Phase 9.4 GUI shipped 'SCEP Intune Monitoring' at /scep/intune, which
made the per-profile observability surface look Intune-only — operators
running EJBCA + Jamf would never click that nav link expecting per-
profile RA cert + mTLS observability. The page is per-profile keyed
under the hood; this commit rebrands + restructures so the surface
matches what operators actually need.
Spec: cowork/scep-gui-restructure-prompt.md.
User-visible change:
- Nav link renamed: 'SCEP Intune' → 'SCEP Admin'.
- Route: /scep is the new canonical path; /scep/intune kept as a
backward-compat alias that lands directly on the Intune tab.
- Page header: 'SCEP Administration'.
- Three tabs:
* Profiles (default) — per-profile lean cards with RA cert
expiry countdown, mTLS sibling-route status badge, Intune
enabled/disabled badge, challenge-password-set indicator.
'View Intune details →' link on Intune-enabled cards
deep-links into the Intune tab.
* Intune Monitoring — the existing Phase 9.4 deep-dive
(per-status counters, trust anchor expiry, recent failures
table, reload-trust button + confirmation modal).
* Recent Activity — full SCEP audit log filter merging all
four action codes (scep_pkcsreq + scep_renewalreq +
scep_pkcsreq_intune + scep_renewalreq_intune); chip filters
for All / Initial / Renewal / Intune / Static.
Backend:
* internal/service/scep.go — new SCEPProfileStatsSnapshot type +
IntuneSection sub-block + ProfileStats(now) accessor. Adds
raCertSubject/raCertNotBefore/raCertNotAfter + mtlsEnabled +
mtlsTrustBundlePath fields with SetRACert + SetMTLSConfig setters.
Existing IntuneStatsSnapshot + IntuneStats(now) preserved
UNCHANGED for /admin/scep/intune/stats backward compat (the
JSON shape stays byte-stable for external consumers — the
aliasing approach the prompt initially suggested doesn't work
because the new shape nests Intune while the old one is flat).
ChallengePasswordSet is derived from challengePassword != ''
(the secret value itself is never surfaced).
* internal/api/handler/admin_scep_intune.go — new Profiles handler
method on AdminSCEPIntuneHandler with the same M-008 admin gate.
AdminSCEPIntuneServiceImpl extended (in place; same
map[string]*service.SCEPService) to satisfy the new
AdminSCEPProfileService interface. Single handler file gets the
third method so the M-008 pin entry count stays steady (no new
file, no new triplet of admin-gate test files — just three new
Profiles tests inside the existing test file).
* internal/api/router/router.go — one new route
'GET /api/v1/admin/scep/profiles' registered to
reg.AdminSCEPIntune.Profiles. HandlerRegistry unchanged.
* api/openapi.yaml — new operation 'listSCEPProfiles' documenting
the request body / response shape / error mapping. Existing
Intune entries unchanged.
* cmd/server/main.go — per-profile loop now calls
scepService.SetMTLSConfig(profile.MTLSEnabled,
profile.MTLSClientCATrustBundlePath) right after SetPathID, and
scepService.SetRACert(raCert) right after loadSCEPRAPair returns
the leaf cert. Both setters are nil-safe.
* internal/api/handler/m008_admin_gate_test.go — extended the
existing admin_scep_intune.go entry's justification to mention
the third endpoint. No new map entry needed (file already
listed).
Backend tests (8 new):
* TestAdminSCEPProfiles_NonAdmin_Returns403
* TestAdminSCEPProfiles_AdminExplicitFalse_Returns403
* TestAdminSCEPProfiles_AdminPermitted_ForwardsActor — also pins
that Intune-enabled profiles emit an 'intune' sub-block while
Intune-disabled profiles OMIT it.
* TestAdminSCEPProfiles_RejectsNonGetMethod
* TestAdminSCEPProfiles_PropagatesServiceError
* TestAdminSCEPProfilesServiceImpl_NilMapReturnsEmpty
* (existing 16 Phase 9 admin tests still pass — backward-compat
preserved)
Frontend:
* web/src/api/types.ts — new SCEPProfileStatsSnapshot +
IntuneSection + SCEPProfilesResponse types. Existing
IntuneStatsSnapshot et al unchanged.
* web/src/api/client.ts — new getAdminSCEPProfiles helper.
* web/src/pages/SCEPAdminPage.tsx — full rewrite as the tabbed
surface. Reuses the existing ConfirmReloadModal and Intune
deep-dive card components verbatim; adds ProfileSummaryCard
(lean card for the Profiles tab) and ActivityTab. URL state
sync via useSearchParams so deep links survive reloads + browser
back/forward. The legacy /scep/intune route alias defaults the
activeTab to 'intune' on mount.
* web/src/main.tsx — new <Route path='scep' /> + preserved
<Route path='scep/intune' /> alias. Both render SCEPAdminPage.
* web/src/components/Layout.tsx — nav link rebranded:
label 'SCEP Intune' → 'SCEP Admin', to '/scep/intune' → '/scep'.
Frontend tests (20 — full rebuild):
* Admin gate (non-admin sees gated banner + zero admin API calls)
* Profiles tab default + Intune tab tabswitch + ?tab=intune deep
link + legacy /scep/intune alias all land on Intune
* Profiles tab status badges (Intune + mTLS + challenge-set)
reflect each profile's flags
* RA cert expiry tone bands (good ≥30d / warn 7-30d / bad <7d /
EXPIRED) verified across three fixture profiles
* 'View Intune details →' only renders for Intune-enabled
profiles AND switches tabs on click
* Empty-state banner when no profiles configured
* Intune tab counters render with the existing Phase 9 deep-dive
shape; reload modal Open/Confirm/Cancel/Error paths all pinned
* Recent Activity tab merges all four SCEP audit actions across
four parallel useQuery calls; filter chips
(all/initial/renewal/intune/static) narrow correctly
* Error path surfaces ErrorState on the active tab
Docs:
* docs/scep-intune.md — Operational monitoring section heading
expanded to '(SCEP Administration → Intune Monitoring tab)'.
Page-surface description rewritten for the tabbed shape;
admin-endpoints list extended with the new /admin/scep/profiles
entry.
* docs/architecture.md — Microsoft Intune Connector trust anchor
subsection updated to reference the Intune Monitoring tab inside
the SCEP Administration page + lists all three admin endpoints.
* docs/legacy-est-scep.md — forward-ref expanded with a parallel
sentence for the per-profile observability surface (independent
of Intune).
* README.md — Enrollment Protocols bullet for Intune updated to
'admin GUI SCEP Administration page at /scep' with the three
tabs called out.
Verification:
* gofmt clean on touched files
* go vet ./... clean
* staticcheck on intune+service+handler+router+cmd-server clean
* go test -short across intune+service+handler+router+cmd-server:
all green (existing Phase 9 tests + new Profiles tests)
* Frontend tsc --noEmit clean
* Vitest: 20/20 SCEPAdminPage tests + 3/3 sibling AuditPage tests
pass
* G-3 docs-drift CI guard reproduced locally: clean (no new env
vars; existing CERTCTL_SCEP_ allowlist prefix covers everything)
* M-009 hard-zero useMutation guard reproduced locally: clean
(the existing reload mutation already used useTrackedMutation
from the Phase 9 follow-up commit
|
||
|
shankar0123
|
77e0281a0e |
feat(scep-intune): GUI monitoring tab + admin endpoints
Phase 9 of the SCEP RFC 8894 + Intune master bundle. Lands the operator-
facing Intune Monitoring tab plus the two admin-gated endpoints it reads
from. Per the constitutional 'complete path' rule: counters tick on
every typed dispatcher branch, the GUI poll is live (30s for stats,
60s for the audit log filter), and the SIGHUP-equivalent reload action
is one click + a confirmation modal — no follow-up plumbing required.
Backend (Phase 9.1 + 9.2 + 9.3):
* internal/service/scep.go gains:
- intuneCounterTab — atomic per-status counters keyed by the same
labels intuneFailReason() emits (success / signature_invalid /
expired / not_yet_valid / wrong_audience / replay / rate_limited /
claim_mismatch / compliance_failed / malformed / unknown_version).
Lock-free on the dispatcher hot path; snapshot() returns a
zero-allocation map for the admin endpoint.
- dispatchIntuneChallenge wires intuneCounters.inc(...) on every
typed return path INCLUDING the success leg (credited before
processEnrollment so a downstream issuer-connector failure
doesn't double-count).
- SetPathID + PathID accessors (so admin rows surface the SCEP
profile path ID per row).
- IntuneStatsSnapshot + IntuneTrustAnchorInfo public types, plus
IntuneStats(now) accessor that walks the trust holder pool and
packages a per-profile snapshot. ReloadIntuneTrust() is the
typed wrapper around TrustAnchorHolder.Reload that returns
ErrSCEPProfileIntuneDisabled when called on a profile where
Intune isn't enabled (admin endpoint maps that to HTTP 409).
* internal/api/handler/admin_scep_intune.go:
- AdminSCEPIntuneService narrow interface (Stats + ReloadTrust)
so the handler depends on a small surface; AdminSCEPIntuneServiceImpl
is the production walker over the per-profile SCEPService map.
- AdminSCEPIntuneHandler.Stats handles GET /api/v1/admin/scep/intune/stats
with the M-008 admin gate (non-admin → 403 + service never
invoked); returns {profiles, profile_count, generated_at}.
- AdminSCEPIntuneHandler.ReloadTrust handles POST
/api/v1/admin/scep/intune/reload-trust. Body is {path_id: '<id>'};
empty body targets the legacy /scep root profile. Returns 200 on
success / 404 on unknown PathID / 409 when the profile is Intune-
disabled / 500 on a parse error from intune.LoadTrustAnchor (the
holder retains its previous pool — fail-safe). 400 on malformed
JSON.
- ErrAdminSCEPProfileNotFound typed error so the handler can
distinguish 'wrong profile' from 'broken file'.
* internal/api/router/router.go: HandlerRegistry gains
AdminSCEPIntune; both routes registered as bearer-auth-required
(the admin-gate is at the handler layer per the M-008 pattern).
* cmd/server/main.go: declares scepServices map[string]*service.SCEPService
BEFORE HandlerRegistry construction so the same map can be referenced
from both the admin handler (constructed early) and the SCEP startup
loop (which populates it later by reference). The per-profile loop
now calls scepService.SetPathID(profile.PathID) and stores the service
pointer into the shared map. AdminSCEPIntune handler is constructed
at the same time as AdminCRLCache.
* internal/api/handler/m008_admin_gate_test.go: AdminGatedHandlers
map gains 'admin_scep_intune.go' with a one-line justification —
the regression scanner enforces the per-handler test triplet
(TestAdminSCEPIntune_NonAdmin_Returns403 + _AdminExplicitFalse_Returns403
+ _AdminPermitted_ForwardsActor) plus their POST siblings for
ReloadTrust.
* api/openapi.yaml: documents both endpoints with request body /
response shape / error mapping; openapi-parity-test now matches
the registered routes.
Frontend (Phase 9.4):
* web/src/pages/SCEPAdminPage.tsx — single-page Intune Monitoring
surface:
- Per-profile cards (one card per SCEP profile). Enabled profiles
get the full counter grid + trust-anchor-expiry badge tone
(good ≥30d / warn 7-30d / bad <7d / EXPIRED). Disabled profiles
get an off-state pill with the env-var hint to opt in.
- Counters polled every 30s via TanStack Query against
GET /admin/scep/intune/stats.
- Recent failures table (last 50) populated from the audit log
filtered to action=scep_pkcsreq_intune AND scep_renewalreq_intune;
merged + sorted by timestamp descending. Polled every 60s.
- Reload trust anchor button per profile + confirmation modal that
explains the SIGHUP equivalence and the fail-safe behavior.
onConfirm runs a TanStack mutation, refetches the stats query
on success, surfaces the underlying error (eg 'trust anchor
cert expired') in the modal on failure (modal stays open so
operator can retry).
- Admin gate: when authRequired && !admin the page renders an
'Admin access required' banner and the underlying admin API
requests are never issued (React Query enabled flag gated on
auth.admin) — server-side enforcement is M-008.
* web/src/api/types.ts: IntuneStatsSnapshot + IntuneTrustAnchorInfo +
IntuneStatsResponse + IntuneReloadTrustResponse.
* web/src/api/client.ts: getAdminSCEPIntuneStats +
reloadAdminSCEPIntuneTrust(pathID).
* web/src/main.tsx: new route /scep/intune. The route is unconditional;
the gating is at the page level so deep-links land cleanly.
* web/src/components/Layout.tsx: 'SCEP Intune' nav link between
Observability and Audit Trail with the appropriate sidebar icon.
Tests (Phase 9.5):
* internal/api/handler/admin_scep_intune_test.go (16 tests):
- M-008 admin-gate triplet for both Stats (GET) and ReloadTrust
(POST): NonAdmin / AdminExplicitFalse / AdminPermitted.
- Method-gate tests (Stats rejects POST, ReloadTrust rejects GET).
- Stats propagates service errors as 500.
- ReloadTrust maps ErrAdminSCEPProfileNotFound→404,
ErrSCEPProfileIntuneDisabled→409, generic err→500.
- Empty body targets legacy root PathID.
- Malformed JSON→400.
- AdminSCEPIntuneServiceImpl handles nil map + unknown PathID.
* web/src/pages/SCEPAdminPage.test.tsx (13 tests):
- Admin gate (non-admin sees gated banner + zero admin API calls;
admin sees the page; no-auth dev mode also passes).
- Profile rendering (counters with correct labels, expiry badge
tone for ≥30d / EXPIRED states, off-state pill for disabled
profiles, empty-state banner when no profiles configured).
- Reload modal (opens on click, calls mutation on Confirm,
keeps modal open + shows error on failure, Cancel skips
mutation).
- Error path renders ErrorState with retry.
- Audit log filter merges PKCSReq + RenewalReq events and sorts
descending.
Verification:
* gofmt clean on touched files
* go vet ./... clean
* staticcheck on intune/service/api/cmd-server clean
* go test -short across api+service+intune+cmd-server: all green
* web tsc --noEmit clean
* Vitest: SCEPAdminPage.test.tsx 13/13 + sibling page suites all
pass
* G-3 docs-drift CI guard: Phase 9 adds no new CERTCTL_* env vars
so the guard does not fire
* openapi-parity-test green (both new admin endpoints documented)
* M-008 regression scanner enforces the per-handler test triplet —
pin updated, all triplets present
Refs: cowork/scep-rfc8894-intune-master-prompt.md::Phase 9
cowork/scep-rfc8894-intune/progress.md
|
||
|
shankar0123
|
7612da783a |
feat(scep-intune): per-profile dispatcher + SIGHUP reload + per-device rate limit + compliance hook seam
Phase 8 of the SCEP RFC 8894 + Intune master bundle. Wires the internal/scep/intune validator from Phase 7 into the SCEPService dispatch path, with a SIGHUP-reloadable trust anchor holder, a per-(Subject, Issuer) sliding-window rate limiter, and a nil-default ComplianceCheck seam for V3-Pro. Operator-visible surface (per-profile, all default to off): CERTCTL_SCEP_PROFILE_<NAME>_INTUNE_ENABLED=true CERTCTL_SCEP_PROFILE_<NAME>_INTUNE_CONNECTOR_CERT_PATH=/etc/certctl/intune.pem CERTCTL_SCEP_PROFILE_<NAME>_INTUNE_AUDIENCE=https://certctl.example.com/scep/corp CERTCTL_SCEP_PROFILE_<NAME>_INTUNE_CHALLENGE_VALIDITY=60m CERTCTL_SCEP_PROFILE_<NAME>_INTUNE_PER_DEVICE_RATE_LIMIT_24H=3 Per-profile dispatch (Phase 8.8): an operator running corp-laptops through Intune AND IoT devices through static challenge configures INTUNE_ENABLED=true on the corp profile only — the IoT profile's PKCSReq path skips the dispatcher entirely. Mirrors the per-profile shape established by Phase 1.5. Wire-in surfaces: * config.go (Phase 8.1): SCEPProfileConfig.Intune sub-config of type SCEPIntuneProfileConfig (Enabled/ConnectorCertPath/Audience/ ChallengeValidity/PerDeviceRateLimit24h). Loaded from the indexed CERTCTL_SCEP_PROFILE_<NAME>_INTUNE_* env-var family. Per-profile Validate gate refuses INTUNE_ENABLED=true with empty ConnectorCertPath OR negative PerDeviceRateLimit24h. * cmd/server/main.go (Phase 8.2 + wire-in): preflightSCEPIntuneTrustAnchor helper mirrors preflightSCEPRACertKey/preflightSCEPMTLSTrustBundle shape — fail-loud at boot when the trust anchor file is missing / unreadable / empty / contains an expired cert. The per-profile loop builds the holder + replay cache + rate limiter, calls SetIntuneIntegration on the SCEPService, and starts the SIGHUP watcher. A deferred sweep stops every watcher at shutdown. * internal/scep/intune/trust_anchor_holder.go (Phase 8.5): TrustAnchorHolder mirrors cmd/server/tls.go::certHolder. RWMutex- guarded pool + Reload that swaps a fresh slice on success + WatchSIGHUP goroutine that responds to the same SIGHUP the existing TLS-cert watcher uses. A bad reload (parse error, expired cert) keeps the OLD pool in place so a half-rotation doesn't take Intune enrollment down — same fail-safe pattern. Operators rotate via the on-disk file then 'kill -HUP <certctl-pid>'. * internal/scep/intune/rate_limit.go (Phase 8.6): hand-rolled sliding-window-log limiter keyed by (Subject, Issuer). 100k-entry map cap (matches replay cache); at-cap drops the bucket whose newest timestamp is the oldest. Default 3 enrollments per 24h covers legitimate first-cert + recovery + post-wipe re-enrollment but blocks bulk enumeration from a compromised Connector signing key. maxN <= 0 disables the limiter for tests + the rare operator who wants no per-device cap. Empty subject short-circuits to allow (defense-in-depth: caller's claim validation rejects empty-subject upstream; no shared bucket on ''). Why hand-rolled instead of golang.org/x/time/rate: the rate package is in go.sum as an indirect transitive but not a direct dep. ~30 LoC of stdlib avoids creating a new direct dep. * internal/service/scep.go (Phase 8.3 + 8.4 + 8.7): - SCEPService gains intuneEnabled / intuneTrust / intuneAudience / intuneValidity / intuneReplayCache / intuneRateLimiter / complianceCheck fields. - SetIntuneIntegration() constructor-time injection wires the per-profile state. Profiles with INTUNE_ENABLED=false never call this method, so they pay zero overhead. - SetComplianceCheck() installs the V3-Pro plug-in (see Phase 8.7). - looksIntuneShaped(): JWT-shape pre-check (length > 200 + exactly two dots). Allowed to false-positive (validator catches malformed → ErrChallengeMalformed); MUST NOT false-negative on real Intune challenges. - dispatchIntuneChallenge(): the load-bearing core. Runs ValidateChallenge → CSR-binding via DeviceMatchesCSR → replay cache CheckAndInsert → per-device Allow → optional ComplianceCheck. Each failure leg increments a typed metric label and emits an audit-friendly Warn log line. - PKCSReq + PKCSReqWithEnvelope + RenewalReqWithEnvelope all call dispatchIntuneChallenge first; on outcome.decided=true they either short-circuit (with a typed-error → SCEPFailInfo mapping) or call processEnrollment with action='scep_pkcsreq_intune' (so audit greps can count Intune-vs-static enrollments). - mapIntuneErrorToFailInfo(): typed-error → SCEPFailInfo per RFC 8894 §3.2.1.4.5 (signature/replay/expired → BadMessageCheck; claim-mismatch → BadRequest; default → BadRequest). - intuneFailReason(): typed-error → metric label ('signature_invalid' / 'expired' / 'rate_limited' / etc.). Default 'malformed' so a previously-unseen error category still surfaces in the metric for follow-up. - ComplianceCheck (Phase 8.7): nil-default no-op gate. V3-Pro plugs in via SetComplianceCheck to call Microsoft Graph's compliance API. Returns (compliant, reason, err). nil-err + compliant=false → CertRep FAILURE + 'compliance' reason in audit. err != nil → fail-safe deny (V3-Pro module is responsible for any 'permit on API failure' policy). * internal/service/scep.go also gains parseCSRForIntune() — small private wrapper around encoding/pem + x509 used by the dispatcher for the claim ↔ CSR binding check (separated from the broader processEnrollment because we want to bind BEFORE consuming the replay-cache slot). Tests (gates: ≥85% coverage on intune package, ≥70% on service): * scep_intune_test.go (in internal/service): 14 dispatcher tests covering happy-path Intune enrollment + static-challenge fallback + tampered-challenge reject + claim-mismatch reject + replay detected + rate-limited + compliance-hook nil-default + compliance- hook denies non-compliant + compliance-hook error fails closed + IntuneEnabled accessor + 'no IntuneEnabled = static path unchanged' regression pin + intuneFailReason mapping for every typed error + looksIntuneShaped boundary cases. * trust_anchor_holder_test.go (in internal/scep/intune): NewLoadsBundle, NewRequiresLogger, NewSurfacesLoadError, ReloadHappyPath, ReloadKeepsOldOnFailure, ReloadKeepsOldOnExpired (the fail-safe semantics that make the SIGHUP path operator-friendly), WatchSIGHUPReloadsPool (real SIGHUP to self with poll-for-swap pattern mirroring cmd/server/tls_test.go), WatchSIGHUPStopIsClean (does NOT fire SIGHUP after stop — same caveat as the TLS test: the Go runtime would otherwise terminate the test runner on the next SIGHUP since signal.Stop has removed the handler). * rate_limit_test.go (in internal/scep/intune): AllowsUpToCap, DistinctKeysIndependent, WindowExpiry, DisabledBypass (maxN=0), NegativeCapDisabled, EmptySubjectShortCircuits (defense-in-depth against an empty-subject DoS chokepoint), DefaultCapsHonored, MapCapEvictsOldest (at-cap eviction branch), ConcurrentRaceFree (50 goroutines × 200 inserts), pruneOlderThan + the no-op case. Verification: * gofmt -l on all touched files: clean * go vet ./... : clean * staticcheck on intune/service/config/cmd-server: clean * go test -count=1 -cover ./internal/scep/intune/...: 94.8% (target ≥85%) * go test -short across intune+service+config+handler+cmd-server: all green * G-3 docs-drift CI guard reproduced locally: docs-only filtered= empty, config-only=empty. The new env vars match the existing CERTCTL_SCEP_ allowlist prefix. Refs: cowork/scep-rfc8894-intune-master-prompt.md::Phase 8 cowork/scep-rfc8894-intune/progress.md Constitutional rule: 'Always take the complete path, not the easy path' (cowork/CLAUDE.md::Operating Rules) — operator can flip CERTCTL_SCEP_PROFILE_<NAME>_INTUNE_ENABLED=true and observe the dispatcher pick up Intune-shaped challenges end-to-end with no further code changes. Foundation + plumbing ship together. |
||
|
shankar0123
|
01f6eb9d09 |
feat(scep): plumb CertificateProfile.MustStaple end-to-end through service layer
SCEP RFC 8894 + Intune master bundle Phase 5.6 follow-up.
Closes the 'lying field' gap from the original Phase 5.6 commit (
|
||
|
shankar0123
|
b33b843908 |
feat(scep): RenewalReq + GetCertInitial + ChromeOS E2E + caps + must-staple
SCEP RFC 8894 + Intune master bundle — Phase 4 + Phase 5 of 14.
Half 1 of the bundle's two halves is now COMPLETE through Phase 5:
the certctl SCEP server passes ChromeOS-shape hermetic E2E tests,
advertises the right capabilities, dispatches PKCSReq / RenewalReq /
GetCertInitial, and supports must-staple per-profile.
== Phase 4: RenewalReq + GetCertInitial wiring ============================
internal/service/scep.go
* RenewalReqWithEnvelope (RFC 8894 §3.3.1.2) — re-enrollment with an
existing valid cert. Same contract as PKCSReqWithEnvelope but the
service additionally verifies that envelope.SignerCert chains to
the issuer's CA (verifyRenewalSignerCertChain). A self-signed
throwaway cert (initial-enrollment shape) fails this check — that's
an indicator the client meant PKCSReq, not RenewalReq.
* GetCertInitialWithEnvelope (RFC 8894 §3.3.3) — polling stub.
Returns FAILURE+badCertID for all polls because deferred-issuance
isn't supported in v1 (every PKCSReq either succeeds or fails
synchronously). Wiring stays in place for a future enhancement.
* Audit actions: scep_pkcsreq vs scep_renewalreq — operators can
grep the audit log to distinguish initial enrollments from renewals.
internal/api/handler/scep.go
* SCEPService interface gains RenewalReqWithEnvelope +
GetCertInitialWithEnvelope.
* pkiOperation RFC 8894 path now switches on envelope.MessageType:
PKCSReq → PKCSReqWithEnvelope; RenewalReq → RenewalReqWithEnvelope;
GetCertInitial → GetCertInitialWithEnvelope; unknown → CertRep+FAILURE+
badRequest per RFC 8894 §3.3.2.2.
== Phase 5.1: GetCACaps capability advertisement =========================
internal/service/scep.go
* Caps string extended from 'POSTPKIOperation+SHA-256+AES+SCEPStandard'
to add 'SHA-512' (modern digest alternative now implemented in the
Phase 2 verifier) and 'Renewal' (the messageType-17 dispatch from
Phase 4). ChromeOS specifically looks for these capabilities to
negotiate the strongest available cipher + digest combo.
* scep_test.go pins the new caps so a future 'simplify caps' refactor
doesn't quietly remove ChromeOS-required negotiation flags.
== Phase 5.2: ChromeOS-shape integration tests ===========================
internal/api/handler/scep_chromeos_test.go (new, ~570 LoC)
* 6 hermetic E2E tests + ~12 helpers. Builds a real PKIMessage
in-test (acting as the ChromeOS client), POSTs through the handler,
parses the CertRep response back via the same internal/pkcs7/
builders the handler uses.
* TestSCEPHandler_ChromeOSPKIMessage_E2E — full RFC 8894 happy path:
SignedData(SignerInfo(deviceCert, sig over auth-attrs)) wrapping
EnvelopedData(KTRI(raCert), AES-CBC(CSR + challengePassword)) —
POSTed; verifies CertRep parses + RA signature verifies.
* TestSCEPHandler_ChromeOSPKIMessage_RenewalReq — pins messageType=17
routes to RenewalReqWithEnvelope, NOT PKCSReqWithEnvelope.
* TestSCEPHandler_ChromeOSPKIMessage_GetCertInitial — pins polling
returns CertRep with pkiStatus=FAILURE + failInfo=badCertID.
* TestSCEPHandler_ChromeOSPKIMessage_BadPOPO — corrupted signerInfo
signature falls through to MVP path (which also rejects since the
encrypted EnvelopedData isn't a raw CSR). No silent acceptance.
* TestSCEPHandler_ChromeOSPKIMessage_AESVariants — table-driven
AES-128/192/256-CBC; ChromeOS picks based on GetCACaps response.
* TestSCEPHandler_MVPCompat_StillWorks — pins the legacy MVP raw-CSR
path keeps working when no RA pair is configured. Backward compat
is non-negotiable.
== Phase 5.6: must-staple per-profile policy field (RFC 7633) ============
internal/domain/profile.go
* Added MustStaple bool to CertificateProfile. Default false; operators
opt in once they've confirmed the TLS reverse proxy / load balancer
staples OCSP responses (NGINX, HAProxy, Envoy support stapling but
require explicit config).
internal/connector/issuer/interface.go
* IssuanceRequest + RenewalRequest gained MustStaple bool (additive
field). Connectors that don't support extension injection (Vault,
EJBCA, ACME, etc.) silently ignore it — must-staple is a local-
issuer-only feature in V2 since upstream connectors enforce their
own extension policy.
internal/connector/issuer/local/local.go
* Added oidMustStaple (1.3.6.1.5.5.7.1.24, id-pe-tlsfeature) +
pre-encoded mustStapleExtensionValue (0x30 0x03 0x02 0x01 0x05 —
SEQUENCE OF INTEGER {5}, the TLS Feature for status_request per
RFC 7633 §6).
* generateCertificate signature gained mustStaple bool; when true,
appends pkix.Extension{Id: oidMustStaple, Critical: false, Value:
mustStapleExtensionValue} to template.ExtraExtensions before
x509.CreateCertificate.
internal/connector/issuer/local/must_staple_test.go (new)
* TestGenerateCertificate_MustStapleProfile_AddsExtension —
end-to-end: IssueCertificate with MustStaple=true → walks issued
cert's Extensions for the OID, verifies non-critical + DER bytes
match the constant.
* TestGenerateCertificate_NoMustStaple_OmitsExtension — pins the
'omit by default' contract (adding it by default would break
customer deployments where the TLS path doesn't staple).
* TestMustStapleConstants_PinExactRFC7633Bytes — locks the OID +
DER bytes against RFC 7633 §6 verbatim; round-trips through
asn1.Unmarshal as []int{5}.
Note: full service-layer plumbing (CertificateProfile.MustStaple →
IssuanceRequest.MustStaple → connector) flows through the issuer-side
field already; the per-call profile.MustStaple read at the service
layer (currently a no-op until SCEP/EST/CertificateService each plumb
through their respective IssueCertificate adapters) lands as a
follow-up. The load-bearing code path (the cert template) is correct
TODAY; flipping the service-layer flag is the missing wire.
== Phase 5.4: docs/legacy-est-scep.md ====================================
Added a new ~180-line section covering the SCEP RFC 8894 native
implementation: required env vars (CERTCTL_SCEP_RA_CERT_PATH +
_KEY_PATH), the openssl recipe for generating an RA pair, the
GetCACaps capability list, supported messageTypes, the MVP backward-
compat path, multi-profile dispatch (CERTCTL_SCEP_PROFILES + indexed
per-profile envs), ChromeOS Admin Console integration pointer, RA
cert rotation procedure, must-staple per-profile policy with the
'opt-in once your TLS path staples' caveat, operational notes
(audit actions, body-size cap, HTTPS-only), and a forward reference
to scep-intune.md (Phase 11).
== Verification ==========================================================
* gofmt + go vet clean for the files I touched.
* staticcheck ./internal/api/handler/... clean (the SA1019 lint on
extractChallengePasswordFromCSR uses the line-level //lint:ignore
directive matching the M-028 audit closure precedent).
* go test -short -count=1 green across api/handler / api/router /
service / pkcs7 / connector/issuer/local / domain / cmd/server.
* G-3 docs-drift CI guard local check: empty diff in both directions.
Phase 4 + Phase 5 of 14 in SCEP RFC 8894 + Intune master bundle.
Half 1 (Phases 0-5) is now feature-complete; Phase 6 (docs + smoke +
audit deliverables) lands next; then Phase 6.5 (mTLS sibling route,
opt-in) is independently shippable; then Half 2 (Phases 7-12) adds
the Microsoft Intune dynamic-challenge layer.
Living progress at cowork/scep-rfc8894-intune/progress.md.
|
||
|
shankar0123
|
a546a1bbef |
feat(scep): EnvelopedData decrypt + signerInfo POPO verify (RFC 8894 §3.2)
SCEP RFC 8894 + Intune master bundle — Phase 2 of 14.
Implements the new RFC 8894 PKIMessage parse path: EnvelopedData parser
+ decryptor, signerInfo parser + signature verifier, handler dispatch
that tries the RFC 8894 path FIRST and falls through to the legacy MVP
raw-CSR path on any parse failure. Backward compat with lightweight SCEP
clients is preserved by design — no behavior change for any existing
deploy that doesn't set CERTCTL_SCEP_RA_*.
internal/pkcs7/envelopeddata.go (new, ~330 LoC)
* ParseEnvelopedData: parses CMS EnvelopedData per RFC 5652 §6.1, with
optional outer ContentInfo unwrapping. Handles SET OF RecipientInfo
+ IssuerAndSerial form rid (RFC 8894 §3.2.2).
* EnvelopedData.Decrypt: RSA PKCS#1 v1.5 key-trans + AES-CBC (128/192/
256) or DES-EDE3-CBC content decryption with **constant-time PKCS#7
padding strip** (no branch on padding-byte values; closes the
padding-oracle leak surface). Recipient mismatch is BadMessageCheck
per RFC 8894 §3.3.2.2 (NOT BadCertID); every failure mode returns
the same ErrEnvelopedDataDecrypt sentinel to close timing-leak legs
of Bleichenbacher attacks.
* Equivalent to micromdm/scep's cryptoutil/cryptoutil.go::DecryptPKCS-
Envelope (cited in code comments; not vendored — fuzz-target
ownership stays in this sub-package per the operating rule).
internal/pkcs7/signedinfo.go (new, ~370 LoC)
* ParseSignedData / ParseSignerInfos: parses CMS SignedData per RFC
5652 §5.3. Resolves each SignerInfo's SID (IssuerAndSerial v1 OR
[0] SubjectKeyId v3) against the SignedData certificates SET to
pluck the device's transient signing cert.
* SignerInfo.VerifySignature: re-serialises signedAttrs as the
canonical SET OF Attribute (the RFC 5652 §5.4 quirk every CMS
implementation hits — wire form is [0] IMPLICIT but the signature
is over EXPLICIT SET OF). Hashes with SHA-1/SHA-256/SHA-512 +
verifies via RSA PKCS1v15 or ECDSA per the cert's pubkey type.
* Auth-attr extractors: GetMessageType (PrintableString-decimal),
GetTransactionID, GetSenderNonce, GetMessageDigest. SCEP attr OIDs
pinned (RFC 8894 §3.2.1.4).
internal/pkcs7/{envelopeddata,signedinfo}_fuzz_test.go (new)
* FuzzParseEnvelopedData / FuzzParseSignedData / FuzzParseSignerInfos
/ FuzzVerifySignerInfoSignature — every parser certctl adds gets a
panic-safety fuzzer (the fuzz-target-ownership rule from
cowork/CLAUDE.md::Operating Rules). Local 5s runs hit ~270k
executions per parser without panic. Errors are expected for
arbitrary inputs; only panics are bugs.
internal/pkcs7/{envelopeddata,signedinfo}_test.go (new)
* Round-trip tests that materialise real RSA/ECDSA pairs, hand-build
the wire bytes, parse + decrypt + verify, and assert plaintext /
auth-attr equality. The build helpers use this package's ASN1Wrap
primitives directly (asn1.Marshal of structs containing nested
asn1.RawValue is finicky for mixed Class/Tag); gives byte-level
control matching what real SCEP clients emit.
* Negative tests: tampered ciphertext / tampered auth-attrs / wrong
RA / wrong key / mismatched recipients / random garbage all return
the appropriate sentinel error without panic.
internal/service/scep.go
* PKCSReqWithEnvelope: RFC 8894 envelope-aware variant. Returns
*SCEPResponseEnvelope (not error + *SCEPEnrollResult) because RFC
8894 §3.3 mandates a CertRep PKIMessage on every response, even
failures — the handler shouldn't translate Go errors into SCEP
failInfo codes. Returns nil to signal 'invalid challenge password'
so the caller can translate to HTTP 403 (matches MVP path's wire
shape; RFC 8894 §3.3.1 is silent on this case).
* mapServiceErrorToFailInfo: exact mapping table from the prompt
(CSR parse → BadRequest, CSR sig → BadMessageCheck, crypto policy
→ BadAlg, default → BadRequest).
internal/api/handler/scep.go
* SCEPService interface gains PKCSReqWithEnvelope.
* SCEPHandler now optionally carries an RA cert + key pair. SetRAPair
upgrades the handler to the RFC 8894 path; without that call the
handler stays MVP-only (the v2.0.x behavior).
* pkiOperation: tries the RFC 8894 path FIRST when the RA pair is
set. tryParseRFC8894 helper does the full pipeline (ParseSignedData
→ VerifySignature → extract auth-attrs → ParseEnvelopedData → Decrypt
→ x509.ParseCertificateRequest the recovered bytes). On any failure
it falls through to the legacy extractCSRFromPKCS7 MVP path —
backward compat is non-negotiable.
* Phase 2 emits the legacy certs-only response on RFC 8894 success;
Phase 3 (next commit) swaps in writeCertRepPKIMessage with the
proper status / failInfo / nonce-echo wire shape.
cmd/server/main.go
* Per-profile loop now calls loadSCEPRAPair after preflight to load
the cert + key + inject via SetRAPair. crypto + crypto/tls imports
added.
* loadSCEPRAPair helper: tls.X509KeyPair-based parse + leaf cert
extraction. Failures here indicate TOCTOU between preflight + load.
internal/api/handler/scep_handler_test.go +
internal/api/router/router_scep_profiles_test.go
* mockSCEPService / scepProfileMockService gain PKCSReqWithEnvelope
stubs to satisfy the extended interface. Existing test cases
unchanged (they exercise the MVP path; RA pair is unset).
Verification:
* gofmt + go vet clean for the files I touched.
* go test -short -count=1 green across pkcs7 / api/handler /
api/router / service / cmd/server.
* Coverage: pkcs7 78.4% (was 100% — drops because new code includes
paths the round-trip tests don't yet hit, like decryption alg
fall-through and v3 SubjectKeyId SID matching).
* Fuzz-target seed-corpus runs (5s each, ~270k execs/parser): no
panic. Pre-merge fuzz-time bumps to 30s per the prompt's
verification gate.
Phase 2 of 14 in SCEP RFC 8894 + Intune master bundle.
Living progress at cowork/scep-rfc8894-intune/progress.md.
|
||
|
shankar0123
|
db71b47c24 |
main: wire CRL/OCSP responder services into runtime
Activates the CRL/OCSP responder pipeline that landed dormant in phases 1-4 (commits |
||
|
shankar0123
|
dc1e0bfbaa |
crl/ocsp: POST OCSP endpoint (RFC 6960 §A.1.1) + cache integration
Phase 4 (final phase) of the CRL/OCSP responder bundle. Closes the
backend slice; HTTP layer is now production-ready for relying parties.
What landed:
* POST /.well-known/pki/ocsp/{issuer_id} (handler.HandleOCSPPost)
- Accepts binary application/ocsp-request body per RFC 6960 §A.1.1
- Tolerant of missing Content-Type (some clients omit); validates
via ocsp.ParseRequest, returns 400 on malformed
- Returns 415 on explicit wrong Content-Type
- Reuses the existing service path (h.svc.GetOCSPResponse) — the
only new logic is body decoding + serial-from-OCSPRequest extraction
- GET form preserved unchanged for ad-hoc curl + human URL paths
- Auth-exempt under /.well-known/pki/ prefix (already in
AuthExemptDispatchPrefixes — no router changes for that)
- 7 new tests: success, method-not-allowed, wrong content-type,
missing content-type accepted, malformed body, missing issuer,
service error propagation
* router.go: r.Register("POST /.well-known/pki/ocsp/{issuer_id}", ...)
* CertificateService.GenerateDERCRL — cache-aware:
- New SetCRLCacheSvc(svc) setter (matches existing SetCAOperationsSvc
pattern — optional dep)
- When wired, GenerateDERCRL calls crlCacheSvc.Get → cheap DB read
on cache hit, singleflight-coalesced regen on miss
- When unwired, falls back to historical caSvc.GenerateDERCRL path
- GET /.well-known/pki/crl/{issuer_id} handler unchanged — calls
the same service method, gets cache benefit transparently when
the cache service is wired in cmd/server/main.go
Coverage: handler 79.8% (floor 75), service unchanged, scheduler 78%.
What's deferred (intentional scope cut for this session):
* cmd/server/main.go wiring of CRLCacheService + responder service
setters into the local issuer factory + scheduler. The wiring is
mechanical (NewCRLCacheService + scheduler.SetCRLCacheService call
in the existing wiring block); deferring keeps this commit focused
on the responder + cache primitives. Operator can wire when ready.
* Phase 5 (GUI), Phase 6 (e2e test against kind), Phase 7 (release
prep) — separate follow-up sessions.
* OCSP cache integration: today's GET/POST OCSP path goes through
the on-demand SignOCSPResponse (already cheap with the dedicated
responder cert from Phase 2). A cached-OCSP path is V3-Pro polish.
The bundle's V2 backend slice (Phases 0-4) is complete. All 4 phases
shipped 4 commits + 1 amend on this branch. CI will validate the
testcontainers repository tests on push.
|
||
|
shankar0123
|
dc326942db |
scheduler/service: crlGenerationLoop + CRLCacheService with singleflight
Phase 3 of the CRL/OCSP responder bundle. Adds the scheduler-driven
pre-generation pipeline that lets the /.well-known/pki/crl/{issuer_id}
HTTP handler (Phase 4) serve from cache instead of regenerating per
request.
What landed:
* internal/scheduler/scheduler.go:
- CRLCacheServicer interface (RegenerateAll(ctx))
- Scheduler struct gains crlCacheService + crlGenerationInterval +
crlGenerationRunning fields; default interval 1h
- SetCRLCacheService + SetCRLGenerationInterval setters following
the existing Set* convention (cloudDiscovery, digest, etc.)
- Wired into Start: optional loop, gated on crlCacheService != nil
- crlGenerationLoop: ticker + atomic.Bool re-entry guard +
WaitGroup integration mirroring digestLoop
- runCRLGeneration: 5-minute timeout per cycle; per-issuer
failures are caught inside RegenerateAll itself
* internal/service/crl_cache.go — CRLCacheService:
- Get(ctx, issuerID) → (der, thisUpdate, err)
cache hit → DB read; miss/stale → singleflight regenerate
- RegenerateAll(ctx) — walks every issuer in registry; per-issuer
failures logged + audited (crl_generation_events) but don't
abort the cycle
- In-tree singleflight gate (~30 LoC, sync.Map[issuerID]*flightEntry)
— collapses concurrent miss requests for the same issuer into
one underlying generation. No new dep on golang.org/x/sync
- Uses existing CAOperationsSvc.GenerateDERCRL for the heavy work
(no duplication of CRL-build logic); parses returned DER to
recover thisUpdate / nextUpdate / number / count
- Failure-event recording is best-effort (failure to record does
not fail the operation) — events are an audit aid, not a gate
* internal/service/crl_cache_test.go — 8 tests:
- Cache hit, miss, staleness paths
- RegenerateAll happy + cancelled ctx
- Singleflight: 20 concurrent misses → 1 generation
- Failure event recording when issuer is missing from registry
- Nil cache repo returns error
Coverage: service 73.5% (floor 70), scheduler 78.1% (floor 60).
Backward compat: unchanged for any caller that doesn't call
SetCRLCacheService. cmd/server/main.go wiring lands in Phase 4
alongside the POST OCSP endpoint + handler refactor to consult
the cache.
|
||
|
shankar0123
|
8326d95210 |
Bundle N.C-extended (Coverage Audit Extension): service + handler round-out — M-002 + M-003 partial-closed
Three new round-out test files targeting handler-interface delegators
on CertificateService + AgentService + IssuerHandler/HealthCheckHandler.
Coverage deltas
=================
internal/service: 70.5% -> 73.4% (+2.9pp; 17 new tests)
internal/api/handler: 79.4% -> 79.8% (+0.4pp; 4 new tests)
Service round-out tests (certificate_round_out_test.go, ~165 LoC)
=================
- GetCertificate (delegate-to-repo + NotFound)
- CreateCertificate (defaults populated + repo error)
- UpdateCertificate (patch merge + NotFound + repo error)
- ArchiveCertificate (delegate + repo error)
- GetCertificateVersions (pagination defaults + page-out-of-range +
repo error)
- SetJobRepo / SetKeygenMode (no-crash setters)
Service round-out tests (agent_round_out_test.go, ~140 LoC)
=================
- GetAgent (delegate)
- RegisterAgent (defaults populated + repo error)
- GetWork / GetWorkWithTargets (no-jobs path)
- UpdateJobStatus (delegate to ReportJobStatus)
- CSRSubmit / CSRSubmitForCert (invalid-CSR error)
- CertificatePickup (agent-not-found)
- GetAgentByAPIKey (unknown key)
- GetCertificateForAgent (missing agent)
- SetProfileRepo (no-crash)
Handler round-out tests (round_out_test.go, ~40 LoC)
=================
- NewIssuerHandlerWithLogger (logger wired through)
- UpdateHealthCheck dispatch arm with bad ID
- GetHealthCheckHistory dispatch arm with bad ID
Why partial
=================
M-002 / M-003 prescribed >=80%. Service at 73.4% and handler at 79.8%
miss the gate by 6.6pp / 0.2pp respectively. The remaining service
gap is in CSR-submit happy-path and large-population list-filter
flows that need deeper repo plumbing (3-4 hr more focused work).
The handler 0.2pp is in parseSignedDataForCSR (SCEP), DeleteHealthCheck,
AcknowledgeHealthCheck — needs repo fixtures.
These extensions are a meaningful step but don't fully close M-002
and M-003. Tracked as N.C-final follow-on; not blocking on a CI
floor at 73 / 79.
Audit deliverables
=================
- gap-backlog.md M-002, M-003: partial-strikethrough with progress
note + remaining-gap analysis
- extension-progress.md: N.C-extended marked PARTIAL
Closes (partial): M-002, M-003
Bundle: N.C-extended (Coverage Audit Extension)
|
||
|
shankar0123
|
1b4de3fb2d |
Bundle E: Mechanical sweeps & defensive polish — 6 findings closed; L-004 deferred
Closes L-009 + L-010 + L-011 + L-013 + L-020 + L-021 from
comprehensive-audit-2026-04-25. L-004 deferred — recon found NO
rotation infrastructure exists at all; building it from scratch is
a feature project, not a Bundle-E mechanical sweep.
L-009 — ZeroSSL EAB URL configurable
Audit's 'no timeout' claim was wrong: ari.go:329 has 15s timeout.
internal/connector/issuer/acme/acme.go: zeroSSLEABEndpoint now
lazily reads CERTCTL_ZEROSSL_EAB_URL from env at package init;
defaults to ZeroSSL public endpoint. Pre-existing test override
path preserved.
L-010 — Verified-already-clean
grep -rn 'mock\.Anything' --include='*_test.go' . returned 0.
certctl uses hand-rolled struct mocks (mockJobRepo, mockAuditRepo,
etc.) with explicit method bodies; no testify-style mocks anywhere.
L-011 — IPv6 bracket-aware dialing pinned
Every production net.Dial / DialTimeout site audited:
cmd/agent/main.go:293 — intentional IPv4 literal '8.8.8.8:80'
verify.go / tlsprobe / network_scan — net.Dialer (no string addr)
email.go — net.JoinHostPort (bracket-aware)
ssh.go — addr derives from JoinHostPort upstream
ssrf.go — net.Dialer
internal/connector/notifier/email/email_ipv6_test.go (NEW):
TestJoinHostPort_IPv6BracketsRoundTrip pins IPv4/IPv6/zone variants;
TestSMTPDialerUsesJoinHostPort source-greps email.go and fails CI
if a future refactor swaps in 'host:port' concatenation.
L-013 — Verified-already-clean (monotonic-safe)
Only one site uses now.Sub: middleware.go:393 in tokenBucket.allow().
Both 'now' and tb.lastRefill come from time.Now() which carries
monotonic-clock readings per Go's time package contract;
intra-process now.Sub is monotonic-safe by construction. Doc
comment block added above the call to make the invariant explicit.
L-020 (CWE-563) — ineffassign sweep, 8 unique sites
certificate.go:135 — sortDir initial value dropped (set
unconditionally below by SortDesc branch).
certificate.go:169,175 — argCount post-increments dropped (var
not read past the LIMIT/OFFSET formatting).
agent_group.go, profile.go — page/perPage truly vestigial,
replaced with _ = page; _ = perPage.
issuer.go:633, owner.go:131, target.go:267, team.go:131 — same
treatment for the audit-flagged second-function ListXxx clamps.
First-function List() in issuer/owner/target/team KEEPS its
clamp because page/perPage is used for in-memory slice
pagination — ineffassign correctly didn't flag those.
Build + tests green post-sweep.
L-021 — Transitive CVE bump
go get golang.org/x/crypto@v0.45.0 golang.org/x/net@v0.47.0
(crypto required net@0.47.0). go-text@v0.31.0 transitively
bumped.
Per tool-output govulncheck-verbose: x/net@v0.45.0 fixes
GO-2026-4441 + GO-2026-4440; x/crypto@v0.45.0 fixes
GO-2025-4134 + GO-2025-4135 + GO-2025-4116 — all 5 advisories
cleared. Bundle B's ISV grep guard + Bundle D's release-time
govulncheck step are the going-forward monitor + bump pass.
L-004 — Deferred to dedicated bundle
Recon: zero hits for RotateAPIKey / rotated_at / key_status
anywhere in source. API keys configured via
CERTCTL_API_KEYS_NAMED env var; rotation is operator-managed
(edit env + restart). Building rotation infrastructure from
scratch is a feature project, not a mechanical sweep.
Documented in audit-report.md with scope-pivot note.
Audit deliverables:
audit-report.md: score 46/55 -> 52/55 closed
(Low 14/19 -> 19/19 — 100% Low closed except L-004 deferred)
findings.yaml: 6 status flips
certctl/CHANGELOG.md: Bundle E section
Verification:
go test -count=1 -short ./internal/service ./internal/connector/issuer/acme
./internal/connector/notifier/email green
go vet on changed packages clean
|
||
|
shankar0123
|
e720474fb7 |
Bundle D: Documentation & transparency sweep — 8 findings closed
Closes H-009 + L-001 + L-007 + L-008 + L-016 + L-017 + L-018 + M-027
from comprehensive-audit-2026-04-25.
H-009 — README JWT verified-already-clean
README has zero JWT mentions at audit time. docs/architecture.md
correctly documents JWT/OIDC integration via authenticating-gateway
pattern (line 905-912).
.github/workflows/ci.yml: new step
'Forbidden README JWT advertising regression guard (H-009)'
greps README for JWT-as-supported phrasing; passes verbatim
(gateway / pre-G-1) but fails build on net-new advertising.
L-001 (CWE-295) — InsecureSkipVerify per-site justification
Audit count was 8; recon found 13 production sites.
docs/tls.md: new 'InsecureSkipVerify justifications' table
enumerates each site by file:line with per-site rationale.
cmd/agent/verify.go:78, internal/tlsprobe/probe.go:54,
internal/service/network_scan.go:460: each previously-bare
InsecureSkipVerify: true now carries //nolint:gosec.
.github/workflows/ci.yml: new step
'Forbidden bare InsecureSkipVerify regression guard (L-001)'
fails build if any net-new ISV lands in non-test .go without
nolint:gosec on the same or preceding line.
L-007 — README dependency-audit commands
README.md: new Dependencies section with go list -m all | wc -l,
go mod why, govulncheck ./.... Honors operating-rules invariant.
L-008 — Release-time govulncheck gate
.github/workflows/release.yml: new 'Install govulncheck' +
'Run govulncheck (release gate)' steps in the matrix job.
Pinned to same install path as ci.yml. Default exit code
semantics (fail on called-vuln only, deferred-call advisories
tracked on master via L-021) keeps the gate appropriate.
L-016 — architecture.md drift fixes
docs/architecture.md: system-components diagram's '21 tables'
annotation removed (current 23; replaced with TEXT-keys
descriptor); connector-architecture '9 connectors' prose
replaced with grep ref + current 12-issuer list (added
Entrust/GlobalSign/EJBCA which were missing); API-design
'97 operations / 107 total' replaced with grep commands.
Connector subgraphs verified-current at 12/13/6.
L-017 — workspace CLAUDE.md verified-already-clean
Bundle B's pre-commit-gate refactor already converted current-
state numeric claims to grep commands. Phase 0 recon confirmed
zero remaining hardcoded counts.
L-018 — Defect age table
cowork/comprehensive-audit-2026-04-25/defect-age.md (NEW):
Tabulates all 9 High findings with first-mentioned commit,
closing bundle, days-open. Methodology snippet for re-running.
Key finding: 8 of 9 closed within 24h of audit publication.
M-027 — OpenAPI parity verified-already-clean
Audit's 'router 121 vs OpenAPI 125 — 4-op gap' was wrong
methodology. The 4-op 'gap' was exactly the 4 routes registered
via r.mux.Handle (auth-exempt allowlist) instead of r.Register.
When you count both dispatch shapes the totals match exactly.
internal/api/router/openapi_parity_test.go (NEW):
TestRouter_OpenAPIParity AST-walks router.go for both
Register and mux.Handle calls + walks api/openapi.yaml's
path/method nesting + asserts the sets match. Adding a route
without updating the spec fails CI permanently.
Audit deliverables:
audit-report.md: score 38/55 -> 46/55 closed
(High 7/9 -> 8/9; Medium 20/27 -> 21/27; Low 8/19 -> 14/19)
findings.yaml: 8 status flips open -> closed
defect-age.md: new file
certctl/CHANGELOG.md: Bundle D section
Verification:
TestRouter_OpenAPIParity PASS
L-001 grep guard self-test (after //nolint:gosec adds) PASS
H-009 grep guard self-test PASS
go test -count=1 -short on changed packages green
|
||
|
shankar0123
|
62a412c488 |
Bundle C: Renewal/reliability cluster — 7 findings closed
Closes M-006 + M-007 + M-008 + M-015 + M-016 + M-019 + M-020 from
comprehensive-audit-2026-04-25. M-028 was already closed by the
Bundle B CI follow-up.
M-006 (CWE-913) — Idempotent migration 000014
migrations/000014_policy_violation_severity_check.up.sql:
Prepended ALTER TABLE ... DROP CONSTRAINT IF EXISTS before the
ADD. Mirrors the down migration's existing IF EXISTS shape and
the M-7 idempotent-index idiom. Re-runs against partially-applied
DBs now succeed.
M-007 — Bulk-op partial-failure tests (3 new)
internal/api/handler/bulk_partial_failure_test.go:
TestBulkRevoke_PartialFailure_ReportsBoth
TestBulkRenew_PartialFailure_ReportsBoth
TestBulkReassign_PartialFailure_ReportsBoth
Each asserts HTTP 200 + both success/failure counters round-trip
+ per-cert errors[] preserved with non-empty messages so operators
can correlate each failure to its certificate ID.
M-008 — Admin-gated handler enumeration pin (verified-already-clean)
Recon: only one admin-gated handler — bulk_revocation.go — with
full 3-branch test triplet already in place. health.go calls
IsAdmin informationally to surface the flag to the GUI without
gating.
internal/api/handler/m008_admin_gate_test.go:
Walks every handler .go file, asserts every middleware.IsAdmin
call site is in AdminGatedHandlers (with required test triplet)
or InformationalIsAdminCallers (justified). Adding a new admin
gate without updating both the constant AND adding the test
triplet fails CI.
M-015 — Single-profile cardinality pin (verified-already-clean)
Audit claim 'no cardinality validation' was wrong — enforced at
struct level. domain.ManagedCertificate.{CertificateProfileID,
RenewalPolicyID,IssuerID,OwnerID} and RenewalPolicy.
CertificateProfileID are bare strings, not slices.
internal/domain/m015_cardinality_test.go:
reflect-based pin on kind=String. Schema change to N:N would
have to update renewal.go's lookup loop in the same commit.
M-016 (CWE-754) — Reap stale-agent jobs
internal/repository/postgres/job.go::ListJobsWithOfflineAgents:
JOIN jobs to agents on agent_id, filter (status=Running AND
a.last_heartbeat_at < cutoff), exclude server-keygen jobs.
internal/service/job.go::ReapJobsWithOfflineAgents:
Flips matched jobs to Failed reason agent_offline so I-001
retry loop re-queues them on a healthy agent. Records audit
event per reap.
internal/scheduler/scheduler.go:
Scheduler.runJobTimeout cycle now calls both reaper arms.
agentOfflineJobTTL default 5min (5x agent-health-check default);
SetAgentOfflineJobTTL knob for operator override.
internal/service/job_offline_agent_reaper_test.go: 6 unit tests
cover happy path, server-keygen-skip, non-Running-skip, non-
positive-TTL fail-loud, repo-error propagation, audit-event
recording.
M-019 — Configurable ARI HTTP timeout
Audit claim 'no fallback timeout' was wrong — ari.go:52 already
had a 15s timeout. Bundle C makes it configurable.
internal/connector/issuer/acme/acme.go:
Config.ARIHTTPTimeoutSeconds field with env path
CERTCTL_ACME_ARI_HTTP_TIMEOUT_SECONDS.
internal/connector/issuer/acme/ari.go:
Both HTTP clients (GetRenewalInfo + getARIEndpoint) now use the
new ariHTTPTimeout() helper. Zero / negative / nil-config all
fall back to the historic 15s default.
ari_timeout_test.go: 4 dispatch arm tests.
M-020 (CWE-770) — OCSP DoS hardening
Pre-bundle the noAuthHandler chain had no rate limit. An attacker
could DoS the OCSP responder, which for fail-open relying parties
is a revocation bypass.
cmd/server/main.go:
noAuthHandler refactored from fixed middleware.Chain(...) to a
conditional slice that appends middleware.NewRateLimiter when
cfg.RateLimit.Enabled. Per-IP keying applies; OCSP/CRL/EST/SCEP
are unauth.
docs/security.md (NEW):
Operator runbook documenting Must-Staple TLS Feature extension
RFC 7633 as the architectural fix for fail-open relying parties.
Profile-flip guidance + nginx/Apache/HAProxy/Envoy stapling
snippets + explicit scope statement on what the rate limiter
alone does NOT solve.
Audit deliverables:
cowork/comprehensive-audit-2026-04-25/audit-report.md: score
31/55 -> 38/55 closed (Medium 13/27 -> 20/27).
cowork/comprehensive-audit-2026-04-25/findings.yaml: 7 status
flips open -> closed with closure notes citing the Bundle C
mechanism.
certctl/CHANGELOG.md: Bundle C section under [unreleased].
Verification:
go vet ./internal/service ./internal/scheduler ./internal/connector/issuer/acme
./internal/api/handler ./internal/domain ./cmd/server clean
go test -count=1 -short on the same packages all green
helm template + helm lint clean
internal/repository/postgres setup-fail sandbox disk
pressure (same on master HEAD before this branch)
|
||
|
shankar0123
|
1d6c7a0552 |
fix(bundle-6): Audit Integrity + Privacy — 3 audit findings closed
Closes Audit-2026-04-25 H-008 (High), M-017 (Medium), M-022 (Medium).
Hardens audit-trail tamper-resistance + minimizes PII leakage in one
cohesive change, with both controls applying automatically and no
operator action required at install time.
What changed
- internal/service/audit_redact.go (NEW) — RedactDetailsForAudit:
* credentialKeys deny-list (api_key, password, *_pem, eab_secret, ...)
* piiKeys deny-list (email, phone, ssn, name, address, ip_address, ...)
* case-insensitive key match; recurses into nested maps + arrays
* mutation-free; surfaces redacted_keys array for operator visibility
* nil/empty input → nil out (preserves pre-Bundle-6 behaviour)
- internal/service/audit.go — RecordEvent now routes details through
RedactDetailsForAudit BEFORE marshaling. No call-site changes required.
- internal/service/audit_redact_test.go (NEW) — full coverage:
* credential keys (~30 entries)
* PII keys (~20 entries)
* nested maps + arrays
* case-insensitivity
* mutation-free invariant
* JSON round-trip (catches type-assertion regressions)
* scalar pass-through (no panic on int/bool/nil)
- migrations/000018_audit_events_worm.up.sql (NEW) — DB-level WORM:
* BEFORE UPDATE OR DELETE trigger raises check_violation with
diagnostic citing the rationale + compliance-superuser hint
* REVOKE UPDATE,DELETE ON audit_events FROM certctl (defence-in-depth)
* REVOKE wrapped in pg_roles existence check so test fixtures
without the certctl role stay idempotent
- migrations/000018_audit_events_worm.down.sql (NEW) — clean teardown
for dev resets; not for production use.
- internal/repository/postgres/audit_worm_test.go (NEW, testcontainers,
-short gated) — INSERT succeeds; UPDATE + DELETE fail with
check_violation; second INSERT after blocked modification still
succeeds (no trigger-state corruption).
- docs/compliance.md — new section "Audit-Trail Integrity & Privacy
(Bundle 6)" with verification psql snippet, compliance-superuser
pattern (NOT auto-created), redactor before/after example, and a
maintenance note for adding new credential keys.
Compliance mapping
- H-008 (CWE-532 Insertion of Sensitive Information into Log File)
- M-017 (HIPAA Technical Safeguards §164.312(b) — audit controls)
- M-022 (GDPR Art. 32 — data minimization)
Threat model: TB-3 (audit log tampering), TB-1 (operator/orchestrator).
Verification
- go vet ./... → clean
- go build ./... → clean
- go test -short -count=1 ./... → all packages pass
- go test -count=1 -run TestRedactDetailsForAudit ./internal/service/...
→ all pass
- (testcontainers, gated by -short) audit_worm_test.go pins WORM contract
- npx tsc --noEmit (web) → clean (no frontend changes)
- python3 yaml.safe_load(api/openapi.yaml) → 89 paths
Backward compatibility
- Trigger applies forward only — existing rows unchanged.
- nil/empty details from RecordEvent callers → nil out (preserves prior
behaviour for the many existing call sites that pass nil).
- Compliance superusers (provisioned out-of-band) bypass the trigger.
Bundle 6 of the 2026-04-25 comprehensive audit.
|
||
|
shankar0123
|
f0865bb051 |
fix(api,web,mcp): add bulk-renew + bulk-reassign endpoints, drop client-side N×HTTP loops (L-1 master)
Two audit findings, both category cat-l, both rooted in
web/src/pages/CertificatesPage.tsx. Pre-L-1 the GUI looped per-cert
HTTP calls — 100 selected certs = 100 sequential round-trips × ~50–200
ms each = a 5–20-second wedge during which the operator stared at a
progress bar. Post-L-1 each workflow is a single POST.
cat-l-fa0c1ac07ab5 [P1, primary] — bulk renew loop
handleBulkRenewal: for/await triggerRenewal(id)
cat-l-8a1fb258a38a [P2] — bulk reassign loop
handleReassign: for/await updateCertificate(id, {owner_id})
The bulk-revoke endpoint (POST /api/v1/certificates/bulk-revoke +
BulkRevocationCriteria/Result) already existed as the canonical shape
in v2.0.x — L-1 ports that pattern to renew + reassign with per-action
twists.
Backend (Go)
- internal/domain/bulk_renewal.go: BulkRenewalCriteria mirrors
BulkRevocationCriteria (criteria + IDs modes); BulkRenewalResult
envelope adds EnqueuedJobs[] for per-cert {certificate_id, job_id};
shared BulkOperationError type for all bulk paths.
- internal/domain/bulk_reassignment.go: narrower shape — IDs-only,
owner_id required, team_id optional.
- internal/service/bulk_renewal.go::BulkRenewalService.BulkRenew:
resolves criteria → status filter (Archived/Revoked/Expired/
RenewalInProgress all silent-skip) → per-cert status flip + job
create. Keygen-mode-aware so jobs land in the same initial status
as single-cert TriggerRenewal. Single bulk audit event per call,
not N.
- internal/service/bulk_reassignment.go::BulkReassignmentService.
BulkReassign: validates owner_id upfront via the
ErrBulkReassignOwnerNotFound typed sentinel — non-existent owner
returns 400 before any cert is touched. Already-owned-by-target
is silent-skip. Single bulk audit event.
- internal/api/handler/{bulk_renewal,bulk_reassignment}.go: HTTP
shape mirrors bulk_revocation.go. NOT admin-gated (renew is non-
destructive; reassign is a common-case workflow). Sentinel-error
→ 400 mapping for OwnerNotFound.
- internal/api/router/router.go: three bulk-* routes registered as a
block before the {id} routes. HandlerRegistry gains BulkRenewal +
BulkReassignment fields.
- cmd/server/main.go: NewBulkRenewalService threads cfg.Keygen.Mode
so bulk-renew jobs land in same initial state as single-cert path.
Frontend
- web/src/api/client.ts: bulkRenewCertificates(criteria) +
bulkReassignCertificates(request) functions with full TS types.
- web/src/pages/CertificatesPage.tsx: handleBulkRenewal + handleReassign
rewritten from N-call loops to single calls. Result envelope drives
progress UI; first-error message surfaced when total_failed > 0.
Stale triggerRenewal + updateCertificate imports removed.
MCP
- internal/mcp/types.go: BulkRenewCertificatesInput +
BulkReassignCertificatesInput.
- internal/mcp/tools.go: certctl_bulk_renew_certificates +
certctl_bulk_reassign_certificates tools mirroring the existing
certctl_bulk_revoke_certificates pattern.
OpenAPI
- api/openapi.yaml: two new operations (bulkRenewCertificates,
bulkReassignCertificates) under Certificates tag. Four new schemas
(BulkRenewRequest, BulkRenewResult, BulkEnqueuedJob,
BulkReassignRequest, BulkReassignResult).
Tests
- Domain: BulkRenewalCriteria.IsEmpty + BulkReassignmentRequest.IsEmpty
IsEmpty contracts; JSON round-trip shape pinning.
- Service: 7 BulkRenew tests (happy/criteria-mode/skips-RenewalInProgress/
skips-revoked-archived/empty-criteria-error/partial-failure/
audit-event-emitted) + 8 BulkReassign tests (happy/skips-already-
owned/owner-required/empty-IDs/owner-not-found-sentinel/team-id-
optional/team-id-provided/partial-failure/audit-event-emitted).
- Handler: 5 BulkRenew handler tests (happy/empty-body-400/wrong-
method-405/actor-attribution/service-error-500) + 6 BulkReassign
handler tests (happy/empty-IDs-400/missing-owner-400/owner-not-
found-400-via-sentinel/wrong-method-405/generic-error-500).
CI guardrail
- .github/workflows/ci.yml: 'Forbidden client-side bulk-action loop
regression guard (L-1)'. Greps web/src/pages/CertificatesPage.tsx
for 'for(...) await triggerRenewal(...)' and 'for(...) await
updateCertificate(...)' patterns; comment lines exempt; test files
exempt. Verified locally (passes against post-fix tree, fires
against synthetic regression).
Counts (deltas)
- Routes: 119 → 121 (+2)
- OpenAPI operations: 123 → 125 (+2)
- MCP tools: 83 → 85 (+2)
Performance
- 100-cert bulk-renew: ~10s of sequential HTTP → ~100ms (99% latency
reduction on the canonical operator workflow).
- Audit event volume: 1 + N per operation → 1.
Out of scope (deferred follow-ups)
- cat-b-31ceb6aaa9f1: updateOwner/updateTeam/updateAgentGroup orphan
(different shape — wire existing PUT to GUI, not new bulk endpoint).
- cat-k-e85d1099b2d7: CertificatesPage no pagination UI.
- cat-i-b0924b6675f8: MCP missing claim/dismiss/acknowledge (L-1 added
2 new tools but does not close that finding).
Verification
- go build / vet / test -short / test -short -race all clean.
- web tsc --noEmit + vitest run all clean (296 tests passing).
- OpenAPI YAML parses (89 paths, 125 ops).
- L-1 CI guardrail passes against post-fix tree, fires against
synthetic regression.
No push.
|
||
|
shankar0123
|
9834b4e4a4 |
G-1: renewal-policies API + frontend FK-drift fix
Three frontend call sites (OnboardingWizard.tsx:603, CertificatesPage.tsx:52,
CertificateDetailPage.tsx:169) populated the renewal_policy_id dropdown from
getPolicies() — the compliance-rule endpoint returning pol-* IDs — which
violated the FK managed_certificates.renewal_policy_id REFERENCES
renewal_policies(id) ON DELETE RESTRICT. Create would fail pg 23503 at insert.
Backend (new):
- RenewalPolicyRepository CRUD + ListAll/ExistsByID (pg 23503 → ErrRenewalPolicyInUse
→ HTTP 409; pg 23505 → ErrRenewalPolicyDuplicateName → HTTP 409)
- RenewalPolicyService with repo-only constructor. Service sentinels
var-alias the repo sentinels so errors.Is walks across layers.
- RenewalPolicyHandler with validation bounds: name 1–255;
renewal_window_days [1,365] default 30; max_retries [0,10] not defaulted;
retry_interval_seconds [60,86400] default 3600; alert_thresholds_days
[0,365] default [30,14,7,0]. Auto-generated IDs rp-<slug(name)>.
- Router registers 5 routes under /api/v1/renewal-policies[/{id}].
Frontend:
- CertificatesPage/CertificateDetailPage/OnboardingWizard now call
getRenewalPolicies() and render rp-* IDs.
- client.ts adds getRenewalPolicies/createRenewalPolicy/updateRenewalPolicy/
deleteRenewalPolicy. types.ts adds the RenewalPolicy shape.
OpenAPI: RenewalPolicies tag + 5 operations + 3 schemas (RenewalPolicy,
RenewalPolicyCreateRequest, RenewalPolicyUpdateRequest). 409 responses
on create/update duplicate-name and delete FK-in-use.
No migration — renewal_policies table already exists from the initial
schema (000001).
Tests:
- internal/service/renewal_policy_test.go: CRUD + validation + sentinel
error wrapping.
- internal/api/handler/renewal_policy_handler_test.go: handler endpoint
contracts including 400/404/409.
- web/src/api/client.test.ts: 4 subtests covering the 4 new API functions.
Phase 3 gates all green: go vet, build, short tests, race tests (service/
handler/router/scheduler), staticcheck (G-1 packages), govulncheck (0
reachable), coverage (service 69.7%, handler 79.0%, domain 86.9%,
middleware 80.6% — all above thresholds), tsc, vitest (256 passed),
vite build, OpenAPI structural validation.
|
||
|
shankar0123
|
4e5522a999 |
F-001/F-002/F-003: CRL prefix-scan, digest error sanitization, ctx-aware sleeps
F-001 (P3): GenerateDERCRL scoped to issuer via composite index
- Add RevocationRepository.ListByIssuer leveraging migration 000012's
idx_certificate_revocations_issuer_serial composite index as a
prefix-scan target. Previously CAOperationsSvc.GenerateDERCRL called
ListAll() and filtered by IssuerID in Go — O(total revocations)
regardless of how many revocations belonged to the target issuer.
- Rewrite GenerateDERCRL to call ListByIssuer(ctx, issuerID) so PostgreSQL
drives a prefix scan of the composite index. Drops the in-memory filter.
- New regression test in ca_operations_test.go asserts the CRL hot path
invokes ListByIssuer exactly once and never ListAll, and that the
issuerID is threaded through correctly.
F-002 (P3): digest.go admin-auth endpoints no longer leak internal errors
- PreviewDigest (GET /api/v1/digest/preview) and SendDigest
(POST /api/v1/digest/send) previously wrote err.Error() into the HTTP
response body on 500s. Replace with slog.Error server-side logging plus
a generic "internal error" response body, matching the house pattern
in certificates.go and export.go.
F-003 (P4): three blocking time.Sleep sites now honor ctx cancellation
- internal/connector/issuer/acme/acme.go:672 (DNS-01 propagation wait)
now runs under a select{case <-ctx.Done(): CleanUp + return ctx.Err();
case <-time.After(d):} so graceful shutdown doesn't get stuck behind
the propagation delay.
- internal/connector/issuer/acme/acme.go:786 (dns-persist-01 propagation
wait) same pattern, returns ctx.Err() on cancel.
- cmd/agent/main.go:272 (polling backoff inside the heartbeat loop) now
wraps the sleep in select{case <-ctx.Done(): continue; case <-time.After(backoff):}
so the outer <-ctx.Done() case on the parent loop fires cleanly.
Verification: build, vet, and race-enabled short tests green across all
55+ packages. govulncheck reports zero vulnerabilities in the code path.
No migration needed — F-001 reuses the existing 000012 composite index.
No frontend changes.
|
||
|
shankar0123
|
675b87ba63 |
I-005: notification retry loop + dead-letter queue
Critical alerts can no longer be silently dropped by a transient
notifier failure. Failed notification attempts now ride an exponential
backoff retry loop, with a 5-attempt budget before promotion to the
dead-letter queue for operator intervention.
Schema (migration 000016, idempotent):
- retry_count INTEGER NOT NULL DEFAULT 0
- next_retry_at TIMESTAMPTZ
- last_error TEXT
- idx_notification_events_retry_sweep partial index
(next_retry_at) WHERE status='failed' AND next_retry_at IS NOT NULL
Dead rows clear next_retry_at so the index stops matching them.
Service contract:
- NotificationService.RetryFailedNotifications drives 2^n-minute
exponential backoff capped at 1h (notifRetryBackoffCap) with
5-attempt budget (notifRetryMaxAttempts).
- Exhaustion (RetryCount >= notifRetryMaxAttempts-1) promotes to
status='dead' via MarkAsDead.
- Non-terminal failures record via RecordFailedAttempt.
- Success path promotes to 'sent' without touching retry_count
(audit preserves "delivered on attempt N").
- Missing-notifier branch defensively promotes to 'sent' to avoid
wedging a row on a deleted channel.
- RequeueNotification operator escape hatch atomically resets
retry_count -> 0, next_retry_at -> NULL, last_error -> NULL,
status -> pending via notifRepo.Requeue.
Scheduler:
- New always-on notificationRetryLoop wired into the base loop set at
CERTCTL_NOTIFICATION_RETRY_INTERVAL (default 2m).
- sync/atomic.Bool idempotency guard.
- sync.WaitGroup shutdown drain via WaitForCompletion.
StatsService:
- SetNotifRepo setter pattern preserves 9 pre-existing
NewStatsService call sites (main.go + stats_test.go + 8 digest
tests) without touching the constructor signature.
- DashboardSummary.NotificationsDead populated via
notifRepo.CountByStatus(ctx, "dead") — nil-safe when unwired
(reports zero on systems without a notification repository).
- CountByStatus error is non-fatal (dashboard summary is
best-effort for this field).
- Prometheus certctl_notification_dead_total counter emitted from
the same snapshot.
Handler:
- New POST /api/v1/notifications/{id}/requeue endpoint.
- dead status surfaces to MCP + CLI.
Frontend:
- NotificationsPage gains two-tab toolbar ("All" / "Dead letter")
with queryKey: ['notifications', activeTab] so switching tabs
doesn't serve stale data until the 30s refetch.
- Dead rows surface "Retry {n}/5" + truncated last_error with
full-text title tooltip.
- Requeue mutation wrapped as
mutationFn: (id: string) => requeueNotification(id)
to prevent react-query v5's positional context argument from
leaking into the API client — pinned against future refactors
by strict-match toHaveBeenCalledWith('notif-dead-001') in
NotificationsPage.test.tsx:181.
Closes I-005.
|