fix: security audit remediation (AUDIT-001, 003, 004, 005, 006, 018)

- AUDIT-001: Validate OpenSSL revoke inputs (hex-only serials, RFC 5280 reasons)
- AUDIT-003: Enforce /20 CIDR size cap at API level (create + update)
- AUDIT-004: Support comma-separated CERTCTL_AUTH_SECRET for zero-downtime key rotation
- AUDIT-005: Add ReadHeaderTimeout (5s) to prevent Slowloris
- AUDIT-006: Document audit trail query parameter exclusion rationale
- AUDIT-018: Add immediate-run-on-start to short-lived expiry scheduler loop

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
shankar0123
2026-03-28 14:11:16 -04:00
parent 591dcfb139
commit 6d508cf53f
15 changed files with 595 additions and 34 deletions
+6 -1
View File
@@ -78,7 +78,12 @@ func NewAuditLog(recorder AuditRecorder, cfg AuditConfig) func(http.Handler) htt
latency := time.Since(start).Milliseconds()
// Record audit event asynchronously (best-effort, don't block response)
// Record audit event asynchronously (best-effort, don't block response).
// SECURITY: We intentionally use r.URL.Path (not r.URL.String() or r.RequestURI)
// to prevent query parameters from being recorded in the immutable audit trail.
// Query strings may contain cursor tokens, API keys passed as params, or other
// sensitive filter values. Since the audit trail is append-only with no deletion
// capability, any sensitive data recorded would persist permanently.
go func() {
if err := recorder.RecordAPICall(
context.Background(),
+40
View File
@@ -328,6 +328,46 @@ func TestAuditLog_CapturesLatency(t *testing.T) {
}
}
func TestAuditLog_ExcludesQueryParamsFromPath(t *testing.T) {
recorder := newWaitableAuditRecorder()
mw := NewAuditLog(recorder, AuditConfig{})
handler := mw(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusOK)
}))
// Send a request with sensitive query parameters
req := httptest.NewRequest(http.MethodGet, "/api/v1/certificates?api_key=secret123&cursor=abc&status=active", nil)
rr := httptest.NewRecorder()
handler.ServeHTTP(rr, req)
if !recorder.Wait(1 * time.Second) {
t.Fatal("timeout waiting for audit record")
}
calls := recorder.getCalls()
if len(calls) != 1 {
t.Fatalf("expected 1 audit call, got %d", len(calls))
}
// Path should contain ONLY the path, no query parameters
if calls[0].Path != "/api/v1/certificates" {
t.Errorf("expected path /api/v1/certificates (no query params), got %s", calls[0].Path)
}
if strings.Contains(calls[0].Path, "api_key") {
t.Error("audit path contains 'api_key' — query parameters leaked into audit trail")
}
if strings.Contains(calls[0].Path, "secret123") {
t.Error("audit path contains sensitive value 'secret123' — query parameters leaked into audit trail")
}
if strings.Contains(calls[0].Path, "cursor") {
t.Error("audit path contains 'cursor' — query parameters leaked into audit trail")
}
if strings.Contains(calls[0].Path, "?") {
t.Error("audit path contains '?' — query string leaked into audit trail")
}
}
func TestAuditServiceAdapter_TranslatesCallToEvent(t *testing.T) {
var capturedActor, capturedActorType, capturedAction, capturedResourceType, capturedResourceID string
var capturedDetails map[string]interface{}
+189
View File
@@ -0,0 +1,189 @@
package middleware
import (
"net/http"
"net/http/httptest"
"testing"
)
func TestNewAuth_MultiKeyAcceptsBothKeys(t *testing.T) {
cfg := AuthConfig{
Type: "api-key",
Secret: "key-one,key-two",
}
mw := NewAuth(cfg)
handler := mw(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusOK)
}))
// First key should work
req1 := httptest.NewRequest(http.MethodGet, "/api/v1/certificates", nil)
req1.Header.Set("Authorization", "Bearer key-one")
rr1 := httptest.NewRecorder()
handler.ServeHTTP(rr1, req1)
if rr1.Code != http.StatusOK {
t.Errorf("expected 200 for first key, got %d", rr1.Code)
}
// Second key should work
req2 := httptest.NewRequest(http.MethodGet, "/api/v1/certificates", nil)
req2.Header.Set("Authorization", "Bearer key-two")
rr2 := httptest.NewRecorder()
handler.ServeHTTP(rr2, req2)
if rr2.Code != http.StatusOK {
t.Errorf("expected 200 for second key, got %d", rr2.Code)
}
}
func TestNewAuth_MultiKeyRejectsInvalidKey(t *testing.T) {
cfg := AuthConfig{
Type: "api-key",
Secret: "key-one,key-two",
}
mw := NewAuth(cfg)
handler := mw(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusOK)
}))
// Invalid key should be rejected
req := httptest.NewRequest(http.MethodGet, "/api/v1/certificates", nil)
req.Header.Set("Authorization", "Bearer wrong-key")
rr := httptest.NewRecorder()
handler.ServeHTTP(rr, req)
if rr.Code != http.StatusUnauthorized {
t.Errorf("expected 401 for invalid key, got %d", rr.Code)
}
}
func TestNewAuth_MultiKeyWithSpaces(t *testing.T) {
// Keys with leading/trailing spaces should be trimmed
cfg := AuthConfig{
Type: "api-key",
Secret: " key-one , key-two ",
}
mw := NewAuth(cfg)
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("Authorization", "Bearer key-one")
rr := httptest.NewRecorder()
handler.ServeHTTP(rr, req)
if rr.Code != http.StatusOK {
t.Errorf("expected 200 for trimmed key, got %d", rr.Code)
}
}
func TestNewAuth_SingleKeyStillWorks(t *testing.T) {
cfg := AuthConfig{
Type: "api-key",
Secret: "my-single-key",
}
mw := NewAuth(cfg)
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("Authorization", "Bearer my-single-key")
rr := httptest.NewRecorder()
handler.ServeHTTP(rr, req)
if rr.Code != http.StatusOK {
t.Errorf("expected 200 for single key, got %d", rr.Code)
}
}
func TestNewAuth_NoneMode(t *testing.T) {
cfg := AuthConfig{
Type: "none",
Secret: "",
}
mw := NewAuth(cfg)
handler := mw(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusOK)
}))
// No auth header needed in none mode
req := httptest.NewRequest(http.MethodGet, "/api/v1/certificates", nil)
rr := httptest.NewRecorder()
handler.ServeHTTP(rr, req)
if rr.Code != http.StatusOK {
t.Errorf("expected 200 in none mode, got %d", rr.Code)
}
}
func TestNewAuth_MissingAuthHeader(t *testing.T) {
cfg := AuthConfig{
Type: "api-key",
Secret: "test-key",
}
mw := NewAuth(cfg)
handler := mw(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusOK)
}))
req := httptest.NewRequest(http.MethodGet, "/api/v1/certificates", nil)
rr := httptest.NewRecorder()
handler.ServeHTTP(rr, req)
if rr.Code != http.StatusUnauthorized {
t.Errorf("expected 401 for missing auth, got %d", rr.Code)
}
}
func TestNewAuth_InvalidBearerFormat(t *testing.T) {
cfg := AuthConfig{
Type: "api-key",
Secret: "test-key",
}
mw := NewAuth(cfg)
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("Authorization", "Basic dGVzdDp0ZXN0")
rr := httptest.NewRecorder()
handler.ServeHTTP(rr, req)
if rr.Code != http.StatusUnauthorized {
t.Errorf("expected 401 for non-Bearer auth, got %d", rr.Code)
}
}
func TestNewAuth_RemovedKeyIsRejected(t *testing.T) {
// Simulate key rotation: only key-two is configured (key-one was removed)
cfg := AuthConfig{
Type: "api-key",
Secret: "key-two",
}
mw := NewAuth(cfg)
handler := mw(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusOK)
}))
// Old key should be rejected
req := httptest.NewRequest(http.MethodGet, "/api/v1/certificates", nil)
req.Header.Set("Authorization", "Bearer key-one")
rr := httptest.NewRecorder()
handler.ServeHTTP(rr, req)
if rr.Code != http.StatusUnauthorized {
t.Errorf("expected 401 for removed key, got %d", rr.Code)
}
// New key should work
req2 := httptest.NewRequest(http.MethodGet, "/api/v1/certificates", nil)
req2.Header.Set("Authorization", "Bearer key-two")
rr2 := httptest.NewRecorder()
handler.ServeHTTP(rr2, req2)
if rr2.Code != http.StatusOK {
t.Errorf("expected 200 for current key, got %d", rr2.Code)
}
}
+32 -5
View File
@@ -8,6 +8,7 @@ import (
"log"
"log/slog"
"net/http"
"strings"
"sync"
"time"
@@ -100,12 +101,17 @@ func HashAPIKey(key string) string {
// AuthConfig holds configuration for the Auth middleware.
type AuthConfig struct {
Type string // "api-key", "jwt", "none"
Secret string // The raw API key (server compares against this)
Secret string // The raw API key or comma-separated list of valid API keys
}
// NewAuth creates an authentication middleware based on config.
// When Type is "none", all requests pass through (demo/development mode).
// When Type is "api-key", requests must include a valid Bearer token.
// The Secret field supports a comma-separated list of valid API keys for
// zero-downtime key rotation. Rotation workflow:
// 1. Add new key to comma-separated list, restart server
// 2. Update all agents/clients to use new key
// 3. Remove old key from list, restart server
func NewAuth(cfg AuthConfig) func(http.Handler) http.Handler {
if cfg.Type == "none" {
return func(next http.Handler) http.Handler {
@@ -113,8 +119,21 @@ func NewAuth(cfg AuthConfig) func(http.Handler) http.Handler {
}
}
// Pre-compute hash of the expected key for constant-time comparison
expectedHash := HashAPIKey(cfg.Secret)
// Pre-compute hashes of all valid keys for constant-time comparison.
// Supports comma-separated list for zero-downtime key rotation.
keys := strings.Split(cfg.Secret, ",")
var expectedHashes []string
for _, k := range keys {
k = strings.TrimSpace(k)
if k != "" {
expectedHashes = append(expectedHashes, HashAPIKey(k))
}
}
// Warn if only one key is configured in production mode
if len(expectedHashes) == 1 {
slog.Warn("only one API key configured — consider adding a rotation key via comma-separated CERTCTL_AUTH_SECRET for zero-downtime rotation")
}
return func(next http.Handler) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
@@ -136,8 +155,16 @@ func NewAuth(cfg AuthConfig) func(http.Handler) http.Handler {
token := authHeader[7:]
tokenHash := HashAPIKey(token)
// Constant-time comparison to prevent timing attacks
if subtle.ConstantTimeCompare([]byte(tokenHash), []byte(expectedHash)) != 1 {
// Check against all valid keys using constant-time comparison
authorized := false
for _, expectedHash := range expectedHashes {
if subtle.ConstantTimeCompare([]byte(tokenHash), []byte(expectedHash)) == 1 {
authorized = true
break
}
}
if !authorized {
w.Header().Set("Content-Type", "application/json; charset=utf-8")
http.Error(w, `{"error":"Invalid API key"}`, http.StatusUnauthorized)
return