mirror of
https://github.com/gesellix/Bose-SoundTouch.git
synced 2026-08-19 09:06:14 +00:00
feat(handlers): proxy-aware RemoteAddr via opt-in TrustForwardedHeaders
Wire up X-Real-IP / X-Forwarded-For / True-Client-IP support for deployments fronted by a reverse proxy, while staying safe on flat-LAN deployments where a malicious speaker could spoof those headers directly. Two new fields on `datastore.Settings`: * TrustForwardedHeaders (bool, default false) — opt-in switch. * TrustedProxyCIDRs ([]string, default `["127.0.0.0/8", "::1/128"]`) — only requests whose immediate TCP peer falls in one of these blocks may have their source IP rewritten from forwarded headers. Loopback default matches the documented same-host nginx layout in docs/guides/HTTPS-SETUP.md. New middleware in `pkg/service/handlers/middleware_realip.go`: * TrustedRealIP wraps `chi/middleware.RealIP` with a trusted-peer gate. When the immediate TCP peer is in the allowlist, chi's parsing handles the actual header → IP rewrite. When it isn't (e.g. a speaker sending forwarded headers itself), we ignore the headers and r.RemoteAddr stays as-is. * ParseTrustedProxyCIDRs converts string CIDRs into *net.IPNet, applying the loopback default on empty input and erroring loudly on invalid entries. Server.TrustedRealIPMiddleware() returns the middleware (or nil) by reading the live settings; the router setup in cmd/soundtouch-service/main.go installs it as the very first middleware so SnapshotMiddleware and downstream handlers see the correct r.RemoteAddr. HandleMargePowerOn now prefers r.RemoteAddr over the body's self-reported `<IPAddress>` for outbound credential push: * The body field is treated as a hint only — a malicious LAN speaker could set it to any value; using it for outbound HTTP requests is the SSRF surface the previous zeroconf hardening was guarding against from the sink side. Fixing it at the source as well closes the gap entirely. * When body IP and TCP source disagree, a log line names both and the device ID so the discrepancy is investigable. * RemoteAddr is unparseable → fall back to the body so we don't silently drop the priming. docs/guides/HTTPS-SETUP.md gains a follow-up note next to the existing nginx snippet explaining the new flag, the loopback-only default, and the explicit warning against enabling the flag on a flat-LAN deployment without a real proxy. Eleven test cases in middleware_realip_test.go lock in the gate behaviour: trusted peers honoured for X-Real-IP / X-Forwarded-For / no-headers / IPv6, untrusted peers' headers ignored, garbage values rejected, ParseTrustedProxyCIDRs covers default / override / invalid. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.7
parent
dc1f811a81
commit
f951fc92df
@@ -1806,6 +1806,22 @@ type Settings struct {
|
||||
// keeps verification on; opt in only when the upstream certificate chain is
|
||||
// broken (post end-of-service) and a temporary unblock is required.
|
||||
AllowInsecureUpstreamTLS bool `json:"allow_insecure_upstream_tls,omitempty"`
|
||||
|
||||
// TrustForwardedHeaders enables proxy-aware client IP resolution: when the
|
||||
// immediate TCP peer is one of the TrustedProxyCIDRs, the X-Real-IP /
|
||||
// X-Forwarded-For / True-Client-IP headers are honoured and replace
|
||||
// r.RemoteAddr. Required when the service is fronted by nginx, Caddy, or
|
||||
// any other reverse proxy. Default false — direct LAN deployments must
|
||||
// not enable this, otherwise a malicious LAN-resident client could spoof
|
||||
// its source IP via these headers.
|
||||
TrustForwardedHeaders bool `json:"trust_forwarded_headers,omitempty"`
|
||||
|
||||
// TrustedProxyCIDRs is the list of CIDR blocks whose immediate TCP peers
|
||||
// are allowed to set X-Forwarded-* headers when TrustForwardedHeaders is
|
||||
// true. Defaults to loopback (127.0.0.0/8 and ::1/128) — i.e. only a
|
||||
// reverse proxy on the same host. Override only if the proxy lives on a
|
||||
// different host within a known-good private subnet.
|
||||
TrustedProxyCIDRs []string `json:"trusted_proxy_cidrs,omitempty"`
|
||||
}
|
||||
|
||||
// GetSettings retrieves the global service settings.
|
||||
|
||||
@@ -289,13 +289,32 @@ func (s *Server) HandleMargePowerOn(w http.ResponseWriter, r *http.Request) {
|
||||
}
|
||||
}
|
||||
|
||||
if deviceIP != "" {
|
||||
go s.PrimeDeviceWithSpotify(deviceIP)
|
||||
} else {
|
||||
// Fallback to remote address if IP is missing from XML
|
||||
if host, _, err := net.SplitHostPort(r.RemoteAddr); err == nil {
|
||||
go s.PrimeDeviceWithSpotify(host)
|
||||
}
|
||||
// Prefer the TCP source address over the body's self-reported IP for
|
||||
// any outbound credential push. The body field is attacker-controllable
|
||||
// (a malicious LAN-resident speaker can set it to any value), while
|
||||
// r.RemoteAddr is the actual peer — and if the service runs behind a
|
||||
// trusted reverse proxy, the TrustedRealIP middleware has already
|
||||
// rewritten it from X-Real-IP / X-Forwarded-For. We log when the two
|
||||
// disagree so the discrepancy is investigable but never trust the body.
|
||||
remoteHost := ""
|
||||
if h, _, err := net.SplitHostPort(r.RemoteAddr); err == nil {
|
||||
remoteHost = h
|
||||
}
|
||||
|
||||
if deviceIP != "" && remoteHost != "" && deviceIP != remoteHost {
|
||||
log.Printf("[Marge] power_on body IP %q differs from TCP source %q for device %s — using TCP source for credential push",
|
||||
deviceIP, remoteHost, deviceID)
|
||||
}
|
||||
|
||||
target := remoteHost
|
||||
if target == "" {
|
||||
// RemoteAddr was unparseable (shouldn't happen under net/http) —
|
||||
// fall back to the body so we don't silently skip the push.
|
||||
target = deviceIP
|
||||
}
|
||||
|
||||
if target != "" {
|
||||
go s.PrimeDeviceWithSpotify(target)
|
||||
}
|
||||
|
||||
w.WriteHeader(http.StatusOK)
|
||||
|
||||
@@ -0,0 +1,95 @@
|
||||
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
|
||||
}
|
||||
@@ -0,0 +1,155 @@
|
||||
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.168.1.10",
|
||||
wantRemoteAddr: "192.168.1.10",
|
||||
},
|
||||
{
|
||||
name: "trusted peer with X-Forwarded-For is honoured",
|
||||
remoteAddr: "127.0.0.1:54321",
|
||||
xForwardedFor: "192.168.1.20, 10.0.0.1",
|
||||
wantRemoteAddr: "192.168.1.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.168.1.99:54321",
|
||||
xRealIP: "1.2.3.4",
|
||||
wantRemoteAddr: "192.168.1.99:54321",
|
||||
},
|
||||
{
|
||||
name: "untrusted peer's X-Forwarded-For is ignored",
|
||||
remoteAddr: "192.168.1.99:54321",
|
||||
xForwardedFor: "1.2.3.4",
|
||||
wantRemoteAddr: "192.168.1.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")
|
||||
}
|
||||
})
|
||||
}
|
||||
@@ -100,6 +100,35 @@ func NewServer(ds *datastore.DataStore, sm *setup.Manager, serverURL string, pro
|
||||
return s
|
||||
}
|
||||
|
||||
// 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.
|
||||
//
|
||||
// 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 {
|
||||
settings, err := s.ds.GetSettings()
|
||||
if err != nil {
|
||||
log.Printf("[RealIP] failed to load settings: %v — skipping forwarded-header trust", err)
|
||||
return 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)
|
||||
}
|
||||
|
||||
// SetVersionInfo sets the version information for the server.
|
||||
func (s *Server) SetVersionInfo(version, commit, date, repoURL string) {
|
||||
s.mu.Lock()
|
||||
|
||||
Reference in New Issue
Block a user