Skip to content

Commit

Permalink
crypto: migrate timingSafeEqual to internal/errors
Browse files Browse the repository at this point in the history
PR-URL: #16448
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
  • Loading branch information
jasnell committed Oct 26, 2017
1 parent 6645126 commit eeada6c
Show file tree
Hide file tree
Showing 6 changed files with 60 additions and 21 deletions.
7 changes: 7 additions & 0 deletions doc/api/errors.md
Original file line number Diff line number Diff line change
Expand Up @@ -648,6 +648,12 @@ Used when an invalid crypto engine identifier is passed to

Used when an invalid [crypto digest algorithm][] is specified.

<a id="ERR_CRYPTO_TIMING_SAFE_EQUAL_LENGTH"></a>
### ERR_CRYPTO_TIMING_SAFE_EQUAL_LENGTH

Used when calling [`crypto.timingSafeEqual()`][] with `Buffer`, `TypedArray`,
or `DataView` arguments of different lengths.

<a id="ERR_DNS_SET_SERVERS_FAILED"></a>
### ERR_DNS_SET_SERVERS_FAILED

Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion lib/crypto.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ const constants = process.binding('constants').crypto;
const {
getFipsCrypto,
setFipsCrypto,
timingSafeEqual
} = process.binding('crypto');
const {
randomBytes,
Expand Down Expand Up @@ -75,6 +74,7 @@ const {
getHashes,
setDefaultEncoding,
setEngine,
timingSafeEqual,
toBuf
} = require('internal/crypto/util');
const Certificate = require('internal/crypto/certificate');
Expand Down
22 changes: 21 additions & 1 deletion lib/internal/crypto/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@ const {
getCiphers: _getCiphers,
getCurves: _getCurves,
getHashes: _getHashes,
setEngine: _setEngine
setEngine: _setEngine,
timingSafeEqual: _timingSafeEqual
} = process.binding('crypto');

const {
Expand All @@ -17,6 +18,9 @@ const {
cachedResult,
filterDuplicateStrings
} = require('internal/util');
const {
isArrayBufferView
} = require('internal/util/types');

var defaultEncoding = 'buffer';

Expand Down Expand Up @@ -60,12 +64,28 @@ 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,
getDefaultEncoding,
getHashes,
setDefaultEncoding,
setEngine,
timingSafeEqual,
toBuf
};
2 changes: 2 additions & 0 deletions lib/internal/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
10 changes: 3 additions & 7 deletions src/node_crypto.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5848,15 +5848,11 @@ void ExportChallenge(const FunctionCallbackInfo<Value>& args) {
}

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");
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]);
Expand Down
38 changes: 26 additions & 12 deletions test/sequential/test-crypto-timing-safe-equal.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'
}
);

0 comments on commit eeada6c

Please sign in to comment.