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

Add crypto.timingSafeEqual() #8040

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
9 changes: 9 additions & 0 deletions doc/api/crypto.md
Original file line number Diff line number Diff line change
Expand Up @@ -1217,6 +1217,15 @@ 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`.
Expand Down
3 changes: 3 additions & 0 deletions lib/crypto.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ 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');
Expand Down Expand Up @@ -649,6 +650,8 @@ Object.defineProperty(exports, 'fips', {
set: setFipsCrypto
});

exports.timingSafeEqual = timingSafeEqual;

// Legacy API
Object.defineProperty(exports, 'createCredentials', {
configurable: true,
Expand Down
17 changes: 17 additions & 0 deletions src/node_crypto.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5771,6 +5771,22 @@ 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);
Expand Down Expand Up @@ -5903,6 +5919,7 @@ 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);
Expand Down
5 changes: 5 additions & 0 deletions test/sequential/sequential.status
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,11 @@ 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]
Expand Down
144 changes: 144 additions & 0 deletions test/sequential/test-crypto-timing-safe-equal.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,144 @@
// Flags: --allow_natives_syntax
'use strict';
const common = require('../common');
const assert = require('assert');

if (!common.hasCrypto) {
common.skip('missing crypto');
return;
}

const crypto = require('crypto');

assert.strictEqual(
crypto.timingSafeEqual(Buffer.from('foo'), Buffer.from('foo')),
true,
'should consider equal strings to be equal'
);

assert.strictEqual(
crypto.timingSafeEqual(Buffer.from('foo'), Buffer.from('bar')),
false,
'should consider unequal strings to be unequal'
);

assert.throws(function() {
crypto.timingSafeEqual(Buffer.from([1, 2, 3]), Buffer.from([1, 2]));
}, 'should throw when given buffers with different lengths');

assert.throws(function() {
crypto.timingSafeEqual('not a buffer', Buffer.from([1, 2]));
}, 'should throw if the first argument is not a buffer');

assert.throws(function() {
crypto.timingSafeEqual(Buffer.from([1, 2]), 'not a buffer');
}, 'should throw if the second argument is not a buffer');

function runBenchmark(compareFunc, bufferA, bufferB, expectedResult) {
const startTime = process.hrtime();
const result = compareFunc(bufferA, bufferB);
const endTime = process.hrtime(startTime);

// Ensure that the result of the function call gets used, so that it doesn't
// get discarded due to engine optimizations.
assert.strictEqual(result, expectedResult);
return endTime[0] * 1e9 + endTime[1];
}

function getTValue(compareFunc) {
const numTrials = 10000;
const testBufferSize = 10000;
// Perform benchmarks to verify that timingSafeEqual is actually timing-safe.
const bufferA1 = Buffer.alloc(testBufferSize, 'A');
const bufferA2 = Buffer.alloc(testBufferSize, 'A');
const bufferB = Buffer.alloc(testBufferSize, 'B');
const bufferC = Buffer.alloc(testBufferSize, 'C');

const rawEqualBenches = Array(numTrials);
const rawUnequalBenches = Array(numTrials);

for (let i = 0; i < numTrials; i++) {
// First benchmark: comparing two equal buffers
rawEqualBenches[i] = runBenchmark(compareFunc, bufferA1, bufferA2, true);
// Second benchmark: comparing two unequal buffers
rawUnequalBenches[i] = runBenchmark(compareFunc, bufferB, bufferC, false);
}

const equalBenches = filterOutliers(rawEqualBenches);
const unequalBenches = filterOutliers(rawUnequalBenches);

// Use a two-sample t-test to determine whether the timing difference between
// the benchmarks is statistically significant.
// https://wikipedia.org/wiki/Student%27s_t-test#Independent_two-sample_t-test

const equalMean = mean(equalBenches);
const unequalMean = mean(unequalBenches);

const equalLen = equalBenches.length;
const unequalLen = unequalBenches.length;

const combinedStd = combinedStandardDeviation(equalBenches, unequalBenches);
const standardErr = combinedStd * Math.sqrt(1 / equalLen + 1 / unequalLen);

return (equalMean - unequalMean) / standardErr;
}

// Returns the mean of an array
function mean(array) {
return array.reduce((sum, val) => sum + val, 0) / array.length;
}

// Returns the sample standard deviation of an array
function standardDeviation(array) {
const arrMean = mean(array);
const total = array.reduce((sum, val) => sum + Math.pow(val - arrMean, 2), 0);
return Math.sqrt(total / (array.length - 1));
}

// Returns the common standard deviation of two arrays
function combinedStandardDeviation(array1, array2) {
const sum1 = Math.pow(standardDeviation(array1), 2) * (array1.length - 1);
const sum2 = Math.pow(standardDeviation(array2), 2) * (array2.length - 1);
return Math.sqrt((sum1 + sum2) / (array1.length + array2.length - 2));
}

// Filter large outliers from an array. A 'large outlier' is a value that is at
// least 50 times larger than the mean. This prevents the tests from failing
// due to the standard deviation increase when a function unexpectedly takes
// a very long time to execute.
function filterOutliers(array) {
const arrMean = mean(array);
return array.filter((value) => value / arrMean < 50);
}

// Force optimization before starting the benchmark
runBenchmark(crypto.timingSafeEqual, Buffer.from('A'), Buffer.from('A'), true);
eval('%OptimizeFunctionOnNextCall(runBenchmark)');
runBenchmark(crypto.timingSafeEqual, Buffer.from('A'), Buffer.from('A'), true);

// t_(0.9995, ∞)
// i.e. If a given comparison function is indeed timing-safe, the t-test result
// has a 99.9% chance to be below this threshold. Unfortunately, this means that
// this test will be a bit flakey and will fail 0.1% of the time even if
// crypto.timingSafeEqual is working properly.
// t-table ref: http://www.sjsu.edu/faculty/gerstman/StatPrimer/t-table.pdf
// Note that in reality there are roughly `2 * numTrials - 2` degrees of
// freedom, not ∞. However, assuming `numTrials` is large, this doesn't
// significantly affect the threshold.
const T_THRESHOLD = 3.291;

const t = getTValue(crypto.timingSafeEqual);
assert(
Math.abs(t) < T_THRESHOLD,
`timingSafeEqual should not leak information from its execution time (t=${t})`
);

// As a sanity check to make sure the statistical tests are working, run the
// same benchmarks again, this time with an unsafe comparison function. In this
// case the t-value should be above the threshold.
const unsafeCompare = (bufA, bufB) => bufA.equals(bufB);
const t2 = getTValue(unsafeCompare);
assert(
Math.abs(t2) > T_THRESHOLD,
`Buffer#equals should leak information from its execution time (t=${t2})`
);