Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce response header limits #453

Merged
merged 5 commits into from
Nov 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -460,7 +460,10 @@ type Config struct {
DisableKeepAlives bool `yaml:"disable_keep_alives"`
MaxIdleConns int `yaml:"max_idle_conns,omitempty"`
MaxIdleConnsPerHost int `yaml:"max_idle_conns_per_host,omitempty"`
MaxHeaderBytes int `yaml:"max_header_bytes"`
MaxRequestHeaderBytes int `yaml:"max_request_header_bytes"`
MaxResponseHeaderBytes int `yaml:"max_response_header_bytes"`
MaxRequestHeaders int `yaml:"max_request_headers"`
maxmoehl marked this conversation as resolved.
Show resolved Hide resolved
MaxResponseHeaders int `yaml:"max_response_headers"`
KeepAlive100ContinueRequests bool `yaml:"keep_alive_100_continue_requests"`

HTTPRewrite HTTPRewrite `yaml:"http_rewrite,omitempty"`
Expand Down
4 changes: 2 additions & 2 deletions config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -213,13 +213,13 @@ status:
})
It("sets MaxHeaderBytes", func() {
var b = []byte(`
max_header_bytes: 10
max_request_header_bytes: 10
`)

err := config.Initialize(b)
Expect(err).ToNot(HaveOccurred())

Expect(config.MaxHeaderBytes).To(Equal(10))
Expect(config.MaxRequestHeaderBytes).To(Equal(10))
})

It("sets prometheus endpoint config", func() {
Expand Down
43 changes: 27 additions & 16 deletions handlers/max_request_size.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,17 +11,18 @@ import (
)

type MaxRequestSize struct {
cfg *config.Config
MaxSize int
logger *slog.Logger
cfg *config.Config
MaxSize int
MaxCount int
logger *slog.Logger
}

const ONE_MB = 1024 * 1024 // bytes * kb

// NewAccessLog creates a new handler that handles logging requests to the
// access log
func NewMaxRequestSize(cfg *config.Config, logger *slog.Logger) *MaxRequestSize {
maxSize := cfg.MaxHeaderBytes
maxSize := cfg.MaxRequestHeaderBytes

if maxSize < 1 {
maxSize = ONE_MB
Expand All @@ -33,27 +34,37 @@ func NewMaxRequestSize(cfg *config.Config, logger *slog.Logger) *MaxRequestSize
}

return &MaxRequestSize{
MaxSize: maxSize,
logger: logger,
cfg: cfg,
MaxSize: maxSize,
MaxCount: cfg.MaxRequestHeaders,
logger: logger,
cfg: cfg,
}
}

func (m *MaxRequestSize) ServeHTTP(rw http.ResponseWriter, r *http.Request, next http.HandlerFunc) {
logger := LoggerWithTraceInfo(m.logger, r)
reqSize := len(r.Method) + len(r.URL.RequestURI()) + len(r.Proto) + 5 // add 5 bytes for space-separation of method, URI, protocol, and /r/n

for k, v := range r.Header {
valueLen := 0
for _, value := range r.Header.Values(k) {
valueLen += len(value)
// Four additional bytes for the two spaces and \r\n:
// GET / HTTP/1.1\r\n
reqSize := len(r.Method) + len(r.URL.RequestURI()) + len(r.Proto) + 4

// Host header which is not passed on to us, plus eight bytes for 'Host: ' and \r\n
reqSize += len(r.Host) + 8

hdrCount := 0

// Go doesn't split header values on commas, instead it only splits the value when it's
// provided via repeated header keys. Therefore we have to account for each value of a repeated
// header as well as its key.
for k, vv := range r.Header {
for _, v := range vv {
// Four additional bytes for the colon and space after the header key and \r\n.
reqSize += len(k) + len(v) + 4
hdrCount++
}
reqSize += len(k) + valueLen + 4 + len(v) - 1 // add padding for ': ' and newlines and comma delimiting of multiple values
}

reqSize += len(r.Host) + 8 // add padding for "Host: " and newlines

if reqSize >= m.MaxSize {
if reqSize >= m.MaxSize || (m.MaxCount > 0 && hdrCount > m.MaxCount) {
reqInfo, err := ContextRequestInfo(r)
if err != nil {
logger.Error("request-info-err", log.ErrAttr(err))
Expand Down
34 changes: 29 additions & 5 deletions handlers/max_request_size_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,8 @@ var _ = Describe("MaxRequestSize", func() {

BeforeEach(func() {
cfg = &config.Config{
MaxHeaderBytes: 89,
MaxRequestHeaderBytes: 89,
MaxRequestHeaders: 15,
LoadBalance: config.LOAD_BALANCE_RR,
StickySessionCookieNames: config.StringSet{"blarg": struct{}{}},
}
Expand Down Expand Up @@ -166,6 +167,29 @@ var _ = Describe("MaxRequestSize", func() {
Expect(result.StatusCode).To(Equal(http.StatusRequestHeaderFieldsTooLarge))
})
})
Context("when a repeated header has a short value and long key taking it over the limit", func() {
BeforeEach(func() {
for i := 0; i < 10; i++ {
header.Add("foobar", "m")
}
})
It("throws an http 431", func() {
handleRequest()
Expect(result.StatusCode).To(Equal(http.StatusRequestHeaderFieldsTooLarge))
})
})
Context("when there are too many headers", func() {
BeforeEach(func() {
for i := 0; i < 16; i++ {
header.Add("f", "m")
}
})
It("throws an http 431", func() {
handleRequest()
Expect(result.StatusCode).To(Equal(http.StatusRequestHeaderFieldsTooLarge))
maxmoehl marked this conversation as resolved.
Show resolved Hide resolved
Expect(result.Header).To(HaveKeyWithValue("X-Cf-Routererror", []string{"max-request-size-exceeded"}))
})
})
Context("when enough normally-sized headers put the request over the limit", func() {
BeforeEach(func() {
header.Add("header1", "smallRequest")
Expand Down Expand Up @@ -205,7 +229,7 @@ var _ = Describe("MaxRequestSize", func() {
Describe("NewMaxRequestSize()", func() {
Context("when using a custom MaxHeaderBytes", func() {
BeforeEach(func() {
cfg.MaxHeaderBytes = 1234
cfg.MaxRequestHeaderBytes = 1234
})
It("returns a new requestSizeHandler using the provided size", func() {
Expect(rh.MaxSize).To(Equal(1234))
Expand All @@ -214,15 +238,15 @@ var _ = Describe("MaxRequestSize", func() {

Context("when using a negative MaxHeaderBytes", func() {
BeforeEach(func() {
cfg.MaxHeaderBytes = -1
cfg.MaxRequestHeaderBytes = -1
})
It("defaults to 1mb", func() {
Expect(rh.MaxSize).To(Equal(1024 * 1024))
})
})
Context("when using a zero-value MaxHeaderBytes", func() {
BeforeEach(func() {
cfg.MaxHeaderBytes = 0
cfg.MaxRequestHeaderBytes = 0
})
It("defaults to 1mb", func() {
Expect(rh.MaxSize).To(Equal(1024 * 1024))
Expand All @@ -231,7 +255,7 @@ var _ = Describe("MaxRequestSize", func() {

Context("when using a >1mb MaxHeaderBytes", func() {
BeforeEach(func() {
cfg.MaxHeaderBytes = handlers.ONE_MB * 2
cfg.MaxRequestHeaderBytes = handlers.ONE_MB * 2
})
It("defaults to 1mb if the provided size", func() {
Expect(rh.MaxSize).To(Equal(1024 * 1024))
Expand Down
2 changes: 1 addition & 1 deletion integration/common_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ func NewTestState() *testState {
}
cfg.OAuth.TokenEndpoint, cfg.OAuth.Port = hostnameAndPort(oauthServer.Addr())

cfg.MaxHeaderBytes = 48 * 1024 //1kb
cfg.MaxRequestHeaderBytes = 48 * 1024 //1kb

return &testState{
cfg: cfg,
Expand Down
24 changes: 24 additions & 0 deletions integration/header_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"io"
"net/http"
"net/http/httptest"
"strings"

. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
Expand Down Expand Up @@ -197,4 +198,27 @@ var _ = Describe("Headers", func() {
Expect(resp.Header.Get(HeaderKeySignature)).To(BeEmpty())
})
})

Context("Header Limits", func() {
Context("when a response header size limit is configured", func() {
BeforeEach(func() {
testApp = NewUnstartedTestApp(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.Header().Set("Too-Large", strings.Repeat("0123456789", 10))
w.WriteHeader(200)
}))
testState.cfg.MaxResponseHeaderBytes = 80
testState.StartGorouterOrFail()
testApp.Start()
testState.register(testApp.Server, testAppRoute)
})

It("fails with 502 when the app exceeds the limit", func() {
req := testState.newRequest(fmt.Sprintf("http://%s", testAppRoute))
resp, err := testState.client.Do(req)
Expect(err).NotTo(HaveOccurred())
Expect(resp.StatusCode).To(Equal(http.StatusBadGateway))
Expect(resp.Header).To(HaveKeyWithValue("X-Cf-Routererror", []string{"endpoint_failure (net/http: HTTP/1.x transport connection broken: net/http: server response headers exceeded 80 bytes; aborted)"}))
})
})
})
})
2 changes: 1 addition & 1 deletion integration/large_request_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ var _ = Describe("Large requests", func() {
testState = NewTestState()
testState.EnableAccessLog()
testState.EnableMetron()
testState.cfg.MaxHeaderBytes = 1 * 1024 // 1kb
testState.cfg.MaxRequestHeaderBytes = 1 * 1024 // 1kb
testState.StartGorouterOrFail()

appURL = "echo-app." + test_util.LocalhostDNS
Expand Down
36 changes: 19 additions & 17 deletions proxy/proxy.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,25 +84,27 @@ func NewProxy(

roundTripperFactory := &round_tripper.FactoryImpl{
BackendTemplate: &http.Transport{
DialContext: dialer.DialContext,
DisableKeepAlives: cfg.DisableKeepAlives,
MaxIdleConns: cfg.MaxIdleConns,
IdleConnTimeout: 90 * time.Second, // setting the value to golang default transport
MaxIdleConnsPerHost: cfg.MaxIdleConnsPerHost,
DisableCompression: true,
TLSClientConfig: backendTLSConfig,
TLSHandshakeTimeout: cfg.TLSHandshakeTimeout,
ExpectContinueTimeout: 1 * time.Second,
DialContext: dialer.DialContext,
DisableKeepAlives: cfg.DisableKeepAlives,
MaxIdleConns: cfg.MaxIdleConns,
IdleConnTimeout: 90 * time.Second, // setting the value to golang default transport
MaxIdleConnsPerHost: cfg.MaxIdleConnsPerHost,
DisableCompression: true,
TLSClientConfig: backendTLSConfig,
TLSHandshakeTimeout: cfg.TLSHandshakeTimeout,
ExpectContinueTimeout: 1 * time.Second,
MaxResponseHeaderBytes: int64(cfg.MaxResponseHeaderBytes),
},
RouteServiceTemplate: &http.Transport{
DialContext: dialer.DialContext,
DisableKeepAlives: cfg.DisableKeepAlives,
MaxIdleConns: cfg.MaxIdleConns,
IdleConnTimeout: 90 * time.Second, // setting the value to golang default transport
MaxIdleConnsPerHost: cfg.MaxIdleConnsPerHost,
DisableCompression: true,
TLSClientConfig: routeServiceTLSConfig,
ExpectContinueTimeout: 1 * time.Second,
DialContext: dialer.DialContext,
DisableKeepAlives: cfg.DisableKeepAlives,
MaxIdleConns: cfg.MaxIdleConns,
IdleConnTimeout: 90 * time.Second, // setting the value to golang default transport
MaxIdleConnsPerHost: cfg.MaxIdleConnsPerHost,
DisableCompression: true,
TLSClientConfig: routeServiceTLSConfig,
ExpectContinueTimeout: 1 * time.Second,
MaxResponseHeaderBytes: int64(cfg.MaxResponseHeaderBytes),
peanball marked this conversation as resolved.
Show resolved Hide resolved
},
IsInstrumented: cfg.SendHttpStartStopClientEvent,
}
Expand Down
21 changes: 11 additions & 10 deletions proxy/round_tripper/dropsonde_round_tripper.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,16 +45,17 @@ func (t *FactoryImpl) New(expectedServerName string, isRouteService bool, isHttp
customTLSConfig := utils.TLSConfigWithServerName(expectedServerName, template.TLSClientConfig, isRouteService)

newTransport := &http.Transport{
DialContext: template.DialContext,
DisableKeepAlives: template.DisableKeepAlives,
MaxIdleConns: template.MaxIdleConns,
IdleConnTimeout: template.IdleConnTimeout,
MaxIdleConnsPerHost: template.MaxIdleConnsPerHost,
DisableCompression: template.DisableCompression,
TLSClientConfig: customTLSConfig,
TLSHandshakeTimeout: template.TLSHandshakeTimeout,
ForceAttemptHTTP2: isHttp2,
ExpectContinueTimeout: template.ExpectContinueTimeout,
DialContext: template.DialContext,
DisableKeepAlives: template.DisableKeepAlives,
MaxIdleConns: template.MaxIdleConns,
IdleConnTimeout: template.IdleConnTimeout,
MaxIdleConnsPerHost: template.MaxIdleConnsPerHost,
DisableCompression: template.DisableCompression,
TLSClientConfig: customTLSConfig,
TLSHandshakeTimeout: template.TLSHandshakeTimeout,
ForceAttemptHTTP2: isHttp2,
ExpectContinueTimeout: template.ExpectContinueTimeout,
MaxResponseHeaderBytes: template.MaxResponseHeaderBytes,
}
if t.IsInstrumented {
return NewDropsondeRoundTripper(newTransport)
Expand Down
Loading