From cd1415c8b29546b7997590359f5a86ccef172bea Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Tobias=20Nie=C3=9Fen?= <tniessen@tnie.de>
Date: Sat, 8 Jun 2024 21:11:24 +0000
Subject: [PATCH] Revert "crypto: make timingSafeEqual faster for Uint8Array"
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This reverts commit 0f784c96ba1f41f59b7a54c3177496e789199a68 because it
triggers a bug in the V8 version that Node.js 20.x uses.

PR-URL: https://github.com/nodejs/node/pull/53390
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io>
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
---
 benchmark/crypto/timingSafeEqual.js | 22 ----------------------
 src/crypto/crypto_timing.cc         | 26 ++------------------------
 src/node_external_reference.h       |  6 ------
 3 files changed, 2 insertions(+), 52 deletions(-)
 delete mode 100644 benchmark/crypto/timingSafeEqual.js

diff --git a/benchmark/crypto/timingSafeEqual.js b/benchmark/crypto/timingSafeEqual.js
deleted file mode 100644
index 475807dba4ea4e..00000000000000
--- a/benchmark/crypto/timingSafeEqual.js
+++ /dev/null
@@ -1,22 +0,0 @@
-'use strict';
-
-const common = require('../common.js');
-const assert = require('node:assert');
-const { randomBytes, timingSafeEqual } = require('node:crypto');
-
-const bench = common.createBenchmark(main, {
-  n: [1e5],
-  bufferSize: [10, 100, 200, 2_100, 22_023],
-});
-
-function main({ n, bufferSize }) {
-  const bufs = [randomBytes(bufferSize), randomBytes(bufferSize)];
-  bench.start();
-  let count = 0;
-  for (let i = 0; i < n; i++) {
-    const ret = timingSafeEqual(bufs[i % 2], bufs[1]);
-    if (ret) count++;
-  }
-  bench.end(n);
-  assert.strictEqual(count, Math.floor(n / 2));
-}
diff --git a/src/crypto/crypto_timing.cc b/src/crypto/crypto_timing.cc
index 103a620d63726f..3d876fc4c3035f 100644
--- a/src/crypto/crypto_timing.cc
+++ b/src/crypto/crypto_timing.cc
@@ -9,8 +9,6 @@
 
 namespace node {
 
-using v8::FastApiCallbackOptions;
-using v8::FastApiTypedArray;
 using v8::FunctionCallbackInfo;
 using v8::Local;
 using v8::Object;
@@ -48,32 +46,12 @@ void TimingSafeEqual(const FunctionCallbackInfo<Value>& args) {
       CRYPTO_memcmp(buf1.data(), buf2.data(), buf1.size()) == 0);
 }
 
-bool FastTimingSafeEqual(Local<Value> receiver,
-                         const FastApiTypedArray<uint8_t>& a,
-                         const FastApiTypedArray<uint8_t>& b,
-                         // NOLINTNEXTLINE(runtime/references)
-                         FastApiCallbackOptions& options) {
-  uint8_t* data_a;
-  uint8_t* data_b;
-  if (a.length() != b.length() || !a.getStorageIfAligned(&data_a) ||
-      !b.getStorageIfAligned(&data_b)) {
-    options.fallback = true;
-    return false;
-  }
-
-  return CRYPTO_memcmp(data_a, data_b, a.length()) == 0;
-}
-
-static v8::CFunction fast_equal(v8::CFunction::Make(FastTimingSafeEqual));
-
 void Initialize(Environment* env, Local<Object> target) {
-  SetFastMethodNoSideEffect(
-      env->context(), target, "timingSafeEqual", TimingSafeEqual, &fast_equal);
+  SetMethodNoSideEffect(
+      env->context(), target, "timingSafeEqual", TimingSafeEqual);
 }
 void RegisterExternalReferences(ExternalReferenceRegistry* registry) {
   registry->Register(TimingSafeEqual);
-  registry->Register(FastTimingSafeEqual);
-  registry->Register(fast_equal.GetTypeInfo());
 }
 }  // namespace Timing
 
diff --git a/src/node_external_reference.h b/src/node_external_reference.h
index 17c0b2d7e1a440..a3317d25ad6a96 100644
--- a/src/node_external_reference.h
+++ b/src/node_external_reference.h
@@ -27,11 +27,6 @@ using CFunctionCallbackWithStrings =
     bool (*)(v8::Local<v8::Value>,
              const v8::FastOneByteString& input,
              const v8::FastOneByteString& base);
-using CFunctionCallbackWithTwoUint8ArraysFallback =
-    bool (*)(v8::Local<v8::Value>,
-             const v8::FastApiTypedArray<uint8_t>&,
-             const v8::FastApiTypedArray<uint8_t>&,
-             v8::FastApiCallbackOptions&);
 using CFunctionWithUint32 = uint32_t (*)(v8::Local<v8::Value>,
                                          const uint32_t input);
 using CFunctionWithDoubleReturnDouble = double (*)(v8::Local<v8::Value>,
@@ -56,7 +51,6 @@ class ExternalReferenceRegistry {
   V(CFunctionCallbackWithBool)                                                 \
   V(CFunctionCallbackWithString)                                               \
   V(CFunctionCallbackWithStrings)                                              \
-  V(CFunctionCallbackWithTwoUint8ArraysFallback)                               \
   V(CFunctionWithUint32)                                                       \
   V(CFunctionWithDoubleReturnDouble)                                           \
   V(CFunctionWithInt64Fallback)                                                \