Context
src/auth/ratelimit.ts:42-63: check() reads the count of failed attempts in the window, and record() inserts a new row after authFinish completes. These are two separate prepared statements with no transaction.
Problem / Observation
With 5 concurrent in-flight login attempts for the same account (browser opens 5 retries while a slow OPAQUE call runs), all 5 read n=0 from stmtCountAccount, all 5 are allowed, all 5 then run authFinish, and all 5 record a failure. The 6th attempt sees n=5 and is correctly blocked — but the attacker just got 5 attempts in parallel instead of 1.
Better-sqlite3 is synchronous so the same node process doesn't really exhibit this, but multiple processes (e.g. a future fleet, or a paused process resuming) would. More directly: the gap between check at /login/start and record at /login/finish is the entire OPAQUE round-trip, during which any number of additional /login/start calls can succeed.
Proposed approach
- Combine
check + insert-on-failure in a single db.transaction. Use INSERT-then-check semantics: insert a "pending" row at /start, count rows in the window, reject if over cap. At /finish, UPDATE the pending row to mark success or leave failure.
- Or: use SQLite's
BEGIN IMMEDIATE for the check+record sequence in /login/finish so concurrent /start calls are serialized.
- Reuse the same primitive for admin login (which has no rate limiter at all today — see related issue).
Acceptance criteria
Context
src/auth/ratelimit.ts:42-63:
check()reads the count of failed attempts in the window, andrecord()inserts a new row afterauthFinishcompletes. These are two separate prepared statements with no transaction.Problem / Observation
With 5 concurrent in-flight login attempts for the same account (browser opens 5 retries while a slow OPAQUE call runs), all 5 read
n=0fromstmtCountAccount, all 5 are allowed, all 5 then runauthFinish, and all 5 record a failure. The 6th attempt seesn=5and is correctly blocked — but the attacker just got 5 attempts in parallel instead of 1.Better-sqlite3 is synchronous so the same node process doesn't really exhibit this, but multiple processes (e.g. a future fleet, or a paused process resuming) would. More directly: the gap between
checkat/login/startandrecordat/login/finishis the entire OPAQUE round-trip, during which any number of additional/login/startcalls can succeed.Proposed approach
check+ insert-on-failure in a singledb.transaction. Use INSERT-then-check semantics: insert a "pending" row at /start, count rows in the window, reject if over cap. At /finish, UPDATE the pending row to mark success or leave failure.BEGIN IMMEDIATEfor the check+record sequence in/login/finishso concurrent /start calls are serialized.Acceptance criteria
LoginRateLimiter