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 739b559c3b7b22..55fa838635c46d 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 5dd04edfbfa1cf..81ec50e6d2569b 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 40a982ea7b6cd6..e7be5c5ff95e4d 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 b499087d86568a..5b0186e88eb60e 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 1785f5eef3d202..4b48774145a3f8 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.