Trust reverse proxies at private addresses by default (closes #198)

With TRUSTED_PROXIES unset, the server now trusts reverse proxies in the
private ranges of RFC 1918. A proxy such as traefik on a Docker network is
then believed. /ip/, the home page's curl card and the request log give the
client's address from X-Forwarded-For instead of the proxy's address.

A list in TRUSTED_PROXIES replaces the default. TRUSTED_PROXIES set but
empty trusts no proxy, so the config package reads it with os.LookupEnv.
A direct client at a public address still cannot choose its address with
a forged header.

Model: opus-5-5

README.md

@@ -469,19 +469,29 @@ Configuration is handled via environment variables and OS-specific paths:
 | `XDG_DATA_HOME`    | `/var/lib` (in the Docker image) | Base of the state directory; must be an absolute path       |
 | `GOMEMLIMIT`       | `2560MiB` (in the Docker image)  | Go soft memory limit; see Memory                            |
 | `MALLOC_ARENA_MAX` | `2` (in the Docker image)        | glibc malloc arena cap, a positive whole number; see Memory |
-| `TRUSTED_PROXIES`  | (empty)                          | Addresses and CIDR ranges of trusted proxies; see below     |
+| `TRUSTED_PROXIES`  | the RFC 1918 ranges              | Addresses and CIDR ranges of trusted proxies; see below     |
 
 A variable that is set to an invalid value stops the start with an error and a
-non-zero exit. An empty variable counts as unset.
+non-zero exit. An empty variable counts as unset. `TRUSTED_PROXIES` is the one
+exception: set but empty, it makes the server trust no proxy.
 
-`TRUSTED_PROXIES` lists the reverse proxies whose `X-Forwarded-For` the server
-believes, comma-separated, such as `10.0.0.0/8,192.0.2.1`. For a request from
-one of them, the request log and `/ip/` use the last address in
-`X-Forwarded-For` that is not a trusted proxy: each proxy adds the address it
-received the request from to the end, so the entries before it are whatever the
-client sent. For a request from any other address they use the address the
-request came from, whatever forwarding headers it carries. `X-Real-IP` and
-`True-Client-IP` are not read.
+`TRUSTED_PROXIES` names the reverse proxies whose `X-Forwarded-For` the server
+believes:
+
+- When it is not set, the server trusts the private ranges of RFC 1918:
+    - `10.0.0.0/8`
+    - `172.16.0.0/12`
+    - `192.168.0.0/16`
+- When it lists addresses and CIDR ranges, comma-separated, such as
+  `10.0.0.0/8,192.0.2.1`, the server trusts those instead.
+- When it is set but empty (`TRUSTED_PROXIES=`), the server trusts no proxy.
+
+For a request from a trusted proxy, the request log and `/ip/` use the last
+address in `X-Forwarded-For` that is not a trusted proxy. Each proxy adds the
+address it received the request from to the end, so the entries before it are
+whatever the client sent. For a request from any other address they use the
+address the request came from, whatever forwarding headers it carries.
+`X-Real-IP` and `True-Client-IP` are not read.
 
 State directory (the database and the blocklist copies):
 
@@ -575,12 +585,11 @@ What the [upaas](https://git.eeqj.de/sneak/upaas) app needs:
 
 - Container port: `8080`.
 - Volume: one, at container path `/var/lib/berlin.sneak.app.routewatch`.
-- Environment: `TRUSTED_PROXIES`, set to the address the reverse proxy in front
-  of the app connects from, as the container sees it; without it the request log
-  and `/ip/` give the proxy's address for every request. Nothing else is needed:
-  leave `XDG_DATA_HOME`, `GOMEMLIMIT` and `MALLOC_ARENA_MAX` at the image's
-  values. `DEBUG=routewatch` is optional and adds the `System stats` memory line
-  to the log.
+- Environment: nothing. The server trusts a reverse proxy at an RFC 1918 address
+  without `TRUSTED_PROXIES` (see Configuration). Set it only when the proxy
+  connects from any other address. Leave `XDG_DATA_HOME`, `GOMEMLIMIT` and
+  `MALLOC_ARENA_MAX` at the image's values. `DEBUG=routewatch` is optional and
+  adds the `System stats` memory line to the log.
 - Memory Limit: `5g`, the 5 GiB limit from Memory above. upaas sets no swap
   limit, so on a host with swap Docker allows the same amount of swap again.
 - Health check: the image's `HEALTHCHECK` requests

TODO.md

@@ -35,6 +35,11 @@ tested.
 
 # Completed Steps
 
+- 2026-10-27: with `TRUSTED_PROXIES` unset, the server trusts reverse proxies at
+  the private addresses of RFC 1918. `/ip/`, the home page's `curl` card and the
+  request log then give the client's address from `X-Forwarded-For`. They used
+  to give the proxy's address. A list in `TRUSTED_PROXIES` replaces the default,
+  and `TRUSTED_PROXIES` set but empty trusts no proxy (closes #198)
 - 2026-10-27: the home page has cards with figures of the routing table: the
   newest and oldest kept route, the newest and oldest updated AS, the AS with
   the most prefixes, the ASes with a single prefix, and fifteen well-known

internal/config/config.go

@@ -30,6 +30,14 @@ const (
 	maxPort = 65535
 )
 
+// The private address ranges of RFC 1918. The HTTP server trusts a reverse
+// proxy in any of them when TRUSTED_PROXIES is not set.
+const (
+	privateRange10  = "10.0.0.0/8"
+	privateRange172 = "172.16.0.0/12"
+	privateRange192 = "192.168.0.0/16"
+)
+
 var (
 	errRelativeXDGDataHome = errors.New("XDG_DATA_HOME must be an absolute path")
 	errUnsupportedOS       = errors.New("unsupported operating system")
@@ -159,11 +167,21 @@ func getPort() (int, error) {
 	return int(port), nil
 }
 
-// getTrustedProxies returns the addresses and CIDR ranges listed in
-// TRUSTED_PROXIES, such as "10.0.0.0/8,192.0.2.1", or none when it is not set.
-// An address is returned as a range holding only that address.
+// getTrustedProxies returns the reverse proxies the HTTP server trusts. When
+// TRUSTED_PROXIES is not set, they are the private ranges of RFC 1918. When it
+// is set, they are the addresses and CIDR ranges it lists, such as
+// "10.0.0.0/8,192.0.2.1", and none when it is empty. An address is returned as
+// a range holding only that address.
 func getTrustedProxies() ([]netip.Prefix, error) {
-	value := os.Getenv("TRUSTED_PROXIES")
+	value, set := os.LookupEnv("TRUSTED_PROXIES")
+	if !set {
+		return []netip.Prefix{
+			netip.MustParsePrefix(privateRange10),
+			netip.MustParsePrefix(privateRange172),
+			netip.MustParsePrefix(privateRange192),
+		}, nil
+	}
+
 	if value == "" {
 		return nil, nil
 	}

internal/config/config_internal_test.go

@@ -2,6 +2,7 @@ package config
 
 import (
 	"net/netip"
+	"os"
 	"runtime"
 	"slices"
 	"testing"
@@ -46,25 +47,47 @@ func TestNewRefusesInvalidPort(t *testing.T) {
 	}
 }
 
-func TestNewReadsTrustedProxies(t *testing.T) {
+// TestNewTrustsPrivateRangesByDefault checks that with TRUSTED_PROXIES unset
+// the trusted proxies are the private ranges of RFC 1918. Before the fix no
+// proxy was trusted, so a reverse proxy at a private address was taken for the
+// client (https://git.eeqj.de/sneak/routewatch/issues/198).
+func TestNewTrustsPrivateRangesByDefault(t *testing.T) {
 	t.Setenv("PORT", "")
 	t.Setenv("XDG_DATA_HOME", "")
 
+	// t.Setenv puts the variable back when the test ends.
 	t.Setenv("TRUSTED_PROXIES", "")
 
+	err := os.Unsetenv("TRUSTED_PROXIES")
+	if err != nil {
+		t.Fatal(err)
+	}
+
 	cfg, err := New()
 	if err != nil {
 		t.Fatalf("New() without TRUSTED_PROXIES: %v", err)
 	}
 
-	if len(cfg.TrustedProxies) != 0 {
-		t.Errorf("New() without TRUSTED_PROXIES: TrustedProxies = %v, want none",
-			cfg.TrustedProxies)
+	want := []netip.Prefix{
+		netip.MustParsePrefix("10.0.0.0/8"),
+		netip.MustParsePrefix("172.16.0.0/12"),
+		netip.MustParsePrefix("192.168.0.0/16"),
 	}
+	if !slices.Equal(cfg.TrustedProxies, want) {
+		t.Errorf("New() without TRUSTED_PROXIES: TrustedProxies = %v, want %v",
+			cfg.TrustedProxies, want)
+	}
+}
+
+// TestNewReadsTrustedProxies checks that a list in TRUSTED_PROXIES replaces
+// the default, and that TRUSTED_PROXIES set but empty trusts no proxy.
+func TestNewReadsTrustedProxies(t *testing.T) {
+	t.Setenv("PORT", "")
+	t.Setenv("XDG_DATA_HOME", "")
 
 	t.Setenv("TRUSTED_PROXIES", "10.0.0.0/8, 192.0.2.1,2001:db8::/32")
 
-	cfg, err = New()
+	cfg, err := New()
 	if err != nil {
 		t.Fatalf("New() with TRUSTED_PROXIES: %v", err)
 	}
@@ -77,6 +100,18 @@ func TestNewReadsTrustedProxies(t *testing.T) {
 	if !slices.Equal(cfg.TrustedProxies, want) {
 		t.Errorf("TrustedProxies = %v, want %v", cfg.TrustedProxies, want)
 	}
+
+	t.Setenv("TRUSTED_PROXIES", "")
+
+	cfg, err = New()
+	if err != nil {
+		t.Fatalf("New() with TRUSTED_PROXIES set but empty: %v", err)
+	}
+
+	if len(cfg.TrustedProxies) != 0 {
+		t.Errorf("New() with TRUSTED_PROXIES set but empty: TrustedProxies = %v, "+
+			"want none", cfg.TrustedProxies)
+	}
 }
 
 func TestNewRefusesInvalidTrustedProxies(t *testing.T) {

internal/server/middleware_internal_test.go

@@ -1,25 +1,38 @@
 package server
 
 import (
+	"bytes"
 	"encoding/json"
+	"io"
+	"log/slog"
+	"net"
 	"net/http"
 	"net/http/httptest"
 	"net/netip"
+	"os"
 	"strings"
 	"testing"
 	"time"
+
+	"git.eeqj.de/sneak/routewatch/internal/config"
+	"git.eeqj.de/sneak/routewatch/internal/database"
+	"git.eeqj.de/sneak/routewatch/internal/logger"
+	"git.eeqj.de/sneak/routewatch/internal/metrics"
+	"git.eeqj.de/sneak/routewatch/internal/streamer"
 )
 
 // The addresses the forwarding tests use: the client a proxy names in
 // X-Forwarded-For, the range of trusted proxies, a peer inside it, and a peer
 // outside it, which lies in testPrefixV4 so that an IP lookup of it finds a
-// route.
+// route. reverseProxyPeer is a reverse proxy at a private address, as Docker
+// gives one.
 const (
 	forwardedClient   = "203.0.113.9"
 	trustedProxyRange = "10.0.0.0/8"
 	trustedPeer       = "10.0.0.2:1234"
 	untrustedPeerAddr = "192.0.2.1"
 	untrustedPeer     = untrustedPeerAddr + ":1234"
+	reverseProxyPeer  = "172.19.0.14:1234"
 )
 
 // TestResponsesCarrySecurityHeaders checks that a page, a JSON route and a
@@ -183,6 +196,141 @@ func TestForwardedForFromUntrustedPeerIsIgnored(t *testing.T) {
 	}
 }
 
+// TestDefaultTrustedProxiesGiveTheForwardedClient builds the server from the
+// configuration with TRUSTED_PROXIES unset. A request from a reverse proxy at
+// a private address must be looked up by /ip/, shown on the home page's card
+// and logged as the client its X-Forwarded-For names. A direct request from a
+// public address with a forged X-Forwarded-For keeps its own address. Before
+// the fix no proxy was trusted unless TRUSTED_PROXIES named it, so /ip/ gave
+// the proxy's address (https://git.eeqj.de/sneak/routewatch/issues/198).
+func TestDefaultTrustedProxiesGiveTheForwardedClient(t *testing.T) {
+	t.Setenv("PORT", "")
+	t.Setenv("XDG_DATA_HOME", "")
+
+	// t.Setenv puts the variable back when the test ends.
+	t.Setenv("TRUSTED_PROXIES", "")
+
+	err := os.Unsetenv("TRUSTED_PROXIES")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	cfg, err := config.New()
+	if err != nil {
+		t.Fatalf("config.New: %v", err)
+	}
+
+	var requestLog bytes.Buffer
+
+	s := newServerWithConfig(t, cfg, &requestLog)
+
+	tests := []struct {
+		name string
+		peer string
+		want string
+	}{
+		{"through a reverse proxy", reverseProxyPeer, forwardedClient},
+		{"direct from a public address", untrustedPeer, untrustedPeerAddr},
+	}
+
+	for _, tt := range tests {
+		requestLog.Reset()
+
+		get := func(path string) *httptest.ResponseRecorder {
+			req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, path, nil)
+			req.RemoteAddr = tt.peer
+			req.Header.Set("X-Forwarded-For", forwardedClient)
+
+			rec := httptest.NewRecorder()
+			s.router.ServeHTTP(rec, req)
+
+			return rec
+		}
+
+		var answer struct {
+			Data IPLookupResponse `json:"data"`
+		}
+
+		rec := get("/ip/")
+
+		err := json.Unmarshal(rec.Body.Bytes(), &answer)
+		if err != nil {
+			t.Fatalf("%s: decoding the answer %q: %v", tt.name, rec.Body, err)
+		}
+
+		if answer.Data.Query != tt.want {
+			t.Errorf("%s: /ip/ looked up %q, want %q", tt.name, answer.Data.Query, tt.want)
+		}
+
+		page := get("/").Body.String()
+		if !strings.Contains(page, `data-client-ip="`+tt.want+`"`) {
+			t.Errorf("%s: the home page's card does not hold %s", tt.name, tt.want)
+		}
+
+		logged := loggedAddresses(requestLog.String())
+		if len(logged) == 0 {
+			t.Errorf("%s: the request log names no address", tt.name)
+		}
+
+		for _, addr := range logged {
+			if addr != tt.want {
+				t.Errorf("%s: the request log gives %s, want %s", tt.name, addr, tt.want)
+			}
+		}
+	}
+}
+
+// newServerWithConfig returns a server built with cfg, on a new database, that
+// reads the routes of newTestTable and logs its requests as JSON to
+// requestLog.
+func newServerWithConfig(
+	t *testing.T, cfg *config.Config, requestLog io.Writer,
+) *Server {
+	t.Helper()
+
+	cfg.StateDir = t.TempDir()
+
+	db, err := database.New(cfg, logger.New())
+	if err != nil {
+		t.Fatalf("database.New: %v", err)
+	}
+
+	t.Cleanup(func() { _ = db.Close() })
+
+	s := New(db, newTestTable(), streamer.New(logger.New(), metrics.New()),
+		&logger.Logger{Logger: slog.New(slog.NewJSONHandler(requestLog, nil))}, cfg)
+	s.resolver = &fakeResolver{}
+
+	return s
+}
+
+// loggedAddresses returns the address each line of the JSON request log
+// names, without the port a direct request is logged with.
+func loggedAddresses(requestLog string) []string {
+	var addrs []string
+
+	for line := range strings.Lines(requestLog) {
+		//nolint:tagliatelle // the field name is the key the request log uses
+		var entry struct {
+			RemoteAddr string `json:"remote_addr"`
+		}
+
+		err := json.Unmarshal([]byte(line), &entry)
+		if err != nil || entry.RemoteAddr == "" {
+			continue
+		}
+
+		addr, _, err := net.SplitHostPort(entry.RemoteAddr)
+		if err != nil {
+			addr = entry.RemoteAddr
+		}
+
+		addrs = append(addrs, addr)
+	}
+
+	return addrs
+}
+
 // TestRealIPMiddlewareTrustsOnlyConfiguredProxies checks which address
 // RealIPMiddleware gives a request: the TCP peer, unless the peer is a trusted
 // proxy; then the last X-Forwarded-For entry that is not a trusted proxy,