diff --git a/.golangci.yml b/.golangci.yml index 61dfe53..807775f 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -14,9 +14,12 @@ output: linters: default: all + enable: + - gomodguard_v2 # replaces deprecated gomodguard disable: # Deprecated - wsl + - gomodguard # superseded by gomodguard_v2 # Too noisy / impractical - exhaustruct diff --git a/cmd/api/server/server.go b/cmd/api/server/server.go index 344c0aa..10f4eff 100644 --- a/cmd/api/server/server.go +++ b/cmd/api/server/server.go @@ -3,6 +3,7 @@ package server import ( "context" _ "embed" + "net" "net/http" "time" @@ -97,7 +98,7 @@ func newRouter( // Middleware r.Use(middleware.RequestID) - r.Use(middleware.RealIP) + r.Use(authmw.RealIP(mustParseTrustedProxies(cfg.Server.TrustedProxies))) r.Use(middleware.Logger) r.Use(middleware.Recoverer) r.Use(middleware.Timeout(60 * time.Second)) @@ -286,6 +287,18 @@ func isNoOpAuthenticator(a authmw.Authenticator) bool { return ok } +// mustParseTrustedProxies parses the configured CIDRs and panics on invalid +// input. Misconfiguration here is a deployment error that should fail fast at +// startup rather than silently disabling client-IP resolution. +func mustParseTrustedProxies(in []string) []*net.IPNet { + nets, err := authmw.ParseTrustedProxies(in) + if err != nil { + panic("invalid server.trusted_proxies: " + err.Error()) + } + + return nets +} + func corsMiddleware(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { // When an Origin header is present, echo it back and allow credentials diff --git a/config.example.yaml b/config.example.yaml index 68d015c..4ad1495 100644 --- a/config.example.yaml +++ b/config.example.yaml @@ -32,6 +32,12 @@ server: addr: ":8080" read_timeout: 15s write_timeout: 60s + # CIDRs (or bare IPs) whose X-Forwarded-For headers will be honored to + # set r.RemoteAddr. Leave empty to keep the actual TCP peer address. + # Set this to your ingress / load balancer CIDR when running behind one. + # trusted_proxies: + # - 10.0.0.0/8 + # - 172.16.0.0/12 # Database configuration # Env: CS_DATABASE_DRIVER, CS_DATABASE_URL, CS_DATABASE_MAX_OPEN_CONNS, CS_DATABASE_MAX_IDLE_CONNS, CS_DATABASE_CONN_MAX_LIFETIME diff --git a/internal/config/config.go b/internal/config/config.go index 9bf7e6e..fc62f9f 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -62,6 +62,11 @@ type ServerConfig struct { Addr string `mapstructure:"addr"` ReadTimeout time.Duration `mapstructure:"read_timeout"` WriteTimeout time.Duration `mapstructure:"write_timeout"` + // TrustedProxies is the list of CIDRs (or bare IPs) whose X-Forwarded-For + // headers will be honored to populate r.RemoteAddr. Leave empty to keep + // the actual TCP peer address — the safe default for direct exposure. + // Set this to the ingress/load-balancer CIDR when running behind one. + TrustedProxies []string `mapstructure:"trusted_proxies"` } type DatabaseConfig struct { diff --git a/internal/indexer/server.go b/internal/indexer/server.go index 5124187..c2e91f2 100644 --- a/internal/indexer/server.go +++ b/internal/indexer/server.go @@ -65,7 +65,6 @@ func (s *Server) Start() error { // Middleware r.Use(middleware.RequestID) - r.Use(middleware.RealIP) r.Use(middleware.Recoverer) r.Use(middleware.Timeout(60 * time.Second)) diff --git a/internal/middleware/realip.go b/internal/middleware/realip.go new file mode 100644 index 0000000..4abb2aa --- /dev/null +++ b/internal/middleware/realip.go @@ -0,0 +1,111 @@ +package middleware + +import ( + "fmt" + "net" + "net/http" + "slices" + "strings" +) + +// RealIP returns chi-compatible middleware that rewrites r.RemoteAddr to the +// real client IP by walking the X-Forwarded-For chain — but ONLY when the +// immediate TCP peer is in trustedProxies. This closes the spoofing vector +// that caused chi's own middleware.RealIP to be deprecated +// (GHSA-3fxj-6jh8-hvhx, GHSA-rjr7-jggh-pgcp, GHSA-9g5q-2w5x-hmxf). +// +// If trustedProxies is empty, the middleware is a no-op — r.RemoteAddr keeps +// the actual TCP peer address. That is the safe default: better to log the +// proxy IP than to trust a client-supplied header from an unknown network. +func RealIP(trustedProxies []*net.IPNet) func(http.Handler) http.Handler { + if len(trustedProxies) == 0 { + return func(next http.Handler) http.Handler { return next } + } + + return func(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if client := realClientIP(r, trustedProxies); client != "" { + r.RemoteAddr = client + } + + next.ServeHTTP(w, r) + }) + } +} + +func realClientIP(r *http.Request, trusted []*net.IPNet) string { + peerHost, _, err := net.SplitHostPort(r.RemoteAddr) + if err != nil { + peerHost = r.RemoteAddr + } + + peerIP := net.ParseIP(peerHost) + if peerIP == nil || !ipInAny(peerIP, trusted) { + return "" + } + + xff := r.Header.Get("X-Forwarded-For") + if xff == "" { + return "" + } + + parts := strings.Split(xff, ",") + for _, raw := range slices.Backward(parts) { + candidate := strings.TrimSpace(raw) + + ip := net.ParseIP(candidate) + if ip == nil { + continue + } + + if !ipInAny(ip, trusted) { + return candidate + } + } + + return "" +} + +func ipInAny(ip net.IP, nets []*net.IPNet) bool { + for _, n := range nets { + if n.Contains(ip) { + return true + } + } + + return false +} + +// ParseTrustedProxies parses CIDR notation strings into []*net.IPNet. +// Bare IPv4/IPv6 addresses are accepted and converted to single-host CIDRs. +func ParseTrustedProxies(in []string) ([]*net.IPNet, error) { + out := make([]*net.IPNet, 0, len(in)) + + for _, s := range in { + s = strings.TrimSpace(s) + if s == "" { + continue + } + + if _, n, err := net.ParseCIDR(s); err == nil { + out = append(out, n) + continue + } + + if ip := net.ParseIP(s); ip != nil { + suffix := "/32" + if ip.To4() == nil { + suffix = "/128" + } + + if _, n, err := net.ParseCIDR(s + suffix); err == nil { + out = append(out, n) + continue + } + } + + return nil, fmt.Errorf("invalid trusted_proxy value %q: expected CIDR or IP", s) + } + + return out, nil +} diff --git a/internal/middleware/realip_test.go b/internal/middleware/realip_test.go new file mode 100644 index 0000000..ce5d2af --- /dev/null +++ b/internal/middleware/realip_test.go @@ -0,0 +1,130 @@ +package middleware + +import ( + "net/http" + "net/http/httptest" + "testing" +) + +func TestRealIP_NoTrustedProxies_IsNoOp(t *testing.T) { + captured := "" + h := RealIP(nil)(http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) { + captured = r.RemoteAddr + })) + + r := httptest.NewRequest(http.MethodGet, "/", nil) + r.RemoteAddr = "1.2.3.4:5555" + r.Header.Set("X-Forwarded-For", "9.9.9.9") + h.ServeHTTP(httptest.NewRecorder(), r) + + if captured != "1.2.3.4:5555" { + t.Fatalf("expected RemoteAddr untouched, got %q", captured) + } +} + +func TestRealIP_TrustedPeer_UsesXFF(t *testing.T) { + trusted, err := ParseTrustedProxies([]string{"10.0.0.0/8"}) + if err != nil { + t.Fatal(err) + } + + captured := "" + h := RealIP(trusted)(http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) { + captured = r.RemoteAddr + })) + + r := httptest.NewRequest(http.MethodGet, "/", nil) + r.RemoteAddr = "10.0.5.6:5555" + r.Header.Set("X-Forwarded-For", "203.0.113.7") + h.ServeHTTP(httptest.NewRecorder(), r) + + if captured != "203.0.113.7" { + t.Fatalf("expected client IP from XFF, got %q", captured) + } +} + +func TestRealIP_UntrustedPeer_IgnoresXFF(t *testing.T) { + trusted, err := ParseTrustedProxies([]string{"10.0.0.0/8"}) + if err != nil { + t.Fatal(err) + } + + captured := "" + h := RealIP(trusted)(http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) { + captured = r.RemoteAddr + })) + + r := httptest.NewRequest(http.MethodGet, "/", nil) + r.RemoteAddr = "8.8.8.8:5555" + r.Header.Set("X-Forwarded-For", "203.0.113.7") + h.ServeHTTP(httptest.NewRecorder(), r) + + if captured != "8.8.8.8:5555" { + t.Fatalf("expected RemoteAddr untouched for untrusted peer, got %q", captured) + } +} + +func TestRealIP_ChainOfTrustedProxies(t *testing.T) { + trusted, err := ParseTrustedProxies([]string{"10.0.0.0/8", "172.16.0.0/12"}) + if err != nil { + t.Fatal(err) + } + + captured := "" + h := RealIP(trusted)(http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) { + captured = r.RemoteAddr + })) + + r := httptest.NewRequest(http.MethodGet, "/", nil) + r.RemoteAddr = "10.0.5.6:5555" + // Chain: client → ext LB (untrusted) → internal LB (trusted) → peer (trusted) + r.Header.Set("X-Forwarded-For", "203.0.113.7, 172.16.0.1, 10.0.5.1") + h.ServeHTTP(httptest.NewRecorder(), r) + + if captured != "203.0.113.7" { + t.Fatalf("expected first untrusted IP from right, got %q", captured) + } +} + +func TestRealIP_AllForwardersTrusted_NoMatch(t *testing.T) { + trusted, err := ParseTrustedProxies([]string{"10.0.0.0/8"}) + if err != nil { + t.Fatal(err) + } + + captured := "" + h := RealIP(trusted)(http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) { + captured = r.RemoteAddr + })) + + r := httptest.NewRequest(http.MethodGet, "/", nil) + r.RemoteAddr = "10.0.5.6:5555" + r.Header.Set("X-Forwarded-For", "10.0.5.1, 10.0.5.2") + h.ServeHTTP(httptest.NewRecorder(), r) + + if captured != "10.0.5.6:5555" { + t.Fatalf("expected RemoteAddr untouched when no untrusted hop, got %q", captured) + } +} + +func TestParseTrustedProxies(t *testing.T) { + cases := []struct { + in []string + wantOK bool + }{ + {[]string{"10.0.0.0/8"}, true}, + {[]string{"::1/128"}, true}, + {[]string{"192.168.1.1"}, true}, // bare IP → /32 + {[]string{"2001:db8::1"}, true}, // bare IPv6 → /128 + {[]string{""}, true}, // empty entries skipped + {[]string{"not-an-ip"}, false}, + {[]string{"10.0.0.0/99"}, false}, + } + + for _, c := range cases { + _, err := ParseTrustedProxies(c.in) + if (err == nil) != c.wantOK { + t.Errorf("ParseTrustedProxies(%v): err=%v, wantOK=%v", c.in, err, c.wantOK) + } + } +}