From 930e43961eb36ab14d7a54b549967405342c372c Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Mon, 19 Oct 2026 05:29:05 +0200 Subject: [PATCH] Read a trusted_proxies entry in the IPv4-mapped form as its IPv4 range (closes #303) An entry such as ::ffff:10.0.0.0/104 in trusted_proxies was accepted but trusted nothing, because the client address is matched as the IPv4 address an IPv4-mapped address holds. The conversion that blocked_networks gained in https://git.eeqj.de/sneak/pixa/issues/301 now lives in parseCIDRList, which reads both settings: such an entry is read as the IPv4 range it covers, and one shorter than /96 stops startup with an error naming the setting being read and quoting the entry as written. README.md says so for trusted_proxies. Model: opus-5-5 --- README.md | 26 ++++++---- TODO.md | 4 ++ internal/config/config.go | 52 +++++++------------ .../config/trusted_proxies_internal_test.go | 38 ++++++++++++++ 4 files changed, 78 insertions(+), 42 deletions(-) diff --git a/README.md b/README.md index b473c1a..9eae101 100644 --- a/README.md +++ b/README.md @@ -713,16 +713,22 @@ Key settings in more detail: 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 the address pixa sees for requests that come through your proxy, such as - `172.17.0.1/32`, when the defaults do not cover it, or to trust nothing else - (see the login limit and `image_requests_per_minute` under Routes). For a - proxy on the Docker host that connects to pixa over `127.0.0.1`, that address - is the gateway of the container's Docker network (`172.17.0.1` on the default - bridge), not the proxy's own address; a proxy that connects through another of - the host's addresses is seen with that address. To be sure which address it - is, set this to `[]` (or `PIXA_TRUSTED_PROXIES` to empty), send a request - through the proxy, and read `remoteIP` in pixa's request log line for it + explicit list replaces the default. An invalid CIDR aborts startup. An IPv6 + address in the IPv4-mapped form (`::ffff:a.b.c.d`) is matched only as the IPv4 + address it holds, so an IPv6 range such as `::/0` trusts neither it nor a + plain IPv4 address. An entry written in that form is read as the IPv4 range it + covers, so `::ffff:10.0.0.0/104` trusts what `10.0.0.0/8` trusts; such an + entry shorter than `/96` covers more than IPv4 addresses and aborts startup. + Set this to the address pixa sees for requests that come through your proxy, + such as `172.17.0.1/32`, when the defaults do not cover it, or to trust + nothing else (see the login limit and `image_requests_per_minute` under + Routes). For a proxy on the Docker host that connects to pixa over + `127.0.0.1`, that address is the gateway of the container's Docker network + (`172.17.0.1` on the default bridge), not the proxy's own address; a proxy + that connects through another of the host's addresses is seen with that + address. To be sure which address it is, set this to `[]` (or + `PIXA_TRUSTED_PROXIES` to empty), send a request through the proxy, and read + `remoteIP` in pixa's request log line for it - `upstream_fetch_timeout` — time allowed for one fetch from an upstream host, as a duration such as `30s` (the default) or `2m` - `upstream_max_response_size` — largest upstream response accepted, in bytes; diff --git a/TODO.md b/TODO.md index 44ae4c1..284d19d 100644 --- a/TODO.md +++ b/TODO.md @@ -30,6 +30,10 @@ P2: security: per-origin rate limiting # Completed Steps +- 2026-10-19 a `trusted_proxies` entry in the IPv4-mapped form trusted nothing; + it is now read as the IPv4 range it covers by the same code that reads + `blocked_networks`, and such an entry shorter than `/96` aborts startup + (closes #303). - 2026-10-18 the next reconciliation pass waits at least ten times as long as the last one took, so passes on a large cache no longer run back to back (closes #250). diff --git a/internal/config/config.go b/internal/config/config.go index 4a7bd72..eff51ce 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -272,7 +272,7 @@ func newFromSmartConfig(sc *smartconfig.Config) (*Config, error) { return nil, err } - blockedNetworks, err := parseBlockedNetworks(sc) + blockedNetworks, err := parseCIDRList(sc, keyBlockedNetworks) if err != nil { return nil, err } @@ -1249,6 +1249,13 @@ func defaultTrustedProxies() []netip.Prefix { // 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. +// +// An entry written in the IPv4-mapped form (::ffff:a.b.c.d) is kept as the +// IPv4 range it covers: ::ffff:10.0.0.0/104 is kept as 10.0.0.0/8. Both +// blocked_networks and trusted_proxies are matched against an IPv4-mapped +// address as the IPv4 address it holds, so the entry as written would match +// nothing. Such an entry shorter than /96 covers more than the IPv4 +// addresses and aborts startup. func parseCIDRList(sc *smartconfig.Config, key string) ([]netip.Prefix, error) { raw, ok := lookupValue(sc, key) if !ok { @@ -1264,6 +1271,10 @@ func parseCIDRList(sc *smartconfig.Config, key string) ([]netip.Prefix, error) { return nil, err } + // An IPv4-mapped address is the 96 bits of ::ffff:0:0/96 followed by + // the 32 bits of the IPv4 address it holds. + const mappedPrefixBits = 96 + prefixes := make([]netip.Prefix, 0, len(entries)) for _, entry := range entries { @@ -1273,40 +1284,17 @@ func parseCIDRList(sc *smartconfig.Config, key string) ([]netip.Prefix, error) { settingName(key), entry, errNotAValidCIDR) } - prefixes = append(prefixes, prefix) - } - - return prefixes, nil -} - -// parseBlockedNetworks parses blocked_networks with parseCIDRList, and keeps -// an entry written in the IPv4-mapped form (::ffff:a.b.c.d) as the IPv4 -// range it covers: ::ffff:10.0.0.0/104 is kept as 10.0.0.0/8. The fetcher -// matches an IPv4-mapped address as the IPv4 address it holds, so the entry -// as written would match nothing. Such an entry shorter than /96 covers more -// than the IPv4 addresses and aborts startup. -func parseBlockedNetworks(sc *smartconfig.Config) ([]netip.Prefix, error) { - prefixes, err := parseCIDRList(sc, keyBlockedNetworks) - if err != nil { - return nil, err - } - - // An IPv4-mapped address is the 96 bits of ::ffff:0:0/96 followed by - // the 32 bits of the IPv4 address it holds. - const mappedPrefixBits = 96 - - for i, prefix := range prefixes { - if !prefix.Addr().Is4In6() { - continue - } - - if prefix.Bits() < mappedPrefixBits { + if prefix.Addr().Is4In6() && prefix.Bits() < mappedPrefixBits { return nil, fmt.Errorf("%s: value %q is %w", - settingName(keyBlockedNetworks), prefix, errMappedTooShort) + settingName(key), entry, errMappedTooShort) } - prefixes[i] = netip.PrefixFrom(prefix.Addr().Unmap(), - prefix.Bits()-mappedPrefixBits) + if prefix.Addr().Is4In6() { + prefix = netip.PrefixFrom(prefix.Addr().Unmap(), + prefix.Bits()-mappedPrefixBits) + } + + prefixes = append(prefixes, prefix) } return prefixes, nil diff --git a/internal/config/trusted_proxies_internal_test.go b/internal/config/trusted_proxies_internal_test.go index ae61bfd..41a184d 100644 --- a/internal/config/trusted_proxies_internal_test.go +++ b/internal/config/trusted_proxies_internal_test.go @@ -83,3 +83,41 @@ func TestTrustedProxiesInvalidAbortsStartup(t *testing.T) { }, }) } + +// TestTrustedProxiesMappedEntryIsIPv4Range checks that an entry written in +// the IPv4-mapped form (::ffff:a.b.c.d) is kept as the IPv4 range it covers, +// since the client address is matched as the IPv4 address it holds. /96, the +// shortest such entry, covers every IPv4 address. +func TestTrustedProxiesMappedEntryIsIPv4Range(t *testing.T) { + t.Parallel() + + c, err := configFromYAML(t, signingKeyLine+ + `trusted_proxies: ["::ffff:10.0.0.0/104", "::ffff:0:0/96"]`+"\n") + if err != nil { + t.Fatalf("IPv4-mapped trusted_proxies entries 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,0.0.0.0/0" { + t.Errorf("TrustedProxies = %v, want 10.0.0.0/8 and 0.0.0.0/0", got) + } +} + +// TestTrustedProxiesMappedEntryShorterThan96AbortsStartup checks that an +// entry in the IPv4-mapped form shorter than /96, which covers more than the +// IPv4 addresses, aborts startup with an error naming the key and the value. +func TestTrustedProxiesMappedEntryShorterThan96AbortsStartup(t *testing.T) { + t.Parallel() + + runAbortCases(t, []abortCase{ + { + name: "mapped-95", + yaml: signingKeyLine + `trusted_proxies: ["::ffff:0:0/95"]` + "\n", + wantErrSubstrings: []string{keyTrustedProxies, "::ffff:0:0/95"}, + }, + }) +}