From 911dfbb82554ecad94bc0d3de8b960a724253679 Mon Sep 17 00:00:00 2001 From: sneak Date: Mon, 21 Sep 2026 12:49:58 +0000 Subject: [PATCH] =?UTF-8?q?feat(backend):=20server=20hardening=20=E2=80=94?= =?UTF-8?q?=20timeouts,=20security=20headers,=20trusted-proxy=20client=20I?= =?UTF-8?q?P=20(closes=20#19)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add ReadHeaderTimeout and IdleTimeout to the http.Server (named constants beside the existing timeouts) to close the slowloris and idle-keep-alive gaps. Add a SecurityHeaders middleware setting HSTS, a JSON-API CSP (default-src 'none'; frame-ancestors 'none'), X-Frame-Options: DENY, X-Content-Type-Options: nosniff, Referrer-Policy, and Permissions-Policy. It is registered before CORS so the headers ride on preflight responses. Resolve the client IP from X-Forwarded-For / X-Real-IP only when the direct peer is in a trusted-proxy allowlist, defaulting to loopback plus RFC1918 and configurable via TRUSTED_PROXIES; an untrusted peer's forwarded headers are ignored and the direct peer is logged. Uses net/netip; no new dependency. Model: opus-4-8 --- TODO.md | 5 + backend/README.md | 16 +- backend/internal/config/config.go | 28 ++++ backend/internal/middleware/export_test.go | 21 +++ backend/internal/middleware/middleware.go | 133 ++++++++++++++++- .../internal/middleware/middleware_test.go | 139 ++++++++++++++++++ backend/internal/server/http.go | 20 ++- backend/internal/server/routes.go | 1 + 8 files changed, 347 insertions(+), 16 deletions(-) create mode 100644 backend/internal/middleware/export_test.go create mode 100644 backend/internal/middleware/middleware_test.go diff --git a/TODO.md b/TODO.md index 1da8930..b284278 100644 --- a/TODO.md +++ b/TODO.md @@ -22,6 +22,11 @@ files, so merging it also closes most compliance gaps. # Completed Steps +- 2026-09-21: backend HTTP hardening (issue #19): added `ReadHeaderTimeout` and + `IdleTimeout` to the server, a `SecurityHeaders` middleware (HSTS, tight CSP, + frame/sniff/referrer/permissions headers) registered before CORS, and + trusted-proxy client IP resolution honouring `X-Forwarded-For` / `X-Real-IP` + only from a `TRUSTED_PROXIES` allowlist (loopback plus RFC1918 by default) - 2026-08-10: every interactive control now meets the 44x44 CSS px minimum tap target (`.pin-btn`, `#interval-select`, the debug-log label and, on narrow viewports, `#pause-btn`). The pin button's hit area grows via matching diff --git a/backend/README.md b/backend/README.md index a7de988..3cad737 100644 --- a/backend/README.md +++ b/backend/README.md @@ -42,11 +42,17 @@ Internal packages in `internal/` follow standard Go project layout: ### Configuration -| Variable | Default | Description | -| ---------- | ------------------ | --------------------------------- | -| `PORT` | `8080` | HTTP listen port | -| `DATA_DIR` | `./data/reports` | Directory for compressed reports | -| `DEBUG` | `false` | Enable debug logging | +| Variable | Default | Description | +| ----------------- | -------------------- | -------------------------------------------------------------------------------------------------------- | +| `PORT` | `8080` | HTTP listen port | +| `DATA_DIR` | `./data/reports` | Directory for compressed reports | +| `DEBUG` | `false` | Enable debug logging | +| `TRUSTED_PROXIES` | loopback + RFC1918 | Comma-separated CIDRs whose `X-Forwarded-For` / `X-Real-IP` headers are trusted for client IP resolution | + +`TRUSTED_PROXIES` defaults to `127.0.0.1/32,::1/128,10.0.0.0/8,172.16.0.0/12,192.168.0.0/16`. +The loopback entries cover the reverse proxy that shares the container; the +RFC1918 ranges match `nginx.conf`. A request whose direct peer is outside this +set has its forwarded headers ignored, and the direct peer is logged instead. ### Report storage diff --git a/backend/internal/config/config.go b/backend/internal/config/config.go index 6966d57..02a6db9 100644 --- a/backend/internal/config/config.go +++ b/backend/internal/config/config.go @@ -5,6 +5,7 @@ package config import ( "errors" "log/slog" + "strings" "sneak.berlin/go/netwatch/internal/globals" "sneak.berlin/go/netwatch/internal/logger" @@ -14,6 +15,14 @@ import ( "go.uber.org/fx" ) +// defaultTrustedProxies lists the networks whose forwarded +// headers are honoured by default. It covers the RFC1918 +// ranges (to match nginx.conf) plus IPv4 and IPv6 loopback, +// because the reverse proxy shares the container and reaches +// the backend over loopback. +const defaultTrustedProxies = "127.0.0.1/32,::1/128," + + "10.0.0.0/8,172.16.0.0/12,192.168.0.0/16" + // Params defines the dependencies for Config. type Params struct { fx.In @@ -30,6 +39,7 @@ type Config struct { MetricsUsername string Port int SentryDSN string + TrustedProxies []string log *slog.Logger params *Params } @@ -56,6 +66,7 @@ func New( viper.SetDefault("SENTRY_DSN", "") viper.SetDefault("METRICS_USERNAME", "") viper.SetDefault("METRICS_PASSWORD", "") + viper.SetDefault("TRUSTED_PROXIES", defaultTrustedProxies) err := viper.ReadInConfig() if err != nil { @@ -73,6 +84,7 @@ func New( MetricsUsername: viper.GetString("METRICS_USERNAME"), Port: viper.GetInt("PORT"), SentryDSN: viper.GetString("SENTRY_DSN"), + TrustedProxies: splitList(viper.GetString("TRUSTED_PROXIES")), log: log, params: ¶ms, } @@ -84,3 +96,19 @@ func New( return s, nil } + +// splitList turns a comma-separated setting into a trimmed +// slice, dropping empty entries. +func splitList(raw string) []string { + parts := strings.Split(raw, ",") + + out := make([]string, 0, len(parts)) + for _, p := range parts { + p = strings.TrimSpace(p) + if p != "" { + out = append(out, p) + } + } + + return out +} diff --git a/backend/internal/middleware/export_test.go b/backend/internal/middleware/export_test.go new file mode 100644 index 0000000..d1961c0 --- /dev/null +++ b/backend/internal/middleware/export_test.go @@ -0,0 +1,21 @@ +package middleware + +import ( + "net/http" + "net/netip" +) + +// Test-only wrappers exposing unexported helpers to the +// external middleware_test package. + +func ClientIP( + remoteAddr string, + header http.Header, + trusted []netip.Prefix, +) string { + return clientIP(remoteAddr, header, trusted) +} + +func ParseTrustedProxies(cidrs []string) ([]netip.Prefix, error) { + return parseTrustedProxies(cidrs) +} diff --git a/backend/internal/middleware/middleware.go b/backend/internal/middleware/middleware.go index 01cbe8d..5f5194b 100644 --- a/backend/internal/middleware/middleware.go +++ b/backend/internal/middleware/middleware.go @@ -3,9 +3,12 @@ package middleware import ( + "fmt" "log/slog" "net" "net/http" + "net/netip" + "strings" "time" "sneak.berlin/go/netwatch/internal/config" @@ -19,6 +22,15 @@ import ( const corsMaxAgeSec = 300 +// Security header values. The backend is a JSON API with no +// HTML surface, so the CSP forbids every resource type and +// framing outright. +const ( + hstsValue = "max-age=31536000; includeSubDomains" + cspValue = "default-src 'none'; frame-ancestors 'none'" + permissionsPolicyValue = "camera=(), microphone=(), geolocation=()" +) + // Params defines the dependencies for Middleware. type Params struct { fx.In @@ -30,8 +42,9 @@ type Params struct { // Middleware holds shared state for middleware factories. type Middleware struct { - log *slog.Logger - params *Params + log *slog.Logger + params *Params + trustedProxies []netip.Prefix } // New creates a Middleware instance. @@ -39,13 +52,38 @@ func New( _ fx.Lifecycle, params Params, ) (*Middleware, error) { + trusted, err := parseTrustedProxies(params.Config.TrustedProxies) + if err != nil { + return nil, err + } + s := new(Middleware) s.params = ¶ms s.log = params.Logger.Get() + s.trustedProxies = trusted return s, nil } +// parseTrustedProxies converts CIDR strings into prefixes, +// failing fast on any malformed entry. +func parseTrustedProxies(cidrs []string) ([]netip.Prefix, error) { + prefixes := make([]netip.Prefix, 0, len(cidrs)) + + for _, cidr := range cidrs { + prefix, err := netip.ParsePrefix(cidr) + if err != nil { + return nil, fmt.Errorf( + "trusted proxy %q: %w", cidr, err, + ) + } + + prefixes = append(prefixes, prefix.Masked()) + } + + return prefixes, nil +} + type loggingResponseWriter struct { http.ResponseWriter @@ -72,6 +110,70 @@ func ipFromHostPort(hostPort string) string { return host } +// clientIP resolves the caller's address. X-Forwarded-For and +// X-Real-IP are honoured only when the direct peer is a +// trusted proxy; otherwise the direct peer is returned so a +// spoofed header cannot forge the logged address. +func clientIP( + remoteAddr string, + header http.Header, + trusted []netip.Prefix, +) string { + peer := ipFromHostPort(remoteAddr) + + if !addrInAny(peer, trusted) { + return peer + } + + if xff := firstForwardedFor(header.Get("X-Forwarded-For")); xff != "" { + return xff + } + + if xr := strings.TrimSpace(header.Get("X-Real-IP")); validIP(xr) { + return xr + } + + return peer +} + +// firstForwardedFor returns the left-most valid address in an +// X-Forwarded-For list (the original client), or "" if none. +func firstForwardedFor(value string) string { + for part := range strings.SplitSeq(value, ",") { + candidate := strings.TrimSpace(part) + if validIP(candidate) { + return candidate + } + } + + return "" +} + +func validIP(s string) bool { + _, err := netip.ParseAddr(s) + + return err == nil +} + +// addrInAny reports whether s parses as an address contained +// in any of the trusted prefixes. +func addrInAny(s string, trusted []netip.Prefix) bool { + addr, err := netip.ParseAddr(s) + if err != nil { + return false + } + + addr = addr.Unmap() + + for _, prefix := range trusted { + if prefix.Contains(addr) { + return true + } + } + + return false +} + // Logging returns middleware that logs each request with // timing, status code, and client information. func (s *Middleware) Logging() func(http.Handler) http.Handler { @@ -96,7 +198,11 @@ func (s *Middleware) Logging() func(http.Handler) http.Handler { "referer", r.Referer(), "proto", r.Proto, "remote_ip", - ipFromHostPort(r.RemoteAddr), + clientIP( + r.RemoteAddr, + r.Header, + s.trustedProxies, + ), "status", lrw.statusCode, "latency_ms", latency.Milliseconds(), @@ -109,6 +215,27 @@ func (s *Middleware) Logging() func(http.Handler) http.Handler { } } +// SecurityHeaders returns middleware that sets response +// security headers. It runs before CORS so the headers are +// present on preflight responses the CORS handler writes. +func (s *Middleware) SecurityHeaders() func(http.Handler) http.Handler { + return func(next http.Handler) http.Handler { + return http.HandlerFunc( + func(w http.ResponseWriter, r *http.Request) { + h := w.Header() + h.Set("Strict-Transport-Security", hstsValue) + h.Set("Content-Security-Policy", cspValue) + h.Set("X-Frame-Options", "DENY") + h.Set("X-Content-Type-Options", "nosniff") + h.Set("Referrer-Policy", "no-referrer") + h.Set("Permissions-Policy", permissionsPolicyValue) + + next.ServeHTTP(w, r) + }, + ) + } +} + // CORS returns middleware that adds permissive CORS headers. func (s *Middleware) CORS() func(http.Handler) http.Handler { return cors.Handler(cors.Options{ diff --git a/backend/internal/middleware/middleware_test.go b/backend/internal/middleware/middleware_test.go new file mode 100644 index 0000000..2965593 --- /dev/null +++ b/backend/internal/middleware/middleware_test.go @@ -0,0 +1,139 @@ +package middleware_test + +import ( + "net/http" + "net/http/httptest" + "net/netip" + "testing" + + "sneak.berlin/go/netwatch/internal/middleware" +) + +func mustPrefixes(t *testing.T, cidrs ...string) []netip.Prefix { + t.Helper() + + prefixes, err := middleware.ParseTrustedProxies(cidrs) + if err != nil { + t.Fatalf("ParseTrustedProxies(%v): %v", cidrs, err) + } + + return prefixes +} + +func TestParseTrustedProxiesRejectsMalformed(t *testing.T) { + t.Parallel() + + _, err := middleware.ParseTrustedProxies([]string{"not-a-cidr"}) + if err == nil { + t.Fatal("expected error for malformed CIDR, got nil") + } +} + +type clientIPCase struct { + name string + remoteAddr string + xff string + xRealIP string + want string +} + +func clientIPCases() []clientIPCase { + return []clientIPCase{ + { + name: "trusted proxy uses forwarded-for", + remoteAddr: "127.0.0.1:5000", + xff: "203.0.113.7", + want: "203.0.113.7", + }, + { + name: "trusted proxy uses left-most of chain", + remoteAddr: "10.1.2.3:5000", + xff: "203.0.113.7, 10.1.2.3", + want: "203.0.113.7", + }, + { + name: "trusted proxy falls back to x-real-ip", + remoteAddr: "127.0.0.1:5000", + xRealIP: "203.0.113.9", + want: "203.0.113.9", + }, + { + name: "untrusted peer ignores forwarded-for", + remoteAddr: "198.51.100.4:5000", + xff: "203.0.113.7", + want: "198.51.100.4", + }, + { + name: "untrusted peer ignores x-real-ip", + remoteAddr: "198.51.100.4:5000", + xRealIP: "203.0.113.9", + want: "198.51.100.4", + }, + { + name: "trusted proxy with no headers uses peer", + remoteAddr: "10.1.2.3:5000", + want: "10.1.2.3", + }, + { + name: "trusted proxy with garbage header uses peer", + remoteAddr: "127.0.0.1:5000", + xff: "not-an-ip", + want: "127.0.0.1", + }, + } +} + +func TestClientIP(t *testing.T) { + t.Parallel() + + trusted := mustPrefixes(t, "127.0.0.1/32", "::1/128", "10.0.0.0/8") + + for _, tc := range clientIPCases() { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + header := http.Header{} + if tc.xff != "" { + header.Set("X-Forwarded-For", tc.xff) + } + + if tc.xRealIP != "" { + header.Set("X-Real-IP", tc.xRealIP) + } + + got := middleware.ClientIP(tc.remoteAddr, header, trusted) + if got != tc.want { + t.Errorf("ClientIP() = %q, want %q", got, tc.want) + } + }) + } +} + +func TestSecurityHeaders(t *testing.T) { + t.Parallel() + + handler := (&middleware.Middleware{}).SecurityHeaders()( + http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusOK) + }), + ) + + rec := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodGet, "/", http.NoBody) + handler.ServeHTTP(rec, req) + + want := map[string]string{ + "Strict-Transport-Security": "max-age=31536000; includeSubDomains", + "Content-Security-Policy": "default-src 'none'; frame-ancestors 'none'", + "X-Frame-Options": "DENY", + "X-Content-Type-Options": "nosniff", + "Referrer-Policy": "no-referrer", + "Permissions-Policy": "camera=(), microphone=(), geolocation=()", + } + + for name, value := range want { + if got := rec.Header().Get(name); got != value { + t.Errorf("header %s = %q, want %q", name, got, value) + } + } +} diff --git a/backend/internal/server/http.go b/backend/internal/server/http.go index d3824c6..a0d3c68 100644 --- a/backend/internal/server/http.go +++ b/backend/internal/server/http.go @@ -8,20 +8,24 @@ import ( ) const ( - readTimeout = 10 * time.Second - writeTimeout = 10 * time.Second - maxHeaderBytes = 1 << 20 // 1 MiB + readTimeout = 10 * time.Second + readHeaderTimeout = 5 * time.Second + writeTimeout = 10 * time.Second + idleTimeout = 60 * time.Second + maxHeaderBytes = 1 << 20 // 1 MiB ) func (s *Server) serveUntilShutdown() { listenAddr := fmt.Sprintf(":%d", s.params.Config.Port) s.httpServer = &http.Server{ - Addr: listenAddr, - Handler: s, - MaxHeaderBytes: maxHeaderBytes, - ReadTimeout: readTimeout, - WriteTimeout: writeTimeout, + Addr: listenAddr, + Handler: s, + MaxHeaderBytes: maxHeaderBytes, + ReadTimeout: readTimeout, + ReadHeaderTimeout: readHeaderTimeout, + WriteTimeout: writeTimeout, + IdleTimeout: idleTimeout, } s.SetupRoutes() diff --git a/backend/internal/server/routes.go b/backend/internal/server/routes.go index da74cf6..5f2dd8b 100644 --- a/backend/internal/server/routes.go +++ b/backend/internal/server/routes.go @@ -17,6 +17,7 @@ func (s *Server) SetupRoutes() { s.router.Use(middleware.Recoverer) s.router.Use(middleware.RequestID) s.router.Use(s.mw.Logging()) + s.router.Use(s.mw.SecurityHeaders()) s.router.Use(s.mw.CORS()) s.router.Use(middleware.Timeout(requestTimeout))