From ff0d2dc6e1062701d3430c609c786a313bb7197a Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Sat, 31 Oct 2026 23:07:12 +0100 Subject: [PATCH] feat: stop sending a refused stored password from any instance once one has seen it refused #1066 --- app/api/account-archive/route.ts | 6 +- app/api/auth/stalwart-context/route.ts | 2 +- .../shared-stored-credential-budget.test.ts | 84 +++++++++++ .../stored-credential-budget.test.ts | 46 +++--- lib/stalwart/server-fetch.ts | 4 +- lib/stalwart/stored-credential-budget.ts | 140 +++++++++++++++--- 6 files changed, 235 insertions(+), 47 deletions(-) create mode 100644 lib/__tests__/shared-stored-credential-budget.test.ts diff --git a/app/api/account-archive/route.ts b/app/api/account-archive/route.ts index 9c77f6ce8..aff396ef9 100644 --- a/app/api/account-archive/route.ts +++ b/app/api/account-archive/route.ts @@ -49,12 +49,12 @@ async function verifiedOwner(request: NextRequest): Promise('jmapServerUrl', ''), parseJmapServers(configManager.get('jmapServers', []))); - if (isStoredCredentialRefused(ctx.authHeader)) return denied; + if (await isStoredCredentialRefused(ctx.authHeader)) return denied; await verifyJmapAuth(ctx.serverUrl, ctx.authHeader, { trusted: !!trusted, clientIp: loginClientAddress(request) }); - recordStoredCredentialStatus(ctx.authHeader, 200); + await recordStoredCredentialStatus(ctx.authHeader, 200); } catch (error) { if (error instanceof JmapAuthVerificationError && error.upstreamStatus === 401) { - recordStoredCredentialStatus(ctx.authHeader, 401); + await recordStoredCredentialStatus(ctx.authHeader, 401); } return denied; } diff --git a/app/api/auth/stalwart-context/route.ts b/app/api/auth/stalwart-context/route.ts index b82ce7c1b..c2afc77b6 100644 --- a/app/api/auth/stalwart-context/route.ts +++ b/app/api/auth/stalwart-context/route.ts @@ -103,7 +103,7 @@ export async function POST(request: NextRequest) { } await refund(); // Just signed in: an older refusal of this credential no longer applies. - recordStoredCredentialStatus(authHeader, 200); + await recordStoredCredentialStatus(authHeader, 200); // A token sign-in's stamp (lib/auth/session-age.ts) knows no account: // the token routes never see one. The first verified account names it, diff --git a/lib/__tests__/shared-stored-credential-budget.test.ts b/lib/__tests__/shared-stored-credential-budget.test.ts new file mode 100644 index 000000000..8d0270f0c --- /dev/null +++ b/lib/__tests__/shared-stored-credential-budget.test.ts @@ -0,0 +1,84 @@ +// @vitest-environment node +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { + isStoredCredentialRefused, + recordStoredCredentialStatus, + resetStoredCredentialBudgetForTests, +} from '@/lib/stalwart/stored-credential-budget'; +import { setStoreForTests } from '@/lib/shared-store'; +import { resetLocalStoreForTests } from '@/lib/shared-store/local'; +import { switchableSharedStore } from './helpers/shared-store-down'; + +// Each replica kept its own list of refused stored credentials, so a stale +// password was sent to the mail server once per replica and hour, and the +// global ceiling of 20 refusals became 20 per replica (#1066). + +const basic = (user: string) => `Basic ${Buffer.from(`${user}:secret`).toString('base64')}`; +const MIN = 60 * 1000; +const T0 = 1_800_000_000_000; +let store: ReturnType; + +/** Another replica: this process forgets everything it kept itself. */ +const otherReplica = () => resetStoredCredentialBudgetForTests(); + +beforeEach(() => { + resetLocalStoreForTests(); + resetStoredCredentialBudgetForTests(); + store = switchableSharedStore(); + setStoreForTests(store); +}); + +afterEach(() => { + setStoreForTests(null); + resetLocalStoreForTests(); + resetStoredCredentialBudgetForTests(); +}); + +describe('stored credential budget with a shared store', () => { + it('refuses on every replica a credential one replica saw refused', async () => { + await recordStoredCredentialStatus(basic('stale'), 401, T0); + otherReplica(); + expect(await isStoredCredentialRefused(basic('stale'), T0 + 59 * MIN)).toBe(true); + expect(await isStoredCredentialRefused(basic('other'), T0 + 59 * MIN)).toBe(false); + expect(await isStoredCredentialRefused(basic('stale'), T0 + 61 * MIN)).toBe(false); + }); + + it('counts the 20 refusals of the window across replicas', async () => { + await recordStoredCredentialStatus(basic('active'), 200, T0); + for (let i = 0; i < 20; i++) { + if (i % 5 === 0) otherReplica(); + await recordStoredCredentialStatus(basic(`stale-${i}`), 401, T0 + i); + } + otherReplica(); + expect(await isStoredCredentialRefused(basic('fresh'), T0 + MIN)).toBe(true); + // Worked within the window on another replica: still goes upstream. + expect(await isStoredCredentialRefused(basic('active'), T0 + MIN)).toBe(false); + expect(await isStoredCredentialRefused(basic('fresh'), T0 + 16 * MIN)).toBe(false); + }); + + it('lifts a refusal everywhere when the credential signs in again', async () => { + await recordStoredCredentialStatus(basic('a'), 401, T0); + otherReplica(); + await recordStoredCredentialStatus(basic('a'), 200, T0 + MIN); + otherReplica(); + expect(await isStoredCredentialRefused(basic('a'), T0 + 2 * MIN)).toBe(false); + }); + + it('writes a working credential to the store at most once a minute', async () => { + const set = vi.spyOn(store.kv, 'set'); + for (let i = 0; i < 5; i++) await recordStoredCredentialStatus(basic('busy'), 200, T0 + i * 1000); + expect(set).toHaveBeenCalledTimes(1); + await recordStoredCredentialStatus(basic('busy'), 200, T0 + MIN); + expect(set).toHaveBeenCalledTimes(2); + }); + + it('falls back to what this replica knows when the store is down', async () => { + await recordStoredCredentialStatus(basic('stale'), 401, T0); + store.down = true; + expect(await isStoredCredentialRefused(basic('stale'), T0 + MIN)).toBe(true); + expect(await isStoredCredentialRefused(basic('good'), T0 + MIN)).toBe(false); + // Recorded here even though the store can't take it. + await recordStoredCredentialStatus(basic('later'), 401, T0 + MIN); + expect(await isStoredCredentialRefused(basic('later'), T0 + 2 * MIN)).toBe(true); + }); +}); diff --git a/lib/__tests__/stored-credential-budget.test.ts b/lib/__tests__/stored-credential-budget.test.ts index ef79876ef..effea4b42 100644 --- a/lib/__tests__/stored-credential-budget.test.ts +++ b/lib/__tests__/stored-credential-budget.test.ts @@ -18,40 +18,40 @@ const MIN = 60 * 1000; beforeEach(() => resetStoredCredentialBudgetForTests()); describe('stored credential budget', () => { - it('refuses a credential for an hour after the server refused it', () => { + it('refuses a credential for an hour after the server refused it', async () => { const t0 = 1_000_000; - recordStoredCredentialStatus(basic('a'), 401, t0); - expect(isStoredCredentialRefused(basic('a'), t0 + 59 * MIN)).toBe(true); - expect(isStoredCredentialRefused(basic('b'), t0 + 59 * MIN)).toBe(false); - expect(isStoredCredentialRefused(basic('a'), t0 + 61 * MIN)).toBe(false); + await recordStoredCredentialStatus(basic('a'), 401, t0); + expect(await isStoredCredentialRefused(basic('a'), t0 + 59 * MIN)).toBe(true); + expect(await isStoredCredentialRefused(basic('b'), t0 + 59 * MIN)).toBe(false); + expect(await isStoredCredentialRefused(basic('a'), t0 + 61 * MIN)).toBe(false); }); - it('past 20 failures in 15 minutes passes only credentials that worked in that time', () => { + it('past 20 failures in 15 minutes passes only credentials that worked in that time', async () => { const t0 = 1_000_000; - recordStoredCredentialStatus(basic('active'), 200, t0); - recordStoredCredentialStatus(basic('idle'), 200, t0 - 20 * MIN); - for (let i = 0; i < 19; i++) recordStoredCredentialStatus(basic(`stale-${i}`), 401, t0 + i); - expect(isStoredCredentialRefused(basic('fresh'), t0 + MIN)).toBe(false); + await recordStoredCredentialStatus(basic('active'), 200, t0); + await recordStoredCredentialStatus(basic('idle'), 200, t0 - 20 * MIN); + for (let i = 0; i < 19; i++) await recordStoredCredentialStatus(basic(`stale-${i}`), 401, t0 + i); + expect(await isStoredCredentialRefused(basic('fresh'), t0 + MIN)).toBe(false); - recordStoredCredentialStatus(basic('stale-19'), 401, t0 + MIN); - expect(isStoredCredentialRefused(basic('fresh'), t0 + 2 * MIN)).toBe(true); - expect(isStoredCredentialRefused(basic('idle'), t0 + 2 * MIN)).toBe(true); - expect(isStoredCredentialRefused(basic('active'), t0 + 2 * MIN)).toBe(false); + await recordStoredCredentialStatus(basic('stale-19'), 401, t0 + MIN); + expect(await isStoredCredentialRefused(basic('fresh'), t0 + 2 * MIN)).toBe(true); + expect(await isStoredCredentialRefused(basic('idle'), t0 + 2 * MIN)).toBe(true); + expect(await isStoredCredentialRefused(basic('active'), t0 + 2 * MIN)).toBe(false); // The window ends 15 minutes after it started. - expect(isStoredCredentialRefused(basic('fresh'), t0 + 16 * MIN)).toBe(false); + expect(await isStoredCredentialRefused(basic('fresh'), t0 + 16 * MIN)).toBe(false); }); - it('a successful sign-in lifts an earlier refusal of the same credential', () => { - recordStoredCredentialStatus(basic('a'), 401, 1000); - recordStoredCredentialStatus(basic('a'), 200, 2000); - expect(isStoredCredentialRefused(basic('a'), 3000)).toBe(false); + it('a successful sign-in lifts an earlier refusal of the same credential', async () => { + await recordStoredCredentialStatus(basic('a'), 401, 1000); + await recordStoredCredentialStatus(basic('a'), 200, 2000); + expect(await isStoredCredentialRefused(basic('a'), 3000)).toBe(false); }); - it('leaves access tokens alone', () => { - for (let i = 0; i < 30; i++) recordStoredCredentialStatus(`Bearer expired-${i}`, 401, 1000); - expect(isStoredCredentialRefused('Bearer expired-0', 2000)).toBe(false); - expect(isStoredCredentialRefused(basic('a'), 2000)).toBe(false); + it('leaves access tokens alone', async () => { + for (let i = 0; i < 30; i++) await recordStoredCredentialStatus(`Bearer expired-${i}`, 401, 1000); + expect(await isStoredCredentialRefused('Bearer expired-0', 2000)).toBe(false); + expect(await isStoredCredentialRefused(basic('a'), 2000)).toBe(false); }); }); diff --git a/lib/stalwart/server-fetch.ts b/lib/stalwart/server-fetch.ts index 72c11bd6a..ebc402c86 100644 --- a/lib/stalwart/server-fetch.ts +++ b/lib/stalwart/server-fetch.ts @@ -70,9 +70,9 @@ export async function fetchJmapServer( trusted: boolean | undefined, ): Promise { const authHeader = new Headers(givenInit.headers).get('authorization'); - if (authHeader && isStoredCredentialRefused(authHeader)) return refusedCredentialResponse(); + if (authHeader && (await isStoredCredentialRefused(authHeader))) return refusedCredentialResponse(); const response = await fetchUnbudgeted(url, givenInit, trusted); - if (authHeader) recordStoredCredentialStatus(authHeader, response.status); + if (authHeader) await recordStoredCredentialStatus(authHeader, response.status); return response; } diff --git a/lib/stalwart/stored-credential-budget.ts b/lib/stalwart/stored-credential-budget.ts index da81a6ed4..d84283f1e 100644 --- a/lib/stalwart/stored-credential-budget.ts +++ b/lib/stalwart/stored-credential-budget.ts @@ -1,4 +1,6 @@ import { createHash } from 'node:crypto'; +import { getStore, isSharedMode } from '@/lib/shared-store'; +import { logger } from '@/lib/logger'; /** * Failure budget for the credentials the context cookies hold (extends @@ -18,27 +20,57 @@ import { createHash } from 'node:crypto'; * * Only Basic credentials count: Stalwart does not count an expired or unknown * access token as a failed login, and access tokens expire all the time. + * + * With a shared store (#1066) refusals and the failure window are kept there, + * so a credential one replica saw refused isn't tried again by the others. + * Successes are remembered in this process and written to the store at most + * once a minute per credential: this check runs on every server-side JMAP + * call. If the store can't be reached, this process's own copy decides, as a + * single instance does; refusing every call would take files, push previews + * and calendar sync down with the store. */ const REFUSE_MS = 60 * 60 * 1000; const WINDOW_MS = 15 * 60 * 1000; const GLOBAL_MAX_FAILURES = 20; const MAX_TRACKED = 10_000; +/** How often one process writes a credential's success to the shared store. */ +const SHARED_SUCCESS_EVERY_MS = 60 * 1000; +/** Shared keys outlive their window a little, so clock skew can't drop them early. */ +const SHARED_GRACE_MS = 60 * 1000; +const FAILURES_KEY = 'stored-credential:failures'; -/** Credential key -> time until which it is refused. */ -const refused = new Map(); -/** Credential key -> time it last got a successful answer. */ -const succeeded = new Map(); -let failureWindow = { failures: 0, resetAt: 0 }; +interface BudgetState { + /** Credential key -> time until which it is refused. */ + refused: Map; + /** Credential key -> time it last got a successful answer. */ + succeeded: Map; + failureWindow: { failures: number; resetAt: number }; + /** Credential key -> when this process last wrote its success to the shared store. */ + sharedSuccess: Map; +} + +// On globalThis: one copy per process, not one per bundle. +const STATE_KEY = Symbol.for('bulwark.storedCredentialBudget'); + +function state(): BudgetState { + const g = globalThis as typeof globalThis & { [STATE_KEY]?: BudgetState }; + g[STATE_KEY] ??= { refused: new Map(), succeeded: new Map(), failureWindow: { failures: 0, resetAt: 0 }, sharedSuccess: new Map() }; + return g[STATE_KEY]; +} function keyOf(authHeader: string): string | null { if (!/^basic /i.test(authHeader)) return null; return createHash('sha256').update(authHeader).digest('base64url'); } +const refusedKey = (key: string) => `stored-credential:refused:${key}`; +const succeededKey = (key: string) => `stored-credential:ok:${key}`; + function currentWindow(now: number) { - if (failureWindow.resetAt <= now) failureWindow = { failures: 0, resetAt: now + WINDOW_MS }; - return failureWindow; + const s = state(); + if (s.failureWindow.resetAt <= now) s.failureWindow = { failures: 0, resetAt: now + WINDOW_MS }; + return s.failureWindow; } function remember(map: Map, key: string, value: number): void { @@ -51,10 +83,10 @@ function remember(map: Map, key: string, value: number): void { map.set(key, value); } -/** True when `authHeader` must not be sent to the mail server right now. */ -export function isStoredCredentialRefused(authHeader: string, now: number = Date.now()): boolean { - const key = keyOf(authHeader); - if (!key) return false; +// ─── this process ────────────────────────────────────────────────────────── + +function isRefusedLocally(key: string, now: number): boolean { + const { refused, succeeded } = state(); const until = refused.get(key); if (until !== undefined) { if (until > now) return true; @@ -65,10 +97,8 @@ export function isStoredCredentialRefused(authHeader: string, now: number = Date return lastSuccess === undefined || now - lastSuccess > WINDOW_MS; } -/** Record the mail server's answer to a request that carried `authHeader`. */ -export function recordStoredCredentialStatus(authHeader: string, status: number, now: number = Date.now()): void { - const key = keyOf(authHeader); - if (!key) return; +function recordLocally(key: string, status: number, now: number): void { + const { refused, succeeded } = state(); if (status === 401) { succeeded.delete(key); remember(refused, key, now + REFUSE_MS); @@ -79,13 +109,87 @@ export function recordStoredCredentialStatus(authHeader: string, status: number, } } +// ─── shared store ────────────────────────────────────────────────────────── + +function isUnavailable(error: unknown): boolean { + return error instanceof Error && error.name === 'StoreUnavailableError'; +} + +async function isRefusedShared(key: string, now: number): Promise { + const store = getStore(); + const [until, window] = await Promise.all([store.kv.get(refusedKey(key)), store.counters.get(FAILURES_KEY, now)]); + const refusedUntil = Number(until); + if (until !== null && refusedUntil > now) { + // Kept here too, for when the store is down. + remember(state().refused, key, refusedUntil); + return true; + } + if ((window?.count ?? 0) < GLOBAL_MAX_FAILURES) return false; + const local = state().succeeded.get(key); + if (local !== undefined && now - local <= WINDOW_MS) return false; + const shared = Number(await store.kv.get(succeededKey(key))); + return !(shared > 0 && now - shared <= WINDOW_MS); +} + +async function recordShared(key: string, status: number, now: number, wasRefusedHere: boolean): Promise { + const store = getStore(); + const { sharedSuccess } = state(); + if (status === 401) { + sharedSuccess.delete(key); + await Promise.all([ + store.kv.set(refusedKey(key), String(now + REFUSE_MS), { ttlMs: REFUSE_MS + SHARED_GRACE_MS }), + store.kv.del(succeededKey(key)), + store.counters.hit(FAILURES_KEY, WINDOW_MS, now), + ]); + } else if (status >= 200 && status < 400) { + const last = sharedSuccess.get(key); + // A success that lifts a refusal is written at once; otherwise once a minute. + if (!wasRefusedHere && last !== undefined && now - last >= 0 && now - last < SHARED_SUCCESS_EVERY_MS) return; + remember(sharedSuccess, key, now); + await Promise.all([ + store.kv.del(refusedKey(key)), + store.kv.set(succeededKey(key), String(now), { ttlMs: WINDOW_MS + SHARED_GRACE_MS }), + ]); + } +} + +// ─── API ─────────────────────────────────────────────────────────────────── + +/** True when `authHeader` must not be sent to the mail server right now. */ +export async function isStoredCredentialRefused(authHeader: string, now: number = Date.now()): Promise { + const key = keyOf(authHeader); + if (!key) return false; + if (!isSharedMode()) return isRefusedLocally(key, now); + try { + return await isRefusedShared(key, now); + } catch (error) { + if (!isUnavailable(error)) throw error; + return isRefusedLocally(key, now); + } +} + +/** Record the mail server's answer to a request that carried `authHeader`. */ +export async function recordStoredCredentialStatus(authHeader: string, status: number, now: number = Date.now()): Promise { + const key = keyOf(authHeader); + if (!key) return; + const wasRefusedHere = state().refused.has(key); + // Always kept here: it is what decides while the shared store is down. + recordLocally(key, status, now); + if (!isSharedMode()) return; + try { + await recordShared(key, status, now, wasRefusedHere); + } catch (error) { + if (!isUnavailable(error)) throw error; + logger.debug('Stored credential status kept in this process only: the shared store is unavailable'); + } +} + /** The answer a refused credential gets instead of an upstream request. */ export function refusedCredentialResponse(): Response { return Response.json({ error: 'Authentication failed', reason: 'credential_refused' }, { status: 401 }); } export function resetStoredCredentialBudgetForTests(): void { - refused.clear(); - succeeded.clear(); - failureWindow = { failures: 0, resetAt: 0 }; + const g = globalThis as typeof globalThis & { [STATE_KEY]?: BudgetState }; + delete g[STATE_KEY]; }