From 78e2dab1559b8465db81798a2c6a92aeb41ee691 Mon Sep 17 00:00:00 2001 From: Landry Benguigui Date: Fri, 19 Dec 2025 11:18:04 +0100 Subject: [PATCH] feat: add global option to disable X-Forwarded-For appending --- .../configuration-options.md | 2 + .../install-configuration/entrypoints.md | 1 + integration/fixtures/x_forwarded_for.toml | 12 ++ .../fixtures/x_forwarded_for_enabled.toml | 11 + .../fixtures/x_forwarded_for_fastproxy.toml | 16 ++ .../x_forwarded_for_fastproxy_enabled.toml | 15 ++ .../resources/compose/x_forwarded_for.toml | 10 + integration/simple_test.go | 191 ++++++++++++++++++ pkg/config/static/entrypoints.go | 7 +- pkg/config/static/static_config.go | 5 +- .../forwardedheaders/forwarded_header.go | 33 +-- .../forwardedheaders/forwarded_header_test.go | 4 +- pkg/proxy/fast/proxy.go | 24 ++- pkg/proxy/fast/proxy_test.go | 85 ++++++++ pkg/proxy/httputil/proxy.go | 99 +++++++-- pkg/proxy/httputil/proxy_test.go | 62 ++++-- pkg/server/server_entrypoint_tcp.go | 1 + 17 files changed, 514 insertions(+), 64 deletions(-) create mode 100644 integration/fixtures/x_forwarded_for.toml create mode 100644 integration/fixtures/x_forwarded_for_enabled.toml create mode 100644 integration/fixtures/x_forwarded_for_fastproxy.toml create mode 100644 integration/fixtures/x_forwarded_for_fastproxy_enabled.toml create mode 100644 integration/resources/compose/x_forwarded_for.toml diff --git a/docs/content/reference/install-configuration/configuration-options.md b/docs/content/reference/install-configuration/configuration-options.md index 3da346ee6..dcbcfeec1 100644 --- a/docs/content/reference/install-configuration/configuration-options.md +++ b/docs/content/reference/install-configuration/configuration-options.md @@ -83,6 +83,7 @@ THIS FILE MUST NOT BE EDITED BY HAND | entrypoints._name_.asdefault | Adds this EntryPoint to the list of default EntryPoints to be used on routers that don't have any Entrypoint defined. | false | | entrypoints._name_.forwardedheaders.connection | List of Connection headers that are allowed to pass through the middleware chain before being removed. | | | entrypoints._name_.forwardedheaders.insecure | Trust all forwarded headers. | false | +| entrypoints._name_.forwardedheaders.notappendxforwardedfor | Disable appending RemoteAddr to X-Forwarded-For header. Defaults to false (appending is enabled). | false | | entrypoints._name_.forwardedheaders.trustedips | Trust only forwarded headers from selected IPs. | | | entrypoints._name_.http | HTTP configuration. | | | entrypoints._name_.http.encodequerysemicolons | Defines whether request query semicolons should be URLEncoded. | false | @@ -141,6 +142,7 @@ THIS FILE MUST NOT BE EDITED BY HAND | experimental.plugins._name_.settings.useunsafe | Allow the plugin to use unsafe and syscall packages. | false | | experimental.plugins._name_.version | plugin's version. | | | global.checknewversion | Periodically check if a new version has been released. | true | +| global.notappendxforwardedfor | Disable appending RemoteAddr to X-Forwarded-For header. Defaults to false (appending is enabled). | false | | global.sendanonymoususage | Periodically send anonymous usage statistics. If the option is not specified, it will be disabled by default. | false | | hostresolver | Enable CNAME Flattening. | false | | hostresolver.cnameflattening | A flag to enable/disable CNAME flattening | false | diff --git a/docs/content/reference/install-configuration/entrypoints.md b/docs/content/reference/install-configuration/entrypoints.md index 9ae9e4616..64b1068ca 100644 --- a/docs/content/reference/install-configuration/entrypoints.md +++ b/docs/content/reference/install-configuration/entrypoints.md @@ -90,6 +90,7 @@ additionalArguments: | `asDefault` | Mark the `entryPoint` to be in the list of default `entryPoints`.
`entryPoints`in this list are used (by default) on HTTP and TCP routers that do not define their own `entryPoints` option.
More information [here](#asdefault). | false | No | | `forwardedHeaders.trustedIPs` | Set the IPs or CIDR from where Traefik trusts the forwarded headers information (`X-Forwarded-*`). | - | No | | `forwardedHeaders.insecure` | Set the insecure mode to always trust the forwarded headers information (`X-Forwarded-*`).
We recommend to use this option only for tests purposes, not in production. | false | No | +| `forwardedHeaders.`
`notAppendXForwardedFor` | When set to `true`, Traefik will not append the client's `RemoteAddr` to the `X-Forwarded-For` header. The existing header is preserved as-is. If no `X-Forwarded-For` header exists, none will be added. | false | No | | `http.redirections.`
`entryPoint.to` | The target element to enable (permanent) redirecting of all incoming requests on an entry point to another one.
The target element can be an entry point name (ex: `websecure`), or a port (`:443`). | - | Yes | | `http.redirections.`
`entryPoint.scheme` | The target scheme to use for (permanent) redirection of all incoming requests. | https | No | | `http.redirections.`
`entryPoint.permanent` | Enable permanent redirecting of all incoming requests on an entry point to another one changing the scheme.
The target element, it can be an entry point name (ex: `websecure`), or a port (`:443`). | false | No | diff --git a/integration/fixtures/x_forwarded_for.toml b/integration/fixtures/x_forwarded_for.toml new file mode 100644 index 000000000..804f567b4 --- /dev/null +++ b/integration/fixtures/x_forwarded_for.toml @@ -0,0 +1,12 @@ +[entryPoints] + [entryPoints.web] + address = ":8000" + [entryPoints.web.forwardedHeaders] + insecure = true + notAppendXForwardedFor = true + +[api] + insecure = true + +[providers.file] + filename = "{{ .DynamicConfPath }}" \ No newline at end of file diff --git a/integration/fixtures/x_forwarded_for_enabled.toml b/integration/fixtures/x_forwarded_for_enabled.toml new file mode 100644 index 000000000..9843ae10d --- /dev/null +++ b/integration/fixtures/x_forwarded_for_enabled.toml @@ -0,0 +1,11 @@ +[entryPoints] + [entryPoints.web] + address = ":8000" + [entryPoints.web.forwardedHeaders] + insecure = true + +[api] + insecure = true + +[providers.file] + filename = "{{ .DynamicConfPath }}" \ No newline at end of file diff --git a/integration/fixtures/x_forwarded_for_fastproxy.toml b/integration/fixtures/x_forwarded_for_fastproxy.toml new file mode 100644 index 000000000..c0716bc3f --- /dev/null +++ b/integration/fixtures/x_forwarded_for_fastproxy.toml @@ -0,0 +1,16 @@ +[entryPoints] + [entryPoints.web] + address = ":8000" + [entryPoints.web.forwardedHeaders] + insecure = true + notAppendXForwardedFor = true + +[api] + insecure = true + +[experimental] + [experimental.fastProxy] + debug = true + +[providers.file] + filename = "{{ .DynamicConfPath }}" \ No newline at end of file diff --git a/integration/fixtures/x_forwarded_for_fastproxy_enabled.toml b/integration/fixtures/x_forwarded_for_fastproxy_enabled.toml new file mode 100644 index 000000000..16eae2c21 --- /dev/null +++ b/integration/fixtures/x_forwarded_for_fastproxy_enabled.toml @@ -0,0 +1,15 @@ +[entryPoints] + [entryPoints.web] + address = ":8000" + [entryPoints.web.forwardedHeaders] + insecure = true + +[api] + insecure = true + +[experimental] + [experimental.fastProxy] + debug = true + +[providers.file] + filename = "{{ .DynamicConfPath }}" \ No newline at end of file diff --git a/integration/resources/compose/x_forwarded_for.toml b/integration/resources/compose/x_forwarded_for.toml new file mode 100644 index 000000000..3a6f993e0 --- /dev/null +++ b/integration/resources/compose/x_forwarded_for.toml @@ -0,0 +1,10 @@ +[http.routers] + [http.routers.router1] + entryPoints = ["web"] + rule = "PathPrefix(`/`)" + service = "service1" + +[http.services] + [http.services.service1.loadBalancer] + [[http.services.service1.loadBalancer.servers]] + url = "{{ .Server }}" \ No newline at end of file diff --git a/integration/simple_test.go b/integration/simple_test.go index ed30d7428..55ae1205e 100644 --- a/integration/simple_test.go +++ b/integration/simple_test.go @@ -94,6 +94,197 @@ func (s *SimpleSuite) TestSimpleFastProxy() { assert.GreaterOrEqual(s.T(), 1, callCount) } +func (s *SimpleSuite) TestXForwardedForDisabled() { + srv1 := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) { + // Echo back the X-Forwarded-For header + xff := req.Header.Get("X-Forwarded-For") + _, _ = rw.Write([]byte(xff)) + })) + defer srv1.Close() + + dynamicConf := s.adaptFile("resources/compose/x_forwarded_for.toml", struct { + Server string + }{ + Server: srv1.URL, + }) + + staticConf := s.adaptFile("fixtures/x_forwarded_for.toml", struct { + DynamicConfPath string + }{ + DynamicConfPath: dynamicConf, + }) + + s.traefikCmd(withConfigFile(staticConf)) + + // Wait for Traefik to start + err := try.GetRequest("http://127.0.0.1:8080/api/rawdata", 10*time.Second, try.BodyContains("service1")) + require.NoError(s.T(), err) + + // Test with appendXForwardedFor = false + req, err := http.NewRequest(http.MethodGet, "http://127.0.0.1:8000/", nil) + require.NoError(s.T(), err) + + // Set an existing X-Forwarded-For header + req.Header.Set("X-Forwarded-For", "1.2.3.4") + + resp, err := http.DefaultClient.Do(req) + require.NoError(s.T(), err) + defer resp.Body.Close() + + body, err := io.ReadAll(resp.Body) + require.NoError(s.T(), err) + + // The backend should receive the original X-Forwarded-For header unchanged + // (Traefik should NOT append RemoteAddr when appendXForwardedFor = false) + assert.Equal(s.T(), "1.2.3.4", string(body)) +} + +func (s *SimpleSuite) TestXForwardedForEnabled() { + srv1 := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) { + // Echo back the X-Forwarded-For header + xff := req.Header.Get("X-Forwarded-For") + _, _ = rw.Write([]byte(xff)) + })) + defer srv1.Close() + + dynamicConf := s.adaptFile("resources/compose/x_forwarded_for.toml", struct { + Server string + }{ + Server: srv1.URL, + }) + + // Use a config with appendXForwardedFor = true + staticConf := s.adaptFile("fixtures/x_forwarded_for_enabled.toml", struct { + DynamicConfPath string + }{ + DynamicConfPath: dynamicConf, + }) + + s.traefikCmd(withConfigFile(staticConf)) + + // Wait for Traefik to start + err := try.GetRequest("http://127.0.0.1:8080/api/rawdata", 10*time.Second, try.BodyContains("service1")) + require.NoError(s.T(), err) + + // Test with default appendXForwardedFor = true + req, err := http.NewRequest(http.MethodGet, "http://127.0.0.1:8000/", nil) + require.NoError(s.T(), err) + + // Set an existing X-Forwarded-For header + req.Header.Set("X-Forwarded-For", "1.2.3.4") + + resp, err := http.DefaultClient.Do(req) + require.NoError(s.T(), err) + defer resp.Body.Close() + + body, err := io.ReadAll(resp.Body) + require.NoError(s.T(), err) + + // The backend should receive the X-Forwarded-For header with RemoteAddr appended + // (should be "1.2.3.4, 127.0.0.1" since the request comes from localhost) + assert.Contains(s.T(), string(body), "1.2.3.4,") + assert.Contains(s.T(), string(body), "127.0.0.1") +} + +func (s *SimpleSuite) TestXForwardedForDisabledFastProxy() { + srv1 := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) { + // Verify FastProxy is being used + assert.Contains(s.T(), req.Header, "X-Traefik-Fast-Proxy") + + // Echo back the X-Forwarded-For header + xff := req.Header.Get("X-Forwarded-For") + _, _ = rw.Write([]byte(xff)) + })) + defer srv1.Close() + + dynamicConf := s.adaptFile("resources/compose/x_forwarded_for.toml", struct { + Server string + }{ + Server: srv1.URL, + }) + + staticConf := s.adaptFile("fixtures/x_forwarded_for_fastproxy.toml", struct { + DynamicConfPath string + }{ + DynamicConfPath: dynamicConf, + }) + + s.traefikCmd(withConfigFile(staticConf)) + + // Wait for Traefik to start + err := try.GetRequest("http://127.0.0.1:8080/api/rawdata", 10*time.Second, try.BodyContains("service1")) + require.NoError(s.T(), err) + + // Test with appendXForwardedFor = false + req, err := http.NewRequest(http.MethodGet, "http://127.0.0.1:8000/", nil) + require.NoError(s.T(), err) + + // Set an existing X-Forwarded-For header + req.Header.Set("X-Forwarded-For", "1.2.3.4") + + resp, err := http.DefaultClient.Do(req) + require.NoError(s.T(), err) + defer resp.Body.Close() + + body, err := io.ReadAll(resp.Body) + require.NoError(s.T(), err) + + // The backend should receive the original X-Forwarded-For header unchanged + // (FastProxy should NOT append RemoteAddr when notAppendXForwardedFor = true) + assert.Equal(s.T(), "1.2.3.4", string(body)) +} + +func (s *SimpleSuite) TestXForwardedForEnabledFastProxy() { + srv1 := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) { + // Verify FastProxy is being used + assert.Contains(s.T(), req.Header, "X-Traefik-Fast-Proxy") + + // Echo back the X-Forwarded-For header + xff := req.Header.Get("X-Forwarded-For") + _, _ = rw.Write([]byte(xff)) + })) + defer srv1.Close() + + dynamicConf := s.adaptFile("resources/compose/x_forwarded_for.toml", struct { + Server string + }{ + Server: srv1.URL, + }) + + // Use a config with appendXForwardedFor = false (default) + staticConf := s.adaptFile("fixtures/x_forwarded_for_fastproxy_enabled.toml", struct { + DynamicConfPath string + }{ + DynamicConfPath: dynamicConf, + }) + + s.traefikCmd(withConfigFile(staticConf)) + + // Wait for Traefik to start + err := try.GetRequest("http://127.0.0.1:8080/api/rawdata", 10*time.Second, try.BodyContains("service1")) + require.NoError(s.T(), err) + + // Test with default appendXForwardedFor = true + req, err := http.NewRequest(http.MethodGet, "http://127.0.0.1:8000/", nil) + require.NoError(s.T(), err) + + // Set an existing X-Forwarded-For header + req.Header.Set("X-Forwarded-For", "1.2.3.4") + + resp, err := http.DefaultClient.Do(req) + require.NoError(s.T(), err) + defer resp.Body.Close() + + body, err := io.ReadAll(resp.Body) + require.NoError(s.T(), err) + + // The backend should receive the X-Forwarded-For header with RemoteAddr appended + // (FastProxy should append RemoteAddr when notAppendXForwardedFor = false) + // (should be "1.2.3.4, 127.0.0.1" since the request comes from localhost) + assert.Contains(s.T(), string(body), "1.2.3.4,") + assert.Contains(s.T(), string(body), "127.0.0.1") +} + func (s *SimpleSuite) TestWithWebConfig() { s.cmdTraefik(withConfigFile("fixtures/simple_web.toml")) diff --git a/pkg/config/static/entrypoints.go b/pkg/config/static/entrypoints.go index 6a09d29d2..0a9dbb5db 100644 --- a/pkg/config/static/entrypoints.go +++ b/pkg/config/static/entrypoints.go @@ -128,9 +128,10 @@ type TLSConfig struct { // ForwardedHeaders Trust client forwarding headers. type ForwardedHeaders struct { - Insecure bool `description:"Trust all forwarded headers." json:"insecure,omitempty" toml:"insecure,omitempty" yaml:"insecure,omitempty" export:"true"` - TrustedIPs []string `description:"Trust only forwarded headers from selected IPs." json:"trustedIPs,omitempty" toml:"trustedIPs,omitempty" yaml:"trustedIPs,omitempty"` - Connection []string `description:"List of Connection headers that are allowed to pass through the middleware chain before being removed." json:"connection,omitempty" toml:"connection,omitempty" yaml:"connection,omitempty"` + Insecure bool `description:"Trust all forwarded headers." json:"insecure,omitempty" toml:"insecure,omitempty" yaml:"insecure,omitempty" export:"true"` + TrustedIPs []string `description:"Trust only forwarded headers from selected IPs." json:"trustedIPs,omitempty" toml:"trustedIPs,omitempty" yaml:"trustedIPs,omitempty"` + Connection []string `description:"List of Connection headers that are allowed to pass through the middleware chain before being removed." json:"connection,omitempty" toml:"connection,omitempty" yaml:"connection,omitempty"` + NotAppendXForwardedFor bool `description:"Disable appending RemoteAddr to X-Forwarded-For header. Defaults to false (appending is enabled)." json:"notAppendXForwardedFor,omitempty" toml:"notAppendXForwardedFor,omitempty" yaml:"notAppendXForwardedFor,omitempty" label:"allowEmpty" file:"allowEmpty" export:"true"` } // ProxyProtocol contains Proxy-Protocol configuration. diff --git a/pkg/config/static/static_config.go b/pkg/config/static/static_config.go index 8b1aac36d..6491c354a 100644 --- a/pkg/config/static/static_config.go +++ b/pkg/config/static/static_config.go @@ -112,8 +112,9 @@ type CertificateResolver struct { // Global holds the global configuration. type Global struct { - CheckNewVersion bool `description:"Periodically check if a new version has been released." json:"checkNewVersion,omitempty" toml:"checkNewVersion,omitempty" yaml:"checkNewVersion,omitempty" label:"allowEmpty" file:"allowEmpty" export:"true"` - SendAnonymousUsage bool `description:"Periodically send anonymous usage statistics. If the option is not specified, it will be disabled by default." json:"sendAnonymousUsage,omitempty" toml:"sendAnonymousUsage,omitempty" yaml:"sendAnonymousUsage,omitempty" label:"allowEmpty" file:"allowEmpty" export:"true"` + CheckNewVersion bool `description:"Periodically check if a new version has been released." json:"checkNewVersion,omitempty" toml:"checkNewVersion,omitempty" yaml:"checkNewVersion,omitempty" label:"allowEmpty" file:"allowEmpty" export:"true"` + SendAnonymousUsage bool `description:"Periodically send anonymous usage statistics. If the option is not specified, it will be disabled by default." json:"sendAnonymousUsage,omitempty" toml:"sendAnonymousUsage,omitempty" yaml:"sendAnonymousUsage,omitempty" label:"allowEmpty" file:"allowEmpty" export:"true"` + NotAppendXForwardedFor bool `description:"Disable appending RemoteAddr to X-Forwarded-For header. Defaults to false (appending is enabled)." json:"notAppendXForwardedFor,omitempty" toml:"notAppendXForwardedFor,omitempty" yaml:"notAppendXForwardedFor,omitempty" label:"allowEmpty" file:"allowEmpty" export:"true"` } // ServersTransport options to configure communication between Traefik and the servers. diff --git a/pkg/middlewares/forwardedheaders/forwarded_header.go b/pkg/middlewares/forwardedheaders/forwarded_header.go index d37d12d44..5c27dff63 100644 --- a/pkg/middlewares/forwardedheaders/forwarded_header.go +++ b/pkg/middlewares/forwardedheaders/forwarded_header.go @@ -9,6 +9,7 @@ import ( "strings" "github.com/traefik/traefik/v3/pkg/ip" + "github.com/traefik/traefik/v3/pkg/proxy/httputil" "golang.org/x/net/http/httpguts" ) @@ -47,16 +48,17 @@ var xHeaders = []string{ // Unless insecure is set, // it first removes all the existing values for those headers if the remote address is not one of the trusted ones. type XForwarded struct { - insecure bool - trustedIPs []string - connectionHeaders []string - ipChecker *ip.Checker - next http.Handler - hostname string + insecure bool + trustedIPs []string + connectionHeaders []string + notAppendXForwardedFor bool + ipChecker *ip.Checker + next http.Handler + hostname string } // NewXForwarded creates a new XForwarded. -func NewXForwarded(insecure bool, trustedIPs []string, connectionHeaders []string, next http.Handler) (*XForwarded, error) { +func NewXForwarded(insecure bool, trustedIPs []string, connectionHeaders []string, notAppendXForwardedFor bool, next http.Handler) (*XForwarded, error) { var ipChecker *ip.Checker if len(trustedIPs) > 0 { var err error @@ -72,12 +74,13 @@ func NewXForwarded(insecure bool, trustedIPs []string, connectionHeaders []strin } return &XForwarded{ - insecure: insecure, - trustedIPs: trustedIPs, - connectionHeaders: connectionHeaders, - ipChecker: ipChecker, - next: next, - hostname: hostname, + insecure: insecure, + trustedIPs: trustedIPs, + connectionHeaders: connectionHeaders, + notAppendXForwardedFor: notAppendXForwardedFor, + ipChecker: ipChecker, + next: next, + hostname: hostname, }, nil } @@ -198,6 +201,10 @@ func (x *XForwarded) ServeHTTP(w http.ResponseWriter, r *http.Request) { x.removeConnectionHeaders(r) + if x.notAppendXForwardedFor { + r = r.WithContext(httputil.SetNotAppendXFF(r.Context())) + } + x.next.ServeHTTP(w, r) } diff --git a/pkg/middlewares/forwardedheaders/forwarded_header_test.go b/pkg/middlewares/forwardedheaders/forwarded_header_test.go index 8289e8c69..322bd59aa 100644 --- a/pkg/middlewares/forwardedheaders/forwarded_header_test.go +++ b/pkg/middlewares/forwardedheaders/forwarded_header_test.go @@ -516,7 +516,7 @@ func TestServeHTTP(t *testing.T) { } } - m, err := NewXForwarded(test.insecure, test.trustedIps, test.connectionHeaders, + m, err := NewXForwarded(test.insecure, test.trustedIps, test.connectionHeaders, false, http.HandlerFunc(func(_ http.ResponseWriter, _ *http.Request) {})) require.NoError(t, err) @@ -655,7 +655,7 @@ func TestConnection(t *testing.T) { t.Run(test.desc, func(t *testing.T) { t.Parallel() - forwarded, err := NewXForwarded(true, nil, test.connectionHeaders, nil) + forwarded, err := NewXForwarded(true, nil, test.connectionHeaders, false, nil) require.NoError(t, err) req := httptest.NewRequest(http.MethodGet, "https://localhost", nil) diff --git a/pkg/proxy/fast/proxy.go b/pkg/proxy/fast/proxy.go index 76b44a1db..bba15000a 100644 --- a/pkg/proxy/fast/proxy.go +++ b/pkg/proxy/fast/proxy.go @@ -212,18 +212,20 @@ func (p *ReverseProxy) ServeHTTP(rw http.ResponseWriter, req *http.Request) { outReq.Header.SetMethod(req.Method) - if clientIP, _, err := net.SplitHostPort(req.RemoteAddr); err == nil { - // If we aren't the first proxy retain prior - // X-Forwarded-For information as a comma+space - // separated list and fold multiple headers into one. - prior, ok := req.Header["X-Forwarded-For"] - if len(prior) > 0 { - clientIP = strings.Join(prior, ", ") + ", " + clientIP - } + if !proxyhttputil.ShouldNotAppendXFF(req.Context()) { + if clientIP, _, err := net.SplitHostPort(req.RemoteAddr); err == nil { + // If we aren't the first proxy retain prior + // X-Forwarded-For information as a comma+space + // separated list and fold multiple headers into one. + prior, ok := req.Header["X-Forwarded-For"] + if len(prior) > 0 { + clientIP = strings.Join(prior, ", ") + ", " + clientIP + } - omit := ok && prior == nil // Go Issue 38079: nil now means don't populate the header - if !omit { - outReq.Header.Set("X-Forwarded-For", clientIP) + omit := ok && prior == nil // Go Issue 38079: nil now means don't populate the header + if !omit { + outReq.Header.Set("X-Forwarded-For", clientIP) + } } } diff --git a/pkg/proxy/fast/proxy_test.go b/pkg/proxy/fast/proxy_test.go index 70b1256aa..261bff308 100644 --- a/pkg/proxy/fast/proxy_test.go +++ b/pkg/proxy/fast/proxy_test.go @@ -19,6 +19,7 @@ import ( "github.com/stretchr/testify/require" "github.com/traefik/traefik/v3/pkg/config/dynamic" "github.com/traefik/traefik/v3/pkg/config/static" + proxyhttputil "github.com/traefik/traefik/v3/pkg/proxy/httputil" "github.com/traefik/traefik/v3/pkg/testhelpers" ) @@ -406,6 +407,90 @@ func TestTransferEncodingChunked(t *testing.T) { assert.Equal(t, "chunk 0\nchunk 1\nchunk 2\n", string(body)) } +func TestXForwardedFor(t *testing.T) { + testCases := []struct { + desc string + notAppendXFF bool + incomingXFF string + expectedXFF string + expectedXFFNotPresent bool + }{ + { + desc: "appends RemoteAddr when notAppendXFF is false", + notAppendXFF: false, + incomingXFF: "", + expectedXFF: "192.0.2.1", + }, + { + desc: "appends RemoteAddr to existing XFF when notAppendXFF is false", + notAppendXFF: false, + incomingXFF: "203.0.113.1", + expectedXFF: "203.0.113.1, 192.0.2.1", + }, + { + desc: "does not append RemoteAddr when notAppendXFF is true and no incoming XFF", + notAppendXFF: true, + incomingXFF: "", + expectedXFFNotPresent: true, + }, + { + desc: "preserves existing XFF when notAppendXFF is true", + notAppendXFF: true, + incomingXFF: "203.0.113.1", + expectedXFF: "203.0.113.1", + }, + { + desc: "preserves multiple XFF values when notAppendXFF is true", + notAppendXFF: true, + incomingXFF: "203.0.113.1, 198.51.100.1", + expectedXFF: "203.0.113.1, 198.51.100.1", + }, + } + + for _, test := range testCases { + t.Run(test.desc, func(t *testing.T) { + var receivedXFF string + var xffPresent bool + + server := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) { + receivedXFF = req.Header.Get("X-Forwarded-For") + xffPresent = req.Header.Get("X-Forwarded-For") != "" || len(req.Header["X-Forwarded-For"]) > 0 + rw.WriteHeader(http.StatusOK) + })) + t.Cleanup(server.Close) + + builder := NewProxyBuilder(&transportManagerMock{}, static.FastProxyConfig{}) + + proxyHandler, err := builder.Build("", testhelpers.MustParseURL(server.URL), true, false) + require.NoError(t, err) + + ctx := t.Context() + if test.notAppendXFF { + ctx = proxyhttputil.SetNotAppendXFF(ctx) + } + + req := httptest.NewRequest(http.MethodGet, "/", http.NoBody) + req = req.WithContext(ctx) + req.RemoteAddr = "192.0.2.1:12345" + + if test.incomingXFF != "" { + req.Header.Set("X-Forwarded-For", test.incomingXFF) + } + + res := httptest.NewRecorder() + proxyHandler.ServeHTTP(res, req) + + assert.Equal(t, http.StatusOK, res.Code) + + if test.expectedXFFNotPresent { + assert.False(t, xffPresent, "X-Forwarded-For header should not be present") + } else { + assert.Equal(t, test.expectedXFF, receivedXFF) + } + }) + } +} + type transportManagerMock struct { tlsConfig *tls.Config } diff --git a/pkg/proxy/httputil/proxy.go b/pkg/proxy/httputil/proxy.go index a6c93cade..6ff22a31c 100644 --- a/pkg/proxy/httputil/proxy.go +++ b/pkg/proxy/httputil/proxy.go @@ -19,17 +19,41 @@ import ( "golang.org/x/net/http/httpguts" ) +type key string + const ( // StatusClientClosedRequest non-standard HTTP status code for client disconnection. StatusClientClosedRequest = 499 // StatusClientClosedRequestText non-standard HTTP status for client disconnection. StatusClientClosedRequestText = "Client Closed Request" + + notAppendXFFKey key = "NotAppendXFF" ) +// SetNotAppendXFF indicates xff should not be appended. +func SetNotAppendXFF(ctx context.Context) context.Context { + return context.WithValue(ctx, notAppendXFFKey, true) +} + +// ShouldNotAppendXFF returns whether X-Forwarded-For should not be appended. +func ShouldNotAppendXFF(ctx context.Context) bool { + val := ctx.Value(notAppendXFFKey) + if val == nil { + return false + } + + notAppendXFF, ok := val.(bool) + if !ok { + return false + } + + return notAppendXFF +} + func buildSingleHostProxy(target *url.URL, passHostHeader bool, preservePath bool, flushInterval time.Duration, roundTripper http.RoundTripper, bufferPool httputil.BufferPool) http.Handler { return &httputil.ReverseProxy{ - Director: directorBuilder(target, passHostHeader, preservePath), + Rewrite: rewriteRequestBuilder(target, passHostHeader, preservePath), Transport: roundTripper, FlushInterval: flushInterval, BufferPool: bufferPool, @@ -38,45 +62,82 @@ func buildSingleHostProxy(target *url.URL, passHostHeader bool, preservePath boo } } -func directorBuilder(target *url.URL, passHostHeader bool, preservePath bool) func(req *http.Request) { - return func(outReq *http.Request) { - outReq.URL.Scheme = target.Scheme - outReq.URL.Host = target.Host +func rewriteRequestBuilder(target *url.URL, passHostHeader bool, preservePath bool) func(*httputil.ProxyRequest) { + return func(pr *httputil.ProxyRequest) { + copyForwardedHeader(pr.Out.Header, pr.In.Header) + if !ShouldNotAppendXFF(pr.In.Context()) { + if clientIP, _, err := net.SplitHostPort(pr.In.RemoteAddr); err == nil { + // If we aren't the first proxy retain prior + // X-Forwarded-For information as a comma+space + // separated list and fold multiple headers into one. + prior, ok := pr.Out.Header["X-Forwarded-For"] + omit := ok && prior == nil // Issue 38079: nil now means don't populate the header + if len(prior) > 0 { + clientIP = strings.Join(prior, ", ") + ", " + clientIP + } + if !omit { + pr.Out.Header.Set("X-Forwarded-For", clientIP) + } + } + } - u := outReq.URL - if outReq.RequestURI != "" { - parsedURL, err := url.ParseRequestURI(outReq.RequestURI) + pr.Out.URL.Scheme = target.Scheme + pr.Out.URL.Host = target.Host + + u := pr.Out.URL + if pr.Out.RequestURI != "" { + parsedURL, err := url.ParseRequestURI(pr.Out.RequestURI) if err == nil { u = parsedURL } } - outReq.URL.Path = u.Path - outReq.URL.RawPath = u.RawPath + pr.Out.URL.Path = u.Path + pr.Out.URL.RawPath = u.RawPath if preservePath { - outReq.URL.Path, outReq.URL.RawPath = JoinURLPath(target, u) + pr.Out.URL.Path, pr.Out.URL.RawPath = JoinURLPath(target, u) } // If a plugin/middleware adds semicolons in query params, they should be urlEncoded. - outReq.URL.RawQuery = strings.ReplaceAll(u.RawQuery, ";", "&") - outReq.RequestURI = "" // Outgoing request should not have RequestURI + pr.Out.URL.RawQuery = strings.ReplaceAll(u.RawQuery, ";", "&") + pr.Out.RequestURI = "" // Outgoing request should not have RequestURI - outReq.Proto = "HTTP/1.1" - outReq.ProtoMajor = 1 - outReq.ProtoMinor = 1 + pr.Out.Proto = "HTTP/1.1" + pr.Out.ProtoMajor = 1 + pr.Out.ProtoMinor = 1 // Do not pass client Host header unless option PassHostHeader is set. if !passHostHeader { - outReq.Host = outReq.URL.Host + pr.Out.Host = pr.Out.URL.Host } - if isWebSocketUpgrade(outReq) { - cleanWebSocketHeaders(outReq) + if isWebSocketUpgrade(pr.Out) { + cleanWebSocketHeaders(pr.Out) } } } +// copyForwardedHeader copies header that are removed by the reverseProxy when a rewriteRequest is used. +func copyForwardedHeader(dst, src http.Header) { + prior, ok := src["X-Forwarded-For"] + if ok { + dst["X-Forwarded-For"] = prior + } + prior, ok = src["Forwarded"] + if ok { + dst["Forwarded"] = prior + } + prior, ok = src["X-Forwarded-Host"] + if ok { + dst["X-Forwarded-Host"] = prior + } + prior, ok = src["X-Forwarded-Proto"] + if ok { + dst["X-Forwarded-Proto"] = prior + } +} + // cleanWebSocketHeaders Even if the websocket RFC says that headers should be case-insensitive, // some servers need Sec-WebSocket-Key, Sec-WebSocket-Extensions, Sec-WebSocket-Accept, // Sec-WebSocket-Protocol and Sec-WebSocket-Version to be case-sensitive. diff --git a/pkg/proxy/httputil/proxy_test.go b/pkg/proxy/httputil/proxy_test.go index 93a27f4e0..23bb36a82 100644 --- a/pkg/proxy/httputil/proxy_test.go +++ b/pkg/proxy/httputil/proxy_test.go @@ -5,6 +5,7 @@ import ( "errors" "net/http" "net/http/httptest" + "net/http/httputil" "net/url" "testing" @@ -13,7 +14,7 @@ import ( "github.com/traefik/traefik/v3/pkg/testhelpers" ) -func Test_directorBuilder(t *testing.T) { +func Test_rewriteRequestBuilder(t *testing.T) { tests := []struct { name string target *url.URL @@ -25,6 +26,7 @@ func Test_directorBuilder(t *testing.T) { expectedPath string expectedRawPath string expectedQuery string + notAppendXFF bool }{ { name: "Basic proxy", @@ -37,6 +39,18 @@ func Test_directorBuilder(t *testing.T) { expectedPath: "/test", expectedQuery: "param=value", }, + { + name: "Basic proxy - notAppendXFF", + target: testhelpers.MustParseURL("http://example.com"), + passHostHeader: false, + preservePath: false, + incomingURL: "http://localhost/test?param=value", + expectedScheme: "http", + expectedHost: "example.com", + expectedPath: "/test", + expectedQuery: "param=value", + notAppendXFF: true, + }, { name: "HTTPS target", target: testhelpers.MustParseURL("https://secure.example.com"), @@ -85,21 +99,41 @@ func Test_directorBuilder(t *testing.T) { t.Run(test.name, func(t *testing.T) { t.Parallel() - director := directorBuilder(test.target, test.passHostHeader, test.preservePath) + rewriteRequest := rewriteRequestBuilder(test.target, test.passHostHeader, test.preservePath) - req := httptest.NewRequest(http.MethodGet, test.incomingURL, http.NoBody) - director(req) + ctx := t.Context() + if test.notAppendXFF { + ctx = SetNotAppendXFF(ctx) + } - assert.Equal(t, test.expectedScheme, req.URL.Scheme) - assert.Equal(t, test.expectedHost, req.Host) - assert.Equal(t, test.expectedPath, req.URL.Path) - assert.Equal(t, test.expectedRawPath, req.URL.RawPath) - assert.Equal(t, test.expectedQuery, req.URL.RawQuery) - assert.Empty(t, req.RequestURI) - assert.Equal(t, "HTTP/1.1", req.Proto) - assert.Equal(t, 1, req.ProtoMajor) - assert.Equal(t, 1, req.ProtoMinor) - assert.False(t, !test.passHostHeader && req.Host != req.URL.Host) + reqIn := httptest.NewRequest(http.MethodGet, test.incomingURL, http.NoBody) + reqIn = reqIn.WithContext(ctx) + reqIn.Header.Add("X-Forwarded-For", "1.2.3.4") + reqIn.RemoteAddr = "127.0.0.1:1234" + + reqOut := httptest.NewRequest(http.MethodGet, test.incomingURL, http.NoBody) + pr := &httputil.ProxyRequest{ + In: reqIn, + Out: reqOut, + } + rewriteRequest(pr) + + if test.notAppendXFF { + assert.Equal(t, "1.2.3.4", reqOut.Header.Get("X-Forwarded-For")) + } else { + // When not disabled, X-Forwarded-For should have RemoteAddr appended + assert.Equal(t, "1.2.3.4, 127.0.0.1", reqOut.Header.Get("X-Forwarded-For")) + } + assert.Equal(t, test.expectedScheme, reqOut.URL.Scheme) + assert.Equal(t, test.expectedHost, reqOut.Host) + assert.Equal(t, test.expectedPath, reqOut.URL.Path) + assert.Equal(t, test.expectedRawPath, reqOut.URL.RawPath) + assert.Equal(t, test.expectedQuery, reqOut.URL.RawQuery) + assert.Empty(t, reqOut.RequestURI) + assert.Equal(t, "HTTP/1.1", reqOut.Proto) + assert.Equal(t, 1, reqOut.ProtoMajor) + assert.Equal(t, 1, reqOut.ProtoMinor) + assert.False(t, !test.passHostHeader && reqOut.Host != reqOut.URL.Host) }) } } diff --git a/pkg/server/server_entrypoint_tcp.go b/pkg/server/server_entrypoint_tcp.go index 55a1f7a1a..3f60f9f79 100644 --- a/pkg/server/server_entrypoint_tcp.go +++ b/pkg/server/server_entrypoint_tcp.go @@ -650,6 +650,7 @@ func newHTTPServer(ctx context.Context, ln net.Listener, configuration *static.E configuration.ForwardedHeaders.Insecure, configuration.ForwardedHeaders.TrustedIPs, configuration.ForwardedHeaders.Connection, + configuration.ForwardedHeaders.NotAppendXForwardedFor, next) if err != nil { return nil, err