From 8f8dd97072d9ee0ac21a2f71fb01b4b68fda7d43 Mon Sep 17 00:00:00 2001 From: Ryan Kelly Date: Mon, 29 May 2017 14:20:04 +1000 Subject: [PATCH] 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 Backport-PR-URL: https://github.com/nodejs/node/pull/13397 Reviewed-By: Fedor Indutny Reviewed-By: Ben Noordhuis Reviewed-By: Sam Roberts Reviewed-By: James M Snell --- src/node_crypto.cc | 4 ++++ test/parallel/test-crypto-dh.js | 20 ++++++++++++++++++++ 2 files changed, 24 insertions(+) diff --git a/src/node_crypto.cc b/src/node_crypto.cc index db015339880d06..6e5db66313f61c 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -5011,6 +5011,8 @@ void ECDH::ComputeSecret(const FunctionCallbackInfo& args) { ECDH* ecdh; ASSIGN_OR_RETURN_UNWRAP(&ecdh, args.Holder()); + MarkPopErrorOnReturn mark_pop_error_on_return; + if (!ecdh->IsKeyPairValid()) return env->ThrowError("Invalid key pair"); @@ -5160,6 +5162,8 @@ void ECDH::SetPublicKey(const FunctionCallbackInfo& args) { 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()), Buffer::Length(args[0].As())); if (pub == nullptr) diff --git a/test/parallel/test-crypto-dh.js b/test/parallel/test-crypto-dh.js index cfe896026f6af5..50b0b2d07b9aea 100644 --- a/test/parallel/test-crypto-dh.js +++ b/test/parallel/test-crypto-dh.js @@ -270,6 +270,26 @@ ecdh5.setPrivateKey(cafebabeKey, 'hex'); assert.strictEqual(ecdh5.getPrivateKey('hex'), cafebabeKey); }); +// Use of invalid keys was not cleaning up ERR stack, and was causing +// unexpected failure in subsequent signing operations. +const ecdh6 = crypto.createECDH('prime256v1'); +const invalidKey = Buffer.alloc(65); +invalidKey.fill('\0'); +ecdh6.generateKeys(); +assert.throws(() => { + ecdh6.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 assert.throws(() => { crypto.createECDH();