Skip to content

Commit 7dc563b

Browse files
panvaaduh95
panva
authored and
aduh95
committed
crypto: wire AES-KW in Web Cryptography when using BoringSSL
Signed-off-by: Filip Skokan <panva.ip@gmail.com> PR-URL: #63255 Refs: electron/electron#36256 Refs: electron/electron#41720 Refs: electron/electron#51127 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
1 parent 592f741 commit 7dc563b

12 files changed

Lines changed: 136 additions & 100 deletions

‎lib/internal/crypto/util.js‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -396,7 +396,6 @@ const kAlgorithmDefinitions = {
396396

397397
// Conditionally supported algorithms
398398
const conditionalAlgorithms = {
399-
'AES-KW': !process.features.openssl_is_boringssl,
400399
'AES-OCB': !!hasAesOcbMode,
401400
'Argon2d': !!Argon2Job,
402401
'Argon2i': !!Argon2Job,

‎src/crypto/crypto_aes.cc‎

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -181,6 +181,68 @@ WebCryptoCipherStatus AES_Cipher(Environment* env,
181181
return WebCryptoCipherStatus::OK;
182182
}
183183

184+
#ifdef OPENSSL_IS_BORINGSSL
185+
// AES Key Wrap using BoringSSL's low-level AES_wrap_key / AES_unwrap_key.
186+
// BoringSSL does not expose EVP_aes_*_wrap via the
187+
// EVP_CIPHER registry, so the EVP-based AES_Cipher path is unusable for
188+
// AES-KW. This matches Chromium's WebCrypto AES-KW implementation.
189+
WebCryptoCipherStatus AES_KW_Cipher(Environment* env,
190+
const KeyObjectData& key_data,
191+
WebCryptoCipherMode cipher_mode,
192+
const AESCipherConfig& params,
193+
const ByteSource& in,
194+
ByteSource* out) {
195+
CHECK_EQ(key_data.GetKeyType(), kKeyTypeSecret);
196+
197+
const unsigned key_bits =
198+
static_cast<unsigned>(key_data.GetSymmetricKeySize()) * 8;
199+
const auto key_bytes =
200+
reinterpret_cast<const unsigned char*>(key_data.GetSymmetricKey());
201+
const bool encrypt = cipher_mode == kWebCryptoCipherEncrypt;
202+
203+
AES_KEY aes_key;
204+
if (encrypt) {
205+
// Input must be a multiple of 8 bytes and at least 16 bytes.
206+
if (in.size() < 16 || in.size() % 8 != 0) {
207+
return WebCryptoCipherStatus::FAILED;
208+
}
209+
if (AES_set_encrypt_key(key_bytes, key_bits, &aes_key) != 0) {
210+
return WebCryptoCipherStatus::FAILED;
211+
}
212+
auto buf = DataPointer::Alloc(in.size() + 8);
213+
int len = AES_wrap_key(&aes_key,
214+
nullptr,
215+
static_cast<unsigned char*>(buf.get()),
216+
in.data<unsigned char>(),
217+
in.size());
218+
if (len < 0 || static_cast<size_t>(len) != in.size() + 8) {
219+
return WebCryptoCipherStatus::FAILED;
220+
}
221+
*out = ByteSource::Allocated(buf.release());
222+
} else {
223+
// Input must be a multiple of 8 bytes and at least 24 bytes.
224+
if (in.size() < 24 || in.size() % 8 != 0) {
225+
return WebCryptoCipherStatus::FAILED;
226+
}
227+
if (AES_set_decrypt_key(key_bytes, key_bits, &aes_key) != 0) {
228+
return WebCryptoCipherStatus::FAILED;
229+
}
230+
auto buf = DataPointer::Alloc(in.size() - 8);
231+
int len = AES_unwrap_key(&aes_key,
232+
nullptr,
233+
static_cast<unsigned char*>(buf.get()),
234+
in.data<unsigned char>(),
235+
in.size());
236+
if (len < 0 || static_cast<size_t>(len) != in.size() - 8) {
237+
return WebCryptoCipherStatus::FAILED;
238+
}
239+
*out = ByteSource::Allocated(buf.release());
240+
}
241+
242+
return WebCryptoCipherStatus::OK;
243+
}
244+
#endif // OPENSSL_IS_BORINGSSL
245+
184246
// The AES_CTR implementation here takes it's inspiration from the chromium
185247
// implementation here:
186248
// https://github.com/chromium/chromium/blob/7af6cfd/components/webcrypto/algorithms/aes_ctr.cc
@@ -465,6 +527,19 @@ Maybe<void> AESCipherTraits::AdditionalConfig(
465527
}
466528
#undef V
467529

530+
#ifdef OPENSSL_IS_BORINGSSL
531+
// On BoringSSL the KW variants have no backing EVP_CIPHER; they use
532+
// low-level AES_wrap_key / AES_unwrap_key instead.
533+
const bool is_kw = params->variant == AESKeyVariant::KW_128 ||
534+
params->variant == AESKeyVariant::KW_192 ||
535+
params->variant == AESKeyVariant::KW_256;
536+
537+
if (is_kw) {
538+
UseDefaultIV(params);
539+
return JustVoid();
540+
}
541+
#endif
542+
468543
if (!params->cipher) {
469544
THROW_ERR_CRYPTO_UNKNOWN_CIPHER(env);
470545
return Nothing<void>();

‎src/crypto/crypto_aes.h‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,9 +22,21 @@ constexpr unsigned kNoAuthTagLength = static_cast<unsigned>(-1);
2222
V(GCM_128, AES_Cipher, ncrypto::Cipher::AES_128_GCM) \
2323
V(GCM_192, AES_Cipher, ncrypto::Cipher::AES_192_GCM) \
2424
V(GCM_256, AES_Cipher, ncrypto::Cipher::AES_256_GCM) \
25+
VARIANTS_KW(V)
26+
27+
#ifdef OPENSSL_IS_BORINGSSL
28+
// BoringSSL does not expose EVP_aes_*_wrap via the EVP_CIPHER registry.
29+
// Route AES-KW through low-level AES_wrap_key / AES_unwrap_key instead.
30+
#define VARIANTS_KW(V) \
31+
V(KW_128, AES_KW_Cipher, static_cast<const EVP_CIPHER*>(nullptr)) \
32+
V(KW_192, AES_KW_Cipher, static_cast<const EVP_CIPHER*>(nullptr)) \
33+
V(KW_256, AES_KW_Cipher, static_cast<const EVP_CIPHER*>(nullptr))
34+
#else
35+
#define VARIANTS_KW(V) \
2536
V(KW_128, AES_Cipher, ncrypto::Cipher::AES_128_KW) \
2637
V(KW_192, AES_Cipher, ncrypto::Cipher::AES_192_KW) \
2738
V(KW_256, AES_Cipher, ncrypto::Cipher::AES_256_KW)
39+
#endif
2840

2941
#if OPENSSL_WITH_AES_OCB
3042
#define VARIANTS_OCB(V) \

‎test/fixtures/webcrypto/supports-level-2.mjs‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ export const vectors = {
7474
[false, { name: 'AES-CBC', length: 25 }],
7575
[true, { name: 'AES-GCM', length: 128 }],
7676
[false, { name: 'AES-GCM', length: 25 }],
77-
[!boringSSL, { name: 'AES-KW', length: 128 }],
77+
[true, { name: 'AES-KW', length: 128 }],
7878
[false, { name: 'AES-KW', length: 25 }],
7979
[true, { name: 'HMAC', hash: 'SHA-256' }],
8080
[true, { name: 'HMAC', hash: 'SHA-256', length: 256 }],
@@ -192,7 +192,7 @@ export const vectors = {
192192
[true, 'AES-CTR'],
193193
[true, 'AES-CBC'],
194194
[true, 'AES-GCM'],
195-
[!boringSSL, 'AES-KW'],
195+
[true, 'AES-KW'],
196196
[true, { name: 'HMAC', hash: 'SHA-256' }],
197197
[true, { name: 'HMAC', hash: 'SHA-256', length: 256 }],
198198
[false, { name: 'HMAC', hash: 'SHA-256', length: 25 }],
@@ -214,18 +214,18 @@ export const vectors = {
214214
[true, 'AES-CTR'],
215215
[true, 'AES-CBC'],
216216
[true, 'AES-GCM'],
217-
[!boringSSL, 'AES-KW'],
217+
[true, 'AES-KW'],
218218
[true, 'Ed25519'],
219219
[true, 'X25519'],
220220
],
221221
'wrapKey': [
222222
[false, 'AES-KW'],
223-
[!boringSSL, 'AES-KW', 'AES-CTR'],
224-
[!boringSSL, 'AES-KW', 'HMAC'],
223+
[true, 'AES-KW', 'AES-CTR'],
224+
[true, 'AES-KW', 'HMAC'],
225225
],
226226
'unwrapKey': [
227227
[false, 'AES-KW'],
228-
[!boringSSL, 'AES-KW', 'AES-CTR'],
228+
[true, 'AES-KW', 'AES-CTR'],
229229
],
230230
'unsupported operation': [
231231
[false, ''],

‎test/parallel/test-crypto-key-objects-to-crypto-key.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,8 @@ function assertCryptoKey(cryptoKey, keyObject, algorithm, extractable, usages) {
3131
algorithms.push('ChaCha20-Poly1305');
3232

3333
if (process.features.openssl_is_boringssl) {
34-
algorithms = algorithms.filter((a) => a !== 'AES-KW' && a !== 'ChaCha20-Poly1305');
35-
common.printSkipMessage('Skipping unsupported AES-KW/ChaCha20-Poly1305 test cases');
34+
algorithms = algorithms.filter((a) => a !== 'ChaCha20-Poly1305');
35+
common.printSkipMessage('Skipping unsupported ChaCha20-Poly1305 test case');
3636
}
3737

3838
for (const algorithm of algorithms) {

‎test/parallel/test-webcrypto-deduplicate-usages.js‎

Lines changed: 9 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -42,17 +42,10 @@ function assertSameSet(actual, expected, msg) {
4242
{ algorithm: { name: 'AES-GCM', length: 128 },
4343
usages: ['decrypt', 'encrypt', 'decrypt'],
4444
expected: ['encrypt', 'decrypt'] },
45-
];
46-
47-
if (!process.features.openssl_is_boringssl) {
48-
symmetric.push({
49-
algorithm: { name: 'AES-KW', length: 128 },
45+
{ algorithm: { name: 'AES-KW', length: 128 },
5046
usages: ['wrapKey', 'unwrapKey', 'wrapKey', 'unwrapKey'],
51-
expected: ['wrapKey', 'unwrapKey'],
52-
});
53-
} else {
54-
common.printSkipMessage('AES-KW is not supported in BoringSSL');
55-
}
47+
expected: ['wrapKey', 'unwrapKey'] },
48+
];
5649

5750
if (hasOpenSSL(3)) {
5851
symmetric.push({
@@ -172,17 +165,10 @@ function assertSameSet(actual, expected, msg) {
172165
{ algorithm: { name: 'HMAC', hash: 'SHA-256' }, keyData: new Uint8Array(32),
173166
usages: ['verify', 'sign', 'verify', 'sign'],
174167
expected: ['sign', 'verify'] },
175-
];
176-
177-
if (!process.features.openssl_is_boringssl) {
178-
rawSymmetric.push({
179-
algorithm: { name: 'AES-KW' }, keyData: new Uint8Array(16),
168+
{ algorithm: { name: 'AES-KW' }, keyData: new Uint8Array(16),
180169
usages: ['wrapKey', 'unwrapKey', 'wrapKey'],
181-
expected: ['wrapKey', 'unwrapKey'],
182-
});
183-
} else {
184-
common.printSkipMessage('AES-KW is not supported in BoringSSL');
185-
}
170+
expected: ['wrapKey', 'unwrapKey'] },
171+
];
186172

187173
if (hasOpenSSL(3)) {
188174
// KMAC does not support `raw` format, only `raw-secret` and `jwk`.
@@ -455,17 +441,10 @@ function assertSameSet(actual, expected, msg) {
455441
{ algorithm: { name: 'AES-GCM', length: 128 },
456442
usages: ['decrypt', 'encrypt', 'decrypt'],
457443
expected: ['encrypt', 'decrypt'] },
458-
];
459-
460-
if (!process.features.openssl_is_boringssl) {
461-
jwkVectors.push({
462-
algorithm: { name: 'AES-KW', length: 128 },
444+
{ algorithm: { name: 'AES-KW', length: 128 },
463445
usages: ['wrapKey', 'unwrapKey', 'wrapKey', 'unwrapKey'],
464-
expected: ['wrapKey', 'unwrapKey'],
465-
});
466-
} else {
467-
common.printSkipMessage('AES-KW is not supported in BoringSSL');
468-
}
446+
expected: ['wrapKey', 'unwrapKey'] },
447+
];
469448

470449
if (hasOpenSSL(3)) {
471450
jwkVectors.push({

‎test/parallel/test-webcrypto-derivebits-hkdf.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,12 +24,12 @@ const kDerivedKeyTypes = [
2424
['HMAC', 256, 'SHA-256', 'sign', 'verify'],
2525
['HMAC', 256, 'SHA-384', 'sign', 'verify'],
2626
['HMAC', 256, 'SHA-512', 'sign', 'verify'],
27+
['AES-KW', 128, undefined, 'wrapKey', 'unwrapKey'],
28+
['AES-KW', 256, undefined, 'wrapKey', 'unwrapKey'],
2729
];
2830

2931
if (!process.features.openssl_is_boringssl) {
3032
kDerivedKeyTypes.push(
31-
['AES-KW', 128, undefined, 'wrapKey', 'unwrapKey'],
32-
['AES-KW', 256, undefined, 'wrapKey', 'unwrapKey'],
3333
['HMAC', 256, 'SHA3-256', 'sign', 'verify'],
3434
['HMAC', 256, 'SHA3-384', 'sign', 'verify'],
3535
['HMAC', 256, 'SHA3-512', 'sign', 'verify'],

‎test/parallel/test-webcrypto-keygen.js‎

Lines changed: 10 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,14 @@ const vectors = {
135135
'deriveBits',
136136
],
137137
},
138+
'AES-KW': {
139+
algorithm: { length: 256 },
140+
result: 'CryptoKey',
141+
usages: [
142+
'wrapKey',
143+
'unwrapKey',
144+
],
145+
}
138146
};
139147

140148
if (!process.features.openssl_is_boringssl) {
@@ -152,14 +160,6 @@ if (!process.features.openssl_is_boringssl) {
152160
'deriveBits',
153161
],
154162
};
155-
vectors['AES-KW'] = {
156-
algorithm: { length: 256 },
157-
result: 'CryptoKey',
158-
usages: [
159-
'wrapKey',
160-
'unwrapKey',
161-
],
162-
};
163163
vectors['ChaCha20-Poly1305'] = {
164164
result: 'CryptoKey',
165165
usages: [
@@ -606,17 +606,10 @@ if (hasOpenSSL(3, 5)) {
606606
[ 'AES-CBC', 256, ['encrypt', 'decrypt']],
607607
[ 'AES-GCM', 128, ['encrypt', 'decrypt']],
608608
[ 'AES-GCM', 256, ['encrypt', 'decrypt']],
609+
[ 'AES-KW', 128, ['wrapKey', 'unwrapKey']],
610+
[ 'AES-KW', 256, ['wrapKey', 'unwrapKey']],
609611
];
610612

611-
if (!process.features.openssl_is_boringssl) {
612-
kTests.push(
613-
[ 'AES-KW', 128, ['wrapKey', 'unwrapKey']],
614-
[ 'AES-KW', 256, ['wrapKey', 'unwrapKey']],
615-
);
616-
} else {
617-
common.printSkipMessage('Skipping unsupported AES-KW test cases');
618-
}
619-
620613
const tests = Promise.all(kTests.map((args) => test(...args)));
621614

622615
tests.then(common.mustCall());

‎test/parallel/test-webcrypto-promise-prototype-pollution.mjs‎

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -59,21 +59,17 @@ await subtle.deriveKey(
5959
true,
6060
['encrypt', 'decrypt']);
6161

62-
if (!process.features.openssl_is_boringssl) {
63-
const wrappingKey = await subtle.generateKey(
64-
{ name: 'AES-KW', length: 256 }, true, ['wrapKey', 'unwrapKey']);
62+
const wrappingKey = await subtle.generateKey(
63+
{ name: 'AES-KW', length: 256 }, true, ['wrapKey', 'unwrapKey']);
6564

66-
const keyToWrap = await subtle.generateKey(
67-
{ name: 'AES-CBC', length: 256 }, true, ['encrypt', 'decrypt']);
65+
const keyToWrap = await subtle.generateKey(
66+
{ name: 'AES-CBC', length: 256 }, true, ['encrypt', 'decrypt']);
6867

69-
const wrapped = await subtle.wrapKey('raw', keyToWrap, wrappingKey, 'AES-KW');
68+
const wrapped = await subtle.wrapKey('raw', keyToWrap, wrappingKey, 'AES-KW');
7069

71-
await subtle.unwrapKey(
72-
'raw', wrapped, wrappingKey, 'AES-KW',
73-
{ name: 'AES-CBC', length: 256 }, true, ['encrypt', 'decrypt']);
74-
} else {
75-
common.printSkipMessage('Skipping unsupported AES-KW test case');
76-
}
70+
await subtle.unwrapKey(
71+
'raw', wrapped, wrappingKey, 'AES-KW',
72+
{ name: 'AES-CBC', length: 256 }, true, ['encrypt', 'decrypt']);
7773

7874
const { privateKey } = await subtle.generateKey(
7975
{ name: 'ECDSA', namedCurve: 'P-256' }, true, ['sign', 'verify']);

‎test/parallel/test-webcrypto-wrap-unwrap.js‎

Lines changed: 10 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -39,14 +39,15 @@ const kWrappingData = {
3939
},
4040
pair: false
4141
},
42-
};
43-
44-
if (!process.features.openssl_is_boringssl) {
45-
kWrappingData['AES-KW'] = {
42+
'AES-KW': {
4643
generate: { length: 128 },
4744
wrap: { },
4845
pair: false
49-
};
46+
},
47+
};
48+
49+
50+
if (!process.features.openssl_is_boringssl) {
5051
kWrappingData['ChaCha20-Poly1305'] = {
5152
wrap: {
5253
iv: new Uint8Array(12),
@@ -56,7 +57,7 @@ if (!process.features.openssl_is_boringssl) {
5657
pair: false
5758
};
5859
} else {
59-
common.printSkipMessage('Skipping unsupported AES-KW test case');
60+
common.printSkipMessage('Skipping unsupported ChaCha20-Poly1305 test case');
6061
}
6162

6263
if (hasOpenSSL(3)) {
@@ -188,20 +189,15 @@ async function generateKeysToWrap() {
188189
usages: ['sign', 'verify'],
189190
pair: false,
190191
},
191-
];
192-
193-
if (!process.features.openssl_is_boringssl) {
194-
parameters.push({
192+
{
195193
algorithm: {
196194
name: 'AES-KW',
197195
length: 128
198196
},
199197
usages: ['wrapKey', 'unwrapKey'],
200198
pair: false,
201-
});
202-
} else {
203-
common.printSkipMessage('Skipping unsupported AES-KW test case');
204-
}
199+
},
200+
];
205201

206202
if (!process.features.openssl_is_boringssl) {
207203
parameters.push({

0 commit comments

Comments
 (0)