github/Bose-SoundTouch
1
0
Fork 0
mirror of https://github.com/gesellix/Bose-SoundTouch.git synced 2026-08-18 08:36:13 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(handlers): resolve client IP via chi ClientIP, drop deprecated RealIP

Browse Source 
chi v5.3.0 deprecates middleware.RealIP (IP-spoofing advisories), which
failed the Lint and Static Security Analysis CI jobs (SA1019). Replace the
RealIP wrapper with chi's middleware.ClientIP: ClientIPFromRemoteAddr is
always applied so middleware.GetClientIP is populated, and when
trust_forwarded_headers is set and the immediate peer is a trusted-proxy
CIDR, ClientIPFromXFF resolves the real client from X-Forwarded-For
(rightmost entry outside the trusted CIDRs). The immediate-peer trust gate
is preserved, so a non-trusted peer's XFF is ignored. CIDR strings are
validated with netip.ParsePrefix first to avoid ClientIPFromXFF's panic.

Behavior change: only X-Forwarded-For is honored now (RealIP also read
X-Real-IP / True-Client-IP). Docs and a release note follow.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Tobias Gesellchen
2026-06-28 13:15:49 +02:00
co-authored by Claude Opus 4.8
parent 28d7675fc4
commit 67c30850cd
7 changed files with 345 additions and 278 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/service/handlers/middleware_clientip.go
+139
View File
@@ -0,0 +1,139 @@
package handlers
import (
"fmt"
"log"
"net"
"net/http"
"net/netip"
"github.com/go-chi/chi/v5/middleware"
)
// defaultTrustedProxyCIDRs is the safe-by-default list applied when
// Settings.TrustedProxyCIDRs is empty. Only loopback addresses are trusted —
// i.e. a reverse proxy on the same host. Anyone deploying behind a proxy on a
// different host must override this in settings.json.
var defaultTrustedProxyCIDRs = []string{
"127.0.0.0/8",
"::1/128",
}
// clientIPMiddleware resolves the client IP into the request context (read via
// middleware.GetClientIP). The socket peer is always recorded. When
// trustForwardedHeaders is set AND the immediate TCP peer is one of trustedPeers,
// the X-Forwarded-For chain is consulted (chi walks it right-to-left, skipping
// trustedCIDRStrings, taking the first untrusted entry). On a flat LAN a
// non-trusted peer's XFF is ignored, so a malicious speaker can't spoof its IP.
func clientIPMiddleware(trustForwardedHeaders bool, trustedPeers []*net.IPNet, trustedCIDRStrings []string) func(http.Handler) http.Handler {
base := middleware.ClientIPFromRemoteAddr
if !trustForwardedHeaders || len(trustedPeers) == 0 {
return func(next http.Handler) http.Handler { return base(next) }
}
xff := middleware.ClientIPFromXFF(trustedCIDRStrings...)
return func(next http.Handler) http.Handler {
trusted := base(xff(next)) // peer set first, XFF overrides when found
untrusted := base(next) // peer only
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if isFromTrustedPeer(r.RemoteAddr, trustedPeers) {
trusted.ServeHTTP(w, r)
return
}
untrusted.ServeHTTP(w, r)
})
}
}
// isFromTrustedPeer reports whether remoteAddr (in the host:port shape that
// net/http populates) is contained in any of the supplied CIDR blocks.
func isFromTrustedPeer(remoteAddr string, trustedPeers []*net.IPNet) bool {
host, _, err := net.SplitHostPort(remoteAddr)
if err != nil {
host = remoteAddr
}
ip := net.ParseIP(host)
if ip == nil {
return false
}
for _, n := range trustedPeers {
if n.Contains(ip) {
return true
}
}
return false
}
// ParseTrustedProxyCIDRs converts string CIDRs into *net.IPNet values, falling
// back to defaultTrustedProxyCIDRs when the input is empty. An invalid CIDR
// in the input list is reported as an error and stops parsing — better to
// fail loud than silently fall back.
func ParseTrustedProxyCIDRs(cidrs []string) ([]*net.IPNet, error) {
if len(cidrs) == 0 {
cidrs = defaultTrustedProxyCIDRs
}
out := make([]*net.IPNet, 0, len(cidrs))
for _, c := range cidrs {
_, n, err := net.ParseCIDR(c)
if err != nil {
return nil, fmt.Errorf("invalid trusted proxy CIDR %q: %w", c, err)
}
out = append(out, n)
}
return out, nil
}
// validateCIDRStringsForXFF checks that every CIDR string can be parsed by
// netip.ParsePrefix, which is what middleware.ClientIPFromXFF uses internally
// (it calls netip.MustParsePrefix and panics on failure). Returns an error
// listing the first bad entry, so the caller can fall back to peer-only mode
// rather than panicking at startup.
func validateCIDRStringsForXFF(cidrs []string) error {
for _, c := range cidrs {
if _, err := netip.ParsePrefix(c); err != nil {
return fmt.Errorf("CIDR %q is not valid for ClientIPFromXFF: %w", c, err)
}
}
return nil
}
// buildClientIPMiddleware is the server-level helper that reads settings and
// returns a ready-to-use chi middleware. It is split out of ClientIPMiddleware
// so tests can drive the logic without a full Server.
func buildClientIPMiddleware(trustForwardedHeaders bool, cidrStrings []string) func(http.Handler) http.Handler {
if !trustForwardedHeaders {
return clientIPMiddleware(false, nil, nil)
}
if len(cidrStrings) == 0 {
cidrStrings = defaultTrustedProxyCIDRs
}
// Validate for netip.MustParsePrefix (panic guard).
if err := validateCIDRStringsForXFF(cidrStrings); err != nil {
log.Printf("[ClientIP] invalid trusted_proxy_cidrs: %v — falling back to peer-only", err)
return clientIPMiddleware(false, nil, nil)
}
// Parse for the peer gate (net.IPNet).
cidrs, err := ParseTrustedProxyCIDRs(cidrStrings)
if err != nil {
log.Printf("[ClientIP] invalid trusted_proxy_cidrs: %v — falling back to peer-only", err)
return clientIPMiddleware(false, nil, nil)
}
return clientIPMiddleware(true, cidrs, cidrStrings)
}
pkg/service/handlers/middleware_clientip_test.go
+187
View File
@@ -0,0 +1,187 @@
package handlers
import (
"net"
"net/http"
"net/http/httptest"
"testing"
"github.com/go-chi/chi/v5/middleware"
)
// captureClientIP is a tiny handler that records the resolved client IP from
// the request context (as set by clientIPMiddleware).
func captureClientIP(got *string) http.Handler {
return http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) {
*got = middleware.GetClientIP(r.Context())
})
}
func TestClientIPMiddleware(t *testing.T) {
defaultCIDRs := []string{"127.0.0.0/8", "::1/128"}
defaultPeers, err := ParseTrustedProxyCIDRs(defaultCIDRs)
if err != nil {
t.Fatalf("ParseTrustedProxyCIDRs: %v", err)
}
cases := []struct {
name string
trustForwardedHeaders bool
trustedPeers []*net.IPNet
trustedCIDRStrings []string
remoteAddr string
xForwardedFor string
wantClientIP string
}{
{
name: "trust disabled: peer recorded, XFF ignored",
trustForwardedHeaders: false,
remoteAddr: "1.2.3.4:5555",
xForwardedFor: "9.9.9.9",
wantClientIP: "1.2.3.4",
},
{
name: "trust enabled, trusted loopback peer, XFF honoured",
trustForwardedHeaders: true,
trustedPeers: defaultPeers,
trustedCIDRStrings: defaultCIDRs,
remoteAddr: "127.0.0.1:5555",
xForwardedFor: "9.9.9.9",
wantClientIP: "9.9.9.9",
},
{
name: "trust enabled, untrusted peer, XFF ignored (peer gate)",
trustForwardedHeaders: true,
trustedPeers: defaultPeers,
trustedCIDRStrings: defaultCIDRs,
remoteAddr: "8.8.8.8:5555",
xForwardedFor: "9.9.9.9",
wantClientIP: "8.8.8.8",
},
{
name: "trust enabled, trusted peer, XFF chain rightmost-untrusted wins",
trustForwardedHeaders: true,
trustedPeers: defaultPeers,
trustedCIDRStrings: defaultCIDRs,
remoteAddr: "127.0.0.1:5555",
xForwardedFor: "9.9.9.9, 127.0.0.2",
wantClientIP: "9.9.9.9",
},
{
name: "trust enabled, trusted peer, garbage XFF falls back to peer",
trustForwardedHeaders: true,
trustedPeers: defaultPeers,
trustedCIDRStrings: defaultCIDRs,
remoteAddr: "127.0.0.1:5555",
xForwardedFor: "not-an-ip",
wantClientIP: "127.0.0.1",
},
{
name: "trust enabled, trusted IPv6 loopback peer, XFF honoured",
trustForwardedHeaders: true,
trustedPeers: defaultPeers,
trustedCIDRStrings: defaultCIDRs,
remoteAddr: "[::1]:5555",
xForwardedFor: "9.9.9.9",
wantClientIP: "9.9.9.9",
},
{
name: "trust enabled, trusted peer, no XFF header: peer recorded",
trustForwardedHeaders: true,
trustedPeers: defaultPeers,
trustedCIDRStrings: defaultCIDRs,
remoteAddr: "127.0.0.1:5555",
xForwardedFor: "",
wantClientIP: "127.0.0.1",
},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
var got string
mw := clientIPMiddleware(tc.trustForwardedHeaders, tc.trustedPeers, tc.trustedCIDRStrings)
h := mw(captureClientIP(&got))
req := httptest.NewRequest(http.MethodGet, "/", nil)
req.RemoteAddr = tc.remoteAddr
if tc.xForwardedFor != "" {
req.Header.Set("X-Forwarded-For", tc.xForwardedFor)
}
h.ServeHTTP(httptest.NewRecorder(), req)
if got != tc.wantClientIP {
t.Errorf("GetClientIP = %q, want %q", got, tc.wantClientIP)
}
})
}
}
func TestClientIPMiddleware_AlwaysPopulated(t *testing.T) {
// Regardless of trust settings, GetClientIP must always return a non-empty
// string after clientIPMiddleware has run (the socket peer is the fallback).
mw := clientIPMiddleware(false, nil, nil)
var got string
h := mw(captureClientIP(&got))
req := httptest.NewRequest(http.MethodGet, "/", nil)
req.RemoteAddr = "10.0.0.1:1234"
h.ServeHTTP(httptest.NewRecorder(), req)
if got == "" {
t.Error("GetClientIP must not be empty after clientIPMiddleware runs")
}
}
func TestParseTrustedProxyCIDRs(t *testing.T) {
t.Run("empty input yields loopback default", func(t *testing.T) {
got, err := ParseTrustedProxyCIDRs(nil)
if err != nil {
t.Fatalf("ParseTrustedProxyCIDRs: %v", err)
}
if len(got) != 2 {
t.Fatalf("default CIDR count = %d, want 2 (127/8 + ::1/128)", len(got))
}
// Should contain 127.0.0.1 and ::1.
if !isFromTrustedPeer("127.0.0.1:1", got) {
t.Error("default CIDRs should include 127.0.0.1")
}
if !isFromTrustedPeer("[::1]:1", got) {
t.Error("default CIDRs should include ::1")
}
})
t.Run("custom CIDRs override defaults", func(t *testing.T) {
got, err := ParseTrustedProxyCIDRs([]string{"10.0.0.0/8"})
if err != nil {
t.Fatalf("ParseTrustedProxyCIDRs: %v", err)
}
if len(got) != 1 {
t.Errorf("custom CIDR count = %d, want 1", len(got))
}
if !isFromTrustedPeer("10.1.2.3:1", got) {
t.Error("10.1.2.3 should be in 10.0.0.0/8")
}
if isFromTrustedPeer("127.0.0.1:1", got) {
t.Error("127.0.0.1 should NOT match when default is overridden")
}
})
t.Run("invalid CIDR returns error", func(t *testing.T) {
_, err := ParseTrustedProxyCIDRs([]string{"not-a-cidr"})
if err == nil {
t.Fatal("expected error on invalid CIDR")
}
})
}
pkg/service/handlers/middleware_peer_observer.go
+2 -2
View File
@@ -12,8 +12,8 @@ import (
// the handler runs, so passive reachability probes can register a device
// IP and learn whether any inbound landed in their wait window.
//
// Placement: after TrustedRealIPMiddleware (so r.RemoteAddr reflects the
// trusted client IP) and after Recoverer (so any panic inside this
// Placement: after ClientIPMiddleware (so the resolved client IP is available
// via middleware.GetClientIP) and after Recoverer (so any panic inside this
// middleware is contained). Before any short-circuiting middleware
// would be unnecessary — Signal runs before next.ServeHTTP, so the
// observation lands regardless of how later middleware handles the
pkg/service/handlers/middleware_realip.go
-95
View File
@@ -1,95 +0,0 @@
package handlers
import (
"fmt"
"net"
"net/http"
"github.com/go-chi/chi/v5/middleware"
)
// defaultTrustedProxyCIDRs is the safe-by-default list applied when
// Settings.TrustedProxyCIDRs is empty. Only loopback addresses are trusted —
// i.e. a reverse proxy on the same host. Anyone deploying behind a proxy on a
// different host must override this in settings.json.
var defaultTrustedProxyCIDRs = []string{
"127.0.0.0/8",
"::1/128",
}
// TrustedRealIP returns a middleware that delegates to chi's RealIP — which
// rewrites r.RemoteAddr from True-Client-IP / X-Real-IP / X-Forwarded-For
// headers — but only when the immediate TCP peer is in `trustedPeers`. For
// any request whose peer is *not* trusted (i.e. anything other than the
// configured reverse proxy), the headers are ignored and r.RemoteAddr stays
// as-is.
//
// This avoids the standard X-Forwarded-* spoofing pitfall: on a flat LAN
// where a malicious speaker could send the headers itself, we won't honour
// them; behind a reverse proxy we will.
//
// Returns nil if trustedPeers is empty — caller should not Use a nil mw.
func TrustedRealIP(trustedPeers []*net.IPNet) func(http.Handler) http.Handler {
if len(trustedPeers) == 0 {
return nil
}
delegate := middleware.RealIP
return func(next http.Handler) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if isFromTrustedPeer(r.RemoteAddr, trustedPeers) {
delegate(next).ServeHTTP(w, r)
return
}
next.ServeHTTP(w, r)
})
}
}
// isFromTrustedPeer reports whether remoteAddr (in the host:port shape that
// net/http populates) is contained in any of the supplied CIDR blocks.
func isFromTrustedPeer(remoteAddr string, trustedPeers []*net.IPNet) bool {
host, _, err := net.SplitHostPort(remoteAddr)
if err != nil {
host = remoteAddr
}
ip := net.ParseIP(host)
if ip == nil {
return false
}
for _, n := range trustedPeers {
if n.Contains(ip) {
return true
}
}
return false
}
// ParseTrustedProxyCIDRs converts string CIDRs into *net.IPNet values, falling
// back to defaultTrustedProxyCIDRs when the input is empty. An invalid CIDR
// in the input list is reported as an error and stops parsing — better to
// fail loud than silently fall back.
func ParseTrustedProxyCIDRs(cidrs []string) ([]*net.IPNet, error) {
if len(cidrs) == 0 {
cidrs = defaultTrustedProxyCIDRs
}
out := make([]*net.IPNet, 0, len(cidrs))
for _, c := range cidrs {
_, n, err := net.ParseCIDR(c)
if err != nil {
return nil, fmt.Errorf("invalid trusted proxy CIDR %q: %w", c, err)
}
out = append(out, n)
}
return out, nil
}
pkg/service/handlers/middleware_realip_test.go
-155
View File
@@ -1,155 +0,0 @@
package handlers
import (
"net"
"net/http"
"net/http/httptest"
"testing"
)
func TestTrustedRealIP(t *testing.T) {
cidrs, err := ParseTrustedProxyCIDRs([]string{"127.0.0.0/8", "::1/128"})
if err != nil {
t.Fatalf("ParseTrustedProxyCIDRs: %v", err)
}
mw := TrustedRealIP(cidrs)
if mw == nil {
t.Fatal("TrustedRealIP returned nil for non-empty trustedPeers")
}
cases := []struct {
name string
remoteAddr string
xRealIP string
xForwardedFor string
wantRemoteAddr string
}{
{
name: "trusted peer with X-Real-IP is honoured",
remoteAddr: "127.0.0.1:54321",
xRealIP: "192.0.2.10",
wantRemoteAddr: "192.0.2.10",
},
{
name: "trusted peer with X-Forwarded-For is honoured",
remoteAddr: "127.0.0.1:54321",
xForwardedFor: "192.0.2.20, 10.0.0.1",
wantRemoteAddr: "192.0.2.20",
},
{
name: "trusted peer with no headers leaves RemoteAddr alone",
remoteAddr: "127.0.0.1:54321",
wantRemoteAddr: "127.0.0.1:54321",
},
{
name: "untrusted peer's X-Real-IP is ignored",
remoteAddr: "192.0.2.99:54321",
xRealIP: "1.2.3.4",
wantRemoteAddr: "192.0.2.99:54321",
},
{
name: "untrusted peer's X-Forwarded-For is ignored",
remoteAddr: "192.0.2.99:54321",
xForwardedFor: "1.2.3.4",
wantRemoteAddr: "192.0.2.99:54321",
},
{
name: "trusted peer with garbage X-Real-IP leaves RemoteAddr alone",
remoteAddr: "127.0.0.1:54321",
xRealIP: "not-an-ip",
wantRemoteAddr: "127.0.0.1:54321",
},
{
name: "trusted IPv6 loopback peer is honoured",
remoteAddr: "[::1]:54321",
xRealIP: "fe80::1",
wantRemoteAddr: "fe80::1",
},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
var got string
h := mw(http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) {
got = r.RemoteAddr
}))
req := httptest.NewRequest(http.MethodGet, "/", nil)
req.RemoteAddr = tc.remoteAddr
if tc.xRealIP != "" {
req.Header.Set("X-Real-IP", tc.xRealIP)
}
if tc.xForwardedFor != "" {
req.Header.Set("X-Forwarded-For", tc.xForwardedFor)
}
h.ServeHTTP(httptest.NewRecorder(), req)
if got != tc.wantRemoteAddr {
t.Errorf("RemoteAddr = %q, want %q", got, tc.wantRemoteAddr)
}
})
}
}
func TestTrustedRealIP_NilForEmptyPeers(t *testing.T) {
if mw := TrustedRealIP(nil); mw != nil {
t.Error("TrustedRealIP(nil) returned non-nil; expected nil so caller can skip Use()")
}
if mw := TrustedRealIP([]*net.IPNet{}); mw != nil {
t.Error("TrustedRealIP([]) returned non-nil; expected nil so caller can skip Use()")
}
}
func TestParseTrustedProxyCIDRs(t *testing.T) {
t.Run("empty input yields loopback default", func(t *testing.T) {
got, err := ParseTrustedProxyCIDRs(nil)
if err != nil {
t.Fatalf("ParseTrustedProxyCIDRs: %v", err)
}
if len(got) != 2 {
t.Fatalf("default CIDR count = %d, want 2 (127/8 + ::1/128)", len(got))
}
// Should contain 127.0.0.1 and ::1.
if !isFromTrustedPeer("127.0.0.1:1", got) {
t.Error("default CIDRs should include 127.0.0.1")
}
if !isFromTrustedPeer("[::1]:1", got) {
t.Error("default CIDRs should include ::1")
}
})
t.Run("custom CIDRs override defaults", func(t *testing.T) {
got, err := ParseTrustedProxyCIDRs([]string{"10.0.0.0/8"})
if err != nil {
t.Fatalf("ParseTrustedProxyCIDRs: %v", err)
}
if len(got) != 1 {
t.Errorf("custom CIDR count = %d, want 1", len(got))
}
if !isFromTrustedPeer("10.1.2.3:1", got) {
t.Error("10.1.2.3 should be in 10.0.0.0/8")
}
if isFromTrustedPeer("127.0.0.1:1", got) {
t.Error("127.0.0.1 should NOT match when default is overridden")
}
})
t.Run("invalid CIDR returns error", func(t *testing.T) {
_, err := ParseTrustedProxyCIDRs([]string{"not-a-cidr"})
if err == nil {
t.Fatal("expected error on invalid CIDR")
}
})
}
pkg/service/handlers/server.go
+12 -19
View File
@@ -485,33 +485,26 @@ func (s *Server) loadOwnCACert() *x509.Certificate {
return s.ownCACache.cert
}
// TrustedRealIPMiddleware returns a chi middleware that rewrites
// r.RemoteAddr from X-Real-IP / X-Forwarded-For / True-Client-IP, but only
// when the immediate TCP peer is in the configured trusted-proxy list.
// Returns nil when Settings.TrustForwardedHeaders is false (the safe
// default), so the caller can skip wiring the middleware entirely.
// ClientIPMiddleware returns a chi middleware that resolves the client IP into
// the request context (read via middleware.GetClientIP). Always returns a
// non-nil middleware: at minimum, the socket peer is recorded.
//
// When Settings.TrustForwardedHeaders is true and the immediate TCP peer is in
// the configured trusted-proxy list, the X-Forwarded-For header is also
// consulted: chi walks the chain right-to-left, skipping entries that fall
// within the trusted CIDRs, and stores the first untrusted entry as the client.
//
// The trusted-peer gate prevents the typical X-Forwarded-* spoofing surface:
// on a flat LAN where a malicious speaker could send the headers itself, we
// won't honour them; behind a documented reverse proxy on loopback we will.
func (s *Server) TrustedRealIPMiddleware() func(http.Handler) http.Handler {
func (s *Server) ClientIPMiddleware() func(http.Handler) http.Handler {
settings, err := s.ds.GetSettings()
if err != nil {
log.Printf("[RealIP] failed to load settings: %v — skipping forwarded-header trust", err)
return nil
log.Printf("[ClientIP] failed to load settings: %v - falling back to peer-only", err)
return clientIPMiddleware(false, nil, nil)
}
if !settings.TrustForwardedHeaders {
return nil
}
cidrs, err := ParseTrustedProxyCIDRs(settings.TrustedProxyCIDRs)
if err != nil {
log.Printf("[RealIP] invalid trusted_proxy_cidrs: %v — skipping forwarded-header trust", err)
return nil
}
return TrustedRealIP(cidrs)
return buildClientIPMiddleware(settings.TrustForwardedHeaders, settings.TrustedProxyCIDRs)
}
// SetVersionInfo sets the version information for the server.