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, }, }