From 4d9031bdb2efafebc746429102ec8ccf617c7eec Mon Sep 17 00:00:00 2001 From: Romain Date: Mon, 18 May 2026 15:06:09 +0200 Subject: [PATCH 1/5] Add error on basic auth build if users is empty --- docs/content/migration/v2.md | 10 +++++++++- pkg/middlewares/auth/basic_auth.go | 4 ++++ pkg/middlewares/auth/basic_auth_test.go | 9 +++++++++ 3 files changed, 22 insertions(+), 1 deletion(-) diff --git a/docs/content/migration/v2.md b/docs/content/migration/v2.md index c2a154e1e6..69baa2c9a9 100644 --- a/docs/content/migration/v2.md +++ b/docs/content/migration/v2.md @@ -826,7 +826,7 @@ environment variable to override the API version used by Traefik. ## v2.11.46 -### Kubernetes providers: `crossProviderNamespaces` +### Kubernetes Providers: `crossProviderNamespaces` In `v2.11.46`, a new `crossProviderNamespaces` option is available on the Kubernetes CRD, Ingress, and Gateway providers. @@ -848,3 +848,11 @@ The behavior is as follows: Please check out the [Kubernetes CRD](../providers/kubernetes-crd.md#crossprovidernamespaces), [Kubernetes Ingress](../providers/kubernetes-ingress.md#crossprovidernamespaces), and [Kubernetes Gateway](../providers/kubernetes-gateway.md#crossprovidernamespaces) provider documentation for more details. + +## v2.11.47 + +### BasicAuth Middleware + +From version `v2.11.47` onwards, the BasicAuth middleware requires a non-empty users configuration in order to be built successfully. +Previously, the middleware would be built successfully but always return a 401 status code for any request. +Now, an error occurs and any routers using it will be unmounted. For the same request, a 404 status code is served instead of a 401 status code. diff --git a/pkg/middlewares/auth/basic_auth.go b/pkg/middlewares/auth/basic_auth.go index 198c8ca84e..dcdb1f45c8 100644 --- a/pkg/middlewares/auth/basic_auth.go +++ b/pkg/middlewares/auth/basic_auth.go @@ -41,6 +41,10 @@ func NewBasic(ctx context.Context, next http.Handler, authConfig dynamic.BasicAu return nil, err } + if len(users) == 0 { + return nil, fmt.Errorf("no users found in %s", authConfig.UsersFile) + } + // 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. diff --git a/pkg/middlewares/auth/basic_auth_test.go b/pkg/middlewares/auth/basic_auth_test.go index baabfba195..b7dd770133 100644 --- a/pkg/middlewares/auth/basic_auth_test.go +++ b/pkg/middlewares/auth/basic_auth_test.go @@ -14,6 +14,15 @@ import ( "github.com/traefik/traefik/v2/pkg/testhelpers" ) +func TestNewBasicEmpty(t *testing.T) { + auth := dynamic.BasicAuth{ + Users: []string{}, + } + + _, err := NewBasic(t.Context(), nil, auth, "authName") + require.Error(t, err) +} + func TestNewBasicNotFoundSecretIsSet(t *testing.T) { auth := dynamic.BasicAuth{ Users: []string{"test:$apr1$H6uskkkW$IgXLP6ewTrSuBkTrqE8wj/"}, From f9d9b72380856c2ea579171053e418ea6db2ea9a Mon Sep 17 00:00:00 2001 From: Romain Date: Wed, 27 May 2026 16:32:10 +0200 Subject: [PATCH 2/5] Avoid ingress path matcher injection and backport 11d251415 --- .../rawdata-ingress-label-selector.json | 2 +- integration/testdata/rawdata-ingress.json | 8 +- .../testdata/rawdata-ingressclass.json | 2 +- ...nvalid-pathmatcher-annotation_endpoint.yml | 11 ++ ...invalid-pathmatcher-annotation_ingress.yml | 18 +++ ...invalid-pathmatcher-annotation_service.yml | 10 ++ pkg/provider/kubernetes/ingress/kubernetes.go | 27 +++- .../kubernetes/ingress/kubernetes_test.go | 150 ++++++++++-------- 8 files changed, 151 insertions(+), 77 deletions(-) create mode 100644 pkg/provider/kubernetes/ingress/fixtures/Ingress-with-invalid-pathmatcher-annotation_endpoint.yml create mode 100644 pkg/provider/kubernetes/ingress/fixtures/Ingress-with-invalid-pathmatcher-annotation_ingress.yml create mode 100644 pkg/provider/kubernetes/ingress/fixtures/Ingress-with-invalid-pathmatcher-annotation_service.yml diff --git a/integration/testdata/rawdata-ingress-label-selector.json b/integration/testdata/rawdata-ingress-label-selector.json index 101e829251..6863c50af2 100644 --- a/integration/testdata/rawdata-ingress-label-selector.json +++ b/integration/testdata/rawdata-ingress-label-selector.json @@ -33,7 +33,7 @@ "web" ], "service": "default-whoami-http", - "rule": "Host(`whoami.test`) \u0026\u0026 PathPrefix(`/whoami`)", + "rule": "Host(\"whoami.test\") \u0026\u0026 PathPrefix(\"/whoami\")", "status": "enabled", "using": [ "web" diff --git a/integration/testdata/rawdata-ingress.json b/integration/testdata/rawdata-ingress.json index 564a15e20c..f6edb43844 100644 --- a/integration/testdata/rawdata-ingress.json +++ b/integration/testdata/rawdata-ingress.json @@ -33,7 +33,7 @@ "web" ], "service": "default-whoami-http", - "rule": "Host(`whoami.test.https`) \u0026\u0026 PathPrefix(`/whoami`)", + "rule": "Host(\"whoami.test.https\") \u0026\u0026 PathPrefix(\"/whoami\")", "status": "enabled", "using": [ "web" @@ -44,7 +44,7 @@ "web" ], "service": "default-whoami-http", - "rule": "Host(`whoami.test`) \u0026\u0026 PathPrefix(`/whoami`)", + "rule": "Host(\"whoami.test\") \u0026\u0026 PathPrefix(\"/whoami\")", "status": "enabled", "using": [ "web" @@ -55,7 +55,7 @@ "web" ], "service": "default-whoami-80", - "rule": "Host(`whoami.test.drop`) \u0026\u0026 PathPrefix(`/drop`)", + "rule": "Host(\"whoami.test.drop\") \u0026\u0026 PathPrefix(\"/drop\")", "status": "enabled", "using": [ "web" @@ -66,7 +66,7 @@ "web" ], "service": "default-whoami-80", - "rule": "Host(`whoami.test.keep`) \u0026\u0026 PathPrefix(`/keep`)", + "rule": "Host(\"whoami.test.keep\") \u0026\u0026 PathPrefix(\"/keep\")", "status": "enabled", "using": [ "web" diff --git a/integration/testdata/rawdata-ingressclass.json b/integration/testdata/rawdata-ingressclass.json index 952bc95b2d..448a9af2cc 100644 --- a/integration/testdata/rawdata-ingressclass.json +++ b/integration/testdata/rawdata-ingressclass.json @@ -33,7 +33,7 @@ "web" ], "service": "default-whoami-80", - "rule": "Host(`whoami.test.keep`) \u0026\u0026 PathPrefix(`/keep`)", + "rule": "Host(\"whoami.test.keep\") \u0026\u0026 PathPrefix(\"/keep\")", "status": "enabled", "using": [ "web" diff --git a/pkg/provider/kubernetes/ingress/fixtures/Ingress-with-invalid-pathmatcher-annotation_endpoint.yml b/pkg/provider/kubernetes/ingress/fixtures/Ingress-with-invalid-pathmatcher-annotation_endpoint.yml new file mode 100644 index 0000000000..6ed60d79ce --- /dev/null +++ b/pkg/provider/kubernetes/ingress/fixtures/Ingress-with-invalid-pathmatcher-annotation_endpoint.yml @@ -0,0 +1,11 @@ +kind: Endpoints +apiVersion: v1 +metadata: + name: service1 + namespace: testing + +subsets: +- addresses: + - ip: 10.10.0.1 + ports: + - port: 8080 diff --git a/pkg/provider/kubernetes/ingress/fixtures/Ingress-with-invalid-pathmatcher-annotation_ingress.yml b/pkg/provider/kubernetes/ingress/fixtures/Ingress-with-invalid-pathmatcher-annotation_ingress.yml new file mode 100644 index 0000000000..2aac8eaf69 --- /dev/null +++ b/pkg/provider/kubernetes/ingress/fixtures/Ingress-with-invalid-pathmatcher-annotation_ingress.yml @@ -0,0 +1,18 @@ +kind: Ingress +apiVersion: networking.k8s.io/v1 +metadata: + name: "" + namespace: testing + annotations: + traefik.ingress.kubernetes.io/router.pathmatcher: 'Host("injection") || PathPrefix' +spec: + rules: + - http: + paths: + - path: /bar + pathType: ImplementationSpecific + backend: + service: + name: service1 + port: + number: 80 diff --git a/pkg/provider/kubernetes/ingress/fixtures/Ingress-with-invalid-pathmatcher-annotation_service.yml b/pkg/provider/kubernetes/ingress/fixtures/Ingress-with-invalid-pathmatcher-annotation_service.yml new file mode 100644 index 0000000000..0ec7e2269c --- /dev/null +++ b/pkg/provider/kubernetes/ingress/fixtures/Ingress-with-invalid-pathmatcher-annotation_service.yml @@ -0,0 +1,10 @@ +kind: Service +apiVersion: v1 +metadata: + name: service1 + namespace: testing + +spec: + ports: + - port: 80 + clusterIP: 10.0.0.1 diff --git a/pkg/provider/kubernetes/ingress/kubernetes.go b/pkg/provider/kubernetes/ingress/kubernetes.go index 42c7aae1d1..bf62999d9d 100644 --- a/pkg/provider/kubernetes/ingress/kubernetes.go +++ b/pkg/provider/kubernetes/ingress/kubernetes.go @@ -283,7 +283,7 @@ func (p *Provider) loadConfigurationFromIngresses(ctx context.Context, client Cl } rt := &dynamic.Router{ - Rule: "PathPrefix(`/`)", + Rule: `PathPrefix("/")`, Priority: math.MinInt32, Service: "default-backend", } @@ -349,7 +349,14 @@ func (p *Provider) loadConfigurationFromIngresses(ctx context.Context, client Cl serviceName := provider.Normalize(ingress.Namespace + "-" + pa.Backend.Service.Name + "-" + portString) conf.HTTP.Services[serviceName] = service - rt := loadRouter(rule, pa, rtConfig, serviceName) + rt, err := loadRouter(rule, pa, rtConfig, serviceName) + if err != nil { + log.FromContext(ctxIngress). + WithField("serviceName", pa.Backend.Service.Name). + WithField("path", pa.Path). + Errorf("Skipping path: %s", err) + continue + } p.applyRouterTransform(ctxIngress, rt, ingress) @@ -447,10 +454,10 @@ func (p *Provider) shouldProcessIngress(ingress *netv1.Ingress, ingressClasses [ func buildHostRule(host string) string { if strings.HasPrefix(host, "*.") { - return "HostRegexp(`" + strings.Replace(host, "*.", "{subdomain:[a-zA-Z0-9-]+}.", 1) + "`)" + return fmt.Sprintf("HostRegexp(%q)", strings.Replace(host, "*.", "{subdomain:[a-zA-Z0-9-]+}.", 1)) } - return "Host(`" + host + "`)" + return fmt.Sprintf("Host(%q)", host) } func getCertificates(ctx context.Context, ingress *netv1.Ingress, k8sClient Client, tlsConfigs map[string]*tls.CertAndStores) error { @@ -693,7 +700,7 @@ func makeRouterKeyWithHash(key, rule string) (string, error) { return dupKey, nil } -func loadRouter(rule netv1.IngressRule, pa netv1.HTTPIngressPath, rtConfig *RouterConfig, serviceName string) *dynamic.Router { +func loadRouter(rule netv1.IngressRule, pa netv1.HTTPIngressPath, rtConfig *RouterConfig, serviceName string) (*dynamic.Router, error) { var rules []string if len(rule.Host) > 0 { rules = []string{buildHostRule(rule.Host)} @@ -704,13 +711,19 @@ func loadRouter(rule netv1.IngressRule, pa netv1.HTTPIngressPath, rtConfig *Rout if pa.PathType == nil || *pa.PathType == "" || *pa.PathType == netv1.PathTypeImplementationSpecific { if rtConfig != nil && rtConfig.Router != nil && rtConfig.Router.PathMatcher != "" { + switch rtConfig.Router.PathMatcher { + case "Path", "PathPrefix": + default: + return nil, fmt.Errorf("invalid router path matcher %q: must be one of Path, PathPrefix", rtConfig.Router.PathMatcher) + } + matcher = rtConfig.Router.PathMatcher } } else if *pa.PathType == netv1.PathTypeExact { matcher = "Path" } - rules = append(rules, fmt.Sprintf("%s(`%s`)", matcher, pa.Path)) + rules = append(rules, fmt.Sprintf("%s(%q)", matcher, pa.Path)) } rt := &dynamic.Router{ @@ -728,7 +741,7 @@ func loadRouter(rule netv1.IngressRule, pa netv1.HTTPIngressPath, rtConfig *Rout } } - return rt + return rt, nil } func throttleEvents(ctx context.Context, throttleDuration time.Duration, pool *safe.Pool, eventsChan <-chan any) chan any { diff --git a/pkg/provider/kubernetes/ingress/kubernetes_test.go b/pkg/provider/kubernetes/ingress/kubernetes_test.go index c02b308b41..f7cf748d6f 100644 --- a/pkg/provider/kubernetes/ingress/kubernetes_test.go +++ b/pkg/provider/kubernetes/ingress/kubernetes_test.go @@ -60,7 +60,7 @@ func TestLoadConfigurationFromIngresses(t *testing.T) { Middlewares: map[string]*dynamic.Middleware{}, Routers: map[string]*dynamic.Router{ "testing-bar": { - Rule: "PathPrefix(`/bar`)", + Rule: `PathPrefix("/bar")`, Service: "testing-service1-80", }, }, @@ -90,7 +90,7 @@ func TestLoadConfigurationFromIngresses(t *testing.T) { Middlewares: map[string]*dynamic.Middleware{}, Routers: map[string]*dynamic.Router{ "testing-bar": { - Rule: "Path(`/bar`)", + Rule: `Path("/bar")`, EntryPoints: []string{"ep1", "ep2"}, Service: "testing-service1-80", Middlewares: []string{"md1", "md2"}, @@ -145,11 +145,11 @@ func TestLoadConfigurationFromIngresses(t *testing.T) { Middlewares: map[string]*dynamic.Middleware{}, Routers: map[string]*dynamic.Router{ "testing-bar": { - Rule: "PathPrefix(`/bar`)", + Rule: `PathPrefix("/bar")`, Service: "testing-service1-80", }, "testing-foo": { - Rule: "PathPrefix(`/foo`)", + Rule: `PathPrefix("/foo")`, Service: "testing-service1-80", }, }, @@ -178,12 +178,12 @@ func TestLoadConfigurationFromIngresses(t *testing.T) { HTTP: &dynamic.HTTPConfiguration{ Middlewares: map[string]*dynamic.Middleware{}, Routers: map[string]*dynamic.Router{ - "testing-bar-bar-3be6cfd7daba66cf2fdd": { - Rule: "HostRegexp(`{subdomain:[a-zA-Z0-9-]+}.bar`) && PathPrefix(`/bar`)", + "testing-bar-bar-19f852c6ac4fff6a1896": { + Rule: `HostRegexp("{subdomain:[a-zA-Z0-9-]+}.bar") && PathPrefix("/bar")`, Service: "testing-service1-80", }, - "testing-bar-bar-636bf36c00fedaab3d44": { - Rule: "Host(`bar`) && PathPrefix(`/bar`)", + "testing-bar-bar-605945111a3c9f84dc65": { + Rule: `Host("bar") && PathPrefix("/bar")`, Service: "testing-service1-80", }, }, @@ -212,12 +212,12 @@ func TestLoadConfigurationFromIngresses(t *testing.T) { HTTP: &dynamic.HTTPConfiguration{ Middlewares: map[string]*dynamic.Middleware{}, Routers: map[string]*dynamic.Router{ - "testing-foo-bar-d0b30949e54d6a7515ca": { - Rule: "PathPrefix(`/foo/bar`)", + "testing-foo-bar-207cc2245cb31ba18e29": { + Rule: `PathPrefix("/foo-bar")`, Service: "testing-service1-80", }, - "testing-foo-bar-dcd54bae39a6d7557f48": { - Rule: "PathPrefix(`/foo-bar`)", + "testing-foo-bar-930f0e8b221e60bc7ab7": { + Rule: `PathPrefix("/foo/bar")`, Service: "testing-service1-80", }, }, @@ -247,11 +247,11 @@ func TestLoadConfigurationFromIngresses(t *testing.T) { Middlewares: map[string]*dynamic.Middleware{}, Routers: map[string]*dynamic.Router{ "testing-bar": { - Rule: "PathPrefix(`/bar`)", + Rule: `PathPrefix("/bar")`, Service: "testing-service1-80", }, "testing-foo": { - Rule: "PathPrefix(`/foo`)", + Rule: `PathPrefix("/foo")`, Service: "testing-service1-80", }, }, @@ -281,7 +281,7 @@ func TestLoadConfigurationFromIngresses(t *testing.T) { Middlewares: map[string]*dynamic.Middleware{}, Routers: map[string]*dynamic.Router{ "testing-traefik-tchouk-bar": { - Rule: "Host(`traefik.tchouk`) && PathPrefix(`/bar`)", + Rule: `Host("traefik.tchouk") && PathPrefix("/bar")`, Service: "testing-service1-80", }, }, @@ -311,7 +311,7 @@ func TestLoadConfigurationFromIngresses(t *testing.T) { Middlewares: map[string]*dynamic.Middleware{}, Routers: map[string]*dynamic.Router{ "testing-example-com": { - Rule: "Host(`example.com`)", + Rule: `Host("example.com")`, Service: "testing-example-com-80", }, }, @@ -338,11 +338,11 @@ func TestLoadConfigurationFromIngresses(t *testing.T) { Middlewares: map[string]*dynamic.Middleware{}, Routers: map[string]*dynamic.Router{ "testing-traefik-tchouk-bar": { - Rule: "Host(`traefik.tchouk`) && PathPrefix(`/bar`)", + Rule: `Host("traefik.tchouk") && PathPrefix("/bar")`, Service: "testing-service1-80", }, "testing-traefik-tchouk-foo": { - Rule: "Host(`traefik.tchouk`) && PathPrefix(`/foo`)", + Rule: `Host("traefik.tchouk") && PathPrefix("/foo")`, Service: "testing-service1-80", }, }, @@ -372,11 +372,11 @@ func TestLoadConfigurationFromIngresses(t *testing.T) { Middlewares: map[string]*dynamic.Middleware{}, Routers: map[string]*dynamic.Router{ "testing-traefik-tchouk-bar": { - Rule: "Host(`traefik.tchouk`) && PathPrefix(`/bar`)", + Rule: `Host("traefik.tchouk") && PathPrefix("/bar")`, Service: "testing-service1-80", }, "testing-traefik-courgette-carotte": { - Rule: "Host(`traefik.courgette`) && PathPrefix(`/carotte`)", + Rule: `Host("traefik.courgette") && PathPrefix("/carotte")`, Service: "testing-service1-80", }, }, @@ -406,11 +406,11 @@ func TestLoadConfigurationFromIngresses(t *testing.T) { Middlewares: map[string]*dynamic.Middleware{}, Routers: map[string]*dynamic.Router{ "testing-traefik-tchouk-bar": { - Rule: "Host(`traefik.tchouk`) && PathPrefix(`/bar`)", + Rule: `Host("traefik.tchouk") && PathPrefix("/bar")`, Service: "testing-service1-80", }, "testing-traefik-courgette-carotte": { - Rule: "Host(`traefik.courgette`) && PathPrefix(`/carotte`)", + Rule: `Host("traefik.courgette") && PathPrefix("/carotte")`, Service: "testing-service2-8082", }, }, @@ -454,7 +454,7 @@ func TestLoadConfigurationFromIngresses(t *testing.T) { Middlewares: map[string]*dynamic.Middleware{}, Routers: map[string]*dynamic.Router{ "testing-traefik-tchouk-bar": { - Rule: "Host(`traefik.tchouk`) && PathPrefix(`/bar`)", + Rule: `Host("traefik.tchouk") && PathPrefix("/bar")`, Service: "testing-service1-80", }, }, @@ -511,7 +511,7 @@ func TestLoadConfigurationFromIngresses(t *testing.T) { Middlewares: map[string]*dynamic.Middleware{}, Routers: map[string]*dynamic.Router{ "default-router": { - Rule: "PathPrefix(`/`)", + Rule: `PathPrefix("/")`, Service: "default-backend", Priority: math.MinInt32, }, @@ -542,7 +542,7 @@ func TestLoadConfigurationFromIngresses(t *testing.T) { Middlewares: map[string]*dynamic.Middleware{}, Routers: map[string]*dynamic.Router{ "testing-traefik-tchouk-bar": { - Rule: "Host(`traefik.tchouk`) && PathPrefix(`/bar`)", + Rule: `Host("traefik.tchouk") && PathPrefix("/bar")`, Service: "testing-service1-80", }, }, @@ -572,7 +572,7 @@ func TestLoadConfigurationFromIngresses(t *testing.T) { Middlewares: map[string]*dynamic.Middleware{}, Routers: map[string]*dynamic.Router{ "testing-traefik-tchouk-bar": { - Rule: "Host(`traefik.tchouk`) && PathPrefix(`/bar`)", + Rule: `Host("traefik.tchouk") && PathPrefix("/bar")`, Service: "testing-service1-tchouk", }, }, @@ -602,7 +602,7 @@ func TestLoadConfigurationFromIngresses(t *testing.T) { Middlewares: map[string]*dynamic.Middleware{}, Routers: map[string]*dynamic.Router{ "testing-traefik-tchouk-bar": { - Rule: "Host(`traefik.tchouk`) && PathPrefix(`/bar`)", + Rule: `Host("traefik.tchouk") && PathPrefix("/bar")`, Service: "testing-service1-tchouk", }, }, @@ -632,11 +632,11 @@ func TestLoadConfigurationFromIngresses(t *testing.T) { Middlewares: map[string]*dynamic.Middleware{}, Routers: map[string]*dynamic.Router{ "testing-traefik-tchouk-bar": { - Rule: "Host(`traefik.tchouk`) && PathPrefix(`/bar`)", + Rule: `Host("traefik.tchouk") && PathPrefix("/bar")`, Service: "testing-service1-tchouk", }, "testing-traefik-tchouk-foo": { - Rule: "Host(`traefik.tchouk`) && PathPrefix(`/foo`)", + Rule: `Host("traefik.tchouk") && PathPrefix("/foo")`, Service: "testing-service1-carotte", }, }, @@ -679,7 +679,7 @@ func TestLoadConfigurationFromIngresses(t *testing.T) { Middlewares: map[string]*dynamic.Middleware{}, Routers: map[string]*dynamic.Router{ "testing-traefik-tchouk-bar": { - Rule: "Host(`traefik.tchouk`) && PathPrefix(`/bar`)", + Rule: `Host("traefik.tchouk") && PathPrefix("/bar")`, Service: "testing-service1-tchouk", }, }, @@ -709,11 +709,11 @@ func TestLoadConfigurationFromIngresses(t *testing.T) { Middlewares: map[string]*dynamic.Middleware{}, Routers: map[string]*dynamic.Router{ "testing-traefik-tchouk-bar": { - Rule: "Host(`traefik.tchouk`) && PathPrefix(`/bar`)", + Rule: `Host("traefik.tchouk") && PathPrefix("/bar")`, Service: "testing-service1-tchouk", }, "toto-toto-traefik-tchouk-bar": { - Rule: "Host(`toto.traefik.tchouk`) && PathPrefix(`/bar`)", + Rule: `Host("toto.traefik.tchouk") && PathPrefix("/bar")`, Service: "toto-service1-tchouk", }, }, @@ -778,7 +778,7 @@ func TestLoadConfigurationFromIngresses(t *testing.T) { Middlewares: map[string]*dynamic.Middleware{}, Routers: map[string]*dynamic.Router{ "testing-traefik-port-port": { - Rule: "Host(`traefik.port`) && PathPrefix(`/port`)", + Rule: `Host("traefik.port") && PathPrefix("/port")`, Service: "testing-service1-8080", }, }, @@ -805,7 +805,7 @@ func TestLoadConfigurationFromIngresses(t *testing.T) { Middlewares: map[string]*dynamic.Middleware{}, Routers: map[string]*dynamic.Router{ "testing-example-com": { - Rule: "Host(`example.com`)", + Rule: `Host("example.com")`, Service: "testing-example-com-80", TLS: &dynamic.RouterTLSConfig{}, }, @@ -843,7 +843,7 @@ func TestLoadConfigurationFromIngresses(t *testing.T) { Middlewares: map[string]*dynamic.Middleware{}, Routers: map[string]*dynamic.Router{ "testing-bar": { - Rule: "PathPrefix(`/bar`)", + Rule: `PathPrefix("/bar")`, Service: "testing-service1-443", }, }, @@ -873,7 +873,7 @@ func TestLoadConfigurationFromIngresses(t *testing.T) { Middlewares: map[string]*dynamic.Middleware{}, Routers: map[string]*dynamic.Router{ "testing-bar": { - Rule: "PathPrefix(`/bar`)", + Rule: `PathPrefix("/bar")`, Service: "testing-service1-8443", }, }, @@ -904,7 +904,7 @@ func TestLoadConfigurationFromIngresses(t *testing.T) { Routers: map[string]*dynamic.Router{ "testing-bar": { - Rule: "PathPrefix(`/bar`)", + Rule: `PathPrefix("/bar")`, Service: "testing-service1-8443", }, }, @@ -934,7 +934,7 @@ func TestLoadConfigurationFromIngresses(t *testing.T) { Middlewares: map[string]*dynamic.Middleware{}, Routers: map[string]*dynamic.Router{ "default-router": { - Rule: "PathPrefix(`/`)", + Rule: `PathPrefix("/")`, Service: "default-backend", Priority: math.MinInt32, }, @@ -965,7 +965,7 @@ func TestLoadConfigurationFromIngresses(t *testing.T) { Middlewares: map[string]*dynamic.Middleware{}, Routers: map[string]*dynamic.Router{ "testing-bar": { - Rule: "PathPrefix(`/bar`)", + Rule: `PathPrefix("/bar")`, Service: "testing-service1-80", }, }, @@ -1039,7 +1039,7 @@ func TestLoadConfigurationFromIngresses(t *testing.T) { Middlewares: map[string]*dynamic.Middleware{}, Routers: map[string]*dynamic.Router{ "testing-foobar-com-bar": { - Rule: "HostRegexp(`{subdomain:[a-zA-Z0-9-]+}.foobar.com`) && PathPrefix(`/bar`)", + Rule: `HostRegexp("{subdomain:[a-zA-Z0-9-]+}.foobar.com") && PathPrefix("/bar")`, Service: "testing-service1-80", }, }, @@ -1069,7 +1069,7 @@ func TestLoadConfigurationFromIngresses(t *testing.T) { Middlewares: map[string]*dynamic.Middleware{}, Routers: map[string]*dynamic.Router{ "testing-bar": { - Rule: "PathPrefix(`/bar`)", + Rule: `PathPrefix("/bar")`, Service: "testing-service1-80", }, }, @@ -1097,11 +1097,11 @@ func TestLoadConfigurationFromIngresses(t *testing.T) { Middlewares: map[string]*dynamic.Middleware{}, Routers: map[string]*dynamic.Router{ "testing-foo": { - Rule: "PathPrefix(`/foo`)", + Rule: `PathPrefix("/foo")`, Service: "testing-service1-80", }, "testing-bar": { - Rule: "PathPrefix(`/bar`)", + Rule: `PathPrefix("/bar")`, Service: "testing-service1-80", }, }, @@ -1129,7 +1129,7 @@ func TestLoadConfigurationFromIngresses(t *testing.T) { Middlewares: map[string]*dynamic.Middleware{}, Routers: map[string]*dynamic.Router{ "testing-bar": { - Rule: "Path(`/bar`)", + Rule: `Path("/bar")`, Service: "testing-service1-80", }, }, @@ -1157,7 +1157,7 @@ func TestLoadConfigurationFromIngresses(t *testing.T) { Middlewares: map[string]*dynamic.Middleware{}, Routers: map[string]*dynamic.Router{ "testing-bar": { - Rule: "Path(`/bar`)", + Rule: `Path("/bar")`, Service: "testing-service1-80", }, }, @@ -1185,7 +1185,7 @@ func TestLoadConfigurationFromIngresses(t *testing.T) { Middlewares: map[string]*dynamic.Middleware{}, Routers: map[string]*dynamic.Router{ "testing-bar": { - Rule: "Path(`/bar`)", + Rule: `Path("/bar")`, Service: "testing-service1-80", }, }, @@ -1213,7 +1213,7 @@ func TestLoadConfigurationFromIngresses(t *testing.T) { Middlewares: map[string]*dynamic.Middleware{}, Routers: map[string]*dynamic.Router{ "testing-bar": { - Rule: "PathPrefix(`/bar`)", + Rule: `PathPrefix("/bar")`, Service: "testing-service1-80", }, }, @@ -1241,7 +1241,7 @@ func TestLoadConfigurationFromIngresses(t *testing.T) { Middlewares: map[string]*dynamic.Middleware{}, Routers: map[string]*dynamic.Router{ "testing-bar": { - Rule: "Path(`/bar`)", + Rule: `Path("/bar")`, Service: "testing-service1-80", }, }, @@ -1281,7 +1281,7 @@ func TestLoadConfigurationFromIngresses(t *testing.T) { Middlewares: map[string]*dynamic.Middleware{}, Routers: map[string]*dynamic.Router{ "testing-bar": { - Rule: "PathPrefix(`/bar`)", + Rule: `PathPrefix("/bar")`, Service: "testing-service1-80", }, }, @@ -1310,7 +1310,7 @@ func TestLoadConfigurationFromIngresses(t *testing.T) { Middlewares: map[string]*dynamic.Middleware{}, Routers: map[string]*dynamic.Router{ "testing-foo": { - Rule: "PathPrefix(`/foo`)", + Rule: `PathPrefix("/foo")`, Service: "testing-service1-80", }, }, @@ -1338,7 +1338,7 @@ func TestLoadConfigurationFromIngresses(t *testing.T) { Middlewares: map[string]*dynamic.Middleware{}, Routers: map[string]*dynamic.Router{ "testing-bar": { - Rule: "PathPrefix(`/bar`)", + Rule: `PathPrefix("/bar")`, Service: "testing-service1-80", }, }, @@ -1366,7 +1366,7 @@ func TestLoadConfigurationFromIngresses(t *testing.T) { Middlewares: map[string]*dynamic.Middleware{}, Routers: map[string]*dynamic.Router{ "testing-bar": { - Rule: "Path(`/bar`)", + Rule: `Path("/bar")`, Service: "testing-service1-80", }, }, @@ -1394,7 +1394,7 @@ func TestLoadConfigurationFromIngresses(t *testing.T) { Middlewares: map[string]*dynamic.Middleware{}, Routers: map[string]*dynamic.Router{ "testing-bar": { - Rule: "Path(`/bar`)", + Rule: `Path("/bar")`, Service: "testing-service1-80", }, }, @@ -1422,7 +1422,7 @@ func TestLoadConfigurationFromIngresses(t *testing.T) { Middlewares: map[string]*dynamic.Middleware{}, Routers: map[string]*dynamic.Router{ "testing-bar": { - Rule: "Path(`/bar`)", + Rule: `Path("/bar")`, Service: "testing-service1-80", }, }, @@ -1450,7 +1450,7 @@ func TestLoadConfigurationFromIngresses(t *testing.T) { Middlewares: map[string]*dynamic.Middleware{}, Routers: map[string]*dynamic.Router{ "testing-bar": { - Rule: "Path(`/bar`)", + Rule: `Path("/bar")`, Service: "testing-service1-80", }, }, @@ -1478,7 +1478,7 @@ func TestLoadConfigurationFromIngresses(t *testing.T) { Middlewares: map[string]*dynamic.Middleware{}, Routers: map[string]*dynamic.Router{ "testing-bar": { - Rule: "PathPrefix(`/bar`)", + Rule: `PathPrefix("/bar")`, Service: "testing-service1-80", }, }, @@ -1506,7 +1506,7 @@ func TestLoadConfigurationFromIngresses(t *testing.T) { Middlewares: map[string]*dynamic.Middleware{}, Routers: map[string]*dynamic.Router{ "testing-bar": { - Rule: "PathPrefix(`/bar`)", + Rule: `PathPrefix("/bar")`, Service: "testing-service1-80", }, }, @@ -1534,7 +1534,7 @@ func TestLoadConfigurationFromIngresses(t *testing.T) { Middlewares: map[string]*dynamic.Middleware{}, Routers: map[string]*dynamic.Router{ "testing-bar": { - Rule: "PathPrefix(`/bar`)", + Rule: `PathPrefix("/bar")`, Service: "testing-service1-80", }, }, @@ -1562,7 +1562,7 @@ func TestLoadConfigurationFromIngresses(t *testing.T) { Middlewares: map[string]*dynamic.Middleware{}, Routers: map[string]*dynamic.Router{ "testing-bar": { - Rule: "PathPrefix(`/bar`)", + Rule: `PathPrefix("/bar")`, Service: "testing-service1-foobar", }, }, @@ -1602,7 +1602,7 @@ func TestLoadConfigurationFromIngresses(t *testing.T) { Middlewares: map[string]*dynamic.Middleware{}, Routers: map[string]*dynamic.Router{ "default-router": { - Rule: "PathPrefix(`/`)", + Rule: `PathPrefix("/")`, Priority: math.MinInt32, Service: "default-backend", }, @@ -1622,6 +1622,28 @@ func TestLoadConfigurationFromIngresses(t *testing.T) { }, }, }, + { + desc: "Ingress with invalid pathmatcher annotation", + expected: &dynamic.Configuration{ + TCP: &dynamic.TCPConfiguration{}, + HTTP: &dynamic.HTTPConfiguration{ + Middlewares: map[string]*dynamic.Middleware{}, + Routers: map[string]*dynamic.Router{}, + Services: map[string]*dynamic.Service{ + "testing-service1-80": { + LoadBalancer: &dynamic.ServersLoadBalancer{ + PassHostHeader: pointer(true), + Servers: []dynamic.Server{ + { + URL: "http://10.10.0.1:8080", + }, + }, + }, + }, + }, + }, + }, + }, } for _, test := range testCases { @@ -1692,7 +1714,7 @@ func TestLoadConfigurationFromIngressesWithExternalNameServices(t *testing.T) { Middlewares: map[string]*dynamic.Middleware{}, Routers: map[string]*dynamic.Router{ "testing-traefik-tchouk-bar": { - Rule: "Host(`traefik.tchouk`) && PathPrefix(`/bar`)", + Rule: `Host("traefik.tchouk") && PathPrefix("/bar")`, Service: "testing-service1-8080", }, }, @@ -1719,7 +1741,7 @@ func TestLoadConfigurationFromIngressesWithExternalNameServices(t *testing.T) { Middlewares: map[string]*dynamic.Middleware{}, Routers: map[string]*dynamic.Router{ "testing-example-com-bar": { - Rule: "PathPrefix(`/bar`)", + Rule: `PathPrefix("/bar")`, Service: "testing-service-bar-8080", }, }, @@ -1747,7 +1769,7 @@ func TestLoadConfigurationFromIngressesWithExternalNameServices(t *testing.T) { Middlewares: map[string]*dynamic.Middleware{}, Routers: map[string]*dynamic.Router{ "testing-example-com-foo": { - Rule: "PathPrefix(`/foo`)", + Rule: `PathPrefix("/foo")`, Service: "testing-service-foo-8080", }, }, @@ -1825,7 +1847,7 @@ func TestLoadConfigurationFromIngressesWithNativeLB(t *testing.T) { Middlewares: map[string]*dynamic.Middleware{}, Routers: map[string]*dynamic.Router{ "testing-traefik-tchouk-bar": { - Rule: "Host(`traefik.tchouk`) && PathPrefix(`/bar`)", + Rule: `Host("traefik.tchouk") && PathPrefix("/bar")`, Service: "testing-service1-8080", }, }, From 5026ca97d0266e2ad260947cfb22c3f5453fa673 Mon Sep 17 00:00:00 2001 From: Julien Salleyron Date: Thu, 28 May 2026 10:30:07 +0200 Subject: [PATCH 3/5] Move snicheck to ctx instead of simulated routing --- .../fixtures/https/https_domain_fronting.toml | 47 ++- integration/https_test.go | 61 ++-- integration/simple_test.go | 2 +- integration/try/try.go | 6 +- pkg/config/dynamic/http_config.go | 7 +- pkg/middlewares/snicheck/snicheck.go | 101 ++---- pkg/middlewares/snicheck/snicheck_test.go | 59 ---- pkg/muxer/tcp/mux.go | 6 +- pkg/muxer/tcp/mux_test.go | 2 +- pkg/server/aggregator.go | 96 +++++- pkg/server/router/router.go | 7 + pkg/server/router/tcp/manager.go | 107 ++----- pkg/server/router/tcp/manager_test.go | 300 +----------------- pkg/server/router/tcp/router.go | 51 ++- pkg/server/router/tcp/router_test.go | 21 +- pkg/server/server_entrypoint_tcp.go | 10 + pkg/server/server_entrypoint_tcp_http3.go | 43 ++- .../server_entrypoint_tcp_http3_test.go | 4 +- pkg/tcp/tls.go | 29 +- 19 files changed, 362 insertions(+), 597 deletions(-) delete mode 100644 pkg/middlewares/snicheck/snicheck_test.go diff --git a/integration/fixtures/https/https_domain_fronting.toml b/integration/fixtures/https/https_domain_fronting.toml index 80011ee8ed..7d34884f84 100644 --- a/integration/fixtures/https/https_domain_fronting.toml +++ b/integration/fixtures/https/https_domain_fronting.toml @@ -7,6 +7,10 @@ [entryPoints.websecure] address = ":4443" + [entryPoints.websecure.http3] + +[experimental] + http3 = true [api] insecure = true @@ -32,6 +36,35 @@ [http.routers.router3.tls] options = "mytls" +[http.routers.router4] + rule = "Host(`site4.www.snitest.com`)" + service = "service4" + [http.routers.router4.tls] + +[http.routers.router4path] + rule = "Host(`site4.www.snitest.com`) && PathPrefix(`/foo`)" + service = "service4" + [http.routers.router4path.tls] + options = "mytls" + +[http.routers.router5] + rule = "Host(`site5.www.snitest.com`)" + service = "service5" + [http.routers.router5.tls] + options = "mytls" + +[http.routers.router5path] + rule = "Host(`site5.www.snitest.com`) && PathPrefix(`/bar`)" + service = "service5" + [http.routers.router5path.tls] + options = "mytls" + +[http.routers.router6] + rule = "Host(`site6.www.snitest.com.`)" + service = "service6" + [http.routers.router6.tls] + options = "mytls" + [http.services.service1] [[http.services.service1.loadBalancer.servers]] url = "http://127.0.0.1:9010" @@ -44,10 +77,22 @@ [[http.services.service3.loadBalancer.servers]] url = "http://127.0.0.1:9030" +[http.services.service4] + [[http.services.service4.loadBalancer.servers]] + url = "http://127.0.0.1:9040" + +[http.services.service5] + [[http.services.service5.loadBalancer.servers]] + url = "http://127.0.0.1:9050" + +[http.services.service6] + [[http.services.service6.loadBalancer.servers]] + url = "http://127.0.0.1:9060" + [[tls.certificates]] certFile = "fixtures/https/wildcard.www.snitest.com.cert" keyFile = "fixtures/https/wildcard.www.snitest.com.key" [tls.options] [tls.options.mytls] - maxVersion = "VersionTLS12" + maxVersion = "VersionTLS13" diff --git a/integration/https_test.go b/integration/https_test.go index a0ab2c1bc3..8bdf5c1c45 100644 --- a/integration/https_test.go +++ b/integration/https_test.go @@ -13,6 +13,7 @@ import ( "time" "github.com/BurntSushi/toml" + "github.com/quic-go/quic-go/http3" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/stretchr/testify/suite" @@ -254,7 +255,7 @@ func (s *HTTPSSuite) TestWithConflictingTLSOptions() { assert.ErrorContains(s.T(), err, "tls: no supported versions satisfy MinVersion and MaxVersion") // with unknown tls option - err = try.GetRequest("http://127.0.0.1:8080/api/rawdata", 1*time.Second, try.BodyContains(fmt.Sprintf("found different TLS options for routers on the same host %v, so using the default TLS options instead", tr4.TLSClientConfig.ServerName))) + err = try.GetRequest("http://127.0.0.1:8080/api/rawdata", 1*time.Second, try.BodyContains("found different TLS options for routers on the same host, so using the default TLS options instead")) require.NoError(s.T(), err) } @@ -995,19 +996,20 @@ func (s *HTTPSSuite) TestWithDomainFronting() { defer backend2.Close() backend3 := startTestServer("9030", http.StatusOK, "server3") defer backend3.Close() + backend5 := startTestServer("9050", http.StatusOK, "server5") + defer backend5.Close() file := s.adaptFile("fixtures/https/https_domain_fronting.toml", struct{}{}) s.traefikCmd(withConfigFile(file)) // wait for Traefik - err := try.GetRequest("http://127.0.0.1:8080/api/rawdata", 500*time.Millisecond, try.BodyContains("Host(`site1.www.snitest.com`)")) + err := try.GetRequest("http://127.0.0.1:8080/api/rawdata", 1000*time.Millisecond, try.BodyContains("Host(`site1.www.snitest.com`)")) require.NoError(s.T(), err) testCases := []struct { desc string hostHeader string serverName string - expectedError bool expectedContent string expectedStatusCode int }{ @@ -1025,14 +1027,6 @@ func (s *HTTPSSuite) TestWithDomainFronting() { expectedContent: "server3", expectedStatusCode: http.StatusOK, }, - { - desc: "Spaces after the host header", - hostHeader: "site3.www.snitest.com ", - serverName: "site3.www.snitest.com", - expectedError: true, - expectedContent: "server3", - expectedStatusCode: http.StatusOK, - }, { desc: "Spaces after the servername", hostHeader: "site3.www.snitest.com", @@ -1040,14 +1034,6 @@ func (s *HTTPSSuite) TestWithDomainFronting() { expectedContent: "server3", expectedStatusCode: http.StatusOK, }, - { - desc: "Spaces after the servername and host header", - hostHeader: "site3.www.snitest.com ", - serverName: "site3.www.snitest.com ", - expectedError: true, - expectedContent: "server3", - expectedStatusCode: http.StatusOK, - }, { desc: "Domain Fronting with same tlsOptions should follow header", hostHeader: "site1.www.snitest.com", @@ -1083,6 +1069,34 @@ func (s *HTTPSSuite) TestWithDomainFronting() { expectedContent: "server1", expectedStatusCode: http.StatusOK, }, + { + desc: "Domain Fronting with ambiguous TLS options should produce a 421", + hostHeader: "site4.www.snitest.com", + serverName: "site3.www.snitest.com", + expectedContent: "", + expectedStatusCode: http.StatusMisdirectedRequest, + }, + { + desc: "Domain Fronting with same non-default TLS options should not produce a 421", + hostHeader: "site5.www.snitest.com", + serverName: "site3.www.snitest.com", + expectedContent: "server5", + expectedStatusCode: http.StatusOK, + }, + { + desc: "FQDN host header with empty SNI to non-default TLS options route should produce a 421", + hostHeader: "site3.www.snitest.com.", + serverName: "", + expectedContent: "", + expectedStatusCode: http.StatusMisdirectedRequest, + }, + { + desc: "Non-FQDN host header with empty SNI matching FQDN route rule should produce a 421", + hostHeader: "site6.www.snitest.com", + serverName: "", + expectedContent: "", + expectedStatusCode: http.StatusMisdirectedRequest, + }, } for _, test := range testCases { @@ -1091,11 +1105,10 @@ func (s *HTTPSSuite) TestWithDomainFronting() { req.Host = test.hostHeader err = try.RequestWithTransport(req, 500*time.Millisecond, &http.Transport{TLSClientConfig: &tls.Config{InsecureSkipVerify: true, ServerName: test.serverName}}, try.StatusCodeIs(test.expectedStatusCode), try.BodyContains(test.expectedContent)) - if test.expectedError { - assert.Error(s.T(), err) - } else { - require.NoError(s.T(), err) - } + assert.NoError(s.T(), err, "test %s failed with: %v", test.desc, err) + + err = try.RequestWithTransport(req, 500*time.Millisecond, &http3.Transport{TLSClientConfig: &tls.Config{InsecureSkipVerify: true, ServerName: test.serverName}}, try.StatusCodeIs(test.expectedStatusCode), try.BodyContains(test.expectedContent)) + assert.NoError(s.T(), err, "test %s failed with: %v", test.desc, err) } } diff --git a/integration/simple_test.go b/integration/simple_test.go index e8938c8943..40620f8f9d 100644 --- a/integration/simple_test.go +++ b/integration/simple_test.go @@ -667,7 +667,7 @@ func (s *SimpleSuite) TestRouterConfigErrors() { s.traefikCmd(withConfigFile(file)) // All errors - err := try.GetRequest("http://127.0.0.1:8080/api/http/routers", 1000*time.Millisecond, try.BodyContains(`["middleware \"unknown@file\" does not exist","found different TLS options for routers on the same host snitest.net, so using the default TLS options instead"]`)) + err := try.GetRequest("http://127.0.0.1:8080/api/http/routers", 1000*time.Millisecond, try.BodyContains(`["middleware \"unknown@file\" does not exist","found different TLS options for routers on the same host, so using the default TLS options instead"]`)) require.NoError(s.T(), err) // router3 has an error because it uses an unknown entrypoint diff --git a/integration/try/try.go b/integration/try/try.go index 20bc8bc1ac..73a3decf71 100644 --- a/integration/try/try.go +++ b/integration/try/try.go @@ -76,7 +76,7 @@ func Request(req *http.Request, timeout time.Duration, conditions ...ResponseCon // the condition on the response. // ResponseCondition may be nil, in which case only the request against the URL must // succeed. -func RequestWithTransport(req *http.Request, timeout time.Duration, transport *http.Transport, conditions ...ResponseCondition) error { +func RequestWithTransport(req *http.Request, timeout time.Duration, transport http.RoundTripper, conditions ...ResponseCondition) error { resp, err := doTryRequest(req, timeout, transport, conditions...) if resp != nil && resp.Body != nil { @@ -140,12 +140,12 @@ func doTryRequest(request *http.Request, timeout time.Duration, transport http.R func doRequest(action timedAction, timeout time.Duration, request *http.Request, transport http.RoundTripper, conditions ...ResponseCondition) (*http.Response, error) { var resp *http.Response return resp, action(timeout, func() error { - var err error - client := http.DefaultClient + var client http.Client if transport != nil { client.Transport = transport } + var err error resp, err = client.Do(request) if err != nil { return err diff --git a/pkg/config/dynamic/http_config.go b/pkg/config/dynamic/http_config.go index d397d23069..7d68c18dce 100644 --- a/pkg/config/dynamic/http_config.go +++ b/pkg/config/dynamic/http_config.go @@ -103,9 +103,10 @@ func (r *RouterDeniedEncodedPathCharacters) Map() map[string]struct{} { // RouterTLSConfig holds the TLS configuration for a router. type RouterTLSConfig struct { - Options string `json:"options,omitempty" toml:"options,omitempty" yaml:"options,omitempty" export:"true"` - CertResolver string `json:"certResolver,omitempty" toml:"certResolver,omitempty" yaml:"certResolver,omitempty" export:"true"` - Domains []types.Domain `json:"domains,omitempty" toml:"domains,omitempty" yaml:"domains,omitempty" export:"true"` + Options string `json:"options,omitempty" toml:"options,omitempty" yaml:"options,omitempty" export:"true"` + ResolvedOptions string `json:"-" toml:"-" yaml:"-" label:"-" file:"-" kv:"-" export:"false"` + CertResolver string `json:"certResolver,omitempty" toml:"certResolver,omitempty" yaml:"certResolver,omitempty" export:"true"` + Domains []types.Domain `json:"domains,omitempty" toml:"domains,omitempty" yaml:"domains,omitempty" export:"true"` } // +k8s:deepcopy-gen=true diff --git a/pkg/middlewares/snicheck/snicheck.go b/pkg/middlewares/snicheck/snicheck.go index e18b605cbf..d2f7015472 100644 --- a/pkg/middlewares/snicheck/snicheck.go +++ b/pkg/middlewares/snicheck/snicheck.go @@ -1,24 +1,26 @@ package snicheck import ( - "net" "net/http" - "strings" "github.com/traefik/traefik/v2/pkg/log" - "github.com/traefik/traefik/v2/pkg/middlewares/requestdecorator" - traefiktls "github.com/traefik/traefik/v2/pkg/tls" + "github.com/traefik/traefik/v2/pkg/tcp" ) // SNICheck is an HTTP handler that checks whether the TLS configuration for the server name is the same as for the host header. type SNICheck struct { - next http.Handler - tlsOptionsForHost map[string]string + next http.Handler + routerName string + tlsOptionsName string } // New creates a new SNICheck. -func New(tlsOptionsForHost map[string]string, next http.Handler) *SNICheck { - return &SNICheck{next: next, tlsOptionsForHost: tlsOptionsForHost} +func New(routerName, tlsOptionsName string, next http.Handler) *SNICheck { + return &SNICheck{ + next: next, + routerName: routerName, + tlsOptionsName: tlsOptionsName, + } } func (s SNICheck) ServeHTTP(rw http.ResponseWriter, req *http.Request) { @@ -27,81 +29,16 @@ func (s SNICheck) ServeHTTP(rw http.ResponseWriter, req *http.Request) { return } - host := getHost(req) - serverName := strings.TrimSpace(req.TLS.ServerName) - - // Domain Fronting - if !strings.EqualFold(host, serverName) { - tlsOptionHeader := findTLSOptionName(s.tlsOptionsForHost, host, true) - tlsOptionSNI := findTLSOptionName(s.tlsOptionsForHost, serverName, false) - - if tlsOptionHeader != tlsOptionSNI { - log.WithoutContext(). - WithField("host", host). - WithField("req.Host", req.Host). - WithField("req.TLS.ServerName", req.TLS.ServerName). - Debugf("TLS options difference: SNI:%s, Header:%s", tlsOptionSNI, tlsOptionHeader) - http.Error(rw, http.StatusText(http.StatusMisdirectedRequest), http.StatusMisdirectedRequest) - return - } + tlsOptionsNameUsed := tcp.GetTLSOptionsName(req.Context()) + if s.tlsOptionsName != tlsOptionsNameUsed { + log.WithoutContext(). + WithField("routerName", s.routerName). + WithField("req.Host", req.Host). + WithField("req.TLS.ServerName", req.TLS.ServerName). + Debugf("TLS options difference: SNI:%s, Header:%s", tlsOptionsNameUsed, s.tlsOptionsName) + http.Error(rw, http.StatusText(http.StatusMisdirectedRequest), http.StatusMisdirectedRequest) + return } s.next.ServeHTTP(rw, req) } - -func getHost(req *http.Request) string { - h := requestdecorator.GetCNAMEFlatten(req.Context()) - if h != "" { - return h - } - - h = requestdecorator.GetCanonizedHost(req.Context()) - if h != "" { - return h - } - - host, _, err := net.SplitHostPort(req.Host) - if err != nil { - host = req.Host - } - - return strings.TrimSpace(host) -} - -func findTLSOptionName(tlsOptionsForHost map[string]string, host string, fqdn bool) string { - name := findTLSOptName(tlsOptionsForHost, host, fqdn) - if name != "" { - return name - } - - name = findTLSOptName(tlsOptionsForHost, strings.ToLower(host), fqdn) - if name != "" { - return name - } - - return traefiktls.DefaultTLSConfigName -} - -func findTLSOptName(tlsOptionsForHost map[string]string, host string, fqdn bool) string { - if tlsOptions, ok := tlsOptionsForHost[host]; ok { - return tlsOptions - } - - if !fqdn { - return "" - } - - if last := len(host) - 1; last >= 0 && host[last] == '.' { - if tlsOptions, ok := tlsOptionsForHost[host[:last]]; ok { - return tlsOptions - } - - return "" - } - - if tlsOptions, ok := tlsOptionsForHost[host+"."]; ok { - return tlsOptions - } - - return "" -} diff --git a/pkg/middlewares/snicheck/snicheck_test.go b/pkg/middlewares/snicheck/snicheck_test.go deleted file mode 100644 index d7411e555e..0000000000 --- a/pkg/middlewares/snicheck/snicheck_test.go +++ /dev/null @@ -1,59 +0,0 @@ -package snicheck - -import ( - "net/http" - "net/http/httptest" - "testing" - - "github.com/stretchr/testify/assert" -) - -func TestSNICheck_ServeHTTP(t *testing.T) { - testCases := []struct { - desc string - tlsOptionsForHost map[string]string - host string - expected int - }{ - { - desc: "no TLS options", - expected: http.StatusOK, - }, - { - desc: "with TLS options", - tlsOptionsForHost: map[string]string{ - "example.com": "foo", - }, - expected: http.StatusOK, - }, - { - desc: "server name and host doesn't have the same TLS configuration", - tlsOptionsForHost: map[string]string{ - "example.com": "foo", - }, - host: "example.com", - expected: http.StatusMisdirectedRequest, - }, - } - - for _, test := range testCases { - t.Run(test.desc, func(t *testing.T) { - t.Parallel() - - next := http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) {}) - - sniCheck := New(test.tlsOptionsForHost, next) - - req := httptest.NewRequest(http.MethodGet, "https://localhost", nil) - if test.host != "" { - req.Host = test.host - } - - recorder := httptest.NewRecorder() - - sniCheck.ServeHTTP(recorder, req) - - assert.Equal(t, test.expected, recorder.Code) - }) - } -} diff --git a/pkg/muxer/tcp/mux.go b/pkg/muxer/tcp/mux.go index 8302ab3408..2a4fa057cb 100644 --- a/pkg/muxer/tcp/mux.go +++ b/pkg/muxer/tcp/mux.go @@ -61,10 +61,10 @@ type ConnData struct { } // NewConnData builds a connData struct from the given parameters. -func NewConnData(serverName string, conn tcp.WriteCloser, alpnProtos []string) (ConnData, error) { - remoteIP, _, err := net.SplitHostPort(conn.RemoteAddr().String()) +func NewConnData(serverName string, remoteAddr net.Addr, alpnProtos []string) (ConnData, error) { + remoteIP, _, err := net.SplitHostPort(remoteAddr.String()) if err != nil { - return ConnData{}, fmt.Errorf("error while parsing remote address %q: %w", conn.RemoteAddr().String(), err) + return ConnData{}, fmt.Errorf("parsing remote address %q: %w", remoteAddr.String(), err) } // as per https://datatracker.ietf.org/doc/html/rfc6066: diff --git a/pkg/muxer/tcp/mux_test.go b/pkg/muxer/tcp/mux_test.go index e82432a75b..7ac0d70b5c 100644 --- a/pkg/muxer/tcp/mux_test.go +++ b/pkg/muxer/tcp/mux_test.go @@ -532,7 +532,7 @@ func Test_addTCPRoute(t *testing.T) { remoteAddr: fakeAddr{addr: addr}, } - connData, err := NewConnData(test.serverName, conn, test.protos) + connData, err := NewConnData(test.serverName, conn.RemoteAddr(), test.protos) require.NoError(t, err) matchingHandler, _ := router.Match(connData) diff --git a/pkg/server/aggregator.go b/pkg/server/aggregator.go index 28d07dda9a..2dcabdae2b 100644 --- a/pkg/server/aggregator.go +++ b/pkg/server/aggregator.go @@ -1,13 +1,16 @@ package server import ( + "context" + "fmt" "slices" "github.com/go-acme/lego/v4/challenge/tlsalpn01" "github.com/traefik/traefik/v2/pkg/config/dynamic" "github.com/traefik/traefik/v2/pkg/log" + httpmuxer "github.com/traefik/traefik/v2/pkg/muxer/http" "github.com/traefik/traefik/v2/pkg/server/provider" - "github.com/traefik/traefik/v2/pkg/tls" + traefiktls "github.com/traefik/traefik/v2/pkg/tls" ) func mergeConfiguration(configurations dynamic.Configurations, defaultEntryPoints []string) dynamic.Configuration { @@ -31,8 +34,8 @@ func mergeConfiguration(configurations dynamic.Configurations, defaultEntryPoint Services: make(map[string]*dynamic.UDPService), }, TLS: &dynamic.TLSConfiguration{ - Stores: make(map[string]tls.Store), - Options: make(map[string]tls.Options), + Stores: make(map[string]traefiktls.Store), + Options: make(map[string]traefiktls.Options), }, } @@ -101,7 +104,7 @@ func mergeConfiguration(configurations dynamic.Configurations, defaultEntryPoint } for key, store := range configuration.TLS.Stores { - if key != tls.DefaultTLSStoreName { + if key != traefiktls.DefaultTLSStoreName { key = provider.MakeQualifiedName(pvd, key) } else { defaultTLSStoreProviders = append(defaultTLSStoreProviders, pvd) @@ -123,19 +126,96 @@ func mergeConfiguration(configurations dynamic.Configurations, defaultEntryPoint if len(defaultTLSStoreProviders) > 1 { log.WithoutContext().Errorf("Default TLS Store defined in multiple providers: %v", defaultTLSStoreProviders) - delete(conf.TLS.Stores, tls.DefaultTLSStoreName) + delete(conf.TLS.Stores, traefiktls.DefaultTLSStoreName) } if len(defaultTLSOptionProviders) == 0 { - conf.TLS.Options[tls.DefaultTLSConfigName] = tls.DefaultTLSOptions + conf.TLS.Options[traefiktls.DefaultTLSConfigName] = traefiktls.DefaultTLSOptions } else if len(defaultTLSOptionProviders) > 1 { log.WithoutContext().Errorf("Default TLS Options defined in multiple providers %v", defaultTLSOptionProviders) // We do not set an empty tls.TLS{} as above so that we actually get a "cascading failure" later on, // i.e. routers depending on this missing TLS option will fail to initialize as well. - delete(conf.TLS.Options, tls.DefaultTLSConfigName) + delete(conf.TLS.Options, traefiktls.DefaultTLSConfigName) } - return conf + return resolveHTTPTLSOptions(conf) +} + +func resolveHTTPTLSOptions(cfg dynamic.Configuration) dynamic.Configuration { + if cfg.HTTP == nil || len(cfg.HTTP.Routers) == 0 { + return cfg + } + + rts := make(map[string]*dynamic.Router) + + // Keyed by domain, then by options reference. + // The actual source of truth for what TLS options will actually be used for the connection. + // As opposed to tlsOptionsForHost, it keeps track of all the (different) TLS + // options that occur for a given host name, so that later on we can set relevant + // errors and logging for all the routers concerned (i.e. wrongly configured). + tlsOptionsForHostSNI := map[string]map[string][]string{} + + for routerHTTPName, routerHTTPConfig := range cfg.HTTP.Routers { + rts[routerHTTPName] = routerHTTPConfig.DeepCopy() + + if routerHTTPConfig.TLS == nil { + continue + } + + ctxRouter := log.With(provider.AddInContext(context.Background(), routerHTTPName), log.Str(log.RouterName, routerHTTPName)) + logger := log.FromContext(ctxRouter) + + tlsOptionsName := traefiktls.DefaultTLSConfigName + if len(routerHTTPConfig.TLS.Options) > 0 && routerHTTPConfig.TLS.Options != traefiktls.DefaultTLSConfigName { + tlsOptionsName = provider.GetQualifiedName(ctxRouter, routerHTTPConfig.TLS.Options) + } + + domains, err := httpmuxer.ParseDomains(routerHTTPConfig.Rule) + if err != nil { + routerErr := fmt.Errorf("invalid rule %s, error: %w", routerHTTPConfig.Rule, err) + logger.Error(routerErr) + continue + } + + if len(domains) == 0 { + rts[routerHTTPName].TLS.ResolvedOptions = "default" + logger.Warnf("No domain found in rule %v, the TLS options applied for this router will depend on the SNI of each request", routerHTTPConfig.Rule) + } + + for _, domain := range domains { + // domain is already in lower case thanks to the domain parsing + if tlsOptionsForHostSNI[domain] == nil { + tlsOptionsForHostSNI[domain] = make(map[string][]string) + } + tlsOptionsForHostSNI[domain][tlsOptionsName] = append(tlsOptionsForHostSNI[domain][tlsOptionsName], routerHTTPName) + } + } + + for hostSNI, tlsConfigs := range tlsOptionsForHostSNI { + if len(tlsConfigs) == 1 { + for optionsName, v := range tlsConfigs { + log.WithoutContext().Debugf("Adding route for %s with TLS options %s", hostSNI, optionsName) + for _, s := range v { + rts[s].TLS.ResolvedOptions = optionsName + } + } + continue + } + + // multiple tlsConfigs + routers := make([]string, 0, len(tlsConfigs)) + for _, v := range tlsConfigs { + for _, s := range v { + rts[s].TLS.ResolvedOptions = traefiktls.DefaultTLSConfigName + routers = append(routers, s) + } + } + + log.WithoutContext().Warnf("Found different TLS options for routers on the same host %v, so using the default TLS options instead for these routers: %#v", hostSNI, routers) + } + + cfg.HTTP.Routers = rts + return cfg } func applyModel(cfg dynamic.Configuration) dynamic.Configuration { diff --git a/pkg/server/router/router.go b/pkg/server/router/router.go index 5fc2aa2b23..b6192eaee4 100644 --- a/pkg/server/router/router.go +++ b/pkg/server/router/router.go @@ -16,6 +16,7 @@ import ( "github.com/traefik/traefik/v2/pkg/middlewares/denyrouterrecursion" metricsMiddle "github.com/traefik/traefik/v2/pkg/middlewares/metrics" "github.com/traefik/traefik/v2/pkg/middlewares/recovery" + "github.com/traefik/traefik/v2/pkg/middlewares/snicheck" "github.com/traefik/traefik/v2/pkg/middlewares/tracing" httpmuxer "github.com/traefik/traefik/v2/pkg/muxer/http" "github.com/traefik/traefik/v2/pkg/server/middleware" @@ -229,6 +230,12 @@ func (m *Manager) buildHTTPHandler(ctx context.Context, router *runtime.RouterIn }) } + if router.TLS != nil { + chain = chain.Append(func(next http.Handler) (http.Handler, error) { + return snicheck.New(routerName, router.TLS.ResolvedOptions, next), nil + }) + } + return chain.Extend(*mHandler).Append(tHandler).Then(sHandler) } diff --git a/pkg/server/router/tcp/manager.go b/pkg/server/router/tcp/manager.go index de4d94d2ec..0a17dfb272 100644 --- a/pkg/server/router/tcp/manager.go +++ b/pkg/server/router/tcp/manager.go @@ -2,7 +2,6 @@ package tcp import ( "context" - "crypto/tls" "errors" "fmt" "math" @@ -11,7 +10,6 @@ import ( "github.com/traefik/traefik/v2/pkg/config/runtime" "github.com/traefik/traefik/v2/pkg/log" - "github.com/traefik/traefik/v2/pkg/middlewares/snicheck" httpmuxer "github.com/traefik/traefik/v2/pkg/muxer/http" tcpmuxer "github.com/traefik/traefik/v2/pkg/muxer/tcp" "github.com/traefik/traefik/v2/pkg/server/provider" @@ -91,11 +89,6 @@ func (m *Manager) getHTTPRouters(ctx context.Context, entryPoints []string, tls return make(map[string]map[string]*runtime.RouterInfo) } -type nameAndConfig struct { - routerName string // just so we have it as additional information when logging - TLSConfig *tls.Config -} - func (m *Manager) buildEntryPointHandler(ctx context.Context, configs map[string]*runtime.TCPRouterInfo, configsHTTP map[string]*runtime.RouterInfo, handlerHTTP, handlerHTTPS http.Handler) (*Router, error) { // Build a new Router. router, err := NewRouter() @@ -113,18 +106,6 @@ func (m *Manager) buildEntryPointHandler(ctx context.Context, configs map[string log.FromContext(ctx).Errorf("Error during the build of the default TLS configuration: %v", err) } - // Keyed by domain. The source of truth for doing SNI checking (domain fronting). - // As soon as there's (at least) two different tlsOptions found for the same domain, - // we set the value to the default TLS conf. - tlsOptionsForHost := map[string]string{} - - // Keyed by domain, then by options reference. - // The actual source of truth for what TLS options will actually be used for the connection. - // As opposed to tlsOptionsForHost, it keeps track of all the (different) TLS - // options that occur for a given host name, so that later on we can set relevant - // errors and logging for all the routers concerned (i.e. wrongly configured). - tlsOptionsForHostSNI := map[string]map[string]nameAndConfig{} - for routerHTTPName, routerHTTPConfig := range configsHTTP { if routerHTTPConfig.TLS == nil { continue @@ -133,11 +114,6 @@ func (m *Manager) buildEntryPointHandler(ctx context.Context, configs map[string ctxRouter := log.With(provider.AddInContext(ctx, routerHTTPName), log.Str(log.RouterName, routerHTTPName)) logger := log.FromContext(ctxRouter) - tlsOptionsName := traefiktls.DefaultTLSConfigName - if len(routerHTTPConfig.TLS.Options) > 0 && routerHTTPConfig.TLS.Options != traefiktls.DefaultTLSConfigName { - tlsOptionsName = provider.GetQualifiedName(ctxRouter, routerHTTPConfig.TLS.Options) - } - domains, err := httpmuxer.ParseDomains(routerHTTPConfig.Rule) if err != nil { routerErr := fmt.Errorf("invalid rule %s, error: %w", routerHTTPConfig.Rule, err) @@ -152,7 +128,7 @@ func (m *Manager) buildEntryPointHandler(ctx context.Context, configs map[string // This is only about choosing the TLS configuration. // The actual routing will be done further on by the HTTPS handler. // See examples below. - router.AddHTTPTLSConfig("*", defaultTLSConf) + router.AddHTTPTLSConfig("*", defaultTLSConf, traefiktls.DefaultTLSConfigName) // The server name (from a Host(SNI) rule) is the only parameter (available in HTTP routing rules) on which we can map a TLS config, // because it is the only one accessible before decryption (we obtain it during the ClientHello). @@ -180,79 +156,43 @@ func (m *Manager) buildEntryPointHandler(ctx context.Context, configs map[string logger.Warnf("No domain found in rule %v, the TLS options applied for this router will depend on the SNI of each request", routerHTTPConfig.Rule) } + // Even if the TLS options mismatch between the configured and the resolved one is handled in the aggregator + // we also have to handle it here to be able to mark the router in error. + tlsOptionsName := traefiktls.DefaultTLSConfigName + if len(routerHTTPConfig.TLS.Options) > 0 && routerHTTPConfig.TLS.Options != traefiktls.DefaultTLSConfigName { + tlsOptionsName = provider.GetQualifiedName(ctxRouter, routerHTTPConfig.TLS.Options) + } + + if routerHTTPConfig.TLS.ResolvedOptions != tlsOptionsName { + routerHTTPConfig.AddError(errors.New("found different TLS options for routers on the same host, so using the default TLS options instead"), false) + } + // Even though the error is seemingly ignored (aside from logging it), // we actually rely later on the fact that a tls config is nil (which happens when an error is returned) to take special steps // when assigning a handler to a route. - tlsConf, tlsConfErr := m.tlsManager.Get(traefiktls.DefaultTLSStoreName, tlsOptionsName) + tlsConf, tlsConfErr := m.tlsManager.Get(traefiktls.DefaultTLSStoreName, routerHTTPConfig.TLS.ResolvedOptions) if tlsConfErr != nil { // Note: we do not call AddError here because we already did so when buildRouterHandler errored for the same reason. logger.Error(tlsConfErr) } for _, domain := range domains { - // domain is already in lower case thanks to the domain parsing - if tlsOptionsForHostSNI[domain] == nil { - tlsOptionsForHostSNI[domain] = make(map[string]nameAndConfig) - } - tlsOptionsForHostSNI[domain][tlsOptionsName] = nameAndConfig{ - routerName: routerHTTPName, - TLSConfig: tlsConf, - } - - if name, ok := tlsOptionsForHost[domain]; ok && name != tlsOptionsName { - // Different tlsOptions on the same domain, so fallback to default - tlsOptionsForHost[domain] = traefiktls.DefaultTLSConfigName - } else { - tlsOptionsForHost[domain] = tlsOptionsName - } - } - } - - sniCheck := snicheck.New(tlsOptionsForHost, handlerHTTPS) - - // Keep in mind that defaultTLSConf might be nil here. - router.SetHTTPSHandler(sniCheck, defaultTLSConf) - - logger := log.FromContext(ctx) - for hostSNI, tlsConfigs := range tlsOptionsForHostSNI { - if len(tlsConfigs) == 1 { - var optionsName string - var config *tls.Config - for k, v := range tlsConfigs { - optionsName = k - config = v.TLSConfig - break - } - - if config == nil { + if tlsConf == nil { // we use nil config as a signal to insert a handler // that enforces that TLS connection attempts to the corresponding (broken) router should fail. - logger.Debugf("Adding special closing route for %s because broken TLS options %s", hostSNI, optionsName) - router.AddHTTPTLSConfig(hostSNI, nil) + logger.Debugf("Adding special closing route for %s because of a broken TLS options %s", domain, routerHTTPConfig.TLS.ResolvedOptions) + router.AddHTTPTLSConfig(domain, nil, "") continue } - logger.Debugf("Adding route for %s with TLS options %s", hostSNI, optionsName) - router.AddHTTPTLSConfig(hostSNI, config) - continue + logger.Debugf("Adding route for %s with TLS options %s", domain, routerHTTPConfig.TLS.ResolvedOptions) + router.AddHTTPTLSConfig(domain, tlsConf, routerHTTPConfig.TLS.ResolvedOptions) } - - // multiple tlsConfigs - - routers := make([]string, 0, len(tlsConfigs)) - for _, v := range tlsConfigs { - configsHTTP[v.routerName].AddError(fmt.Errorf("found different TLS options for routers on the same host %v, so using the default TLS options instead", hostSNI), false) - routers = append(routers, v.routerName) - } - - logger.Warnf("Found different TLS options for routers on the same host %v, so using the default TLS options instead for these routers: %#v", hostSNI, routers) - if defaultTLSConf == nil { - logger.Debugf("Adding special closing route for %s because broken default TLS options", hostSNI) - } - - router.AddHTTPTLSConfig(hostSNI, defaultTLSConf) } + // Keep in mind that defaultTLSConf might be nil here. + router.SetHTTPSHandler(handlerHTTPS, defaultTLSConf) + m.addTCPHandlers(ctx, configs, router) return router, nil @@ -385,8 +325,9 @@ func (m *Manager) addTCPHandlers(ctx context.Context, configs map[string]*runtim } handler = &tcp.TLSHandler{ - Next: handler, - Config: tlsConf, + Next: handler, + Config: tlsConf, + TLSOptionsName: tlsOptionsName, } logger.Debugf("Adding TLS route for %q", routerConfig.Rule) diff --git a/pkg/server/router/tcp/manager_test.go b/pkg/server/router/tcp/manager_test.go index d005f1b0e2..a24f58424f 100644 --- a/pkg/server/router/tcp/manager_test.go +++ b/pkg/server/router/tcp/manager_test.go @@ -1,14 +1,10 @@ package tcp import ( - "crypto/tls" "math" - "net/http" - "net/http/httptest" "testing" "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" "github.com/traefik/traefik/v2/pkg/config/dynamic" "github.com/traefik/traefik/v2/pkg/config/runtime" tcpmiddleware "github.com/traefik/traefik/v2/pkg/server/middleware/tcp" @@ -129,7 +125,8 @@ func TestRuntimeConfiguration(t *testing.T) { Service: "foo-service", Rule: "Host(`bar.foo`)", TLS: &dynamic.RouterTLSConfig{ - Options: "foo", + Options: "foo", + ResolvedOptions: "default", }, }, }, @@ -139,7 +136,8 @@ func TestRuntimeConfiguration(t *testing.T) { Service: "foo-service", Rule: "Host(`bar.foo`) && PathPrefix(`/path`)", TLS: &dynamic.RouterTLSConfig{ - Options: "bar", + Options: "bar", + ResolvedOptions: "default", }, }, }, @@ -396,293 +394,3 @@ func TestRuntimeConfiguration(t *testing.T) { }) } } - -func TestDomainFronting(t *testing.T) { - tlsOptionsBase := map[string]traefiktls.Options{ - "default": { - MinVersion: "VersionTLS10", - }, - "host1@file": { - MinVersion: "VersionTLS12", - }, - "host1@crd": { - MinVersion: "VersionTLS12", - }, - } - - entryPoints := []string{"web"} - - tests := []struct { - desc string - routers map[string]*runtime.RouterInfo - tlsOptions map[string]traefiktls.Options - host string - ServerName string - expectedStatus int - }{ - { - desc: "Request is misdirected when TLS options are different", - routers: map[string]*runtime.RouterInfo{ - "router-1@file": { - Router: &dynamic.Router{ - EntryPoints: entryPoints, - Rule: "Host(`host1.local`)", - TLS: &dynamic.RouterTLSConfig{ - Options: "host1", - }, - }, - }, - "router-2@file": { - Router: &dynamic.Router{ - EntryPoints: entryPoints, - Rule: "Host(`host2.local`)", - TLS: &dynamic.RouterTLSConfig{}, - }, - }, - }, - tlsOptions: tlsOptionsBase, - host: "host1.local", - ServerName: "host2.local", - expectedStatus: http.StatusMisdirectedRequest, - }, - { - desc: "Request is OK when TLS options are the same", - routers: map[string]*runtime.RouterInfo{ - "router-1@file": { - Router: &dynamic.Router{ - EntryPoints: entryPoints, - Rule: "Host(`host1.local`)", - TLS: &dynamic.RouterTLSConfig{ - Options: "host1", - }, - }, - }, - "router-2@file": { - Router: &dynamic.Router{ - EntryPoints: entryPoints, - Rule: "Host(`host2.local`)", - TLS: &dynamic.RouterTLSConfig{ - Options: "host1", - }, - }, - }, - }, - tlsOptions: tlsOptionsBase, - host: "host1.local", - ServerName: "host2.local", - expectedStatus: http.StatusOK, - }, - { - desc: "Default TLS options is used when options are ambiguous for the same host", - routers: map[string]*runtime.RouterInfo{ - "router-1@file": { - Router: &dynamic.Router{ - EntryPoints: entryPoints, - Rule: "Host(`host1.local`)", - TLS: &dynamic.RouterTLSConfig{ - Options: "host1", - }, - }, - }, - "router-2@file": { - Router: &dynamic.Router{ - EntryPoints: entryPoints, - Rule: "Host(`host1.local`) && PathPrefix(`/foo`)", - TLS: &dynamic.RouterTLSConfig{ - Options: "default", - }, - }, - }, - "router-3@file": { - Router: &dynamic.Router{ - EntryPoints: entryPoints, - Rule: "Host(`host2.local`)", - TLS: &dynamic.RouterTLSConfig{ - Options: "host1", - }, - }, - }, - }, - tlsOptions: tlsOptionsBase, - host: "host1.local", - ServerName: "host2.local", - expectedStatus: http.StatusMisdirectedRequest, - }, - { - desc: "Default TLS options should not be used when options are the same for the same host", - routers: map[string]*runtime.RouterInfo{ - "router-1@file": { - Router: &dynamic.Router{ - EntryPoints: entryPoints, - Rule: "Host(`host1.local`)", - TLS: &dynamic.RouterTLSConfig{ - Options: "host1", - }, - }, - }, - "router-2@file": { - Router: &dynamic.Router{ - EntryPoints: entryPoints, - Rule: "Host(`host1.local`) && PathPrefix(`/bar`)", - TLS: &dynamic.RouterTLSConfig{ - Options: "host1", - }, - }, - }, - "router-3@file": { - Router: &dynamic.Router{ - EntryPoints: entryPoints, - Rule: "Host(`host2.local`)", - TLS: &dynamic.RouterTLSConfig{ - Options: "host1", - }, - }, - }, - }, - tlsOptions: tlsOptionsBase, - host: "host1.local", - ServerName: "host2.local", - expectedStatus: http.StatusOK, - }, - { - desc: "Request is misdirected when TLS options have the same name but from different providers", - routers: map[string]*runtime.RouterInfo{ - "router-1@file": { - Router: &dynamic.Router{ - EntryPoints: entryPoints, - Rule: "Host(`host1.local`)", - TLS: &dynamic.RouterTLSConfig{ - Options: "host1", - }, - }, - }, - "router-2@crd": { - Router: &dynamic.Router{ - EntryPoints: entryPoints, - Rule: "Host(`host2.local`)", - TLS: &dynamic.RouterTLSConfig{ - Options: "host1", - }, - }, - }, - }, - tlsOptions: tlsOptionsBase, - host: "host1.local", - ServerName: "host2.local", - expectedStatus: http.StatusMisdirectedRequest, - }, - { - desc: "Request is OK when TLS options reference from a different provider is the same", - routers: map[string]*runtime.RouterInfo{ - "router-1@file": { - Router: &dynamic.Router{ - EntryPoints: entryPoints, - Rule: "Host(`host1.local`)", - TLS: &dynamic.RouterTLSConfig{ - Options: "host1@crd", - }, - }, - }, - "router-2@crd": { - Router: &dynamic.Router{ - EntryPoints: entryPoints, - Rule: "Host(`host2.local`)", - TLS: &dynamic.RouterTLSConfig{ - Options: "host1@crd", - }, - }, - }, - }, - tlsOptions: tlsOptionsBase, - host: "host1.local", - ServerName: "host2.local", - expectedStatus: http.StatusOK, - }, - { - desc: "Request is misdirected when server name is empty and the host name is an FQDN, but router's rule is not", - routers: map[string]*runtime.RouterInfo{ - "router-1@file": { - Router: &dynamic.Router{ - EntryPoints: entryPoints, - Rule: "Host(`host1.local`)", - TLS: &dynamic.RouterTLSConfig{ - Options: "host1@file", - }, - }, - }, - }, - tlsOptions: map[string]traefiktls.Options{ - "default": { - MinVersion: "VersionTLS13", - }, - "host1@file": { - MinVersion: "VersionTLS12", - }, - }, - host: "host1.local.", - expectedStatus: http.StatusMisdirectedRequest, - }, - { - desc: "Request is misdirected when server name is empty and the host name is not FQDN, but router's rule is", - routers: map[string]*runtime.RouterInfo{ - "router-1@file": { - Router: &dynamic.Router{ - EntryPoints: entryPoints, - Rule: "Host(`host1.local.`)", - TLS: &dynamic.RouterTLSConfig{ - Options: "host1@file", - }, - }, - }, - }, - tlsOptions: map[string]traefiktls.Options{ - "default": { - MinVersion: "VersionTLS13", - }, - "host1@file": { - MinVersion: "VersionTLS12", - }, - }, - host: "host1.local", - expectedStatus: http.StatusMisdirectedRequest, - }, - } - - for _, test := range tests { - t.Run(test.desc, func(t *testing.T) { - conf := &runtime.Configuration{ - Routers: test.routers, - } - - serviceManager := tcp.NewManager(conf) - - tlsManager := traefiktls.NewManager() - tlsManager.UpdateConfigs(t.Context(), map[string]traefiktls.Store{}, test.tlsOptions, []*traefiktls.CertAndStores{}) - - httpsHandler := map[string]http.Handler{ - "web": http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {}), - } - - middlewaresBuilder := tcpmiddleware.NewBuilder(conf.TCPMiddlewares) - - routerManager := NewManager(conf, serviceManager, middlewaresBuilder, nil, httpsHandler, tlsManager) - - routers := routerManager.BuildHandlers(t.Context(), entryPoints) - - router, ok := routers["web"] - require.True(t, ok) - - req := httptest.NewRequest(http.MethodGet, "/", nil) - req.Host = test.host - req.TLS = &tls.ConnectionState{ - ServerName: test.ServerName, - } - - rw := httptest.NewRecorder() - - router.GetHTTPSHandler().ServeHTTP(rw, req) - - assert.Equal(t, test.expectedStatus, rw.Code) - }) - } -} diff --git a/pkg/server/router/tcp/router.go b/pkg/server/router/tcp/router.go index 23f624c0c1..c40f64eae5 100644 --- a/pkg/server/router/tcp/router.go +++ b/pkg/server/router/tcp/router.go @@ -17,11 +17,17 @@ import ( "github.com/traefik/traefik/v2/pkg/log" tcpmuxer "github.com/traefik/traefik/v2/pkg/muxer/tcp" "github.com/traefik/traefik/v2/pkg/tcp" + traefiktls "github.com/traefik/traefik/v2/pkg/tls" ) // errClientHelloRead is used as a sentinel error to break the TLS handshake once we have read the ClientHello. var errClientHelloRead = errors.New("client hello successfully read") +type tlsConfigWithOptionsName struct { + cfg *tls.Config + optionsName string +} + // Router is a TCP router. type Router struct { acmeTLSPassthrough bool @@ -48,7 +54,7 @@ type Router struct { httpsTLSConfig *tls.Config // default TLS config // hostHTTPTLSConfig contains TLS configs keyed by SNI. // A nil config is the hint to set up a brokenTLSRouter. - hostHTTPTLSConfig map[string]*tls.Config // TLS configs keyed by SNI + hostHTTPTLSConfig map[string]tlsConfigWithOptionsName // TLS configs keyed by SNI } // NewRouter returns a new TCP router. @@ -75,14 +81,20 @@ func NewRouter() (*Router, error) { }, nil } -// GetTLSGetClientInfo is called after a ClientHello is received from a client. -func (r *Router) GetTLSGetClientInfo() func(info *tls.ClientHelloInfo) (*tls.Config, error) { - return func(info *tls.ClientHelloInfo) (*tls.Config, error) { - if tlsConfig, ok := r.hostHTTPTLSConfig[info.ServerName]; ok { - return tlsConfig, nil +// HTTP3TLSConfigMatcherFunc returns a matcher func for HTTP/3 which returns a tls.Config with its corresponding +// TLSOptionName matching the given HostSNI in the connection data, or the default TLS config if there is no match. +func (r *Router) HTTP3TLSConfigMatcherFunc() func(connData tcpmuxer.ConnData) (*tls.Config, string, error) { + return func(connData tcpmuxer.ConnData) (*tls.Config, string, error) { + h, _ := r.muxerHTTPS.Match(connData) + if h == nil { + return r.httpsTLSConfig, traefiktls.DefaultTLSConfigName, nil } - return r.httpsTLSConfig, nil + if tlsHandler, ok := h.(*tcp.TLSHandler); ok { + return tlsHandler.Config, tlsHandler.TLSOptionsName, nil + } + + return nil, "", errors.New("matching handler is not a TLSHandler") } } @@ -94,7 +106,7 @@ func (r *Router) ServeTCP(conn tcp.WriteCloser) { // we would block forever on clientHelloInfo, // which is why we want to detect and handle that case first and foremost. if r.muxerTCP.HasRoutes() && !r.muxerTCPTLS.HasRoutes() && !r.muxerHTTPS.HasRoutes() { - connData, err := tcpmuxer.NewConnData("", conn, nil) + connData, err := tcpmuxer.NewConnData("", conn.RemoteAddr(), nil) if err != nil { log.WithoutContext().Errorf("Error while reading TCP connection data: %v", err) conn.Close() @@ -136,7 +148,7 @@ func (r *Router) ServeTCP(conn tcp.WriteCloser) { log.WithoutContext().Errorf("Error while setting deadline: %v", err) } - connData, err := tcpmuxer.NewConnData(hello.serverName, conn, hello.protos) + connData, err := tcpmuxer.NewConnData(hello.serverName, conn.RemoteAddr(), hello.protos) if err != nil { log.WithoutContext().Errorf("Error while reading TCP connection data: %v", err) conn.Close() @@ -212,12 +224,15 @@ func (r *Router) AddRoute(rule string, priority int, target tcp.Handler) error { } // AddHTTPTLSConfig defines a handler for a given sniHost and sets the matching tlsConfig. -func (r *Router) AddHTTPTLSConfig(sniHost string, config *tls.Config) { +func (r *Router) AddHTTPTLSConfig(sniHost string, config *tls.Config, optionsName string) { if r.hostHTTPTLSConfig == nil { - r.hostHTTPTLSConfig = map[string]*tls.Config{} + r.hostHTTPTLSConfig = map[string]tlsConfigWithOptionsName{} } - r.hostHTTPTLSConfig[sniHost] = config + r.hostHTTPTLSConfig[sniHost] = tlsConfigWithOptionsName{ + cfg: config, + optionsName: optionsName, + } } // GetConn creates a connection proxy with a peeked string. @@ -262,12 +277,13 @@ func (t *brokenTLSRouter) ServeTCP(conn tcp.WriteCloser) { func (r *Router) SetHTTPSForwarder(handler tcp.Handler) { for sniHost, tlsConf := range r.hostHTTPTLSConfig { var tcpHandler tcp.Handler - if tlsConf == nil { + if tlsConf.cfg == nil { tcpHandler = &brokenTLSRouter{} } else { tcpHandler = &tcp.TLSHandler{ - Next: handler, - Config: tlsConf, + Next: handler, + Config: tlsConf.cfg, + TLSOptionsName: tlsConf.optionsName, } } @@ -285,8 +301,9 @@ func (r *Router) SetHTTPSForwarder(handler tcp.Handler) { } r.httpsForwarder = &tcp.TLSHandler{ - Next: handler, - Config: r.httpsTLSConfig, + Next: handler, + Config: r.httpsTLSConfig, + TLSOptionsName: "default", } } diff --git a/pkg/server/router/tcp/router_test.go b/pkg/server/router/tcp/router_test.go index e12a564610..79ca37c438 100644 --- a/pkg/server/router/tcp/router_test.go +++ b/pkg/server/router/tcp/router_test.go @@ -2,6 +2,7 @@ package tcp import ( "bytes" + "context" "crypto/tls" "errors" "fmt" @@ -20,7 +21,7 @@ import ( "github.com/traefik/traefik/v2/pkg/config/runtime" tcpmiddleware "github.com/traefik/traefik/v2/pkg/server/middleware/tcp" "github.com/traefik/traefik/v2/pkg/server/service/tcp" - tcp2 "github.com/traefik/traefik/v2/pkg/tcp" + traefiktcp "github.com/traefik/traefik/v2/pkg/tcp" traefiktls "github.com/traefik/traefik/v2/pkg/tls" "github.com/traefik/traefik/v2/pkg/tls/generate" ) @@ -52,7 +53,7 @@ func (h *httpForwarder) Close() error { } // ServeTCP uses the connection to serve it later in "Accept". -func (h *httpForwarder) ServeTCP(conn tcp2.WriteCloser) { +func (h *httpForwarder) ServeTCP(conn traefiktcp.WriteCloser) { h.connChan <- conn } @@ -621,6 +622,16 @@ func Test_Routing(t *testing.T) { _, err = fmt.Fprint(w, "HTTPS") require.NoError(t, err) }), + + ConnContext: func(ctx context.Context, c net.Conn) context.Context { + if tlsConn, ok := c.(*tls.Conn); ok { + if tlsConnWithOptionsName, ok := tlsConn.NetConn().(traefiktcp.TLSConn); ok { + return traefiktcp.AddTLSOptionsNameInContext(ctx, tlsConnWithOptionsName.TLSOptionsName) + } + } + + return ctx + }, } stoppedHTTPS := make(chan struct{}) @@ -812,7 +823,8 @@ func routerHTTPSPathPrefix(conf *runtime.Configuration) { Service: "http", Rule: "PathPrefix(`/`)", TLS: &dynamic.RouterTLSConfig{ - Options: "tls10", + Options: "tls10", + ResolvedOptions: "tls10", }, }, } @@ -826,7 +838,8 @@ func routerHTTPS(conf *runtime.Configuration) { Service: "http", Rule: "Host(`foo.bar`)", TLS: &dynamic.RouterTLSConfig{ - Options: "tls12", + Options: "tls12", + ResolvedOptions: "tls12", }, }, } diff --git a/pkg/server/server_entrypoint_tcp.go b/pkg/server/server_entrypoint_tcp.go index 5814411b16..39dd3b42fe 100644 --- a/pkg/server/server_entrypoint_tcp.go +++ b/pkg/server/server_entrypoint_tcp.go @@ -2,6 +2,7 @@ package server import ( "context" + "crypto/tls" "errors" "expvar" "fmt" @@ -610,6 +611,15 @@ func createHTTPServer(ctx context.Context, ln net.Listener, configuration *stati HTTP2: &http.HTTP2Config{ MaxConcurrentStreams: int(configuration.HTTP2.MaxConcurrentStreams), }, + ConnContext: func(ctx context.Context, c net.Conn) context.Context { + if tlsConn, ok := c.(*tls.Conn); ok { + if tlsConnWithOptionsName, ok := tlsConn.NetConn().(tcp.TLSConn); ok { + return tcp.AddTLSOptionsNameInContext(ctx, tlsConnWithOptionsName.TLSOptionsName) + } + } + + return ctx + }, } if debugConnection || (configuration.Transport != nil && (configuration.Transport.KeepAliveMaxTime > 0 || configuration.Transport.KeepAliveMaxRequests > 0)) { serverHTTP.ConnContext = func(ctx context.Context, c net.Conn) context.Context { diff --git a/pkg/server/server_entrypoint_tcp_http3.go b/pkg/server/server_entrypoint_tcp_http3.go index d8821267c7..b971d97892 100644 --- a/pkg/server/server_entrypoint_tcp_http3.go +++ b/pkg/server/server_entrypoint_tcp_http3.go @@ -13,7 +13,9 @@ import ( "github.com/quic-go/quic-go/http3" "github.com/traefik/traefik/v2/pkg/config/static" "github.com/traefik/traefik/v2/pkg/log" + tcpmuxer "github.com/traefik/traefik/v2/pkg/muxer/tcp" tcprouter "github.com/traefik/traefik/v2/pkg/server/router/tcp" + "github.com/traefik/traefik/v2/pkg/tcp" ) type http3server struct { @@ -22,7 +24,7 @@ type http3server struct { http3conn net.PacketConn lock sync.RWMutex - getter func(info *tls.ClientHelloInfo) (*tls.Config, error) + getter func(data tcpmuxer.ConnData) (*tls.Config, string, error) } func newHTTP3Server(ctx context.Context, configuration *static.EntryPoint, httpsServer *httpServer) (*http3server, error) { @@ -41,8 +43,8 @@ func newHTTP3Server(ctx context.Context, configuration *static.EntryPoint, https h3 := &http3server{ http3conn: conn, - getter: func(info *tls.ClientHelloInfo) (*tls.Config, error) { - return nil, errors.New("no tls config") + getter: func(data tcpmuxer.ConnData) (*tls.Config, string, error) { + return nil, "", errors.New("no TLS config") }, } @@ -50,10 +52,18 @@ func newHTTP3Server(ctx context.Context, configuration *static.EntryPoint, https Addr: configuration.GetAddress(), Port: configuration.HTTP3.AdvertisedPort, Handler: httpsServer.Server.(*http.Server).Handler, - TLSConfig: &tls.Config{GetConfigForClient: h3.getGetConfigForClient}, + TLSConfig: &tls.Config{GetConfigForClient: h3.getTLSConfigForClient}, QUICConfig: &quic.Config{ Allow0RTT: false, }, + ConnContext: func(ctx context.Context, c *quic.Conn) context.Context { + tlsOptionsName, err := h3.getTLSOptionsName(c) + if err != nil { + log.WithoutContext().Errorf("Error getting TLS options name for client: %v", err) + return ctx + } + return tcp.AddTLSOptionsNameInContext(ctx, tlsOptionsName) + }, } previousHandler := httpsServer.Server.(*http.Server).Handler @@ -77,7 +87,7 @@ func (e *http3server) Switch(rt *tcprouter.Router) { e.lock.Lock() defer e.lock.Unlock() - e.getter = rt.GetTLSGetClientInfo() + e.getter = rt.HTTP3TLSConfigMatcherFunc() } func (e *http3server) Shutdown(_ context.Context) error { @@ -85,9 +95,28 @@ func (e *http3server) Shutdown(_ context.Context) error { return e.Server.Close() } -func (e *http3server) getGetConfigForClient(info *tls.ClientHelloInfo) (*tls.Config, error) { +func (e *http3server) getTLSConfigForClient(info *tls.ClientHelloInfo) (*tls.Config, error) { e.lock.RLock() defer e.lock.RUnlock() - return e.getter(info) + connData, err := tcpmuxer.NewConnData(info.ServerName, info.Conn.RemoteAddr(), info.SupportedProtos) + if err != nil { + return nil, fmt.Errorf("creating ConnData from client hello: %w", err) + } + + conf, _, err := e.getter(connData) + return conf, err +} + +func (e *http3server) getTLSOptionsName(c *quic.Conn) (string, error) { + e.lock.RLock() + defer e.lock.RUnlock() + + connData, err := tcpmuxer.NewConnData(c.ConnectionState().TLS.ServerName, c.RemoteAddr(), []string{c.ConnectionState().TLS.NegotiatedProtocol}) + if err != nil { + return "", fmt.Errorf("creating ConnData from quic Conn: %w", err) + } + + _, name, err := e.getter(connData) + return name, err } diff --git a/pkg/server/server_entrypoint_tcp_http3_test.go b/pkg/server/server_entrypoint_tcp_http3_test.go index bcaa1c028d..9adc1d43d7 100644 --- a/pkg/server/server_entrypoint_tcp_http3_test.go +++ b/pkg/server/server_entrypoint_tcp_http3_test.go @@ -102,7 +102,7 @@ func TestHTTP3AdvertisedPort(t *testing.T) { router.AddHTTPTLSConfig("*", &tls.Config{ Certificates: []tls.Certificate{tlsCert}, - }) + }, traefiktls.DefaultTLSConfigName) router.SetHTTPSHandler(http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) { rw.WriteHeader(http.StatusOK) }), nil) @@ -164,7 +164,7 @@ func TestHTTP30RTT(t *testing.T) { router.AddHTTPTLSConfig("example.com", &tls.Config{ Certificates: []tls.Certificate{tlsCert}, - }) + }, traefiktls.DefaultTLSConfigName) router.SetHTTPSHandler(http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) { rw.WriteHeader(http.StatusOK) }), nil) diff --git a/pkg/tcp/tls.go b/pkg/tcp/tls.go index 207aebb150..33914e5e53 100644 --- a/pkg/tcp/tls.go +++ b/pkg/tcp/tls.go @@ -1,16 +1,39 @@ package tcp import ( + "context" "crypto/tls" ) +// TLSConn is a TLS connection that also carries the name of the TLS config used. +type TLSConn struct { + WriteCloser + + TLSOptionsName string +} + // TLSHandler handles TLS connections. type TLSHandler struct { - Next Handler - Config *tls.Config + Next Handler + Config *tls.Config + TLSOptionsName string } // ServeTCP terminates the TLS connection. func (t *TLSHandler) ServeTCP(conn WriteCloser) { - t.Next.ServeTCP(tls.Server(conn, t.Config)) + t.Next.ServeTCP(tls.Server(TLSConn{WriteCloser: conn, TLSOptionsName: t.TLSOptionsName}, t.Config)) +} + +type tlsOptionsNameKey struct{} + +func AddTLSOptionsNameInContext(ctx context.Context, name string) context.Context { + return context.WithValue(ctx, tlsOptionsNameKey{}, name) +} + +func GetTLSOptionsName(ctx context.Context) string { + if name, ok := ctx.Value(tlsOptionsNameKey{}).(string); ok { + return name + } + + return "" } From 892bcc288b1c99f7c18a977c9bcab617d5cfa37e Mon Sep 17 00:00:00 2001 From: Romain Date: Thu, 28 May 2026 15:56:25 +0200 Subject: [PATCH 4/5] Reject requests with different paths after StripPrefix and StripPrefixRegex normalisation --- docs/content/migration/v2.md | 17 +++++++++++++++++ pkg/middlewares/stripprefix/strip_prefix.go | 12 +++++++++++- .../stripprefix/strip_prefix_test.go | 19 +++++++------------ .../stripprefixregex/strip_prefix_regex.go | 12 +++++++++++- .../strip_prefix_regex_test.go | 12 ++---------- 5 files changed, 48 insertions(+), 24 deletions(-) diff --git a/docs/content/migration/v2.md b/docs/content/migration/v2.md index 69baa2c9a9..8c09fd7d5f 100644 --- a/docs/content/migration/v2.md +++ b/docs/content/migration/v2.md @@ -856,3 +856,20 @@ and [Kubernetes Gateway](../providers/kubernetes-gateway.md#crossprovidernamespa From version `v2.11.47` onwards, the BasicAuth middleware requires a non-empty users configuration in order to be built successfully. Previously, the middleware would be built successfully but always return a 401 status code for any request. Now, an error occurs and any routers using it will be unmounted. For the same request, a 404 status code is served instead of a 401 status code. + +### StripPrefix and StripPrefixRegex Middleware + +From version `v2.11.47` onwards, the StripPrefix middleware and the StripPrefixRegex middleware reject requests (`400 Bad Request`) +when stripping the configured prefix produces a path that differs from its normalised form +(i.e. a path containing `.` or `..` segments that would be collapsed by normalisation). + +This prevents the stripped path from being interpreted as a different resource by the upstream service. + +Examples with a configured prefix of `/api`: + +| Request path | Path after strip | Normalised path | Result | +|--------------|------------------|-----------------|--------------| +| `/api/foo` | `/foo` | `/foo` | `200` (sent) | +| `/api/` | `/` | `/` | `200` (sent) | +| `/api./foo` | `/./foo` | `/foo` | `400` | +| `/api../foo` | `/../foo` | `/foo` | `400` | diff --git a/pkg/middlewares/stripprefix/strip_prefix.go b/pkg/middlewares/stripprefix/strip_prefix.go index 6abd4391e4..29a6afddca 100644 --- a/pkg/middlewares/stripprefix/strip_prefix.go +++ b/pkg/middlewares/stripprefix/strip_prefix.go @@ -48,6 +48,8 @@ func (s *stripPrefix) GetTracingInformation() (string, ext.SpanKindEnum) { } func (s *stripPrefix) ServeHTTP(rw http.ResponseWriter, req *http.Request) { + logger := log.FromContext(middlewares.GetLoggerCtx(req.Context(), s.name, typeName)) + for _, prefix := range s.prefixes { if strings.HasPrefix(req.URL.Path, prefix) { req.URL.Path = s.getPathStripped(req.URL.Path, prefix) @@ -58,10 +60,18 @@ func (s *stripPrefix) ServeHTTP(rw http.ResponseWriter, req *http.Request) { // 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 != "" { + path := req.URL.Path + if path != "" { req.URL = req.URL.JoinPath() } + // Stop here if the normalization of the path produces a different path. + if path != req.URL.Path { + logger.Debugf("Rejecting request, sanitized path: %q is not equivalent to stripped path: %q", path, req.URL.Path) + http.Error(rw, http.StatusText(http.StatusBadRequest), http.StatusBadRequest) + return + } + req.Header.Add(ForwardedPrefixHeader, prefix) req.RequestURI = req.URL.RequestURI() break diff --git a/pkg/middlewares/stripprefix/strip_prefix_test.go b/pkg/middlewares/stripprefix/strip_prefix_test.go index 1fa0330de1..b551154af7 100644 --- a/pkg/middlewares/stripprefix/strip_prefix_test.go +++ b/pkg/middlewares/stripprefix/strip_prefix_test.go @@ -191,10 +191,7 @@ func TestStripPrefix(t *testing.T) { Prefixes: []string{"/api"}, }, path: "/api./foo", - expectedStatusCode: http.StatusOK, - expectedPath: "/foo", - expectedRawPath: "", - expectedHeader: "/api", + expectedStatusCode: http.StatusBadRequest, }, { desc: "multiple dots in the path not stripped by the prefix", @@ -202,10 +199,7 @@ func TestStripPrefix(t *testing.T) { Prefixes: []string{"/api"}, }, path: "/api../foo", - expectedStatusCode: http.StatusOK, - expectedPath: "/foo", - expectedRawPath: "", - expectedHeader: "/api", + expectedStatusCode: http.StatusBadRequest, }, { desc: "multiple dots in the path not stripped by the prefix with forceSlash", @@ -214,10 +208,7 @@ func TestStripPrefix(t *testing.T) { ForceSlash: true, }, path: "/api../foo", - expectedStatusCode: http.StatusOK, - expectedPath: "/foo", - expectedRawPath: "", - expectedHeader: "/api", + expectedStatusCode: http.StatusBadRequest, }, } @@ -244,6 +235,10 @@ func TestStripPrefix(t *testing.T) { handler.ServeHTTP(resp, req) assert.Equal(t, test.expectedStatusCode, resp.Code, "Unexpected status code.") + if test.expectedStatusCode != http.StatusOK { + return + } + assert.Equal(t, test.expectedPath, actualPath, "Unexpected path.") assert.Equal(t, test.expectedRawPath, actualRawPath, "Unexpected raw path.") assert.Equal(t, test.expectedHeader, actualHeader, "Unexpected '%s' header.", ForwardedPrefixHeader) diff --git a/pkg/middlewares/stripprefixregex/strip_prefix_regex.go b/pkg/middlewares/stripprefixregex/strip_prefix_regex.go index b07966fd51..825e91bd77 100644 --- a/pkg/middlewares/stripprefixregex/strip_prefix_regex.go +++ b/pkg/middlewares/stripprefixregex/strip_prefix_regex.go @@ -50,6 +50,8 @@ func (s *stripPrefixRegex) GetTracingInformation() (string, ext.SpanKindEnum) { } func (s *stripPrefixRegex) ServeHTTP(rw http.ResponseWriter, req *http.Request) { + logger := log.FromContext(middlewares.GetLoggerCtx(req.Context(), s.name, typeName)) + for _, exp := range s.expressions { parts := exp.FindStringSubmatch(req.URL.Path) if len(parts) > 0 && len(parts[0]) > 0 { @@ -68,10 +70,18 @@ func (s *stripPrefixRegex) ServeHTTP(rw http.ResponseWriter, req *http.Request) // 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 != "" { + path := req.URL.Path + if path != "" { req.URL = req.URL.JoinPath() } + // Stop here if the normalization of the path produces a different path. + if path != req.URL.Path { + logger.Debugf("Rejecting request, sanitized path: %q is not equivalent to stripped path: %q", path, req.URL.Path) + http.Error(rw, http.StatusText(http.StatusBadRequest), http.StatusBadRequest) + return + } + req.RequestURI = req.URL.RequestURI() break } diff --git a/pkg/middlewares/stripprefixregex/strip_prefix_regex_test.go b/pkg/middlewares/stripprefixregex/strip_prefix_regex_test.go index c9c6b59347..3803421960 100644 --- a/pkg/middlewares/stripprefixregex/strip_prefix_regex_test.go +++ b/pkg/middlewares/stripprefixregex/strip_prefix_regex_test.go @@ -201,21 +201,13 @@ func TestStripPrefixRegex(t *testing.T) { desc: "/api./foo", config: dynamic.StripPrefixRegex{Regex: []string{"/api"}}, path: "/api./foo", - expectedStatusCode: http.StatusOK, - expectedPath: "/foo", - expectedRawPath: "", - expectedRequestURI: "/foo", - expectedHeader: "/api", + expectedStatusCode: http.StatusBadRequest, }, { desc: "/api../foo", config: dynamic.StripPrefixRegex{Regex: []string{"/api"}}, path: "/api../foo", - expectedStatusCode: http.StatusOK, - expectedPath: "/foo", - expectedRawPath: "", - expectedRequestURI: "/foo", - expectedHeader: "/api", + expectedStatusCode: http.StatusBadRequest, }, } From fc83948b1eba226bef903d06063fa2b3df3c514f Mon Sep 17 00:00:00 2001 From: Kevin Pollet Date: Fri, 29 May 2026 09:14:05 +0200 Subject: [PATCH 5/5] Bump golang.org/x/net to v0.55.0 --- go.mod | 10 +++++----- go.sum | 20 ++++++++++---------- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/go.mod b/go.mod index cc56298a02..b4324a7ee5 100644 --- a/go.mod +++ b/go.mod @@ -75,8 +75,8 @@ require ( go.elastic.co/apm/module/apmot/v2 v2.4.8 go.elastic.co/apm/v2 v2.4.8 golang.org/x/mod v0.35.0 - golang.org/x/net v0.53.0 - golang.org/x/text v0.36.0 + golang.org/x/net v0.55.0 + golang.org/x/text v0.37.0 golang.org/x/time v0.15.0 golang.org/x/tools v0.44.0 google.golang.org/grpc v1.80.0 @@ -393,12 +393,12 @@ require ( go.uber.org/ratelimit v0.3.1 // indirect go.uber.org/zap v1.27.0 // indirect go.yaml.in/yaml/v3 v3.0.4 // indirect - golang.org/x/crypto v0.50.0 // indirect + golang.org/x/crypto v0.51.0 // indirect golang.org/x/exp v0.0.0-20260410095643-746e56fc9e2f // indirect golang.org/x/oauth2 v0.36.0 // indirect golang.org/x/sync v0.20.0 // indirect - golang.org/x/sys v0.43.0 // indirect - golang.org/x/term v0.42.0 // indirect + golang.org/x/sys v0.45.0 // indirect + golang.org/x/term v0.43.0 // indirect golang.org/x/xerrors v0.0.0-20231012003039-104605ab7028 // indirect google.golang.org/api v0.276.0 // indirect google.golang.org/genproto/googleapis/api v0.0.0-20260319201613-d00831a3d3e7 // indirect diff --git a/go.sum b/go.sum index b4d6490011..fbfb2385b4 100644 --- a/go.sum +++ b/go.sum @@ -2363,8 +2363,8 @@ golang.org/x/crypto v0.19.0/go.mod h1:Iy9bg/ha4yyC70EfRS8jz+B6ybOBKMaSxLj6P6oBDf golang.org/x/crypto v0.21.0/go.mod h1:0BP7YvVV9gBbVKyeTG0Gyn+gZm94bibOW5BjDEYAOMs= golang.org/x/crypto v0.23.0/go.mod h1:CKFgDieR+mRhux2Lsu27y0fO304Db0wZe70UKqHu0v8= golang.org/x/crypto v0.24.0/go.mod h1:Z1PMYSOR5nyMcyAVAIQSKCDwalqy85Aqn1x3Ws4L5DM= -golang.org/x/crypto v0.50.0 h1:zO47/JPrL6vsNkINmLoo/PH1gcxpls50DNogFvB5ZGI= -golang.org/x/crypto v0.50.0/go.mod h1:3muZ7vA7PBCE6xgPX7nkzzjiUq87kRItoJQM1Yo8S+Q= +golang.org/x/crypto v0.51.0 h1:IBPXwPfKxY7cWQZ38ZCIRPI50YLeevDLlLnyC5wRGTI= +golang.org/x/crypto v0.51.0/go.mod h1:8AdwkbraGNABw2kOX6YFPs3WM22XqI4EXEd8g+x7Oc8= golang.org/x/exp v0.0.0-20180321215751-8460e604b9de/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA= golang.org/x/exp v0.0.0-20180807140117-3d87b88a115f/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA= golang.org/x/exp v0.0.0-20190121172915-509febef88a4/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA= @@ -2520,8 +2520,8 @@ golang.org/x/net v0.21.0/go.mod h1:bIjVDfnllIU7BJ2DNgfnXvpSvtn8VRwhlsaeUTyUS44= golang.org/x/net v0.23.0/go.mod h1:JKghWKKOSdJwpW2GEx0Ja7fmaKnMsbu+MWVZTokSYmg= golang.org/x/net v0.25.0/go.mod h1:JkAGAh7GEvH74S6FOH42FLoXpXbE/aqXSrIQjXgsiwM= golang.org/x/net v0.26.0/go.mod h1:5YKkiSynbBIh3p6iOc/vibscux0x38BZDkn8sCUPxHE= -golang.org/x/net v0.53.0 h1:d+qAbo5L0orcWAr0a9JweQpjXF19LMXJE8Ey7hwOdUA= -golang.org/x/net v0.53.0/go.mod h1:JvMuJH7rrdiCfbeHoo3fCQU24Lf5JJwT9W3sJFulfgs= +golang.org/x/net v0.55.0 h1:bcvxaJn3e1U6InsFWt1JUq1aSjnRxLzT2rtD2KfkDF8= +golang.org/x/net v0.55.0/go.mod h1:L5U2KuzuOe1lY7Z+aWVIKK6qEeJXnXV9yzGA+WCHJww= golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U= golang.org/x/oauth2 v0.0.0-20190226205417-e64efc72b421/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw= golang.org/x/oauth2 v0.0.0-20190604053449-0f29369cfe45/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw= @@ -2709,8 +2709,8 @@ golang.org/x/sys v0.17.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= golang.org/x/sys v0.18.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= golang.org/x/sys v0.20.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= golang.org/x/sys v0.21.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= -golang.org/x/sys v0.43.0 h1:Rlag2XtaFTxp19wS8MXlJwTvoh8ArU6ezoyFsMyCTNI= -golang.org/x/sys v0.43.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw= +golang.org/x/sys v0.45.0 h1:dO4czNzziLiiXplLQgBCEpCvXQ3dnkn0SdaZSYdQ+FY= +golang.org/x/sys v0.45.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw= golang.org/x/telemetry v0.0.0-20240228155512-f48c80bd79b2/go.mod h1:TeRTkGYfJXctD9OcfyVLyj2J3IxLnKwHJR8f4D8a3YE= golang.org/x/term v0.0.0-20201117132131-f5c789dd3221/go.mod h1:Nr5EML6q2oocZ2LXRh80K7BxOlk5/8JxuGnuhpl+muw= golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= @@ -2733,8 +2733,8 @@ golang.org/x/term v0.17.0/go.mod h1:lLRBjIVuehSbZlaOtGMbcMncT+aqLLLmKrsjNrUguwk= golang.org/x/term v0.18.0/go.mod h1:ILwASektA3OnRv7amZ1xhE/KTR+u50pbXfZ03+6Nx58= golang.org/x/term v0.20.0/go.mod h1:8UkIAJTvZgivsXaD6/pH6U9ecQzZ45awqEOzuCvwpFY= golang.org/x/term v0.21.0/go.mod h1:ooXLefLobQVslOqselCNF4SxFAaoS6KujMbsGzSDmX0= -golang.org/x/term v0.42.0 h1:UiKe+zDFmJobeJ5ggPwOshJIVt6/Ft0rcfrXZDLWAWY= -golang.org/x/term v0.42.0/go.mod h1:Dq/D+snpsbazcBG5+F9Q1n2rXV8Ma+71xEjTRufARgY= +golang.org/x/term v0.43.0 h1:S4RLU2sB31O/NCl+zFN9Aru9A/Cq2aqKpTZJ6B+DwT4= +golang.org/x/term v0.43.0/go.mod h1:lrhlHNdQJHO+1qVYiHfFKVuVioJIheAc3fBSMFYEIsk= golang.org/x/text v0.0.0-20170915032832-14c0d48ead0c/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.1-0.20180807135948-17ff2d5776d2/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= @@ -2756,8 +2756,8 @@ golang.org/x/text v0.13.0/go.mod h1:TvPlkZtksWOMsz7fbANvkp4WM8x/WCo/om8BMLbz+aE= golang.org/x/text v0.14.0/go.mod h1:18ZOQIKpY8NJVqYksKHtTdi31H5itFRjB5/qKTNYzSU= golang.org/x/text v0.15.0/go.mod h1:18ZOQIKpY8NJVqYksKHtTdi31H5itFRjB5/qKTNYzSU= golang.org/x/text v0.16.0/go.mod h1:GhwF1Be+LQoKShO3cGOHzqOgRrGaYc9AvblQOmPVHnI= -golang.org/x/text v0.36.0 h1:JfKh3XmcRPqZPKevfXVpI1wXPTqbkE5f7JA92a55Yxg= -golang.org/x/text v0.36.0/go.mod h1:NIdBknypM8iqVmPiuco0Dh6P5Jcdk8lJL0CUebqK164= +golang.org/x/text v0.37.0 h1:Cqjiwd9eSg8e0QAkyCaQTNHFIIzWtidPahFWR83rTrc= +golang.org/x/text v0.37.0/go.mod h1:a5sjxXGs9hsn/AJVwuElvCAo9v8QYLzvavO5z2PiM38= golang.org/x/time v0.0.0-20180412165947-fbb02b2291d2/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ= golang.org/x/time v0.0.0-20181108054448-85acf8d2951c/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ= golang.org/x/time v0.0.0-20190308202827-9d24e82272b4/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ=