mirror of
https://github.com/shankar0123/certctl.git
synced 2026-06-07 18:11:32 +00:00
30f9f1e712
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
118 lines
4.4 KiB
Go
118 lines
4.4 KiB
Go
package main
|
|
|
|
import (
|
|
"net/http"
|
|
"net/http/httptest"
|
|
"strings"
|
|
"testing"
|
|
|
|
"github.com/shankar0123/certctl/internal/api/router"
|
|
)
|
|
|
|
// Bundle B / Audit M-002 (CWE-862): pin the dispatch-layer auth-exempt
|
|
// allowlist. cmd/server/main.go::buildFinalHandler decides per-request
|
|
// whether a path goes through the authenticated apiHandler or the
|
|
// no-auth handler. This test:
|
|
//
|
|
// - constructs a buildFinalHandler with two sentinel handlers (one
|
|
// for "auth", one for "no-auth") so we can observe which path is
|
|
// taken from the response body.
|
|
// - probes every prefix listed in router.AuthExemptDispatchPrefixes
|
|
// and confirms it routes to no-auth.
|
|
// - probes a few representative authenticated routes and confirms
|
|
// they route to auth.
|
|
// - probes the static-route allowlist (/health, /ready, etc.) that
|
|
// also bypasses auth at this layer.
|
|
//
|
|
// Adding a new auth-bypass to buildFinalHandler without updating the
|
|
// router.AuthExemptDispatchPrefixes constant fails this test.
|
|
|
|
func TestBuildFinalHandler_AuthExemptDispatchAllowlist(t *testing.T) {
|
|
apiHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
|
_, _ = w.Write([]byte("AUTH"))
|
|
})
|
|
noAuthHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
|
_, _ = w.Write([]byte("NOAUTH"))
|
|
})
|
|
|
|
// dashboardEnabled=false keeps the dispatch logic deterministic — no
|
|
// fileServer fallback to muddy the result.
|
|
final := buildFinalHandler(apiHandler, noAuthHandler, "/nonexistent", false)
|
|
|
|
cases := []struct {
|
|
name string
|
|
path string
|
|
want string
|
|
}{
|
|
// AuthExemptRouterRoutes (also enforced at this layer)
|
|
{"health", "/health", "NOAUTH"},
|
|
{"ready", "/ready", "NOAUTH"},
|
|
{"auth_info", "/api/v1/auth/info", "NOAUTH"},
|
|
{"version", "/api/v1/version", "NOAUTH"},
|
|
|
|
// AuthExemptDispatchPrefixes — every documented prefix
|
|
{"pki_crl", "/.well-known/pki/crl", "NOAUTH"},
|
|
{"pki_ocsp", "/.well-known/pki/ocsp", "NOAUTH"},
|
|
{"est_simpleenroll", "/.well-known/est/simpleenroll", "NOAUTH"},
|
|
{"est_cacerts", "/.well-known/est/cacerts", "NOAUTH"},
|
|
{"scep_root", "/scep", "NOAUTH"},
|
|
{"scep_op", "/scep/pkiclient.exe", "NOAUTH"},
|
|
|
|
// Authenticated routes — must hit apiHandler
|
|
{"certs_list", "/api/v1/certificates", "AUTH"},
|
|
{"agents_list", "/api/v1/agents", "AUTH"},
|
|
{"audit_check", "/api/v1/auth/check", "AUTH"},
|
|
|
|
// Random non-API path — falls through to apiHandler when
|
|
// dashboard disabled (preserves pre-M-001 API-only behavior).
|
|
{"unknown", "/some-other-path", "AUTH"},
|
|
}
|
|
|
|
for _, tc := range cases {
|
|
t.Run(tc.name, func(t *testing.T) {
|
|
req := httptest.NewRequest(http.MethodGet, tc.path, nil)
|
|
rec := httptest.NewRecorder()
|
|
final.ServeHTTP(rec, req)
|
|
got := rec.Body.String()
|
|
if got != tc.want {
|
|
t.Errorf("path %q routed to %q; want %q (this is the M-002 dispatch-layer pin)", tc.path, got, tc.want)
|
|
}
|
|
})
|
|
}
|
|
}
|
|
|
|
// TestDispatch_NoUndocumentedBypasses asserts that for every prefix the
|
|
// dispatch layer routes to noAuthHandler, that prefix appears in the
|
|
// router.AuthExemptDispatchPrefixes constant. This is the inverse pin —
|
|
// adding a new bypass to buildFinalHandler without updating the constant
|
|
// fails this test.
|
|
//
|
|
// We probe a curated set of "would-be-bypasses" derived from the actual
|
|
// dispatch source by reading buildFinalHandler's lines. If the dispatch
|
|
// logic adds a new prefix that ends up in the no-auth chain, the
|
|
// curated set must be extended in the same commit that updates the
|
|
// constant — this fails-loud rather than silently allowing a bypass.
|
|
func TestDispatch_NoUndocumentedBypasses(t *testing.T) {
|
|
for _, prefix := range router.AuthExemptDispatchPrefixes {
|
|
if !strings.HasPrefix(prefix, "/") {
|
|
t.Errorf("AuthExemptDispatchPrefixes entry %q must start with / for prefix matching", prefix)
|
|
}
|
|
}
|
|
// Every entry in router.AuthExemptDispatchPrefixes must round-trip
|
|
// through buildFinalHandler to noAuthHandler (covered by the table
|
|
// test above). This test additionally asserts the inverse: known
|
|
// authenticated prefixes do NOT match any documented bypass prefix.
|
|
authenticatedPrefixes := []string{
|
|
"/api/v1/certificates",
|
|
"/api/v1/agents",
|
|
"/api/v1/audit",
|
|
}
|
|
for _, ap := range authenticatedPrefixes {
|
|
for _, bypass := range router.AuthExemptDispatchPrefixes {
|
|
if strings.HasPrefix(ap, bypass) {
|
|
t.Errorf("authenticated prefix %q overlaps with documented bypass %q — auth bypass risk", ap, bypass)
|
|
}
|
|
}
|
|
}
|
|
}
|