Resolve real client IP behind trusted proxies (closes #94) #127

Open
clawbot wants to merge 6 commits from issue-94-trusted-proxies into next
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, added to the always-enforced built-in ranges (loopback, private,
link-local, CGNAT, benchmark, NAT64, and the like); an invalid CIDR link-local, CGNAT, benchmark, NAT64, and the like); an invalid CIDR
aborts startup 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_fetch_timeout` — timeout for origin requests
- `upstream_max_response_size` — max origin response size - `upstream_max_response_size` — max origin response size
- `downstream_timeout` — client response timeout - `downstream_timeout` — client response timeout
+11
View File
@@ -30,6 +30,17 @@ exhaustion
# Completed Steps # 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 - 2026-09-21 blocked networks configuration extending SSRF protection: a
`blocked_networks` config key taking a list of CIDRs (parsed with `blocked_networks` config key taking a list of CIDRs (parsed with
`net/netip`, an invalid entry aborts startup naming the key and value), `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 # - 100.64.0.0/10
# - 2001:db8::/32 # - 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 upstream (only for testing, always use HTTPS in production)
allow_http: false 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" keyUpstreamConnectionsPerHost = "upstream_connections_per_host"
keyCacheMaxBytes = "cache_max_bytes" keyCacheMaxBytes = "cache_max_bytes"
keyBlockedNetworks = "blocked_networks" keyBlockedNetworks = "blocked_networks"
keyTrustedProxies = "trusted_proxies"
) )
// placeholderSigningKey is the dummy signing_key shipped in // placeholderSigningKey is the dummy signing_key shipped in
@@ -117,6 +118,14 @@ type Config struct {
// fetcher's dialer; the built-in ranges always apply. // fetcher's dialer; the built-in ranges always apply.
BlockedNetworks []netip.Prefix 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 // CacheMaxBytes is the disk cache size limit in bytes. Zero
// disables the disk cache entirely. When cache_max_bytes is // disables the disk cache entirely. When cache_max_bytes is
// omitted from the configuration, this holds the computed default // 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 { if err != nil {
return nil, err return nil, err
} }
@@ -207,6 +221,7 @@ func newFromSmartConfig(sc *smartconfig.Config) (*Config, error) {
keyUpstreamConnectionsPerHost, DefaultUpstreamConnectionsPerHost), keyUpstreamConnectionsPerHost, DefaultUpstreamConnectionsPerHost),
CacheMaxBytes: loader.int64Val(keyCacheMaxBytes, 0), CacheMaxBytes: loader.int64Val(keyCacheMaxBytes, 0),
BlockedNetworks: blockedNetworks, BlockedNetworks: blockedNetworks,
TrustedProxies: trustedProxies,
} }
// The computed default for cache_max_bytes needs a validated // The computed default for cache_max_bytes needs a validated
@@ -322,7 +337,8 @@ func isKnownConfigKey(key string) bool {
switch key { switch key {
case keyDebug, keyMaintenanceMode, keyPort, keyStateDir, keySentryDSN, case keyDebug, keyMaintenanceMode, keyPort, keyStateDir, keySentryDSN,
keyDBURL, keyMetrics, keySigningKey, keyAllowlistHosts, keyAllowHTTP, keyDBURL, keyMetrics, keySigningKey, keyAllowlistHosts, keyAllowHTTP,
keyUpstreamConnectionsPerHost, keyCacheMaxBytes, keyBlockedNetworks, "env": keyUpstreamConnectionsPerHost, keyCacheMaxBytes, keyBlockedNetworks,
keyTrustedProxies, "env":
return true return true
} }
@@ -817,27 +833,27 @@ func getStringSlice(sc *smartconfig.Config) []string {
return nil return nil
} }
// getBlockedNetworks parses the blocked_networks value into CIDR prefixes, // parseCIDRList parses the value of the named config key into CIDR
// or returns nil if the key is omitted. It accepts a YAML list of strings // prefixes, or returns nil if the key is omitted. It accepts a YAML list
// or a comma-separated string. An explicitly null value, a wrong type, an // of strings or a comma-separated string. An explicitly null value, a
// empty entry, a non-string entry, or an unparseable CIDR aborts startup // wrong type, an empty entry, a non-string entry, or an unparseable CIDR
// naming the key and the offending value; a default (the built-in // aborts startup naming the key and the offending value; the default
// blocklist alone) applies only to an omitted key. // (an empty list) applies only to an omitted key.
func getBlockedNetworks(sc *smartconfig.Config) ([]netip.Prefix, error) { func parseCIDRList(sc *smartconfig.Config, key string) ([]netip.Prefix, error) {
if sc == nil { if sc == nil {
return nil, nil return nil, nil
} }
raw, ok := sc.Get(keyBlockedNetworks) raw, ok := sc.Get(key)
if !ok { if !ok {
return nil, nil return nil, nil
} }
if raw == nil { if raw == nil {
return nil, errNullConfigValue(keyBlockedNetworks) return nil, errNullConfigValue(key)
} }
entries, err := blockedNetworkEntries(raw) entries, err := cidrListEntries(raw, key)
if err != nil { if err != nil {
return nil, err return nil, err
} }
@@ -848,7 +864,7 @@ func getBlockedNetworks(sc *smartconfig.Config) ([]netip.Prefix, error) {
prefix, err := netip.ParsePrefix(entry) prefix, err := netip.ParsePrefix(entry)
if err != nil { if err != nil {
return nil, fmt.Errorf("config key %q: value %q is %w", return nil, fmt.Errorf("config key %q: value %q is %w",
keyBlockedNetworks, entry, errNotAValidCIDR) key, entry, errNotAValidCIDR)
} }
prefixes = append(prefixes, prefix) prefixes = append(prefixes, prefix)
@@ -857,10 +873,10 @@ func getBlockedNetworks(sc *smartconfig.Config) ([]netip.Prefix, error) {
return prefixes, nil 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 // trimmed, non-empty strings, from either a YAML list of strings or a
// comma-separated string. Any other shape is a configuration error. // 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) { switch val := raw.(type) {
case []any: case []any:
entries := make([]string, 0, len(val)) entries := make([]string, 0, len(val))
@@ -869,12 +885,12 @@ func blockedNetworkEntries(raw any) ([]string, error) {
str, ok := item.(string) str, ok := item.(string)
if !ok { if !ok {
return nil, fmt.Errorf("config key %q: list entry %v (%T) is %w", 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) == "" { if strings.TrimSpace(str) == "" {
return nil, fmt.Errorf("config key %q: %w", return nil, fmt.Errorf("config key %q: %w",
keyBlockedNetworks, errEmptyListEntry) key, errEmptyListEntry)
} }
entries = append(entries, strings.TrimSpace(str)) entries = append(entries, strings.TrimSpace(str))
@@ -888,7 +904,7 @@ func blockedNetworkEntries(raw any) ([]string, error) {
trimmed := strings.TrimSpace(part) trimmed := strings.TrimSpace(part)
if trimmed == "" { if trimmed == "" {
return nil, fmt.Errorf("config key %q: value %q %w", return nil, fmt.Errorf("config key %q: value %q %w",
keyBlockedNetworks, val, errEmptyEntry) key, val, errEmptyEntry)
} }
entries = append(entries, trimmed) entries = append(entries, trimmed)
@@ -897,6 +913,6 @@ func blockedNetworkEntries(raw any) ([]string, error) {
return entries, nil return entries, nil
default: default:
return nil, fmt.Errorf("config key %q: value %v (%T) is %w", 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" "strconv"
"time" "time"
"sneak.berlin/go/pixa/internal/clientip"
"sneak.berlin/go/pixa/internal/encurl" "sneak.berlin/go/pixa/internal/encurl"
"sneak.berlin/go/pixa/internal/imgcache" "sneak.berlin/go/pixa/internal/imgcache"
"sneak.berlin/go/pixa/internal/templates" "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 // Constant-time comparison to prevent timing attacks
if subtle.ConstantTimeCompare([]byte(submittedKey), []byte(s.config.SigningKey)) != 1 { 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") s.renderLogin(w, r, "Invalid signing key")
return return
@@ -62,7 +64,8 @@ func (s *Handlers) handleLoginPost(w http.ResponseWriter, r *http.Request) {
return 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 // Redirect to generator page
http.Redirect(w, r, "/", http.StatusSeeOther) 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())
}
}
+19 -16
View File
@@ -3,7 +3,6 @@ package middleware
import ( import (
"log/slog" "log/slog"
"net"
"net/http" "net/http"
"time" "time"
@@ -14,6 +13,7 @@ import (
ghmm "github.com/slok/go-http-metrics/middleware" ghmm "github.com/slok/go-http-metrics/middleware"
"github.com/slok/go-http-metrics/middleware/std" "github.com/slok/go-http-metrics/middleware/std"
"go.uber.org/fx" "go.uber.org/fx"
"sneak.berlin/go/pixa/internal/clientip"
"sneak.berlin/go/pixa/internal/config" "sneak.berlin/go/pixa/internal/config"
"sneak.berlin/go/pixa/internal/logger" "sneak.berlin/go/pixa/internal/logger"
) )
@@ -58,31 +58,34 @@ type Params struct {
// Middleware provides HTTP middleware functions. // Middleware provides HTTP middleware functions.
type Middleware struct { type Middleware struct {
log *slog.Logger log *slog.Logger
config *config.Config config *config.Config
clientIP *clientip.Resolver
} }
// New creates a new Middleware instance. // New creates a new Middleware instance.
func New(_ fx.Lifecycle, params Params) (*Middleware, error) { func New(_ fx.Lifecycle, params Params) (*Middleware, error) {
s := &Middleware{ s := &Middleware{
log: params.Logger.Get(), log: params.Logger.Get(),
config: params.Config, config: params.Config,
clientIP: clientip.NewResolver(params.Config.TrustedProxies),
} }
return s, nil return s, nil
} }
func ipFromHostPort(hp string) string { // ClientIP returns a middleware that resolves the real client IP,
h, _, err := net.SplitHostPort(hp) // honoring X-Forwarded-For only from trusted proxies, and stores it in
if err != nil { // the request context for the logging middleware and handlers to read.
return "" 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 { type loggingResponseWriter struct {
@@ -127,7 +130,7 @@ func (s *Middleware) Logging() func(http.Handler) http.Handler {
"request_id", reqID, "request_id", reqID,
"referer", r.Referer(), "referer", r.Referer(),
"proto", r.Proto, "proto", r.Proto,
"remoteIP", ipFromHostPort(r.RemoteAddr), "remoteIP", clientip.FromContext(ctx),
"status", lrw.statusCode, "status", lrw.statusCode,
"response_bytes", lrw.bytesWritten, "response_bytes", lrw.bytesWritten,
"latency_ms", latency.Milliseconds(), "latency_ms", latency.Milliseconds(),
+1
View File
@@ -18,6 +18,7 @@ func (s *Server) SetupRoutes() {
s.router.Use(middleware.Recoverer) s.router.Use(middleware.Recoverer)
s.router.Use(middleware.RequestID) s.router.Use(middleware.RequestID)
s.router.Use(s.mw.ClientIP())
s.router.Use(s.mw.SecurityHeaders()) s.router.Use(s.mw.SecurityHeaders())
s.router.Use(s.mw.Logging()) s.router.Use(s.mw.Logging())