crypto: always accept certificates as public keys

PR-URL: #24234 Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: James M Snell <[email protected]>
nodejs · Dec 26, 2018 · e6c1e8d · e6c1e8d
1 parent 3b53df0
commit e6c1e8d
Show file tree

Hide file tree

Showing 2 changed files with 14 additions and 19 deletions.
diff --git a/doc/api/crypto.md b/doc/api/crypto.md
@@ -1831,6 +1831,8 @@ Creates and returns a new key object containing a public key. If `key` is a
 string or `Buffer`, `format` is assumed to be `'pem'`; otherwise, `key`
 must be an object with the properties described above.
 
+If the format is `'pem'`, the `'key'` may also be an X.509 certificate.
+
 ### crypto.createSecretKey(key)
 <!-- YAML
 added: REPLACEME

diff --git a/src/node_crypto.cc b/src/node_crypto.cc
@@ -2710,8 +2710,7 @@ static ParsePublicKeyResult TryParsePublicKey(
 
 static ParsePublicKeyResult ParsePublicKeyPEM(EVPKeyPointer* pkey,
                                               const char* key_pem,
-                                              int key_pem_len,
-                                              bool allow_certificate) {
+                                              int key_pem_len) {
   BIOPointer bp(BIO_new_mem_buf(const_cast<char*>(key_pem), key_pem_len));
   if (!bp)
     return ParsePublicKeyResult::kParsePublicFailed;
@@ -2732,8 +2731,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 ||
-      !allow_certificate)
+  if (ret != ParsePublicKeyResult::kParsePublicNotRecognized)
     return ret;
 
   // X.509 fallback.
@@ -2748,11 +2746,10 @@ static ParsePublicKeyResult ParsePublicKeyPEM(EVPKeyPointer* pkey,
 static bool ParsePublicKey(EVPKeyPointer* pkey,
                            const PublicKeyEncodingConfig& config,
                            const char* key,
-                           size_t key_len,
-                           bool allow_certificate) {
+                           size_t key_len) {
   if (config.format_ == kKeyFormatPEM) {
     ParsePublicKeyResult r =
-        ParsePublicKeyPEM(pkey, key, key_len, allow_certificate);
+        ParsePublicKeyPEM(pkey, key, key_len);
     return r == ParsePublicKeyResult::kParsePublicOk;
   } else {
     CHECK_EQ(config.format_, kKeyFormatDER);
@@ -3002,15 +2999,14 @@ static PublicKeyEncodingConfig GetPublicKeyEncodingFromJs(
 static ManagedEVPPKey GetPublicKeyFromJs(
     const FunctionCallbackInfo<Value>& args,
     unsigned int* offset,
-    bool allow_key_object,
-    bool allow_certificate) {
+    bool allow_key_object) {
   if (args[*offset]->IsString() || Buffer::HasInstance(args[*offset])) {
     Environment* env = Environment::GetCurrent(args);
     ByteSource key = ByteSource::FromStringOrBuffer(env, args[(*offset)++]);
     PublicKeyEncodingConfig config =
         GetPublicKeyEncodingFromJs(args, offset, kKeyContextInput);
     EVPKeyPointer pkey;
-    ParsePublicKey(&pkey, config, key.get(), key.size(), allow_certificate);
+    ParsePublicKey(&pkey, config, key.get(), key.size());
     if (!pkey)
       ThrowCryptoError(env, ERR_get_error(), "Failed to read public key");
     return ManagedEVPPKey(pkey.release());
@@ -3131,8 +3127,7 @@ static bool IsRSAPrivateKey(const unsigned char* data, size_t size) {
 static ManagedEVPPKey GetPublicOrPrivateKeyFromJs(
     const FunctionCallbackInfo<Value>& args,
     unsigned int* offset,
-    bool allow_key_object,
-    bool allow_certificate) {
+    bool allow_key_object) {
   if (args[*offset]->IsString() || Buffer::HasInstance(args[*offset])) {
     Environment* env = Environment::GetCurrent(args);
     ByteSource data = ByteSource::FromStringOrBuffer(env, args[(*offset)++]);
@@ -3146,8 +3141,7 @@ static ManagedEVPPKey GetPublicOrPrivateKeyFromJs(
       // 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(),
-                                                   allow_certificate);
+                                                   data.size());
       if (ret == ParsePublicKeyResult::kParsePublicNotRecognized) {
         pkey = ParsePrivateKey(config, data.get(), data.size());
       }
@@ -3172,8 +3166,7 @@ static ManagedEVPPKey GetPublicOrPrivateKeyFromJs(
       }
 
       if (is_public) {
-        ParsePublicKey(&pkey, config, data.get(), data.size(),
-                       allow_certificate);
+        ParsePublicKey(&pkey, config, data.get(), data.size());
       } else {
         pkey = ParsePrivateKey(config, data.get(), data.size());
       }
@@ -3386,7 +3379,7 @@ void KeyObject::Init(const FunctionCallbackInfo<Value>& args) {
     CHECK_EQ(args.Length(), 3);
 
     offset = 0;
-    pkey = GetPublicKeyFromJs(args, &offset, false, false);
+    pkey = GetPublicKeyFromJs(args, &offset, false);
     if (!pkey)
       return;
     key->InitPublic(pkey);
@@ -4668,7 +4661,7 @@ void Verify::VerifyFinal(const FunctionCallbackInfo<Value>& args) {
   ASSIGN_OR_RETURN_UNWRAP(&verify, args.Holder());
 
   unsigned int offset = 0;
-  ManagedEVPPKey pkey = GetPublicKeyFromJs(args, &offset, true, true);
+  ManagedEVPPKey pkey = GetPublicKeyFromJs(args, &offset, true);
 
   char* hbuf = Buffer::Data(args[offset]);
   ssize_t hlen = Buffer::Length(args[offset]);
@@ -4724,7 +4717,7 @@ void PublicKeyCipher::Cipher(const FunctionCallbackInfo<Value>& args) {
   Environment* env = Environment::GetCurrent(args);
 
   unsigned int offset = 0;
-  ManagedEVPPKey pkey = GetPublicOrPrivateKeyFromJs(args, &offset, true, true);
+  ManagedEVPPKey pkey = GetPublicOrPrivateKeyFromJs(args, &offset, true);
   if (!pkey)
     return;