Files
certctl/internal/api/middleware/cors_test.go
T
shankar0123 30f9f1e712 Bundle B: Auth & transport surface tightening — 5 findings closed
Closes M-001 + M-002 + M-013 + M-018 + M-025 from
comprehensive-audit-2026-04-25.

M-001 (CWE-916) — PBKDF2 100k -> 600k via v3 blob format
  internal/crypto/encryption.go:
    - New v3Magic (0x03), pbkdf2IterationsV3 (600,000 — OWASP 2024
      Password Storage Cheat Sheet floor), v3SaltSize (16 bytes),
      deriveKeyWithSaltV3 helper.
    - EncryptIfKeySet now unconditionally writes v3:
        magic(0x03) || salt(16) || nonce(12) || ciphertext+tag
    - DecryptIfKeySet falls through v3 -> v2 -> v1 with AEAD verification
      at each step. Wrong-passphrase v3 reads cannot be silently
      misattributed to v2/v1.
    - IsLegacyFormat updated to recognize 0x03 as non-legacy.
  internal/crypto/encryption_v3_test.go (NEW, 7 tests):
    V3 round-trip / V2 read-fallback against deterministic v2 fixture /
    V3 wrong-passphrase fails / V3-vs-V2 dispatch order / V2 vs V3 keys
    differ for same (passphrase, salt) / iteration-count pin at OWASP
    2024 floor / IsLegacyFormat-recognises-V3.
  Coverage internal/crypto: 86.7% -> 88.2%.

M-002 (CWE-862) — Auth-exempt allowlist constants + AST regression test
  Recon found auth-exempt surface spans TWO layers (audit's claim was
  incomplete):
    Layer 1 (router.go direct r.mux.Handle):
      GET /health, GET /ready, GET /api/v1/auth/info, GET /api/v1/version
    Layer 2 (cmd/server/main.go::buildFinalHandler URL-prefix dispatch):
      /.well-known/pki/*, /.well-known/est/*, /scep[/...]*
  internal/api/router/router.go:
    - New AuthExemptRouterRoutes constant with per-entry justifications.
    - New AuthExemptDispatchPrefixes constant.
  internal/api/router/auth_exempt_test.go (NEW, 2 tests):
    AST-walks router.go for every direct mux.Handle call and asserts
    set equals AuthExemptRouterRoutes; reads source bytes of Register /
    RegisterFunc and asserts they still wrap with middleware.Chain.
  cmd/server/auth_exempt_test.go (NEW, 2 tests):
    14-case table test on buildFinalHandler asserting documented
    prefixes route to noAuthHandler and authenticated routes route to
    apiHandler; inverse-overlap pin proves no documented bypass shadows
    an authenticated prefix.

M-013 (CWE-942) — CORS deny-by-default verified-already-clean + pin
  Audit claim 'default allows all origins if env-var unset' was WRONG.
  internal/api/middleware/middleware.go::NewCORS already denies cross-
  origin requests when len(cfg.AllowedOrigins) == 0 (no
  Access-Control-Allow-Origin header is emitted, same-origin policy
  applies).
  internal/api/middleware/cors_test.go: +TestNewCORS_NilOriginsDeniesAll
  + TestNewCORS_M013_ContractDocumentedInOrder (5-case table test
  pinning the 3-arm dispatch contract).

M-018 (CWE-319 / PCI-DSS Req 4) — Postgres TLS opt-in toggle
  deploy/helm/certctl/values.yaml: new postgresql.tls.{mode,caSecretRef}
    operator-facing knobs. Default 'disable' preserves in-cluster pod-
    network behavior; PCI-scoped operators set verify-full.
  deploy/helm/certctl/templates/_helpers.tpl: certctl.databaseURL helper
    pipes postgresql.tls.mode into ?sslmode=.
  deploy/helm/certctl/templates/server-secret.yaml: uses the helper
    instead of hardcoded sslmode=disable.
  deploy/docker-compose.yml: CERTCTL_DATABASE_URL is now
    ${CERTCTL_DATABASE_URL:-...} so operators override without editing.
  docs/database-tls.md (NEW): operator runbook covering 4 deployment
    shapes, RDS verify-full example with PGSSLROOTCERT mount, and
    pg_stat_ssl verification query.
  helm template + helm lint clean.

M-025 (OWASP ASVS L2 §11.2.1) — Per-key rate limiting
  internal/api/middleware/middleware.go::NewRateLimiter rewritten from
  a single global tokenBucket to a keyedRateLimiter map keyed on
    'user:'+GetUser(ctx)  for authenticated callers
    'ip:'+RemoteAddr-host for unauthenticated
  - Empty UserKey strings treated as unauthenticated.
  - X-Forwarded-For intentionally NOT consulted (header-spoofing risk).
  - Create-on-demand bucket allocation under sync.RWMutex with double-
    check pattern.
  RateLimitConfig.PerUserRPS / PerUserBurstSize fields with env vars
    CERTCTL_RATE_LIMIT_PER_USER_RPS / CERTCTL_RATE_LIMIT_PER_USER_BURST
    allow per-user budgets distinct from per-IP.
  internal/api/middleware/ratelimit_keyed_test.go (NEW, 5 tests):
    TwoIPsHaveIndependentBuckets / SameUserDifferentIPsShareBucket /
    TwoUsersHaveIndependentBuckets / PerUserBudgetOverride /
    EmptyUserKeyTreatedAsAnonymous.
  Coverage internal/api/middleware: 82.1% -> 83.7%.

Audit deliverables:
  cowork/comprehensive-audit-2026-04-25/audit-report.md: score
    25/55 -> 30/55 closed (High 7/9, Medium 7/27 -> 12/27, Low 8/19).
  cowork/comprehensive-audit-2026-04-25/findings.yaml: 5 status flips
    open -> closed with closure notes citing the Bundle B mechanism.
  certctl/CHANGELOG.md: Bundle B section under [unreleased].

Verification:
  go test -count=1 -short ./...                     all green
  staticcheck on changed packages                   no new SA*/ST* hits
    (the 4 pre-existing SA1019 sites in cmd/server/main_test.go are
    Bundle 9 / M-028 partial closure leftovers tracked in Bundle C)
  helm template + helm lint                         clean
  internal/repository/postgres setup-fail            sandbox disk pressure,
    same on master HEAD before this branch — environmental, not Bundle B
2026-04-26 23:09:10 +00:00

347 lines
13 KiB
Go

package middleware
import (
"net/http"
"net/http/httptest"
"testing"
)
// Bundle B / Audit M-013 (CWE-942) regression pins.
//
// The audit-finding text reads: "CORS configuration default allows all
// origins if env-var unset". Phase 0 recon proves that claim is WRONG —
// internal/api/middleware/middleware.go::NewCORS already denies when
// len(cfg.AllowedOrigins) == 0 (no Access-Control-Allow-Origin header is
// emitted, so same-origin policy applies). Bundle B's M-013 closure is
// "verified-already-clean": these tests pin the deny-by-default contract
// in BOTH shapes (nil slice and empty slice) so a future refactor that
// inverts the default fails CI.
// TestNewCORS_NilOriginsDeniesAll pins the deny-by-default contract for
// the nil-slice shape (which is what propagates from a missing
// CERTCTL_CORS_ORIGINS env var via internal/config/config.go::getEnvList).
func TestNewCORS_NilOriginsDeniesAll(t *testing.T) {
mw := NewCORS(CORSConfig{AllowedOrigins: nil})
handler := mw(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusOK)
}))
req := httptest.NewRequest(http.MethodGet, "/api/v1/certificates", nil)
req.Header.Set("Origin", "https://attacker.example.com")
rr := httptest.NewRecorder()
handler.ServeHTTP(rr, req)
if got := rr.Header().Get("Access-Control-Allow-Origin"); got != "" {
t.Errorf("nil AllowedOrigins must NOT emit Access-Control-Allow-Origin, got %q", got)
}
if got := rr.Header().Get("Vary"); got != "" {
t.Errorf("nil AllowedOrigins must NOT emit Vary, got %q", got)
}
}
// TestNewCORS_M013_ContractDocumentedInOrder pins the documented dispatch
// order so a refactor cannot silently invert the cases:
//
// 1. len(AllowedOrigins) == 0 → deny (no CORS headers)
// 2. AllowedOrigins == ["*"] → allow all (Access-Control-Allow-Origin: *)
// 3. else → exact-match allowlist with Vary: Origin
//
// If a refactor accidentally falls through to the allow-all branch when
// AllowedOrigins is empty, this test fails on case 1.
func TestNewCORS_M013_ContractDocumentedInOrder(t *testing.T) {
cases := []struct {
name string
origins []string
incomingOrigin string
wantHeader string // "" means no header expected
}{
{"deny_empty_slice", []string{}, "https://app.example.com", ""},
{"deny_nil", nil, "https://app.example.com", ""},
{"allow_all_with_star", []string{"*"}, "https://app.example.com", "*"},
{"exact_allow_match", []string{"https://app.example.com"}, "https://app.example.com", "https://app.example.com"},
{"exact_deny_mismatch", []string{"https://app.example.com"}, "https://attacker.example.com", ""},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
mw := NewCORS(CORSConfig{AllowedOrigins: tc.origins})
handler := mw(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusOK)
}))
req := httptest.NewRequest(http.MethodGet, "/", nil)
req.Header.Set("Origin", tc.incomingOrigin)
rr := httptest.NewRecorder()
handler.ServeHTTP(rr, req)
if got := rr.Header().Get("Access-Control-Allow-Origin"); got != tc.wantHeader {
t.Errorf("got Access-Control-Allow-Origin=%q, want %q (incoming origin=%q)", got, tc.wantHeader, tc.incomingOrigin)
}
})
}
}
// TestNewCORS_EmptyOriginList denies CORS by default (secure default).
func TestNewCORS_EmptyOriginList(t *testing.T) {
mw := NewCORS(CORSConfig{AllowedOrigins: []string{}})
handler := mw(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusOK)
w.Write([]byte(`{"ok":true}`))
}))
req := httptest.NewRequest(http.MethodGet, "/api/v1/certificates", nil)
req.Header.Set("Origin", "https://evil.example.com")
rr := httptest.NewRecorder()
handler.ServeHTTP(rr, req)
// Response should be OK, but no CORS headers should be set
if rr.Code != http.StatusOK {
t.Fatalf("expected 200, got %d", rr.Code)
}
// Verify no CORS headers are present
if rr.Header().Get("Access-Control-Allow-Origin") != "" {
t.Errorf("expected no Access-Control-Allow-Origin header, got %q", rr.Header().Get("Access-Control-Allow-Origin"))
}
if rr.Header().Get("Vary") != "" {
t.Errorf("expected no Vary header, got %q", rr.Header().Get("Vary"))
}
}
// TestNewCORS_EmptyOriginList_Preflight denies preflight when empty allowlist.
func TestNewCORS_EmptyOriginList_Preflight(t *testing.T) {
mw := NewCORS(CORSConfig{AllowedOrigins: []string{}})
handler := mw(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusOK)
}))
req := httptest.NewRequest(http.MethodOptions, "/api/v1/certificates", nil)
req.Header.Set("Origin", "https://app.example.com")
rr := httptest.NewRecorder()
handler.ServeHTTP(rr, req)
// Preflight should return 204, but no CORS headers
if rr.Code != http.StatusNoContent {
t.Fatalf("expected 204, got %d", rr.Code)
}
// No CORS headers should be set
if rr.Header().Get("Access-Control-Allow-Origin") != "" {
t.Errorf("expected no Access-Control-Allow-Origin header, got %q", rr.Header().Get("Access-Control-Allow-Origin"))
}
}
// TestNewCORS_WildcardAllowsAll allows all origins with wildcard.
func TestNewCORS_WildcardAllowsAll(t *testing.T) {
mw := NewCORS(CORSConfig{AllowedOrigins: []string{"*"}})
handler := mw(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusOK)
}))
req := httptest.NewRequest(http.MethodGet, "/api/v1/certificates", nil)
req.Header.Set("Origin", "https://any-origin.example.com")
rr := httptest.NewRecorder()
handler.ServeHTTP(rr, req)
if rr.Code != http.StatusOK {
t.Fatalf("expected 200, got %d", rr.Code)
}
// Wildcard should set Access-Control-Allow-Origin: *
if rr.Header().Get("Access-Control-Allow-Origin") != "*" {
t.Errorf("expected Access-Control-Allow-Origin: *, got %q", rr.Header().Get("Access-Control-Allow-Origin"))
}
// Verify other CORS headers are present
if rr.Header().Get("Access-Control-Allow-Methods") == "" {
t.Errorf("expected Access-Control-Allow-Methods header")
}
}
// TestNewCORS_ExactMatchAllows allows only exact matches from allowlist.
func TestNewCORS_ExactMatchAllows(t *testing.T) {
mw := NewCORS(CORSConfig{AllowedOrigins: []string{"https://app.example.com", "https://admin.example.com"}})
handler := mw(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusOK)
}))
// Test 1: Origin in allowlist
req1 := httptest.NewRequest(http.MethodGet, "/api/v1/certificates", nil)
req1.Header.Set("Origin", "https://app.example.com")
rr1 := httptest.NewRecorder()
handler.ServeHTTP(rr1, req1)
if rr1.Header().Get("Access-Control-Allow-Origin") != "https://app.example.com" {
t.Errorf("expected https://app.example.com, got %q", rr1.Header().Get("Access-Control-Allow-Origin"))
}
if rr1.Header().Get("Vary") != "Origin" {
t.Errorf("expected Vary: Origin, got %q", rr1.Header().Get("Vary"))
}
// Test 2: Different origin in allowlist
req2 := httptest.NewRequest(http.MethodGet, "/api/v1/certificates", nil)
req2.Header.Set("Origin", "https://admin.example.com")
rr2 := httptest.NewRecorder()
handler.ServeHTTP(rr2, req2)
if rr2.Header().Get("Access-Control-Allow-Origin") != "https://admin.example.com" {
t.Errorf("expected https://admin.example.com, got %q", rr2.Header().Get("Access-Control-Allow-Origin"))
}
// Test 3: Origin NOT in allowlist
req3 := httptest.NewRequest(http.MethodGet, "/api/v1/certificates", nil)
req3.Header.Set("Origin", "https://evil.example.com")
rr3 := httptest.NewRecorder()
handler.ServeHTTP(rr3, req3)
if rr3.Header().Get("Access-Control-Allow-Origin") != "" {
t.Errorf("expected no Access-Control-Allow-Origin for non-allowlisted origin, got %q", rr3.Header().Get("Access-Control-Allow-Origin"))
}
}
// TestNewCORS_NoOriginHeader denies CORS without Origin header.
func TestNewCORS_NoOriginHeader(t *testing.T) {
mw := NewCORS(CORSConfig{AllowedOrigins: []string{"https://app.example.com"}})
handler := mw(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusOK)
}))
// Request without Origin header
req := httptest.NewRequest(http.MethodGet, "/api/v1/certificates", nil)
// Don't set Origin header
rr := httptest.NewRecorder()
handler.ServeHTTP(rr, req)
if rr.Code != http.StatusOK {
t.Fatalf("expected 200, got %d", rr.Code)
}
// No CORS headers should be set (Origin header was missing)
if rr.Header().Get("Access-Control-Allow-Origin") != "" {
t.Errorf("expected no Access-Control-Allow-Origin without Origin header, got %q", rr.Header().Get("Access-Control-Allow-Origin"))
}
}
// TestNewCORS_PreflightRequestMatches tests OPTIONS preflight with matching origin.
func TestNewCORS_PreflightRequestMatches(t *testing.T) {
mw := NewCORS(CORSConfig{AllowedOrigins: []string{"https://app.example.com"}})
handler := mw(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusOK)
}))
req := httptest.NewRequest(http.MethodOptions, "/api/v1/certificates", nil)
req.Header.Set("Origin", "https://app.example.com")
req.Header.Set("Access-Control-Request-Method", "POST")
rr := httptest.NewRecorder()
handler.ServeHTTP(rr, req)
if rr.Code != http.StatusNoContent {
t.Fatalf("expected 204, got %d", rr.Code)
}
if rr.Header().Get("Access-Control-Allow-Origin") != "https://app.example.com" {
t.Errorf("expected https://app.example.com, got %q", rr.Header().Get("Access-Control-Allow-Origin"))
}
// Verify preflight response headers
if rr.Header().Get("Access-Control-Allow-Methods") == "" {
t.Errorf("expected Access-Control-Allow-Methods header")
}
if rr.Header().Get("Access-Control-Allow-Headers") == "" {
t.Errorf("expected Access-Control-Allow-Headers header")
}
if rr.Header().Get("Access-Control-Max-Age") == "" {
t.Errorf("expected Access-Control-Max-Age header")
}
}
// TestNewCORS_PreflightRequestMismatch tests OPTIONS preflight with non-matching origin.
func TestNewCORS_PreflightRequestMismatch(t *testing.T) {
mw := NewCORS(CORSConfig{AllowedOrigins: []string{"https://app.example.com"}})
handler := mw(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusOK)
}))
req := httptest.NewRequest(http.MethodOptions, "/api/v1/certificates", nil)
req.Header.Set("Origin", "https://evil.example.com")
req.Header.Set("Access-Control-Request-Method", "POST")
rr := httptest.NewRecorder()
handler.ServeHTTP(rr, req)
if rr.Code != http.StatusNoContent {
t.Fatalf("expected 204, got %d", rr.Code)
}
// No CORS headers should be set (origin not in allowlist)
if rr.Header().Get("Access-Control-Allow-Origin") != "" {
t.Errorf("expected no Access-Control-Allow-Origin for mismatched origin, got %q", rr.Header().Get("Access-Control-Allow-Origin"))
}
}
// TestNewCORS_MultipleOrigins tests with multiple configured origins.
func TestNewCORS_MultipleOrigins(t *testing.T) {
mw := NewCORS(CORSConfig{AllowedOrigins: []string{
"https://app.example.com",
"https://admin.example.com",
"http://localhost:3000",
}})
handler := mw(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusOK)
}))
tests := []struct {
origin string
shouldAllow bool
description string
}{
{"https://app.example.com", true, "first origin in list"},
{"https://admin.example.com", true, "second origin in list"},
{"http://localhost:3000", true, "third origin in list"},
{"https://evil.example.com", false, "origin not in list"},
{"http://localhost:8080", false, "different port than configured"},
{"", false, "no origin header"},
}
for _, tt := range tests {
req := httptest.NewRequest(http.MethodGet, "/api/v1/certificates", nil)
if tt.origin != "" {
req.Header.Set("Origin", tt.origin)
}
rr := httptest.NewRecorder()
handler.ServeHTTP(rr, req)
headerValue := rr.Header().Get("Access-Control-Allow-Origin")
if tt.shouldAllow {
if headerValue != tt.origin {
t.Errorf("test %q: expected %q, got %q", tt.description, tt.origin, headerValue)
}
} else {
if headerValue != "" {
t.Errorf("test %q: expected no header, got %q", tt.description, headerValue)
}
}
}
}
// TestNewCORS_NoOriginHeaderWithWildcard tests wildcard doesn't set origin without Origin header.
func TestNewCORS_NoOriginHeaderWithWildcard(t *testing.T) {
mw := NewCORS(CORSConfig{AllowedOrigins: []string{"*"}})
handler := mw(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusOK)
}))
req := httptest.NewRequest(http.MethodGet, "/api/v1/certificates", nil)
// Don't set Origin header
rr := httptest.NewRecorder()
handler.ServeHTTP(rr, req)
// Wildcard should still set * even without Origin header
if rr.Header().Get("Access-Control-Allow-Origin") != "*" {
t.Errorf("expected *, got %q", rr.Header().Get("Access-Control-Allow-Origin"))
}
}