sneak/webhooker
1
0
Fork 0
Code Issues 34 Pull Requests 1 Actions Packages Projects Releases Wiki Activity

Bucket IPv6 rate-limit keys by /64 (closes #125)
All checks were successful
check / check (push) Successful in 3m5s
Details

Browse Source 
Rate-limit keys were per-address, i.e. per /128 for IPv6. A routed /64
is the normal residential and mobile allocation, so a client rotated
source addresses inside its own prefix and minted a fresh bucket per
request — evading every limiter at the network layer, with no spoofing
and nothing to detect. #88 closed the header half of this control; this
is the network half.

IPv6 now keys on the /64, IPv4 on the full address, via stdlib
net/netip. IPv4-mapped form is unmapped rather than masked, so clients
behind a mapping proxy do not collapse into one bucket.

Independently reviewed twice. The first round found the trusted-proxy
forwarded path — the one carrying production traffic — had no coverage
at all, so a silent revert there was undetectable; that is now pinned.
The reviewer confirmed both branches are independently mutation-tested:
reverting either the direct-peer return or the forwarded return alone
fails only that branch's tests. The 18-site test-constant refactor was
verified byte-identical against next, with no pre-existing assertion
changed.

Known remaining coverage gap, judged not a defect: the fallback when the
peer is trusted but the forwarded address does not parse has no test.
Only operator-controlled addresses inside TRUSTED_PROXIES reach it, they
already share the proxy's single bucket, and masking there can only
merge operator proxies — fail-closed, nothing attacker-controlled.
This commit was merged in pull request #162.
This commit is contained in:
clawbot
2026-08-17 23:52:15 +02:00
parent 39064a3d6c
commit c3b6623be1
3 changed files with 463 additions and 20 deletions
Download Patch File Download Diff File Expand all files Collapse all files

59
internal/middleware/ratelimit.go
View File

@@ -48,6 +48,12 @@ const (
// bound every request pays a walk proportional to whatever the
// client sent.
maxForwardedHops = 64
// ipv6BucketBits is the prefix length IPv6 clients are bucketed
// on. A routed /64 is the normal residential and mobile
// allocation, so it is the unit an attacker gets addresses in
// and therefore the unit worth limiting.
ipv6BucketBits = 64
)
// normalizeAddr strips the IPv4-in-IPv6 wrapper and any zone from
@@ -56,6 +62,40 @@ func normalizeAddr(addr netip.Addr) netip.Addr {
return addr.Unmap().WithZone("")
}
// bucketKey is the rate-limit bucket identity of a client address.
// IPv4 keys on the full address; IPv6 keys on its /64 prefix,
// because keying IPv6 per /128 lets one ordinary subscriber rotate
// source addresses inside its own routed /64 and mint a fresh bucket
// per request — evading every limiter here at the network layer,
// with no spoofing and nothing to detect.
//
// An IPv4-mapped address (::ffff:1.2.3.4) is keyed as the IPv4
// address it carries, never masked to a /64: mapped form all shares
// the ::ffff:0:0/96 prefix, so masking would collapse every IPv4
// client reaching a proxy that emits it into one bucket. Callers
// pass addresses through normalizeAddr, which already unmaps; the
// unmap here keeps the property true of the key function itself.
//
// The two families cannot collide: an IPv4 key is a bare dotted
// quad, and an IPv6 key always carries a "/64" suffix.
func bucketKey(addr netip.Addr) string {
addr = addr.Unmap()
if addr.Is4() {
return addr.String()
}
// Prefix errors only on a negative bit count, on over 32 bits
// for an IPv4 address, or on over 128 for IPv6. The count here
// is the constant 64 and the IPv4 case returned above, so the
// error is unreachable. (The zero Addr does not error either: it
// yields the zero Prefix. Neither call site can produce one,
// since both parse the address first.)
prefix, _ := addr.Prefix(ipv6BucketBits)
return prefix.String()
}
// isTrustedProxy reports whether addr belongs to a network the
// operator listed in TRUSTED_PROXIES. The list is empty by default,
// so by default nothing is trusted.
@@ -143,6 +183,9 @@ func (m *Middleware) forwardedClientAddr(
// another client's bucket, by picking an X-Forwarded-For value —
// which makes every limit here decorative against a deliberate
// attacker.
//
// The address that identifies the client is then reduced to a bucket
// by bucketKey: full address for IPv4, /64 prefix for IPv6.
func (m *Middleware) rateLimitKey(r *http.Request) (string, error) {
return m.clientKey(r), nil
}
@@ -152,23 +195,25 @@ func (m *Middleware) clientKey(r *http.Request) string {
peer, err := netip.ParseAddr(ipFromHostPort(r.RemoteAddr))
if err != nil {
// Not an address we can reason about; key on the raw
// value, the most specific identity left. On a
// Unix-socket listener every peer carries the same
// RemoteAddr and so shares one bucket, which is the
// fail-closed direction.
// value, the most specific identity left. Distinct
// RemoteAddr values stay in distinct buckets, so this
// path cannot silently collapse unrelated clients
// together. On a Unix-socket listener every peer
// carries the same RemoteAddr and so shares one bucket,
// which is the fail-closed direction.
return r.RemoteAddr
}
peer = normalizeAddr(peer)
if !m.isTrustedProxy(peer) {
return peer.String()
return bucketKey(peer)
}
if addr, ok := m.forwardedClientAddr(r); ok {
return addr.String()
return bucketKey(addr)
}
return peer.String()
return bucketKey(peer)
}
// tooManyRequests returns the 429 handler used by the login,

415
internal/middleware/ratelimit_test.go
View File

@@ -15,6 +15,7 @@ import (
"time"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"sneak.berlin/go/webhooker/internal/config"
"sneak.berlin/go/webhooker/internal/middleware"
)
@@ -370,6 +371,30 @@ const (
headerXFF = "X-Forwarded-For"
headerReal = "X-Real-IP"
headerTrue = "True-Client-IP"
// clientIPv4 is the sample IPv4 client address these tests key
// on, both directly and in IPv4-mapped form. clientIPv4Alt is
// its neighbour, used to show the two do not share a bucket.
clientIPv4 = "198.51.100.7"
clientIPv4Alt = "198.51.100.8"
// clientIPv6 and clientIPv6Same are two addresses inside one
// routed /64, so both must key on clientBucketV6.
// clientIPv6Other is a different allocation and must key on
// clientOtherBucketV6.
clientIPv6 = "2001:db8:1:2:3:4:5:6"
clientIPv6Same = "2001:db8:1:2:aaaa:bbbb:cccc:dddd"
clientIPv6Other = "2001:db8:1:3::1"
clientBucketV6 = "2001:db8:1:2::/64"
clientOtherBucketV6 = "2001:db8:1:3::/64"
// trustedProxyCIDR is the proxy network the forwarded-path
// tests configure, and trustedPeer an address inside it. A
// production deployment is required to run behind a reverse
// proxy with TRUSTED_PROXIES set, so this is the shape the
// bucketing has to hold in.
trustedProxyCIDR = "10.0.0.0/8"
trustedPeer = "10.0.0.1:44444"
)
// assertSharedBucket drives the login limiter from peer with the
@@ -458,8 +483,8 @@ func TestRateLimitKey_SingleValuedHeadersIgnoredFromTrustedPeer(
t.Parallel()
assertSharedBucket(
t, trustedProxies("10.0.0.0/8"),
"10.0.0.1:44444",
t, trustedProxies(trustedProxyCIDR),
trustedPeer,
func(i int) map[string]string {
return map[string]string{
header: fmt.Sprintf(
@@ -495,8 +520,8 @@ func TestRateLimitKey_MalformedRightmostHopFallsBackToPeer(
t.Parallel()
assertSharedBucket(
t, trustedProxies("10.0.0.0/8"),
"10.0.0.1:44444",
t, trustedProxies(trustedProxyCIDR),
trustedPeer,
func(i int) map[string]string {
return map[string]string{
headerXFF: fmt.Sprintf(
@@ -522,13 +547,13 @@ func TestRateLimitKey_ForwardedHonouredFromTrustedPeer(
t.Parallel()
m := rateLimitMiddleware(t, &config.Config{
TrustedProxies: trustedProxies("10.0.0.0/8"),
TrustedProxies: trustedProxies(trustedProxyCIDR),
})
handler := m.LoginRateLimit()(okHandler())
const peer = "10.0.0.1:44444"
const peer = trustedPeer
first := map[string]string{headerXFF: "198.51.100.7"}
first := map[string]string{headerXFF: clientIPv4}
for range middleware.LoginRateLimitConst {
postWithHeaders(handler, peer, loginPath, first)
@@ -542,7 +567,7 @@ func TestRateLimitKey_ForwardedHonouredFromTrustedPeer(
w = postWithHeaders(
handler, peer, loginPath,
map[string]string{headerXFF: "198.51.100.8"},
map[string]string{headerXFF: clientIPv4Alt},
)
assert.Equal(
t, http.StatusOK, w.Code,
@@ -559,7 +584,7 @@ func TestRateLimitKey_ChainWalkSkipsClientPrepended(t *testing.T) {
t.Parallel()
assertSharedBucket(
t, trustedProxies("10.0.0.0/8"), "10.0.0.1:44444",
t, trustedProxies(trustedProxyCIDR), trustedPeer,
func(i int) map[string]string {
return map[string]string{
headerXFF: fmt.Sprintf(
@@ -594,7 +619,7 @@ func TestRateLimitKey_LongChainCapsWalkAndFallsBackToPeer(
start := time.Now()
assertSharedBucket(
t, trustedProxies("10.0.0.0/8"), "10.0.0.1:44444",
t, trustedProxies(trustedProxyCIDR), trustedPeer,
func(i int) map[string]string {
return map[string]string{
headerXFF: fmt.Sprintf("9.9.9.%d%s", i+1, padding),
@@ -633,13 +658,13 @@ func TestRateLimitKey_LongChainAllocationIsBounded(t *testing.T) {
)
m := rateLimitMiddleware(t, &config.Config{
TrustedProxies: trustedProxies("10.0.0.0/8"),
TrustedProxies: trustedProxies(trustedProxyCIDR),
})
req := httptest.NewRequestWithContext(
context.Background(), http.MethodPost, loginPath, nil,
)
req.RemoteAddr = "10.0.0.1:44444"
req.RemoteAddr = trustedPeer
req.Header.Set(
headerXFF, "9.9.9.9"+strings.Repeat(", 10.0.0.2", hops),
)
@@ -835,3 +860,369 @@ func TestReceiverRateLimit_IgnoresForwardedFromUntrustedPeer(
"not mint a fresh receiver bucket",
)
}
// clientKeyFor returns the bucket key m computes for a request whose
// direct peer is remoteAddr and which carries no forwarded headers.
func clientKeyFor(
t *testing.T, m *middleware.Middleware, remoteAddr string,
) string {
t.Helper()
req := httptest.NewRequestWithContext(
context.Background(), http.MethodPost, loginPath, nil,
)
req.RemoteAddr = remoteAddr
return middleware.ClientKeyForTest(m, req)
}
// TestRateLimitKey_IPv6BucketsByPrefix pins the key function's
// address-family behaviour. IPv6 clients must bucket by /64 — a
// routed /64 is the normal residential and mobile allocation, so
// per-/128 keying lets one subscriber rotate source addresses and
// mint a fresh bucket per request — while IPv4 keeps keying on the
// full address and IPv4-mapped form is keyed as the IPv4 address it
// carries.
func TestRateLimitKey_IPv6BucketsByPrefix(t *testing.T) {
t.Parallel()
m := rateLimitMiddleware(t, &config.Config{})
for _, tc := range []struct {
name string
peer string
want string
about string
}{{
name: "ipv6",
peer: "[" + clientIPv6 + "]:44444",
want: clientBucketV6,
about: "an IPv6 peer must key on its /64",
}, {
name: "ipv6-other-in-same-64",
peer: "[" + clientIPv6Same + "]:1",
want: clientBucketV6,
about: "another address in the same /64 must key the same",
}, {
name: "ipv6-different-64",
peer: "[" + clientIPv6Other + "]:44444",
want: clientOtherBucketV6,
about: "a different /64 must key differently",
}, {
name: "ipv4",
peer: clientIPv4 + ":44444",
want: clientIPv4,
about: "IPv4 must keep keying on the full address",
}, {
name: "ipv4-neighbour",
peer: clientIPv4Alt + ":44444",
want: clientIPv4Alt,
about: "adjacent IPv4 addresses must not share a bucket",
}, {
name: "ipv4-mapped",
peer: "[::ffff:" + clientIPv4 + "]:44444",
want: clientIPv4,
about: "IPv4-mapped form must key as the IPv4 address, " +
"not be masked to a /64: mapped addresses all share " +
"::ffff:0:0/96, so masking would collapse every IPv4 " +
"client behind a mapping proxy into one bucket",
}} {
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
assert.Equal(
t, tc.want, clientKeyFor(t, m, tc.peer), tc.about,
)
})
}
}
// TestRateLimitKey_FamiliesDoNotCollide pins the structure the
// no-collision property rests on, rather than one sample pair: every
// IPv4 key is a bare address and every IPv6 key is a /64 in CIDR
// form, so the two name spaces are disjoint by shape. Dropping the
// masking strips the suffix that guarantees it, which is why this
// asserts the form of each key and not just that two of them differ.
func TestRateLimitKey_FamiliesDoNotCollide(t *testing.T) {
t.Parallel()
// Restated here rather than imported from the package under
// test, so that changing the production bucket width fails this
// test instead of silently moving with it.
const wantBits = 64
m := rateLimitMiddleware(t, &config.Config{})
v4Keys := map[string]bool{}
for _, peer := range []string{
clientIPv4 + ":44444",
clientIPv4Alt + ":44444",
"[::ffff:" + clientIPv4 + "]:44444",
} {
key := clientKeyFor(t, m, peer)
addr, err := netip.ParseAddr(key)
require.NoError(
t, err, "%s: an IPv4 key must be a bare address", peer,
)
assert.True(
t, addr.Is4(),
"%s: an IPv4 key must be a dotted quad, got %q", peer, key,
)
v4Keys[key] = true
}
for _, peer := range []string{
"[" + clientIPv6 + "]:44444",
"[" + clientIPv6Same + "]:44444",
"[" + clientIPv6Other + "]:44444",
"[2001:db8::" + clientIPv4 + "]:44444",
} {
key := clientKeyFor(t, m, peer)
prefix, err := netip.ParsePrefix(key)
require.NoError(
t, err, "%s: an IPv6 key must be a CIDR prefix", peer,
)
assert.Equal(
t, wantBits, prefix.Bits(),
"%s: an IPv6 key must name a /64", peer,
)
assert.False(
t, v4Keys[key],
"%s: an IPv6 key must never equal an IPv4 key", peer,
)
}
}
// TestRateLimitKey_UnparseablePeerKeepsDistinctBuckets covers the
// fallback path. A RemoteAddr that is not an address must not panic,
// and must not drop unrelated clients into one shared bucket by
// accident: the raw value is the most specific identity left, so
// distinct values stay in distinct buckets.
func TestRateLimitKey_UnparseablePeerKeepsDistinctBuckets(
t *testing.T,
) {
t.Parallel()
m := rateLimitMiddleware(t, &config.Config{})
first := clientKeyFor(t, m, "not-an-address")
second := clientKeyFor(t, m, "also-not-an-address:1234")
assert.NotEmpty(t, first)
assert.NotEqual(
t, first, second,
"unparseable peers must not collapse into one bucket",
)
}
// TestLoginRateLimit_IPv6SharesBucketWithinSlash64 is the behavioural
// half, and the regression test for the bypass itself: a client that
// rotates source addresses inside its own routed /64 must stay in one
// bucket. Reverting the masking makes this test fail, because each
// rotated address would mint a fresh bucket and nothing would be
// rejected.
func TestLoginRateLimit_IPv6SharesBucketWithinSlash64(t *testing.T) {
t.Parallel()
m := rateLimitMiddleware(t, &config.Config{})
handler := m.LoginRateLimit()(okHandler())
for i := range middleware.LoginRateLimitConst {
w := postWithHeaders(
handler,
fmt.Sprintf("[2001:db8:1:2::%d]:44444", i+1),
loginPath, nil,
)
assert.Equal(
t, http.StatusOK, w.Code, "request %d should pass", i,
)
}
w := postWithHeaders(
handler, "[2001:db8:1:2::ffff]:44444", loginPath, nil,
)
assert.Equal(
t, http.StatusTooManyRequests, w.Code,
"rotating source addresses inside one routed /64 must not "+
"mint fresh buckets",
)
}
// TestLoginRateLimit_IPv6IndependentAcrossSlash64 is the other side
// of the trade: bucketing by /64 must not merge separate allocations,
// so a client in a different /64 keeps its own limit.
func TestLoginRateLimit_IPv6IndependentAcrossSlash64(t *testing.T) {
t.Parallel()
m := rateLimitMiddleware(t, &config.Config{})
handler := m.LoginRateLimit()(okHandler())
for range middleware.LoginRateLimitConst + 1 {
postWithHeaders(
handler, "[2001:db8:1:2::1]:44444", loginPath, nil,
)
}
w := postWithHeaders(
handler, "[2001:db8:1:3::1]:44444", loginPath, nil,
)
assert.Equal(
t, http.StatusOK, w.Code,
"a different /64 must have its own bucket",
)
}
// TestLoginRateLimit_IPv4IndependentPerAddress guards against the
// masking leaking into IPv4: two addresses one apart must still hold
// separate buckets.
func TestLoginRateLimit_IPv4IndependentPerAddress(t *testing.T) {
t.Parallel()
m := rateLimitMiddleware(t, &config.Config{})
handler := m.LoginRateLimit()(okHandler())
for range middleware.LoginRateLimitConst + 1 {
postWithHeaders(
handler, clientIPv4+":44444", loginPath, nil,
)
}
w := postWithHeaders(
handler, clientIPv4Alt+":44444", loginPath, nil,
)
assert.Equal(
t, http.StatusOK, w.Code,
"a second IPv4 address must have its own bucket",
)
}
// forwardedKeyFor returns the bucket key m computes for a request
// that arrives from trustedPeer — a configured trusted proxy — and
// names forwarded as its client in X-Forwarded-For. That is the
// production path: a deployment is required to run behind a reverse
// proxy with TRUSTED_PROXIES set, so the forwarded address, not the
// peer, is what the limiters bucket on there.
func forwardedKeyFor(
t *testing.T, m *middleware.Middleware, forwarded string,
) string {
t.Helper()
req := httptest.NewRequestWithContext(
context.Background(), http.MethodPost, loginPath, nil,
)
req.RemoteAddr = trustedPeer
req.Header.Set(headerXFF, forwarded)
return middleware.ClientKeyForTest(m, req)
}
// TestRateLimitKey_ForwardedIPv6BucketsByPrefix pins the /64
// bucketing on the trusted-proxy branch. The direct-peer tests above
// cannot reach it, so without this the masking could be reverted for
// forwarded clients alone — the only shape a production deployment
// runs in — and the rest of the suite would stay green.
func TestRateLimitKey_ForwardedIPv6BucketsByPrefix(t *testing.T) {
t.Parallel()
m := rateLimitMiddleware(t, &config.Config{
TrustedProxies: trustedProxies(trustedProxyCIDR),
})
for _, tc := range []struct {
name string
forwarded string
want string
about string
}{{
name: "ipv6",
forwarded: clientIPv6,
want: clientBucketV6,
about: "a forwarded IPv6 client must key on its /64",
}, {
name: "ipv6-other-in-same-64",
forwarded: clientIPv6Same,
want: clientBucketV6,
about: "another forwarded address in the same /64 must " +
"key the same",
}, {
name: "ipv6-different-64",
forwarded: clientIPv6Other,
want: clientOtherBucketV6,
about: "a forwarded address in another /64 must differ",
}, {
name: "ipv4",
forwarded: clientIPv4,
want: clientIPv4,
about: "a forwarded IPv4 client must key on the address",
}, {
name: "ipv4-mapped",
forwarded: "::ffff:" + clientIPv4,
want: clientIPv4,
about: "a proxy that forwards IPv4-mapped form must key as " +
"the IPv4 address it carries, not be masked to a /64: " +
"mapped addresses all share ::ffff:0:0/96",
}} {
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
assert.Equal(
t, tc.want,
forwardedKeyFor(t, m, tc.forwarded), tc.about,
)
})
}
}
// TestLoginRateLimit_ForwardedIPv6SharesBucketWithinSlash64 is the
// behavioural half on the production path: behind a trusted proxy, a
// client rotating source addresses inside its own routed /64 must
// stay in one bucket.
func TestLoginRateLimit_ForwardedIPv6SharesBucketWithinSlash64(
t *testing.T,
) {
t.Parallel()
assertSharedBucket(
t, trustedProxies(trustedProxyCIDR), trustedPeer,
func(i int) map[string]string {
return map[string]string{
headerXFF: fmt.Sprintf("2001:db8:1:2::%d", i+1),
}
},
"rotating forwarded source addresses inside one routed /64 "+
"must not mint fresh buckets",
)
}
// TestLoginRateLimit_ForwardedIPv6IndependentAcrossSlash64 is the
// other side of that trade on the same path: bucketing by /64 must
// not merge two allocations reaching the proxy.
func TestLoginRateLimit_ForwardedIPv6IndependentAcrossSlash64(
t *testing.T,
) {
t.Parallel()
m := rateLimitMiddleware(t, &config.Config{
TrustedProxies: trustedProxies(trustedProxyCIDR),
})
handler := m.LoginRateLimit()(okHandler())
spent := map[string]string{headerXFF: clientIPv6}
for range middleware.LoginRateLimitConst + 1 {
postWithHeaders(handler, trustedPeer, loginPath, spent)
}
w := postWithHeaders(
handler, trustedPeer, loginPath,
map[string]string{headerXFF: clientIPv6Other},
)
assert.Equal(
t, http.StatusOK, w.Code,
"a forwarded client in a different /64 must have its own "+
"bucket",
)
}