Add suffix for oauth cookies (envoyproxy#2664)

* add suffix for oauth cookies Signed-off-by: huabing zhao <[email protected]> * use hash as suffix Signed-off-by: huabing zhao <[email protected]> --------- Signed-off-by: huabing zhao <[email protected]>
zhaohuabing · Feb 22, 2024 · fc7d6bc · fc7d6bc
1 parent 5a0baf0
commit fc7d6bc
Show file tree

Hide file tree

Showing 11 changed files with 47 additions and 0 deletions.
diff --git a/internal/gatewayapi/securitypolicy.go b/internal/gatewayapi/securitypolicy.go
@@ -9,6 +9,7 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
+	"hash/fnv"
 	"net/http"
 	"net/netip"
 	"net/url"
@@ -494,6 +495,12 @@ func (t *Translator) buildOIDC(
 		logoutPath = *oidc.LogoutPath
 	}
 
+	h := fnv.New32a()
+	if _, err = h.Write([]byte(policy.UID)); err != nil {
+		return nil, fmt.Errorf("error generating oauth cookie suffix: %w", err)
+	}
+	suffix := fmt.Sprintf("%X", h.Sum32())
+
 	return &ir.OIDC{
 		Provider:     *provider,
 		ClientID:     oidc.ClientID,
@@ -502,6 +509,7 @@ func (t *Translator) buildOIDC(
 		RedirectURL:  redirectURL,
 		RedirectPath: redirectPath,
 		LogoutPath:   logoutPath,
+		CookieSuffix: suffix,
 	}, nil
 }
 

diff --git a/internal/gatewayapi/testdata/securitypolicy-with-oidc-invalid-issuer.in.yaml b/internal/gatewayapi/testdata/securitypolicy-with-oidc-invalid-issuer.in.yaml
@@ -27,6 +27,7 @@ securityPolicies:
   metadata:
     namespace: default
     name: policy-non-exist-secretRef
+    uid: b8284d0f-de82-4c65-b204-96a0d3f258a1
   spec:
     targetRef:
       group: gateway.networking.k8s.io

diff --git a/internal/gatewayapi/testdata/securitypolicy-with-oidc-invalid-issuer.out.yaml b/internal/gatewayapi/testdata/securitypolicy-with-oidc-invalid-issuer.out.yaml
@@ -62,6 +62,7 @@ securityPolicies:
     creationTimestamp: null
     name: policy-non-exist-secretRef
     namespace: default
+    uid: b8284d0f-de82-4c65-b204-96a0d3f258a1
   spec:
     oidc:
       clientID: client1.apps.foo.bar.com

diff --git a/internal/gatewayapi/testdata/securitypolicy-with-oidc-invalid-secretref.in.yaml b/internal/gatewayapi/testdata/securitypolicy-with-oidc-invalid-secretref.in.yaml
@@ -63,6 +63,7 @@ securityPolicies:
   metadata:
     namespace: default
     name: policy-non-exist-secretRef
+    uid: b8284d0f-de82-4c65-b204-96a0d3f258a1
   spec:
     targetRef:
       group: gateway.networking.k8s.io
@@ -81,6 +82,7 @@ securityPolicies:
   metadata:
     namespace: default
     name: policy-no-referenceGrant
+    uid: 08335a80-83ba-4592-888f-6ac0bba44ce4
   spec:
     targetRef:
       group: gateway.networking.k8s.io

diff --git a/internal/gatewayapi/testdata/securitypolicy-with-oidc-invalid-secretref.out.yaml b/internal/gatewayapi/testdata/securitypolicy-with-oidc-invalid-secretref.out.yaml
@@ -172,6 +172,7 @@ securityPolicies:
     creationTimestamp: null
     name: policy-non-exist-secretRef
     namespace: default
+    uid: b8284d0f-de82-4c65-b204-96a0d3f258a1
   spec:
     oidc:
       clientID: client1.apps.googleusercontent.com
@@ -200,6 +201,7 @@ securityPolicies:
     creationTimestamp: null
     name: policy-no-referenceGrant
     namespace: default
+    uid: 08335a80-83ba-4592-888f-6ac0bba44ce4
   spec:
     oidc:
       clientID: client1.apps.googleusercontent.com

diff --git a/internal/gatewayapi/testdata/securitypolicy-with-oidc.in.yaml b/internal/gatewayapi/testdata/securitypolicy-with-oidc.in.yaml
@@ -80,6 +80,7 @@ securityPolicies:
   metadata:
     namespace: envoy-gateway
     name: policy-for-gateway-discover-endpoints  # This policy should attach httproute-2
+    uid: b8284d0f-de82-4c65-b204-96a0d3f258a1
   spec:
     targetRef:
       group: gateway.networking.k8s.io
@@ -99,6 +100,7 @@ securityPolicies:
   metadata:
     namespace: default
     name: policy-for-http-route   # This policy should attach httproute-1
+    uid: 08335a80-83ba-4592-888f-6ac0bba44ce4
   spec:
     targetRef:
       group: gateway.networking.k8s.io

diff --git a/internal/gatewayapi/testdata/securitypolicy-with-oidc.out.yaml b/internal/gatewayapi/testdata/securitypolicy-with-oidc.out.yaml
@@ -139,6 +139,7 @@ securityPolicies:
     creationTimestamp: null
     name: policy-for-http-route
     namespace: default
+    uid: 08335a80-83ba-4592-888f-6ac0bba44ce4
   spec:
     oidc:
       clientID: client2.oauth.foo.com
@@ -174,6 +175,7 @@ securityPolicies:
     creationTimestamp: null
     name: policy-for-gateway-discover-endpoints
     namespace: envoy-gateway
+    uid: b8284d0f-de82-4c65-b204-96a0d3f258a1
   spec:
     oidc:
       clientID: client1.apps.googleusercontent.com
@@ -230,6 +232,7 @@ xdsIR:
         oidc:
           clientID: client2.oauth.foo.com
           clientSecret: Y2xpZW50MTpzZWNyZXQK
+          cookieSuffix: 5F93C2E4
           logoutPath: /foo/logout
           provider:
             authorizationEndpoint: https://oauth.foo.com/oauth2/v2/auth
@@ -261,6 +264,7 @@ xdsIR:
         oidc:
           clientID: client1.apps.googleusercontent.com
           clientSecret: Y2xpZW50MTpzZWNyZXQK
+          cookieSuffix: B0A1B740
           logoutPath: /bar/logout
           provider:
             authorizationEndpoint: https://accounts.google.com/o/oauth2/v2/auth

diff --git a/internal/ir/xds.go b/internal/ir/xds.go
@@ -538,6 +538,12 @@ type OIDC struct {
 
 	// The path to log a user out, clearing their credential cookies.
 	LogoutPath string `json:"logoutPath,omitempty"`
+
+	// 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: BearerToken,
+	// OauthHMAC, OauthExpires, IdToken, and RefreshToken.
+	CookieSuffix string `json:"cookieSuffix,omitempty"`
 }
 
 type OIDCProvider struct {

diff --git a/internal/xds/translator/oidc.go b/internal/xds/translator/oidc.go
@@ -153,6 +153,13 @@ func oauth2Config(route *ir.HTTPRoute) (*oauth2v3.OAuth2, error) {
 						SdsConfig: makeConfigSource(),
 					},
 				},
+				CookieNames: &oauth2v3.OAuth2Credentials_CookieNames{
+					BearerToken:  fmt.Sprintf("BearerToken-%s", route.OIDC.CookieSuffix),
+					OauthHmac:    fmt.Sprintf("OauthHMAC-%s", route.OIDC.CookieSuffix),
+					OauthExpires: fmt.Sprintf("OauthExpires-%s", route.OIDC.CookieSuffix),
+					IdToken:      fmt.Sprintf("IdToken-%s", route.OIDC.CookieSuffix),
+					RefreshToken: fmt.Sprintf("RefreshToken-%s", route.OIDC.CookieSuffix),
+				},
 			},
 			// every OIDC provider supports basic auth
 			AuthType:   oauth2v3.OAuth2Config_BASIC_AUTH,

diff --git a/internal/xds/translator/testdata/in/xds-ir/oidc.yaml b/internal/xds/translator/testdata/in/xds-ir/oidc.yaml
@@ -31,6 +31,7 @@ http:
       redirectURL: "https://www.example.com/foo/oauth2/callback"
       redirectPath: "/foo/oauth2/callback"
       logoutPath: "/foo/logout"
+      cookieSuffix: 5F93C2E4
   - name: "second-route"
     hostname: "*"
     pathMatch:
@@ -54,3 +55,4 @@ http:
       redirectURL: "https://www.example.com/bar/oauth2/callback"
       redirectPath: "/bar/oauth2/callback"
       logoutPath: "/bar/logout"
+      cookieSuffix: B0A1B740
diff --git a/internal/xds/translator/testdata/out/xds-ir/oidc.listeners.yaml b/internal/xds/translator/testdata/out/xds-ir/oidc.listeners.yaml
@@ -27,6 +27,12 @@
               authorizationEndpoint: https://oauth.foo.com/oauth2/v2/auth
               credentials:
                 clientId: client.oauth.foo.com
+                cookieNames:
+                  bearerToken: BearerToken-5F93C2E4
+                  idToken: IdToken-5F93C2E4
+                  oauthExpires: OauthExpires-5F93C2E4
+                  oauthHmac: OauthHMAC-5F93C2E4
+                  refreshToken: RefreshToken-5F93C2E4
                 hmacSecret:
                   name: first-route/oauth2/hmac_secret
                   sdsConfig:
@@ -62,6 +68,12 @@
               authorizationEndpoint: https://oauth.bar.com/oauth2/v2/auth
               credentials:
                 clientId: client.oauth.bar.com
+                cookieNames:
+                  bearerToken: BearerToken-B0A1B740
+                  idToken: IdToken-B0A1B740
+                  oauthExpires: OauthExpires-B0A1B740
+                  oauthHmac: OauthHMAC-B0A1B740
+                  refreshToken: RefreshToken-B0A1B740
                 hmacSecret:
                   name: second-route/oauth2/hmac_secret
                   sdsConfig: