diff --git a/doc/api/errors.md b/doc/api/errors.md index 804d26ab2968f0..4a20e14306fe53 100644 --- a/doc/api/errors.md +++ b/doc/api/errors.md @@ -648,6 +648,12 @@ Used when an invalid crypto engine identifier is passed to Used when an invalid [crypto digest algorithm][] is specified. + +### ERR_CRYPTO_TIMING_SAFE_EQUAL_LENGTH + +Used when calling [`crypto.timingSafeEqual()`][] with `Buffer`, `TypedArray`, +or `DataView` arguments of different lengths. + ### ERR_DNS_SET_SERVERS_FAILED @@ -1348,6 +1354,7 @@ Used when a given value is out of the accepted range. Used when an attempt is made to use a `zlib` object after it has already been closed. +[`crypto.timingSafeEqual()`]: crypto.html#crypto_crypto_timingsafeequal_a_b [`ERR_INVALID_ARG_TYPE`]: #ERR_INVALID_ARG_TYPE [`subprocess.kill()`]: child_process.html#child_process_subprocess_kill_signal [`subprocess.send()`]: child_process.html#child_process_subprocess_send_message_sendhandle_options_callback diff --git a/lib/crypto.js b/lib/crypto.js index 1337cccf2c3ce1..0082172c5c4a09 100644 --- a/lib/crypto.js +++ b/lib/crypto.js @@ -34,7 +34,6 @@ const constants = process.binding('constants').crypto; const { getFipsCrypto, setFipsCrypto, - timingSafeEqual } = process.binding('crypto'); const { randomBytes, @@ -75,6 +74,7 @@ const { getHashes, setDefaultEncoding, setEngine, + timingSafeEqual, toBuf } = require('internal/crypto/util'); const Certificate = require('internal/crypto/certificate'); diff --git a/lib/internal/crypto/util.js b/lib/internal/crypto/util.js index 84ad1fb2c7ef1d..ac01a447bc4324 100644 --- a/lib/internal/crypto/util.js +++ b/lib/internal/crypto/util.js @@ -4,7 +4,8 @@ const { getCiphers: _getCiphers, getCurves: _getCurves, getHashes: _getHashes, - setEngine: _setEngine + setEngine: _setEngine, + timingSafeEqual: _timingSafeEqual } = process.binding('crypto'); const { @@ -17,6 +18,9 @@ const { cachedResult, filterDuplicateStrings } = require('internal/util'); +const { + isArrayBufferView +} = require('internal/util/types'); var defaultEncoding = 'buffer'; @@ -60,6 +64,21 @@ function setEngine(id, flags) { throw new errors.Error('ERR_CRYPTO_ENGINE_UNKNOWN', id); } +function timingSafeEqual(a, b) { + if (!isArrayBufferView(a)) { + throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'a', + ['Buffer', 'TypedArray', 'DataView']); + } + if (!isArrayBufferView(b)) { + throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'b', + ['Buffer', 'TypedArray', 'DataView']); + } + if (a.length !== b.length) { + throw new errors.RangeError('ERR_CRYPTO_TIMING_SAFE_EQUAL_LENGTH'); + } + return _timingSafeEqual(a, b); +} + module.exports = { getCiphers, getCurves, @@ -67,5 +86,6 @@ module.exports = { getHashes, setDefaultEncoding, setEngine, + timingSafeEqual, toBuf }; diff --git a/lib/internal/errors.js b/lib/internal/errors.js index e21133364835da..48eb6b89356ac8 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -160,6 +160,8 @@ E('ERR_CRYPTO_HASH_FINALIZED', 'Digest already called'); E('ERR_CRYPTO_HASH_UPDATE_FAILED', 'Hash update failed'); E('ERR_CRYPTO_INVALID_DIGEST', 'Invalid digest: %s'); E('ERR_CRYPTO_SIGN_KEY_REQUIRED', 'No key provided to sign'); +E('ERR_CRYPTO_TIMING_SAFE_EQUAL_LENGTH', + 'Input buffers must have the same length'); E('ERR_DNS_SET_SERVERS_FAILED', (err, servers) => `c-ares failed to set servers: "${err}" [${servers}]`); E('ERR_ENCODING_INVALID_ENCODED_DATA', diff --git a/src/node_crypto.cc b/src/node_crypto.cc index 450a841eaea0da..1b8d14825f5c89 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -5848,15 +5848,11 @@ void ExportChallenge(const FunctionCallbackInfo& args) { } void TimingSafeEqual(const FunctionCallbackInfo& 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"); + CHECK(Buffer::HasInstance(args[0])); + CHECK(Buffer::HasInstance(args[1])); 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"); - } + CHECK_EQ(buf_length, Buffer::Length(args[1])); const char* buf1 = Buffer::Data(args[0]); const char* buf2 = Buffer::Data(args[1]); diff --git a/test/sequential/test-crypto-timing-safe-equal.js b/test/sequential/test-crypto-timing-safe-equal.js index 2847af9ef8bdb5..08cfc04755d638 100644 --- a/test/sequential/test-crypto-timing-safe-equal.js +++ b/test/sequential/test-crypto-timing-safe-equal.js @@ -18,17 +18,31 @@ assert.strictEqual( 'should consider unequal strings to be unequal' ); -assert.throws(function() { - crypto.timingSafeEqual(Buffer.from([1, 2, 3]), Buffer.from([1, 2])); -}, /^TypeError: Input buffers must have the same length$/, - 'should throw when given buffers with different lengths'); +common.expectsError( + () => crypto.timingSafeEqual(Buffer.from([1, 2, 3]), Buffer.from([1, 2])), + { + code: 'ERR_CRYPTO_TIMING_SAFE_EQUAL_LENGTH', + type: RangeError, + message: 'Input buffers must have the same length' + } +); -assert.throws(function() { - crypto.timingSafeEqual('not a buffer', Buffer.from([1, 2])); -}, /^TypeError: First argument must be a buffer$/, - 'should throw if the first argument is not a buffer'); +common.expectsError( + () => crypto.timingSafeEqual('not a buffer', Buffer.from([1, 2])), + { + code: 'ERR_INVALID_ARG_TYPE', + type: TypeError, + message: + 'The "a" argument must be one of type Buffer, TypedArray, or DataView' + } +); -assert.throws(function() { - crypto.timingSafeEqual(Buffer.from([1, 2]), 'not a buffer'); -}, /^TypeError: Second argument must be a buffer$/, - 'should throw if the second argument is not a buffer'); +common.expectsError( + () => crypto.timingSafeEqual(Buffer.from([1, 2]), 'not a buffer'), + { + code: 'ERR_INVALID_ARG_TYPE', + type: TypeError, + message: + 'The "b" argument must be one of type Buffer, TypedArray, or DataView' + } +);