From 2efd015efb24d581e3c93d245f1194c8fc6302cb Mon Sep 17 00:00:00 2001
From: Filip Skokan <panva.ip@gmail.com>
Date: Fri, 20 Nov 2020 12:59:13 +0100
Subject: [PATCH 1/4] crypto: add keyObject.asymmetricKeyDetails for asymmetric
 keys

This API exposes key details. It is conceptually different from the
previously discussed keyObject.fields property since it does not give
access to information that could compromise the security of the key, and
the obtained information cannot be used to uniquely identify a key.

The intended purpose is to determine "security properties" of keys, e.g.
to generate a new key pair with the same parameters, or to decide
whether a key is secure enough.

closes #30045
---
 doc/api/crypto.md                        | 21 +++++++++
 lib/internal/crypto/keys.js              | 23 ++++++++++
 src/crypto/crypto_keys.cc                |  3 +-
 test/parallel/test-crypto-key-objects.js |  1 +
 test/parallel/test-crypto-keygen.js      | 56 ++++++++++++++++++++++--
 5 files changed, 98 insertions(+), 6 deletions(-)

diff --git a/doc/api/crypto.md b/doc/api/crypto.md
index b7977120fa4924..0789e2ca483076 100644
--- a/doc/api/crypto.md
+++ b/doc/api/crypto.md
@@ -1284,6 +1284,27 @@ passing keys as strings or `Buffer`s due to improved security features.
 The receiver obtains a cloned `KeyObject`, and the `KeyObject` does not need to
 be listed in the `transferList` argument.
 
+### `keyObject.asymmetricKeyDetails`
+<!-- YAML
+added: REPLACEME
+-->
+
+* {Object}
+
+This property exists only on asymmetric keys. Depending on the type of the key,
+this object contains information about the key. None of the information obtained
+through this property can be used to uniquely identify a key or to compromise
+the security of the key.
+
+For `'rsa'` and `'rsa-pss'` keys, this object has the properties `modulusLength`
+and `publicExponent`.
+
+For `'dsa'` keys, this object has the properties `modulusLength` and
+`divisorLength`.
+
+For `'ec'` keys with a known curve, this object has the string property
+`namedCurve`.
+
 ### `keyObject.asymmetricKeyType`
 <!-- YAML
 added: v11.6.0
diff --git a/lib/internal/crypto/keys.js b/lib/internal/crypto/keys.js
index f54393a5e3b4bd..f8400441481d95 100644
--- a/lib/internal/crypto/keys.js
+++ b/lib/internal/crypto/keys.js
@@ -5,6 +5,7 @@ const {
   ObjectDefineProperty,
   ObjectSetPrototypeOf,
   Symbol,
+  Uint8Array,
 } = primordials;
 
 const {
@@ -36,6 +37,7 @@ const {
   kHandle,
   kKeyObject,
   getArrayBufferOrView,
+  bigIntArrayToUnsignedInt,
 } = require('internal/crypto/util');
 
 const {
@@ -128,12 +130,33 @@ const [
   }
 
   const kAsymmetricKeyType = Symbol('kAsymmetricKeyType');
+  const kAsymmetricKeyDetails = Symbol('kAsymmetricKeyDetails');
+
+  // TODO: should the returne details object be frozen or otherwise protected
+  // from manipulation?
+  function normalizeKeyDetails(details = {}) {
+    if ('publicExponent' in details) {
+      return {
+        ...details,
+        publicExponent:
+          bigIntArrayToUnsignedInt(new Uint8Array(details.publicExponent))
+      };
+    }
+    return details;
+  }
 
   class AsymmetricKeyObject extends KeyObject {
     get asymmetricKeyType() {
       return this[kAsymmetricKeyType] ||
              (this[kAsymmetricKeyType] = this[kHandle].getAsymmetricKeyType());
     }
+
+    get asymmetricKeyDetails() {
+      return this[kAsymmetricKeyDetails] ||
+             (this[kAsymmetricKeyDetails] = normalizeKeyDetails(
+               this[kHandle].keyDetail({})
+             ));
+    }
   }
 
   class PublicKeyObject extends AsymmetricKeyObject {
diff --git a/src/crypto/crypto_keys.cc b/src/crypto/crypto_keys.cc
index f80a39ce5de575..5247bdfed548f6 100644
--- a/src/crypto/crypto_keys.cc
+++ b/src/crypto/crypto_keys.cc
@@ -543,8 +543,7 @@ Maybe<bool> GetAsymmetricKeyDetail(
     case EVP_PKEY_EC: return GetEcKeyDetail(env, key, target);
     case EVP_PKEY_DH: return GetDhKeyDetail(env, key, target);
   }
-  THROW_ERR_CRYPTO_INVALID_KEYTYPE(env);
-  return Nothing<bool>();
+  return Just(false);
 }
 }  // namespace
 
diff --git a/test/parallel/test-crypto-key-objects.js b/test/parallel/test-crypto-key-objects.js
index fdb63afa7659f3..34c2cef4c7677c 100644
--- a/test/parallel/test-crypto-key-objects.js
+++ b/test/parallel/test-crypto-key-objects.js
@@ -70,6 +70,7 @@ const privateDsa = fixtures.readKey('dsa_private_encrypted_1025.pem',
   assert.strictEqual(key.type, 'secret');
   assert.strictEqual(key.symmetricKeySize, 32);
   assert.strictEqual(key.asymmetricKeyType, undefined);
+  assert.strictEqual(key.asymmetricKeyDetails, undefined);
 
   const exportedKey = key.export();
   assert(keybuf.equals(exportedKey));
diff --git a/test/parallel/test-crypto-keygen.js b/test/parallel/test-crypto-keygen.js
index f59a26a422f96d..0c2516a354c741 100644
--- a/test/parallel/test-crypto-keygen.js
+++ b/test/parallel/test-crypto-keygen.js
@@ -123,10 +123,18 @@ const sec1EncExp = (cipher) => getRegExpForPEM('EC PRIVATE KEY', cipher);
   assert.strictEqual(typeof publicKey, 'object');
   assert.strictEqual(publicKey.type, 'public');
   assert.strictEqual(publicKey.asymmetricKeyType, 'rsa');
+  assert.deepStrictEqual(publicKey.asymmetricKeyDetails, {
+    modulusLength: 512,
+    publicExponent: 65537
+  });
 
   assert.strictEqual(typeof privateKey, 'object');
   assert.strictEqual(privateKey.type, 'private');
   assert.strictEqual(privateKey.asymmetricKeyType, 'rsa');
+  assert.deepStrictEqual(privateKey.asymmetricKeyDetails, {
+    modulusLength: 512,
+    publicExponent: 65537
+  });
 }
 
 {
@@ -268,9 +276,17 @@ const sec1EncExp = (cipher) => getRegExpForPEM('EC PRIVATE KEY', cipher);
   }, common.mustSucceed((publicKey, privateKey) => {
     assert.strictEqual(publicKey.type, 'public');
     assert.strictEqual(publicKey.asymmetricKeyType, 'rsa-pss');
+    assert.deepStrictEqual(publicKey.asymmetricKeyDetails, {
+      modulusLength: 512,
+      publicExponent: 65537
+    });
 
     assert.strictEqual(privateKey.type, 'private');
     assert.strictEqual(privateKey.asymmetricKeyType, 'rsa-pss');
+    assert.deepStrictEqual(privateKey.asymmetricKeyDetails, {
+      modulusLength: 512,
+      publicExponent: 65537
+    });
 
     // Unlike RSA, RSA-PSS does not allow encryption.
     assert.throws(() => {
@@ -342,6 +358,28 @@ const sec1EncExp = (cipher) => getRegExpForPEM('EC PRIVATE KEY', cipher);
   }));
 }
 
+{
+  // Test async DSA key object generation.
+  generateKeyPair('dsa', {
+    modulusLength: 512,
+    divisorLength: 256
+  }, common.mustSucceed((publicKey, privateKey) => {
+    assert.strictEqual(publicKey.type, 'public');
+    assert.strictEqual(publicKey.asymmetricKeyType, 'dsa');
+    assert.deepStrictEqual(publicKey.asymmetricKeyDetails, {
+      modulusLength: 512,
+      divisorLength: 256
+    });
+
+    assert.strictEqual(privateKey.type, 'private');
+    assert.strictEqual(privateKey.asymmetricKeyType, 'dsa');
+    assert.deepStrictEqual(privateKey.asymmetricKeyDetails, {
+      modulusLength: 512,
+      divisorLength: 256
+    });
+  }));
+}
+
 {
   // Test async elliptic curve key generation, e.g. for ECDSA, with a SEC1
   // private key.
@@ -925,16 +963,24 @@ const sec1EncExp = (cipher) => getRegExpForPEM('EC PRIVATE KEY', cipher);
   // It should recognize both NIST and standard curve names.
   generateKeyPair('ec', {
     namedCurve: 'P-256',
-    publicKeyEncoding: { type: 'spki', format: 'pem' },
-    privateKeyEncoding: { type: 'pkcs8', format: 'pem' }
   }, common.mustSucceed((publicKey, privateKey) => {
+    assert.deepStrictEqual(publicKey.asymmetricKeyDetails, {
+      namedCurve: 'prime256v1'
+    });
+    assert.deepStrictEqual(privateKey.asymmetricKeyDetails, {
+      namedCurve: 'prime256v1'
+    });
   }));
 
   generateKeyPair('ec', {
     namedCurve: 'secp256k1',
-    publicKeyEncoding: { type: 'spki', format: 'pem' },
-    privateKeyEncoding: { type: 'pkcs8', format: 'pem' }
   }, common.mustSucceed((publicKey, privateKey) => {
+    assert.deepStrictEqual(publicKey.asymmetricKeyDetails, {
+      namedCurve: 'secp256k1'
+    });
+    assert.deepStrictEqual(privateKey.asymmetricKeyDetails, {
+      namedCurve: 'secp256k1'
+    });
   }));
 }
 
@@ -945,9 +991,11 @@ const sec1EncExp = (cipher) => getRegExpForPEM('EC PRIVATE KEY', cipher);
       generateKeyPair(keyType, common.mustSucceed((publicKey, privateKey) => {
         assert.strictEqual(publicKey.type, 'public');
         assert.strictEqual(publicKey.asymmetricKeyType, keyType);
+        assert.deepStrictEqual(publicKey.asymmetricKeyDetails, {});
 
         assert.strictEqual(privateKey.type, 'private');
         assert.strictEqual(privateKey.asymmetricKeyType, keyType);
+        assert.deepStrictEqual(privateKey.asymmetricKeyDetails, {});
       }));
     });
   }

From 95fc8836a06351b51c0f3db650f2784c45864bdc Mon Sep 17 00:00:00 2001
From: Filip Skokan <panva.ip@gmail.com>
Date: Fri, 20 Nov 2020 16:30:58 +0100
Subject: [PATCH 2/4] fixup! crypto: add keyObject.asymmetricKeyDetails for
 asymmetric keys

---
 doc/api/crypto.md           | 13 ++++---------
 lib/internal/crypto/keys.js |  4 +---
 2 files changed, 5 insertions(+), 12 deletions(-)

diff --git a/doc/api/crypto.md b/doc/api/crypto.md
index 0789e2ca483076..31d62802340819 100644
--- a/doc/api/crypto.md
+++ b/doc/api/crypto.md
@@ -1290,21 +1290,16 @@ added: REPLACEME
 -->
 
 * {Object}
+  * `modulusLength`: {number} Key size in bits (RSA, DSA).
+  * `publicExponent`: {number} Public exponent (RSA).
+  * `divisorLength`: {number} Size of `q` in bits (DSA).
+  * `namedCurve`: {string} Name of the curve (EC).
 
 This property exists only on asymmetric keys. Depending on the type of the key,
 this object contains information about the key. None of the information obtained
 through this property can be used to uniquely identify a key or to compromise
 the security of the key.
 
-For `'rsa'` and `'rsa-pss'` keys, this object has the properties `modulusLength`
-and `publicExponent`.
-
-For `'dsa'` keys, this object has the properties `modulusLength` and
-`divisorLength`.
-
-For `'ec'` keys with a known curve, this object has the string property
-`namedCurve`.
-
 ### `keyObject.asymmetricKeyType`
 <!-- YAML
 added: v11.6.0
diff --git a/lib/internal/crypto/keys.js b/lib/internal/crypto/keys.js
index f8400441481d95..6550079e476b72 100644
--- a/lib/internal/crypto/keys.js
+++ b/lib/internal/crypto/keys.js
@@ -132,10 +132,8 @@ const [
   const kAsymmetricKeyType = Symbol('kAsymmetricKeyType');
   const kAsymmetricKeyDetails = Symbol('kAsymmetricKeyDetails');
 
-  // TODO: should the returne details object be frozen or otherwise protected
-  // from manipulation?
   function normalizeKeyDetails(details = {}) {
-    if ('publicExponent' in details) {
+    if (details.publicExponent !== undefined) {
       return {
         ...details,
         publicExponent:

From 3559e946d0733692c15bfa514a48cf19a2751764 Mon Sep 17 00:00:00 2001
From: Filip Skokan <panva.ip@gmail.com>
Date: Mon, 30 Nov 2020 12:23:42 +0100
Subject: [PATCH 3/4] fixup! crypto: add keyObject.asymmetricKeyDetails for
 asymmetric keys

---
 doc/api/crypto.md                   |  2 +-
 lib/internal/crypto/keys.js         |  4 ++--
 lib/internal/crypto/util.js         | 13 ++++++++++++
 test/parallel/test-crypto-keygen.js | 33 +++++++++++++++++++++++++----
 4 files changed, 45 insertions(+), 7 deletions(-)

diff --git a/doc/api/crypto.md b/doc/api/crypto.md
index 31d62802340819..fc4a5468474f2e 100644
--- a/doc/api/crypto.md
+++ b/doc/api/crypto.md
@@ -1291,7 +1291,7 @@ added: REPLACEME
 
 * {Object}
   * `modulusLength`: {number} Key size in bits (RSA, DSA).
-  * `publicExponent`: {number} Public exponent (RSA).
+  * `publicExponent`: {bigint} Public exponent (RSA).
   * `divisorLength`: {number} Size of `q` in bits (DSA).
   * `namedCurve`: {string} Name of the curve (EC).
 
diff --git a/lib/internal/crypto/keys.js b/lib/internal/crypto/keys.js
index 6550079e476b72..a0c56de41a6949 100644
--- a/lib/internal/crypto/keys.js
+++ b/lib/internal/crypto/keys.js
@@ -37,7 +37,7 @@ const {
   kHandle,
   kKeyObject,
   getArrayBufferOrView,
-  bigIntArrayToUnsignedInt,
+  bigIntArrayToBigInt,
 } = require('internal/crypto/util');
 
 const {
@@ -137,7 +137,7 @@ const [
       return {
         ...details,
         publicExponent:
-          bigIntArrayToUnsignedInt(new Uint8Array(details.publicExponent))
+          bigIntArrayToBigInt(new Uint8Array(details.publicExponent))
       };
     }
     return details;
diff --git a/lib/internal/crypto/util.js b/lib/internal/crypto/util.js
index 67cb9841928a2d..2564765f77de6e 100644
--- a/lib/internal/crypto/util.js
+++ b/lib/internal/crypto/util.js
@@ -3,6 +3,7 @@
 const {
   ArrayPrototypeIncludes,
   ArrayPrototypePush,
+  BigInt,
   FunctionPrototypeBind,
   Number,
   Promise,
@@ -308,6 +309,17 @@ function bigIntArrayToUnsignedInt(input) {
   return result;
 }
 
+function bigIntArrayToBigInt(input) {
+  let result = 0n;
+
+  for (let n = 0; n < input.length; ++n) {
+    const n_reversed = input.length - n - 1;
+    result |= BigInt(input[n]) << 8n * BigInt(n_reversed);
+  }
+
+  return result;
+}
+
 function getStringOption(options, key) {
   let value;
   if (options && (value = options[key]) != null)
@@ -413,6 +425,7 @@ module.exports = {
   jobPromise,
   lazyRequire,
   validateMaxBufferLength,
+  bigIntArrayToBigInt,
   bigIntArrayToUnsignedInt,
   getStringOption,
   getUsagesUnion,
diff --git a/test/parallel/test-crypto-keygen.js b/test/parallel/test-crypto-keygen.js
index 0c2516a354c741..c27d5fe166220e 100644
--- a/test/parallel/test-crypto-keygen.js
+++ b/test/parallel/test-crypto-keygen.js
@@ -114,6 +114,31 @@ const sec1EncExp = (cipher) => getRegExpForPEM('EC PRIVATE KEY', cipher);
   testSignVerify(publicKey, privateKey);
 }
 
+{
+  // Test sync key generation with key objects with a non-standard
+  // publicExpononent
+  const { publicKey, privateKey } = generateKeyPairSync('rsa', {
+    publicExponent: 3,
+    modulusLength: 512
+  });
+
+  assert.strictEqual(typeof publicKey, 'object');
+  assert.strictEqual(publicKey.type, 'public');
+  assert.strictEqual(publicKey.asymmetricKeyType, 'rsa');
+  assert.deepStrictEqual(publicKey.asymmetricKeyDetails, {
+    modulusLength: 512,
+    publicExponent: 3n
+  });
+
+  assert.strictEqual(typeof privateKey, 'object');
+  assert.strictEqual(privateKey.type, 'private');
+  assert.strictEqual(privateKey.asymmetricKeyType, 'rsa');
+  assert.deepStrictEqual(privateKey.asymmetricKeyDetails, {
+    modulusLength: 512,
+    publicExponent: 3n
+  });
+}
+
 {
   // Test sync key generation with key objects.
   const { publicKey, privateKey } = generateKeyPairSync('rsa', {
@@ -125,7 +150,7 @@ const sec1EncExp = (cipher) => getRegExpForPEM('EC PRIVATE KEY', cipher);
   assert.strictEqual(publicKey.asymmetricKeyType, 'rsa');
   assert.deepStrictEqual(publicKey.asymmetricKeyDetails, {
     modulusLength: 512,
-    publicExponent: 65537
+    publicExponent: 65537n
   });
 
   assert.strictEqual(typeof privateKey, 'object');
@@ -133,7 +158,7 @@ const sec1EncExp = (cipher) => getRegExpForPEM('EC PRIVATE KEY', cipher);
   assert.strictEqual(privateKey.asymmetricKeyType, 'rsa');
   assert.deepStrictEqual(privateKey.asymmetricKeyDetails, {
     modulusLength: 512,
-    publicExponent: 65537
+    publicExponent: 65537n
   });
 }
 
@@ -278,14 +303,14 @@ const sec1EncExp = (cipher) => getRegExpForPEM('EC PRIVATE KEY', cipher);
     assert.strictEqual(publicKey.asymmetricKeyType, 'rsa-pss');
     assert.deepStrictEqual(publicKey.asymmetricKeyDetails, {
       modulusLength: 512,
-      publicExponent: 65537
+      publicExponent: 65537n
     });
 
     assert.strictEqual(privateKey.type, 'private');
     assert.strictEqual(privateKey.asymmetricKeyType, 'rsa-pss');
     assert.deepStrictEqual(privateKey.asymmetricKeyDetails, {
       modulusLength: 512,
-      publicExponent: 65537
+      publicExponent: 65537n
     });
 
     // Unlike RSA, RSA-PSS does not allow encryption.

From ec80c32be1143aa78e0e95f3f9776976e2198f9e Mon Sep 17 00:00:00 2001
From: Filip Skokan <panva.ip@gmail.com>
Date: Thu, 14 Jan 2021 12:42:47 +0100
Subject: [PATCH 4/4] fixup! crypto: add keyObject.asymmetricKeyDetails for
 asymmetric keys

---
 doc/api/crypto.md           |  3 +++
 lib/internal/crypto/keys.js | 14 +++++++++++---
 lib/internal/crypto/util.js |  4 ++--
 src/crypto/crypto_keys.cc   |  3 ++-
 4 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/doc/api/crypto.md b/doc/api/crypto.md
index fc4a5468474f2e..fa1cb4480d12ed 100644
--- a/doc/api/crypto.md
+++ b/doc/api/crypto.md
@@ -1300,6 +1300,9 @@ this object contains information about the key. None of the information obtained
 through this property can be used to uniquely identify a key or to compromise
 the security of the key.
 
+RSA-PSS parameters, DH, or any future key type details might be exposed via this
+API using additional attributes.
+
 ### `keyObject.asymmetricKeyType`
 <!-- YAML
 added: v11.6.0
diff --git a/lib/internal/crypto/keys.js b/lib/internal/crypto/keys.js
index a0c56de41a6949..9d6f86d32b0b78 100644
--- a/lib/internal/crypto/keys.js
+++ b/lib/internal/crypto/keys.js
@@ -37,7 +37,7 @@ const {
   kHandle,
   kKeyObject,
   getArrayBufferOrView,
-  bigIntArrayToBigInt,
+  bigIntArrayToUnsignedBigInt,
 } = require('internal/crypto/util');
 
 const {
@@ -137,7 +137,7 @@ const [
       return {
         ...details,
         publicExponent:
-          bigIntArrayToBigInt(new Uint8Array(details.publicExponent))
+          bigIntArrayToUnsignedBigInt(new Uint8Array(details.publicExponent))
       };
     }
     return details;
@@ -150,10 +150,18 @@ const [
     }
 
     get asymmetricKeyDetails() {
-      return this[kAsymmetricKeyDetails] ||
+      switch (this.asymmetricKeyType) {
+        case 'rsa':
+        case 'rsa-pss':
+        case 'dsa':
+        case 'ec':
+          return this[kAsymmetricKeyDetails] ||
              (this[kAsymmetricKeyDetails] = normalizeKeyDetails(
                this[kHandle].keyDetail({})
              ));
+        default:
+          return {};
+      }
     }
   }
 
diff --git a/lib/internal/crypto/util.js b/lib/internal/crypto/util.js
index 2564765f77de6e..9d2fee2b2dd57a 100644
--- a/lib/internal/crypto/util.js
+++ b/lib/internal/crypto/util.js
@@ -309,7 +309,7 @@ function bigIntArrayToUnsignedInt(input) {
   return result;
 }
 
-function bigIntArrayToBigInt(input) {
+function bigIntArrayToUnsignedBigInt(input) {
   let result = 0n;
 
   for (let n = 0; n < input.length; ++n) {
@@ -425,7 +425,7 @@ module.exports = {
   jobPromise,
   lazyRequire,
   validateMaxBufferLength,
-  bigIntArrayToBigInt,
+  bigIntArrayToUnsignedBigInt,
   bigIntArrayToUnsignedInt,
   getStringOption,
   getUsagesUnion,
diff --git a/src/crypto/crypto_keys.cc b/src/crypto/crypto_keys.cc
index 5247bdfed548f6..f80a39ce5de575 100644
--- a/src/crypto/crypto_keys.cc
+++ b/src/crypto/crypto_keys.cc
@@ -543,7 +543,8 @@ Maybe<bool> GetAsymmetricKeyDetail(
     case EVP_PKEY_EC: return GetEcKeyDetail(env, key, target);
     case EVP_PKEY_DH: return GetDhKeyDetail(env, key, target);
   }
-  return Just(false);
+  THROW_ERR_CRYPTO_INVALID_KEYTYPE(env);
+  return Nothing<bool>();
 }
 }  // namespace