mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-08 12:32:43 +00:00
Make every login refusal cost the same verification
Sign-in returned the moment it found no account, or an account carrying no password hash, before it reached the verifier. An account that did have a password paid a full Argon2id verification at 19 MiB and t=2. The three refusals answered with the identical 401 body and three very different durations, so the clock told an anonymous caller whether an address is registered and whether it carries a password. `verifyPasswordOrDummy` verifies against a stand-in hash minted once per process at the same cost parameters real hashes use, and returns false whenever there is nothing to verify, so the passkey-only account takes the same path as one with a password. The dummy arm discards its own verdict and swallows a throw, so no outcome there can be read as a success or answer with a different status.
This commit is contained in:
1 parent
6ef139c7b2
commit
6a5fb338b1
8 files changed
+332
-17
No files matched your search
@@ -12,7 +12,7 @@ vi.mock("@/lib/db", () => ({
|
||||
},
|
||||
}));
|
||||
vi.mock("@/lib/auth/password", () => ({
|
||||
verifyPassword: vi.fn().mockResolvedValue(true),
|
||||
verifyPasswordOrDummy: vi.fn().mockResolvedValue(true),
|
||||
}));
|
||||
vi.mock("@/lib/auth/audit", () => ({
|
||||
auditLog: vi.fn().mockResolvedValue(undefined),
|
||||
@@ -142,8 +142,8 @@ describe("login MFA branch", () => {
|
||||
});
|
||||
|
||||
it("wrong password never reaches the MFA branch", async () => {
|
||||
const { verifyPassword } = await import("@/lib/auth/password");
|
||||
vi.mocked(verifyPassword).mockResolvedValueOnce(false);
|
||||
const { verifyPasswordOrDummy } = await import("@/lib/auth/password");
|
||||
vi.mocked(verifyPasswordOrDummy).mockResolvedValueOnce(false);
|
||||
vi.mocked(prisma.user.findFirst).mockResolvedValue({
|
||||
...BASE_USER,
|
||||
totpConfirmedAt: new Date(),
|
||||
|
||||
@@ -0,0 +1,155 @@
|
||||
/**
|
||||
* Every login outcome reaches the verifier.
|
||||
*
|
||||
* The three refusals sign-in can produce — no such account, an account with
|
||||
* no password hash, a wrong password — return the identical 401 body, and
|
||||
* used to take three very different amounts of time to produce it, because
|
||||
* the first two returned before any hashing happened. Argon2id at 19 MiB and
|
||||
* t=2 against one indexed SELECT is a gap a caller can read over a network.
|
||||
*
|
||||
* The passkey-only account is the case that matters most. If it kept the
|
||||
* cheap path, the channel would still answer "this account exists and has no
|
||||
* password" — which is exactly what the removed discovery endpoint published
|
||||
* in a field.
|
||||
*
|
||||
* What this proves is that the call happens on all three arms, not that the
|
||||
* three durations match: the verifier is mocked here, and wall-clock equality
|
||||
* is not something a unit test can assert. `password-dummy-verify.test.ts`
|
||||
* carries the other half, that the verifier really does Argon2id work at the
|
||||
* real cost parameters when there is no stored hash.
|
||||
*/
|
||||
import { describe, it, expect, vi, beforeEach } from "vitest";
|
||||
import { NextRequest } from "next/server";
|
||||
|
||||
vi.mock("@/lib/db", () => ({
|
||||
prisma: {
|
||||
user: { findFirst: vi.fn() },
|
||||
webauthnMfaCredential: { count: vi.fn().mockResolvedValue(0) },
|
||||
},
|
||||
}));
|
||||
|
||||
vi.mock("@/lib/auth/password", () => ({
|
||||
verifyPasswordOrDummy: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.mock("@/lib/auth/audit", () => ({
|
||||
auditLog: vi.fn().mockResolvedValue(undefined),
|
||||
}));
|
||||
|
||||
vi.mock("@/lib/rate-limit", () => ({
|
||||
checkAuthSurfaceRateLimit: vi.fn().mockResolvedValue({
|
||||
allowed: true,
|
||||
remaining: 5,
|
||||
reset: 0,
|
||||
ip: "1.2.3.4",
|
||||
}),
|
||||
rateLimitHeaders: vi.fn(() => ({})),
|
||||
}));
|
||||
|
||||
vi.mock("@/lib/db-compat", () => ({
|
||||
ensureDbCompatibility: vi.fn().mockResolvedValue(undefined),
|
||||
}));
|
||||
|
||||
vi.mock("@/lib/auth/hmac", () => ({
|
||||
hashToken: vi.fn(() => "aa".repeat(32)),
|
||||
}));
|
||||
|
||||
vi.mock("@/lib/logging/transports", () => ({ emitIfSampled: vi.fn() }));
|
||||
|
||||
vi.mock("next/headers", () => ({
|
||||
headers: vi.fn(async () => ({ get: () => null })),
|
||||
cookies: vi.fn(async () => ({
|
||||
get: () => undefined,
|
||||
set: () => {},
|
||||
delete: () => {},
|
||||
})),
|
||||
}));
|
||||
|
||||
import { POST } from "../route";
|
||||
import { prisma } from "@/lib/db";
|
||||
import { verifyPasswordOrDummy } from "@/lib/auth/password";
|
||||
|
||||
function makeRequest(): NextRequest {
|
||||
return new NextRequest("http://localhost/api/auth/login", {
|
||||
method: "POST",
|
||||
headers: { "content-type": "application/json" },
|
||||
body: JSON.stringify({
|
||||
email: "someone@example.com",
|
||||
password: "a-typed-password",
|
||||
}),
|
||||
});
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
vi.mocked(verifyPasswordOrDummy).mockReset();
|
||||
vi.mocked(verifyPasswordOrDummy).mockResolvedValue(false);
|
||||
});
|
||||
|
||||
describe("POST /api/auth/login — constant work across the refusals", () => {
|
||||
it("verifies even when no account matches the identifier", async () => {
|
||||
vi.mocked(prisma.user.findFirst).mockResolvedValue(null as never);
|
||||
|
||||
const res = await POST(makeRequest());
|
||||
|
||||
expect(res.status).toBe(401);
|
||||
expect(verifyPasswordOrDummy).toHaveBeenCalledTimes(1);
|
||||
expect(verifyPasswordOrDummy).toHaveBeenCalledWith(
|
||||
undefined,
|
||||
"a-typed-password",
|
||||
);
|
||||
});
|
||||
|
||||
it("verifies for a passkey-only account, which carries no password hash", async () => {
|
||||
vi.mocked(prisma.user.findFirst).mockResolvedValue({
|
||||
id: "user-1",
|
||||
passwordHash: null,
|
||||
totpConfirmedAt: null,
|
||||
mfaEnforced: false,
|
||||
} as never);
|
||||
|
||||
const res = await POST(makeRequest());
|
||||
|
||||
expect(res.status).toBe(401);
|
||||
expect(verifyPasswordOrDummy).toHaveBeenCalledTimes(1);
|
||||
expect(verifyPasswordOrDummy).toHaveBeenCalledWith(
|
||||
null,
|
||||
"a-typed-password",
|
||||
);
|
||||
});
|
||||
|
||||
it("verifies for an account that does carry a password hash", async () => {
|
||||
vi.mocked(prisma.user.findFirst).mockResolvedValue({
|
||||
id: "user-2",
|
||||
passwordHash: "$argon2id$stored",
|
||||
totpConfirmedAt: null,
|
||||
mfaEnforced: false,
|
||||
} as never);
|
||||
|
||||
const res = await POST(makeRequest());
|
||||
|
||||
expect(res.status).toBe(401);
|
||||
expect(verifyPasswordOrDummy).toHaveBeenCalledTimes(1);
|
||||
expect(verifyPasswordOrDummy).toHaveBeenCalledWith(
|
||||
"$argon2id$stored",
|
||||
"a-typed-password",
|
||||
);
|
||||
});
|
||||
|
||||
it("answers the three refusals with the identical status and body", async () => {
|
||||
const bodies: string[] = [];
|
||||
const statuses: number[] = [];
|
||||
const accounts = [
|
||||
null,
|
||||
{ id: "user-1", passwordHash: null },
|
||||
{ id: "user-2", passwordHash: "$argon2id$stored" },
|
||||
];
|
||||
for (const account of accounts) {
|
||||
vi.mocked(prisma.user.findFirst).mockResolvedValue(account as never);
|
||||
const res = await POST(makeRequest());
|
||||
statuses.push(res.status);
|
||||
bodies.push(await res.text());
|
||||
}
|
||||
expect(new Set(statuses)).toEqual(new Set([401]));
|
||||
expect(new Set(bodies).size).toBe(1);
|
||||
});
|
||||
});
|
||||
@@ -14,7 +14,7 @@ vi.mock("@/lib/db", () => ({
|
||||
}));
|
||||
|
||||
vi.mock("@/lib/auth/password", () => ({
|
||||
verifyPassword: vi.fn(),
|
||||
verifyPasswordOrDummy: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.mock("@/lib/auth/mfa-enrollment", () => ({
|
||||
@@ -73,7 +73,7 @@ vi.mock("next/headers", () => ({
|
||||
|
||||
import { POST } from "../route";
|
||||
import { prisma } from "@/lib/db";
|
||||
import { verifyPassword } from "@/lib/auth/password";
|
||||
import { verifyPasswordOrDummy } from "@/lib/auth/password";
|
||||
import {
|
||||
checkRateLimit,
|
||||
checkAuthSurfaceRateLimit,
|
||||
@@ -108,7 +108,7 @@ beforeEach(() => {
|
||||
vi.resetAllMocks();
|
||||
vi.mocked(prisma.user.findFirst).mockResolvedValue(FAKE_USER as never);
|
||||
vi.mocked(prisma.webauthnMfaCredential.count).mockResolvedValue(0 as never);
|
||||
vi.mocked(verifyPassword).mockResolvedValue(true);
|
||||
vi.mocked(verifyPasswordOrDummy).mockResolvedValue(true);
|
||||
vi.mocked(prisma.apiToken.create).mockResolvedValue({ id: "tok-1" } as never);
|
||||
vi.mocked(prisma.refreshToken.create).mockResolvedValue({
|
||||
id: "rt-1",
|
||||
|
||||
@@ -17,7 +17,7 @@ vi.mock("@/lib/db", () => ({
|
||||
}));
|
||||
|
||||
vi.mock("@/lib/auth/password", () => ({
|
||||
verifyPassword: vi.fn(),
|
||||
verifyPasswordOrDummy: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.mock("@/lib/auth/session", () => ({
|
||||
@@ -71,7 +71,7 @@ vi.mock("next/headers", () => ({
|
||||
|
||||
import { POST } from "../route";
|
||||
import { prisma } from "@/lib/db";
|
||||
import { verifyPassword } from "@/lib/auth/password";
|
||||
import { verifyPasswordOrDummy } from "@/lib/auth/password";
|
||||
import { auditLog } from "@/lib/auth/audit";
|
||||
import {
|
||||
checkRateLimit,
|
||||
@@ -93,7 +93,7 @@ function makeRequest(): NextRequest {
|
||||
beforeEach(() => {
|
||||
vi.resetAllMocks();
|
||||
vi.mocked(prisma.user.findFirst).mockResolvedValue(null as never);
|
||||
vi.mocked(verifyPassword).mockResolvedValue(false);
|
||||
vi.mocked(verifyPasswordOrDummy).mockResolvedValue(false);
|
||||
vi.mocked(auditLog).mockResolvedValue(undefined as never);
|
||||
vi.mocked(checkRateLimit).mockResolvedValue({
|
||||
allowed: true,
|
||||
@@ -175,7 +175,7 @@ describe("POST /api/auth/login — OIDC_ONLY server-side enforcement", () => {
|
||||
|
||||
const res = await POST(makeRequest());
|
||||
// Falls through to normal password verification (which fails here
|
||||
// since verifyPassword is mocked to return false) rather than 403 —
|
||||
// since verifyPasswordOrDummy is mocked to return false) rather than 403 —
|
||||
// a half-set OIDC group must never lock everyone out.
|
||||
expect(res.status).toBe(401);
|
||||
});
|
||||
|
||||
@@ -8,7 +8,7 @@ vi.mock("@/lib/db", () => ({
|
||||
},
|
||||
}));
|
||||
|
||||
vi.mock("@/lib/auth/password", () => ({ verifyPassword: vi.fn() }));
|
||||
vi.mock("@/lib/auth/password", () => ({ verifyPasswordOrDummy: vi.fn() }));
|
||||
|
||||
vi.mock("@/lib/auth/login-response", () => ({
|
||||
finishLogin: vi.fn(async () => new Response(null, { status: 200 })),
|
||||
@@ -58,7 +58,7 @@ vi.mock("next/headers", () => ({
|
||||
|
||||
import { POST } from "../route";
|
||||
import { prisma } from "@/lib/db";
|
||||
import { verifyPassword } from "@/lib/auth/password";
|
||||
import { verifyPasswordOrDummy } from "@/lib/auth/password";
|
||||
import { finishLogin } from "@/lib/auth/login-response";
|
||||
import { createMfaChallenge } from "@/lib/auth/mfa/challenge";
|
||||
import { consumeTrustedDevice } from "@/lib/auth/trusted-device";
|
||||
@@ -88,7 +88,7 @@ beforeEach(() => {
|
||||
|
||||
describe("POST /api/auth/login — trusted device", () => {
|
||||
it("skips the second factor when the device is trusted (no challenge minted)", async () => {
|
||||
vi.mocked(verifyPassword).mockResolvedValue(true);
|
||||
vi.mocked(verifyPasswordOrDummy).mockResolvedValue(true);
|
||||
vi.mocked(consumeTrustedDevice).mockResolvedValue(true);
|
||||
|
||||
const res = await POST(req());
|
||||
@@ -103,7 +103,7 @@ describe("POST /api/auth/login — trusted device", () => {
|
||||
});
|
||||
|
||||
it("falls back to the MFA challenge when the device is not trusted", async () => {
|
||||
vi.mocked(verifyPassword).mockResolvedValue(true);
|
||||
vi.mocked(verifyPasswordOrDummy).mockResolvedValue(true);
|
||||
vi.mocked(consumeTrustedDevice).mockResolvedValue(false);
|
||||
|
||||
const res = await POST(req());
|
||||
@@ -115,7 +115,7 @@ describe("POST /api/auth/login — trusted device", () => {
|
||||
});
|
||||
|
||||
it("still requires the password — a wrong password never consults the trusted device", async () => {
|
||||
vi.mocked(verifyPassword).mockResolvedValue(false);
|
||||
vi.mocked(verifyPasswordOrDummy).mockResolvedValue(false);
|
||||
|
||||
const res = await POST(req());
|
||||
expect(res.status).toBe(401);
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
import { prisma } from "@/lib/db";
|
||||
import { loginPasswordSchema } from "@/lib/validations/auth";
|
||||
import { verifyPassword } from "@/lib/auth/password";
|
||||
import { verifyPasswordOrDummy } from "@/lib/auth/password";
|
||||
import { auditLog } from "@/lib/auth/audit";
|
||||
import { hashToken } from "@/lib/auth/hmac";
|
||||
import {
|
||||
@@ -84,6 +84,18 @@ export const POST = apiHandler(async (request: NextRequest) => {
|
||||
},
|
||||
});
|
||||
|
||||
// One verification for every outcome. An unknown identifier and an
|
||||
// account carrying no password hash (a passkey-only account) used to
|
||||
// return here before any hashing, so the three answers cost one indexed
|
||||
// SELECT, one indexed SELECT, and a full Argon2id verification
|
||||
// respectively — identical bodies, three different durations, which is a
|
||||
// readable answer to "is this address registered, and does it have a
|
||||
// password". `verifyPasswordOrDummy` verifies against a fixed dummy hash
|
||||
// minted at the same cost parameters when there is no stored hash, and
|
||||
// returns false. Keep the call ahead of the branch: moving it back under
|
||||
// an `if` restores the channel.
|
||||
const valid = await verifyPasswordOrDummy(user?.passwordHash, password);
|
||||
|
||||
if (!user || !user.passwordHash) {
|
||||
// v1.4.43 W3-SECURITY (H-1): never write the typed identifier into
|
||||
// the audit row — `reason` already tells the operator what
|
||||
@@ -98,7 +110,6 @@ export const POST = apiHandler(async (request: NextRequest) => {
|
||||
return apiError("Invalid credentials", 401);
|
||||
}
|
||||
|
||||
const valid = await verifyPassword(user.passwordHash, password);
|
||||
if (!valid) {
|
||||
await auditLog("auth.login.failed", {
|
||||
userId: user.id,
|
||||
|
||||
@@ -0,0 +1,93 @@
|
||||
/**
|
||||
* Signing in must cost the same whether or not the account exists.
|
||||
*
|
||||
* The login route used to return the moment it found no account, or an
|
||||
* account with no password hash, before it reached the verifier. An account
|
||||
* that did have a password paid a full Argon2id verification. Same status,
|
||||
* same body, very different durations — so the clock answered "is this
|
||||
* address registered, and does it carry a password", which is the question
|
||||
* the removed discovery endpoint used to answer out loud.
|
||||
*
|
||||
* What this file pins is the work, not the wall clock: that the empty arms
|
||||
* reach the verifier at all, that the hash they verify against was minted at
|
||||
* the same cost parameters every real hash carries, and that no outcome on
|
||||
* those arms can be read as a successful verification. A unit test cannot
|
||||
* prove two durations are equal — it can only prove the same call is made.
|
||||
*
|
||||
* `@node-rs/argon2` is mocked so the assertions are about which calls happen
|
||||
* with which arguments, and so the suite does not spend real Argon2 time.
|
||||
*/
|
||||
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.
|
||||
const { hashMock, verifyMock } = vi.hoisted(() => ({
|
||||
hashMock: vi.fn(async (input: string) => `hashed:${input}`),
|
||||
verifyMock: vi.fn(async (_hash: string, _password: string) => false),
|
||||
}));
|
||||
|
||||
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(() => {
|
||||
verifyMock.mockReset();
|
||||
verifyMock.mockResolvedValue(false);
|
||||
});
|
||||
|
||||
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.
|
||||
expect(hashMock).toHaveBeenCalledWith(
|
||||
expect.any(String),
|
||||
ARGON2_HASH_OPTIONS,
|
||||
);
|
||||
});
|
||||
|
||||
it("verifies against the stored hash when the account has one", async () => {
|
||||
verifyMock.mockResolvedValueOnce(true);
|
||||
await expect(
|
||||
verifyPasswordOrDummy("$argon2id$stored", "correct horse"),
|
||||
).resolves.toBe(true);
|
||||
expect(verifyMock).toHaveBeenCalledTimes(1);
|
||||
expect(verifyMock).toHaveBeenCalledWith(
|
||||
"$argon2id$stored",
|
||||
"correct horse",
|
||||
);
|
||||
});
|
||||
|
||||
it("still verifies when there is no account at all (null hash)", async () => {
|
||||
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,
|
||||
);
|
||||
});
|
||||
|
||||
it("still verifies for a passkey-only account (undefined hash)", async () => {
|
||||
// The passkey-only case is the one that matters most: if it took the
|
||||
// cheap path the channel would still answer "this account has no
|
||||
// password", which is half of what the removed endpoint published.
|
||||
await expect(verifyPasswordOrDummy(undefined, "anything")).resolves.toBe(
|
||||
false,
|
||||
);
|
||||
expect(verifyMock).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("never reports success from the dummy arm, whatever the verifier says", async () => {
|
||||
verifyMock.mockResolvedValue(true);
|
||||
await expect(verifyPasswordOrDummy(null, "anything")).resolves.toBe(false);
|
||||
await expect(verifyPasswordOrDummy(undefined, "anything")).resolves.toBe(
|
||||
false,
|
||||
);
|
||||
});
|
||||
|
||||
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);
|
||||
});
|
||||
});
|
||||
@@ -20,6 +20,62 @@ export async function verifyPassword(
|
||||
return verify(storedHash, password);
|
||||
}
|
||||
|
||||
/**
|
||||
* A stand-in stored hash for the login arms that have nothing to verify.
|
||||
*
|
||||
* Sign-in used to return the moment it found no account, or an account with
|
||||
* no password — one indexed SELECT — while an account that did have a
|
||||
* password paid a full Argon2id verification at 19 MiB and t=2. The two
|
||||
* answers were byte-identical and the two costs were not, so the clock told
|
||||
* a caller which addresses are registered and which of those carry a
|
||||
* 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.
|
||||
*
|
||||
* 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
|
||||
* verifies against it discards the verdict and returns false. There is no
|
||||
* 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);
|
||||
|
||||
/**
|
||||
* Verify `password` against `storedHash`, or against the dummy hash when
|
||||
* there is no stored hash to verify against.
|
||||
*
|
||||
* Returns false whenever `storedHash` is absent — an account without a
|
||||
* password, or no account at all. The dummy verification's own verdict is
|
||||
* thrown away, so a false here is never a rejected credential being
|
||||
* mistaken for anything else, and a true can only come from a real stored
|
||||
* hash.
|
||||
*
|
||||
* Callers on an anonymous surface should use this rather than branching on
|
||||
* the absent hash themselves: the branch is what the clock reads.
|
||||
*/
|
||||
export async function verifyPasswordOrDummy(
|
||||
storedHash: string | null | undefined,
|
||||
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);
|
||||
return false;
|
||||
}
|
||||
|
||||
export interface PasswordStrength {
|
||||
score: number; // 0-4
|
||||
feedback: string[];
|
||||
|
||||
Reference in new issue
Block a user