Repository: denoland/deno
Pull request: fix(ext/node): crypto.timingSafeEqual

This PR fixes behaviors of `crypto.timingSafeEqual` and enables `sequential/test-crypto-timing-safe-equal.js`

towards #29637 

Changed files:
filename: ext/node/polyfills/internal/errors.ts
patch: @@ -681,7 +681,7 @@ export class ERR_CRYPTO_TIMING_SAFE_EQUAL_LENGTH extends NodeRangeError {
   constructor() {
     super(
       "ERR_CRYPTO_TIMING_SAFE_EQUAL_LENGTH",
-      "Input buffers must have the same length",
+      "Input buffers must have the same byte length",
     );
   }
 }

filename: ext/node/polyfills/internal_binding/_timingSafeEqual.ts
patch: @@ -5,6 +5,7 @@
 
 import { Buffer } from "node:buffer";
 import { ERR_CRYPTO_TIMING_SAFE_EQUAL_LENGTH } from "ext:deno_node/internal/errors.ts";
+import { validateBuffer } from "ext:deno_node/internal/validators.mjs";
 
 function toDataView(ab: ArrayBufferLike | ArrayBufferView): DataView {
   if (ArrayBuffer.isView(ab)) {
@@ -38,14 +39,16 @@ function stdTimingSafeEqual(
 }
 
 export const timingSafeEqual = (
-  a: Buffer | DataView | ArrayBuffer,
-  b: Buffer | DataView | ArrayBuffer,
+  buf1: Buffer | DataView | ArrayBuffer,
+  buf2: Buffer | DataView | ArrayBuffer,
 ): boolean => {
-  if (a instanceof Buffer) {
-    a = new DataView(a.buffer, a.byteOffset, a.byteLength);
+  validateBuffer(buf1, "buf1");
+  validateBuffer(buf2, "buf2");
+  if (buf1 instanceof Buffer) {
+    buf1 = new DataView(buf1.buffer, buf1.byteOffset, buf1.byteLength);
   }
-  if (b instanceof Buffer) {
-    b = new DataView(b.buffer, b.byteOffset, b.byteLength);
+  if (buf2 instanceof Buffer) {
+    buf2 = new DataView(buf2.buffer, buf2.byteOffset, buf2.byteLength);
   }
-  return stdTimingSafeEqual(a, b);
+  return stdTimingSafeEqual(buf1, buf2);
 };

filename: tests/node_compat/config.jsonc
patch: @@ -1410,6 +1410,7 @@
     "sequential": [
       "test-buffer-creation-regression.js",
       "test-child-process-exit.js",
+      "test-crypto-timing-safe-equal.js",
       "test-http-server-keep-alive-timeout-slow-server.js",
       "test-net-connect-local-error.js",
       "test-net-response-size.js",

filename: tests/node_compat/runner/TODO.md
patch: @@ -1,7 +1,7 @@
 <!-- deno-fmt-ignore-file -->
 # Remaining Node Tests
 
-1207 tests out of 3993 have been ported from Node 23.9.0 (30.23% ported, 70.40% remaining).
+1208 tests out of 3993 have been ported from Node 23.9.0 (30.25% ported, 70.37% remaining).
 
 NOTE: This file should not be manually edited. Please edit `tests/node_compat/config.json` and run `deno task setup` in `tests/node_compat/runner` dir instead.
 
@@ -2731,7 +2731,6 @@ NOTE: This file should not be manually edited. Please edit `tests/node_compat/co
 - [sequential/test-cpu-prof-kill.js](https://github.com/nodejs/node/tree/v23.9.0/test/sequential/test-cpu-prof-kill.js)
 - [sequential/test-cpu-prof-name.js](https://github.com/nodejs/node/tree/v23.9.0/test/sequential/test-cpu-prof-name.js)
 - [sequential/test-cpu-prof-worker-argv.js](https://github.com/nodejs/node/tree/v23.9.0/test/sequential/test-cpu-prof-worker-argv.js)
-- [sequential/test-crypto-timing-safe-equal.js](https://github.com/nodejs/node/tree/v23.9.0/test/sequential/test-crypto-timing-safe-equal.js)
 - [sequential/test-debug-prompt.js](https://github.com/nodejs/node/tree/v23.9.0/test/sequential/test-debug-prompt.js)
 - [sequential/test-debugger-custom-port.js](https://github.com/nodejs/node/tree/v23.9.0/test/sequential/test-debugger-custom-port.js)
 - [sequential/test-debugger-debug-brk.js](https://github.com/nodejs/node/tree/v23.9.0/test/sequential/test-debugger-debug-brk.js)

filename: tests/node_compat/test/sequential/test-crypto-timing-safe-equal.js
patch: @@ -0,0 +1,126 @@
+// deno-fmt-ignore-file
+// deno-lint-ignore-file
+
+// Copyright Joyent and Node contributors. All rights reserved. MIT license.
+// Taken from Node 23.9.0
+// This file is automatically generated by `tests/node_compat/runner/setup.ts`. Do not modify this file manually.
+
+// Flags: --expose-internals --no-warnings --allow-natives-syntax
+'use strict';
+const common = require('../common');
+if (!common.hasCrypto)
+  common.skip('missing crypto');
+
+const assert = require('assert');
+const crypto = require('crypto');
+
+// 'should consider equal strings to be equal'
+assert.strictEqual(
+  crypto.timingSafeEqual(Buffer.from('foo'), Buffer.from('foo')),
+  true
+);
+
+// 'should consider unequal strings to be unequal'
+assert.strictEqual(
+  crypto.timingSafeEqual(Buffer.from('foo'), Buffer.from('bar')),
+  false
+);
+
+{
+  // Test TypedArrays with different lengths but equal byteLengths.
+  const buf = crypto.randomBytes(16).buffer;
+  const a1 = new Uint8Array(buf);
+  const a2 = new Uint16Array(buf);
+  const a3 = new Uint32Array(buf);
+
+  for (const left of [a1, a2, a3]) {
+    for (const right of [a1, a2, a3]) {
+      assert.strictEqual(crypto.timingSafeEqual(left, right), true);
+    }
+  }
+}
+
+{
+  // When the inputs are floating-point numbers, timingSafeEqual neither has
+  // equality nor SameValue semantics. It just compares the underlying bytes,
+  // ignoring the TypedArray type completely.
+
+  const cmp = (fn) => (a, b) => a.every((x, i) => fn(x, b[i]));
+  const eq = cmp((a, b) => a === b);
+  const is = cmp(Object.is);
+
+  function test(a, b, { equal, sameValue, timingSafeEqual }) {
+    assert.strictEqual(eq(a, b), equal);
+    assert.strictEqual(is(a, b), sameValue);
+    assert.strictEqual(crypto.timingSafeEqual(a, b), timingSafeEqual);
+  }
+
+  test(new Float32Array([NaN]), new Float32Array([NaN]), {
+    equal: false,
+    sameValue: true,
+    timingSafeEqual: true
+  });
+
+  test(new Float64Array([0]), new Float64Array([-0]), {
+    equal: true,
+    sameValue: false,
+    timingSafeEqual: false
+  });
+
+  const x = new BigInt64Array([0x7ff0000000000001n, 0xfff0000000000001n]);
+  test(new Float64Array(x.buffer), new Float64Array([NaN, NaN]), {
+    equal: false,
+    sameValue: true,
+    timingSafeEqual: false
+  });
+}
+
+assert.throws(
+  () => crypto.timingSafeEqual(Buffer.from([1, 2, 3]), Buffer.from([1, 2])),
+  {
+    code: 'ERR_CRYPTO_TIMING_SAFE_EQUAL_LENGTH',
+    name: 'RangeError',
+    message: 'Input buffers must have the same byte length'
+  }
+);
+
+assert.throws(
+  () => crypto.timingSafeEqual('not a buffer', Buffer.from([1, 2])),
+  {
+    code: 'ERR_INVALID_ARG_TYPE',
+    name: 'TypeError',
+  }
+);
+
+assert.throws(
+  () => crypto.timingSafeEqual(Buffer.from([1, 2]), 'not a buffer'),
+  {
+    code: 'ERR_INVALID_ARG_TYPE',
+    name: 'TypeError',
+  }
+);
+
+{
+  // V8 Fast API
+  const foo = Buffer.from('foo');
+  const bar = Buffer.from('bar');
+  const longer = Buffer.from('longer');
+  function testFastPath(buf1, buf2) {
+    return crypto.timingSafeEqual(buf1, buf2);
+  }
+  eval('%PrepareFunctionForOptimization(testFastPath)');
+  assert.strictEqual(testFastPath(foo, bar), false);
+  eval('%OptimizeFunctionOnNextCall(testFastPath)');
+  assert.strictEqual(testFastPath(foo, bar), false);
+  assert.strictEqual(testFastPath(foo, foo), true);
+  assert.throws(() => testFastPath(foo, longer), {
+    code: 'ERR_CRYPTO_TIMING_SAFE_EQUAL_LENGTH',
+  });
+
+  if (common.isDebug) {
+    const { internalBinding } = require('internal/test/binding');
+    const { getV8FastApiCallCount } = internalBinding('debug');
+    assert.strictEqual(getV8FastApiCallCount('crypto.timingSafeEqual.ok'), 2);
+    assert.strictEqual(getV8FastApiCallCount('crypto.timingSafeEqual.error'), 1);
+  }
+}