From f0ebd808d7c8f1aa6aea09b1ecd2f79c870193fb Mon Sep 17 00:00:00 2001 From: Matt Moyer Date: Thu, 3 Dec 2020 21:23:58 -0600 Subject: [PATCH] Switch CSRF cookie from `Same-Site=Strict` to `Same-Site=Lax`. This CSRF cookie needs to be included on the request to the callback endpoint triggered by the redirect from the OIDC upstream provider. This is not allowed by `Same-Site=Strict` but is allowed by `Same-Site=Lax` because it is a "cross-site top-level navigation" [1]. We didn't catch this earlier with our Dex-based tests because the upstream and downstream issuers were on the same parent domain `*.svc.cluster.local` so the cookie was allowed even with `Strict` mode. [1]: https://tools.ietf.org/html/draft-ietf-httpbis-cookie-same-site-00#section-3.2 Signed-off-by: Matt Moyer --- internal/oidc/auth/auth_handler.go | 2 +- internal/oidc/auth/auth_handler_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/oidc/auth/auth_handler.go b/internal/oidc/auth/auth_handler.go index f3a305f18..231cf8ee3 100644 --- a/internal/oidc/auth/auth_handler.go +++ b/internal/oidc/auth/auth_handler.go @@ -225,7 +225,7 @@ func addCSRFSetCookieHeader(w http.ResponseWriter, csrfValue csrftoken.CSRFToken Name: oidc.CSRFCookieName, Value: encodedCSRFValue, HttpOnly: true, - SameSite: http.SameSiteStrictMode, + SameSite: http.SameSiteLaxMode, Secure: true, Path: "/", }) diff --git a/internal/oidc/auth/auth_handler_test.go b/internal/oidc/auth/auth_handler_test.go index 73176d082..032d863f2 100644 --- a/internal/oidc/auth/auth_handler_test.go +++ b/internal/oidc/auth/auth_handler_test.go @@ -751,7 +751,7 @@ func TestAuthorizationEndpoint(t *testing.T) { if test.wantCSRFValueInCookieHeader != "" { require.Len(t, rsp.Header().Values("Set-Cookie"), 1) actualCookie := rsp.Header().Get("Set-Cookie") - regex := regexp.MustCompile("__Host-pinniped-csrf=([^;]+); Path=/; HttpOnly; Secure; SameSite=Strict") + regex := regexp.MustCompile("__Host-pinniped-csrf=([^;]+); Path=/; HttpOnly; Secure; SameSite=Lax") submatches := regex.FindStringSubmatch(actualCookie) require.Len(t, submatches, 2) captured := submatches[1]