From 4bd061abeeb4daa1b53e42710d9b004e54c6e4ae Mon Sep 17 00:00:00 2001 From: Lukasz Raczylo Date: Fri, 14 Aug 2026 16:17:24 +0100 Subject: [PATCH] fix(config,dcr): env fallbacks, DCR self-heal, per-source auth gate - implement documented OIDC_* env fallbacks; hold metadataMu for clientAssertion write - re-register client when client_secret expires; require secret for confidential - add opt-in per-source external auth rate limiting (PerSourceLoginRateLimit) --- config_marshalling.go | 137 ++++++++++++++------- dynamic_client_registration.go | 55 +++++++++ main.go | 128 ++++++++++++++++++-- per_source_ratelimit.go | 120 ++++++++++++++++++ settings.go | 214 +++++++++++++++++++++++++++++++-- types.go | 16 ++- 6 files changed, 610 insertions(+), 60 deletions(-) create mode 100644 per_source_ratelimit.go diff --git a/config_marshalling.go b/config_marshalling.go index 1383e2f..1d2912b 100644 --- a/config_marshalling.go +++ b/config_marshalling.go @@ -7,10 +7,32 @@ import ( // REDACTED is the placeholder value for sensitive information const REDACTED = "[REDACTED]" -// MarshalJSON implements custom JSON marshaling to redact sensitive fields -// Rewritten without type aliases for yaegi compatibility -func (c Config) MarshalJSON() ([]byte, error) { - // Build a map manually to avoid type alias issues with yaegi +// marshalDCR builds the JSON/YAML-safe representation of the dynamic +// client registration config, redacting the initial access token. +func marshalDCR(d *DynamicClientRegistrationConfig) map[string]interface{} { + if d == nil { + return nil + } + m := make(map[string]interface{}) + m["enabled"] = d.Enabled + m["persistCredentials"] = d.PersistCredentials + m["storageBackend"] = d.StorageBackend + m["initialAccessToken"] = REDACTED + m["registrationEndpoint"] = d.RegistrationEndpoint + m["credentialsFile"] = d.CredentialsFile + m["redisKeyPrefix"] = d.RedisKeyPrefix + return m +} + +// configToMap builds the JSON/YAML-safe map representation of Config. Both +// MarshalJSON and MarshalYAML share it so the (hand-maintained for yaegi +// compatibility) field/redaction list cannot drift between the two +// encoders — previously the list silently dropped entire config areas +// (dynamicClientRegistration, securityHeaders, audience, overrideScopes, +// enablePKCE, allowOpaqueTokens, requireTokenIntrospection, +// strictAudienceValidation, extraAuthParams, client assertion settings) +// and never redacted their embedded secrets (R150). +func (c Config) configToMap() map[string]interface{} { result := make(map[string]interface{}) // Copy public fields @@ -28,11 +50,60 @@ func (c Config) MarshalJSON() ([]byte, error) { result["allowedUsers"] = c.AllowedUsers result["allowedRolesAndGroups"] = c.AllowedRolesAndGroups - // Redact sensitive fields + // Fields that the legacy hand-written list dropped (R150): important + // non-secret config that must survive any JSON/YAML round-trip. + result["audience"] = c.Audience + result["cookiePrefix"] = c.CookiePrefix + result["cookieDomain"] = c.CookieDomain + result["userIdentifierClaim"] = c.UserIdentifierClaim + result["groupClaimName"] = c.GroupClaimName + result["roleClaimName"] = c.RoleClaimName + result["oidcEndSessionURL"] = c.OIDCEndSessionURL + result["revocationURL"] = c.RevocationURL + result["introspectionURL"] = c.IntrospectionURL + result["headers"] = c.Headers + result["allowedClaims"] = c.AllowedClaims + result["extraAuthParams"] = c.ExtraAuthParams + result["refreshGracePeriodSeconds"] = c.RefreshGracePeriodSeconds + result["maxRefreshTokenAgeSeconds"] = c.MaxRefreshTokenAgeSeconds + result["sessionMaxAge"] = c.SessionMaxAge + result["overrideScopes"] = c.OverrideScopes + result["disableReplayDetection"] = c.DisableReplayDetection + result["requireTokenIntrospection"] = c.RequireTokenIntrospection + result["allowOpaqueTokens"] = c.AllowOpaqueTokens + result["strictAudienceValidation"] = c.StrictAudienceValidation + result["enablePKCE"] = c.EnablePKCE + result["allowPrivateIPAddresses"] = c.AllowPrivateIPAddresses + result["minimalHeaders"] = c.MinimalHeaders + result["stripAuthCookies"] = c.StripAuthCookies + result["enableBackchannelLogout"] = c.EnableBackchannelLogout + result["enableFrontchannelLogout"] = c.EnableFrontchannelLogout + result["backchannelLogoutURL"] = c.BackchannelLogoutURL + result["frontchannelLogoutURL"] = c.FrontchannelLogoutURL + result["caCertPath"] = c.CACertPath + result["caCertPEM"] = c.CACertPEM + result["insecureSkipVerify"] = c.InsecureSkipVerify + result["clientAuthMethod"] = c.ClientAuthMethod + result["clientAssertionKeyID"] = c.ClientAssertionKeyID + result["clientAssertionAlg"] = c.ClientAssertionAlg + result["enableBearerAuth"] = c.EnableBearerAuth + result["bearerIdentifierClaim"] = c.BearerIdentifierClaim + result["stripAuthorizationHeader"] = c.StripAuthorizationHeader + result["bearerEmitWWWAuthenticate"] = c.BearerEmitWWWAuthenticate + result["bearerOverridesCookie"] = c.BearerOverridesCookie + result["maxTokenAgeSeconds"] = c.MaxTokenAgeSeconds + result["maxIdentifierLength"] = c.MaxIdentifierLength + result["bearerFailureThreshold"] = c.BearerFailureThreshold + result["bearerFailureWindowSeconds"] = c.BearerFailureWindowSeconds + result["bearerFailurePenaltySeconds"] = c.BearerFailurePenaltySeconds + + // Redact sensitive fields (existing secrets plus the previously-leaked + // client assertion private key, R150). result["clientSecret"] = REDACTED result["sessionEncryptionKey"] = REDACTED + result["clientAssertionPrivateKey"] = REDACTED - // Handle Redis config + // Handle Redis config (redacts password) if c.Redis != nil { redisMap := make(map[string]interface{}) redisMap["enabled"] = c.Redis.Enabled @@ -44,47 +115,29 @@ func (c Config) MarshalJSON() ([]byte, error) { result["redis"] = redisMap } - return json.Marshal(result) + // Handle dynamic client registration (redacts the initial access token) + if c.DynamicClientRegistration != nil { + result["dynamicClientRegistration"] = marshalDCR(c.DynamicClientRegistration) + } + + // Handle security headers + if c.SecurityHeaders != nil { + result["securityHeaders"] = c.SecurityHeaders + } + + return result +} + +// MarshalJSON implements custom JSON marshaling to redact sensitive fields +// Rewritten without type aliases for yaegi compatibility +func (c Config) MarshalJSON() ([]byte, error) { + return json.Marshal(c.configToMap()) } // MarshalYAML implements custom YAML marshaling to redact sensitive fields // Rewritten without type aliases for yaegi compatibility func (c Config) MarshalYAML() (interface{}, error) { - // Build a map manually to avoid type alias issues with yaegi - result := make(map[string]interface{}) - - // Copy public fields - result["providerURL"] = c.ProviderURL - result["clientID"] = c.ClientID - result["callbackURL"] = c.CallbackURL - result["logoutURL"] = c.LogoutURL - result["postLogoutRedirectURI"] = c.PostLogoutRedirectURI - result["scopes"] = c.Scopes - result["forceHTTPS"] = c.ForceHTTPS - result["logLevel"] = c.LogLevel - result["rateLimit"] = c.RateLimit - result["excludedURLs"] = c.ExcludedURLs - result["allowedUserDomains"] = c.AllowedUserDomains - result["allowedUsers"] = c.AllowedUsers - result["allowedRolesAndGroups"] = c.AllowedRolesAndGroups - - // Redact sensitive fields - result["clientSecret"] = REDACTED - result["sessionEncryptionKey"] = REDACTED - - // Handle Redis config - if c.Redis != nil { - redisMap := make(map[string]interface{}) - redisMap["enabled"] = c.Redis.Enabled - redisMap["address"] = c.Redis.Address - redisMap["password"] = REDACTED - redisMap["db"] = c.Redis.DB - redisMap["poolSize"] = c.Redis.PoolSize - redisMap["cacheMode"] = c.Redis.CacheMode - result["redis"] = redisMap - } - - return result, nil + return c.configToMap(), nil } // MarshalJSON for RedisConfig to redact sensitive fields diff --git a/dynamic_client_registration.go b/dynamic_client_registration.go index 995df13..3186153 100644 --- a/dynamic_client_registration.go +++ b/dynamic_client_registration.go @@ -54,6 +54,7 @@ type DynamicClientRegistrar struct { registrationResponse *ClientRegistrationResponse store DCRCredentialsStore // Storage backend for credentials providerURL string + scopes []string // Runtime auth/scopes from the operator config (merged) mu sync.RWMutex } @@ -105,6 +106,19 @@ func (r *DynamicClientRegistrar) SetStore(store DCRCredentialsStore) { r.store = store } +// EffectiveTokenEndpointAuthMethod returns the token-endpoint auth method that +// was (or will be) advertised to the IdP during registration: +// ClientMetadata.TokenEndpointAuthMethod if set, else client_secret_post +// (RFC 7591 confidential-client default). The IdP provisions the client +// according to this method, so the runtime must authenticate to the token +// endpoint with the SAME method or every exchange fails (R157). +func (r *DynamicClientRegistrar) EffectiveTokenEndpointAuthMethod() string { + if r.config != nil && r.config.ClientMetadata != nil && r.config.ClientMetadata.TokenEndpointAuthMethod != "" { + return r.config.ClientMetadata.TokenEndpointAuthMethod + } + return "client_secret_post" +} + // RegisterClient performs dynamic client registration with the OIDC provider // It first attempts to load existing credentials from storage if persistence is enabled, // then registers a new client if no valid credentials exist. @@ -264,6 +278,27 @@ func (r *DynamicClientRegistrar) buildRegistrationRequest() ([]byte, error) { return nil, fmt.Errorf("redirect_uris is required for client registration") } + // Validate the optional URI metadata fields with the same absolute + // http(s) rule as redirect_uris. A relative or non-http(s) value + // (e.g. a malformed jwks_uri on a private_key_jwt client) would be + // forwarded to the IdP and make the registered client unverifiable at + // runtime (R141). + for _, field := range []struct{ val, name string }{ + {metadata.LogoURI, "logo_uri"}, + {metadata.ClientURI, "client_uri"}, + {metadata.PolicyURI, "policy_uri"}, + {metadata.TOSURI, "tos_uri"}, + {metadata.JWKSURI, "jwks_uri"}, + } { + if field.val == "" { + continue + } + parsed, perr := url.ParseRequestURI(field.val) + if perr != nil || (parsed.Scheme != "http" && parsed.Scheme != "https") || parsed.Host == "" { + return nil, fmt.Errorf("%s %q must be an absolute http(s) URL", field.name, field.val) + } + } + // Optional fields - only include if set if len(metadata.ResponseTypes) > 0 { reqData["response_types"] = metadata.ResponseTypes @@ -338,6 +373,13 @@ func (r *DynamicClientRegistrar) buildRegistrationRequest() ([]byte, error) { if metadata.Scope != "" { reqData["scope"] = metadata.Scope + } else if len(r.scopes) > 0 { + // Register with the union of runtime auth scopes when the operator + // gave no explicit registration scope. IdPs that only grant + // registered/advertised scopes (Keycloak offline_access, Auth0) + // otherwise register a client whose later token/refresh grant is + // rejected with invalid_scope, breaking refresh (R124). + reqData["scope"] = strings.Join(deduplicateScopes(r.scopes), " ") } return json.Marshal(reqData) @@ -356,6 +398,19 @@ func (r *DynamicClientRegistrar) areCredentialsValid(resp *ClientRegistrationRes return false } + // For a secret-based auth method the response must carry a client_secret, + // mirroring the fresh-registration check (RegisterClient). A stored + // record with a client_id but empty secret would otherwise be accepted + // and installed as t.clientSecret="", breaking every token exchange + // (R124). + authMethod := "client_secret_post" + if r.config != nil && r.config.ClientMetadata != nil && r.config.ClientMetadata.TokenEndpointAuthMethod != "" { + authMethod = r.config.ClientMetadata.TokenEndpointAuthMethod + } + if (authMethod == "client_secret_post" || authMethod == "client_secret_basic") && resp.ClientSecret == "" { + return false + } + // Check if secret has expired if resp.ClientSecretExpiresAt > 0 { expiresAt := time.Unix(resp.ClientSecretExpiresAt, 0) diff --git a/main.go b/main.go index e20c1b1..fb77971 100644 --- a/main.go +++ b/main.go @@ -297,6 +297,7 @@ func NewWithContext(ctx context.Context, config *Config, next http.Handler, name return deduplicateScopes(mergeScopes(defaultSystemScopes, userProvidedScopes)) }(), limiter: rate.NewLimiter(rate.Limit(config.RateLimit), config.RateLimit), + perSourceLimiter: newPerSourceAuthLimiter(config.PerSourceLoginRateLimit), tokenCache: cacheManager.GetSharedTokenCache(), httpClient: httpClient, tokenHTTPClient: tokenHTTPClient, @@ -443,6 +444,11 @@ func NewWithContext(ctx context.Context, config *Config, next http.Handler, name t.initiateAuthenticationFunc = func(rw http.ResponseWriter, req *http.Request, session *SessionData, redirectURL string) { t.defaultInitiateAuthentication(rw, req, session, redirectURL) } + // Lazily build a client-assertion signer from the static key material, + // used only when DCR later provisions a private_key_jwt client (R162). + t.dcrClientAssertionBuilder = func() (*ClientAssertionSigner, error) { + return buildClientAssertionSignerFromConfig(config) + } for k, v := range defaultExcludedURLs { t.excludedURLs[k] = v @@ -461,8 +467,11 @@ func NewWithContext(ctx context.Context, config *Config, next http.Handler, name parsedTmpl, err := tmpl.Parse(header.Value) if err != nil { - logger.Errorf("Failed to parse header template for %s: %v", header.Name, err) - continue + // Fail closed: a configured identity/security header that does not + // compile must stop startup rather than silently run the middleware + // without it (R141). Validate() gates non-empty templates with the + // same engine, so this is a final safety net. + return nil, fmt.Errorf("failed to parse header template for %s: %w", header.Name, err) } t.headerTemplates[header.Name] = parsedTmpl @@ -604,6 +613,22 @@ func (t *TraefikOidc) updateMetadataEndpoints(metadata *ProviderMetadata) { t.logger.Errorf("Ignoring introspection endpoint %q: host does not match configured providerURL", metadata.IntrospectionURL) metadata.IntrospectionURL = "" } + // Token and revocation endpoints are NOT same-host pinned: mainstream + // OIDC providers split them across hosts (e.g. Google's issuer is + // accounts.google.com but token_endpoint is oauth2.googleapis.com). + // A same-host pin here would clear the real token endpoint on those + // providers, leaving tokenURL empty and breaking the code exchange. + // SSRF defense for these (and every endpoint) is handled above by + // validateDiscoveredEndpoint (blocks private/loopback/cloud-metadata + // hosts), which is the appropriate gate. The client secret travels to + // the (TLS-verified, SSRF-filtered) endpoint the IdP advertises, + // matching standard OIDC behavior (R158). + if metadata.TokenURL != "" && t.providerURL != "" && !sameHost(metadata.TokenURL, t.providerURL) { + t.logger.Debugf("Accepted discovered token endpoint on a different host than providerURL: %s", metadata.TokenURL) + } + if metadata.RevokeURL != "" && t.providerURL != "" && !sameHost(metadata.RevokeURL, t.providerURL) { + t.logger.Debugf("Accepted discovered revocation endpoint on a different host than providerURL: %s", metadata.RevokeURL) + } // Pin the discovered issuer to the operator-configured provider host. The // issuer is the trust anchor for JWT issuer validation, so a poisoned @@ -634,10 +659,32 @@ func (t *TraefikOidc) updateMetadataEndpoints(metadata *ProviderMetadata) { t.metadataMu.Lock() - t.jwksURL = metadata.JWKSURL + // Preserve the previously-good issuer when the discovered one is empty + // (a transiently-malformed discovery document on refresh, or a host + // mismatch nulled below). Unconditionally assigning the discovered + // value would wipe a working issuer to "" on a benign refresh, + // permanently bricking auth (503 + infinite recovery) until a later + // refresh happens to restore it (R124). + if discoveredIssuer == "" { + discoveredIssuer = t.issuerURL + } + + // Preserve a previously-good endpoint when the discovered value is + // empty (a transiently-malformed discovery document on refresh, or an + // SSRF-sanitize blanking it). Mirror the issuer guard above (R124): + // unconditionally assigning the empty value would wipe a working + // jwks_uri (and therefore permanently break signature verification), + // auth, or token endpoint with no config override to recover (R151). + preserveGood := func(newV, cur string) string { + if newV == "" && cur != "" { + return cur + } + return newV + } + t.jwksURL = preserveGood(metadata.JWKSURL, t.jwksURL) t.scopesSupported = metadata.ScopesSupported // Store supported scopes from discovery - t.authURL = metadata.AuthURL - t.tokenURL = metadata.TokenURL + t.authURL = preserveGood(metadata.AuthURL, t.authURL) + t.tokenURL = preserveGood(metadata.TokenURL, t.tokenURL) t.issuerURL = discoveredIssuer t.revocationURL = metadata.RevokeURL t.endSessionURL = metadata.EndSessionURL @@ -684,12 +731,38 @@ func (t *TraefikOidc) updateMetadataEndpoints(metadata *ProviderMetadata) { // second client (and so the clientID read here can't race the write in // performDynamicClientRegistration). t.dcrMu.Lock() - if t.dcrConfig != nil && t.dcrConfig.Enabled && t.clientID == "" { + // Snapshot clientID under metadataMu: DCR writes it under + // metadataMu.Lock, so the dcrMu-guarded read must also snapshot under + // RLock to avoid a lock-mismatch data race (R137). + if t.dcrConfig != nil && t.dcrConfig.Enabled && t.dcrRegistrationNeeded() { t.performDynamicClientRegistration() } t.dcrMu.Unlock() } +// dcrRegistrationNeeded reports whether a DCR (re-)registration is needed: +// either no client is installed yet, or the installed client's secret has +// already expired (or is within the same 5-minute buffer used by +// areCredentialsValid when loading persisted credentials). Without the +// expiry branch a provider that returns a finite client_secret_expires_at +// would have its install be a one-shot: once the secret lapsed, the +// 2-hour metadata refresh (the only path that re-runs this gate) saw +// clientID != "" and never re-registered, so every token exchange then +// failed invalid_client with no self-heal (R180). +func (t *TraefikOidc) dcrRegistrationNeeded() bool { + t.metadataMu.RLock() + hasClient := t.clientID != "" + exp := t.clientSecretExpiresAt + t.metadataMu.RUnlock() + if !hasClient { + return true + } + if exp <= 0 { + return false // non-expiring secret → keep the installed client + } + return time.Now().Add(5 * time.Minute).After(time.Unix(exp, 0)) +} + // performDynamicClientRegistration performs automatic client registration with the OIDC provider func (t *TraefikOidc) performDynamicClientRegistration() { t.logger.Info("Dynamic Client Registration enabled - registering client with provider") @@ -702,6 +775,10 @@ func (t *TraefikOidc) performDynamicClientRegistration() { t.dcrConfig, t.providerURL, ) + // Feed the merged runtime auth scopes so registration may advertise + // them (buildRegistrationRequest) for IdPs that only grant + // registered scopes (R124). + t.dynamicClientRegistrar.scopes = append([]string(nil), t.scopes...) // Set up storage backend for credentials persistence if t.dcrConfig.PersistCredentials { @@ -717,8 +794,13 @@ func (t *TraefikOidc) performDynamicClientRegistration() { } } - // Get registration endpoint (from metadata or config override) + // Get registration endpoint (from metadata or config override). Guarded + // by metadataMu because updateMetadataEndpoints (refresh loop / recovery + // goroutine) writes registrationURL under metadataMu.Lock concurrently + // (R117 data race otherwise). + t.metadataMu.RLock() registrationEndpoint := t.registrationURL + t.metadataMu.RUnlock() if t.dcrConfig.RegistrationEndpoint != "" { registrationEndpoint = t.dcrConfig.RegistrationEndpoint } @@ -737,9 +819,41 @@ func (t *TraefikOidc) performDynamicClientRegistration() { t.metadataMu.Lock() t.clientID = resp.ClientID t.clientSecret = resp.ClientSecret + t.clientSecretExpiresAt = resp.ClientSecretExpiresAt + // The IdP provisioned this client according to the auth method requested + // at registration (ClientMetadata.TokenEndpointAuthMethod, RFC 7591). + // Point the runtime at the SAME method, or token exchanges will + // authenticate with a method the registered client does not support + // and fail silently (R157). The IdP is authoritative here, so this + // overrides any clientAuthMethod inherited from the static config. + if t.dynamicClientRegistrar != nil { + t.clientAuthMethod = t.dynamicClientRegistrar.EffectiveTokenEndpointAuthMethod() + } if t.audience == "" { t.audience = resp.ClientID // Default audience to client ID } + // R162: DCR can provision a client whose method is private_key_jwt + // (RFC 7591 ClientMetadata.TokenEndpointAuthMethod) even when the + // static config did not set ClientAuthMethod=private_key_jwt. In that + // case no ClientAssertionSigner was built at construction and every + // token exchange silently fell back to client_secret (empty for a + // secretless private_key_jwt client) and failed invalid_client. + // Reconcile: if the effective method is private_key_jwt but no + // signer exists, build one from the static key material, and log + // loudly (rather than silently) if that material is missing. + // Written while holding metadataMu so it matches every reader of + // t.clientAssertion (clientCredentials) which snapshots under + // metadataMu.RLock — writing it after Unlock was a lock-mismatch + // data race with the request path (R180). + if t.clientAuthMethod == "private_key_jwt" && t.clientAssertion == nil { + if t.dcrClientAssertionBuilder == nil { + t.logger.Errorf("DCR registered a private_key_jwt client but no client-assertion signer builder is configured; token exchanges will fail") + } else if signer, err := t.dcrClientAssertionBuilder(); err != nil { + t.logger.Errorf("DCR registered a private_key_jwt client but no client-assertion signer could be built: %v", err) + } else { + t.clientAssertion = signer + } + } t.metadataMu.Unlock() t.logger.Infof("Dynamic Client Registration successful - client_id: %s", resp.ClientID) diff --git a/per_source_ratelimit.go b/per_source_ratelimit.go new file mode 100644 index 0000000..3a681de --- /dev/null +++ b/per_source_ratelimit.go @@ -0,0 +1,120 @@ +package traefikoidc + +import ( + "net" + "net/http" + "strings" + "sync" + "time" + + "golang.org/x/time/rate" +) + +// perSourceAuthEntry is one source's throttling state, retained briefly +// after last use then evicted so a handful of spoofed X-Forwarded-For +// values can't grow the map unboundedly. +type perSourceAuthEntry struct { + limiter *rate.Limiter + lastSeen time.Time +} + +// perSourceAuthLimiter rate-limits OIDC authentication events per client +// source (external IP). Internal (private / loopback / link-local / +// unspecified) sources are trusted and never throttled, so a reverse +// proxy or in-cluster caller is unbounded while a single external host +// roughly generating many authorize callbacks or login initiations can't +// degrade the shared token-origin token bucket for every user (R185). +type perSourceAuthLimiter struct { + mu sync.Mutex + limit rate.Limit + burst int + entries map[string]*perSourceAuthEntry + lastSweep time.Time +} + +func newPerSourceAuthLimiter(perMinute int) *perSourceAuthLimiter { + if perMinute <= 0 { + return nil // disabled + } + return &perSourceAuthLimiter{ + limit: rate.Limit(float64(perMinute) / 60.0), + burst: perMinute, + entries: make(map[string]*perSourceAuthEntry), + lastSweep: time.Now(), + } +} + +// allow reports whether the request's source may proceed (true = allowed). +func (l *perSourceAuthLimiter) allow(req *http.Request) bool { + ip := sourceIP(req) + if ip == "" { + return true // cannot determine source; do not block + } + if isInternalSource(ip) { + // Trusted source (loopback / RFC 1918 / ULA / link-local / + // unspecified): a reverse proxy or in-cluster caller, never + // throttled. + return true + } + now := time.Now() + l.mu.Lock() + defer l.mu.Unlock() + if now.Sub(l.lastSweep) >= time.Minute { + l.sweep(now) + } + e := l.entries[ip] + if e == nil { + e = &perSourceAuthEntry{limiter: rate.NewLimiter(l.limit, l.burst)} + l.entries[ip] = e + } + e.lastSeen = now + return e.limiter.Allow() +} + +// sweep evicts entries idle for over 10 minutes to bound memory. +func (l *perSourceAuthLimiter) sweep(now time.Time) { + for k, e := range l.entries { + if now.Sub(e.lastSeen) > 10*time.Minute { + delete(l.entries, k) + } + } + l.lastSweep = now +} + +// sourceIP returns the client source IP, preferring the first +// X-Forwarded-For value (original client in a proxy chain behind Traefik) +// with RemoteAddr as fallback. Returns "" if none is parseable. +func sourceIP(req *http.Request) string { + if req == nil { + return "" + } + if xff := req.Header.Get("X-Forwarded-For"); xff != "" { + if i := strings.IndexByte(xff, ','); i >= 0 { + xff = xff[:i] + } + if ip := net.ParseIP(strings.TrimSpace(xff)); ip != nil { + return ip.String() + } + } + if req.RemoteAddr != "" { + host, _, err := net.SplitHostPort(req.RemoteAddr) + if err != nil { + host = req.RemoteAddr + } + if ip := net.ParseIP(strings.TrimSpace(host)); ip != nil { + return ip.String() + } + } + return "" +} + +// isInternalSource reports whether ip is loopback, private (RFC 1918 / +// ULA), link-local, or unspecified — i.e. a trusted in-cluster source. +func isInternalSource(ip string) bool { + p := net.ParseIP(ip) + if p == nil { + return false + } + return p.IsLoopback() || p.IsPrivate() || p.IsLinkLocalUnicast() || + p.IsLinkLocalMulticast() || p.IsUnspecified() +} diff --git a/settings.go b/settings.go index a7bed05..b622099 100644 --- a/settings.go +++ b/settings.go @@ -5,12 +5,14 @@ import ( "fmt" "io" "log" + "math" "net" "net/http" "net/url" "os" "strconv" "strings" + "time" ) // TemplatedHeader represents a custom HTTP header with a templated value. @@ -75,9 +77,16 @@ type Config struct { // IdPs do not expose RT TTL on the wire, so this is intentionally a // conservative heuristic; tune to match your provider configuration. // Default 21600 (6h). Set to 0 to disable the check. - MaxRefreshTokenAgeSeconds int `json:"maxRefreshTokenAgeSeconds"` - SessionMaxAge int `json:"sessionMaxAge"` - RateLimit int `json:"rateLimit"` + MaxRefreshTokenAgeSeconds int `json:"maxRefreshTokenAgeSeconds"` + SessionMaxAge int `json:"sessionMaxAge"` + RateLimit int `json:"rateLimit"` + // PerSourceLoginRateLimit throttles OIDC authentication events + // (authorization-code callback + login initiation) per client source IP, + // for external (non-internal) sources only. Internal / loopback / + // link-local sources (reverse proxies, in-cluster callers) are trusted + // and never throttled. Value = max auth events per minute per external + // source; 0 disables. See per_source_ratelimit.go (R185). + PerSourceLoginRateLimit int `json:"perSourceLoginRateLimit,omitempty"` OverrideScopes bool `json:"overrideScopes"` DisableReplayDetection bool `json:"disableReplayDetection,omitempty"` RequireTokenIntrospection bool `json:"requireTokenIntrospection,omitempty"` @@ -405,7 +414,39 @@ func createDefaultSecurityConfig() *SecurityHeadersConfig { // Returns: // - nil if the configuration is valid. // - An error describing the first validation failure encountered. +// +// ApplyEnvFallbacks applies environment variable values as fallbacks for +// empty top-level OIDC config fields. Mirrors RedisConfig.ApplyEnvFallbacks: +// the plugin dynamic configuration (Traefik) takes precedence, env vars only +// fill empty fields. The example deployment documents OIDC_CLIENT_ID / +// OIDC_CLIENT_SECRET / OIDC_PROVIDER_URL as optional fallbacks; previously +// those vars were documented but never read, so an operator following the +// docs got a silent no-op and then "providerURL is required" (R180). +func (c *Config) ApplyEnvFallbacks() { + if c.ClientID == "" { + if v := os.Getenv("OIDC_CLIENT_ID"); v != "" { + c.ClientID = v + } + } + if c.ClientSecret == "" { + if v := os.Getenv("OIDC_CLIENT_SECRET"); v != "" { + c.ClientSecret = v + } + } + if c.ProviderURL == "" { + if v := os.Getenv("OIDC_PROVIDER_URL"); v != "" { + c.ProviderURL = v + } + } +} + func (c *Config) Validate() error { + // Apply OIDC env fallbacks BEFORE the required-field checks: an + // operator supplying these via env (the documented pattern) has empty + // fields at config load, so without this the "providerURL is + // required" / "clientID is required" guards would fire and reject a + // valid env-supplied configuration (R180). + c.ApplyEnvFallbacks() // Validate provider URL if c.ProviderURL == "" { return fmt.Errorf("providerURL is required") @@ -430,6 +471,21 @@ func (c *Config) Validate() error { return fmt.Errorf("callbackURL must not contain a query string or fragment") } + // LogoutURL is matched by exact request-path comparison + // (logout.go handleLogout) and used to build the RP-initiated + // end-session URL. An empty value is allowed (defaults to + // CallbackURL + "/logout"), but a provided value must be a bare + // path: a query string or fragment, or a relative URI, silently + // breaks RP-initiated logout with no runtime error (R134). + if c.LogoutURL != "" { + if !strings.HasPrefix(c.LogoutURL, "/") { + return fmt.Errorf("logoutURL must start with /") + } + if strings.ContainsAny(c.LogoutURL, "?#") { + return fmt.Errorf("logoutURL must not contain a query string or fragment") + } + } + // Validate client credentials. With Dynamic Client Registration (RFC 7591) // enabled, clientID (and typically clientSecret) are not known at // construction time — the provider provisions them at registration. Skip @@ -472,8 +528,12 @@ func (c *Config) Validate() error { // Reject the validated-but-broken config where introspection is enabled but // no client secret is supplied (e.g. private_key_jwt with no secret), which // would otherwise send empty Basic credentials and fail every introspection. - if c.RequireTokenIntrospection && c.ClientSecret == "" { - return fmt.Errorf("clientSecret is required when requireTokenIntrospection is enabled (introspection authenticates via client_secret_basic)") + // allowOpaqueTokens also routes opaque tokens through the same + // client_secret_basic introspection, so it must be covered too (R117). + // Under DCR the secret is provisioned at runtime (main.go), so like the + // clientID check above it is skipped when DCR is enabled (R135). + if (c.RequireTokenIntrospection || c.AllowOpaqueTokens) && c.ClientSecret == "" && !(dcrEnabled && c.ClientID == "") { + return fmt.Errorf("clientSecret is required when requireTokenIntrospection or allowOpaqueTokens is enabled (opaque-token introspection authenticates via client_secret_basic)") } // Validate session encryption key @@ -532,11 +592,26 @@ func (c *Config) Validate() error { if c.RefreshGracePeriodSeconds < 0 { return fmt.Errorf("refreshGracePeriodSeconds cannot be negative") } + // Upper bound (mirroring the SessionMaxAge guard below): this seconds + // value is converted to time.Duration at startup + // (time.Duration(x) * time.Second). Values above ~9.22e9 seconds + // (~292 years) overflow int64 nanoseconds and wrap negative, pulling + // the refresh threshold into the past and forcing perpetual refresh / + // re-auth. Reject at startup rather than fail at runtime. + if int64(c.RefreshGracePeriodSeconds) > math.MaxInt64/int64(time.Second) { + return fmt.Errorf("refreshGracePeriodSeconds is too large; must be at most %d seconds", math.MaxInt64/int64(time.Second)) + } // Validate refresh-token max-age heuristic if c.MaxRefreshTokenAgeSeconds < 0 { return fmt.Errorf("maxRefreshTokenAgeSeconds cannot be negative") } + // Upper bound (same overflow class): a wrapped-negative max-age makes + // every session with an issued_at look refresh-token-expired (mass + // re-auth). Reject at startup. + if int64(c.MaxRefreshTokenAgeSeconds) > math.MaxInt64/int64(time.Second) { + return fmt.Errorf("maxRefreshTokenAgeSeconds is too large; must be at most %d seconds", math.MaxInt64/int64(time.Second)) + } // Validate session max age. A negative value would make every session // "older than maxAge" immediately (time-since-createdAt > negative), @@ -544,6 +619,14 @@ func (c *Config) Validate() error { if c.SessionMaxAge < 0 { return fmt.Errorf("sessionMaxAge cannot be negative") } + // Upper bound: sessionMaxAge is converted to a time.Duration as + // SessionMaxAge * time.Second (main.go). Values above ~9.22e9 seconds + // (~292 years) overflow int64 nanoseconds and wrap negative, which the + // growth comparison would then treat as "always expired": an instant + // mass lockout. Reject at startup rather than fail at runtime. + if int64(c.SessionMaxAge) > math.MaxInt64/int64(time.Second) { + return fmt.Errorf("sessionMaxAge is too large; must be at most %d seconds", math.MaxInt64/int64(time.Second)) + } // Validate audience if specified if c.Audience != "" { @@ -568,13 +651,42 @@ func (c *Config) Validate() error { } } + // EnableBearerAuth requires an explicit Audience: it cannot default to + // clientID (that path accepts ID tokens, a spoofing vector for the + // bearer M2M path). Enforced here in Validate so a standalone call + // fails closed, matching the NewWithContext constructor check (R134). + if c.EnableBearerAuth && c.Audience == "" { + return fmt.Errorf("EnableBearerAuth=true requires Audience to be set explicitly (cannot default to clientID — that path accepts ID tokens)") + } + // Validate Redis configuration if provided - if c.Redis != nil && c.Redis.Enabled { - if err := c.Redis.Validate(); err != nil { - return fmt.Errorf("redis configuration error: %w", err) + if c.Redis != nil { + // Apply env fallbacks + defaults BEFORE validation: an operator + // enabling Redis via REDIS_ENABLED (the documented pattern) has + // Enabled==false at config load, so without this the guards + // within RedisConfig.Validate (address required, no negative + // fields) would be skipped, and the backend would then be built + // with an empty address and silently degrade to memory-only at + // runtime (R123). + c.Redis.ApplyEnvFallbacks() + c.Redis.ApplyDefaults() + if c.Redis.Enabled { + if err := c.Redis.Validate(); err != nil { + return fmt.Errorf("redis configuration error: %w", err) + } } } + // Validate HSTS max-age when HSTS is enabled. RFC 6797 s.6.1: a + // max-age of 0 directs the UA to REMOVE the host's HSTS policy, + // silently disabling the operator's intended protection; negative is + // invalid. The header emitter (GetSecurityHeadersApplier) uses the + // configured value verbatim, so a non-positive value must be + // rejected here rather than emitted as max-age=0 (R123). + if c.SecurityHeaders != nil && c.SecurityHeaders.StrictTransportSecurity && c.SecurityHeaders.StrictTransportSecurityMaxAge <= 0 { + return fmt.Errorf("strictTransportSecurityMaxAge must be positive when strictTransportSecurity is enabled") + } + // Validate headers configuration for template security. Header templates may // emit the built-in claims plus any operator-configured AllowedClaims. allowedClaims := claimsWhitelist(c.AllowedClaims) @@ -582,6 +694,12 @@ func (c *Config) Validate() error { if header.Name == "" { return fmt.Errorf("header name cannot be empty") } + // An HTTP field-name must be an RFC 7230 token; anything else + // (space, tab, ':', controls) would be written verbatim by Go's + // server into every response line, corrupting it (R148). + if strings.ContainsAny(header.Name, " \t:()<>@,;\\\"/[]?={}") || strings.ContainsAny(header.Name, "\r\n\x00") { + return fmt.Errorf("header name %q contains characters invalid in an HTTP field name", header.Name) + } if header.Value == "" { return fmt.Errorf("header value template cannot be empty") } @@ -621,6 +739,78 @@ func (c *Config) Validate() error { } } + // SecurityHeader CORS lists and custom header values are emitted + // verbatim into response headers (GetSecurityHeadersApplier); a CR/LF + // would split or corrupt the header (R148). Static header template + // values are already checked above; guard these string lists too. + checkNoCRLF := func(kind, val string) error { + if strings.ContainsAny(val, "\r\n\x00") { + return fmt.Errorf("%s %q contains CR/LF/NUL, which would split the response header", kind, val) + } + return nil + } + if c.SecurityHeaders != nil { + for _, v := range c.SecurityHeaders.CORSAllowedMethods { + if err := checkNoCRLF("CORSAllowedMethods entry", v); err != nil { + return err + } + } + for _, v := range c.SecurityHeaders.CORSAllowedHeaders { + if err := checkNoCRLF("CORSAllowedHeaders entry", v); err != nil { + return err + } + } + for _, v := range c.SecurityHeaders.CORSAllowedOrigins { + if err := checkNoCRLF("CORSAllowedOrigins entry", v); err != nil { + return err + } + } + for name, value := range c.SecurityHeaders.CustomHeaders { + if err := checkNoCRLF("CustomHeaders value", value); err != nil { + return err + } + if err := checkNoCRLF("CustomHeaders name", name); err != nil { + return err + } + } + } + + // Scopes flow verbatim into the OIDC authorize request (buildAuthURL). + // An empty scope element (e.g. a trailing comma in a Traefik label + // list) or whitespace-bearing scope is rejected by strict OIDC + // providers, and an override that drops the mandatory "openid" scope + // makes every login fail at runtime (OIDC Core 3.1.2.1). Reject at + // startup instead (R148). + seenOpenID := false + for i, sc := range c.Scopes { + s := strings.TrimSpace(sc) + if s == "" { + return fmt.Errorf("scope %d must not be empty", i) + } + if strings.ContainsAny(s, " \t\r\n") { + return fmt.Errorf("scope %d must not contain whitespace: %q", i, sc) + } + if s == "openid" { + seenOpenID = true + } + } + if c.OverrideScopes && !seenOpenID { + return fmt.Errorf("overrideScopes is set but the mandatory \"openid\" scope is missing; add it to scopes") + } + + // cookieDomain flows verbatim into the session cookie's Domain option; + // an invalid value (a full URL, whitespace, or characters illegal in a + // cookie domain) makes every browser reject the Set-Cookie, surfacing + // only as a login loop at runtime. Reject at startup instead. + if c.CookieDomain != "" { + if u, err := url.Parse(c.CookieDomain); err == nil && (u.Scheme != "" || u.Host != "") { + return fmt.Errorf("cookieDomain must be a bare host name or domain (not a full URL): %q", c.CookieDomain) + } + if strings.ContainsAny(c.CookieDomain, " \t\r\n;,") { + return fmt.Errorf("cookieDomain contains characters invalid in a cookie domain: %q", c.CookieDomain) + } + } + return nil } @@ -1248,13 +1438,17 @@ func isOriginAllowed(origin string, allowedOrigins []string) bool { if strings.Contains(allowed, "*") { if strings.HasPrefix(allowed, "https://*.") { domain := strings.TrimPrefix(allowed, "https://*.") - if strings.HasSuffix(origin, "."+domain) || origin == "https://"+domain { + // The scheme part of the wildcard must be honored: an https + // wildcard must not admit an http sibling (which, with + // AllowCredentials, would be reflected and credentialed) + // (R136). + if (strings.HasPrefix(origin, "https://") && strings.HasSuffix(origin, "."+domain)) || origin == "https://"+domain { return true } } if strings.HasPrefix(allowed, "http://*.") { domain := strings.TrimPrefix(allowed, "http://*.") - if strings.HasSuffix(origin, "."+domain) || origin == "http://"+domain { + if (strings.HasPrefix(origin, "http://") && strings.HasSuffix(origin, "."+domain)) || origin == "http://"+domain { return true } } diff --git a/types.go b/types.go index 1e94984..2b9ae92 100644 --- a/types.go +++ b/types.go @@ -116,6 +116,7 @@ type TraefikOidc struct { introspectionCache CacheInterface initComplete chan struct{} limiter *rate.Limiter + perSourceLimiter *perSourceAuthLimiter // optional per-external-source auth throttle (R185), nil when disabled headerTemplates map[string]*template.Template sessionManager *SessionManager tokenCleanupStopChan chan struct{} @@ -160,7 +161,20 @@ type TraefikOidc struct { clientSecret string clientAuthMethod string clientAssertion *ClientAssertionSigner - registrationURL string + // clientSecretExpiresAt is the RFC 7591 userinfo client_secret_expires_at + // (unix seconds) returned at DCR registration, or 0 when the IdP + // grants a non-expiring secret. Guarded by metadataMu like the other + // DCR-installed credential fields; used by dcrRegistrationNeeded to + // re-register a client whose secret has lapsed (R180). + clientSecretExpiresAt int64 + + // dcrClientAssertionBuilder lazily builds a client-assertion signer + // from the static config's key material. Used when DCR provisions a + // private_key_jwt client but no signer was built at construction + // (because the static ClientAuthMethod was not private_key_jwt); see + // R162 / updateMetadataEndpoints. + dcrClientAssertionBuilder func() (*ClientAssertionSigner, error) + registrationURL string // configRevocationURL, configEndSessionURL, and configIntrospectionURL // hold operator-configured endpoint overrides (Config.RevocationURL, // Config.OIDCEndSessionURL, Config.IntrospectionURL). They are set once at