server: fail closed when a limiter shard is full

evictShard deleted arbitrary live buckets once a shard hit its key
cap. An attacker minting fresh keys (spoofed XFF, IPv6 /64 rotation)
could churn a shard until their own full bucket — or a victim's — was
evicted, resetting the counter and defeating the per-IP limit
entirely. Unknown keys are now denied while the shard is full;
expired buckets are still reclaimed first. Regression test pins the
fail-closed behavior and bucket survival for existing keys.
This commit is contained in:
lakshit verma 2026-08-24 00:42:04 +05:30
parent 0da168062c
commit c8f89f009e
No known key found for this signature in database
2 changed files with 44 additions and 18 deletions

View file

@ -41,7 +41,14 @@ func (l *Limiter) Allow(key string) (remaining int64, ok bool, retryAfter time.D
shard.mu.Lock()
defer shard.mu.Unlock()
if len(shard.buckets) >= l.maxBuckets && shard.buckets[key] == nil {
l.evictShard(shard, now)
l.trimShard(shard, now.Add(-l.window))
if len(shard.buckets) >= l.maxBuckets {
// Fail closed: evicting a live bucket would let an attacker
// mint fresh keys until their own (or a victim's) full bucket
// is dropped, resetting its counter and defeating the limit.
// Deny unknown keys while the shard is full instead.
return 0, false, l.window
}
}
hits := shard.buckets[key]
cutoff := now.Add(-l.window)
@ -73,23 +80,7 @@ func (l *Limiter) sweep() {
}
}
// evictShard makes room in a full shard: expired buckets first, then
// arbitrary victims if the attack is still filling the window.
func (l *Limiter) evictShard(shard *limShard, now time.Time) {
l.trimShard(shard, now.Add(-l.window))
for len(shard.buckets) >= l.maxBuckets {
victimized := false
for k := range shard.buckets {
delete(shard.buckets, k)
victimized = true
break
}
if !victimized {
break
}
}
}
// trimShard drops buckets whose hits have all expired past cutoff.
func (l *Limiter) trimShard(shard *limShard, cutoff time.Time) {
for k, hits := range shard.buckets {
j := 0

View file

@ -4,6 +4,7 @@ import (
"fmt"
"net/http/httptest"
"testing"
"time"
)
func TestClientKeyIgnoresBogusXFF(t *testing.T) {
@ -37,3 +38,37 @@ func TestLimiterBucketsBounded(t *testing.T) {
t.Errorf("buckets = %d, want <= %d", total, maxBucketsPerShard*64)
}
}
func TestLimiterFullShardFailsClosed(t *testing.T) {
l := &Limiter{limit: 10, window: time.Hour, maxBuckets: 1}
for i := range l.shards {
l.shards[i].buckets = make(map[string][]time.Time)
}
first := "client-a"
peer := ""
for i := 0; i < 10000 && peer == ""; i++ {
if k := fmt.Sprintf("k%d", i); shardIndex(k) == shardIndex(first) && k != first {
peer = k
}
}
if peer == "" {
t.Fatal("no key found hashing to the same shard")
}
if _, ok, _ := l.Allow(first); !ok {
t.Fatal("first key should be admitted")
}
// Shard is full (maxBuckets=1). The unknown peer key must be denied
// rather than evicting the live bucket, which would reset first's
// counter.
_, ok, retry := l.Allow(peer)
if ok {
t.Fatal("unknown key should be denied when shard is full")
}
if retry <= 0 {
t.Fatalf("retryAfter = %v; want > 0", retry)
}
if _, ok, _ := l.Allow(first); !ok {
t.Fatal("existing key's bucket must survive peer admission attempts")
}
}