From d0b06bd6a81c3f70077c58c45e790e6e2377d6ed Mon Sep 17 00:00:00 2001 From: mfwolffe Date: Wed, 2 Sep 2026 03:11:10 -0400 Subject: [PATCH 1/2] auth: make constant-time login test measure hash cost instead of request ratio --- internal/web/handlers/auth/auth_test.go | 46 +++++++++++++++++-------- 1 file changed, 32 insertions(+), 14 deletions(-) diff --git a/internal/web/handlers/auth/auth_test.go b/internal/web/handlers/auth/auth_test.go index 3c840c67..6cd0ee0a 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,13 @@ 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 + const trials = 15 + 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 +655,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) } } From 9919988a8e6ae0f7fed733cc794cb120ce979993 Mon Sep 17 00:00:00 2001 From: mfwolffe Date: Wed, 2 Sep 2026 03:17:59 -0400 Subject: [PATCH 2/2] auth: keep constant-time login trials under the login throttle --- internal/web/handlers/auth/auth_test.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/internal/web/handlers/auth/auth_test.go b/internal/web/handlers/auth/auth_test.go index 6cd0ee0a..9529d514 100644 --- a/internal/web/handlers/auth/auth_test.go +++ b/internal/web/handlers/auth/auth_test.go @@ -639,7 +639,11 @@ func TestLogin_ConstantTime(t *testing.T) { tok := extractTokenFromMessage(t, sender.all()[0], "/verify-email") _ = cli.get(t, "/verify-email/"+tok).Body.Close() - const trials = 15 + // 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]