diff --git a/services/auth/src/auth.test.ts b/services/auth/src/auth.test.ts index ac8d734..57997a6 100644 --- a/services/auth/src/auth.test.ts +++ b/services/auth/src/auth.test.ts @@ -212,3 +212,34 @@ describe("TOTP code parsing", () => { } }); }); + +describe("recovery code generation stays inside the container's memory", () => { + // Argon2id here is configured for 64 MiB per hash. Hashing the ten codes in + // parallel reserved 640 MiB, over the auth container's 512 MiB limit, and the + // process was OOM-killed in the middle of enrolment - which reached the + // browser as "upstream service is unreachable" and left the account with a + // confirmed second factor and no recovery codes. + // + // Timed against the real hash rather than a mock: the cost here is memory + // held concurrently, and only the real implementation reserves it. Four codes + // sequentially must take roughly four times one hash; in parallel they would + // take about one. + it("hashes the codes one at a time, not all at once", { timeout: 60000 }, async () => { + const { hashPassword } = await import("./crypto.js"); + const { generateRecoveryCodes } = await import("./mfaHelpers.js"); + + const singleStart = Date.now(); + await hashPassword("baseline"); + const single = Date.now() - singleStart; + + const batchStart = Date.now(); + const out = await generateRecoveryCodes(4); + const batch = Date.now() - batchStart; + + expect(out.rawCodes).toHaveLength(4); + expect(out.hashedCodes).toHaveLength(4); + expect(out.hashedCodes.every((h) => h.startsWith("$argon2id$"))).toBe(true); + // Generous lower bound: parallel would land near 1x, sequential near 4x. + expect(batch).toBeGreaterThan(single * 2.5); + }); +}); diff --git a/services/auth/src/mfaHelpers.ts b/services/auth/src/mfaHelpers.ts index 3674626..9565e93 100644 --- a/services/auth/src/mfaHelpers.ts +++ b/services/auth/src/mfaHelpers.ts @@ -10,7 +10,19 @@ export async function generateRecoveryCodes(count = 10): Promise<{ rawCodes: str for (let i = 0; i < count; i++) { rawCodes.push(crypto.randomBytes(6).toString("hex").substring(0, 10).toUpperCase()); } - const hashedCodes = await Promise.all(rawCodes.map((c) => hashPassword(c))); + // Hashed one at a time, deliberately. Argon2id is configured for 64 MiB per + // hash, so ten of them in parallel reserve 640 MiB - more than the auth + // container is allowed - and the process was OOM-killed partway through + // enrolment. From the browser that surfaced as "upstream service is + // unreachable"; in the database it left an account with a confirmed second + // factor and no recovery codes, because the row is written before this runs. + // + // Sequentially it peaks at 64 MiB and takes a few seconds, which is the + // right trade for an operation that happens once per enrolment. + const hashedCodes: string[] = []; + for (const code of rawCodes) { + hashedCodes.push(await hashPassword(code)); + } return { rawCodes, hashedCodes }; } diff --git a/services/auth/src/repository.ts b/services/auth/src/repository.ts index 6fa29cf..61ef4f6 100644 --- a/services/auth/src/repository.ts +++ b/services/auth/src/repository.ts @@ -201,19 +201,34 @@ async function loadTotp(mailboxId: string, status: "PENDING" | "CONFIRMED"): Pro } /** Confirms enrollment by proving one code against the stored secret. */ -export async function confirmTotpEnrollment(mailboxId: string, code: string): Promise { +/** + * Checks a code against the pending enrolment without committing anything. + * + * Split from the commit so the caller can store the recovery codes first. + * Confirming and then generating them meant a failure in between - which + * happened, on memory - left an account with a second factor enabled and no + * recovery codes, and the person enrolling never saw the codes they are told + * to save. Leaving the enrolment pending is the recoverable end of that. + */ +export async function verifyPendingTotp( + mailboxId: string, + code: string, +): Promise<{ id: string; step: number } | null> { const row = await loadTotp(mailboxId, "PENDING"); - if (!row) return false; + if (!row) return null; const secret = decryptSecret(row.secret_enc, row.secret_nonce, row.key_version); const result = verifyTotpCode(secret, code, row.last_used_step === null ? null : Number(row.last_used_step)); - if (!result.valid) return false; + if (!result.valid) return null; + return { id: row.id, step: result.step }; +} +/** Marks a verified enrolment confirmed. */ +export async function markTotpConfirmed(id: string, step: number): Promise { await query( `UPDATE mfa_totp SET status = 'CONFIRMED', confirmed_at = NOW(), last_used_step = $2 WHERE id = $1`, - [row.id, result.step], + [id, step], ); - return true; } /** diff --git a/services/auth/src/router.ts b/services/auth/src/router.ts index 7ef9e91..c5905ab 100644 --- a/services/auth/src/router.ts +++ b/services/auth/src/router.ts @@ -422,11 +422,18 @@ async function totpEnroll( async function totpConfirm(req: IncomingMessage, res: ServerResponse, session: SessionTokenPayload): Promise { const body = await parseJsonBody(req); const code = typeof body?.code === "string" ? body.code : ""; - if (!(await repo.confirmTotpEnrollment(session.sub, code))) { + const verified = await repo.verifyPendingTotp(session.sub, code); + if (!verified) { return sendJson(res, 400, { error: "INVALID_TOTP_CODE", message: "TOTP verification failed" }); } + + // The codes are generated and stored before the factor is marked confirmed. + // The other order enables the factor first, so anything that goes wrong here + // leaves an account locked behind a second factor with no way back in. const recovery = await generateRecoveryCodes(10); await repo.storeRecoveryCodes(session.sub, recovery.hashedCodes); + await repo.markTotpConfirmed(verified.id, verified.step); + // The raw codes are shown exactly once; only their hashes are kept. sendJson(res, 200, { confirmed: true, recovery_codes: recovery.rawCodes }); }