diff --git a/app/api/admin/auth/route.ts b/app/api/admin/auth/route.ts index b167b68..bef3f8b 100644 --- a/app/api/admin/auth/route.ts +++ b/app/api/admin/auth/route.ts @@ -6,6 +6,7 @@ import { isAdminEnabled, COOKIE_NAME, SESSION_DURATION_MS, + MAX_PASSWORD_LENGTH, } from '@/lib/admin/auth'; import { checkRateLimit, getClientIp, retryAfterSeconds } from '@/lib/rate-limit'; @@ -37,6 +38,11 @@ export async function POST(request: NextRequest) { if (!body?.password || typeof body.password !== 'string') { return NextResponse.json({ error: 'Password is required' }, { status: 400 }); } + // Reject before hashing rather than after: nothing downstream should have to + // carry an unbounded request body. + if (body.password.length > MAX_PASSWORD_LENGTH) { + return NextResponse.json({ error: 'Password is too long' }, { status: 400 }); + } if (!verifyAdminPassword(body.password)) { return NextResponse.json({ error: 'Invalid password' }, { status: 401 }); diff --git a/lib/__tests__/admin-auth.test.ts b/lib/__tests__/admin-auth.test.ts index c49e126..b6de250 100644 --- a/lib/__tests__/admin-auth.test.ts +++ b/lib/__tests__/admin-auth.test.ts @@ -134,4 +134,28 @@ describe('verifyAdminPassword', () => { expect(verifyAdminPassword('')).toBe(false); expect(verifyAdminPassword('anything')).toBe(false); }); + + // The comparison used to run on the raw strings, so a length mismatch + // returned before timingSafeEqual could. That early exit is what leaked the + // length of ADMIN_PASSWORD; a wrong password of the *right* length has to be + // rejected by the same code path as any other. + it('rejects a wrong password of exactly the right length', async () => { + const { verifyAdminPassword } = await loadAuth(); + const sameLength = 'x'.repeat(ADMIN_PASSWORD.length); + expect(sameLength).toHaveLength(ADMIN_PASSWORD.length); + expect(sameLength).not.toBe(ADMIN_PASSWORD); + expect(verifyAdminPassword(sameLength)).toBe(false); + }); + + it('rejects an oversized attempt without hashing it', async () => { + const { verifyAdminPassword, MAX_PASSWORD_LENGTH } = await loadAuth(); + expect(verifyAdminPassword('x'.repeat(MAX_PASSWORD_LENGTH + 1))).toBe(false); + }); + + // A cap that a real passphrase could hit would be a lockout, not a fix. + it('still accepts the configured password when it is long', async () => { + const { verifyAdminPassword, MAX_PASSWORD_LENGTH } = await loadAuth(); + expect(ADMIN_PASSWORD.length).toBeLessThanOrEqual(MAX_PASSWORD_LENGTH); + expect(verifyAdminPassword(ADMIN_PASSWORD)).toBe(true); + }); }); diff --git a/lib/admin/auth.ts b/lib/admin/auth.ts index 66c3929..7a96588 100644 --- a/lib/admin/auth.ts +++ b/lib/admin/auth.ts @@ -11,6 +11,15 @@ import { cookies } from 'next/headers'; const COOKIE_NAME = 'folio_admin_session'; const SESSION_DURATION_MS = 24 * 60 * 60 * 1000; // 24 hours +/** + * Longest password attempt the login endpoint will look at. + * + * Generous enough that no real passphrase hits it — the point is only to keep + * an unbounded request body out of the hashing path, not to constrain what + * users may choose. + */ +export const MAX_PASSWORD_LENGTH = 1024; + function getSigningKey(): Buffer { // Bind the key to ADMIN_PASSWORD as well, so rotating the password // immediately invalidates every outstanding session token. @@ -62,20 +71,28 @@ export function verifyAdminToken(token: string): boolean { } } -/** Verify admin password. */ +/** + * Verify admin password. + * + * Both sides are HMAC'd to a fixed 32 bytes before the constant-time compare. + * Comparing the raw strings meant `timingSafeEqual` could only run when the + * lengths already matched, so the early return on a length mismatch leaked how + * long ADMIN_PASSWORD is. Hashing first makes every attempt take the same path. + * + * Keyed with AUTH_SECRET rather than a bare digest, matching how `lib/auth.ts` + * compares subpage passwords: the intermediate values are then useless to + * anyone who cannot also read the secret. + */ export function verifyAdminPassword(password: string): boolean { const adminPw = env.ADMIN_PASSWORD; if (!adminPw) return false; + if (password.length > MAX_PASSWORD_LENGTH) return false; - // Constant-time comparison - const pwBuf = Buffer.from(password); - const expectedBuf = Buffer.from(adminPw); - if (pwBuf.length !== expectedBuf.length) { - // Still do a comparison to prevent timing attacks on length - crypto.timingSafeEqual(pwBuf, Buffer.alloc(pwBuf.length)); - return false; - } - return crypto.timingSafeEqual(pwBuf, expectedBuf); + const secret = resolveAuthSecret(); + const attemptHash = crypto.createHmac('sha256', secret).update(password).digest(); + const expectedHash = crypto.createHmac('sha256', secret).update(adminPw).digest(); + + return crypto.timingSafeEqual(attemptHash, expectedHash); } /** Check if the current request has a valid admin session. */