From 8c4fc89579d0410e2b312429a074ace50be3acca Mon Sep 17 00:00:00 2001 From: Romain Date: Mon, 13 Apr 2026 10:24:08 +0200 Subject: [PATCH 1/6] Remove map lookup making the basic auth notFoundSecret empty --- pkg/middlewares/auth/basic_auth.go | 2 +- pkg/middlewares/auth/basic_auth_test.go | 11 +++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/pkg/middlewares/auth/basic_auth.go b/pkg/middlewares/auth/basic_auth.go index 2dce9775c0..198c8ca84e 100644 --- a/pkg/middlewares/auth/basic_auth.go +++ b/pkg/middlewares/auth/basic_auth.go @@ -44,7 +44,7 @@ func NewBasic(ctx context.Context, next http.Handler, authConfig dynamic.BasicAu // To prevent timing attacks, we need to compute a hash even if the user is not found. // We assume it to be safe only when the users hashes are all from the same algorithm, // so we can pick the first one as a random hash to compute. - notFoundSecret := users[slices.Collect(maps.Values(users))[0]] + notFoundSecret := slices.Collect(maps.Values(users))[0] ba := &basicAuth{ next: next, diff --git a/pkg/middlewares/auth/basic_auth_test.go b/pkg/middlewares/auth/basic_auth_test.go index e70aa38d8a..baabfba195 100644 --- a/pkg/middlewares/auth/basic_auth_test.go +++ b/pkg/middlewares/auth/basic_auth_test.go @@ -14,6 +14,17 @@ import ( "github.com/traefik/traefik/v2/pkg/testhelpers" ) +func TestNewBasicNotFoundSecretIsSet(t *testing.T) { + auth := dynamic.BasicAuth{ + Users: []string{"test:$apr1$H6uskkkW$IgXLP6ewTrSuBkTrqE8wj/"}, + } + middleware, err := NewBasic(t.Context(), nil, auth, "authName") + require.NoError(t, err) + + ba := middleware.(*basicAuth) + assert.Equal(t, "$apr1$H6uskkkW$IgXLP6ewTrSuBkTrqE8wj/", ba.notFoundSecret) +} + func TestBasicAuthFail(t *testing.T) { next := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { fmt.Fprintln(w, "traefik") From 61b5bc4ad17fd694308db8b409a302b85dad963c Mon Sep 17 00:00:00 2001 From: Romain Date: Tue, 14 Apr 2026 16:38:06 +0200 Subject: [PATCH 2/6] Remove untrusted X headers with underscores --- .../forwardedheaders/forwarded_header.go | 57 ++++++++++++++----- .../forwardedheaders/forwarded_header_test.go | 30 ++++++++++ 2 files changed, 72 insertions(+), 15 deletions(-) diff --git a/pkg/middlewares/forwardedheaders/forwarded_header.go b/pkg/middlewares/forwardedheaders/forwarded_header.go index 73bc948f86..ba1ca94fca 100644 --- a/pkg/middlewares/forwardedheaders/forwarded_header.go +++ b/pkg/middlewares/forwardedheaders/forwarded_header.go @@ -28,18 +28,40 @@ const ( upgrade = "Upgrade" ) -var xHeaders = []string{ - xForwardedProto, - xForwardedFor, - xForwardedHost, - xForwardedPort, - xForwardedServer, - xForwardedURI, - xForwardedMethod, - xForwardedPrefix, - xForwardedTLSClientCert, - xForwardedTLSClientCertInfo, - xRealIP, +// xHeadersSet contains the canonical X-headers managed by Traefik. Used by +// isManagedXHeader to detect both the canonical form and underscore variants +// that Go's HTTP server preserves (e.g. X_Forwarded_Proto). +var xHeadersSet = map[string]struct{}{ + xForwardedProto: {}, + xForwardedFor: {}, + xForwardedHost: {}, + xForwardedPort: {}, + xForwardedServer: {}, + xForwardedURI: {}, + xForwardedMethod: {}, + xForwardedPrefix: {}, + xForwardedTLSClientCert: {}, + xForwardedTLSClientCertInfo: {}, + xRealIP: {}, +} + +// isManagedXHeader reports whether key matches one of Traefik's X-headers, +// treating '_' as '-'. Every managed header starts with 'X', so a byte check +// skips most headers without any map work; the underscore branch is only +// reached for the rare attacker-injected variants. +func isManagedXHeader(key string) bool { + if len(key) == 0 || key[0] != 'X' { + return false + } + if _, ok := xHeadersSet[key]; ok { + return true + } + if strings.IndexByte(key, '_') < 0 { + return false + } + canonical := http.CanonicalHeaderKey(strings.ReplaceAll(key, "_", "-")) + _, ok := xHeadersSet[canonical] + return ok } // XForwarded is an HTTP handler wrapper that sets the X-Forwarded headers, @@ -137,8 +159,13 @@ func forwardedPort(req *http.Request) string { // ServeHTTP implements http.Handler. func (x *XForwarded) ServeHTTP(w http.ResponseWriter, r *http.Request) { if !x.insecure && !x.isTrustedIP(r.RemoteAddr) { - for _, h := range xHeaders { - unsafeHeader(r.Header).Del(h) + // Strip X-Forwarded headers and their underscore variants + // (e.g. X_Forwarded_Proto), which Go's HTTP server preserves + // alongside the canonical dash form. + for key := range r.Header { + if isManagedXHeader(key) { + delete(r.Header, key) + } } } @@ -219,7 +246,7 @@ func (x *XForwarded) removeConnectionHeaders(req *http.Request) { // as per rfc7230 https://datatracker.ietf.org/doc/html/rfc7230#section-6.1, // A proxy or gateway MUST ... and then remove the Connection header field itself // (or replace it with the intermediary's own connection options for the forwarded message). - if slices.Contains(xHeaders, key) { + if isManagedXHeader(key) { continue } diff --git a/pkg/middlewares/forwardedheaders/forwarded_header_test.go b/pkg/middlewares/forwardedheaders/forwarded_header_test.go index 505349781c..711e96b001 100644 --- a/pkg/middlewares/forwardedheaders/forwarded_header_test.go +++ b/pkg/middlewares/forwardedheaders/forwarded_header_test.go @@ -50,6 +50,8 @@ func TestServeHTTP(t *testing.T) { xForwardedTLSClientCert: {"Cert"}, xForwardedTLSClientCertInfo: {"CertInfo"}, xForwardedPrefix: {"/prefix"}, + "X_forwarded_proto": {"https"}, + "X_forwarded_for": {"10.0.0.1"}, }, expectedHeaders: map[string]string{ xForwardedFor: "10.0.1.0, 10.0.1.12", @@ -58,6 +60,8 @@ func TestServeHTTP(t *testing.T) { xForwardedTLSClientCert: "Cert", xForwardedTLSClientCertInfo: "CertInfo", xForwardedPrefix: "/prefix", + "X_forwarded_proto": "https", + "X_forwarded_for": "10.0.0.1", }, }, { @@ -72,6 +76,9 @@ func TestServeHTTP(t *testing.T) { xForwardedTLSClientCert: {"Cert"}, xForwardedTLSClientCertInfo: {"CertInfo"}, xForwardedPrefix: {"/prefix"}, + "X_forwarded_proto": {"https"}, + "X_forwarded_for": {"10.0.0.1"}, + "X_forwarded_host": {"evil.example"}, }, expectedHeaders: map[string]string{ xForwardedFor: "", @@ -80,6 +87,9 @@ func TestServeHTTP(t *testing.T) { xForwardedTLSClientCert: "", xForwardedTLSClientCertInfo: "", xForwardedPrefix: "", + "X_forwarded_proto": "", + "X_forwarded_for": "", + "X_forwarded_host": "", }, }, { @@ -94,6 +104,8 @@ func TestServeHTTP(t *testing.T) { xForwardedTLSClientCert: {"Cert"}, xForwardedTLSClientCertInfo: {"CertInfo"}, xForwardedPrefix: {"/prefix"}, + "X_forwarded_proto": {"https"}, + "X_forwarded_for": {"10.0.0.1"}, }, expectedHeaders: map[string]string{ xForwardedFor: "10.0.1.0, 10.0.1.12", @@ -102,6 +114,8 @@ func TestServeHTTP(t *testing.T) { xForwardedTLSClientCert: "Cert", xForwardedTLSClientCertInfo: "CertInfo", xForwardedPrefix: "/prefix", + "X_forwarded_proto": "https", + "X_forwarded_for": "10.0.0.1", }, }, { @@ -298,6 +312,7 @@ func TestServeHTTP(t *testing.T) { xForwardedTLSClientCertInfo, xForwardedPrefix, xRealIP, + "X_forwarded_proto", }, xForwardedProto: {"foo"}, xForwardedFor: {"foo"}, @@ -309,6 +324,7 @@ func TestServeHTTP(t *testing.T) { xForwardedTLSClientCertInfo: {"foo"}, xForwardedPrefix: {"foo"}, xRealIP: {"foo"}, + "X_forwarded_proto": {"spoofed"}, }, expectedHeaders: map[string]string{ xForwardedProto: "http", @@ -321,6 +337,7 @@ func TestServeHTTP(t *testing.T) { xForwardedTLSClientCertInfo: "", xForwardedPrefix: "", xRealIP: "", + "X_forwarded_proto": "", connection: "", }, }, @@ -583,6 +600,19 @@ func TestServeHTTP(t *testing.T) { "Foo": "bar", }, }, + { + desc: "insecure false preserves non-matching underscore headers", + insecure: false, + remoteAddr: "10.0.1.101:80", + incomingHeaders: map[string][]string{ + "X_custom_header": {"value"}, + "X_forwarded_proto": {"spoofed"}, + }, + expectedHeaders: map[string]string{ + "X_custom_header": "value", + "X_forwarded_proto": "", + }, + }, } for _, test := range testCases { From df00d82fc7f12e07199551832b54de6d0e55414d Mon Sep 17 00:00:00 2001 From: Romain Date: Wed, 15 Apr 2026 10:36:06 +0200 Subject: [PATCH 3/6] Honor allowCrossNamespace with chain middleware CRD --- docs/content/migration/v2.md | 10 ++++ .../with_middleware_cross_namespace.yml | 35 ++++++++++++ pkg/provider/kubernetes/crd/kubernetes.go | 33 ++++++++--- .../kubernetes/crd/kubernetes_http.go | 4 +- .../kubernetes/crd/kubernetes_test.go | 56 +++++++++++++++++++ 5 files changed, 129 insertions(+), 9 deletions(-) diff --git a/docs/content/migration/v2.md b/docs/content/migration/v2.md index 6dd45615c4..645323d813 100644 --- a/docs/content/migration/v2.md +++ b/docs/content/migration/v2.md @@ -785,3 +785,13 @@ The default value for this option is -1, which means there is no limit to the re However, it is strongly recommended to set this option to a suitable value to avoid performance issues such as memory exhaustion. Please check out the [HTTP](../providers/http.md#maxresponsebodysize) provider documentation for more details. + +## v2.11.43 + +### Kubernetes CRD: Chain middleware and `allowCrossNamespace` + +In `v2.11.43`, the `Chain` middleware now honors the Kubernetes CRD provider's `allowCrossNamespace` option. +Previously, a `Chain` could reference middlewares in other namespaces regardless of the `allowCrossNamespace` configuration. + +If `allowCrossNamespace` is set to `false` (the default) and a `Chain` middleware references a middleware in a different namespace from its own, +the whole `Chain` is now rejected and an error is logged. diff --git a/pkg/provider/kubernetes/crd/fixtures/with_middleware_cross_namespace.yml b/pkg/provider/kubernetes/crd/fixtures/with_middleware_cross_namespace.yml index 3c1e899562..faf9e953fa 100644 --- a/pkg/provider/kubernetes/crd/fixtures/with_middleware_cross_namespace.yml +++ b/pkg/provider/kubernetes/crd/fixtures/with_middleware_cross_namespace.yml @@ -37,6 +37,16 @@ spec: port: 80 middlewares: - name: cross-ns-stripprefix@kubernetescrd + - match: Host(`foo.com`) && PathPrefix(`/chain`) + kind: Rule + priority: 12 + services: + - name: whoami + namespace: default + port: 80 + middlewares: + - name: test-chain + - name: test-chain-cross-provider --- apiVersion: traefik.io/v1alpha1 @@ -50,6 +60,31 @@ spec: prefixes: - /stripit +--- +apiVersion: traefik.io/v1alpha1 +kind: Middleware +metadata: + name: test-chain + namespace: default + +spec: + chain: + middlewares: + - name: stripprefix + namespace: cross-ns + +--- +apiVersion: traefik.io/v1alpha1 +kind: Middleware +metadata: + name: test-chain-cross-provider + namespace: default + +spec: + chain: + middlewares: + - name: other-middleware@kubernetescrd + --- apiVersion: traefik.io/v1alpha1 kind: Middleware diff --git a/pkg/provider/kubernetes/crd/kubernetes.go b/pkg/provider/kubernetes/crd/kubernetes.go index eca0a876d7..a3f23d4ed0 100644 --- a/pkg/provider/kubernetes/crd/kubernetes.go +++ b/pkg/provider/kubernetes/crd/kubernetes.go @@ -279,13 +279,19 @@ func (p *Provider) loadConfigurationFromCRD(ctx context.Context, client Client) continue } + chain, err := createChainMiddleware(ctxMid, middleware.Namespace, middleware.Spec.Chain, p.AllowCrossNamespace) + if err != nil { + log.FromContext(ctxMid).Errorf("Error while reading chain middleware: %v", err) + continue + } + conf.HTTP.Middlewares[id] = &dynamic.Middleware{ AddPrefix: middleware.Spec.AddPrefix, StripPrefix: middleware.Spec.StripPrefix, StripPrefixRegex: middleware.Spec.StripPrefixRegex, ReplacePath: middleware.Spec.ReplacePath, ReplacePathRegex: middleware.Spec.ReplacePathRegex, - Chain: createChainMiddleware(ctxMid, middleware.Namespace, middleware.Spec.Chain), + Chain: chain, IPWhiteList: middleware.Spec.IPWhiteList, IPAllowList: middleware.Spec.IPAllowList, Headers: middleware.Spec.Headers, @@ -854,13 +860,20 @@ func loadAuthCredentials(secret *corev1.Secret) ([]string, error) { return credentials, nil } -func createChainMiddleware(ctx context.Context, namespace string, chain *traefikv1alpha1.Chain) *dynamic.Chain { +func createChainMiddleware(ctx context.Context, parentNamespace string, chain *traefikv1alpha1.Chain, allowCrossNamespace bool) (*dynamic.Chain, error) { if chain == nil { - return nil + return nil, nil } var mds []string for _, mi := range chain.Middlewares { + if !allowCrossNamespace && strings.HasSuffix(mi.Name, providerNamespaceSeparator+providerName) { + // Since we are not able to know if another namespace is in the name (namespace-name@kubernetescrd), + // if the provider namespace kubernetescrd is used, + // we don't allow this format to avoid cross-namespace references. + return nil, fmt.Errorf("invalid reference to middleware %s: when allowCrossNamespace is disabled @kubernetescrd provider references are disallowed", mi.Name) + } + if strings.Contains(mi.Name, providerNamespaceSeparator) { if len(mi.Namespace) > 0 { log.FromContext(ctx). @@ -870,13 +883,19 @@ func createChainMiddleware(ctx context.Context, namespace string, chain *traefik continue } - ns := mi.Namespace - if len(ns) == 0 { - ns = namespace + ns := parentNamespace + if len(mi.Namespace) > 0 { + if !isNamespaceAllowed(allowCrossNamespace, parentNamespace, mi.Namespace) { + return nil, fmt.Errorf("middleware %s/%s is not in the chain namespace %s", mi.Namespace, mi.Name, parentNamespace) + } + + ns = mi.Namespace } + mds = append(mds, makeID(ns, mi.Name)) } - return &dynamic.Chain{Middlewares: mds} + + return &dynamic.Chain{Middlewares: mds}, nil } func buildTLSOptions(ctx context.Context, client Client) map[string]tls.Options { diff --git a/pkg/provider/kubernetes/crd/kubernetes_http.go b/pkg/provider/kubernetes/crd/kubernetes_http.go index 065115cdc7..ca429dd17e 100644 --- a/pkg/provider/kubernetes/crd/kubernetes_http.go +++ b/pkg/provider/kubernetes/crd/kubernetes_http.go @@ -166,8 +166,8 @@ func (p *Provider) makeMiddlewareKeys(ctx context.Context, ingRouteNamespace str if !p.AllowCrossNamespace && strings.HasSuffix(mi.Name, providerNamespaceSeparator+providerName) { // Since we are not able to know if another namespace is in the name (namespace-name@kubernetescrd), // if the provider namespace kubernetescrd is used, - // we don't allow this format to avoid cross namespace references. - return nil, fmt.Errorf("invalid reference to middleware %s: with crossnamespace disallowed, the namespace field needs to be explicitly specified", mi.Name) + // we don't allow this format to avoid cross-namespace references. + return nil, fmt.Errorf("invalid reference to middleware %s: when allowCrossNamespace is disabled @kubernetescrd provider references are disallowed", mi.Name) } if strings.Contains(name, providerNamespaceSeparator) { diff --git a/pkg/provider/kubernetes/crd/kubernetes_test.go b/pkg/provider/kubernetes/crd/kubernetes_test.go index f20503ee59..526b0e73d0 100644 --- a/pkg/provider/kubernetes/crd/kubernetes_test.go +++ b/pkg/provider/kubernetes/crd/kubernetes_test.go @@ -5074,6 +5074,16 @@ func TestCrossNamespace(t *testing.T) { Priority: 12, Middlewares: []string{"default-test-errorpage"}, }, + "default-test-crossnamespace-route-4932ffbbcd99474df323": { + EntryPoints: []string{"foo"}, + Service: "default-test-crossnamespace-route-4932ffbbcd99474df323", + Rule: "Host(`foo.com`) && PathPrefix(`/chain`)", + Priority: 12, + Middlewares: []string{ + "default-test-chain", + "default-test-chain-cross-provider", + }, + }, }, Middlewares: map[string]*dynamic.Middleware{ "cross-ns-stripprefix": { @@ -5097,6 +5107,19 @@ func TestCrossNamespace(t *testing.T) { PassHostHeader: pointer(true), }, }, + "default-test-crossnamespace-route-4932ffbbcd99474df323": { + LoadBalancer: &dynamic.ServersLoadBalancer{ + Servers: []dynamic.Server{ + { + URL: "http://10.10.0.1:80", + }, + { + URL: "http://10.10.0.2:80", + }, + }, + PassHostHeader: pointer(true), + }, + }, }, ServersTransports: map[string]*dynamic.ServersTransport{}, }, @@ -5142,6 +5165,16 @@ func TestCrossNamespace(t *testing.T) { Priority: 12, Middlewares: []string{"cross-ns-stripprefix@kubernetescrd"}, }, + "default-test-crossnamespace-route-4932ffbbcd99474df323": { + EntryPoints: []string{"foo"}, + Service: "default-test-crossnamespace-route-4932ffbbcd99474df323", + Rule: "Host(`foo.com`) && PathPrefix(`/chain`)", + Priority: 12, + Middlewares: []string{ + "default-test-chain", + "default-test-chain-cross-provider", + }, + }, }, Middlewares: map[string]*dynamic.Middleware{ "cross-ns-stripprefix": { @@ -5157,6 +5190,16 @@ func TestCrossNamespace(t *testing.T) { Query: "/{status}.html", }, }, + "default-test-chain": { + Chain: &dynamic.Chain{ + Middlewares: []string{"cross-ns-stripprefix"}, + }, + }, + "default-test-chain-cross-provider": { + Chain: &dynamic.Chain{ + Middlewares: []string{"other-middleware@kubernetescrd"}, + }, + }, }, Services: map[string]*dynamic.Service{ "default-test-crossnamespace-route-6b204d94623b3df4370c": { @@ -5211,6 +5254,19 @@ func TestCrossNamespace(t *testing.T) { PassHostHeader: pointer(true), }, }, + "default-test-crossnamespace-route-4932ffbbcd99474df323": { + LoadBalancer: &dynamic.ServersLoadBalancer{ + Servers: []dynamic.Server{ + { + URL: "http://10.10.0.1:80", + }, + { + URL: "http://10.10.0.2:80", + }, + }, + PassHostHeader: pointer(true), + }, + }, }, ServersTransports: map[string]*dynamic.ServersTransport{}, }, From 1a435053876544b9c46705e3971e8c398132307a Mon Sep 17 00:00:00 2001 From: Kevin Pollet Date: Thu, 16 Apr 2026 14:26:06 +0200 Subject: [PATCH 4/6] Sanitize the request URL after stripping the prefix --- pkg/middlewares/stripprefix/strip_prefix.go | 27 ++++++++++----- .../stripprefix/strip_prefix_test.go | 34 +++++++++++++++++++ .../stripprefixregex/strip_prefix_regex.go | 10 ++++-- .../strip_prefix_regex_test.go | 22 +++++++++++- 4 files changed, 81 insertions(+), 12 deletions(-) diff --git a/pkg/middlewares/stripprefix/strip_prefix.go b/pkg/middlewares/stripprefix/strip_prefix.go index 53e54c0ece..6abd4391e4 100644 --- a/pkg/middlewares/stripprefix/strip_prefix.go +++ b/pkg/middlewares/stripprefix/strip_prefix.go @@ -29,8 +29,14 @@ type stripPrefix struct { // New creates a new strip prefix middleware. func New(ctx context.Context, next http.Handler, config dynamic.StripPrefix, name string) (http.Handler, error) { log.FromContext(middlewares.GetLoggerCtx(ctx, name, typeName)).Debug("Creating middleware") + + prefixes := make([]string, len(config.Prefixes)) + for i, p := range config.Prefixes { + prefixes[i] = strings.TrimSpace(p) + } + return &stripPrefix{ - prefixes: config.Prefixes, + prefixes: prefixes, forceSlash: config.ForceSlash, next: next, name: name, @@ -48,19 +54,22 @@ func (s *stripPrefix) ServeHTTP(rw http.ResponseWriter, req *http.Request) { if req.URL.RawPath != "" { req.URL.RawPath = s.getRawPathStripped(req.URL.RawPath, prefix) } - s.serveRequest(rw, req, strings.TrimSpace(prefix)) - return + + // Here we are sanitizing the URL when the path is not empty, + // as the JoinPath method is adding a leading slash if the path is empty + // to be aligned with ensureLeadingSlash behavior. + if req.URL.Path != "" { + req.URL = req.URL.JoinPath() + } + + req.Header.Add(ForwardedPrefixHeader, prefix) + req.RequestURI = req.URL.RequestURI() + break } } s.next.ServeHTTP(rw, req) } -func (s *stripPrefix) serveRequest(rw http.ResponseWriter, req *http.Request, prefix string) { - req.Header.Add(ForwardedPrefixHeader, prefix) - req.RequestURI = req.URL.RequestURI() - s.next.ServeHTTP(rw, req) -} - func (s *stripPrefix) getPathStripped(urlPath, prefix string) string { if s.forceSlash { // Only for compatibility reason with the previous behavior, diff --git a/pkg/middlewares/stripprefix/strip_prefix_test.go b/pkg/middlewares/stripprefix/strip_prefix_test.go index 18d66c7145..1fa0330de1 100644 --- a/pkg/middlewares/stripprefix/strip_prefix_test.go +++ b/pkg/middlewares/stripprefix/strip_prefix_test.go @@ -185,6 +185,40 @@ func TestStripPrefix(t *testing.T) { expectedRawPath: "/a%2Fb", expectedHeader: "/api/", }, + { + desc: "dot in the path not stripped by the prefix", + config: dynamic.StripPrefix{ + Prefixes: []string{"/api"}, + }, + path: "/api./foo", + expectedStatusCode: http.StatusOK, + expectedPath: "/foo", + expectedRawPath: "", + expectedHeader: "/api", + }, + { + desc: "multiple dots in the path not stripped by the prefix", + config: dynamic.StripPrefix{ + Prefixes: []string{"/api"}, + }, + path: "/api../foo", + expectedStatusCode: http.StatusOK, + expectedPath: "/foo", + expectedRawPath: "", + expectedHeader: "/api", + }, + { + desc: "multiple dots in the path not stripped by the prefix with forceSlash", + config: dynamic.StripPrefix{ + Prefixes: []string{"/api"}, + ForceSlash: true, + }, + path: "/api../foo", + expectedStatusCode: http.StatusOK, + expectedPath: "/foo", + expectedRawPath: "", + expectedHeader: "/api", + }, } for _, test := range testCases { diff --git a/pkg/middlewares/stripprefixregex/strip_prefix_regex.go b/pkg/middlewares/stripprefixregex/strip_prefix_regex.go index 8a1997941f..b07966fd51 100644 --- a/pkg/middlewares/stripprefixregex/strip_prefix_regex.go +++ b/pkg/middlewares/stripprefixregex/strip_prefix_regex.go @@ -65,9 +65,15 @@ func (s *stripPrefixRegex) ServeHTTP(rw http.ResponseWriter, req *http.Request) req.URL.RawPath = ensureLeadingSlash(req.URL.RawPath[encodedPrefixLen(req.URL.RawPath, prefix):]) } + // Here we are sanitizing the URL when the path is not empty, + // as the JoinPath method is adding a leading slash if the path is empty + // to be aligned with ensureLeadingSlash behavior. + if req.URL.Path != "" { + req.URL = req.URL.JoinPath() + } + req.RequestURI = req.URL.RequestURI() - s.next.ServeHTTP(rw, req) - return + break } } diff --git a/pkg/middlewares/stripprefixregex/strip_prefix_regex_test.go b/pkg/middlewares/stripprefixregex/strip_prefix_regex_test.go index c0caf73056..c9c6b59347 100644 --- a/pkg/middlewares/stripprefixregex/strip_prefix_regex_test.go +++ b/pkg/middlewares/stripprefixregex/strip_prefix_regex_test.go @@ -153,7 +153,7 @@ func TestStripPrefixRegex(t *testing.T) { path: "/b/ap%69/test", expectedStatusCode: http.StatusOK, expectedPath: "/test", - expectedRawPath: "/test", + expectedRawPath: "", expectedRequestURI: "/test", expectedHeader: "/b/api/", }, @@ -197,6 +197,26 @@ func TestStripPrefixRegex(t *testing.T) { expectedRequestURI: "/a%2Fb", expectedHeader: "/t /test", }, + { + desc: "/api./foo", + config: dynamic.StripPrefixRegex{Regex: []string{"/api"}}, + path: "/api./foo", + expectedStatusCode: http.StatusOK, + expectedPath: "/foo", + expectedRawPath: "", + expectedRequestURI: "/foo", + expectedHeader: "/api", + }, + { + desc: "/api../foo", + config: dynamic.StripPrefixRegex{Regex: []string{"/api"}}, + path: "/api../foo", + expectedStatusCode: http.StatusOK, + expectedPath: "/foo", + expectedRawPath: "", + expectedRequestURI: "/foo", + expectedHeader: "/api", + }, } for _, test := range testCases { From 184de70aef5e6a4870ebad566f8dac6c27437688 Mon Sep 17 00:00:00 2001 From: Michel Loiseleur <97035654+mloiseleur@users.noreply.github.com> Date: Fri, 17 Apr 2026 10:52:06 +0200 Subject: [PATCH 5/6] Remove duplicate step in CI --- .github/workflows/validate.yaml | 5 ----- 1 file changed, 5 deletions(-) diff --git a/.github/workflows/validate.yaml b/.github/workflows/validate.yaml index c2b075cbad..ee489b9b9c 100644 --- a/.github/workflows/validate.yaml +++ b/.github/workflows/validate.yaml @@ -75,11 +75,6 @@ jobs: make generate git diff --exit-code - - name: go mod tidy - run: | - go mod tidy - git diff --exit-code - - name: make generate-crd run: | make generate-crd From 5e1de225847184dbb250718cd0e9639bfffa5287 Mon Sep 17 00:00:00 2001 From: Julien Salleyron Date: Fri, 17 Apr 2026 15:42:05 +0200 Subject: [PATCH 6/6] Fix trustForwardHeader on forward auth middleware --- docs/content/middlewares/http/forwardauth.md | 16 + docs/content/migration/v2.md | 9 + pkg/config/dynamic/middlewares.go | 2 +- pkg/config/dynamic/zz_generated.deepcopy.go | 5 + pkg/config/label/label_test.go | 4 +- pkg/middlewares/accesslog/logger.go | 4 +- pkg/middlewares/auth/forward.go | 136 +++--- pkg/middlewares/auth/forward_test.go | 277 ++++++++++-- .../forwardedheaders/forwarded_header.go | 174 +++---- .../forwardedheaders/forwarded_header_test.go | 424 +++++++++--------- .../crd/traefikio/v1alpha1/middleware.go | 2 +- .../v1alpha1/zz_generated.deepcopy.go | 5 + pkg/provider/kv/kv_test.go | 2 +- pkg/redactor/redactor_config_test.go | 2 +- .../loadbalancer/mirror/mirror_test.go | 6 +- 15 files changed, 681 insertions(+), 387 deletions(-) diff --git a/docs/content/middlewares/http/forwardauth.md b/docs/content/middlewares/http/forwardauth.md index b98184cf89..34ce05efcf 100644 --- a/docs/content/middlewares/http/forwardauth.md +++ b/docs/content/middlewares/http/forwardauth.md @@ -130,6 +130,22 @@ http: ### `trustForwardHeader` +!!! warning + + If `trustForwardHeader` is not explicitly set, Traefik will log a warning at startup and use a legacy behavior where some `X-Forwarded-*` headers (e.g. `X-Forwarded-For`, `X-Forwarded-Proto`) are removed but others (e.g. `X-Forwarded-Prefix`) are forwarded untouched. + To silence this warning, explicitly set `trustForwardHeader` to `true` or `false`. + +!!! tip "Recommended configuration" + + The recommended approach is to configure trusted IPs at the [EntryPoint level](../../routing/entrypoints.md#forwarded-headers) using `forwardedHeaders.trustedIPs`, and set `trustForwardHeader: true` on this middleware. + + With this setup, the EntryPoint is responsible for sanitizing incoming `X-Forwarded-*` headers: + it strips any such headers sent by untrusted clients and only preserves those coming from trusted upstream proxies. + By the time the ForwardAuth middleware processes the request, all `X-Forwarded-*` headers are guaranteed to be trustworthy, + including those intentionally added by other middlewares in the chain — for example, the `X-Forwarded-Prefix` header set by the [StripPrefix](stripprefix.md) middleware. + + Setting `trustForwardHeader: true` on this middleware then simply tells ForwardAuth to forward all those (already sanitized) headers to the authentication server. + Set the `trustForwardHeader` option to `true` to trust all `X-Forwarded-*` headers. ```yaml tab="Docker" diff --git a/docs/content/migration/v2.md b/docs/content/migration/v2.md index 645323d813..d5aa7d6930 100644 --- a/docs/content/migration/v2.md +++ b/docs/content/migration/v2.md @@ -795,3 +795,12 @@ Previously, a `Chain` could reference middlewares in other namespaces regardless If `allowCrossNamespace` is set to `false` (the default) and a `Chain` middleware references a middleware in a different namespace from its own, the whole `Chain` is now rejected and an error is logged. + +### ForwardAuth middleware: `trustForwardHeader` + +In `v2.11.43`, when `trustForwardHeader` is not explicitly set, Traefik logs a warning as its behavior is inconsistent: +some `X-Forwarded-*` headers (e.g. `X-Forwarded-For`, `X-Forwarded-Proto`) are removed while others (e.g. `X-Forwarded-Prefix`) are forwarded untouched. + +To silence the warning and avoid security concerns, explicitly set `trustForwardHeader` to `true` or `false` in your ForwardAuth middleware configuration. + +Please check out the [ForwardAuth](../middlewares/http/forwardauth.md#trustforwardheader) middleware documentation for more details. diff --git a/pkg/config/dynamic/middlewares.go b/pkg/config/dynamic/middlewares.go index a1d49f75c2..780e1d8fd2 100644 --- a/pkg/config/dynamic/middlewares.go +++ b/pkg/config/dynamic/middlewares.go @@ -207,7 +207,7 @@ type ForwardAuth struct { // TLS defines the configuration used to secure the connection to the authentication server. TLS *types.ClientTLS `json:"tls,omitempty" toml:"tls,omitempty" yaml:"tls,omitempty" export:"true"` // TrustForwardHeader defines whether to trust (ie: forward) all X-Forwarded-* headers. - TrustForwardHeader bool `json:"trustForwardHeader,omitempty" toml:"trustForwardHeader,omitempty" yaml:"trustForwardHeader,omitempty" export:"true"` + TrustForwardHeader *bool `json:"trustForwardHeader,omitempty" toml:"trustForwardHeader,omitempty" yaml:"trustForwardHeader,omitempty" export:"true"` // AuthResponseHeaders defines the list of headers to copy from the authentication server response and set on forwarded request, replacing any existing conflicting headers. AuthResponseHeaders []string `json:"authResponseHeaders,omitempty" toml:"authResponseHeaders,omitempty" yaml:"authResponseHeaders,omitempty" export:"true"` // AuthResponseHeadersRegex defines the regex to match headers to copy from the authentication server response and set on forwarded request, after stripping all headers that match the regex. diff --git a/pkg/config/dynamic/zz_generated.deepcopy.go b/pkg/config/dynamic/zz_generated.deepcopy.go index c9cf5d446f..e467967e3b 100644 --- a/pkg/config/dynamic/zz_generated.deepcopy.go +++ b/pkg/config/dynamic/zz_generated.deepcopy.go @@ -314,6 +314,11 @@ func (in *ForwardAuth) DeepCopyInto(out *ForwardAuth) { *out = new(types.ClientTLS) **out = **in } + if in.TrustForwardHeader != nil { + in, out := &in.TrustForwardHeader, &out.TrustForwardHeader + *out = new(bool) + **out = **in + } if in.AuthResponseHeaders != nil { in, out := &in.AuthResponseHeaders, &out.AuthResponseHeaders *out = make([]string, len(*in)) diff --git a/pkg/config/label/label_test.go b/pkg/config/label/label_test.go index e620ffb7ab..61b96d91fb 100644 --- a/pkg/config/label/label_test.go +++ b/pkg/config/label/label_test.go @@ -539,7 +539,7 @@ func TestDecodeConfiguration(t *testing.T) { Key: "foobar", InsecureSkipVerify: true, }, - TrustForwardHeader: true, + TrustForwardHeader: pointer(true), AuthResponseHeaders: []string{ "foobar", "fiibar", @@ -1053,7 +1053,7 @@ func TestEncodeConfiguration(t *testing.T) { Key: "foobar", InsecureSkipVerify: true, }, - TrustForwardHeader: true, + TrustForwardHeader: pointer(true), AuthResponseHeaders: []string{ "foobar", "fiibar", diff --git a/pkg/middlewares/accesslog/logger.go b/pkg/middlewares/accesslog/logger.go index 3d030b2e1c..5bf54e2ce5 100644 --- a/pkg/middlewares/accesslog/logger.go +++ b/pkg/middlewares/accesslog/logger.go @@ -397,8 +397,8 @@ func (h *Handler) keepAccessLog(statusCode, retryAttempts int, duration time.Dur return false } -var requestCounter uint64 // Request ID +var requestCounter atomic.Uint64 // Request ID func nextRequestCount() uint64 { - return atomic.AddUint64(&requestCounter, 1) + return requestCounter.Add(1) } diff --git a/pkg/middlewares/auth/forward.go b/pkg/middlewares/auth/forward.go index cf0801af45..8f58b520be 100644 --- a/pkg/middlewares/auth/forward.go +++ b/pkg/middlewares/auth/forward.go @@ -5,9 +5,11 @@ import ( "errors" "fmt" "io" + "maps" "net" "net/http" "regexp" + "slices" "strings" "time" @@ -15,16 +17,15 @@ import ( "github.com/traefik/traefik/v2/pkg/config/dynamic" "github.com/traefik/traefik/v2/pkg/log" "github.com/traefik/traefik/v2/pkg/middlewares" + "github.com/traefik/traefik/v2/pkg/middlewares/forwardedheaders" "github.com/traefik/traefik/v2/pkg/tracing" "github.com/vulcand/oxy/v2/forward" "github.com/vulcand/oxy/v2/utils" ) -const ( - xForwardedURI = "X-Forwarded-Uri" - xForwardedMethod = "X-Forwarded-Method" - forwardedTypeName = "ForwardedAuthType" -) +const forwardedTypeName = "ForwardedAuthType" + +var errResponseBodyTooLarge = errors.New("response body too large") // hopHeaders Hop-by-hop headers to be removed in the authentication request. // http://www.w3.org/Protocols/rfc2616/rfc2616-sec13.html @@ -45,7 +46,7 @@ type forwardAuth struct { next http.Handler name string client http.Client - trustForwardHeader bool + trustForwardHeader *bool authRequestHeaders []string maxResponseBodySize int64 } @@ -98,6 +99,12 @@ func NewForward(ctx context.Context, next http.Handler, config dynamic.ForwardAu fa.authResponseHeadersRegex = re } + if config.TrustForwardHeader == nil { + logger.Warn("TrustForwardHeader is not set: this creates an inconsistent security behavior where some X-Forwarded headers (e.g. X-Forwarded-For, X-Forwarded-Proto) are removed but others (e.g. X-Forwarded-Prefix) are forwarded untouched. Set it to false to remove all X-Forwarded headers, or true to trust them all.") + } else if *config.TrustForwardHeader && len(fa.authRequestHeaders) > 0 { + fa.authRequestHeaders = append(fa.authRequestHeaders, slices.Collect(maps.Keys(forwardedheaders.XHeadersSet))...) + } + return fa, nil } @@ -122,7 +129,11 @@ func (fa *forwardAuth) ServeHTTP(rw http.ResponseWriter, req *http.Request) { // forwardReq. tracing.InjectRequestHeaders(req) - writeHeader(req, forwardReq, fa.trustForwardHeader, fa.authRequestHeaders) + if fa.trustForwardHeader != nil { + writeHeader(req, forwardReq, *fa.trustForwardHeader, fa.authRequestHeaders) + } else { + oldWriteHeader(req, forwardReq, fa.authRequestHeaders) + } forwardResponse, forwardErr := fa.client.Do(forwardReq) if forwardErr != nil { @@ -209,8 +220,6 @@ func (fa *forwardAuth) ServeHTTP(rw http.ResponseWriter, req *http.Request) { fa.next.ServeHTTP(rw, req) } -var errResponseBodyTooLarge = errors.New("response body too large") - func (fa *forwardAuth) readResponseBodyBytes(res *http.Response) ([]byte, error) { if fa.maxResponseBodySize < 0 { return io.ReadAll(res.Body) @@ -236,62 +245,68 @@ func writeHeader(req, forwardReq *http.Request, trustForwardHeader bool, allowed RemoveConnectionHeaders(forwardReq) utils.RemoveHeaders(forwardReq.Header, hopHeaders...) + if !trustForwardHeader { + forwardedheaders.DeleteXForwardedHeaders(forwardReq.Header) + } + forwardReq.Header = filterForwardRequestHeaders(forwardReq.Header, allowedHeaders) if clientIP, _, err := net.SplitHostPort(req.RemoteAddr); err == nil { - if trustForwardHeader { - if prior, ok := req.Header[forward.XForwardedFor]; ok { - clientIP = strings.Join(prior, ", ") + ", " + clientIP - } + if prior, ok := forwardReq.Header[forwardedheaders.XForwardedFor]; ok { + clientIP = strings.Join(prior, ", ") + ", " + clientIP } - forwardReq.Header.Set(forward.XForwardedFor, clientIP) + forwardReq.Header.Set(forwardedheaders.XForwardedFor, clientIP) } - xMethod := req.Header.Get(xForwardedMethod) - switch { - case xMethod != "" && trustForwardHeader: - forwardReq.Header.Set(xForwardedMethod, xMethod) - case req.Method != "": - forwardReq.Header.Set(xForwardedMethod, req.Method) - default: - forwardReq.Header.Del(xForwardedMethod) + if _, ok := forwardReq.Header[forwardedheaders.XForwardedMethod]; !ok { + forwardReq.Header.Set(forwardedheaders.XForwardedMethod, req.Method) } - xfp := req.Header.Get(forward.XForwardedProto) - switch { - case xfp != "" && trustForwardHeader: - forwardReq.Header.Set(forward.XForwardedProto, xfp) - case req.TLS != nil: - forwardReq.Header.Set(forward.XForwardedProto, "https") - default: - forwardReq.Header.Set(forward.XForwardedProto, "http") + if _, ok := forwardReq.Header[forwardedheaders.XForwardedProto]; !ok { + forwardReq.Header.Set(forwardedheaders.XForwardedProto, "http") + if req.TLS != nil { + forwardReq.Header.Set(forwardedheaders.XForwardedProto, "https") + } } - if xfp := req.Header.Get(forward.XForwardedPort); xfp != "" && trustForwardHeader { - forwardReq.Header.Set(forward.XForwardedPort, xfp) + if _, ok := forwardReq.Header[forwardedheaders.XForwardedPort]; !ok { + forwardReq.Header.Set(forwardedheaders.XForwardedPort, forwardedPort(req)) } - xfh := req.Header.Get(forward.XForwardedHost) - switch { - case xfh != "" && trustForwardHeader: - forwardReq.Header.Set(forward.XForwardedHost, xfh) - case req.Host != "": - forwardReq.Header.Set(forward.XForwardedHost, req.Host) - default: - forwardReq.Header.Del(forward.XForwardedHost) + if _, ok := forwardReq.Header[forwardedheaders.XForwardedHost]; !ok { + forwardReq.Header.Set(forwardedheaders.XForwardedHost, req.Host) } - xfURI := req.Header.Get(xForwardedURI) - switch { - case xfURI != "" && trustForwardHeader: - forwardReq.Header.Set(xForwardedURI, xfURI) - case req.URL.RequestURI() != "": - forwardReq.Header.Set(xForwardedURI, req.URL.RequestURI()) - default: - forwardReq.Header.Del(xForwardedURI) + if _, ok := forwardReq.Header[forwardedheaders.XForwardedURI]; !ok { + forwardReq.Header.Set(forwardedheaders.XForwardedURI, req.URL.RequestURI()) } } +// oldWriteHeader is the legacy implementation of writeHeader, which is used when TrustForwardHeader is not set (old false behavior). +// It is kept to avoid breaking existing configurations that rely on the previous behavior. +func oldWriteHeader(req, forwardReq *http.Request, allowedHeaders []string) { + utils.CopyHeaders(forwardReq.Header, req.Header) + + RemoveConnectionHeaders(forwardReq) + utils.RemoveHeaders(forwardReq.Header, hopHeaders...) + + forwardReq.Header = filterForwardRequestHeaders(forwardReq.Header, allowedHeaders) + + if clientIP, _, err := net.SplitHostPort(req.RemoteAddr); err == nil { + forwardReq.Header.Set(forwardedheaders.XForwardedFor, clientIP) + } + + proto := "http" + if req.TLS != nil { + proto = "https" + } + forwardReq.Header.Set(forwardedheaders.XForwardedProto, proto) + + forwardReq.Header.Set(forwardedheaders.XForwardedMethod, req.Method) + forwardReq.Header.Set(forwardedheaders.XForwardedHost, req.Host) + forwardReq.Header.Set(forwardedheaders.XForwardedURI, req.URL.RequestURI()) +} + func filterForwardRequestHeaders(forwardRequestHeaders http.Header, allowedHeaders []string) http.Header { if len(allowedHeaders) == 0 { return forwardRequestHeaders @@ -299,11 +314,30 @@ func filterForwardRequestHeaders(forwardRequestHeaders http.Header, allowedHeade filteredHeaders := http.Header{} for _, headerName := range allowedHeaders { - values := forwardRequestHeaders.Values(headerName) - if len(values) > 0 { - filteredHeaders[http.CanonicalHeaderKey(headerName)] = append([]string(nil), values...) + if values := forwardRequestHeaders.Values(headerName); len(values) > 0 { + filteredHeaders[http.CanonicalHeaderKey(headerName)] = values } } return filteredHeaders } + +func forwardedPort(req *http.Request) string { + if req == nil { + return "" + } + + if _, port, err := net.SplitHostPort(req.Host); err == nil && port != "" { + return port + } + + if req.Header.Get(forwardedheaders.XForwardedProto) == "https" || req.Header.Get(forwardedheaders.XForwardedProto) == "wss" { + return "443" + } + + if req.TLS != nil { + return "443" + } + + return "80" +} diff --git a/pkg/middlewares/auth/forward_test.go b/pkg/middlewares/auth/forward_test.go index 952fc8b751..9196166420 100644 --- a/pkg/middlewares/auth/forward_test.go +++ b/pkg/middlewares/auth/forward_test.go @@ -244,9 +244,9 @@ func TestForwardAuthFailResponseHeaders(t *testing.T) { assert.Equal(t, "Forbidden\n", string(body)) } -func Test_writeHeader(t *testing.T) { +func TestForwardAuth_writeHeader(t *testing.T) { testCases := []struct { - name string + desc string headers map[string]string authRequestHeaders []string trustForwardHeader bool @@ -255,7 +255,7 @@ func Test_writeHeader(t *testing.T) { checkForUnexpectedHeaders bool }{ { - name: "trust Forward Header", + desc: "trust forward header", headers: map[string]string{ "Accept": "application/json", "X-Forwarded-Host": "fii.bir", @@ -267,7 +267,7 @@ func Test_writeHeader(t *testing.T) { }, }, { - name: "not trust Forward Header", + desc: "not trust forward header", headers: map[string]string{ "Accept": "application/json", "X-Forwarded-Host": "fii.bir", @@ -279,7 +279,7 @@ func Test_writeHeader(t *testing.T) { }, }, { - name: "trust Forward Header with empty Host", + desc: "trust forward header with empty Host", headers: map[string]string{ "Accept": "application/json", "X-Forwarded-Host": "fii.bir", @@ -292,7 +292,7 @@ func Test_writeHeader(t *testing.T) { }, }, { - name: "not trust Forward Header with empty Host", + desc: "not trust forward header with empty Host", headers: map[string]string{ "Accept": "application/json", "X-Forwarded-Host": "fii.bir", @@ -305,7 +305,7 @@ func Test_writeHeader(t *testing.T) { }, }, { - name: "trust Forward Header with forwarded URI", + desc: "trust forward header with forwarded URI", headers: map[string]string{ "Accept": "application/json", "X-Forwarded-Host": "fii.bir", @@ -319,7 +319,7 @@ func Test_writeHeader(t *testing.T) { }, }, { - name: "not trust Forward Header with forward requested URI", + desc: "not trust forward header with forward requested URI", headers: map[string]string{ "Accept": "application/json", "X-Forwarded-Host": "fii.bir", @@ -333,7 +333,7 @@ func Test_writeHeader(t *testing.T) { }, }, { - name: "trust Forward Header with forwarded request Method", + desc: "trust forward header with forwarded request Method", headers: map[string]string{ "X-Forwarded-Method": "OPTIONS", }, @@ -343,7 +343,7 @@ func Test_writeHeader(t *testing.T) { }, }, { - name: "not trust Forward Header with forward request Method", + desc: "not trust forward header with forward request Method", headers: map[string]string{ "X-Forwarded-Method": "OPTIONS", }, @@ -353,7 +353,7 @@ func Test_writeHeader(t *testing.T) { }, }, { - name: "remove hop-by-hop headers", + desc: "remove hop-by-hop headers", headers: map[string]string{ forward.Connection: "Connection", forward.KeepAlive: "KeepAlive", @@ -372,13 +372,14 @@ func Test_writeHeader(t *testing.T) { "X-Forwarded-Host": "foo.bar", "X-Forwarded-Uri": "/path?q=1", "X-Forwarded-Method": "GET", + "X-Forwarded-Port": "80", forward.ProxyAuthenticate: "ProxyAuthenticate", forward.ProxyAuthorization: "ProxyAuthorization", }, checkForUnexpectedHeaders: true, }, { - name: "filter forward request headers", + desc: "filter forward request headers", headers: map[string]string{ "X-CustomHeader": "CustomHeader", "Content-Type": "multipart/form-data; boundary=---123456", @@ -393,11 +394,12 @@ func Test_writeHeader(t *testing.T) { "X-Forwarded-Host": "foo.bar", "X-Forwarded-Uri": "/path?q=1", "X-Forwarded-Method": "GET", + "X-Forwarded-Port": "80", }, checkForUnexpectedHeaders: true, }, { - name: "filter forward request headers doesn't add new headers", + desc: "filter forward request headers doesn't add new headers", headers: map[string]string{ "X-CustomHeader": "CustomHeader", "Content-Type": "multipart/form-data; boundary=---123456", @@ -413,13 +415,63 @@ func Test_writeHeader(t *testing.T) { "X-Forwarded-Host": "foo.bar", "X-Forwarded-Uri": "/path?q=1", "X-Forwarded-Method": "GET", + "X-Forwarded-Port": "80", }, checkForUnexpectedHeaders: true, }, + { + desc: "authRequestHeaders and XForwarded are kept if trusted", + headers: map[string]string{ + "X-CustomHeader": "CustomHeader", + "X-Forwarded-Uri": "/path?q=2", + }, + authRequestHeaders: []string{ + "X-CustomHeader", + }, + trustForwardHeader: true, + expectedHeaders: map[string]string{ + "X-CustomHeader": "CustomHeader", + "X-Forwarded-Proto": "http", + "X-Forwarded-Host": "foo.bar", + "X-Forwarded-Uri": "/path?q=2", + "X-Forwarded-Method": "GET", + "X-Forwarded-Port": "80", + }, + checkForUnexpectedHeaders: true, + }, + { + desc: "X-Forwarded and X_forwarded headers are removed when not trusted", + headers: map[string]string{ + "X-CustomHeader": "CustomHeader", + "X_forwarded_for": "127.0.0.1", + "X-Forwarded-Proto": "xxx", + }, + trustForwardHeader: false, + expectedHeaders: map[string]string{ + "X-CustomHeader": "CustomHeader", + "X-Forwarded-Proto": "http", + "X-Forwarded-Host": "foo.bar", + "X-Forwarded-Uri": "/path?q=1", + "X-Forwarded-Method": "GET", + "X-Forwarded-Port": "80", + }, + }, } for _, test := range testCases { - t.Run(test.name, func(t *testing.T) { + t.Run(test.desc, func(t *testing.T) { + t.Parallel() + + cfg := dynamic.ForwardAuth{ + TrustForwardHeader: &test.trustForwardHeader, + AuthRequestHeaders: test.authRequestHeaders, + } + hdl, err := NewForward(t.Context(), nil, cfg, "test") + require.NoError(t, err) + + fwdAuth, ok := hdl.(*forwardAuth) + require.True(t, ok) + req := testhelpers.MustNewRequest(http.MethodGet, "http://foo.bar/path?q=1", nil) for key, value := range test.headers { req.Header.Set(key, value) @@ -431,17 +483,186 @@ func Test_writeHeader(t *testing.T) { forwardReq := testhelpers.MustNewRequest(http.MethodGet, "http://foo.bar/path?q=1", nil) - writeHeader(req, forwardReq, test.trustForwardHeader, test.authRequestHeaders) - - actualHeaders := forwardReq.Header + writeHeader(req, forwardReq, *fwdAuth.trustForwardHeader, fwdAuth.authRequestHeaders) expectedHeaders := test.expectedHeaders for key, value := range expectedHeaders { - assert.Equal(t, value, actualHeaders.Get(key)) - actualHeaders.Del(key) + assert.Equal(t, value, forwardReq.Header.Get(key)) + forwardReq.Header.Del(key) } if test.checkForUnexpectedHeaders { - for key := range actualHeaders { + for key := range forwardReq.Header { + assert.Fail(t, "Unexpected header found", key) + } + } + }) + } +} + +func TestForwardAuth_oldWriteHeader(t *testing.T) { + testCases := []struct { + desc string + headers map[string]string + authRequestHeaders []string + emptyHost bool + expectedHeaders map[string]string + checkForUnexpectedHeaders bool + }{ + { + desc: "not trust forward header", + headers: map[string]string{ + "Accept": "application/json", + "X-Forwarded-Host": "fii.bir", + }, + expectedHeaders: map[string]string{ + "Accept": "application/json", + "X-Forwarded-Host": "foo.bar", + }, + }, + { + desc: "not trust forward header with empty Host", + headers: map[string]string{ + "Accept": "application/json", + "X-Forwarded-Host": "fii.bir", + }, + emptyHost: true, + expectedHeaders: map[string]string{ + "Accept": "application/json", + "X-Forwarded-Host": "", + }, + }, + { + desc: "not trust forward header with forward requested URI", + headers: map[string]string{ + "Accept": "application/json", + "X-Forwarded-Host": "fii.bir", + "X-Forwarded-Uri": "/forward?q=1", + }, + expectedHeaders: map[string]string{ + "Accept": "application/json", + "X-Forwarded-Host": "foo.bar", + "X-Forwarded-Uri": "/path?q=1", + }, + }, + { + desc: "not trust forward header with forward request Method", + headers: map[string]string{ + "X-Forwarded-Method": "OPTIONS", + }, + expectedHeaders: map[string]string{ + "X-Forwarded-Method": "GET", + }, + }, + { + desc: "remove hop-by-hop headers", + headers: map[string]string{ + forward.Connection: "Connection", + forward.KeepAlive: "KeepAlive", + forward.ProxyAuthenticate: "ProxyAuthenticate", + forward.ProxyAuthorization: "ProxyAuthorization", + forward.Te: "Te", + forward.Trailers: "Trailers", + forward.TransferEncoding: "TransferEncoding", + forward.Upgrade: "Upgrade", + "X-CustomHeader": "CustomHeader", + }, + expectedHeaders: map[string]string{ + "X-CustomHeader": "CustomHeader", + "X-Forwarded-Proto": "http", + "X-Forwarded-Host": "foo.bar", + "X-Forwarded-Uri": "/path?q=1", + "X-Forwarded-Method": "GET", + forward.ProxyAuthenticate: "ProxyAuthenticate", + forward.ProxyAuthorization: "ProxyAuthorization", + }, + checkForUnexpectedHeaders: true, + }, + { + desc: "filter forward request headers", + headers: map[string]string{ + "X-CustomHeader": "CustomHeader", + "Content-Type": "multipart/form-data; boundary=---123456", + }, + authRequestHeaders: []string{ + "X-CustomHeader", + }, + expectedHeaders: map[string]string{ + "x-customHeader": "CustomHeader", + "X-Forwarded-Proto": "http", + "X-Forwarded-Host": "foo.bar", + "X-Forwarded-Uri": "/path?q=1", + "X-Forwarded-Method": "GET", + }, + checkForUnexpectedHeaders: true, + }, + { + desc: "filter forward request headers doesn't add new headers", + headers: map[string]string{ + "X-CustomHeader": "CustomHeader", + "Content-Type": "multipart/form-data; boundary=---123456", + }, + authRequestHeaders: []string{ + "X-CustomHeader", + "X-Non-Exists-Header", + }, + expectedHeaders: map[string]string{ + "X-CustomHeader": "CustomHeader", + "X-Forwarded-Proto": "http", + "X-Forwarded-Host": "foo.bar", + "X-Forwarded-Uri": "/path?q=1", + "X-Forwarded-Method": "GET", + }, + checkForUnexpectedHeaders: true, + }, + + { + desc: "X-Forwarded-Prefix is kept for non-breaking behavior", + headers: map[string]string{ + "X-CustomHeader": "CustomHeader", + "X-Forwarded-Prefix": "foo.bar", + }, + expectedHeaders: map[string]string{ + "X-CustomHeader": "CustomHeader", + "X-Forwarded-Proto": "http", + "X-Forwarded-Host": "foo.bar", + "X-Forwarded-Uri": "/path?q=1", + "X-Forwarded-Method": "GET", + "X-Forwarded-Prefix": "foo.bar", + }, + checkForUnexpectedHeaders: true, + }, + } + + for _, test := range testCases { + t.Run(test.desc, func(t *testing.T) { + t.Parallel() + + hdl, err := NewForward(t.Context(), nil, dynamic.ForwardAuth{AuthRequestHeaders: test.authRequestHeaders}, "test") + require.NoError(t, err) + + fwdAuth, ok := hdl.(*forwardAuth) + require.True(t, ok) + + req := testhelpers.MustNewRequest(http.MethodGet, "http://foo.bar/path?q=1", nil) + for key, value := range test.headers { + req.Header.Set(key, value) + } + + if test.emptyHost { + req.Host = "" + } + + forwardReq := testhelpers.MustNewRequest(http.MethodGet, "http://foo.bar/path?q=1", nil) + + oldWriteHeader(req, forwardReq, fwdAuth.authRequestHeaders) + + expectedHeaders := test.expectedHeaders + for key, value := range expectedHeaders { + assert.Equal(t, value, forwardReq.Header.Get(key)) + forwardReq.Header.Del(key) + } + if test.checkForUnexpectedHeaders { + for key := range forwardReq.Header { assert.Fail(t, "Unexpected header found", key) } } @@ -482,9 +703,9 @@ func TestForwardAuthUsesTracing(t *testing.T) { assert.Equal(t, http.StatusOK, res.StatusCode) } -func Test_ForwardAuthMaxResponseBodySize(t *testing.T) { +func TestForwardAuthMaxResponseBodySize(t *testing.T) { testCases := []struct { - name string + desc string maxResponseBodySize int64 status int body string @@ -492,7 +713,7 @@ func Test_ForwardAuthMaxResponseBodySize(t *testing.T) { expectedBody string }{ { - name: "auth failure, unlimited response body", + desc: "auth failure, unlimited response body", maxResponseBodySize: -1, status: http.StatusForbidden, body: "Forbidden", @@ -500,7 +721,7 @@ func Test_ForwardAuthMaxResponseBodySize(t *testing.T) { expectedBody: "Forbidden", }, { - name: "auth failure, response body exceeds the limit", + desc: "auth failure, response body exceeds the limit", maxResponseBodySize: 1, status: http.StatusForbidden, body: "Forbidden", @@ -508,7 +729,7 @@ func Test_ForwardAuthMaxResponseBodySize(t *testing.T) { expectedBody: "", }, { - name: "auth success within limit", + desc: "auth success within limit", maxResponseBodySize: 100, status: http.StatusOK, body: "ok", @@ -516,7 +737,7 @@ func Test_ForwardAuthMaxResponseBodySize(t *testing.T) { expectedBody: "traefik\n", }, { - name: "auth success body exceeds limit", + desc: "auth success body exceeds limit", maxResponseBodySize: 1, status: http.StatusOK, body: "large auth response", @@ -526,7 +747,7 @@ func Test_ForwardAuthMaxResponseBodySize(t *testing.T) { } for _, test := range testCases { - t.Run(test.name, func(t *testing.T) { + t.Run(test.desc, func(t *testing.T) { server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(test.status) fmt.Fprint(w, test.body) @@ -569,6 +790,6 @@ type mockBackend struct { opentracing.Tracer } -func (b *mockBackend) Setup(componentName string) (opentracing.Tracer, io.Closer, error) { +func (b *mockBackend) Setup(_ string) (opentracing.Tracer, io.Closer, error) { return b.Tracer, io.NopCloser(nil), nil } diff --git a/pkg/middlewares/forwardedheaders/forwarded_header.go b/pkg/middlewares/forwardedheaders/forwarded_header.go index ba1ca94fca..f80e45acda 100644 --- a/pkg/middlewares/forwardedheaders/forwarded_header.go +++ b/pkg/middlewares/forwardedheaders/forwarded_header.go @@ -13,14 +13,14 @@ import ( ) const ( - xForwardedProto = "X-Forwarded-Proto" - xForwardedFor = "X-Forwarded-For" - xForwardedHost = "X-Forwarded-Host" - xForwardedPort = "X-Forwarded-Port" + XForwardedProto = "X-Forwarded-Proto" + XForwardedFor = "X-Forwarded-For" + XForwardedHost = "X-Forwarded-Host" + XForwardedPort = "X-Forwarded-Port" xForwardedServer = "X-Forwarded-Server" - xForwardedURI = "X-Forwarded-Uri" - xForwardedMethod = "X-Forwarded-Method" - xForwardedPrefix = "X-Forwarded-Prefix" + 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" @@ -28,18 +28,18 @@ const ( upgrade = "Upgrade" ) -// xHeadersSet contains the canonical X-headers managed by Traefik. Used by +// XHeadersSet contains the canonical X-headers managed by Traefik. Used by // isManagedXHeader to detect both the canonical form and underscore variants // that Go's HTTP server preserves (e.g. X_Forwarded_Proto). -var xHeadersSet = map[string]struct{}{ - xForwardedProto: {}, - xForwardedFor: {}, - xForwardedHost: {}, - xForwardedPort: {}, +var XHeadersSet = map[string]struct{}{ + XForwardedProto: {}, + XForwardedFor: {}, + XForwardedHost: {}, + XForwardedPort: {}, xForwardedServer: {}, - xForwardedURI: {}, - xForwardedMethod: {}, - xForwardedPrefix: {}, + XForwardedURI: {}, + XForwardedMethod: {}, + XForwardedPrefix: {}, xForwardedTLSClientCert: {}, xForwardedTLSClientCertInfo: {}, xRealIP: {}, @@ -53,14 +53,14 @@ func isManagedXHeader(key string) bool { if len(key) == 0 || key[0] != 'X' { return false } - if _, ok := xHeadersSet[key]; ok { + if _, ok := XHeadersSet[key]; ok { return true } if strings.IndexByte(key, '_') < 0 { return false } canonical := http.CanonicalHeaderKey(strings.ReplaceAll(key, "_", "-")) - _, ok := xHeadersSet[canonical] + _, ok := XHeadersSet[canonical] return ok } @@ -108,65 +108,10 @@ func NewXForwarded(insecure bool, trustedIPs []string, connectionHeaders []strin }, nil } -// removeIPv6Zone removes the zone if the given IP is an ipv6 address and it has {zone} information in it, -// like "[fe80::d806:a55d:eb1b:49cc%vEthernet (vmxnet3 Ethernet Adapter - Virtual Switch)]:64692". -func removeIPv6Zone(clientIP string) string { - if before, _, found := strings.Cut(clientIP, "%"); found { - return before - } - return clientIP -} - -// isWebsocketRequest returns whether the specified HTTP request is a websocket handshake request. -func isWebsocketRequest(req *http.Request) bool { - containsHeader := func(name, value string) bool { - h := unsafeHeader(req.Header).Get(name) - for { - before, after, found := strings.Cut(h, ",") - if strings.EqualFold(value, strings.TrimSpace(before)) { - return true - } - if !found { - return false - } - h = after - } - } - - return containsHeader(connection, "upgrade") && containsHeader(upgrade, "websocket") -} - -func forwardedPort(req *http.Request) string { - if req == nil { - return "" - } - - if _, port, err := net.SplitHostPort(req.Host); err == nil && port != "" { - return port - } - - if unsafeHeader(req.Header).Get(xForwardedProto) == "https" || unsafeHeader(req.Header).Get(xForwardedProto) == "wss" { - return "443" - } - - if req.TLS != nil { - return "443" - } - - return "80" -} - // ServeHTTP implements http.Handler. func (x *XForwarded) ServeHTTP(w http.ResponseWriter, r *http.Request) { if !x.insecure && !x.isTrustedIP(r.RemoteAddr) { - // Strip X-Forwarded headers and their underscore variants - // (e.g. X_Forwarded_Proto), which Go's HTTP server preserves - // alongside the canonical dash form. - for key := range r.Header { - if isManagedXHeader(key) { - delete(r.Header, key) - } - } + DeleteXForwardedHeaders(r.Header) } x.rewrite(r) @@ -192,38 +137,38 @@ func (x *XForwarded) rewrite(outreq *http.Request) { } } - xfProto := unsafeHeader(outreq.Header).Get(xForwardedProto) + xfProto := unsafeHeader(outreq.Header).Get(XForwardedProto) if xfProto == "" { // TODO: is this expected to set the X-Forwarded-Proto header value to // ws(s) as the underlying request used to upgrade the connection is // made over HTTP(S)? if isWebsocketRequest(outreq) { if outreq.TLS != nil { - unsafeHeader(outreq.Header).Set(xForwardedProto, "wss") + unsafeHeader(outreq.Header).Set(XForwardedProto, "wss") } else { - unsafeHeader(outreq.Header).Set(xForwardedProto, "ws") + unsafeHeader(outreq.Header).Set(XForwardedProto, "ws") } } else { if outreq.TLS != nil { - unsafeHeader(outreq.Header).Set(xForwardedProto, "https") + unsafeHeader(outreq.Header).Set(XForwardedProto, "https") } else { - unsafeHeader(outreq.Header).Set(xForwardedProto, "http") + unsafeHeader(outreq.Header).Set(XForwardedProto, "http") } } } - if xfPort := unsafeHeader(outreq.Header).Get(xForwardedPort); xfPort == "" { - unsafeHeader(outreq.Header).Set(xForwardedPort, forwardedPort(outreq)) + if xfPort := unsafeHeader(outreq.Header).Get(XForwardedPort); xfPort == "" { + unsafeHeader(outreq.Header).Set(XForwardedPort, forwardedPort(outreq)) } - if xfHost := unsafeHeader(outreq.Header).Get(xForwardedHost); xfHost == "" && outreq.Host != "" { - unsafeHeader(outreq.Header).Set(xForwardedHost, outreq.Host) + if xfHost := unsafeHeader(outreq.Header).Get(XForwardedHost); xfHost == "" && outreq.Host != "" { + unsafeHeader(outreq.Header).Set(XForwardedHost, outreq.Host) } // Per https://www.rfc-editor.org/rfc/rfc2616#section-4.2, the Forwarded IPs list is in // the same order as the values in the X-Forwarded-For header(s). - if xffs := unsafeHeader(outreq.Header).Values(xForwardedFor); len(xffs) > 0 { - unsafeHeader(outreq.Header).Set(xForwardedFor, strings.Join(xffs, ", ")) + if xffs := unsafeHeader(outreq.Header).Values(XForwardedFor); len(xffs) > 0 { + unsafeHeader(outreq.Header).Set(XForwardedFor, strings.Join(xffs, ", ")) } if x.hostname != "" { @@ -274,6 +219,65 @@ func (x *XForwarded) removeConnectionHeaders(req *http.Request) { unsafeHeader(req.Header).Del(connection) } +// DeleteXForwardedHeaders Strip X-Forwarded headers and their underscore variants +// (e.g. X_Forwarded_Proto), which Go's HTTP server preserves +// alongside the canonical dash form. +func DeleteXForwardedHeaders(headers http.Header) { + for key := range headers { + if isManagedXHeader(key) { + delete(headers, key) + } + } +} + +// removeIPv6Zone removes the zone if the given IP is an ipv6 address and it has {zone} information in it, +// like "[fe80::d806:a55d:eb1b:49cc%vEthernet (vmxnet3 Ethernet Adapter - Virtual Switch)]:64692". +func removeIPv6Zone(clientIP string) string { + if before, _, found := strings.Cut(clientIP, "%"); found { + return before + } + return clientIP +} + +// isWebsocketRequest returns whether the specified HTTP request is a websocket handshake request. +func isWebsocketRequest(req *http.Request) bool { + containsHeader := func(name, value string) bool { + h := unsafeHeader(req.Header).Get(name) + for { + before, after, found := strings.Cut(h, ",") + if strings.EqualFold(value, strings.TrimSpace(before)) { + return true + } + if !found { + return false + } + h = after + } + } + + return containsHeader(connection, "upgrade") && containsHeader(upgrade, "websocket") +} + +func forwardedPort(req *http.Request) string { + if req == nil { + return "" + } + + if _, port, err := net.SplitHostPort(req.Host); err == nil && port != "" { + return port + } + + if unsafeHeader(req.Header).Get(XForwardedProto) == "https" || unsafeHeader(req.Header).Get(XForwardedProto) == "wss" { + return "443" + } + + if req.TLS != nil { + return "443" + } + + return "80" +} + // unsafeHeader allows to manage Header values. // Must be used only when the header name is already a canonical key. type unsafeHeader map[string][]string diff --git a/pkg/middlewares/forwardedheaders/forwarded_header_test.go b/pkg/middlewares/forwardedheaders/forwarded_header_test.go index 711e96b001..50b734bf80 100644 --- a/pkg/middlewares/forwardedheaders/forwarded_header_test.go +++ b/pkg/middlewares/forwardedheaders/forwarded_header_test.go @@ -31,9 +31,9 @@ func TestServeHTTP(t *testing.T) { remoteAddr: "", incomingHeaders: map[string][]string{}, expectedHeaders: map[string]string{ - xForwardedFor: "", - xForwardedURI: "", - xForwardedMethod: "", + XForwardedFor: "", + XForwardedURI: "", + XForwardedMethod: "", xForwardedTLSClientCert: "", xForwardedTLSClientCertInfo: "", }, @@ -44,22 +44,22 @@ func TestServeHTTP(t *testing.T) { trustedIps: nil, remoteAddr: "", incomingHeaders: map[string][]string{ - xForwardedFor: {"10.0.1.0, 10.0.1.12"}, - xForwardedURI: {"/bar"}, - xForwardedMethod: {"GET"}, + XForwardedFor: {"10.0.1.0, 10.0.1.12"}, + XForwardedURI: {"/bar"}, + XForwardedMethod: {"GET"}, xForwardedTLSClientCert: {"Cert"}, xForwardedTLSClientCertInfo: {"CertInfo"}, - xForwardedPrefix: {"/prefix"}, + XForwardedPrefix: {"/prefix"}, "X_forwarded_proto": {"https"}, "X_forwarded_for": {"10.0.0.1"}, }, expectedHeaders: map[string]string{ - xForwardedFor: "10.0.1.0, 10.0.1.12", - xForwardedURI: "/bar", - xForwardedMethod: "GET", + XForwardedFor: "10.0.1.0, 10.0.1.12", + XForwardedURI: "/bar", + XForwardedMethod: "GET", xForwardedTLSClientCert: "Cert", xForwardedTLSClientCertInfo: "CertInfo", - xForwardedPrefix: "/prefix", + XForwardedPrefix: "/prefix", "X_forwarded_proto": "https", "X_forwarded_for": "10.0.0.1", }, @@ -70,23 +70,23 @@ func TestServeHTTP(t *testing.T) { trustedIps: nil, remoteAddr: "", incomingHeaders: map[string][]string{ - xForwardedFor: {"10.0.1.0, 10.0.1.12"}, - xForwardedURI: {"/bar"}, - xForwardedMethod: {"GET"}, + XForwardedFor: {"10.0.1.0, 10.0.1.12"}, + XForwardedURI: {"/bar"}, + XForwardedMethod: {"GET"}, xForwardedTLSClientCert: {"Cert"}, xForwardedTLSClientCertInfo: {"CertInfo"}, - xForwardedPrefix: {"/prefix"}, + XForwardedPrefix: {"/prefix"}, "X_forwarded_proto": {"https"}, "X_forwarded_for": {"10.0.0.1"}, "X_forwarded_host": {"evil.example"}, }, expectedHeaders: map[string]string{ - xForwardedFor: "", - xForwardedURI: "", - xForwardedMethod: "", + XForwardedFor: "", + XForwardedURI: "", + XForwardedMethod: "", xForwardedTLSClientCert: "", xForwardedTLSClientCertInfo: "", - xForwardedPrefix: "", + XForwardedPrefix: "", "X_forwarded_proto": "", "X_forwarded_for": "", "X_forwarded_host": "", @@ -98,22 +98,22 @@ func TestServeHTTP(t *testing.T) { trustedIps: []string{"10.0.1.100"}, remoteAddr: "10.0.1.100:80", incomingHeaders: map[string][]string{ - xForwardedFor: {"10.0.1.0, 10.0.1.12"}, - xForwardedURI: {"/bar"}, - xForwardedMethod: {"GET"}, + XForwardedFor: {"10.0.1.0, 10.0.1.12"}, + XForwardedURI: {"/bar"}, + XForwardedMethod: {"GET"}, xForwardedTLSClientCert: {"Cert"}, xForwardedTLSClientCertInfo: {"CertInfo"}, - xForwardedPrefix: {"/prefix"}, + XForwardedPrefix: {"/prefix"}, "X_forwarded_proto": {"https"}, "X_forwarded_for": {"10.0.0.1"}, }, expectedHeaders: map[string]string{ - xForwardedFor: "10.0.1.0, 10.0.1.12", - xForwardedURI: "/bar", - xForwardedMethod: "GET", + XForwardedFor: "10.0.1.0, 10.0.1.12", + XForwardedURI: "/bar", + XForwardedMethod: "GET", xForwardedTLSClientCert: "Cert", xForwardedTLSClientCertInfo: "CertInfo", - xForwardedPrefix: "/prefix", + XForwardedPrefix: "/prefix", "X_forwarded_proto": "https", "X_forwarded_for": "10.0.0.1", }, @@ -124,20 +124,20 @@ func TestServeHTTP(t *testing.T) { trustedIps: []string{"10.0.1.100"}, remoteAddr: "10.0.1.101:80", incomingHeaders: map[string][]string{ - xForwardedFor: {"10.0.1.0, 10.0.1.12"}, - xForwardedURI: {"/bar"}, - xForwardedMethod: {"GET"}, + XForwardedFor: {"10.0.1.0, 10.0.1.12"}, + XForwardedURI: {"/bar"}, + XForwardedMethod: {"GET"}, xForwardedTLSClientCert: {"Cert"}, xForwardedTLSClientCertInfo: {"CertInfo"}, - xForwardedPrefix: {"/prefix"}, + XForwardedPrefix: {"/prefix"}, }, expectedHeaders: map[string]string{ - xForwardedFor: "", - xForwardedURI: "", - xForwardedMethod: "", + XForwardedFor: "", + XForwardedURI: "", + XForwardedMethod: "", xForwardedTLSClientCert: "", xForwardedTLSClientCertInfo: "", - xForwardedPrefix: "", + XForwardedPrefix: "", }, }, { @@ -146,20 +146,20 @@ func TestServeHTTP(t *testing.T) { trustedIps: []string{"1.2.3.4/24"}, remoteAddr: "1.2.3.156:80", incomingHeaders: map[string][]string{ - xForwardedFor: {"10.0.1.0, 10.0.1.12"}, - xForwardedURI: {"/bar"}, - xForwardedMethod: {"GET"}, + XForwardedFor: {"10.0.1.0, 10.0.1.12"}, + XForwardedURI: {"/bar"}, + XForwardedMethod: {"GET"}, xForwardedTLSClientCert: {"Cert"}, xForwardedTLSClientCertInfo: {"CertInfo"}, - xForwardedPrefix: {"/prefix"}, + XForwardedPrefix: {"/prefix"}, }, expectedHeaders: map[string]string{ - xForwardedFor: "10.0.1.0, 10.0.1.12", - xForwardedURI: "/bar", - xForwardedMethod: "GET", + XForwardedFor: "10.0.1.0, 10.0.1.12", + XForwardedURI: "/bar", + XForwardedMethod: "GET", xForwardedTLSClientCert: "Cert", xForwardedTLSClientCertInfo: "CertInfo", - xForwardedPrefix: "/prefix", + XForwardedPrefix: "/prefix", }, }, { @@ -168,34 +168,34 @@ func TestServeHTTP(t *testing.T) { trustedIps: []string{"1.2.3.4/24"}, remoteAddr: "10.0.1.101:80", incomingHeaders: map[string][]string{ - xForwardedFor: {"10.0.1.0, 10.0.1.12"}, - xForwardedURI: {"/bar"}, - xForwardedMethod: {"GET"}, + XForwardedFor: {"10.0.1.0, 10.0.1.12"}, + XForwardedURI: {"/bar"}, + XForwardedMethod: {"GET"}, xForwardedTLSClientCert: {"Cert"}, xForwardedTLSClientCertInfo: {"CertInfo"}, - xForwardedPrefix: {"/prefix"}, + XForwardedPrefix: {"/prefix"}, }, expectedHeaders: map[string]string{ - xForwardedFor: "", - xForwardedURI: "", - xForwardedMethod: "", + XForwardedFor: "", + XForwardedURI: "", + XForwardedMethod: "", xForwardedTLSClientCert: "", xForwardedTLSClientCertInfo: "", - xForwardedPrefix: "", + XForwardedPrefix: "", }, }, { desc: "xForwardedFor with multiple header(s) values", insecure: true, incomingHeaders: map[string][]string{ - xForwardedFor: { + XForwardedFor: { "10.0.0.4, 10.0.0.3", "10.0.0.2, 10.0.0.1", "10.0.0.0", }, }, expectedHeaders: map[string]string{ - xForwardedFor: "10.0.0.4, 10.0.0.3, 10.0.0.2, 10.0.0.1, 10.0.0.0", + XForwardedFor: "10.0.0.4, 10.0.0.3, 10.0.0.2, 10.0.0.1, 10.0.0.0", }, }, { @@ -220,14 +220,14 @@ func TestServeHTTP(t *testing.T) { desc: "xForwardedProto with no tls", tls: false, expectedHeaders: map[string]string{ - xForwardedProto: "http", + XForwardedProto: "http", }, }, { desc: "xForwardedProto with tls", tls: true, expectedHeaders: map[string]string{ - xForwardedProto: "https", + XForwardedProto: "https", }, }, { @@ -235,7 +235,7 @@ func TestServeHTTP(t *testing.T) { tls: false, websocket: true, expectedHeaders: map[string]string{ - xForwardedProto: "ws", + XForwardedProto: "ws", }, }, { @@ -243,7 +243,7 @@ func TestServeHTTP(t *testing.T) { tls: true, websocket: true, expectedHeaders: map[string]string{ - xForwardedProto: "wss", + XForwardedProto: "wss", }, }, { @@ -251,17 +251,17 @@ func TestServeHTTP(t *testing.T) { tls: true, websocket: true, incomingHeaders: map[string][]string{ - xForwardedProto: {"wss"}, + XForwardedProto: {"wss"}, }, expectedHeaders: map[string]string{ - xForwardedProto: "wss", + XForwardedProto: "wss", }, }, { desc: "xForwardedPort with explicit port", host: "foo.com:8080", expectedHeaders: map[string]string{ - xForwardedPort: "8080", + XForwardedPort: "8080", }, }, { @@ -269,25 +269,25 @@ func TestServeHTTP(t *testing.T) { // setting insecure just so our initial xForwardedProto does not get cleaned insecure: true, incomingHeaders: map[string][]string{ - xForwardedProto: {"https"}, + XForwardedProto: {"https"}, }, expectedHeaders: map[string]string{ - xForwardedProto: "https", - xForwardedPort: "443", + XForwardedProto: "https", + XForwardedPort: "443", }, }, { desc: "xForwardedPort with implicit tls port from TLS in req", tls: true, expectedHeaders: map[string]string{ - xForwardedPort: "443", + XForwardedPort: "443", }, }, { desc: "xForwardedHost from req host", host: "foo.com:8080", expectedHeaders: map[string]string{ - xForwardedHost: "foo.com:8080", + XForwardedHost: "foo.com:8080", }, }, { @@ -302,40 +302,40 @@ func TestServeHTTP(t *testing.T) { insecure: false, incomingHeaders: map[string][]string{ connection: { - xForwardedProto, - xForwardedFor, - xForwardedURI, - xForwardedMethod, - xForwardedHost, - xForwardedPort, + XForwardedProto, + XForwardedFor, + XForwardedURI, + XForwardedMethod, + XForwardedHost, + XForwardedPort, xForwardedTLSClientCert, xForwardedTLSClientCertInfo, - xForwardedPrefix, + XForwardedPrefix, xRealIP, "X_forwarded_proto", }, - xForwardedProto: {"foo"}, - xForwardedFor: {"foo"}, - xForwardedURI: {"foo"}, - xForwardedMethod: {"foo"}, - xForwardedHost: {"foo"}, - xForwardedPort: {"foo"}, + XForwardedProto: {"foo"}, + XForwardedFor: {"foo"}, + XForwardedURI: {"foo"}, + XForwardedMethod: {"foo"}, + XForwardedHost: {"foo"}, + XForwardedPort: {"foo"}, xForwardedTLSClientCert: {"foo"}, xForwardedTLSClientCertInfo: {"foo"}, - xForwardedPrefix: {"foo"}, + XForwardedPrefix: {"foo"}, xRealIP: {"foo"}, "X_forwarded_proto": {"spoofed"}, }, expectedHeaders: map[string]string{ - xForwardedProto: "http", - xForwardedFor: "", - xForwardedURI: "", - xForwardedMethod: "", - xForwardedHost: "", - xForwardedPort: "80", + XForwardedProto: "http", + XForwardedFor: "", + XForwardedURI: "", + XForwardedMethod: "", + XForwardedHost: "", + XForwardedPort: "80", xForwardedTLSClientCert: "", xForwardedTLSClientCertInfo: "", - xForwardedPrefix: "", + XForwardedPrefix: "", xRealIP: "", "X_forwarded_proto": "", connection: "", @@ -346,38 +346,38 @@ func TestServeHTTP(t *testing.T) { insecure: true, incomingHeaders: map[string][]string{ connection: { - xForwardedProto, - xForwardedFor, - xForwardedURI, - xForwardedMethod, - xForwardedHost, - xForwardedPort, + XForwardedProto, + XForwardedFor, + XForwardedURI, + XForwardedMethod, + XForwardedHost, + XForwardedPort, xForwardedTLSClientCert, xForwardedTLSClientCertInfo, - xForwardedPrefix, + XForwardedPrefix, xRealIP, }, - xForwardedProto: {"foo"}, - xForwardedFor: {"foo"}, - xForwardedURI: {"foo"}, - xForwardedMethod: {"foo"}, - xForwardedHost: {"foo"}, - xForwardedPort: {"foo"}, + XForwardedProto: {"foo"}, + XForwardedFor: {"foo"}, + XForwardedURI: {"foo"}, + XForwardedMethod: {"foo"}, + XForwardedHost: {"foo"}, + XForwardedPort: {"foo"}, xForwardedTLSClientCert: {"foo"}, xForwardedTLSClientCertInfo: {"foo"}, - xForwardedPrefix: {"foo"}, + XForwardedPrefix: {"foo"}, xRealIP: {"foo"}, }, expectedHeaders: map[string]string{ - xForwardedProto: "foo", - xForwardedFor: "foo", - xForwardedURI: "foo", - xForwardedMethod: "foo", - xForwardedHost: "foo", - xForwardedPort: "foo", + XForwardedProto: "foo", + XForwardedFor: "foo", + XForwardedURI: "foo", + XForwardedMethod: "foo", + XForwardedHost: "foo", + XForwardedPort: "foo", xForwardedTLSClientCert: "foo", xForwardedTLSClientCertInfo: "foo", - xForwardedPrefix: "foo", + XForwardedPrefix: "foo", xRealIP: "foo", connection: "", }, @@ -386,51 +386,51 @@ func TestServeHTTP(t *testing.T) { desc: "Untrusted and Connection: Connection header has no effect on X- forwarded headers", insecure: false, connectionHeaders: []string{ - xForwardedProto, - xForwardedFor, - xForwardedURI, - xForwardedMethod, - xForwardedHost, - xForwardedPort, + XForwardedProto, + XForwardedFor, + XForwardedURI, + XForwardedMethod, + XForwardedHost, + XForwardedPort, xForwardedTLSClientCert, xForwardedTLSClientCertInfo, - xForwardedPrefix, + XForwardedPrefix, xRealIP, }, incomingHeaders: map[string][]string{ connection: { - xForwardedProto, - xForwardedFor, - xForwardedURI, - xForwardedMethod, - xForwardedHost, - xForwardedPort, + XForwardedProto, + XForwardedFor, + XForwardedURI, + XForwardedMethod, + XForwardedHost, + XForwardedPort, xForwardedTLSClientCert, xForwardedTLSClientCertInfo, - xForwardedPrefix, + XForwardedPrefix, xRealIP, }, - xForwardedProto: {"foo"}, - xForwardedFor: {"foo"}, - xForwardedURI: {"foo"}, - xForwardedMethod: {"foo"}, - xForwardedHost: {"foo"}, - xForwardedPort: {"foo"}, + XForwardedProto: {"foo"}, + XForwardedFor: {"foo"}, + XForwardedURI: {"foo"}, + XForwardedMethod: {"foo"}, + XForwardedHost: {"foo"}, + XForwardedPort: {"foo"}, xForwardedTLSClientCert: {"foo"}, xForwardedTLSClientCertInfo: {"foo"}, - xForwardedPrefix: {"foo"}, + XForwardedPrefix: {"foo"}, xRealIP: {"foo"}, }, expectedHeaders: map[string]string{ - xForwardedProto: "http", - xForwardedFor: "", - xForwardedURI: "", - xForwardedMethod: "", - xForwardedHost: "", - xForwardedPort: "80", + XForwardedProto: "http", + XForwardedFor: "", + XForwardedURI: "", + XForwardedMethod: "", + XForwardedHost: "", + XForwardedPort: "80", xForwardedTLSClientCert: "", xForwardedTLSClientCertInfo: "", - xForwardedPrefix: "", + XForwardedPrefix: "", xRealIP: "", connection: "", }, @@ -439,51 +439,51 @@ func TestServeHTTP(t *testing.T) { desc: "Trusted (insecure) and Connection: Connection header has no effect on X- forwarded headers", insecure: true, connectionHeaders: []string{ - xForwardedProto, - xForwardedFor, - xForwardedURI, - xForwardedMethod, - xForwardedHost, - xForwardedPort, + XForwardedProto, + XForwardedFor, + XForwardedURI, + XForwardedMethod, + XForwardedHost, + XForwardedPort, xForwardedTLSClientCert, xForwardedTLSClientCertInfo, - xForwardedPrefix, + XForwardedPrefix, xRealIP, }, incomingHeaders: map[string][]string{ connection: { - xForwardedProto, - xForwardedFor, - xForwardedURI, - xForwardedMethod, - xForwardedHost, - xForwardedPort, + XForwardedProto, + XForwardedFor, + XForwardedURI, + XForwardedMethod, + XForwardedHost, + XForwardedPort, xForwardedTLSClientCert, xForwardedTLSClientCertInfo, - xForwardedPrefix, + XForwardedPrefix, xRealIP, }, - xForwardedProto: {"foo"}, - xForwardedFor: {"foo"}, - xForwardedURI: {"foo"}, - xForwardedMethod: {"foo"}, - xForwardedHost: {"foo"}, - xForwardedPort: {"foo"}, + XForwardedProto: {"foo"}, + XForwardedFor: {"foo"}, + XForwardedURI: {"foo"}, + XForwardedMethod: {"foo"}, + XForwardedHost: {"foo"}, + XForwardedPort: {"foo"}, xForwardedTLSClientCert: {"foo"}, xForwardedTLSClientCertInfo: {"foo"}, - xForwardedPrefix: {"foo"}, + XForwardedPrefix: {"foo"}, xRealIP: {"foo"}, }, expectedHeaders: map[string]string{ - xForwardedProto: "foo", - xForwardedFor: "foo", - xForwardedURI: "foo", - xForwardedMethod: "foo", - xForwardedHost: "foo", - xForwardedPort: "foo", + XForwardedProto: "foo", + XForwardedFor: "foo", + XForwardedURI: "foo", + XForwardedMethod: "foo", + XForwardedHost: "foo", + XForwardedPort: "foo", xForwardedTLSClientCert: "foo", xForwardedTLSClientCertInfo: "foo", - xForwardedPrefix: "foo", + XForwardedPrefix: "foo", xRealIP: "foo", connection: "", }, @@ -492,51 +492,51 @@ func TestServeHTTP(t *testing.T) { desc: "Trusted (insecure) and Connection: Testing case sensitivity on connection Headers param", insecure: true, connectionHeaders: []string{ - strings.ToLower(xForwardedProto), - strings.ToLower(xForwardedFor), - strings.ToLower(xForwardedURI), - strings.ToLower(xForwardedMethod), - strings.ToLower(xForwardedHost), - strings.ToLower(xForwardedPort), + strings.ToLower(XForwardedProto), + strings.ToLower(XForwardedFor), + strings.ToLower(XForwardedURI), + strings.ToLower(XForwardedMethod), + strings.ToLower(XForwardedHost), + strings.ToLower(XForwardedPort), strings.ToLower(xForwardedTLSClientCert), strings.ToLower(xForwardedTLSClientCertInfo), - strings.ToLower(xForwardedPrefix), + strings.ToLower(XForwardedPrefix), strings.ToLower(xRealIP), }, incomingHeaders: map[string][]string{ connection: { - xForwardedProto, - xForwardedFor, - xForwardedURI, - xForwardedMethod, - xForwardedHost, - xForwardedPort, + XForwardedProto, + XForwardedFor, + XForwardedURI, + XForwardedMethod, + XForwardedHost, + XForwardedPort, xForwardedTLSClientCert, xForwardedTLSClientCertInfo, - xForwardedPrefix, + XForwardedPrefix, xRealIP, }, - xForwardedProto: {"foo"}, - xForwardedFor: {"foo"}, - xForwardedURI: {"foo"}, - xForwardedMethod: {"foo"}, - xForwardedHost: {"foo"}, - xForwardedPort: {"foo"}, + XForwardedProto: {"foo"}, + XForwardedFor: {"foo"}, + XForwardedURI: {"foo"}, + XForwardedMethod: {"foo"}, + XForwardedHost: {"foo"}, + XForwardedPort: {"foo"}, xForwardedTLSClientCert: {"foo"}, xForwardedTLSClientCertInfo: {"foo"}, - xForwardedPrefix: {"foo"}, + XForwardedPrefix: {"foo"}, xRealIP: {"foo"}, }, expectedHeaders: map[string]string{ - xForwardedProto: "foo", - xForwardedFor: "foo", - xForwardedURI: "foo", - xForwardedMethod: "foo", - xForwardedHost: "foo", - xForwardedPort: "foo", + XForwardedProto: "foo", + XForwardedFor: "foo", + XForwardedURI: "foo", + XForwardedMethod: "foo", + XForwardedHost: "foo", + XForwardedPort: "foo", xForwardedTLSClientCert: "foo", xForwardedTLSClientCertInfo: "foo", - xForwardedPrefix: "foo", + XForwardedPrefix: "foo", xRealIP: "foo", connection: "", }, @@ -546,38 +546,38 @@ func TestServeHTTP(t *testing.T) { insecure: true, incomingHeaders: map[string][]string{ connection: { - strings.ToLower(xForwardedProto), - strings.ToLower(xForwardedFor), - strings.ToLower(xForwardedURI), - strings.ToLower(xForwardedMethod), - strings.ToLower(xForwardedHost), - strings.ToLower(xForwardedPort), + strings.ToLower(XForwardedProto), + strings.ToLower(XForwardedFor), + strings.ToLower(XForwardedURI), + strings.ToLower(XForwardedMethod), + strings.ToLower(XForwardedHost), + strings.ToLower(XForwardedPort), strings.ToLower(xForwardedTLSClientCert), strings.ToLower(xForwardedTLSClientCertInfo), - strings.ToLower(xForwardedPrefix), + strings.ToLower(XForwardedPrefix), strings.ToLower(xRealIP), }, - xForwardedProto: {"foo"}, - xForwardedFor: {"foo"}, - xForwardedURI: {"foo"}, - xForwardedMethod: {"foo"}, - xForwardedHost: {"foo"}, - xForwardedPort: {"foo"}, + XForwardedProto: {"foo"}, + XForwardedFor: {"foo"}, + XForwardedURI: {"foo"}, + XForwardedMethod: {"foo"}, + XForwardedHost: {"foo"}, + XForwardedPort: {"foo"}, xForwardedTLSClientCert: {"foo"}, xForwardedTLSClientCertInfo: {"foo"}, - xForwardedPrefix: {"foo"}, + XForwardedPrefix: {"foo"}, xRealIP: {"foo"}, }, expectedHeaders: map[string]string{ - xForwardedProto: "foo", - xForwardedFor: "foo", - xForwardedURI: "foo", - xForwardedMethod: "foo", - xForwardedHost: "foo", - xForwardedPort: "foo", + XForwardedProto: "foo", + XForwardedFor: "foo", + XForwardedURI: "foo", + XForwardedMethod: "foo", + XForwardedHost: "foo", + XForwardedPort: "foo", xForwardedTLSClientCert: "foo", xForwardedTLSClientCertInfo: "foo", - xForwardedPrefix: "foo", + XForwardedPrefix: "foo", xRealIP: "foo", connection: "", }, diff --git a/pkg/provider/kubernetes/crd/traefikio/v1alpha1/middleware.go b/pkg/provider/kubernetes/crd/traefikio/v1alpha1/middleware.go index 7e448faaee..56cc140989 100644 --- a/pkg/provider/kubernetes/crd/traefikio/v1alpha1/middleware.go +++ b/pkg/provider/kubernetes/crd/traefikio/v1alpha1/middleware.go @@ -144,7 +144,7 @@ type ForwardAuth struct { // Address defines the authentication server address. Address string `json:"address,omitempty"` // TrustForwardHeader defines whether to trust (ie: forward) all X-Forwarded-* headers. - TrustForwardHeader bool `json:"trustForwardHeader,omitempty"` + TrustForwardHeader *bool `json:"trustForwardHeader,omitempty"` // AuthResponseHeaders defines the list of headers to copy from the authentication server response and set on forwarded request, replacing any existing conflicting headers. AuthResponseHeaders []string `json:"authResponseHeaders,omitempty"` // AuthResponseHeadersRegex defines the regex to match headers to copy from the authentication server response and set on forwarded request, after stripping all headers that match the regex. diff --git a/pkg/provider/kubernetes/crd/traefikio/v1alpha1/zz_generated.deepcopy.go b/pkg/provider/kubernetes/crd/traefikio/v1alpha1/zz_generated.deepcopy.go index 07f33f0cac..7bd10b4320 100644 --- a/pkg/provider/kubernetes/crd/traefikio/v1alpha1/zz_generated.deepcopy.go +++ b/pkg/provider/kubernetes/crd/traefikio/v1alpha1/zz_generated.deepcopy.go @@ -200,6 +200,11 @@ func (in *ErrorPage) DeepCopy() *ErrorPage { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ForwardAuth) DeepCopyInto(out *ForwardAuth) { *out = *in + if in.TrustForwardHeader != nil { + in, out := &in.TrustForwardHeader, &out.TrustForwardHeader + *out = new(bool) + **out = **in + } if in.AuthResponseHeaders != nil { in, out := &in.AuthResponseHeaders, &out.AuthResponseHeaders *out = make([]string, len(*in)) diff --git a/pkg/provider/kv/kv_test.go b/pkg/provider/kv/kv_test.go index 92038905e7..ce68242b2c 100644 --- a/pkg/provider/kv/kv_test.go +++ b/pkg/provider/kv/kv_test.go @@ -419,7 +419,7 @@ func Test_buildConfiguration(t *testing.T) { Key: "foobar", InsecureSkipVerify: true, }, - TrustForwardHeader: true, + TrustForwardHeader: pointer(true), AuthResponseHeaders: []string{ "foobar", "foobar", diff --git a/pkg/redactor/redactor_config_test.go b/pkg/redactor/redactor_config_test.go index 2364494b01..86766fb001 100644 --- a/pkg/redactor/redactor_config_test.go +++ b/pkg/redactor/redactor_config_test.go @@ -283,7 +283,7 @@ func init() { Key: "cert.pem", InsecureSkipVerify: true, }, - TrustForwardHeader: true, + TrustForwardHeader: pointer(true), AuthResponseHeaders: []string{"foo"}, AuthResponseHeadersRegex: "foo", AuthRequestHeaders: []string{"foo"}, diff --git a/pkg/server/service/loadbalancer/mirror/mirror_test.go b/pkg/server/service/loadbalancer/mirror/mirror_test.go index 3cb59b2f29..ef41ae9f39 100644 --- a/pkg/server/service/loadbalancer/mirror/mirror_test.go +++ b/pkg/server/service/loadbalancer/mirror/mirror_test.go @@ -139,7 +139,7 @@ func TestMirroringWithBody(t *testing.T) { const numMirrors = 10 var ( - countMirror int32 + countMirror atomic.Int32 body = []byte(`body`) ) @@ -161,7 +161,7 @@ func TestMirroringWithBody(t *testing.T) { bb, err := io.ReadAll(r.Body) assert.NoError(t, err) assert.Equal(t, body, bb) - atomic.AddInt32(&countMirror, 1) + countMirror.Add(1) }), 100) assert.NoError(t, err) } @@ -172,7 +172,7 @@ func TestMirroringWithBody(t *testing.T) { pool.Stop() - val := atomic.LoadInt32(&countMirror) + val := countMirror.Load() assert.Equal(t, numMirrors, int(val)) }