diff --git a/docs/specs/hosted.md b/docs/specs/hosted.md index 40df20063..c85553e14 100644 --- a/docs/specs/hosted.md +++ b/docs/specs/hosted.md @@ -118,6 +118,7 @@ A session-gated route answers a session of an account no longer entitled with th - **Must sweep every Relay table's expired rows from the relay's hourly Cron Trigger** (rationale); sign-in challenges, minted unauthenticated and flat, are bounded by it and the per-address limit. - **Must answer 429 with `Retry-After` past the per-address limit on `signin/*` (`RELAY_SIGNIN_LIMIT`) and `setup/begin`/`finish` (`RELAY_SETUP_LIMIT`)**, before the body limit and any database read: 30 a minute, a ceremony's two routes sharing one budget (rationale). - **Must restore a token a refused `finish` spent on its original expiry, within the Burrow's cap, and never once that expiry has passed.** +- **Must compare expiries under an advisory lock against `LOCKED_NOW` in `hosted/server/relay-auth.ts`**, never `now()`, which predates the lock wait. **Push.** The push routes keep `docs/specs/relay.md` -> "Web Push" and its "State files" upsert rules; a send is HTTPS from the Burrow to the relay Worker, independent of terminal transport. diff --git a/hosted/server/relay-api.ts b/hosted/server/relay-api.ts index 3db77c690..38bef5bd5 100644 --- a/hosted/server/relay-api.ts +++ b/hosted/server/relay-api.ts @@ -63,6 +63,7 @@ import { relayPushRoutes } from "./relay-push"; import { OWNER_COLUMNS, database, + LOCKED_NOW, locked, ownerOf, requireBurrow, @@ -522,7 +523,7 @@ export function relayApiRoutes(app: Hono<{ Bindings: RelayEnv }>) { `WITH spent AS ( UPDATE dormouse_relay_enrollment_approvals SET "redeemedBurrowId" = $3, "redeemedAt" = now() - WHERE "userCode" = $1 AND "userId" = $2 AND "expiresAt" > now() + WHERE "userCode" = $1 AND "userId" = $2 AND "expiresAt" > ${LOCKED_NOW} AND "redeemedAt" IS NULL RETURNING "userId" ) @@ -537,7 +538,7 @@ export function relayApiRoutes(app: Hono<{ Bindings: RelayEnv }>) { rows: [spent], } = await db.query<{ redeemedBurrowId: string }>( `SELECT "redeemedBurrowId" FROM dormouse_relay_enrollment_approvals - WHERE "userCode" = $1 AND "expiresAt" > now() AND "redeemedBurrowId" IS NOT NULL`, + WHERE "userCode" = $1 AND "expiresAt" > ${LOCKED_NOW} AND "redeemedBurrowId" IS NOT NULL`, [userCode], ); return spent ? { redeemed: spent.redeemedBurrowId } : "expired"; @@ -642,8 +643,8 @@ async function consumeChallenge(db: Client, challenge: string, burrowId: string * in one statement: its own expired rows pruned, its live rows trimmed to * leave one slot under the cap (latest expiry kept), then the row. No other * owner's rows are read. `expiry` is a TTL in milliseconds or an instant; a - * row already expired is not inserted. Answers its expiry in epoch - * milliseconds, or undefined when nothing was inserted. + * row already expired is not inserted and trims nothing. Answers its expiry + * in epoch milliseconds, or undefined when nothing was inserted. */ export async function admit( db: Client, @@ -660,16 +661,16 @@ export async function admit( return locked(db, `${table}:${ownerValue}`, async () => { const { rows } = await db.query<{ expiresAt: number }>( `WITH pruned AS ( - DELETE FROM ${table} WHERE ${owner} = $1 AND "expiresAt" <= now() + DELETE FROM ${table} WHERE ${owner} = $1 AND "expiresAt" <= ${LOCKED_NOW} ), trimmed AS ( - DELETE FROM ${table} WHERE ${pk} IN ( - SELECT ${pk} FROM ${table} WHERE ${owner} = $1 AND "expiresAt" > now() + DELETE FROM ${table} WHERE ${expiresAt} > ${LOCKED_NOW} AND ${pk} IN ( + SELECT ${pk} FROM ${table} WHERE ${owner} = $1 AND "expiresAt" > ${LOCKED_NOW} ORDER BY "expiresAt" DESC, ${pk} OFFSET $2 ) ) INSERT INTO ${table} (${columns.map((column) => `"${column}"`).join(", ")}, "expiresAt") SELECT ${columns.map((_, i) => `$${i + 3}`).join(", ")}, ${expiresAt} - WHERE ${expiresAt} > now() + WHERE ${expiresAt} > ${LOCKED_NOW} ON CONFLICT (${pk}) DO NOTHING RETURNING ${epochMs('"expiresAt"')} AS "expiresAt"`, [ownerValue, cap - 1, ...values, expiry], diff --git a/hosted/server/relay-auth.ts b/hosted/server/relay-auth.ts index 2102c25fd..e44988172 100644 --- a/hosted/server/relay-auth.ts +++ b/hosted/server/relay-auth.ts @@ -110,6 +110,13 @@ export function database( return withClient(c.env.HYPERDRIVE.connectionString, action); } +/** + * "Now" for a liveness check inside `locked`'s action: the statement's start, + * after the lock wait. Not `now()`, the transaction's start before that wait, + * so a row that expired while its writer waited would still read as live. + */ +export const LOCKED_NOW = "statement_timestamp()"; + /** Runs `action` in a transaction holding `key`'s advisory lock, so a cap check and its insert cannot interleave. */ export async function locked(db: Client, key: string, action: () => Promise) { await db.query("BEGIN"); diff --git a/hosted/server/tests/relay.test.ts b/hosted/server/tests/relay.test.ts index 3f24d1f47..facec4ab6 100644 --- a/hosted/server/tests/relay.test.ts +++ b/hosted/server/tests/relay.test.ts @@ -765,6 +765,35 @@ test("restoring a setup token that has since expired never evicts a live one", a expect(after).toContain(digest(back)); }); +test("a setup token that expires while its restore waits on the lock is not restored", async ({ + onTestFinished, +}) => { + const f = await fixture(); + onTestFinished(f.close); + const owner = await f.account(ADMIN_EMAIL); + const laptop = await f.burrow(owner); + const live: string[] = []; + for (let i = 0; i < MAX_TOKENS_PER_BURROW; i++) live.push(digest(await f.setupToken(laptop.token))); + const expiresAt = new Date(Date.now() + 1000); + // Another write holds the Burrow's token lock across the restored token's expiry. + await withClient(f.url, async (holder) => { + await holder.query("BEGIN"); + await holder.query("SELECT pg_advisory_xact_lock(hashtextextended($1, 0))", [ + `dormouse-relay:dormouse_relay_setup_tokens:${laptop.burrowId}`, + ]); + const restoring = withClient(f.url, (db) => + restoreSetupToken(db, randomSecret(), { burrowId: laptop.burrowId, userId: owner, expiresAt }), + ); + await new Promise((resolve) => setTimeout(resolve, expiresAt.getTime() - Date.now() + 500)); + await holder.query("COMMIT"); + await restoring; + }); + const hashes = (await f.sql<{ tokenHash: string }>(`SELECT "tokenHash" FROM dormouse_relay_setup_tokens`)) + .map((row) => row.tokenHash) + .sort(); + expect(hashes).toEqual([...live].sort()); +}); + test("a de-entitled account signs in to nothing, and its sessions answer as expired", async ({ onTestFinished, }) => {