1
0
Fork 0

Fix deny encoded characters

Co-authored-by: Kevin Pollet <pollet.kevin@gmail.com>
This commit is contained in:
Romain 2025-12-23 16:02:04 +01:00 committed by GitHub
parent 8e6ce08f33
commit 23788e90cb
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
21 changed files with 435 additions and 303 deletions

View file

@ -212,6 +212,12 @@ func applyModel(cfg dynamic.Configuration) dynamic.Configuration {
cp.Middlewares = append(m.Middlewares, cp.Middlewares...)
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
}
if cp.Observability == nil {
cp.Observability = &dynamic.RouterObservabilityConfig{}
}

View file

@ -39,14 +39,13 @@ 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
deniedEncodedPathCharacters map[string]map[string]struct{}
routerHandlers map[string]http.Handler
serviceManager serviceManager
observabilityMgr *middleware.ObservabilityMgr
middlewaresBuilder middlewareBuilder
conf *runtime.Configuration
tlsManager *tls.Manager
parser httpmuxer.SyntaxParser
}
// NewManager creates a new Manager.
@ -56,17 +55,15 @@ func NewManager(conf *runtime.Configuration,
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,
deniedEncodedPathCharacters: deniedEncodedPathCharacters,
routerHandlers: make(map[string]http.Handler),
serviceManager: serviceManager,
observabilityMgr: observabilityMgr,
middlewaresBuilder: middlewaresBuilder,
conf: conf,
tlsManager: tlsManager,
parser: parser,
}
}
@ -282,7 +279,7 @@ func (m *Manager) buildHTTPHandler(ctx context.Context, router *runtime.RouterIn
return denyFragment(next), nil
})
chain = chain.Append(func(next http.Handler) (http.Handler, error) {
return denyEncodedPathCharacters(m.deniedEncodedPathCharacters[entryPointName], next), nil
return denyEncodedPathCharacters(router.DeniedEncodedPathCharacters.Map(), next), nil
})
}

View file

@ -18,7 +18,6 @@ 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"
@ -333,7 +332,7 @@ func TestRouterManager_Get(t *testing.T) {
parser, err := httpmuxer.NewSyntaxParser()
require.NoError(t, err)
routerManager := NewManager(rtConf, serviceManager, middlewaresBuilder, nil, tlsManager, parser, nil)
routerManager := NewManager(rtConf, serviceManager, middlewaresBuilder, nil, tlsManager, parser)
handlers := routerManager.BuildHandlers(t.Context(), test.entryPoints, false)
@ -721,7 +720,7 @@ func TestRuntimeConfiguration(t *testing.T) {
parser, err := httpmuxer.NewSyntaxParser()
require.NoError(t, err)
routerManager := NewManager(rtConf, serviceManager, middlewaresBuilder, nil, tlsManager, parser, nil)
routerManager := NewManager(rtConf, serviceManager, middlewaresBuilder, nil, tlsManager, parser)
_ = routerManager.BuildHandlers(t.Context(), entryPoints, false)
_ = routerManager.BuildHandlers(t.Context(), entryPoints, true)
@ -802,7 +801,7 @@ func TestProviderOnMiddlewares(t *testing.T) {
parser, err := httpmuxer.NewSyntaxParser()
require.NoError(t, err)
routerManager := NewManager(rtConf, serviceManager, middlewaresBuilder, nil, tlsManager, parser, nil)
routerManager := NewManager(rtConf, serviceManager, middlewaresBuilder, nil, tlsManager, parser)
_ = routerManager.BuildHandlers(t.Context(), entryPoints, false)
@ -857,7 +856,7 @@ func BenchmarkRouterServe(b *testing.B) {
parser, err := httpmuxer.NewSyntaxParser()
require.NoError(b, err)
routerManager := NewManager(rtConf, serviceManager, middlewaresBuilder, nil, tlsManager, parser, nil)
routerManager := NewManager(rtConf, serviceManager, middlewaresBuilder, nil, tlsManager, parser)
handlers := routerManager.BuildHandlers(b.Context(), entryPoints, false)
@ -1450,7 +1449,7 @@ func TestManager_buildChildRoutersMuxer(t *testing.T) {
parser, err := httpmuxer.NewSyntaxParser()
require.NoError(t, err)
manager := NewManager(conf, serviceManager, middlewareBuilder, nil, nil, parser, nil)
manager := NewManager(conf, serviceManager, middlewareBuilder, nil, nil, parser)
// Compute multi-layer routing to populate ChildRefs
manager.ParseRouterTree()
@ -1641,7 +1640,7 @@ func TestManager_buildHTTPHandler_WithChildRouters(t *testing.T) {
parser, err := httpmuxer.NewSyntaxParser()
require.NoError(t, err)
manager := NewManager(conf, serviceManager, middlewareBuilder, nil, nil, parser, nil)
manager := NewManager(conf, serviceManager, middlewareBuilder, nil, nil, parser)
// Run ParseRouterTree to validate configuration and populate ChildRefs/errors
manager.ParseRouterTree()
@ -1788,7 +1787,7 @@ func TestManager_BuildHandlers_WithChildRouters(t *testing.T) {
parser, err := httpmuxer.NewSyntaxParser()
require.NoError(t, err)
manager := NewManager(conf, serviceManager, middlewareBuilder, nil, nil, parser, nil)
manager := NewManager(conf, serviceManager, middlewareBuilder, nil, nil, parser)
// Compute multi-layer routing to set up parent-child relationships
manager.ParseRouterTree()
@ -1819,11 +1818,10 @@ func TestManager_BuildHandlers_Deny(t *testing.T) {
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",
desc: "parent router without child routers, request with encoded slash",
requestPath: "/foo%2F",
routers: map[string]*dynamic.Router{
"parent": {
@ -1842,7 +1840,7 @@ func TestManager_BuildHandlers_Deny(t *testing.T) {
expectedStatusCode: http.StatusBadRequest,
},
{
desc: "parent router with child routers request with encoded slash",
desc: "parent router with child routers, request with encoded slash",
requestPath: "/foo%2F",
routers: map[string]*dynamic.Router{
"parent": {
@ -1865,13 +1863,16 @@ func TestManager_BuildHandlers_Deny(t *testing.T) {
expectedStatusCode: http.StatusBadRequest,
},
{
desc: "parent router without child router allowing encoded slash",
desc: "parent router allowing encoded slash without child router",
requestPath: "/foo%2F",
routers: map[string]*dynamic.Router{
"parent": {
EntryPoints: []string{"web"},
Rule: "PathPrefix(`/`)",
Service: "service",
DeniedEncodedPathCharacters: dynamic.RouterDeniedEncodedPathCharacters{
AllowEncodedSlash: true,
},
},
},
services: map[string]*dynamic.Service{
@ -1881,18 +1882,18 @@ func TestManager_BuildHandlers_Deny(t *testing.T) {
},
},
},
encodedCharacters: static.EncodedCharacters{
AllowEncodedSlash: true,
},
expectedStatusCode: http.StatusOK,
},
{
desc: "parent router with child routers allowing encoded slash",
desc: "parent router allowing encoded slash with child routers",
requestPath: "/foo%2F",
routers: map[string]*dynamic.Router{
"parent": {
EntryPoints: []string{"web"},
Rule: "PathPrefix(`/`)",
DeniedEncodedPathCharacters: dynamic.RouterDeniedEncodedPathCharacters{
AllowEncodedSlash: true,
},
},
"child1": {
Rule: "PathPrefix(`/`)",
@ -1907,13 +1908,10 @@ func TestManager_BuildHandlers_Deny(t *testing.T) {
},
},
},
encodedCharacters: static.EncodedCharacters{
AllowEncodedSlash: true,
},
expectedStatusCode: http.StatusOK,
},
{
desc: "parent router without child routers request with fragment",
desc: "parent router without child routers, request with fragment",
requestPath: "/foo#",
routers: map[string]*dynamic.Router{
"parent": {
@ -1932,7 +1930,7 @@ func TestManager_BuildHandlers_Deny(t *testing.T) {
expectedStatusCode: http.StatusBadRequest,
},
{
desc: "parent router with child routers request with fragment",
desc: "parent router with child routers, request with fragment",
requestPath: "/foo#",
routers: map[string]*dynamic.Router{
"parent": {
@ -1986,8 +1984,7 @@ func TestManager_BuildHandlers_Deny(t *testing.T) {
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)
manager := NewManager(conf, serviceManager, middlewareBuilder, nil, nil, parser)
// Compute multi-layer routing to set up parent-child relationships
manager.ParseRouterTree()

View file

@ -26,8 +26,7 @@ type RouterFactory struct {
entryPointsTCP []string
entryPointsUDP []string
allowACMEByPass map[string]bool
deniedEncodedPathCharacters map[string]map[string]struct{}
allowACMEByPass map[string]bool
managerFactory *service.ManagerFactory
@ -73,27 +72,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()
}
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,
deniedEncodedPathCharacters: deniedEncodedPathCharacters,
parser: parser,
entryPointsTCP: entryPointsTCP,
entryPointsUDP: entryPointsUDP,
managerFactory: managerFactory,
observabilityMgr: observabilityMgr,
tlsManager: tlsManager,
pluginBuilder: pluginBuilder,
dialerManager: dialerManager,
allowACMEByPass: allowACMEByPass,
parser: parser,
}, nil
}
@ -111,7 +104,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, f.deniedEncodedPathCharacters)
routerManager := router.NewManager(rtConf, serviceManager, middlewaresBuilder, f.observabilityMgr, f.tlsManager, f.parser)
routerManager.ParseRouterTree()

View file

@ -526,8 +526,10 @@ func TestPathOperations(t *testing.T) {
configuration.SetDefaults()
// We need to allow some of the suspicious encoded characters to test the path operations in case they are authorized.
configuration.HTTP.EncodedCharacters.AllowEncodedSlash = true
configuration.HTTP.EncodedCharacters.AllowEncodedPercent = true
configuration.HTTP.EncodedCharacters = &static.EncodedCharacters{
AllowEncodedSlash: true,
AllowEncodedPercent: true,
}
// Create the HTTP server using newHTTPServer.
server, err := newHTTPServer(t.Context(), ln, configuration, false, requestdecorator.New(nil))