Queue password checks and shorten busy retries (Refs #95)

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:

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'
+                      }
+                    : {})
             }
         }
     );

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<typeof setTimeout>}>} */
+const queue = [];
+
+export class PasswordServiceBusyError extends Error {
+    constructor() {
+        super('Password service is busy. Try again shortly.');
+        this.name = 'PasswordServiceBusyError';
+        this.retryAfterSeconds = 2;
+    }
+}
+
+/** @returns {Promise<void>} */
+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<Buffer>} */
-function derive(password, salt) {
+async function derive(password, salt) {
-    if (active >= 2)
+    await acquireSlot();
-        return Promise.reject(new Error('Password service is busy. Try again shortly.'));
+    try {
-    active++;
+        return await new Promise((resolve, reject) => {
-    return new Promise((resolve, reject) => {
             scrypt(password, salt, 64, parameters, (error, result) => {
-            active--;
                 if (error) reject(error);
                 else resolve(result);
             });
         });
+    } finally {
+        releaseSlot();
+    }
 }
 
 /** @param {unknown} password */

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<typeof import('node:crypto')>()),
+    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);
+        }
+    );
+});

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 () => {
+    it('allows three simultaneous logins to complete with production-strength scrypt', async () => {
-        const { hashPassword } = await import('../../src/lib/server/auth/passwords.js');
+        const users = [seedUser('first'), seedUser('second'), seedUser('third')];
-        const first = hashPassword(password),
+        const results = await Promise.all(
-            second = hashPassword(replacement);
+            users.map((user, index) =>
-        await expect(hashPassword('third sufficiently long passphrase')).rejects.toThrow('busy');
+                auth.authenticatePassword(
-        await expect(Promise.all([first, second])).resolves.toHaveLength(2);
+                    { username: user.username, password },
-        await expect(hashPassword('subsequent sufficiently long passphrase')).resolves.toMatch(
+                    `192.0.2.${index + 1}`
-            /^scrypt\$/
+                )
+            )
         );
+        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 () => {