Revert "crypto: add crypto.timingSafeEqual"

jasnell · jasnell · commit b7c67a9c7ca8 · 2016-08-22T11:15:00.000-07:00
This reverts commit 0fc5e0d. Additional testing indicates that there may still be timing issues with this implementation. Revert in order to give more time for testing before this goes out into a release... Refs: nodejs#8040 Refs: nodejs#8203
diff --git a/doc/api/crypto.md b/doc/api/crypto.md
@@ -1217,15 +1217,6 @@ keys:
 
 All paddings are defined in `crypto.constants`.
 
-### crypto.timingSafeEqual(a, b)
-
-Returns true if `a` is equal to `b`, without leaking timing information that
-would allow an attacker to guess one of the values. This is suitable for
-comparing HMAC digests or secret values like authentication cookies or
-[capability urls](https://www.w3.org/TR/capability-urls/).
-
-`a` and `b` must both be `Buffer`s, and they must have the same length.
-
 ### crypto.privateEncrypt(private_key, buffer)
 
 Encrypts `buffer` with `private_key`.
diff --git a/lib/crypto.js b/lib/crypto.js
@@ -16,7 +16,6 @@ const getHashes = binding.getHashes;
 const getCurves = binding.getCurves;
 const getFipsCrypto = binding.getFipsCrypto;
 const setFipsCrypto = binding.setFipsCrypto;
-const timingSafeEqual = binding.timingSafeEqual;
 
 const Buffer = require('buffer').Buffer;
 const stream = require('stream');
@@ -650,8 +649,6 @@ Object.defineProperty(exports, 'fips', {
   set: setFipsCrypto
 });
 
-exports.timingSafeEqual = timingSafeEqual;
-
 // Legacy API
 Object.defineProperty(exports, 'createCredentials', {
   configurable: true,
diff --git a/src/node_crypto.cc b/src/node_crypto.cc
@@ -5771,22 +5771,6 @@ void ExportChallenge(const FunctionCallbackInfo<Value>& args) {
   args.GetReturnValue().Set(outString);
 }
 
-void TimingSafeEqual(const FunctionCallbackInfo<Value>& args) {
-  Environment* env = Environment::GetCurrent(args);
-
-  THROW_AND_RETURN_IF_NOT_BUFFER(args[0], "First argument");
-  THROW_AND_RETURN_IF_NOT_BUFFER(args[1], "Second argument");
-
-  size_t buf_length = Buffer::Length(args[0]);
-  if (buf_length != Buffer::Length(args[1])) {
-    return env->ThrowTypeError("Input buffers must have the same length");
-  }
-
-  const char* buf1 = Buffer::Data(args[0]);
-  const char* buf2 = Buffer::Data(args[1]);
-
-  return args.GetReturnValue().Set(CRYPTO_memcmp(buf1, buf2, buf_length) == 0);
-}
 
 void InitCryptoOnce() {
   OPENSSL_config(NULL);
@@ -5919,7 +5903,6 @@ void InitCrypto(Local<Object> target,
   env->SetMethod(target, "setFipsCrypto", SetFipsCrypto);
   env->SetMethod(target, "PBKDF2", PBKDF2);
   env->SetMethod(target, "randomBytes", RandomBytes);
-  env->SetMethod(target, "timingSafeEqual", TimingSafeEqual);
   env->SetMethod(target, "getSSLCiphers", GetSSLCiphers);
   env->SetMethod(target, "getCiphers", GetCiphers);
   env->SetMethod(target, "getHashes", GetHashes);
diff --git a/test/sequential/sequential.status b/test/sequential/sequential.status
@@ -6,11 +6,6 @@ prefix sequential
 
 [true] # This section applies to all platforms
 
-# crypto.timingSafeEqual contains a statistical timing test to verify that the
-# function is timing-safe. As a result, the test sometimes fails due to random
-# timing fluctuations.
-test-crypto-timing-safe-equal           : PASS,FLAKY
-
 [$system==win32]
 
 [$system==linux]
diff --git a/test/sequential/test-crypto-timing-safe-equal.js b/test/sequential/test-crypto-timing-safe-equal.js
