NETOBSERV-1102: fine-tune http servers (#428)

pkg/server/common.go

@@ -0,0 +1,42 @@
+package server
+
+import (
+	"crypto/tls"
+	"net/http"
+	"time"
+)
+
+func defaultServer(srv *http.Server) *http.Server {
+	// defaults taken from https://bruinsslot.jp/post/go-secure-webserver/ can be overriden by caller
+	if srv.Handler != nil {
+		// No more than 2MB body
+		srv.Handler = http.MaxBytesHandler(srv.Handler, 2<<20)
+	} else {
+		slog.Warnf("Handler not yet set on server while securing defaults. Make sure a MaxByte middleware is used.")
+	}
+	if srv.ReadTimeout == 0 {
+		srv.ReadTimeout = 10 * time.Second
+	}
+	if srv.ReadHeaderTimeout == 0 {
+		srv.ReadHeaderTimeout = 5 * time.Second
+	}
+	if srv.WriteTimeout == 0 {
+		srv.WriteTimeout = 10 * time.Second
+	}
+	if srv.IdleTimeout == 0 {
+		srv.IdleTimeout = 120 * time.Second
+	}
+	if srv.MaxHeaderBytes == 0 {
+		srv.MaxHeaderBytes = 1 << 20 // 1MB
+	}
+	if srv.TLSConfig == nil {
+		srv.TLSConfig = &tls.Config{}
+	}
+	if srv.TLSConfig.MinVersion == 0 {
+		srv.TLSConfig.MinVersion = tls.VersionTLS13
+	}
+	// Disable http/2
+	srv.TLSNextProto = make(map[string]func(*http.Server, *tls.Conn, http.Handler), 0)
+
+	return srv
+}

pkg/server/metrics_server.go

@@ -1,7 +1,6 @@
 package server
 
 import (
-	"crypto/tls"
 	"fmt"
 	"net/http"
 
@@ -18,17 +17,13 @@ type MetricsConfig struct {
 }
 
 func StartMetrics(cfg *MetricsConfig) {
-	promServer := &http.Server{
-		Addr: fmt.Sprintf(":%d", cfg.Port),
-		// TLS clients must use TLS 1.2 or higher
-		TLSConfig: &tls.Config{
-			MinVersion: tls.VersionTLS12,
-		},
-	}
+	mux := http.NewServeMux()
+	mux.Handle("/metrics", promhttp.Handler())
 
-	// The Handler function provides a default handler to expose metrics
-	// via an HTTP server. "/metrics" is the usual endpoint for that.
-	http.Handle("/metrics", promhttp.Handler())
+	promServer := defaultServer(&http.Server{
+		Addr:    fmt.Sprintf(":%d", cfg.Port),
+		Handler: mux,
+	})
 
 	if cfg.CertPath != "" && cfg.KeyPath != "" {
 		mlog.Infof("listening on https://:%d", cfg.Port)

pkg/server/server.go

@@ -1,7 +1,6 @@
 package server
 
 import (
-	"crypto/tls"
 	"fmt"
 	"net/http"
 	"time"
@@ -32,18 +31,11 @@ func Start(cfg *Config, authChecker auth.Checker) {
 	router := setupRoutes(cfg, authChecker)
 	router.Use(corsHeader(cfg))
 
-	// Clients must use TLS 1.2 or higher
-	tlsConfig := &tls.Config{
-		MinVersion: tls.VersionTLS12,
-	}
-
-	httpServer := &http.Server{
+	httpServer := defaultServer(&http.Server{
 		Handler:      router,
 		Addr:         fmt.Sprintf(":%d", cfg.Port),
-		TLSConfig:    tlsConfig,
-		ReadTimeout:  30 * time.Second,
 		WriteTimeout: 30 * time.Second,
-	}
+	})
 
 	if cfg.CertPath != "" && cfg.KeyPath != "" {
 		slog.Infof("listening on https://:%d", cfg.Port)

pkg/server/server_test.go

@@ -250,6 +250,58 @@ func TestSecureComm(t *testing.T) {
 	}
 }
 
+func TestServerHeaderLimits(t *testing.T) {
+	testPort, err := getFreePort(testHostname)
+	if err != nil {
+		t.Fatalf("Cannot get a free port to run tests on host [%v]", testHostname)
+	} else {
+		t.Logf("Will use free port [%v] on host [%v] for tests", testPort, testHostname)
+	}
+
+	testServerHostPort := fmt.Sprintf("%v:%v", testHostname, testPort)
+	serverURL := fmt.Sprintf("http://%s", testServerHostPort)
+
+	// Prepare directory to serve web files
+	tmpDir := prepareServerAssets(t)
+	defer os.RemoveAll(tmpDir)
+
+	authM := authMock{}
+	authM.MockGranted()
+
+	go func() {
+		Start(&Config{
+			Loki: loki.Config{
+				URL: &url.URL{Scheme: "http", Host: "localhost:3100"},
+			},
+			Port: testPort,
+		}, &authM)
+	}()
+
+	t.Logf("Started test http server: %v", serverURL)
+
+	httpConfig := httpClientConfig{}
+	httpClient, err := httpConfig.buildHTTPClient()
+	if err != nil {
+		t.Fatalf("Failed to create http client")
+	}
+
+	// wait for our test http server to come up
+	checkHTTPReady(httpClient, serverURL)
+
+	r, err := http.NewRequest("GET", serverURL, nil)
+	require.NoError(t, err)
+	// Set many headers
+	oneKBString := strings.Repeat(".", 1024)
+	for i := 0; i < 1025; i++ {
+		r.Header.Set(fmt.Sprintf("test-header-%d", i), oneKBString)
+	}
+
+	resp, err := httpClient.Do(r)
+	require.NoError(t, err)
+	defer resp.Body.Close()
+	assert.Equal(t, http.StatusRequestHeaderFieldsTooLarge, resp.StatusCode)
+}
+
 func TestLokiConfiguration(t *testing.T) {
 	// GIVEN a Loki service
 	lokiMock := httpMock{}