3 Commits
Author SHA1 Message Date
clawbot 10eab440e7 Resolve real client IP behind trusted proxies (closes #94)
check / check (push) Successful in 2m31s
RFC1918 ranges are the default trusted proxy set on an omitted key; an explicit list replaces the default; an explicit empty list trusts no one; unparseable values abort startup; forwarded headers honored only from trusted peers. Independent review passed: #127 (comment)

model: claude-opus-4-8 (implementation and review); merged by claude-fable-5
2026-09-22 10:25:41 +02:00
clawbot 3cfcda0730 feat: blocked_networks config and extended SSRF ranges (closes #67)
check / check (push) Failing after 1s
Adds the blocked_networks config key: a list of CIDRs, parsed with net/netip, that is added to the built-in list of address ranges the fetcher refuses to contact and can never remove an entry from it. An invalid CIDR aborts startup naming the key and the value.

The built-in list gains CGNAT 100.64.0.0/10, IETF protocol assignments 192.0.0.0/24, benchmark 198.18.0.0/15 and NAT64 64:ff9b::/96. Resolved addresses are unmapped before matching, so IPv4-mapped IPv6 forms are caught too. Enforcement stays in the dial-time re-resolution, which is what closes the DNS rebinding window.

What a reader would trip over: 192.0.0.0/24 is now blocked but TEST-NET-1 (192.0.2.0/24), which the Fetch tests use as a public upstream, is a different range and stays dialable. The package-level dialer enforces the built-in ranges only; operator entries are applied by the fetcher.

Disclosure: one nolint:gochecknoglobals on the immutable built-in prefix list.

Model: opus-4-8 (implementation, review); fable-5-1 (landing message)
2026-09-22 00:43:27 +02:00
clawbot 1798cba96c Take the image signing key from PIXA_SIGNING_KEY and refuse the example placeholder (closes #110)
check / check (push) Failing after 1s
The Docker image now ships config.docker.yml, which sets only signing_key (read from the PIXA_SIGNING_KEY environment variable), state_dir and port. The placeholder key and the five-host allowlist from config.example.yml are no longer in the image; anything else is configured by mounting a file over /etc/pixa/config.yml. A container started without PIXA_SIGNING_KEY exits naming it.

Startup now refuses the exact placeholder signing_key from config.example.yml. It is 45 characters long and used to pass the length check, so a deployment could sign URLs with a key that is public in this repository. README Getting Started is corrected to match.

What a reader would trip over: the unset-variable error comes from config interpolation, not from validate(); the signing key checks moved into validateSigningKey to stay under the complexity limit.

Disclosure: TODO.md is not updated by this change.

Model: opus-4-8 (implementation, review); fable-5-1 (landing message)
2026-09-21 21:59:24 +02:00
15 changed files with 739 additions and 46 deletions
+3 -2
View File
@@ -67,8 +67,9 @@ RUN adduser -D -H -s /sbin/nologin pixad && \
mkdir -p /var/lib/pixa /etc/pixa && \
chown pixad:pixad /var/lib/pixa
# Copy default config (edit signing_key before use)
COPY config.example.yml /etc/pixa/config.yml
# Copy the image config; signing_key comes from PIXA_SIGNING_KEY.
# Mount a file over /etc/pixa/config.yml to override anything else.
COPY config.docker.yml /etc/pixa/config.yml
USER pixad
WORKDIR /var/lib/pixa
+26 -3
View File
@@ -15,14 +15,25 @@ git clone https://git.eeqj.de/sneak/pixa.git
cd pixa
make build
# run with a config file
./bin/pixad --config config.example.yml
# run with a config file: copy the example and set a real signing key
# (the example placeholder is refused at startup), e.g. with
# openssl rand -base64 32
cp config.example.yml config.yml
$EDITOR config.yml # replace the signing_key placeholder
./bin/pixad --config config.yml
# or build and run via Docker
make docker
docker run -p 8080:8080 pixad:latest
docker run -p 8080:8080 -e PIXA_SIGNING_KEY="$(openssl rand -base64 32)" pixa:latest
```
A container is configured two ways. The signing key comes from the
`PIXA_SIGNING_KEY` environment variable, which the baked-in config
reads; if it is unset the container exits at startup naming the
variable. Everything else uses built-in defaults, so to change any
other setting mount your own file over `/etc/pixa/config.yml` (see
`config.example.yml` for the full set of keys).
## Rationale
Image-heavy web applications need a fast, caching reverse proxy that
@@ -115,6 +126,18 @@ 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.
An omitted key defaults to the RFC 1918 private ranges (`10.0.0.0/8`,
`172.16.0.0/12`, `192.168.0.0/16`), since pixa is deployed behind a
proxy on a private network; an explicitly empty list (`[]`) trusts no
one, and an explicit list replaces the default. An invalid CIDR aborts
startup. Set this to your proxy's address range if it is not already
covered by the defaults
- `upstream_fetch_timeout` — timeout for origin requests
- `upstream_max_response_size` — max origin response size
- `downstream_timeout` — client response timeout
+13
View File
@@ -30,6 +30,19 @@ 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; an omitted key defaults to the RFC 1918
private ranges, an explicitly empty list trusts no one, and an explicit
list replaces the default); 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),
+11
View File
@@ -0,0 +1,11 @@
# Pixa configuration baked into the Docker image.
#
# The signing key is read from the PIXA_SIGNING_KEY environment
# variable; startup aborts naming it when it is unset. Every other key
# is omitted so its default applies. Operators who need more (an
# allowlist, metrics, and so on) mount their own file over
# /etc/pixa/config.yml.
signing_key: "${ENV:PIXA_SIGNING_KEY}"
state_dir: /var/lib/pixa
port: 8080
+15
View File
@@ -31,6 +31,21 @@ 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. When
# omitted, this defaults to the RFC 1918 private ranges (10.0.0.0/8,
# 172.16.0.0/12, 192.168.0.0/16), since pixa is deployed behind a proxy on
# a private network. An explicitly empty list ([]) trusts no one; an
# explicit list replaces the default. An invalid CIDR aborts startup.
# Uncomment to override the defaults with your proxy's address range.
# 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)
}
}
+89 -23
View File
@@ -44,8 +44,15 @@ const (
keyUpstreamConnectionsPerHost = "upstream_connections_per_host"
keyCacheMaxBytes = "cache_max_bytes"
keyBlockedNetworks = "blocked_networks"
keyTrustedProxies = "trusted_proxies"
)
// placeholderSigningKey is the dummy signing_key shipped in
// config.example.yml. It is 45 characters, so it passes the length
// check, but it is public in this repository and must be rejected at
// startup so no deployment ever signs URLs with it.
const placeholderSigningKey = "CHANGE_ME_generate_with_openssl_rand_base64_32"
// Static validation errors. Each use site attaches the offending key
// and value by wrapping these with fmt.Errorf and %w.
var (
@@ -64,6 +71,9 @@ var (
errPortOutOfRange = errors.New("outside the valid port range")
errTooFewConnections = errors.New("must be at least 1")
errValueTooShort = errors.New("value too short")
errPlaceholderKey = errors.New(
"is the placeholder from config.example.yml; " +
"generate a real key with: openssl rand -base64 32")
errMustBeSetTogether = errors.New("must be set together")
errMustNotBeNegative = errors.New("must not be negative")
errOverflowsInt64 = errors.New("overflows a 64-bit integer")
@@ -108,6 +118,17 @@ 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. An omitted key
// defaults to the RFC 1918 private ranges (see defaultTrustedProxies),
// since pixa is deployed behind a proxy on a private network; an
// explicitly empty list trusts nothing and always uses the peer
// address, and an explicit list replaces the default.
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
@@ -176,11 +197,24 @@ 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
}
// parseCIDRList returns a nil slice only when the key is absent; an
// explicitly empty list ([]) comes back non-nil and empty. An omitted
// key takes the RFC 1918 default, while an explicit empty list is left
// as trust-nothing.
if trustedProxies == nil {
trustedProxies = defaultTrustedProxies()
}
loader := &strictLoader{sc: sc}
c := &Config{
@@ -198,6 +232,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
@@ -313,7 +348,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
}
@@ -355,10 +391,10 @@ func (c *Config) ensureStateDirWritable() error {
return nil
}
// validate checks that all required configuration values are set and
// that every value is within its valid range.
func (c *Config) validate() error {
// The signing key value is never echoed in error messages.
// validateSigningKey checks that the signing key is present, long
// enough, and not the public placeholder from config.example.yml. The
// key value itself is never echoed in error messages.
func (c *Config) validateSigningKey() error {
if c.SigningKey == "" {
return fmt.Errorf("config key %q: %w", keySigningKey, errValueRequired)
}
@@ -370,6 +406,21 @@ func (c *Config) validate() error {
keySigningKey, errValueTooShort, minKeyLength, len(c.SigningKey))
}
if c.SigningKey == placeholderSigningKey {
return fmt.Errorf("config key %q: %w", keySigningKey, errPlaceholderKey)
}
return nil
}
// validate checks that all required configuration values are set and
// that every value is within its valid range.
func (c *Config) validate() error {
err := c.validateSigningKey()
if err != nil {
return err
}
const maxPort = 65535
if c.Port < 1 || c.Port > maxPort {
return fmt.Errorf("config key %q: value %d is %w 1-%d",
@@ -793,27 +844,42 @@ 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) {
// defaultTrustedProxies returns the trusted_proxies default: the three RFC
// 1918 private ranges. pixa is always deployed behind a TLS-terminating
// reverse proxy, which in practice sits on a private network, so its
// forwarding headers are believed unless the operator says otherwise.
// Loopback is deliberately excluded: it is not an RFC 1918 range, and no
// deployment reaches pixa over it. A fresh slice is returned on each call so
// callers may hold it without aliasing shared state.
func defaultTrustedProxies() []netip.Prefix {
return []netip.Prefix{
netip.MustParsePrefix("10.0.0.0/8"),
netip.MustParsePrefix("172.16.0.0/12"),
netip.MustParsePrefix("192.168.0.0/16"),
}
}
// 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
}
@@ -824,7 +890,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)
@@ -833,10 +899,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))
@@ -845,12 +911,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))
@@ -864,7 +930,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)
@@ -873,6 +939,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)
}
}
@@ -303,6 +303,11 @@ func invalidHostAndCredentialCases() []abortCase {
yaml: "signing_key: short\n",
wantErrSubstrings: []string{keySigningKey},
},
{
name: "signing_key is the documented placeholder",
yaml: "signing_key: " + placeholderSigningKey + "\n",
wantErrSubstrings: []string{keySigningKey},
},
{
name: "signing_key missing",
yaml: "port: 8080\n",
@@ -0,0 +1,85 @@
package config
import (
"strings"
"testing"
)
// TestTrustedProxiesConfig checks the trusted_proxies key wiring: an
// explicit CIDR list lands in TrustedProxies in order and replaces the
// default, an omitted key falls back to the RFC 1918 private ranges, and an
// explicitly empty list 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("explicit list replaces the default 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 defaults to the RFC 1918 ranges", func(t *testing.T) {
t.Parallel()
c, err := configFromYAML(t, signingKeyLine)
if err != nil {
t.Fatalf("minimal config should load: %v", err)
}
got := make([]string, len(c.TrustedProxies))
for i, p := range c.TrustedProxies {
got[i] = p.String()
}
want := "10.0.0.0/8,172.16.0.0/12,192.168.0.0/16"
if joined := strings.Join(got, ","); joined != want {
t.Errorf("TrustedProxies = %v, want the RFC 1918 ranges %q", got, want)
}
})
t.Run("explicitly empty list trusts no one", func(t *testing.T) {
t.Parallel()
c, err := configFromYAML(t, signingKeyLine+"trusted_proxies: []\n")
if err != nil {
t.Fatalf("empty trusted_proxies 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())