Read a blocked_networks entry in the IPv4-mapped form as its IPv4 range (closes #301)
check / check (push) Canceled after 0s
check / check (push) Canceled after 0s
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
This commit was merged in pull request #302.
This commit is contained in:
@@ -697,7 +697,9 @@ Key settings in more detail:
|
|||||||
benchmark, NAT64, and the like); an invalid CIDR aborts startup. An IPv6
|
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 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
|
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
|
- `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
|
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
|
of these ranges; the logged and login-recorded client address is then the
|
||||||
|
|||||||
@@ -30,6 +30,9 @@ P2: security: per-origin rate limiting
|
|||||||
|
|
||||||
# Completed Steps
|
# 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
|
- 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
|
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
|
refuses `64:ff9b::/96`, and refuses an IPv6 address in the IPv4-compatible
|
||||||
|
|||||||
@@ -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},
|
||||||
|
},
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|||||||
@@ -91,6 +91,7 @@ var (
|
|||||||
errNotABoolean = errors.New("not a boolean")
|
errNotABoolean = errors.New("not a boolean")
|
||||||
errNotAStringList = errors.New("not a list of strings")
|
errNotAStringList = errors.New("not a list of strings")
|
||||||
errNotAValidCIDR = errors.New("not a valid CIDR network")
|
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")
|
errNotAMetricsMap = errors.New("not a map of metrics settings")
|
||||||
errEmptyListEntry = errors.New("list contains an empty entry")
|
errEmptyListEntry = errors.New("list contains an empty entry")
|
||||||
errEmptyEntry = errors.New("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
|
return nil, err
|
||||||
}
|
}
|
||||||
|
|
||||||
blockedNetworks, err := parseCIDRList(sc, keyBlockedNetworks)
|
blockedNetworks, err := parseBlockedNetworks(sc)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return nil, err
|
return nil, err
|
||||||
}
|
}
|
||||||
@@ -1278,6 +1279,39 @@ func parseCIDRList(sc *smartconfig.Config, key string) ([]netip.Prefix, error) {
|
|||||||
return prefixes, nil
|
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,
|
// 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
|
// 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
|
// comma-separated string. An explicitly null value, a wrong type, an empty
|
||||||
|
|||||||
Reference in New Issue
Block a user