Browse Source

crypto: clear err stack after ECDH::BufferToPoint

Functions that call `ECDH::BufferToPoint` were not clearing the
error stack on failure, so an invalid key could leave leftover
error state and cause subsequent (unrelated) signing operations
to fail.

PR-URL: https://github.com/nodejs/node/pull/13275
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
v6
Ryan Kelly 8 years ago
committed by Sam Roberts
parent
commit
f6665334e7
  1. 4
      src/node_crypto.cc
  2. 23
      test/parallel/test-crypto-dh.js

4
src/node_crypto.cc

@ -5136,6 +5136,8 @@ void ECDH::ComputeSecret(const FunctionCallbackInfo<Value>& args) {
ECDH* ecdh; ECDH* ecdh;
ASSIGN_OR_RETURN_UNWRAP(&ecdh, args.Holder()); ASSIGN_OR_RETURN_UNWRAP(&ecdh, args.Holder());
MarkPopErrorOnReturn mark_pop_error_on_return;
if (!ecdh->IsKeyPairValid()) if (!ecdh->IsKeyPairValid())
return env->ThrowError("Invalid key pair"); return env->ThrowError("Invalid key pair");
@ -5282,6 +5284,8 @@ void ECDH::SetPublicKey(const FunctionCallbackInfo<Value>& args) {
THROW_AND_RETURN_IF_NOT_BUFFER(args[0], "Public key"); THROW_AND_RETURN_IF_NOT_BUFFER(args[0], "Public key");
MarkPopErrorOnReturn mark_pop_error_on_return;
EC_POINT* pub = ecdh->BufferToPoint(Buffer::Data(args[0].As<Object>()), EC_POINT* pub = ecdh->BufferToPoint(Buffer::Data(args[0].As<Object>()),
Buffer::Length(args[0].As<Object>())); Buffer::Length(args[0].As<Object>()));
if (pub == nullptr) if (pub == nullptr)

23
test/parallel/test-crypto-dh.js

@ -172,6 +172,7 @@ assert.strictEqual(bad_dh.verifyError, DH_NOT_SUITABLE_GENERATOR);
const availableCurves = new Set(crypto.getCurves()); const availableCurves = new Set(crypto.getCurves());
const availableHashes = new Set(crypto.getHashes());
// Oakley curves do not clean up ERR stack, it was causing unexpected failure // Oakley curves do not clean up ERR stack, it was causing unexpected failure
// when accessing other OpenSSL APIs afterwards. // when accessing other OpenSSL APIs afterwards.
@ -307,6 +308,28 @@ if (availableCurves.has('prime256v1') && availableCurves.has('secp256k1')) {
}); });
} }
// Use of invalid keys was not cleaning up ERR stack, and was causing
// unexpected failure in subsequent signing operations.
if (availableCurves.has('prime256v1') && availableHashes.has('sha256')) {
const curve = crypto.createECDH('prime256v1');
const invalidKey = Buffer.alloc(65);
invalidKey.fill('\0');
curve.generateKeys();
assert.throws(() => {
curve.computeSecret(invalidKey);
}, /^Error: Failed to translate Buffer to a EC_POINT$/);
// Check that signing operations are not impacted by the above error.
const ecPrivateKey =
'-----BEGIN EC PRIVATE KEY-----\n' +
'MHcCAQEEIF+jnWY1D5kbVYDNvxxo/Y+ku2uJPDwS0r/VuPZQrjjVoAoGCCqGSM49\n' +
'AwEHoUQDQgAEurOxfSxmqIRYzJVagdZfMMSjRNNhB8i3mXyIMq704m2m52FdfKZ2\n' +
'pQhByd5eyj3lgZ7m7jbchtdgyOF8Io/1ng==\n' +
'-----END EC PRIVATE KEY-----';
assert.doesNotThrow(() => {
crypto.createSign('SHA256').sign(ecPrivateKey);
});
}
// invalid test: curve argument is undefined // invalid test: curve argument is undefined
assert.throws(() => { assert.throws(() => {
crypto.createECDH(); crypto.createECDH();

Loading…
Cancel
Save