crypto: don't crash on unknown asymmetricKeyType

PR-URL: #26786 Fixes: #26775 Reviewed-By: Sam Roberts <vieuxtech@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de>
nodejs · Mar 19, 2019 · 7c1fc93e30e4dac664782c7d229823fd10fa71ba · 7c1fc93
1 parent 66b9536
commit 7c1fc93e30e4dac664782c7d229823fd10fa71ba
Unified Split

Showing with 45 additions and 5 deletions.

+7 −2 doc/api/crypto.md

+2 −2 src/node_crypto.cc

+1 −1 src/node_crypto.h

+28 −0 test/fixtures/test_unknown_privkey.pem

+7 −0 test/parallel/test-crypto-key-objects.js
diff --git a/doc/api/crypto.md b/doc/api/crypto.md
@@ -1129,18 +1129,23 @@ passing keys as strings or `Buffer`s due to improved security features.
 <!-- YAML
 added: v11.6.0
 changes:
+  
+  - version: REPLACEME
+  
+    pr-url: https://github.com/nodejs/node/pull/26786
+  
+    description: This property now returns `undefined` for KeyObject
+  
+                 instances of unrecognized type instead of aborting.
  - version: REPLACEME
    pr-url: https://github.com/nodejs/node/pull/26774
    description: Added support for `'x25519'` and `'x448'`
  - version: REPLACEME
    pr-url: https://github.com/nodejs/node/pull/26319
-  
-    description: Added support for `'ed25519'` and `'ed448'`
+  
+    description: Added support for `'ed25519'` and `'ed448'`.
 -->
 * {string}
 For asymmetric keys, this property represents the type of the embedded key
 (`'rsa'`, `'dsa'`, `'ec'`, `'ed25519'`, `'ed448'`, `'x25519'` or `'x448'`).
-  
-This property is `undefined` for symmetric keys.
+  
+This property is `undefined` for unrecognized `KeyObject` types and symmetric
+  
+keys.
 ### keyObject.export([options])
 <!-- YAML

diff --git a/src/node_crypto.cc b/src/node_crypto.cc
@@ -3690,7 +3690,7 @@ void KeyObject::InitPrivate(const ManagedEVPPKey& pkey) {
  this->asymmetric_key_ = pkey;
 }
-  
-Local<String> KeyObject::GetAsymmetricKeyType() const {
+  
+Local<Value> KeyObject::GetAsymmetricKeyType() const {
  CHECK_NE(this->key_type_, kKeyTypeSecret);
  switch (EVP_PKEY_id(this->asymmetric_key_.get())) {
  case EVP_PKEY_RSA:
@@ -3708,7 +3708,7 @@ Local<String> KeyObject::GetAsymmetricKeyType() const {
  case EVP_PKEY_X448:
    return env()->crypto_x448_string();
  default:
-  
-    CHECK(false);
+  
+    return Undefined(env()->isolate());
  }
 }

diff --git a/src/node_crypto.h b/src/node_crypto.h
@@ -470,7 +470,7 @@ class KeyObject : public BaseObject {
  static void GetAsymmetricKeyType(
      const v8::FunctionCallbackInfo<v8::Value>& args);
-  
-  v8::Local<v8::String> GetAsymmetricKeyType() const;
+  
+  v8::Local<v8::Value> GetAsymmetricKeyType() const;
  static void GetSymmetricKeySize(
      const v8::FunctionCallbackInfo<v8::Value>& args);

diff --git a/test/fixtures/test_unknown_privkey.pem b/test/fixtures/test_unknown_privkey.pem
@@ -0,0 +1,28 @@
+  
+-----BEGIN PRIVATE KEY-----
+  
+MIIEugIBADALBgkqhkiG9w0BAQoEggSmMIIEogIBAAKCAQEAwMSNbT9SbSHvXmPt
+  
+j1x2Ipk1tUM06301UD91xGcA0232zrIQcKjsPM7bE6YXN0zRxfLJUqalewCk80Ct
+  
+6V+E5XtMHUFQt1Ne8HW9U930KnfnQEyU8UwRPoWWeZQhs+sa8ZfggtfN7gq4/wiS
+  
+KFNNtSJb24NKoLis31P0nILGC4/JewgE0QaFUoOL+Oc3dMhwWg9/H64sSjhI/SGW
+  
+9Sv3M6WcSn7vQCe8oM2vslf3Xm8rHNqZMlXujs7zhRtcr5alKz9BwMJIoGouQrk9
+  
+9cgupdYsddgNh2bC4TQR9BMKHj8tV5Uf3Pbf4EoZOFffCbyBZxmKtsYsmhh2FDLA
+  
+RzNhKwIDAQABAoIBAHBRlj4ziSmBfmG3Q/ImY8chEkQ9lpYn7GqHr2zyv25yQj6J
+  
+Tj72jj+YH9pBCoH0Rr5aCqgX5Y/X/kSmSS8TsvGrd9wL9KX88/KUB+7YAq7EEoBK
+  
+nvZB5kJRwC2y/DhDIv3mCrDyYVDz+nrPWaoZb8u861zqEQ+4yzGNT5fqMs8Ewm8A
+  
+hxg3GA2R6FC2CymZO884XOxlVac6SNURfA+U+xrcMIXbXpok2Z5eh/kMOeIKwmL0
+  
+QEO0U6DEnZm4rJjywu8fEkKbX00YfaDQaiGzRZfvmzkTPIQemXPWARdIvFtJU8Fx
+  
+OWWeMumJD1KiU9ISW4e76l7F8UOviT6jEg9rxFECgYEA96WCEIB+O4aO7+s56kOv
+  
+vQkEXn959lz7e++S9AV3R19PpBCh50l5v9NSjGQlA4FU4AdBB5EmiX/bLZRHFwHI
+  
+KLDsMFuq9id3OPHYIzFP4YjVHTGRPZToJHwy4ePIdZEaeJHY39EEz8oHsSSJlLdm
+  
+o0417RsFAfApW2VN63c3JFcCgYEAx0Um/ATsT4ELguVQ+XlquLQdS12XS7zjcwWv
+  
+PL8UyooSxcjcbLcJB6DRWXM0NOry7KPUCIF4m3KSjIZypV/v2KVFPCfD3vxZcdB7
+  
+xgccqXJMUx7MSs9AMZXTtv5hG7RS5z+ig7Yi/6nzBm21jKYKbFDbqfq8MSfiR6cT
+  
+KjR+RU0CgYAm/iFnlcPKfZpd/mylDTlLi3Lrqii6+NMEJam+0GmCjGhOzeugLjqE
+  
+ULLLtiz5y1Bg4eOEXH9z4PTSzWkQH1Czz3+w8Y4OqhIknjfI+se4HEJqEVbsGlke
+  
+/YtJdAMpN8qyN0ytmQyn5wilBLrA9surZPIqvjlgn77zTBUjwSamiwKBgAqIVS8s
+  
+83CgWYNpq4YELOfmXUYGhGC0czE5M7H6R5cNBUD/BOeaJRgKIAaiWDgT0xM+9Y4d
+  
+icptm+Fhmd2z3HGPCsHLOEco/3FMm74z0ggCypX6IsIxgiscyDv75hYYyej/LA/a
+  
+KK9qxDWqxtXQUOy4uWOapSfT+9ndst2gOKxhAoGAVFcfedCLxummgTtZE91n59pL
+  
+TWTk4GgYpWyv6XbHjYrFW2y18qmn0hmEpO+440So0NmGGDtNnPYNUKY/MPjHScwC
+  
+FoZMFqqnkmshXz0uDx3gMQK2JDmdF+s3VwZq4Rtb3NJ9v4/WMgWftxaUpAm1/aRC
+  
+IHc67mAAez4i8fg2wTQ=
+  
+-----END PRIVATE KEY-----
diff --git a/test/parallel/test-crypto-key-objects.js b/test/parallel/test-crypto-key-objects.js
@@ -178,6 +178,13 @@ const privateDsa = fixtures.readKey('dsa_private_encrypted_1025.pem',
  });
 }
+  
+{
+  
+  // This should not cause a crash: https://github.com/nodejs/node/pull/26786
+  
+  const pem = fixtures.readSync('test_unknown_privkey.pem', 'ascii');
+  
+  const key = createPrivateKey(pem);
+  
+  assert.strictEqual(key.asymmetricKeyType, undefined);
+  
+}
+  
+
 [
  { private: fixtures.readSync('test_ed25519_privkey.pem', 'ascii'),
    public: fixtures.readSync('test_ed25519_pubkey.pem', 'ascii'),