Print access logs for rejected requests and warn about new behavior
This commit is contained in:
parent
e0e49533ab
commit
60b19b7b81
10 changed files with 371 additions and 147 deletions
62
pkg/server/router/deny.go
Normal file
62
pkg/server/router/deny.go
Normal file
|
|
@ -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)
|
||||
})
|
||||
}
|
||||
98
pkg/server/router/deny_test.go
Normal file
98
pkg/server/router/deny_test.go
Normal file
|
|
@ -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)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
|
@ -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)
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue