From 60b19b7b81b547f26c337c308b33d5b95ba5c9be Mon Sep 17 00:00:00 2001 From: Romain Date: Tue, 16 Dec 2025 16:20:05 +0100 Subject: [PATCH] Print access logs for rejected requests and warn about new behavior --- cmd/traefik/traefik.go | 5 + integration/fixtures/simple_deny.toml | 20 ++++ integration/simple_test.go | 7 +- pkg/server/router/deny.go | 62 +++++++++++ pkg/server/router/deny_test.go | 98 +++++++++++++++++ pkg/server/router/router.go | 60 +++++++---- pkg/server/router/router_test.go | 129 ++++++++++++++++++++++- pkg/server/routerfactory.go | 32 +++--- pkg/server/server_entrypoint_tcp.go | 52 --------- pkg/server/server_entrypoint_tcp_test.go | 53 ---------- 10 files changed, 371 insertions(+), 147 deletions(-) create mode 100644 integration/fixtures/simple_deny.toml create mode 100644 pkg/server/router/deny.go create mode 100644 pkg/server/router/deny_test.go diff --git a/cmd/traefik/traefik.go b/cmd/traefik/traefik.go index b918c9f41..7900bb21f 100644 --- a/cmd/traefik/traefik.go +++ b/cmd/traefik/traefik.go @@ -87,6 +87,11 @@ Complete documentation is available at https://traefik.io`, func runCmd(staticConfiguration *static.Configuration) error { configureLogging(staticConfiguration) + // Display warning to advertise for new behavior of rejecting encoded characters in the request path. + // Deprecated: this has to be removed in the next minor/major version. + log.WithoutContext().Warnf("Starting with v2.11.32, Traefik now rejects some encoded characters in the request path by default. " + + "Refer to the documentation for more details: https://doc.traefik.io/traefik/v2.11/migration/v2/#encoded-characters-in-request-path") + http.DefaultTransport.(*http.Transport).Proxy = http.ProxyFromEnvironment if err := roundrobin.SetDefaultWeight(0); err != nil { diff --git a/integration/fixtures/simple_deny.toml b/integration/fixtures/simple_deny.toml new file mode 100644 index 000000000..1e29a3945 --- /dev/null +++ b/integration/fixtures/simple_deny.toml @@ -0,0 +1,20 @@ +[global] + checkNewVersion = false + sendAnonymousUsage = false + +[api] + insecure = true + +[entryPoints] + [entryPoints.web] + address = ":8000" + +[providers.file] + filename = "{{ .SelfFilename }}" + +## dynamic configuration ## + +[http.routers] + [http.routers.router] + service = "noop@internal" + rule = "Host(`deny.localhost`)" diff --git a/integration/simple_test.go b/integration/simple_test.go index 316c78797..d820ee6ee 100644 --- a/integration/simple_test.go +++ b/integration/simple_test.go @@ -1369,16 +1369,15 @@ func (s *SimpleSuite) TestDenyFragment() { s.composeUp() defer s.composeDown() - s.traefikCmd(withConfigFile("fixtures/simple_default.toml")) + s.traefikCmd(withConfigFile(s.adaptFile("fixtures/simple_deny.toml", struct{}{}))) - // Expected a 404 as we did not configure anything - err := try.GetRequest("http://127.0.0.1:8000/", 1*time.Second, try.StatusCodeIs(http.StatusNotFound)) + err := try.GetRequest("http://127.0.0.1:8080/api/rawdata", 1*time.Second, try.BodyContains("Host(`deny.localhost`)")) require.NoError(s.T(), err) conn, err := net.Dial("tcp", "127.0.0.1:8000") require.NoError(s.T(), err) - _, err = conn.Write([]byte("GET /#/?bar=toto;boo=titi HTTP/1.1\nHost: other.localhost\n\n")) + _, err = conn.Write([]byte("GET /#/?bar=toto;boo=titi HTTP/1.1\nHost: deny.localhost\n\n")) require.NoError(s.T(), err) resp, err := http.ReadResponse(bufio.NewReader(conn), nil) diff --git a/pkg/server/router/deny.go b/pkg/server/router/deny.go new file mode 100644 index 000000000..9c64af119 --- /dev/null +++ b/pkg/server/router/deny.go @@ -0,0 +1,62 @@ +package router + +import ( + "net/http" + "strings" + + "github.com/traefik/traefik/v2/pkg/log" +) + +// denyFragment rejects the request if the URL path contains a fragment (hash character). +// When go receives an HTTP request, it assumes the absence of fragment URL. +// However, it is still possible to send a fragment in the request. +// In this case, Traefik will encode the '#' character, altering the request's intended meaning. +// To avoid this behavior, the following function rejects requests that include a fragment in the URL. +func denyFragment(h http.Handler) http.Handler { + return http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) { + if strings.Contains(req.URL.RawPath, "#") { + log.WithoutContext().Debugf("Rejecting request because it contains a fragment in the URL path: %s", req.URL.RawPath) + rw.WriteHeader(http.StatusBadRequest) + + return + } + + h.ServeHTTP(rw, req) + }) +} + +// denyEncodedPathCharacters reject the request if the escaped path contains encoded characters in the given list. +func denyEncodedPathCharacters(encodedCharacters map[string]struct{}, h http.Handler) http.Handler { + return http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) { + if len(encodedCharacters) == 0 { + h.ServeHTTP(rw, req) + return + } + + escapedPath := req.URL.EscapedPath() + + for i := 0; i < len(escapedPath); i++ { + if escapedPath[i] != '%' { + continue + } + + // This should never happen as the standard library will reject requests containing invalid percent-encodings. + // This discards URLs with a percent character at the end. + if i+2 >= len(escapedPath) { + rw.WriteHeader(http.StatusBadRequest) + return + } + + // This rejects a request with a path containing the given encoded characters. + if _, exists := encodedCharacters[escapedPath[i:i+3]]; exists { + log.WithoutContext().Debugf("Rejecting request because it contains encoded character %s in the URL path: %s", escapedPath[i:i+3], escapedPath) + rw.WriteHeader(http.StatusBadRequest) + return + } + + i += 2 + } + + h.ServeHTTP(rw, req) + }) +} diff --git a/pkg/server/router/deny_test.go b/pkg/server/router/deny_test.go new file mode 100644 index 000000000..19ece6013 --- /dev/null +++ b/pkg/server/router/deny_test.go @@ -0,0 +1,98 @@ +package router + +import ( + "net/http" + "net/http/httptest" + "testing" + + "github.com/stretchr/testify/assert" +) + +func Test_denyFragment(t *testing.T) { + tests := []struct { + name string + url string + wantStatus int + }{ + { + name: "Rejects fragment character", + url: "http://example.com/#", + wantStatus: http.StatusBadRequest, + }, + { + name: "Allows without fragment", + url: "http://example.com/", + wantStatus: http.StatusOK, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + t.Parallel() + + handler := denyFragment(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + })) + + req := httptest.NewRequest(http.MethodGet, test.url, nil) + res := httptest.NewRecorder() + + handler.ServeHTTP(res, req) + + assert.Equal(t, test.wantStatus, res.Code) + }) + } +} + +func Test_denyEncodedPathCharacters(t *testing.T) { + tests := []struct { + name string + encoded map[string]struct{} + url string + wantStatus int + }{ + { + name: "Rejects disallowed characters", + encoded: map[string]struct{}{ + "%0A": {}, + "%0D": {}, + }, + url: "http://example.com/foo%0Abar", + wantStatus: http.StatusBadRequest, + }, + { + name: "Allows valid paths", + encoded: map[string]struct{}{ + "%0A": {}, + "%0D": {}, + }, + url: "http://example.com/foo%20bar", + wantStatus: http.StatusOK, + }, + { + name: "Handles empty path", + encoded: map[string]struct{}{ + "%0A": {}, + }, + url: "http://example.com/", + wantStatus: http.StatusOK, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + t.Parallel() + + handler := denyEncodedPathCharacters(test.encoded, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + })) + + req := httptest.NewRequest(http.MethodGet, test.url, nil) + res := httptest.NewRecorder() + + handler.ServeHTTP(res, req) + + assert.Equal(t, test.wantStatus, res.Code) + }) + } +} diff --git a/pkg/server/router/router.go b/pkg/server/router/router.go index 0b88b0faf..519ff975b 100644 --- a/pkg/server/router/router.go +++ b/pkg/server/router/router.go @@ -36,25 +36,34 @@ type serviceManager interface { // Manager A route/router manager. type Manager struct { - routerHandlers map[string]http.Handler - serviceManager serviceManager - metricsRegistry metrics.Registry - middlewaresBuilder middlewareBuilder - chainBuilder *middleware.ChainBuilder - conf *runtime.Configuration - tlsManager *tls.Manager + routerHandlers map[string]http.Handler + serviceManager serviceManager + metricsRegistry metrics.Registry + middlewaresBuilder middlewareBuilder + chainBuilder *middleware.ChainBuilder + conf *runtime.Configuration + tlsManager *tls.Manager + deniedEncodedPathCharacters map[string]map[string]struct{} } // NewManager creates a new Manager. -func NewManager(conf *runtime.Configuration, serviceManager serviceManager, middlewaresBuilder middlewareBuilder, chainBuilder *middleware.ChainBuilder, metricsRegistry metrics.Registry, tlsManager *tls.Manager) *Manager { +func NewManager(conf *runtime.Configuration, + serviceManager serviceManager, + middlewaresBuilder middlewareBuilder, + chainBuilder *middleware.ChainBuilder, + metricsRegistry metrics.Registry, + tlsManager *tls.Manager, + deniedEncodedPathCharacters map[string]map[string]struct{}, +) *Manager { return &Manager{ - routerHandlers: make(map[string]http.Handler), - serviceManager: serviceManager, - metricsRegistry: metricsRegistry, - middlewaresBuilder: middlewaresBuilder, - chainBuilder: chainBuilder, - conf: conf, - tlsManager: tlsManager, + routerHandlers: make(map[string]http.Handler), + serviceManager: serviceManager, + metricsRegistry: metricsRegistry, + middlewaresBuilder: middlewaresBuilder, + chainBuilder: chainBuilder, + conf: conf, + tlsManager: tlsManager, + deniedEncodedPathCharacters: deniedEncodedPathCharacters, } } @@ -73,7 +82,7 @@ func (m *Manager) BuildHandlers(rootCtx context.Context, entryPoints []string, t for entryPointName, routers := range m.getHTTPRouters(rootCtx, entryPoints, tls) { ctx := log.With(rootCtx, log.Str(log.EntryPointName, entryPointName)) - handler, err := m.buildEntryPointHandler(ctx, routers) + handler, err := m.buildEntryPointHandler(ctx, entryPointName, routers) if err != nil { log.FromContext(ctx).Error(err) continue @@ -109,7 +118,7 @@ func (m *Manager) BuildHandlers(rootCtx context.Context, entryPoints []string, t return entryPointHandlers } -func (m *Manager) buildEntryPointHandler(ctx context.Context, configs map[string]*runtime.RouterInfo) (http.Handler, error) { +func (m *Manager) buildEntryPointHandler(ctx context.Context, entryPointName string, configs map[string]*runtime.RouterInfo) (http.Handler, error) { muxer, err := httpmuxer.NewMuxer() if err != nil { return nil, err @@ -126,7 +135,7 @@ func (m *Manager) buildEntryPointHandler(ctx context.Context, configs map[string continue } - handler, err := m.buildRouterHandler(ctxRouter, routerName, routerConfig) + handler, err := m.buildRouterHandler(ctxRouter, entryPointName, routerName, routerConfig) if err != nil { routerConfig.AddError(err, true) logger.Error(err) @@ -151,7 +160,7 @@ func (m *Manager) buildEntryPointHandler(ctx context.Context, configs map[string return chain.Then(muxer) } -func (m *Manager) buildRouterHandler(ctx context.Context, routerName string, routerConfig *runtime.RouterInfo) (http.Handler, error) { +func (m *Manager) buildRouterHandler(ctx context.Context, entryPointName, routerName string, routerConfig *runtime.RouterInfo) (http.Handler, error) { if handler, ok := m.routerHandlers[routerName]; ok { return handler, nil } @@ -167,7 +176,7 @@ func (m *Manager) buildRouterHandler(ctx context.Context, routerName string, rou } } - handler, err := m.buildHTTPHandler(ctx, routerConfig, routerName) + handler, err := m.buildHTTPHandler(ctx, routerConfig, entryPointName, routerName) if err != nil { return nil, err } @@ -185,7 +194,7 @@ func (m *Manager) buildRouterHandler(ctx context.Context, routerName string, rou return m.routerHandlers[routerName], nil } -func (m *Manager) buildHTTPHandler(ctx context.Context, router *runtime.RouterInfo, routerName string) (http.Handler, error) { +func (m *Manager) buildHTTPHandler(ctx context.Context, router *runtime.RouterInfo, entryPointName, routerName string) (http.Handler, error) { var qualifiedNames []string for _, name := range router.Middlewares { qualifiedNames = append(qualifiedNames, provider.GetQualifiedName(ctx, name)) @@ -217,6 +226,15 @@ func (m *Manager) buildHTTPHandler(ctx context.Context, router *runtime.RouterIn chain = chain.Append(denyrouterrecursion.WrapHandler(routerName)) } + // Here we are adding deny handlers for encoded path characters and fragment. + // Deny handler are only added for root routers, child routers are protected by their parent router deny handlers. + chain = chain.Append(func(next http.Handler) (http.Handler, error) { + return denyFragment(next), nil + }) + chain = chain.Append(func(next http.Handler) (http.Handler, error) { + return denyEncodedPathCharacters(m.deniedEncodedPathCharacters[entryPointName], next), nil + }) + return chain.Extend(*mHandler).Append(tHandler).Then(sHandler) } diff --git a/pkg/server/router/router_test.go b/pkg/server/router/router_test.go index c1c5e3b8a..7ade16d54 100644 --- a/pkg/server/router/router_test.go +++ b/pkg/server/router/router_test.go @@ -13,6 +13,7 @@ import ( "github.com/stretchr/testify/require" "github.com/traefik/traefik/v2/pkg/config/dynamic" "github.com/traefik/traefik/v2/pkg/config/runtime" + "github.com/traefik/traefik/v2/pkg/config/static" "github.com/traefik/traefik/v2/pkg/metrics" "github.com/traefik/traefik/v2/pkg/middlewares/accesslog" "github.com/traefik/traefik/v2/pkg/middlewares/capture" @@ -319,7 +320,7 @@ func TestRouterManager_Get(t *testing.T) { chainBuilder := middleware.NewChainBuilder(nil, nil, nil) tlsManager := tls.NewManager() - routerManager := NewManager(rtConf, serviceManager, middlewaresBuilder, chainBuilder, metrics.NewVoidRegistry(), tlsManager) + routerManager := NewManager(rtConf, serviceManager, middlewaresBuilder, chainBuilder, metrics.NewVoidRegistry(), tlsManager, nil) handlers := routerManager.BuildHandlers(t.Context(), test.entryPoints, false) @@ -426,7 +427,7 @@ func TestAccessLog(t *testing.T) { chainBuilder := middleware.NewChainBuilder(nil, nil, nil) tlsManager := tls.NewManager() - routerManager := NewManager(rtConf, serviceManager, middlewaresBuilder, chainBuilder, metrics.NewVoidRegistry(), tlsManager) + routerManager := NewManager(rtConf, serviceManager, middlewaresBuilder, chainBuilder, metrics.NewVoidRegistry(), tlsManager, nil) handlers := routerManager.BuildHandlers(t.Context(), test.entryPoints, false) @@ -814,7 +815,7 @@ func TestRuntimeConfiguration(t *testing.T) { tlsManager := tls.NewManager() tlsManager.UpdateConfigs(t.Context(), nil, test.tlsOptions, nil) - routerManager := NewManager(rtConf, serviceManager, middlewaresBuilder, chainBuilder, metrics.NewVoidRegistry(), tlsManager) + routerManager := NewManager(rtConf, serviceManager, middlewaresBuilder, chainBuilder, metrics.NewVoidRegistry(), tlsManager, nil) _ = routerManager.BuildHandlers(t.Context(), entryPoints, false) _ = routerManager.BuildHandlers(t.Context(), entryPoints, true) @@ -891,7 +892,7 @@ func TestProviderOnMiddlewares(t *testing.T) { chainBuilder := middleware.NewChainBuilder(nil, nil, nil) tlsManager := tls.NewManager() - routerManager := NewManager(rtConf, serviceManager, middlewaresBuilder, chainBuilder, metrics.NewVoidRegistry(), tlsManager) + routerManager := NewManager(rtConf, serviceManager, middlewaresBuilder, chainBuilder, metrics.NewVoidRegistry(), tlsManager, nil) _ = routerManager.BuildHandlers(t.Context(), entryPoints, false) @@ -901,6 +902,124 @@ func TestProviderOnMiddlewares(t *testing.T) { assert.Equal(t, []string{"m1@docker", "m2@docker", "m1@file"}, rtConf.Middlewares["chain@docker"].Chain.Middlewares) } +func TestManager_BuildHandlers_Deny(t *testing.T) { + testCases := []struct { + desc string + routers map[string]*dynamic.Router + services map[string]*dynamic.Service + requestPath string + encodedCharacters static.EncodedCharacters + expectedStatusCode int + }{ + { + desc: "unallowed request with encoded slash", + requestPath: "/foo%2F", + routers: map[string]*dynamic.Router{ + "parent": { + EntryPoints: []string{"web"}, + Rule: "PathPrefix(`/`)", + Service: "service", + }, + }, + services: map[string]*dynamic.Service{ + "service": { + LoadBalancer: &dynamic.ServersLoadBalancer{ + Servers: []dynamic.Server{{URL: "http://localhost:8080"}}, + }, + }, + }, + expectedStatusCode: http.StatusBadRequest, + }, + { + desc: "allowed request with encoded slash", + requestPath: "/foo%2F", + routers: map[string]*dynamic.Router{ + "parent": { + EntryPoints: []string{"web"}, + Rule: "PathPrefix(`/`)", + Service: "service", + }, + }, + services: map[string]*dynamic.Service{ + "service": { + LoadBalancer: &dynamic.ServersLoadBalancer{ + Servers: []dynamic.Server{{URL: "http://localhost:8080"}}, + }, + }, + }, + encodedCharacters: static.EncodedCharacters{ + AllowEncodedSlash: true, + }, + expectedStatusCode: http.StatusBadGateway, + }, + { + desc: "unallowed request with fragment", + requestPath: "/foo#", + routers: map[string]*dynamic.Router{ + "parent": { + EntryPoints: []string{"web"}, + Rule: "PathPrefix(`/`)", + Service: "service", + }, + }, + services: map[string]*dynamic.Service{ + "service": { + LoadBalancer: &dynamic.ServersLoadBalancer{ + Servers: []dynamic.Server{{URL: "http://localhost:8080"}}, + }, + }, + }, + expectedStatusCode: http.StatusBadRequest, + }, + } + + for _, test := range testCases { + t.Run(test.desc, func(t *testing.T) { + // Create runtime routers + runtimeRouters := make(map[string]*runtime.RouterInfo) + for name, router := range test.routers { + runtimeRouters[name] = &runtime.RouterInfo{ + Router: router, + } + } + + // Create runtime services + runtimeServices := make(map[string]*runtime.ServiceInfo) + for name, service := range test.services { + runtimeServices[name] = &runtime.ServiceInfo{ + Service: service, + } + } + + conf := &runtime.Configuration{ + Routers: runtimeRouters, + Services: runtimeServices, + } + + deniedEncodedPathCharacters := map[string]map[string]struct{}{"web": test.encodedCharacters.Map()} + roundTripperManager := service.NewRoundTripperManager() + roundTripperManager.Update(map[string]*dynamic.ServersTransport{"default@internal": {}}) + serviceManager := service.NewManager(conf.Services, nil, nil, roundTripperManager) + middlewaresBuilder := middleware.NewBuilder(conf.Middlewares, serviceManager, nil) + chainBuilder := middleware.NewChainBuilder(nil, nil, nil) + tlsManager := tls.NewManager() + + routerManager := NewManager(conf, serviceManager, middlewaresBuilder, chainBuilder, metrics.NewVoidRegistry(), tlsManager, deniedEncodedPathCharacters) + + // Build handlers + ctx := t.Context() + handlers := routerManager.BuildHandlers(ctx, []string{"web"}, false) + + recorder := httptest.NewRecorder() + request := httptest.NewRequest(http.MethodGet, test.requestPath, http.NoBody) + + handlers["web"].ServeHTTP(recorder, request) + + assert.Equal(t, test.expectedStatusCode, recorder.Code) + }) + } +} + type staticRoundTripperGetter struct { res *http.Response } @@ -960,7 +1079,7 @@ func BenchmarkRouterServe(b *testing.B) { chainBuilder := middleware.NewChainBuilder(nil, nil, nil) tlsManager := tls.NewManager() - routerManager := NewManager(rtConf, serviceManager, middlewaresBuilder, chainBuilder, metrics.NewVoidRegistry(), tlsManager) + routerManager := NewManager(rtConf, serviceManager, middlewaresBuilder, chainBuilder, metrics.NewVoidRegistry(), tlsManager, nil) handlers := routerManager.BuildHandlers(b.Context(), entryPoints, false) diff --git a/pkg/server/routerfactory.go b/pkg/server/routerfactory.go index e2941028e..5b8162481 100644 --- a/pkg/server/routerfactory.go +++ b/pkg/server/routerfactory.go @@ -21,9 +21,11 @@ import ( // RouterFactory the factory of TCP/UDP routers. type RouterFactory struct { - entryPointsTCP []string - entryPointsUDP []string - allowACMEByPass map[string]bool + entryPointsTCP []string + entryPointsUDP []string + + allowACMEByPass map[string]bool + deniedEncodedPathCharacters map[string]map[string]struct{} managerFactory *service.ManagerFactory @@ -64,15 +66,21 @@ func NewRouterFactory(staticConfiguration static.Configuration, managerFactory * } } + deniedEncodedPathCharacters := map[string]map[string]struct{}{} + for name, ep := range staticConfiguration.EntryPoints { + deniedEncodedPathCharacters[name] = ep.HTTP.EncodedCharacters.Map() + } + return &RouterFactory{ - entryPointsTCP: entryPointsTCP, - entryPointsUDP: entryPointsUDP, - managerFactory: managerFactory, - metricsRegistry: metricsRegistry, - tlsManager: tlsManager, - chainBuilder: chainBuilder, - pluginBuilder: pluginBuilder, - allowACMEByPass: allowACMEByPass, + entryPointsTCP: entryPointsTCP, + entryPointsUDP: entryPointsUDP, + managerFactory: managerFactory, + metricsRegistry: metricsRegistry, + tlsManager: tlsManager, + chainBuilder: chainBuilder, + pluginBuilder: pluginBuilder, + allowACMEByPass: allowACMEByPass, + deniedEncodedPathCharacters: deniedEncodedPathCharacters, } } @@ -85,7 +93,7 @@ func (f *RouterFactory) CreateRouters(rtConf *runtime.Configuration) (map[string middlewaresBuilder := middleware.NewBuilder(rtConf.Middlewares, serviceManager, f.pluginBuilder) - routerManager := router.NewManager(rtConf, serviceManager, middlewaresBuilder, f.chainBuilder, f.metricsRegistry, f.tlsManager) + routerManager := router.NewManager(rtConf, serviceManager, middlewaresBuilder, f.chainBuilder, f.metricsRegistry, f.tlsManager, f.deniedEncodedPathCharacters) handlersNonTLS := routerManager.BuildHandlers(ctx, f.entryPointsTCP, false) handlersTLS := routerManager.BuildHandlers(ctx, f.entryPointsTCP, true) diff --git a/pkg/server/server_entrypoint_tcp.go b/pkg/server/server_entrypoint_tcp.go index feff5f8d3..51b946c09 100644 --- a/pkg/server/server_entrypoint_tcp.go +++ b/pkg/server/server_entrypoint_tcp.go @@ -606,10 +606,6 @@ func createHTTPServer(ctx context.Context, ln net.Listener, configuration *stati handler = normalizePath(handler) - handler = denyFragment(handler) - - handler = denyEncodedCharacters(configuration.HTTP.EncodedCharacters.Map(), handler) - serverHTTP := &http.Server{ Protocols: &protocols, Handler: handler, @@ -709,54 +705,6 @@ func encodeQuerySemicolons(h http.Handler) http.Handler { }) } -// denyEncodedCharacters reject the request if the escaped path contains encoded characters. -func denyEncodedCharacters(encodedCharacters map[string]struct{}, h http.Handler) http.Handler { - return http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) { - escapedPath := req.URL.EscapedPath() - - for i := 0; i < len(escapedPath); i++ { - if escapedPath[i] != '%' { - continue - } - - // This should never happen as the standard library will reject requests containing invalid percent-encodings. - // This discards URLs with a percent character at the end. - if i+2 >= len(escapedPath) { - rw.WriteHeader(http.StatusBadRequest) - return - } - - // This rejects a request with a path containing the given encoded characters. - if _, exists := encodedCharacters[escapedPath[i:i+3]]; exists { - log.FromContext(req.Context()).Debugf("Rejecting request because it contains encoded character %s in the URL path: %s", escapedPath[i:i+3], escapedPath) - rw.WriteHeader(http.StatusBadRequest) - return - } - - i += 2 - } - - h.ServeHTTP(rw, req) - }) -} - -// When go receives an HTTP request, it assumes the absence of fragment URL. -// However, it is still possible to send a fragment in the request. -// In this case, Traefik will encode the '#' character, altering the request's intended meaning. -// To avoid this behavior, the following function rejects requests that include a fragment in the URL. -func denyFragment(h http.Handler) http.Handler { - return http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) { - if strings.Contains(req.URL.RawPath, "#") { - log.WithoutContext().Debugf("Rejecting request because it contains a fragment in the URL path: %s", req.URL.RawPath) - rw.WriteHeader(http.StatusBadRequest) - - return - } - - h.ServeHTTP(rw, req) - }) -} - // sanitizePath removes the "..", "." and duplicate slash segments from the URL according to https://datatracker.ietf.org/doc/html/rfc3986#section-6.2.2.3. // It cleans the request URL Path and RawPath, and updates the request URI. func sanitizePath(h http.Handler) http.Handler { diff --git a/pkg/server/server_entrypoint_tcp_test.go b/pkg/server/server_entrypoint_tcp_test.go index 2d6bda4f3..9b134f2e7 100644 --- a/pkg/server/server_entrypoint_tcp_test.go +++ b/pkg/server/server_entrypoint_tcp_test.go @@ -429,59 +429,6 @@ func TestSanitizePath(t *testing.T) { } } -func TestDenyEncodedCharacters(t *testing.T) { - tests := []struct { - name string - encoded map[string]struct{} - url string - wantStatus int - }{ - { - name: "Rejects disallowed characters", - encoded: map[string]struct{}{ - "%0A": {}, - "%0D": {}, - }, - url: "http://example.com/foo%0Abar", - wantStatus: http.StatusBadRequest, - }, - { - name: "Allows valid paths", - encoded: map[string]struct{}{ - "%0A": {}, - "%0D": {}, - }, - url: "http://example.com/foo%20bar", - wantStatus: http.StatusOK, - }, - { - name: "Handles empty path", - encoded: map[string]struct{}{ - "%0A": {}, - }, - url: "http://example.com/", - wantStatus: http.StatusOK, - }, - } - - for _, test := range tests { - t.Run(test.name, func(t *testing.T) { - t.Parallel() - - handler := denyEncodedCharacters(test.encoded, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.WriteHeader(http.StatusOK) - })) - - req := httptest.NewRequest(http.MethodGet, test.url, nil) - res := httptest.NewRecorder() - - handler.ServeHTTP(res, req) - - assert.Equal(t, test.wantStatus, res.Code) - }) - } -} - func TestNormalizePath(t *testing.T) { unreservedDecoded := "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789-._~" unreserved := []string{