Revert "crypto: add crypto.timingSafeEqual"

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: #8040 Refs: #8203 PR-URL: #8225 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]>
nodejs · Aug 23, 2016 · 0764bc4 · 0764bc4
1 parent e3cb0bf
commit 0764bc4
Show file tree

Hide file tree

Showing 5 changed files with 0 additions and 178 deletions.
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