From 3d15a3e5af1561ceb01b174a1943a631656a9d63 Mon Sep 17 00:00:00 2001 From: shankar0123 Date: Thu, 30 Apr 2026 04:55:06 +0000 Subject: [PATCH] =?UTF-8?q?feat(ocsp):=20RFC=206960=20=C2=A74.4.1=20nonce?= =?UTF-8?q?=20extension=20support=20=E2=80=94=20echo=20client=20nonce=20in?= =?UTF-8?q?=20response,=20reject=20malformed?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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). --- .../api/handler/certificate_handler_test.go | 16 ++ internal/api/handler/certificates.go | 31 ++- internal/connector/issuer/interface.go | 6 + internal/connector/issuer/local/local.go | 17 ++ internal/service/ca_operations.go | 18 ++ internal/service/certificate.go | 12 +- internal/service/issuer_adapter.go | 1 + internal/service/ocsp_counters.go | 110 ++++++++++ internal/service/ocsp_nonce.go | 170 +++++++++++++++ internal/service/ocsp_nonce_test.go | 201 ++++++++++++++++++ internal/service/renewal.go | 5 + 11 files changed, 584 insertions(+), 3 deletions(-) create mode 100644 internal/service/ocsp_counters.go create mode 100644 internal/service/ocsp_nonce.go create mode 100644 internal/service/ocsp_nonce_test.go diff --git a/internal/api/handler/certificate_handler_test.go b/internal/api/handler/certificate_handler_test.go index 2a83f2b..5dedd27 100644 --- a/internal/api/handler/certificate_handler_test.go +++ b/internal/api/handler/certificate_handler_test.go @@ -38,6 +38,7 @@ type MockCertificateService struct { GetRevokedCertificatesFn func(ctx context.Context) ([]*domain.CertificateRevocation, error) GenerateDERCRLFn func(ctx context.Context, issuerID string) ([]byte, error) GetOCSPResponseFn func(ctx context.Context, issuerID string, serialHex string) ([]byte, error) + GetOCSPResponseWithNonceFn func(ctx context.Context, issuerID string, serialHex string, nonce []byte) ([]byte, error) GetCertificateDeploymentsFn func(ctx context.Context, certID string) ([]domain.DeploymentTarget, error) } @@ -125,6 +126,21 @@ func (m *MockCertificateService) GetOCSPResponse(ctx context.Context, issuerID s return nil, nil } +// GetOCSPResponseWithNonce — production hardening II Phase 1. +// Falls through to the legacy GetOCSPResponseFn when a per-test +// nonce-aware override isn't set, mirroring the behavior of the +// real CertificateService where the nonce-less variant is just a +// nil-nonce wrapper around the nonce-aware path. +func (m *MockCertificateService) GetOCSPResponseWithNonce(ctx context.Context, issuerID string, serialHex string, nonce []byte) ([]byte, error) { + if m.GetOCSPResponseWithNonceFn != nil { + return m.GetOCSPResponseWithNonceFn(ctx, issuerID, serialHex, nonce) + } + if m.GetOCSPResponseFn != nil { + return m.GetOCSPResponseFn(ctx, issuerID, serialHex) + } + return nil, nil +} + func (m *MockCertificateService) ListCertificatesWithFilter(ctx context.Context, filter *repository.CertificateFilter) ([]domain.ManagedCertificate, int, error) { if m.ListCertificatesWithFilterFn != nil { return m.ListCertificatesWithFilterFn(ctx, filter) diff --git a/internal/api/handler/certificates.go b/internal/api/handler/certificates.go index a9fd449..cda1487 100644 --- a/internal/api/handler/certificates.go +++ b/internal/api/handler/certificates.go @@ -17,6 +17,7 @@ import ( "github.com/shankar0123/certctl/internal/api/middleware" "github.com/shankar0123/certctl/internal/domain" "github.com/shankar0123/certctl/internal/repository" + "github.com/shankar0123/certctl/internal/service" ) // CertificateService defines the service interface for certificate operations. @@ -34,6 +35,11 @@ type CertificateService interface { GetRevokedCertificates(ctx context.Context) ([]*domain.CertificateRevocation, error) GenerateDERCRL(ctx context.Context, issuerID string) ([]byte, error) GetOCSPResponse(ctx context.Context, issuerID string, serialHex string) ([]byte, error) + // GetOCSPResponseWithNonce is the nonce-aware variant added in + // production hardening II Phase 1. When nonce is non-nil, the + // responder echoes it in the response per RFC 6960 §4.4.1. A nil + // nonce produces a response without the nonce extension. + GetOCSPResponseWithNonce(ctx context.Context, issuerID string, serialHex string, nonce []byte) ([]byte, error) GetCertificateDeployments(ctx context.Context, certID string) ([]domain.DeploymentTarget, error) } @@ -687,11 +693,34 @@ func (h CertificateHandler) HandleOCSPPost(w http.ResponseWriter, r *http.Reques return } + // Production hardening II Phase 1: extract the optional RFC 6960 + // §4.4.1 nonce extension from the request. golang.org/x/crypto/ocsp + // doesn't expose the request's extensions, so we walk the raw DER + // ourselves via service.ParseOCSPRequestNonce. + // + // Failure modes: + // - no nonce (most relying parties): nonce=nil, present=false, + // err=nil -> proceed without echoing. + // - well-formed nonce <= 32 bytes: nonce=bytes, present=true, + // err=nil -> plumb through GetOCSPResponseWithNonce. + // - malformed nonce (empty or > 32 bytes): err=ErrOCSPNonceMalformed + // -> respond with the OCSP "unauthorized" status (RFC 6960 §2.3 + // status code 6) rather than echoing potentially-malicious bytes. + nonce, _, nonceErr := service.ParseOCSPRequestNonce(body) + if errors.Is(nonceErr, service.ErrOCSPNonceMalformed) { + w.Header().Set("Content-Type", "application/ocsp-response") + w.WriteHeader(http.StatusOK) + // ocsp.UnauthorizedErrorResponse is the canonical pre-built + // error response (status 6) per RFC 6960 §4.2.1. + w.Write(ocsp.UnauthorizedErrorResponse) + return + } + // Reuse the existing service path. The serial extracted from the // parsed OCSPRequest is converted to hex (the on-disk format for // certctl serials matches certificate.SerialNumber.Text(16)). serialHex := fmt.Sprintf("%x", ocspReq.SerialNumber) - derBytes, err := h.svc.GetOCSPResponse(r.Context(), issuerID, serialHex) + derBytes, err := h.svc.GetOCSPResponseWithNonce(r.Context(), issuerID, serialHex, nonce) if err != nil { errMsg := err.Error() if strings.Contains(errMsg, "not found") { diff --git a/internal/connector/issuer/interface.go b/internal/connector/issuer/interface.go index c967692..55e7208 100644 --- a/internal/connector/issuer/interface.go +++ b/internal/connector/issuer/interface.go @@ -123,4 +123,10 @@ type OCSPSignRequest struct { RevocationReason int ThisUpdate time.Time NextUpdate time.Time + // Nonce — RFC 6960 §4.4.1 OCSP-nonce extension echo. When non-nil, + // the responder MUST include this value in the response's + // singleExtensions field. When nil, the response MUST NOT carry + // a nonce extension (back-compat with relying parties that don't + // understand it). Production hardening II Phase 1. + Nonce []byte } diff --git a/internal/connector/issuer/local/local.go b/internal/connector/issuer/local/local.go index d856d5c..c681dc6 100644 --- a/internal/connector/issuer/local/local.go +++ b/internal/connector/issuer/local/local.go @@ -1050,6 +1050,23 @@ func (c *Connector) SignOCSPResponse(ctx context.Context, req issuer.OCSPSignReq template.Status = ocsp.Unknown } + // Production hardening II Phase 1.4: echo the request's nonce in + // the response's singleExtensions field per RFC 6960 §4.4.1. + // The handler walks the inbound request's extensions and populates + // req.Nonce when a well-formed nonce extension is present; we just + // re-marshal it here as the extnValue OCTET STRING. + if len(req.Nonce) > 0 { + nonceExtnValue, err := asn1.Marshal(req.Nonce) + if err != nil { + return nil, fmt.Errorf("marshal OCSP nonce extension: %w", err) + } + template.ExtraExtensions = append(template.ExtraExtensions, pkix.Extension{ + Id: asn1.ObjectIdentifier{1, 3, 6, 1, 5, 5, 7, 48, 1, 2}, // id-pkix-ocsp-nonce + Critical: false, // RFC 6960 §4.4 — nonce is non-critical + Value: nonceExtnValue, + }) + } + // ocsp.CreateResponse(issuer, responder, template, signer): // - issuer: always c.caCert (the CA that issued the cert // being checked, NOT the responder cert) diff --git a/internal/service/ca_operations.go b/internal/service/ca_operations.go index e2e5fab..ae5c4fc 100644 --- a/internal/service/ca_operations.go +++ b/internal/service/ca_operations.go @@ -98,7 +98,21 @@ func (s *CAOperationsSvc) GenerateDERCRL(ctx context.Context, issuerID string) ( } // GetOCSPResponse generates a signed OCSP response for the given certificate serial. +// Back-compat wrapper around GetOCSPResponseWithNonce: passes nil nonce, +// which produces a response without the RFC 6960 §4.4.1 nonce extension. +// Older callers that don't carry a nonce see no behavior change. func (s *CAOperationsSvc) GetOCSPResponse(ctx context.Context, issuerID string, serialHex string) ([]byte, error) { + return s.GetOCSPResponseWithNonce(ctx, issuerID, serialHex, nil) +} + +// GetOCSPResponseWithNonce generates a signed OCSP response for the +// given certificate serial. When nonce is non-nil, the responder echoes +// it in the response per RFC 6960 §4.4.1 (nonce extension). nil nonce +// omits the extension entirely (back-compat with relying parties that +// do not include one). +// +// Production hardening II Phase 1. +func (s *CAOperationsSvc) GetOCSPResponseWithNonce(ctx context.Context, issuerID string, serialHex string, nonce []byte) ([]byte, error) { if s.revocationRepo == nil { return nil, fmt.Errorf("revocation repository not configured") } @@ -133,6 +147,7 @@ func (s *CAOperationsSvc) GetOCSPResponse(ctx context.Context, issuerID string, CertStatus: 0, // good — short-lived exemption ThisUpdate: now, NextUpdate: now.Add(1 * time.Hour), + Nonce: nonce, }) } } @@ -150,6 +165,7 @@ func (s *CAOperationsSvc) GetOCSPResponse(ctx context.Context, issuerID string, RevocationReason: domain.CRLReasonCode(domain.RevocationReason(rev.Reason)), ThisUpdate: now, NextUpdate: now.Add(1 * time.Hour), + Nonce: nonce, }) } @@ -175,6 +191,7 @@ func (s *CAOperationsSvc) GetOCSPResponse(ctx context.Context, issuerID string, CertStatus: 2, // unknown ThisUpdate: now, NextUpdate: now.Add(1 * time.Hour), + Nonce: nonce, }) } } @@ -185,5 +202,6 @@ func (s *CAOperationsSvc) GetOCSPResponse(ctx context.Context, issuerID string, CertStatus: 0, // good ThisUpdate: now, NextUpdate: now.Add(1 * time.Hour), + Nonce: nonce, }) } diff --git a/internal/service/certificate.go b/internal/service/certificate.go index afacd9a..1849376 100644 --- a/internal/service/certificate.go +++ b/internal/service/certificate.go @@ -521,12 +521,20 @@ func (s *CertificateService) GenerateDERCRL(ctx context.Context, issuerID string } // GetOCSPResponse generates a signed OCSP response for the given certificate serial. -// Delegates to CAOperationsSvc. +// Back-compat wrapper around GetOCSPResponseWithNonce; passes nil nonce so the +// response omits the RFC 6960 §4.4.1 nonce extension. func (s *CertificateService) GetOCSPResponse(ctx context.Context, issuerID string, serialHex string) ([]byte, error) { + return s.GetOCSPResponseWithNonce(ctx, issuerID, serialHex, nil) +} + +// GetOCSPResponseWithNonce generates a signed OCSP response and (when +// nonce != nil) echoes the nonce in the response per RFC 6960 §4.4.1. +// Production hardening II Phase 1. +func (s *CertificateService) GetOCSPResponseWithNonce(ctx context.Context, issuerID string, serialHex string, nonce []byte) ([]byte, error) { if s.caSvc == nil { return nil, fmt.Errorf("CA operations service not configured") } - return s.caSvc.GetOCSPResponse(ctx, issuerID, serialHex) + return s.caSvc.GetOCSPResponseWithNonce(ctx, issuerID, serialHex, nonce) } // GetCertificateDeployments returns all deployment targets for a certificate (M20). diff --git a/internal/service/issuer_adapter.go b/internal/service/issuer_adapter.go index 33df486..4e616ac 100644 --- a/internal/service/issuer_adapter.go +++ b/internal/service/issuer_adapter.go @@ -104,6 +104,7 @@ func (a *IssuerConnectorAdapter) SignOCSPResponse(ctx context.Context, req OCSPS RevocationReason: req.RevocationReason, ThisUpdate: req.ThisUpdate, NextUpdate: req.NextUpdate, + Nonce: req.Nonce, // RFC 6960 §4.4.1 echo (production hardening II Phase 1) }) } diff --git a/internal/service/ocsp_counters.go b/internal/service/ocsp_counters.go new file mode 100644 index 0000000..5ecf717 --- /dev/null +++ b/internal/service/ocsp_counters.go @@ -0,0 +1,110 @@ +package service + +import "sync/atomic" + +// Production hardening II Phase 1.3 — OCSP per-request counters. +// +// Mirrors the pattern in est_counters.go and scep_counters.go: +// sync/atomic primitives keep the hot path lock-free, while a snapshot +// accessor produces a stable map for the Prometheus exposition handler +// (Phase 8). +// +// Counter labels are stable strings — the Prometheus phase converts +// them into `certctl_ocsp_