diff --git a/cmd/traefik/traefik.go b/cmd/traefik/traefik.go index 32e88791e..530e3a7bd 100644 --- a/cmd/traefik/traefik.go +++ b/cmd/traefik/traefik.go @@ -97,6 +97,11 @@ func runCmd(staticConfiguration *static.Configuration) error { return fmt.Errorf("setting up logger: %w", err) } + // 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.Warn().Msg("Starting with v3.6.3, Traefik now rejects some encoded characters in the request path by default. " + + "Refer to the documentation for more details: https://doc.traefik.io/traefik/migrate/v3/#encoded-characters-in-request-path") + http.DefaultTransport.(*http.Transport).Proxy = http.ProxyFromEnvironment staticConfiguration.SetEffectiveConfiguration() diff --git a/integration/fixtures/simple_deny.toml b/integration/fixtures/simple_deny.toml new file mode 100644 index 000000000..36168fd40 --- /dev/null +++ b/integration/fixtures/simple_deny.toml @@ -0,0 +1,20 @@ +[global] + checkNewVersion = false + sendAnonymousUsage = false + +[entryPoints] + [entryPoints.web] + address = ":8000" + +[api] + insecure = true + +[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 ed30d7428..2a19c165d 100644 --- a/integration/simple_test.go +++ b/integration/simple_test.go @@ -1695,16 +1695,16 @@ func (s *SimpleSuite) TestDenyFragment() { s.composeUp() defer s.composeDown() - s.traefikCmd(withConfigFile("fixtures/simple_default.toml")) + file := s.adaptFile("fixtures/simple_deny.toml", struct{}{}) + _, _ = s.cmdTraefik(withConfigFile(file)) - // 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..8029ad4a8 --- /dev/null +++ b/pkg/server/router/deny.go @@ -0,0 +1,62 @@ +package router + +import ( + "net/http" + "strings" + + "github.com/rs/zerolog/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.Debug().Msgf("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.Debug().Msgf("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 590dec468..02d404815 100644 --- a/pkg/server/router/router.go +++ b/pkg/server/router/router.go @@ -39,25 +39,34 @@ type serviceManager interface { // Manager A route/router manager. type Manager struct { - routerHandlers map[string]http.Handler - serviceManager serviceManager - observabilityMgr *middleware.ObservabilityMgr - middlewaresBuilder middlewareBuilder - conf *runtime.Configuration - tlsManager *tls.Manager - parser httpmuxer.SyntaxParser + routerHandlers map[string]http.Handler + serviceManager serviceManager + observabilityMgr *middleware.ObservabilityMgr + middlewaresBuilder middlewareBuilder + conf *runtime.Configuration + tlsManager *tls.Manager + parser httpmuxer.SyntaxParser + deniedEncodedPathCharacters map[string]map[string]struct{} } // NewManager creates a new Manager. -func NewManager(conf *runtime.Configuration, serviceManager serviceManager, middlewaresBuilder middlewareBuilder, observabilityMgr *middleware.ObservabilityMgr, tlsManager *tls.Manager, parser httpmuxer.SyntaxParser) *Manager { +func NewManager(conf *runtime.Configuration, + serviceManager serviceManager, + middlewaresBuilder middlewareBuilder, + observabilityMgr *middleware.ObservabilityMgr, + tlsManager *tls.Manager, + parser httpmuxer.SyntaxParser, + deniedEncodedPathCharacters map[string]map[string]struct{}, +) *Manager { return &Manager{ - routerHandlers: make(map[string]http.Handler), - serviceManager: serviceManager, - observabilityMgr: observabilityMgr, - middlewaresBuilder: middlewaresBuilder, - conf: conf, - tlsManager: tlsManager, - parser: parser, + routerHandlers: make(map[string]http.Handler), + serviceManager: serviceManager, + observabilityMgr: observabilityMgr, + middlewaresBuilder: middlewaresBuilder, + conf: conf, + tlsManager: tlsManager, + parser: parser, + deniedEncodedPathCharacters: deniedEncodedPathCharacters, } } @@ -157,7 +166,7 @@ func (m *Manager) buildEntryPointHandler(ctx context.Context, entryPointName str 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(err).Send() @@ -191,7 +200,7 @@ func (m *Manager) buildEntryPointHandler(ctx context.Context, entryPointName str 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 } @@ -207,16 +216,16 @@ 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 } m.routerHandlers[routerName] = handler - return m.routerHandlers[routerName], nil + return handler, 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)) @@ -239,7 +248,7 @@ func (m *Manager) buildHTTPHandler(ctx context.Context, router *runtime.RouterIn switch { case len(router.ChildRefs) > 0: // This router routes to child routers - create a muxer for them - nextHandler, err = m.buildChildRoutersMuxer(ctx, router.ChildRefs) + nextHandler, err = m.buildChildRoutersMuxer(ctx, entryPointName, router.ChildRefs) if err != nil { return nil, fmt.Errorf("building child routers muxer: %w", err) } @@ -266,6 +275,17 @@ func (m *Manager) buildHTTPHandler(ctx context.Context, router *runtime.RouterIn return accesslog.NewConcatFieldHandler(next, accesslog.RouterName, routerName), nil }) + // 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. + if len(router.ParentRefs) == 0 { + 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 + }) + } + mHandler := m.middlewaresBuilder.BuildChain(ctx, router.Middlewares) return chain.Extend(*mHandler).Then(nextHandler) @@ -441,7 +461,7 @@ func (m *Manager) handleCycle(victimRouter string, path []string) { } // buildChildRoutersMuxer creates a muxer for child routers. -func (m *Manager) buildChildRoutersMuxer(ctx context.Context, childRefs []string) (http.Handler, error) { +func (m *Manager) buildChildRoutersMuxer(ctx context.Context, entryPointName string, childRefs []string) (http.Handler, error) { childMuxer := httpmuxer.NewMuxer(m.parser) // Set a default handler for the child muxer (404 Not Found). @@ -468,7 +488,7 @@ func (m *Manager) buildChildRoutersMuxer(ctx context.Context, childRefs []string } // Build the child router handler. - childHandler, err := m.buildRouterHandler(ctxChild, childName, childRouter) + childHandler, err := m.buildRouterHandler(ctxChild, entryPointName, childName, childRouter) if err != nil { childRouter.AddError(err, true) logger.Error().Err(err).Send() diff --git a/pkg/server/router/router_test.go b/pkg/server/router/router_test.go index 839494f0b..d36938d16 100644 --- a/pkg/server/router/router_test.go +++ b/pkg/server/router/router_test.go @@ -18,6 +18,7 @@ import ( ptypes "github.com/traefik/paerser/types" "github.com/traefik/traefik/v3/pkg/config/dynamic" "github.com/traefik/traefik/v3/pkg/config/runtime" + "github.com/traefik/traefik/v3/pkg/config/static" "github.com/traefik/traefik/v3/pkg/middlewares/requestdecorator" httpmuxer "github.com/traefik/traefik/v3/pkg/muxer/http" "github.com/traefik/traefik/v3/pkg/server/middleware" @@ -332,7 +333,7 @@ func TestRouterManager_Get(t *testing.T) { parser, err := httpmuxer.NewSyntaxParser() require.NoError(t, err) - routerManager := NewManager(rtConf, serviceManager, middlewaresBuilder, nil, tlsManager, parser) + routerManager := NewManager(rtConf, serviceManager, middlewaresBuilder, nil, tlsManager, parser, nil) handlers := routerManager.BuildHandlers(t.Context(), test.entryPoints, false) @@ -720,7 +721,7 @@ func TestRuntimeConfiguration(t *testing.T) { parser, err := httpmuxer.NewSyntaxParser() require.NoError(t, err) - routerManager := NewManager(rtConf, serviceManager, middlewaresBuilder, nil, tlsManager, parser) + routerManager := NewManager(rtConf, serviceManager, middlewaresBuilder, nil, tlsManager, parser, nil) _ = routerManager.BuildHandlers(t.Context(), entryPoints, false) _ = routerManager.BuildHandlers(t.Context(), entryPoints, true) @@ -801,7 +802,7 @@ func TestProviderOnMiddlewares(t *testing.T) { parser, err := httpmuxer.NewSyntaxParser() require.NoError(t, err) - routerManager := NewManager(rtConf, serviceManager, middlewaresBuilder, nil, tlsManager, parser) + routerManager := NewManager(rtConf, serviceManager, middlewaresBuilder, nil, tlsManager, parser, nil) _ = routerManager.BuildHandlers(t.Context(), entryPoints, false) @@ -811,30 +812,6 @@ func TestProviderOnMiddlewares(t *testing.T) { assert.Equal(t, []string{"m1@docker", "m2@docker", "m1@file"}, rtConf.Middlewares["chain@docker"].Chain.Middlewares) } -type staticTransportManager struct { - res *http.Response -} - -func (s staticTransportManager) GetRoundTripper(_ string) (http.RoundTripper, error) { - return &staticTransport{res: s.res}, nil -} - -func (s staticTransportManager) GetTLSConfig(_ string) (*tls.Config, error) { - panic("implement me") -} - -func (s staticTransportManager) Get(_ string) (*dynamic.ServersTransport, error) { - panic("implement me") -} - -type staticTransport struct { - res *http.Response -} - -func (t *staticTransport) RoundTrip(_ *http.Request) (*http.Response, error) { - return t.res, nil -} - func BenchmarkRouterServe(b *testing.B) { server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {})) @@ -880,7 +857,7 @@ func BenchmarkRouterServe(b *testing.B) { parser, err := httpmuxer.NewSyntaxParser() require.NoError(b, err) - routerManager := NewManager(rtConf, serviceManager, middlewaresBuilder, nil, tlsManager, parser) + routerManager := NewManager(rtConf, serviceManager, middlewaresBuilder, nil, tlsManager, parser, nil) handlers := routerManager.BuildHandlers(b.Context(), entryPoints, false) @@ -1473,14 +1450,14 @@ func TestManager_buildChildRoutersMuxer(t *testing.T) { parser, err := httpmuxer.NewSyntaxParser() require.NoError(t, err) - manager := NewManager(conf, serviceManager, middlewareBuilder, nil, nil, parser) + manager := NewManager(conf, serviceManager, middlewareBuilder, nil, nil, parser, nil) // Compute multi-layer routing to populate ChildRefs manager.ParseRouterTree() // Build the child routers muxer ctx := t.Context() - muxer, err := manager.buildChildRoutersMuxer(ctx, test.childRefs) + muxer, err := manager.buildChildRoutersMuxer(ctx, "test", test.childRefs) if test.expectedError != "" { require.Error(t, err) @@ -1664,14 +1641,14 @@ func TestManager_buildHTTPHandler_WithChildRouters(t *testing.T) { parser, err := httpmuxer.NewSyntaxParser() require.NoError(t, err) - manager := NewManager(conf, serviceManager, middlewareBuilder, nil, nil, parser) + manager := NewManager(conf, serviceManager, middlewareBuilder, nil, nil, parser, nil) // Run ParseRouterTree to validate configuration and populate ChildRefs/errors manager.ParseRouterTree() // Build the HTTP handler ctx := t.Context() - handler, err := manager.buildHTTPHandler(ctx, test.router, "test-router") + handler, err := manager.buildHTTPHandler(ctx, test.router, "test", "test-router") if test.expectedError != "" { assert.Error(t, err) @@ -1696,12 +1673,10 @@ func TestManager_buildHTTPHandler_WithChildRouters(t *testing.T) { func TestManager_BuildHandlers_WithChildRouters(t *testing.T) { testCases := []struct { - desc string - routers map[string]*dynamic.Router - services map[string]*dynamic.Service - entryPoints []string - expectedEntryPoint string - expectedRequests []struct { + desc string + routers map[string]*dynamic.Router + services map[string]*dynamic.Service + expectedRequests []struct { path string statusCode int } @@ -1736,8 +1711,6 @@ func TestManager_BuildHandlers_WithChildRouters(t *testing.T) { }, }, }, - entryPoints: []string{"web"}, - expectedEntryPoint: "web", expectedRequests: []struct { path string statusCode int @@ -1779,8 +1752,6 @@ func TestManager_BuildHandlers_WithChildRouters(t *testing.T) { }, }, }, - entryPoints: []string{"web"}, - expectedEntryPoint: "web", expectedRequests: []struct { path string statusCode int @@ -1817,17 +1788,16 @@ func TestManager_BuildHandlers_WithChildRouters(t *testing.T) { parser, err := httpmuxer.NewSyntaxParser() require.NoError(t, err) - manager := NewManager(conf, serviceManager, middlewareBuilder, nil, nil, parser) + manager := NewManager(conf, serviceManager, middlewareBuilder, nil, nil, parser, nil) // Compute multi-layer routing to set up parent-child relationships manager.ParseRouterTree() // Build handlers ctx := t.Context() - handlers := manager.BuildHandlers(ctx, test.entryPoints, false) + handlers := manager.BuildHandlers(ctx, []string{"web"}, false) - require.Contains(t, handlers, test.expectedEntryPoint) - handler := handlers[test.expectedEntryPoint] + handler := handlers["web"] require.NotNil(t, handler) // Test that the handler routes requests correctly @@ -1843,8 +1813,225 @@ func TestManager_BuildHandlers_WithChildRouters(t *testing.T) { } } +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: "parent router without child routers 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: "parent router with child routers request with encoded slash", + requestPath: "/foo%2F", + routers: map[string]*dynamic.Router{ + "parent": { + EntryPoints: []string{"web"}, + Rule: "PathPrefix(`/`)", + }, + "child1": { + Rule: "PathPrefix(`/`)", + Service: "child1-service", + ParentRefs: []string{"parent"}, + }, + }, + services: map[string]*dynamic.Service{ + "child1-service": { + LoadBalancer: &dynamic.ServersLoadBalancer{ + Servers: []dynamic.Server{{URL: "http://localhost:8080"}}, + }, + }, + }, + expectedStatusCode: http.StatusBadRequest, + }, + { + desc: "parent router without child router allowing 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.StatusOK, + }, + { + desc: "parent router with child routers allowing encoded slash", + requestPath: "/foo%2F", + routers: map[string]*dynamic.Router{ + "parent": { + EntryPoints: []string{"web"}, + Rule: "PathPrefix(`/`)", + }, + "child1": { + Rule: "PathPrefix(`/`)", + Service: "child1-service", + ParentRefs: []string{"parent"}, + }, + }, + services: map[string]*dynamic.Service{ + "child1-service": { + LoadBalancer: &dynamic.ServersLoadBalancer{ + Servers: []dynamic.Server{{URL: "http://localhost:8080"}}, + }, + }, + }, + encodedCharacters: static.EncodedCharacters{ + AllowEncodedSlash: true, + }, + expectedStatusCode: http.StatusOK, + }, + { + desc: "parent router without child routers 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, + }, + { + desc: "parent router with child routers request with fragment", + requestPath: "/foo#", + routers: map[string]*dynamic.Router{ + "parent": { + EntryPoints: []string{"web"}, + Rule: "PathPrefix(`/`)", + }, + "child1": { + Rule: "Path(`/v1`)", + Service: "child1-service", + ParentRefs: []string{"parent"}, + }, + }, + services: map[string]*dynamic.Service{ + "child1-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, + } + + // Set up the manager with mocks + serviceManager := &mockServiceManager{} + middlewareBuilder := &mockMiddlewareBuilder{} + + parser, err := httpmuxer.NewSyntaxParser() + require.NoError(t, err) + + deniedEncodedPathCharacters := map[string]map[string]struct{}{"web": test.encodedCharacters.Map()} + manager := NewManager(conf, serviceManager, middlewareBuilder, nil, nil, parser, deniedEncodedPathCharacters) + + // Compute multi-layer routing to set up parent-child relationships + manager.ParseRouterTree() + + // Build handlers + ctx := t.Context() + handlers := manager.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) + }) + } +} + // Mock implementations for testing +type staticTransportManager struct { + res *http.Response +} + +func (s staticTransportManager) GetRoundTripper(_ string) (http.RoundTripper, error) { + return &staticTransport{res: s.res}, nil +} + +func (s staticTransportManager) GetTLSConfig(_ string) (*tls.Config, error) { + panic("implement me") +} + +func (s staticTransportManager) Get(_ string) (*dynamic.ServersTransport, error) { + panic("implement me") +} + +type staticTransport struct { + res *http.Response +} + +func (t *staticTransport) RoundTrip(_ *http.Request) (*http.Response, error) { + return t.res, nil +} + type mockServiceManager struct{} func (m *mockServiceManager) BuildHTTP(_ context.Context, _ string) (http.Handler, error) { diff --git a/pkg/server/routerfactory.go b/pkg/server/routerfactory.go index a2eaf296b..ffb6b8199 100644 --- a/pkg/server/routerfactory.go +++ b/pkg/server/routerfactory.go @@ -23,9 +23,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 @@ -71,21 +73,27 @@ func NewRouterFactory(staticConfiguration static.Configuration, managerFactory * } } + deniedEncodedPathCharacters := map[string]map[string]struct{}{} + for name, ep := range staticConfiguration.EntryPoints { + deniedEncodedPathCharacters[name] = ep.HTTP.EncodedCharacters.Map() + } + parser, err := httpmuxer.NewSyntaxParser() if err != nil { return nil, fmt.Errorf("creating parser: %w", err) } return &RouterFactory{ - entryPointsTCP: entryPointsTCP, - entryPointsUDP: entryPointsUDP, - managerFactory: managerFactory, - observabilityMgr: observabilityMgr, - tlsManager: tlsManager, - pluginBuilder: pluginBuilder, - dialerManager: dialerManager, - allowACMEByPass: allowACMEByPass, - parser: parser, + entryPointsTCP: entryPointsTCP, + entryPointsUDP: entryPointsUDP, + managerFactory: managerFactory, + observabilityMgr: observabilityMgr, + tlsManager: tlsManager, + pluginBuilder: pluginBuilder, + dialerManager: dialerManager, + allowACMEByPass: allowACMEByPass, + deniedEncodedPathCharacters: deniedEncodedPathCharacters, + parser: parser, }, nil } @@ -103,7 +111,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.observabilityMgr, f.tlsManager, f.parser) + routerManager := router.NewManager(rtConf, serviceManager, middlewaresBuilder, f.observabilityMgr, f.tlsManager, f.parser, f.deniedEncodedPathCharacters) routerManager.ParseRouterTree() diff --git a/pkg/server/server_entrypoint_tcp.go b/pkg/server/server_entrypoint_tcp.go index 18634f024..2b098b2f3 100644 --- a/pkg/server/server_entrypoint_tcp.go +++ b/pkg/server/server_entrypoint_tcp.go @@ -683,10 +683,6 @@ func newHTTPServer(ctx context.Context, ln net.Listener, configuration *static.E handler = normalizePath(handler) - handler = denyFragment(handler) - - handler = denyEncodedCharacters(configuration.HTTP.EncodedCharacters.Map(), handler) - serverHTTP := &http.Server{ Protocols: &protocols, Handler: handler, @@ -789,54 +785,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.Debug().Msgf("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.Debug().Msgf("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 cbcb986e5..324a2e988 100644 --- a/pkg/server/server_entrypoint_tcp_test.go +++ b/pkg/server/server_entrypoint_tcp_test.go @@ -428,59 +428,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{