fix(auth): constant-time owner-token comparison
Owner bearer token was compared with === / !==, which short-circuits on the first differing byte and leaks token length+prefix via response timing (security-sweep-2026-06-01.md). New timingSafeStrEqual (crypto.timingSafeEqual with a length pre-check so it never throws on length mismatch); wired into both owner.js and agent_auth.js. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -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();
|
||||
}
|
||||
|
||||
@@ -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' }
|
||||
});
|
||||
|
||||
23
lib/auth/safe_compare.js
Normal file
23
lib/auth/safe_compare.js
Normal file
@@ -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);
|
||||
}
|
||||
28
tests/auth/safe_compare.test.js
Normal file
28
tests/auth/safe_compare.test.js
Normal file
@@ -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);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user