Author SHA1 Message Date
sneak 518e84ce67 docs: document trusted_proxies and record in TODO.md
check / check (push) Failing after 1s
Describe the trusted_proxies key in README.md and config.example.yml,
including the rightmost-non-proxy selection rule and the safe default of
trusting no one. Record the unit in TODO.md.

Model: opus-4-8
2026-09-21 23:23:56 +00:00
sneak 20a18c7345 feat: use resolved client IP in request and auth logs
Add a ClientIP middleware that resolves the client address once per
request and stores it in the context, placed ahead of logging in the
route chain. The request-logging middleware and both login-attempt logs
now read the resolved address instead of the raw peer. Behind a trusted
proxy these records show the real client; a direct, untrusted client
still shows its own address and cannot forge one. Removes the local
host:port helper now that the clientip package owns that parsing.

Model: opus-4-8
2026-09-21 23:23:56 +00:00
sneak a280fd2c34 test: cover resolved client IP in request and auth logs
Assert the ClientIP middleware stores the resolved address in the
request context for trusted and untrusted peers, that the request log
records the forwarded client rather than the proxy peer, and that a
failed-login record carries the resolved address.

Model: opus-4-8
2026-09-21 23:23:55 +00:00
sneak 4f14cd86c3 feat: add trusted-proxy-aware client IP resolver
New internal/clientip package. Resolve honors X-Forwarded-For only when
the direct peer falls inside a configured trusted-proxy range; otherwise
it returns the peer address and ignores the header, so a client
connecting directly cannot spoof its address. When the peer is trusted,
the chain is walked right to left and the first entry that is not itself
a trusted proxy is returned. Also provides request-context helpers so the
resolved address can be shared across middleware and handlers.

Tests cover the honored, ignored, and spoofed-chain cases, rightmost
selection across a mixed chain, multi-line headers, IPv6 and
IPv4-mapped peers, and the context round-trip.

Model: opus-4-8
2026-09-21 23:23:47 +00:00
sneak c5f4682b0b feat: add trusted_proxies config key
Add a trusted_proxies CIDR-list config key alongside blocked_networks.
Generalize the blocked_networks parser into parseCIDRList and
cidrListEntries, which take the key name as a parameter, so both keys
share one parser rather than a second copy. An invalid entry aborts
startup naming the key and value; an omitted or empty key leaves the
list empty.

Model: opus-4-8
2026-09-21 23:23:47 +00:00
sneak 7697822c53 test: add failing config tests for trusted_proxies
Check that a valid CIDR list lands in TrustedProxies in order, an omitted
key trusts no one, and an invalid or null value aborts startup naming the
key and value. The list parser is shared with blocked_networks, whose
tests exercise the remaining shapes.

Model: opus-4-8
2026-09-21 23:23:47 +00:00
12 changed files with 624 additions and 37 deletions
+9
View File
@@ -126,6 +126,15 @@ Configured via YAML file (`--config`). Key settings:
added to the always-enforced built-in ranges (loopback, private,
link-local, CGNAT, benchmark, NAT64, and the like); an invalid CIDR
aborts startup
- `trusted_proxies` — list of CIDR ranges of the reverse proxies in front
of pixa. `X-Forwarded-For` is believed only when the direct peer falls
inside one of these ranges; the logged and login-recorded client
address is then the rightmost forwarded entry that is not itself a
trusted proxy. Otherwise the direct peer address is used and the header
is ignored, so a client connecting directly cannot spoof its address.
Omitted or empty trusts no one; an invalid CIDR aborts startup. Set
this to your proxy's address range when deploying behind a reverse
proxy
- `upstream_fetch_timeout` — timeout for origin requests
- `upstream_max_response_size` — max origin response size
- `downstream_timeout` — client response timeout
+11
View File
@@ -30,6 +30,17 @@ exhaustion
# Completed Steps
- 2026-09-21 trusted-proxy client IP resolution (closes #94): a
`trusted_proxies` config key taking a list of CIDRs, parsed by the same
`net/netip` list parser as `blocked_networks` (an invalid entry aborts
startup naming the key and value; omitted or empty trusts no one); a new
`internal/clientip` package resolves the client address by honoring
`X-Forwarded-For` only when the direct peer is a trusted proxy, walking
the chain right-to-left to the rightmost non-proxy entry, so a client
connecting directly cannot spoof its address; the resolved address is
stored in the request context by a new middleware and used by the
request-logging middleware and the login-attempt logs in place of the
raw peer address; documented in `README.md` and `config.example.yml`.
- 2026-09-21 blocked networks configuration extending SSRF protection: a
`blocked_networks` config key taking a list of CIDRs (parsed with
`net/netip`, an invalid entry aborts startup naming the key and value),
+12
View File
@@ -31,6 +31,18 @@ allowlist_hosts:
# - 100.64.0.0/10
# - 2001:db8::/32
# CIDR ranges of the reverse proxies in front of pixa. X-Forwarded-For
# is believed only when the direct peer is inside one of these ranges;
# the client address in the access log and login records is then the
# rightmost forwarded entry that is not itself a trusted proxy. A client
# connecting directly (peer outside these ranges) cannot spoof its
# address: the header is ignored and the peer address is used. Omitted or
# empty trusts no one; an invalid CIDR aborts startup. Set this when
# deploying behind a proxy.
# trusted_proxies:
# - 10.0.0.0/8
# - 2001:db8::/32
# Allow HTTP upstream (only for testing, always use HTTPS in production)
allow_http: false
+119
View File
@@ -0,0 +1,119 @@
// Package clientip resolves the real client IP address of an HTTP request
// when pixa runs behind a reverse proxy. Forwarding headers are believed
// only when the immediate peer is a configured trusted proxy, so an
// untrusted client cannot spoof its address by sending the header.
package clientip
import (
"context"
"net"
"net/netip"
"slices"
"strings"
)
// ForwardedForHeader is the request header carrying the proxy chain. It is
// honored only when the immediate peer is a trusted proxy.
const ForwardedForHeader = "X-Forwarded-For"
// Resolver determines the client IP of a request against a fixed set of
// trusted proxy networks.
type Resolver struct {
trusted []netip.Prefix
}
// NewResolver returns a Resolver that trusts forwarding headers only from
// peers inside the given CIDR ranges. A nil or empty list trusts no one,
// so the peer address is always used.
func NewResolver(trusted []netip.Prefix) *Resolver {
return &Resolver{trusted: trusted}
}
// Resolve returns the client IP for a request whose direct peer is
// remoteAddr (a "host:port" string as in http.Request.RemoteAddr) and
// whose X-Forwarded-For header lines are forwardedFor (as returned by
// http.Header.Values). When the peer is not a trusted proxy, the peer
// address is returned and the header is ignored entirely. When the peer is
// trusted, the header is walked right to left and the first address that is
// not itself a trusted proxy is returned; this is the client the outermost
// trusted proxy observed, and entries an untrusted client may have prepended
// sit to its left and are never reached.
func (r *Resolver) Resolve(remoteAddr string, forwardedFor []string) string {
peer := hostOnly(remoteAddr)
peerAddr, err := netip.ParseAddr(peer)
if err != nil || !r.isTrusted(peerAddr) {
return peer
}
for _, hop := range slices.Backward(forwardedForChain(forwardedFor)) {
hopAddr, err := netip.ParseAddr(hop)
if err != nil || r.isTrusted(hopAddr) {
continue
}
return hopAddr.String()
}
return peerAddr.String()
}
// isTrusted reports whether addr falls inside one of the trusted proxy
// ranges. Addresses are unmapped first so an IPv4-mapped IPv6 form matches
// an IPv4 range, matching the fetcher's blocklist comparison.
func (r *Resolver) isTrusted(addr netip.Addr) bool {
if !addr.IsValid() {
return false
}
unmapped := addr.Unmap()
return slices.ContainsFunc(r.trusted, func(prefix netip.Prefix) bool {
return prefix.Contains(unmapped)
})
}
// hostOnly strips the port from a "host:port" address. A value without a
// port (already a bare host) is returned unchanged.
func hostOnly(remoteAddr string) string {
host, _, err := net.SplitHostPort(remoteAddr)
if err != nil {
return remoteAddr
}
return host
}
// forwardedForChain flattens the comma-separated entries of every
// X-Forwarded-For header line into a single ordered, trimmed list.
func forwardedForChain(values []string) []string {
var chain []string
for _, value := range values {
for part := range strings.SplitSeq(value, ",") {
trimmed := strings.TrimSpace(part)
if trimmed != "" {
chain = append(chain, trimmed)
}
}
}
return chain
}
// contextKey is the private key type under which the resolved client IP is
// stored in a request context.
type contextKey struct{}
// WithClientIP returns a copy of ctx carrying the resolved client IP.
func WithClientIP(ctx context.Context, ip string) context.Context {
return context.WithValue(ctx, contextKey{}, ip)
}
// FromContext returns the resolved client IP stored in ctx, or an empty
// string if none was set.
func FromContext(ctx context.Context) string {
ip, _ := ctx.Value(contextKey{}).(string)
return ip
}
+189
View File
@@ -0,0 +1,189 @@
package clientip_test
import (
"net/netip"
"testing"
"sneak.berlin/go/pixa/internal/clientip"
)
// Addresses reused across the resolver cases.
const (
trustedRangeV4 = "10.0.0.0/8"
forwardedV4 = "203.0.113.7"
untrustedV4 = "198.51.100.9"
trustedPeer = "10.0.0.1:5000"
)
// mustPrefixes parses CIDR strings into prefixes for building a resolver.
func mustPrefixes(t *testing.T, cidrs ...string) []netip.Prefix {
t.Helper()
prefixes := make([]netip.Prefix, 0, len(cidrs))
for _, c := range cidrs {
p, err := netip.ParsePrefix(c)
if err != nil {
t.Fatalf("netip.ParsePrefix(%q) error = %v", c, err)
}
prefixes = append(prefixes, p)
}
return prefixes
}
type resolveCase struct {
name string
trusted []string
remoteAddr string
forwardedFor []string
want string
}
// runResolveCases runs each case against a resolver built from its trusted
// list and checks the resolved address.
func runResolveCases(t *testing.T, cases []resolveCase) {
t.Helper()
for _, tt := range cases {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
r := clientip.NewResolver(mustPrefixes(t, tt.trusted...))
got := r.Resolve(tt.remoteAddr, tt.forwardedFor)
if got != tt.want {
t.Errorf("Resolve(%q, %v) = %q, want %q",
tt.remoteAddr, tt.forwardedFor, got, tt.want)
}
})
}
}
// TestResolvePeerTrust covers the trust decision on the direct peer: a
// forwarded header is believed only from a trusted peer, and a client
// connecting directly cannot spoof its address.
func TestResolvePeerTrust(t *testing.T) {
t.Parallel()
runResolveCases(t, []resolveCase{
{
name: "trusted peer honors forwarded client",
trusted: []string{trustedRangeV4},
remoteAddr: trustedPeer,
forwardedFor: []string{forwardedV4},
want: forwardedV4,
},
{
name: "untrusted peer ignores forwarded header",
trusted: []string{trustedRangeV4},
remoteAddr: untrustedV4 + ":33333",
forwardedFor: []string{forwardedV4},
want: untrustedV4,
},
{
name: "spoofed chain from untrusted peer cannot influence result",
trusted: []string{trustedRangeV4},
remoteAddr: untrustedV4 + ":33333",
forwardedFor: []string{"1.2.3.4, 10.9.9.9, 127.0.0.1"},
want: untrustedV4,
},
{
name: "empty trusted list always uses peer",
trusted: nil,
remoteAddr: forwardedV4 + ":80",
forwardedFor: []string{"10.0.0.5"},
want: forwardedV4,
},
{
name: "trusted peer with no forwarded header uses peer",
trusted: []string{trustedRangeV4},
remoteAddr: trustedPeer,
forwardedFor: nil,
want: "10.0.0.1",
},
{
name: "unparseable peer is returned unchanged",
trusted: []string{trustedRangeV4},
remoteAddr: "garbage",
forwardedFor: []string{forwardedV4},
want: "garbage",
},
})
}
// TestResolveChainWalk covers walking the X-Forwarded-For chain from a
// trusted peer to the rightmost entry that is not itself a trusted proxy.
func TestResolveChainWalk(t *testing.T) {
t.Parallel()
runResolveCases(t, []resolveCase{
{
name: "rightmost untrusted entry across a mixed chain",
trusted: []string{trustedRangeV4, "192.168.0.0/16"},
remoteAddr: trustedPeer,
forwardedFor: []string{forwardedV4 + ", 192.168.1.1, 10.0.0.2"},
want: forwardedV4,
},
{
name: "spoofed client behind a trusted proxy is not believed",
trusted: []string{trustedRangeV4},
remoteAddr: trustedPeer,
forwardedFor: []string{"1.2.3.4, " + untrustedV4},
want: untrustedV4,
},
{
name: "chain split across multiple header lines",
trusted: []string{trustedRangeV4},
remoteAddr: trustedPeer,
forwardedFor: []string{forwardedV4, "10.0.0.2"},
want: forwardedV4,
},
{
name: "garbage entries are skipped",
trusted: []string{trustedRangeV4},
remoteAddr: trustedPeer,
forwardedFor: []string{forwardedV4 + ", not-an-ip"},
want: forwardedV4,
},
{
name: "all-trusted chain falls back to peer",
trusted: []string{trustedRangeV4},
remoteAddr: trustedPeer,
forwardedFor: []string{"10.0.0.9, 10.0.0.2"},
want: "10.0.0.1",
},
{
name: "trusted IPv6 peer honors forwarded client",
trusted: []string{"2001:db8::/32"},
remoteAddr: "[2001:db8::1]:9000",
forwardedFor: []string{forwardedV4},
want: forwardedV4,
},
{
name: "IPv4-mapped peer matches IPv4 trusted range",
trusted: []string{trustedRangeV4},
remoteAddr: "[::ffff:10.0.0.1]:5000",
forwardedFor: []string{forwardedV4},
want: forwardedV4,
},
})
}
func TestContextRoundTrip(t *testing.T) {
t.Parallel()
ctx := clientip.WithClientIP(t.Context(), forwardedV4)
if got := clientip.FromContext(ctx); got != forwardedV4 {
t.Errorf("FromContext = %q, want %q", got, forwardedV4)
}
}
func TestFromContextAbsent(t *testing.T) {
t.Parallel()
if got := clientip.FromContext(t.Context()); got != "" {
t.Errorf("FromContext with no value = %q, want empty", got)
}
}
+35 -19
View File
@@ -44,6 +44,7 @@ const (
keyUpstreamConnectionsPerHost = "upstream_connections_per_host"
keyCacheMaxBytes = "cache_max_bytes"
keyBlockedNetworks = "blocked_networks"
keyTrustedProxies = "trusted_proxies"
)
// placeholderSigningKey is the dummy signing_key shipped in
@@ -117,6 +118,14 @@ type Config struct {
// fetcher's dialer; the built-in ranges always apply.
BlockedNetworks []netip.Prefix
// TrustedProxies are the CIDR ranges of reverse proxies whose
// forwarding headers may be believed. Forwarded headers are honored
// only when the immediate peer falls inside one of these ranges;
// otherwise the peer address is used and the headers are ignored, so
// an untrusted client cannot spoof its address. Empty means trust
// nothing and always use the peer address.
TrustedProxies []netip.Prefix
// CacheMaxBytes is the disk cache size limit in bytes. Zero
// disables the disk cache entirely. When cache_max_bytes is
// omitted from the configuration, this holds the computed default
@@ -185,7 +194,12 @@ func newFromSmartConfig(sc *smartconfig.Config) (*Config, error) {
}
}
blockedNetworks, err := getBlockedNetworks(sc)
blockedNetworks, err := parseCIDRList(sc, keyBlockedNetworks)
if err != nil {
return nil, err
}
trustedProxies, err := parseCIDRList(sc, keyTrustedProxies)
if err != nil {
return nil, err
}
@@ -207,6 +221,7 @@ func newFromSmartConfig(sc *smartconfig.Config) (*Config, error) {
keyUpstreamConnectionsPerHost, DefaultUpstreamConnectionsPerHost),
CacheMaxBytes: loader.int64Val(keyCacheMaxBytes, 0),
BlockedNetworks: blockedNetworks,
TrustedProxies: trustedProxies,
}
// The computed default for cache_max_bytes needs a validated
@@ -322,7 +337,8 @@ func isKnownConfigKey(key string) bool {
switch key {
case keyDebug, keyMaintenanceMode, keyPort, keyStateDir, keySentryDSN,
keyDBURL, keyMetrics, keySigningKey, keyAllowlistHosts, keyAllowHTTP,
keyUpstreamConnectionsPerHost, keyCacheMaxBytes, keyBlockedNetworks, "env":
keyUpstreamConnectionsPerHost, keyCacheMaxBytes, keyBlockedNetworks,
keyTrustedProxies, "env":
return true
}
@@ -817,27 +833,27 @@ func getStringSlice(sc *smartconfig.Config) []string {
return nil
}
// getBlockedNetworks parses the blocked_networks value into CIDR prefixes,
// or returns nil if the key is omitted. It accepts a YAML list of strings
// or a comma-separated string. An explicitly null value, a wrong type, an
// empty entry, a non-string entry, or an unparseable CIDR aborts startup
// naming the key and the offending value; a default (the built-in
// blocklist alone) applies only to an omitted key.
func getBlockedNetworks(sc *smartconfig.Config) ([]netip.Prefix, error) {
// parseCIDRList parses the value of the named config key into CIDR
// prefixes, or returns nil if the key is omitted. It accepts a YAML list
// of strings or a comma-separated string. An explicitly null value, a
// wrong type, an empty entry, a non-string entry, or an unparseable CIDR
// aborts startup naming the key and the offending value; the default
// (an empty list) applies only to an omitted key.
func parseCIDRList(sc *smartconfig.Config, key string) ([]netip.Prefix, error) {
if sc == nil {
return nil, nil
}
raw, ok := sc.Get(keyBlockedNetworks)
raw, ok := sc.Get(key)
if !ok {
return nil, nil
}
if raw == nil {
return nil, errNullConfigValue(keyBlockedNetworks)
return nil, errNullConfigValue(key)
}
entries, err := blockedNetworkEntries(raw)
entries, err := cidrListEntries(raw, key)
if err != nil {
return nil, err
}
@@ -848,7 +864,7 @@ func getBlockedNetworks(sc *smartconfig.Config) ([]netip.Prefix, error) {
prefix, err := netip.ParsePrefix(entry)
if err != nil {
return nil, fmt.Errorf("config key %q: value %q is %w",
keyBlockedNetworks, entry, errNotAValidCIDR)
key, entry, errNotAValidCIDR)
}
prefixes = append(prefixes, prefix)
@@ -857,10 +873,10 @@ func getBlockedNetworks(sc *smartconfig.Config) ([]netip.Prefix, error) {
return prefixes, nil
}
// blockedNetworkEntries extracts the raw blocked_networks entries as
// cidrListEntries extracts the raw entries of the named CIDR-list key as
// trimmed, non-empty strings, from either a YAML list of strings or a
// comma-separated string. Any other shape is a configuration error.
func blockedNetworkEntries(raw any) ([]string, error) {
func cidrListEntries(raw any, key string) ([]string, error) {
switch val := raw.(type) {
case []any:
entries := make([]string, 0, len(val))
@@ -869,12 +885,12 @@ func blockedNetworkEntries(raw any) ([]string, error) {
str, ok := item.(string)
if !ok {
return nil, fmt.Errorf("config key %q: list entry %v (%T) is %w",
keyBlockedNetworks, item, item, errNotAString)
key, item, item, errNotAString)
}
if strings.TrimSpace(str) == "" {
return nil, fmt.Errorf("config key %q: %w",
keyBlockedNetworks, errEmptyListEntry)
key, errEmptyListEntry)
}
entries = append(entries, strings.TrimSpace(str))
@@ -888,7 +904,7 @@ func blockedNetworkEntries(raw any) ([]string, error) {
trimmed := strings.TrimSpace(part)
if trimmed == "" {
return nil, fmt.Errorf("config key %q: value %q %w",
keyBlockedNetworks, val, errEmptyEntry)
key, val, errEmptyEntry)
}
entries = append(entries, trimmed)
@@ -897,6 +913,6 @@ func blockedNetworkEntries(raw any) ([]string, error) {
return entries, nil
default:
return nil, fmt.Errorf("config key %q: value %v (%T) is %w",
keyBlockedNetworks, raw, raw, errNotAStringList)
key, raw, raw, errNotAStringList)
}
}
@@ -0,0 +1,65 @@
package config
import (
"strings"
"testing"
)
// TestTrustedProxiesConfig checks the trusted_proxies key wiring: a valid
// CIDR list lands in TrustedProxies in order, and an omitted key trusts no
// one. The list parser itself is shared with blocked_networks and is
// exercised in depth by that key's tests.
func TestTrustedProxiesConfig(t *testing.T) {
t.Parallel()
t.Run("valid list is parsed in order", func(t *testing.T) {
t.Parallel()
c, err := configFromYAML(t,
signingKeyLine+`trusted_proxies: ["10.0.0.0/8", "2001:db8::/32"]`+"\n")
if err != nil {
t.Fatalf("valid trusted_proxies should load: %v", err)
}
got := make([]string, len(c.TrustedProxies))
for i, p := range c.TrustedProxies {
got[i] = p.String()
}
if joined := strings.Join(got, ","); joined != "10.0.0.0/8,2001:db8::/32" {
t.Errorf("TrustedProxies = %v, want the two ranges in order", got)
}
})
t.Run("omitted key trusts no one", func(t *testing.T) {
t.Parallel()
c, err := configFromYAML(t, signingKeyLine)
if err != nil {
t.Fatalf("minimal config should load: %v", err)
}
if len(c.TrustedProxies) != 0 {
t.Errorf("TrustedProxies = %v, want empty", c.TrustedProxies)
}
})
}
// TestTrustedProxiesInvalidAbortsStartup checks that an invalid or null
// value aborts startup with an error naming the key and the offending value.
func TestTrustedProxiesInvalidAbortsStartup(t *testing.T) {
t.Parallel()
runAbortCases(t, []abortCase{
{
name: "invalid cidr",
yaml: signingKeyLine + `trusted_proxies: ["999.0.0.0/8"]` + "\n",
wantErrSubstrings: []string{keyTrustedProxies, "999.0.0.0/8"},
},
{
name: "null value",
yaml: signingKeyLine + "trusted_proxies:\n",
wantErrSubstrings: []string{keyTrustedProxies, nullValueText},
},
})
}
+5 -2
View File
@@ -8,6 +8,7 @@ import (
"strconv"
"time"
"sneak.berlin/go/pixa/internal/clientip"
"sneak.berlin/go/pixa/internal/encurl"
"sneak.berlin/go/pixa/internal/imgcache"
"sneak.berlin/go/pixa/internal/templates"
@@ -47,7 +48,8 @@ func (s *Handlers) handleLoginPost(w http.ResponseWriter, r *http.Request) {
// Constant-time comparison to prevent timing attacks
if subtle.ConstantTimeCompare([]byte(submittedKey), []byte(s.config.SigningKey)) != 1 {
s.log.Warn("failed login attempt", "remote_addr", r.RemoteAddr)
s.log.Warn("failed login attempt",
"remote_addr", clientip.FromContext(r.Context()))
s.renderLogin(w, r, "Invalid signing key")
return
@@ -62,7 +64,8 @@ func (s *Handlers) handleLoginPost(w http.ResponseWriter, r *http.Request) {
return
}
s.log.Info("successful login", "remote_addr", r.RemoteAddr)
s.log.Info("successful login",
"remote_addr", clientip.FromContext(r.Context()))
// Redirect to generator page
http.Redirect(w, r, "/", http.StatusSeeOther)
@@ -0,0 +1,41 @@
package handlers
import (
"bytes"
"log/slog"
"net/http"
"net/http/httptest"
"net/url"
"strings"
"testing"
"sneak.berlin/go/pixa/internal/clientip"
"sneak.berlin/go/pixa/internal/config"
)
// TestFailedLoginLogsResolvedClientIP verifies the failed-login record
// carries the resolved client IP from the request context, not the raw
// proxy peer address.
func TestFailedLoginLogsResolvedClientIP(t *testing.T) {
t.Parallel()
var buf bytes.Buffer
h := &Handlers{
log: slog.New(slog.NewJSONHandler(&buf, nil)),
config: &config.Config{SigningKey: testSigningKey},
}
form := url.Values{loginKeyField: {"wrong-key"}}
req := httptest.NewRequestWithContext(
t.Context(), http.MethodPost, "/",
strings.NewReader(form.Encode()))
req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
req = req.WithContext(clientip.WithClientIP(req.Context(), "203.0.113.7"))
h.handleLoginPost(httptest.NewRecorder(), req)
if !strings.Contains(buf.String(), `"remote_addr":"203.0.113.7"`) {
t.Errorf("failed-login log missing resolved client IP; got %q", buf.String())
}
}
@@ -0,0 +1,118 @@
package middleware
import (
"bytes"
"log/slog"
"net/http"
"net/http/httptest"
"net/netip"
"strings"
"testing"
"sneak.berlin/go/pixa/internal/clientip"
"sneak.berlin/go/pixa/internal/config"
)
// testForwardedClient is the client address the proxy forwards.
const testForwardedClient = "203.0.113.7"
// newTestMiddleware builds a Middleware whose resolver trusts the given
// CIDRs and whose logger writes JSON to buf.
func newTestMiddleware(t *testing.T, buf *bytes.Buffer, trusted ...string) *Middleware {
t.Helper()
prefixes := make([]netip.Prefix, 0, len(trusted))
for _, c := range trusted {
p, err := netip.ParsePrefix(c)
if err != nil {
t.Fatalf("netip.ParsePrefix(%q) error = %v", c, err)
}
prefixes = append(prefixes, p)
}
return &Middleware{
log: slog.New(slog.NewJSONHandler(buf, nil)),
config: &config.Config{TrustedProxies: prefixes},
clientIP: clientip.NewResolver(prefixes),
}
}
// TestClientIPMiddlewareStoresResolvedIP verifies the ClientIP middleware
// puts the resolved address into the request context for a trusted and an
// untrusted peer.
func TestClientIPMiddlewareStoresResolvedIP(t *testing.T) {
t.Parallel()
tests := []struct {
name string
remoteAddr string
forwarded string
want string
}{
{
name: "trusted peer honors forwarded client",
remoteAddr: "10.0.0.1:5000",
forwarded: testForwardedClient,
want: testForwardedClient,
},
{
name: "untrusted peer ignores forwarded header",
remoteAddr: "198.51.100.9:5000",
forwarded: testForwardedClient,
want: "198.51.100.9",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
mw := newTestMiddleware(t, &bytes.Buffer{}, "10.0.0.0/8")
var got string
handler := mw.ClientIP()(http.HandlerFunc(
func(_ http.ResponseWriter, r *http.Request) {
got = clientip.FromContext(r.Context())
}))
req := httptest.NewRequestWithContext(
t.Context(), http.MethodGet, "/", nil)
req.RemoteAddr = tt.remoteAddr
req.Header.Set("X-Forwarded-For", tt.forwarded)
handler.ServeHTTP(httptest.NewRecorder(), req)
if got != tt.want {
t.Errorf("client IP in context = %q, want %q", got, tt.want)
}
})
}
}
// TestLoggingUsesResolvedClientIP verifies the logging middleware records
// the resolved forwarded client IP rather than the proxy peer address.
func TestLoggingUsesResolvedClientIP(t *testing.T) {
t.Parallel()
var buf bytes.Buffer
mw := newTestMiddleware(t, &buf, "10.0.0.0/8")
handler := mw.ClientIP()(mw.Logging()(http.HandlerFunc(
func(w http.ResponseWriter, _ *http.Request) {
w.WriteHeader(http.StatusOK)
})))
req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/", nil)
req.RemoteAddr = "10.0.0.1:5000"
req.Header.Set("X-Forwarded-For", testForwardedClient)
handler.ServeHTTP(httptest.NewRecorder(), req)
if !strings.Contains(buf.String(), `"remoteIP":"`+testForwardedClient+`"`) {
t.Errorf("log output missing resolved client IP; got %q", buf.String())
}
}
+15 -12
View File
@@ -3,7 +3,6 @@ package middleware
import (
"log/slog"
"net"
"net/http"
"time"
@@ -14,6 +13,7 @@ import (
ghmm "github.com/slok/go-http-metrics/middleware"
"github.com/slok/go-http-metrics/middleware/std"
"go.uber.org/fx"
"sneak.berlin/go/pixa/internal/clientip"
"sneak.berlin/go/pixa/internal/config"
"sneak.berlin/go/pixa/internal/logger"
)
@@ -60,6 +60,7 @@ type Params struct {
type Middleware struct {
log *slog.Logger
config *config.Config
clientIP *clientip.Resolver
}
// New creates a new Middleware instance.
@@ -67,22 +68,24 @@ func New(_ fx.Lifecycle, params Params) (*Middleware, error) {
s := &Middleware{
log: params.Logger.Get(),
config: params.Config,
clientIP: clientip.NewResolver(params.Config.TrustedProxies),
}
return s, nil
}
func ipFromHostPort(hp string) string {
h, _, err := net.SplitHostPort(hp)
if err != nil {
return ""
// ClientIP returns a middleware that resolves the real client IP,
// honoring X-Forwarded-For only from trusted proxies, and stores it in
// the request context for the logging middleware and handlers to read.
func (s *Middleware) ClientIP() func(http.Handler) http.Handler {
return func(next http.Handler) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
ip := s.clientIP.Resolve(
r.RemoteAddr, r.Header.Values(clientip.ForwardedForHeader))
ctx := clientip.WithClientIP(r.Context(), ip)
next.ServeHTTP(w, r.WithContext(ctx))
})
}
if len(h) > 0 && h[0] == '[' {
return h[1 : len(h)-1]
}
return h
}
type loggingResponseWriter struct {
@@ -127,7 +130,7 @@ func (s *Middleware) Logging() func(http.Handler) http.Handler {
"request_id", reqID,
"referer", r.Referer(),
"proto", r.Proto,
"remoteIP", ipFromHostPort(r.RemoteAddr),
"remoteIP", clientip.FromContext(ctx),
"status", lrw.statusCode,
"response_bytes", lrw.bytesWritten,
"latency_ms", latency.Milliseconds(),
+1
View File
@@ -18,6 +18,7 @@ func (s *Server) SetupRoutes() {
s.router.Use(middleware.Recoverer)
s.router.Use(middleware.RequestID)
s.router.Use(s.mw.ClientIP())
s.router.Use(s.mw.SecurityHeaders())
s.router.Use(s.mw.Logging())