Skip to content

Commit c41a7a8

Browse files
committed
Evict the oldest /begin row instead of 429 at cap
/login/passkey/begin used to refuse with 429 and Retry-After: 300 once the table held 64 unexpired challenges. Because the endpoint is unauthenticated, any actor could send 64 POSTs in a burst and park the table at the cap for the full 5-minute TTL, which forced every legitimate passkey sign-in into 429 for that window and turned the anti-bloat safeguard into a trivial availability DoS. Switch the at-cap branch from "return 429" to "evict the oldest unexpired row, then insert." The cap still holds (size never exceeds 64) but no caller ever sees a 429, so an attacker can't hold the door shut. Eviction picks the row with the smallest expires_at, which is the FIFO head since expires_at is just insert time + TTL. There's a residual displacement risk: a sustained burst could race a legitimate user's row out of the table before they reach /finish. That's a much harder attack than the cap-park one this fixes, and it lives at the same threat tier (network-bandwidth abuse) where a reverse proxy is the right defense layer. The 429 test in src/pages/login.test.ts is replaced with one that asserts the eviction behaviour: pre-fill the table to 64 rows with ascending expires_at, hit /begin, and check the response is 200, the table size is still 64, and the oldest seeded row is gone. #487 (comment) Assisted-by: Claude Code:claude-opus-4-7
1 parent 9682924 commit c41a7a8

2 files changed

Lines changed: 48 additions & 34 deletions

File tree

src/pages/login.test.ts

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -102,27 +102,36 @@ describe("login passkeys", () => {
102102
expect(setCookie).not.toMatch(/passkey_login=/);
103103
});
104104

105-
it("returns 429 when the outstanding-challenge cap is reached", async () => {
105+
it("evicts the oldest outstanding row instead of 429 when at cap", async () => {
106+
// An unauthenticated attacker should not be able to park the
107+
// table at the cap to lock out legitimate sign-ins. When /begin
108+
// hits the cap it evicts the oldest unexpired row to make space
109+
// and still returns a fresh challenge.
106110
await seedCredential();
107111
await seedPasskey();
108-
// Pre-fill the table up to the cap so the next /begin tips over.
109-
const futureExpiry = new Date(Date.now() + 60_000);
112+
const baseExpiry = Date.now() + 60_000;
110113
const rows = Array.from({ length: 64 }, (_, i) => ({
111-
id: `seeded-${i.toString()}`,
114+
id: `seeded-${i.toString().padStart(2, "0")}`,
112115
challenge: "seeded-challenge",
113-
expiresAt: futureExpiry,
116+
// Older rows expire sooner, so the i=0 row is the FIFO head.
117+
expiresAt: new Date(baseExpiry + i * 1000),
114118
}));
115119
await db.insert(passkeyLoginChallenges).values(rows);
116120

117121
const response = await app.request(
118122
"http://hollo.test/login/passkey/begin",
119123
{ method: "POST" },
120124
);
121-
expect(response.status).toBe(429);
122-
expect(response.headers.get("Retry-After")).toBe("300");
123-
// The seeded rows are still there; no new one was inserted.
125+
expect(response.status).toBe(200);
126+
expect(response.headers.get("Retry-After")).toBeNull();
127+
// Table size is unchanged (one evicted, one inserted).
124128
const total = await db.$count(passkeyLoginChallenges);
125129
expect(total).toBe(64);
130+
// The oldest seeded row is the one that got dropped.
131+
const evicted = await db.query.passkeyLoginChallenges.findFirst({
132+
where: eq(passkeyLoginChallenges.id, "seeded-00"),
133+
});
134+
expect(evicted).toBeUndefined();
126135
});
127136

128137
it("returns authn options and sets a challenge cookie when at least one is enrolled", async () => {

src/pages/login.tsx

Lines changed: 31 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { randomUUID } from "node:crypto";
22

33
import { zValidator } from "@hono/zod-validator";
4-
import { and, eq, gte, lt, sql } from "drizzle-orm";
4+
import { and, asc, eq, gte, lt, sql } from "drizzle-orm";
55
import { Hono } from "hono";
66
import { deleteCookie, getSignedCookie, setSignedCookie } from "hono/cookie";
77
import { z } from "zod";
@@ -21,12 +21,13 @@ import { credentials, passkeyLoginChallenges, passkeys } from "../schema.ts";
2121

2222
const PASSKEY_LOGIN_COOKIE = "passkey_login";
2323
const PASSKEY_LOGIN_MAX_AGE_SECONDS = 5 * 60;
24-
// Soft cap on unexpired rows in `passkey_login_challenges`. Hollo is a
24+
// Hard cap on unexpired rows in `passkey_login_challenges`. Hollo is a
2525
// single-user instance, so even on a popular profile a handful of
2626
// in-flight ceremonies at a time is the realistic ceiling — well below
27-
// this number. The cap is here to keep an unauthenticated caller from
28-
// pumping rows into the table during the TTL window. Going above the
29-
// cap returns 429 without writing a row.
27+
// this number. The cap exists to bound the table under unauthenticated
28+
// abuse; when it's reached, /begin evicts the oldest unexpired row to
29+
// make space rather than refusing the new request, so an attacker can
30+
// never force a legitimate sign-in into 429.
3031
const PASSKEY_LOGIN_MAX_OUTSTANDING_CHALLENGES = 64;
3132
// Stable, arbitrary key for the Postgres advisory lock that serialises
3233
// the GC + count + insert sequence inside /login/passkey/begin so the
@@ -265,12 +266,12 @@ login.post("/passkey/begin", async (c) => {
265266
const { options, challenge } = await buildAuthenticationOptions({ rpInfo });
266267
const id = randomUUID();
267268
const expiresAt = new Date(Date.now() + PASSKEY_LOGIN_MAX_AGE_SECONDS * 1000);
268-
// GC + count + insert are wrapped in one transaction guarded by a
269-
// Postgres advisory transaction lock, so concurrent /begin requests
270-
// serialise on the lock instead of racing between the count and the
271-
// insert. This is the only place that takes this lock, so contention
272-
// is limited to this endpoint.
273-
const tooManyInFlight = await db.transaction(async (tx) => {
269+
// GC + count + (evict +) insert are wrapped in one transaction
270+
// guarded by a Postgres advisory transaction lock, so concurrent
271+
// /begin requests serialise on the lock instead of racing between the
272+
// count and the insert. This is the only place that takes this
273+
// lock, so contention is limited to this endpoint.
274+
await db.transaction(async (tx) => {
274275
await tx.execute(
275276
sql`SELECT pg_advisory_xact_lock(${PASSKEY_LOGIN_BEGIN_LOCK})`,
276277
);
@@ -284,33 +285,37 @@ login.post("/passkey/begin", async (c) => {
284285
await tx
285286
.delete(passkeyLoginChallenges)
286287
.where(lt(passkeyLoginChallenges.expiresAt, now));
287-
// Soft cap on outstanding (unexpired) rows to keep an
288-
// unauthenticated caller from pumping the table during the TTL
289-
// window. Counting after the GC makes the cap reflect
290-
// "still-usable" rows only.
288+
// Count after the GC so the cap reflects "still-usable" rows only.
291289
const outstanding = await tx.$count(
292290
passkeyLoginChallenges,
293291
gte(passkeyLoginChallenges.expiresAt, now),
294292
);
293+
// At the cap, evict the oldest unexpired row to make space.
294+
// Refusing the new request would let any unauthenticated caller
295+
// park the cap at 64 outstanding rows for the full TTL and force
296+
// every legitimate sign-in into 429; eviction keeps the table
297+
// bounded without that DoS surface. An attacker can still race
298+
// the legitimate user's row out before /finish, but that's a much
299+
// harder attack than holding the door shut.
295300
if (outstanding >= PASSKEY_LOGIN_MAX_OUTSTANDING_CHALLENGES) {
296-
return true;
301+
const oldest = await tx
302+
.select({ id: passkeyLoginChallenges.id })
303+
.from(passkeyLoginChallenges)
304+
.where(gte(passkeyLoginChallenges.expiresAt, now))
305+
.orderBy(asc(passkeyLoginChallenges.expiresAt))
306+
.limit(1);
307+
if (oldest.length > 0) {
308+
await tx
309+
.delete(passkeyLoginChallenges)
310+
.where(eq(passkeyLoginChallenges.id, oldest[0].id));
311+
}
297312
}
298313
await tx.insert(passkeyLoginChallenges).values({
299314
id,
300315
challenge,
301316
expiresAt,
302317
});
303-
return false;
304318
});
305-
if (tooManyInFlight) {
306-
c.header("Retry-After", PASSKEY_LOGIN_MAX_AGE_SECONDS.toString());
307-
return c.json(
308-
{
309-
error: "Too many passkey login ceremonies in flight; try again later.",
310-
},
311-
429,
312-
);
313-
}
314319
// The cookie only carries the row id — the challenge itself never
315320
// leaves the server. /finish does the atomic consume so a captured
316321
// cookie + assertion pair is good for at most one request.

0 commit comments

Comments
 (0)