crypto: remove webcrypto "DSA" JWK Key Type operations

"DSA" is not a registered JWK key type. https://www.iana.org/assignments/jose/jose.xhtml#web-key-types PR-URL: #37203 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de>
nodejs · Feb 5, 2021 · 2ff1c83 · 2ff1c83
1 parent e8286bb
commit 2ff1c83
Show file tree

Hide file tree

Showing 5 changed files with 11 additions and 210 deletions.
diff --git a/doc/api/webcrypto.md b/doc/api/webcrypto.md
@@ -605,6 +605,10 @@ The algorithms currently supported include:
 ### `subtle.exportKey(format, key)`
 <!-- YAML
 added: v15.0.0
+changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/37203
+    description: Removed `'NODE-DSA'` JWK export.
 -->
 
 * `format`: {string} Must be one of `'raw'`, `'pkcs8'`, `'spki'`, `'jwk'`, or
@@ -642,7 +646,7 @@ extension that allows converting a {CryptoKey} into a Node.js {KeyObject}.
 | `'RSA-OAEP'`          | ✔ | ✔ | ✔ |   |
 | `'RSA-PSS'`           | ✔ | ✔ | ✔ |   |
 | `'RSASSA-PKCS1-v1_5'` | ✔ | ✔ | ✔ |   |
-| `'NODE-DSA'` <sup>1</sup> | ✔ | ✔ | ✔ |   |
+| `'NODE-DSA'` <sup>1</sup> | ✔ | ✔ |   |   |
 | `'NODE-DH'` <sup>1</sup> | ✔ | ✔ |   |   |
 | `'NODE-SCRYPT'` <sup>1</sup> |   |   |   |   |
 | `'NODE-ED25519'` <sup>1</sup> | ✔ | ✔ | ✔ | ✔ |
@@ -692,6 +696,10 @@ The {CryptoKey} (secret key) generating algorithms supported include:
 ### `subtle.importKey(format, keyData, algorithm, extractable, keyUsages)`
 <!-- YAML
 added: v15.0.0
+changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/37203
+    description: Removed `'NODE-DSA'` JWK import.
 -->
 
 * `format`: {string} Must be one of `'raw'`, `'pkcs8'`, `'spki'`, `'jwk'`, or
@@ -730,7 +738,7 @@ The algorithms currently supported include:
 | `'RSA-OAEP'`          | ✔ | ✔ | ✔ |   |
 | `'RSA-PSS'`           | ✔ | ✔ | ✔ |   |
 | `'RSASSA-PKCS1-v1_5'` | ✔ | ✔ | ✔ |   |
-| `'NODE-DSA'` <sup>1</sup> | ✔ | ✔ | ✔ |   |
+| `'NODE-DSA'` <sup>1</sup> | ✔ | ✔ |   |   |
 | `'NODE-DH'` <sup>1</sup> | ✔ | ✔ |   |   |
 | `'NODE-SCRYPT'` <sup>1</sup> |   |   |   | ✔ |
 | `'NODE-ED25519'` <sup>1</sup> | ✔ | ✔ | ✔ | ✔ |

diff --git a/src/crypto/crypto_dsa.cc b/src/crypto/crypto_dsa.cc
@@ -22,7 +22,6 @@ using v8::Maybe;
 using v8::Nothing;
 using v8::Number;
 using v8::Object;
-using v8::String;
 using v8::Uint32;
 using v8::Value;
 
@@ -127,107 +126,6 @@ WebCryptoKeyExportStatus DSAKeyExportTraits::DoExport(
   }
 }
 
-Maybe<bool> ExportJWKDsaKey(
-    Environment* env,
-    std::shared_ptr<KeyObjectData> key,
-    Local<Object> target) {
-  ManagedEVPPKey pkey = key->GetAsymmetricKey();
-  CHECK_EQ(EVP_PKEY_id(pkey.get()), EVP_PKEY_DSA);
-
-  DSA* dsa = EVP_PKEY_get0_DSA(pkey.get());
-  CHECK_NOT_NULL(dsa);
-
-  const BIGNUM* y;
-  const BIGNUM* x;
-  const BIGNUM* p;
-  const BIGNUM* q;
-  const BIGNUM* g;
-
-  DSA_get0_key(dsa, &y, &x);
-  DSA_get0_pqg(dsa, &p, &q, &g);
-
-  if (target->Set(
-          env->context(),
-          env->jwk_kty_string(),
-          env->jwk_dsa_string()).IsNothing()) {
-    return Nothing<bool>();
-  }
-
-  if (SetEncodedValue(env, target, env->jwk_y_string(), y).IsNothing() ||
-      SetEncodedValue(env, target, env->jwk_p_string(), p).IsNothing() ||
-      SetEncodedValue(env, target, env->jwk_q_string(), q).IsNothing() ||
-      SetEncodedValue(env, target, env->jwk_g_string(), g).IsNothing()) {
-    return Nothing<bool>();
-  }
-
-  if (key->GetKeyType() == kKeyTypePrivate &&
-      SetEncodedValue(env, target, env->jwk_x_string(), x).IsNothing()) {
-    return Nothing<bool>();
-  }
-
-  return Just(true);
-}
-
-std::shared_ptr<KeyObjectData> ImportJWKDsaKey(
-    Environment* env,
-    Local<Object> jwk,
-    const FunctionCallbackInfo<Value>& args,
-    unsigned int offset) {
-  Local<Value> y_value;
-  Local<Value> p_value;
-  Local<Value> q_value;
-  Local<Value> g_value;
-  Local<Value> x_value;
-
-  if (!jwk->Get(env->context(), env->jwk_y_string()).ToLocal(&y_value) ||
-      !jwk->Get(env->context(), env->jwk_p_string()).ToLocal(&p_value) ||
-      !jwk->Get(env->context(), env->jwk_q_string()).ToLocal(&q_value) ||
-      !jwk->Get(env->context(), env->jwk_g_string()).ToLocal(&g_value) ||
-      !jwk->Get(env->context(), env->jwk_x_string()).ToLocal(&x_value)) {
-    return std::shared_ptr<KeyObjectData>();
-  }
-
-  if (!y_value->IsString() ||
-      !p_value->IsString() ||
-      !q_value->IsString() ||
-      !q_value->IsString() ||
-      (!x_value->IsUndefined() && !x_value->IsString())) {
-    THROW_ERR_CRYPTO_INVALID_JWK(env, "Invalid JWK DSA key");
-    return std::shared_ptr<KeyObjectData>();
-  }
-
-  KeyType type = x_value->IsString() ? kKeyTypePrivate : kKeyTypePublic;
-
-  DsaPointer dsa(DSA_new());
-
-  ByteSource y = ByteSource::FromEncodedString(env, y_value.As<String>());
-  ByteSource p = ByteSource::FromEncodedString(env, p_value.As<String>());
-  ByteSource q = ByteSource::FromEncodedString(env, q_value.As<String>());
-  ByteSource g = ByteSource::FromEncodedString(env, g_value.As<String>());
-
-  if (!DSA_set0_key(dsa.get(), y.ToBN().release(), nullptr) ||
-      !DSA_set0_pqg(dsa.get(),
-                    p.ToBN().release(),
-                    q.ToBN().release(),
-                    g.ToBN().release())) {
-    THROW_ERR_CRYPTO_INVALID_JWK(env, "Invalid JWK DSA key");
-    return std::shared_ptr<KeyObjectData>();
-  }
-
-  if (type == kKeyTypePrivate) {
-    ByteSource x = ByteSource::FromEncodedString(env, x_value.As<String>());
-    if (!DSA_set0_key(dsa.get(), nullptr, x.ToBN().release())) {
-      THROW_ERR_CRYPTO_INVALID_JWK(env, "Invalid JWK DSA key");
-      return std::shared_ptr<KeyObjectData>();
-    }
-  }
-
-  EVPKeyPointer pkey(EVP_PKEY_new());
-  CHECK_EQ(EVP_PKEY_set1_DSA(pkey.get(), dsa.get()), 1);
-
-  return KeyObjectData::CreateAsymmetric(type, ManagedEVPPKey(std::move(pkey)));
-}
-
 Maybe<bool> GetDsaKeyDetail(
     Environment* env,
     std::shared_ptr<KeyObjectData> key,
@@ -269,4 +167,3 @@ void Initialize(Environment* env, Local<Object> target) {
 }  // namespace DSAAlg
 }  // namespace crypto
 }  // namespace node
-
diff --git a/src/crypto/crypto_dsa.h b/src/crypto/crypto_dsa.h
@@ -61,17 +61,6 @@ struct DSAKeyExportTraits final {
 
 using DSAKeyExportJob = KeyExportJob<DSAKeyExportTraits>;
 
-v8::Maybe<bool> ExportJWKDsaKey(
-    Environment* env,
-    std::shared_ptr<KeyObjectData> key,
-    v8::Local<v8::Object> target);
-
-std::shared_ptr<KeyObjectData> ImportJWKDsaKey(
-    Environment* env,
-    v8::Local<v8::Object> jwk,
-    const v8::FunctionCallbackInfo<v8::Value>& args,
-    unsigned int offset);
-
 v8::Maybe<bool> GetDsaKeyDetail(
     Environment* env,
     std::shared_ptr<KeyObjectData> key,

diff --git a/src/crypto/crypto_keys.cc b/src/crypto/crypto_keys.cc
@@ -490,7 +490,6 @@ Maybe<bool> ExportJWKAsymmetricKey(
     case EVP_PKEY_RSA:
       // Fall through
     case EVP_PKEY_RSA_PSS: return ExportJWKRsaKey(env, key, target);
-    case EVP_PKEY_DSA: return ExportJWKDsaKey(env, key, target);
     case EVP_PKEY_EC: return ExportJWKEcKey(env, key, target);
     case EVP_PKEY_ED25519:
       // Fall through
@@ -512,14 +511,12 @@ std::shared_ptr<KeyObjectData> ImportJWKAsymmetricKey(
     unsigned int offset) {
   if (strcmp(kty, "RSA") == 0) {
     return ImportJWKRsaKey(env, jwk, args, offset);
-  } else if (strcmp(kty, "DSA") == 0) {
-    return ImportJWKDsaKey(env, jwk, args, offset);
   } else if (strcmp(kty, "EC") == 0) {
     return ImportJWKEcKey(env, jwk, args, offset);
   }
 
   char msg[1024];
-  snprintf(msg, sizeof(msg), "%s is not a support JWK key type", kty);
+  snprintf(msg, sizeof(msg), "%s is not a supported JWK key type", kty);
   THROW_ERR_CRYPTO_INVALID_JWK(env, msg);
   return std::shared_ptr<KeyObjectData>();
 }

diff --git a/test/parallel/test-webcrypto-export-import-dsa.js b/test/parallel/test-webcrypto-export-import-dsa.js
@@ -46,20 +46,6 @@ const keyData = {
       'f26fe27c533587b765e57948439084e76fd6a4fd004f5c78d972cf7f100ec9494a902' +
       '645baca4b4c6f3993041e021c600daa0a9c4cc674c98bb07956374c84ac1c33af8816' +
       '3ea7e2587876', 'hex'),
-    jwk: {
-      kty: 'DSA',
-      y: 'mo32ny_jIYaeIJTjh7wdwrXzv_Ki4jz7pR08EZ-6a0wVpJSF-oEbaVXZHSjJ4uBEWn' +
-         'ndxUJrL-ROAKbJJUx3bxP9ENvJNCYgd7HfcsFryEiBfGH7amB6vmDH0RUoq5vfVd5F' +
-         'SVczoEe9daSLgWbxqj3qtoGiV0pPNRBvDXi2Qdc',
-      p: '1fNapXMOJhZv0-qB-PDusFvRJQ4WS3x2sYC22ulQltE97mlW4Vqa6nzxig33xdwybM' +
-         '7xy_l2NtIvhwt28mB_moZ9snVq7PZVBapI_epfXuVPUIoF2drna_JitMo2YswXa3xi' +
-         'jHvuIHbfB_mmTgQCYw3-5j6vDtZNSLRp_hyaxKE',
-      q: 'sUITImz8-1njoDeeVZx0_4pzg-tMQc7Lbzcytw',
-      g: 'oIZbf4lU565YfI5qieOR6CZXxY8FzNlN5hdI6J4hfvqz2bX6hC68YlJZZpFq0revQi' +
-         'qbJAeBels4K2WBQ0_RoWnHWtTQ44YqP0hOn58qgW-UOo5gYPJv4nxTNYe3ZeV5SEOQ' +
-         'hOdv1qT9AE9ceNlyz38QDslJSpAmRbrKS0xvOZM',
-      x: 'YA2qCpxMxnTJi7B5VjdMhKwcM6-IFj6n4lh4dg',
-    }
   },
 };
 
@@ -123,81 +109,6 @@ async function testImportPkcs8(
   }
 }
 
-async function testImportJwk(
-  { name, publicUsages, privateUsages },
-  size,
-  hash,
-  extractable) {
-
-  const jwk = keyData[size].jwk;
-
-  const [
-    publicKey,
-    privateKey,
-  ] = await Promise.all([
-    subtle.importKey(
-      'jwk',
-      {
-        kty: jwk.kty,
-        y: jwk.y,
-        p: jwk.p,
-        q: jwk.q,
-        g: jwk.g,
-        alg: `NODE-DSA-${hash}`
-      },
-      { name, hash },
-      extractable,
-      publicUsages),
-    subtle.importKey(
-      'jwk',
-      { ...jwk, alg: `NODE-DSA-${hash}` },
-      { name, hash },
-      extractable,
-      privateUsages)
-  ]);
-
-  assert.strictEqual(publicKey.type, 'public');
-  assert.strictEqual(privateKey.type, 'private');
-  assert.strictEqual(publicKey.extractable, extractable);
-  assert.strictEqual(privateKey.extractable, extractable);
-  assert.strictEqual(publicKey.algorithm.name, name);
-  assert.strictEqual(privateKey.algorithm.name, name);
-  assert.strictEqual(publicKey.algorithm.modulusLength, size);
-  assert.strictEqual(privateKey.algorithm.modulusLength, size);
-
-  if (extractable) {
-    const [
-      pubJwk,
-      pvtJwk
-    ] = await Promise.all([
-      subtle.exportKey('jwk', publicKey),
-      subtle.exportKey('jwk', privateKey)
-    ]);
-
-    assert.strictEqual(pubJwk.kty, 'DSA');
-    assert.strictEqual(pvtJwk.kty, 'DSA');
-    assert.strictEqual(pubJwk.y, jwk.y);
-    assert.strictEqual(pvtJwk.y, jwk.y);
-    assert.strictEqual(pubJwk.p, jwk.p);
-    assert.strictEqual(pvtJwk.p, jwk.p);
-    assert.strictEqual(pubJwk.q, jwk.q);
-    assert.strictEqual(pvtJwk.q, jwk.q);
-    assert.strictEqual(pubJwk.g, jwk.g);
-    assert.strictEqual(pvtJwk.g, jwk.g);
-    assert.strictEqual(pvtJwk.x, jwk.x);
-    assert.strictEqual(pubJwk.x, undefined);
-  } else {
-    await assert.rejects(
-      subtle.exportKey('jwk', publicKey), {
-        message: /key is not extractable/
-      });
-    await assert.rejects(
-      subtle.exportKey('jwk', privateKey), {
-        message: /key is not extractable/
-      });
-  }
-}
-
 // combinations to test
 const testVectors = [
   {
@@ -215,7 +126,6 @@ const testVectors = [
         testVectors.forEach((vector) => {
           variations.push(testImportSpki(vector, size, hash, extractable));
           variations.push(testImportPkcs8(vector, size, hash, extractable));
-          variations.push(testImportJwk(vector, size, hash, extractable));
         });
       });
     });