From 9c5c77dad799f5c4cf78e0a0650f327819e9190c Mon Sep 17 00:00:00 2001 From: Florian Merz <flomerz@gmail.com> Date: Thu, 25 Jul 2019 14:33:44 +0200 Subject: [PATCH] [KEYCLOAK-10938] Support SameSite cookie attribute Adds an option to the config to enable the SameSite attribute for cookies set by gatekeeper. SameSiteCookie can be Strict|Lax|None None will not set the SameSite flag. --- config.go | 4 ++++ cookies.go | 20 ++++++++++++++++++++ cookies_test.go | 47 ++++++++++++++++++++++++++++++++++++++++++++++- doc.go | 2 ++ 4 files changed, 72 insertions(+), 1 deletion(-) diff --git a/config.go b/config.go index 5c4509f..080af6a 100644 --- a/config.go +++ b/config.go @@ -56,6 +56,7 @@ func newDefaultConfig() *Config { SelfSignedTLSHostnames: hostnames, RequestIDHeader: "X-Request-ID", ResponseHeaders: make(map[string]string), + SameSiteCookie: SameSiteLax, SecureCookie: true, ServerIdleTimeout: 120 * time.Second, ServerReadTimeout: 10 * time.Second, @@ -93,6 +94,9 @@ func (r *Config) isValid() error { if r.MaxIdleConnsPerHost < 0 || r.MaxIdleConnsPerHost > r.MaxIdleConns { return errors.New("maxi-idle-connections-per-host must be a number > 0 and <= max-idle-connections") } + if r.SameSiteCookie != "" && r.SameSiteCookie != SameSiteStrict && r.SameSiteCookie != SameSiteLax && r.SameSiteCookie != SameSiteNone { + return errors.New("same-site-cookie must be one of Strict|Lax|None") + } if r.TLSCertificate != "" && r.TLSPrivateKey == "" { return errors.New("you have not provided a private key") } diff --git a/cookies.go b/cookies.go index d3d2b08..3194024 100644 --- a/cookies.go +++ b/cookies.go @@ -25,6 +25,13 @@ import ( uuid "github.com/satori/go.uuid" ) +// SameSite cookie config options +const ( + SameSiteStrict = "Strict" + SameSiteLax = "Lax" + SameSiteNone = "None" +) + // dropCookie drops a cookie into the response func (r *oauthProxy) dropCookie(w http.ResponseWriter, host, name, value string, duration time.Duration) { // step: default to the host header, else the config domain @@ -48,6 +55,13 @@ func (r *oauthProxy) dropCookie(w http.ResponseWriter, host, name, value string, cookie.Expires = time.Now().Add(duration) } + switch r.config.SameSiteCookie { + case SameSiteStrict: + cookie.SameSite = http.SameSiteStrictMode + case SameSiteLax: + cookie.SameSite = http.SameSiteLaxMode + } + http.SetCookie(w, cookie) } @@ -65,6 +79,12 @@ func (r *oauthProxy) getMaxCookieChunkLength(req *http.Request, cookieName strin if !r.config.EnableSessionCookies { maxCookieChunkLength -= len("Expires=Mon, 02 Jan 2006 03:04:05 MST; ") } + switch r.config.SameSiteCookie { + case SameSiteStrict: + maxCookieChunkLength -= len("SameSite=Strict ") + case SameSiteLax: + maxCookieChunkLength -= len("SameSite=Lax ") + } if r.config.SecureCookie { maxCookieChunkLength -= len("Secure") } diff --git a/cookies_test.go b/cookies_test.go index 2563f1a..5b2e738 100644 --- a/cookies_test.go +++ b/cookies_test.go @@ -166,6 +166,45 @@ func TestSessionOnlyCookie(t *testing.T) { "we have not set the cookie, headers: %v", resp.Header()) } +func TestSameSiteCookie(t *testing.T) { + p, _, _ := newTestProxyService(nil) + + req := newFakeHTTPRequest("GET", "/admin") + resp := httptest.NewRecorder() + p.dropCookie(resp, req.Host, "test-cookie", "test-value", 0) + + assert.Equal(t, resp.Header().Get("Set-Cookie"), + "test-cookie=test-value; Path=/; Domain=127.0.0.1", + "we have not set the cookie, headers: %v", resp.Header()) + + req = newFakeHTTPRequest("GET", "/admin") + resp = httptest.NewRecorder() + p.config.SameSiteCookie = SameSiteStrict + p.dropCookie(resp, req.Host, "test-cookie", "test-value", 0) + + assert.Equal(t, resp.Header().Get("Set-Cookie"), + "test-cookie=test-value; Path=/; Domain=127.0.0.1; SameSite=Strict", + "we have not set the cookie, headers: %v", resp.Header()) + + req = newFakeHTTPRequest("GET", "/admin") + resp = httptest.NewRecorder() + p.config.SameSiteCookie = SameSiteLax + p.dropCookie(resp, req.Host, "test-cookie", "test-value", 0) + + assert.Equal(t, resp.Header().Get("Set-Cookie"), + "test-cookie=test-value; Path=/; Domain=127.0.0.1; SameSite=Lax", + "we have not set the cookie, headers: %v", resp.Header()) + + req = newFakeHTTPRequest("GET", "/admin") + resp = httptest.NewRecorder() + p.config.SameSiteCookie = SameSiteNone + p.dropCookie(resp, req.Host, "test-cookie", "test-value", 0) + + assert.Equal(t, resp.Header().Get("Set-Cookie"), + "test-cookie=test-value; Path=/; Domain=127.0.0.1", + "we have not set the cookie, headers: %v", resp.Header()) +} + func TestHTTPOnlyCookie(t *testing.T) { p, _, _ := newTestProxyService(nil) @@ -225,13 +264,19 @@ func TestGetMaxCookieChunkLength(t *testing.T) { p.config.HTTPOnlyCookie = true p.config.EnableSessionCookies = true p.config.SecureCookie = true + p.config.SameSiteCookie = "Strict" p.config.CookieDomain = "1234567890" - assert.Equal(t, p.getMaxCookieChunkLength(req, "1234567890"), 4033, + assert.Equal(t, p.getMaxCookieChunkLength(req, "1234567890"), 4017, + "cookie chunk calculation is not correct") + + p.config.SameSiteCookie = "Lax" + assert.Equal(t, p.getMaxCookieChunkLength(req, "1234567890"), 4020, "cookie chunk calculation is not correct") p.config.HTTPOnlyCookie = false p.config.EnableSessionCookies = false p.config.SecureCookie = false + p.config.SameSiteCookie = "None" p.config.CookieDomain = "" assert.Equal(t, p.getMaxCookieChunkLength(req, ""), 4021, "cookie chunk calculation is not correct") diff --git a/doc.go b/doc.go index 9986990..548b6b5 100644 --- a/doc.go +++ b/doc.go @@ -264,6 +264,8 @@ type Config struct { SecureCookie bool `json:"secure-cookie" yaml:"secure-cookie" usage:"enforces the cookie to be secure"` // HTTPOnlyCookie enforces the cookie as http only HTTPOnlyCookie bool `json:"http-only-cookie" yaml:"http-only-cookie" usage:"enforces the cookie is in http only mode"` + // SameSiteCookie enforces cookies to be send only to same site requests. + SameSiteCookie string `json:"same-site-cookie" yaml:"same-site-cookie" usage:"enforces cookies to be send only to same site requests according to the policy (can be Strict|Lax|None)"` // MatchClaims is a series of checks, the claims in the token must match those here MatchClaims map[string]string `json:"match-claims" yaml:"match-claims" usage:"keypair values for matching access token claims e.g. aud=myapp, iss=http://example.*"` -- GitLab