feat(configuration): disallow public suffix domains (#4855)

This adds a check to the domains configuration to ensure the domain value is not part of the public suffix list at https://publicsuffix.org. These domains are special and users cannot write cookies with this domain value, this makes them unusable with Authelia and this more readily makes that apparent.

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
+The value must not match a domain on the [Public Suffix List](https://publicsuffix.org/list/) as 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`
+websites to write cookies for these domains. This includes most Dynamic DNS services such as `duckdns.org`. You should
-for the domain value as browsers will not allow `john.duckdns.org` to read or write cookies for `duckdns.org`.
+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
 

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

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.

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)))
 	}
 }
 

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, nil},
-		{"ShouldNotRaiseErrorOnValidDomain", exampleDotCom, 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)
 			}

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)
+}

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))
+			})
+		})
+	}
+}