security: scope revocation unique index to (issuer_id, serial_number) (fixes H-1)

RFC 5280 §5.2.3 defines certificate serial number uniqueness per issuing CA,
not globally. The prior unique index on `certificate_revocations.serial_number`
enforced a stricter invariant than the spec: with 12 issuer connectors (Local
CA, ACME, Vault, step-ca, OpenSSL, DigiCert, Sectigo, Google CAS, AWS ACM PCA,
Entrust, GlobalSign, EJBCA), two distinct certificates legitimately issued by
different CAs can share a serial number. Recording a revocation for the second
collision silently dropped via `ON CONFLICT DO NOTHING`, leaving the second
cert persistently absent from OCSP/CRL responses.

Changes:

- Migration 000012 drops `idx_certificate_revocations_serial` and creates
  `idx_certificate_revocations_issuer_serial` UNIQUE ON (issuer_id,
  serial_number). Adds a non-unique `idx_certificate_revocations_serial_lookup`
  to preserve the serial-only fast path for OCSP/CRL probes that already know
  the issuer scope.
- `CertificateRevocationRepository.Create` targets the new composite key in
  `ON CONFLICT` — same-issuer idempotency preserved, cross-issuer collisions
  now recorded as distinct rows.
- `GetBySerial(serial)` renamed `GetByIssuerAndSerial(issuerID, serial)` on
  the interface and Postgres impl. All callers (OCSP responder, CRL
  generator, short-lived-cert exemption check) already have `issuerID` in
  scope because the protocol paths carry it (`/api/v1/ocsp/{issuer_id}/{serial}`,
  `/api/v1/crl/{issuer_id}`).
- Repository integration test added: `TestRevocationRepository_CrossIssuerSerialCollision`
  asserts that serial `CAFEBABE01` can be stored under two issuers
  simultaneously, that lookups return the correct row per (issuer, serial),
  and that same-issuer idempotency still works (re-inserting (issuer, serial)
  does not error and does not duplicate).
- Existing tests and service/integration mocks updated for the rename.

Wire-format invariants preserved: CRL DER bytes, OCSP response bytes, and
AES-256-GCM config encryption are unaffected — this change touches only
revocation-record uniqueness scope.

CWE-664.
This commit is contained in:
shankar0123
2026-04-16 21:49:59 +00:00
parent f549a7aa79
commit 387fb555ac
8 changed files with 184 additions and 21 deletions
@@ -0,0 +1,12 @@
-- Rollback Migration 000012: Restore global-serial uniqueness.
--
-- Reverts to the pre-000012 behavior: uniqueness on `serial_number` alone.
-- Operators must ensure no duplicate serial_numbers exist across different
-- issuers before rolling back, otherwise the unique-index creation will fail.
DROP INDEX IF EXISTS idx_certificate_revocations_serial_lookup;
DROP INDEX IF EXISTS idx_certificate_revocations_issuer_serial;
CREATE UNIQUE INDEX IF NOT EXISTS idx_certificate_revocations_serial
ON certificate_revocations(serial_number);
@@ -0,0 +1,31 @@
-- Migration 000012: Scope Revocation Uniqueness to (issuer_id, serial_number)
--
-- RFC 5280 §5.2.3 defines certificate serial number uniqueness per issuing CA.
-- The prior global-unique index on `certificate_revocations.serial_number` was
-- too strict: certctl supports multiple issuer connectors (Local CA, Vault,
-- DigiCert, Sectigo, Google CAS, AWS ACM PCA, step-ca, Entrust, GlobalSign,
-- EJBCA, ACME, OpenSSL), and different CAs legitimately issue distinct certs
-- that share a serial-number value. Under the old index, recording a
-- revocation for such a collision silently dropped via ON CONFLICT DO NOTHING.
--
-- This migration scopes uniqueness to the (issuer_id, serial_number) pair,
-- which matches RFC 5280 and the revocation-recording call site's intent
-- (see RevocationSvc.RevokeCertificateWithActor, which already populates
-- IssuerID at Create time).
--
-- Duplicate detection: if any row pairs exist with identical (issuer_id,
-- serial_number), the unique-index creation will fail — this is intentional.
-- Operators must resolve duplicates manually before re-running the migration.
-- Drop the overly broad global-serial unique index.
DROP INDEX IF EXISTS idx_certificate_revocations_serial;
-- Recreate uniqueness scoped to (issuer_id, serial_number) per RFC 5280 §5.2.3.
CREATE UNIQUE INDEX IF NOT EXISTS idx_certificate_revocations_issuer_serial
ON certificate_revocations(issuer_id, serial_number);
-- Preserve fast serial-only lookup for OCSP/CRL paths that search within a
-- known issuer scope. Non-unique — uniqueness is enforced by the composite
-- index above.
CREATE INDEX IF NOT EXISTS idx_certificate_revocations_serial_lookup
ON certificate_revocations(serial_number);