fix: keep orphaned OIDC PKCE cookies off every request

internal/ir/xds.go

@@ -1463,7 +1463,7 @@ type OIDC struct {
 	// CookieSuffix will be added to the name of the cookies set by the oauth filter.
 	// Adding a suffix avoids multiple oauth filters from overwriting each other's cookies.
 	// These cookies are set by the oauth filter, including: AccessToken,
-	// OauthHMAC, OauthExpires, IdToken, and RefreshToken.
+	// OauthHMAC, OauthExpires, IdToken, RefreshToken, OauthNonce and CodeVerifier.
 	CookieSuffix string `json:"cookieSuffix,omitempty"`
 
 	// CookieNameOverrides can optionally override the generated name of the cookies set by the oauth filter.

internal/xds/translator/oidc.go

@@ -8,6 +8,7 @@ package translator
 import (
 	"errors"
 	"fmt"
+	"regexp"
 	"strings"
 
 	corev3 "github.com/envoyproxy/go-control-plane/envoy/config/core/v3"
@@ -26,6 +27,11 @@ import (
 	"github.com/envoyproxy/gateway/internal/xds/types"
 )
 
+// cookiePathPattern mirrors the constraint the Envoy oauth2 proto puts on
+// CookieConfig.path. It is stricter than what a URL path allows - "," and ";"
+// for example are legal RFC 3986 sub-delims but are rejected here.
+var cookiePathPattern = regexp.MustCompile(`^$|^/[^\x00-\x1f\x7f ",;<>\\]*$`)
+
 func init() {
 	registerHTTPFilter(&oidc{})
 }
@@ -170,6 +176,7 @@ func oauth2Config(securityFeatures *ir.SecurityFeatures) (*oauth2v3.OAuth2PerRou
 					IdToken:      fmt.Sprintf("IdToken-%s", oidc.CookieSuffix),
 					RefreshToken: fmt.Sprintf("RefreshToken-%s", oidc.CookieSuffix),
 					OauthNonce:   fmt.Sprintf("OauthNonce-%s", oidc.CookieSuffix),
+					CodeVerifier: fmt.Sprintf("CodeVerifier-%s", oidc.CookieSuffix),
 				},
 			},
 			// every OIDC provider supports basic auth
@@ -260,24 +267,42 @@ func buildSameSite(config *egv1a1.OIDCCookieConfig) oauth2v3.CookieConfig_SameSi
 	}
 }
 
-// buildCookieConfigs translates the OIDC configuration from the US
+// buildCookieConfigs builds the attributes Envoy sets on the OAuth2 cookies.
 func buildCookieConfigs(oidc *ir.OIDC) *oauth2v3.CookieConfigs {
-	// If the user did not specify any custom cookie configurations at all, return the defaults.
-	if oidc.CookieConfig == nil || oidc.CookieConfig.SameSite == nil {
-		return nil
-	}
-
-	// Apply the user-defined SameSite policy for each cookie if it has been configured.
-	sameSite := buildSameSite(oidc.CookieConfig)
-	return &oauth2v3.CookieConfigs{
-		BearerTokenCookieConfig:  &oauth2v3.CookieConfig{SameSite: sameSite},
-		OauthHmacCookieConfig:    &oauth2v3.CookieConfig{SameSite: sameSite},
-		OauthExpiresCookieConfig: &oauth2v3.CookieConfig{SameSite: sameSite},
-		IdTokenCookieConfig:      &oauth2v3.CookieConfig{SameSite: sameSite},
-		RefreshTokenCookieConfig: &oauth2v3.CookieConfig{SameSite: sameSite},
-		OauthNonceCookieConfig:   &oauth2v3.CookieConfig{SameSite: sameSite},
-		CodeVerifierCookieConfig: &oauth2v3.CookieConfig{SameSite: sameSite},
-	}
+	// The nonce (CSRF) and PKCE code verifier cookies only carry state for an
+	// in-flight authorization flow, and Envoy only reads them back when it
+	// validates the callback from the authorization server. Scoping them to the
+	// redirect path keeps them off every other request: a flow that is started
+	// but never completed - a parallel request from a logged out browser, a user
+	// navigating away from the provider's login page - leaves its cookies behind
+	// until they expire, and at the default path "/" those orphans are sent on
+	// every request until then.
+	// A path Envoy refuses to accept on a cookie would fail xDS validation and take
+	// the whole route down with it, so fall back to leaving the path unset - Envoy
+	// then defaults it to "/", which is the behavior we had before scoping.
+	redirectPath := oidc.RedirectPath
+	if !cookiePathPattern.MatchString(redirectPath) {
+		redirectPath = ""
+	}
+
+	cookieConfigs := &oauth2v3.CookieConfigs{
+		OauthNonceCookieConfig:   &oauth2v3.CookieConfig{Path: redirectPath},
+		CodeVerifierCookieConfig: &oauth2v3.CookieConfig{Path: redirectPath},
+	}
+
+	// Apply the user-defined SameSite policy to each cookie if it has been configured.
+	if oidc.CookieConfig != nil && oidc.CookieConfig.SameSite != nil {
+		sameSite := buildSameSite(oidc.CookieConfig)
+		cookieConfigs.BearerTokenCookieConfig = &oauth2v3.CookieConfig{SameSite: sameSite}
+		cookieConfigs.OauthHmacCookieConfig = &oauth2v3.CookieConfig{SameSite: sameSite}
+		cookieConfigs.OauthExpiresCookieConfig = &oauth2v3.CookieConfig{SameSite: sameSite}
+		cookieConfigs.IdTokenCookieConfig = &oauth2v3.CookieConfig{SameSite: sameSite}
+		cookieConfigs.RefreshTokenCookieConfig = &oauth2v3.CookieConfig{SameSite: sameSite}
+		cookieConfigs.OauthNonceCookieConfig.SameSite = sameSite
+		cookieConfigs.CodeVerifierCookieConfig.SameSite = sameSite
+	}
+
+	return cookieConfigs
 }
 
 func buildDenyRedirectMatcher(oidc *ir.OIDC) []*routev3.HeaderMatcher {

internal/xds/translator/oidc_test.go

@@ -17,101 +17,114 @@ import (
 	"github.com/envoyproxy/gateway/internal/ir"
 )
 
+// expectedCookieConfigs returns the cookie configs EG emits for the given SameSite
+// policy: the nonce and code verifier cookies are always scoped to the redirect path,
+// every cookie carries the SameSite policy.
+func expectedCookieConfigs(sameSite oauth2v3.CookieConfig_SameSite, redirectPath string) *oauth2v3.CookieConfigs {
+	return &oauth2v3.CookieConfigs{
+		BearerTokenCookieConfig:  &oauth2v3.CookieConfig{SameSite: sameSite},
+		OauthHmacCookieConfig:    &oauth2v3.CookieConfig{SameSite: sameSite},
+		OauthExpiresCookieConfig: &oauth2v3.CookieConfig{SameSite: sameSite},
+		IdTokenCookieConfig:      &oauth2v3.CookieConfig{SameSite: sameSite},
+		RefreshTokenCookieConfig: &oauth2v3.CookieConfig{SameSite: sameSite},
+		OauthNonceCookieConfig:   &oauth2v3.CookieConfig{SameSite: sameSite, Path: redirectPath},
+		CodeVerifierCookieConfig: &oauth2v3.CookieConfig{SameSite: sameSite, Path: redirectPath},
+	}
+}
+
+// expectedRedirectPathOnlyCookieConfigs returns the cookie configs EG emits when the
+// user did not configure SameSite: only the two flow cookies are configured, so the
+// session cookies keep Envoy's defaults.
+func expectedRedirectPathOnlyCookieConfigs(redirectPath string) *oauth2v3.CookieConfigs {
+	return &oauth2v3.CookieConfigs{
+		OauthNonceCookieConfig:   &oauth2v3.CookieConfig{Path: redirectPath},
+		CodeVerifierCookieConfig: &oauth2v3.CookieConfig{Path: redirectPath},
+	}
+}
+
 func TestOIDCCookieConfigSameSite(t *testing.T) {
 	tests := []struct {
 		name   string
 		input  ir.OIDC
 		expect *oauth2v3.CookieConfigs
 	}{
 		{
-			name:   "defaults all cookie to unset/niul",
-			input:  ir.OIDC{},
-			expect: nil,
+			name:   "SameSite unset still scopes the flow cookies to the redirect path",
+			input:  ir.OIDC{RedirectPath: "/oauth2/callback"},
+			expect: expectedRedirectPathOnlyCookieConfigs("/oauth2/callback"),
 		},
 		{
 			name: "all cookie configs set to None",
 			input: ir.OIDC{
+				RedirectPath: "/oauth2/callback",
 				CookieConfig: &egv1a1.OIDCCookieConfig{
 					SameSite: new("None"),
 				},
 			},
-			expect: &oauth2v3.CookieConfigs{
-				BearerTokenCookieConfig:  &oauth2v3.CookieConfig{SameSite: oauth2v3.CookieConfig_NONE},
-				OauthHmacCookieConfig:    &oauth2v3.CookieConfig{SameSite: oauth2v3.CookieConfig_NONE},
-				OauthExpiresCookieConfig: &oauth2v3.CookieConfig{SameSite: oauth2v3.CookieConfig_NONE},
-				IdTokenCookieConfig:      &oauth2v3.CookieConfig{SameSite: oauth2v3.CookieConfig_NONE},
-				RefreshTokenCookieConfig: &oauth2v3.CookieConfig{SameSite: oauth2v3.CookieConfig_NONE},
-				OauthNonceCookieConfig:   &oauth2v3.CookieConfig{SameSite: oauth2v3.CookieConfig_NONE},
-				CodeVerifierCookieConfig: &oauth2v3.CookieConfig{SameSite: oauth2v3.CookieConfig_NONE},
-			},
+			expect: expectedCookieConfigs(oauth2v3.CookieConfig_NONE, "/oauth2/callback"),
 		},
 		{
 			name: "all cookie configs set to Lax",
 			input: ir.OIDC{
+				RedirectPath: "/oauth2/callback",
 				CookieConfig: &egv1a1.OIDCCookieConfig{
 					SameSite: new("Lax"),
 				},
 			},
-			expect: &oauth2v3.CookieConfigs{
-				BearerTokenCookieConfig:  &oauth2v3.CookieConfig{SameSite: oauth2v3.CookieConfig_LAX},
-				OauthHmacCookieConfig:    &oauth2v3.CookieConfig{SameSite: oauth2v3.CookieConfig_LAX},
-				OauthExpiresCookieConfig: &oauth2v3.CookieConfig{SameSite: oauth2v3.CookieConfig_LAX},
-				IdTokenCookieConfig:      &oauth2v3.CookieConfig{SameSite: oauth2v3.CookieConfig_LAX},
-				RefreshTokenCookieConfig: &oauth2v3.CookieConfig{SameSite: oauth2v3.CookieConfig_LAX},
-				OauthNonceCookieConfig:   &oauth2v3.CookieConfig{SameSite: oauth2v3.CookieConfig_LAX},
-				CodeVerifierCookieConfig: &oauth2v3.CookieConfig{SameSite: oauth2v3.CookieConfig_LAX},
-			},
+			expect: expectedCookieConfigs(oauth2v3.CookieConfig_LAX, "/oauth2/callback"),
 		},
 		{
 			name: "all cookie configs set to Strict",
 			input: ir.OIDC{
+				RedirectPath: "/oauth2/callback",
 				CookieConfig: &egv1a1.OIDCCookieConfig{
 					SameSite: new("Strict"),
 				},
 			},
-			expect: &oauth2v3.CookieConfigs{
-				BearerTokenCookieConfig:  &oauth2v3.CookieConfig{SameSite: oauth2v3.CookieConfig_STRICT},
-				OauthHmacCookieConfig:    &oauth2v3.CookieConfig{SameSite: oauth2v3.CookieConfig_STRICT},
-				OauthExpiresCookieConfig: &oauth2v3.CookieConfig{SameSite: oauth2v3.CookieConfig_STRICT},
-				IdTokenCookieConfig:      &oauth2v3.CookieConfig{SameSite: oauth2v3.CookieConfig_STRICT},
-				RefreshTokenCookieConfig: &oauth2v3.CookieConfig{SameSite: oauth2v3.CookieConfig_STRICT},
-				OauthNonceCookieConfig:   &oauth2v3.CookieConfig{SameSite: oauth2v3.CookieConfig_STRICT},
-				CodeVerifierCookieConfig: &oauth2v3.CookieConfig{SameSite: oauth2v3.CookieConfig_STRICT},
-			},
+			expect: expectedCookieConfigs(oauth2v3.CookieConfig_STRICT, "/oauth2/callback"),
 		},
 		{
 			name: "all cookie configs set to Disabled",
 			input: ir.OIDC{
+				RedirectPath: "/oauth2/callback",
 				CookieConfig: &egv1a1.OIDCCookieConfig{
 					SameSite: new("Disabled"),
 				},
 			},
-			expect: &oauth2v3.CookieConfigs{
-				BearerTokenCookieConfig:  &oauth2v3.CookieConfig{SameSite: oauth2v3.CookieConfig_DISABLED},
-				OauthHmacCookieConfig:    &oauth2v3.CookieConfig{SameSite: oauth2v3.CookieConfig_DISABLED},
-				OauthExpiresCookieConfig: &oauth2v3.CookieConfig{SameSite: oauth2v3.CookieConfig_DISABLED},
-				IdTokenCookieConfig:      &oauth2v3.CookieConfig{SameSite: oauth2v3.CookieConfig_DISABLED},
-				RefreshTokenCookieConfig: &oauth2v3.CookieConfig{SameSite: oauth2v3.CookieConfig_DISABLED},
-				OauthNonceCookieConfig:   &oauth2v3.CookieConfig{SameSite: oauth2v3.CookieConfig_DISABLED},
-				CodeVerifierCookieConfig: &oauth2v3.CookieConfig{SameSite: oauth2v3.CookieConfig_DISABLED},
-			},
+			expect: expectedCookieConfigs(oauth2v3.CookieConfig_DISABLED, "/oauth2/callback"),
 		},
 		{
 			name: "cookie config received invalid SameSite value will default to Disabled",
 			input: ir.OIDC{
+				RedirectPath: "/oauth2/callback",
 				CookieConfig: &egv1a1.OIDCCookieConfig{
 					SameSite: new("InvalidValue"),
 				},
 			},
-			expect: &oauth2v3.CookieConfigs{
-				BearerTokenCookieConfig:  &oauth2v3.CookieConfig{SameSite: oauth2v3.CookieConfig_DISABLED},
-				OauthHmacCookieConfig:    &oauth2v3.CookieConfig{SameSite: oauth2v3.CookieConfig_DISABLED},
-				OauthExpiresCookieConfig: &oauth2v3.CookieConfig{SameSite: oauth2v3.CookieConfig_DISABLED},
-				IdTokenCookieConfig:      &oauth2v3.CookieConfig{SameSite: oauth2v3.CookieConfig_DISABLED},
-				RefreshTokenCookieConfig: &oauth2v3.CookieConfig{SameSite: oauth2v3.CookieConfig_DISABLED},
-				OauthNonceCookieConfig:   &oauth2v3.CookieConfig{SameSite: oauth2v3.CookieConfig_DISABLED},
-				CodeVerifierCookieConfig: &oauth2v3.CookieConfig{SameSite: oauth2v3.CookieConfig_DISABLED},
+			expect: expectedCookieConfigs(oauth2v3.CookieConfig_DISABLED, "/oauth2/callback"),
+		},
+		{
+			name: "a custom redirect path scopes the flow cookies to that path",
+			input: ir.OIDC{
+				RedirectPath: "/auth/callback",
+				CookieConfig: &egv1a1.OIDCCookieConfig{
+					SameSite: new("Lax"),
+				},
 			},
+			expect: expectedCookieConfigs(oauth2v3.CookieConfig_LAX, "/auth/callback"),
+		},
+		{
+			// Envoy defaults an empty cookie path to "/".
+			name:   "an empty redirect path leaves the cookie path unset",
+			input:  ir.OIDC{},
+			expect: expectedRedirectPathOnlyCookieConfigs(""),
+		},
+		{
+			// Envoy rejects ";" in a cookie path, and an invalid path would fail
+			// xDS validation and drop the route, so the path is left unset.
+			name:   "a redirect path Envoy rejects on a cookie leaves the cookie path unset",
+			input:  ir.OIDC{RedirectPath: "/oauth2;v2/callback"},
+			expect: expectedRedirectPathOnlyCookieConfigs(""),
 		},
 	}
 

internal/xds/translator/testdata/out/xds-ir/multiple-listeners-same-port-with-different-filters.routes.yaml

@@ -49,10 +49,16 @@
             - profile
             authType: BASIC_AUTH
             authorizationEndpoint: https://oauth.foo.com/oauth2/v2/auth
+            cookieConfigs:
+              codeVerifierCookieConfig:
+                path: /foo/oauth2/callback
+              oauthNonceCookieConfig:
+                path: /foo/oauth2/callback
             credentials:
               clientId: client.oauth.foo.com
               cookieNames:
                 bearerToken: AccessToken-5F93C2E4
+                codeVerifier: CodeVerifier-5F93C2E4
                 idToken: IdToken-5F93C2E4
                 oauthExpires: OauthExpires-5F93C2E4
                 oauthHmac: OauthHMAC-5F93C2E4

internal/xds/translator/testdata/out/xds-ir/oidc-and-jwt-with-passthrough.routes.yaml

@@ -23,10 +23,16 @@
             - openid
             authType: BASIC_AUTH
             authorizationEndpoint: https://oauth.foo.com/oauth2/v2/auth
+            cookieConfigs:
+              codeVerifierCookieConfig:
+                path: /oauth2/callback
+              oauthNonceCookieConfig:
+                path: /oauth2/callback
             credentials:
               clientId: client.oauth.foo.com
               cookieNames:
                 bearerToken: AccessToken-b0a1b740
+                codeVerifier: CodeVerifier-b0a1b740
                 idToken: IdToken-b0a1b740
                 oauthExpires: OauthExpires-b0a1b740
                 oauthHmac: OauthHMAC-b0a1b740

internal/xds/translator/testdata/out/xds-ir/oidc-backend-cluster-provider.routes.yaml

@@ -20,10 +20,16 @@
             - openid
             authType: BASIC_AUTH
             authorizationEndpoint: https://oauth.foo.com/oauth2/v2/auth
+            cookieConfigs:
+              codeVerifierCookieConfig:
+                path: /bar/oauth2/callback
+              oauthNonceCookieConfig:
+                path: /bar/oauth2/callback
             credentials:
               clientId: client1.apps.googleusercontent.com
               cookieNames:
                 bearerToken: AccessToken-b0a1b740
+                codeVerifier: CodeVerifier-b0a1b740
                 idToken: IdToken-b0a1b740
                 oauthExpires: OauthExpires-b0a1b740
                 oauthHmac: OauthHMAC-b0a1b740

internal/xds/translator/testdata/out/xds-ir/oidc-provider-traffic-features.routes.yaml

@@ -35,10 +35,16 @@
             - openid
             authType: BASIC_AUTH
             authorizationEndpoint: https://oauth.foo.com/oauth2/v2/auth
+            cookieConfigs:
+              codeVerifierCookieConfig:
+                path: /bar/oauth2/callback
+              oauthNonceCookieConfig:
+                path: /bar/oauth2/callback
             credentials:
               clientId: client1.apps.googleusercontent.com
               cookieNames:
                 bearerToken: AccessToken-b0a1b740
+                codeVerifier: CodeVerifier-b0a1b740
                 idToken: IdToken-b0a1b740
                 oauthExpires: OauthExpires-b0a1b740
                 oauthHmac: OauthHMAC-b0a1b740

internal/xds/translator/testdata/out/xds-ir/oidc.routes.yaml

@@ -22,10 +22,16 @@
             - profile
             authType: BASIC_AUTH
             authorizationEndpoint: https://oauth.foo.com/oauth2/v2/auth
+            cookieConfigs:
+              codeVerifierCookieConfig:
+                path: /foo/oauth2/callback
+              oauthNonceCookieConfig:
+                path: /foo/oauth2/callback
             credentials:
               clientId: client.oauth.foo.com
               cookieNames:
                 bearerToken: AccessToken-5F93C2E4
+                codeVerifier: CodeVerifier-5F93C2E4
                 idToken: IdToken-5F93C2E4
                 oauthExpires: OauthExpires-5F93C2E4
                 oauthHmac: OauthHMAC-5F93C2E4
@@ -81,11 +87,17 @@
             - profile
             authType: BASIC_AUTH
             authorizationEndpoint: https://oauth.bar.com/oauth2/v2/auth
+            cookieConfigs:
+              codeVerifierCookieConfig:
+                path: /bar/oauth2/callback
+              oauthNonceCookieConfig:
+                path: /bar/oauth2/callback
             credentials:
               clientId: client.oauth.bar.com
               cookieDomain: example.com
               cookieNames:
                 bearerToken: CustomAccessTokenOverride
+                codeVerifier: CodeVerifier-5f93c2e4
                 idToken: CustomIdTokenOverride
                 oauthExpires: OauthExpires-5f93c2e4
                 oauthHmac: OauthHMAC-5f93c2e4

internal/xds/translator/testdata/out/xds-ir/securitypolicy-with-oidc-jwt-authz.routes.yaml

@@ -40,10 +40,16 @@
             - profile
             authType: BASIC_AUTH
             authorizationEndpoint: https://oidc.example.com/authorize
+            cookieConfigs:
+              codeVerifierCookieConfig:
+                path: /oauth2/callback
+              oauthNonceCookieConfig:
+                path: /oauth2/callback
             credentials:
               clientId: prometheus
               cookieNames:
                 bearerToken: AccessToken-5f93c2e4
+                codeVerifier: CodeVerifier-5f93c2e4
                 idToken: IdToken
                 oauthExpires: OauthExpires-5f93c2e4
                 oauthHmac: OauthHMAC-5f93c2e4

release-notes/current/bug_fixes/9644-oidc-orphaned-pkce-cookies.md

@@ -0,0 +1,18 @@
+Fixed OIDC flow-state cookies accumulating in the browser and overflowing the request header size limit.
+Envoy mints a nonce (CSRF) and a PKCE code verifier cookie for every authorization flow it starts, but
+only deletes the pair belonging to the flow that completes the callback, so flows that are abandoned -
+parallel requests from a logged out browser, a user navigating away from the provider's login page -
+leave their cookies behind until they expire. Envoy Gateway now scopes both cookies to the OIDC redirect
+path, the only path where Envoy needs their value, so any orphans are no longer sent on every request.
+Note this bounds the damage rather than eliminating it: orphans are still sent to the callback endpoint
+itself until they expire, and logout can no longer purge them early because the browser no longer sends
+them to the signout path, so also consider lowering `csrfTokenTTL`.
+The PKCE code verifier cookie is also now named `CodeVerifier-<suffix>`, carrying the same per-policy
+suffix as the other OAuth2 cookies instead of Envoy's shared default, so SecurityPolicies on the same
+cookie domain no longer delete each other's in-flight flow cookies on logout.
+On upgrade, a browser already holding flow cookies keeps them at the old `path=/`, since the new
+deletion headers are scoped to the redirect path. They expire on the lifetime they were originally
+issued with - 10 minutes by default, and unaffected by any `csrfTokenTTL` you configure during the
+upgrade.
+The old code verifier cookie name is also no longer read, so a login that was in progress across the
+rollout may need to be retried once.