From 845d285e9007fca08d711b26bceee058e3b077dc Mon Sep 17 00:00:00 2001 From: Jonathon Leight Date: Tue, 23 Jun 2026 19:11:54 -0400 Subject: [PATCH] Add trusted reverse proxy settings --- README.md | 1 + internal/config/config.go | 83 +++++++++++++++--- internal/core/proxytest.go | 29 ++++++- internal/core/templates/proxy_test.html | 31 +++++++ internal/web/clientip.go | 83 ++++++++++++++++++ internal/web/env.go | 4 +- internal/web/proxyaddr_test.go | 109 +++++++++++++++--------- 7 files changed, 284 insertions(+), 56 deletions(-) create mode 100644 internal/web/clientip.go diff --git a/README.md b/README.md index a2e139b..dad6833 100644 --- a/README.md +++ b/README.md @@ -80,6 +80,7 @@ must serve the confirm/control pages over HTTPS. | `MESHTENDER_MASTER_KEY` | 64 hex chars (32 bytes); AES-GCM key encrypting the identity seed at rest (required) | | `MESHTENDER_ADDR` | listen address (default `:8080`) | | `MESHTENDER_RP_ID` / `_RP_ORIGIN` / `_RP_NAME` | WebAuthn relying-party settings | +| `MESHTENDER_TRUSTED_PROXIES` | proxies whose `X-Forwarded-For`/`X-Real-IP` are trusted when resolving the client IP — comma-separated CIDRs/IPs, or `private` for the RFC1918/link-local/ULA ranges. Loopback is always trusted. Verify with the admin **Reverse proxy test** page. | > **Note:** `MESHTENDER_MASTER_KEY` is coupled to the stored identity — changing it makes the > existing `server_identity` row undecryptable. Keep it stable. diff --git a/internal/config/config.go b/internal/config/config.go index 227993d..12e83da 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -4,6 +4,7 @@ package config import ( "encoding/hex" "fmt" + "net" "os" "strings" ) @@ -61,6 +62,16 @@ type Config struct { // production TLS usually terminates at a proxy and these stay empty. TLSCert string TLSKey string + + // TrustedProxies are CIDR ranges whose X-Forwarded-For / X-Real-IP headers are + // trusted when resolving a request's client IP. Loopback is always trusted (a + // same-host reverse proxy). The client IP is the rightmost X-Forwarded-For + // entry that is NOT a trusted proxy; if the connecting peer itself isn't + // trusted, forwarding headers are ignored entirely (so they can't be spoofed). + // Configured via MESHTENDER_TRUSTED_PROXIES — a comma-separated list of CIDRs + // or bare IPs, plus the shorthand token "private" (adds the RFC1918, link-local + // and ULA ranges — handy when a home router/LAN sits in front). + TrustedProxies []*net.IPNet } // RadioDefaults is a set of LoRa parameters, used to match a repeater's stored @@ -76,17 +87,18 @@ type RadioDefaults struct { // validating required fields. func Load() (*Config, error) { c := &Config{ - Addr: envOr("MESHTENDER_ADDR", ":8080"), - DatabaseURL: os.Getenv("MESHTENDER_DATABASE_URL"), - RPID: envOr("MESHTENDER_RP_ID", "localhost"), - RPDisplayName: envOr("MESHTENDER_RP_NAME", "MeshTender"), - RPOrigins: splitOrigins(envOr("MESHTENDER_RP_ORIGIN", "http://localhost:8080")), - PrimaryHost: envOr("MESHTENDER_PRIMARY_HOST", envOr("MESHTENDER_RP_ID", "localhost")), - AuthHost: os.Getenv("MESHTENDER_AUTH_HOST"), - RootHost: os.Getenv("MESHTENDER_ROOT_HOST"), - WWWHost: os.Getenv("MESHTENDER_WWW_HOST"), - TLSCert: os.Getenv("MESHTENDER_TLS_CERT"), - TLSKey: os.Getenv("MESHTENDER_TLS_KEY"), + Addr: envOr("MESHTENDER_ADDR", ":8080"), + DatabaseURL: os.Getenv("MESHTENDER_DATABASE_URL"), + RPID: envOr("MESHTENDER_RP_ID", "localhost"), + RPDisplayName: envOr("MESHTENDER_RP_NAME", "MeshTender"), + RPOrigins: splitOrigins(envOr("MESHTENDER_RP_ORIGIN", "http://localhost:8080")), + PrimaryHost: envOr("MESHTENDER_PRIMARY_HOST", envOr("MESHTENDER_RP_ID", "localhost")), + AuthHost: os.Getenv("MESHTENDER_AUTH_HOST"), + RootHost: os.Getenv("MESHTENDER_ROOT_HOST"), + WWWHost: os.Getenv("MESHTENDER_WWW_HOST"), + TLSCert: os.Getenv("MESHTENDER_TLS_CERT"), + TLSKey: os.Getenv("MESHTENDER_TLS_KEY"), + TrustedProxies: parseTrustedProxies(os.Getenv("MESHTENDER_TRUSTED_PROXIES")), } if c.RootHost != "" && c.WWWHost == "" { @@ -120,6 +132,55 @@ func Load() (*Config, error) { return c, nil } +// privateRanges are added by the "private" shorthand token. +var privateRanges = []string{ + "10.0.0.0/8", "172.16.0.0/12", "192.168.0.0/16", "169.254.0.0/16", + "fc00::/7", "fe80::/10", +} + +// parseTrustedProxies parses MESHTENDER_TRUSTED_PROXIES into CIDR ranges. Loopback +// is always included (a same-host reverse proxy). Each entry may be a CIDR, a bare +// IP (treated as a /32 or /128), or the token "private"/"loopback". Unparseable +// entries are skipped. +func parseTrustedProxies(s string) []*net.IPNet { + nets := []*net.IPNet{mustCIDR("127.0.0.0/8"), mustCIDR("::1/128")} + for _, tok := range strings.Split(s, ",") { + tok = strings.TrimSpace(tok) + if tok == "" { + continue + } + switch strings.ToLower(tok) { + case "loopback": + continue // already included + case "private": + for _, c := range privateRanges { + nets = append(nets, mustCIDR(c)) + } + continue + } + if strings.Contains(tok, "/") { + if _, n, err := net.ParseCIDR(tok); err == nil { + nets = append(nets, n) + } + continue + } + if ip := net.ParseIP(tok); ip != nil { + bits := 32 + if ip.To4() == nil { + bits = 128 + } + mask := net.CIDRMask(bits, bits) + nets = append(nets, &net.IPNet{IP: ip.Mask(mask), Mask: mask}) + } + } + return nets +} + +func mustCIDR(s string) *net.IPNet { + _, n, _ := net.ParseCIDR(s) + return n +} + func envOr(key, def string) string { if v := os.Getenv(key); v != "" { return v diff --git a/internal/core/proxytest.go b/internal/core/proxytest.go index 567fae0..3304bf9 100644 --- a/internal/core/proxytest.go +++ b/internal/core/proxytest.go @@ -74,9 +74,34 @@ func (s *Handlers) pageProxyTest(w http.ResponseWriter, r *http.Request) { scheme = "https" } + // Trusted-proxy diagnostics: which ranges are trusted, whether the peer is one, + // and how each X-Forwarded-For hop is classified (the resolved client is the + // rightmost untrusted hop). + trusted := s.Cfg.TrustedProxies + trustedList := make([]string, 0, len(trusted)) + for _, n := range trusted { + trustedList = append(trustedList, n.String()) + } + type xffHop struct { + IP string + Trusted bool + Selected bool + } + var chain []xffHop + for _, raw := range strings.Split(r.Header.Get("X-Forwarded-For"), ",") { + ip := strings.TrimSpace(raw) + if ip == "" { + continue + } + chain = append(chain, xffHop{IP: ip, Trusted: web.IsTrustedProxy(ip, trusted), Selected: ip == resolved}) + } + s.Render(w, r, "proxy_test.html", map[string]any{ - "ResolvedIP": resolved, - "RawPeer": rawPeer, + "ResolvedIP": resolved, + "RawPeer": rawPeer, + "PeerTrusted": web.IsTrustedProxy(rawHost, trusted), + "TrustedProxies": trustedList, + "XFFChain": chain, // HeaderApplied is true when a forwarding header changed the recorded IP // away from the real peer — i.e. the proxy's headers are being trusted. // When false, either this was a direct connection (fine) or the proxy diff --git a/internal/core/templates/proxy_test.html b/internal/core/templates/proxy_test.html index 48dbe10..936936e 100644 --- a/internal/core/templates/proxy_test.html +++ b/internal/core/templates/proxy_test.html @@ -48,6 +48,37 @@ {{end}} +
+

Trusted-proxy resolution

+
+
+
Connecting peer
+ {{.RawPeer}} + {{if .PeerTrusted}}trusted{{else}}not trusted{{end}} + {{if not .PeerTrusted}} +
The connecting peer isn't a trusted proxy, so forwarding headers are ignored and the peer is used as the client IP. If a proxy or router sits in front, add its address to the trusted list below.
+ {{end}} +
+
+
X-Forwarded-For chain (client = rightmost untrusted hop)
+ {{if .XFFChain}} +
+ {{range .XFFChain}} + {{.IP}}{{if .Trusted}} · trusted{{end}}{{if .Selected}} · client{{end}} + {{end}} +
+ {{else}}No X-Forwarded-For header on this request.{{end}} +
+
+
Trusted proxy ranges
+
+ {{range .TrustedProxies}}{{.}}{{end}} +
+ Set MESHTENDER_TRUSTED_PROXIES (comma-separated CIDRs/IPs, or the private shorthand) to trust additional hops. Loopback is always trusted. +
+
+
+

Forwarding headers

diff --git a/internal/web/clientip.go b/internal/web/clientip.go new file mode 100644 index 0000000..6756c16 --- /dev/null +++ b/internal/web/clientip.go @@ -0,0 +1,83 @@ +package web + +import ( + "net" + "net/http" + "strings" +) + +// resolveClientIP replaces chi's RealIP with trusted-proxy-aware resolution. It +// must run after CaptureRemoteAddr (which preserved the true TCP peer) and sets +// r.RemoteAddr to the resolved client IP so ClientIP and downstream code see it. +func (e *Env) resolveClientIP(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + r.RemoteAddr = ClientIPFrom( + RawRemoteAddr(r), + r.Header.Get("X-Forwarded-For"), + r.Header.Get("X-Real-IP"), + e.Cfg.TrustedProxies, + ) + next.ServeHTTP(w, r) + }) +} + +// ClientIPFrom resolves the real client IP from the connecting peer and the +// forwarding headers, honoring those headers only through trusted proxies: +// +// - If the peer is not a trusted proxy, the headers are ignored (they can be +// spoofed by a direct client) and the peer is returned. +// - Otherwise the X-Forwarded-For chain is walked right-to-left and the first +// entry that is not itself a trusted proxy is the client. (X-Real-IP is used +// as the chain when X-Forwarded-For is absent.) +// - If every hop is trusted (or there are no headers), the peer is returned. +func ClientIPFrom(peer, xff, xRealIP string, trusted []*net.IPNet) string { + peerHost := hostOnly(peer) + if !IsTrustedProxy(peerHost, trusted) { + return peerHost + } + chain := splitList(xff) + if len(chain) == 0 { + if x := strings.TrimSpace(xRealIP); x != "" { + chain = []string{x} + } + } + for i := len(chain) - 1; i >= 0; i-- { + if !IsTrustedProxy(chain[i], trusted) { + return chain[i] + } + } + return peerHost +} + +// IsTrustedProxy reports whether ip (a bare address, no port) falls within any of +// the trusted CIDR ranges. +func IsTrustedProxy(ip string, trusted []*net.IPNet) bool { + parsed := net.ParseIP(strings.TrimSpace(ip)) + if parsed == nil { + return false + } + for _, n := range trusted { + if n != nil && n.Contains(parsed) { + return true + } + } + return false +} + +func hostOnly(addr string) string { + if h, _, err := net.SplitHostPort(addr); err == nil { + return h + } + return addr +} + +// splitList splits a comma-separated header value, trimming blanks. +func splitList(s string) []string { + var out []string + for _, p := range strings.Split(s, ",") { + if p = strings.TrimSpace(p); p != "" { + out = append(out, p) + } + } + return out +} diff --git a/internal/web/env.go b/internal/web/env.go index 77081a8..3936ecf 100644 --- a/internal/web/env.go +++ b/internal/web/env.go @@ -198,8 +198,8 @@ func (rn *Renderer) Render(w http.ResponseWriter, r *http.Request, page string, // auth-free. chi requires all Use() calls before any route is registered. func (e *Env) CommonMiddleware(r chi.Router) { r.Use(middleware.RequestID) - r.Use(CaptureRemoteAddr) // keep the true TCP peer before RealIP rewrites it - r.Use(middleware.RealIP) + r.Use(CaptureRemoteAddr) // preserve the true TCP peer before we resolve + r.Use(e.resolveClientIP) // trusted-proxy-aware X-Forwarded-For resolution r.Use(middleware.Recoverer) } diff --git a/internal/web/proxyaddr_test.go b/internal/web/proxyaddr_test.go index 27baf29..4c35423 100644 --- a/internal/web/proxyaddr_test.go +++ b/internal/web/proxyaddr_test.go @@ -1,57 +1,84 @@ package web import ( + "net" "net/http" "net/http/httptest" "testing" - - "github.com/go-chi/chi/v5/middleware" ) -// TestCaptureRemoteAddrPreservesPeer verifies the ordering that the proxy-test -// page relies on: CaptureRemoteAddr (which must run before RealIP) keeps the true -// TCP peer, while RealIP rewrites RemoteAddr from X-Forwarded-For so ClientIP -// reflects the header-derived client. -func TestCaptureRemoteAddrPreservesPeer(t *testing.T) { - var gotClient, gotPeer string - final := http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) { - gotClient = ClientIP(r) - gotPeer = RawRemoteAddr(r) - }) - // Same order as CommonMiddleware: capture, then RealIP. - h := CaptureRemoteAddr(middleware.RealIP(final)) - - req := httptest.NewRequest(http.MethodGet, "/", nil) - req.RemoteAddr = "10.0.0.1:5555" // the proxy/load balancer - req.Header.Set("X-Forwarded-For", "203.0.113.9, 10.0.0.1") - h.ServeHTTP(httptest.NewRecorder(), req) - - if gotClient != "203.0.113.9" { - t.Errorf("ClientIP = %q, want the first X-Forwarded-For entry 203.0.113.9", gotClient) +func cidrs(t *testing.T, ss ...string) []*net.IPNet { + t.Helper() + out := make([]*net.IPNet, 0, len(ss)) + for _, s := range ss { + _, n, err := net.ParseCIDR(s) + if err != nil { + t.Fatalf("bad CIDR %q: %v", s, err) + } + out = append(out, n) } - if gotPeer != "10.0.0.1:5555" { - t.Errorf("RawRemoteAddr = %q, want the original peer 10.0.0.1:5555", gotPeer) + return out +} + +func TestClientIPFrom(t *testing.T) { + loopback := cidrs(t, "127.0.0.0/8") + withRouter := cidrs(t, "127.0.0.0/8", "192.168.3.1/32") + + cases := []struct { + name string + peer string + xff string + xRealIP string + trusted []*net.IPNet + want string + }{ + { + name: "untrusted peer ignores headers (anti-spoof)", + peer: "203.0.113.9:443", xff: "1.2.3.4", trusted: loopback, + want: "203.0.113.9", + }, + { + name: "local proxy + router in XFF, router NOT trusted -> logs router (the bug)", + peer: "127.0.0.1:5000", xff: "203.0.113.9, 192.168.3.1", trusted: loopback, + want: "192.168.3.1", + }, + { + name: "local proxy + router trusted -> recovers real client", + peer: "127.0.0.1:5000", xff: "203.0.113.9, 192.168.3.1", trusted: withRouter, + want: "203.0.113.9", + }, + { + name: "trusted peer, no XFF, X-Real-IP used", + peer: "127.0.0.1:5000", xRealIP: "203.0.113.9", trusted: loopback, + want: "203.0.113.9", + }, + { + name: "trusted peer, all hops trusted -> falls back to peer", + peer: "127.0.0.1:5000", xff: "192.168.3.1", trusted: withRouter, + want: "127.0.0.1", + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := ClientIPFrom(tc.peer, tc.xff, tc.xRealIP, tc.trusted); got != tc.want { + t.Errorf("ClientIPFrom(peer=%q, xff=%q, xRealIP=%q) = %q, want %q", + tc.peer, tc.xff, tc.xRealIP, got, tc.want) + } + }) } } -// TestCaptureRemoteAddrDirect: with no forwarding header, the recorded client IP -// is the peer itself (the "no trusted proxy" case the page warns about). -func TestCaptureRemoteAddrDirect(t *testing.T) { - var gotClient, gotPeer string - final := http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) { - gotClient = ClientIP(r) - gotPeer = RawRemoteAddr(r) - }) - h := CaptureRemoteAddr(middleware.RealIP(final)) - +// TestCaptureRemoteAddr verifies the middleware preserves the true TCP peer in +// the context so RawRemoteAddr can report it after resolution. +func TestCaptureRemoteAddr(t *testing.T) { + var got string + h := CaptureRemoteAddr(http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) { + got = RawRemoteAddr(r) + })) req := httptest.NewRequest(http.MethodGet, "/", nil) - req.RemoteAddr = "198.51.100.7:40000" + req.RemoteAddr = "10.0.0.1:5555" h.ServeHTTP(httptest.NewRecorder(), req) - - if gotClient != "198.51.100.7" { - t.Errorf("ClientIP = %q, want peer host 198.51.100.7", gotClient) - } - if gotPeer != "198.51.100.7:40000" { - t.Errorf("RawRemoteAddr = %q, want 198.51.100.7:40000", gotPeer) + if got != "10.0.0.1:5555" { + t.Errorf("RawRemoteAddr = %q, want 10.0.0.1:5555", got) } }