From 2096fd70815050374f4320655b7c89ff68e246d8 Mon Sep 17 00:00:00 2001 From: Romain Date: Fri, 8 Nov 2024 12:12:35 +0100 Subject: [PATCH] Drop untrusted X-Forwarded-Prefix header Co-authored-by: Kevin Pollet --- pkg/api/dashboard/dashboard.go | 23 ++-------- pkg/api/dashboard/dashboard_test.go | 45 ------------------- .../forwardedheaders/forwarded_header.go | 2 + .../forwardedheaders/forwarded_header_test.go | 26 +++++++++++ 4 files changed, 31 insertions(+), 65 deletions(-) diff --git a/pkg/api/dashboard/dashboard.go b/pkg/api/dashboard/dashboard.go index 667092dbd..61f5ae4fc 100644 --- a/pkg/api/dashboard/dashboard.go +++ b/pkg/api/dashboard/dashboard.go @@ -3,7 +3,7 @@ package dashboard import ( "io/fs" "net/http" - "net/url" + "strings" "github.com/gorilla/mux" "github.com/traefik/traefik/v2/webui" @@ -25,7 +25,8 @@ func Append(router *mux.Router, customAssets fs.FS) { router.Methods(http.MethodGet). Path("/"). HandlerFunc(func(resp http.ResponseWriter, req *http.Request) { - http.Redirect(resp, req, safePrefix(req)+"/dashboard/", http.StatusFound) + prefix := strings.TrimSuffix(req.Header.Get("X-Forwarded-Prefix"), "/") + http.Redirect(resp, req, prefix+"/dashboard/", http.StatusFound) }) router.Methods(http.MethodGet). @@ -48,21 +49,3 @@ func (g Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { w.Header().Set("Content-Security-Policy", "frame-src 'self' https://traefik.io https://*.traefik.io;") http.FileServerFS(assets).ServeHTTP(w, r) } - -func safePrefix(req *http.Request) string { - prefix := req.Header.Get("X-Forwarded-Prefix") - if prefix == "" { - return "" - } - - parse, err := url.Parse(prefix) - if err != nil { - return "" - } - - if parse.Host != "" { - return "" - } - - return parse.Path -} diff --git a/pkg/api/dashboard/dashboard_test.go b/pkg/api/dashboard/dashboard_test.go index 28c734d94..b07bff4f6 100644 --- a/pkg/api/dashboard/dashboard_test.go +++ b/pkg/api/dashboard/dashboard_test.go @@ -10,53 +10,8 @@ import ( "time" "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" ) -func Test_safePrefix(t *testing.T) { - testCases := []struct { - desc string - value string - expected string - }{ - { - desc: "host", - value: "https://example.com", - expected: "", - }, - { - desc: "host with path", - value: "https://example.com/foo/bar?test", - expected: "", - }, - { - desc: "path", - value: "/foo/bar", - expected: "/foo/bar", - }, - { - desc: "path without leading slash", - value: "foo/bar", - expected: "foo/bar", - }, - } - - for _, test := range testCases { - t.Run(test.desc, func(t *testing.T) { - t.Parallel() - - req, err := http.NewRequest(http.MethodGet, "http://localhost", nil) - require.NoError(t, err) - - req.Header.Set("X-Forwarded-Prefix", test.value) - - prefix := safePrefix(req) - - assert.Equal(t, test.expected, prefix) - }) - } -} - func Test_ContentSecurityPolicy(t *testing.T) { testCases := []struct { desc string diff --git a/pkg/middlewares/forwardedheaders/forwarded_header.go b/pkg/middlewares/forwardedheaders/forwarded_header.go index 6f715ccfb..4b037a089 100644 --- a/pkg/middlewares/forwardedheaders/forwarded_header.go +++ b/pkg/middlewares/forwardedheaders/forwarded_header.go @@ -20,6 +20,7 @@ const ( xForwardedServer = "X-Forwarded-Server" xForwardedURI = "X-Forwarded-Uri" xForwardedMethod = "X-Forwarded-Method" + xForwardedPrefix = "X-Forwarded-Prefix" xForwardedTLSClientCert = "X-Forwarded-Tls-Client-Cert" xForwardedTLSClientCertInfo = "X-Forwarded-Tls-Client-Cert-Info" xRealIP = "X-Real-Ip" @@ -35,6 +36,7 @@ var xHeaders = []string{ xForwardedServer, xForwardedURI, xForwardedMethod, + xForwardedPrefix, xForwardedTLSClientCert, xForwardedTLSClientCertInfo, xRealIP, diff --git a/pkg/middlewares/forwardedheaders/forwarded_header_test.go b/pkg/middlewares/forwardedheaders/forwarded_header_test.go index 414fd5007..8289e8c69 100644 --- a/pkg/middlewares/forwardedheaders/forwarded_header_test.go +++ b/pkg/middlewares/forwardedheaders/forwarded_header_test.go @@ -48,6 +48,7 @@ func TestServeHTTP(t *testing.T) { xForwardedMethod: {"GET"}, xForwardedTLSClientCert: {"Cert"}, xForwardedTLSClientCertInfo: {"CertInfo"}, + xForwardedPrefix: {"/prefix"}, }, expectedHeaders: map[string]string{ xForwardedFor: "10.0.1.0, 10.0.1.12", @@ -55,6 +56,7 @@ func TestServeHTTP(t *testing.T) { xForwardedMethod: "GET", xForwardedTLSClientCert: "Cert", xForwardedTLSClientCertInfo: "CertInfo", + xForwardedPrefix: "/prefix", }, }, { @@ -68,6 +70,7 @@ func TestServeHTTP(t *testing.T) { xForwardedMethod: {"GET"}, xForwardedTLSClientCert: {"Cert"}, xForwardedTLSClientCertInfo: {"CertInfo"}, + xForwardedPrefix: {"/prefix"}, }, expectedHeaders: map[string]string{ xForwardedFor: "", @@ -75,6 +78,7 @@ func TestServeHTTP(t *testing.T) { xForwardedMethod: "", xForwardedTLSClientCert: "", xForwardedTLSClientCertInfo: "", + xForwardedPrefix: "", }, }, { @@ -88,6 +92,7 @@ func TestServeHTTP(t *testing.T) { xForwardedMethod: {"GET"}, xForwardedTLSClientCert: {"Cert"}, xForwardedTLSClientCertInfo: {"CertInfo"}, + xForwardedPrefix: {"/prefix"}, }, expectedHeaders: map[string]string{ xForwardedFor: "10.0.1.0, 10.0.1.12", @@ -95,6 +100,7 @@ func TestServeHTTP(t *testing.T) { xForwardedMethod: "GET", xForwardedTLSClientCert: "Cert", xForwardedTLSClientCertInfo: "CertInfo", + xForwardedPrefix: "/prefix", }, }, { @@ -108,6 +114,7 @@ func TestServeHTTP(t *testing.T) { xForwardedMethod: {"GET"}, xForwardedTLSClientCert: {"Cert"}, xForwardedTLSClientCertInfo: {"CertInfo"}, + xForwardedPrefix: {"/prefix"}, }, expectedHeaders: map[string]string{ xForwardedFor: "", @@ -115,6 +122,7 @@ func TestServeHTTP(t *testing.T) { xForwardedMethod: "", xForwardedTLSClientCert: "", xForwardedTLSClientCertInfo: "", + xForwardedPrefix: "", }, }, { @@ -128,6 +136,7 @@ func TestServeHTTP(t *testing.T) { xForwardedMethod: {"GET"}, xForwardedTLSClientCert: {"Cert"}, xForwardedTLSClientCertInfo: {"CertInfo"}, + xForwardedPrefix: {"/prefix"}, }, expectedHeaders: map[string]string{ xForwardedFor: "10.0.1.0, 10.0.1.12", @@ -135,6 +144,7 @@ func TestServeHTTP(t *testing.T) { xForwardedMethod: "GET", xForwardedTLSClientCert: "Cert", xForwardedTLSClientCertInfo: "CertInfo", + xForwardedPrefix: "/prefix", }, }, { @@ -148,6 +158,7 @@ func TestServeHTTP(t *testing.T) { xForwardedMethod: {"GET"}, xForwardedTLSClientCert: {"Cert"}, xForwardedTLSClientCertInfo: {"CertInfo"}, + xForwardedPrefix: {"/prefix"}, }, expectedHeaders: map[string]string{ xForwardedFor: "", @@ -155,6 +166,7 @@ func TestServeHTTP(t *testing.T) { xForwardedMethod: "", xForwardedTLSClientCert: "", xForwardedTLSClientCertInfo: "", + xForwardedPrefix: "", }, }, { @@ -283,6 +295,7 @@ func TestServeHTTP(t *testing.T) { xForwardedPort, xForwardedTLSClientCert, xForwardedTLSClientCertInfo, + xForwardedPrefix, xRealIP, }, xForwardedProto: {"foo"}, @@ -293,6 +306,7 @@ func TestServeHTTP(t *testing.T) { xForwardedPort: {"foo"}, xForwardedTLSClientCert: {"foo"}, xForwardedTLSClientCertInfo: {"foo"}, + xForwardedPrefix: {"foo"}, xRealIP: {"foo"}, }, expectedHeaders: map[string]string{ @@ -304,6 +318,7 @@ func TestServeHTTP(t *testing.T) { xForwardedPort: "80", xForwardedTLSClientCert: "", xForwardedTLSClientCertInfo: "", + xForwardedPrefix: "", xRealIP: "", connection: "", }, @@ -321,6 +336,7 @@ func TestServeHTTP(t *testing.T) { xForwardedPort, xForwardedTLSClientCert, xForwardedTLSClientCertInfo, + xForwardedPrefix, xRealIP, }, xForwardedProto: {"foo"}, @@ -331,6 +347,7 @@ func TestServeHTTP(t *testing.T) { xForwardedPort: {"foo"}, xForwardedTLSClientCert: {"foo"}, xForwardedTLSClientCertInfo: {"foo"}, + xForwardedPrefix: {"foo"}, xRealIP: {"foo"}, }, expectedHeaders: map[string]string{ @@ -342,6 +359,7 @@ func TestServeHTTP(t *testing.T) { xForwardedPort: "foo", xForwardedTLSClientCert: "foo", xForwardedTLSClientCertInfo: "foo", + xForwardedPrefix: "foo", xRealIP: "foo", connection: "", }, @@ -358,6 +376,7 @@ func TestServeHTTP(t *testing.T) { xForwardedPort, xForwardedTLSClientCert, xForwardedTLSClientCertInfo, + xForwardedPrefix, xRealIP, }, incomingHeaders: map[string][]string{ @@ -370,6 +389,7 @@ func TestServeHTTP(t *testing.T) { xForwardedPort, xForwardedTLSClientCert, xForwardedTLSClientCertInfo, + xForwardedPrefix, xRealIP, }, xForwardedProto: {"foo"}, @@ -380,6 +400,7 @@ func TestServeHTTP(t *testing.T) { xForwardedPort: {"foo"}, xForwardedTLSClientCert: {"foo"}, xForwardedTLSClientCertInfo: {"foo"}, + xForwardedPrefix: {"foo"}, xRealIP: {"foo"}, }, expectedHeaders: map[string]string{ @@ -391,6 +412,7 @@ func TestServeHTTP(t *testing.T) { xForwardedPort: "80", xForwardedTLSClientCert: "", xForwardedTLSClientCertInfo: "", + xForwardedPrefix: "", xRealIP: "", connection: "", }, @@ -407,6 +429,7 @@ func TestServeHTTP(t *testing.T) { xForwardedPort, xForwardedTLSClientCert, xForwardedTLSClientCertInfo, + xForwardedPrefix, xRealIP, }, incomingHeaders: map[string][]string{ @@ -419,6 +442,7 @@ func TestServeHTTP(t *testing.T) { xForwardedPort, xForwardedTLSClientCert, xForwardedTLSClientCertInfo, + xForwardedPrefix, xRealIP, }, xForwardedProto: {"foo"}, @@ -429,6 +453,7 @@ func TestServeHTTP(t *testing.T) { xForwardedPort: {"foo"}, xForwardedTLSClientCert: {"foo"}, xForwardedTLSClientCertInfo: {"foo"}, + xForwardedPrefix: {"foo"}, xRealIP: {"foo"}, }, expectedHeaders: map[string]string{ @@ -440,6 +465,7 @@ func TestServeHTTP(t *testing.T) { xForwardedPort: "foo", xForwardedTLSClientCert: "foo", xForwardedTLSClientCertInfo: "foo", + xForwardedPrefix: "foo", xRealIP: "foo", connection: "", },