diff --git a/lib/api/middleware/agent_auth.js b/lib/api/middleware/agent_auth.js index 480def6..a8b6941 100644 --- a/lib/api/middleware/agent_auth.js +++ b/lib/api/middleware/agent_auth.js @@ -1,4 +1,5 @@ import * as agents from '../../db/repos/agents.js'; +import { timingSafeStrEqual } from '../../auth/safe_compare.js'; export async function agentOrOwner(req, res, next) { const expectedOwner = process.env.OWNER_TOKEN; @@ -12,7 +13,7 @@ export async function agentOrOwner(req, res, next) { if (scheme !== 'Bearer' || !token) { return res.status(401).json({ error: { code: 'unauthorized', message: 'missing bearer token' } }); } - if (token === expectedOwner) { + if (timingSafeStrEqual(token, expectedOwner)) { req.actor = { kind: 'user', id: null }; return next(); } diff --git a/lib/auth/owner.js b/lib/auth/owner.js index cefb44a..ced2a92 100644 --- a/lib/auth/owner.js +++ b/lib/auth/owner.js @@ -1,3 +1,5 @@ +import { timingSafeStrEqual } from './safe_compare.js'; + export function ownerOnly(req, res, next) { const expected = process.env.OWNER_TOKEN; if (!expected) { @@ -7,7 +9,7 @@ export function ownerOnly(req, res, next) { } const auth = req.headers.authorization || ''; const [scheme, token] = auth.split(' '); - if (scheme !== 'Bearer' || token !== expected) { + if (scheme !== 'Bearer' || !timingSafeStrEqual(token, expected)) { return res.status(401).json({ error: { code: 'unauthorized', message: 'invalid token' } }); diff --git a/lib/auth/safe_compare.js b/lib/auth/safe_compare.js new file mode 100644 index 0000000..098f12e --- /dev/null +++ b/lib/auth/safe_compare.js @@ -0,0 +1,23 @@ +import crypto from 'node:crypto'; + +/** + * Constant-time string comparison for secrets (e.g. the owner bearer token). + * + * Plain `===` short-circuits on the first differing byte, leaking length and + * prefix via timing. `crypto.timingSafeEqual` is constant-time but THROWS when + * the two buffers differ in length — so we length-check first (length is not + * the secret) and only then do the constant-time compare. Returns false for + * any falsy / non-string input rather than throwing. + * + * @param {string} a + * @param {string} b + * @returns {boolean} + */ +export function timingSafeStrEqual(a, b) { + if (typeof a !== 'string' || typeof b !== 'string') return false; + if (a.length === 0 || b.length === 0) return false; + const ab = Buffer.from(a); + const bb = Buffer.from(b); + if (ab.length !== bb.length) return false; + return crypto.timingSafeEqual(ab, bb); +} diff --git a/tests/auth/safe_compare.test.js b/tests/auth/safe_compare.test.js new file mode 100644 index 0000000..9ee1153 --- /dev/null +++ b/tests/auth/safe_compare.test.js @@ -0,0 +1,28 @@ +import { describe, it, expect } from 'vitest'; +import { timingSafeStrEqual } from '../../lib/auth/safe_compare.js'; + +// The owner bearer token is compared on every request. A naive === leaks +// length+prefix via timing; a naive crypto.timingSafeEqual THROWS when the two +// buffers differ in length. timingSafeStrEqual must be constant-time for equal +// lengths and return false (never throw) for mismatched lengths or bad input. + +describe('timingSafeStrEqual', () => { + it('returns true for identical strings', () => { + expect(timingSafeStrEqual('vk_secrettoken', 'vk_secrettoken')).toBe(true); + }); + + it('returns false for same-length but different strings', () => { + expect(timingSafeStrEqual('aaaaaa', 'aaaaab')).toBe(false); + }); + + it('returns false (no throw) for different-length strings', () => { + expect(timingSafeStrEqual('short', 'a-much-longer-token')).toBe(false); + }); + + it('returns false for null / undefined / empty input', () => { + expect(timingSafeStrEqual(undefined, 'x')).toBe(false); + expect(timingSafeStrEqual('x', undefined)).toBe(false); + expect(timingSafeStrEqual('', '')).toBe(false); + expect(timingSafeStrEqual(null, null)).toBe(false); + }); +});