crypto: fix generateKeyPair with encoding 'jwk'

himself65 · targos · commit cbf6a01c177c · 2021-08-02T09:14:53.000+02:00
Fixes: #39205 PR-URL: #39319 Reviewed-By: Filip Skokan <panva.ip@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
diff --git a/lib/internal/crypto/keygen.js b/lib/internal/crypto/keygen.js
@@ -31,6 +31,7 @@ const {
   SecretKeyObject,
   parsePublicKeyEncoding,
   parsePrivateKeyEncoding,
+  isJwk
 } = require('internal/crypto/keys');
 
 const {
@@ -60,7 +61,9 @@ const {
 const { isArrayBufferView } = require('internal/util/types');
 
 function wrapKey(key, ctor) {
-  if (typeof key === 'string' || isArrayBufferView(key))
+  if (typeof key === 'string' ||
+      isArrayBufferView(key) ||
+      isJwk(key))
     return key;
   return new ctor(key);
 }
diff --git a/lib/internal/crypto/keys.js b/lib/internal/crypto/keys.js
@@ -17,6 +17,7 @@ const {
   kKeyTypePrivate,
   kKeyFormatPEM,
   kKeyFormatDER,
+  kKeyFormatJWK,
   kKeyEncodingPKCS1,
   kKeyEncodingPKCS8,
   kKeyEncodingSPKI,
@@ -37,8 +38,6 @@ const {
     ERR_INVALID_ARG_VALUE,
     ERR_OUT_OF_RANGE,
     ERR_OPERATION_FAILED,
-    ERR_CRYPTO_JWK_UNSUPPORTED_CURVE,
-    ERR_CRYPTO_JWK_UNSUPPORTED_KEY_TYPE,
     ERR_CRYPTO_INVALID_JWK,
   }
 } = require('internal/errors');
@@ -151,7 +150,6 @@ const {
 
   const kAsymmetricKeyType = Symbol('kAsymmetricKeyType');
   const kAsymmetricKeyDetails = Symbol('kAsymmetricKeyDetails');
-  const kAsymmetricKeyJWKProperties = Symbol('kAsymmetricKeyJWKProperties');
 
   function normalizeKeyDetails(details = {}) {
     if (details.publicExponent !== undefined) {
@@ -189,28 +187,6 @@ const {
           return {};
       }
     }
-
-    [kAsymmetricKeyJWKProperties]() {
-      switch (this.asymmetricKeyType) {
-        case 'rsa': return {};
-        case 'ec':
-          switch (this.asymmetricKeyDetails.namedCurve) {
-            case 'prime256v1': return { crv: 'P-256' };
-            case 'secp256k1': return { crv: 'secp256k1' };
-            case 'secp384r1': return { crv: 'P-384' };
-            case 'secp521r1': return { crv: 'P-521' };
-            default:
-              throw new ERR_CRYPTO_JWK_UNSUPPORTED_CURVE(
-                this.asymmetricKeyDetails.namedCurve);
-          }
-        case 'ed25519': return { crv: 'Ed25519' };
-        case 'ed448': return { crv: 'Ed448' };
-        case 'x25519': return { crv: 'X25519' };
-        case 'x448': return { crv: 'X448' };
-        default:
-          throw new ERR_CRYPTO_JWK_UNSUPPORTED_KEY_TYPE();
-      }
-    }
   }
 
   class PublicKeyObject extends AsymmetricKeyObject {
@@ -220,8 +196,7 @@ const {
 
     export(options) {
       if (options && options.format === 'jwk') {
-        const properties = this[kAsymmetricKeyJWKProperties]();
-        return this[kHandle].exportJwk(properties);
+        return this[kHandle].exportJwk({});
       }
       const {
         format,
@@ -242,8 +217,7 @@ const {
           throw new ERR_CRYPTO_INCOMPATIBLE_KEY_OPTIONS(
             'jwk', 'does not support encryption');
         }
-        const properties = this[kAsymmetricKeyJWKProperties]();
-        return this[kHandle].exportJwk(properties);
+        return this[kHandle].exportJwk({});
       }
       const {
         format,
@@ -265,6 +239,8 @@ function parseKeyFormat(formatStr, defaultFormat, optionName) {
     return kKeyFormatPEM;
   else if (formatStr === 'der')
     return kKeyFormatDER;
+  else if (formatStr === 'jwk')
+    return kKeyFormatJWK;
   throw new ERR_INVALID_ARG_VALUE(optionName, formatStr);
 }
 
@@ -305,12 +281,14 @@ function parseKeyFormatAndType(enc, keyType, isPublic, objName) {
                                 isInput ? kKeyFormatPEM : undefined,
                                 option('format', objName));
 
+  const isRequired = (!isInput ||
+                      format === kKeyFormatDER) &&
+                      format !== kKeyFormatJWK;
   const type = parseKeyType(typeStr,
-                            !isInput || format === kKeyFormatDER,
+                            isRequired,
                             keyType,
                             isPublic,
                             option('type', objName));
-
   return { format, type };
 }
 
@@ -766,4 +744,5 @@ module.exports = {
   PrivateKeyObject,
   isKeyObject,
   isCryptoKey,
+  isJwk,
 };
diff --git a/lib/internal/errors.js b/lib/internal/errors.js
@@ -923,8 +923,6 @@ E('ERR_CRYPTO_INVALID_JWK', 'Invalid JWK data', TypeError);
 E('ERR_CRYPTO_INVALID_KEY_OBJECT_TYPE',
   'Invalid key object type %s, expected %s.', TypeError);
 E('ERR_CRYPTO_INVALID_STATE', 'Invalid state for operation %s', Error);
-E('ERR_CRYPTO_JWK_UNSUPPORTED_CURVE', 'Unsupported JWK EC curve: %s.', Error);
-E('ERR_CRYPTO_JWK_UNSUPPORTED_KEY_TYPE', 'Unsupported JWK Key Type.', Error);
 E('ERR_CRYPTO_PBKDF2_ERROR', 'PBKDF2 error', Error);
 E('ERR_CRYPTO_SCRYPT_INVALID_PARAMETER', 'Invalid scrypt parameter', Error);
 E('ERR_CRYPTO_SCRYPT_NOT_SUPPORTED', 'Scrypt algorithm not supported', Error);
diff --git a/src/crypto/crypto_ec.cc b/src/crypto/crypto_ec.cc
@@ -740,6 +740,34 @@ Maybe<bool> ExportJWKEcKey(
     return Nothing<bool>();
   }
 
+  Local<String> crv_name;
+  const int nid = EC_GROUP_get_curve_name(group);
+  switch (nid) {
+    case NID_X9_62_prime256v1:
+      crv_name = OneByteString(env->isolate(), "P-256");
+      break;
+    case NID_secp256k1:
+      crv_name = OneByteString(env->isolate(), "secp256k1");
+      break;
+    case NID_secp384r1:
+      crv_name = OneByteString(env->isolate(), "P-384");
+      break;
+    case NID_secp521r1:
+      crv_name = OneByteString(env->isolate(), "P-521");
+      break;
+    default: {
+      THROW_ERR_CRYPTO_JWK_UNSUPPORTED_CURVE(
+          env, "Unsupported JWK EC curve: %s.", OBJ_nid2sn(nid));
+      return Nothing<bool>();
+    }
+  }
+  if (target->Set(
+      env->context(),
+      env->jwk_crv_string(),
+      crv_name).IsNothing()) {
+    return Nothing<bool>();
+  }
+
   if (key->GetKeyType() == kKeyTypePrivate) {
     const BIGNUM* pvt = EC_KEY_get0_private_key(ec);
     return SetEncodedValue(
diff --git a/src/crypto/crypto_keys.cc b/src/crypto/crypto_keys.cc
@@ -61,7 +61,11 @@ void GetKeyFormatAndTypeFromJs(
       config->type_ = Just<PKEncodingType>(static_cast<PKEncodingType>(
           args[*offset + 1].As<Int32>()->Value()));
     } else {
-      CHECK(context == kKeyContextInput && config->format_ == kKeyFormatPEM);
+      CHECK(
+          (context == kKeyContextInput &&
+           config->format_ == kKeyFormatPEM) ||
+          (context == kKeyContextGenerate &&
+           config->format_ == kKeyFormatJWK));
       CHECK(args[*offset + 1]->IsNullOrUndefined());
       config->type_ = Nothing<PKEncodingType>();
     }
@@ -487,9 +491,7 @@ Maybe<bool> ExportJWKAsymmetricKey(
     std::shared_ptr<KeyObjectData> key,
     Local<Object> target) {
   switch (EVP_PKEY_id(key->GetAsymmetricKey().get())) {
-    case EVP_PKEY_RSA:
-      // Fall through
-    case EVP_PKEY_RSA_PSS: return ExportJWKRsaKey(env, key, target);
+    case EVP_PKEY_RSA: return ExportJWKRsaKey(env, key, target);
     case EVP_PKEY_EC: return ExportJWKEcKey(env, key, target);
     case EVP_PKEY_ED25519:
       // Fall through
@@ -499,7 +501,7 @@ Maybe<bool> ExportJWKAsymmetricKey(
       // Fall through
     case EVP_PKEY_X448: return ExportJWKEdKey(env, key, target);
   }
-  THROW_ERR_CRYPTO_INVALID_KEYTYPE(env);
+  THROW_ERR_CRYPTO_JWK_UNSUPPORTED_KEY_TYPE(env);
   return Just(false);
 }
 
@@ -605,6 +607,21 @@ static inline Maybe<bool> Tristate(bool b) {
   return b ? Just(true) : Nothing<bool>();
 }
 
+Maybe<bool> ExportJWKInner(Environment* env,
+                           std::shared_ptr<KeyObjectData> key,
+                           Local<Value> result) {
+  switch (key->GetKeyType()) {
+    case kKeyTypeSecret:
+      return ExportJWKSecretKey(env, key, result.As<Object>());
+    case kKeyTypePublic:
+      // Fall through
+    case kKeyTypePrivate:
+      return ExportJWKAsymmetricKey(env, key, result.As<Object>());
+    default:
+      UNREACHABLE();
+  }
+}
+
 Maybe<bool> ManagedEVPPKey::ToEncodedPublicKey(
     Environment* env,
     ManagedEVPPKey key,
@@ -617,6 +634,11 @@ Maybe<bool> ManagedEVPPKey::ToEncodedPublicKey(
     std::shared_ptr<KeyObjectData> data =
           KeyObjectData::CreateAsymmetric(kKeyTypePublic, std::move(key));
     return Tristate(KeyObjectHandle::Create(env, data).ToLocal(out));
+  } else if (config.format_ == kKeyFormatJWK) {
+    std::shared_ptr<KeyObjectData> data =
+        KeyObjectData::CreateAsymmetric(kKeyTypePublic, std::move(key));
+    *out = Object::New(env->isolate());
+    return ExportJWKInner(env, data, *out);
   }
 
   return Tristate(WritePublicKey(env, key.get(), config).ToLocal(out));
@@ -632,6 +654,11 @@ Maybe<bool> ManagedEVPPKey::ToEncodedPrivateKey(
     std::shared_ptr<KeyObjectData> data =
         KeyObjectData::CreateAsymmetric(kKeyTypePrivate, std::move(key));
     return Tristate(KeyObjectHandle::Create(env, data).ToLocal(out));
+  } else if (config.format_ == kKeyFormatJWK) {
+    std::shared_ptr<KeyObjectData> data =
+        KeyObjectData::CreateAsymmetric(kKeyTypePrivate, std::move(key));
+    *out = Object::New(env->isolate());
+    return ExportJWKInner(env, data, *out);
   }
 
   return Tristate(WritePrivateKey(env, key.get(), config).ToLocal(out));
@@ -1211,24 +1238,7 @@ void KeyObjectHandle::ExportJWK(
 
   CHECK(args[0]->IsObject());
 
-  switch (key->Data()->GetKeyType()) {
-    case kKeyTypeSecret:
-      if (ExportJWKSecretKey(env, key->Data(), args[0].As<Object>())
-              .IsNothing()) {
-        return;
-      }
-      break;
-    case kKeyTypePublic:
-      // Fall through
-    case kKeyTypePrivate:
-      if (ExportJWKAsymmetricKey(env, key->Data(), args[0].As<Object>())
-              .IsNothing()) {
-        return;
-      }
-      break;
-    default:
-      UNREACHABLE();
-  }
+  ExportJWKInner(env, key->Data(), args[0]);
 
   args.GetReturnValue().Set(args[0]);
 }
@@ -1380,6 +1390,7 @@ void Initialize(Environment* env, Local<Object> target) {
   NODE_DEFINE_CONSTANT(target, kKeyEncodingSEC1);
   NODE_DEFINE_CONSTANT(target, kKeyFormatDER);
   NODE_DEFINE_CONSTANT(target, kKeyFormatPEM);
+  NODE_DEFINE_CONSTANT(target, kKeyFormatJWK);
   NODE_DEFINE_CONSTANT(target, kKeyTypeSecret);
   NODE_DEFINE_CONSTANT(target, kKeyTypePublic);
   NODE_DEFINE_CONSTANT(target, kKeyTypePrivate);
diff --git a/src/crypto/crypto_keys.h b/src/crypto/crypto_keys.h
@@ -31,7 +31,8 @@ enum PKEncodingType {
 
 enum PKFormatType {
   kKeyFormatDER,
-  kKeyFormatPEM
+  kKeyFormatPEM,
+  kKeyFormatJWK
 };
 
 enum KeyType {
diff --git a/src/node_errors.h b/src/node_errors.h
@@ -49,6 +49,8 @@ void OnFatalError(const char* location, const char* message);
   V(ERR_CRYPTO_INVALID_SCRYPT_PARAMS, RangeError)                              \
   V(ERR_CRYPTO_INVALID_STATE, Error)                                           \
   V(ERR_CRYPTO_INVALID_TAG_LENGTH, RangeError)                                 \
+  V(ERR_CRYPTO_JWK_UNSUPPORTED_CURVE, Error)                                   \
+  V(ERR_CRYPTO_JWK_UNSUPPORTED_KEY_TYPE, Error)                                \
   V(ERR_CRYPTO_OPERATION_FAILED, Error)                                        \
   V(ERR_CRYPTO_TIMING_SAFE_EQUAL_LENGTH, RangeError)                           \
   V(ERR_CRYPTO_UNKNOWN_CIPHER, Error)                                          \
@@ -136,6 +138,7 @@ ERRORS_WITH_CODE(V)
   V(ERR_CRYPTO_INVALID_SCRYPT_PARAMS, "Invalid scrypt params")                 \
   V(ERR_CRYPTO_INVALID_STATE, "Invalid state")                                 \
   V(ERR_CRYPTO_INVALID_TAG_LENGTH, "Invalid taglength")                        \
+  V(ERR_CRYPTO_JWK_UNSUPPORTED_KEY_TYPE, "Unsupported JWK Key Type.")          \
   V(ERR_CRYPTO_OPERATION_FAILED, "Operation failed")                           \
   V(ERR_CRYPTO_TIMING_SAFE_EQUAL_LENGTH,                                       \
     "Input buffers must have the same byte length")                            \
diff --git a/test/parallel/test-crypto-keygen.js b/test/parallel/test-crypto-keygen.js
