From e32dd980759d0877287f7db88f1be30718f7228a Mon Sep 17 00:00:00 2001 From: Stojan Dimitrovski Date: Tue, 22 Oct 2024 10:52:35 +0200 Subject: [PATCH] fix: external host validation --- internal/api/mail_test.go | 7 ++ internal/api/middleware.go | 90 ++++++++++++++++++++---- internal/api/middleware_test.go | 118 +++++++++++++++++++++++++++++--- internal/conf/configuration.go | 2 + 4 files changed, 194 insertions(+), 23 deletions(-) diff --git a/internal/api/mail_test.go b/internal/api/mail_test.go index 677a94f7ed..87fa946f25 100644 --- 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 index 7103c29a63..30f19e3ad6 100644 --- 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 index ced249f6cb..ac41de9c21 100644 --- 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 index ad4e486354..5d50605239 100644 --- 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 {