Add disabled option for cookie samesite attribute (#21472)

Breaking change: If disabled the cookie samesite cookie attribute
will not be set, but if none the attribute will be set and is a
breaking change compared to before where none did not render the
attribute. This was due to a known issue in Safari.

Co-Authored-By: Arve Knudsen <arve.knudsen@gmail.com>
Co-Authored-By: Diana Payton <52059945+oddlittlebird@users.noreply.github.com>

Fixes #19847

CHANGELOG.md

@@ -6,6 +6,7 @@
 
 ## Breaking changes
 * **PagerDuty**: Change `payload.custom_details` field in PagerDuty notification to be a JSON object instead of a string.
+* **Security**: The `[security]` setting `cookie_samesite` configured to `none` now renders cookies with `SameSite=None` attribute compared to before where no `SameSite` attribute was added to cookies. To get the old behavior, use value `disabled` instead of `none`. Refer to [Upgrade Grafana](https://grafana.com/docs/grafana/latest/installation/upgrading/#upgrading-to-v6-6) for more information.
 
 # 6.5.2 (2019-12-11)
 

conf/defaults.ini

@@ -179,7 +179,7 @@ disable_brute_force_login_protection = false
 # set to true if you host Grafana behind HTTPS. default is false.
 cookie_secure = false
 
-# set cookie SameSite attribute. defaults to `lax`. can be set to "lax", "strict" and "none"
+# set cookie SameSite attribute. defaults to `lax`. can be set to "lax", "strict", "none" and "disabled"
 cookie_samesite = lax
 
 # set to true if you want to allow browsers to render Grafana in a <frame>, <iframe>, <embed> or <object>. default is false.

conf/sample.ini

@@ -180,7 +180,7 @@
 # set to true if you host Grafana behind HTTPS. default is false.
 ;cookie_secure = false
 
-# set cookie SameSite attribute. defaults to `lax`. can be set to "lax", "strict" and "none"
+# set cookie SameSite attribute. defaults to `lax`. can be set to "lax", "strict", "none" and "disabled"
 ;cookie_samesite = lax
 
 # set to true if you want to allow browsers to render Grafana in a <frame>, <iframe>, <embed> or <object>. default is false.

docs/sources/installation/configuration.md

@@ -361,7 +361,7 @@ Set to `true` if you host Grafana behind HTTPS. Default is `false`.
 
 ### cookie_samesite
 
-Sets the `SameSite` cookie attribute and prevents the browser from sending this cookie along with cross-site requests. The main goal is mitigate the risk of cross-origin information leakage. It also provides some protection against cross-site request forgery attacks (CSRF),  [read more here](https://www.owasp.org/index.php/SameSite). Valid values are `lax`, `strict` and `none`. Default is `lax`.
+Sets the `SameSite` cookie attribute and prevents the browser from sending this cookie along with cross-site requests. The main goal is to mitigate the risk of cross-origin information leakage. This setting also provides some protection against cross-site request forgery attacks (CSRF),  [read more about SameSite here](https://www.owasp.org/index.php/SameSite). Valid values are `lax`, `strict`, `none`, and `disabled`. Default is `lax`. Using value `disabled` does not add any `SameSite` attribute to cookies.
 
 ### allow_embedding
 

docs/sources/installation/upgrading.md

@@ -226,3 +226,14 @@ The handling of multi-valued template variables in dimension values has been cha
 ## Upgrading to v6.6
 
 The Generic OAuth setting `send_client_credentials_via_post`, used for supporting non-compliant providers, has been removed. From now on, Grafana will automatically detect if credentials should be sent as part of the URL or request body for a specific provider. The result will be remembered and used for additional OAuth requests for that provider.
+
+### Important changes regarding SameSite cookie attribute
+
+Chrome 80 treats cookies as `SameSite=Lax` by default if no `SameSite` attribute is specified, see https://www.chromestatus.com/feature/5088147346030592.
+
+Due to this change in Chrome, the `[security]` setting `cookie_samesite` configured to `none` now renders cookies with `SameSite=None` attribute compared to before where no `SameSite` attribute was added to cookies. To get the old behavior, use value `disabled` instead of `none`, see [cookie_samesite in Configuration]({{< relref "configuration/#cookie-samesite" >}}) for more information.
+
+**Note:** There is currently a bug affecting Mac OSX and iOS that causes `SameSite=None` cookies to be treated as `SameSite=Strict` and therefore not sent with cross-site requests. (See https://bugs.webkit.org/show_bug.cgi?id=198181.) Until this is fixed, `SameSite=None` might not work properly on Safari.
+
+This version of Chrome also rejects insecure `SameSite=None` cookies. See https://www.chromestatus.com/feature/5633521622188032 for more information. Make sure that you
+change the `[security]` setting `cookie_secure` to `true` and use HTTPS when `cookie_samesite` is configured to `none`, otherwise authentication in Grafana won't work properly.

docs/sources/reference/share_panel.md

@@ -32,7 +32,7 @@ https://play.grafana.org/d/000000012/grafana-play-home?orgId=1&from=156871968017
 
 You can embed a panel using an iframe on another web site. This tab will show you the html that you need to use.
 
-> *Notice* This sharing require either anonymous access or setting [cookie_samesite]({{< relref "../installation/configuration.md#cookie-samesite" >}}) to none
+> **Note:** This sharing requires [allow_embedding]({{< relref "../installation/configuration.md#allow-embedding" >}}) enabled and anonymous access, or proper configuration of the [cookie_samesite]({{< relref "../installation/configuration.md#cookie-samesite" >}}) setting.
 
 Example:
 

pkg/api/login.go

@@ -45,7 +45,8 @@ func (hs *HTTPServer) cookieOptionsFromCfg() middleware.CookieOptions {
 	return middleware.CookieOptions{
 		Path:             hs.Cfg.AppSubUrl + "/",
 		Secure:           hs.Cfg.CookieSecure,
-		SameSite: hs.Cfg.CookieSameSite,
+		SameSiteDisabled: hs.Cfg.CookieSameSiteDisabled,
+		SameSiteMode:     hs.Cfg.CookieSameSiteMode,
 	}
 }
 

pkg/api/login_test.go

@@ -113,7 +113,7 @@ func TestLoginErrorCookieApiEndpoint(t *testing.T) {
 		HttpOnly: true,
 		Path:     setting.AppSubUrl + "/",
 		Secure:   hs.Cfg.CookieSecure,
-		SameSite: hs.Cfg.CookieSameSite,
+		SameSite: hs.Cfg.CookieSameSiteMode,
 	}
 	sc.m.Get(sc.url, sc.defaultHandler)
 	sc.fakeReqNoAssertionsWithCookie("GET", sc.url, cookie).exec()
@@ -204,7 +204,7 @@ func TestLoginViewRedirect(t *testing.T) {
 				HttpOnly: true,
 				Path:     hs.Cfg.AppSubUrl + "/",
 				Secure:   hs.Cfg.CookieSecure,
-				SameSite: hs.Cfg.CookieSameSite,
+				SameSite: hs.Cfg.CookieSameSiteMode,
 			}
 			sc.m.Get(sc.url, sc.defaultHandler)
 			sc.fakeReqNoAssertionsWithCookie("GET", sc.url, cookie).exec()
@@ -312,7 +312,7 @@ func TestLoginPostRedirect(t *testing.T) {
 				HttpOnly: true,
 				Path:     hs.Cfg.AppSubUrl + "/",
 				Secure:   hs.Cfg.CookieSecure,
-				SameSite: hs.Cfg.CookieSameSite,
+				SameSite: hs.Cfg.CookieSameSiteMode,
 			}
 			sc.m.Post(sc.url, sc.defaultHandler)
 			sc.fakeReqNoAssertionsWithCookie("POST", sc.url, cookie).exec()

pkg/middleware/cookie.go

@@ -9,14 +9,16 @@ import (
 type CookieOptions struct {
 	Path             string
 	Secure           bool
-	SameSite http.SameSite
+	SameSiteDisabled bool
+	SameSiteMode     http.SameSite
 }
 
 func newCookieOptions() CookieOptions {
 	return CookieOptions{
 		Path:             setting.AppSubUrl + "/",
 		Secure:           setting.CookieSecure,
-		SameSite: setting.CookieSameSite,
+		SameSiteDisabled: setting.CookieSameSiteDisabled,
+		SameSiteMode:     setting.CookieSameSiteMode,
 	}
 }
 
@@ -36,8 +38,8 @@ func WriteCookie(w http.ResponseWriter, name string, value string, maxAge int, g
 		Path:     options.Path,
 		Secure:   options.Secure,
 	}
-	if options.SameSite != http.SameSiteDefaultMode {
+	if !options.SameSiteDisabled {
-		cookie.SameSite = options.SameSite
+		cookie.SameSite = options.SameSiteMode
 	}
 	http.SetCookie(w, &cookie)
 }

pkg/middleware/middleware_test.go

@@ -256,12 +256,12 @@ func TestMiddlewareContext(t *testing.T) {
 			maxAge := (maxAgeHours + time.Hour).Seconds()
 
 			sameSitePolicies := []http.SameSite{
-				http.SameSiteDefaultMode,
+				http.SameSiteNoneMode,
 				http.SameSiteLaxMode,
 				http.SameSiteStrictMode,
 			}
 			for _, sameSitePolicy := range sameSitePolicies {
-				setting.CookieSameSite = sameSitePolicy
+				setting.CookieSameSiteMode = sameSitePolicy
 				expectedCookie := &http.Cookie{
 					Name:     setting.LoginCookieName,
 					Value:    "rotated",
@@ -269,9 +269,7 @@ func TestMiddlewareContext(t *testing.T) {
 					HttpOnly: true,
 					MaxAge:   int(maxAge),
 					Secure:   setting.CookieSecure,
-				}
+					SameSite: sameSitePolicy,
-				if sameSitePolicy != http.SameSiteDefaultMode {
-					expectedCookie.SameSite = sameSitePolicy
 				}
 
 				sc.fakeReq("GET", "/").exec()
@@ -287,6 +285,22 @@ func TestMiddlewareContext(t *testing.T) {
 					So(sc.resp.Header().Get("Set-Cookie"), ShouldEqual, expectedCookie.String())
 				})
 			}
+
+			Convey("Should not set cookie with SameSite attribute when setting.CookieSameSiteDisabled is true", func() {
+				setting.CookieSameSiteDisabled = true
+				setting.CookieSameSiteMode = http.SameSiteLaxMode
+				expectedCookie := &http.Cookie{
+					Name:     setting.LoginCookieName,
+					Value:    "rotated",
+					Path:     setting.AppSubUrl + "/",
+					HttpOnly: true,
+					MaxAge:   int(maxAge),
+					Secure:   setting.CookieSecure,
+				}
+
+				sc.fakeReq("GET", "/").exec()
+				So(sc.resp.Header().Get("Set-Cookie"), ShouldEqual, expectedCookie.String())
+			})
 		})
 
 		middlewareScenario(t, "Invalid/expired auth token in cookie", func(sc *scenarioContext) {

pkg/setting/setting.go

@@ -101,7 +101,8 @@ var (
 	DataProxyWhiteList                map[string]bool
 	DisableBruteForceLoginProtection  bool
 	CookieSecure                      bool
-	CookieSameSite                    http.SameSite
+	CookieSameSiteDisabled            bool
+	CookieSameSiteMode                http.SameSite
 	AllowEmbedding                    bool
 	XSSProtectionHeader               bool
 	ContentTypeProtectionHeader       bool
@@ -248,7 +249,8 @@ type Cfg struct {
 	DisableInitAdminCreation         bool
 	DisableBruteForceLoginProtection bool
 	CookieSecure                     bool
-	CookieSameSite                   http.SameSite
+	CookieSameSiteDisabled           bool
+	CookieSameSiteMode               http.SameSite
 
 	TempDataLifetime                 time.Duration
 	MetricsEndpointEnabled           bool
@@ -719,18 +721,24 @@ func (cfg *Cfg) Load(args *CommandLineArgs) error {
 	if err != nil {
 		return err
 	}
+
+	if samesiteString == "disabled" {
+		CookieSameSiteDisabled = true
+		cfg.CookieSameSiteDisabled = CookieSameSiteDisabled
+	} else {
 		validSameSiteValues := map[string]http.SameSite{
 			"lax":    http.SameSiteLaxMode,
 			"strict": http.SameSiteStrictMode,
-		"none":   http.SameSiteDefaultMode,
+			"none":   http.SameSiteNoneMode,
 		}
 
 		if samesite, ok := validSameSiteValues[samesiteString]; ok {
-		CookieSameSite = samesite
+			CookieSameSiteMode = samesite
-		cfg.CookieSameSite = CookieSameSite
+			cfg.CookieSameSiteMode = CookieSameSiteMode
 		} else {
-		CookieSameSite = http.SameSiteLaxMode
+			CookieSameSiteMode = http.SameSiteLaxMode
-		cfg.CookieSameSite = CookieSameSite
+			cfg.CookieSameSiteMode = CookieSameSiteMode
+		}
 	}
 
 	AllowEmbedding = security.Key("allow_embedding").MustBool(false)