diff --git a/httpserver/handler_wrapper.go b/httpserver/handler_wrapper.go index 35055d9..38b44be 100644 --- a/httpserver/handler_wrapper.go +++ b/httpserver/handler_wrapper.go @@ -168,6 +168,17 @@ func newHandlerWrapper( // TODO X-Forwaded-* support func (hw *handlerWrapper) ServeHTTP(w http.ResponseWriter, r *http.Request) { + if err := checkHeadersLen(r); err != nil { + hw.logger.ErrorCtx(r.Context(), "invalid header length", log.Error(err)) + + w.Header().Set("content-type", "application/json; charset=utf-8") + w.WriteHeader(http.StatusRequestHeaderFieldsTooLarge) + if _, err := w.Write([]byte("{}")); err != nil { + hw.logger.ErrorCtx(r.Context(), "cannot write body", log.Error(err)) + } + return + } + // Bypass for OPTIONS request to avoid telemetry, metrics and // logging noise. if r.Method == http.MethodOptions { diff --git a/httpserver/header_limit.go b/httpserver/header_limit.go new file mode 100644 index 0000000..d77a705 --- /dev/null +++ b/httpserver/header_limit.go @@ -0,0 +1,57 @@ +// Copyright (c) 2024 Bryan Frimin . +// +// Permission to use, copy, modify, and/or distribute this software +// for any purpose with or without fee is hereby granted, provided +// that the above copyright notice and this permission notice appear +// in all copies. +// +// THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL +// WARRANTIES WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED +// WARRANTIES OF MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE +// AUTHOR BE LIABLE FOR ANY SPECIAL, DIRECT, INDIRECT, OR +// CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS +// OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, +// NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN +// CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. + +package httpserver + +import ( + "fmt" + "net/http" +) + +const ( + // maxHostLen is the longest Host value. A domain name in text form + // is at most 254 octets with the trailing root dot (RFC 1035), and a + // port adds ":" plus 5 digits. + maxHostLen = 260 // bytes + + // maxHeaderValueLen is the maximum size of one header value. + // nginx limits one header field to 8 KiB. 16 KiB leaves room for a + // long JWT and for a Cookie header that carries several cookies. + maxHeaderValueLen = 16 << 10 // 16 KiB + + // maxHeaderBytes is the maximum size of the request line and + // headers read by net/http. The net/http default is 1 MiB. + maxHeaderBytes = 100 << 10 // 100 KiB +) + +// checkHeadersLen reports the first header that exceeds its limit. +// Host is limited to maxHostLen. Every other value is limited to +// maxHeaderValueLen. A nil error means every value is within its limit. +func checkHeadersLen(r *http.Request) error { + if len(r.Host) > maxHostLen { + return fmt.Errorf("header value too long: Host") + } + + for name, values := range r.Header { + for _, value := range values { + if len(value) > maxHeaderValueLen { + return fmt.Errorf("header value too long: %s", name) + } + } + } + + return nil +} diff --git a/httpserver/httpserver.go b/httpserver/httpserver.go index d1496cd..def4ee4 100644 --- a/httpserver/httpserver.go +++ b/httpserver/httpserver.go @@ -118,6 +118,7 @@ func NewServer(addr string, h http.Handler, options ...Option) *http.Server { ErrorLog: stdlog.New(logger.NewWriter(log.LevelError), "", 0), ReadHeaderTimeout: 5 * time.Second, ReadTimeout: opts.readTimeout, + MaxHeaderBytes: maxHeaderBytes, WriteTimeout: opts.writeTimeout, IdleTimeout: 15 * time.Second, ConnContext: func(ctx context.Context, c net.Conn) context.Context { diff --git a/httpserver/httpserver_test.go b/httpserver/httpserver_test.go index 0c2e354..4ce17b2 100644 --- a/httpserver/httpserver_test.go +++ b/httpserver/httpserver_test.go @@ -395,3 +395,71 @@ func TestHTTPServer_Health(t *testing.T) { logOutput := logBuf.String() assert.NotContains(t, logOutput, "/health") } + +func TestHTTPServer_MaxHeaderBytes(t *testing.T) { + server := NewServer(":8080", http.HandlerFunc(func(http.ResponseWriter, *http.Request) {}), + WithRegisterer(prometheus.NewRegistry()), + ) + assert.Equal(t, 100<<10, server.MaxHeaderBytes) +} + +func TestHTTPServer_HeaderValueLimit(t *testing.T) { + tests := []struct { + name string + host string + header string + value string + want string + }{ + { + name: "host", + host: strings.Repeat("a", maxHostLen+1), + want: "Host", + }, + { + name: "value", + header: "Cookie", + value: strings.Repeat("c", maxHeaderValueLen+1), + want: "Cookie", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + called := false + handler := http.HandlerFunc(func(http.ResponseWriter, *http.Request) { + called = true + }) + + var logBuf bytes.Buffer + server := NewServer(":8080", handler, + WithLogger(log.NewLogger(log.WithOutput(&logBuf))), + WithRegisterer(prometheus.NewRegistry()), + ) + + req := httptest.NewRequest(http.MethodGet, "/test", nil) + if tt.host != "" { + req.Host = tt.host + } + if tt.header != "" { + req.Header.Set(tt.header, tt.value) + } + + rec := httptest.NewRecorder() + server.Handler.ServeHTTP(rec, req) + + assert.Equal(t, http.StatusRequestHeaderFieldsTooLarge, rec.Code) + assert.False(t, called) + assert.Equal(t, "application/json; charset=utf-8", rec.Header().Get("content-type")) + assert.Equal(t, "{}", rec.Body.String()) + assert.Contains(t, logBuf.String(), "header value too long: "+tt.want) + + absent := tt.host + if tt.value != "" { + absent = tt.value + } + assert.NotContains(t, logBuf.String(), absent) + assert.NotContains(t, rec.Body.String(), absent) + }) + } +}