Make encoded character options opt-in

pkg/config/dynamic/http_config.go

@@ -43,14 +43,14 @@ type Service struct {
 
 // Router holds the router configuration.
 type Router struct {
-	EntryPoints                 []string                          `json:"entryPoints,omitempty" toml:"entryPoints,omitempty" yaml:"entryPoints,omitempty" export:"true"`
-	Middlewares                 []string                          `json:"middlewares,omitempty" toml:"middlewares,omitempty" yaml:"middlewares,omitempty" export:"true"`
-	Service                     string                            `json:"service,omitempty" toml:"service,omitempty" yaml:"service,omitempty" export:"true"`
-	Rule                        string                            `json:"rule,omitempty" toml:"rule,omitempty" yaml:"rule,omitempty"`
-	Priority                    int                               `json:"priority,omitempty" toml:"priority,omitempty,omitzero" yaml:"priority,omitempty" export:"true"`
-	TLS                         *RouterTLSConfig                  `json:"tls,omitempty" toml:"tls,omitempty" yaml:"tls,omitempty" label:"allowEmpty" file:"allowEmpty" kv:"allowEmpty" export:"true"`
-	DefaultRule                 bool                              `json:"-" toml:"-" yaml:"-" label:"-" file:"-"`
-	DeniedEncodedPathCharacters RouterDeniedEncodedPathCharacters `json:"-" toml:"-" yaml:"-" label:"-" file:"-"`
+	EntryPoints                 []string                           `json:"entryPoints,omitempty" toml:"entryPoints,omitempty" yaml:"entryPoints,omitempty" export:"true"`
+	Middlewares                 []string                           `json:"middlewares,omitempty" toml:"middlewares,omitempty" yaml:"middlewares,omitempty" export:"true"`
+	Service                     string                             `json:"service,omitempty" toml:"service,omitempty" yaml:"service,omitempty" export:"true"`
+	Rule                        string                             `json:"rule,omitempty" toml:"rule,omitempty" yaml:"rule,omitempty"`
+	Priority                    int                                `json:"priority,omitempty" toml:"priority,omitempty,omitzero" yaml:"priority,omitempty" export:"true"`
+	TLS                         *RouterTLSConfig                   `json:"tls,omitempty" toml:"tls,omitempty" yaml:"tls,omitempty" label:"allowEmpty" file:"allowEmpty" kv:"allowEmpty" export:"true"`
+	DefaultRule                 bool                               `json:"-" toml:"-" yaml:"-" label:"-" file:"-"`
+	DeniedEncodedPathCharacters *RouterDeniedEncodedPathCharacters `json:"-" toml:"-" yaml:"-" label:"-" file:"-" kv:"-"`
 }
 
 // +k8s:deepcopy-gen=true

pkg/config/dynamic/zz_generated.deepcopy.go

@@ -1035,7 +1035,11 @@ func (in *Router) DeepCopyInto(out *Router) {
 		*out = new(RouterTLSConfig)
 		(*in).DeepCopyInto(*out)
 	}
-	out.DeniedEncodedPathCharacters = in.DeniedEncodedPathCharacters
+	if in.DeniedEncodedPathCharacters != nil {
+		in, out := &in.DeniedEncodedPathCharacters, &out.DeniedEncodedPathCharacters
+		*out = new(RouterDeniedEncodedPathCharacters)
+		**out = **in
+	}
 	return
 }
 

pkg/config/static/entrypoints.go

@@ -84,6 +84,16 @@ type EncodedCharacters struct {
 	AllowEncodedHash          bool `description:"Defines whether requests with encoded hash characters in the path are allowed." json:"allowEncodedHash,omitempty" toml:"allowEncodedHash,omitempty" yaml:"allowEncodedHash,omitempty" export:"true"`
 }
 
+func (ec *EncodedCharacters) SetDefaults() {
+	ec.AllowEncodedSlash = true
+	ec.AllowEncodedBackSlash = true
+	ec.AllowEncodedNullCharacter = true
+	ec.AllowEncodedSemicolon = true
+	ec.AllowEncodedPercent = true
+	ec.AllowEncodedQuestionMark = true
+	ec.AllowEncodedHash = true
+}
+
 // HTTP2Config is the HTTP2 configuration of an entry point.
 type HTTP2Config struct {
 	MaxConcurrentStreams int32 `description:"Specifies the number of concurrent streams per connection that each client is allowed to initiate." json:"maxConcurrentStreams,omitempty" toml:"maxConcurrentStreams,omitempty" yaml:"maxConcurrentStreams,omitempty" export:"true"`

pkg/server/aggregator.go

@@ -167,7 +167,7 @@ func applyModel(cfg dynamic.Configuration) dynamic.Configuration {
 				if m.DeniedEncodedPathCharacters != nil {
 					// As the denied encoded path characters option is not configurable at the router level,
 					// we can simply copy the whole structure to override the router's default config.
-					cp.DeniedEncodedPathCharacters = *m.DeniedEncodedPathCharacters
+					cp.DeniedEncodedPathCharacters = m.DeniedEncodedPathCharacters
 				}
 
 				rtName := name

pkg/server/router/deny.go

@@ -2,29 +2,10 @@ 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) {

pkg/server/router/deny_test.go

@@ -8,42 +8,6 @@ import (
 	"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

pkg/server/router/router.go

@@ -223,14 +223,11 @@ 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(router.DeniedEncodedPathCharacters.Map(), next), nil
-	})
+	if router.DeniedEncodedPathCharacters != nil {
+		chain = chain.Append(func(next http.Handler) (http.Handler, error) {
+			return denyEncodedPathCharacters(router.DeniedEncodedPathCharacters.Map(), next), nil
+		})
+	}
 
 	return chain.Extend(*mHandler).Append(tHandler).Then(sHandler)
 }

pkg/server/router/router_test.go

@@ -910,13 +910,16 @@ func TestManager_BuildHandlers_Deny(t *testing.T) {
 		expectedStatusCode int
 	}{
 		{
-			desc:        "unallowed request with encoded slash",
+			desc:        "disallow request with encoded slash",
 			requestPath: "/foo%2F",
 			routers: map[string]*dynamic.Router{
 				"parent": {
 					EntryPoints: []string{"web"},
 					Rule:        "PathPrefix(`/`)",
 					Service:     "service",
+					DeniedEncodedPathCharacters: &dynamic.RouterDeniedEncodedPathCharacters{
+						AllowEncodedSlash: false,
+					},
 				},
 			},
 			services: map[string]*dynamic.Service{
@@ -936,9 +939,6 @@ func TestManager_BuildHandlers_Deny(t *testing.T) {
 					EntryPoints: []string{"web"},
 					Rule:        "PathPrefix(`/`)",
 					Service:     "service",
-					DeniedEncodedPathCharacters: dynamic.RouterDeniedEncodedPathCharacters{
-						AllowEncodedSlash: true,
-					},
 				},
 			},
 			services: map[string]*dynamic.Service{
@@ -950,25 +950,6 @@ func TestManager_BuildHandlers_Deny(t *testing.T) {
 			},
 			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 {

pkg/server/server_entrypoint_tcp.go

@@ -606,6 +606,8 @@ func createHTTPServer(ctx context.Context, ln net.Listener, configuration *stati
 
 	handler = normalizePath(handler)
 
+	handler = denyFragment(handler)
+
 	serverHTTP := &http.Server{
 		Protocols:    &protocols,
 		Handler:      handler,
@@ -685,6 +687,24 @@ func (t *trackedConnection) Close() error {
 	return t.WriteCloser.Close()
 }
 
+// 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)
+	})
+}
+
 // This function is inspired by http.AllowQuerySemicolons.
 func encodeQuerySemicolons(h http.Handler) http.Handler {
 	return http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) {

pkg/server/server_entrypoint_tcp_test.go

@@ -388,6 +388,42 @@ func TestKeepAliveH2c(t *testing.T) {
 	require.Contains(t, err.Error(), "use of closed network connection")
 }
 
+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 TestSanitizePath(t *testing.T) {
 	tests := []struct {
 		path     string