fix: external host validation (#1808)

Adds a new config option `GOTRUE_MAILER_EXTERNAL_HOSTS` that serves as an allowlist to all the acceptable hosts provided to the APIs. Fixes #1228
supabase · Oct 22, 2024 · 4f6a461 · 4f6a461
1 parent 14b375f
commit 4f6a461
Show file tree

Hide file tree

Showing 4 changed files with 194 additions and 23 deletions.
diff --git a/internal/api/mail_test.go b/internal/api/mail_test.go
@@ -206,6 +206,11 @@ func (ts *MailTestSuite) TestGenerateLink() {
 	customDomainUrl, err := url.ParseRequestURI("https://example.gotrue.com")
 	require.NoError(ts.T(), err)
 
+	originalHosts := ts.API.config.Mailer.ExternalHosts
+	ts.API.config.Mailer.ExternalHosts = []string{
+		"example.gotrue.com",
+	}
+
 	for _, c := range cases {
 		ts.Run(c.Desc, func() {
 			var buffer bytes.Buffer
@@ -239,6 +244,8 @@ func (ts *MailTestSuite) TestGenerateLink() {
 			require.Equal(ts.T(), req.Host, u.Host)
 		})
 	}
+
+	ts.API.config.Mailer.ExternalHosts = originalHosts
 }
 
 func (ts *MailTestSuite) setURIAllowListMap(uris ...string) {

diff --git a/internal/api/middleware.go b/internal/api/middleware.go
@@ -141,26 +141,86 @@ func (a *API) isValidExternalHost(w http.ResponseWriter, req *http.Request) (con
 	ctx := req.Context()
 	config := a.config
 
-	var u *url.URL
-	var err error
-
-	baseUrl := config.API.ExternalURL
 	xForwardedHost := req.Header.Get("X-Forwarded-Host")
 	xForwardedProto := req.Header.Get("X-Forwarded-Proto")
-	if xForwardedHost != "" && xForwardedProto != "" {
-		baseUrl = fmt.Sprintf("%s://%s", xForwardedProto, xForwardedHost)
-	} else if req.URL.Scheme != "" && req.URL.Hostname() != "" {
-		baseUrl = fmt.Sprintf("%s://%s", req.URL.Scheme, req.URL.Hostname())
+	reqHost := req.URL.Hostname()
+
+	if len(config.Mailer.ExternalHosts) > 0 {
+		// this server is configured to accept multiple external hosts, validate the host from the X-Forwarded-Host or Host headers
+
+		hostname := ""
+		protocol := "https"
+
+		if xForwardedHost != "" {
+			for _, host := range config.Mailer.ExternalHosts {
+				if host == xForwardedHost {
+					hostname = host
+					break
+				}
+			}
+		} else if reqHost != "" {
+			for _, host := range config.Mailer.ExternalHosts {
+				if host == reqHost {
+					hostname = host
+					break
+				}
+			}
+		}
+
+		if hostname != "" {
+			if hostname == "localhost" {
+				// allow the use of HTTP only if the accepted hostname was localhost
+				if xForwardedProto == "http" || req.URL.Scheme == "http" {
+					protocol = "http"
+				}
+			}
+
+			externalHostURL, err := url.ParseRequestURI(fmt.Sprintf("%s://%s", protocol, hostname))
+			if err != nil {
+				return ctx, err
+			}
+
+			return withExternalHost(ctx, externalHostURL), nil
+		}
 	}
-	if u, err = url.ParseRequestURI(baseUrl); err != nil {
-		// fallback to the default hostname
-		log := observability.GetLogEntry(req).Entry
-		log.WithField("request_url", baseUrl).Warn(err)
-		if u, err = url.ParseRequestURI(config.API.ExternalURL); err != nil {
-			return ctx, err
+
+	if xForwardedHost != "" || reqHost != "" {
+		// host has been provided to the request, but it hasn't been
+		// added to the allow list, raise a log message
+		// in Supabase platform the X-Forwarded-Host and full request
+		// URL are likely sanitzied before they reach the server
+
+		fields := make(logrus.Fields)
+
+		if xForwardedHost != "" {
+			fields["x_forwarded_host"] = xForwardedHost
+		}
+
+		if xForwardedProto != "" {
+			fields["x_forwarded_proto"] = xForwardedProto
+		}
+
+		if reqHost != "" {
+			fields["request_url_host"] = reqHost
+
+			if req.URL.Scheme != "" {
+				fields["request_url_scheme"] = req.URL.Scheme
+			}
 		}
+
+		logrus.WithFields(fields).Info("Request received external host in X-Forwarded-Host or Host headers, but the values have not been added to GOTRUE_MAILER_EXTERNAL_HOSTS and will not be used. To suppress this message add the host, or sanitize the headers before the request reaches Auth.")
+	}
+
+	// either the provided external hosts don't match the allow list, or
+	// the server is not configured to accept multiple hosts -- use the
+	// configured external URL instead
+
+	externalHostURL, err := url.ParseRequestURI(config.API.ExternalURL)
+	if err != nil {
+		return ctx, err
 	}
-	return withExternalHost(ctx, u), nil
+
+	return withExternalHost(ctx, externalHostURL), nil
 }
 
 func (a *API) requireSAMLEnabled(w http.ResponseWriter, req *http.Request) (context.Context, error) {

diff --git a/internal/api/middleware_test.go b/internal/api/middleware_test.go
@@ -7,7 +7,6 @@ import (
 	"io"
 	"net/http"
 	"net/http/httptest"
-	"net/url"
 	"testing"
 	"time"
 
@@ -187,25 +186,128 @@ func (ts *MiddlewareTestSuite) TestVerifyCaptchaInvalid() {
 
 func (ts *MiddlewareTestSuite) TestIsValidExternalHost() {
 	cases := []struct {
-		desc        string
-		requestURL  string
+		desc          string
+		externalHosts []string
+
+		requestURL string
+		headers    http.Header
+
 		expectedURL string
 	}{
 		{
-			desc:        "Valid custom external url",
-			requestURL:  "https://example.custom.com",
-			expectedURL: "https://example.custom.com",
+			desc:        "no defined external hosts, no headers, no absolute request URL",
+			requestURL:  "/some-path",
+			expectedURL: ts.API.config.API.ExternalURL,
+		},
+
+		{
+			desc: "no defined external hosts, unauthorized X-Forwarded-Host without any external hosts",
+			headers: http.Header{
+				"X-Forwarded-Host": []string{
+					"external-host.com",
+				},
+			},
+			requestURL:  "/some-path",
+			expectedURL: ts.API.config.API.ExternalURL,
+		},
+
+		{
+			desc:          "defined external hosts, unauthorized X-Forwarded-Host",
+			externalHosts: []string{"authorized-host.com"},
+			headers: http.Header{
+				"X-Forwarded-Proto": []string{"https"},
+				"X-Forwarded-Host": []string{
+					"external-host.com",
+				},
+			},
+			requestURL:  "/some-path",
+			expectedURL: ts.API.config.API.ExternalURL,
+		},
+
+		{
+			desc:        "no defined external hosts, unauthorized Host",
+			requestURL:  "https://external-host.com/some-path",
+			expectedURL: ts.API.config.API.ExternalURL,
+		},
+
+		{
+			desc:          "defined external hosts, unauthorized Host",
+			externalHosts: []string{"authorized-host.com"},
+			requestURL:    "https://external-host.com/some-path",
+			expectedURL:   ts.API.config.API.ExternalURL,
+		},
+
+		{
+			desc:          "defined external hosts, authorized X-Forwarded-Host",
+			externalHosts: []string{"authorized-host.com"},
+			headers: http.Header{
+				"X-Forwarded-Proto": []string{"http"}, // this should be ignored and default to HTTPS
+				"X-Forwarded-Host": []string{
+					"authorized-host.com",
+				},
+			},
+			requestURL:  "https://X-Forwarded-Host-takes-precedence.com/some-path",
+			expectedURL: "https://authorized-host.com",
+		},
+
+		{
+			desc:          "defined external hosts, authorized Host",
+			externalHosts: []string{"authorized-host.com"},
+			requestURL:    "https://authorized-host.com/some-path",
+			expectedURL:   "https://authorized-host.com",
+		},
+
+		{
+			desc:          "defined external hosts, authorized X-Forwarded-Host",
+			externalHosts: []string{"authorized-host.com"},
+			headers: http.Header{
+				"X-Forwarded-Proto": []string{"http"}, // this should be ignored and default to HTTPS
+				"X-Forwarded-Host": []string{
+					"authorized-host.com",
+				},
+			},
+			requestURL:  "https://X-Forwarded-Host-takes-precedence.com/some-path",
+			expectedURL: "https://authorized-host.com",
+		},
+
+		{
+			desc:          "defined external hosts, authorized localhost in X-Forwarded-Host with HTTP",
+			externalHosts: []string{"localhost"},
+			headers: http.Header{
+				"X-Forwarded-Proto": []string{"http"},
+				"X-Forwarded-Host": []string{
+					"localhost",
+				},
+			},
+			requestURL:  "/some-path",
+			expectedURL: "http://localhost",
+		},
+
+		{
+			desc:          "defined external hosts, authorized localhost in Host with HTTP",
+			externalHosts: []string{"localhost"},
+			requestURL:    "http://localhost:3000/some-path",
+			expectedURL:   "http://localhost",
 		},
 	}
 
-	_, err := url.ParseRequestURI("https://example.custom.com")
-	require.NoError(ts.T(), err)
+	require.NotEmpty(ts.T(), ts.API.config.API.ExternalURL)
 
 	for _, c := range cases {
 		ts.Run(c.desc, func() {
 			req := httptest.NewRequest(http.MethodPost, c.requestURL, nil)
+			if c.headers != nil {
+				req.Header = c.headers
+			}
+
+			originalHosts := ts.API.config.Mailer.ExternalHosts
+			ts.API.config.Mailer.ExternalHosts = c.externalHosts
+
 			w := httptest.NewRecorder()
 			ctx, err := ts.API.isValidExternalHost(w, req)
+
+			ts.API.config.Mailer.ExternalHosts = originalHosts
+
 			require.NoError(ts.T(), err)
 
 			externalURL := getExternalHost(ctx)

diff --git a/internal/conf/configuration.go b/internal/conf/configuration.go
@@ -397,6 +397,8 @@ type MailerConfiguration struct {
 
 	OtpExp    uint `json:"otp_exp" split_words:"true"`
 	OtpLength int  `json:"otp_length" split_words:"true"`
+
+	ExternalHosts []string `json:"external_hosts" split_words:"true"`
 }
 
 type PhoneProviderConfiguration struct {