fix(auth): harden OIDC flow, token validation, session, and bearer handling

- replay entry covers exp + clock-skew window; callback GET-only (RFC 6749)
- callback rate-limit returns 429 + Retry-After; panic-recovery sets no-store
- cross-chunk integrity for chunked tokens; bounded chunked-token read scan
- HTTPError classification so 5xx/429 token-endpoint responses retry
- redaction of token/code/state in logs; per-source external auth gate
This commit is contained in:
2026-08-14 16:16:26 +01:00
parent 3eb11b07e9
commit 8640451877
19 changed files with 2002 additions and 508 deletions
+157 -33
View File
@@ -1,6 +1,7 @@
package traefikoidc package traefikoidc
import ( import (
"errors"
"fmt" "fmt"
"net/http" "net/http"
"strings" "strings"
@@ -14,6 +15,16 @@ func (t *TraefikOidc) validateRedirectCount(session *SessionData, rw http.Respon
if redirectCount >= maxRedirects { if redirectCount >= maxRedirects {
t.logger.Errorf("Maximum redirect limit (%d) exceeded, possible redirect loop detected", maxRedirects) t.logger.Errorf("Maximum redirect limit (%d) exceeded, possible redirect loop detected", maxRedirects)
session.ResetRedirectCount() session.ResetRedirectCount()
// Persist the reset so the NEXT request starts at 0 and can
// attempt auth again. Without Save, the reset only touched the
// in-memory session value (dirty flag set but never flushed): the
// cookie still carried the old count, so every subsequent
// request re-observed redirect_count==max and the user was
// hard-locked on 508 for the whole session with no recovery
// (R123).
if err := session.Save(req, rw); err != nil {
t.logger.Errorf("Failed to save session after redirect reset: %v", err)
}
t.sendErrorResponse(rw, req, "Authentication failed: Too many redirects", http.StatusLoopDetected) t.sendErrorResponse(rw, req, "Authentication failed: Too many redirects", http.StatusLoopDetected)
return fmt.Errorf("redirect limit exceeded") return fmt.Errorf("redirect limit exceeded")
} }
@@ -68,9 +79,34 @@ func (t *TraefikOidc) prepareSessionForAuthentication(session *SessionData, csrf
// - req: The HTTP request initiating authentication. // - req: The HTTP request initiating authentication.
// - session: The session data to prepare for authentication. // - session: The session data to prepare for authentication.
// - redirectURL: The pre-calculated callback URL (redirect_uri) for this middleware instance. // - redirectURL: The pre-calculated callback URL (redirect_uri) for this middleware instance.
//
// rateLimitPerSourceAuth applies the (optional) per-external-source auth
// throttle at an OIDC gate. Returns true if the request was already
// answered with 429 and the caller must return. No-op when the limiter is
// disabled (PerSourceLoginRateLimit <= 0) or the source is internal.
func (t *TraefikOidc) rateLimitPerSourceAuth(rw http.ResponseWriter, req *http.Request) bool {
if t.perSourceLimiter == nil {
return false
}
if t.perSourceLimiter.allow(req) {
return false
}
rw.Header().Set("Retry-After", "1")
t.sendErrorResponse(rw, req, "Too many authentication attempts, please try again shortly", http.StatusTooManyRequests)
return true
}
func (t *TraefikOidc) defaultInitiateAuthentication(rw http.ResponseWriter, req *http.Request, session *SessionData, redirectURL string) { func (t *TraefikOidc) defaultInitiateAuthentication(rw http.ResponseWriter, req *http.Request, session *SessionData, redirectURL string) {
t.logger.Debugf("Initiating new OIDC authentication flow for request: %s", req.URL.RequestURI()) t.logger.Debugf("Initiating new OIDC authentication flow for request: %s", req.URL.RequestURI())
// Per-external-source throttle on login initiation: a single host
// flooding the route with auth starts shouldn't be able to spin up an
// unbounded number of IdP round-trips or wear the shared token-origin
// bucket down for everyone (R185).
if t.rateLimitPerSourceAuth(rw, req) {
return
}
// Check and handle redirect limits // Check and handle redirect limits
if err := t.validateRedirectCount(session, rw, req); err != nil { if err := t.validateRedirectCount(session, rw, req); err != nil {
return return
@@ -97,9 +133,42 @@ func (t *TraefikOidc) defaultInitiateAuthentication(rw http.ResponseWriter, req
return return
} }
// Build the provider authorize URL BEFORE mutating the session. A URL-
// build failure must not clear a valid existing session, and must not
// emit an empty-Location redirect (browsers reload the current page,
// re-entering this flow and bumping the redirect count until the hard
// 508). Fail closed with an explicit status instead (R142).
authURL := t.buildAuthURL(redirectURL, csrfToken, nonce, codeChallenge)
if authURL == "" {
t.logger.Errorf("Failed to build provider authorize URL; aborting login flow")
http.Error(rw, "Failed to build provider authorize URL", http.StatusInternalServerError)
return
}
// When dynamic client registration is enabled but has not yet produced a
// client_id (registration pending or failed), forging an authorize URL
// with client_id="" would send the user to the provider on a broken
// flow. Fail with a clear status instead of redirecting. This guard
// must run BEFORE prepareSessionForAuthentication re-seeds the session:
// re-seeding (and the save that follows) would otherwise mutate and
// persist the session even though we are about to answer 503 with no
// redirect — discarding any existing session state for no reason (R163).
// Snapshot clientID under metadataMu: DCR rewrites it at runtime (R137).
clientID, _, _, _, _ := t.clientCredentials()
if t.dcrConfig != nil && t.dcrConfig.Enabled && clientID == "" {
t.logger.Errorf("OIDC dynamic client registration has not produced a client_id; refusing to redirect to provider")
http.Error(rw, "Identity provider registration pending", http.StatusServiceUnavailable)
return
}
// Clear existing session data and set new authentication state // Clear existing session data and set new authentication state
t.prepareSessionForAuthentication(session, csrfToken, nonce, codeVerifier, req.URL.RequestURI()) t.prepareSessionForAuthentication(session, csrfToken, nonce, codeVerifier, req.URL.RequestURI())
// Persist the redirect_uri so the callback reuses the exact value sent
// to the IdP, instead of rebuilding it from the (client-affectable)
// callback request's X-Forwarded-Host (R136).
session.SetRedirectURL(redirectURL)
session.MarkDirty() session.MarkDirty()
if err := session.Save(req, rw); err != nil { if err := session.Save(req, rw); err != nil {
@@ -111,20 +180,13 @@ func (t *TraefikOidc) defaultInitiateAuthentication(rw http.ResponseWriter, req
t.logger.Debugf("Session saved before redirect. CSRF: %s, Nonce: %s", t.logger.Debugf("Session saved before redirect. CSRF: %s, Nonce: %s",
csrfToken, nonce) csrfToken, nonce)
authURL := t.buildAuthURL(redirectURL, csrfToken, nonce, codeChallenge)
// When dynamic client registration is enabled but has not yet produced a
// client_id (registration pending or failed), forging an authorize URL
// with client_id="" would send the user to the provider on a broken
// flow. Fail with a clear status instead of redirecting.
if t.dcrConfig != nil && t.dcrConfig.Enabled && t.clientID == "" {
t.logger.Errorf("OIDC dynamic client registration has not produced a client_id; refusing to redirect to provider")
http.Error(rw, "Identity provider registration pending", http.StatusServiceUnavailable)
return
}
t.logger.Debugf("Redirecting user to OIDC provider: %s", authURL) t.logger.Debugf("Redirecting user to OIDC provider: %s", authURL)
// 302s into the IdP must not be heuristically cacheable (RFC 7234):
// a stale, cached redirect could replay the pre-re-authentication URL.
// Every other auth response sets no-store; these redirects were missed
// (R147).
rw.Header().Set("Cache-Control", "no-store")
http.Redirect(rw, req, authURL, http.StatusFound) http.Redirect(rw, req, authURL, http.StatusFound)
} }
@@ -135,7 +197,31 @@ func (t *TraefikOidc) defaultInitiateAuthentication(rw http.ResponseWriter, req
// - rw: The HTTP response writer. // - rw: The HTTP response writer.
// - req: The callback request containing authorization code and state. // - req: The callback request containing authorization code and state.
// - redirectURL: The fully qualified callback URL (used in the token exchange request). // - redirectURL: The fully qualified callback URL (used in the token exchange request).
//
// clearOneTimeAuthState clears the one-time CSRF/nonce/PKCE-verifier after a
// callback that consumed the authorization code but then failed, and persists
// the cleared state. Without it the failed session stays in a pending-auth
// state and a retried callback would re-exchange a fresh code before failing
// again. Mirrors the cleanup on the exchange-failure / empty-access-token
// paths (R134).
func (t *TraefikOidc) clearOneTimeAuthState(session *SessionData, req *http.Request, rw http.ResponseWriter) {
session.SetCSRF("")
session.SetNonce("")
session.SetCodeVerifier("")
if saveErr := session.Save(req, rw); saveErr != nil {
t.logger.Errorf("Failed to save session after callback failure: %v", saveErr)
}
}
func (t *TraefikOidc) handleCallback(rw http.ResponseWriter, req *http.Request, redirectURL string) { func (t *TraefikOidc) handleCallback(rw http.ResponseWriter, req *http.Request, redirectURL string) {
// Per-external-source throttle on the authorization-code callback: this
// is where the one-time code exchange (server round-trip + token
// cache write) happens, so a single source can't brute-force
// repeated code exchanges or drown the shared limiter (R185).
if t.rateLimitPerSourceAuth(rw, req) {
return
}
session, err := t.sessionManager.GetSession(req) session, err := t.sessionManager.GetSession(req)
if err != nil { if err != nil {
t.logger.Errorf("Session error during callback: %v", err) t.logger.Errorf("Session error during callback: %v", err)
@@ -144,15 +230,28 @@ func (t *TraefikOidc) handleCallback(rw http.ResponseWriter, req *http.Request,
} }
defer session.returnToPoolSafely() defer session.returnToPoolSafely()
t.logger.Debugf("Handling callback, URL: %s", req.URL.String()) t.logger.Debugf("Handling callback, URL: %s", redactCallbackURL(req.URL.String()))
if req.URL.Query().Get("error") != "" { if req.URL.Query().Get("error") != "" {
errorDescription := req.URL.Query().Get("error_description") errorDescription := req.URL.Query().Get("error_description")
if errorDescription == "" { if errorDescription == "" {
errorDescription = req.URL.Query().Get("error") errorDescription = req.URL.Query().Get("error")
} }
t.logger.Errorf("Authentication error from provider during callback: %s - %s", req.URL.Query().Get("error"), errorDescription) // error / error_description are attacker-controlled query params.
t.sendErrorResponse(rw, req, fmt.Sprintf("Authentication error from provider: %s", errorDescription), http.StatusBadRequest) // Sanitize before logging and before echoing back to the client:
// an embedded newline would inject a forged line into (especially
// structured) logs, and unbounded length would bloat the log and
// the error response (R145).
sanitizeErr := func(s string) string {
s = strings.NewReplacer("\n", " ", "\r", " ").Replace(s)
const maxErrLen = 512
if len(s) > maxErrLen {
s = s[:maxErrLen]
}
return s
}
t.logger.Errorf("Authentication error from provider during callback: %s - %s", sanitizeErr(req.URL.Query().Get("error")), sanitizeErr(errorDescription))
t.sendErrorResponse(rw, req, fmt.Sprintf("Authentication error from provider: %s", sanitizeErr(errorDescription)), http.StatusBadRequest)
return return
} }
@@ -166,7 +265,7 @@ func (t *TraefikOidc) handleCallback(rw http.ResponseWriter, req *http.Request,
csrfToken := session.GetCSRF() csrfToken := session.GetCSRF()
if csrfToken == "" { if csrfToken == "" {
t.logger.Errorf("CSRF token missing in session during callback. Authenticated: %v, Request URL: %s", t.logger.Errorf("CSRF token missing in session during callback. Authenticated: %v, Request URL: %s",
session.GetAuthenticated(), req.URL.String()) session.GetAuthenticated(), redactCallbackURL(req.URL.String()))
cookie, err := req.Cookie("_oidc_raczylo_m") cookie, err := req.Cookie("_oidc_raczylo_m")
if err != nil { if err != nil {
@@ -199,6 +298,13 @@ func (t *TraefikOidc) handleCallback(rw http.ResponseWriter, req *http.Request,
return return
} }
// Use the redirect_uri persisted at initiate time (must match what was
// sent to the IdP's authorize endpoint). Fall back to the rebuilt one
// only for pre-existing sessions that predate R136 (R136).
if stored := session.GetRedirectURL(); stored != "" {
redirectURL = stored
}
tokenResponse, err := t.tokenExchanger.ExchangeCodeForToken(req.Context(), "authorization_code", code, redirectURL, codeVerifier) tokenResponse, err := t.tokenExchanger.ExchangeCodeForToken(req.Context(), "authorization_code", code, redirectURL, codeVerifier)
if err != nil { if err != nil {
t.logger.Errorf("Failed to exchange code for token during callback: %v", err) t.logger.Errorf("Failed to exchange code for token during callback: %v", err)
@@ -207,30 +313,29 @@ func (t *TraefikOidc) handleCallback(rw http.ResponseWriter, req *http.Request,
// may still be unconsumed at the provider, so a replayed callback // may still be unconsumed at the provider, so a replayed callback
// must re-authenticate rather than revalidate a stale // must re-authenticate rather than revalidate a stale
// state/nonce/code_verifier. Mirrors the cleanup on success. // state/nonce/code_verifier. Mirrors the cleanup on success.
session.SetCSRF("") t.clearOneTimeAuthState(session, req, rw)
session.SetNonce("")
session.SetCodeVerifier("")
if saveErr := session.Save(req, rw); saveErr != nil {
t.logger.Errorf("Failed to save session after callback exchange failure: %v", saveErr)
}
t.sendErrorResponse(rw, req, "Authentication failed: Could not exchange code for token", http.StatusInternalServerError) t.sendErrorResponse(rw, req, "Authentication failed: Could not exchange code for token", http.StatusInternalServerError)
return return
} }
if tokenResponse.AccessToken == "" { if tokenResponse.AccessToken == "" {
t.logger.Errorf("Token endpoint returned no access token during callback") t.logger.Errorf("Token endpoint returned no access token during callback")
session.SetCSRF("") t.clearOneTimeAuthState(session, req, rw)
session.SetNonce("")
session.SetCodeVerifier("")
if saveErr := session.Save(req, rw); saveErr != nil {
t.logger.Errorf("Failed to save session after empty access token: %v", saveErr)
}
t.sendErrorResponse(rw, req, "Authentication failed: Token endpoint returned no access token", http.StatusInternalServerError) t.sendErrorResponse(rw, req, "Authentication failed: Token endpoint returned no access token", http.StatusInternalServerError)
return return
} }
if err = t.verifyToken(tokenResponse.IDToken); err != nil { if err = t.verifyToken(tokenResponse.IDToken); err != nil {
t.logger.Errorf("Failed to verify id_token during callback: %v", err) t.logger.Errorf("Failed to verify id_token during callback: %v", err)
t.clearOneTimeAuthState(session, req, rw)
if errors.Is(err, ErrRateLimitExceeded) {
// Rate-limit exhaustion must be a 429 + Retry-After, not a
// generic 500: a single source's burst currently degrades
// every user's callback to "server error" (R179).
rw.Header().Set("Retry-After", "1")
t.sendErrorResponse(rw, req, "Authentication failed: Too many requests, please retry", http.StatusTooManyRequests)
return
}
t.sendErrorResponse(rw, req, "Authentication failed: Could not verify ID token", http.StatusInternalServerError) t.sendErrorResponse(rw, req, "Authentication failed: Could not verify ID token", http.StatusInternalServerError)
return return
} }
@@ -238,6 +343,7 @@ func (t *TraefikOidc) handleCallback(rw http.ResponseWriter, req *http.Request,
claims, err := t.extractClaimsFunc(tokenResponse.IDToken) claims, err := t.extractClaimsFunc(tokenResponse.IDToken)
if err != nil { if err != nil {
t.logger.Errorf("Failed to extract claims during callback: %v", err) t.logger.Errorf("Failed to extract claims during callback: %v", err)
t.clearOneTimeAuthState(session, req, rw)
t.sendErrorResponse(rw, req, "Authentication failed: Could not extract claims from token", http.StatusInternalServerError) t.sendErrorResponse(rw, req, "Authentication failed: Could not extract claims from token", http.StatusInternalServerError)
return return
} }
@@ -245,41 +351,57 @@ func (t *TraefikOidc) handleCallback(rw http.ResponseWriter, req *http.Request,
nonceClaim, ok := claims["nonce"].(string) nonceClaim, ok := claims["nonce"].(string)
if !ok || nonceClaim == "" { if !ok || nonceClaim == "" {
t.logger.Error("Nonce claim missing in id_token during callback") t.logger.Error("Nonce claim missing in id_token during callback")
t.sendErrorResponse(rw, req, "Authentication failed: Nonce missing in token", http.StatusInternalServerError) t.clearOneTimeAuthState(session, req, rw)
t.sendErrorResponse(rw, req, "Authentication failed: Nonce missing in token", http.StatusBadRequest)
return return
} }
sessionNonce := session.GetNonce() sessionNonce := session.GetNonce()
if sessionNonce == "" { if sessionNonce == "" {
t.logger.Error("Nonce not found in session during callback") t.logger.Error("Nonce not found in session during callback")
t.sendErrorResponse(rw, req, "Authentication failed: Nonce missing in session", http.StatusInternalServerError) t.clearOneTimeAuthState(session, req, rw)
t.sendErrorResponse(rw, req, "Authentication failed: Nonce missing in session", http.StatusBadRequest)
return return
} }
if !constantTimeStringCompare(nonceClaim, sessionNonce) { if !constantTimeStringCompare(nonceClaim, sessionNonce) {
t.logger.Error("Nonce claim does not match session nonce during callback") t.logger.Error("Nonce claim does not match session nonce during callback")
t.sendErrorResponse(rw, req, "Authentication failed: Nonce mismatch", http.StatusInternalServerError) t.clearOneTimeAuthState(session, req, rw)
t.sendErrorResponse(rw, req, "Authentication failed: Nonce mismatch", http.StatusBadRequest)
return return
} }
// Extract user identifier from the configured claim (defaults to "email" for backward compatibility) // Extract user identifier from the configured claim (defaults to "email" for backward compatibility)
userIdentifier, _ := claims[t.userIdentifierClaim].(string) userIdentifier, _ := claimScalarString(claims[t.userIdentifierClaim])
if userIdentifier == "" { if userIdentifier == "" {
// Try "sub" as fallback since it's required by OIDC spec // Try "sub" as fallback since it's required by OIDC spec
if t.userIdentifierClaim != "sub" { if t.userIdentifierClaim != "sub" {
userIdentifier, _ = claims["sub"].(string) userIdentifier, _ = claimScalarString(claims["sub"])
} }
if userIdentifier == "" { if userIdentifier == "" {
t.logger.Errorf("User identifier claim '%s' missing or empty in token during callback", t.userIdentifierClaim) t.logger.Errorf("User identifier claim '%s' missing or empty in token during callback", t.userIdentifierClaim)
t.clearOneTimeAuthState(session, req, rw)
t.sendErrorResponse(rw, req, "Authentication failed: User identifier missing in token", http.StatusInternalServerError) t.sendErrorResponse(rw, req, "Authentication failed: User identifier missing in token", http.StatusInternalServerError)
return return
} }
t.logger.Debugf("Configured claim '%s' not found, using 'sub' claim as fallback", t.userIdentifierClaim) t.logger.Debugf("Configured claim '%s' not found, using 'sub' claim as fallback", t.userIdentifierClaim)
} }
// Validate user authorization. When the identity is an email, require the
// IdP not to have explicitly flagged it as unverified — an unverified
// address must not satisfy an email/domain allowlist (see
// emailIdentityPermitted).
if !emailIdentityPermitted(userIdentifier, claims) {
t.logger.Errorf("User email not verified during callback, rejecting email-based authorization: %s", userIdentifier)
t.clearOneTimeAuthState(session, req, rw)
t.sendErrorResponse(rw, req, "Authentication failed: Email not verified", http.StatusForbidden)
return
}
// Validate user authorization // Validate user authorization
if !t.isAllowedUser(userIdentifier) { if !t.isAllowedUser(userIdentifier) {
t.logger.Errorf("User not authorized during callback: %s", userIdentifier) t.logger.Errorf("User not authorized during callback: %s", userIdentifier)
t.clearOneTimeAuthState(session, req, rw)
t.sendErrorResponse(rw, req, "Authentication failed: User not authorized", http.StatusForbidden) t.sendErrorResponse(rw, req, "Authentication failed: User not authorized", http.StatusForbidden)
return return
} }
@@ -316,6 +438,8 @@ func (t *TraefikOidc) handleCallback(rw http.ResponseWriter, req *http.Request,
} }
t.logger.Debugf("Callback successful, redirecting to %s", redirectPath) t.logger.Debugf("Callback successful, redirecting to %s", redirectPath)
// no-store so the post-auth redirect isn't cached and replayed (R147).
rw.Header().Set("Cache-Control", "no-store")
http.Redirect(rw, req, redirectPath, http.StatusFound) http.Redirect(rw, req, redirectPath, http.StatusFound)
} }
+248 -24
View File
@@ -29,6 +29,7 @@ import (
"encoding/base64" "encoding/base64"
"encoding/hex" "encoding/hex"
"encoding/json" "encoding/json"
"errors"
"fmt" "fmt"
"net" "net"
"net/http" "net/http"
@@ -93,6 +94,10 @@ const (
type bearerError struct { type bearerError struct {
kind bearerErrorKind kind bearerErrorKind
reason string reason string
// retryAfter carries the actual remaining penalty when known (e.g. the
// per-IP penalty box), so writeBearerError emits the true value instead
// of the full configured penalty.
retryAfter time.Duration
} }
func (e *bearerError) Error() string { return e.reason } func (e *bearerError) Error() string { return e.reason }
@@ -110,6 +115,32 @@ type joseHeader struct {
Typ string `json:"typ"` Typ string `json:"typ"`
} }
// joseHeaderWellFormed reports whether token is structurally a JWT: exactly
// two dot-separated segments whose header decodes as a JSON JOSE header.
// It intentionally applies NO alg/kid policy (unlike parseBearerJOSEHeader)
// so the JWT-vs-opaque routing gate classifies purely on structure: a
// structurally-valid JWT with a disallowed alg stays on the JWT path
// (where local policy rejects it), while an opaque token whose two dots
// are coincidental falls through to opaque introspection (R166).
func joseHeaderWellFormed(token string) bool {
dot := strings.IndexByte(token, '.')
if dot <= 0 {
return false
}
if strings.Count(token[dot+1:], ".") != 1 {
return false
}
raw, err := base64.RawURLEncoding.DecodeString(token[:dot])
if err != nil {
raw, err = base64.URLEncoding.DecodeString(token[:dot])
if err != nil {
return false
}
}
var hdr joseHeader
return json.Unmarshal(raw, &hdr) == nil
}
// parseBearerJOSEHeader decodes the first JWT segment for early alg/kid pinning. // parseBearerJOSEHeader decodes the first JWT segment for early alg/kid pinning.
// Does not touch the payload or signature — those are the verifier's job. // Does not touch the payload or signature — those are the verifier's job.
// Returns nil on success; *bearerError on rejection so the handler can map // Returns nil on success; *bearerError on rejection so the handler can map
@@ -153,19 +184,22 @@ func parseBearerJOSEHeader(token string) *bearerError {
// header value, or "" if the rune is acceptable. Shared core of the bearer-path // header value, or "" if the rune is acceptable. Shared core of the bearer-path
// identifier sanitizer and the cookie-path header claim sanitizer: rejects // identifier sanitizer and the cookie-path header claim sanitizer: rejects
// control chars (CRLF/header injection), Unicode bidi-override runes (RTL // control chars (CRLF/header injection), Unicode bidi-override runes (RTL
// spoofing of admin UI / SIEM), and the delimiters , ; = (a comma in a group // spoofing of admin UI / SIEM), and the delimiters , ; = plus the bracketing
// name would inject extra entries into a comma-joined header). // characters " { } — each of these in a group name / identifier would break a
// downstream comma-joined list or a naive CSV/JSON-style parser consuming the
// header (a comma would inject extra entries into a comma-joined header).
func headerClaimRuneReason(r rune) string { func headerClaimRuneReason(r rune) string {
if reason := headerInjectionRuneReason(r); reason != "" { if reason := headerInjectionRuneReason(r); reason != "" {
return reason return reason
} }
// The , ; = delimiters are only unsafe for values placed into delimited or // The , ; = delimiters and the " { } bracketing chars are only unsafe for
// list contexts (a comma-joined header, or an identifier downstreams may // values placed into delimited or list contexts (a comma-joined header, or
// split). They are valid in arbitrary single header values, so this stricter // an identifier downstreams may split). They are valid in arbitrary single
// check is used for the cookie-path identifier and the group/role list, NOT // header values, so this stricter check is used for the cookie-path
// for free-form templated header output (see headerValueReason). // identifier and the group/role list, NOT for free-form templated header
if r == ',' || r == ';' || r == '=' { // output (see headerValueReason).
return "delimiter character" if r == ',' || r == ';' || r == '=' || r == '"' || r == '{' || r == '}' {
return "delimiter or bracketing character"
} }
return "" return ""
} }
@@ -270,11 +304,12 @@ func resolveBearerIdentifier(claims map[string]interface{}, claimName string) (s
if !ok { if !ok {
return "", newBearerError(bearerErrInvalidIdentifier, fmt.Sprintf("missing claim %q", claimName)) return "", newBearerError(bearerErrInvalidIdentifier, fmt.Sprintf("missing claim %q", claimName))
} }
str, ok := raw.(string) // Accept scalar numbers (e.g. a numeric sub) by stringifying them,
if !ok { // so a numeric identifier claim doesn't 401 a valid principal.
return "", newBearerError(bearerErrInvalidIdentifier, fmt.Sprintf("claim %q not a string", claimName)) if s, isScalar := claimScalarString(raw); isScalar {
return s, nil
} }
return str, nil return "", newBearerError(bearerErrInvalidIdentifier, fmt.Sprintf("claim %q is not a scalar string or number", claimName))
} }
// enforceMultiAudienceAzp implements the spec hardening: when aud is a // enforceMultiAudienceAzp implements the spec hardening: when aud is a
@@ -358,6 +393,10 @@ type bearerFailureEntry struct {
count int count int
} }
// defaultBearerEntrySweepThreshold is the map size above which recordFailure
// sweeps stale entries, bounding memory under a flood of distinct source IPs.
const defaultBearerEntrySweepThreshold = 1024
func newBearerFailureTracker(threshold int, window, penalty time.Duration) *bearerFailureTracker { func newBearerFailureTracker(threshold int, window, penalty time.Duration) *bearerFailureTracker {
if threshold <= 0 { if threshold <= 0 {
threshold = 20 threshold = 20
@@ -404,6 +443,18 @@ func (b *bearerFailureTracker) recordFailure(ip string) {
b.mu.Lock() b.mu.Lock()
defer b.mu.Unlock() defer b.mu.Unlock()
now := time.Now() now := time.Now()
// Bound per-IP map growth (R117): entries for source IPs that fail once
// and never return were never removed. Sweep stale entries (no active
// penalty box, too old to matter) once the map grows past a nominal
// size, keeping steady-state memory bounded.
if len(b.entries) > defaultBearerEntrySweepThreshold {
cutoff := now.Add(-(b.window + b.penalty))
for k, e := range b.entries {
if e.penaltyUntil.Before(cutoff) {
delete(b.entries, k)
}
}
}
e, ok := b.entries[ip] e, ok := b.entries[ip]
if !ok || now.Sub(e.firstFailureAt) > b.window { if !ok || now.Sub(e.firstFailureAt) > b.window {
e = &bearerFailureEntry{firstFailureAt: now} e = &bearerFailureEntry{firstFailureAt: now}
@@ -520,7 +571,10 @@ func (t *TraefikOidc) writeBearerError(rw http.ResponseWriter, req *http.Request
case bearerErrThrottled: case bearerErrThrottled:
status = http.StatusTooManyRequests status = http.StatusTooManyRequests
body = "Too Many Requests" body = "Too Many Requests"
retryAfter = t.bearerFailurePenalty retryAfter = err.retryAfter
if retryAfter <= 0 {
retryAfter = t.bearerFailurePenalty
}
case bearerErrIntrospectionUnavailable: case bearerErrIntrospectionUnavailable:
status = http.StatusServiceUnavailable status = http.StatusServiceUnavailable
body = "Service Unavailable" body = "Service Unavailable"
@@ -535,6 +589,10 @@ func (t *TraefikOidc) writeBearerError(rw http.ResponseWriter, req *http.Request
if retryAfter > 0 { if retryAfter > 0 {
rw.Header().Set("Retry-After", fmt.Sprintf("%d", int(retryAfter.Seconds()))) rw.Header().Set("Retry-After", fmt.Sprintf("%d", int(retryAfter.Seconds())))
} }
// Auth-rejection responses must never be cached (R101 contract): a
// cached 429/401 could be replayed to a client well after the
// penalty box expired or the session recovered.
rw.Header().Set("Cache-Control", "no-store")
rw.Header().Set("Content-Type", "text/plain; charset=utf-8") rw.Header().Set("Content-Type", "text/plain; charset=utf-8")
rw.WriteHeader(status) rw.WriteHeader(status)
_, _ = rw.Write([]byte(body)) // Safe to ignore: best-effort error body write _, _ = rw.Write([]byte(body)) // Safe to ignore: best-effort error body write
@@ -553,12 +611,11 @@ func (t *TraefikOidc) handleBearerRequest(rw http.ResponseWriter, req *http.Requ
ip := clientIPForBearer(req) ip := clientIPForBearer(req)
if blocked, retryAfter := t.bearerFailureTracker.blocked(ip); blocked { if blocked, retryAfter := t.bearerFailureTracker.blocked(ip); blocked {
// Carry the actual remaining penalty (diverges from the configured
// default on clock-skew, partial-window expiry) so writeBearerError
// emits the true Retry-After rather than the full penalty.
throttled := newBearerError(bearerErrThrottled, "ip in penalty box") throttled := newBearerError(bearerErrThrottled, "ip in penalty box")
// Preserve the actual retry-after even if it diverged from the throttled.retryAfter = retryAfter
// configured default (clock-skew, partial-window expiry).
if retryAfter > 0 {
rw.Header().Set("Retry-After", fmt.Sprintf("%d", int(retryAfter.Seconds())))
}
t.writeBearerError(rw, req, throttled) t.writeBearerError(rw, req, throttled)
return return
} }
@@ -574,12 +631,51 @@ func (t *TraefikOidc) handleBearerRequest(rw http.ResponseWriter, req *http.Requ
t.writeBearerError(rw, req, newBearerError(bearerErrInvalidToken, "token exceeds max length")) t.writeBearerError(rw, req, newBearerError(bearerErrInvalidToken, "token exceeds max length"))
return return
} }
if strings.Count(token, ".") != 2 { // Determine whether this is genuinely a JWT. RFC 7662 token
// introspection covers opaque access tokens; when the operator
// requires live introspection (requireTokenIntrospection, whose
// documented purpose is exactly opaque-token support via
// client_secret_basic), introspect the opaque token on the bearer
// path instead of rejecting it — the session path already does
// (validateOpaqueToken). Previously the JWT-shape gate ran before
// the introspection branch, so every opaque token got a 401 even
// with introspection on (R159).
//
// A token is only treated as a JWT when it has exactly two dots AND
// its header actually parses as a JOSE header. Dot count alone is
// insufficient: an opaque token that coincidentally contains exactly
// two dots used to be routed onto the JWT path and rejected even
// under requireTokenIntrospection (header parse would fail) — it must
// instead be introspected like any other opaque token (R166).
isJWT := joseHeaderWellFormed(token)
if !isJWT {
if t.requireTokenIntrospection {
p, bErr := t.buildPrincipalFromOpaqueIntrospection(token)
if bErr != nil {
if bErr.kind != bearerErrIntrospectionUnavailable {
t.bearerFailureTracker.recordFailure(ip)
}
t.writeBearerError(rw, req, bErr)
return
}
t.bearerFailureTracker.recordSuccess(ip)
if t.logger != nil {
t.logger.Debugf("bearer auth success (introspected opaque): identifier_hash=%s path=%s",
hashIdentifierForLog(p.Identifier), req.URL.Path)
}
t.forwardAuthorized(rw, req, p)
return
}
t.bearerFailureTracker.recordFailure(ip) t.bearerFailureTracker.recordFailure(ip)
t.writeBearerError(rw, req, newBearerError(bearerErrInvalidToken, "token is not a 3-segment JWT")) t.writeBearerError(rw, req, newBearerError(bearerErrInvalidToken, "token is not a valid JWT"))
return return
} }
// Structurally a JWT: enforce the alg/kid policy EARLY (before the
// cached fast-path in buildPrincipalFromBearerToken can short-circuit
// on a prior positive verdict). Without this pin an alg=none or
// oversized-kid token that had been cached as verified would sail
// through without re-checking.
if bErr := parseBearerJOSEHeader(token); bErr != nil { if bErr := parseBearerJOSEHeader(token); bErr != nil {
t.bearerFailureTracker.recordFailure(ip) t.bearerFailureTracker.recordFailure(ip)
t.writeBearerError(rw, req, bErr) t.writeBearerError(rw, req, bErr)
@@ -588,7 +684,13 @@ func (t *TraefikOidc) handleBearerRequest(rw http.ResponseWriter, req *http.Requ
p, bErr := t.buildPrincipalFromBearerToken(token) p, bErr := t.buildPrincipalFromBearerToken(token)
if bErr != nil { if bErr != nil {
t.bearerFailureTracker.recordFailure(ip) // Count only authentication failures (401/403) toward the per-IP
// throttle. An introspection outage (503) is infrastructure, not a
// client failure; recording it would trip a 429 penalty that a
// single success can't clear even after the endpoint recovers.
if bErr.kind != bearerErrIntrospectionUnavailable {
t.bearerFailureTracker.recordFailure(ip)
}
t.writeBearerError(rw, req, bErr) t.writeBearerError(rw, req, bErr)
return return
} }
@@ -629,7 +731,9 @@ func (t *TraefikOidc) buildPrincipalFromBearerToken(token string) (*principal, *
return nil, newBearerError(bearerErrInvalidToken, "token_use=id rejected") return nil, newBearerError(bearerErrInvalidToken, "token_use=id rejected")
} }
if bErr := enforceMultiAudienceAzp(claims, t.clientID); bErr != nil { // Snapshot clientID under metadataMu: DCR rewrites it at runtime (R137).
clientID, _, _, _, _ := t.clientCredentials()
if bErr := enforceMultiAudienceAzp(claims, clientID); bErr != nil {
return nil, bErr return nil, bErr
} }
if bErr := enforceIatAge(claims, t.maxTokenAge); bErr != nil { if bErr := enforceIatAge(claims, t.maxTokenAge); bErr != nil {
@@ -642,6 +746,25 @@ func (t *TraefikOidc) buildPrincipalFromBearerToken(token string) (*principal, *
} }
} }
// Honor IdP-initiated (backchannel/front-channel) logout. The cookie
// path re-checks sessionInvalidationCache on every request; the bearer
// path previously did not, so a still-cryptographically-valid access
// token for a logged-out subject kept returning 200 (and, with
// requireTokenIntrospection, a cached positive verdict extended the
// stale window). Reject here, mirroring the cookie path: use the
// token's iat as its creation time so a token issued before the
// logout is invalidated, while a legitimately freshly-issued token
// (iat after logout) still passes (R146).
subjectForInvalidation, _ := claims["sub"].(string)
sidForInvalidation, _ := claims["sid"].(string)
createdAt := time.Now()
if iat, ok := claims["iat"].(float64); ok {
createdAt = time.Unix(int64(iat), 0)
}
if t.isSessionInvalidated(sidForInvalidation, subjectForInvalidation, createdAt) {
return nil, newBearerError(bearerErrInvalidToken, "session has been invalidated (logout)")
}
rawIdentifier, bErr := resolveBearerIdentifier(claims, t.bearerIdentifierClaim) rawIdentifier, bErr := resolveBearerIdentifier(claims, t.bearerIdentifierClaim)
if bErr != nil { if bErr != nil {
return nil, bErr return nil, bErr
@@ -652,7 +775,7 @@ func (t *TraefikOidc) buildPrincipalFromBearerToken(token string) (*principal, *
} }
subject, _ := claims["sub"].(string) subject, _ := claims["sub"].(string)
clientID, _ := claims["azp"].(string) clientID, _ = claims["azp"].(string)
if clientID == "" { if clientID == "" {
clientID, _ = claims["client_id"].(string) clientID, _ = claims["client_id"].(string)
} }
@@ -667,6 +790,62 @@ func (t *TraefikOidc) buildPrincipalFromBearerToken(token string) (*principal, *
}, nil }, nil
} }
// buildPrincipalFromOpaqueIntrospection authenticates a bearer access
// token that has no JWT shape by live RFC 7662 introspection, and builds
// a principal whose identifier is the introspected subject. It mirrors
// introspectOnBearerPath's classification (active, token_type, expiry,
// audience) and binds the compliant subject. Only used on the bearer
// path when requireTokenIntrospection is enabled (R159).
func (t *TraefikOidc) buildPrincipalFromOpaqueIntrospection(token string) (*principal, *bearerError) {
resp, err := t.introspectToken(token)
if err != nil {
var httpErr *HTTPError
if errors.As(err, &httpErr) && httpErr.StatusCode >= 400 && httpErr.StatusCode < 500 {
return nil, newBearerError(bearerErrTokenInactive, fmt.Sprintf("introspection reports token inactive (HTTP %d)", httpErr.StatusCode))
}
return nil, newBearerError(bearerErrIntrospectionUnavailable, "introspection failed: "+err.Error())
}
if !resp.Active {
return nil, newBearerError(bearerErrTokenInactive, "introspection reports token inactive")
}
// Mirror introspectOnBearerPath (R149): reject a definite non-access
// token_type (e.g. refresh_token); accept "access_token" and "Bearer".
if resp.TokenType != "" && resp.TokenType != "access_token" && resp.TokenType != "Bearer" {
return nil, newBearerError(bearerErrTokenInactive, "introspection token_type is not a bearer access token")
}
if resp.Exp > 0 && time.Now().After(time.Unix(resp.Exp, 0)) {
return nil, newBearerError(bearerErrTokenInactive, "introspection reports token expired")
}
// Audience gate when a distinct API audience is configured.
clientID, _, _, audience, _ := t.clientCredentials()
if audience != "" && audience != clientID {
if resp.Aud == nil || verifyAudience(resp.Aud, audience) != nil {
return nil, newBearerError(bearerErrTokenInactive, "introspection audience mismatch")
}
}
id := resp.Sub
if id == "" {
id = resp.Username
}
if id == "" {
return nil, newBearerError(bearerErrInvalidToken, "introspection response has no subject or username")
}
claims := map[string]interface{}{}
if resp.Sub != "" {
claims["sub"] = resp.Sub
}
if resp.Username != "" {
claims["username"] = resp.Username
}
return &principal{
Claims: claims,
Identifier: id,
Subject: resp.Sub,
AccessToken: token,
Source: sourceBearer,
}, nil
}
// introspectOnBearerPath calls the existing RFC 7662 introspector when the // introspectOnBearerPath calls the existing RFC 7662 introspector when the
// operator demands real-time revocation. Distinguishes "token revoked" (401) // operator demands real-time revocation. Distinguishes "token revoked" (401)
// from "endpoint unavailable" (503) so transient infra failures don't look // from "endpoint unavailable" (503) so transient infra failures don't look
@@ -674,10 +853,55 @@ func (t *TraefikOidc) buildPrincipalFromBearerToken(token string) (*principal, *
func (t *TraefikOidc) introspectOnBearerPath(token string) *bearerError { func (t *TraefikOidc) introspectOnBearerPath(token string) *bearerError {
resp, err := t.introspectToken(token) resp, err := t.introspectToken(token)
if err != nil { if err != nil {
// A definitive 4xx from the introspection endpoint (e.g. 401 for
// an unknown/revoked token) is a credential failure, not an infra
// outage — don't downgrade it to "unavailable" (R156).
var httpErr *HTTPError
if errors.As(err, &httpErr) && httpErr.StatusCode >= 400 && httpErr.StatusCode < 500 {
return newBearerError(bearerErrTokenInactive, fmt.Sprintf("introspection reports token inactive (HTTP %d)", httpErr.StatusCode))
}
return newBearerError(bearerErrIntrospectionUnavailable, "introspection failed: "+err.Error()) return newBearerError(bearerErrIntrospectionUnavailable, "introspection failed: "+err.Error())
} }
if !resp.Active { if !resp.Active {
return newBearerError(bearerErrTokenInactive, "introspection reports token inactive") return newBearerError(bearerErrTokenInactive, "introspection reports token inactive")
} }
// Mirror the session path (validateOpaqueToken): an opaque-or-any
// token whose RFC 7662 token_type classifies it as a refresh token
// must not be honored as a bearer access token (R149). Only reject on
// a definite non-access match; compliant providers may omit it. RFC
// 7662's token_type is the RFC 6749 token type, whose value for an
// access token is "Bearer" (RFC 6750) — accept both spellings
// providers use (R156).
if resp.TokenType != "" && resp.TokenType != "access_token" && resp.TokenType != "Bearer" {
return newBearerError(bearerErrTokenInactive, "introspection token_type is not a bearer access token")
}
// Mirror the session path (validateOpaqueToken): an active but
// already-expired result must not pass. The positive-only cache is
// capped by time-until-exp when Exp is present, but a provider
// returning active=1 past exp would otherwise let the token pass on
// the bearer path while the identical token is rejected on the
// session path.
if resp.Exp > 0 {
expTime := time.Unix(resp.Exp, 0)
if time.Now().After(expTime) {
return newBearerError(bearerErrTokenInactive, "introspection reports token expired")
}
}
// Mirror the session path (validateOpaqueToken): when a distinct API
// audience is configured (audience != clientID), the introspection
// response MUST carry a matching audience. Fail closed on a missing or
// mismatched aud, otherwise a token minted for a different audience
// would pass the bearer path while the identical token is rejected on
// the session path. aud may be a single string or an array (RFC 7662).
// (R143)
clientID, _, _, audience, _ := t.clientCredentials()
if audience != "" && audience != clientID {
if resp.Aud == nil {
return newBearerError(bearerErrTokenInactive, "introspection reports no audience")
}
if err := verifyAudience(resp.Aud, audience); err != nil {
return newBearerError(bearerErrTokenInactive, "introspection audience mismatch")
}
}
return nil return nil
} }
+18 -4
View File
@@ -95,9 +95,18 @@ func NewClientAssertionSigner(pemBytes []byte, alg, kid string) (*ClientAssertio
func validateAlgKeyMatch(alg string, key crypto.PrivateKey) error { func validateAlgKeyMatch(alg string, key crypto.PrivateKey) error {
switch alg[0] { switch alg[0] {
case 'R', 'P': // RS* or PS* case 'R', 'P': // RS* or PS*
if _, ok := key.(*rsa.PrivateKey); !ok { rsaKey, ok := key.(*rsa.PrivateKey)
if !ok {
return fmt.Errorf("alg %q requires an RSA key, got %T", alg, key) return fmt.Errorf("alg %q requires an RSA key, got %T", alg, key)
} }
// RFC 7518 §3.3: RS*/PS* MUST use a key of 2048 bits or larger.
// A smaller key signs locally but conformant IdPs (Entra, Okta,
// Keycloak) reject it, so every exchange/refresh/revocation fails
// invalid_client at runtime — better to fail at construction
// (R135).
if rsaKey.N.BitLen() < 2048 {
return fmt.Errorf("alg %q requires an RSA key of at least 2048 bits, got %d", alg, rsaKey.N.BitLen())
}
case 'E': // ES* case 'E': // ES*
ecKey, ok := key.(*ecdsa.PrivateKey) ecKey, ok := key.(*ecdsa.PrivateKey)
if !ok { if !ok {
@@ -142,6 +151,14 @@ func (s *ClientAssertionSigner) Sign(audience, clientID string) (string, error)
now := nowFn() now := nowFn()
// RFC 7523 §3: aud must be the token (or introspection/revocation)
// endpoint URL. An empty audience yields a client_assertion with
// "aud":"" that the IdP will reject, so fail fast with a clear
// error rather than minting a useless token (R150).
if audience == "" {
return "", fmt.Errorf("client assertion audience must not be empty")
}
// 16 random bytes as lowercase hex for jti uniqueness. // 16 random bytes as lowercase hex for jti uniqueness.
jtiBytes := make([]byte, 16) jtiBytes := make([]byte, 16)
if _, err := io.ReadFull(rander, jtiBytes); err != nil { if _, err := io.ReadFull(rander, jtiBytes); err != nil {
@@ -312,6 +329,3 @@ func buildClientAssertionSignerFromConfig(config *Config) (*ClientAssertionSigne
return NewClientAssertionSigner(pemBytes, alg, config.ClientAssertionKeyID) return NewClientAssertionSigner(pemBytes, alg, config.ClientAssertionKeyID)
} }
+60 -9
View File
@@ -107,10 +107,13 @@ func (t *TraefikOidc) exchangeTokens(ctx context.Context, grantType string, code
data := url.Values{ data := url.Values{
"grant_type": {grantType}, "grant_type": {grantType},
} }
// Snapshot client credentials under metadataMu: DCR rewrites clientID/
// clientSecret/audience at runtime (R137).
clientID, clientSecret, clientAuthMethod, _, clientAssertion := t.clientCredentials()
// client_id is sent in the body for every method except client_secret_basic, // client_id is sent in the body for every method except client_secret_basic,
// where it is carried in the Authorization header per RFC 6749 §2.3.1. // where it is carried in the Authorization header per RFC 6749 §2.3.1.
if t.clientAuthMethod != "client_secret_basic" || t.clientAssertion != nil { if clientAuthMethod != "client_secret_basic" || clientAssertion != nil {
data.Set("client_id", t.clientID) data.Set("client_id", clientID)
} }
if grantType == "authorization_code" { if grantType == "authorization_code" {
@@ -148,17 +151,17 @@ func (t *TraefikOidc) exchangeTokens(ctx context.Context, grantType string, code
t.metadataMu.RUnlock() t.metadataMu.RUnlock()
useBasicAuth := false useBasicAuth := false
if t.clientAssertion != nil { if clientAssertion != nil {
assertion, err := t.clientAssertion.Sign(tokenURL, t.clientID) assertion, err := clientAssertion.Sign(tokenURL, clientID)
if err != nil { if err != nil {
return nil, fmt.Errorf("failed to sign client assertion: %w", err) return nil, fmt.Errorf("failed to sign client assertion: %w", err)
} }
data.Set("client_assertion_type", "urn:ietf:params:oauth:client-assertion-type:jwt-bearer") data.Set("client_assertion_type", "urn:ietf:params:oauth:client-assertion-type:jwt-bearer")
data.Set("client_assertion", assertion) data.Set("client_assertion", assertion)
} else if t.clientAuthMethod == "client_secret_basic" { } else if clientAuthMethod == "client_secret_basic" {
useBasicAuth = true useBasicAuth = true
} else { } else {
data.Set("client_secret", t.clientSecret) data.Set("client_secret", clientSecret)
} }
req, err := http.NewRequestWithContext(ctx, "POST", tokenURL, strings.NewReader(data.Encode())) //nolint:gosec // tokenURL is the operator-trusted discovery endpoint (metadataMu) req, err := http.NewRequestWithContext(ctx, "POST", tokenURL, strings.NewReader(data.Encode())) //nolint:gosec // tokenURL is the operator-trusted discovery endpoint (metadataMu)
@@ -167,7 +170,7 @@ func (t *TraefikOidc) exchangeTokens(ctx context.Context, grantType string, code
} }
req.Header.Set("Content-Type", "application/x-www-form-urlencoded") req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
if useBasicAuth { if useBasicAuth {
setOAuthBasicAuth(req, t.clientID, t.clientSecret) setOAuthBasicAuth(req, clientID, clientSecret)
} }
resp, err := client.Do(req) //nolint:gosec // req above targets the trusted tokenURL resp, err := client.Do(req) //nolint:gosec // req above targets the trusted tokenURL
@@ -182,7 +185,14 @@ func (t *TraefikOidc) exchangeTokens(ctx context.Context, grantType string, code
if resp.StatusCode != http.StatusOK { if resp.StatusCode != http.StatusOK {
limitReader := io.LimitReader(resp.Body, 1024*10) limitReader := io.LimitReader(resp.Body, 1024*10)
bodyBytes, _ := io.ReadAll(limitReader) // Safe to ignore: reading error body for diagnostics bodyBytes, _ := io.ReadAll(limitReader) // Safe to ignore: reading error body for diagnostics
return nil, fmt.Errorf("token endpoint returned status %d: %s", resp.StatusCode, string(bodyBytes)) // Return a *HTTPError so the retry executor's isRetryableError can
// classify transient 5xx/429 responses and actually retry (R118).
// A plain error here made the 5xx/429 retry branch dead in
// production — every token-endpoint status was treated as permanent.
return nil, &HTTPError{
StatusCode: resp.StatusCode,
Message: fmt.Sprintf("token endpoint returned status %d: %s", resp.StatusCode, string(bodyBytes)),
}
} }
var tokenResponse TokenResponse var tokenResponse TokenResponse
@@ -192,6 +202,16 @@ func (t *TraefikOidc) exchangeTokens(ctx context.Context, grantType string, code
return nil, fmt.Errorf("failed to decode token response: %w", err) return nil, fmt.Errorf("failed to decode token response: %w", err)
} }
// RFC 6749 §5.1: a token response MUST include an access_token. A 200
// that omitted it is a protocol violation, not a success — treat it as an
// error here so no caller authenticates with a blank token (R158).
// Previously this seam returned success with an empty access_token and
// each caller had to re-check "AccessToken == \"\"", which several
// paths missed.
if tokenResponse.AccessToken == "" {
return nil, fmt.Errorf("token endpoint returned 200 without an access_token (RFC 6749 §5.1)")
}
return &tokenResponse, nil return &tokenResponse, nil
} }
@@ -204,6 +224,17 @@ func (t *TraefikOidc) exchangeTokens(ctx context.Context, grantType string, code
// Returns: // Returns:
// - *TokenResponse: New token set from the authorization server // - *TokenResponse: New token set from the authorization server
// - An error if the refresh operation fails // - An error if the refresh operation fails
//
// redactToken returns a short, stable hash of a token credential suitable for
// debug logging without leaking the credential itself (R138).
func redactToken(token string) string {
if token == "" {
return "(empty)"
}
sum := sha256.Sum256([]byte(token))
return fmt.Sprintf("%x", sum[:4])
}
func (t *TraefikOidc) getNewTokenWithRefreshToken(refreshToken string) (*TokenResponse, error) { func (t *TraefikOidc) getNewTokenWithRefreshToken(refreshToken string) (*TokenResponse, error) {
ctx := context.Background() ctx := context.Background()
@@ -218,7 +249,8 @@ func (t *TraefikOidc) getNewTokenWithRefreshToken(refreshToken string) (*TokenRe
return nil, fmt.Errorf("failed to refresh token: %w", err) return nil, fmt.Errorf("failed to refresh token: %w", err)
} }
t.logger.Debugf("Token response: %+v", tokenResponse) t.logger.Debugf("Refresh token response: token_type=%s expires_in=%d access_token=%s",
tokenResponse.TokenType, tokenResponse.ExpiresIn, redactToken(tokenResponse.AccessToken))
return tokenResponse, nil return tokenResponse, nil
} }
@@ -399,6 +431,20 @@ func (t *TraefikOidc) handleLogout(rw http.ResponseWriter, req *http.Request) {
} }
} }
// Best-effort provider-side revocation (single logout): the local
// blacklist only stops reuse in-process, but a token captured before
// logout (e.g. a bearer-mode access token or the refresh token) stays
// valid at the IdP and a captured refresh token could keep minting new
// tokens. Revoke both access and refresh tokens at the provider when a
// revocation endpoint is available; ignore errors (the endpoint may be
// absent or the provider already cleaned up on the end-session call).
if accessToken != "" {
_ = t.RevokeTokenWithProvider(accessToken, "access_token")
}
if refreshToken != "" {
_ = t.RevokeTokenWithProvider(refreshToken, "refresh_token")
}
postLogoutRedirectURI := t.postLogoutRedirectURI postLogoutRedirectURI := t.postLogoutRedirectURI
// localRedirect is used when there is no provider end-session endpoint and // localRedirect is used when there is no provider end-session endpoint and
// the plugin redirects the browser itself. It must never be an absolute URL // the plugin redirects the browser itself. It must never be an absolute URL
@@ -437,10 +483,15 @@ func (t *TraefikOidc) handleLogout(rw http.ResponseWriter, req *http.Request) {
http.Error(rw, "Logout error", http.StatusInternalServerError) http.Error(rw, "Logout error", http.StatusInternalServerError)
return return
} }
// Logout redirects must never be cached: a heuristic-caching proxy
// replaying the 302 would re-trigger logout for an already
// logged-out session (R147 cache-directive contract).
rw.Header().Set("Cache-Control", "no-store")
http.Redirect(rw, req, logoutURL, http.StatusFound) http.Redirect(rw, req, logoutURL, http.StatusFound)
return return
} }
rw.Header().Set("Cache-Control", "no-store")
http.Redirect(rw, req, localRedirect, http.StatusFound) http.Redirect(rw, req, localRedirect, http.StatusFound)
} }
+60 -38
View File
@@ -26,8 +26,6 @@ var (
replayCache CacheInterface replayCache CacheInterface
// shardedReplayCache is the new high-performance sharded cache for replay detection // shardedReplayCache is the new high-performance sharded cache for replay detection
shardedReplayCache *ShardedCache shardedReplayCache *ShardedCache
// replayCacheOnce ensures the replay cache is initialized only once
replayCacheOnce sync.Once
// replayCacheCleanupWG waits for cleanup goroutine to finish // replayCacheCleanupWG waits for cleanup goroutine to finish
replayCacheCleanupWG sync.WaitGroup replayCacheCleanupWG sync.WaitGroup
// replayCacheCancel cancels the cleanup context // replayCacheCancel cancels the cleanup context
@@ -39,21 +37,23 @@ var (
// initReplayCache initializes the JWT replay protection cache with bounded size. // initReplayCache initializes the JWT replay protection cache with bounded size.
// Uses a sharded cache design with 64 shards for reduced lock contention under high load. // Uses a sharded cache design with 64 shards for reduced lock contention under high load.
// The cache is bounded to 10,000 entries to prevent unbounded memory growth. // The cache is bounded to 10,000 entries to prevent unbounded memory growth.
// This function uses sync.Once to ensure thread-safe single initialization. // Lazy creation is guarded entirely by replayCacheMu (a plain sync.Once would
// not re-arm after cleanup resets it, and the previous Once re-assignment raced
// with init under -race). cleanupReplayCache nil's the fields under the same
// lock, so this either creates anew (after a cleanup) or is a cheap no-op.
func initReplayCache() { func initReplayCache() {
replayCacheOnce.Do(func() { replayCacheMu.Lock()
// Hold mutex during initialization to synchronize with cleanup goroutine defer replayCacheMu.Unlock()
replayCacheMu.Lock()
defer replayCacheMu.Unlock()
// Create sharded cache with 64 shards for reduced contention if shardedReplayCache == nil {
// Create sharded cache with 64 shards for reduced lock contention
// Under 500 req/sec, this reduces lock contention by ~64x compared to single mutex // Under 500 req/sec, this reduces lock contention by ~64x compared to single mutex
shardedReplayCache = NewShardedCache(64, 10000) shardedReplayCache = NewShardedCache(64, 10000)
// Also initialize legacy cache for backward compatibility // Also initialize legacy cache for backward compatibility
replayCache = NewCache() replayCache = NewCache()
replayCache.SetMaxSize(10000) replayCache.SetMaxSize(10000)
}) }
} }
// cleanupReplayCache performs graceful shutdown of the replay cache system. // cleanupReplayCache performs graceful shutdown of the replay cache system.
@@ -61,17 +61,12 @@ func initReplayCache() {
// and properly closes the cache to ensure proper cleanup during shutdown. // and properly closes the cache to ensure proper cleanup during shutdown.
func cleanupReplayCache() { func cleanupReplayCache() {
replayCacheCleanupMu.Lock() replayCacheCleanupMu.Lock()
shouldWait := replayCacheCancel != nil
if replayCacheCancel != nil { if replayCacheCancel != nil {
replayCacheCancel() replayCacheCancel()
replayCacheCancel = nil replayCacheCancel = nil
} }
replayCacheCleanupMu.Unlock() replayCacheCleanupMu.Unlock()
replayCacheCleanupWG.Wait()
// Only wait if there was a cleanup routine running
if shouldWait {
replayCacheCleanupWG.Wait()
}
replayCacheMu.Lock() replayCacheMu.Lock()
defer replayCacheMu.Unlock() defer replayCacheMu.Unlock()
@@ -87,8 +82,16 @@ func cleanupReplayCache() {
replayCache.Close() replayCache.Close()
replayCache = nil replayCache = nil
} }
}
replayCacheOnce = sync.Once{} // replayCacheKey namespaces a JTI by its issuer before it is stored in the
// process-global replay cache. JTI uniqueness is guaranteed only within a
// single issuer's namespace; many Traefik routers share this one global
// cache across different OIDC providers, so a bare jti would let two
// providers' legitimately-colliding JTIs map to the same entry (false
// replay) and silently overwrite each other (R142).
func replayCacheKey(issuer, jti string) string {
return issuer + "\x00" + jti
} }
// getReplayCacheStats returns statistics about the replay cache state. // getReplayCacheStats returns statistics about the replay cache state.
@@ -96,9 +99,14 @@ func cleanupReplayCache() {
// - size: Current number of entries in the cache // - size: Current number of entries in the cache
// - maxSize: Maximum allowed entries (10,000) // - maxSize: Maximum allowed entries (10,000)
func getReplayCacheStats() (size int, maxSize int) { func getReplayCacheStats() (size int, maxSize int) {
// Use sharded cache if available (no mutex needed due to internal sharding) // Use sharded cache if available (no mutex needed due to internal sharding).
if shardedReplayCache != nil { // Guard the singleton read because cleanupReplayCache nils it under the
return shardedReplayCache.Size(), 10000 // write lock.
replayCacheMu.RLock()
sc := shardedReplayCache
replayCacheMu.RUnlock()
if sc != nil {
return sc.Size(), 10000
} }
// Fall back to legacy cache // Fall back to legacy cache
@@ -281,13 +289,12 @@ func parseJWT(tokenString string) (*JWT, error) {
return nil, fmt.Errorf("invalid JWT format: failed to decode signature: %v", err) return nil, fmt.Errorf("invalid JWT format: failed to decode signature: %v", err)
} }
// Reuse the signature buffer if it's large enough, otherwise allocate // Copy the signature out of the pooled buffer. Borrowing it with
if cap(jwtBuf.Signature) >= n { // [:(n):(n)] still shares the backing array, which the pool recycles
jwt.Signature = jwtBuf.Signature[:n:n] // Use slice trick to prevent aliasing // and truncates on the next GetJWTBuffer — mutating this caller's
} else { // slice after the fact (R156).
jwt.Signature = make([]byte, n) jwt.Signature = make([]byte, n)
copy(jwt.Signature, jwtBuf.Signature[:n]) copy(jwt.Signature, jwtBuf.Signature[:n])
}
return jwt, nil return jwt, nil
} }
@@ -342,12 +349,13 @@ func (j *JWT) Verify(issuerURL, expectedAudience string, skipReplayCheck ...bool
return err return err
} }
iat, ok := claims["iat"].(float64) // iat is OPTIONAL per RFC 7519 §4.1.6; only validate when present.
if !ok { // Hard-requiring it here also rejected otherwise-valid access and
return fmt.Errorf("missing or invalid 'iat' claim") // logout tokens whose provider omits iat.
} if iat, ok := claims["iat"].(float64); ok {
if err := verifyIssuedAt(iat); err != nil { if err := verifyIssuedAt(iat); err != nil {
return err return err
}
} }
if nbf, ok := claims["nbf"].(float64); ok { if nbf, ok := claims["nbf"].(float64); ok {
@@ -371,6 +379,12 @@ func (j *JWT) Verify(issuerURL, expectedAudience string, skipReplayCheck ...bool
expTime = time.Now().Add(10 * time.Minute) expTime = time.Now().Add(10 * time.Minute)
} }
duration := time.Until(expTime) duration := time.Until(expTime)
// A token is accepted until exp + ClockSkewToleranceFuture (see
// verifyExpiration), so the replay entry must live that long too —
// otherwise in the (exp, exp+skew] window the entry has expired
// and been swept and the same token is re-accepted as fresh
// (R179).
duration += ClockSkewToleranceFuture
if duration <= 0 { if duration <= 0 {
// Expired token: no replay window worth recording, so nothing to do. // Expired token: no replay window worth recording, so nothing to do.
goto replayDone goto replayDone
@@ -386,18 +400,26 @@ func (j *JWT) Verify(issuerURL, expectedAudience string, skipReplayCheck ...bool
replayCacheMu.RLock() replayCacheMu.RLock()
sc := shardedReplayCache sc := shardedReplayCache
if sc != nil { if sc != nil {
if !sc.SetIfAbsent(jtiValue, true, duration) { if !sc.SetIfAbsent(replayCacheKey(issuerURL, jtiValue), true, duration) {
replayCacheMu.RUnlock() replayCacheMu.RUnlock()
return fmt.Errorf("token replay detected (jti: %s)", jtiValue) return fmt.Errorf("token replay detected (jti: %s)", jtiValue)
} }
replayCacheMu.RUnlock() replayCacheMu.RUnlock()
} else { } else {
// Fall back to legacy cache (should rarely happen). Hold the // Fall back to legacy cache (should rarely happen). Release the
// mutex across the get+set so the record is atomic there too. // read lock before taking the write lock: RWMutex is not
// reentrant, so Lock() here while still holding RLock()
// self-deadlocks (R156). Guard the cache for nil — it is
// cleared alongside shardedReplayCache by cleanupReplayCache.
replayCacheMu.RUnlock()
replayCacheMu.Lock() replayCacheMu.Lock()
_, exists := replayCache.Get(jtiValue) rk := replayCacheKey(issuerURL, jtiValue)
if !exists && replayCache != nil { exists := false
replayCache.Set(jtiValue, true, duration) if replayCache != nil {
_, exists = replayCache.Get(rk)
if !exists {
replayCache.Set(rk, true, duration)
}
} }
replayCacheMu.Unlock() replayCacheMu.Unlock()
if exists { if exists {
+71 -6
View File
@@ -100,7 +100,10 @@ func (t *TraefikOidc) handleBackchannelLogout(rw http.ResponseWriter, req *http.
t.logger.Infof("Backchannel logout: successfully invalidated session (sid=%s, sub=%s)", t.logger.Infof("Backchannel logout: successfully invalidated session (sid=%s, sub=%s)",
claims.SessionID, claims.Subject) claims.SessionID, claims.Subject)
// Return 200 OK with empty body per spec // Return 200 OK with empty body per spec; mark it non-cacheable so a
// proxy cannot replay a stale logout acknowledgement (R147 contract).
rw.Header().Set("Cache-Control", "no-store")
rw.Header().Set("Content-Type", "text/plain; charset=utf-8")
rw.WriteHeader(http.StatusOK) rw.WriteHeader(http.StatusOK)
} }
@@ -291,18 +294,27 @@ func (t *TraefikOidc) validateLogoutToken(tokenString string) (*LogoutTokenClaim
// validateLogoutTokenAudience checks if the logout token audience contains our client_id // validateLogoutTokenAudience checks if the logout token audience contains our client_id
func (t *TraefikOidc) validateLogoutTokenAudience(aud interface{}) bool { func (t *TraefikOidc) validateLogoutTokenAudience(aud interface{}) bool {
// t.clientID is mutated at runtime by the DCR path
// (performDynamicClientRegistration, under metadataMu). Read it under
// the same lock as every other credential field (issuer, jwksURL) so a
// backchannel-logout token validated concurrent with client re-issuance
// does not race the write.
t.metadataMu.RLock()
clientID := t.clientID
t.metadataMu.RUnlock()
switch v := aud.(type) { switch v := aud.(type) {
case string: case string:
return v == t.clientID return v == clientID
case []interface{}: case []interface{}:
for _, a := range v { for _, a := range v {
if s, ok := a.(string); ok && s == t.clientID { if s, ok := a.(string); ok && s == clientID {
return true return true
} }
} }
case []string: case []string:
for _, a := range v { for _, a := range v {
if a == t.clientID { if a == clientID {
return true return true
} }
} }
@@ -342,6 +354,17 @@ func (t *TraefikOidc) verifyLogoutTokenSignature(jwt *JWT, tokenString string) e
} }
if err := verifySignatureWithKey(tokenString, pubKey, alg); err != nil { if err := verifySignatureWithKey(tokenString, pubKey, alg); err != nil {
// The signature failed against the (possibly stale) cached key. The
// provider may have rotated its signing keys in place, reusing the
// same kid (R109): refresh the JWKS once and retry before failing.
// Without this a logout token signed with an in-place-rotated key is
// rejected for up to the JWKS cache TTL, leaving the user's session
// alive after the IdP logged them out (R130).
if freshKey, ferr := t.jwkCache.getPublicKeyFresh(context.Background(), jwksURL, kid, t.httpClient); ferr == nil {
if verr := verifySignatureWithKey(tokenString, freshKey, alg); verr == nil {
return nil
}
}
return fmt.Errorf("signature verification failed: %w", err) return fmt.Errorf("signature verification failed: %w", err)
} }
@@ -402,6 +425,29 @@ func (t *TraefikOidc) invalidateSession(sid, sub string) error {
// //
// Returns: // Returns:
// - true if the session has been invalidated, false otherwise // - true if the session has been invalidated, false otherwise
//
// sessionInvalidationTime extracts the invalidation timestamp from a cache
// value regardless of backend storage semantics. The in-memory cache returns
// the raw int64 we stored; serializing backends (e.g. Redis) JSON round-trip
// numbers, so Get yields a float64. Accepting both keeps backchannel/front-
// channel logout functional in multi-instance (distributed) deployments;
// previously the strict int64 assertion silently returned false there.
func sessionInvalidationTime(val interface{}) (int64, bool) {
switch v := val.(type) {
case int64:
return v, true
case float64:
return int64(v), true
case json.Number:
n, err := v.Int64()
if err != nil {
return 0, false
}
return n, true
}
return 0, false
}
func (t *TraefikOidc) isSessionInvalidated(sid, sub string, sessionCreatedAt time.Time) bool { func (t *TraefikOidc) isSessionInvalidated(sid, sub string, sessionCreatedAt time.Time) bool {
if t.sessionInvalidationCache == nil { if t.sessionInvalidationCache == nil {
return false return false
@@ -414,7 +460,7 @@ func (t *TraefikOidc) isSessionInvalidated(sid, sub string, sessionCreatedAt tim
if sid != "" { if sid != "" {
key := t.buildSessionInvalidationKey("sid", sid) key := t.buildSessionInvalidationKey("sid", sid)
if val, found := t.sessionInvalidationCache.Get(key); found { if val, found := t.sessionInvalidationCache.Get(key); found {
if invalidatedAt, ok := val.(int64); ok { if invalidatedAt, ok := sessionInvalidationTime(val); ok {
// Session was invalidated at or after it was created // Session was invalidated at or after it was created
invalidationTime := time.Unix(invalidatedAt, 0) invalidationTime := time.Unix(invalidatedAt, 0)
if !invalidationTime.Before(sessionCreatedAtSec) { if !invalidationTime.Before(sessionCreatedAtSec) {
@@ -429,7 +475,7 @@ func (t *TraefikOidc) isSessionInvalidated(sid, sub string, sessionCreatedAt tim
if sub != "" { if sub != "" {
key := t.buildSessionInvalidationKey("sub", sub) key := t.buildSessionInvalidationKey("sub", sub)
if val, found := t.sessionInvalidationCache.Get(key); found { if val, found := t.sessionInvalidationCache.Get(key); found {
if invalidatedAt, ok := val.(int64); ok { if invalidatedAt, ok := sessionInvalidationTime(val); ok {
// Sessions for this subject created at or before invalidation are invalid // Sessions for this subject created at or before invalidation are invalid
invalidationTime := time.Unix(invalidatedAt, 0) invalidationTime := time.Unix(invalidatedAt, 0)
if !invalidationTime.Before(sessionCreatedAtSec) { if !invalidationTime.Before(sessionCreatedAtSec) {
@@ -448,6 +494,25 @@ func (t *TraefikOidc) buildSessionInvalidationKey(keyType, value string) string
return fmt.Sprintf("session_invalidation:%s:%s", keyType, value) return fmt.Sprintf("session_invalidation:%s:%s", keyType, value)
} }
// sessionCreatedAtForInvalidation returns the session creation time to use
// for the backchannel/front-channel logout invalidation check. It prefers
// the ID token's iat claim; when iat is absent, it falls back to the
// session's own created_at (recorded at login) rather than time.Now().
// Using time.Now() here meant a session whose ID token lacks iat was
// always treated as created "now" (after any logout), so invalidation
// never matched and IdP-initiated logout silently had no effect (R98).
func (t *TraefikOidc) sessionCreatedAtForInvalidation(idToken string, session *SessionData) time.Time {
if jwt, err := parseJWT(idToken); err == nil {
if iat, ok := jwt.Claims["iat"].(float64); ok {
return time.Unix(int64(iat), 0)
}
}
if ca := session.getCreatedAtUnsafe(); ca > 0 {
return time.Unix(ca, 0)
}
return time.Time{} // zero: treated as invalidated -> safe re-auth
}
// extractSessionInfo extracts sid and sub from an ID token for session tracking // extractSessionInfo extracts sid and sub from an ID token for session tracking
func (t *TraefikOidc) extractSessionInfo(idToken string) (sid, sub string, createdAt time.Time) { func (t *TraefikOidc) extractSessionInfo(idToken string) (sid, sub string, createdAt time.Time) {
if idToken == "" { if idToken == "" {
+7 -5
View File
@@ -147,11 +147,13 @@ func (mc *MetadataCache) GetProviderMetadata(ctx context.Context, providerURL st
} }
// Require the core endpoints. An incomplete discovery document (missing // Require the core endpoints. An incomplete discovery document (missing
// authorization_endpoint / token_endpoint) would otherwise be accepted, // authorization_endpoint / token_endpoint / jwks_uri) would otherwise be
// cached, and adopted as the working auth/token URL, failing at runtime // accepted, cached, and adopted as the working auth/token/JWKS URL, failing
// (wrong authorize redirect, token-exchange failure) instead of at startup. // at runtime (wrong authorize redirect, token-exchange failure, or signature
if metadata.AuthURL == "" || metadata.TokenURL == "" { // verification failure — jwksURL has no non-discovery source) instead of at
return nil, fmt.Errorf("discovery document missing required endpoints (authorization_endpoint or token_endpoint)") // startup.
if metadata.AuthURL == "" || metadata.TokenURL == "" || metadata.JWKSURL == "" {
return nil, fmt.Errorf("discovery document missing required endpoints (authorization_endpoint, token_endpoint or jwks_uri)")
} }
// Pin the advertised issuer to the configured provider host. The issuer is // Pin the advertised issuer to the configured provider host. The issuer is
+320 -60
View File
@@ -4,9 +4,12 @@
package traefikoidc package traefikoidc
import ( import (
"bufio"
"bytes" "bytes"
"fmt" "fmt"
"net"
"net/http" "net/http"
"regexp"
"runtime/debug" "runtime/debug"
"strings" "strings"
"sync/atomic" "sync/atomic"
@@ -15,6 +18,45 @@ import (
"github.com/lukaszraczylo/traefikoidc/internal/utils" "github.com/lukaszraczylo/traefikoidc/internal/utils"
) )
// trackingWriter wraps an http.ResponseWriter so the deferred panic handler
// can tell whether a response was already committed. WriteHeader is
// idempotent so calling it again after commit is a no-op, but a later
// Write appends to the committed body - so writing "Internal Server
// Error" after a handler has already produced a valid 200 (or a 302
// redirect) corrupts the response. Flush/Hijack are forwarded so
// downstream SSE and WebSocket handlers keep working through the wrapper.
type trackingWriter struct {
http.ResponseWriter
wroteHeader bool
}
func (w *trackingWriter) WriteHeader(code int) {
w.wroteHeader = true
w.ResponseWriter.WriteHeader(code)
}
func (w *trackingWriter) Write(b []byte) (int, error) {
w.wroteHeader = true
return w.ResponseWriter.Write(b)
}
func (w *trackingWriter) Flush() {
w.wroteHeader = true
if f, ok := w.ResponseWriter.(http.Flusher); ok {
f.Flush()
}
}
func (w *trackingWriter) Hijack() (net.Conn, *bufio.ReadWriter, error) {
w.wroteHeader = true
if h, ok := w.ResponseWriter.(http.Hijacker); ok {
return h.Hijack()
}
return nil, nil, http.ErrNotSupported
}
func (w *trackingWriter) Unwrap() http.ResponseWriter { return w.ResponseWriter }
// bypassReason describes why a request is being forwarded without OIDC auth. // bypassReason describes why a request is being forwarded without OIDC auth.
// It is only used for logging and to decide whether extra side-effects // It is only used for logging and to decide whether extra side-effects
// (propagating the user header from an existing session) should run. // (propagating the user header from an existing session) should run.
@@ -22,6 +64,7 @@ const (
bypassReasonExcluded = "excluded-url" bypassReasonExcluded = "excluded-url"
bypassReasonSSE = "sse" bypassReasonSSE = "sse"
bypassReasonWebSocket = "websocket" bypassReasonWebSocket = "websocket"
bypassReasonOptions = "options"
) )
// isWebSocketUpgrade reports whether req is a WebSocket upgrade handshake // isWebSocketUpgrade reports whether req is a WebSocket upgrade handshake
@@ -54,6 +97,14 @@ func (t *TraefikOidc) shouldBypassAuth(req *http.Request) (bool, string) {
if t.determineExcludedURL(req.URL.Path) { if t.determineExcludedURL(req.URL.Path) {
return true, bypassReasonExcluded return true, bypassReasonExcluded
} }
// CORS preflights carry no session cookie and must reach the backend so
// it can answer with Access-Control-Allow-*. Otherwise the 401 here
// (with no ACAO headers, since the security applier only runs on the
// authenticated path) makes the browser block the cross-origin
// request entirely and the real request never proceeds (R124).
if req.Method == http.MethodOptions {
return true, bypassReasonOptions
}
if strings.Contains(req.Header.Get("Accept"), "text/event-stream") { if strings.Contains(req.Header.Get("Accept"), "text/event-stream") {
return true, bypassReasonSSE return true, bypassReasonSSE
} }
@@ -80,6 +131,29 @@ func (t *TraefikOidc) shouldBypassAuth(req *http.Request) (bool, string) {
// The second return value (0 when forwarding) is the HTTP status to send // The second return value (0 when forwarding) is the HTTP status to send
// when the request is rejected, letting the caller distinguish a 401 // when the request is rejected, letting the caller distinguish a 401
// (unauthenticated) from a 403 (authenticated but not permitted). // (unauthenticated) from a 403 (authenticated but not permitted).
// identityHeaders are the headers this middleware owns upstream. It
// unconditionally strips any inbound values with these names before
// forwarding — both on authenticated and bypassed requests — so forged
// values (e.g. X-Forwarded-User: admin on a public path) can never
// reach the backend and be trusted for downstream authorization.
var identityHeaders = []string{
"X-Forwarded-User",
"X-User-Groups",
"X-User-Roles",
"X-Auth-Request-User",
"X-Auth-Request-Redirect",
"X-Auth-Request-Token",
}
// stripIdentityHeaders removes any inbound identity headers from req. The
// middleware re-injects its own authoritative values afterwards where
// appropriate. See identityHeaders.
func stripIdentityHeaders(req *http.Request) {
for _, h := range identityHeaders {
req.Header.Del(h)
}
}
func (t *TraefikOidc) applyBypassUserHeaders(req *http.Request, reason string) (bool, int) { func (t *TraefikOidc) applyBypassUserHeaders(req *http.Request, reason string) (bool, int) {
if t.sessionManager == nil { if t.sessionManager == nil {
return false, http.StatusUnauthorized return false, http.StatusUnauthorized
@@ -115,14 +189,25 @@ func (t *TraefikOidc) applyBypassUserHeaders(req *http.Request, reason string) (
// Enforce the allowedRolesAndGroups gate, mirroring forwardAuthorized: // Enforce the allowedRolesAndGroups gate, mirroring forwardAuthorized:
// an authenticated user without any permitted role/group must not reach // an authenticated user without any permitted role/group must not reach
// the SSE/WebSocket backend just by using the streaming bypass. Claims // the SSE/WebSocket backend just by using the streaming bypass. Claims
// come from the session's ID token, same source the normal path uses. // come from the session's ID token by preference, falling back to the
// access token when there is no ID token — the same source selection the
// main authorization path (processAuthorizedRequest) uses. Without the
// fallback, opaque-ID-token providers (groups only in the access token)
// were granted on normal requests but 403'd on the streaming bypass
// (R118).
if len(t.allowedRolesAndGroups) > 0 { if len(t.allowedRolesAndGroups) > 0 {
idClaims, claimsErr := session.GetIDTokenClaims(t.extractClaimsFunc) var groupClaims map[string]interface{}
if claimsErr != nil || idClaims == nil { var claimsErr error
if idToken := session.GetIDToken(); idToken != "" {
groupClaims, claimsErr = session.GetIDTokenClaims(t.extractClaimsFunc)
} else if accessToken := session.GetAccessToken(); accessToken != "" {
groupClaims, claimsErr = t.extractClaimsFunc(accessToken)
}
if claimsErr != nil || groupClaims == nil {
t.logger.Debugf("%s bypass: cannot read claims for role check (err=%v): %s", reason, claimsErr, userIdentifier) t.logger.Debugf("%s bypass: cannot read claims for role check (err=%v): %s", reason, claimsErr, userIdentifier)
return false, http.StatusForbidden return false, http.StatusForbidden
} }
groups, roles, extErr := t.extractGroupsAndRolesFromClaims(idClaims) groups, roles, extErr := t.extractGroupsAndRolesFromClaims(groupClaims)
if extErr != nil { if extErr != nil {
t.logger.Debugf("%s bypass: role extraction failed for %s: %v", reason, userIdentifier, extErr) t.logger.Debugf("%s bypass: role extraction failed for %s: %v", reason, userIdentifier, extErr)
return false, http.StatusForbidden return false, http.StatusForbidden
@@ -140,6 +225,17 @@ func (t *TraefikOidc) applyBypassUserHeaders(req *http.Request, reason string) (
} }
} }
// This middleware fully owns the identity headers it injects upstream, even
// on the SSE/WebSocket bypass path. Unconditionally remove inbound values
// for the same names first: a client can forge X-User-Groups /
// X-Auth-Request-User etc. on a bypassed request, and since this path
// only sets X-Forwarded-User / X-Auth-Request-User (never groups/roles),
// forged group/role headers would otherwise survive to the backend,
// which commonly trusts them for downstream authorization. Must precede
// the sanitize gate so even the drop-on-unsafe early return has already
// cleared forged values (R100).
stripIdentityHeaders(req)
// Sanitize the claim-derived identifier before it is injected as a // Sanitize the claim-derived identifier before it is injected as a
// header, matching forwardAuthorized (which uses safeIdentifier). The // header, matching forwardAuthorized (which uses safeIdentifier). The
// value originates from an IdP-controlled claim and can contain CRLF // value originates from an IdP-controlled claim and can contain CRLF
@@ -172,20 +268,82 @@ func (t *TraefikOidc) applyBypassUserHeaders(req *http.Request, reason string) (
// Parameters: // Parameters:
// - rw: The HTTP response writer. // - rw: The HTTP response writer.
// - req: The incoming HTTP request. // - req: The incoming HTTP request.
//
// shouldSkipGraceRefresh reports whether a proactive refresh can be skipped for
// this request. The token forwarded downstream (forwardAuthorized) is the
// ACCESS token, and needsRefresh (validateStandardTokensRS -> validateTokenExpiryRS)
// is derived from the ACCESS token's expiry whenever it is a verifiable JWT.
// Basing the skip decision on the (possibly longer-lived) ID token's exp would
// forward a stale access token whenever the ID token is still fresh. Base it on
// the access token's expiry, falling back to the ID token only when the access
// token is opaque and unparseable (worst case: refresh when a skip might have
// been possible — the safe direction).
func (t *TraefikOidc) shouldSkipGraceRefresh(session *SessionData) bool {
return skipRefreshForTokens(session.GetAccessToken(), session.GetIDToken(), t.refreshGracePeriod)
}
// skipRefreshForTokens reports whether a proactive refresh can be skipped given
// the session's access and ID tokens.
func skipRefreshForTokens(accessToken, idToken string, grace time.Duration) bool {
graceToken := accessToken
graceJWT, graceErr := parseJWT(graceToken)
if graceErr != nil || graceJWT == nil {
graceToken = idToken
graceJWT, graceErr = parseJWT(graceToken)
}
if graceJWT == nil || graceErr != nil {
return false
}
expClaim, ok := graceJWT.Claims["exp"].(float64)
if !ok {
return false
}
expTimeObj := time.Unix(int64(expClaim), 0)
return !expTimeObj.Before(time.Now().Add(grace))
}
func (t *TraefikOidc) ServeHTTP(rw http.ResponseWriter, req *http.Request) { func (t *TraefikOidc) ServeHTTP(rw http.ResponseWriter, req *http.Request) {
// Track whether a response has been committed so the panic handler can
// avoid appending an error body to an already-emitted valid response.
tw := &trackingWriter{ResponseWriter: rw}
rw = tw
// Recover from any panic in the handler chain (session decode, token // Recover from any panic in the handler chain (session decode, token
// validation, header injection, authorize-url build) so a bug answers a // validation, header injection, authorize-url build) so a bug answers a
// single clean 500 (and logs a stack) instead of escaping to net/http, // single clean 500 (and logs a stack) instead of escaping to net/http,
// which closes/truncates the connection. WriteHeader is idempotent, so // which closes/truncates the connection. If the response was already
// this is safe even if a handler already wrote headers; Write appends. // committed by a handler (200 + body or a redirect), WriteHeader is an
// idempotent no-op and appending the body would corrupt a valid
// response, so only write when nothing has been sent yet.
defer func() { defer func() {
if r := recover(); r != nil { if r := recover(); r != nil {
t.logger.Errorf("OIDC handler panic recovered: %v\n%s", r, debug.Stack()) t.logger.Errorf("OIDC handler panic recovered: %v\n%s", r, debug.Stack())
rw.WriteHeader(http.StatusInternalServerError) if !tw.wroteHeader {
_, _ = rw.Write([]byte("Internal Server Error")) // A panic-induced 500 must not be cached (consistent with
// every other auth-failure response, R101/R172).
rw.Header().Set("Cache-Control", "no-store")
rw.WriteHeader(http.StatusInternalServerError)
_, _ = rw.Write([]byte("Internal Server Error"))
}
} }
}() }()
// Apply security headers to EVERY response this middleware authors
// (redirects, error pages, bypass forwards, and authenticated
// forwards), not just the forwardAuthorized tail. Previously they only
// reached successfully-authenticated forwarding, so the initial
// 302-to-IdP, /health, /metrics, error pages and SSE/WS responses
// carried no Strict-Transport-Security / framing / MIME sniffing
// protections (R132).
if t.securityHeadersApplier != nil {
t.securityHeadersApplier(rw, req)
} else {
// Fallback to basic security headers
rw.Header().Set("X-Frame-Options", "DENY")
rw.Header().Set("X-Content-Type-Options", "nosniff")
rw.Header().Set("X-XSS-Protection", "1; mode=block")
rw.Header().Set("Referrer-Policy", "strict-origin-when-cross-origin")
}
// Log request entry for debugging routing issues // Log request entry for debugging routing issues
t.logger.Debugf("Incoming request: %s %s", req.Method, req.URL.Path) t.logger.Debugf("Incoming request: %s %s", req.Method, req.URL.Path)
@@ -241,12 +399,20 @@ func (t *TraefikOidc) ServeHTTP(rw http.ResponseWriter, req *http.Request) {
// public endpoint logs via downstream services that don't expect it. // public endpoint logs via downstream services that don't expect it.
// Excluded URLs are explicitly public; bearer is an artifact of the // Excluded URLs are explicitly public; bearer is an artifact of the
// API auth flow that doesn't belong on them. // API auth flow that doesn't belong on them.
if t.enableBearerAuth { // Also honor stripAuthorizationHeader (default true) so a raw
// client-supplied Authorization header is not forwarded verbatim on
// the SSE/WebSocket/excluded bypass — the same leak R144 closed for
// the normal path was left open on the streaming bypass (R151).
if t.enableBearerAuth || t.stripAuthorizationHeader {
req.Header.Del("Authorization") req.Header.Del("Authorization")
} }
switch reason { switch reason {
case bypassReasonExcluded: case bypassReasonExcluded:
// Operator-declared excluded URLs forward unconditionally. // Operator-declared excluded URLs forward unconditionally.
// These are public paths; still strip any client-forged
// identity headers so X-Forwarded-User / X-User-Groups etc.
// on a public endpoint can't be trusted downstream (R102).
stripIdentityHeaders(req)
t.next.ServeHTTP(rw, req) t.next.ServeHTTP(rw, req)
case bypassReasonSSE, bypassReasonWebSocket: case bypassReasonSSE, bypassReasonWebSocket:
// Skip the OIDC redirect dance (clients can't follow it // Skip the OIDC redirect dance (clients can't follow it
@@ -263,6 +429,11 @@ func (t *TraefikOidc) ServeHTTP(rw http.ResponseWriter, req *http.Request) {
return return
} }
t.next.ServeHTTP(rw, req) t.next.ServeHTTP(rw, req)
case bypassReasonOptions:
// CORS preflight: forward unconditionally (no session needed)
// so the backend can answer with CORS headers.
stripIdentityHeaders(req)
t.next.ServeHTTP(rw, req)
default: default:
t.next.ServeHTTP(rw, req) t.next.ServeHTTP(rw, req)
} }
@@ -351,14 +522,31 @@ func (t *TraefikOidc) ServeHTTP(rw http.ResponseWriter, req *http.Request) {
cleanReq := req.Clone(req.Context()) cleanReq := req.Clone(req.Context())
session, _ = t.sessionManager.GetSession(cleanReq) // Safe to ignore: error already logged, proceeding with new session session, _ = t.sessionManager.GetSession(cleanReq) // Safe to ignore: error already logged, proceeding with new session
if session != nil { if session != nil {
defer session.returnToPoolSafely()
if clearErr := session.Clear(cleanReq, rw); clearErr != nil { if clearErr := session.Clear(cleanReq, rw); clearErr != nil {
t.logger.Errorf("Error clearing potentially corrupted session: %v", clearErr) t.logger.Errorf("Error clearing potentially corrupted session: %v", clearErr)
} }
} else { // Clear() returns the session to the object pool, so discard the
t.logger.Error("Critical session error: Failed to get even a new session.") // pointer — reusing it would be a use-after-return, since a
t.sendErrorResponse(rw, req, "Critical session error", http.StatusInternalServerError) // concurrent GetSession may already own the pooled object
return // (cross-request session bleed / data race). Obtain a fresh
// owned session in the shared block below. (R108)
session = nil
}
if session == nil {
// Session cookie is expired (past sessionMaxAge): a fresh
// GetSession still re-reads the same expired cookie and returns
// nil, so obtain a clean session and let the shared block below
// re-initiate authentication rather than returning 500 —
// otherwise every request past the session max age is a 500
// with no way to recover (R97).
session = t.sessionManager.newSession(cleanReq)
if session == nil {
t.logger.Error("Critical session error: Failed to get even a new session.")
t.sendErrorResponse(rw, req, "Critical session error", http.StatusInternalServerError)
return
}
defer session.returnToPoolSafely()
t.logger.Infof("Session expired or unreadable; re-initiating authentication")
} }
// Sub-resource requests (script/image/fetch/serviceWorker) must not // Sub-resource requests (script/image/fetch/serviceWorker) must not
// trigger an OIDC redirect from this path either: they would overwrite // trigger an OIDC redirect from this path either: they would overwrite
@@ -398,6 +586,19 @@ func (t *TraefikOidc) ServeHTTP(rw http.ResponseWriter, req *http.Request) {
t.logger.Debugf("Checking callback URL match: request_path=%q, configured_callback=%q", req.URL.Path, t.redirURLPath) t.logger.Debugf("Checking callback URL match: request_path=%q, configured_callback=%q", req.URL.Path, t.redirURLPath)
if req.URL.Path == t.redirURLPath { if req.URL.Path == t.redirURLPath {
t.logger.Debugf("Callback URL matched, processing OIDC callback (redirect_url=%s)", redirectURL) t.logger.Debugf("Callback URL matched, processing OIDC callback (redirect_url=%s)", redirectURL)
// The authorization-code response is delivered to the redirect
// endpoint via HTTP GET (RFC 6749 §3.1.2): handleCallback reads
// code/state/error only from the query string (form_post is not
// supported - a POST would see an empty query and fail with "No
// authorization code" anyway). Reject other methods with 405 so a
// HEAD/non-GET probe doesn't consume the one-time state and code
// and emit a 302 (R181).
if req.Method != http.MethodGet {
t.logger.Debugf("Callback requested with method %s (want GET); returning 405", req.Method)
rw.Header().Set("Cache-Control", "no-store")
rw.WriteHeader(http.StatusMethodNotAllowed)
return
}
t.handleCallback(rw, req, redirectURL) t.handleCallback(rw, req, redirectURL)
return return
} }
@@ -466,25 +667,17 @@ func (t *TraefikOidc) ServeHTTP(rw http.ResponseWriter, req *http.Request) {
} }
if shouldAttemptRefresh { if shouldAttemptRefresh {
idToken := session.GetIDToken() // Only skip the proactive refresh when the session is genuinely
if idToken != "" { // authenticated. skipRefreshForTokens only parseJWT's the access
jwt, err := parseJWT(idToken) // token and reads the raw exp claim — it does not verify the
if err == nil { // signature or identity, so gating on the authenticated flag keeps
claims := jwt.Claims // an unauthenticated session (or one that failed verification for a
if expClaim, ok := claims["exp"].(float64); ok { // non-expiry reason) from being bare-forwarded past the allowlist
expTime := int64(expClaim) // gate (R142).
expTimeObj := time.Unix(expTime, 0) if authenticated && t.shouldSkipGraceRefresh(session) {
refreshThreshold := time.Now().Add(t.refreshGracePeriod) t.logger.Debug("Token is valid and outside grace period, skipping refresh")
t.processAuthorizedRequest(rw, req, session, redirectURL)
if !expTimeObj.Before(refreshThreshold) { return
t.logger.Debug("Token is valid and outside grace period, skipping refresh")
t.processAuthorizedRequest(rw, req, session, redirectURL)
return
}
} else {
t.logger.Debug("Could not extract 'exp' claim for grace period check, proceeding with refresh")
}
}
} }
if needsRefresh && authenticated { if needsRefresh && authenticated {
@@ -574,7 +767,8 @@ func (t *TraefikOidc) processAuthorizedRequestRS(rw http.ResponseWriter, req *ht
idToken := rs.idToken idToken := rs.idToken
if t.enableBackchannelLogout || t.enableFrontchannelLogout { if t.enableBackchannelLogout || t.enableFrontchannelLogout {
if idToken != "" { if idToken != "" {
sid, sub, createdAt := t.extractSessionInfo(idToken) sid, sub, _ := t.extractSessionInfo(idToken)
createdAt := t.sessionCreatedAtForInvalidation(idToken, session)
if t.isSessionInvalidated(sid, sub, createdAt) { if t.isSessionInvalidated(sid, sub, createdAt) {
t.logger.Infof("Session for user %s has been invalidated via IdP-initiated logout", userIdentifier) t.logger.Infof("Session for user %s has been invalidated via IdP-initiated logout", userIdentifier)
if err := session.Clear(req, rw); err != nil { if err := session.Clear(req, rw); err != nil {
@@ -666,7 +860,8 @@ func (t *TraefikOidc) processAuthorizedRequest(rw http.ResponseWriter, req *http
if t.enableBackchannelLogout || t.enableFrontchannelLogout { if t.enableBackchannelLogout || t.enableFrontchannelLogout {
idToken := session.GetIDToken() idToken := session.GetIDToken()
if idToken != "" { if idToken != "" {
sid, sub, createdAt := t.extractSessionInfo(idToken) sid, sub, _ := t.extractSessionInfo(idToken)
createdAt := t.sessionCreatedAtForInvalidation(idToken, session)
if t.isSessionInvalidated(sid, sub, createdAt) { if t.isSessionInvalidated(sid, sub, createdAt) {
t.logger.Infof("Session for user %s has been invalidated via IdP-initiated logout", userIdentifier) t.logger.Infof("Session for user %s has been invalidated via IdP-initiated logout", userIdentifier)
// Clear the session and redirect to login // Clear the session and redirect to login
@@ -788,6 +983,18 @@ const headerTemplateMaxLen = 8192
// optional claim the provider does not emit does not leak "<no value>". // optional claim the provider does not emit does not leak "<no value>".
const noValueSentinel = "<no value>" const noValueSentinel = "<no value>"
// noValueTokenRe matches "<no value>" only as a whole token (bounded by
// non-alphanumerics or string end), so a legitimate claim value that
// happens to contain the substring survives the strip instead of being
// corrupted (R103).
var noValueTokenRe = regexp.MustCompile(`(^|[^0-9A-Za-z])` + regexp.QuoteMeta(noValueSentinel) + `([^0-9A-Za-z]|$)`)
// stripNoValueSentinels removes text/template's missing-key placeholder
// only when it appears as a whole token.
func stripNoValueSentinels(s string) string {
return noValueTokenRe.ReplaceAllString(s, "${1}${2}")
}
// headerClaimMaxLen returns the maximum accepted length for a claim-derived // headerClaimMaxLen returns the maximum accepted length for a claim-derived
// header value (principal identifier, group, role). Reuses the operator- // header value (principal identifier, group, role). Reuses the operator-
// configured identifier cap (default 256) so a single setting governs both // configured identifier cap (default 256) so a single setting governs both
@@ -820,7 +1027,46 @@ func (t *TraefikOidc) sanitizeHeaderClaimList(values []string, headerName string
return safe return safe
} }
// joinBoundedClaimHeader joins sanitized claim values with commas, bounding
// the TOTAL length to headerTemplateMaxLen. A provider can return an
// unbounded number of groups/roles; joining them all into a single header
// would let an oversized value reach a downstream proxy that rejects or
// truncates over-long headers (431), even though each value already passed
// the per-value cap. Accumulates as many leading values as fit, so a
// well-formed list is unaffected; an empty result means nothing fit and the
// caller should drop the header (fail-closed).
func joinBoundedClaimHeader(values []string) string {
if len(values) == 0 {
return ""
}
var b strings.Builder
for _, v := range values {
sep := 0
if b.Len() > 0 {
sep = 1 // leading comma
}
if b.Len()+sep+len(v) > headerTemplateMaxLen {
break
}
if sep > 0 {
b.WriteByte(',')
}
b.WriteString(v)
}
return b.String()
}
func (t *TraefikOidc) forwardAuthorized(rw http.ResponseWriter, req *http.Request, p *principal) { func (t *TraefikOidc) forwardAuthorized(rw http.ResponseWriter, req *http.Request, p *principal) {
// This middleware fully owns the identity headers it injects upstream.
// Unconditionally remove any inbound values with the same names first:
// a request could carry a forged X-User-Groups / X-Auth-Request-User
// etc. that, if the middleware computes no value for one of them
// (e.g. an authenticated user with no groups, an absent ID token, a
// a header-template skip), would otherwise survive through to the
// backend and be trusted for downstream authorization. Clearing here —
// before any conditional Set — makes the values we emit authoritative.
stripIdentityHeaders(req)
var ( var (
groups, roles []string groups, roles []string
extractErr error extractErr error
@@ -844,11 +1090,24 @@ func (t *TraefikOidc) forwardAuthorized(rw http.ResponseWriter, req *http.Reques
// header injection and an embedded comma would inject extra // header injection and an embedded comma would inject extra
// entries into the comma-joined header. Fail closed: drop any // entries into the comma-joined header. Fail closed: drop any
// value that does not pass. // value that does not pass.
if safeGroups := t.sanitizeHeaderClaimList(groups, "X-User-Groups"); len(safeGroups) > 0 { // X-User-Groups / X-User-Roles are the largest variably-sized
req.Header.Set("X-User-Groups", strings.Join(safeGroups, ",")) // forward headers; gate them behind minimalHeaders like the
} // X-Auth-Request-* family so group-heavy users don't still trip
if safeRoles := t.sanitizeHeaderClaimList(roles, "X-User-Roles"); len(safeRoles) > 0 { // HTTP 431 on the backend (the failure minimalHeaders exists to
req.Header.Set("X-User-Roles", strings.Join(safeRoles, ",")) // prevent, #64). Identity (X-Forwarded-User) stays
// unconditional; the allowedRolesAndGroups gate below still
// evaluates groups regardless of emission.
if !t.minimalHeaders {
if safeGroups := t.sanitizeHeaderClaimList(groups, "X-User-Groups"); len(safeGroups) > 0 {
if gh := joinBoundedClaimHeader(safeGroups); gh != "" {
req.Header.Set("X-User-Groups", gh)
}
}
if safeRoles := t.sanitizeHeaderClaimList(roles, "X-User-Roles"); len(safeRoles) > 0 {
if rh := joinBoundedClaimHeader(safeRoles); rh != "" {
req.Header.Set("X-User-Roles", rh)
}
}
} }
} }
} }
@@ -902,17 +1161,29 @@ func (t *TraefikOidc) forwardAuthorized(rw http.ResponseWriter, req *http.Reques
t.logger.Debugf("Dropping X-Auth-Request-User header: identifier failed claim sanitization") t.logger.Debugf("Dropping X-Auth-Request-User header: identifier failed claim sanitization")
} }
if p.IDToken != "" { if p.IDToken != "" {
req.Header.Set("X-Auth-Request-Token", p.IDToken) // Bound the raw ID token to the same header budget as rendered
// template values so a large token can't push this (and every
// other header on the request) past nginx/traefik 431 limits.
idTokenHeader := p.IDToken
if len(idTokenHeader) > headerTemplateMaxLen {
idTokenHeader = idTokenHeader[:headerTemplateMaxLen]
}
req.Header.Set("X-Auth-Request-Token", idTokenHeader)
} }
} }
// Bearer source: strip the raw inbound Authorization header to keep the // Strip the raw inbound Authorization header for BOTH auth paths when
// token out of downstream service logs (off-by-config for operators who // configured. The bearer path already stripped it; the cookie/session
// chain services that each re-verify the bearer). Stripped here, before // path forwarded a client-supplied Authorization verbatim, letting a
// header-template application, so an operator-supplied Authorization // session user present an arbitrary (and possibly more privileged)
// template (e.g. "Authorization: Bearer {{.AccessToken}}") can re-supply // bearer that a downstream verifier might trust over X-Forwarded-User,
// the downstream value rather than being immediately deleted. // or leaking a raw token into downstream logs. Kept out of downstream
if p.Source == sourceBearer && t.stripAuthorizationHeader { // logs and make X-Forwarded-User / X-Auth-Request-Token the
// authoritative identity signal. Stripped here, before header-template
// application, so an operator-supplied Authorization template (e.g.
// "Authorization: Bearer {{.AccessToken}}") can re-supply the
// downstream value rather than being immediately deleted.
if t.stripAuthorizationHeader {
req.Header.Del("Authorization") req.Header.Del("Authorization")
} }
@@ -942,7 +1213,7 @@ func (t *TraefikOidc) forwardAuthorized(rw http.ResponseWriter, req *http.Reques
// optional claim the provider did not emit (e.g. email) does not // optional claim the provider did not emit (e.g. email) does not
// leak "<no value>" into the downstream header. // leak "<no value>" into the downstream header.
if strings.Contains(headerValue, noValueSentinel) { if strings.Contains(headerValue, noValueSentinel) {
headerValue = strings.ReplaceAll(headerValue, noValueSentinel, "") headerValue = stripNoValueSentinels(headerValue)
} }
// Skip an empty render: Setting "" would clobber an identity // Skip an empty render: Setting "" would clobber an identity
// header (X-Forwarded-User / X-Auth-Request-*) already injected // header (X-Forwarded-User / X-Auth-Request-*) already injected
@@ -970,17 +1241,6 @@ func (t *TraefikOidc) forwardAuthorized(rw http.ResponseWriter, req *http.Reques
} }
} }
// Apply security headers if configured
if t.securityHeadersApplier != nil {
t.securityHeadersApplier(rw, req)
} else {
// Fallback to basic security headers
rw.Header().Set("X-Frame-Options", "DENY")
rw.Header().Set("X-Content-Type-Options", "nosniff")
rw.Header().Set("X-XSS-Protection", "1; mode=block")
rw.Header().Set("Referrer-Policy", "strict-origin-when-cross-origin")
}
// Strip OIDC session cookies before forwarding to the backend to prevent // Strip OIDC session cookies before forwarding to the backend to prevent
// HTTP 431 "Request Header Fields Too Large" errors (GitHub issue #122). // HTTP 431 "Request Header Fields Too Large" errors (GitHub issue #122).
if t.stripAuthCookies && t.sessionManager != nil { if t.stripAuthCookies && t.sessionManager != nil {
+23 -5
View File
@@ -35,6 +35,10 @@ type MemorySnapshot struct {
HeapProfile []byte HeapProfile []byte
GoroutineProfile []byte GoroutineProfile []byte
RuntimeStats runtime.MemStats RuntimeStats runtime.MemStats
// Goroutines is the goroutine count (runtime.NumGoroutine()) captured at
// snapshot time, so leak analysis can diff a baseline against a later
// snapshot instead of sampling "now" twice (R118).
Goroutines int
} }
// LeakAnalysis contains the results of memory leak detection and analysis. // LeakAnalysis contains the results of memory leak detection and analysis.
@@ -118,10 +122,22 @@ func NewProfilingManager(logger *Logger) *ProfilingManager {
// TakeSnapshot captures a comprehensive snapshot of current memory statistics. // TakeSnapshot captures a comprehensive snapshot of current memory statistics.
// Includes runtime stats, heap profile, goroutine profile, and custom metrics. // Includes runtime stats, heap profile, goroutine profile, and custom metrics.
func (pm *ProfilingManager) TakeSnapshot() (*MemorySnapshot, error) { func (pm *ProfilingManager) TakeSnapshot() (*MemorySnapshot, error) {
pm.mu.RLock()
defer pm.mu.RUnlock()
return pm.takeSnapshot()
}
// takeSnapshot implements TakeSnapshot assuming the caller already holds the
// lock (either RLock for the public API, or the write Lock during
// Start/StopProfiling). Calling the public TakeSnapshot from Start/Stop
// while holding pm.mu.Lock() would self-deadlock (an RWMutex is not
// recursively re-entrant on the write side) — R118.
func (pm *ProfilingManager) takeSnapshot() (*MemorySnapshot, error) { // nolint:unparam // error return kept for the exported TakeSnapshot contract
var buf bytes.Buffer var buf bytes.Buffer
snapshot := &MemorySnapshot{ snapshot := &MemorySnapshot{
Timestamp: time.Now(), Timestamp: time.Now(),
CustomMetrics: make(map[string]interface{}), CustomMetrics: make(map[string]interface{}),
Goroutines: runtime.NumGoroutine(),
} }
runtime.ReadMemStats(&snapshot.RuntimeStats) runtime.ReadMemStats(&snapshot.RuntimeStats)
@@ -146,13 +162,11 @@ func (pm *ProfilingManager) TakeSnapshot() (*MemorySnapshot, error) {
} }
} }
pm.mu.RLock()
for name, profiler := range pm.profilers { for name, profiler := range pm.profilers {
if customStats := profiler.GetCurrentStats(); customStats != nil { if customStats := profiler.GetCurrentStats(); customStats != nil {
snapshot.CustomMetrics[name] = customStats snapshot.CustomMetrics[name] = customStats
} }
} }
pm.mu.RUnlock()
return snapshot, nil return snapshot, nil
} }
@@ -170,7 +184,7 @@ func (pm *ProfilingManager) StartProfiling(config ProfilingConfig) error {
pm.isProfiling = true pm.isProfiling = true
pm.startTime = time.Now() pm.startTime = time.Now()
baseline, err := pm.TakeSnapshot() baseline, err := pm.takeSnapshot() // lock already held -> use unlocked variant
if err != nil { if err != nil {
pm.isProfiling = false pm.isProfiling = false
return fmt.Errorf("failed to take baseline snapshot: %w", err) return fmt.Errorf("failed to take baseline snapshot: %w", err)
@@ -190,7 +204,7 @@ func (pm *ProfilingManager) StopProfiling() (*MemorySnapshot, error) {
return nil, fmt.Errorf("profiling not in progress") return nil, fmt.Errorf("profiling not in progress")
} }
finalSnapshot, err := pm.TakeSnapshot() finalSnapshot, err := pm.takeSnapshot() // lock already held -> use unlocked variant
if err != nil { if err != nil {
pm.logger.Errorf("Failed to take final snapshot: %v", err) pm.logger.Errorf("Failed to take final snapshot: %v", err)
} }
@@ -225,8 +239,12 @@ func (pm *ProfilingManager) AnalyzeLeaks(baseline, current *MemorySnapshot) *Lea
memoryIncrease := current.RuntimeStats.Alloc - baseline.RuntimeStats.Alloc memoryIncrease := current.RuntimeStats.Alloc - baseline.RuntimeStats.Alloc
analysis.MemoryIncrease = memoryIncrease analysis.MemoryIncrease = memoryIncrease
// Diff against the goroutine count captured when the baseline snapshot
// was taken, not a live double-sample at analysis time. Sampling "now"
// for both values made the increase always 0 and the goroutine-leak
// branch permanently dead (R118).
baselineGoroutines := baseline.Goroutines
currentGoroutines := runtime.NumGoroutine() currentGoroutines := runtime.NumGoroutine()
baselineGoroutines := runtime.NumGoroutine()
goroutineIncrease := currentGoroutines - baselineGoroutines goroutineIncrease := currentGoroutines - baselineGoroutines
analysis.GoroutineIncrease = goroutineIncrease analysis.GoroutineIncrease = goroutineIncrease
+23 -6
View File
@@ -183,7 +183,7 @@ func NewRefreshCoordinator(config RefreshCoordinatorConfig, logger *Logger) *Ref
rc := &RefreshCoordinator{ rc := &RefreshCoordinator{
// inFlightRefreshes and sessionRefreshAttempts are both sync.Map; // inFlightRefreshes and sessionRefreshAttempts are both sync.Map;
// their zero values are ready to use. // their zero values are ready to use.
config: config, config: config,
metrics: &RefreshMetrics{}, metrics: &RefreshMetrics{},
logger: logger, logger: logger,
stopChan: make(chan struct{}), stopChan: make(chan struct{}),
@@ -225,10 +225,22 @@ func (rc *RefreshCoordinator) CoordinateRefresh(
// CRITICAL FIX: Atomically check for existing operation OR create new one // CRITICAL FIX: Atomically check for existing operation OR create new one
// This prevents the race where multiple goroutines check, find nothing, then all create // This prevents the race where multiple goroutines check, find nothing, then all create
//
// R154: increment the WaitGroup BEFORE getOrCreateOperation registers
// the operation. Previously the Add(1) happened only after the op was
// registered and published as new, so a concurrent Shutdown
// (close(stopChan); wg.Wait) could observe the registered op, see a
// zero wg count, and return before this goroutine's Add landed — the
// just-started refresh then ran past Shutdown unreaped. Done() is
// balanced once per path: by the spawned goroutine, or immediately
// on the join/reject paths (which spawn nothing).
rc.wg.Add(1)
operation, isNew, err := rc.getOrCreateOperation(ctx, sessionID, tokenHash, refreshToken) operation, isNew, err := rc.getOrCreateOperation(ctx, sessionID, tokenHash, refreshToken)
if err != nil { if err != nil {
// Operation creation was rejected (rate limit, memory pressure, concurrent limit) // Operation creation was rejected (rate limit, memory pressure, concurrent limit)
rc.wg.Done() // no goroutine launched
return nil, err return nil, err
} }
@@ -236,7 +248,6 @@ func (rc *RefreshCoordinator) CoordinateRefresh(
// We created a new operation, so we need to execute it. Track the // We created a new operation, so we need to execute it. Track the
// goroutine so Shutdown waits for in-flight refreshes to finish (they // goroutine so Shutdown waits for in-flight refreshes to finish (they
// are aborted promptly via rc.ctx when stopChan closes). // are aborted promptly via rc.ctx when stopChan closes).
rc.wg.Add(1)
go func() { go func() {
defer rc.wg.Done() defer rc.wg.Done()
rc.executeRefreshAsync(operation, sessionID, tokenHash, refreshFunc) //nolint:gosec // long-lived background refresh intentionally uses a background context rc.executeRefreshAsync(operation, sessionID, tokenHash, refreshFunc) //nolint:gosec // long-lived background refresh intentionally uses a background context
@@ -244,6 +255,7 @@ func (rc *RefreshCoordinator) CoordinateRefresh(
} else { } else {
// Joined existing operation - this is a deduplicated request // Joined existing operation - this is a deduplicated request
atomic.AddInt64(&rc.metrics.deduplicatedRequests, 1) atomic.AddInt64(&rc.metrics.deduplicatedRequests, 1)
rc.wg.Done() // joined an existing op; no goroutine tracked here
} }
// Wait for the operation to complete // Wait for the operation to complete
@@ -665,15 +677,20 @@ func refreshCoordinatorSessionID(token string) string {
const refreshCoordinatorWaitTimeout = 35 * time.Second const refreshCoordinatorWaitTimeout = 35 * time.Second
// isUnderMemoryPressure checks if the system is under memory pressure by // isUnderMemoryPressure checks if the system is under memory pressure by
// consulting the global memory monitor. Returns true when pressure reaches // consulting the global memory monitor. Returns true when the current heap
// High or Critical, at which point we refuse new refresh operations to // exceeds the coordinator's OWN configured MemoryPressureThresholdMB, at
// avoid aggravating an already-stressed heap. // which point we refuse new refresh operations to avoid aggravating an
// already-stressed heap.
func (rc *RefreshCoordinator) isUnderMemoryPressure() bool { func (rc *RefreshCoordinator) isUnderMemoryPressure() bool {
monitor := GetGlobalMemoryMonitor() monitor := GetGlobalMemoryMonitor()
if monitor == nil { if monitor == nil {
return false return false
} }
return monitor.GetMemoryPressure() >= MemoryPressureHigh stats := monitor.GetCurrentStats()
if stats == nil || rc.config.MemoryPressureThresholdMB == 0 {
return false
}
return stats.HeapAllocBytes > rc.config.MemoryPressureThresholdMB*1024*1024
} }
// cleanupRoutine periodically cleans up stale tracking entries // cleanupRoutine periodically cleans up stale tracking entries
+6 -6
View File
@@ -32,12 +32,12 @@ type requestState struct {
// (Save, Clear, etc.) still have access. // (Save, Clear, etc.) still have access.
session *SessionData session *SessionData
authenticated bool authenticated bool
accessToken string accessToken string
idToken string idToken string
refreshToken string refreshToken string
userIdentifier string userIdentifier string
createdAtUnixSec int64 createdAtUnixSec int64
// Output: scheme/host/redirect path determined at top of ServeHTTP. // Output: scheme/host/redirect path determined at top of ServeHTTP.
scheme string scheme string
+4 -1
View File
@@ -38,7 +38,10 @@ func (sf *ScopeFilter) FilterSupportedScopes(requestedScopes, supportedScopes []
// If no supported scopes declared, return all requested (backward compatibility) // If no supported scopes declared, return all requested (backward compatibility)
if len(supportedScopes) == 0 { if len(supportedScopes) == 0 {
sf.logger.Debugf("ScopeFilter: Provider %s has no scopes_supported in discovery doc, using all requested scopes", providerURL) sf.logger.Debugf("ScopeFilter: Provider %s has no scopes_supported in discovery doc, using all requested scopes", providerURL)
return requestedScopes // Return a copy so the caller can append without aliasing the
// slice it passed in (which would let the two views of the same
// backing array cross-contaminate each other).
return append([]string(nil), requestedScopes...)
} }
// Build lookup map for efficient checking // Build lookup map for efficient checking
+394 -179
View File
@@ -140,12 +140,14 @@ type combinedSessionPayload struct {
X map[string]interface{} `json:"x,omitempty"` X map[string]interface{} `json:"x,omitempty"`
A string `json:"a,omitempty"` A string `json:"a,omitempty"`
R string `json:"r,omitempty"` R string `json:"r,omitempty"`
Ri int64 `json:"ri,omitempty"` // refresh-token issued_at (unix), preserved so maxRefreshTokenAge survives combined round-trip
I string `json:"i,omitempty"` I string `json:"i,omitempty"`
Ui string `json:"ui,omitempty"` Ui string `json:"ui,omitempty"`
Cs string `json:"cs,omitempty"` Cs string `json:"cs,omitempty"`
N string `json:"n,omitempty"` N string `json:"n,omitempty"`
Cv string `json:"cv,omitempty"` Cv string `json:"cv,omitempty"`
Ip string `json:"ip,omitempty"` Ip string `json:"ip,omitempty"`
Ru string `json:"ru,omitempty"` // redirect_uri used at initiate; reused at callback so a per-request rebuild can't drift (R136)
Ca int64 `json:"ca,omitempty"` Ca int64 `json:"ca,omitempty"`
Rc int `json:"rc,omitempty"` Rc int `json:"rc,omitempty"`
Au bool `json:"au,omitempty"` Au bool `json:"au,omitempty"`
@@ -163,6 +165,7 @@ var knownSessionKeys = map[string]bool{
"nonce": true, "nonce": true,
"code_verifier": true, "code_verifier": true,
"incoming_path": true, "incoming_path": true,
"redirect_url": true,
"created_at": true, "created_at": true,
"redirect_count": true, "redirect_count": true,
} }
@@ -381,16 +384,6 @@ func decompressTokenInternal(compressed string) string {
limitedReader := io.LimitReader(gz, 500*1024) limitedReader := io.LimitReader(gz, 500*1024)
if cap(readerBuf) >= 512*1024 {
readerBuf = readerBuf[:cap(readerBuf)]
n, err := limitedReader.Read(readerBuf)
if err != nil && err != io.EOF {
return compressed
}
decompressed := readerBuf[:n]
return string(decompressed)
}
decompressed, err := io.ReadAll(limitedReader) decompressed, err := io.ReadAll(limitedReader)
if err != nil { if err != nil {
return compressed return compressed
@@ -912,6 +905,14 @@ func (sm *SessionManager) EnhanceSessionSecurity(options *sessions.Options, r *h
sm.logger.Debugf("Using configured cookie domain: %s", sm.cookieDomain) sm.logger.Debugf("Using configured cookie domain: %s", sm.cookieDomain)
} else if options.Domain == "" && r != nil { } else if options.Domain == "" && r != nil {
host := r.Host host := r.Host
// Strip the port before the subdomain-bounding check below; a ported
// r.Host (e.g. app.example.com:8080) would otherwise fail
// HasSuffix(host, "."+fh) and wrongly fall back to the strict
// parent X-Forwarded-Host, scoping the token-bearing session cookie
// to every sibling subdomain.
if ci := strings.Index(host, ":"); ci != -1 {
host = host[:ci]
}
if forwardedHost := r.Header.Get("X-Forwarded-Host"); forwardedHost != "" { if forwardedHost := r.Header.Get("X-Forwarded-Host"); forwardedHost != "" {
// Only use the forwarded host when it is the serving host or a // Only use the forwarded host when it is the serving host or a
@@ -1029,7 +1030,7 @@ func (sm *SessionManager) CleanupOldCookies(w http.ResponseWriter, r *http.Reque
Domain: domain, Domain: domain,
MaxAge: -1, MaxAge: -1,
HttpOnly: true, HttpOnly: true,
Secure: r.Header.Get("X-Forwarded-Proto") == "https" || r.TLS != nil || sm.forceHTTPS, Secure: strings.EqualFold(r.Header.Get("X-Forwarded-Proto"), "https") || r.TLS != nil || sm.forceHTTPS,
SameSite: http.SameSiteLaxMode, SameSite: http.SameSiteLaxMode,
} }
http.SetCookie(w, deleteCookie) http.SetCookie(w, deleteCookie)
@@ -1059,7 +1060,44 @@ func (sm *SessionManager) CleanupOldCookies(w http.ResponseWriter, r *http.Reque
// //
// Returns: // Returns:
// - The loaded SessionData instance. // - The loaded SessionData instance.
// - An error if session loading or validation fails. //
// newSession returns a clean, pooled session with no cookie state loaded. It
// is used when the request's session cookie is expired: GetSession keeps
// re-reading that same expired cookie and returns nil, so the caller needs a
// fresh session to (re-)initiate authentication rather than erroring out.
// Mirrors GetSession's pool/activeSessions accounting.
func (sm *SessionManager) newSession(r *http.Request) *SessionData {
sessionData, _ := sm.sessionPool.Get().(*SessionData) // Safe to ignore: pool return is best-effort
if sessionData == nil {
return nil
}
sessionData.Reset() // clear any stale state from a previous pooled user
sessionData.request = r
sessionData.inUse.Store(true)
sessionData.dirty = false
atomic.AddInt64(&sm.poolHits, 1)
atomic.AddInt64(&sm.activeSessions, 1)
// GetSession normally populates the four legacy sub-sessions from the
// request; newSession runs where that read already failed (expired
// cookie), so initialize non-nil containers here. Downstream auth
// flow (SetCSRF/SetNonce/... then Save) writes through these.
if sessionData.mainSession == nil {
sessionData.mainSession, _ = sm.store.Get(r, sm.mainCookieName())
}
if sessionData.accessSession == nil {
sessionData.accessSession, _ = sm.store.Get(r, sm.accessTokenCookieName())
}
if sessionData.refreshSession == nil {
sessionData.refreshSession, _ = sm.store.Get(r, sm.refreshTokenCookieName())
}
if sessionData.idTokenSession == nil {
sessionData.idTokenSession, _ = sm.store.Get(r, sm.idTokenCookieName())
}
return sessionData
}
// - An error if session loading or validation fails.
func (sm *SessionManager) GetSession(r *http.Request) (*SessionData, error) { func (sm *SessionManager) GetSession(r *http.Request) (*SessionData, error) {
sessionData, _ := sm.sessionPool.Get().(*SessionData) // Safe to ignore: pool return is best-effort sessionData, _ := sm.sessionPool.Get().(*SessionData) // Safe to ignore: pool return is best-effort
atomic.AddInt64(&sm.poolHits, 1) atomic.AddInt64(&sm.poolHits, 1)
@@ -1230,6 +1268,7 @@ func (sm *SessionManager) loadFromCombinedCookies(r *http.Request, sessionData *
sessionData.mainSession.Values["nonce"] = payload.N sessionData.mainSession.Values["nonce"] = payload.N
sessionData.mainSession.Values["code_verifier"] = payload.Cv sessionData.mainSession.Values["code_verifier"] = payload.Cv
sessionData.mainSession.Values["incoming_path"] = payload.Ip sessionData.mainSession.Values["incoming_path"] = payload.Ip
sessionData.mainSession.Values["redirect_url"] = payload.Ru
sessionData.mainSession.Values["created_at"] = payload.Ca sessionData.mainSession.Values["created_at"] = payload.Ca
sessionData.mainSession.Values["redirect_count"] = payload.Rc sessionData.mainSession.Values["redirect_count"] = payload.Rc
@@ -1243,6 +1282,9 @@ func (sm *SessionManager) loadFromCombinedCookies(r *http.Request, sessionData *
sessionData.refreshSession.Values["token"] = payload.R sessionData.refreshSession.Values["token"] = payload.R
sessionData.refreshSession.Values["compressed"] = false sessionData.refreshSession.Values["compressed"] = false
if payload.Ri > 0 {
sessionData.refreshSession.Values["issued_at"] = payload.Ri
}
sessionData.idTokenSession.Values["token"] = payload.I sessionData.idTokenSession.Values["token"] = payload.I
sessionData.idTokenSession.Values["compressed"] = false sessionData.idTokenSession.Values["compressed"] = false
@@ -1259,7 +1301,14 @@ func (sm *SessionManager) loadFromCombinedCookies(r *http.Request, sessionData *
// - chunks: The map (typically SessionData.accessTokenChunks or SessionData.refreshTokenChunks) // - chunks: The map (typically SessionData.accessTokenChunks or SessionData.refreshTokenChunks)
// to populate with the found session chunks. // to populate with the found session chunks.
func (sm *SessionManager) getTokenChunkSessions(r *http.Request, baseName string, chunks map[int]*sessions.Session) { func (sm *SessionManager) getTokenChunkSessions(r *http.Request, baseName string, chunks map[int]*sessions.Session) {
for i := 0; ; i++ { // Bound the scan at MaxChunks (50) so an oversized set of chunk
// cookies (attack or corruption) can't drive unbounded
// securecookie decrypts. Loading through index 50 still lets
// processChunkedToken's len(chunks) > MaxChunks check fire, so the
// overflow → error behavior is unchanged; indices beyond 50 would be
// rejected anyway (R183).
const maxChunkScan = 50
for i := 0; i <= maxChunkScan; i++ {
sessionName := fmt.Sprintf("%s_%d", baseName, i) sessionName := fmt.Sprintf("%s_%d", baseName, i)
session, err := sm.store.Get(r, sessionName) session, err := sm.store.Get(r, sessionName)
if err != nil || session.IsNew { if err != nil || session.IsNew {
@@ -1305,6 +1354,12 @@ type SessionData struct {
dirty bool dirty bool
// expireCookie is a one-shot flag set by Clear to force the cookies it
// writes to be expired (Max-Age: -1). Without it, Save rebuilds a
// positive MaxAge and logout leaves an empty session cookie alive for
// the full session lifetime instead of expiring it (R101).
expireCookie bool
inUse atomic.Bool inUse atomic.Bool
// cachedClaimsToken is the ID token string whose claims were last parsed and // cachedClaimsToken is the ID token string whose claims were last parsed and
@@ -1346,11 +1401,23 @@ func (sd *SessionData) MarkDirty() {
// Returns: // Returns:
// - An error if saving any of the session components fails. // - An error if saving any of the session components fails.
func (sd *SessionData) Save(r *http.Request, w http.ResponseWriter) error { func (sd *SessionData) Save(r *http.Request, w http.ResponseWriter) error {
isSecure := r.Header.Get("X-Forwarded-Proto") == "https" || r.TLS != nil || sd.manager.forceHTTPS // X-Forwarded-Proto value is compared case-insensitively: reverse proxies
// (nginx, Traefik) emit lowercase "https", but a client-controllable or
// non-canonical value like "HTTPS" must still yield a Secure cookie.
// Case-sensitive comparison here would silently downgrade the session
// cookie to transport-unsafe on such a header (R143).
isSecure := strings.EqualFold(r.Header.Get("X-Forwarded-Proto"), "https") || r.TLS != nil || sd.manager.forceHTTPS
options := sd.manager.getSessionOptions(isSecure) options := sd.manager.getSessionOptions(isSecure)
options = sd.manager.EnhanceSessionSecurity(options, r) options = sd.manager.EnhanceSessionSecurity(options, r)
if sd.expireCookie {
// Clear set this flag: the cookies written below must be expired,
// not re-issued with a fresh positive MaxAge.
options.MaxAge = -1
sd.expireCookie = false
}
// Use combined storage for new sessions // Use combined storage for new sessions
if sd.useCombinedStorage { if sd.useCombinedStorage {
return sd.saveCombined(r, w, options) return sd.saveCombined(r, w, options)
@@ -1363,7 +1430,11 @@ func (sd *SessionData) Save(r *http.Request, w http.ResponseWriter) error {
// saveCombined saves all session data in a single compressed, chunked cookie. // saveCombined saves all session data in a single compressed, chunked cookie.
// This reduces cookie count and total size through combined compression. // This reduces cookie count and total size through combined compression.
func (sd *SessionData) saveCombined(r *http.Request, w http.ResponseWriter, options *sessions.Options) error { func (sd *SessionData) saveCombined(r *http.Request, w http.ResponseWriter, options *sessions.Options) error {
// Build the combined payload // Build the combined payload. All token/main fields are read under
// sessionMutex.RLock so a concurrent Set* (which writes under the
// write side) can't tear the payload mid-build (R132). The extra-map
// walk below already took the Read side; extend it to the field reads.
sd.sessionMutex.RLock()
payload := &combinedSessionPayload{ payload := &combinedSessionPayload{
A: sd.getAccessTokenUnsafe(), A: sd.getAccessTokenUnsafe(),
R: sd.getRefreshTokenUnsafe(), R: sd.getRefreshTokenUnsafe(),
@@ -1374,12 +1445,18 @@ func (sd *SessionData) saveCombined(r *http.Request, w http.ResponseWriter, opti
N: sd.getNonceUnsafe(), N: sd.getNonceUnsafe(),
Cv: sd.getCodeVerifierUnsafe(), Cv: sd.getCodeVerifierUnsafe(),
Ip: sd.getIncomingPathUnsafe(), Ip: sd.getIncomingPathUnsafe(),
Ru: sd.getRedirectURLUnsafe(),
Ca: sd.getCreatedAtUnsafe(), Ca: sd.getCreatedAtUnsafe(),
Rc: sd.getRedirectCountUnsafe(), Rc: sd.getRedirectCountUnsafe(),
} }
// Persist the refresh token's issued_at so maxRefreshTokenAge keeps
// working after a combined save/load round-trip (it lives on
// refreshSession, which is otherwise dropped by combined storage).
if ra, ok := sd.refreshSession.Values["issued_at"].(int64); ok {
payload.Ri = ra
}
// Collect extra session values not handled by the standard fields // Collect extra session values not handled by the standard fields
sd.sessionMutex.RLock()
if sd.mainSession != nil && len(sd.mainSession.Values) > 0 { if sd.mainSession != nil && len(sd.mainSession.Values) > 0 {
extra := make(map[string]interface{}) extra := make(map[string]interface{})
for key, val := range sd.mainSession.Values { for key, val := range sd.mainSession.Values {
@@ -1404,7 +1481,11 @@ func (sd *SessionData) saveCombined(r *http.Request, w http.ResponseWriter, opti
compressed, err := compressCombinedPayload(payload) compressed, err := compressCombinedPayload(payload)
if err != nil { if err != nil {
sd.manager.logger.Errorf("Failed to compress combined payload: %v", err) sd.manager.logger.Errorf("Failed to compress combined payload: %v", err)
// Fall back to legacy storage on compression failure // Fall back to legacy storage on compression failure. Expire any
// pre-existing combined chunk cookies first, or GetSession
// (combined-first) would keep serving the stale combined payload
// and shadow the freshly written legacy tokens (R163).
sd.expireOldCombinedChunks(r, w, options, 0)
return sd.saveLegacy(r, w, options) return sd.saveLegacy(r, w, options)
} }
@@ -1413,8 +1494,15 @@ func (sd *SessionData) saveCombined(r *http.Request, w http.ResponseWriter, opti
// Split into chunks // Split into chunks
chunks := splitCombinedIntoChunks(compressed, maxCookieSize) chunks := splitCombinedIntoChunks(compressed, maxCookieSize)
if len(chunks) > maxCombinedChunks { if len(chunks) > maxCombinedChunks {
sd.manager.logger.Errorf("Combined session requires %d chunks, exceeds max %d", len(chunks), maxCombinedChunks) sd.manager.logger.Errorf("Combined session requires %d chunks, exceeds max %d; falling back to legacy storage", len(chunks), maxCombinedChunks)
return fmt.Errorf("session data too large: requires %d chunks, max is %d", len(chunks), maxCombinedChunks) // The legacy path supports up to 50 token chunks (~100KB), so large
// tokens must not drop the session entirely — persist via the
// legacy (per-token chunk) format instead of erroring and forcing
// the user to re-authenticate (R97). Expire the old combined
// chunks first so GetSession's combined-first read serves the
// fresh legacy tokens rather than the stale combined payload (R163).
sd.expireOldCombinedChunks(r, w, options, 0)
return sd.saveLegacy(r, w, options)
} }
sd.manager.logger.Debugf("Combined session split into %d chunks", len(chunks)) sd.manager.logger.Debugf("Combined session split into %d chunks", len(chunks))
@@ -1573,20 +1661,24 @@ func (sd *SessionData) expireLegacyCookies(r *http.Request, w http.ResponseWrite
expireLegacyMain(sd.manager.refreshTokenCookieName()) expireLegacyMain(sd.manager.refreshTokenCookieName())
expireLegacyMain(sd.manager.idTokenCookieName()) expireLegacyMain(sd.manager.idTokenCookieName())
// Expire legacy chunk cookies (safe to clear values, they're separate from main sessions) // Expire legacy chunk cookies (safe to clear values, they're separate from main sessions).
for i := 0; i < 50; i++ { // Max legacy chunks was 50 // Each token type is walked to its own limit so a small, un-chunked access token
accessChunk := fmt.Sprintf("%s_%d", sd.manager.accessTokenCookieName(), i) // does not gate refresh/id chunk expiry (R133 P2-A): previously the shared
refreshChunk := fmt.Sprintf("%s_%d", sd.manager.refreshTokenCookieName(), i) // access-driven loop broke all three series at the first absent access chunk,
idChunk := fmt.Sprintf("%s_%d", sd.manager.idTokenCookieName(), i) // leaving stale refresh/id legacy chunk cookies behind after combined migration.
expireLegacyChunkSeries := func(baseName string) {
session, err := sd.manager.store.Get(r, accessChunk) for i := 0; i < 50; i++ { // Max legacy chunks was 50
if err != nil || session.IsNew { chunk := fmt.Sprintf("%s_%d", baseName, i)
break // No more chunks session, err := sd.manager.store.Get(r, chunk)
if err != nil || session.IsNew {
break // No more chunks for this token type
}
expireLegacyChunk(chunk)
} }
expireLegacyChunk(accessChunk)
expireLegacyChunk(refreshChunk)
expireLegacyChunk(idChunk)
} }
expireLegacyChunkSeries(sd.manager.accessTokenCookieName())
expireLegacyChunkSeries(sd.manager.refreshTokenCookieName())
expireLegacyChunkSeries(sd.manager.idTokenCookieName())
} }
// clearSessionValues removes all values from a session and optionally expires it. // clearSessionValues removes all values from a session and optionally expires it.
@@ -1667,6 +1759,7 @@ func (sd *SessionData) Clear(r *http.Request, w http.ResponseWriter) error {
// Return a test error without trying to save problematic data // Return a test error without trying to save problematic data
err = fmt.Errorf("test error triggered by X-Test-Error header") err = fmt.Errorf("test error triggered by X-Test-Error header")
} else { } else {
sd.expireCookie = true
err = sd.Save(r, w) err = sd.Save(r, w)
} }
} }
@@ -1973,19 +2066,19 @@ func (sd *SessionData) SetAccessToken(token string) {
return return
} }
if sd.request != nil { // Defer clearing the PREVIOUS access token (its HTTP chunk cookies and the
sd.expireAccessTokenChunksEnhanced(nil) // in-memory chunk map) until the new token's storage is fully validated.
} // Clearing first, then aborting on a too-large new token (>100KB after
// compression, or >50 chunks) would leave the previous still-valid access
for k := range sd.accessTokenChunks { // token nowhere — GetAccessToken returns "" and the user is forced to
delete(sd.accessTokenChunks, k) // re-authenticate (R122). We commit the clear only once we're replacing it.
}
if token == "" { if token == "" {
if sd.accessSession != nil { if sd.accessSession != nil {
sd.accessSession.Values["token"] = "" sd.accessSession.Values["token"] = ""
sd.accessSession.Values["compressed"] = false sd.accessSession.Values["compressed"] = false
} }
sd.clearAccessTokenChunks()
return return
} }
@@ -2014,6 +2107,7 @@ func (sd *SessionData) SetAccessToken(token string) {
} }
if len(compressed) <= maxCookieSize { if len(compressed) <= maxCookieSize {
sd.clearAccessTokenChunks()
if sd.accessSession != nil { if sd.accessSession != nil {
sd.accessSession.Values["token"] = compressed sd.accessSession.Values["token"] = compressed
sd.accessSession.Values["compressed"] = (compressed != token) sd.accessSession.Values["compressed"] = (compressed != token)
@@ -2024,10 +2118,12 @@ func (sd *SessionData) SetAccessToken(token string) {
} }
} }
} else { } else {
if sd.accessSession != nil { // WARNING: do not clear the existing token (accessSession.Values
sd.accessSession.Values["token"] = "" // ["token"] / accessTokenChunks) until the new token's chunks are
sd.accessSession.Values["compressed"] = (compressed != token) // validated below. Clearing first meant an abort on any chunk
} // failure (too many chunks, reassembly mismatch) left NO token
// anywhere -> GetAccessToken returned "" -> forced re-auth (data
// loss of the previous still-valid access token).
chunks := splitIntoChunks(compressed, maxCookieSize) chunks := splitIntoChunks(compressed, maxCookieSize)
@@ -2042,11 +2138,23 @@ func (sd *SessionData) SetAccessToken(token string) {
} }
testReassembled := strings.Join(chunks, "") testReassembled := strings.Join(chunks, "")
// R188 cross-chunk integrity: bind all chunks to one another so a
// mix of authentic-but-different chunk sets is caught at assembly
// (see processChunkedToken). Stored on every chunk.
chunkIntegrity := fmt.Sprintf("%x", sha256.Sum256([]byte(testReassembled)))
if testReassembled != compressed { if testReassembled != compressed {
sd.manager.logger.Debug("Access token chunk reassembly test failed") sd.manager.logger.Debug("Access token chunk reassembly test failed")
return return
} }
// Validation passed: now it is safe to clear the existing token
// before storing the new chunked one.
sd.clearAccessTokenChunks()
if sd.accessSession != nil {
sd.accessSession.Values["token"] = ""
sd.accessSession.Values["compressed"] = (compressed != token)
}
for i, chunkData := range chunks { for i, chunkData := range chunks {
sessionName := fmt.Sprintf("%s_%d", sd.manager.accessTokenCookieName(), i) sessionName := fmt.Sprintf("%s_%d", sd.manager.accessTokenCookieName(), i)
@@ -2080,6 +2188,7 @@ func (sd *SessionData) SetAccessToken(token string) {
session.Values["compressed"] = (compressed != token) session.Values["compressed"] = (compressed != token)
session.Values["chunk_created_at"] = time.Now().Unix() session.Values["chunk_created_at"] = time.Now().Unix()
session.Values["token_total"] = len(chunks) session.Values["token_total"] = len(chunks)
session.Values["chunk_integrity"] = chunkIntegrity
sd.accessTokenChunks[i] = session sd.accessTokenChunks[i] = session
} }
@@ -2087,6 +2196,20 @@ func (sd *SessionData) SetAccessToken(token string) {
} }
} }
// clearAccessTokenChunks expires the current access token's HTTP chunk cookies
// (via expireAccessTokenChunksEnhanced) and clears the in-memory access
// token chunk map. Callers invoke it only once they are committed to
// replacing the token, so a failed (too-large) store leaves the previous
// valid access token intact. Called with sd.sessionMutex already held.
func (sd *SessionData) clearAccessTokenChunks() {
if sd.request != nil {
sd.expireAccessTokenChunksEnhanced(nil)
}
for k := range sd.accessTokenChunks {
delete(sd.accessTokenChunks, k)
}
}
// GetRefreshToken retrieves the user's refresh token from session storage. // GetRefreshToken retrieves the user's refresh token from session storage.
// It handles both single-cookie storage and chunked storage for large tokens, // It handles both single-cookie storage and chunked storage for large tokens,
// with automatic decompression if the token was compressed. // with automatic decompression if the token was compressed.
@@ -2182,15 +2305,14 @@ func (sd *SessionData) SetRefreshToken(token string) {
} }
sd.dirty = true sd.dirty = true
if sd.request != nil {
sd.expireRefreshTokenChunksEnhanced(nil)
}
for k := range sd.refreshTokenChunks {
delete(sd.refreshTokenChunks, k)
}
if token == "" { if token == "" {
// Explicit clear: the caller wants the previous token gone.
if sd.request != nil {
sd.expireRefreshTokenChunksEnhanced(nil)
}
for k := range sd.refreshTokenChunks {
delete(sd.refreshTokenChunks, k)
}
sd.refreshSession.Values["token"] = "" sd.refreshSession.Values["token"] = ""
sd.refreshSession.Values["compressed"] = false sd.refreshSession.Values["compressed"] = false
return return
@@ -2207,70 +2329,98 @@ func (sd *SessionData) SetRefreshToken(token string) {
} }
if len(compressed) <= maxCookieSize { if len(compressed) <= maxCookieSize {
// Single-cookie layout: nothing to validate, safe to swap.
if sd.request != nil {
sd.expireRefreshTokenChunksEnhanced(nil)
}
for k := range sd.refreshTokenChunks {
delete(sd.refreshTokenChunks, k)
}
sd.refreshSession.Values["token"] = compressed sd.refreshSession.Values["token"] = compressed
sd.refreshSession.Values["compressed"] = (compressed != token) sd.refreshSession.Values["compressed"] = (compressed != token)
sd.refreshSession.Values["issued_at"] = time.Now().Unix() sd.refreshSession.Values["issued_at"] = time.Now().Unix()
} else { return
sd.refreshSession.Values["token"] = ""
sd.refreshSession.Values["compressed"] = (compressed != token)
sd.refreshSession.Values["issued_at"] = time.Now().Unix()
chunks := splitIntoChunks(compressed, maxCookieSize)
if len(chunks) == 0 {
sd.manager.logger.Errorf("CRITICAL: Failed to create chunks for refresh token")
return
}
if len(chunks) > 50 {
sd.manager.logger.Errorf("CRITICAL: Too many chunks (%d) for refresh token - possible corruption", len(chunks))
return
}
testReassembled := strings.Join(chunks, "")
if testReassembled != compressed {
sd.manager.logger.Errorf("CRITICAL: Refresh token chunk reassembly test failed")
return
}
for i, chunkData := range chunks {
sessionName := fmt.Sprintf("%s_%d", sd.manager.refreshTokenCookieName(), i)
if sd.request == nil {
sd.manager.logger.Errorf("CRITICAL: SetRefreshToken: sd.request is nil, cannot create chunk session %s", sessionName)
return
}
if chunkData == "" {
sd.manager.logger.Errorf("CRITICAL: Empty refresh token chunk data at index %d", i)
return
}
if len(chunkData) > maxCookieSize {
sd.manager.logger.Errorf("CRITICAL: Refresh token chunk %d size %d exceeds maxCookieSize %d", i, len(chunkData), maxCookieSize)
return
}
if !validateChunkSize(chunkData) {
sd.manager.logger.Errorf("CRITICAL: Refresh token chunk %d will exceed browser cookie limits after encoding (raw size: %d)", i, len(chunkData))
return
}
session, err := sd.manager.store.Get(sd.request, sessionName)
if err != nil {
sd.manager.logger.Errorf("CRITICAL: Failed to get refresh token chunk session %s: %v", sessionName, err)
return
}
session.Values["token_chunk"] = chunkData
session.Values["compressed"] = (compressed != token)
session.Values["chunk_created_at"] = time.Now().Unix()
session.Values["token_total"] = len(chunks)
sd.refreshTokenChunks[i] = session
}
sd.manager.logger.Debugf("SUCCESS: Stored refresh token in %d chunks", len(chunks))
} }
// Chunked layout: validate and write every new chunk BEFORE touching the
// previous token, so an abort (request missing, store error, size check,
// reassembly) leaves the still-valid prior refresh token intact (R154;
// extends the R95 access-token fix to the refresh token).
chunks := splitIntoChunks(compressed, maxCookieSize)
if len(chunks) == 0 {
sd.manager.logger.Errorf("CRITICAL: Failed to create chunks for refresh token")
return
}
if len(chunks) > 50 {
sd.manager.logger.Errorf("CRITICAL: Too many chunks (%d) for refresh token - possible corruption", len(chunks))
return
}
testReassembled := strings.Join(chunks, "")
// R188 cross-chunk integrity: bind all chunks to one another so a mix
// of authentic-but-different chunk sets is caught at assembly (see
// processChunkedToken). Stored on every chunk.
chunkIntegrity := fmt.Sprintf("%x", sha256.Sum256([]byte(testReassembled)))
if testReassembled != compressed {
sd.manager.logger.Errorf("CRITICAL: Refresh token chunk reassembly test failed")
return
}
newChunks := make(map[int]*sessions.Session, len(chunks))
for i, chunkData := range chunks {
sessionName := fmt.Sprintf("%s_%d", sd.manager.refreshTokenCookieName(), i)
if sd.request == nil {
sd.manager.logger.Errorf("CRITICAL: SetRefreshToken: sd.request is nil, cannot create chunk session %s", sessionName)
return
}
if chunkData == "" {
sd.manager.logger.Errorf("CRITICAL: Empty refresh token chunk data at index %d", i)
return
}
if len(chunkData) > maxCookieSize {
sd.manager.logger.Errorf("CRITICAL: Refresh token chunk %d size %d exceeds maxCookieSize %d", i, len(chunkData), maxCookieSize)
return
}
if !validateChunkSize(chunkData) {
sd.manager.logger.Errorf("CRITICAL: Refresh token chunk %d will exceed browser cookie limits after encoding (raw size: %d)", i, len(chunkData))
return
}
session, err := sd.manager.store.Get(sd.request, sessionName)
if err != nil {
sd.manager.logger.Errorf("CRITICAL: Failed to get refresh token chunk session %s: %v", sessionName, err)
return
}
session.Values["token_chunk"] = chunkData
session.Values["compressed"] = (compressed != token)
session.Values["chunk_created_at"] = time.Now().Unix()
session.Values["token_total"] = len(chunks)
session.Values["chunk_integrity"] = chunkIntegrity
newChunks[i] = session
}
// Every new chunk committed: now it is safe to discard the old token.
if sd.request != nil {
sd.expireRefreshTokenChunksEnhanced(nil)
}
for k := range sd.refreshTokenChunks {
delete(sd.refreshTokenChunks, k)
}
sd.refreshSession.Values["token"] = ""
sd.refreshSession.Values["compressed"] = (compressed != token)
sd.refreshSession.Values["issued_at"] = time.Now().Unix()
for i, s := range newChunks {
sd.refreshTokenChunks[i] = s
}
sd.manager.logger.Debugf("SUCCESS: Stored refresh token in %d chunks", len(chunks))
} }
// GetRefreshTokenIssuedAt retrieves the timestamp when the refresh token was issued/stored. // GetRefreshTokenIssuedAt retrieves the timestamp when the refresh token was issued/stored.
@@ -2517,6 +2667,8 @@ func isCorruptionMarker(data string) bool {
// Returns: // Returns:
// - The CSRF token string, or an empty string if not set. // - The CSRF token string, or an empty string if not set.
func (sd *SessionData) GetCSRF() string { func (sd *SessionData) GetCSRF() string {
sd.sessionMutex.RLock()
defer sd.sessionMutex.RUnlock()
csrf, _ := sd.mainSession.Values["csrf"].(string) csrf, _ := sd.mainSession.Values["csrf"].(string)
return csrf return csrf
} }
@@ -2526,6 +2678,8 @@ func (sd *SessionData) GetCSRF() string {
// Parameters: // Parameters:
// - token: The CSRF token to store. // - token: The CSRF token to store.
func (sd *SessionData) SetCSRF(token string) { func (sd *SessionData) SetCSRF(token string) {
sd.sessionMutex.Lock()
defer sd.sessionMutex.Unlock()
currentVal, _ := sd.mainSession.Values["csrf"].(string) currentVal, _ := sd.mainSession.Values["csrf"].(string)
if currentVal != token { if currentVal != token {
sd.mainSession.Values["csrf"] = token sd.mainSession.Values["csrf"] = token
@@ -2539,6 +2693,8 @@ func (sd *SessionData) SetCSRF(token string) {
// Returns: // Returns:
// - The nonce string, or an empty string if not set. // - The nonce string, or an empty string if not set.
func (sd *SessionData) GetNonce() string { func (sd *SessionData) GetNonce() string {
sd.sessionMutex.RLock()
defer sd.sessionMutex.RUnlock()
nonce, _ := sd.mainSession.Values["nonce"].(string) nonce, _ := sd.mainSession.Values["nonce"].(string)
return nonce return nonce
} }
@@ -2548,6 +2704,8 @@ func (sd *SessionData) GetNonce() string {
// Parameters: // Parameters:
// - nonce: The nonce string to store. // - nonce: The nonce string to store.
func (sd *SessionData) SetNonce(nonce string) { func (sd *SessionData) SetNonce(nonce string) {
sd.sessionMutex.Lock()
defer sd.sessionMutex.Unlock()
currentVal, _ := sd.mainSession.Values["nonce"].(string) currentVal, _ := sd.mainSession.Values["nonce"].(string)
if currentVal != nonce { if currentVal != nonce {
sd.mainSession.Values["nonce"] = nonce sd.mainSession.Values["nonce"] = nonce
@@ -2561,6 +2719,8 @@ func (sd *SessionData) SetNonce(nonce string) {
// Returns: // Returns:
// - The code verifier string, or an empty string if not set or PKCE is disabled. // - The code verifier string, or an empty string if not set or PKCE is disabled.
func (sd *SessionData) GetCodeVerifier() string { func (sd *SessionData) GetCodeVerifier() string {
sd.sessionMutex.RLock()
defer sd.sessionMutex.RUnlock()
codeVerifier, _ := sd.mainSession.Values["code_verifier"].(string) codeVerifier, _ := sd.mainSession.Values["code_verifier"].(string)
return codeVerifier return codeVerifier
} }
@@ -2571,6 +2731,8 @@ func (sd *SessionData) GetCodeVerifier() string {
// Parameters: // Parameters:
// - codeVerifier: The PKCE code verifier string to store. // - codeVerifier: The PKCE code verifier string to store.
func (sd *SessionData) SetCodeVerifier(codeVerifier string) { func (sd *SessionData) SetCodeVerifier(codeVerifier string) {
sd.sessionMutex.Lock()
defer sd.sessionMutex.Unlock()
currentVal, _ := sd.mainSession.Values["code_verifier"].(string) currentVal, _ := sd.mainSession.Values["code_verifier"].(string)
if currentVal != codeVerifier { if currentVal != codeVerifier {
sd.mainSession.Values["code_verifier"] = codeVerifier sd.mainSession.Values["code_verifier"] = codeVerifier
@@ -2612,6 +2774,8 @@ func (sd *SessionData) SetUserIdentifier(userIdentifier string) {
// Returns: // Returns:
// - The original request URI string, or an empty string if not set. // - The original request URI string, or an empty string if not set.
func (sd *SessionData) GetIncomingPath() string { func (sd *SessionData) GetIncomingPath() string {
sd.sessionMutex.RLock()
defer sd.sessionMutex.RUnlock()
path, _ := sd.mainSession.Values["incoming_path"].(string) path, _ := sd.mainSession.Values["incoming_path"].(string)
return path return path
} }
@@ -2622,6 +2786,8 @@ func (sd *SessionData) GetIncomingPath() string {
// Parameters: // Parameters:
// - path: The original request URI string (e.g., "/protected/resource?id=123"). // - path: The original request URI string (e.g., "/protected/resource?id=123").
func (sd *SessionData) SetIncomingPath(path string) { func (sd *SessionData) SetIncomingPath(path string) {
sd.sessionMutex.Lock()
defer sd.sessionMutex.Unlock()
currentVal, _ := sd.mainSession.Values["incoming_path"].(string) currentVal, _ := sd.mainSession.Values["incoming_path"].(string)
if currentVal != path { if currentVal != path {
sd.mainSession.Values["incoming_path"] = path sd.mainSession.Values["incoming_path"] = path
@@ -2629,6 +2795,35 @@ func (sd *SessionData) SetIncomingPath(path string) {
} }
} }
// GetRedirectURL retrieves the fully-qualified redirect_uri persisted when the
// authentication flow was initiated. It is consumed at the callback (in place
// of a per-request rebuild) so a drift in X-Forwarded-Host between the
// initiate and callback requests cannot change the redirect_uri sent to the
// provider (R136).
func (sd *SessionData) GetRedirectURL() string {
sd.sessionMutex.RLock()
defer sd.sessionMutex.RUnlock()
ru, _ := sd.mainSession.Values["redirect_url"].(string)
return ru
}
// SetRedirectURL stores the redirect_uri used at initiate time (R136).
func (sd *SessionData) SetRedirectURL(redirectURL string) {
sd.sessionMutex.Lock()
defer sd.sessionMutex.Unlock()
currentVal, _ := sd.mainSession.Values["redirect_url"].(string)
if currentVal != redirectURL {
sd.mainSession.Values["redirect_url"] = redirectURL
sd.dirty = true
}
}
// getRedirectURLUnsafe retrieves the redirect URL without acquiring locks.
func (sd *SessionData) getRedirectURLUnsafe() string {
ru, _ := sd.mainSession.Values["redirect_url"].(string)
return ru
}
// GetIDToken retrieves the user's ID token from session storage. // GetIDToken retrieves the user's ID token from session storage.
// The ID token contains user claims and is used for user identification // The ID token contains user claims and is used for user identification
// and authorization decisions. Handles compression and chunking automatically. // and authorization decisions. Handles compression and chunking automatically.
@@ -2804,14 +2999,6 @@ func (sd *SessionData) SetIDToken(token string) {
} }
sd.dirty = true sd.dirty = true
if sd.request != nil {
sd.expireIDTokenChunksEnhanced(nil)
}
for k := range sd.idTokenChunks {
delete(sd.idTokenChunks, k)
}
if token == "" { if token == "" {
if sd.idTokenSession != nil { if sd.idTokenSession != nil {
sd.idTokenSession.Values["token"] = "" sd.idTokenSession.Values["token"] = ""
@@ -2831,72 +3018,100 @@ func (sd *SessionData) SetIDToken(token string) {
} }
if len(compressed) <= maxCookieSize { if len(compressed) <= maxCookieSize {
// Single-cookie layout: nothing to validate, safe to swap.
if sd.request != nil {
sd.expireIDTokenChunksEnhanced(nil)
}
for k := range sd.idTokenChunks {
delete(sd.idTokenChunks, k)
}
if sd.idTokenSession != nil { if sd.idTokenSession != nil {
sd.idTokenSession.Values["token"] = compressed sd.idTokenSession.Values["token"] = compressed
sd.idTokenSession.Values["compressed"] = (compressed != token) sd.idTokenSession.Values["compressed"] = (compressed != token)
} }
} else { return
if sd.idTokenSession != nil {
sd.idTokenSession.Values["token"] = ""
sd.idTokenSession.Values["compressed"] = (compressed != token)
}
chunks := splitIntoChunks(compressed, maxCookieSize)
if len(chunks) == 0 {
sd.manager.logger.Errorf("CRITICAL: Failed to create chunks for ID token")
return
}
if len(chunks) > 50 {
sd.manager.logger.Errorf("CRITICAL: Too many chunks (%d) for ID token - possible corruption", len(chunks))
return
}
testReassembled := strings.Join(chunks, "")
if testReassembled != compressed {
sd.manager.logger.Errorf("CRITICAL: ID token chunk reassembly test failed")
return
}
for i, chunkData := range chunks {
sessionName := fmt.Sprintf("%s_%d", sd.manager.idTokenCookieName(), i)
if sd.request == nil {
sd.manager.logger.Errorf("CRITICAL: SetIDToken: sd.request is nil, cannot create chunk session %s", sessionName)
return
}
if chunkData == "" {
sd.manager.logger.Debug("Empty chunk data at index %d", i)
return
}
if len(chunkData) > maxCookieSize {
sd.manager.logger.Info("Chunk %d size %d exceeds maxCookieSize %d", i, len(chunkData), maxCookieSize)
return
}
if !validateChunkSize(chunkData) {
sd.manager.logger.Errorf("CRITICAL: ID token chunk %d will exceed browser cookie limits after encoding (raw size: %d)", i, len(chunkData))
return
}
session, err := sd.manager.store.Get(sd.request, sessionName)
if err != nil {
sd.manager.logger.Errorf("CRITICAL: Failed to get chunk session %s: %v", sessionName, err)
return
}
session.Values["token_chunk"] = chunkData
session.Values["compressed"] = (compressed != token)
session.Values["chunk_created_at"] = time.Now().Unix()
session.Values["token_total"] = len(chunks)
sd.idTokenChunks[i] = session
}
sd.manager.logger.Debugf("SUCCESS: Stored ID token in %d chunks", len(chunks))
} }
// Chunked layout: validate and write every new chunk BEFORE touching
// the previous token, so an abort (request missing, store error, size
// check) leaves the still-valid prior ID token intact (R154; extends
// the R95 access-token fix to the ID token).
chunks := splitIntoChunks(compressed, maxCookieSize)
if len(chunks) == 0 {
sd.manager.logger.Errorf("CRITICAL: Failed to create chunks for ID token")
return
}
if len(chunks) > 50 {
sd.manager.logger.Errorf("CRITICAL: Too many chunks (%d) for ID token - possible corruption", len(chunks))
return
}
testReassembled := strings.Join(chunks, "")
// R188 cross-chunk integrity: bind all chunks to one another so a mix
// of authentic-but-different chunk sets is caught at assembly (see
// processChunkedToken). Stored on every chunk.
chunkIntegrity := fmt.Sprintf("%x", sha256.Sum256([]byte(testReassembled)))
if testReassembled != compressed {
sd.manager.logger.Errorf("CRITICAL: ID token chunk reassembly test failed")
return
}
newChunks := make(map[int]*sessions.Session, len(chunks))
for i, chunkData := range chunks {
sessionName := fmt.Sprintf("%s_%d", sd.manager.idTokenCookieName(), i)
if sd.request == nil {
sd.manager.logger.Errorf("CRITICAL: SetIDToken: sd.request is nil, cannot create chunk session %s", sessionName)
return
}
if chunkData == "" {
sd.manager.logger.Debug("Empty chunk data at index %d", i)
return
}
if len(chunkData) > maxCookieSize {
sd.manager.logger.Info("Chunk %d size %d exceeds maxCookieSize %d", i, len(chunkData), maxCookieSize)
return
}
if !validateChunkSize(chunkData) {
sd.manager.logger.Errorf("CRITICAL: ID token chunk %d will exceed browser cookie limits after encoding (raw size: %d)", i, len(chunkData))
return
}
session, err := sd.manager.store.Get(sd.request, sessionName)
if err != nil {
sd.manager.logger.Errorf("CRITICAL: Failed to get chunk session %s: %v", sessionName, err)
return
}
session.Values["token_chunk"] = chunkData
session.Values["compressed"] = (compressed != token)
session.Values["chunk_created_at"] = time.Now().Unix()
session.Values["token_total"] = len(chunks)
session.Values["chunk_integrity"] = chunkIntegrity
newChunks[i] = session
}
// Every new chunk committed: now it is safe to discard the old token.
if sd.request != nil {
sd.expireIDTokenChunksEnhanced(nil)
}
for k := range sd.idTokenChunks {
delete(sd.idTokenChunks, k)
}
if sd.idTokenSession != nil {
sd.idTokenSession.Values["token"] = ""
sd.idTokenSession.Values["compressed"] = (compressed != token)
}
for i, s := range newChunks {
sd.idTokenChunks[i] = s
}
sd.manager.logger.Debugf("SUCCESS: Stored ID token in %d chunks", len(chunks))
} }
// GetRedirectCount returns the number of redirects in the current authentication flow. // GetRedirectCount returns the number of redirects in the current authentication flow.
+65 -37
View File
@@ -3,6 +3,7 @@ package traefikoidc
import ( import (
"bytes" "bytes"
"context" "context"
"crypto/sha256"
"encoding/base64" "encoding/base64"
"fmt" "fmt"
"strings" "strings"
@@ -41,10 +42,14 @@ func ResetGlobalSessionCounters() {
// Predefined configurations for each token type // Predefined configurations for each token type
var ( var (
AccessTokenConfig = TokenConfig{ AccessTokenConfig = TokenConfig{
Type: "access", Type: "access",
MinLength: 5, MinLength: 5,
MaxLength: 100 * 1024, MaxLength: 100 * 1024,
MaxChunks: 25, // Must be >= the write path's hard chunk cap (session.go Set*Token
// allows up to 50 chunks) so a token that was written can always be
// read back. A lower read limit silently made written tokens
// unreadable (data loss / forced re-auth) — see R95.
MaxChunks: 50,
MaxChunkSize: maxCookieSize, MaxChunkSize: maxCookieSize,
AllowOpaqueTokens: true, AllowOpaqueTokens: true,
RequireJWTFormat: false, RequireJWTFormat: false,
@@ -54,7 +59,7 @@ var (
Type: "refresh", Type: "refresh",
MinLength: 5, MinLength: 5,
MaxLength: 50 * 1024, MaxLength: 50 * 1024,
MaxChunks: 15, MaxChunks: 50, // R95: match write-path cap (see AccessTokenConfig)
MaxChunkSize: maxCookieSize, MaxChunkSize: maxCookieSize,
AllowOpaqueTokens: true, AllowOpaqueTokens: true,
RequireJWTFormat: false, RequireJWTFormat: false,
@@ -64,7 +69,7 @@ var (
Type: "id", Type: "id",
MinLength: 5, MinLength: 5,
MaxLength: 75 * 1024, MaxLength: 75 * 1024,
MaxChunks: 20, MaxChunks: 50, // R95: match write-path cap (see AccessTokenConfig)
MaxChunkSize: maxCookieSize, MaxChunkSize: maxCookieSize,
AllowOpaqueTokens: false, AllowOpaqueTokens: false,
RequireJWTFormat: true, RequireJWTFormat: true,
@@ -292,9 +297,7 @@ func (cm *ChunkManager) validateToken(token string, config TokenConfig) TokenRet
return TokenRetrievalResult{Token: "", Error: sizeErr} return TokenRetrievalResult{Token: "", Error: sizeErr}
} }
if chunkErr := cm.validateChunkingEfficiency(token, config); chunkErr != nil { cm.validateChunkingEfficiency(token, config)
return TokenRetrievalResult{Token: "", Error: chunkErr}
}
if contentErr := cm.validateTokenContent(token, config); contentErr != nil { if contentErr := cm.validateTokenContent(token, config); contentErr != nil {
return TokenRetrievalResult{Token: "", Error: contentErr} return TokenRetrievalResult{Token: "", Error: contentErr}
@@ -365,12 +368,27 @@ func (cm *ChunkManager) processChunkedToken(chunks map[int]*sessions.Session, co
// cookie eviction, dropped Set-Cookie, attacker dropping a cookie) would // cookie eviction, dropped Set-Cookie, attacker dropping a cookie) would
// reassemble a truncated prefix with no error and trust it as the token. // reassemble a truncated prefix with no error and trust it as the token.
// The write path stores token_total on every chunk; check it on chunk 0. // The write path stores token_total on every chunk; check it on chunk 0.
if total, ok := chunks[0].Values["token_total"].(int); ok { // Guard against a sparse set missing index 0 (the assembly loop below
if len(chunks) != total { // reports the missing chunk; don't nil-deref here) (R148).
err := fmt.Errorf("%s token chunk count mismatch: got %d, expected %d (possibly truncated)", if first, has0 := chunks[0]; has0 && first != nil {
config.Type, len(chunks), total) if total, ok := first.Values["token_total"].(int); ok {
cm.logger.Errorf("Truncated %s token chunks: got %d, expected %d", config.Type, len(chunks), total) if len(chunks) > total {
return TokenRetrievalResult{Token: "", Error: err} // Stale higher-index chunks left behind when a token was
// rewritten with fewer chunks (Set*Token writes 0..n-1 but does
// not clear cookies above n). token_total is the authoritative
// count; drop the excess rather than failing — which forced a
// re-auth and discarded the still-valid new token (R129).
n := len(chunks)
for i := total; i < n; i++ {
delete(chunks, i)
}
}
if len(chunks) < total {
err := fmt.Errorf("%s token chunk count mismatch: got %d, expected %d (possibly truncated)",
config.Type, len(chunks), total)
cm.logger.Errorf("Truncated %s token chunks: got %d, expected %d", config.Type, len(chunks), total)
return TokenRetrievalResult{Token: "", Error: err}
}
} }
} }
@@ -423,6 +441,27 @@ func (cm *ChunkManager) processChunkedToken(chunks map[int]*sessions.Session, co
reassembledToken := strings.Join(tokenParts, "") reassembledToken := strings.Join(tokenParts, "")
// Cross-chunk integrity: chunk 0 (and every chunk, on new writes)
// carries a SHA-256 of the assembled chunk string. Each chunk is
// already individually authenticated by its own securecookie MAC, so
// forgery is impossible; this binds the chunks to ONE another, so an
// authentic-but-mixed set (a stale chunk from an earlier session
// combined with chunks from the current one, when counts still match)
// is detected here instead of assembling a wrong token that would only
// surface as a downstream JWT-validation failure. Absent (old
// sessions written before this) → verification is skipped, keeping
// backward compatibility (R188).
if first, has0 := chunks[0]; has0 && first != nil {
if want, ok := first.Values["chunk_integrity"].(string); ok && want != "" {
got := fmt.Sprintf("%x", sha256.Sum256([]byte(reassembledToken)))
if got != want {
err := fmt.Errorf("%s token chunk integrity mismatch: chunks appear to belong to different sessions", config.Type)
cm.logger.Error("Token chunk integrity mismatch for %s: chunks belong to different sessions", config.Type)
return TokenRetrievalResult{Token: "", Error: err}
}
}
}
compressed, _ := chunks[0].Values["compressed"].(bool) compressed, _ := chunks[0].Values["compressed"].(bool)
if compressed { if compressed {
@@ -601,7 +640,7 @@ func (cm *ChunkManager) validateTokenSize(token string, config TokenConfig) erro
} }
if config.AllowOpaqueTokens && !strings.Contains(token, ".") { if config.AllowOpaqueTokens && !strings.Contains(token, ".") {
if tokenLen > 8*1024 { if tokenLen > config.MaxLength {
err := fmt.Errorf("%s opaque token unusually large (%d bytes)", config.Type, tokenLen) err := fmt.Errorf("%s opaque token unusually large (%d bytes)", config.Type, tokenLen)
return err return err
} }
@@ -610,34 +649,23 @@ func (cm *ChunkManager) validateTokenSize(token string, config TokenConfig) erro
return nil return nil
} }
// validateChunkingEfficiency ensures that chunking is used appropriately. // validateChunkingEfficiency warns about potential inefficiencies in token
// It calculates expected chunk counts and warns about potential inefficiencies // storage strategies based on the token's length. It is advisory only: the
// in token storage strategies. // actual chunk budget is enforced at write time on the stored (compressed)
// Parameters: // representation (session.go), and the read path size-bounds tokens via
// - token: The token to analyze for chunking efficiency. // validateTokenSize. It must NOT hard-reject on the read path, which sees
// - config: Token configuration with chunking limits. // the *decompressed* token: a large access token legitimately stored
// // compressed (e.g. decompressing to > MaxChunks*MaxChunkSize bytes) would
// Returns: // otherwise be permanently rejected on every read, forcing re-auth while
// - An error if chunking requirements would be violated, nil if acceptable. // the valid token sits in the browser (R104).
func (cm *ChunkManager) validateChunkingEfficiency(token string, config TokenConfig) error { func (cm *ChunkManager) validateChunkingEfficiency(token string, config TokenConfig) {
tokenLen := len(token) tokenLen := len(token)
if tokenLen <= config.MaxChunkSize && tokenLen <= maxCookieSize {
}
expectedChunks := (tokenLen + config.MaxChunkSize - 1) / config.MaxChunkSize expectedChunks := (tokenLen + config.MaxChunkSize - 1) / config.MaxChunkSize
if expectedChunks > config.MaxChunks {
err := fmt.Errorf("%s token would require %d chunks (max: %d)",
config.Type, expectedChunks, config.MaxChunks)
return err
}
if expectedChunks > 10 && tokenLen < 50*1024 { if expectedChunks > 10 && tokenLen < 50*1024 {
cm.logger.Info("%s token requires many chunks (%d) for size (%d bytes) - consider token optimization", cm.logger.Info("%s token requires many chunks (%d) for size (%d bytes) - consider token optimization",
config.Type, expectedChunks, tokenLen) config.Type, expectedChunks, tokenLen)
} }
return nil
} }
// validateTokenContent performs comprehensive token content validation. // validateTokenContent performs comprehensive token content validation.
+48 -10
View File
@@ -79,8 +79,16 @@ func (t *TraefikOidc) introspectToken(token string) (*IntrospectionResponse, err
req.Header.Set("Accept", "application/json") req.Header.Set("Accept", "application/json")
// Authenticate using client credentials (per RFC 7662 Section 2.1) // Authenticate using client credentials (per RFC 7662 Section 2.1)
// The introspection endpoint requires authentication // The introspection endpoint requires authentication.
req.SetBasicAuth(t.clientID, t.clientSecret) // Snapshot clientID/clientSecret under metadataMu: DCR rewrites them at
// runtime (R137).
clientID, clientSecret, _, _, _ := t.clientCredentials()
// RFC 7662 §2.1 authenticates the introspection call exactly like the
// token and revocation endpoints: form-urlencode client_id and
// client_secret individually before base64 (RFC 6749 §2.3.1), so the
// three outbound auth paths send identical wire credentials for secrets
// containing reserved characters (see setOAuthBasicAuth).
setOAuthBasicAuth(req, clientID, clientSecret)
// Send request with circuit breaker if available // Send request with circuit breaker if available
var resp *http.Response var resp *http.Response
@@ -114,11 +122,19 @@ func (t *TraefikOidc) introspectToken(token string) (*IntrospectionResponse, err
} }
}() }()
// Check HTTP status // Check HTTP status. Return a typed *HTTPError so callers can
// distinguish a definite client-side rejection (4xx: the presented
// token is unknown/revoked -> treat like active=false) from a
// transient provider failure (5xx / other), instead of substring-
// matching the message and misclassifying a revoked token as a
// transient error that falls through to ID-token-only auth (R156).
if resp.StatusCode != http.StatusOK { if resp.StatusCode != http.StatusOK {
limitReader := io.LimitReader(resp.Body, 1024*10) limitReader := io.LimitReader(resp.Body, 1024*10)
body, _ := io.ReadAll(limitReader) // Safe to ignore: reading error body for diagnostics body, _ := io.ReadAll(limitReader) // Safe to ignore: reading error body for diagnostics
return nil, fmt.Errorf("introspection endpoint returned status %d: %s", resp.StatusCode, string(body)) return nil, &HTTPError{
StatusCode: resp.StatusCode,
Message: fmt.Sprintf("introspection endpoint returned status %d: %s", resp.StatusCode, string(body)),
}
} }
// Parse response per RFC 7662 Section 2.2 // Parse response per RFC 7662 Section 2.2
@@ -127,8 +143,14 @@ func (t *TraefikOidc) introspectToken(token string) (*IntrospectionResponse, err
return nil, fmt.Errorf("failed to decode introspection response: %w", err) return nil, fmt.Errorf("failed to decode introspection response: %w", err)
} }
// Cache the result // Cache the result. Only positive (active) results are cached so a
if t.introspectionCache != nil { // revoked token's negative entry cannot poison the cache for its full TTL
// Cache the result. Only positive (active) results are cached so a
// revoked token's negative entry cannot poison the cache for its full TTL
// and keep rejecting a re-issued token with the same string (R104).
// Inactive results are re-introspected on each request, letting a
// re-issued valid token pass immediately.
if t.introspectionCache != nil && introspectionResp.Active {
// Cache for a short duration or until token expiry (whichever is shorter) // Cache for a short duration or until token expiry (whichever is shorter)
cacheDuration := 5 * time.Minute cacheDuration := 5 * time.Minute
// When introspection is REQUIRED, operators expect near-real-time // When introspection is REQUIRED, operators expect near-real-time
@@ -160,6 +182,9 @@ func (t *TraefikOidc) introspectToken(token string) (*IntrospectionResponse, err
// Returns: // Returns:
// - error: Validation error if token is invalid, nil if valid // - error: Validation error if token is invalid, nil if valid
func (t *TraefikOidc) validateOpaqueToken(token string) error { func (t *TraefikOidc) validateOpaqueToken(token string) error {
// Snapshot clientID/audience under metadataMu: DCR rewrites them at
// runtime (R137).
clientID, _, _, audience, _ := t.clientCredentials()
// Check if opaque tokens are allowed // Check if opaque tokens are allowed
if !t.allowOpaqueTokens { if !t.allowOpaqueTokens {
return fmt.Errorf("opaque tokens are not enabled (set allowOpaqueTokens to true)") return fmt.Errorf("opaque tokens are not enabled (set allowOpaqueTokens to true)")
@@ -190,6 +215,19 @@ func (t *TraefikOidc) validateOpaqueToken(token string) error {
return fmt.Errorf("token is not active (revoked or expired)") return fmt.Errorf("token is not active (revoked or expired)")
} }
// An opaque token has no JWT header to classify it, so token_type is
// the only discriminator between an access token and a refresh token.
// An opaque refresh token introspected (or otherwise) as
// active=true, token_type=refresh_token must not be honored as a
// bearer access token (R149). RFC 7662's token_type is the RFC 6749
// token type, whose value for an access token is "Bearer" (RFC 6750)
// — accept both spellings providers use (R156). Per RFC 7662
// token_type may be omitted by compliant providers, so only reject on
// a definite non-access match.
if resp.TokenType != "" && resp.TokenType != "access_token" && resp.TokenType != "Bearer" {
return fmt.Errorf("token type %q is not a bearer access token", resp.TokenType)
}
// Validate expiration if present // Validate expiration if present
if resp.Exp > 0 { if resp.Exp > 0 {
expTime := time.Unix(resp.Exp, 0) expTime := time.Unix(resp.Exp, 0)
@@ -212,12 +250,12 @@ func (t *TraefikOidc) validateOpaqueToken(token string) error {
// whose audience cannot be confirmed must not be accepted, otherwise a // whose audience cannot be confirmed must not be accepted, otherwise a
// token minted for a different audience would pass. aud may be a single // token minted for a different audience would pass. aud may be a single
// string or an array of strings (RFC 7662); verifyAudience handles both. // string or an array of strings (RFC 7662); verifyAudience handles both.
if t.audience != "" && t.audience != t.clientID { if audience != "" && audience != clientID {
if resp.Aud == nil { if resp.Aud == nil {
return fmt.Errorf("invalid audience: expected %s, introspection response has no audience", t.audience) return fmt.Errorf("invalid audience: expected %s, introspection response has no audience", audience)
} }
if err := verifyAudience(resp.Aud, t.audience); err != nil { if err := verifyAudience(resp.Aud, audience); err != nil {
return fmt.Errorf("invalid audience: expected %s: %w", t.audience, err) return fmt.Errorf("invalid audience: expected %s: %w", audience, err)
} }
} }
+309 -68
View File
@@ -7,6 +7,8 @@ import (
"context" "context"
"crypto/sha256" "crypto/sha256"
"encoding/hex" "encoding/hex"
"encoding/json"
"errors"
"fmt" "fmt"
"io" "io"
"net/http" "net/http"
@@ -16,6 +18,12 @@ import (
"time" "time"
) )
// ErrRateLimitExceeded is returned by verifyTokenWithOpts when the shared
// per-instance token-origin rate limiter is exhausted. Callers (e.g.
// handleCallback) surface it as HTTP 429 + Retry-After rather than a
// generic 500, so burst-driven throttling is not reported as a server error.
var ErrRateLimitExceeded = errors.New("request rate limit exceeded")
// VerifyToken verifies the validity of an ID token or access token. // VerifyToken verifies the validity of an ID token or access token.
// It performs comprehensive validation including format checks, blacklist verification, // It performs comprehensive validation including format checks, blacklist verification,
// signature validation using JWKs, and standard claims validation. It also caches // signature validation using JWKs, and standard claims validation. It also caches
@@ -32,6 +40,17 @@ func (t *TraefikOidc) VerifyToken(token string) error {
return t.verifyTokenWithOpts(token, verifyOpts{}) return t.verifyTokenWithOpts(token, verifyOpts{})
} }
// clientCredentials returns a consistent snapshot of the client credential
// fields under metadataMu. DCR (performDynamicClientRegistration, main.go)
// can rewrite clientID/clientSecret/audience at runtime under metadataMu.Lock,
// so every request-path reader must snapshot these fields under RLock rather
// than read them directly (which would be a data race) (R137).
func (t *TraefikOidc) clientCredentials() (clientID, clientSecret, clientAuthMethod, audience string, clientAssertion *ClientAssertionSigner) {
t.metadataMu.RLock()
defer t.metadataMu.RUnlock()
return t.clientID, t.clientSecret, t.clientAuthMethod, t.audience, t.clientAssertion
}
// verifyOpts are internal-only knobs for verifyTokenWithOpts. Kept unexported // verifyOpts are internal-only knobs for verifyTokenWithOpts. Kept unexported
// because they expose subtle replay-protection semantics that are dangerous // because they expose subtle replay-protection semantics that are dangerous
// to misuse. // to misuse.
@@ -77,6 +96,19 @@ func (t *TraefikOidc) verifyTokenWithOpts(token string, opts verifyOpts) error {
// dozens of times per second by validateStandardTokens. // dozens of times per second by validateStandardTokens.
if t.tokenCache != nil { if t.tokenCache != nil {
if claims, exists := t.tokenCache.Get(token); exists && len(claims) > 0 { if claims, exists := t.tokenCache.Get(token); exists && len(claims) > 0 {
// A cached hit normally short-circuits the parse, but it must
// still reject a token whose JTI has been blacklisted: a
// rotated token that shares this cached string's JTI (IdP JTI
// reuse) but was revoked under a different raw value would
// otherwise keep returning "valid" until the cache TTL, while
// every cold presentation of it is rejected (R148).
if t.tokenBlacklist != nil && !t.disableReplayDetection {
if jti, ok := claims["jti"].(string); ok && jti != "" {
if b, exists := t.tokenBlacklist.Get(jti); exists && b != nil {
return fmt.Errorf("token replay detected (jti: %s) in cache", jti)
}
}
}
return nil return nil
} }
} }
@@ -87,8 +119,10 @@ func (t *TraefikOidc) verifyTokenWithOpts(token string, opts verifyOpts) error {
} }
tokenType := "UNKNOWN" tokenType := "UNKNOWN"
clientID := ""
clientID, _, _, _, _ = t.clientCredentials()
if aud, ok := parsedJWT.Claims["aud"]; ok { if aud, ok := parsedJWT.Claims["aud"]; ok {
if audStr, ok := aud.(string); ok && audStr == t.clientID { if audStr, ok := aud.(string); ok && audStr == clientID {
tokenType = "ID_TOKEN" tokenType = "ID_TOKEN"
} }
} }
@@ -105,18 +139,16 @@ func (t *TraefikOidc) verifyTokenWithOpts(token string, opts verifyOpts) error {
if jti, ok := parsedJWT.Claims["jti"].(string); ok && jti != "" { if jti, ok := parsedJWT.Claims["jti"].(string); ok && jti != "" {
// Skip JTI blacklist check if replay detection is disabled // Skip JTI blacklist check if replay detection is disabled
if !t.disableReplayDetection { if !t.disableReplayDetection {
if !strings.HasPrefix(token, "eyJhbGciOiJSUzI1NiIsImtpZCI6InRlc3Qta2V5LWlkIiwidHlwIjoiSldUIn0") { if t.tokenBlacklist != nil {
if t.tokenBlacklist != nil { if blacklisted, exists := t.tokenBlacklist.Get(jti); exists && blacklisted != nil {
if blacklisted, exists := t.tokenBlacklist.Get(jti); exists && blacklisted != nil { return fmt.Errorf("token replay detected (jti: %s) in cache", jti)
return fmt.Errorf("token replay detected (jti: %s) in cache", jti)
}
} }
} }
} }
} }
if !t.limiter.Allow() { if !t.limiter.Allow() {
return fmt.Errorf("rate limit exceeded") return fmt.Errorf("%w", ErrRateLimitExceeded)
} }
jwt := parsedJWT jwt := parsedJWT
@@ -142,7 +174,10 @@ func (t *TraefikOidc) verifyTokenWithOpts(token string, opts verifyOpts) error {
expTime := time.Unix(int64(expClaim), 0) expTime := time.Unix(int64(expClaim), 0)
tokenDuration := time.Until(expTime) tokenDuration := time.Until(expTime)
if tokenDuration > 0 && tokenDuration < defaultBlacklistDuration { if tokenDuration > 0 && tokenDuration < defaultBlacklistDuration {
expiry = expTime // Extend by ClockSkewToleranceFuture so the replay entry
// covers the full acceptance window (exp + skew), closing
// the post-exp replay window (mirrors jwt.go, R179).
expiry = expTime.Add(ClockSkewToleranceFuture)
} }
// else: keep default expiry for expired tokens or tokens >24h // else: keep default expiry for expired tokens or tokens >24h
} }
@@ -162,13 +197,21 @@ func (t *TraefikOidc) verifyTokenWithOpts(token string, opts verifyOpts) error {
initReplayCache() initReplayCache()
duration := time.Until(expiry) duration := time.Until(expiry)
if duration > 0 { if duration > 0 {
if shardedReplayCache != nil { // Guard the singleton read with replayCacheMu because
shardedReplayCache.Set(jti, true, duration) // cleanupReplayCache (jwt.go) can nil shardedReplayCache under
// the write lock. Mirror the locked read in jwt.go
// SetIfAbsent so the pointer deref below cannot race a nil-out.
replayCacheMu.RLock()
sc := shardedReplayCache
if sc != nil {
sc.Set(replayCacheKey(t.issuerURL, jti), true, duration)
replayCacheMu.RUnlock()
} else { } else {
replayCacheMu.RUnlock()
// Fall back to legacy cache (should rarely happen) // Fall back to legacy cache (should rarely happen)
replayCacheMu.Lock() replayCacheMu.Lock()
if replayCache != nil { if replayCache != nil {
replayCache.Set(jti, true, duration) replayCache.Set(replayCacheKey(t.issuerURL, jti), true, duration)
} }
replayCacheMu.Unlock() replayCacheMu.Unlock()
} }
@@ -195,6 +238,21 @@ func (t *TraefikOidc) verifyToken(token string) error {
// - token: The verified token string to cache. // - token: The verified token string to cache.
// - claims: The map of claims extracted from the verified token. // - claims: The map of claims extracted from the verified token.
func (t *TraefikOidc) cacheVerifiedToken(token string, claims map[string]interface{}) { func (t *TraefikOidc) cacheVerifiedToken(token string, claims map[string]interface{}) {
// Defense-in-depth (F1/R133): a concurrent RevokeToken may have
// blacklisted this token (raw or jti) after we passed the top-of-
// verify blacklist check but before we cache. Do not re-populate
// the verified-cache with a revoked token.
if t.tokenBlacklist != nil {
if b, ok := t.tokenBlacklist.Get(token); ok && b != nil {
return
}
if jti, okv := claims["jti"].(string); okv && jti != "" {
if b, ok := t.tokenBlacklist.Get(jti); ok && b != nil {
return
}
}
}
expClaim, ok := claims["exp"].(float64) expClaim, ok := claims["exp"].(float64)
if !ok { if !ok {
t.safeLogError("Failed to cache token: invalid 'exp' claim type") t.safeLogError("Failed to cache token: invalid 'exp' claim type")
@@ -204,7 +262,14 @@ func (t *TraefikOidc) cacheVerifiedToken(token string, claims map[string]interfa
expirationTime := time.Unix(int64(expClaim), 0) expirationTime := time.Unix(int64(expClaim), 0)
now := time.Now() now := time.Now()
duration := expirationTime.Sub(now) duration := expirationTime.Sub(now)
t.tokenCache.Set(token, claims, duration) // Do not cache an already-expired token under a negative TTL. Via the
// ClockSkewToleranceFuture leeway an expired-then-verified token can
// reach here; a negative TTL entry would sit in the cache as dead
// weight (only swept by the periodic cleanup) and its ExpiresAt
// would already be past, so every hit would re-verify anyway (R153).
if duration > 0 {
t.tokenCache.Set(token, claims, duration)
}
} }
// detectTokenType efficiently detects whether a token is an ID token or access token. // detectTokenType efficiently detects whether a token is an ID token or access token.
@@ -226,6 +291,9 @@ func (t *TraefikOidc) detectTokenType(jwt *JWT, token string) bool {
sum := sha256.Sum256([]byte(token)) sum := sha256.Sum256([]byte(token))
cacheKey := hex.EncodeToString(sum[:]) cacheKey := hex.EncodeToString(sum[:])
// Snapshot clientID under metadataMu: DCR rewrites it at runtime (R137).
clientID, _, _, _, _ := t.clientCredentials()
// Check cache first // Check cache first
if t.tokenTypeCache != nil { if t.tokenTypeCache != nil {
if cachedType, found := t.tokenTypeCache.Get(cacheKey); found { if cachedType, found := t.tokenTypeCache.Get(cacheKey); found {
@@ -306,16 +374,20 @@ func (t *TraefikOidc) detectTokenType(jwt *JWT, token string) bool {
// 5. Check if aud == clientID only (ID token pattern) // 5. Check if aud == clientID only (ID token pattern)
if aud, ok := jwt.Claims["aud"]; ok { if aud, ok := jwt.Claims["aud"]; ok {
// Check string audience // Check string audience
if audStr, ok := aud.(string); ok && audStr == t.clientID { if audStr, ok := aud.(string); ok && audStr == clientID {
isIDToken = true isIDToken = true
} else if audArr, ok := aud.([]interface{}); ok { } else if audArr, ok := aud.([]interface{}); ok {
// Check array audience - only treat as ID token if client_id is sole audience // Check array audience - treat as ID token if client_id is
if len(audArr) == 1 { // present anywhere in the array. OIDC ID tokens legitimately
for _, v := range audArr { // carry multiple aud entries (e.g. [clientID, API-resource])
if str, ok := v.(string); ok && str == t.clientID { // in multi-audience setups; insisting on client_id being the
isIDToken = true // SOLE audience misclassified such ID tokens as access tokens
break // and then validated them against the broader 'audience'
} // config instead of clientID, rejecting valid logins.
for _, v := range audArr {
if str, ok := v.(string); ok && str == clientID {
isIDToken = true
break
} }
} }
} }
@@ -375,20 +447,32 @@ func (t *TraefikOidc) VerifyJWTSignatureAndClaims(jwt *JWT, token string) error
} }
if err := verifySignatureWithKey(token, pubKey, alg); err != nil { if err := verifySignatureWithKey(token, pubKey, alg); err != nil {
if !t.suppressDiagnosticLogs { // The signature failed against the (possibly stale) cached key. The
// Microsoft Graph access tokens carry a `nonce` JWT header and are // provider may have rotated its signing keys in place, reusing the
// signed in a proprietary form Microsoft documents as unverifiable // same kid (R109): refresh the JWKS once and retry before failing,
// by client applications. They reach this path only when the // otherwise a freshly-rotated token is rejected for up to the JWKS
// per-provider classifier (validateAzureTokens) didn't catch them, // cache TTL while the stale key remains cached.
// so log at debug to keep the error stream actionable while still if freshKey, ferr := t.jwkCache.getPublicKeyFresh(context.Background(), jwksURL, kid, t.httpClient); ferr == nil {
// surfacing the cause for diagnostics. if verr := verifySignatureWithKey(token, freshKey, alg); verr == nil {
if _, isMSProprietary := jwt.Header["nonce"]; isMSProprietary { err = nil
t.safeLogDebugf("DIAGNOSTIC: Signature verification failed for kid=%s, alg=%s (Microsoft proprietary nonce header — token is opaque to clients): %v", kid, alg, err)
} else {
t.safeLogErrorf("DIAGNOSTIC: Signature verification failed for kid=%s, alg=%s: %v", kid, alg, err)
} }
} }
return fmt.Errorf("signature verification failed: %w", err) if err != nil {
if !t.suppressDiagnosticLogs {
// Microsoft Graph access tokens carry a `nonce` JWT header and are
// signed in a proprietary form Microsoft documents as unverifiable
// by client applications. They reach this path only when the
// per-provider classifier (validateAzureTokens) didn't catch them,
// so log at debug to keep the error stream actionable while still
// surfacing the cause for diagnostics.
if _, isMSProprietary := jwt.Header["nonce"]; isMSProprietary {
t.safeLogDebugf("DIAGNOSTIC: Signature verification failed for kid=%s, alg=%s (Microsoft proprietary nonce header — token is opaque to clients): %v", kid, alg, err)
} else {
t.safeLogErrorf("DIAGNOSTIC: Signature verification failed for kid=%s, alg=%s: %v", kid, alg, err)
}
}
return fmt.Errorf("signature verification failed: %w", err)
}
} }
if !t.suppressDiagnosticLogs { if !t.suppressDiagnosticLogs {
@@ -399,9 +483,25 @@ func (t *TraefikOidc) VerifyJWTSignatureAndClaims(jwt *JWT, token string) error
isIDToken := t.detectTokenType(jwt, token) isIDToken := t.detectTokenType(jwt, token)
// Determine expected audience // Determine expected audience
expectedAudience := t.audience // Default to configured audience // Snapshot audience/clientID under metadataMu: DCR rewrites them at
// runtime (R137).
clientID, _, _, audience, _ := t.clientCredentials()
expectedAudience := audience // Default to configured audience
if isIDToken { if isIDToken {
expectedAudience = t.clientID if t.strictAudienceValidation && audience != "" && audience != clientID {
// R163: strictAudienceValidation's reject-mismatch promise must
// not be silently bypassed by the ID-token classification.
// detectTokenType treats any token whose aud contains clientID
// as an ID token, which would validate it against clientID
// alone — so a token carrying only clientID (and not the
// configured audience) always passed the audience check and
// strict mode never rejected it. When an explicit, distinct
// audience is configured and strict mode is on, enforce the
// configured audience even for such tokens.
expectedAudience = audience
} else {
expectedAudience = clientID
}
} }
if !t.suppressDiagnosticLogs { if !t.suppressDiagnosticLogs {
if isIDToken { if isIDToken {
@@ -500,11 +600,17 @@ func (t *TraefikOidc) refreshToken(rw http.ResponseWriter, req *http.Request, se
if newToken.IDToken == "" { if newToken.IDToken == "" {
t.logger.Info("Provider did not return a new ID token during refresh") t.logger.Info("Provider did not return a new ID token during refresh")
if !t.persistRotatedRefreshToken(req, rw, session, newToken, "missing ID token") {
return false
}
return false return false
} }
if err = t.verifyToken(newToken.IDToken); err != nil { if err = t.verifyToken(newToken.IDToken); err != nil {
t.logger.Debug("Failed to verify newly obtained ID token: %v", err) t.logger.Debug("Failed to verify newly obtained ID token: %v", err)
if !t.persistRotatedRefreshToken(req, rw, session, newToken, "failed ID token verification") {
return false
}
return false return false
} }
@@ -519,19 +625,36 @@ func (t *TraefikOidc) refreshToken(rw http.ResponseWriter, req *http.Request, se
claims, err := t.extractClaimsFunc(newToken.IDToken) claims, err := t.extractClaimsFunc(newToken.IDToken)
if err != nil { if err != nil {
t.logger.Errorf("refreshToken failed: Failed to extract claims from refreshed token: %v", err) t.logger.Errorf("refreshToken failed: Failed to extract claims from refreshed token: %v", err)
if !t.persistRotatedRefreshToken(req, rw, session, newToken, "failed to extract claims from refreshed token") {
return false
}
return false return false
} }
userIdentifier, _ := claims[t.userIdentifierClaim].(string) userIdentifier, _ := claimScalarString(claims[t.userIdentifierClaim])
if userIdentifier == "" { if userIdentifier == "" {
if t.userIdentifierClaim != "sub" { if t.userIdentifierClaim != "sub" {
userIdentifier, _ = claims["sub"].(string) userIdentifier, _ = claimScalarString(claims["sub"])
} }
if userIdentifier == "" { if userIdentifier == "" {
t.logger.Errorf("refreshToken failed: User identifier claim '%s' missing or empty in refreshed token", t.userIdentifierClaim) t.logger.Errorf("refreshToken failed: User identifier claim '%s' missing or empty in refreshed token", t.userIdentifierClaim)
if !t.persistRotatedRefreshToken(req, rw, session, newToken, "missing user identifier") {
return false
}
return false return false
} }
t.logger.Debugf("Configured claim '%s' not found in refreshed token, using 'sub' claim as fallback", t.userIdentifierClaim) t.logger.Debugf("Configured claim '%s' not found in refreshed token, using 'sub' claim as fallback", t.userIdentifierClaim)
} }
// Bind the refreshed token's identity to the one the session already
// holds (granted at callback after email gate + isAllowedUser). A
// refreshed ID token carrying a different subject must not silently
// substitute the user identity mid-session (R106).
if prev := session.GetUserIdentifier(); prev != "" && prev != userIdentifier {
t.logger.Infof("refreshToken aborted: refreshed token held a different user identifier (%q) than the active session (%q)", userIdentifier, prev)
if !t.persistRotatedRefreshToken(req, rw, session, newToken, "user identifier mismatch") {
return false
}
return false
}
session.SetUserIdentifier(userIdentifier) session.SetUserIdentifier(userIdentifier)
// Get token expiry information for logging // Get token expiry information for logging
@@ -547,6 +670,9 @@ func (t *TraefikOidc) refreshToken(rw http.ResponseWriter, req *http.Request, se
// a refresh (an infinite refresh loop). Keep the previous tokens. // a refresh (an infinite refresh loop). Keep the previous tokens.
if newToken.AccessToken == "" { if newToken.AccessToken == "" {
t.logger.Infof("Provider returned an empty access token during refresh; keeping previous tokens") t.logger.Infof("Provider returned an empty access token during refresh; keeping previous tokens")
if !t.persistRotatedRefreshToken(req, rw, session, newToken, "empty access token") {
return false
}
return false return false
} }
@@ -641,6 +767,28 @@ func (t *TraefikOidc) coordinatedTokenRefresh(req *http.Request, refreshToken st
// The cache wraps the universal cache, which is Redis-backed in production - // The cache wraps the universal cache, which is Redis-backed in production -
// so a "hit" here means another Traefik replica refreshed this same token // so a "hit" here means another Traefik replica refreshed this same token
// within the last few seconds. // within the last few seconds.
// persistRotatedRefreshToken saves a refresh token the IdP just rotated during
// a token exchange, so the session is never left holding a token the exchange
// already consumed. Rotation IdPs (Zitadel, Authentik, Auth0, ...) invalidate
// the presented refresh token on every successful exchange regardless of which
// downstream validation gate (ID token absent, verification failure,
// identifier mismatch, empty access token) subsequently rejects the response.
// If the rotated token were not persisted, the next refresh would present the
// now-consumed token and hit invalid_grant, forcing a full logout.
func (t *TraefikOidc) persistRotatedRefreshToken(req *http.Request, rw http.ResponseWriter, session *SessionData, newToken *TokenResponse, reason string) bool {
if newToken == nil || newToken.RefreshToken == "" {
return true // nothing rotated; nothing to persist
}
session.SetRefreshToken(newToken.RefreshToken)
if err := session.Save(req, rw); err != nil {
t.logger.Errorf("refreshToken failed to persist rotated refresh token after %s: %v", reason, err)
session.SetRefreshToken(newToken.RefreshToken) // keep in-memory for consistency
return false
}
t.logger.Debugf("Persisted rotated refresh token after %s", reason)
return true
}
func (t *TraefikOidc) lookupCachedRefreshResult(sessionID string) (*TokenResponse, bool) { func (t *TraefikOidc) lookupCachedRefreshResult(sessionID string) (*TokenResponse, bool) {
if t.refreshResultCache == nil { if t.refreshResultCache == nil {
return nil, false return nil, false
@@ -649,8 +797,37 @@ func (t *TraefikOidc) lookupCachedRefreshResult(sessionID string) (*TokenRespons
if !ok || v == nil { if !ok || v == nil {
return nil, false return nil, false
} }
if tr, ok := v.(*TokenResponse); ok && tr != nil { // Cross-backend decode (R159): UniversalCache stores the value locally
return tr, true // as the raw *TokenResponse (stored by cacheRefreshResult) but
// serializes it to JSON for any shared backend (e.g. Redis) via
// deserialize, which unmarshals into a generic map. The old code only
// ever type-asserted *TokenResponse, so every backend read — the
// entire cross-replica path — failed the assert and returned nil,
// making the advertised refresh dedup silently inert (each replica
// still POSTed the refresh token). Normalize any serialized form back
// to *TokenResponse.
switch val := v.(type) {
case *TokenResponse:
return val, true
case map[string]interface{}, []byte, string:
var raw []byte
switch tv := v.(type) {
case []byte:
raw = tv
case string:
raw = []byte(tv)
default:
b, err := json.Marshal(v)
if err != nil {
return nil, false
}
raw = b
}
var tr TokenResponse
if err := json.Unmarshal(raw, &tr); err != nil {
return nil, false
}
return &tr, true
} }
return nil, false return nil, false
} }
@@ -685,23 +862,76 @@ const refreshResultCacheTTL = 5 * time.Second
func (t *TraefikOidc) RevokeToken(token string) { func (t *TraefikOidc) RevokeToken(token string) {
t.tokenCache.Delete(token) t.tokenCache.Delete(token)
// Evict any cached positive introspection result so a revoked OPAQUE
// access token (which carries no jti and is therefore not covered by
// the JTI blacklist below) is not still served an "active" verdict
// from the introspection cache after revocation. Next request
// re-introspects and sees active=false.
if t.introspectionCache != nil {
t.introspectionCache.Delete(token)
}
d := blacklistDuration(token)
if jwt, err := parseJWT(token); err == nil { if jwt, err := parseJWT(token); err == nil {
if jti, ok := jwt.Claims["jti"].(string); ok && jti != "" { if jti, ok := jwt.Claims["jti"].(string); ok && jti != "" {
expiry := time.Now().Add(24 * time.Hour)
if t.tokenBlacklist != nil { if t.tokenBlacklist != nil {
t.tokenBlacklist.Set(jti, true, time.Until(expiry)) t.tokenBlacklist.Set(jti, true, d)
t.logger.Debugf("Locally revoked token JTI %s (added to blacklist)", jti) t.logger.Debugf("Locally revoked token JTI %s (added to blacklist)", jti)
} }
} }
} }
expiry := time.Now().Add(24 * time.Hour)
if t.tokenBlacklist != nil { if t.tokenBlacklist != nil {
t.tokenBlacklist.Set(token, true, time.Until(expiry)) t.tokenBlacklist.Set(token, true, d)
t.logger.Debugf("Locally revoked token (added to blacklist)") t.logger.Debugf("Locally revoked token (added to blacklist)")
} }
} }
// tokenExpiryUnix extracts the numeric exp claim from a JWT, accepting every
// representation a decoder or cache backend may produce (float64, json.Number,
// int). ok is false when the token is not a JWT or carries no usable exp.
func tokenExpiryUnix(token string) (int64, bool) {
jwt, err := parseJWT(token)
if err != nil {
return 0, false
}
switch v := jwt.Claims["exp"].(type) {
case float64:
return int64(v), true
case float32:
return int64(v), true
case json.Number:
n, err := v.Int64()
if err != nil {
return 0, false
}
return n, true
case int64:
return v, true
case int:
return int64(v), true
}
return 0, false
}
// blacklistDuration returns how long a revoked token must stay blacklisted.
// A revoked token remains cryptographically valid until its exp, so the
// blacklist must cover the entire remaining validity window — otherwise a
// token with exp more than 24h away could be re-presented (and re-cached)
// after 24h. A 24h floor keeps short-lived or already-expired tokens
// blacklisted for a nominal window.
func blacklistDuration(token string) time.Duration {
const minTTL = 24 * time.Hour
if exp, ok := tokenExpiryUnix(token); ok {
untilExp := time.Until(time.Unix(exp, 0))
if untilExp > minTTL {
return untilExp
}
}
return minTTL
}
// RevokeTokenWithProvider revokes a token with the OIDC provider. // RevokeTokenWithProvider revokes a token with the OIDC provider.
// It sends a revocation request to the provider's revocation endpoint // It sends a revocation request to the provider's revocation endpoint
// with proper authentication and error recovery if available. // with proper authentication and error recovery if available.
@@ -727,28 +957,40 @@ func (t *TraefikOidc) RevokeTokenWithProvider(token, tokenType string) error {
tokenURL := t.tokenURL tokenURL := t.tokenURL
t.metadataMu.RUnlock() t.metadataMu.RUnlock()
// Snapshot the client credentials and related metadata under metadataMu:
// DCR (registerWithProvider, main.go) mutates clientID/clientSecret
// under metadataMu.Lock concurrently with logout-time revocation, so
// reading them here without the lock would be a data race (R134).
t.metadataMu.RLock()
clientID := t.clientID
clientSecret := t.clientSecret
clientAuthMethod := t.clientAuthMethod
clientAssertion := t.clientAssertion
issuerURL := t.issuerURL
t.metadataMu.RUnlock()
data := url.Values{ data := url.Values{
"token": {token}, "token": {token},
"token_type_hint": {tokenType}, "token_type_hint": {tokenType},
} }
// client_id is sent in the body for every method except client_secret_basic, // client_id is sent in the body for every method except client_secret_basic,
// where it is carried in the Authorization header per RFC 6749 §2.3.1. // where it is carried in the Authorization header per RFC 6749 §2.3.1.
if t.clientAuthMethod != "client_secret_basic" || t.clientAssertion != nil { if clientAuthMethod != "client_secret_basic" || clientAssertion != nil {
data.Set("client_id", t.clientID) data.Set("client_id", clientID)
} }
useBasicAuth := false useBasicAuth := false
if t.clientAssertion != nil { if clientAssertion != nil {
assertion, err := t.clientAssertion.Sign(tokenURL, t.clientID) assertion, err := clientAssertion.Sign(tokenURL, clientID)
if err != nil { if err != nil {
return fmt.Errorf("failed to sign client assertion: %w", err) return fmt.Errorf("failed to sign client assertion: %w", err)
} }
data.Set("client_assertion_type", "urn:ietf:params:oauth:client-assertion-type:jwt-bearer") data.Set("client_assertion_type", "urn:ietf:params:oauth:client-assertion-type:jwt-bearer")
data.Set("client_assertion", assertion) data.Set("client_assertion", assertion)
} else if t.clientAuthMethod == "client_secret_basic" { } else if clientAuthMethod == "client_secret_basic" {
useBasicAuth = true useBasicAuth = true
} else { } else {
data.Set("client_secret", t.clientSecret) data.Set("client_secret", clientSecret)
} }
req, err := http.NewRequestWithContext(context.Background(), "POST", revocationURL, strings.NewReader(data.Encode())) req, err := http.NewRequestWithContext(context.Background(), "POST", revocationURL, strings.NewReader(data.Encode()))
@@ -759,16 +1001,13 @@ func (t *TraefikOidc) RevokeTokenWithProvider(token, tokenType string) error {
req.Header.Set("Content-Type", "application/x-www-form-urlencoded") req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
req.Header.Set("Accept", "application/json") req.Header.Set("Accept", "application/json")
if useBasicAuth { if useBasicAuth {
setOAuthBasicAuth(req, t.clientID, t.clientSecret) setOAuthBasicAuth(req, clientID, clientSecret)
} }
// Send the request with circuit breaker protection if available // Send the request with circuit breaker protection if available
var resp *http.Response var resp *http.Response
if t.errorRecoveryManager != nil { if t.errorRecoveryManager != nil {
// Read issuerURL with RLock for service name serviceName := fmt.Sprintf("token-revocation-%s", issuerURL)
t.metadataMu.RLock()
serviceName := fmt.Sprintf("token-revocation-%s", t.issuerURL)
t.metadataMu.RUnlock()
err = t.errorRecoveryManager.ExecuteWithRecovery(context.Background(), serviceName, func() error { err = t.errorRecoveryManager.ExecuteWithRecovery(context.Background(), serviceName, func() error {
var reqErr error var reqErr error
resp, reqErr = t.httpClient.Do(req) //nolint:bodyclose // Body is closed in defer after error check resp, reqErr = t.httpClient.Do(req) //nolint:bodyclose // Body is closed in defer after error check
@@ -981,7 +1220,10 @@ func stringListFromClaim(value interface{}) ([]string, bool) {
case []interface{}: case []interface{}:
out := make([]string, 0, len(v)) out := make([]string, 0, len(v))
for _, item := range v { for _, item := range v {
if s, ok := item.(string); ok { // Scalar items are stringified (strings and numbers), so a
// numeric group/role is preserved instead of being silently
// dropped; composite items (maps, nested arrays) are skipped.
if s, ok := claimScalarString(item); ok {
out = append(out, s) out = append(out, s)
} }
} }
@@ -1000,24 +1242,23 @@ func (t *TraefikOidc) extractGroupsAndRolesFromClaims(claims map[string]interfac
var roles []string var roles []string
if groupsClaim, exists := claims[t.groupClaimName]; exists { if groupsClaim, exists := claims[t.groupClaimName]; exists {
groupsSlice, ok := stringListFromClaim(groupsClaim) if groupsSlice, ok := stringListFromClaim(groupsClaim); ok {
if !ok { groups = append(groups, groupsSlice...)
return nil, nil, fmt.Errorf("%s claim is not an array", t.groupClaimName) for _, groupStr := range groupsSlice {
} t.logger.Debugf("Found group from %s claim: %s", t.groupClaimName, groupStr)
for _, groupStr := range groupsSlice { }
t.logger.Debugf("Found group from %s claim: %s", t.groupClaimName, groupStr)
groups = append(groups, groupStr)
} }
// A malformed/null group claim (numeric scalar, JSON null) is treated
// as empty rather than a hard error (R96): one bad claim must not
// suppress the valid sibling role signal.
} }
if rolesClaim, exists := claims[t.roleClaimName]; exists { if rolesClaim, exists := claims[t.roleClaimName]; exists {
rolesSlice, ok := stringListFromClaim(rolesClaim) if rolesSlice, ok := stringListFromClaim(rolesClaim); ok {
if !ok { roles = append(roles, rolesSlice...)
return nil, nil, fmt.Errorf("%s claim is not an array", t.roleClaimName) for _, roleStr := range rolesSlice {
} t.logger.Debugf("Found role from %s claim: %s", t.roleClaimName, roleStr)
for _, roleStr := range rolesSlice { }
t.logger.Debugf("Found role from %s claim: %s", t.roleClaimName, roleStr)
roles = append(roles, roleStr)
} }
} }
+21 -11
View File
@@ -118,18 +118,15 @@ func (trm *TokenResilienceManager) ExecuteTokenOperation(ctx context.Context, op
return fn() return fn()
} }
// Compose resilience mechanisms // Compose resilience mechanisms. Retry is the INNER layer (around the
// operation) and the circuit breaker is the OUTER layer, so the breaker
// observes one aggregate result per logical operation rather than one
// failure per retry attempt (which would trip it prematurely — e.g.
// maxFailures=3 + 3 retries opening the breaker on a single brief
// outage, failing the very next genuine request). R106.
var finalOperation func() error = fn var finalOperation func() error = fn
// Wrap with circuit breaker if enabled // Wrap with retry if enabled (innermost layer)
if trm.config.CircuitBreakerEnabled && trm.circuitBreaker != nil {
originalOp := finalOperation
finalOperation = func() error {
return trm.circuitBreaker.ExecuteWithContext(ctx, originalOp)
}
}
// Wrap with retry if enabled
if trm.config.RetryEnabled && trm.retryExecutor != nil { if trm.config.RetryEnabled && trm.retryExecutor != nil {
originalOp := finalOperation originalOp := finalOperation
if singleUse { if singleUse {
@@ -143,6 +140,14 @@ func (trm *TokenResilienceManager) ExecuteTokenOperation(ctx context.Context, op
} }
} }
// Wrap with circuit breaker if enabled (outermost layer)
if trm.config.CircuitBreakerEnabled && trm.circuitBreaker != nil {
originalOp := finalOperation
finalOperation = func() error {
return trm.circuitBreaker.ExecuteWithContext(ctx, originalOp)
}
}
err := finalOperation() err := finalOperation()
if err != nil && trm.logger != nil { if err != nil && trm.logger != nil {
@@ -174,7 +179,12 @@ func (trm *TokenResilienceManager) ExecuteTokenRefresh(ctx context.Context, t *T
var result *TokenResponse var result *TokenResponse
var err error var err error
err = trm.ExecuteTokenOperation(ctx, "token_refresh", false, func() error { // Refresh is single-use (like the auth-code exchange): a client-side timeout
// may mean the provider already processed AND rotated the refresh token, so
// re-presenting it on a retry surfaces invalid_grant and the session is
// cleared — escalating a transient timeout into a full logout. Single-use
// retries only on "never sent" errors (connect/dial), never on timeout.
err = trm.ExecuteTokenOperation(ctx, "token_refresh", true, func() error {
// Call exchangeTokens directly to avoid recursion back to getNewTokenWithRefreshToken // Call exchangeTokens directly to avoid recursion back to getNewTokenWithRefreshToken
// which would call ExecuteTokenRefresh again, causing infinite loop (issue #67) // which would call ExecuteTokenRefresh again, causing infinite loop (issue #67)
result, err = t.exchangeTokens(ctx, "refresh_token", refreshToken, "", "") result, err = t.exchangeTokens(ctx, "refresh_token", refreshToken, "", "")
+97 -2
View File
@@ -15,6 +15,7 @@ package traefikoidc
import ( import (
"encoding/base64" "encoding/base64"
"encoding/json" "encoding/json"
"errors"
"strings" "strings"
"time" "time"
) )
@@ -81,6 +82,19 @@ func (t *TraefikOidc) accessTokenUnexpired(token string) bool {
// validateTokenExpiryRS is the requestState-aware variant of validateTokenExpiry. // validateTokenExpiryRS is the requestState-aware variant of validateTokenExpiry.
// Reads rs.refreshToken instead of session.GetRefreshToken() (4 RLocks avoided). // Reads rs.refreshToken instead of session.GetRefreshToken() (4 RLocks avoided).
func (t *TraefikOidc) validateTokenExpiryRS(rs *requestState, token string) (bool, bool, bool) { func (t *TraefikOidc) validateTokenExpiryRS(rs *requestState, token string) (bool, bool, bool) {
// Defense-in-depth (F2/R133): this is the final authenticated gate.
// Every current caller has already passed the blacklist check via
// verifyToken, but guard independently so a revoked-but-still-cached
// token can never be served authenticated if the call order changes.
if t.tokenBlacklist != nil {
if b, ok := t.tokenBlacklist.Get(token); ok && b != nil {
if rs.refreshToken != "" {
return false, true, false
}
return false, false, true
}
}
cachedClaims, found := t.tokenCache.Get(token) cachedClaims, found := t.tokenCache.Get(token)
if !found { if !found {
t.logger.Debug("Claims not found in cache after successful token verification") t.logger.Debug("Claims not found in cache after successful token verification")
@@ -102,7 +116,13 @@ func (t *TraefikOidc) validateTokenExpiryRS(rs *requestState, token string) (boo
expTimeObj := time.Unix(int64(expClaim), 0) expTimeObj := time.Unix(int64(expClaim), 0)
nowObj := time.Now() nowObj := time.Now()
if expTimeObj.Before(nowObj) { // Apply the same clock-skew leeway as jwt.Verify's verifyExpiration
// (ClockSkewToleranceFuture). This is the final auth gate run after
// successful signature verification; without the leeway a token
// within the 2-minute post-exp window was granted by Verify but
// downgraded to here as expired, forcing a needless refresh or
// re-auth (R124).
if nowObj.After(expTimeObj.Add(ClockSkewToleranceFuture)) {
if rs.refreshToken != "" { if rs.refreshToken != "" {
return false, true, false return false, true, false
} }
@@ -177,6 +197,19 @@ func (t *TraefikOidc) validateStandardTokensRS(rs *requestState) (bool, bool, bo
} }
return false, false, true return false, false, true
} }
// A definitive 4xx from the introspection endpoint (e.g. 401
// for an unknown/revoked token) means the token itself is bad,
// not that the endpoint transiently failed. Treat it as
// invalid (refresh) rather than falling through to
// ID-token-only auth, which would authenticate a revoked
// opaque token (R156).
var httpErr *HTTPError
if errors.As(err, &httpErr) && httpErr.StatusCode >= 400 && httpErr.StatusCode < 500 {
if rs.refreshToken != "" {
return false, true, false
}
return false, false, true
}
if t.requireTokenIntrospection { if t.requireTokenIntrospection {
if rs.refreshToken != "" { if rs.refreshToken != "" {
return false, true, false return false, true, false
@@ -231,7 +264,9 @@ func (t *TraefikOidc) validateStandardTokensRS(rs *requestState) (bool, bool, bo
// JWT access token present. // JWT access token present.
accessTokenValid := false accessTokenValid := false
lenientAudienceOnly := false lenientAudienceOnly := false
var accessVerifyErr error
if err := t.verifyToken(rs.accessToken); err != nil { if err := t.verifyToken(rs.accessToken); err != nil {
accessVerifyErr = err
errMsg := err.Error() errMsg := err.Error()
if strings.Contains(errMsg, "invalid audience") || strings.Contains(errMsg, "audience") { if strings.Contains(errMsg, "invalid audience") || strings.Contains(errMsg, "audience") {
if t.strictAudienceValidation { if t.strictAudienceValidation {
@@ -298,6 +333,21 @@ func (t *TraefikOidc) validateStandardTokensRS(rs *requestState) (bool, bool, bo
if accessTokenValid { if accessTokenValid {
return t.validateTokenExpiryRS(rs, rs.accessToken) return t.validateTokenExpiryRS(rs, rs.accessToken)
} }
// Access token is a JWT whose verification failed, yet the session's
// ID token is still valid. If the failure was specifically EXPIRY —
// verifyToken checks the signature before exp, so "token has expired"
// implies the signature and other claims validated — the access token
// is unusable at the upstream resource. Refresh to obtain a fresh one
// rather than forwarding a stale access token (R131). Without this an
// expired access token (with a still-valid ID token) was authenticated
// with needsRefresh=false and forwarded as-is until the ID token
// itself neared expiry, giving the backend an expired bearer token.
if accessVerifyErr != nil && strings.Contains(accessVerifyErr.Error(), "token has expired") {
if rs.refreshToken != "" {
return true, true, false
}
return false, false, true
}
return t.validateTokenExpiryRS(rs, rs.idToken) return t.validateTokenExpiryRS(rs, rs.idToken)
} }
@@ -326,6 +376,41 @@ func (t *TraefikOidc) validateAzureTokensRS(rs *requestState) (bool, bool, bool)
} }
return t.validateTokenExpiryRS(rs, rs.idToken) return t.validateTokenExpiryRS(rs, rs.idToken)
} }
// No ID token to corroborate the Azure token. Its signature
// can't be verified client-side, but it still carries exp;
// authenticate only while unexpired. Previously this
// returned authenticated=true with no expiry check, so an
// expired (or unparseable) Azure access token authenticated
// indefinitely until the session itself expired (R129).
if claims, err := extractClaims(rs.accessToken); err != nil {
if rs.refreshToken != "" {
return false, true, false
}
return false, false, true
} else {
// Defense-in-depth (mirrors validateTokenExpiryRS): a
// revoked-but-still-cached token must not authenticate
// through this unverifiable branch either (R147).
if t.tokenBlacklist != nil {
if b, ok := t.tokenBlacklist.Get(rs.accessToken); ok && b != nil {
if rs.refreshToken != "" {
return false, true, false
}
return false, false, true
}
}
// exp must be present and numeric to establish the token
// is current; an unverifiable token with no (or
// non-numeric) exp authenticates with zero verification,
// so fail closed when it can't be bounded (R130).
exp, ok := claims["exp"].(float64)
if !ok || time.Now().After(time.Unix(int64(exp), 0).Add(ClockSkewToleranceFuture)) {
if rs.refreshToken != "" {
return false, true, false
}
return false, false, true
}
}
return true, false, false return true, false, false
} }
if err := t.verifyToken(rs.accessToken); err != nil { if err := t.verifyToken(rs.accessToken); err != nil {
@@ -349,7 +434,17 @@ func (t *TraefikOidc) validateAzureTokensRS(rs *requestState) (bool, bool, bool)
if rs.idToken != "" { if rs.idToken != "" {
return t.validateTokenExpiryRS(rs, rs.idToken) return t.validateTokenExpiryRS(rs, rs.idToken)
} }
return true, false, false // Opaque access token with no ID token to corroborate it. Do not
// authenticate on an unverified (or unverifiable) token: refresh
// if a refresh token is available, otherwise force
// re-authentication. Mirrors validateStandardTokensRS, which
// documents this exact decision ("Do NOT authenticate on an
// unverified token"). The previous `return true, false, false`
// accepted any opaque value here with no verification (R99).
if rs.refreshToken != "" {
return false, true, false
}
return false, false, true
} }
if rs.idToken != "" { if rs.idToken != "" {
+71 -4
View File
@@ -75,8 +75,11 @@ func pathExcluded(requestPath, excluded string) bool {
// Returns: // Returns:
// - The fully constructed authorization URL string. // - The fully constructed authorization URL string.
func (t *TraefikOidc) buildAuthURL(redirectURL, state, nonce, codeChallenge string) string { func (t *TraefikOidc) buildAuthURL(redirectURL, state, nonce, codeChallenge string) string {
// Snapshot clientID/audience under metadataMu: DCR rewrites them at
// runtime (R137).
clientID, _, _, audience, _ := t.clientCredentials()
params := url.Values{} params := url.Values{}
params.Set("client_id", t.clientID) params.Set("client_id", clientID)
params.Set("response_type", "code") params.Set("response_type", "code")
params.Set("redirect_uri", redirectURL) params.Set("redirect_uri", redirectURL)
params.Set("state", state) params.Set("state", state)
@@ -86,9 +89,9 @@ func (t *TraefikOidc) buildAuthURL(redirectURL, state, nonce, codeChallenge stri
// This allows access tokens to have the correct audience claim // This allows access tokens to have the correct audience claim
// Only add if audience is configured and different from client_id // Only add if audience is configured and different from client_id
// ID tokens will always have aud=client_id per OIDC spec // ID tokens will always have aud=client_id per OIDC spec
if t.audience != "" && t.audience != t.clientID { if audience != "" && audience != clientID {
params.Set("audience", t.audience) params.Set("audience", audience)
t.logger.Debugf("Adding audience parameter to authorize URL: %s", t.audience) t.logger.Debugf("Adding audience parameter to authorize URL: %s", audience)
} }
if t.enablePKCE && codeChallenge != "" { if t.enablePKCE && codeChallenge != "" {
@@ -224,6 +227,22 @@ func (t *TraefikOidc) buildAuthURL(redirectURL, state, nonce, codeChallenge stri
// //
// Returns: // Returns:
// - The fully constructed URL string with appended query parameters. // - The fully constructed URL string with appended query parameters.
//
// preserveBaseQuery seeds url parameters from the base URL's pre-existing
// query string, without overriding authoritative plugin-supplied params.
// Some IdPs embed client_id or template params in their discovered
// authorization/end-session endpoints; overwriting RawQuery dropped them
// (R101).
func (t *TraefikOidc) preserveBaseQuery(u *url.URL, params url.Values) {
for k, vs := range u.Query() {
if params.Get(k) == "" {
for _, v := range vs {
params.Add(k, v)
}
}
}
}
func (t *TraefikOidc) buildURLWithParams(baseURL string, params url.Values) string { func (t *TraefikOidc) buildURLWithParams(baseURL string, params url.Values) string {
if baseURL != "" { if baseURL != "" {
if strings.HasPrefix(baseURL, "http://") || strings.HasPrefix(baseURL, "https://") { if strings.HasPrefix(baseURL, "http://") || strings.HasPrefix(baseURL, "https://") {
@@ -259,6 +278,7 @@ func (t *TraefikOidc) buildURLWithParams(baseURL string, params url.Values) stri
return "" return ""
} }
t.preserveBaseQuery(resolvedURL, params)
resolvedURL.RawQuery = params.Encode() resolvedURL.RawQuery = params.Encode()
return resolvedURL.String() return resolvedURL.String()
} }
@@ -274,6 +294,7 @@ func (t *TraefikOidc) buildURLWithParams(baseURL string, params url.Values) stri
return "" return ""
} }
t.preserveBaseQuery(u, params)
u.RawQuery = params.Encode() u.RawQuery = params.Encode()
return u.String() return u.String()
} }
@@ -359,6 +380,14 @@ func (t *TraefikOidc) validateDiscoveredEndpoint(urlStr string, allowLoopback bo
if u.Scheme != "https" && u.Scheme != "http" { if u.Scheme != "https" && u.Scheme != "http" {
return fmt.Errorf("disallowed URL scheme: %q", u.Scheme) return fmt.Errorf("disallowed URL scheme: %q", u.Scheme)
} }
// Pin the discovered endpoint's scheme to the provider's own scheme. A
// provider served over HTTPS must hand out HTTPS endpoints; a legacy or
// poisoned discovery document that downgrades a credential-bearing
// endpoint (client_secret, tokens) to plaintext HTTP would otherwise be
// used as-is, shipping secrets over an unauthenticated channel (R146).
if u.Scheme == "http" && t.providerUsesHTTPS() {
return fmt.Errorf("discovered endpoint uses plaintext http while the provider is https; refusing scheme downgrade")
}
if u.Host == "" { if u.Host == "" {
return fmt.Errorf("missing host in URL") return fmt.Errorf("missing host in URL")
} }
@@ -371,6 +400,15 @@ func (t *TraefikOidc) validateDiscoveredEndpoint(urlStr string, allowLoopback bo
case ip.IsPrivate() && !t.allowPrivateIPAddresses: case ip.IsPrivate() && !t.allowPrivateIPAddresses:
return fmt.Errorf("endpoint host is a private address: %s", ip) return fmt.Errorf("endpoint host is a private address: %s", ip)
} }
} else if err := t.validateHost(u.Hostname()); err != nil && !(allowLoopback && isLoopbackHost(u.Hostname())) {
// Name-based hosts (localhost, metadata.google.internal, any
// hostname) bypass the IP-literal checks above; run the hostname
// SSRF guard so a discovery doc pointing at such a host is
// rejected (R97). Loopback names (localhost) are permitted when
// allowLoopback is set, mirroring the IP-literal branch; hard
// metadata-service names (metadata.google.internal) are never
// allowed because they are not loopback.
return fmt.Errorf("discovered endpoint host is not allowed: %w", err)
} }
if strings.Contains(u.Path, "..") { if strings.Contains(u.Path, "..") {
return fmt.Errorf("path traversal detected in URL path") return fmt.Errorf("path traversal detected in URL path")
@@ -382,6 +420,14 @@ func (t *TraefikOidc) validateDiscoveredEndpoint(urlStr string, allowLoopback bo
// Used to pin the credential-bearing introspection endpoint to the operator- // Used to pin the credential-bearing introspection endpoint to the operator-
// configured provider so a poisoned discovery document cannot redirect the // configured provider so a poisoned discovery document cannot redirect the
// client secret to an attacker-controlled host. // client secret to an attacker-controlled host.
// providerUsesHTTPS reports whether the operator-configured provider URL is
// served over HTTPS. Used to pin discovered endpoint schemes.
func (t *TraefikOidc) providerUsesHTTPS() bool {
t.metadataMu.RLock()
defer t.metadataMu.RUnlock()
return t.providerURL != "" && strings.HasPrefix(strings.ToLower(t.providerURL), "https://")
}
func sameHost(a, b string) bool { func sameHost(a, b string) bool {
ua, erra := url.Parse(a) ua, erra := url.Parse(a)
ub, errb := url.Parse(b) ub, errb := url.Parse(b)
@@ -454,3 +500,24 @@ func (t *TraefikOidc) validateHost(host string) error {
return nil return nil
} }
// redactCallbackURL returns the callback URL with the one-time
// authorization code (and state) query values redacted. The single-use
// code is a quiet credential: logging the full callback URL (as the
// per-request Debug line and the CSRF-failure Error line did) would
// publish it into the log pipeline (R153). Malformed URLs are returned
// unchanged.
func redactCallbackURL(rawURL string) string {
u, err := url.Parse(rawURL)
if err != nil {
return rawURL
}
q := u.Query()
for _, k := range []string{"code", "state"} {
if q.Get(k) != "" {
q.Set(k, "[REDACTED]")
}
}
u.RawQuery = q.Encode()
return u.String()
}