diff --git a/internal/web/handlers/auth/auth_test.go b/internal/web/handlers/auth/auth_test.go index 3c840c67..9529d514 100644 --- a/internal/web/handlers/auth/auth_test.go +++ b/internal/web/handlers/auth/auth_test.go @@ -14,6 +14,7 @@ import ( "net/http/httptest" "net/url" "regexp" + "sort" "strings" "sync" "testing" @@ -638,9 +639,17 @@ func TestLogin_ConstantTime(t *testing.T) { tok := extractTokenFromMessage(t, sender.all()[0], "/verify-email") _ = cli.get(t, "/verify-email/"+tok).Body.Close() - const trials = 10 + // The login throttle allows 6 attempts per (ip, identifier) per + // window; the 7th onwards short-circuits before any hashing, which + // would poison the medians (and did, when this test used 10–15 + // trials). Stay under the limit for each identifier. + const trials = 5 + median := func(samples []time.Duration) time.Duration { + sort.Slice(samples, func(i, j int) bool { return samples[i] < samples[j] }) + return samples[len(samples)/2] + } measure := func(username string) time.Duration { - var total time.Duration + samples := make([]time.Duration, 0, trials) for i := 0; i < trials; i++ { cli2 := newClient(t, srv) csrf := cli2.extractCSRF(t, "/login") @@ -650,22 +659,35 @@ func TestLogin_ConstantTime(t *testing.T) { "username": {username}, "password": {"any-wrong-password"}, }) - total += time.Since(start) + samples = append(samples, time.Since(start)) _ = resp.Body.Close() } - return total / trials - } + return median(samples) + } + // Baseline: what one argon2 verify costs on this machine right now. + // The property under test is that the missing-user branch still + // pays for a hash instead of short-circuiting on the lookup, so + // compare against that cost directly rather than against the + // existing-user request, which also does throttle/audit writes + // whose latency swings with CI database load. + hashSamples := make([]time.Duration, 0, trials) + for i := 0; i < trials; i++ { + start := time.Now() + password.VerifyAgainstDummy("any-wrong-password") + hashSamples = append(hashSamples, time.Since(start)) + } + hashCost := median(hashSamples) + existing := measure("dave") missing := measure("does-not-exist") - delta := existing - missing - if delta < 0 { - delta = -delta - } - // On the test argon params (~5–15ms) any user-existence shortcut would - // shave off most of the time; allow generous slack for CI noise but - // reject a 5x divergence. - if existing > missing*5 || missing > existing*5 { - t.Fatalf("login timing diverges too much: existing=%v missing=%v delta=%v", existing, missing, delta) + if missing < hashCost/2 { + t.Fatalf("missing-user login skipped the hash: missing=%v hash=%v existing=%v", missing, hashCost, existing) + } + // Still reject an egregious divergence in either direction; the + // bound is loose because the existing-user path legitimately does + // more database work and medians on a loaded CI runner are noisy. + if existing > missing*10 || missing > existing*10 { + t.Fatalf("login timing diverges too much: existing=%v missing=%v hash=%v", existing, missing, hashCost) } }