mirror of https://github.com/shankar0123/certctl.git synced 2026-06-07 21:51:30 +00:00

Files

T

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)

2026-04-27 00:08:25 +00:00

4.5 KiB

Raw Blame History

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:

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.
Confirm the relying party honors the extension. OpenSSL ≥ 1.1.0, Firefox, and Chrome 84+ all enforce Must-Staple. Older clients silently ignore it.
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. 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 and the accompanying tests for the format spec.

Authentication surface

Bundle B / M-002. Two layers decide auth-exempt status:

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).
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.

4.5 KiB Raw Blame History