mirror of
https://github.com/shankar0123/certctl.git
synced 2026-06-07 19:31:31 +00:00
shankar0123
62a412c488
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)
98 lines
4.5 KiB
Markdown
98 lines
4.5 KiB
Markdown
# certctl Security Posture & Operator Guidance
|
|
|
|
This document collects the operator-facing security guidance that the source
|
|
code's per-finding comment blocks reference. Each section names the audit
|
|
finding it closes, the threat model, and the operator action required (if
|
|
any).
|
|
|
|
## OCSP responder availability
|
|
|
|
**Audit reference:** Bundle C / M-020. CWE-770 (uncontrolled resource
|
|
consumption); RFC 6960 (OCSP); RFC 7633 (Must-Staple).
|
|
|
|
certctl ships an OCSP responder at `/.well-known/pki/ocsp/{issuer_id}/{serial}`
|
|
that signs a fresh response per request. Pre-Bundle-C the unauth handler
|
|
chain had no rate limit, so an attacker could DoS the responder and force
|
|
fail-open relying parties to accept revoked certificates as valid. Bundle C
|
|
adds the same per-key rate limiter to the unauth chain that the authenticated
|
|
chain has used since Bundle B. Per-IP keying applies because OCSP traffic is
|
|
unauthenticated.
|
|
|
|
The rate limiter alone does not solve the underlying revocation-bypass risk.
|
|
**The architectural fix is for issued certificates to carry the OCSP
|
|
Must-Staple TLS Feature extension** (RFC 7633, OID 1.3.6.1.5.5.7.1.24). When
|
|
present, conforming TLS clients refuse to negotiate a session unless the
|
|
server staples a fresh signed OCSP response in the TLS handshake. This shifts
|
|
revocation enforcement from the client's discretion (which most fail-open by
|
|
default) to a hard requirement that the connection cannot complete without
|
|
proof of non-revocation.
|
|
|
|
### Operator action
|
|
|
|
For certificates issued to systems where revocation correctness matters:
|
|
|
|
1. **Configure the issuer profile to set `must-staple: true`.** Out-of-the-box
|
|
profiles in `migrations/seed.sql` do not set this; operators add it at
|
|
profile-creation time via the API or by editing seed data.
|
|
2. **Confirm the relying party honors the extension.** OpenSSL ≥ 1.1.0,
|
|
Firefox, and Chrome 84+ all enforce Must-Staple. Older clients silently
|
|
ignore it.
|
|
3. **Confirm the deployment target is configured for OCSP stapling** so the
|
|
server can actually deliver the stapled response in the handshake.
|
|
- **nginx:** `ssl_stapling on; ssl_stapling_verify on;`
|
|
- **Apache:** `SSLUseStapling on`
|
|
- **HAProxy:** `set ssl ocsp-response /path/to/response.der`
|
|
- **Envoy:** `ocsp_staple_policy: must_staple`
|
|
|
|
### What this does NOT cover
|
|
|
|
- **CRL fallback.** Must-Staple does not affect CRL behavior. Operators with
|
|
CRL-based relying parties should use the rate-limit + caching defense
|
|
alone; there is no client-side equivalent to Must-Staple for CRLs.
|
|
- **Self-issued certs in air-gapped networks.** When the relying party
|
|
cannot reach the OCSP responder at all (the threat model the audit
|
|
cited), Must-Staple is the only mechanism that closes the bypass. CRL
|
|
distribution similarly requires the relying party to fetch the CRL,
|
|
which is also subject to the same network-availability concern.
|
|
|
|
## Postgres transport encryption
|
|
|
|
See [docs/database-tls.md](database-tls.md). Bundle B / M-018.
|
|
|
|
## Encryption at rest
|
|
|
|
Bundle B / M-001. PBKDF2-SHA256 at 600,000 rounds (OWASP 2024 Password
|
|
Storage Cheat Sheet floor) for the operator-supplied passphrase that
|
|
derives the AES-256-GCM key for sensitive config columns. v3 blob format
|
|
with a per-ciphertext random salt; v1/v2 read fallback for legacy rows.
|
|
See [internal/crypto/encryption.go](../internal/crypto/encryption.go) and
|
|
the accompanying tests for the format spec.
|
|
|
|
## Authentication surface
|
|
|
|
Bundle B / M-002. Two layers decide auth-exempt status:
|
|
|
|
1. **Router layer:** `internal/api/router/router.go::AuthExemptRouterRoutes`
|
|
— the 4 endpoints registered via direct `r.mux.Handle` without going
|
|
through the middleware chain (`/health`, `/ready`, `/api/v1/auth/info`,
|
|
`/api/v1/version`).
|
|
2. **Dispatch layer:** `internal/api/router/router.go::AuthExemptDispatchPrefixes`
|
|
— URL-prefix routing in `cmd/server/main.go::buildFinalHandler` for
|
|
`/.well-known/pki/*`, `/.well-known/est/*`, and `/scep[/...]*`.
|
|
|
|
Both lists have AST-walking regression tests (`auth_exempt_test.go`) that
|
|
fail CI if a new bypass lands without an updating the documented constant.
|
|
|
|
## Per-user rate limiting
|
|
|
|
Bundle B / M-025. Authenticated callers are bucketed by API-key name;
|
|
unauthenticated callers (probes, OCSP relying parties, EST/SCEP enrollees)
|
|
are bucketed by source IP. `RPS` and `BurstSize` are per-key budgets.
|
|
`PerUserRPS` / `PerUserBurstSize` give authenticated clients a separate
|
|
budget when set non-zero.
|
|
|
|
## Reporting a vulnerability
|
|
|
|
Email `certctl@proton.me`. Coordinated disclosure preferred; we will
|
|
acknowledge within 72h.
|