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)
}
}