2 Commits

Author	SHA1	Message	Date
lukaszraczylo	546ceb949c	security: remediate audit findings (ranks 1–16 + 22 Lows) + yaegi load validation (#144) ... * fix(security): encrypt session cookies + fail closed on invalid config Batch 1 of security audit remediation (ranks 1, 2, 6). - session.go: derive independent HMAC + AES-256 keys via stdlib HKDF-SHA256 and build the gorilla cookie store with both, so session cookies are now encrypted, not merely signed. The single-key store previously left OIDC access/refresh/ID tokens recoverable from raw cookie bytes. Cookie format changes, so existing sessions are invalidated on deploy (one-time re-login). - main.go: call config.Validate() at construction and error out on failure, instead of silently substituting a public hardcoded encryption key for empty/short keys (which allowed session forgery). The yaegi analyzer passes via .traefik.yml testData. - settings.go: isValidSecureURL permits plaintext HTTP for loopback hosts only (RFC 8252); remote providers must still use HTTPS. - tests: complete configs that did not satisfy Validate(); add regression tests in security_audit_fixes_test.go. Configs below documented minimums (rateLimit < 10, key < 32 chars) are now rejected at startup (fail closed). * fix(security): validate discovered OIDC endpoints + pin introspection host Batch 2 of security audit remediation (ranks 3, 4). - url_helpers.go: add validateDiscoveredEndpoint, an SSRF screen for endpoints taken from the provider discovery document (jwks_uri, token, authorization, revocation, end_session, introspection, registration). Blocks link-local (cloud metadata 169.254.169.254), multicast, unspecified and private addresses (unless allowPrivateIPAddresses); blocks loopback unless the configured providerURL is itself loopback (dev/test). Cross-domain JWKS hosts (e.g. Google) stay allowed. Add sameHost helper. - main.go: updateMetadataEndpoints screens every discovered endpoint and blanks any that fail (fail closed downstream). The introspection endpoint carries the client secret via HTTP Basic, so it is additionally pinned to the providerURL host to stop a poisoned discovery document exfiltrating the secret to an attacker-controlled host. - tests: regression tests for the SSRF guard and the host pin. * fix(security): close open redirects + anchor excluded-URL matching Batch 3 of security audit remediation (ranks 5, 14, 15). - auth_flow.go: run the stored incoming path through normalizeLogoutPath before using it as the post-login redirect, so //evil.com and /\evil.com payloads become host-relative (open-redirect, rank 5). - url_helpers.go: excluded-URL matching is anchored at a natural boundary (exact, sub-path "/", or file extension "."), so excluding "/public" no longer also bypasses auth on "/publicsecret"; "/favicon" still matches "/favicon.ico" (rank 14). - internal/utils: X-Forwarded-Host is sanitized (first value only; reject CRLF/whitespace/multi-value) before building redirect URLs (rank 15). - helpers.go: the logout redirect used when there is no provider end-session endpoint is host-relative, never an absolute URL derived from the client-controllable request host (logout open-redirect, rank 15). - tests: update two logout cases that asserted the old absolute redirect; add regression tests. * fix(security): reject unverified Azure tokens; fix transport TLS reuse Batch 4 of security audit remediation (ranks 7, 11). - token_validation_rs.go: an Azure nonce-bearing access token that cannot be cryptographically verified no longer returns "authenticated" when there is no ID token to corroborate it; it refreshes (if possible) or forces re-authentication instead of failing open (rank 7). - http_client_pool.go: the at-limit transport-reuse path now takes the write lock before mutating refCount (fixes a data race) and only reuses a transport whose TLS settings (CA pool + InsecureSkipVerify) match the caller's, never one with a different trust store; if none matches it returns nil so the caller falls back to a verifying default transport (rank 11). - tests: add a transport-pool TLS-isolation regression test. * fix(security): stop logging templated header values (token leak) Batch 5 of security audit remediation (rank 16). middleware.go: templated downstream headers commonly carry the access token (e.g. "Authorization: Bearer {{.AccessToken}}"). The debug log line printed the full header value, leaking credentials into logs. Log the header name and byte length instead. * fix(security): cache-key collision, cache-config divergence, fleet cleanup Batch 6 of security audit remediation (ranks 9, 10, 12). - token_manager.go: detectTokenType keys its cache on a SHA-256 hash of the full token instead of the first 32 chars (which are only the base64url JWT header). Distinct tokens sharing alg+kid no longer collide and get mis-classified (rank 10). - cache_manager.go: the process-global cache manager is initialized once and shared across plugin instances; it now logs a loud warning when a later instance requests a different explicit Redis backend that is silently ignored, surfacing the cross-instance state-isolation hazard (rank 9). - singleton_resources.go / main.go / utilities.go: track a process-global live instance count; the shared singleton-token-cleanup task is stopped only when the LAST instance shuts down, so one instance's Close() (e.g. a config reload) no longer kills cleanup for surviving instances (rank 12). - tests: update TestDetectTokenTypeCaching for the new key; add regression tests. * fix(security): bound introspection cache + cookie lifetime to config Batch 7 of security audit remediation (ranks 8, 13). - token_introspection.go: when requireTokenIntrospection is enabled, cap the positive introspection-result cache at 30s (instead of 5m) so a token revoked at the provider stops passing within ~30s, matching the operator's near-real-time revocation expectation (rank 8). - session.go: bind the cookie store's MaxAge to the configured sessionMaxAge, so the cookie codec's cryptographic timestamp validity is no longer fixed at gorilla's 30-day default; a stolen cookie is valid only for the configured session lifetime (rank 13). - tests: add a cookie-lifetime regression test. * fix(security): low-severity hardening (cache, DoS caps, PKCE, throttle) Batch 8 of security audit remediation — low severity (ranks 24, 25, 27, 29, 31, 36, 37, 41, 45, 46, 49). - universal_cache.go: updateLocalCache updates an existing key in place instead of orphaning its LRU element and double-counting currentSize/currentMemory (rank 36 — the only production-reachable bug in this batch). - jwk.go / metadata_cache.go / token_introspection.go: bound response bodies with io.LimitReader (1 MiB) to prevent memory exhaustion from a hostile or buggy provider (ranks 24, 25). - jwk.go: skip JWKs not usable for signature verification (use != sig, or key_ops without "verify") when building the key set (rank 49). - auth_flow.go: fail closed at the callback when PKCE is enabled but the code verifier is missing, instead of silently dropping it (rank 27). - utilities.go / main.go: match allowedUserDomains case-insensitively (rank 31). - bearer_auth.go: a single success no longer wipes an active per-IP penalty; the counter resets only when no penalty is in effect (rank 29). - main.go: handle (not discard) the NewSessionManager error (rank 37). - error_recovery.go: take a write lock in isServiceDegraded (it deletes from a map); compare retryable-error substrings case-insensitively (ranks 45, 46). - singleton_resources.go: bind the generic-cache cleanup goroutine to the resource-manager shutdown channel so it cannot outlive its owner (rank 41). - tests: update the bearer throttle test to the corrected penalty semantics. * fix(security): header sanitization, issuer pinning, fail-closed paths Batch 9 of security audit remediation (ranks 18, 19, 20, 21, 22, 30, 33, 34). - middleware.go / bearer_auth.go: sanitize claim-derived values on the cookie auth path before injecting them into downstream headers. Drop group/role and identifier values containing control chars, bidi-override runes, or the , ; = delimiters (a comma would inject phantom entries into X-User-Groups); reject control/bidi/over-length in rendered templated header output (but permit , ; = in free-form values such as a bearer token). The bearer path already sanitized; the cookie path did not (ranks 33, 34). - main.go / metadata_cache.go: pin the discovered issuer to the configured provider host (sameHost) and refuse/never-cache a mismatch, so a poisoned discovery document cannot redefine the JWT trust anchor (ranks 21, 22). - token_introspection.go: when a distinct API audience is configured, fail closed on a missing or mismatched introspection audience; aud parsed as string-or-array per RFC 7662 (rank 19). - logout.go: front-channel logout requires a matching issuer; an empty iss is rejected (blocks unauthenticated forced-logout via a known sid) (rank 30). - token_validation_rs.go: an opaque access token with no ID token and no successful introspection fails closed (re-auth) instead of authenticating (ranks 18, 20). - tests: realistic same-host provider mocks; regression tests for the header sanitization distinction and the fail-closed paths. * chore(security): remove unwired dead code with latent footguns Batch 10 of security audit remediation — delete confirmed-dead, unwired subsystems (ranks 26, 35, 50). None had a production caller (grep-verified); removal eliminates the latent footguns and ~2.1k lines of dead code. - token_validator.go (deleted): an unused *TokenValidator whose validateJWT set Valid=true with NO signature verification — a severe footgun if ever wired (rank 50). The wired RS-aware validators are unaffected. - security_monitoring.go (deleted): an unused *SecurityMonitor / ExtractClientIP that trusted spoofable X-Forwarded-For / X-Real-IP. The live bearer throttle uses clientIPForBearer (RemoteAddr-only), unchanged (rank 35). - dynamic_client_registration.go: removed the RFC 7592 management methods (Update/Read/DeleteClientRegistration) that dereferenced an attacker- influenced RegistrationClientURI with the registration token attached and no HTTPS/SSRF gate, and had no callers. The wired RFC 7591 RegisterClient and credential-store helpers are kept (rank 26). - tests: removed the tests covering the deleted code. * chore: add Makefile with yaegi load validation No Makefile existed. The new `yaegi-validate` target interprets the plugin under the yaegi interpreter the same way Traefik loads it, catching yaegi-only incompatibilities (unsupported stdlib symbols, reflection edge cases) that the native `go build` / `go test` toolchain does not. Importing the plugin forces yaegi to interpret every file plus its vendored deps; CreateConfig + New exercise the instantiation path. - cmd/yaegicheck/main.go: the load driver, marked //go:build ignore so it is excluded from `go build ./...` (avoids VCS-stamping a main binary, which fails in git-worktree layouts) yet is run explicitly by yaegi. - Makefile: build / fmt / vet / lint / test / vendor / yaegi-validate / check targets; `make check` runs vet + tests + yaegi-validate. Verified: `make yaegi-validate` passes on this branch — the HKDF cookie encryption, net-based endpoint validation, and claim sanitizers all interpret and instantiate cleanly under yaegi. * ci: bump workflow Go toolchain to 1.25; pin yaegi-validate to v0.16.1 Traefik v3.7.1 (the deployed version) is built with `go 1.25.0`, so the PR and release workflows now use Go 1.25.x to match the toolchain Traefik uses. Important distinction: the CI Go version is the build TOOLCHAIN. The plugin's actual interpreter-compatibility ceiling is the yaegi version Traefik bundles (v0.16.1, which declares go 1.21 and ships a ~Go 1.22 stdlib symbol surface), NOT the CI Go version. That ceiling is enforced by `make yaegi-validate` plus the go.mod language directive — e.g. it is why HKDF is hand-rolled with hmac+sha256 rather than Go 1.24's crypto/hkdf, which yaegi v0.16.1 lacks. Also pin Makefile YAEGI_VERSION to v0.16.1 (what Traefik v3.7.1 vendors) so yaegi-validate exercises the real deployed interpreter instead of @latest, which could pass on a newer yaegi that supports symbols the deployed one does not. * docs: align README/CONFIGURATION with branch behavior changes - excludedURLs: documented as segment/extension-boundary matching (was "prefix-matched") — "/public" no longer also matches "/publicsecret" (rank 14). - Front-channel logout now requires a matching `iss`; requests without one are rejected with 400 (rank 30). - Add an "Upgrading from an earlier release" note: session cookies are now AES-256 encrypted with lifetime tracking sessionMaxAge (one-time re-login on upgrade), and invalid configuration (rateLimit < 10, key < 32 bytes, missing callbackURL, non-HTTPS remote providerURL) now fails closed at startup. * fix: remove staticcheck-flagged unused functions; wire staticcheck into make check CI Static Analysis (standalone staticcheck) failed with U1000 "unused": - dynamic_client_registration.go: deleteCredentialsFromStore — its only caller was the RFC 7592 DeleteClientRegistration removed in the dead-code batch. - token_test.go: createTestJWTSimple — its only callers were the TokenValidator tests removed in the same batch. Both confirmed to have zero remaining callers and removed. build / vet / go test ./... / staticcheck ./... all green. The pre-commit hook runs golangci-lint, but CI runs standalone staticcheck (which flags U1000). Add a `staticcheck` Makefile target and include it in `make check` so this class of finding is caught locally before push. * fix(test): stabilize flaky TestWorkerPool_TaskPanic tasksFailed is incremented in the worker's deferred recover(), which runs after the panicking task's own defer wg.Done(). wg.Wait() could therefore return before the failure was recorded, so reading the counter immediately raced and flaked on slow CI runners. Poll until the failure lands (2s budget) instead. Verified 200x plain + 50x under -race/GOMAXPROCS=1.	2026-05-30 14:10:32 +01:00
lukaszraczylo	c474bbafd6	Cleanup [dec2025] (#101) ... * Cleanup excessive comments. * Remove leftovers hanging around from previous refactor * Improve test coverage	2025-12-09 01:38:02 +00:00