Skip to content

Commit 3e2746f

Browse files
panvadanielleadams
panva
authored and
danielleadams
committed
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>
1 parent 760f126 commit 3e2746f

File tree

5 files changed

+11
-210
lines changed

5 files changed

+11
-210
lines changed

‎doc/api/webcrypto.md‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -605,6 +605,10 @@ The algorithms currently supported include:
605605
### `subtle.exportKey(format, key)`
606606
<!-- YAML
607607
added: v15.0.0
608+
changes:
609+
- version: REPLACEME
610+
pr-url: https://github.com/nodejs/node/pull/37203
611+
description: Removed `'NODE-DSA'` JWK export.
608612
-->
609613

610614
* `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}.
642646
| `'RSA-OAEP'` | ✔ | ✔ | ✔ | |
643647
| `'RSA-PSS'` | ✔ | ✔ | ✔ | |
644648
| `'RSASSA-PKCS1-v1_5'` | ✔ | ✔ | ✔ | |
645-
| `'NODE-DSA'` <sup>1</sup> | ✔ | ✔ | ✔ | |
649+
| `'NODE-DSA'` <sup>1</sup> | ✔ | ✔ | | |
646650
| `'NODE-DH'` <sup>1</sup> | ✔ | ✔ | | |
647651
| `'NODE-SCRYPT'` <sup>1</sup> | | | | |
648652
| `'NODE-ED25519'` <sup>1</sup> | ✔ | ✔ | ✔ | ✔ |
@@ -692,6 +696,10 @@ The {CryptoKey} (secret key) generating algorithms supported include:
692696
### `subtle.importKey(format, keyData, algorithm, extractable, keyUsages)`
693697
<!-- YAML
694698
added: v15.0.0
699+
changes:
700+
- version: REPLACEME
701+
pr-url: https://github.com/nodejs/node/pull/37203
702+
description: Removed `'NODE-DSA'` JWK import.
695703
-->
696704

697705
* `format`: {string} Must be one of `'raw'`, `'pkcs8'`, `'spki'`, `'jwk'`, or
@@ -730,7 +738,7 @@ The algorithms currently supported include:
730738
| `'RSA-OAEP'` | ✔ | ✔ | ✔ | |
731739
| `'RSA-PSS'` | ✔ | ✔ | ✔ | |
732740
| `'RSASSA-PKCS1-v1_5'` | ✔ | ✔ | ✔ | |
733-
| `'NODE-DSA'` <sup>1</sup> | ✔ | ✔ | ✔ | |
741+
| `'NODE-DSA'` <sup>1</sup> | ✔ | ✔ | | |
734742
| `'NODE-DH'` <sup>1</sup> | ✔ | ✔ | | |
735743
| `'NODE-SCRYPT'` <sup>1</sup> | | | | ✔ |
736744
| `'NODE-ED25519'` <sup>1</sup> | ✔ | ✔ | ✔ | ✔ |

‎src/crypto/crypto_dsa.cc‎

Lines changed: 0 additions & 103 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@ using v8::Maybe;
2222
using v8::Nothing;
2323
using v8::Number;
2424
using v8::Object;
25-
using v8::String;
2625
using v8::Uint32;
2726
using v8::Value;
2827

@@ -127,107 +126,6 @@ WebCryptoKeyExportStatus DSAKeyExportTraits::DoExport(
127126
}
128127
}
129128

130-
Maybe<bool> ExportJWKDsaKey(
131-
Environment* env,
132-
std::shared_ptr<KeyObjectData> key,
133-
Local<Object> target) {
134-
ManagedEVPPKey pkey = key->GetAsymmetricKey();
135-
CHECK_EQ(EVP_PKEY_id(pkey.get()), EVP_PKEY_DSA);
136-
137-
DSA* dsa = EVP_PKEY_get0_DSA(pkey.get());
138-
CHECK_NOT_NULL(dsa);
139-
140-
const BIGNUM* y;
141-
const BIGNUM* x;
142-
const BIGNUM* p;
143-
const BIGNUM* q;
144-
const BIGNUM* g;
145-
146-
DSA_get0_key(dsa, &y, &x);
147-
DSA_get0_pqg(dsa, &p, &q, &g);
148-
149-
if (target->Set(
150-
env->context(),
151-
env->jwk_kty_string(),
152-
env->jwk_dsa_string()).IsNothing()) {
153-
return Nothing<bool>();
154-
}
155-
156-
if (SetEncodedValue(env, target, env->jwk_y_string(), y).IsNothing() ||
157-
SetEncodedValue(env, target, env->jwk_p_string(), p).IsNothing() ||
158-
SetEncodedValue(env, target, env->jwk_q_string(), q).IsNothing() ||
159-
SetEncodedValue(env, target, env->jwk_g_string(), g).IsNothing()) {
160-
return Nothing<bool>();
161-
}
162-
163-
if (key->GetKeyType() == kKeyTypePrivate &&
164-
SetEncodedValue(env, target, env->jwk_x_string(), x).IsNothing()) {
165-
return Nothing<bool>();
166-
}
167-
168-
return Just(true);
169-
}
170-
171-
std::shared_ptr<KeyObjectData> ImportJWKDsaKey(
172-
Environment* env,
173-
Local<Object> jwk,
174-
const FunctionCallbackInfo<Value>& args,
175-
unsigned int offset) {
176-
Local<Value> y_value;
177-
Local<Value> p_value;
178-
Local<Value> q_value;
179-
Local<Value> g_value;
180-
Local<Value> x_value;
181-
182-
if (!jwk->Get(env->context(), env->jwk_y_string()).ToLocal(&y_value) ||
183-
!jwk->Get(env->context(), env->jwk_p_string()).ToLocal(&p_value) ||
184-
!jwk->Get(env->context(), env->jwk_q_string()).ToLocal(&q_value) ||
185-
!jwk->Get(env->context(), env->jwk_g_string()).ToLocal(&g_value) ||
186-
!jwk->Get(env->context(), env->jwk_x_string()).ToLocal(&x_value)) {
187-
return std::shared_ptr<KeyObjectData>();
188-
}
189-
190-
if (!y_value->IsString() ||
191-
!p_value->IsString() ||
192-
!q_value->IsString() ||
193-
!q_value->IsString() ||
194-
(!x_value->IsUndefined() && !x_value->IsString())) {
195-
THROW_ERR_CRYPTO_INVALID_JWK(env, "Invalid JWK DSA key");
196-
return std::shared_ptr<KeyObjectData>();
197-
}
198-
199-
KeyType type = x_value->IsString() ? kKeyTypePrivate : kKeyTypePublic;
200-
201-
DsaPointer dsa(DSA_new());
202-
203-
ByteSource y = ByteSource::FromEncodedString(env, y_value.As<String>());
204-
ByteSource p = ByteSource::FromEncodedString(env, p_value.As<String>());
205-
ByteSource q = ByteSource::FromEncodedString(env, q_value.As<String>());
206-
ByteSource g = ByteSource::FromEncodedString(env, g_value.As<String>());
207-
208-
if (!DSA_set0_key(dsa.get(), y.ToBN().release(), nullptr) ||
209-
!DSA_set0_pqg(dsa.get(),
210-
p.ToBN().release(),
211-
q.ToBN().release(),
212-
g.ToBN().release())) {
213-
THROW_ERR_CRYPTO_INVALID_JWK(env, "Invalid JWK DSA key");
214-
return std::shared_ptr<KeyObjectData>();
215-
}
216-
217-
if (type == kKeyTypePrivate) {
218-
ByteSource x = ByteSource::FromEncodedString(env, x_value.As<String>());
219-
if (!DSA_set0_key(dsa.get(), nullptr, x.ToBN().release())) {
220-
THROW_ERR_CRYPTO_INVALID_JWK(env, "Invalid JWK DSA key");
221-
return std::shared_ptr<KeyObjectData>();
222-
}
223-
}
224-
225-
EVPKeyPointer pkey(EVP_PKEY_new());
226-
CHECK_EQ(EVP_PKEY_set1_DSA(pkey.get(), dsa.get()), 1);
227-
228-
return KeyObjectData::CreateAsymmetric(type, ManagedEVPPKey(std::move(pkey)));
229-
}
230-
231129
Maybe<bool> GetDsaKeyDetail(
232130
Environment* env,
233131
std::shared_ptr<KeyObjectData> key,
@@ -269,4 +167,3 @@ void Initialize(Environment* env, Local<Object> target) {
269167
} // namespace DSAAlg
270168
} // namespace crypto
271169
} // namespace node
272-

‎src/crypto/crypto_dsa.h‎

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -61,17 +61,6 @@ struct DSAKeyExportTraits final {
6161

6262
using DSAKeyExportJob = KeyExportJob<DSAKeyExportTraits>;
6363

64-
v8::Maybe<bool> ExportJWKDsaKey(
65-
Environment* env,
66-
std::shared_ptr<KeyObjectData> key,
67-
v8::Local<v8::Object> target);
68-
69-
std::shared_ptr<KeyObjectData> ImportJWKDsaKey(
70-
Environment* env,
71-
v8::Local<v8::Object> jwk,
72-
const v8::FunctionCallbackInfo<v8::Value>& args,
73-
unsigned int offset);
74-
7564
v8::Maybe<bool> GetDsaKeyDetail(
7665
Environment* env,
7766
std::shared_ptr<KeyObjectData> key,

‎src/crypto/crypto_keys.cc‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -490,7 +490,6 @@ Maybe<bool> ExportJWKAsymmetricKey(
490490
case EVP_PKEY_RSA:
491491
// Fall through
492492
case EVP_PKEY_RSA_PSS: return ExportJWKRsaKey(env, key, target);
493-
case EVP_PKEY_DSA: return ExportJWKDsaKey(env, key, target);
494493
case EVP_PKEY_EC: return ExportJWKEcKey(env, key, target);
495494
case EVP_PKEY_ED25519:
496495
// Fall through
@@ -512,14 +511,12 @@ std::shared_ptr<KeyObjectData> ImportJWKAsymmetricKey(
512511
unsigned int offset) {
513512
if (strcmp(kty, "RSA") == 0) {
514513
return ImportJWKRsaKey(env, jwk, args, offset);
515-
} else if (strcmp(kty, "DSA") == 0) {
516-
return ImportJWKDsaKey(env, jwk, args, offset);
517514
} else if (strcmp(kty, "EC") == 0) {
518515
return ImportJWKEcKey(env, jwk, args, offset);
519516
}
520517

521518
char msg[1024];
522-
snprintf(msg, sizeof(msg), "%s is not a support JWK key type", kty);
519+
snprintf(msg, sizeof(msg), "%s is not a supported JWK key type", kty);
523520
THROW_ERR_CRYPTO_INVALID_JWK(env, msg);
524521
return std::shared_ptr<KeyObjectData>();
525522
}

‎test/parallel/test-webcrypto-export-import-dsa.js‎

Lines changed: 0 additions & 90 deletions
Original file line numberDiff line numberDiff line change
@@ -46,20 +46,6 @@ const keyData = {
4646
'f26fe27c533587b765e57948439084e76fd6a4fd004f5c78d972cf7f100ec9494a902' +
4747
'645baca4b4c6f3993041e021c600daa0a9c4cc674c98bb07956374c84ac1c33af8816' +
4848
'3ea7e2587876', 'hex'),
49-
jwk: {
50-
kty: 'DSA',
51-
y: 'mo32ny_jIYaeIJTjh7wdwrXzv_Ki4jz7pR08EZ-6a0wVpJSF-oEbaVXZHSjJ4uBEWn' +
52-
'ndxUJrL-ROAKbJJUx3bxP9ENvJNCYgd7HfcsFryEiBfGH7amB6vmDH0RUoq5vfVd5F' +
53-
'SVczoEe9daSLgWbxqj3qtoGiV0pPNRBvDXi2Qdc',
54-
p: '1fNapXMOJhZv0-qB-PDusFvRJQ4WS3x2sYC22ulQltE97mlW4Vqa6nzxig33xdwybM' +
55-
'7xy_l2NtIvhwt28mB_moZ9snVq7PZVBapI_epfXuVPUIoF2drna_JitMo2YswXa3xi' +
56-
'jHvuIHbfB_mmTgQCYw3-5j6vDtZNSLRp_hyaxKE',
57-
q: 'sUITImz8-1njoDeeVZx0_4pzg-tMQc7Lbzcytw',
58-
g: 'oIZbf4lU565YfI5qieOR6CZXxY8FzNlN5hdI6J4hfvqz2bX6hC68YlJZZpFq0revQi' +
59-
'qbJAeBels4K2WBQ0_RoWnHWtTQ44YqP0hOn58qgW-UOo5gYPJv4nxTNYe3ZeV5SEOQ' +
60-
'hOdv1qT9AE9ceNlyz38QDslJSpAmRbrKS0xvOZM',
61-
x: 'YA2qCpxMxnTJi7B5VjdMhKwcM6-IFj6n4lh4dg',
62-
}
6349
},
6450
};
6551

@@ -123,81 +109,6 @@ async function testImportPkcs8(
123109
}
124110
}
125111

126-
async function testImportJwk(
127-
{ name, publicUsages, privateUsages },
128-
size,
129-
hash,
130-
extractable) {
131-
132-
const jwk = keyData[size].jwk;
133-
134-
const [
135-
publicKey,
136-
privateKey,
137-
] = await Promise.all([
138-
subtle.importKey(
139-
'jwk',
140-
{
141-
kty: jwk.kty,
142-
y: jwk.y,
143-
p: jwk.p,
144-
q: jwk.q,
145-
g: jwk.g,
146-
alg: `NODE-DSA-${hash}`
147-
},
148-
{ name, hash },
149-
extractable,
150-
publicUsages),
151-
subtle.importKey(
152-
'jwk',
153-
{ ...jwk, alg: `NODE-DSA-${hash}` },
154-
{ name, hash },
155-
extractable,
156-
privateUsages)
157-
]);
158-
159-
assert.strictEqual(publicKey.type, 'public');
160-
assert.strictEqual(privateKey.type, 'private');
161-
assert.strictEqual(publicKey.extractable, extractable);
162-
assert.strictEqual(privateKey.extractable, extractable);
163-
assert.strictEqual(publicKey.algorithm.name, name);
164-
assert.strictEqual(privateKey.algorithm.name, name);
165-
assert.strictEqual(publicKey.algorithm.modulusLength, size);
166-
assert.strictEqual(privateKey.algorithm.modulusLength, size);
167-
168-
if (extractable) {
169-
const [
170-
pubJwk,
171-
pvtJwk
172-
] = await Promise.all([
173-
subtle.exportKey('jwk', publicKey),
174-
subtle.exportKey('jwk', privateKey)
175-
]);
176-
177-
assert.strictEqual(pubJwk.kty, 'DSA');
178-
assert.strictEqual(pvtJwk.kty, 'DSA');
179-
assert.strictEqual(pubJwk.y, jwk.y);
180-
assert.strictEqual(pvtJwk.y, jwk.y);
181-
assert.strictEqual(pubJwk.p, jwk.p);
182-
assert.strictEqual(pvtJwk.p, jwk.p);
183-
assert.strictEqual(pubJwk.q, jwk.q);
184-
assert.strictEqual(pvtJwk.q, jwk.q);
185-
assert.strictEqual(pubJwk.g, jwk.g);
186-
assert.strictEqual(pvtJwk.g, jwk.g);
187-
assert.strictEqual(pvtJwk.x, jwk.x);
188-
assert.strictEqual(pubJwk.x, undefined);
189-
} else {
190-
await assert.rejects(
191-
subtle.exportKey('jwk', publicKey), {
192-
message: /key is not extractable/
193-
});
194-
await assert.rejects(
195-
subtle.exportKey('jwk', privateKey), {
196-
message: /key is not extractable/
197-
});
198-
}
199-
}
200-
201112
// combinations to test
202113
const testVectors = [
203114
{
@@ -215,7 +126,6 @@ const testVectors = [
215126
testVectors.forEach((vector) => {
216127
variations.push(testImportSpki(vector, size, hash, extractable));
217128
variations.push(testImportPkcs8(vector, size, hash, extractable));
218-
variations.push(testImportJwk(vector, size, hash, extractable));
219129
});
220130
});
221131
});

0 commit comments

Comments
 (0)