1 Commits
Author SHA1 Message Date
sneak c7ea02ea54 Honour forwarded headers only from trusted proxies (closes #259)
check / check (push) Failing after 7s
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
2026-11-10 21:13:33 +00:00
10 changed files with 670 additions and 46 deletions

No files matched your search

+61 -20
View File
@@ -1339,8 +1339,9 @@ Every session has a hostmask in IRC's form, `nick!username@hostname`:
from, or that IP address when the lookup gives no name. The lookup waits up to from, or that IP address when the lookup gives no name. The lookup waits up to
three seconds. three seconds.
Over the HTTP API, the server takes the client's IP address from the first entry Over the HTTP API, the client's IP address is the address of the TCP connection.
of the `X-Forwarded-For` header, then `X-Real-IP`, then the TCP connection (see When that connection comes from a trusted proxy, the server reads the address
from `X-Forwarded-For` or `X-Real-IP` instead (see
[Reverse proxy](#reverse-proxy)). [Reverse proxy](#reverse-proxy)).
Every user can see a hostmask: in `RPL_WHOISUSER` (`311`), `RPL_WHOREPLY` Every user can see a hostmask: in `RPL_WHOISUSER` (`311`), `RPL_WHOREPLY`
@@ -1450,10 +1451,9 @@ over the limit gets `429` `too many login attempts, try again later` with
attempt is more than 15 minutes old. Session creation has no rate limit; the attempt is more than 15 minutes old. Session creation has no rate limit; the
hashcash stamp is its cost. hashcash stamp is its cost.
The IP address comes from the first entry of `X-Forwarded-For`, else from The IP address is the client's address as described in
`X-Real-IP`, else from the TCP connection. Without a reverse proxy that [Reverse proxy](#reverse-proxy). A client outside the trusted proxies cannot
overwrites those headers, a client can set them itself and so pick any address change it with `X-Forwarded-For` or `X-Real-IP`.
(see [Reverse proxy](#reverse-proxy)).
### Writing a client ### Writing a client
@@ -1714,10 +1714,14 @@ Every setting is an environment variable. The server also reads a `.env` file in
its working directory, and a YAML file `neoirc.yaml` in `/etc/neoirc/` or its working directory, and a YAML file `neoirc.yaml` in `/etc/neoirc/` or
`$HOME/.config/neoirc/` with the same names as keys. A variable from the `$HOME/.config/neoirc/` with the same names as keys. A variable from the
environment wins over one in `.env`, and both win over `neoirc.yaml`. An environment wins over one in `.env`, and both win over `neoirc.yaml`. An
environment variable set to the empty string counts as unset, except environment variable set to the empty string counts as unset, with two
`IRC_LISTEN_ADDR`, where the empty string turns the IRC listener off. A exceptions:
`neoirc.yaml` that is not valid YAML stops the server at startup. A setting that
does not parse or is out of range also stops the server at startup, and the - For `IRC_LISTEN_ADDR`, the empty string turns the IRC listener off.
- For `TRUSTED_PROXIES`, the empty string trusts no proxy.
A `neoirc.yaml` that is not valid YAML stops the server at startup. A setting
that does not parse or is out of range also stops the server at startup, and the
error names the setting. error names the setting.
- `PORT` (default `8080`): the HTTP port. `0` lets the system pick a free port. - `PORT` (default `8080`): the HTTP port. `0` lets the system pick a free port.
@@ -1747,6 +1751,11 @@ error names the setting.
address. A fraction such as `0.5` is allowed. It must be above `0`. address. A fraction such as `0.5` is allowed. It must be above `0`.
- `LOGIN_RATE_BURST` (default `5`): how many attempts an address may make at - `LOGIN_RATE_BURST` (default `5`): how many attempts an address may make at
once before the rate applies. It must be at least `1`. once before the rate applies. It must be at least `1`.
- `TRUSTED_PROXIES` (default `10.0.0.0/8,172.16.0.0/12,192.168.0.0/16`): the
reverse proxies whose `X-Forwarded-For` and `X-Real-IP` headers the server
believes. It is a comma-separated list of networks, such as `10.0.0.0/8`, and
single addresses, such as `192.0.2.1`. A list replaces the default entirely.
The empty string trusts nothing. See [Reverse proxy](#reverse-proxy).
- `SESSION_IDLE_TIMEOUT` (default `720h`, 30 days): a duration with a unit, `h` - `SESSION_IDLE_TIMEOUT` (default `720h`, 30 days): a duration with a unit, `h`
for hours, `m` for minutes or `s` for seconds, such as `720h` or `36h30m`. A for hours, `m` for minutes or `s` for seconds, such as `720h` or `36h30m`. A
client with no request for this long is removed, and a session left with no client with no request for this long is removed, and a session left with no
@@ -1804,14 +1813,34 @@ Run the server behind a reverse proxy that terminates TLS. The server itself
speaks plain HTTP, and the auth cookie is `Secure`, so browsers send it only speaks plain HTTP, and the auth cookie is `Secure`, so browsers send it only
over HTTPS. The proxy must: over HTTPS. The proxy must:
- Set `X-Forwarded-For` to the client's address, replacing any value the client - Connect from an address in `TRUSTED_PROXIES`. The default trusts the private
sent. The server reads that header first and trusts it from anyone; it reads ranges `10.0.0.0/8`, `172.16.0.0/12` and `192.168.0.0/16`. The setting must
`X-Real-IP` only when `X-Forwarded-For` is absent. Without a proxy that hold the address the server sees the proxy connect from. For a proxy on the
overwrites the header, a client can choose its own address, which sets its same host:
hostname and lets it dodge the login rate limit. - With the server outside Docker, that address is `127.0.0.1` or `::1`. The
default does not trust them, so set `TRUSTED_PROXIES=127.0.0.1,::1`.
- Through a published Docker port, that address is the Docker bridge
gateway, such as `172.17.0.1`. The default trusts it.
- Add the client's address to `X-Forwarded-For`.
- Wait at least 60 seconds for a response. A long poll can take 30 seconds, and - Wait at least 60 seconds for a response. A long poll can take 30 seconds, and
the server's own write timeout is 60 seconds. the server's own write timeout is 60 seconds.
The server finds the client's address in these steps:
- A connection from outside `TRUSTED_PROXIES` is the client. The server ignores
its `X-Forwarded-For` and `X-Real-IP` headers.
- For a connection from a trusted proxy, the server reads `X-Forwarded-For` from
right to left. It skips trusted addresses and takes the first untrusted one.
When every entry is trusted, it takes the leftmost.
- Without `X-Forwarded-For`, the server takes `X-Real-IP`.
- When an entry it reads is not an IP address, the server uses the proxy's
address.
The client's address sets the session's hostname and counts toward the login
rate limit. Any connection from a trusted address can choose its client address
through `X-Forwarded-For`. If clients connect directly from private addresses,
set `TRUSTED_PROXIES` to the proxy's address alone.
Caddy: Caddy:
``` ```
@@ -1861,9 +1890,21 @@ The IRC listener has no TLS.
The server logs with Go's `slog`, one line per event, to standard output. The The server logs with Go's `slog`, one line per event, to standard output. The
lines are JSON, or plain text when standard output is a terminal. Every HTTP lines are JSON, or plain text when standard output is a terminal. Every HTTP
request is logged with its method, URL, protocol, status, latency, remote request is logged with:
address, user agent, referrer and request ID. `DEBUG=true` adds debug-level
lines. - start time
- method
- URL
- protocol
- status
- latency
- client address
- user agent
- referrer
- request ID
The client address is the one found as described in
[Reverse proxy](#reverse-proxy). `DEBUG=true` adds debug-level lines.
### The database ### The database
@@ -1946,8 +1987,8 @@ and their sessions to end. Then it closes the database.
gets a message. gets a message.
- `internal/hashcash/`: stamp checking and minting. - `internal/hashcash/`: stamp checking and minting.
- `internal/ratelimit/`: the per-address login rate limiter. - `internal/ratelimit/`: the per-address login rate limiter.
- `internal/middleware/`: request logging, cross-origin requests, the - `internal/middleware/`: finding the client's address, request logging,
`Content-Security-Policy` header and metrics. cross-origin requests, the `Content-Security-Policy` header and metrics.
- `internal/healthcheck/` and `internal/stats/`: the health check and its - `internal/healthcheck/` and `internal/stats/`: the health check and its
counters. counters.
- `internal/logger/` and `internal/globals/`: logging setup and build - `internal/logger/` and `internal/globals/`: logging setup and build
+1
View File
@@ -15,6 +15,7 @@ require (
github.com/slok/go-http-metrics v0.13.0 github.com/slok/go-http-metrics v0.13.0
github.com/spf13/viper v1.21.0 github.com/spf13/viper v1.21.0
go.uber.org/fx v1.24.0 go.uber.org/fx v1.24.0
go4.org/netipx v0.0.0-20260823151212-3075585bcbeb
golang.org/x/crypto v0.48.0 golang.org/x/crypto v0.48.0
golang.org/x/time v0.6.0 golang.org/x/time v0.6.0
modernc.org/sqlite v1.45.0 modernc.org/sqlite v1.45.0
+2
View File
@@ -111,6 +111,8 @@ go.yaml.in/yaml/v2 v2.4.2 h1:DzmwEr2rDGHl7lsFgAHxmNz/1NlQ7xLIrlN2h5d1eGI=
go.yaml.in/yaml/v2 v2.4.2/go.mod h1:081UH+NErpNdqlCXm3TtEran0rJZGxAYx9hb/ELlsPU= go.yaml.in/yaml/v2 v2.4.2/go.mod h1:081UH+NErpNdqlCXm3TtEran0rJZGxAYx9hb/ELlsPU=
go.yaml.in/yaml/v3 v3.0.4 h1:tfq32ie2Jv2UxXFdLJdh3jXuOzWiL1fo0bu/FbuKpbc= go.yaml.in/yaml/v3 v3.0.4 h1:tfq32ie2Jv2UxXFdLJdh3jXuOzWiL1fo0bu/FbuKpbc=
go.yaml.in/yaml/v3 v3.0.4/go.mod h1:DhzuOOF2ATzADvBadXxruRBLzYTpT36CKvDb3+aBEFg= go.yaml.in/yaml/v3 v3.0.4/go.mod h1:DhzuOOF2ATzADvBadXxruRBLzYTpT36CKvDb3+aBEFg=
go4.org/netipx v0.0.0-20260823151212-3075585bcbeb h1:XBM4hvfwGAttkkiTIFfeigdfcL1xIfdKXqFdgiHGtDs=
go4.org/netipx v0.0.0-20260823151212-3075585bcbeb/go.mod h1:PLyyIXexvUFg3Owu6p/WfdlivPbZJsZdgWZlrGope/Y=
golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w=
golang.org/x/crypto v0.0.0-20210921155107-089bfa567519/go.mod h1:GvvjBRRGRdwPK5ydBHafDWAxML/pGHZbMvKqRZ5+Abc= golang.org/x/crypto v0.0.0-20210921155107-089bfa567519/go.mod h1:GvvjBRRGRdwPK5ydBHafDWAxML/pGHZbMvKqRZ5+Abc=
golang.org/x/crypto v0.48.0 h1:/VRzVqiRSggnhY7gNRxPauEQ5Drw9haKdM0jqfcCFts= golang.org/x/crypto v0.48.0 h1:/VRzVqiRSggnhY7gNRxPauEQ5Drw9haKdM0jqfcCFts=
+99 -6
View File
@@ -6,12 +6,15 @@ import (
"fmt" "fmt"
"log/slog" "log/slog"
"math" "math"
"net/netip"
"os" "os"
"strconv" "strconv"
"strings"
"time" "time"
"github.com/spf13/viper" "github.com/spf13/viper"
"go.uber.org/fx" "go.uber.org/fx"
"go4.org/netipx"
"sneak.berlin/go/neoirc/internal/globals" "sneak.berlin/go/neoirc/internal/globals"
"sneak.berlin/go/neoirc/internal/logger" "sneak.berlin/go/neoirc/internal/logger"
@@ -32,8 +35,16 @@ Type /help for available commands.`
const DefaultDBURL = "file:///var/lib/neoirc/state.db" + const DefaultDBURL = "file:///var/lib/neoirc/state.db" +
"?_pragma=journal_mode(WAL)&_pragma=busy_timeout(5000)" "?_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") var errOutOfRange = errors.New("setting out of range")
var errNeverMatches = errors.New("entry can never match a connection")
var errNotString = errors.New("not a string")
// Params defines the dependencies for creating a Config. // Params defines the dependencies for creating a Config.
type Params struct { type Params struct {
fx.In fx.In
@@ -64,13 +75,14 @@ type Config struct {
LoginRateLimit float64 LoginRateLimit float64
LoginRateBurst int LoginRateBurst int
IRCListenAddr string IRCListenAddr string
TrustedProxies *netipx.IPSet
params *Params params *Params
log *slog.Logger log *slog.Logger
} }
// New creates a new Config by reading from files and environment variables. // 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 // It returns an error when a number, true/false, duration or address
// not parse or is out of range. // setting does not parse or is out of range.
func New( func New(
_ fx.Lifecycle, params Params, _ fx.Lifecycle, params Params,
) (*Config, error) { ) (*Config, error) {
@@ -103,6 +115,7 @@ func New(
viper.SetDefault("LOGIN_RATE_LIMIT", "1") viper.SetDefault("LOGIN_RATE_LIMIT", "1")
viper.SetDefault("LOGIN_RATE_BURST", "5") viper.SetDefault("LOGIN_RATE_BURST", "5")
viper.SetDefault("IRC_LISTEN_ADDR", ":6667") viper.SetDefault("IRC_LISTEN_ADDR", ":6667")
viper.SetDefault("TRUSTED_PROXIES", defaultTrustedProxies)
err := viper.ReadInConfig() err := viper.ReadInConfig()
if err != nil { if err != nil {
@@ -151,10 +164,10 @@ func New(
return cfg, nil return cfg, nil
} }
// parseSettings reads the number and true/false settings into cfg and // parseSettings reads the number, true/false and address settings into
// checks the duration settings. Each number and duration must be above // cfg and checks the duration settings. Each number and duration must be
// zero. PORT and NEOIRC_HASHCASH_BITS may also be zero: port 0 lets the // above zero. PORT and NEOIRC_HASHCASH_BITS may also be zero: port 0 lets
// system pick a port, and 0 bits turns proof of work off. The error // 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. // names each setting that does not parse or is out of range.
func parseSettings(cfg *Config) error { func parseSettings(cfg *Config) error {
return errors.Join( return errors.Join(
@@ -168,9 +181,89 @@ func parseSettings(cfg *Config) error {
checkDuration("MESSAGE_MAX_AGE", cfg.MessageMaxAge), checkDuration("MESSAGE_MAX_AGE", cfg.MessageMaxAge),
checkDuration("QUEUE_MAX_AGE", cfg.QueueMaxAge), checkDuration("QUEUE_MAX_AGE", cfg.QueueMaxAge),
checkDuration("SESSION_IDLE_TIMEOUT", cfg.SessionIdleTimeout), 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 {
setting := viper.Get("TRUSTED_PROXIES")
fromEnv, set := os.LookupEnv("TRUSTED_PROXIES")
if set {
setting = fromEnv
}
// A YAML list or map in the config file is not a string, and
// viper.GetString would turn it into the empty string.
raw, isString := setting.(string)
if !isString {
return fmt.Errorf("cannot parse TRUSTED_PROXIES: %v is %w",
setting, errNotString)
}
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. A network in that form and an
// address with a zone are errors, since no connection could match them.
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
}
// A connection from an IPv4 address is checked in IPv4 form.
if prefix.Addr().Is4In6() {
return fmt.Errorf("%w: %s is an IPv4 network in IPv6 form",
errNeverMatches, entry)
}
builder.AddPrefix(prefix)
return nil
}
addr, err := netip.ParseAddr(entry)
if err != nil {
return err //nolint:wrapcheck // the caller names the setting
}
// IPSet.Contains is false for every address with a zone.
if addr.Zone() != "" {
return fmt.Errorf("%w: %s has a zone", errNeverMatches, entry)
}
builder.Add(addr.Unmap())
return nil
}
// parseInt sets *value to the setting key read as a whole number. It // 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. // returns an error when the value does not parse or is below minimum.
func parseInt(key string, minimum int, value *int) error { func parseInt(key string, minimum int, value *int) error {
+129 -2
View File
@@ -1,11 +1,14 @@
package config_test package config_test
import ( import (
"net/netip"
"os" "os"
"path/filepath"
"strings" "strings"
"testing" "testing"
"time" "time"
"github.com/spf13/viper"
"sneak.berlin/go/neoirc/internal/config" "sneak.berlin/go/neoirc/internal/config"
"sneak.berlin/go/neoirc/internal/globals" "sneak.berlin/go/neoirc/internal/globals"
"sneak.berlin/go/neoirc/internal/logger" "sneak.berlin/go/neoirc/internal/logger"
@@ -140,8 +143,7 @@ func settingsOf(cfg *config.Config) settings {
} }
func TestBadSettingStopsStartup(t *testing.T) { func TestBadSettingStopsStartup(t *testing.T) {
// For each setting, the first value does not parse and the others // Each value is invalid for its setting.
// parse but are out of range.
tests := []struct { tests := []struct {
key string key string
values []string values []string
@@ -158,6 +160,10 @@ func TestBadSettingStopsStartup(t *testing.T) {
{key: "MESSAGE_MAX_AGE", values: []string{"30d", "0", "-1h"}}, {key: "MESSAGE_MAX_AGE", values: []string{"30d", "0", "-1h"}},
{key: "QUEUE_MAX_AGE", values: []string{"720", "0s", "-24h"}}, {key: "QUEUE_MAX_AGE", values: []string{"720", "0s", "-24h"}},
{key: "SESSION_IDLE_TIMEOUT", values: []string{"a month", "0", "-1m"}}, {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",
"::ffff:10.0.0.0/104", "fe80::1%eth0",
}},
} }
for _, test := range tests { for _, test := range tests {
@@ -206,3 +212,124 @@ func TestIRCListenAddrEmptyTurnsListenerOff(t *testing.T) {
t.Errorf("IRCListenAddr = %q, want empty", cfg.IRCListenAddr) 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)
}
}
})
}
}
func TestTrustedProxiesNotAStringInYAMLStopsStartup(t *testing.T) {
// YAML reads the first value as a list and the second as a map.
values := []string{"[10.0.0.1]", "{proxy: 10.0.0.1}"}
for _, value := range values {
t.Run(value, func(t *testing.T) {
// t.Setenv puts back whatever value was there after the
// test, including when Unsetenv removes it.
t.Setenv("TRUSTED_PROXIES", "")
err := os.Unsetenv("TRUSTED_PROXIES")
if err != nil {
t.Fatal(err)
}
// loadConfig names the app neoirc-test, so config.New reads
// $HOME/.config/neoirc-test/neoirc-test.yaml.
home := t.TempDir()
t.Setenv("HOME", home)
dir := filepath.Join(home, ".config", "neoirc-test")
err = os.MkdirAll(dir, 0o700)
if err != nil {
t.Fatal(err)
}
err = os.WriteFile(
filepath.Join(dir, "neoirc-test.yaml"),
[]byte("TRUSTED_PROXIES: "+value+"\n"), 0o600,
)
if err != nil {
t.Fatal(err)
}
// Viper keeps the values it read from this file after the
// file is gone. Reset clears them for later tests.
t.Cleanup(viper.Reset)
_, err = loadConfig(t)
if err == nil || !strings.Contains(err.Error(), "TRUSTED_PROXIES") {
t.Errorf("error = %v, want one naming TRUSTED_PROXIES", err)
}
})
}
}
+2 -18
View File
@@ -42,25 +42,9 @@ func (hdlr *Handlers) maxBodySize() int64 {
return defaultMaxBodySize return defaultMaxBodySize
} }
// clientIP extracts the connecting client's IP address // clientIP returns the IP address in request.RemoteAddr, which the
// from the request, checking X-Forwarded-For and // ClientIP middleware has set to the client's address.
// X-Real-IP headers before falling back to RemoteAddr.
func clientIP(request *http.Request) string { func clientIP(request *http.Request) string {
if forwarded := request.Header.Get("X-Forwarded-For"); forwarded != "" {
// X-Forwarded-For can contain a comma-separated list;
// the first entry is the original client.
parts := strings.SplitN(forwarded, ",", 2) //nolint:mnd
ip := strings.TrimSpace(parts[0])
if ip != "" {
return ip
}
}
if realIP := request.Header.Get("X-Real-IP"); realIP != "" {
return strings.TrimSpace(realIP)
}
host, _, err := net.SplitHostPort(request.RemoteAddr) host, _, err := net.SplitHostPort(request.RemoteAddr)
if err != nil { if err != nil {
return request.RemoteAddr return request.RemoteAddr
+111
View File
@@ -4281,6 +4281,117 @@ func TestLoginRateLimitExceeded(t *testing.T) {
} }
} }
func TestLoginRateLimitIgnoresForwardedHeaders(t *testing.T) {
t.Parallel()
// The test client connects from 127.0.0.1, which is outside the
// default trusted proxies, so the server must ignore these headers.
tests := []struct {
name string
header string
}{
{name: "X-Forwarded-For", header: "X-Forwarded-For"},
{name: "X-Real-IP", header: "X-Real-IP"},
}
loginBody, err := json.Marshal(map[string]string{
nickKey: "unknownnick", passwordKey: "wrongpassword",
})
if err != nil {
t.Fatal(err)
}
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
t.Parallel()
tserver := newTestServer(t)
// The default burst is 5 attempts, so the sixth gets 429
// although each claims a new address.
var status int
for attempt := range 6 {
request, rErr := http.NewRequestWithContext(
t.Context(), http.MethodPost,
tserver.url("/api/v1/login"),
bytes.NewReader(loginBody),
)
if rErr != nil {
t.Fatal(rErr)
}
request.Header.Set("Content-Type", "application/json")
request.Header.Set(
test.header, fmt.Sprintf("203.0.113.%d", attempt+1),
)
resp, dErr := http.DefaultClient.Do(request)
if dErr != nil {
t.Fatal(dErr)
}
status = resp.StatusCode
_ = resp.Body.Close()
}
if status != http.StatusTooManyRequests {
t.Fatalf("sixth attempt got %d, want 429", status)
}
})
}
}
func TestLoginRateLimitPerClientBehindTrustedProxy(t *testing.T) {
// The test client connects from 127.0.0.1. Trusted as a proxy, it
// passes each client's address in X-Forwarded-For. t.Setenv rules
// out t.Parallel.
t.Setenv("TRUSTED_PROXIES", "127.0.0.1")
tserver := newTestServer(t)
// An empty JSON object is refused after the rate limit counts the
// attempt, so no attempt checks a password or looks up a hostname.
loginFrom := func(forwardedFor string) int {
t.Helper()
request, err := http.NewRequestWithContext(
t.Context(), http.MethodPost,
tserver.url("/api/v1/login"), strings.NewReader("{}"),
)
if err != nil {
t.Fatal(err)
}
request.Header.Set("Content-Type", "application/json")
request.Header.Set("X-Forwarded-For", forwardedFor)
resp, err := http.DefaultClient.Do(request)
if err != nil {
t.Fatal(err)
}
_ = resp.Body.Close()
return resp.StatusCode
}
// The default burst is 5 attempts per address.
for range 5 {
loginFrom("203.0.113.1")
}
status := loginFrom("203.0.113.1")
if status != http.StatusTooManyRequests {
t.Errorf("sixth attempt from 203.0.113.1 got %d, want 429", status)
}
status = loginFrom("203.0.113.2")
if status == http.StatusTooManyRequests {
t.Error("first attempt from 203.0.113.2 got 429")
}
}
func TestLoginRateLimitAllowsNormalUse(t *testing.T) { func TestLoginRateLimitAllowsNormalUse(t *testing.T) {
t.Parallel() t.Parallel()
+75
View File
@@ -5,6 +5,9 @@ import (
"log/slog" "log/slog"
"net" "net"
"net/http" "net/http"
"net/netip"
"slices"
"strings"
"time" "time"
basicauth "github.com/99designs/basicauth-go" basicauth "github.com/99designs/basicauth-go"
@@ -15,6 +18,7 @@ import (
"github.com/slok/go-http-metrics/middleware/std" "github.com/slok/go-http-metrics/middleware/std"
"github.com/spf13/viper" "github.com/spf13/viper"
"go.uber.org/fx" "go.uber.org/fx"
"go4.org/netipx"
"sneak.berlin/go/neoirc/internal/config" "sneak.berlin/go/neoirc/internal/config"
"sneak.berlin/go/neoirc/internal/globals" "sneak.berlin/go/neoirc/internal/globals"
"sneak.berlin/go/neoirc/internal/logger" "sneak.berlin/go/neoirc/internal/logger"
@@ -62,6 +66,77 @@ func ipFromHostPort(hostPort string) string {
return host return host
} }
// ClientIP returns middleware that replaces request.RemoteAddr with the
// client's address and keeps the port of the TCP connection. The
// X-Forwarded-For and X-Real-IP headers count only when the connection
// comes from an address in TRUSTED_PROXIES.
func (mware *Middleware) ClientIP() func(http.Handler) http.Handler {
trusted := mware.params.Config.TrustedProxies
return func(next http.Handler) http.Handler {
return http.HandlerFunc(
func(
writer http.ResponseWriter,
request *http.Request,
) {
peer, err := netip.ParseAddrPort(request.RemoteAddr)
if err == nil {
client := clientAddr(
peer.Addr().Unmap(), request.Header, trusted,
)
request.RemoteAddr = netip.AddrPortFrom(
client, peer.Port(),
).String()
}
next.ServeHTTP(writer, request)
})
}
}
// clientAddr returns the client's address for a request from peer. A
// peer outside trusted is the client, whatever its headers say. A
// trusted peer is a proxy. Its X-Forwarded-For list is read from the
// right, past the trusted addresses, to the first untrusted one; when
// all are trusted, the client is the leftmost. Without X-Forwarded-For,
// the client is X-Real-IP. An entry that is not an IP address gives
// peer.
func clientAddr(
peer netip.Addr, header http.Header, trusted *netipx.IPSet,
) netip.Addr {
if !trusted.Contains(peer) {
return peer
}
forwarded := strings.Join(header.Values("X-Forwarded-For"), ",")
if forwarded == "" {
realIP, err := netip.ParseAddr(
strings.TrimSpace(header.Get("X-Real-IP")),
)
if err != nil {
return peer
}
return realIP.Unmap()
}
client := peer
for _, entry := range slices.Backward(strings.Split(forwarded, ",")) {
addr, err := netip.ParseAddr(strings.TrimSpace(entry))
if err != nil {
return peer
}
client = addr.Unmap()
if !trusted.Contains(client) {
break
}
}
return client
}
type loggingResponseWriter struct { type loggingResponseWriter struct {
http.ResponseWriter http.ResponseWriter
+189
View File
@@ -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
}
+1
View File
@@ -23,6 +23,7 @@ func (srv *Server) SetupRoutes() {
srv.router.Use(middleware.Recoverer) srv.router.Use(middleware.Recoverer)
srv.router.Use(middleware.RequestID) srv.router.Use(middleware.RequestID)
srv.router.Use(srv.mw.ClientIP())
srv.router.Use(srv.mw.Logging()) srv.router.Use(srv.mw.Logging())
if viper.GetString("METRICS_USERNAME") != "" { if viper.GetString("METRICS_USERNAME") != "" {