Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

crypto: migrate timingSafeEqual to internal/errors #16448

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions doc/api/errors.md
Original file line number Diff line number Diff line change
Expand Up @@ -642,6 +642,12 @@ Used when an invalid value for the `format` argument has been passed to the

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 @@ -1342,6 +1348,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 @@ -59,12 +63,28 @@ function setEngine(id, flags) {
return _setEngine(id, flags);
}

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 @@ -159,6 +159,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
8 changes: 3 additions & 5 deletions src/node_crypto.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5850,13 +5850,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'
}
);