Sanitize request path

This commit is contained in:
Romain 2025-04-17 10:02:04 +02:00 committed by GitHub
parent 299a16f0a4
commit dd5cb68cb1
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
12 changed files with 278 additions and 17 deletions

View File

@ -656,3 +656,21 @@ Please check out the [entrypoint forwarded headers connection option configurati
In `v2.11.14`, the `X-Forwarded-Prefix` header is now handled like the other `X-Forwarded-*` headers: Traefik removes it when it's sent from an untrusted source. In `v2.11.14`, the `X-Forwarded-Prefix` header is now handled like the other `X-Forwarded-*` headers: Traefik removes it when it's sent from an untrusted source.
Please refer to the Forwarded headers [documentation](../routing/entrypoints.md#forwarded-headers) for more details. Please refer to the Forwarded headers [documentation](../routing/entrypoints.md#forwarded-headers) for more details.
## v2.11.23
### Request Path Sanitization
Since `v2.11.23`, the incoming request path is now cleaned before being used to match the router rules and sent to the backends.
Any `/../`, `/./` or duplicate slash segments in the request path is interpreted and/or collapsed.
If you want to disable this behavior, you can set the [`sanitizePath` option](../routing/entrypoints.md#sanitizepath) to `false` in the entryPoint HTTP configuration.
This can be useful when dealing with legacy clients that are not url-encoding data in the request path.
For example, as base64 uses the “/” character internally,
if it's not url encoded,
it can lead to unsafe routing when the `sanitizePath` option is set to `false`.
!!! warning "Security"
Setting the `sanitizePath` option to `false` is not safe.
Ensure every request is properly url encoded instead.

View File

@ -141,6 +141,9 @@ Scheme used for the redirection. (Default: ```https```)
`--entrypoints.<name>.http.redirections.entrypoint.to`: `--entrypoints.<name>.http.redirections.entrypoint.to`:
Targeted entry point of the redirection. Targeted entry point of the redirection.
`--entrypoints.<name>.http.sanitizepath`:
Defines whether to enable request path sanitization (removal of /./, /../ and multiple slash sequences). (Default: ```true```)
`--entrypoints.<name>.http.tls`: `--entrypoints.<name>.http.tls`:
Default TLS configuration for the routers linked to the entry point. (Default: ```false```) Default TLS configuration for the routers linked to the entry point. (Default: ```false```)

View File

@ -150,6 +150,9 @@ Scheme used for the redirection. (Default: ```https```)
`TRAEFIK_ENTRYPOINTS_<NAME>_HTTP_REDIRECTIONS_ENTRYPOINT_TO`: `TRAEFIK_ENTRYPOINTS_<NAME>_HTTP_REDIRECTIONS_ENTRYPOINT_TO`:
Targeted entry point of the redirection. Targeted entry point of the redirection.
`TRAEFIK_ENTRYPOINTS_<NAME>_HTTP_SANITIZEPATH`:
Defines whether to enable request path sanitization (removal of /./, /../ and multiple slash sequences). (Default: ```true```)
`TRAEFIK_ENTRYPOINTS_<NAME>_HTTP_TLS`: `TRAEFIK_ENTRYPOINTS_<NAME>_HTTP_TLS`:
Default TLS configuration for the routers linked to the entry point. (Default: ```false```) Default TLS configuration for the routers linked to the entry point. (Default: ```false```)

View File

@ -37,6 +37,7 @@
[entryPoints.EntryPoint0.http] [entryPoints.EntryPoint0.http]
middlewares = ["foobar", "foobar"] middlewares = ["foobar", "foobar"]
encodeQuerySemicolons = true encodeQuerySemicolons = true
sanitizePath = true
[entryPoints.EntryPoint0.http.redirections] [entryPoints.EntryPoint0.http.redirections]
[entryPoints.EntryPoint0.http.redirections.entryPoint] [entryPoints.EntryPoint0.http.redirections.entryPoint]
to = "foobar" to = "foobar"

View File

@ -63,6 +63,7 @@ entryPoints:
- foobar - foobar
- foobar - foobar
encodeQuerySemicolons: true encodeQuerySemicolons: true
sanitizePath: true
http2: http2:
maxConcurrentStreams: 42 maxConcurrentStreams: 42
http3: http3:

View File

@ -994,6 +994,56 @@ entryPoints:
| false | foo=bar&baz=bar;foo | foo=bar&baz=bar&foo | | false | foo=bar&baz=bar;foo | foo=bar&baz=bar&foo |
| true | foo=bar&baz=bar;foo | foo=bar&baz=bar%3Bfoo | | true | foo=bar&baz=bar;foo | foo=bar&baz=bar%3Bfoo |
### SanitizePath
_Optional, Default=true_
The `sanitizePath` option defines whether to enable the request path sanitization.
When disabled, the incoming request path is passed to the backend as is.
This can be useful when dealing with legacy clients that are not url-encoding data in the request path.
For example, as base64 uses the “/” character internally,
if it's not url encoded,
it can lead to unsafe routing when the `sanitizePath` option is set to `false`.
!!! warning "Security"
Setting the sanitizePath option to false is not safe.
Ensure every request is properly url encoded instead.
```yaml tab="File (YAML)"
entryPoints:
websecure:
address: ':443'
http:
sanitizePath: false
```
```toml tab="File (TOML)"
[entryPoints.websecure]
address = ":443"
[entryPoints.websecure.http]
sanitizePath = false
```
```bash tab="CLI"
--entryPoints.websecure.address=:443
--entryPoints.websecure.http.sanitizePath=false
```
#### Examples
| SanitizePath | Request Path | Resulting Request Path |
|--------------|-----------------|------------------------|
| false | /./foo/bar | /./foo/bar |
| true | /./foo/bar | /foo/bar |
| false | /foo/../bar | /foo/../bar |
| true | /foo/../bar | /bar |
| false | /foo/bar// | /foo/bar// |
| true | /foo/bar// | /foo/bar/ |
| false | /./foo/../bar// | /./foo/../bar// |
| true | /./foo/../bar// | /bar/ |
### Middlewares ### Middlewares
The list of middlewares that are prepended by default to the list of middlewares of each router associated to the named entry point. The list of middlewares that are prepended by default to the list of middlewares of each router associated to the named entry point.

View File

@ -0,0 +1,41 @@
[global]
checkNewVersion = false
sendAnonymousUsage = false
[entryPoints]
[entryPoints.web]
address = ":8000"
[entryPoints.web2]
address = ":8001"
[entryPoints.web2.http]
sanitizePath = false
[log]
level = "DEBUG"
[api]
insecure = true
[providers.file]
filename = "{{ .SelfFilename }}"
# dynamic configuration
[http.routers]
[http.routers.without]
rule = "PathPrefix(`/without`)"
service = "whoami"
[http.routers.with]
rule = "PathPrefix(`/with`)"
middlewares = ["test-redirectscheme"]
service = "whoami"
[http.middlewares]
[http.middlewares.test-redirectscheme.redirectScheme]
scheme = "https"
permanent = false
[http.services]
[http.services.whoami.loadBalancer]
[[http.services.whoami.loadBalancer.servers]]
url = "{{ .Server1 }}"

View File

@ -937,11 +937,6 @@ func (s *HTTPSSuite) TestEntryPointHttpsRedirectAndPathModification() {
hosts: []string{"example.com", "example2.com", "foo.com", "foo2.com", "bar.com", "bar2.com"}, hosts: []string{"example.com", "example2.com", "foo.com", "foo2.com", "bar.com", "bar2.com"},
path: "/api/", path: "/api/",
}, },
{
desc: "Stripped URL with double trailing slash redirect",
hosts: []string{"example.com", "example2.com", "foo.com", "foo2.com", "bar.com", "bar2.com"},
path: "/api//",
},
{ {
desc: "Stripped URL with path redirect", desc: "Stripped URL with path redirect",
hosts: []string{"example.com", "example2.com", "foo.com", "foo2.com", "bar.com", "bar2.com"}, hosts: []string{"example.com", "example2.com", "foo.com", "foo2.com", "bar.com", "bar2.com"},
@ -952,21 +947,11 @@ func (s *HTTPSSuite) TestEntryPointHttpsRedirectAndPathModification() {
hosts: []string{"example.com", "example2.com", "foo.com", "foo2.com", "bar.com", "bar2.com"}, hosts: []string{"example.com", "example2.com", "foo.com", "foo2.com", "bar.com", "bar2.com"},
path: "/api/bacon/", path: "/api/bacon/",
}, },
{
desc: "Stripped URL with path and double trailing slash redirect",
hosts: []string{"example.com", "example2.com", "foo.com", "foo2.com", "bar.com", "bar2.com"},
path: "/api/bacon//",
},
{ {
desc: "Root Path with redirect", desc: "Root Path with redirect",
hosts: []string{"test.com", "test2.com", "pow.com", "pow2.com"}, hosts: []string{"test.com", "test2.com", "pow.com", "pow2.com"},
path: "/", path: "/",
}, },
{
desc: "Root Path with double trailing slash redirect",
hosts: []string{"test.com", "test2.com", "pow.com", "pow2.com"},
path: "//",
},
{ {
desc: "Path modify with redirect", desc: "Path modify with redirect",
hosts: []string{"test.com", "test2.com", "pow.com", "pow2.com"}, hosts: []string{"test.com", "test2.com", "pow.com", "pow2.com"},

View File

@ -1385,3 +1385,88 @@ func (s *SimpleSuite) TestDenyFragment() {
require.NoError(s.T(), err) require.NoError(s.T(), err)
assert.Equal(s.T(), http.StatusBadRequest, resp.StatusCode) assert.Equal(s.T(), http.StatusBadRequest, resp.StatusCode)
} }
func (s *SimpleSuite) TestSanitizePath() {
s.createComposeProject("base")
s.composeUp()
defer s.composeDown()
whoami1URL := "http://" + net.JoinHostPort(s.getComposeServiceIP("whoami1"), "80")
file := s.adaptFile("fixtures/simple_clean_path.toml", struct {
Server1 string
}{whoami1URL})
s.traefikCmd(withConfigFile(file))
err := try.GetRequest("http://127.0.0.1:8080/api/rawdata", 1*time.Second, try.BodyContains("PathPrefix(`/with`)"))
require.NoError(s.T(), err)
testCases := []struct {
desc string
request string
target string
body string
expected int
}{
{
desc: "Explicit call to the route with a middleware",
request: "GET /with HTTP/1.1\r\nHost: other.localhost\r\n\r\n",
target: "127.0.0.1:8000",
expected: http.StatusFound,
},
{
desc: "Explicit call to the route without a middleware",
request: "GET /without HTTP/1.1\r\nHost: other.localhost\r\n\r\n",
target: "127.0.0.1:8000",
expected: http.StatusOK,
body: "GET /without HTTP/1.1",
},
{
desc: "Implicit call to the route with a middleware",
request: "GET /without/../with HTTP/1.1\r\nHost: other.localhost\r\n\r\n",
target: "127.0.0.1:8000",
expected: http.StatusFound,
},
{
desc: "Explicit call to the route with a middleware, and disable path sanitization",
request: "GET /with HTTP/1.1\r\nHost: other.localhost\r\n\r\n",
target: "127.0.0.1:8001",
expected: http.StatusFound,
},
{
desc: "Explicit call to the route without a middleware, and disable path sanitization",
request: "GET /without HTTP/1.1\r\nHost: other.localhost\r\n\r\n",
target: "127.0.0.1:8001",
expected: http.StatusOK,
body: "GET /without HTTP/1.1",
},
{
desc: "Implicit call to the route with a middleware, and disable path sanitization",
request: "GET /without/../with HTTP/1.1\r\nHost: other.localhost\r\n\r\n",
target: "127.0.0.1:8001",
// The whoami is redirecting to /with, but the path is not sanitized.
expected: http.StatusMovedPermanently,
},
}
for _, test := range testCases {
conn, err := net.Dial("tcp", test.target)
require.NoError(s.T(), err)
_, err = conn.Write([]byte(test.request))
require.NoError(s.T(), err)
resp, err := http.ReadResponse(bufio.NewReader(conn), nil)
require.NoError(s.T(), err)
assert.Equalf(s.T(), test.expected, resp.StatusCode, "%s failed with %d instead of %d", test.desc, resp.StatusCode, test.expected)
if test.body != "" {
body, err := io.ReadAll(resp.Body)
require.NoError(s.T(), err)
assert.Contains(s.T(), string(body), test.body)
}
}
}

View File

@ -52,6 +52,7 @@ func (ep *EntryPoint) SetDefaults() {
ep.ForwardedHeaders = &ForwardedHeaders{} ep.ForwardedHeaders = &ForwardedHeaders{}
ep.UDP = &UDPConfig{} ep.UDP = &UDPConfig{}
ep.UDP.SetDefaults() ep.UDP.SetDefaults()
ep.HTTP.SetDefaults()
ep.HTTP2 = &HTTP2Config{} ep.HTTP2 = &HTTP2Config{}
ep.HTTP2.SetDefaults() ep.HTTP2.SetDefaults()
} }
@ -61,7 +62,14 @@ type HTTPConfig struct {
Redirections *Redirections `description:"Set of redirection" json:"redirections,omitempty" toml:"redirections,omitempty" yaml:"redirections,omitempty" export:"true"` Redirections *Redirections `description:"Set of redirection" json:"redirections,omitempty" toml:"redirections,omitempty" yaml:"redirections,omitempty" export:"true"`
Middlewares []string `description:"Default middlewares for the routers linked to the entry point." json:"middlewares,omitempty" toml:"middlewares,omitempty" yaml:"middlewares,omitempty" export:"true"` Middlewares []string `description:"Default middlewares for the routers linked to the entry point." json:"middlewares,omitempty" toml:"middlewares,omitempty" yaml:"middlewares,omitempty" export:"true"`
TLS *TLSConfig `description:"Default TLS configuration for the routers linked to the entry point." json:"tls,omitempty" toml:"tls,omitempty" yaml:"tls,omitempty" label:"allowEmpty" file:"allowEmpty" export:"true"` TLS *TLSConfig `description:"Default TLS configuration for the routers linked to the entry point." json:"tls,omitempty" toml:"tls,omitempty" yaml:"tls,omitempty" label:"allowEmpty" file:"allowEmpty" export:"true"`
EncodeQuerySemicolons bool `description:"Defines whether request query semicolons should be URLEncoded." json:"encodeQuerySemicolons,omitempty" toml:"encodeQuerySemicolons,omitempty" yaml:"encodeQuerySemicolons,omitempty"` EncodeQuerySemicolons bool `description:"Defines whether request query semicolons should be URLEncoded." json:"encodeQuerySemicolons,omitempty" toml:"encodeQuerySemicolons,omitempty" yaml:"encodeQuerySemicolons,omitempty" export:"true"`
SanitizePath *bool `description:"Defines whether to enable request path sanitization (removal of /./, /../ and multiple slash sequences)." json:"sanitizePath,omitempty" toml:"sanitizePath,omitempty" yaml:"sanitizePath,omitempty" export:"true"`
}
// SetDefaults sets the default values.
func (h *HTTPConfig) SetDefaults() {
sanitizePath := true
h.SanitizePath = &sanitizePath
} }
// HTTP2Config is the HTTP2 configuration of an entry point. // HTTP2Config is the HTTP2 configuration of an entry point.

View File

@ -571,7 +571,12 @@ func createHTTPServer(ctx context.Context, ln net.Listener, configuration *stati
return nil, err return nil, err
} }
handler = denyFragment(handler) if configuration.HTTP.SanitizePath != nil && *configuration.HTTP.SanitizePath {
// sanitizePath is used to clean the URL path by removing /../, /./ and duplicate slash sequences,
// to make sure the path is interpreted by the backends as it is evaluated inside rule matchers.
handler = sanitizePath(handler)
}
if configuration.HTTP.EncodeQuerySemicolons { if configuration.HTTP.EncodeQuerySemicolons {
handler = encodeQuerySemicolons(handler) handler = encodeQuerySemicolons(handler)
} else { } else {
@ -589,6 +594,8 @@ func createHTTPServer(ctx context.Context, ln net.Listener, configuration *stati
}) })
} }
handler = denyFragment(handler)
serverHTTP := &http.Server{ serverHTTP := &http.Server{
Handler: handler, Handler: handler,
ErrorLog: httpServerLogger, ErrorLog: httpServerLogger,
@ -713,3 +720,20 @@ func denyFragment(h http.Handler) http.Handler {
h.ServeHTTP(rw, req) h.ServeHTTP(rw, req)
}) })
} }
// sanitizePath removes the "..", "." and duplicate slash segments from the URL.
// It cleans the request URL Path and RawPath, and updates the request URI.
func sanitizePath(h http.Handler) http.Handler {
return http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) {
r2 := new(http.Request)
*r2 = *req
// Cleans the URL raw path and path.
r2.URL = r2.URL.JoinPath()
// Because the reverse proxy director is building query params from requestURI it needs to be updated as well.
r2.RequestURI = r2.URL.RequestURI()
h.ServeHTTP(rw, r2)
})
}

View File

@ -8,6 +8,7 @@ import (
"io" "io"
"net" "net"
"net/http" "net/http"
"net/http/httptest"
"strings" "strings"
"testing" "testing"
"time" "time"
@ -382,3 +383,44 @@ func TestKeepAliveH2c(t *testing.T) {
// to change. // to change.
require.Contains(t, err.Error(), "use of closed network connection") require.Contains(t, err.Error(), "use of closed network connection")
} }
func TestSanitizePath(t *testing.T) {
tests := []struct {
path string
expected string
}{
{path: "/b", expected: "/b"},
{path: "/b/", expected: "/b/"},
{path: "/../../b/", expected: "/b/"},
{path: "/../../b", expected: "/b"},
{path: "/a/b/..", expected: "/a"},
{path: "/a/b/../", expected: "/a/"},
{path: "/a/../../b", expected: "/b"},
{path: "/..///b///", expected: "/b/"},
{path: "/a/../b", expected: "/b"},
{path: "/a/./b", expected: "/a/b"},
{path: "/a//b", expected: "/a/b"},
{path: "/a/../../b", expected: "/b"},
{path: "/a/../c/../b", expected: "/b"},
{path: "/a/../../../c/../b", expected: "/b"},
{path: "/a/../c/../../b", expected: "/b"},
{path: "/a/..//c/.././b", expected: "/b"},
}
for _, test := range tests {
t.Run("Testing case: "+test.path, func(t *testing.T) {
t.Parallel()
var callCount int
clean := sanitizePath(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
callCount++
assert.Equal(t, test.expected, r.URL.Path)
}))
request := httptest.NewRequest(http.MethodGet, "http://foo"+test.path, http.NoBody)
clean.ServeHTTP(httptest.NewRecorder(), request)
assert.Equal(t, 1, callCount)
})
}
}