From a60a849ee2bbf399c132afc7a12fcc92496dc4d5 Mon Sep 17 00:00:00 2001 From: Darshan Sen Date: Sat, 19 Mar 2022 18:51:56 +0530 Subject: [PATCH] crypto: fix error code handling in ParsePrivateKey() This changes the code to select the latest error code instead of the earliest one from the OpenSSL error stack. It helps in getting rid of the inconsistency between the empty passphrase related error codes of OpenSSL 1.1.1 and 3. Refs: https://github.com/nodejs/node/pull/42319#discussion_r829884295 Signed-off-by: Darshan Sen --- src/crypto/crypto_context.cc | 2 +- src/crypto/crypto_keys.cc | 6 +++- test/parallel/test-crypto-key-objects.js | 6 +--- test/parallel/test-crypto-keygen.js | 35 ++++--------------- .../test-crypto-private-decrypt-gh32240.js | 5 +-- 5 files changed, 15 insertions(+), 39 deletions(-) diff --git a/src/crypto/crypto_context.cc b/src/crypto/crypto_context.cc index 739b559c3b7b..55fa838635c4 100644 --- a/src/crypto/crypto_context.cc +++ b/src/crypto/crypto_context.cc @@ -143,7 +143,7 @@ int SSL_CTX_use_certificate_chain(SSL_CTX* ctx, X509Pointer* cert, X509Pointer* issuer) { // Just to ensure that `ERR_peek_last_error` below will return only errors - // that we are interested in + // that we are interested in. ERR_clear_error(); X509Pointer x( diff --git a/src/crypto/crypto_keys.cc b/src/crypto/crypto_keys.cc index 5dd04edfbfa1..81ec50e6d256 100644 --- a/src/crypto/crypto_keys.cc +++ b/src/crypto/crypto_keys.cc @@ -214,6 +214,10 @@ ParseKeyResult ParsePrivateKey(EVPKeyPointer* pkey, const PrivateKeyEncodingConfig& config, const char* key, size_t key_len) { + // Just to ensure that `ERR_peek_last_error` below will return only errors + // that we are interested in. + ERR_clear_error(); + const ByteSource* passphrase = config.passphrase_.get(); if (config.format_ == kKeyFormatPEM) { @@ -255,7 +259,7 @@ ParseKeyResult ParsePrivateKey(EVPKeyPointer* pkey, } // OpenSSL can fail to parse the key but still return a non-null pointer. - unsigned long err = ERR_peek_error(); // NOLINT(runtime/int) + unsigned long err = ERR_peek_last_error(); // NOLINT(runtime/int) if (err != 0) pkey->reset(); diff --git a/test/parallel/test-crypto-key-objects.js b/test/parallel/test-crypto-key-objects.js index 40a982ea7b6c..e7be5c5ff95e 100644 --- a/test/parallel/test-crypto-key-objects.js +++ b/test/parallel/test-crypto-key-objects.js @@ -518,11 +518,7 @@ const privateDsa = fixtures.readKey('dsa_private_encrypted_1025.pem', { // Reading an encrypted key without a passphrase should fail. - assert.throws(() => createPrivateKey(privateDsa), common.hasOpenSSL3 ? { - name: 'Error', - message: 'error:07880109:common libcrypto routines::interrupted or ' + - 'cancelled', - } : { + assert.throws(() => createPrivateKey(privateDsa), { name: 'TypeError', code: 'ERR_MISSING_PASSPHRASE', message: 'Passphrase required for encrypted key' diff --git a/test/parallel/test-crypto-keygen.js b/test/parallel/test-crypto-keygen.js index b499087d8656..5b0186e88eb6 100644 --- a/test/parallel/test-crypto-keygen.js +++ b/test/parallel/test-crypto-keygen.js @@ -211,16 +211,11 @@ const sec1EncExp = (cipher) => getRegExpForPEM('EC PRIVATE KEY', cipher); // Since the private key is encrypted, signing shouldn't work anymore. const publicKey = { key: publicKeyDER, ...publicKeyEncoding }; - const expectedError = common.hasOpenSSL3 ? { - name: 'Error', - message: 'error:07880109:common libcrypto routines::interrupted or ' + - 'cancelled' - } : { + assert.throws(() => testSignVerify(publicKey, privateKey), { name: 'TypeError', code: 'ERR_MISSING_PASSPHRASE', message: 'Passphrase required for encrypted key' - }; - assert.throws(() => testSignVerify(publicKey, privateKey), expectedError); + }); const key = { key: privateKey, passphrase: 'secret' }; testEncryptDecrypt(publicKey, key); @@ -565,10 +560,7 @@ const sec1EncExp = (cipher) => getRegExpForPEM('EC PRIVATE KEY', cipher); // Since the private key is encrypted, signing shouldn't work anymore. assert.throws(() => testSignVerify(publicKey, privateKey), - common.hasOpenSSL3 ? { - message: 'error:07880109:common libcrypto ' + - 'routines::interrupted or cancelled' - } : { + { name: 'TypeError', code: 'ERR_MISSING_PASSPHRASE', message: 'Passphrase required for encrypted key' @@ -599,10 +591,7 @@ const sec1EncExp = (cipher) => getRegExpForPEM('EC PRIVATE KEY', cipher); // Since the private key is encrypted, signing shouldn't work anymore. assert.throws(() => testSignVerify(publicKey, privateKey), - common.hasOpenSSL3 ? { - message: 'error:07880109:common libcrypto ' + - 'routines::interrupted or cancelled' - } : { + { name: 'TypeError', code: 'ERR_MISSING_PASSPHRASE', message: 'Passphrase required for encrypted key' @@ -636,10 +625,7 @@ const sec1EncExp = (cipher) => getRegExpForPEM('EC PRIVATE KEY', cipher); // Since the private key is encrypted, signing shouldn't work anymore. assert.throws(() => testSignVerify(publicKey, privateKey), - common.hasOpenSSL3 ? { - message: 'error:07880109:common libcrypto ' + - 'routines::interrupted or cancelled' - } : { + { name: 'TypeError', code: 'ERR_MISSING_PASSPHRASE', message: 'Passphrase required for encrypted key' @@ -674,10 +660,7 @@ const sec1EncExp = (cipher) => getRegExpForPEM('EC PRIVATE KEY', cipher); // Since the private key is encrypted, signing shouldn't work anymore. assert.throws(() => testSignVerify(publicKey, privateKey), - common.hasOpenSSL3 ? { - message: 'error:07880109:common libcrypto ' + - 'routines::interrupted or cancelled' - } : { + { name: 'TypeError', code: 'ERR_MISSING_PASSPHRASE', message: 'Passphrase required for encrypted key' @@ -1571,11 +1554,7 @@ for (const type of ['pkcs1', 'pkcs8']) { // the key, and not specifying a passphrase should fail when decoding it. assert.throws(() => { return testSignVerify(publicKey, privateKey); - }, common.hasOpenSSL3 ? { - name: 'Error', - code: 'ERR_OSSL_CRYPTO_INTERRUPTED_OR_CANCELLED', - message: 'error:07880109:common libcrypto routines::interrupted or cancelled' - } : { + }, { name: 'TypeError', code: 'ERR_MISSING_PASSPHRASE', message: 'Passphrase required for encrypted key' diff --git a/test/parallel/test-crypto-private-decrypt-gh32240.js b/test/parallel/test-crypto-private-decrypt-gh32240.js index 1785f5eef3d2..4b48774145a3 100644 --- a/test/parallel/test-crypto-private-decrypt-gh32240.js +++ b/test/parallel/test-crypto-private-decrypt-gh32240.js @@ -34,8 +34,5 @@ function decrypt(key) { } decrypt(pkey); -assert.throws(() => decrypt(pkeyEncrypted), common.hasOpenSSL3 ? - { message: 'error:07880109:common libcrypto routines::interrupted or ' + - 'cancelled' } : - { code: 'ERR_MISSING_PASSPHRASE' }); +assert.throws(() => decrypt(pkeyEncrypted), { code: 'ERR_MISSING_PASSPHRASE' }); decrypt(pkey); // Should not throw.