mirror of
https://github.com/shankar0123/certctl.git
synced 2026-06-07 12:21:31 +00:00
fix(api/cors): narrow Bundle-2 routes from wildcard to NewCORS(corsCfg)
Closes CRIT-3 of the 2026-05-10 audit. Bundle 2's OIDC handshake +
back-channel-logout + logout + bootstrap + breakglass-login routes were
wrapped by middleware.CORS — a hard-coded
Access-Control-Allow-Origin: * middleware that ignored the operator's
CERTCTL_CORS_ORIGINS knob (CWE-942). The properly-configured
middleware.NewCORS(corsCfg) exists right next to it but wasn't used here.
The deprecation comment on middleware.CORS said "Kept for health endpoints"
but Bundle 2 added four additional call sites without converting them.
This commit:
- Renames middleware.CORS -> middleware.CORSWildcard with a stronger doc
block making the security tradeoff explicit at every remaining call
site. The doc references the CI guard + the 2026-05-10 audit closure.
- Adds a CorsCfg middleware.CORSConfig field to router.HandlerRegistry
and threads it from cmd/server/main.go using the existing
cfg.CORS.AllowedOrigins value. The same config that drives the global
corsMiddleware now also drives the per-route NewCORS wraps for the
auth-exempt direct r.mux.Handle blocks.
- Swaps middleware.CORS -> middleware.NewCORS(reg.CorsCfg) for the 7
credentialed auth-exempt routes:
- GET /auth/oidc/login
- GET /auth/oidc/callback
- POST /auth/oidc/back-channel-logout
- POST /auth/logout
- POST /auth/breakglass/login
- GET /api/v1/auth/bootstrap
- POST /api/v1/auth/bootstrap
- Keeps middleware.CORSWildcard for the 4 credential-free probe routes:
- GET /health
- GET /ready
- GET /api/v1/version
- GET /api/v1/auth/info
- Adds scripts/ci-guards/cors-wildcard-allowlist.sh — pins the 4-route
allowlist; fails CI when a new middleware.CORSWildcard wrap appears
outside the allowlist. Adding a new wildcard call site requires
updating the allowlist AND documenting why in the commit body.
Operators who configured CERTCTL_CORS_ORIGINS=https://admin.example.com
expecting the OIDC + BCL + breakglass-login routes to honor it now do.
Previously those routes ignored the knob and emitted ACAO: * regardless.
Verification gate green:
- gofmt -l . clean
- go vet ./... clean
- go test -short -count=1 ./internal/api/... ./internal/auth/...
./internal/domain/auth/ ./internal/service/auth/ ./cmd/server/ pass
- go build ./... clean
- scripts/ci-guards/cors-wildcard-allowlist.sh passes (4 allowlisted
routes; zero violations)
CRIT-1 + CRIT-2 from the same audit are already closed on this branch
(commits 68ca42f, ca1e135); CRIT-4 / CRIT-5 remain open and continue
to block the v2.1.0 tag. Spec:
cowork/auth-bundles-fixes-2026-05-10/03-crit-3-cors-narrow.md.
Refs: cowork/auth-bundles-audit-2026-05-10.md CRIT-3
This commit is contained in:
@@ -1335,6 +1335,11 @@ func main() {
|
||||
// admin_scep_intune, admin_est, intermediate_ca). Wraps live in
|
||||
// router.go via rbacGate(reg.Checker, perm, handler).
|
||||
Checker: authCheckerAdapter,
|
||||
// Audit 2026-05-10 CRIT-3 closure — operator-configured CORS
|
||||
// applied to the credentialed auth-exempt routes (OIDC handshake,
|
||||
// BCL, logout, bootstrap, breakglass-login). Health probes
|
||||
// continue to use middleware.CORSWildcard.
|
||||
CorsCfg: middleware.CORSConfig{AllowedOrigins: cfg.CORS.AllowedOrigins},
|
||||
})
|
||||
// Register EST (RFC 7030) handlers if enabled.
|
||||
//
|
||||
|
||||
@@ -371,9 +371,25 @@ func ContentType(next http.Handler) http.Handler {
|
||||
})
|
||||
}
|
||||
|
||||
// CORS middleware adds CORS headers to allow cross-origin requests.
|
||||
// Deprecated: Use NewCORS for configurable origins. Kept for health endpoints.
|
||||
func CORS(next http.Handler) http.Handler {
|
||||
// CORSWildcard emits Access-Control-Allow-Origin: * unconditionally. ONLY use
|
||||
// for endpoints that (a) carry no credentials and (b) must be reachable from
|
||||
// any origin (e.g. K8s/Docker health probes, Prometheus scrapers, the GUI's
|
||||
// pre-login auth-info probe). Every call site MUST appear in
|
||||
// scripts/ci-guards/cors-wildcard-allowlist.sh — adding a new call site
|
||||
// without listing it in the allowlist fails CI.
|
||||
//
|
||||
// For credentialed endpoints (sessions, OIDC handshake, BCL, bootstrap,
|
||||
// breakglass-login, every /api/v1/* mutation route) use
|
||||
// middleware.NewCORS(corsCfg) which honors CERTCTL_CORS_ORIGINS and emits
|
||||
// per-origin headers (with Vary: Origin for cache correctness).
|
||||
//
|
||||
// History: this function was named `CORS` pre-2026-05-10 and was applied as
|
||||
// the default CORS middleware on the OIDC handshake, BCL, logout, bootstrap,
|
||||
// and breakglass-login routes — CRIT-3 of the 2026-05-10 audit
|
||||
// (cowork/auth-bundles-audit-2026-05-10.md). The fix narrowed those call
|
||||
// sites to NewCORS(corsCfg) and renamed the wildcard form to make the
|
||||
// security tradeoff explicit at every remaining call site.
|
||||
func CORSWildcard(next http.Handler) http.Handler {
|
||||
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
w.Header().Set("Access-Control-Allow-Origin", "*")
|
||||
w.Header().Set("Access-Control-Allow-Methods", "GET, POST, PUT, DELETE, PATCH, OPTIONS")
|
||||
|
||||
@@ -195,6 +195,23 @@ type HandlerRegistry struct {
|
||||
// (only valid in tests / demo deployments — production MUST
|
||||
// configure a Checker).
|
||||
Checker auth.PermissionChecker
|
||||
|
||||
// CorsCfg is the operator-configured CORS middleware applied to the
|
||||
// credentialed auth-exempt routes (OIDC handshake, BCL, logout,
|
||||
// bootstrap, breakglass-login). Honors CERTCTL_CORS_ORIGINS — deny-
|
||||
// by-default when AllowedOrigins is empty. Audit 2026-05-10 CRIT-3
|
||||
// closure: previously these routes used middleware.CORSWildcard
|
||||
// (formerly middleware.CORS) which emitted Access-Control-Allow-
|
||||
// Origin: * regardless of operator config, ignoring the
|
||||
// CERTCTL_CORS_ORIGINS knob (CWE-942).
|
||||
//
|
||||
// Health probes (/health, /ready, /api/v1/version, /api/v1/auth/info)
|
||||
// continue to use middleware.CORSWildcard because they must be
|
||||
// reachable from any origin without credentials. Each wildcard call
|
||||
// site is listed in scripts/ci-guards/cors-wildcard-allowlist.sh —
|
||||
// the CI guard fails when a new wildcard wrap appears outside the
|
||||
// allowlist.
|
||||
CorsCfg middleware.CORSConfig
|
||||
// L-1 master closure (cat-l-fa0c1ac07ab5 + cat-l-8a1fb258a38a):
|
||||
// server-side bulk endpoints replace pre-L-1 client-side N×HTTP
|
||||
// loops in CertificatesPage.tsx. See handler/bulk_renewal.go and
|
||||
@@ -306,18 +323,18 @@ func (r *Router) RegisterHandlers(reg HandlerRegistry) {
|
||||
// Health endpoints (no auth middleware — must always be accessible)
|
||||
r.mux.Handle("GET /health", middleware.Chain(
|
||||
http.HandlerFunc(reg.Health.Health),
|
||||
middleware.CORS,
|
||||
middleware.CORSWildcard,
|
||||
middleware.ContentType,
|
||||
))
|
||||
r.mux.Handle("GET /ready", middleware.Chain(
|
||||
http.HandlerFunc(reg.Health.Ready),
|
||||
middleware.CORS,
|
||||
middleware.CORSWildcard,
|
||||
middleware.ContentType,
|
||||
))
|
||||
// Auth info endpoint (no auth middleware — GUI needs this before login)
|
||||
r.mux.Handle("GET /api/v1/auth/info", middleware.Chain(
|
||||
http.HandlerFunc(reg.Health.AuthInfo),
|
||||
middleware.CORS,
|
||||
middleware.CORSWildcard,
|
||||
middleware.ContentType,
|
||||
))
|
||||
// Version endpoint (no auth middleware — used by rollout probes that
|
||||
@@ -328,7 +345,7 @@ func (r *Router) RegisterHandlers(reg HandlerRegistry) {
|
||||
// is preferred when present.
|
||||
r.mux.Handle("GET /api/v1/version", middleware.Chain(
|
||||
reg.Version,
|
||||
middleware.CORS,
|
||||
middleware.CORSWildcard,
|
||||
middleware.ContentType,
|
||||
))
|
||||
// Auth check endpoint (uses full middleware chain via r.Register)
|
||||
@@ -340,12 +357,12 @@ func (r *Router) RegisterHandlers(reg HandlerRegistry) {
|
||||
// AuthExemptRouterRoutes allowlist above.
|
||||
r.mux.Handle("GET /api/v1/auth/bootstrap", middleware.Chain(
|
||||
http.HandlerFunc(reg.Bootstrap.Available),
|
||||
middleware.CORS,
|
||||
middleware.NewCORS(reg.CorsCfg),
|
||||
middleware.ContentType,
|
||||
))
|
||||
r.mux.Handle("POST /api/v1/auth/bootstrap", middleware.Chain(
|
||||
http.HandlerFunc(reg.Bootstrap.Mint),
|
||||
middleware.CORS,
|
||||
middleware.NewCORS(reg.CorsCfg),
|
||||
middleware.ContentType,
|
||||
))
|
||||
|
||||
@@ -407,19 +424,19 @@ func (r *Router) RegisterHandlers(reg HandlerRegistry) {
|
||||
// /auth/logout -> caller's own session cookie
|
||||
r.mux.Handle("GET /auth/oidc/login", middleware.Chain(
|
||||
http.HandlerFunc(reg.AuthSessionOIDC.LoginInitiate),
|
||||
middleware.CORS, middleware.ContentType,
|
||||
middleware.NewCORS(reg.CorsCfg), middleware.ContentType,
|
||||
))
|
||||
r.mux.Handle("GET /auth/oidc/callback", middleware.Chain(
|
||||
http.HandlerFunc(reg.AuthSessionOIDC.LoginCallback),
|
||||
middleware.CORS, middleware.ContentType,
|
||||
middleware.NewCORS(reg.CorsCfg), middleware.ContentType,
|
||||
))
|
||||
r.mux.Handle("POST /auth/oidc/back-channel-logout", middleware.Chain(
|
||||
http.HandlerFunc(reg.AuthSessionOIDC.BackChannelLogout),
|
||||
middleware.CORS, middleware.ContentType,
|
||||
middleware.NewCORS(reg.CorsCfg), middleware.ContentType,
|
||||
))
|
||||
r.mux.Handle("POST /auth/logout", middleware.Chain(
|
||||
http.HandlerFunc(reg.AuthSessionOIDC.Logout),
|
||||
middleware.CORS, middleware.ContentType,
|
||||
middleware.NewCORS(reg.CorsCfg), middleware.ContentType,
|
||||
))
|
||||
|
||||
// Session management. auth.session.list gates the all-actors
|
||||
@@ -457,7 +474,7 @@ func (r *Router) RegisterHandlers(reg HandlerRegistry) {
|
||||
if reg.AuthBreakglass != nil {
|
||||
r.mux.Handle("POST /auth/breakglass/login", middleware.Chain(
|
||||
http.HandlerFunc(reg.AuthBreakglass.Login),
|
||||
middleware.CORS, middleware.ContentType,
|
||||
middleware.NewCORS(reg.CorsCfg), middleware.ContentType,
|
||||
))
|
||||
r.Register("POST /api/v1/auth/breakglass/credentials", rbacGate(reg.Checker, "auth.breakglass.admin", reg.AuthBreakglass.SetPassword))
|
||||
r.Register("POST /api/v1/auth/breakglass/credentials/{actor_id}/unlock", rbacGate(reg.Checker, "auth.breakglass.admin", reg.AuthBreakglass.Unlock))
|
||||
|
||||
Executable
+85
@@ -0,0 +1,85 @@
|
||||
#!/usr/bin/env bash
|
||||
# cors-wildcard-allowlist.sh — Audit 2026-05-10 CRIT-3 ratchet.
|
||||
#
|
||||
# middleware.CORSWildcard (formerly middleware.CORS) emits
|
||||
# Access-Control-Allow-Origin: * unconditionally, ignoring the operator's
|
||||
# CERTCTL_CORS_ORIGINS knob (CWE-942). It is ONLY safe to use on endpoints
|
||||
# that (a) carry no credentials and (b) must be reachable from any origin
|
||||
# (health probes, version probes, the GUI's pre-login auth-info probe).
|
||||
#
|
||||
# This guard greps for every middleware.CORSWildcard call site, extracts
|
||||
# the nearest preceding r.mux.Handle("…") route string, and asserts that
|
||||
# the route appears in the documented ALLOWLIST below. Adding a new
|
||||
# wildcard-CORS wrap therefore requires either:
|
||||
#
|
||||
# 1. Adding the route to ALLOWLIST below AND documenting why in the
|
||||
# commit body, or
|
||||
# 2. Switching the call site to middleware.NewCORS(reg.CorsCfg).
|
||||
#
|
||||
# Closes CRIT-3 of cowork/auth-bundles-audit-2026-05-10.md. See also
|
||||
# internal/api/middleware/middleware.go::CORSWildcard for the doc block.
|
||||
|
||||
set -euo pipefail
|
||||
|
||||
ROUTER=internal/api/router/router.go
|
||||
|
||||
# Routes allowed to use middleware.CORSWildcard. Every entry must be a
|
||||
# credential-free endpoint that operators expect to be reachable from any
|
||||
# origin (Kubernetes probes, Prometheus, the pre-login GUI).
|
||||
ALLOWLIST=(
|
||||
"GET /health" # K8s/Docker liveness probe
|
||||
"GET /ready" # K8s/Docker readiness probe
|
||||
"GET /api/v1/version" # rollout probes; pre-auth
|
||||
"GET /api/v1/auth/info" # GUI reads before login
|
||||
)
|
||||
|
||||
if [[ ! -f "$ROUTER" ]]; then
|
||||
echo "FAIL: $ROUTER not found (run from certctl/ root)"
|
||||
exit 1
|
||||
fi
|
||||
|
||||
# Extract every (route, wrap) pair from the router by finding each
|
||||
# r.mux.Handle("ROUTE", ...) block and checking whether its wrapping list
|
||||
# contains middleware.CORSWildcard.
|
||||
python3 - <<PY
|
||||
import re, sys
|
||||
|
||||
ALLOWLIST = [
|
||||
"GET /health",
|
||||
"GET /ready",
|
||||
"GET /api/v1/version",
|
||||
"GET /api/v1/auth/info",
|
||||
]
|
||||
allowset = set(ALLOWLIST)
|
||||
|
||||
with open("$ROUTER", "r") as f:
|
||||
src = f.read()
|
||||
|
||||
# Find every r.mux.Handle("ROUTE", middleware.Chain(... or direct chain)) block
|
||||
# and check whether middleware.CORSWildcard appears within the next ~400 chars.
|
||||
violations = []
|
||||
seen = []
|
||||
for m in re.finditer(r'r\.mux\.Handle\("([^"]+)",', src):
|
||||
route = m.group(1)
|
||||
region = src[m.end(): m.end() + 600]
|
||||
if "middleware.CORSWildcard" not in region:
|
||||
continue
|
||||
seen.append(route)
|
||||
if route not in allowset:
|
||||
violations.append(route)
|
||||
|
||||
if violations:
|
||||
print("FAIL: middleware.CORSWildcard call sites outside the allowlist:")
|
||||
for r in violations:
|
||||
print(" " + r)
|
||||
print()
|
||||
print("If a new wildcard-CORS endpoint is intentional, add the route to")
|
||||
print("ALLOWLIST in scripts/ci-guards/cors-wildcard-allowlist.sh AND")
|
||||
print("document why in the commit body. Otherwise switch the call site")
|
||||
print("to middleware.NewCORS(reg.CorsCfg).")
|
||||
sys.exit(1)
|
||||
|
||||
print(f"OK: {len(seen)} middleware.CORSWildcard call site(s); all allowlisted.")
|
||||
for r in seen:
|
||||
print(f" - {r}")
|
||||
PY
|
||||
Reference in New Issue
Block a user