From 0ab15324f7d0d82a97fd482c5408fce5bb317014 Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Mon, 19 Oct 2026 03:00:43 +0200 Subject: [PATCH] Read a blocked_networks entry in the IPv4-mapped form as its IPv4 range (closes #301) The fetcher matches an IPv4-mapped address (::ffff:a.b.c.d) as the IPv4 address it holds, so a blocked_networks entry written in that form was accepted and matched nothing. Such an entry is now kept as the IPv4 range it covers: ::ffff:10.0.0.0/104 becomes 10.0.0.0/8, and ::ffff:0:0/96 becomes 0.0.0.0/0. One shorter than /96 covers more than IPv4 addresses and aborts startup naming the setting. README.md says both. trusted_proxies shares the list parser, has the same fault and is unchanged here (issue 303). Model: opus-5-5 --- README.md | 4 +- TODO.md | 3 ++ .../config/blocked_networks_internal_test.go | 53 +++++++++++++++++++ internal/config/config.go | 36 ++++++++++++- 4 files changed, 94 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 6338c40..c069b90 100644 --- a/README.md +++ b/README.md @@ -697,7 +697,9 @@ Key settings in more detail: benchmark, NAT64, and the like); 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` refuses neither it nor a - plain IPv4 address + plain IPv4 address. An entry written in that form is read as the IPv4 range it + covers, so `::ffff:10.0.0.0/104` refuses what `10.0.0.0/8` refuses; such an + entry shorter than `/96` covers more than IPv4 addresses and 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 diff --git a/TODO.md b/TODO.md index 596940e..06e7082 100644 --- a/TODO.md +++ b/TODO.md @@ -30,6 +30,9 @@ P2: security: per-origin rate limiting # Completed Steps +- 2026-10-18 a `blocked_networks` entry in the IPv4-mapped form refused nothing; + `::ffff:10.0.0.0/104` is now read as `10.0.0.0/8`, and such an entry shorter + than `/96` aborts startup (closes #301). - 2026-10-16 P1 security bug: the check that keeps pixa from fetching internal addresses refuses the local-use NAT64 range (`64:ff9b:1::/48`, RFC 8215) as it refuses `64:ff9b::/96`, and refuses an IPv6 address in the IPv4-compatible diff --git a/internal/config/blocked_networks_internal_test.go b/internal/config/blocked_networks_internal_test.go index 25373be..475b1d4 100644 --- a/internal/config/blocked_networks_internal_test.go +++ b/internal/config/blocked_networks_internal_test.go @@ -88,3 +88,56 @@ func TestBlockedNetworksInvalidAbortsStartup(t *testing.T) { }, }) } + +// TestBlockedNetworksMappedEntryIsIPv4Range 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 fetcher matches an IPv4-mapped address as the IPv4 address it +// holds. /96, the shortest such entry, covers every IPv4 address. +func TestBlockedNetworksMappedEntryIsIPv4Range(t *testing.T) { + t.Parallel() + + yamlContent := signingKeyLine + `blocked_networks: + - "::ffff:10.0.0.0/104" + - "::ffff:0:0/96" +` + + c, err := configFromYAML(t, yamlContent) + if err != nil { + t.Fatalf("IPv4-mapped blocked_networks entries should load, got error: %v", err) + } + + want := []string{"10.0.0.0/8", "0.0.0.0/0"} + if len(c.BlockedNetworks) != len(want) { + t.Fatalf("BlockedNetworks = %v, want %d entries", c.BlockedNetworks, len(want)) + } + + for i, w := range want { + if got := c.BlockedNetworks[i].String(); got != w { + t.Errorf("BlockedNetworks[%d] = %q, want %q", i, got, w) + } + } +} + +// TestBlockedNetworksMappedEntryShorterThan96AbortsStartup 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 TestBlockedNetworksMappedEntryShorterThan96AbortsStartup(t *testing.T) { + t.Parallel() + + runAbortCases(t, []abortCase{ + { + name: "mapped-shorter-than-96", + yaml: signingKeyLine + `blocked_networks: + - "::ffff:0.0.0.0/80" +`, + wantErrSubstrings: []string{keyBlockedNetworks, "::ffff:0.0.0.0/80"}, + }, + { + name: "mapped-95", + yaml: signingKeyLine + `blocked_networks: + - "::ffff:0:0/95" +`, + wantErrSubstrings: []string{keyBlockedNetworks}, + }, + }) +} diff --git a/internal/config/config.go b/internal/config/config.go index 40eb799..4a7bd72 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -91,6 +91,7 @@ var ( errNotABoolean = errors.New("not a boolean") errNotAStringList = errors.New("not a list of strings") errNotAValidCIDR = errors.New("not a valid CIDR network") + errMappedTooShort = errors.New("in the IPv4-mapped form but shorter than /96") errNotAMetricsMap = errors.New("not a map of metrics settings") errEmptyListEntry = errors.New("list contains an empty entry") errEmptyEntry = errors.New("contains an empty entry") @@ -271,7 +272,7 @@ func newFromSmartConfig(sc *smartconfig.Config) (*Config, error) { return nil, err } - blockedNetworks, err := parseCIDRList(sc, keyBlockedNetworks) + blockedNetworks, err := parseBlockedNetworks(sc) if err != nil { return nil, err } @@ -1278,6 +1279,39 @@ func parseCIDRList(sc *smartconfig.Config, key string) ([]netip.Prefix, error) { 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 { + return nil, fmt.Errorf("%s: value %q is %w", + settingName(keyBlockedNetworks), prefix, errMappedTooShort) + } + + prefixes[i] = netip.PrefixFrom(prefix.Addr().Unmap(), + prefix.Bits()-mappedPrefixBits) + } + + return prefixes, nil +} + // parseHostList parses the value of the named config key into host patterns, // 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