From 36de9dca3b9ee40b6d8f2575c6a103c9b1a43a75 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Tobias=20Nie=C3=9Fen?= <tniessen@tnie.de>
Date: Sun, 23 Dec 2018 15:13:32 +0100
Subject: [PATCH 1/3] crypto: decode missing passphrase errors

When a user attempts to load an encrypted key without supplying a
passphrase, a cryptic OpenSSL error is thrown. This change intercepts
the OpenSSL error and throws a nice error code instead.
---
 doc/api/errors.md                             |   6 +
 src/node_crypto.cc                            | 177 +++++++++++-------
 src/node_errors.h                             |   1 +
 test/fixtures/keys/Makefile                   |   4 +
 .../keys/dsa_private_encrypted_1025.pem       |  12 ++
 test/parallel/test-crypto-key-objects.js      |  27 +++
 test/parallel/test-crypto-keygen.js           |  44 +++--
 7 files changed, 192 insertions(+), 79 deletions(-)
 create mode 100644 test/fixtures/keys/dsa_private_encrypted_1025.pem

diff --git a/doc/api/errors.md b/doc/api/errors.md
index 44ec5548aeab35..c23a90fcbcbe3d 100644
--- a/doc/api/errors.md
+++ b/doc/api/errors.md
@@ -807,6 +807,12 @@ instance, calling [`cipher.getAuthTag()`][] before calling `cipher.final()`.
 The PBKDF2 algorithm failed for unspecified reasons. OpenSSL does not provide
 more details and therefore neither does Node.js.
 
+<a id="ERR_CRYPTO_READ_KEY"></a>
+### ERR_CRYPTO_READ_KEY
+
+An error occurred while parsing a cryptographic key, e.g., the key is encrypted
+but no decryption passphrase was specified.
+
 <a id="ERR_CRYPTO_SCRYPT_INVALID_PARAMETER"></a>
 ### ERR_CRYPTO_SCRYPT_INVALID_PARAMETER
 
diff --git a/src/node_crypto.cc b/src/node_crypto.cc
index 9658c1d51a208b..99a7783283c9d1 100644
--- a/src/node_crypto.cc
+++ b/src/node_crypto.cc
@@ -153,13 +153,33 @@ template int SSLWrap<TLSWrap>::SelectALPNCallback(
     unsigned int inlen,
     void* arg);
 
+class PasswordCallbackInfo {
+ public:
+  explicit PasswordCallbackInfo(const char* passphrase)
+      : passphrase_(passphrase) {}
+
+  inline const char* GetPassword() {
+    needs_passphrase_ = true;
+    return passphrase_;
+  }
+
+  inline bool CalledButEmpty() {
+    return needs_passphrase_ && passphrase_ == nullptr;
+  }
+
+ private:
+  const char* passphrase_;
+  bool needs_passphrase_ = false;
+};
 
 static int PasswordCallback(char* buf, int size, int rwflag, void* u) {
-  if (u) {
+  PasswordCallbackInfo* info = static_cast<PasswordCallbackInfo*>(u);
+  const char* passphrase = info->GetPassword();
+  if (passphrase != nullptr) {
     size_t buflen = static_cast<size_t>(size);
-    size_t len = strlen(static_cast<const char*>(u));
+    size_t len = strlen(passphrase);
     len = len > buflen ? buflen : len;
-    memcpy(buf, u, len);
+    memcpy(buf, passphrase, len);
     return len;
   }
 
@@ -698,11 +718,12 @@ void SecureContext::SetKey(const FunctionCallbackInfo<Value>& args) {
 
   node::Utf8Value passphrase(env->isolate(), args[1]);
 
+  PasswordCallbackInfo cb_info(len == 1 ? nullptr : *passphrase);
   EVPKeyPointer key(
       PEM_read_bio_PrivateKey(bio.get(),
                               nullptr,
                               PasswordCallback,
-                              len == 1 ? nullptr : *passphrase));
+                              &cb_info));
 
   if (!key) {
     unsigned long err = ERR_get_error();  // NOLINT(runtime/int)
@@ -2899,13 +2920,14 @@ static bool IsSupportedAuthenticatedMode(const EVP_CIPHER_CTX* ctx) {
   return IsSupportedAuthenticatedMode(cipher);
 }
 
-enum class ParsePublicKeyResult {
-  kParsePublicOk,
-  kParsePublicNotRecognized,
-  kParsePublicFailed
+enum class ParseKeyResult {
+  kParseKeyOk,
+  kParseKeyNotRecognized,
+  kParseKeyNeedPassphrase,
+  kParseKeyFailed
 };
 
-static ParsePublicKeyResult TryParsePublicKey(
+static ParseKeyResult TryParsePublicKey(
     EVPKeyPointer* pkey,
     const BIOPointer& bp,
     const char* name,
@@ -2919,7 +2941,7 @@ static ParsePublicKeyResult TryParsePublicKey(
     MarkPopErrorOnReturn mark_pop_error_on_return;
     if (PEM_bytes_read_bio(&der_data, &der_len, nullptr, name,
                            bp.get(), nullptr, nullptr) != 1)
-      return ParsePublicKeyResult::kParsePublicNotRecognized;
+      return ParseKeyResult::kParseKeyNotRecognized;
   }
 
   // OpenSSL might modify the pointer, so we need to make a copy before parsing.
@@ -2927,25 +2949,25 @@ static ParsePublicKeyResult TryParsePublicKey(
   pkey->reset(parse(&p, der_len));
   OPENSSL_clear_free(der_data, der_len);
 
-  return *pkey ? ParsePublicKeyResult::kParsePublicOk :
-                 ParsePublicKeyResult::kParsePublicFailed;
+  return *pkey ? ParseKeyResult::kParseKeyOk :
+                 ParseKeyResult::kParseKeyFailed;
 }
 
-static ParsePublicKeyResult ParsePublicKeyPEM(EVPKeyPointer* pkey,
-                                              const char* key_pem,
-                                              int key_pem_len) {
+static ParseKeyResult ParsePublicKeyPEM(EVPKeyPointer* pkey,
+                                        const char* key_pem,
+                                        int key_pem_len) {
   BIOPointer bp(BIO_new_mem_buf(const_cast<char*>(key_pem), key_pem_len));
   if (!bp)
-    return ParsePublicKeyResult::kParsePublicFailed;
+    return ParseKeyResult::kParseKeyFailed;
 
-  ParsePublicKeyResult ret;
+  ParseKeyResult ret;
 
   // Try parsing as a SubjectPublicKeyInfo first.
   ret = TryParsePublicKey(pkey, bp, "PUBLIC KEY",
       [](const unsigned char** p, long l) {  // NOLINT(runtime/int)
         return d2i_PUBKEY(nullptr, p, l);
       });
-  if (ret != ParsePublicKeyResult::kParsePublicNotRecognized)
+  if (ret != ParseKeyResult::kParseKeyNotRecognized)
     return ret;
 
   // Maybe it is PKCS#1.
@@ -2954,7 +2976,7 @@ static ParsePublicKeyResult ParsePublicKeyPEM(EVPKeyPointer* pkey,
       [](const unsigned char** p, long l) {  // NOLINT(runtime/int)
         return d2i_PublicKey(EVP_PKEY_RSA, nullptr, p, l);
       });
-  if (ret != ParsePublicKeyResult::kParsePublicNotRecognized)
+  if (ret != ParseKeyResult::kParseKeyNotRecognized)
     return ret;
 
   // X.509 fallback.
@@ -2966,25 +2988,25 @@ static ParsePublicKeyResult ParsePublicKeyPEM(EVPKeyPointer* pkey,
       });
 }
 
-static bool ParsePublicKey(EVPKeyPointer* pkey,
-                           const PublicKeyEncodingConfig& config,
-                           const char* key,
-                           size_t key_len) {
+static ParseKeyResult ParsePublicKey(EVPKeyPointer* pkey,
+                                     const PublicKeyEncodingConfig& config,
+                                     const char* key,
+                                     size_t key_len) {
   if (config.format_ == kKeyFormatPEM) {
-    ParsePublicKeyResult r =
-        ParsePublicKeyPEM(pkey, key, key_len);
-    return r == ParsePublicKeyResult::kParsePublicOk;
+    return ParsePublicKeyPEM(pkey, key, key_len);
   } else {
     CHECK_EQ(config.format_, kKeyFormatDER);
+
     const unsigned char* p = reinterpret_cast<const unsigned char*>(key);
     if (config.type_.ToChecked() == kKeyEncodingPKCS1) {
       pkey->reset(d2i_PublicKey(EVP_PKEY_RSA, nullptr, &p, key_len));
-      return pkey;
     } else {
       CHECK_EQ(config.type_.ToChecked(), kKeyEncodingSPKI);
       pkey->reset(d2i_PUBKEY(nullptr, &p, key_len));
-      return pkey;
     }
+
+    return *pkey ? ParseKeyResult::kParseKeyOk :
+                   ParseKeyResult::kParseKeyFailed;
   }
 }
 
@@ -3099,56 +3121,59 @@ static bool IsEncryptedPrivateKeyInfo(const unsigned char* data, size_t size) {
          data[offset] != 2;
 }
 
-static EVPKeyPointer ParsePrivateKey(const PrivateKeyEncodingConfig& config,
-                                     const char* key,
-                                     size_t key_len) {
-  EVPKeyPointer pkey;
+static ParseKeyResult ParsePrivateKey(EVPKeyPointer* pkey,
+                                      const PrivateKeyEncodingConfig& config,
+                                      const char* key,
+                                      size_t key_len) {
+  PasswordCallbackInfo pc_info(config.passphrase_.get());
 
   if (config.format_ == kKeyFormatPEM) {
     BIOPointer bio(BIO_new_mem_buf(key, key_len));
     if (!bio)
-      return pkey;
+      return ParseKeyResult::kParseKeyFailed;
 
-    char* pass = const_cast<char*>(config.passphrase_.get());
-    pkey.reset(PEM_read_bio_PrivateKey(bio.get(),
-                                       nullptr,
-                                       PasswordCallback,
-                                       pass));
+    pkey->reset(PEM_read_bio_PrivateKey(bio.get(),
+                                        nullptr,
+                                        PasswordCallback,
+                                        &pc_info));
   } else {
     CHECK_EQ(config.format_, kKeyFormatDER);
 
     if (config.type_.ToChecked() == kKeyEncodingPKCS1) {
       const unsigned char* p = reinterpret_cast<const unsigned char*>(key);
-      pkey.reset(d2i_PrivateKey(EVP_PKEY_RSA, nullptr, &p, key_len));
+      pkey->reset(d2i_PrivateKey(EVP_PKEY_RSA, nullptr, &p, key_len));
     } else if (config.type_.ToChecked() == kKeyEncodingPKCS8) {
       BIOPointer bio(BIO_new_mem_buf(key, key_len));
       if (!bio)
-        return pkey;
+        return ParseKeyResult::kParseKeyFailed;
 
       if (IsEncryptedPrivateKeyInfo(
               reinterpret_cast<const unsigned char*>(key), key_len)) {
-        char* pass = const_cast<char*>(config.passphrase_.get());
-        pkey.reset(d2i_PKCS8PrivateKey_bio(bio.get(),
-                                           nullptr,
-                                           PasswordCallback,
-                                           pass));
+        pkey->reset(d2i_PKCS8PrivateKey_bio(bio.get(),
+                                            nullptr,
+                                            PasswordCallback,
+                                            &pc_info));
       } else {
         PKCS8Pointer p8inf(d2i_PKCS8_PRIV_KEY_INFO_bio(bio.get(), nullptr));
         if (p8inf)
-          pkey.reset(EVP_PKCS82PKEY(p8inf.get()));
+          pkey->reset(EVP_PKCS82PKEY(p8inf.get()));
       }
     } else {
       CHECK_EQ(config.type_.ToChecked(), kKeyEncodingSEC1);
       const unsigned char* p = reinterpret_cast<const unsigned char*>(key);
-      pkey.reset(d2i_PrivateKey(EVP_PKEY_EC, nullptr, &p, key_len));
+      pkey->reset(d2i_PrivateKey(EVP_PKEY_EC, nullptr, &p, key_len));
     }
   }
 
   // OpenSSL can fail to parse the key but still return a non-null pointer.
   if (ERR_peek_error() != 0)
-    pkey.reset();
+    pkey->reset();
 
-  return pkey;
+  if (*pkey)
+    return ParseKeyResult::kParseKeyOk;
+  if (pc_info.CalledButEmpty())
+    return ParseKeyResult::kParseKeyNeedPassphrase;
+  return ParseKeyResult::kParseKeyFailed;
 }
 
 ByteSource::ByteSource(ByteSource&& other)
@@ -3284,6 +3309,26 @@ static PublicKeyEncodingConfig GetPublicKeyEncodingFromJs(
   return result;
 }
 
+#define READ_FAILURE_MSG(type) ("Failed to read " type " key")
+
+static inline ManagedEVPPKey GetParsedKey(Environment* env,
+                                          EVPKeyPointer&& pkey,
+                                          ParseKeyResult ret,
+                                          const char* default_msg) {
+  switch (ret) {
+    case ParseKeyResult::kParseKeyOk:
+      CHECK(pkey);
+      break;
+    case ParseKeyResult::kParseKeyNeedPassphrase:
+      THROW_ERR_CRYPTO_READ_KEY(env, "Passphrase required for encrypted key");
+      break;
+    default:
+      ThrowCryptoError(env, ERR_get_error(), default_msg);
+  }
+
+  return ManagedEVPPKey(std::move(pkey));
+}
+
 static NonCopyableMaybe<PrivateKeyEncodingConfig> GetPrivateKeyEncodingFromJs(
     const FunctionCallbackInfo<Value>& args,
     unsigned int* offset,
@@ -3339,11 +3384,11 @@ static ManagedEVPPKey GetPrivateKeyFromJs(
         GetPrivateKeyEncodingFromJs(args, offset, kKeyContextInput);
     if (config.IsEmpty())
       return ManagedEVPPKey();
-    EVPKeyPointer pkey =
-        ParsePrivateKey(config.Release(), key.get(), key.size());
-    if (!pkey)
-      ThrowCryptoError(env, ERR_get_error(), "Failed to read private key");
-    return ManagedEVPPKey(std::move(pkey));
+
+    EVPKeyPointer pkey;
+    ParseKeyResult ret =
+        ParsePrivateKey(&pkey, config.Release(), key.get(), key.size());
+    return GetParsedKey(env, std::move(pkey), ret, READ_FAILURE_MSG("private"));
   } else {
     CHECK(args[*offset]->IsObject() && allow_key_object);
     KeyObject* key;
@@ -3364,15 +3409,16 @@ static ManagedEVPPKey GetPublicOrPrivateKeyFromJs(
         GetPrivateKeyEncodingFromJs(args, offset, kKeyContextInput);
     if (config_.IsEmpty())
       return ManagedEVPPKey();
+
+    ParseKeyResult ret;
     PrivateKeyEncodingConfig config = config_.Release();
     EVPKeyPointer pkey;
     if (config.format_ == kKeyFormatPEM) {
       // For PEM, we can easily determine whether it is a public or private key
       // by looking for the respective PEM tags.
-      ParsePublicKeyResult ret = ParsePublicKeyPEM(&pkey, data.get(),
-                                                   data.size());
-      if (ret == ParsePublicKeyResult::kParsePublicNotRecognized) {
-        pkey = ParsePrivateKey(config, data.get(), data.size());
+      ret = ParsePublicKeyPEM(&pkey, data.get(), data.size());
+      if (ret == ParseKeyResult::kParseKeyNotRecognized) {
+        ret = ParsePrivateKey(&pkey, config, data.get(), data.size());
       }
     } else {
       // For DER, the type determines how to parse it. SPKI, PKCS#8 and SEC1 are
@@ -3395,14 +3441,14 @@ static ManagedEVPPKey GetPublicOrPrivateKeyFromJs(
       }
 
       if (is_public) {
-        ParsePublicKey(&pkey, config, data.get(), data.size());
+        ret = ParsePublicKey(&pkey, config, data.get(), data.size());
       } else {
-        pkey = ParsePrivateKey(config, data.get(), data.size());
+        ret = ParsePrivateKey(&pkey, config, data.get(), data.size());
       }
     }
-    if (!pkey)
-      ThrowCryptoError(env, ERR_get_error(), "Failed to read asymmetric key");
-    return ManagedEVPPKey(std::move(pkey));
+
+    return GetParsedKey(env, std::move(pkey), ret,
+                        READ_FAILURE_MSG("asymmetric"));
   } else {
     CHECK(args[*offset]->IsObject());
     KeyObject* key = Unwrap<KeyObject>(args[*offset].As<Object>());
@@ -3585,6 +3631,7 @@ KeyType KeyObject::GetKeyType() const {
 void KeyObject::Init(const FunctionCallbackInfo<Value>& args) {
   KeyObject* key;
   ASSIGN_OR_RETURN_UNWRAP(&key, args.Holder());
+  MarkPopErrorOnReturn mark_pop_error_on_return;
 
   unsigned int offset;
   ManagedEVPPKey pkey;
@@ -4780,6 +4827,8 @@ void Sign::SignFinal(const FunctionCallbackInfo<Value>& args) {
   Sign* sign;
   ASSIGN_OR_RETURN_UNWRAP(&sign, args.Holder());
 
+  ClearErrorOnReturn clear_error_on_return;
+
   unsigned int offset = 0;
   ManagedEVPPKey key = GetPrivateKeyFromJs(args, &offset, true);
   if (!key)
@@ -4791,8 +4840,6 @@ void Sign::SignFinal(const FunctionCallbackInfo<Value>& args) {
   CHECK(args[offset + 1]->IsInt32());
   int salt_len = args[offset + 1].As<Int32>()->Value();
 
-  ClearErrorOnReturn clear_error_on_return;
-
   SignResult ret = sign->SignFinal(
       key,
       padding,
diff --git a/src/node_errors.h b/src/node_errors.h
index 9d3f2ead71279d..b7387f1dca657e 100644
--- a/src/node_errors.h
+++ b/src/node_errors.h
@@ -43,6 +43,7 @@ void FatalException(v8::Isolate* isolate,
   V(ERR_BUFFER_TOO_LARGE, Error)                                             \
   V(ERR_CANNOT_TRANSFER_OBJECT, TypeError)                                   \
   V(ERR_CONSTRUCT_CALL_REQUIRED, Error)                                      \
+  V(ERR_CRYPTO_READ_KEY, TypeError)                                          \
   V(ERR_INVALID_ARG_VALUE, TypeError)                                        \
   V(ERR_INVALID_ARG_TYPE, TypeError)                                         \
   V(ERR_INVALID_MODULE_SPECIFIER, TypeError)                                 \
diff --git a/test/fixtures/keys/Makefile b/test/fixtures/keys/Makefile
index 964fb2f5f1451b..3be2cd6655f7e6 100644
--- a/test/fixtures/keys/Makefile
+++ b/test/fixtures/keys/Makefile
@@ -26,6 +26,7 @@ all: \
   dh2048.pem \
   dsa1025.pem \
   dsa_private_1025.pem \
+  dsa_private_encrypted_1025.pem \
   dsa_public_1025.pem \
   ec-cert.pem \
   ec.pfx \
@@ -549,6 +550,9 @@ dsa1025.pem:
 dsa_private_1025.pem:
 	openssl gendsa -out dsa_private_1025.pem dsa1025.pem
 
+dsa_private_encrypted_1025.pem:
+	openssl pkcs8 -in dsa_private_1025.pem -topk8 -passout 'pass:secret' -out dsa_private_encrypted_1025.pem
+
 dsa_public_1025.pem:
 	openssl dsa -in dsa_private_1025.pem -pubout -out dsa_public_1025.pem
 
diff --git a/test/fixtures/keys/dsa_private_encrypted_1025.pem b/test/fixtures/keys/dsa_private_encrypted_1025.pem
new file mode 100644
index 00000000000000..b75ffaf90df5ea
--- /dev/null
+++ b/test/fixtures/keys/dsa_private_encrypted_1025.pem
@@ -0,0 +1,12 @@
+-----BEGIN ENCRYPTED PRIVATE KEY-----
+MIIBvTBXBgkqhkiG9w0BBQ0wSjApBgkqhkiG9w0BBQwwHAQIqTW00yecdxMCAggA
+MAwGCCqGSIb3DQIJBQAwHQYJYIZIAWUDBAEqBBBKgO4UF0LfCkPyS+iCvSrtBIIB
+YD3W6FyEZ97/crnoyRqjPUtr2Mm4KJMtaB5ZiGFzZEzd6AH7N/dbtAAMIibtsjmd
+RYdIptpET6xTpUhM8TvpULyYaZnhZJKTpVUrTVdvFTS3DYDutu7aWRLTrle6LzcY
+XpIppeP8ZmYFdRBQxhF+KoDsP4O0QA+vWl2W2VmRfr+sK9R+qV89w0YMjEWHsYY+
+VZsDbJBGKkj9gzIvxIsRyack/+RsbiSDrh6WTw+D0jrX/IMbgPjvYfBFhpxGC7zR
+hDn9r3JaO2KdHh9kMtvQfshA1n636kb0X6ewY57BhEs3J4hpMg46c6YFry94to24
+jxl5KutM0CFea7mYGtNf6WJXBsm7JSW03kjlqYoZGK43KNgZhzKAsXaNkoRkA5cw
+BzGfgmG6dHTpeAY9G4vM4inhCmGFA8Tx189g+xzRv16uFXRb8WFIllne1fEFaXRr
+1Rz2G6SPJkA3fsrl8zUIB0Y=
+-----END ENCRYPTED PRIVATE KEY-----
diff --git a/test/parallel/test-crypto-key-objects.js b/test/parallel/test-crypto-key-objects.js
index c9cb90d408500f..f7948b05859fd3 100644
--- a/test/parallel/test-crypto-key-objects.js
+++ b/test/parallel/test-crypto-key-objects.js
@@ -21,6 +21,10 @@ const fixtures = require('../common/fixtures');
 const publicPem = fixtures.readSync('test_rsa_pubkey.pem', 'ascii');
 const privatePem = fixtures.readSync('test_rsa_privkey.pem', 'ascii');
 
+const publicDsa = fixtures.readKey('dsa_public_1025.pem', 'ascii');
+const privateDsa = fixtures.readKey('dsa_private_encrypted_1025.pem',
+                                    'ascii');
+
 {
   // Attempting to create an empty key should throw.
   common.expectsError(() => {
@@ -210,3 +214,26 @@ const privatePem = fixtures.readSync('test_rsa_privkey.pem', 'ascii');
     });
   }
 });
+
+{
+  // Reading an encrypted key without a passphrase should fail.
+  common.expectsError(() => createPrivateKey(privateDsa), {
+    type: TypeError,
+    code: 'ERR_CRYPTO_READ_KEY',
+    message: 'Passphrase required for encrypted key'
+  });
+
+  const publicKey = createPublicKey(publicDsa);
+  assert.strictEqual(publicKey.type, 'public');
+  assert.strictEqual(publicKey.asymmetricKeyType, 'dsa');
+  assert.strictEqual(publicKey.symmetricKeySize, undefined);
+
+  const privateKey = createPrivateKey({
+    key: privateDsa,
+    format: 'pem',
+    passphrase: 'secret'
+  });
+  assert.strictEqual(privateKey.type, 'private');
+  assert.strictEqual(privateKey.asymmetricKeyType, 'dsa');
+  assert.strictEqual(privateKey.symmetricKeySize, undefined);
+}
diff --git a/test/parallel/test-crypto-keygen.js b/test/parallel/test-crypto-keygen.js
index 64964cc7aedf7d..535c676e50f003 100644
--- a/test/parallel/test-crypto-keygen.js
+++ b/test/parallel/test-crypto-keygen.js
@@ -166,9 +166,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 };
-    assert.throws(() => {
-      testSignVerify(publicKey, privateKey);
-    }, /bad decrypt|asn1 encoding routines/);
+    common.expectsError(() => testSignVerify(publicKey, privateKey), {
+      type: TypeError,
+      code: 'ERR_CRYPTO_READ_KEY',
+      message: 'Passphrase required for encrypted key'
+    });
 
     const key = { key: privateKey, passphrase: 'secret' };
     testEncryptDecrypt(publicKey, key);
@@ -196,13 +198,19 @@ const sec1EncExp = (cipher) => getRegExpForPEM('EC PRIVATE KEY', cipher);
 
     // Since the private key is encrypted, signing shouldn't work anymore.
     const publicKey = { key: publicKeyDER, ...publicKeyEncoding };
-    assert.throws(() => {
+    common.expectsError(() => {
       testSignVerify(publicKey, {
         key: privateKeyDER,
         format: 'der',
         type: 'pkcs8'
       });
-    }, /bad decrypt|asn1 encoding routines/);
+    }, {
+      type: TypeError,
+      code: 'ERR_CRYPTO_READ_KEY',
+      message: 'Passphrase required for encrypted key'
+    });
+
+    // Signing should work with the correct password.
 
     const privateKey = {
       key: privateKeyDER,
@@ -274,12 +282,16 @@ const sec1EncExp = (cipher) => getRegExpForPEM('EC PRIVATE KEY', cipher);
     assertApproximateSize(privateKeyDER, 336);
 
     // Since the private key is encrypted, signing shouldn't work anymore.
-    assert.throws(() => {
-      testSignVerify(publicKey, {
+    common.expectsError(() => {
+      return testSignVerify(publicKey, {
         key: privateKeyDER,
         ...privateKeyEncoding
       });
-    }, /bad decrypt|asn1 encoding routines/);
+    }, {
+      type: TypeError,
+      code: 'ERR_CRYPTO_READ_KEY',
+      message: 'Passphrase required for encrypted key'
+    });
 
     // Signing should work with the correct password.
     testSignVerify(publicKey, {
@@ -338,9 +350,11 @@ const sec1EncExp = (cipher) => getRegExpForPEM('EC PRIVATE KEY', cipher);
     assert(sec1EncExp('AES-128-CBC').test(privateKey));
 
     // Since the private key is encrypted, signing shouldn't work anymore.
-    assert.throws(() => {
-      testSignVerify(publicKey, privateKey);
-    }, /bad decrypt|asn1 encoding routines/);
+    common.expectsError(() => testSignVerify(publicKey, privateKey), {
+      type: TypeError,
+      code: 'ERR_CRYPTO_READ_KEY',
+      message: 'Passphrase required for encrypted key'
+    });
 
     testSignVerify(publicKey, { key: privateKey, passphrase: 'secret' });
   }));
@@ -371,9 +385,11 @@ const sec1EncExp = (cipher) => getRegExpForPEM('EC PRIVATE KEY', cipher);
     assert(pkcs8EncExp.test(privateKey));
 
     // Since the private key is encrypted, signing shouldn't work anymore.
-    assert.throws(() => {
-      testSignVerify(publicKey, privateKey);
-    }, /bad decrypt|asn1 encoding routines/);
+    common.expectsError(() => testSignVerify(publicKey, privateKey), {
+      type: TypeError,
+      code: 'ERR_CRYPTO_READ_KEY',
+      message: 'Passphrase required for encrypted key'
+    });
 
     testSignVerify(publicKey, {
       key: privateKey,

From 2cec81dfbeb53937cffca816a20295b5929ac03e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Tobias=20Nie=C3=9Fen?= <tniessen@tnie.de>
Date: Fri, 29 Mar 2019 21:26:49 +0100
Subject: [PATCH 2/3] Rename error code

---
 doc/api/errors.md                        | 11 +++++------
 src/node_crypto.cc                       |  3 ++-
 src/node_errors.h                        |  2 +-
 test/parallel/test-crypto-key-objects.js |  2 +-
 test/parallel/test-crypto-keygen.js      | 10 +++++-----
 5 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/doc/api/errors.md b/doc/api/errors.md
index c23a90fcbcbe3d..2ad0bead8ec0bb 100644
--- a/doc/api/errors.md
+++ b/doc/api/errors.md
@@ -807,12 +807,6 @@ instance, calling [`cipher.getAuthTag()`][] before calling `cipher.final()`.
 The PBKDF2 algorithm failed for unspecified reasons. OpenSSL does not provide
 more details and therefore neither does Node.js.
 
-<a id="ERR_CRYPTO_READ_KEY"></a>
-### ERR_CRYPTO_READ_KEY
-
-An error occurred while parsing a cryptographic key, e.g., the key is encrypted
-but no decryption passphrase was specified.
-
 <a id="ERR_CRYPTO_SCRYPT_INVALID_PARAMETER"></a>
 ### ERR_CRYPTO_SCRYPT_INVALID_PARAMETER
 
@@ -1492,6 +1486,11 @@ a `dynamicInstantiate` hook.
 A `MessagePort` was found in the object passed to a `postMessage()` call,
 but not provided in the `transferList` for that call.
 
+<a id="ERR_MISSING_PASSPHRASE"></a>
+### ERR_MISSING_PASSPHRASE
+
+An attempt was made to read an encrypted key without specifying a passphrase.
+
 <a id="ERR_MISSING_PLATFORM_FOR_WORKER"></a>
 ### ERR_MISSING_PLATFORM_FOR_WORKER
 
diff --git a/src/node_crypto.cc b/src/node_crypto.cc
index 99a7783283c9d1..583671b920c03a 100644
--- a/src/node_crypto.cc
+++ b/src/node_crypto.cc
@@ -3320,7 +3320,8 @@ static inline ManagedEVPPKey GetParsedKey(Environment* env,
       CHECK(pkey);
       break;
     case ParseKeyResult::kParseKeyNeedPassphrase:
-      THROW_ERR_CRYPTO_READ_KEY(env, "Passphrase required for encrypted key");
+      THROW_ERR_MISSING_PASSPHRASE(env,
+                                   "Passphrase required for encrypted key");
       break;
     default:
       ThrowCryptoError(env, ERR_get_error(), default_msg);
diff --git a/src/node_errors.h b/src/node_errors.h
index b7387f1dca657e..61d9b882212275 100644
--- a/src/node_errors.h
+++ b/src/node_errors.h
@@ -43,7 +43,6 @@ void FatalException(v8::Isolate* isolate,
   V(ERR_BUFFER_TOO_LARGE, Error)                                             \
   V(ERR_CANNOT_TRANSFER_OBJECT, TypeError)                                   \
   V(ERR_CONSTRUCT_CALL_REQUIRED, Error)                                      \
-  V(ERR_CRYPTO_READ_KEY, TypeError)                                          \
   V(ERR_INVALID_ARG_VALUE, TypeError)                                        \
   V(ERR_INVALID_ARG_TYPE, TypeError)                                         \
   V(ERR_INVALID_MODULE_SPECIFIER, TypeError)                                 \
@@ -52,6 +51,7 @@ void FatalException(v8::Isolate* isolate,
   V(ERR_MEMORY_ALLOCATION_FAILED, Error)                                     \
   V(ERR_MISSING_ARGS, TypeError)                                             \
   V(ERR_MISSING_MESSAGE_PORT_IN_TRANSFER_LIST, TypeError)                    \
+  V(ERR_MISSING_PASSPHRASE, TypeError)                                       \
   V(ERR_MISSING_PLATFORM_FOR_WORKER, Error)                                  \
   V(ERR_MODULE_NOT_FOUND, Error)                                             \
   V(ERR_OUT_OF_RANGE, RangeError)                                            \
diff --git a/test/parallel/test-crypto-key-objects.js b/test/parallel/test-crypto-key-objects.js
index f7948b05859fd3..e8f6cc3c963d46 100644
--- a/test/parallel/test-crypto-key-objects.js
+++ b/test/parallel/test-crypto-key-objects.js
@@ -219,7 +219,7 @@ const privateDsa = fixtures.readKey('dsa_private_encrypted_1025.pem',
   // Reading an encrypted key without a passphrase should fail.
   common.expectsError(() => createPrivateKey(privateDsa), {
     type: TypeError,
-    code: 'ERR_CRYPTO_READ_KEY',
+    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 535c676e50f003..66840dd43de494 100644
--- a/test/parallel/test-crypto-keygen.js
+++ b/test/parallel/test-crypto-keygen.js
@@ -168,7 +168,7 @@ const sec1EncExp = (cipher) => getRegExpForPEM('EC PRIVATE KEY', cipher);
     const publicKey = { key: publicKeyDER, ...publicKeyEncoding };
     common.expectsError(() => testSignVerify(publicKey, privateKey), {
       type: TypeError,
-      code: 'ERR_CRYPTO_READ_KEY',
+      code: 'ERR_MISSING_PASSPHRASE',
       message: 'Passphrase required for encrypted key'
     });
 
@@ -206,7 +206,7 @@ const sec1EncExp = (cipher) => getRegExpForPEM('EC PRIVATE KEY', cipher);
       });
     }, {
       type: TypeError,
-      code: 'ERR_CRYPTO_READ_KEY',
+      code: 'ERR_MISSING_PASSPHRASE',
       message: 'Passphrase required for encrypted key'
     });
 
@@ -289,7 +289,7 @@ const sec1EncExp = (cipher) => getRegExpForPEM('EC PRIVATE KEY', cipher);
       });
     }, {
       type: TypeError,
-      code: 'ERR_CRYPTO_READ_KEY',
+      code: 'ERR_MISSING_PASSPHRASE',
       message: 'Passphrase required for encrypted key'
     });
 
@@ -352,7 +352,7 @@ const sec1EncExp = (cipher) => getRegExpForPEM('EC PRIVATE KEY', cipher);
     // Since the private key is encrypted, signing shouldn't work anymore.
     common.expectsError(() => testSignVerify(publicKey, privateKey), {
       type: TypeError,
-      code: 'ERR_CRYPTO_READ_KEY',
+      code: 'ERR_MISSING_PASSPHRASE',
       message: 'Passphrase required for encrypted key'
     });
 
@@ -387,7 +387,7 @@ const sec1EncExp = (cipher) => getRegExpForPEM('EC PRIVATE KEY', cipher);
     // Since the private key is encrypted, signing shouldn't work anymore.
     common.expectsError(() => testSignVerify(publicKey, privateKey), {
       type: TypeError,
-      code: 'ERR_CRYPTO_READ_KEY',
+      code: 'ERR_MISSING_PASSPHRASE',
       message: 'Passphrase required for encrypted key'
     });
 

From b1538485b7a81c3a4a9ac9b82c540e00ece4a1e5 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Tobias=20Nie=C3=9Fen?= <tniessen@tnie.de>
Date: Fri, 29 Mar 2019 21:32:58 +0100
Subject: [PATCH 3/3] Do not use a macro

---
 src/node_crypto.cc | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/src/node_crypto.cc b/src/node_crypto.cc
index 583671b920c03a..00c439d2f05d4c 100644
--- a/src/node_crypto.cc
+++ b/src/node_crypto.cc
@@ -3309,8 +3309,6 @@ static PublicKeyEncodingConfig GetPublicKeyEncodingFromJs(
   return result;
 }
 
-#define READ_FAILURE_MSG(type) ("Failed to read " type " key")
-
 static inline ManagedEVPPKey GetParsedKey(Environment* env,
                                           EVPKeyPointer&& pkey,
                                           ParseKeyResult ret,
@@ -3389,7 +3387,8 @@ static ManagedEVPPKey GetPrivateKeyFromJs(
     EVPKeyPointer pkey;
     ParseKeyResult ret =
         ParsePrivateKey(&pkey, config.Release(), key.get(), key.size());
-    return GetParsedKey(env, std::move(pkey), ret, READ_FAILURE_MSG("private"));
+    return GetParsedKey(env, std::move(pkey), ret,
+                        "Failed to read private key");
   } else {
     CHECK(args[*offset]->IsObject() && allow_key_object);
     KeyObject* key;
@@ -3449,7 +3448,7 @@ static ManagedEVPPKey GetPublicOrPrivateKeyFromJs(
     }
 
     return GetParsedKey(env, std::move(pkey), ret,
-                        READ_FAILURE_MSG("asymmetric"));
+                        "Failed to read asymmetric key");
   } else {
     CHECK(args[*offset]->IsObject());
     KeyObject* key = Unwrap<KeyObject>(args[*offset].As<Object>());