From 706fbfdb2ceefe287924b51f25d9580a37f1b354 Mon Sep 17 00:00:00 2001 From: James Elliott Date: Sun, 18 Apr 2021 10:02:04 +1000 Subject: [PATCH] fix(session): ensure default cookie samesite value is lax (#1926) This implements a change to the default behaviour of the cookies generated by the sessions package. The old behaviour was to set the SameSite=None, this changes it to SameSite=Lax. Additionally this puts the option in the hands of the end-user so they can decide for themselves what the best option is. --- config.template.yml | 14 ++++--- docs/configuration/session/index.md | 21 ++++++++++ docs/security/measures.md | 12 +++--- internal/configuration/config.template.yml | 14 ++++--- internal/configuration/schema/session.go | 4 +- internal/configuration/validator/const.go | 3 +- internal/configuration/validator/session.go | 6 +++ .../configuration/validator/session_test.go | 39 +++++++++++++++++++ internal/session/provider_config.go | 12 ++++++ internal/session/provider_config_test.go | 24 +++++++++++- 10 files changed, 131 insertions(+), 18 deletions(-) diff --git a/config.template.yml b/config.template.yml index ef1df435f..154da0aa7 100644 --- a/config.template.yml +++ b/config.template.yml @@ -338,6 +338,15 @@ session: ## The name of the session cookie. name: authelia_session + ## The domain to protect. + ## Note: the authenticator must also be in that domain. + ## If empty, the cookie is restricted to the subdomain of the issuer. + domain: example.com + + ## Sets the Cookie SameSite value. Possible options are none, lax, or strict. + ## Please read https://www.authelia.com/docs/configuration/session.html#same_site + same_site: lax + ## The secret to encrypt the session data. This is only used with Redis / Redis Sentinel. ## Secret can also be set using a secret: https://www.authelia.com/docs/configuration/secrets.html secret: insecure_session_secret @@ -359,11 +368,6 @@ session: ## Value of 0 disables remember me. remember_me_duration: 1M - ## The domain to protect. - ## Note: the authenticator must also be in that domain. - ## If empty, the cookie is restricted to the subdomain of the issuer. - domain: example.com - ## ## Redis Provider ## diff --git a/docs/configuration/session/index.md b/docs/configuration/session/index.md index 3add863b1..a41d4a8cc 100644 --- a/docs/configuration/session/index.md +++ b/docs/configuration/session/index.md @@ -22,6 +22,7 @@ and can then order the reverse proxy to let the request pass through to the appl session: name: authelia_session domain: example.com + same_site: lax secret: unsecure_session_secret expiration: 1h inactivity: 5m @@ -67,6 +68,26 @@ required: yes The domain the cookie is assigned to protect. This must be the same as the domain Authelia is served on or the root of the domain. For example if listening on auth.example.com the cookie should be auth.example.com or example.com. +### same_site +
+type: string +{: .label .label-config .label-purple } +default: lax +{: .label .label-config .label-blue } +required: no +{: .label .label-config .label-green } +
+ +Sets the cookies SameSite value. Prior to offering the configuration choice this defaulted to None. The new default is +Lax. This option is defined in lower-case. So for example if you want to set it to `Strict`, the value in configuration +needs to be `strict`. + +You can read about the SameSite cookie in detail on the +[MDN](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Set-Cookie/SameSite). In short setting SameSite to Lax +is generally the most desirable option for Authelia. None is not recommended unless you absolutely know what you're +doing and trust all the protected apps. Strict is not going to work in many use cases and we have not tested it in this +state but it's available as an option anyway. + ### secret
type: string diff --git a/docs/security/measures.md b/docs/security/measures.md index 10a1e59e8..96b054d5f 100644 --- a/docs/security/measures.md +++ b/docs/security/measures.md @@ -9,11 +9,13 @@ nav_order: 1 ## Protection against cookie theft -Authelia uses two mechanisms to protect against cookie theft: -1. session attribute `httpOnly` set to true make client-side code unable to -read the cookie. -2. session attribute `secure` ensure the cookie will never be sent over an -insecure HTTP connections. +Authelia sets several key cookie attributes to prevent cookie theft: +1. `HttpOnly` is set forbidding client-side code like javascript from access to the cookie. +2. `Secure` is set forbidding the browser from sending the cookie to sites which do not use the https scheme. +3. `SameSite` is by default set to `Lax` which prevents it being sent over cross-origin requests. + +Read about these attributes in detail on the +[MDN](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Set-Cookie). ## Protection against multi-domain cookie attacks diff --git a/internal/configuration/config.template.yml b/internal/configuration/config.template.yml index ef1df435f..154da0aa7 100644 --- a/internal/configuration/config.template.yml +++ b/internal/configuration/config.template.yml @@ -338,6 +338,15 @@ session: ## The name of the session cookie. name: authelia_session + ## The domain to protect. + ## Note: the authenticator must also be in that domain. + ## If empty, the cookie is restricted to the subdomain of the issuer. + domain: example.com + + ## Sets the Cookie SameSite value. Possible options are none, lax, or strict. + ## Please read https://www.authelia.com/docs/configuration/session.html#same_site + same_site: lax + ## The secret to encrypt the session data. This is only used with Redis / Redis Sentinel. ## Secret can also be set using a secret: https://www.authelia.com/docs/configuration/secrets.html secret: insecure_session_secret @@ -359,11 +368,6 @@ session: ## Value of 0 disables remember me. remember_me_duration: 1M - ## The domain to protect. - ## Note: the authenticator must also be in that domain. - ## If empty, the cookie is restricted to the subdomain of the issuer. - domain: example.com - ## ## Redis Provider ## diff --git a/internal/configuration/schema/session.go b/internal/configuration/schema/session.go index c1ce92531..3a29aba3a 100644 --- a/internal/configuration/schema/session.go +++ b/internal/configuration/schema/session.go @@ -31,11 +31,12 @@ type RedisSessionConfiguration struct { // SessionConfiguration represents the configuration related to user sessions. type SessionConfiguration struct { Name string `mapstructure:"name"` + Domain string `mapstructure:"domain"` + SameSite string `mapstructure:"same_site"` Secret string `mapstructure:"secret"` Expiration string `mapstructure:"expiration"` Inactivity string `mapstructure:"inactivity"` RememberMeDuration string `mapstructure:"remember_me_duration"` - Domain string `mapstructure:"domain"` Redis *RedisSessionConfiguration `mapstructure:"redis"` } @@ -45,4 +46,5 @@ var DefaultSessionConfiguration = SessionConfiguration{ Expiration: "1h", Inactivity: "5m", RememberMeDuration: "1M", + SameSite: "lax", } diff --git a/internal/configuration/validator/const.go b/internal/configuration/validator/const.go index 1176657ee..1014c5f79 100644 --- a/internal/configuration/validator/const.go +++ b/internal/configuration/validator/const.go @@ -85,10 +85,11 @@ var validKeys = []string{ // Session Keys. "session.name", + "session.domain", + "session.same_site", "session.expiration", "session.inactivity", "session.remember_me_duration", - "session.domain", // Redis Session Keys. "session.redis.host", diff --git a/internal/configuration/validator/session.go b/internal/configuration/validator/session.go index 635eb79e9..eabef1104 100644 --- a/internal/configuration/validator/session.go +++ b/internal/configuration/validator/session.go @@ -56,6 +56,12 @@ func validateSession(configuration *schema.SessionConfiguration, validator *sche if strings.Contains(configuration.Domain, "*") { validator.Push(errors.New("The domain of the session must be the root domain you're protecting instead of a wildcard domain")) } + + if configuration.SameSite == "" { + configuration.SameSite = schema.DefaultSessionConfiguration.SameSite + } else if configuration.SameSite != "none" && configuration.SameSite != "lax" && configuration.SameSite != "strict" { + validator.Push(errors.New("session same_site is configured incorrectly, must be one of 'none', 'lax', or 'strict'")) + } } func validateRedis(configuration *schema.SessionConfiguration, validator *schema.StructValidator) { diff --git a/internal/configuration/validator/session_test.go b/internal/configuration/validator/session_test.go index 057fc7be9..15bb37d62 100644 --- a/internal/configuration/validator/session_test.go +++ b/internal/configuration/validator/session_test.go @@ -51,6 +51,17 @@ func TestShouldSetDefaultSessionExpiration(t *testing.T) { assert.Equal(t, schema.DefaultSessionConfiguration.Expiration, config.Expiration) } +func TestShouldSetDefaultSessionSameSite(t *testing.T) { + validator := schema.NewStructValidator() + config := newDefaultSessionConfig() + + ValidateSession(&config, validator) + + assert.False(t, validator.HasWarnings()) + assert.False(t, validator.HasErrors()) + assert.Equal(t, schema.DefaultSessionConfiguration.SameSite, config.SameSite) +} + func TestShouldHandleRedisConfigSuccessfully(t *testing.T) { validator := schema.NewStructValidator() config := newDefaultSessionConfig() @@ -381,6 +392,34 @@ func TestShouldRaiseErrorWhenDomainIsWildcard(t *testing.T) { assert.EqualError(t, validator.Errors()[0], "The domain of the session must be the root domain you're protecting instead of a wildcard domain") } +func TestShouldRaiseErrorWhenSameSiteSetIncorrectly(t *testing.T) { + validator := schema.NewStructValidator() + config := newDefaultSessionConfig() + config.SameSite = "NOne" + + ValidateSession(&config, validator) + + assert.False(t, validator.HasWarnings()) + assert.Len(t, validator.Errors(), 1) + assert.EqualError(t, validator.Errors()[0], "session same_site is configured incorrectly, must be one of 'none', 'lax', or 'strict'") +} + +func TestShouldNotRaiseErrorWhenSameSiteSetCorrectly(t *testing.T) { + validator := schema.NewStructValidator() + config := newDefaultSessionConfig() + + validOptions := []string{"none", "lax", "strict"} + + for _, opt := range validOptions { + config.SameSite = opt + + ValidateSession(&config, validator) + + assert.False(t, validator.HasWarnings()) + assert.Len(t, validator.Errors(), 0) + } +} + func TestShouldRaiseErrorWhenBadInactivityAndExpirationSet(t *testing.T) { validator := schema.NewStructValidator() config := newDefaultSessionConfig() diff --git a/internal/session/provider_config.go b/internal/session/provider_config.go index d909a09b3..2c5574b4e 100644 --- a/internal/session/provider_config.go +++ b/internal/session/provider_config.go @@ -24,6 +24,18 @@ func NewProviderConfig(configuration schema.SessionConfiguration, certPool *x509 // Set the cookie to the given domain. config.Domain = configuration.Domain + // Set the cookie SameSite option. + switch configuration.SameSite { + case "strict": + config.CookieSameSite = fasthttp.CookieSameSiteStrictMode + case "none": + config.CookieSameSite = fasthttp.CookieSameSiteNoneMode + case "lax": + config.CookieSameSite = fasthttp.CookieSameSiteLaxMode + default: + config.CookieSameSite = fasthttp.CookieSameSiteLaxMode + } + // Only serve the header over HTTPS. config.Secure = true diff --git a/internal/session/provider_config_test.go b/internal/session/provider_config_test.go index 13edc99ae..40a3438fb 100644 --- a/internal/session/provider_config_test.go +++ b/internal/session/provider_config_test.go @@ -9,6 +9,7 @@ import ( "github.com/fasthttp/session/v2" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/valyala/fasthttp" "github.com/authelia/authelia/internal/configuration/schema" "github.com/authelia/authelia/internal/utils" @@ -27,7 +28,6 @@ func TestShouldCreateInMemorySessionProvider(t *testing.T) { assert.Equal(t, true, providerConfig.config.Secure) assert.Equal(t, time.Duration(40)*time.Second, providerConfig.config.Expiration) assert.True(t, providerConfig.config.IsSecureFunc(nil)) - assert.Equal(t, "memory", providerConfig.providerName) } @@ -185,6 +185,28 @@ func TestShouldCreateRedisSentinelSessionProvider(t *testing.T) { assert.Nil(t, pConfig.TLSConfig) } +func TestShouldSetCookieSameSite(t *testing.T) { + configuration := schema.SessionConfiguration{} + configuration.Domain = testDomain + configuration.Name = testName + configuration.Expiration = testExpiration + + configValueExpectedValue := map[string]fasthttp.CookieSameSite{ + "": fasthttp.CookieSameSiteLaxMode, + "lax": fasthttp.CookieSameSiteLaxMode, + "strict": fasthttp.CookieSameSiteStrictMode, + "none": fasthttp.CookieSameSiteNoneMode, + "invalid": fasthttp.CookieSameSiteLaxMode, + } + + for configValue, expectedValue := range configValueExpectedValue { + configuration.SameSite = configValue + providerConfig := NewProviderConfig(configuration, nil) + + assert.Equal(t, expectedValue, providerConfig.config.CookieSameSite) + } +} + func TestShouldCreateRedisSessionProviderWithUnixSocket(t *testing.T) { configuration := schema.SessionConfiguration{} configuration.Domain = testDomain