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

A new ClientIP middleware sets request.RemoteAddr to the client's
address before the request is logged. X-Forwarded-For and X-Real-IP
count only when the peer is in TRUSTED_PROXIES, which defaults to the
RFC 1918 ranges. The login rate limit, the session's IP address and
hostname, and the request log all use that address.

Model: opus-5-5

internal/config/config.go

@@ -6,12 +6,15 @@ import (
 	"fmt"
 	"log/slog"
 	"math"
+	"net/netip"
 	"os"
 	"strconv"
+	"strings"
 	"time"
 
 	"github.com/spf13/viper"
 	"go.uber.org/fx"
+	"go4.org/netipx"
 	"sneak.berlin/go/neoirc/internal/globals"
 	"sneak.berlin/go/neoirc/internal/logger"
 
@@ -32,6 +35,10 @@ Type /help for available commands.`
 const DefaultDBURL = "file:///var/lib/neoirc/state.db" +
 	"?_pragma=journal_mode(WAL)&_pragma=busy_timeout(5000)"
 
+// defaultTrustedProxies is TRUSTED_PROXIES when it is not set: the
+// private address ranges of RFC 1918.
+const defaultTrustedProxies = "10.0.0.0/8,172.16.0.0/12,192.168.0.0/16"
+
 var errOutOfRange = errors.New("setting out of range")
 
 // Params defines the dependencies for creating a Config.
@@ -64,13 +71,14 @@ type Config struct {
 	LoginRateLimit     float64
 	LoginRateBurst     int
 	IRCListenAddr      string
+	TrustedProxies     *netipx.IPSet
 	params             *Params
 	log                *slog.Logger
 }
 
 // New creates a new Config by reading from files and environment variables.
-// It returns an error when a number, true/false or duration setting does
-// not parse or is out of range.
+// It returns an error when a number, true/false, duration or address
+// setting does not parse or is out of range.
 func New(
 	_ fx.Lifecycle, params Params,
 ) (*Config, error) {
@@ -103,6 +111,7 @@ func New(
 	viper.SetDefault("LOGIN_RATE_LIMIT", "1")
 	viper.SetDefault("LOGIN_RATE_BURST", "5")
 	viper.SetDefault("IRC_LISTEN_ADDR", ":6667")
+	viper.SetDefault("TRUSTED_PROXIES", defaultTrustedProxies)
 
 	err := viper.ReadInConfig()
 	if err != nil {
@@ -151,10 +160,10 @@ func New(
 	return cfg, nil
 }
 
-// parseSettings reads the number and true/false settings into cfg and
-// checks the duration settings. Each number and duration must be above
-// zero. PORT and NEOIRC_HASHCASH_BITS may also be zero: port 0 lets the
-// system pick a port, and 0 bits turns proof of work off. The error
+// parseSettings reads the number, true/false and address settings into
+// cfg and checks the duration settings. Each number and duration must be
+// above zero. PORT and NEOIRC_HASHCASH_BITS may also be zero: port 0 lets
+// the system pick a port, and 0 bits turns proof of work off. The error
 // names each setting that does not parse or is out of range.
 func parseSettings(cfg *Config) error {
 	return errors.Join(
@@ -168,9 +177,69 @@ func parseSettings(cfg *Config) error {
 		checkDuration("MESSAGE_MAX_AGE", cfg.MessageMaxAge),
 		checkDuration("QUEUE_MAX_AGE", cfg.QueueMaxAge),
 		checkDuration("SESSION_IDLE_TIMEOUT", cfg.SessionIdleTimeout),
+		parseTrustedProxies(&cfg.TrustedProxies),
 	)
 }
 
+// parseTrustedProxies sets *value to the addresses in TRUSTED_PROXIES, a
+// comma-separated list of networks such as 10.0.0.0/8 and single
+// addresses such as 192.0.2.1. The empty string trusts nothing. Viper
+// counts an empty environment variable as unset, so the environment is
+// read directly as well.
+func parseTrustedProxies(value **netipx.IPSet) error {
+	raw := viper.GetString("TRUSTED_PROXIES")
+
+	fromEnv, set := os.LookupEnv("TRUSTED_PROXIES")
+	if set {
+		raw = fromEnv
+	}
+
+	var builder netipx.IPSetBuilder
+
+	if strings.TrimSpace(raw) != "" {
+		for entry := range strings.SplitSeq(raw, ",") {
+			err := addTrustedProxy(&builder, strings.TrimSpace(entry))
+			if err != nil {
+				return fmt.Errorf("cannot parse TRUSTED_PROXIES: %w", err)
+			}
+		}
+	}
+
+	ipSet, err := builder.IPSet()
+	if err != nil {
+		return fmt.Errorf("cannot parse TRUSTED_PROXIES: %w", err)
+	}
+
+	*value = ipSet
+
+	return nil
+}
+
+// addTrustedProxy adds entry, a network with a slash or a single
+// address, to builder. An IPv4 address written in IPv6 form, such as
+// ::ffff:192.0.2.1, is added as IPv4.
+func addTrustedProxy(builder *netipx.IPSetBuilder, entry string) error {
+	if strings.Contains(entry, "/") {
+		prefix, err := netip.ParsePrefix(entry)
+		if err != nil {
+			return err //nolint:wrapcheck // the caller names the setting
+		}
+
+		builder.AddPrefix(prefix)
+
+		return nil
+	}
+
+	addr, err := netip.ParseAddr(entry)
+	if err != nil {
+		return err //nolint:wrapcheck // the caller names the setting
+	}
+
+	builder.Add(addr.Unmap())
+
+	return nil
+}
+
 // parseInt sets *value to the setting key read as a whole number. It
 // returns an error when the value does not parse or is below minimum.
 func parseInt(key string, minimum int, value *int) error {

internal/config/config_test.go

@@ -1,6 +1,7 @@
 package config_test
 
 import (
+	"net/netip"
 	"os"
 	"strings"
 	"testing"
@@ -158,6 +159,9 @@ func TestBadSettingStopsStartup(t *testing.T) {
 		{key: "MESSAGE_MAX_AGE", values: []string{"30d", "0", "-1h"}},
 		{key: "QUEUE_MAX_AGE", values: []string{"720", "0s", "-24h"}},
 		{key: "SESSION_IDLE_TIMEOUT", values: []string{"a month", "0", "-1m"}},
+		{key: "TRUSTED_PROXIES", values: []string{
+			"proxy.example.com", "10.0.0.0/33", "192.0.2.1:8080",
+		}},
 	}
 
 	for _, test := range tests {
@@ -206,3 +210,77 @@ func TestIRCListenAddrEmptyTurnsListenerOff(t *testing.T) {
 		t.Errorf("IRCListenAddr = %q, want empty", cfg.IRCListenAddr)
 	}
 }
+
+func TestTrustedProxies(t *testing.T) {
+	// Each case lists addresses that must be in the trusted set and
+	// addresses that must not.
+	tests := []struct {
+		name      string
+		set       bool
+		value     string
+		trusted   []string
+		untrusted []string
+	}{
+		{
+			name:  "unset trusts the RFC 1918 ranges",
+			set:   false,
+			value: "",
+			trusted: []string{
+				"10.1.2.3", "172.16.0.1", "172.31.255.254", "192.168.1.1",
+			},
+			untrusted: []string{
+				"127.0.0.1", "172.32.0.1", "192.0.2.1", "2001:db8::1",
+			},
+		},
+		{
+			name:      "a value replaces the default",
+			set:       true,
+			value:     "198.51.100.0/24, 192.0.2.7,2001:db8::/32",
+			trusted:   []string{"198.51.100.200", "192.0.2.7", "2001:db8::5"},
+			untrusted: []string{"10.4.5.6", "192.168.4.4", "192.0.2.8"},
+		},
+		{
+			name:      "a mapped IPv6 address is trusted as IPv4",
+			set:       true,
+			value:     "::ffff:192.0.2.7",
+			trusted:   []string{"192.0.2.7"},
+			untrusted: []string{"10.7.8.9"},
+		},
+		{
+			name:      "the empty string trusts nothing",
+			set:       true,
+			value:     "",
+			trusted:   nil,
+			untrusted: []string{"10.0.0.1", "192.168.0.1", "::1"},
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			// t.Setenv puts back whatever value was there after the
+			// test, including when Unsetenv removes it.
+			t.Setenv("TRUSTED_PROXIES", test.value)
+
+			if !test.set {
+				err := os.Unsetenv("TRUSTED_PROXIES")
+				if err != nil {
+					t.Fatal(err)
+				}
+			}
+
+			proxies := newConfig(t).TrustedProxies
+
+			for _, addr := range test.trusted {
+				if !proxies.Contains(netip.MustParseAddr(addr)) {
+					t.Errorf("%s is not trusted", addr)
+				}
+			}
+
+			for _, addr := range test.untrusted {
+				if proxies.Contains(netip.MustParseAddr(addr)) {
+					t.Errorf("%s is trusted", addr)
+				}
+			}
+		})
+	}
+}