Skip to content

Commit 108c698

Browse files
bnoordhuisrvagg
authored andcommittedFeb 28, 2019
crypto: make ConvertKey clear openssl error stack
Failed ConvertKey() operations should not leave errors on OpenSSL's error stack because that's observable by subsequent cryptographic operations. More to the point, it makes said operations fail with an unrelated error. PR-URL: #26153 Fixes: #26133 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Daniel Bevenius <[email protected]> Reviewed-By: James M Snell <[email protected]>
1 parent a3f7471 commit 108c698

File tree

2 files changed

+26
-1
lines changed

2 files changed

+26
-1
lines changed
 

‎src/node_crypto.cc

+1
Original file line numberDiff line numberDiff line change
@@ -6095,6 +6095,7 @@ void ExportChallenge(const FunctionCallbackInfo<Value>& args) {
60956095

60966096
// Convert the input public key to compressed, uncompressed, or hybrid formats.
60976097
void ConvertKey(const FunctionCallbackInfo<Value>& args) {
6098+
MarkPopErrorOnReturn mark_pop_error_on_return;
60986099
Environment* env = Environment::GetCurrent(args);
60996100

61006101
CHECK_EQ(args.Length(), 3);

‎test/parallel/test-crypto-ecdh-convert-key.js

+25-1
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ if (!common.hasCrypto)
55

66
const assert = require('assert');
77

8-
const { ECDH, getCurves } = require('crypto');
8+
const { ECDH, createSign, getCurves } = require('crypto');
99

1010
// A valid private key for the secp256k1 curve.
1111
const cafebabeKey = 'cafebabe'.repeat(8);
@@ -99,3 +99,27 @@ if (getCurves().includes('secp256k1')) {
9999
assert.strictEqual(ecdh1.getPublicKey('hex', 'compressed'), compressed);
100100
assert.strictEqual(ecdh1.getPublicKey('hex', 'hybrid'), hybrid);
101101
}
102+
103+
// See https://github.com/nodejs/node/issues/26133, failed ConvertKey
104+
// operations should not leave errors on OpenSSL's error stack because
105+
// that's observable by subsequent operations.
106+
{
107+
const privateKey =
108+
'-----BEGIN EC PRIVATE KEY-----\n' +
109+
'MHcCAQEEIF+jnWY1D5kbVYDNvxxo/Y+ku2uJPDwS0r/VuPZQrjjVoAoGCCqGSM49\n' +
110+
'AwEHoUQDQgAEurOxfSxmqIRYzJVagdZfMMSjRNNhB8i3mXyIMq704m2m52FdfKZ2\n' +
111+
'pQhByd5eyj3lgZ7m7jbchtdgyOF8Io/1ng==\n' +
112+
'-----END EC PRIVATE KEY-----';
113+
114+
const sign = createSign('sha256').update('plaintext');
115+
116+
// TODO(bnoordhuis) This should really bubble up the specific OpenSSL error
117+
// rather than Node's generic error message.
118+
const badKey = 'f'.repeat(128);
119+
assert.throws(
120+
() => ECDH.convertKey(badKey, 'secp256k1', 'hex', 'hex', 'compressed'),
121+
/Failed to convert Buffer to EC_POINT/);
122+
123+
// Next statement should not throw an exception.
124+
sign.sign(privateKey);
125+
}

0 commit comments

Comments
 (0)
Please sign in to comment.