mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-08 12:32:43 +00:00
Guard the dummy mint, not only the dummy verification
The dummy hash was awaited as an argument to the verifier, so a rejected mint threw before the verifier was reached and before the catch on it could apply. The promise was created once at import, and a rejected promise stays rejected, so one failed mint under memory pressure would have made every unknown identifier answer 500 while a real account answered 401 — a cleaner oracle than the timing gap this closes, and passkey-only accounts locked out of the password form until restart. The mint is now awaited inside the guarded region, is lazy rather than an import-time cost in every process that touches this module, and is dropped on failure so the next refusal mints again instead of inheriting a cached rejection. A failure leaves a log line; the answer is false either way. The tests cover a rejecting mint, not only a rejecting verify.
This commit is contained in:
1 parent
80426697db
commit
10d6dc8099
2 files changed
+121
-27
No files matched your search
@@ -19,8 +19,7 @@
|
||||
*/
|
||||
import { describe, it, expect, vi, beforeEach } from "vitest";
|
||||
|
||||
// Hoisted: `password.ts` mints the dummy hash at module load, so the mock
|
||||
// has to exist before the import below runs.
|
||||
// Hoisted so the mock is in place before `password.ts` is imported.
|
||||
const { hashMock, verifyMock } = vi.hoisted(() => ({
|
||||
hashMock: vi.fn(async (input: string) => `hashed:${input}`),
|
||||
verifyMock: vi.fn(async (_hash: string, _password: string) => false),
|
||||
@@ -28,23 +27,45 @@ const { hashMock, verifyMock } = vi.hoisted(() => ({
|
||||
|
||||
vi.mock("@node-rs/argon2", () => ({ hash: hashMock, verify: verifyMock }));
|
||||
|
||||
import { verifyPasswordOrDummy } from "@/lib/auth/password";
|
||||
import { ARGON2_HASH_OPTIONS } from "@/lib/auth/argon2-params.mjs";
|
||||
|
||||
beforeEach(() => {
|
||||
/**
|
||||
* The dummy hash is minted once per module instance, so each test gets a
|
||||
* fresh instance — otherwise the first successful mint is cached and the
|
||||
* mint-failure cases could never reach the mint at all.
|
||||
*/
|
||||
let verifyPasswordOrDummy: (
|
||||
storedHash: string | null | undefined,
|
||||
password: string,
|
||||
) => Promise<boolean>;
|
||||
|
||||
beforeEach(async () => {
|
||||
verifyMock.mockReset();
|
||||
verifyMock.mockResolvedValue(false);
|
||||
hashMock.mockReset();
|
||||
hashMock.mockImplementation(async (input: string) => `hashed:${input}`);
|
||||
vi.resetModules();
|
||||
({ verifyPasswordOrDummy } = await import("@/lib/auth/password"));
|
||||
});
|
||||
|
||||
describe("verifyPasswordOrDummy", () => {
|
||||
it("mints the dummy hash under the same Argon2id parameters real hashes use", () => {
|
||||
// The module mints it once at load, so the call has already happened.
|
||||
it("mints the dummy hash under the same Argon2id parameters real hashes use", async () => {
|
||||
await verifyPasswordOrDummy(null, "anything");
|
||||
expect(hashMock).toHaveBeenCalledWith(
|
||||
expect.any(String),
|
||||
ARGON2_HASH_OPTIONS,
|
||||
);
|
||||
});
|
||||
|
||||
it("mints once and reuses it for every later refusal", async () => {
|
||||
await verifyPasswordOrDummy(null, "anything");
|
||||
await verifyPasswordOrDummy(null, "anything");
|
||||
await verifyPasswordOrDummy(undefined, "anything");
|
||||
// Three refusals, three verifications, one mint.
|
||||
expect(verifyMock).toHaveBeenCalledTimes(3);
|
||||
expect(hashMock).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("verifies against the stored hash when the account has one", async () => {
|
||||
verifyMock.mockResolvedValueOnce(true);
|
||||
await expect(
|
||||
@@ -61,9 +82,7 @@ describe("verifyPasswordOrDummy", () => {
|
||||
await expect(verifyPasswordOrDummy(null, "anything")).resolves.toBe(false);
|
||||
expect(verifyMock).toHaveBeenCalledTimes(1);
|
||||
// Against the dummy, not against nothing.
|
||||
expect(verifyMock.mock.calls[0][0]).toBe(
|
||||
await hashMock.mock.results[0].value,
|
||||
);
|
||||
expect(verifyMock.mock.calls[0][0]).toMatch(/^hashed:/);
|
||||
});
|
||||
|
||||
it("still verifies for a passkey-only account (undefined hash)", async () => {
|
||||
@@ -87,7 +106,54 @@ describe("verifyPasswordOrDummy", () => {
|
||||
it("swallows a throw from the dummy arm rather than answering differently", async () => {
|
||||
// A 500 only the unknown-account arm can produce is the same oracle in
|
||||
// a different field.
|
||||
verifyMock.mockRejectedValueOnce(new Error("malformed hash"));
|
||||
await expect(verifyPasswordOrDummy(null, "anything")).resolves.toBe(false);
|
||||
const logged = vi.spyOn(console, "error").mockImplementation(() => {});
|
||||
try {
|
||||
verifyMock.mockRejectedValueOnce(new Error("malformed hash"));
|
||||
await expect(verifyPasswordOrDummy(null, "anything")).resolves.toBe(
|
||||
false,
|
||||
);
|
||||
expect(logged).toHaveBeenCalled();
|
||||
} finally {
|
||||
logged.mockRestore();
|
||||
}
|
||||
});
|
||||
|
||||
it("swallows a failed MINT too, not only a failed verify", async () => {
|
||||
// The mint is awaited as an argument to the verifier, so an unguarded
|
||||
// `await` on a rejected mint throws before the verifier is reached and
|
||||
// before any `.catch` on it can apply. That answers 500 on exactly the
|
||||
// arm that must look ordinary — and a passkey-only account could not
|
||||
// sign in at all.
|
||||
const logged = vi.spyOn(console, "error").mockImplementation(() => {});
|
||||
try {
|
||||
hashMock.mockRejectedValueOnce(new Error("out of memory"));
|
||||
await expect(verifyPasswordOrDummy(null, "anything")).resolves.toBe(
|
||||
false,
|
||||
);
|
||||
expect(verifyMock).not.toHaveBeenCalled();
|
||||
expect(logged).toHaveBeenCalled();
|
||||
} finally {
|
||||
logged.mockRestore();
|
||||
}
|
||||
});
|
||||
|
||||
it("re-mints after a failure instead of caching the rejection forever", async () => {
|
||||
// A rejected promise stays rejected for the life of the process. Caching
|
||||
// one would send every later refusal back down the cheap path with the
|
||||
// timing channel reopened and nothing to notice it by.
|
||||
const logged = vi.spyOn(console, "error").mockImplementation(() => {});
|
||||
try {
|
||||
hashMock.mockRejectedValueOnce(new Error("out of memory"));
|
||||
await verifyPasswordOrDummy(null, "anything");
|
||||
expect(verifyMock).not.toHaveBeenCalled();
|
||||
|
||||
await expect(verifyPasswordOrDummy(null, "anything")).resolves.toBe(
|
||||
false,
|
||||
);
|
||||
expect(hashMock).toHaveBeenCalledTimes(2);
|
||||
expect(verifyMock).toHaveBeenCalledTimes(1);
|
||||
} finally {
|
||||
logged.mockRestore();
|
||||
}
|
||||
});
|
||||
});
|
||||
+44
-16
@@ -31,10 +31,10 @@ export async function verifyPassword(
|
||||
* password. Verifying against this hash on the empty arms makes the work
|
||||
* comparable.
|
||||
*
|
||||
* Minted once per process from `ARGON2_HASH_OPTIONS`, the same parameters
|
||||
* every real hash carries, because Argon2 encodes its cost in the hash
|
||||
* string and the verifier pays whatever the string says. A dummy minted at
|
||||
* cheaper settings would leave the gap it was added to close.
|
||||
* Minted from `ARGON2_HASH_OPTIONS`, the same parameters every real hash
|
||||
* carries, because Argon2 encodes its cost in the hash string and the
|
||||
* verifier pays whatever the string says. A dummy minted at cheaper settings
|
||||
* would leave the gap it was added to close.
|
||||
*
|
||||
* The input is a fixed constant rather than a random string so the module
|
||||
* has no hidden state, and it is never compared to anything: the arm that
|
||||
@@ -42,13 +42,33 @@ export async function verifyPassword(
|
||||
* path on which a caller can make this hash accept.
|
||||
*/
|
||||
const DUMMY_VERIFY_INPUT = "healthlog: not a credential, never accepted";
|
||||
const DUMMY_PASSWORD_HASH: Promise<string> = hash(
|
||||
DUMMY_VERIFY_INPUT,
|
||||
ARGON2_HASH_OPTIONS,
|
||||
);
|
||||
// Keep a mint failure from surfacing as an unhandled rejection at import
|
||||
// time; `verifyPasswordOrDummy` still awaits and still sees it.
|
||||
void DUMMY_PASSWORD_HASH.catch(() => undefined);
|
||||
|
||||
let dummyHashPromise: Promise<string> | null = null;
|
||||
|
||||
/**
|
||||
* The dummy hash, minted on first use and kept for the process afterwards.
|
||||
*
|
||||
* Lazy rather than minted at import: every process that imports this module
|
||||
* would otherwise spend 19 MiB and a thread-pool slot at boot, including the
|
||||
* queue worker and each build worker, none of which ever authenticates
|
||||
* anybody. From the second refusal onward the cost profile is identical
|
||||
* either way.
|
||||
*
|
||||
* A rejected promise stays rejected for the life of the process, so caching
|
||||
* one would degrade every later refusal back to the cheap path and leave the
|
||||
* timing channel open with no way to notice. A failed mint is therefore
|
||||
* dropped and the next call mints again.
|
||||
*/
|
||||
function dummyHash(): Promise<string> {
|
||||
if (!dummyHashPromise) {
|
||||
const pending = hash(DUMMY_VERIFY_INPUT, ARGON2_HASH_OPTIONS);
|
||||
dummyHashPromise = pending;
|
||||
pending.catch(() => {
|
||||
if (dummyHashPromise === pending) dummyHashPromise = null;
|
||||
});
|
||||
}
|
||||
return dummyHashPromise;
|
||||
}
|
||||
|
||||
/**
|
||||
* Verify `password` against `storedHash`, or against the dummy hash when
|
||||
@@ -68,11 +88,19 @@ export async function verifyPasswordOrDummy(
|
||||
password: string,
|
||||
): Promise<boolean> {
|
||||
if (storedHash) return verify(storedHash, password);
|
||||
// Same verifier, same cost profile, verdict discarded. A throw here (a
|
||||
// malformed dummy, a mint that failed) must not turn into a 500 that only
|
||||
// the unknown-account arm can produce — that would be the oracle again,
|
||||
// in the status code this time.
|
||||
await verify(await DUMMY_PASSWORD_HASH, password).catch(() => false);
|
||||
try {
|
||||
// Same verifier, same cost profile, verdict discarded.
|
||||
await verify(await dummyHash(), password);
|
||||
} catch (error) {
|
||||
// Both the mint and the verify are inside the guard, and the mint has to
|
||||
// be: an unguarded `await` on a rejected mint would throw out of here and
|
||||
// answer 500 on exactly the arm this function exists to make ordinary —
|
||||
// an unknown identifier answering 500 while a real account answers 401 is
|
||||
// the oracle again, in the status code, and cheaper to read than the
|
||||
// timing gap was. The breadcrumb makes a degraded instance visible; the
|
||||
// answer below is the same either way.
|
||||
console.error("[auth] dummy password verification failed", error);
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
|
||||
Reference in new issue
Block a user