Honour forwarded headers only from trusted proxies (closes #259)

Before, the server took the client's address from X-Forwarded-For or X-Real-IP whoever sent them, so a direct client could get past the login rate limit and choose the address and hostname recorded for its session.

A new ClientIP middleware, run before request logging, resolves the address once: forwarded headers count only when the peer is in TRUSTED_PROXIES, which defaults to the RFC 1918 ranges. A set list replaces the default, the empty string trusts nothing, and an invalid entry stops startup. The request log, login rate limiter and session creation all use the resolved address.

A proxy on the same host as a server outside Docker connects from 127.0.0.1 or ::1, which the default does not trust; the README says to list it.

Model: opus-5-5

internal/middleware/middleware_test.go

@@ -0,0 +1,189 @@
+package middleware_test
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"net/netip"
+	"testing"
+	"time"
+
+	"go4.org/netipx"
+	"sneak.berlin/go/neoirc/internal/config"
+	"sneak.berlin/go/neoirc/internal/globals"
+	"sneak.berlin/go/neoirc/internal/logger"
+	"sneak.berlin/go/neoirc/internal/middleware"
+)
+
+// The tests trust 10.0.0.0/8. Clients use the documentation ranges
+// 192.0.2.0/24, 198.51.100.0/24 and 203.0.113.0/24.
+const (
+	trustedNetwork = "10.0.0.0/8"
+	proxyPeer      = "10.0.0.1:4000"
+	directPeer     = "192.0.2.1:4000"
+	client         = "203.0.113.7"
+	clientWithPort = "203.0.113.7:4000"
+	realIPClient   = "203.0.113.8"
+)
+
+type clientIPTest struct {
+	name       string
+	remoteAddr string
+	forwarded  []string
+	realIP     string
+	want       string
+}
+
+func clientIPTests() []clientIPTest {
+	return []clientIPTest{
+		{
+			name:       "untrusted peer ignores forged headers",
+			remoteAddr: directPeer,
+			forwarded:  []string{client},
+			realIP:     realIPClient,
+			want:       directPeer,
+		},
+		{name: "trusted peer without headers", remoteAddr: proxyPeer, want: proxyPeer},
+		{
+			name:       "trusted peer gives rightmost untrusted entry",
+			remoteAddr: proxyPeer,
+			forwarded:  []string{"198.51.100.9, 203.0.113.7, 10.0.0.2"},
+			want:       clientWithPort,
+		},
+		{
+			name:       "entries split over several headers",
+			remoteAddr: proxyPeer,
+			forwarded:  []string{"198.51.100.9", client, "10.0.0.2"},
+			want:       clientWithPort,
+		},
+		{
+			name:       "all-trusted chain gives leftmost entry",
+			remoteAddr: proxyPeer,
+			forwarded:  []string{"10.0.0.3, 10.0.0.2"},
+			want:       "10.0.0.3:4000",
+		},
+		{
+			name:       "X-Real-IP without X-Forwarded-For",
+			remoteAddr: proxyPeer,
+			realIP:     realIPClient,
+			want:       "203.0.113.8:4000",
+		},
+		{
+			name:       "X-Forwarded-For wins over X-Real-IP",
+			remoteAddr: proxyPeer,
+			forwarded:  []string{client},
+			realIP:     realIPClient,
+			want:       clientWithPort,
+		},
+		{
+			name:       "unparseable X-Forwarded-For entry gives peer",
+			remoteAddr: proxyPeer,
+			forwarded:  []string{"203.0.113.7, not-an-ip, 10.0.0.2"},
+			want:       proxyPeer,
+		},
+		{
+			name:       "unparseable entry past the client is not read",
+			remoteAddr: proxyPeer,
+			forwarded:  []string{"not-an-ip, 203.0.113.7"},
+			want:       clientWithPort,
+		},
+		{
+			name:       "unparseable X-Real-IP gives peer",
+			remoteAddr: proxyPeer,
+			realIP:     "not-an-ip",
+			want:       proxyPeer,
+		},
+		{
+			name:       "mapped IPv6 peer is unmapped and trusted",
+			remoteAddr: "[::ffff:10.0.0.1]:4000",
+			forwarded:  []string{client},
+			want:       clientWithPort,
+		},
+		{
+			name:       "mapped IPv6 peer outside the set",
+			remoteAddr: "[::ffff:192.0.2.1]:4000",
+			forwarded:  []string{client},
+			want:       directPeer,
+		},
+		{
+			name:       "IPv6 client",
+			remoteAddr: proxyPeer,
+			forwarded:  []string{"2001:db8::1"},
+			want:       "[2001:db8::1]:4000",
+		},
+	}
+}
+
+func TestClientIP(t *testing.T) {
+	t.Parallel()
+
+	mware := newMiddleware(t, trustedNetwork)
+
+	for _, test := range clientIPTests() {
+		t.Run(test.name, func(t *testing.T) {
+			t.Parallel()
+
+			var got string
+
+			handler := mware.ClientIP()(http.HandlerFunc(func(
+				_ http.ResponseWriter, request *http.Request,
+			) {
+				got = request.RemoteAddr
+			}))
+
+			request := httptest.NewRequestWithContext(
+				t.Context(), http.MethodGet, "/", nil,
+			)
+			request.RemoteAddr = test.remoteAddr
+
+			for _, value := range test.forwarded {
+				request.Header.Add("X-Forwarded-For", value)
+			}
+
+			if test.realIP != "" {
+				request.Header.Set("X-Real-IP", test.realIP)
+			}
+
+			handler.ServeHTTP(httptest.NewRecorder(), request)
+
+			if got != test.want {
+				t.Errorf("RemoteAddr = %q, want %q", got, test.want)
+			}
+		})
+	}
+}
+
+// newMiddleware returns a Middleware that trusts the network trusted.
+func newMiddleware(t *testing.T, trusted string) *middleware.Middleware {
+	t.Helper()
+
+	var builder netipx.IPSetBuilder
+
+	builder.AddPrefix(netip.MustParsePrefix(trusted))
+
+	trustedSet, err := builder.IPSet()
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	globs := &globals.Globals{
+		Appname:   "neoirc-test",
+		Version:   "test",
+		StartTime: time.Now(),
+	}
+
+	log, err := logger.New(nil, logger.Params{Globals: globs})
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	mware, err := middleware.New(nil, middleware.Params{
+		Logger:  log,
+		Globals: globs,
+		Config:  &config.Config{TrustedProxies: trustedSet},
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	return mware
+}