mirror of
https://github.com/shankar0123/certctl.git
synced 2026-06-07 16:11:29 +00:00
feat(security): bodyLimit on noAuth + security headers + encryption-key validation (H-1 master)
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.
This commit is contained in:
@@ -4,6 +4,32 @@ All notable changes to certctl are documented in this file. Dates use ISO 8601.
|
|||||||
|
|
||||||
## [unreleased] — 2026-04-25
|
## [unreleased] — 2026-04-25
|
||||||
|
|
||||||
|
### H-1: Security hardening trio — closed end-to-end
|
||||||
|
|
||||||
|
> Three 2026-04-24 audit findings (all P2) that together complete the HTTPS-Everywhere security baseline. The audit flagged: (1) the unauth surface (EST RFC 7030, SCEP, PKI CRL/OCSP, /health, /ready) accepted arbitrary-size request bodies because the `noAuthHandler` middleware chain was missing the `bodyLimitMiddleware` that the authed `apiHandler` chain has; (2) zero security headers (CSP, HSTS, X-Frame-Options, X-Content-Type-Options, Referrer-Policy) were emitted on any response — enabling clickjacking, MIME-sniffing, and untrusted-origin resource loads against the dashboard and API; (3) `CERTCTL_CONFIG_ENCRYPTION_KEY` was accepted with any non-empty value, including a single character — PBKDF2-SHA256 with 100k rounds does not compensate for low-entropy passphrases at scale (CWE-916 / CWE-329).
|
||||||
|
|
||||||
|
### Breaking Changes
|
||||||
|
|
||||||
|
**Operators with low-entropy `CERTCTL_CONFIG_ENCRYPTION_KEY` will fail to start after upgrade.** Pre-H-1 the field accepted any non-empty string. Post-H-1 it requires ≥32 bytes (e.g. `openssl rand -base64 32`). The startup error names the offending env var, the actual length, the required minimum, and the canonical generation command. Empty (`""`) remains accepted — the existing fail-closed sentinel `crypto.ErrEncryptionKeyRequired` triggers downstream when an empty key tries to encrypt or decrypt. Operators using a short passphrase must rotate before the upgrade.
|
||||||
|
|
||||||
|
### Added
|
||||||
|
|
||||||
|
- **`internal/api/middleware/securityheaders.go`** (new) — `SecurityHeaders` middleware applies HSTS, X-Frame-Options, X-Content-Type-Options, Referrer-Policy, and a conservative Content-Security-Policy on every response. Defaults via `SecurityHeadersDefaults()` are: `Strict-Transport-Security: max-age=31536000; includeSubDomains`, `X-Frame-Options: DENY`, `X-Content-Type-Options: nosniff`, `Referrer-Policy: no-referrer-when-downgrade`, and `Content-Security-Policy: default-src 'self'; img-src 'self' data:; style-src 'self' 'unsafe-inline'; script-src 'self'; connect-src 'self'; frame-ancestors 'none'`. Operators behind a customising reverse proxy can override per-header by setting any field of the config struct to the empty string (omits that header).
|
||||||
|
- **`bodyLimitMiddleware` wired into `noAuthHandler`** in `cmd/server/main.go`. Same default cap (1 MB, configurable via `CERTCTL_MAX_BODY_SIZE`), same 413 response on overflow. Pre-H-1 only the authed surface had this protection.
|
||||||
|
- **`securityHeadersMiddleware` wired into BOTH chains** (`middlewareStack` for authed routes; `noAuthHandler` for unauth routes). Applied before the audit middleware so headers reach 4xx/5xx responses too — critical for security posture (an attacker probing for misconfiguration sees the same headers on a 401 as on a 200).
|
||||||
|
- **`CERTCTL_CONFIG_ENCRYPTION_KEY` length validation** in `internal/config/config.go::Validate()` — rejects keys shorter than 32 bytes with a structured error naming the actual length, the required minimum, and the canonical generation command. Empty keys remain accepted (downstream fail-closed sentinel handles it).
|
||||||
|
- **Tests:** `internal/api/middleware/securityheaders_test.go` (4 cases — defaults present, empty disables single header, override applied, headers on 4xx/5xx). `internal/config/config_test.go` adds 5 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).
|
||||||
|
|
||||||
|
### Audit findings closed
|
||||||
|
|
||||||
|
- `cat-s5-4936a1cf0118` (P2, EST/SCEP/PKI unauth endpoints bypass `http.MaxBytesReader`)
|
||||||
|
- `cat-s11-missing_security_headers` (P2, no CSP / HSTS / X-Frame-Options on responses)
|
||||||
|
- `cat-r-encryption_key_no_length_validation` (P2, encryption key accepted with zero entropy validation)
|
||||||
|
|
||||||
|
### Known follow-ups (deferred from H-1 scope)
|
||||||
|
|
||||||
|
A weak-key dictionary check (reject `password123`, common ASCII patterns) is deferred — adds operational friction with low marginal entropy gain at the 32-byte minimum. CSP `'unsafe-inline'` for styles is required because Tailwind via Vite injects per-component `<style>` blocks at build time; removing it would require an HTML report or component refactor outside H-1 scope. A `Permissions-Policy` (formerly Feature-Policy) header is not in the H-1 baseline because the dashboard uses no advanced browser APIs (camera, microphone, geolocation); deferred until a real consumer needs it.
|
||||||
|
|
||||||
### D-2: TS ↔ Go type drift cluster — closed end-to-end
|
### D-2: TS ↔ Go type drift cluster — closed end-to-end
|
||||||
|
|
||||||
> The 2026-04-24 coverage-gap audit flagged five `diff-05x06-*` findings — every one a TypeScript-vs-Go shape mismatch where the on-wire JSON the backend emits and the TS interface in `web/src/api/types.ts` had drifted apart. D-1 master closed the same pattern for `Certificate` (cat-f-ae0d06b6588f, 5 phantom fields trimmed, plus the cat-f-cert_detail_page_key_render_fallback render-site fix). D-2 closes it for the remaining five entities: Agent, Target, DiscoveredCertificate, Issuer, and Notification. The audit's blunt rule "stricter side is the contract" decides the per-entity verdict — for TS phantoms (fields declared on TS, never emitted by Go) the Go side wins and TS gets trimmed; for TS-missing fields (emitted by Go, absent from TS) the Go side still wins and TS gets the addition. Pre-D-2 the failure modes were: phantom fields silently rendered `'—'` at consumer sites (e.g. AgentDetailPage's "Capabilities" + "Tags" sections always rendered empty; IssuersPage rendered `'Unknown'` for every issuer; NotificationsPage's `n.message || n.subject` fallback always fell through), and missing fields forced `(target as any).retired_at` escapes that lost type-checking. Verify-only side task: Certificate / ManagedCertificate confirmed clean since D-1.
|
> The 2026-04-24 coverage-gap audit flagged five `diff-05x06-*` findings — every one a TypeScript-vs-Go shape mismatch where the on-wire JSON the backend emits and the TS interface in `web/src/api/types.ts` had drifted apart. D-1 master closed the same pattern for `Certificate` (cat-f-ae0d06b6588f, 5 phantom fields trimmed, plus the cat-f-cert_detail_page_key_render_fallback render-site fix). D-2 closes it for the remaining five entities: Agent, Target, DiscoveredCertificate, Issuer, and Notification. The audit's blunt rule "stricter side is the contract" decides the per-entity verdict — for TS phantoms (fields declared on TS, never emitted by Go) the Go side wins and TS gets trimmed; for TS-missing fields (emitted by Go, absent from TS) the Go side still wins and TS gets the addition. Pre-D-2 the failure modes were: phantom fields silently rendered `'—'` at consumer sites (e.g. AgentDetailPage's "Capabilities" + "Tags" sections always rendered empty; IssuersPage rendered `'Unknown'` for every issuer; NotificationsPage's `n.message || n.subject` fallback always fell through), and missing fields forced `(target as any).retired_at` escapes that lost type-checking. Verify-only side task: Certificate / ManagedCertificate confirmed clean since D-1.
|
||||||
|
|||||||
+31
-3
@@ -741,6 +741,17 @@ func main() {
|
|||||||
})
|
})
|
||||||
logger.Info("request body size limit enabled", "max_bytes", cfg.Server.MaxBodySize)
|
logger.Info("request body size limit enabled", "max_bytes", cfg.Server.MaxBodySize)
|
||||||
|
|
||||||
|
// Security headers middleware — applies HSTS, X-Frame-Options,
|
||||||
|
// X-Content-Type-Options, Referrer-Policy, and a conservative CSP
|
||||||
|
// on every response. H-1 closure (cat-s11-missing_security_headers):
|
||||||
|
// pre-H-1 the server emitted zero security headers; an attacker
|
||||||
|
// could clickjack the dashboard, sniff MIME types on JSON/PEM
|
||||||
|
// responses, or load resources from arbitrary origins via inline
|
||||||
|
// scripts. Defaults are conservative — see internal/api/middleware/
|
||||||
|
// securityheaders.go::SecurityHeadersDefaults() for the rationale
|
||||||
|
// per header.
|
||||||
|
securityHeadersMiddleware := middleware.SecurityHeaders(middleware.SecurityHeadersDefaults())
|
||||||
|
|
||||||
// API audit log middleware — records every API call to the audit trail
|
// API audit log middleware — records every API call to the audit trail
|
||||||
auditAdapter := middleware.NewAuditServiceAdapter(
|
auditAdapter := middleware.NewAuditServiceAdapter(
|
||||||
func(ctx context.Context, actor string, actorType string, action string, resourceType string, resourceID string, details map[string]interface{}) error {
|
func(ctx context.Context, actor string, actorType string, action string, resourceType string, resourceID string, details map[string]interface{}) error {
|
||||||
@@ -763,6 +774,7 @@ func main() {
|
|||||||
structuredLogger,
|
structuredLogger,
|
||||||
middleware.Recovery,
|
middleware.Recovery,
|
||||||
bodyLimitMiddleware,
|
bodyLimitMiddleware,
|
||||||
|
securityHeadersMiddleware,
|
||||||
corsMiddleware,
|
corsMiddleware,
|
||||||
authMiddleware,
|
authMiddleware,
|
||||||
auditMiddleware.Middleware,
|
auditMiddleware.Middleware,
|
||||||
@@ -809,13 +821,29 @@ func main() {
|
|||||||
if _, err := os.Stat(webDir + "/index.html"); err != nil {
|
if _, err := os.Stat(webDir + "/index.html"); err != nil {
|
||||||
webDir = "./web"
|
webDir = "./web"
|
||||||
}
|
}
|
||||||
// Health/ready routes bypass the full middleware stack (no auth required).
|
// Health/ready routes + EST/SCEP/PKI unauth surface bypass the full
|
||||||
// These are registered on the inner router without auth, but the outer
|
// middleware stack (no auth required). These are registered on the
|
||||||
// middleware chain wraps everything. Route them directly to the inner router.
|
// inner router without auth, but the outer middleware chain wraps
|
||||||
|
// everything. Route them directly to the inner router.
|
||||||
|
//
|
||||||
|
// H-1 closure (cat-s5-4936a1cf0118): pre-H-1 the noAuthHandler chain
|
||||||
|
// was RequestID → structuredLogger → Recovery only — missing
|
||||||
|
// bodyLimitMiddleware that the authed apiHandler chain has. The
|
||||||
|
// unauth surface includes EST simpleenroll/simplereenroll (RFC 7030),
|
||||||
|
// SCEP, PKI CRL/OCSP (/.well-known/pki/*), and /health|/ready —
|
||||||
|
// every one of which accepts a request body. Without a body-size
|
||||||
|
// cap, an unauthenticated client can send arbitrary-size payloads
|
||||||
|
// (CSRs, CRL/OCSP requests) and trigger memory pressure on the
|
||||||
|
// server before the handler ever rejects the input. Post-H-1 the
|
||||||
|
// same bodyLimitMiddleware that wraps the authed surface also wraps
|
||||||
|
// the unauth surface — same default cap (CERTCTL_MAX_BODY_SIZE,
|
||||||
|
// default 1MB), same 413 response on overflow.
|
||||||
noAuthHandler := middleware.Chain(apiRouter,
|
noAuthHandler := middleware.Chain(apiRouter,
|
||||||
middleware.RequestID,
|
middleware.RequestID,
|
||||||
structuredLogger,
|
structuredLogger,
|
||||||
middleware.Recovery,
|
middleware.Recovery,
|
||||||
|
bodyLimitMiddleware,
|
||||||
|
securityHeadersMiddleware,
|
||||||
)
|
)
|
||||||
|
|
||||||
dashboardEnabled := false
|
dashboardEnabled := false
|
||||||
|
|||||||
@@ -0,0 +1,96 @@
|
|||||||
|
package middleware
|
||||||
|
|
||||||
|
import (
|
||||||
|
"net/http"
|
||||||
|
"strings"
|
||||||
|
)
|
||||||
|
|
||||||
|
// SecurityHeadersConfig configures the SecurityHeaders middleware.
|
||||||
|
//
|
||||||
|
// Each field is the literal value to send. An empty string means
|
||||||
|
// "do not send this header" — operators behind a customising reverse
|
||||||
|
// proxy can disable any header per-deployment without touching code.
|
||||||
|
// Defaults are applied via SecurityHeadersDefaults() which encodes
|
||||||
|
// the H-1 closure's recommended baseline for an HTTPS-only API+UI
|
||||||
|
// host: HSTS, deny-frame, no-MIME-sniff, conservative CSP, and a
|
||||||
|
// no-referrer-when-downgrade fallback.
|
||||||
|
//
|
||||||
|
// H-1 closure (cat-s11-missing_security_headers).
|
||||||
|
type SecurityHeadersConfig struct {
|
||||||
|
HSTS string // Strict-Transport-Security
|
||||||
|
FrameOptions string // X-Frame-Options
|
||||||
|
ContentTypeOptions string // X-Content-Type-Options
|
||||||
|
ReferrerPolicy string // Referrer-Policy
|
||||||
|
ContentSecurityPolicy string // Content-Security-Policy
|
||||||
|
}
|
||||||
|
|
||||||
|
// SecurityHeadersDefaults returns a recommended baseline.
|
||||||
|
//
|
||||||
|
// CSP: default-src 'self' confines fetches to the same origin.
|
||||||
|
// img-src 'self' data: allows inline base64 images (used by the
|
||||||
|
// dashboard's certctl-logo and a few status icons).
|
||||||
|
// style-src 'self' 'unsafe-inline' is required because Tailwind
|
||||||
|
// (via Vite) injects per-component <style> blocks at build time;
|
||||||
|
// without 'unsafe-inline' the dashboard would render unstyled.
|
||||||
|
// 'unsafe-inline' is intentionally NOT in script-src — the
|
||||||
|
// front-end ships as a bundled JS file, no inline scripts.
|
||||||
|
//
|
||||||
|
// HSTS: 1-year max-age + includeSubDomains. No `preload` directive
|
||||||
|
// because preload submission requires explicit operator action and
|
||||||
|
// the deployment topology may not span all subdomains.
|
||||||
|
//
|
||||||
|
// X-Frame-Options: DENY — the dashboard does not need to be embedded
|
||||||
|
// anywhere, and DENY is more conservative than SAMEORIGIN against
|
||||||
|
// clickjacking via subdomain takeover.
|
||||||
|
//
|
||||||
|
// X-Content-Type-Options: nosniff — prevent MIME sniffing on
|
||||||
|
// JSON/PEM responses that browsers might otherwise interpret as HTML.
|
||||||
|
//
|
||||||
|
// Referrer-Policy: no-referrer-when-downgrade — preserves Referer
|
||||||
|
// for same-origin navigation (useful for support/diagnostics) but
|
||||||
|
// strips it on HTTPS→HTTP transitions.
|
||||||
|
func SecurityHeadersDefaults() SecurityHeadersConfig {
|
||||||
|
return SecurityHeadersConfig{
|
||||||
|
HSTS: "max-age=31536000; includeSubDomains",
|
||||||
|
FrameOptions: "DENY",
|
||||||
|
ContentTypeOptions: "nosniff",
|
||||||
|
ReferrerPolicy: "no-referrer-when-downgrade",
|
||||||
|
ContentSecurityPolicy: "default-src 'self'; img-src 'self' data:; style-src 'self' 'unsafe-inline'; script-src 'self'; connect-src 'self'; frame-ancestors 'none'",
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// SecurityHeaders returns a middleware that applies the configured
|
||||||
|
// HTTP response headers on every response. Headers configured to the
|
||||||
|
// empty string are omitted (operator opted out for that deployment).
|
||||||
|
//
|
||||||
|
// Apply BEFORE the audit middleware so headers reach 4xx/5xx responses
|
||||||
|
// — which is where header omissions matter most for the security
|
||||||
|
// posture (an attacker probing for misconfiguration sees the same
|
||||||
|
// headers on a 401 as on a 200).
|
||||||
|
func SecurityHeaders(cfg SecurityHeadersConfig) func(http.Handler) http.Handler {
|
||||||
|
// Pre-trim each value once; the per-request hot path stays a
|
||||||
|
// straight set of map writes.
|
||||||
|
type headerEntry struct{ name, value string }
|
||||||
|
entries := make([]headerEntry, 0, 5)
|
||||||
|
add := func(name, value string) {
|
||||||
|
v := strings.TrimSpace(value)
|
||||||
|
if v != "" {
|
||||||
|
entries = append(entries, headerEntry{name, v})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
add("Strict-Transport-Security", cfg.HSTS)
|
||||||
|
add("X-Frame-Options", cfg.FrameOptions)
|
||||||
|
add("X-Content-Type-Options", cfg.ContentTypeOptions)
|
||||||
|
add("Referrer-Policy", cfg.ReferrerPolicy)
|
||||||
|
add("Content-Security-Policy", cfg.ContentSecurityPolicy)
|
||||||
|
|
||||||
|
return func(next http.Handler) http.Handler {
|
||||||
|
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||||
|
h := w.Header()
|
||||||
|
for _, e := range entries {
|
||||||
|
h.Set(e.name, e.value)
|
||||||
|
}
|
||||||
|
next.ServeHTTP(w, r)
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -0,0 +1,104 @@
|
|||||||
|
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")
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -1176,6 +1176,26 @@ func (c *Config) Validate() error {
|
|||||||
return fmt.Errorf("server TLS cert/key pair invalid (cert=%q key=%q): %w — refuse to start (HTTPS-only; see docs/tls.md)", c.Server.TLS.CertPath, c.Server.TLS.KeyPath, err)
|
return fmt.Errorf("server TLS cert/key pair invalid (cert=%q key=%q): %w — refuse to start (HTTPS-only; see docs/tls.md)", c.Server.TLS.CertPath, c.Server.TLS.KeyPath, err)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// H-1 closure (cat-r-encryption_key_no_length_validation): if
|
||||||
|
// CERTCTL_CONFIG_ENCRYPTION_KEY is set, enforce a minimum length of
|
||||||
|
// 32 bytes. Pre-H-1 the field was accepted with any non-empty value
|
||||||
|
// — including a single character — and PBKDF2-SHA256 (100k rounds)
|
||||||
|
// alone does not compensate for low-entropy passphrases at scale
|
||||||
|
// (CWE-916 Use of Password Hash With Insufficient Computational
|
||||||
|
// Effort + CWE-329 Generation of Predictable IV with CBC Mode).
|
||||||
|
// 32 bytes ≈ 256 bits when generated via `openssl rand -base64 32`,
|
||||||
|
// matching the AES-256-GCM key size the passphrase derives. An
|
||||||
|
// empty key remains accepted — the fail-closed sentinel
|
||||||
|
// crypto.ErrEncryptionKeyRequired triggers downstream when an
|
||||||
|
// empty key is asked to encrypt or decrypt sensitive config.
|
||||||
|
const minEncryptionKeyLength = 32
|
||||||
|
if c.Encryption.ConfigEncryptionKey != "" && len(c.Encryption.ConfigEncryptionKey) < minEncryptionKeyLength {
|
||||||
|
return fmt.Errorf(
|
||||||
|
"CERTCTL_CONFIG_ENCRYPTION_KEY too short (%d bytes; minimum %d). Generate with: openssl rand -base64 32",
|
||||||
|
len(c.Encryption.ConfigEncryptionKey), minEncryptionKeyLength,
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
// Validate database configuration
|
// Validate database configuration
|
||||||
if c.Database.URL == "" {
|
if c.Database.URL == "" {
|
||||||
return fmt.Errorf("database URL is required")
|
return fmt.Errorf("database URL is required")
|
||||||
|
|||||||
@@ -1209,3 +1209,84 @@ func TestConfig_Scheduler_JobTimeoutValidation(t *testing.T) {
|
|||||||
})
|
})
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// H-1 closure (cat-r-encryption_key_no_length_validation): validate
|
||||||
|
// CERTCTL_CONFIG_ENCRYPTION_KEY length. Pre-H-1 the field was accepted
|
||||||
|
// with any non-empty value (including a single character); post-H-1 a
|
||||||
|
// minimum 32-byte length is enforced. Empty stays accepted because the
|
||||||
|
// downstream fail-closed sentinel crypto.ErrEncryptionKeyRequired
|
||||||
|
// handles the missing-key case for the encrypt/decrypt paths.
|
||||||
|
|
||||||
|
func validBaseConfigForEncryption(t *testing.T) *Config {
|
||||||
|
t.Helper()
|
||||||
|
return &Config{
|
||||||
|
Server: validServerConfig(t),
|
||||||
|
Database: DatabaseConfig{URL: "postgres://localhost/certctl", MaxConnections: 25},
|
||||||
|
Log: LogConfig{Level: "info", Format: "json"},
|
||||||
|
Auth: AuthConfig{Type: "api-key", Secret: "test-secret"},
|
||||||
|
Keygen: KeygenConfig{Mode: "agent"},
|
||||||
|
Scheduler: SchedulerConfig{
|
||||||
|
RenewalCheckInterval: 1 * time.Hour,
|
||||||
|
JobProcessorInterval: 30 * time.Second,
|
||||||
|
AgentHealthCheckInterval: 2 * time.Minute,
|
||||||
|
NotificationProcessInterval: 1 * time.Minute,
|
||||||
|
NotificationRetryInterval: 2 * time.Minute,
|
||||||
|
RetryInterval: 5 * time.Minute,
|
||||||
|
JobTimeoutInterval: 10 * time.Minute,
|
||||||
|
AwaitingCSRTimeout: 24 * time.Hour,
|
||||||
|
AwaitingApprovalTimeout: 168 * time.Hour,
|
||||||
|
},
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestValidate_EncryptionKey_EmptyAccepted(t *testing.T) {
|
||||||
|
cfg := validBaseConfigForEncryption(t)
|
||||||
|
cfg.Encryption.ConfigEncryptionKey = ""
|
||||||
|
if err := cfg.Validate(); err != nil {
|
||||||
|
t.Errorf("Validate() returned error for empty key: %v (empty must be accepted; fail-closed sentinel handles it downstream)", err)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestValidate_EncryptionKey_TooShortRejected(t *testing.T) {
|
||||||
|
cfg := validBaseConfigForEncryption(t)
|
||||||
|
cfg.Encryption.ConfigEncryptionKey = "x" // 1 byte
|
||||||
|
err := cfg.Validate()
|
||||||
|
if err == nil {
|
||||||
|
t.Fatal("Validate() = nil, want error for 1-byte key")
|
||||||
|
}
|
||||||
|
if !strings.Contains(err.Error(), "too short") {
|
||||||
|
t.Errorf("Validate() error = %q, want to contain %q", err.Error(), "too short")
|
||||||
|
}
|
||||||
|
if !strings.Contains(err.Error(), "openssl rand -base64 32") {
|
||||||
|
t.Errorf("Validate() error = %q, must include the canonical generation command", err.Error())
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestValidate_EncryptionKey_BoundaryRejected(t *testing.T) {
|
||||||
|
cfg := validBaseConfigForEncryption(t)
|
||||||
|
cfg.Encryption.ConfigEncryptionKey = "12345678901234567890123456789012"[:31] // 31 bytes — one short
|
||||||
|
err := cfg.Validate()
|
||||||
|
if err == nil {
|
||||||
|
t.Fatal("Validate() = nil, want error for 31-byte key (boundary -1)")
|
||||||
|
}
|
||||||
|
if !strings.Contains(err.Error(), "too short") {
|
||||||
|
t.Errorf("Validate() error = %q, want 'too short'", err.Error())
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestValidate_EncryptionKey_MinLengthAccepted(t *testing.T) {
|
||||||
|
cfg := validBaseConfigForEncryption(t)
|
||||||
|
cfg.Encryption.ConfigEncryptionKey = "12345678901234567890123456789012" // exactly 32 bytes
|
||||||
|
if err := cfg.Validate(); err != nil {
|
||||||
|
t.Errorf("Validate() returned error for 32-byte key: %v", err)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestValidate_EncryptionKey_LongAccepted(t *testing.T) {
|
||||||
|
cfg := validBaseConfigForEncryption(t)
|
||||||
|
// Realistic operator key from `openssl rand -base64 32` — 44 characters.
|
||||||
|
cfg.Encryption.ConfigEncryptionKey = "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"
|
||||||
|
if err := cfg.Validate(); err != nil {
|
||||||
|
t.Errorf("Validate() returned error for 44-byte key: %v", err)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user