diff --git a/docs/content/en/configuration/session/introduction.md b/docs/content/en/configuration/session/introduction.md index f60159845..4a5ed19ca 100644 --- a/docs/content/en/configuration/session/introduction.md +++ b/docs/content/en/configuration/session/introduction.md @@ -137,11 +137,12 @@ cookies for this domain. For example if Authelia is accessible via the URL `https://auth.example.com` the domain should be either `auth.example.com` or `example.com`. -Please note most good DynamicDNS solutions fall into a specially protected group of domains and browsers do not allow -you to write cookies for the root domain. i.e. if you have been assigned `john.duckdns.org` you can't use `duckdns.org` -for the domain value as browsers will not allow `john.duckdns.org` to read or write cookies for `duckdns.org`. +The value must not match a domain on the [Public Suffix List](https://publicsuffix.org/list/) as browsers do not allow +websites to write cookies for these domains. This includes most Dynamic DNS services such as `duckdns.org`. You should +use your domain instead of `duckdns.org` for this value, for example `example.duckdns.org`. -Consequently, if you have `john.duckdns.org` and `mary.duckdns.org` you cannot share cookies between these domains. +Consequently, if you have `example.duckdns.org` and `example-auth.duckdns.org` you cannot share cookies between these +domains. #### authelia_url diff --git a/go.mod b/go.mod index 95540f2ec..c2128d819 100644 --- a/go.mod +++ b/go.mod @@ -40,6 +40,7 @@ require ( github.com/trustelem/zxcvbn v1.0.1 github.com/valyala/fasthttp v1.44.0 github.com/wneessen/go-mail v0.3.8 + golang.org/x/net v0.5.0 golang.org/x/sync v0.1.0 golang.org/x/term v0.4.0 golang.org/x/text v0.6.0 @@ -109,7 +110,6 @@ require ( github.com/ysmood/leakless v0.8.0 // indirect golang.org/x/crypto v0.5.0 // indirect golang.org/x/mod v0.7.0 // indirect - golang.org/x/net v0.5.0 // indirect golang.org/x/oauth2 v0.0.0-20221014153046-6fdb5e3db783 // indirect golang.org/x/sys v0.4.0 // indirect golang.org/x/tools v0.4.0 // indirect diff --git a/internal/configuration/validator/const.go b/internal/configuration/validator/const.go index 7573a02cc..786200771 100644 --- a/internal/configuration/validator/const.go +++ b/internal/configuration/validator/const.go @@ -267,7 +267,9 @@ const ( errFmtSessionDomainDuplicateCookieScope = "session: domain config %s: option 'domain' shares the same cookie domain scope as another configured session domain" errFmtSessionDomainPortalURLInsecure = "session: domain config %s: option 'authelia_url' does not have a secure scheme with a value of '%s'" errFmtSessionDomainPortalURLNotInCookieScope = "session: domain config %s: option 'authelia_url' does not share a cookie scope with domain '%s' with a value of '%s'" - errFmtSessionDomainInvalidDomain = "session: domain config %s: option 'domain' is not a valid domain" + errFmtSessionDomainInvalidDomain = "session: domain config %s: option 'domain' is not a valid cookie domain" + errFmtSessionDomainInvalidDomainNoDots = "session: domain config %s: option 'domain' is not a valid cookie domain: must have at least a single period" + errFmtSessionDomainInvalidDomainPublic = "session: domain config %s: option 'domain' is not a valid cookie domain: the domain is part of the special public suffix list" ) // Regulation Error Consts. diff --git a/internal/configuration/validator/session.go b/internal/configuration/validator/session.go index 08fdfeb0e..f63d24ded 100644 --- a/internal/configuration/validator/session.go +++ b/internal/configuration/validator/session.go @@ -104,12 +104,22 @@ func validateSessionDomainName(i int, config *schema.SessionConfiguration, valid switch { case d.Domain == "": validator.Push(fmt.Errorf(errFmtSessionDomainRequired, sessionDomainDescriptor(i, d))) + return case strings.HasPrefix(d.Domain, "*."): validator.Push(fmt.Errorf(errFmtSessionDomainMustBeRoot, sessionDomainDescriptor(i, d), d.Domain)) + return case strings.HasPrefix(d.Domain, "."): validator.PushWarning(fmt.Errorf(errFmtSessionDomainHasPeriodPrefix, sessionDomainDescriptor(i, d))) + case !strings.Contains(d.Domain, "."): + validator.Push(fmt.Errorf(errFmtSessionDomainInvalidDomainNoDots, sessionDomainDescriptor(i, d))) + return case !reDomainCharacters.MatchString(d.Domain): validator.Push(fmt.Errorf(errFmtSessionDomainInvalidDomain, sessionDomainDescriptor(i, d))) + return + } + + if isCookieDomainAPublicSuffix(d.Domain) { + validator.Push(fmt.Errorf(errFmtSessionDomainInvalidDomainPublic, sessionDomainDescriptor(i, d))) } } diff --git a/internal/configuration/validator/session_test.go b/internal/configuration/validator/session_test.go index d677df581..f8db62b5b 100644 --- a/internal/configuration/validator/session_test.go +++ b/internal/configuration/validator/session_test.go @@ -586,45 +586,6 @@ func TestShouldRaiseErrorOnBadRedisTLSOptionsMinVerGreaterThanMax(t *testing.T) assert.EqualError(t, validator.Errors()[0], "session: redis: tls: option combination of 'minimum_version' and 'maximum_version' is invalid: minimum version TLS1.3 is greater than the maximum version TLS1.0") } -func TestShouldRaiseErrorWhenDomainNotSet(t *testing.T) { - validator := schema.NewStructValidator() - config := newDefaultSessionConfig() - config.Domain = "" - config.Cookies = []schema.SessionCookieConfiguration{} - - ValidateSession(&config, validator) - - assert.False(t, validator.HasWarnings()) - assert.Len(t, validator.Errors(), 1) - assert.EqualError(t, validator.Errors()[0], "session: option 'domain' is required") -} - -func TestShouldRaiseErrorWhenDomainIsWildcard(t *testing.T) { - validator := schema.NewStructValidator() - config := newDefaultSessionConfig() - config.Domain = "*.example.com" - - ValidateSession(&config, validator) - - assert.Len(t, validator.Warnings(), 0) - require.Len(t, validator.Errors(), 1) - - assert.EqualError(t, validator.Errors()[0], "session: domain config #1 (domain '*.example.com'): option 'domain' must be the domain you wish to protect not a wildcard domain but it is configured as '*.example.com'") -} - -func TestShouldRaiseErrorWhenDomainNameIsInvalid(t *testing.T) { - validator := schema.NewStructValidator() - config := newDefaultSessionConfig() - config.Domain = "example!.com" - - ValidateSession(&config, validator) - - assert.Len(t, validator.Warnings(), 0) - require.Len(t, validator.Errors(), 1) - - assert.EqualError(t, validator.Errors()[0], "session: domain config #1 (domain 'example!.com'): option 'domain' is not a valid domain") -} - func TestShouldRaiseErrorWhenHaveDuplicatedDomainName(t *testing.T) { validator := schema.NewStructValidator() config := newDefaultSessionConfig() @@ -675,10 +636,18 @@ func TestShouldRaiseErrorWhenDomainIsInvalid(t *testing.T) { testCases := []struct { name string have string + warnings []string expected []string }{ - {"ShouldRaiseErrorOnMissingDomain", "", []string{"session: domain config #1 (domain ''): option 'domain' is required"}}, - {"ShouldNotRaiseErrorOnValidDomain", exampleDotCom, nil}, + {"ShouldNotRaiseErrorOnValidDomain", exampleDotCom, nil, nil}, + {"ShouldRaiseErrorOnMissingDomain", "", nil, []string{"session: domain config #1 (domain ''): option 'domain' is required"}}, + {"ShouldRaiseErrorOnDomainWithInvalidChars", "example!.com", nil, []string{"session: domain config #1 (domain 'example!.com'): option 'domain' is not a valid cookie domain"}}, + {"ShouldRaiseErrorOnDomainWithoutDots", "localhost", nil, []string{"session: domain config #1 (domain 'localhost'): option 'domain' is not a valid cookie domain: must have at least a single period"}}, + {"ShouldRaiseErrorOnPublicDomainDuckDNS", "duckdns.org", nil, []string{"session: domain config #1 (domain 'duckdns.org'): option 'domain' is not a valid cookie domain: the domain is part of the special public suffix list"}}, + {"ShouldNotRaiseErrorOnSuffixOfPublicDomainDuckDNS", "example.duckdns.org", nil, nil}, + {"ShouldRaiseWarningOnDomainWithLeadingDot", ".example.com", []string{"session: domain config #1 (domain '.example.com'): option 'domain' has a prefix of '.' which is not supported or intended behaviour: you can use this at your own risk but we recommend removing it"}, nil}, + {"ShouldRaiseErrorOnDomainWithLeadingStarDot", "*.example.com", nil, []string{"session: domain config #1 (domain '*.example.com'): option 'domain' must be the domain you wish to protect not a wildcard domain but it is configured as '*.example.com'"}}, + {"ShouldRaiseErrorOnDomainNotSet", "", nil, []string{"session: domain config #1 (domain ''): option 'domain' is required"}}, } for _, tc := range testCases { @@ -692,14 +661,18 @@ func TestShouldRaiseErrorWhenDomainIsInvalid(t *testing.T) { SessionCookieCommonConfiguration: schema.SessionCookieCommonConfiguration{ Domain: tc.have, }, - AutheliaURL: MustParseURL("https://auth.example.com")}, + }, } ValidateSession(&config, validator) - assert.Len(t, validator.Warnings(), 0) + require.Len(t, validator.Warnings(), len(tc.warnings)) require.Len(t, validator.Errors(), len(tc.expected)) + for i, expected := range tc.warnings { + assert.EqualError(t, validator.Warnings()[i], expected) + } + for i, expected := range tc.expected { assert.EqualError(t, validator.Errors()[i], expected) } diff --git a/internal/configuration/validator/util.go b/internal/configuration/validator/util.go new file mode 100644 index 000000000..b68bfee7a --- /dev/null +++ b/internal/configuration/validator/util.go @@ -0,0 +1,15 @@ +package validator + +import ( + "strings" + + "golang.org/x/net/publicsuffix" +) + +func isCookieDomainAPublicSuffix(domain string) (valid bool) { + var suffix string + + suffix, _ = publicsuffix.PublicSuffix(domain) + + return len(strings.TrimLeft(domain, ".")) == len(suffix) +} diff --git a/internal/configuration/validator/util_test.go b/internal/configuration/validator/util_test.go new file mode 100644 index 000000000..54c9bc711 --- /dev/null +++ b/internal/configuration/validator/util_test.go @@ -0,0 +1,40 @@ +package validator + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestIsCookieDomainValid(t *testing.T) { + testCases := []struct { + domain string + expected bool + }{ + {"example.com", false}, + {".example.com", false}, + {"*.example.com", false}, + {"authelia.com", false}, + {"duckdns.org", true}, + {".duckdns.org", true}, + {"example.duckdns.org", false}, + {"192.168.2.1", false}, + {"localhost", true}, + {"com", true}, + {"randomnada", true}, + } + + for _, tc := range testCases { + name := "ShouldFail" + + if tc.expected { + name = "ShouldPass" + } + + t.Run(tc.domain, func(t *testing.T) { + t.Run(name, func(t *testing.T) { + assert.Equal(t, tc.expected, isCookieDomainAPublicSuffix(tc.domain)) + }) + }) + } +}