Files
webhooker/internal/middleware/loginguard_test.go
clawbot 977fe87588
All checks were successful
check / check (push) Successful in 2m46s
Verify login credentials before spending rate-limit budget (closes #150)
In the shipped default, any stranger denied the operator the only
administrative path at 5 requests per minute: TRUSTED_PROXIES is empty,
the README requires a reverse proxy, so every login POST shared one
bucket keyed on the proxy.

Credentials are now verified first and only a FAILED attempt spends
budget, so a correct password is never throttled. Failures are counted
per (client bucket, submitted username), bounded. Concurrent Argon2id
verifications are capped at two, and the queue for them at 16 — because
verifying first lets an attacker force a 64 MB hash per request, and
bounding the wait alone bounds nothing.

The issue's own recommendation was insufficient and is rejected here:
keying by username stops an attacker locking out a DIFFERENT account,
but this is a single-admin product with a predictable bootstrap
username, so flooding the operator's own name still locks them out.

This is speculative — it implements a corrected recommendation ahead of
the owner's ruling so the decision can be made by merging or reverting.
Three things are disclosed rather than glossed: online guessing rises
from 5/min to roughly 27/s, because the 429 is a label on the response
and not a gate in front of the hash; the residual exposure is a loss of
login AVAILABILITY, not latency, and a determined flood still denies
login while it runs, at ~400x the cost and clearing the moment it
stops; and the endpoint should be provisioned for ~400 MB resident, not
the 203 MB of live commitment it itemises.

Independently reviewed four times. Reviewers disproved the suspected
FIFO starvation by measurement, then caught two successive memory
bounds the code did not have — the second by parking waiters and
reading the heap rather than checking the arithmetic.
2026-08-18 01:55:41 +02:00

512 lines
13 KiB
Go

package middleware_test
import (
"context"
"fmt"
"sync"
"sync/atomic"
"testing"
"time"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"sneak.berlin/go/webhooker/internal/database"
"sneak.berlin/go/webhooker/internal/middleware"
)
// mib converts the Argon2id memory parameter, which is in KiB, to MB.
const mib = 1024
const (
// guardInterval is the failure window these tests use. It is
// long enough that nothing lapses mid-test on its own; tests
// that need a lapse drive the clock instead.
guardInterval = time.Minute
// guardWait is the slot wait for tests that expect to get a
// slot. Tests that expect to be refused set their own.
guardWait = 2 * time.Second
guardClient = "198.51.100.7"
guardUser = "admin"
)
// newGuard builds a guard with production-shaped defaults and the
// given key-set cap and verification concurrency.
func newGuard(maxKeys, concurrency int) *middleware.LoginGuard {
return middleware.NewLoginGuardForTest(
middleware.LoginRateLimitConst,
guardInterval,
maxKeys,
concurrency,
middleware.PasswordVerifyMaxWaitersConst,
guardWait,
)
}
// TestLoginGuard_ThrottlesRepeatedFailures is the brute-force half:
// wrong passwords for one username from one client key still run out
// of budget and are answered 429.
func TestLoginGuard_ThrottlesRepeatedFailures(t *testing.T) {
t.Parallel()
g := newGuard(middleware.LoginFailureMaxKeysConst, 1)
for i := range middleware.LoginRateLimitConst - 1 {
assert.False(
t, g.FailForTest(guardClient, guardUser),
"failure %d is still inside the budget", i,
)
}
assert.True(
t, g.FailForTest(guardClient, guardUser),
"the last failure of the budget must throttle",
)
assert.True(
t, g.FailForTest(guardClient, guardUser),
"failures past the budget must stay throttled",
)
}
// TestLoginGuard_SuccessForgivesFailures pins the forgiveness rule:
// an operator who mistypes several times and then gets it right must
// not be left throttled.
func TestLoginGuard_SuccessForgivesFailures(t *testing.T) {
t.Parallel()
g := newGuard(middleware.LoginFailureMaxKeysConst, 1)
for range middleware.LoginRateLimitConst {
g.FailForTest(guardClient, guardUser)
}
g.SucceedForTest(guardClient, guardUser)
assert.False(
t, g.FailForTest(guardClient, guardUser),
"a success must reset the counter, so the next mistake "+
"starts a fresh budget",
)
}
// TestLoginGuard_FailuresAreKeyedPerUsername proves the second half
// of the keying: one username's spent budget does not throttle
// another's from the same client.
func TestLoginGuard_FailuresAreKeyedPerUsername(t *testing.T) {
t.Parallel()
g := newGuard(middleware.LoginFailureMaxKeysConst, 1)
for range middleware.LoginRateLimitConst {
g.FailForTest(guardClient, guardUser)
}
assert.True(t, g.FailForTest(guardClient, guardUser))
assert.False(
t, g.FailForTest(guardClient, "someone-else"),
"a different submitted username must have its own budget",
)
}
// TestLoginGuard_WindowLapses covers the interval: a counter that has
// gone quiet for the whole window starts again from zero.
func TestLoginGuard_WindowLapses(t *testing.T) {
t.Parallel()
g := newGuard(middleware.LoginFailureMaxKeysConst, 1)
var now atomic.Int64
now.Store(time.Now().UnixNano())
g.SetNowForTest(func() time.Time {
return time.Unix(0, now.Load())
})
for range middleware.LoginRateLimitConst {
g.FailForTest(guardClient, guardUser)
}
assert.True(t, g.FailForTest(guardClient, guardUser))
now.Add(int64(guardInterval) + 1)
assert.False(
t, g.FailForTest(guardClient, guardUser),
"a lapsed window must start a fresh budget",
)
}
// TestLoginGuard_UsernameKeySetIsBounded is the memory bound. The
// submitted username is attacker-controlled, so an attacker rotating
// usernames must not be able to grow the guard without limit: past
// the cap, tracking falls back to a counter keyed on the client
// address alone.
func TestLoginGuard_UsernameKeySetIsBounded(t *testing.T) {
t.Parallel()
const (
maxKeys = 8
attempts = 500
)
g := newGuard(maxKeys, 1)
for i := range attempts {
g.FailForTest(guardClient, fmt.Sprintf("user-%d", i))
}
byUser, byAddr := g.TrackedKeysForTest()
assert.LessOrEqual(
t, byUser, maxKeys,
"the per-username key set must not grow past its cap",
)
assert.LessOrEqual(
t, byAddr, maxKeys,
"the fallback key set must not grow past its cap either",
)
assert.Positive(
t, byAddr,
"past the cap, failures must fall back to the address "+
"bucket rather than being dropped",
)
assert.Less(
t, byUser+byAddr, attempts,
"memory must not grow with the number of distinct "+
"usernames submitted",
)
}
// TestLoginGuard_BeyondBothCapsStaysThrottled covers the hard stop.
// When both key sets are full of live counters and the client is in
// neither, there is nothing to count without unbounded growth, so the
// failure is answered as throttled. That costs the operator nothing:
// a correct password never reaches this path.
func TestLoginGuard_BeyondBothCapsStaysThrottled(t *testing.T) {
t.Parallel()
const maxKeys = 4
g := newGuard(maxKeys, 1)
// Fill the per-username set from one client, then fill the
// address set from distinct clients.
for i := range maxKeys {
g.FailForTest(guardClient, fmt.Sprintf("user-%d", i))
}
for i := range maxKeys {
g.FailForTest(fmt.Sprintf("203.0.113.%d", i), "whoever")
}
assert.True(
t, g.FailForTest("203.0.113.200", "brand-new"),
"a client that fits in neither full key set must be "+
"answered as throttled rather than tracked",
)
byUser, byAddr := g.TrackedKeysForTest()
assert.LessOrEqual(t, byUser, maxKeys)
assert.LessOrEqual(t, byAddr, maxKeys)
}
// TestLoginGuard_SemaphoreBoundsConcurrentVerifications is the memory
// bound on the hashing itself. Verifying credentials before spending
// limiter budget means an attacker can force one Argon2id hash per
// request, and each allocates 64 MB; without this bound the fix for
// an admin lockout would be a memory-exhaustion DoS instead.
func TestLoginGuard_SemaphoreBoundsConcurrentVerifications(
t *testing.T,
) {
t.Parallel()
const (
concurrency = 2
workers = 12
)
g := newGuard(middleware.LoginFailureMaxKeysConst, concurrency)
var (
mu sync.Mutex
inside int
highest int
wg sync.WaitGroup
)
for range workers {
wg.Go(func() {
release, ok := g.AcquireForTest(context.Background())
if !ok {
return
}
defer release()
mu.Lock()
inside++
if inside > highest {
highest = inside
}
mu.Unlock()
// Hold the slot long enough that the other workers are
// certainly contending for it.
time.Sleep(10 * time.Millisecond)
mu.Lock()
inside--
mu.Unlock()
})
}
wg.Wait()
mu.Lock()
defer mu.Unlock()
assert.Equal(
t, concurrency, highest,
"no more than %d verifications may run at once", concurrency,
)
}
// TestLoginGuard_SaturatedSemaphoreRefusesRatherThanQueueing pins
// what happens when every slot is taken for longer than the wait: the
// request is refused, so the caller answers 503 without allocating
// another 64 MB hash.
func TestLoginGuard_SaturatedSemaphoreRefusesRatherThanQueueing(
t *testing.T,
) {
t.Parallel()
g := middleware.NewLoginGuardForTest(
middleware.LoginRateLimitConst,
guardInterval,
middleware.LoginFailureMaxKeysConst,
1,
middleware.PasswordVerifyMaxWaitersConst,
10*time.Millisecond,
)
release, ok := g.AcquireForTest(context.Background())
require.True(t, ok, "the first acquire must get the only slot")
_, ok = g.AcquireForTest(context.Background())
assert.False(
t, ok,
"with the only slot held, a second request must be refused "+
"rather than wait indefinitely",
)
release()
release, ok = g.AcquireForTest(context.Background())
assert.True(
t, ok, "the slot must be reusable once released",
)
release()
}
// TestLoginGuard_AcquireHonoursCancellation proves a client that
// disconnects while queued frees its place immediately instead of
// holding it for the full wait.
func TestLoginGuard_AcquireHonoursCancellation(t *testing.T) {
t.Parallel()
g := newGuard(middleware.LoginFailureMaxKeysConst, 1)
release, ok := g.AcquireForTest(context.Background())
require.True(t, ok)
defer release()
ctx, cancel := context.WithCancel(context.Background())
cancel()
_, ok = g.AcquireForTest(ctx)
assert.False(
t, ok, "a cancelled request must not wait for a slot",
)
}
// TestPasswordVerifyConcurrency_MatchesMemoryBudget pins the
// concurrency constant to the arithmetic behind it: the number of
// slots is the hashing budget divided by what one Argon2id hash
// actually costs.
//
// The per-hash figure is read out of the shipped password
// parameters rather than copied here. A guard that asserts a literal
// against a literal cannot see the thing it guards: raising
// argon2Memory would leave it green while the real ceiling doubled.
func TestPasswordVerifyConcurrency_MatchesMemoryBudget(t *testing.T) {
t.Parallel()
// Memory is the real argon2Memory, in KiB.
perHashMB := int(database.DefaultPasswordConfig().Memory) / mib
require.Positive(
t, perHashMB,
"the Argon2id memory parameter must be readable in MB",
)
// The memory this service commits to password hashing.
const budgetMB = 128
assert.Equal(
t,
middleware.PasswordVerifyConcurrencyConst,
budgetMB/perHashMB,
"the verification concurrency must be the %d MB hashing "+
"budget divided by the %d MB one Argon2id hash costs; "+
"if the Argon2id parameters changed, the slot count "+
"must change with them",
budgetMB, perHashMB,
)
}
// TestLoginGuard_ShedsPastTheQueueCap pins the memory bound on
// waiting, as distinct from the bound on hashing. A waiter arrives
// with its form already parsed, and the retained parse plus its
// header block cost several MB — far more than maxFormBodySize
// suggests, since that caps only the raw body read — so an unbounded
// queue would hold that much per waiting request for the whole wait;
// past the cap the guard must refuse instantly rather than grow.
func TestLoginGuard_ShedsPastTheQueueCap(t *testing.T) {
t.Parallel()
const (
maxWaiters = 2
// Long enough that a queued waiter never times out on its
// own, so anything the test observes leaving the queue left
// because it was shed.
neverElapses = time.Minute
// The probe carries its own deadline, so a guard that queues
// the probe instead of shedding it fails on the elapsed time
// rather than hanging until the package test timeout.
probeWait = 200 * time.Millisecond
// Shedding takes no measurable time; queueing takes the whole
// probeWait. Anything under half of it is unambiguous.
shedFast = probeWait / 2
)
g := middleware.NewLoginGuardForTest(
middleware.LoginRateLimitConst,
guardInterval,
middleware.LoginFailureMaxKeysConst,
1,
maxWaiters,
neverElapses,
)
// Occupy the only slot, so everything after this queues.
release, ok := g.AcquireForTest(context.Background())
require.True(t, ok)
defer release()
defer fillQueue(t, g, maxWaiters)()
got := probeQueueCap(g, probeWait)
require.NotNil(
t, got,
"a request arriving past the queue cap is still waiting to "+
"be queued; it must have been shed",
)
assert.False(
t, got.ok,
"a request arriving past the queue cap must be shed",
)
assert.Less(
t, got.elapsed, shedFast,
"shedding must be immediate; waiting for a place in the "+
"queue is the memory growth this bounds",
)
assert.Equal(
t, maxWaiters, g.QueuedWaitersForTest(),
"a shed request must not have grown the queue",
)
}
// fillQueue starts n waiters on g and returns once all of them are
// queued for a slot. The returned function releases them and waits
// for them to exit.
func fillQueue(
t *testing.T,
g *middleware.LoginGuard,
n int,
) func() {
t.Helper()
ctx, cancel := context.WithCancel(context.Background())
var wg sync.WaitGroup
for range n {
wg.Go(func() {
done, got := g.AcquireForTest(ctx)
if got {
done()
}
})
}
require.Eventually(
t,
func() bool { return g.QueuedWaitersForTest() == n },
time.Second, time.Millisecond,
"the waiters must reach the queue before the cap is tested",
)
return func() {
cancel()
wg.Wait()
}
}
// probeResult is what the queue-cap probe reports: whether it got a
// slot, and how long it took to find out.
type probeResult struct {
ok bool
elapsed time.Duration
}
// probeQueueCap acquires from another goroutine and reports the
// result, or nil if the call was still blocked after wait.
//
// It runs off the test goroutine deliberately. Joining a full queue
// is not cancellable by context — refusing to join is the property
// under test — so a guard that fails this would otherwise hang the
// package until the test timeout instead of failing here.
func probeQueueCap(
g *middleware.LoginGuard,
wait time.Duration,
) *probeResult {
probed := make(chan probeResult, 1)
go func() {
start := time.Now()
release, ok := g.AcquireForTest(context.Background())
if ok {
release()
}
probed <- probeResult{ok: ok, elapsed: time.Since(start)}
}()
select {
case result := <-probed:
return &result
case <-time.After(wait):
return nil
}
}