mirror of
https://github.com/shankar0123/certctl.git
synced 2026-06-07 18:21:32 +00:00
3e78ecb799
Closes three 2026-04-24 audit findings (all P2):
- cat-s5-4936a1cf0118: noAuthHandler chain accepted arbitrary-size
bodies (EST simpleenroll, SCEP, PKI CRL/OCSP, /health, /ready).
Memory exhaustion vector without HTTP-layer auth gatekeeping.
- cat-s11-missing_security_headers: zero security headers on any
response. Clickjacking, MIME-sniffing, untrusted-origin resource
loads against the dashboard and API.
- cat-r-encryption_key_no_length_validation: CERTCTL_CONFIG_ENCRYPTION_KEY
accepted with any non-empty value including a single character.
PBKDF2-SHA256 (100k rounds) does not compensate for low-entropy
passphrases at scale (CWE-916, CWE-329).
Changes:
- cmd/server/main.go::noAuthHandler chain — added bodyLimitMiddleware
+ securityHeadersMiddleware. Same default cap as authed surface
(1MB via CERTCTL_MAX_BODY_SIZE), same 413 on overflow.
- cmd/server/main.go::middlewareStack (authed) — added
securityHeadersMiddleware before corsMiddleware.
- internal/api/middleware/securityheaders.go (new) — SecurityHeaders
middleware + SecurityHeadersDefaults() with conservative defaults:
HSTS 1y+includeSubDomains, X-Frame-Options DENY, X-Content-Type-
Options nosniff, Referrer-Policy no-referrer-when-downgrade, CSP
default-src 'self' + img/data + style 'unsafe-inline' (Tailwind/Vite
needs it; scripts still 'self' only) + connect 'self' + frame-
ancestors 'none'. Operators behind a customising reverse proxy can
disable any header by setting its config field to empty.
- internal/config/config.go::Validate() — enforce minEncryptionKeyLength
= 32 bytes when CERTCTL_CONFIG_ENCRYPTION_KEY is set. Empty stays
accepted (downstream fail-closed sentinel handles it). Structured
error names the env var, the actual length, the required minimum,
and the canonical generation command (`openssl rand -base64 32`).
Tests:
- internal/api/middleware/securityheaders_test.go (new) — 4 cases
(defaults present, empty value disables single header, override
applied, headers on 4xx/5xx).
- internal/config/config_test.go — 5 new cases for the encryption-key
length check (empty accepted, 1-byte rejected, 31-byte rejected at
boundary, 32-byte accepted, 44-byte realistic operator key accepted).
Documentation:
- CHANGELOG.md — H-1 section above D-2 under [unreleased] with
Breaking-change callout (operators with low-entropy keys must rotate
before upgrade).
- coverage-gap-audit-2026-04-24-v5/unified-audit.md — Live Tracker
25/47 → 33/47, P1 14/14 (zero remaining), P2 11/27 → 16/27. Three
H-1 findings flipped + closed-bundle row added.
Verification:
- go build ./... — clean
- go vet ./... — clean
- golangci-lint v2.11.4 run ./... — 0 issues
- go test ./internal/api/middleware/... — pass (incl. 4 new
SecurityHeaders cases)
- go test ./internal/config/... — pass (incl. 5 new EncryptionKey
cases)
- tsc --noEmit (frontend) — clean
- All sibling guardrails (S-1 / G-3 / D-1 / D-2 / B-1 / L-1) still pass
Audit findings closed:
- cat-s5-4936a1cf0118 (P2)
- cat-s11-missing_security_headers (P2)
- cat-r-encryption_key_no_length_validation (P2)
Breaking change:
- Operators with CERTCTL_CONFIG_ENCRYPTION_KEY shorter than 32 bytes
must rotate before upgrade. Generate via `openssl rand -base64 32`.
Deferred follow-ups:
- Weak-key dictionary check (reject password123, common ASCII patterns)
— adds operational friction with low marginal entropy gain at the
32-byte minimum.
- CSP 'unsafe-inline' for styles — required for Tailwind/Vite
per-component <style> blocks; removing requires HTML report or
component refactor outside H-1 scope.
- Permissions-Policy header — dashboard uses no advanced browser APIs
(camera, mic, geolocation); deferred until a real consumer needs it.
105 lines
3.6 KiB
Go
105 lines
3.6 KiB
Go
package middleware
|
|
|
|
import (
|
|
"net/http"
|
|
"net/http/httptest"
|
|
"testing"
|
|
)
|
|
|
|
// TestSecurityHeaders_DefaultsAllPresent asserts every default header
|
|
// arrives on a 200 response. H-1 closure (cat-s11-missing_security_headers).
|
|
func TestSecurityHeaders_DefaultsAllPresent(t *testing.T) {
|
|
mw := SecurityHeaders(SecurityHeadersDefaults())
|
|
handler := mw(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
|
|
w.WriteHeader(http.StatusOK)
|
|
_, _ = w.Write([]byte("ok"))
|
|
}))
|
|
|
|
rec := httptest.NewRecorder()
|
|
req := httptest.NewRequest(http.MethodGet, "/test", nil)
|
|
handler.ServeHTTP(rec, req)
|
|
|
|
for _, h := range []string{
|
|
"Strict-Transport-Security",
|
|
"X-Frame-Options",
|
|
"X-Content-Type-Options",
|
|
"Referrer-Policy",
|
|
"Content-Security-Policy",
|
|
} {
|
|
if got := rec.Header().Get(h); got == "" {
|
|
t.Errorf("expected header %q to be set, got empty", h)
|
|
}
|
|
}
|
|
if got := rec.Header().Get("X-Content-Type-Options"); got != "nosniff" {
|
|
t.Errorf("X-Content-Type-Options: got %q, want %q", got, "nosniff")
|
|
}
|
|
if got := rec.Header().Get("X-Frame-Options"); got != "DENY" {
|
|
t.Errorf("X-Frame-Options: got %q, want %q", got, "DENY")
|
|
}
|
|
}
|
|
|
|
// TestSecurityHeaders_EmptyValueDisablesHeader asserts an operator can
|
|
// disable a single header by setting its config field to empty without
|
|
// affecting the others.
|
|
func TestSecurityHeaders_EmptyValueDisablesHeader(t *testing.T) {
|
|
cfg := SecurityHeadersDefaults()
|
|
cfg.HSTS = "" // simulate operator override
|
|
mw := SecurityHeaders(cfg)
|
|
handler := mw(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
|
|
w.WriteHeader(http.StatusOK)
|
|
}))
|
|
|
|
rec := httptest.NewRecorder()
|
|
handler.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, "/", nil))
|
|
|
|
if got := rec.Header().Get("Strict-Transport-Security"); got != "" {
|
|
t.Errorf("HSTS should be omitted when config value is empty; got %q", got)
|
|
}
|
|
// Other headers still present
|
|
if got := rec.Header().Get("X-Frame-Options"); got == "" {
|
|
t.Errorf("X-Frame-Options should still be present (empty HSTS only disables HSTS)")
|
|
}
|
|
}
|
|
|
|
// TestSecurityHeaders_OverrideValueApplied asserts a non-default value
|
|
// makes it through.
|
|
func TestSecurityHeaders_OverrideValueApplied(t *testing.T) {
|
|
cfg := SecurityHeadersDefaults()
|
|
cfg.FrameOptions = "SAMEORIGIN"
|
|
mw := SecurityHeaders(cfg)
|
|
handler := mw(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
|
|
w.WriteHeader(http.StatusOK)
|
|
}))
|
|
|
|
rec := httptest.NewRecorder()
|
|
handler.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, "/", nil))
|
|
|
|
if got := rec.Header().Get("X-Frame-Options"); got != "SAMEORIGIN" {
|
|
t.Errorf("X-Frame-Options: got %q, want %q", got, "SAMEORIGIN")
|
|
}
|
|
}
|
|
|
|
// TestSecurityHeaders_AppliedOnErrorResponses asserts headers are
|
|
// present on 4xx/5xx as well as 2xx — this is critical for the
|
|
// security posture (an attacker probing for misconfiguration sees
|
|
// the same headers on a 401 as on a 200).
|
|
func TestSecurityHeaders_AppliedOnErrorResponses(t *testing.T) {
|
|
mw := SecurityHeaders(SecurityHeadersDefaults())
|
|
handler := mw(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
|
|
http.Error(w, "unauthorized", http.StatusUnauthorized)
|
|
}))
|
|
|
|
rec := httptest.NewRecorder()
|
|
handler.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, "/", nil))
|
|
|
|
if rec.Code != http.StatusUnauthorized {
|
|
t.Fatalf("status: got %d, want %d", rec.Code, http.StatusUnauthorized)
|
|
}
|
|
if got := rec.Header().Get("Strict-Transport-Security"); got == "" {
|
|
t.Errorf("HSTS missing on 401 response (must be on every response)")
|
|
}
|
|
if got := rec.Header().Get("Content-Security-Policy"); got == "" {
|
|
t.Errorf("CSP missing on 401 response")
|
|
}
|
|
}
|