From 4059fc30d659134208e0d5233b8bf29975df2402 Mon Sep 17 00:00:00 2001 From: Josh Wright Date: Fri, 9 Oct 2026 16:34:32 -0400 Subject: [PATCH] Queue password checks and shorten busy retries (Refs #95) --- docs/guides/authentication.md | 5 + src/lib/server/auth/http.ts | 12 ++- src/lib/server/auth/passwords.js | 65 ++++++++++-- tests/unit/auth-passwords.test.ts | 163 ++++++++++++++++++++++++++++++ tests/unit/auth-users.test.ts | 45 +++++++-- 5 files changed, 270 insertions(+), 20 deletions(-) create mode 100644 tests/unit/auth-passwords.test.ts diff --git a/docs/guides/authentication.md b/docs/guides/authentication.md index 6de6b4a..54325b5 100644 --- a/docs/guides/authentication.md +++ b/docs/guides/authentication.md @@ -76,6 +76,11 @@ Login, initial setup and password verification are rate-limited with persistent changes and successful management requests without recording passwords, tokens or request bodies. Administrators can review recent entries in Users. +The password service runs at most two scrypt operations at once and queues up to +16 more for up to two seconds. If that queue fills or the wait expires, the +request returns HTTP 429 with `Retry-After: 2`. The persistent rate limits still +return `Retry-After: 900` when an account or address reaches its attempt limit. + An administrator can reset another user's password from Users. If the only administrator forgets their password, an operator with local controller/container access can run the recovery command in an interactive terminal: diff --git a/src/lib/server/auth/http.ts b/src/lib/server/auth/http.ts index 33866c6..36486cd 100644 --- a/src/lib/server/auth/http.ts +++ b/src/lib/server/auth/http.ts @@ -1,5 +1,6 @@ import { error, isHttpError, json, type Cookies } from '@sveltejs/kit'; import { AuthError, SESSION_COOKIE, type AuthResult } from './index.js'; +import { PasswordServiceBusyError } from './passwords.js'; import type { AuthUser } from '../../auth.js'; export function sessionCookie(url: URL): string { @@ -85,7 +86,7 @@ export function authError(cause: unknown): Response { const status = cause instanceof AuthError ? cause.status - : message.includes('busy') + : cause instanceof PasswordServiceBusyError ? 429 : message.startsWith('Use a password') ? 400 @@ -96,7 +97,14 @@ export function authError(cause: unknown): Response { status, headers: { 'Cache-Control': 'no-store', - ...(status === 429 ? { 'Retry-After': '900' } : {}) + ...(status === 429 + ? { + 'Retry-After': + cause instanceof PasswordServiceBusyError + ? String(cause.retryAfterSeconds) + : '900' + } + : {}) } } ); diff --git a/src/lib/server/auth/passwords.js b/src/lib/server/auth/passwords.js index b5ce762..fa4291f 100644 --- a/src/lib/server/auth/passwords.js +++ b/src/lib/server/auth/passwords.js @@ -3,7 +3,51 @@ import { randomBytes, scrypt, timingSafeEqual } from 'node:crypto'; // OWASP's scrypt baseline: 128 MiB, r=8, p=1. Bound concurrent work so // unauthenticated password attempts cannot allocate unlimited memory. const parameters = { N: 131072, r: 8, p: 1, maxmem: 256 * 1024 * 1024 }; +const maxActive = 2; +const maxQueued = 16; +const queueTimeoutMs = 2000; let active = 0; +/** @type {Array<{resolve: () => void, timer: ReturnType}>} */ +const queue = []; + +export class PasswordServiceBusyError extends Error { + constructor() { + super('Password service is busy. Try again shortly.'); + this.name = 'PasswordServiceBusyError'; + this.retryAfterSeconds = 2; + } +} + +/** @returns {Promise} */ +function acquireSlot() { + if (active < maxActive) { + active++; + return Promise.resolve(); + } + // Bound waiting requests as well as running hashes during sustained overload. + if (queue.length >= maxQueued) return Promise.reject(new PasswordServiceBusyError()); + return new Promise((resolve, reject) => { + const entry = { + resolve, + timer: setTimeout(() => { + queue.splice(queue.indexOf(entry), 1); + reject(new PasswordServiceBusyError()); + }, queueTimeoutMs) + }; + queue.push(entry); + }); +} + +function releaseSlot() { + const next = queue.shift(); + if (next) { + clearTimeout(next.timer); + // Transfer the occupied slot directly so new requests cannot jump the queue. + next.resolve(); + } else { + active--; + } +} /** @param {unknown} password */ export function validatePassword(password) { @@ -14,17 +58,18 @@ export function validatePassword(password) { } /** @param {string} password @param {string} salt @returns {Promise} */ -function derive(password, salt) { - if (active >= 2) - return Promise.reject(new Error('Password service is busy. Try again shortly.')); - active++; - return new Promise((resolve, reject) => { - scrypt(password, salt, 64, parameters, (error, result) => { - active--; - if (error) reject(error); - else resolve(result); +async function derive(password, salt) { + await acquireSlot(); + try { + return await new Promise((resolve, reject) => { + scrypt(password, salt, 64, parameters, (error, result) => { + if (error) reject(error); + else resolve(result); + }); }); - }); + } finally { + releaseSlot(); + } } /** @param {unknown} password */ diff --git a/tests/unit/auth-passwords.test.ts b/tests/unit/auth-passwords.test.ts new file mode 100644 index 0000000..324c7b2 --- /dev/null +++ b/tests/unit/auth-passwords.test.ts @@ -0,0 +1,163 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +const { scrypt } = vi.hoisted(() => ({ scrypt: vi.fn() })); +vi.mock('node:crypto', async (importOriginal) => ({ + ...(await importOriginal()), + scrypt +})); + +let passwords: typeof import('../../src/lib/server/auth/passwords.js'); +let running: Array<(error: Error | null, result: Buffer) => void>; +const key = Buffer.alloc(64, 7); +const salt = 'a'.repeat(64); +const encoded = `scrypt$131072$8$1$${salt}$${key.toString('hex')}`; + +beforeEach(async () => { + vi.resetModules(); + vi.useFakeTimers(); + running = []; + scrypt.mockReset().mockImplementation((_password, _salt, _length, _parameters, callback) => { + running.push(callback); + }); + passwords = await import('../../src/lib/server/auth/passwords.js'); +}); +afterEach(() => { + vi.useRealTimers(); +}); + +function finishNext(error: Error | null = null) { + const callback = running.shift(); + if (!callback) throw new Error('No running scrypt operation'); + callback(error, key); +} + +describe('password work queue', () => { + it('shares two slots across hashing and verification and serves waiting requests in order', async () => { + const first = passwords.hashPassword('first password'); + const second = passwords.verifyPassword('second password', encoded); + const third = passwords.verifyPassword('unknown account', null); + const fourth = passwords.hashPassword('fourth password'); + await Promise.resolve(); + expect(scrypt).toHaveBeenCalledTimes(2); + expect(vi.getTimerCount()).toBe(2); + + finishNext(); + const fifth = passwords.verifyPassword('later arrival', encoded); + await first; + expect(scrypt).toHaveBeenCalledTimes(3); + expect(scrypt.mock.calls[2]).toEqual([ + 'unknown account', + '0'.repeat(64), + 64, + { N: 131072, r: 8, p: 1, maxmem: 256 * 1024 * 1024 }, + expect.any(Function) + ]); + expect(running).toHaveLength(2); + + finishNext(); + expect(await second).toBe(true); + expect(scrypt.mock.calls[3][0]).toBe('fourth password'); + expect(running).toHaveLength(2); + + finishNext(); + expect(await third).toBe(false); + expect(scrypt.mock.calls[4][0]).toBe('later arrival'); + expect(running).toHaveLength(2); + expect(vi.getTimerCount()).toBe(0); + + finishNext(); + finishNext(); + await expect(fourth).resolves.toMatch(/^scrypt\$/); + await expect(fifth).resolves.toBe(true); + expect(running).toHaveLength(0); + }); + + it('expires waiting requests after two seconds without deriving them or blocking later work', async () => { + const first = passwords.hashPassword('first password'); + const second = passwords.hashPassword('second password'); + const expired = expect( + passwords.verifyPassword('expired password', encoded) + ).rejects.toBeInstanceOf(passwords.PasswordServiceBusyError); + await vi.advanceTimersByTimeAsync(1999); + expect(vi.getTimerCount()).toBe(1); + expect(scrypt).toHaveBeenCalledTimes(2); + await vi.advanceTimersByTimeAsync(1); + await expired; + expect(vi.getTimerCount()).toBe(0); + + const later = passwords.verifyPassword('later password', encoded); + finishNext(); + await first; + expect(scrypt).toHaveBeenCalledTimes(3); + expect(scrypt.mock.calls[2][0]).toBe('later password'); + expect(vi.getTimerCount()).toBe(0); + finishNext(); + finishNext(); + await second; + await expect(later).resolves.toBe(true); + }); + + it('bounds the waiting queue and recovers capacity after overload', async () => { + const active = [ + passwords.hashPassword('first password'), + passwords.hashPassword('second password') + ]; + const queued = Promise.allSettled( + Array.from({ length: 16 }, (_, index) => + passwords.verifyPassword(`queued ${index}`, encoded) + ) + ); + await expect(passwords.hashPassword('overflow password')).rejects.toBeInstanceOf( + passwords.PasswordServiceBusyError + ); + expect(scrypt).toHaveBeenCalledTimes(2); + expect(vi.getTimerCount()).toBe(16); + + await vi.advanceTimersByTimeAsync(2000); + for (const result of await queued) { + expect(result.status).toBe('rejected'); + if (result.status === 'rejected') + expect(result.reason).toBeInstanceOf(passwords.PasswordServiceBusyError); + } + expect(scrypt).toHaveBeenCalledTimes(2); + expect(vi.getTimerCount()).toBe(0); + finishNext(); + finishNext(); + await Promise.all(active); + + const recovered = [ + passwords.hashPassword('recovered password'), + passwords.verifyPassword('recovered verification', encoded) + ]; + await Promise.resolve(); + expect(running).toHaveLength(2); + expect(vi.getTimerCount()).toBe(0); + finishNext(); + finishNext(); + await expect(Promise.all(recovered)).resolves.toEqual([expect.any(String), true]); + }); + + it.each(['callback', 'throw'] as const)( + 'releases a slot after a scrypt %s failure', + async (mode) => { + const failure = new Error('scrypt failed'); + if (mode === 'throw') + scrypt.mockImplementationOnce(() => { + throw failure; + }); + const failed = expect(passwords.hashPassword('failing password')).rejects.toBe(failure); + const second = passwords.hashPassword('second password'); + const queued = passwords.verifyPassword('queued password', encoded); + await Promise.resolve(); + if (mode === 'callback') finishNext(failure); + await failed; + expect(scrypt).toHaveBeenCalledTimes(3); + expect(running).toHaveLength(2); + expect(vi.getTimerCount()).toBe(0); + finishNext(); + finishNext(); + await second; + await expect(queued).resolves.toBe(true); + } + ); +}); diff --git a/tests/unit/auth-users.test.ts b/tests/unit/auth-users.test.ts index fef1bfb..24c6c27 100644 --- a/tests/unit/auth-users.test.ts +++ b/tests/unit/auth-users.test.ts @@ -193,15 +193,44 @@ describe('password storage and verification', () => { expect(await verifyPassword('a'.repeat(129), passwordHash)).toBe(false); }); - it('bounds concurrent scrypt work and releases capacity when hashes finish', async () => { - const { hashPassword } = await import('../../src/lib/server/auth/passwords.js'); - const first = hashPassword(password), - second = hashPassword(replacement); - await expect(hashPassword('third sufficiently long passphrase')).rejects.toThrow('busy'); - await expect(Promise.all([first, second])).resolves.toHaveLength(2); - await expect(hashPassword('subsequent sufficiently long passphrase')).resolves.toMatch( - /^scrypt\$/ + it('allows three simultaneous logins to complete with production-strength scrypt', async () => { + const users = [seedUser('first'), seedUser('second'), seedUser('third')]; + const results = await Promise.all( + users.map((user, index) => + auth.authenticatePassword( + { username: user.username, password }, + `192.0.2.${index + 1}` + ) + ) ); + expect(results.map((result) => result.user.id)).toEqual(users.map((user) => user.id)); + for (const result of results) + expect(auth.readSession(result.session.token)?.user.id).toBe(result.user.id); + }); + + it('uses a short retry for password capacity while preserving the rate-limit retry', async () => { + const { PasswordServiceBusyError } = await import('../../src/lib/server/auth/passwords.js'); + const { authError } = await import('../../src/lib/server/auth/http.js'); + const busy = authError(new PasswordServiceBusyError()); + expect(busy.status).toBe(429); + expect(busy.headers.get('Retry-After')).toBe('2'); + expect(busy.headers.get('Cache-Control')).toBe('no-store'); + expect(await busy.json()).toEqual({ + error: 'Password service is busy. Try again shortly.' + }); + + const limited = authError( + new auth.AuthError('Too many attempts. Try again in 15 minutes.', 429) + ); + expect(limited.status).toBe(429); + expect(limited.headers.get('Retry-After')).toBe('900'); + + const unexpected = authError(new Error('Database is busy with private details')); + expect(unexpected.status).toBe(500); + expect(unexpected.headers.has('Retry-After')).toBe(false); + expect(await unexpected.json()).toEqual({ + error: 'The authentication request could not be completed.' + }); }); it('returns the same login error for unknown, disabled and incorrect-password accounts', async () => {