Skip to content

Commit

Permalink
crypto: clear err stack after ECDH::BufferToPoint
Browse files Browse the repository at this point in the history
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: #13275
Backport-PR-URL: #13397
Reviewed-By: Fedor Indutny <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Sam Roberts <[email protected]>
Reviewed-By: James M Snell <[email protected]>
  • Loading branch information
rfk authored and MylesBorins committed Jul 11, 2017
1 parent 55cbe24 commit 8f8dd97
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 0 deletions.
4 changes: 4 additions & 0 deletions src/node_crypto.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5011,6 +5011,8 @@ void ECDH::ComputeSecret(const FunctionCallbackInfo<Value>& 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");

Expand Down Expand Up @@ -5160,6 +5162,8 @@ void ECDH::SetPublicKey(const FunctionCallbackInfo<Value>& 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<Object>()),
Buffer::Length(args[0].As<Object>()));
if (pub == nullptr)
Expand Down
20 changes: 20 additions & 0 deletions test/parallel/test-crypto-dh.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down

0 comments on commit 8f8dd97

Please sign in to comment.