mirror of
https://github.com/shankar0123/certctl.git
synced 2026-06-07 19:31:31 +00:00
6b5af27546
Closes the 2026-04-25 audit's final-closure cluster. Score 51/55 -> 54/55
(98% closed); deferred 4/7 -> 7/7 (100%). All severity-graded findings now
closed except M-029 (frontend per-PR migration backlog, by design incremental).
L-004 (CWE-924) — dual-key API rotation overlap window:
internal/config/config.go::ParseNamedAPIKeys rewritten to allow same-name
duplicate entries iff admin flag matches. Mismatched-admin entries rejected
at startup (privilege escalation guard); exact (name,key) duplicates rejected
(typo guard — rotation requires DIFFERENT keys under the same name). Startup
INFO log per name with multiple entries surfaces the active rotation window.
NewAuthWithNamedKeys was already shaped correctly (constant-time hash compare
across all entries, same UserKey + AdminKey for either bearer); Bundle B's
M-025 per-user rate-limit bucket and audit-trail actor inherit consistency
across the rollover automatically. 8 new tests pin the contract end-to-end.
docs/security.md::API key rotation walks the 6-step zero-downtime rollover.
D-003 — Mutation testing wired:
security-deep-scan.yml gets a go-mutesting step covering ./internal/crypto/...,
./internal/pkcs7/..., ./internal/connector/issuer/local/... with per-package
summary lines extracted into go-mutesting.txt artefact.
D-007 — Frontend semgrep wired (recon found Bundle 7's wiring claim was false):
security-deep-scan.yml gets a 'semgrep p/react-security' step running
returntocorp/semgrep:latest --config=p/react-security against /src/web/src;
results uploaded as semgrep-react.json.
D-004 + D-005 — Operator runbook published:
docs/testing-strategy.md (NEW) consolidates per-tool local-run procedures,
acceptance thresholds, and triage paths for go-mutesting, ZAP baseline DAST,
testssl.sh, and semgrep p/react-security. Closes the 'wired CI-only, no
local-run validation' framing for D-004/D-005 by giving operators the same
commands the CI workflow runs.
Verification:
gofmt -l no diff
go vet ./internal/config/... ./internal/api/middleware/... clean
go test -short -count=1 ./internal/config/... ./internal/api/middleware/... PASS
python3 -c 'yaml.safe_load(...)' YAML OK
G-3 env-var docs guard no phantom env-vars
Audit deliverables:
audit-report.md: L-004 + D-003/4/5/7 boxes flipped [x]; score 51/55 -> 54/55
findings.yaml: 5 status flips; new bundle-G-final-closure closure_log entry
CHANGELOG.md: Bundle G entry under [unreleased]; supersedes Bundle E + F
L-004-deferred framing
170 lines
7.6 KiB
Markdown
170 lines
7.6 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.
|
|
|
|
## API key rotation
|
|
|
|
**Audit reference:** L-004. CWE-924 (improper enforcement of message integrity during transmission in a communication channel) — operator UX variant.
|
|
|
|
certctl's API keys are configured via the `CERTCTL_API_KEYS_NAMED` env var
|
|
(format `name1:key1,name2:key2:admin`) and parsed at startup into an
|
|
in-memory list. There is no DB-resident key store, no GUI, no `/api/v1/keys`
|
|
endpoint — the env var IS the key inventory.
|
|
|
|
Pre-Bundle-G the env var rejected duplicate names, so rotating a key
|
|
required: stop accepting OLDKEY → restart → roll NEWKEY out. Any client
|
|
polling against OLDKEY during the restart window hit a 401.
|
|
|
|
Bundle G adds a **double-key rotation window**: two entries can share a
|
|
name during the rollover, and both keys validate. Operators run the
|
|
rotation as:
|
|
|
|
1. **Generate the new key.** `openssl rand -hex 32` produces a 256-bit
|
|
value with sufficient entropy.
|
|
|
|
2. **Append the new entry to `CERTCTL_API_KEYS_NAMED`** alongside the
|
|
existing one:
|
|
```
|
|
CERTCTL_API_KEYS_NAMED="alice:OLDKEY:admin,alice:NEWKEY:admin"
|
|
```
|
|
Both entries MUST carry the same admin flag — startup fails loud if
|
|
they don't (a non-admin shouldn't share an identity with an admin).
|
|
|
|
3. **Restart certctl.** A startup INFO log confirms the rotation window
|
|
is active:
|
|
```
|
|
INFO api-key rotation window active name=alice entries=2 see=docs/security.md::api-key-rotation
|
|
```
|
|
|
|
4. **Roll the new key out to all clients.** Both keys validate during
|
|
this phase. Audit-trail actor + per-user rate-limit bucket stay
|
|
consistent across the rollover (both entries produce the same
|
|
`UserKey` context value, the shared name).
|
|
|
|
5. **Remove the old entry** from `CERTCTL_API_KEYS_NAMED`:
|
|
```
|
|
CERTCTL_API_KEYS_NAMED="alice:NEWKEY:admin"
|
|
```
|
|
|
|
6. **Restart certctl.** OLDKEY now fails with 401. Rotation complete.
|
|
|
|
The rotation window has no operator-set timeout — it lasts for as long
|
|
as both entries are in the env var. Best practice is a 24-72h window
|
|
covering a full deploy cadence; if a client hasn't rolled to NEWKEY by
|
|
the end of step 4, extend the window before step 5.
|
|
|
|
### What the contract guarantees
|
|
|
|
- Two entries with the same `name`: **allowed** if both have the same
|
|
`admin` flag.
|
|
- Two entries with the same `name` but mismatched admin: **rejected at
|
|
startup** (privilege escalation guard).
|
|
- Two entries with the same `(name, key)` pair: **rejected at startup**
|
|
(typo guard — rotation requires DIFFERENT keys under the same name).
|
|
- Single-entry steady state: unchanged from pre-Bundle-G behavior.
|
|
|
|
### What the contract does NOT do
|
|
|
|
- **No automatic expiration of OLDKEY.** The operator removes the entry
|
|
in step 5; certctl doesn't track timestamps. A future enhancement
|
|
could add a `rotated_at` annotation if operators ask for it.
|
|
- **No GUI / API for key management.** Keys are env-var only by design;
|
|
building a key-management surface is a separate feature project.
|
|
- **No revocation list.** If a key leaks, the only path is to remove it
|
|
from the env var and restart. That's appropriate for a small env-var
|
|
inventory; it would not scale to a per-user-key-issued model.
|
|
|
|
## Reporting a vulnerability
|
|
|
|
Email `certctl@proton.me`. Coordinated disclosure preferred; we will
|
|
acknowledge within 72h.
|