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:00:05 +01:00 committed by GitHub
parent 8ebab1b243
commit 90ce858347
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
21 changed files with 427 additions and 300 deletions

View file

@ -164,6 +164,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
}
rtName := name
if len(eps) > 1 {
rtName = epName + "-" + name

View file

@ -36,14 +36,13 @@ 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
deniedEncodedPathCharacters map[string]map[string]struct{}
routerHandlers map[string]http.Handler
serviceManager serviceManager
metricsRegistry metrics.Registry
middlewaresBuilder middlewareBuilder
chainBuilder *middleware.ChainBuilder
conf *runtime.Configuration
tlsManager *tls.Manager
}
// NewManager creates a new Manager.
@ -53,17 +52,15 @@ func NewManager(conf *runtime.Configuration,
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,
deniedEncodedPathCharacters: deniedEncodedPathCharacters,
routerHandlers: make(map[string]http.Handler),
serviceManager: serviceManager,
metricsRegistry: metricsRegistry,
middlewaresBuilder: middlewaresBuilder,
chainBuilder: chainBuilder,
conf: conf,
tlsManager: tlsManager,
}
}
@ -82,7 +79,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, entryPointName, routers)
handler, err := m.buildEntryPointHandler(ctx, routers)
if err != nil {
log.FromContext(ctx).Error(err)
continue
@ -118,7 +115,7 @@ func (m *Manager) BuildHandlers(rootCtx context.Context, entryPoints []string, t
return entryPointHandlers
}
func (m *Manager) buildEntryPointHandler(ctx context.Context, entryPointName string, configs map[string]*runtime.RouterInfo) (http.Handler, error) {
func (m *Manager) buildEntryPointHandler(ctx context.Context, configs map[string]*runtime.RouterInfo) (http.Handler, error) {
muxer, err := httpmuxer.NewMuxer()
if err != nil {
return nil, err
@ -135,7 +132,7 @@ func (m *Manager) buildEntryPointHandler(ctx context.Context, entryPointName str
continue
}
handler, err := m.buildRouterHandler(ctxRouter, entryPointName, routerName, routerConfig)
handler, err := m.buildRouterHandler(ctxRouter, routerName, routerConfig)
if err != nil {
routerConfig.AddError(err, true)
logger.Error(err)
@ -160,7 +157,7 @@ func (m *Manager) buildEntryPointHandler(ctx context.Context, entryPointName str
return chain.Then(muxer)
}
func (m *Manager) buildRouterHandler(ctx context.Context, entryPointName, routerName string, routerConfig *runtime.RouterInfo) (http.Handler, error) {
func (m *Manager) buildRouterHandler(ctx context.Context, routerName string, routerConfig *runtime.RouterInfo) (http.Handler, error) {
if handler, ok := m.routerHandlers[routerName]; ok {
return handler, nil
}
@ -176,7 +173,7 @@ func (m *Manager) buildRouterHandler(ctx context.Context, entryPointName, router
}
}
handler, err := m.buildHTTPHandler(ctx, routerConfig, entryPointName, routerName)
handler, err := m.buildHTTPHandler(ctx, routerConfig, routerName)
if err != nil {
return nil, err
}
@ -194,7 +191,7 @@ func (m *Manager) buildRouterHandler(ctx context.Context, entryPointName, router
return m.routerHandlers[routerName], nil
}
func (m *Manager) buildHTTPHandler(ctx context.Context, router *runtime.RouterInfo, entryPointName, routerName string) (http.Handler, error) {
func (m *Manager) buildHTTPHandler(ctx context.Context, router *runtime.RouterInfo, routerName string) (http.Handler, error) {
var qualifiedNames []string
for _, name := range router.Middlewares {
qualifiedNames = append(qualifiedNames, provider.GetQualifiedName(ctx, name))
@ -232,7 +229,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
})
return chain.Extend(*mHandler).Append(tHandler).Then(sHandler)

View file

@ -13,7 +13,6 @@ 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"
@ -320,7 +319,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, nil)
routerManager := NewManager(rtConf, serviceManager, middlewaresBuilder, chainBuilder, metrics.NewVoidRegistry(), tlsManager)
handlers := routerManager.BuildHandlers(t.Context(), test.entryPoints, false)
@ -427,7 +426,7 @@ func TestAccessLog(t *testing.T) {
chainBuilder := middleware.NewChainBuilder(nil, nil, nil)
tlsManager := tls.NewManager()
routerManager := NewManager(rtConf, serviceManager, middlewaresBuilder, chainBuilder, metrics.NewVoidRegistry(), tlsManager, nil)
routerManager := NewManager(rtConf, serviceManager, middlewaresBuilder, chainBuilder, metrics.NewVoidRegistry(), tlsManager)
handlers := routerManager.BuildHandlers(t.Context(), test.entryPoints, false)
@ -815,7 +814,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, nil)
routerManager := NewManager(rtConf, serviceManager, middlewaresBuilder, chainBuilder, metrics.NewVoidRegistry(), tlsManager)
_ = routerManager.BuildHandlers(t.Context(), entryPoints, false)
_ = routerManager.BuildHandlers(t.Context(), entryPoints, true)
@ -892,7 +891,7 @@ func TestProviderOnMiddlewares(t *testing.T) {
chainBuilder := middleware.NewChainBuilder(nil, nil, nil)
tlsManager := tls.NewManager()
routerManager := NewManager(rtConf, serviceManager, middlewaresBuilder, chainBuilder, metrics.NewVoidRegistry(), tlsManager, nil)
routerManager := NewManager(rtConf, serviceManager, middlewaresBuilder, chainBuilder, metrics.NewVoidRegistry(), tlsManager)
_ = routerManager.BuildHandlers(t.Context(), entryPoints, false)
@ -908,7 +907,6 @@ func TestManager_BuildHandlers_Deny(t *testing.T) {
routers map[string]*dynamic.Router
services map[string]*dynamic.Service
requestPath string
encodedCharacters static.EncodedCharacters
expectedStatusCode int
}{
{
@ -938,6 +936,9 @@ func TestManager_BuildHandlers_Deny(t *testing.T) {
EntryPoints: []string{"web"},
Rule: "PathPrefix(`/`)",
Service: "service",
DeniedEncodedPathCharacters: dynamic.RouterDeniedEncodedPathCharacters{
AllowEncodedSlash: true,
},
},
},
services: map[string]*dynamic.Service{
@ -947,9 +948,6 @@ func TestManager_BuildHandlers_Deny(t *testing.T) {
},
},
},
encodedCharacters: static.EncodedCharacters{
AllowEncodedSlash: true,
},
expectedStatusCode: http.StatusBadGateway,
},
{
@ -996,7 +994,6 @@ func TestManager_BuildHandlers_Deny(t *testing.T) {
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)
@ -1004,7 +1001,7 @@ func TestManager_BuildHandlers_Deny(t *testing.T) {
chainBuilder := middleware.NewChainBuilder(nil, nil, nil)
tlsManager := tls.NewManager()
routerManager := NewManager(conf, serviceManager, middlewaresBuilder, chainBuilder, metrics.NewVoidRegistry(), tlsManager, deniedEncodedPathCharacters)
routerManager := NewManager(conf, serviceManager, middlewaresBuilder, chainBuilder, metrics.NewVoidRegistry(), tlsManager)
// Build handlers
ctx := t.Context()
@ -1079,7 +1076,7 @@ func BenchmarkRouterServe(b *testing.B) {
chainBuilder := middleware.NewChainBuilder(nil, nil, nil)
tlsManager := tls.NewManager()
routerManager := NewManager(rtConf, serviceManager, middlewaresBuilder, chainBuilder, metrics.NewVoidRegistry(), tlsManager, nil)
routerManager := NewManager(rtConf, serviceManager, middlewaresBuilder, chainBuilder, metrics.NewVoidRegistry(), tlsManager)
handlers := routerManager.BuildHandlers(b.Context(), entryPoints, false)

View file

@ -24,8 +24,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
@ -66,21 +65,15 @@ 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,
deniedEncodedPathCharacters: deniedEncodedPathCharacters,
entryPointsTCP: entryPointsTCP,
entryPointsUDP: entryPointsUDP,
managerFactory: managerFactory,
metricsRegistry: metricsRegistry,
tlsManager: tlsManager,
chainBuilder: chainBuilder,
pluginBuilder: pluginBuilder,
allowACMEByPass: allowACMEByPass,
}
}
@ -93,7 +86,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, f.deniedEncodedPathCharacters)
routerManager := router.NewManager(rtConf, serviceManager, middlewaresBuilder, f.chainBuilder, f.metricsRegistry, f.tlsManager)
handlersNonTLS := routerManager.BuildHandlers(ctx, f.entryPointsTCP, false)
handlersTLS := routerManager.BuildHandlers(ctx, f.entryPointsTCP, true)

View file

@ -576,8 +576,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 createHTTPServer.
server, err := createHTTPServer(t.Context(), ln, configuration, false, requestdecorator.New(nil))