From 2ff1c835180906836b9101cc12eb32f8fa45f4d8 Mon Sep 17 00:00:00 2001 From: Filip Skokan Date: Wed, 3 Feb 2021 12:47:56 +0100 Subject: [PATCH] crypto: remove webcrypto "DSA" JWK Key Type operations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit "DSA" is not a registered JWK key type. https://www.iana.org/assignments/jose/jose.xhtml#web-key-types PR-URL: https://github.com/nodejs/node/pull/37203 Reviewed-By: James M Snell Reviewed-By: Tobias Nießen --- doc/api/webcrypto.md | 12 +- src/crypto/crypto_dsa.cc | 103 ------------------ src/crypto/crypto_dsa.h | 11 -- src/crypto/crypto_keys.cc | 5 +- .../test-webcrypto-export-import-dsa.js | 90 --------------- 5 files changed, 11 insertions(+), 210 deletions(-) diff --git a/doc/api/webcrypto.md b/doc/api/webcrypto.md index bad6cedd69bd3f..a19154f76b1cbe 100644 --- a/doc/api/webcrypto.md +++ b/doc/api/webcrypto.md @@ -605,6 +605,10 @@ The algorithms currently supported include: ### `subtle.exportKey(format, key)` * `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'` 1 | ✔ | ✔ | ✔ | | +| `'NODE-DSA'` 1 | ✔ | ✔ | | | | `'NODE-DH'` 1 | ✔ | ✔ | | | | `'NODE-SCRYPT'` 1 | | | | | | `'NODE-ED25519'` 1 | ✔ | ✔ | ✔ | ✔ | @@ -692,6 +696,10 @@ The {CryptoKey} (secret key) generating algorithms supported include: ### `subtle.importKey(format, keyData, algorithm, extractable, keyUsages)` * `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'` 1 | ✔ | ✔ | ✔ | | +| `'NODE-DSA'` 1 | ✔ | ✔ | | | | `'NODE-DH'` 1 | ✔ | ✔ | | | | `'NODE-SCRYPT'` 1 | | | | ✔ | | `'NODE-ED25519'` 1 | ✔ | ✔ | ✔ | ✔ | diff --git a/src/crypto/crypto_dsa.cc b/src/crypto/crypto_dsa.cc index 8a80904d9c1afb..970ab5cf5f048b 100644 --- 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 ExportJWKDsaKey( - Environment* env, - std::shared_ptr key, - Local 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(); - } - - 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(); - } - - if (key->GetKeyType() == kKeyTypePrivate && - SetEncodedValue(env, target, env->jwk_x_string(), x).IsNothing()) { - return Nothing(); - } - - return Just(true); -} - -std::shared_ptr ImportJWKDsaKey( - Environment* env, - Local jwk, - const FunctionCallbackInfo& args, - unsigned int offset) { - Local y_value; - Local p_value; - Local q_value; - Local g_value; - Local 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(); - } - - 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(); - } - - KeyType type = x_value->IsString() ? kKeyTypePrivate : kKeyTypePublic; - - DsaPointer dsa(DSA_new()); - - ByteSource y = ByteSource::FromEncodedString(env, y_value.As()); - ByteSource p = ByteSource::FromEncodedString(env, p_value.As()); - ByteSource q = ByteSource::FromEncodedString(env, q_value.As()); - ByteSource g = ByteSource::FromEncodedString(env, g_value.As()); - - 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(); - } - - if (type == kKeyTypePrivate) { - ByteSource x = ByteSource::FromEncodedString(env, x_value.As()); - if (!DSA_set0_key(dsa.get(), nullptr, x.ToBN().release())) { - THROW_ERR_CRYPTO_INVALID_JWK(env, "Invalid JWK DSA key"); - return std::shared_ptr(); - } - } - - 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 GetDsaKeyDetail( Environment* env, std::shared_ptr key, @@ -269,4 +167,3 @@ void Initialize(Environment* env, Local target) { } // namespace DSAAlg } // namespace crypto } // namespace node - diff --git a/src/crypto/crypto_dsa.h b/src/crypto/crypto_dsa.h index b685d6a88f9412..3f241b33ba06cf 100644 --- a/src/crypto/crypto_dsa.h +++ b/src/crypto/crypto_dsa.h @@ -61,17 +61,6 @@ struct DSAKeyExportTraits final { using DSAKeyExportJob = KeyExportJob; -v8::Maybe ExportJWKDsaKey( - Environment* env, - std::shared_ptr key, - v8::Local target); - -std::shared_ptr ImportJWKDsaKey( - Environment* env, - v8::Local jwk, - const v8::FunctionCallbackInfo& args, - unsigned int offset); - v8::Maybe GetDsaKeyDetail( Environment* env, std::shared_ptr key, diff --git a/src/crypto/crypto_keys.cc b/src/crypto/crypto_keys.cc index 9c9fc4a9fcaf59..891e65dcbc2933 100644 --- a/src/crypto/crypto_keys.cc +++ b/src/crypto/crypto_keys.cc @@ -490,7 +490,6 @@ Maybe 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 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(); } diff --git a/test/parallel/test-webcrypto-export-import-dsa.js b/test/parallel/test-webcrypto-export-import-dsa.js index f73e6513dddaff..e6bcdebc5bbdc7 100644 --- 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)); }); }); });