diff --git a/.changeset/yellow-cows-impress.md b/.changeset/yellow-cows-impress.md new file mode 100644 index 00000000000..2dfeb08d2a2 --- /dev/null +++ b/.changeset/yellow-cows-impress.md @@ -0,0 +1,5 @@ +--- +"@atproto/oauth-provider": patch +--- + +Properly compute sleep time in contantTime util diff --git a/packages/oauth/oauth-provider/src/lib/util/time.ts b/packages/oauth/oauth-provider/src/lib/util/time.ts index dcbf372f83d..3975e2b6593 100644 --- a/packages/oauth/oauth-provider/src/lib/util/time.ts +++ b/packages/oauth/oauth-provider/src/lib/util/time.ts @@ -1,33 +1,50 @@ +import { setTimeout as sleep } from 'node:timers/promises' import { Awaitable } from './type.js' +export function onOvertimeDefault(options: { + start: number + end: number + elapsed: number + time: number +}): void { + console.warn( + `constantTime: execution time was ${options.elapsed}ms (which is greater than ${options.time}ms). You should increase the "time" to properly defend against timing attacks.`, + ) +} + /** * Utility function to protect against timing attacks. */ -export async function constantTime( - delay: number, - fn: () => Awaitable, -): Promise { - if (!Number.isFinite(delay) || delay <= 0) { - throw new TypeError('Delay must be greater than 0') +export async function constantTime( + this: T, + time: number, + fn: (this: T) => Awaitable, + onOvertime = onOvertimeDefault, +): Promise { + if (!Number.isFinite(time) || time <= 0) { + throw new TypeError(`"time" must be a positive number`) } const start = Date.now() try { - return await fn() + return await fn.call(this) } finally { - const delta = Date.now() - start + const end = Date.now() + const elapsed = end - start - // Let's make sure we always wait for a multiple of `delay` milliseconds. - const n = Math.max(1, Math.ceil(delta / delay)) + const remaining = time - elapsed + if (remaining >= 0) { + // Happy path, execution time was smaller than "time" + await sleep(remaining) + } else { + // The function execution took longer than "time" + onOvertime({ start, end, elapsed, time }) - // Ideally, the multiple should always be 1 in order to to properly defend - // against timing attacks. Show a warning if it's not. - if (n > 1) { - console.warn( - `constantTime: execution time was ${delta}ms, waiting for the next multiple of ${delay}ms. You should increase the delay to properly defend against timing attacks.`, - ) - } + // Sleep until the next multiple of "time" to mitigate any attack + const multiplier = Math.ceil(elapsed / time) + const remaining = multiplier * time - elapsed - await new Promise((resolve) => setTimeout(resolve, n * delay)) + await sleep(remaining) + } } }