Skip to content

Commit 4bdf06e

Browse files
panvapanva
committed
crypto: allow length=0 for HKDF and PBKDF2 in SubtleCrypto.deriveBits
PR-URL: #55866 Reviewed-By: Matthew Aitken <maitken033380023@gmail.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Jason Zhang <xzha4350@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de>
1 parent b1aca24 commit 4bdf06e

File tree

50 files changed

+1819
-1270
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

50 files changed

+1819
-1270
lines changed

‎lib/internal/crypto/hkdf.js‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
'use strict';
22

33
const {
4+
ArrayBuffer,
45
FunctionPrototypeCall,
56
} = primordials;
67

@@ -141,7 +142,7 @@ async function hkdfDeriveBits(algorithm, baseKey, length) {
141142
const { hash, salt, info } = algorithm;
142143

143144
if (length === 0)
144-
throw lazyDOMException('length cannot be zero', 'OperationError');
145+
return new ArrayBuffer(0);
145146
if (length === null)
146147
throw lazyDOMException('length cannot be null', 'OperationError');
147148
if (length % 8) {

‎lib/internal/crypto/pbkdf2.js‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
'use strict';
22

33
const {
4+
ArrayBuffer,
45
FunctionPrototypeCall,
56
} = primordials;
67

@@ -98,10 +99,8 @@ async function pbkdf2DeriveBits(algorithm, baseKey, length) {
9899
'iterations cannot be zero',
99100
'OperationError');
100101

101-
const raw = baseKey[kKeyObject].export();
102-
103102
if (length === 0)
104-
throw lazyDOMException('length cannot be zero', 'OperationError');
103+
return new ArrayBuffer(0);
105104
if (length === null)
106105
throw lazyDOMException('length cannot be null', 'OperationError');
107106
if (length % 8) {
@@ -113,7 +112,7 @@ async function pbkdf2DeriveBits(algorithm, baseKey, length) {
113112
let result;
114113
try {
115114
result = await pbkdf2Promise(
116-
raw, salt, iterations, length / 8, normalizeHashName(hash.name),
115+
baseKey[kKeyObject].export(), salt, iterations, length / 8, normalizeHashName(hash.name),
117116
);
118117
} catch (err) {
119118
throw lazyDOMException(

‎test/fixtures/wpt/README.md‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,15 +25,15 @@ Last update:
2525
- interfaces: https://github.com/web-platform-tests/wpt/tree/df731dab88/interfaces
2626
- performance-timeline: https://github.com/web-platform-tests/wpt/tree/17ebc3aea0/performance-timeline
2727
- resource-timing: https://github.com/web-platform-tests/wpt/tree/22d38586d0/resource-timing
28-
- resources: https://github.com/web-platform-tests/wpt/tree/1e140d63ec/resources
28+
- resources: https://github.com/web-platform-tests/wpt/tree/919874f84f/resources
2929
- streams: https://github.com/web-platform-tests/wpt/tree/2bd26e124c/streams
3030
- url: https://github.com/web-platform-tests/wpt/tree/67880a4eb8/url
3131
- user-timing: https://github.com/web-platform-tests/wpt/tree/5ae85bf826/user-timing
3232
- wasm/jsapi: https://github.com/web-platform-tests/wpt/tree/cde25e7e3c/wasm/jsapi
3333
- wasm/webapi: https://github.com/web-platform-tests/wpt/tree/fd1b23eeaa/wasm/webapi
34-
- WebCryptoAPI: https://github.com/web-platform-tests/wpt/tree/5e042cbc4e/WebCryptoAPI
34+
- WebCryptoAPI: https://github.com/web-platform-tests/wpt/tree/b81831169b/WebCryptoAPI
3535
- webidl/ecmascript-binding/es-exceptions: https://github.com/web-platform-tests/wpt/tree/a370aad338/webidl/ecmascript-binding/es-exceptions
3636
- webmessaging/broadcastchannel: https://github.com/web-platform-tests/wpt/tree/e97fac4791/webmessaging/broadcastchannel
3737

3838
[Web Platform Tests]: https://github.com/web-platform-tests/wpt
39-
[`git node wpt`]: https://github.com/nodejs/node-core-utils/blob/main/docs/git-node.md#git-node-wpt
39+
[`git node wpt`]: https://github.com/nodejs/node-core-utils/blob/main/docs/git-node.md#git-node-wpt

‎test/fixtures/wpt/WebCryptoAPI/cryptokey_algorithm_returns_cached_object.https.any.js‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
// META: title=WebCryptoAPI: CryptoKey.algorithm getter returns cached object
2+
3+
// https://w3c.github.io/webcrypto/#dom-cryptokey-algorithm
4+
// https://github.com/servo/servo/issues/33908
5+
6+
promise_test(function() {
7+
return self.crypto.subtle.generateKey(
8+
{
9+
name: "AES-CTR",
10+
length: 256,
11+
},
12+
true,
13+
["encrypt"],
14+
).then(
15+
function(key) {
16+
let a = key.algorithm;
17+
let b = key.algorithm;
18+
assert_true(a === b);
19+
},
20+
function(err) {
21+
assert_unreached("generateKey threw an unexpected error: " + err.toString());
22+
}
23+
);
24+
}, "CryptoKey.algorithm getter returns cached object");

‎test/fixtures/wpt/WebCryptoAPI/derive_bits_keys/cfrg_curves_bits.js‎

Lines changed: 32 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,19 @@
1+
function define_tests_25519() {
2+
return define_tests("X25519");
3+
}
4+
5+
function define_tests_448() {
6+
return define_tests("X448");
7+
}
18

2-
function define_tests() {
9+
function define_tests(algorithmName) {
310
// May want to test prefixed implementations.
411
var subtle = self.crypto.subtle;
512

613
// Verify the derive functions perform checks against the all-zero value results,
714
// ensuring small-order points are rejected.
815
// https://www.rfc-editor.org/rfc/rfc7748#section-6.1
9-
// TODO: The spec states that the check must be done on use, but there is discussion about doing it on import.
10-
// https://github.com/WICG/webcrypto-secure-curves/pull/13
11-
Object.keys(kSmallOrderPoint).forEach(function(algorithmName) {
16+
{
1217
kSmallOrderPoint[algorithmName].forEach(function(test) {
1318
promise_test(async() => {
1419
let derived;
@@ -23,22 +28,23 @@ function define_tests() {
2328
false, [])
2429
derived = await subtle.deriveBits({name: algorithmName, public: publicKey}, privateKey, 8 * sizes[algorithmName]);
2530
} catch (err) {
26-
assert_false(privateKey === undefined, "Private key should be valid.");
27-
assert_false(publicKey === undefined, "Public key should be valid.");
31+
assert_true(privateKey !== undefined, "Private key should be valid.");
32+
assert_true(publicKey !== undefined, "Public key should be valid.");
2833
assert_equals(err.name, "OperationError", "Should throw correct error, not " + err.name + ": " + err.message + ".");
2934
}
3035
assert_equals(derived, undefined, "Operation succeeded, but should not have.");
3136
}, algorithmName + " key derivation checks for all-zero value result with a key of order " + test.order);
3237
});
33-
});
38+
}
3439

3540
return importKeys(pkcs8, spki, sizes)
3641
.then(function(results) {
3742
publicKeys = results.publicKeys;
3843
privateKeys = results.privateKeys;
3944
noDeriveBitsKeys = results.noDeriveBitsKeys;
45+
ecdhKeys = results.ecdhKeys;
4046

41-
Object.keys(sizes).forEach(function(algorithmName) {
47+
{
4248
// Basic success case
4349
promise_test(function(test) {
4450
return subtle.deriveBits({name: algorithmName, public: publicKeys[algorithmName]}, privateKeys[algorithmName], 8 * sizes[algorithmName])
@@ -59,25 +65,6 @@ function define_tests() {
5965
});
6066
}, algorithmName + " mixed case parameters");
6167

62-
// Null length
63-
// "Null" is not valid per the current spec
64-
// - https://github.com/w3c/webcrypto/issues/322
65-
// - https://github.com/w3c/webcrypto/issues/329
66-
//
67-
// Proposal for a spec change:
68-
// - https://github.com/w3c/webcrypto/pull/345
69-
//
70-
// This test case may be replaced by these new tests:
71-
// - https://github.com/web-platform-tests/wpt/pull/43400
72-
promise_test(function(test) {
73-
return subtle.deriveBits({name: algorithmName, public: publicKeys[algorithmName]}, privateKeys[algorithmName], null)
74-
.then(function(derivation) {
75-
assert_true(equalBuffers(derivation, derivations[algorithmName]), "Derived correct bits");
76-
}, function(err) {
77-
assert_unreached("deriveBits failed with error " + err.name + ": " + err.message);
78-
});
79-
}, algorithmName + " with null length");
80-
8168
// Shorter than entire derivation per algorithm
8269
promise_test(function(test) {
8370
return subtle.deriveBits({name: algorithmName, public: publicKeys[algorithmName]}, privateKeys[algorithmName], 8 * sizes[algorithmName] - 32)
@@ -122,11 +109,7 @@ function define_tests() {
122109

123110
// - wrong algorithm
124111
promise_test(function(test) {
125-
publicKey = publicKeys["X25519"];
126-
if (algorithmName === "X25519") {
127-
publicKey = publicKeys["X448"];
128-
}
129-
return subtle.deriveBits({name: algorithmName, public: publicKey}, privateKeys[algorithmName], 8 * sizes[algorithmName])
112+
return subtle.deriveBits({name: algorithmName, public: ecdhKeys[algorithmName]}, privateKeys[algorithmName], 8 * sizes[algorithmName])
130113
.then(function(derivation) {
131114
assert_unreached("deriveBits succeeded but should have failed with InvalidAccessError");
132115
}, function(err) {
@@ -186,16 +169,17 @@ function define_tests() {
186169
assert_equals(err.name, "OperationError", "Should throw correct error, not " + err.name + ": " + err.message);
187170
});
188171
}, algorithmName + " asking for too many bits");
189-
});
172+
}
190173
});
191174

192175
function importKeys(pkcs8, spki, sizes) {
193176
var privateKeys = {};
194177
var publicKeys = {};
195178
var noDeriveBitsKeys = {};
179+
var ecdhPublicKeys = {};
196180

197181
var promises = [];
198-
Object.keys(pkcs8).forEach(function(algorithmName) {
182+
{
199183
var operation = subtle.importKey("pkcs8", pkcs8[algorithmName],
200184
{name: algorithmName},
201185
false, ["deriveBits", "deriveKey"])
@@ -205,8 +189,8 @@ function define_tests() {
205189
privateKeys[algorithmName] = null;
206190
});
207191
promises.push(operation);
208-
});
209-
Object.keys(pkcs8).forEach(function(algorithmName) {
192+
}
193+
{
210194
var operation = subtle.importKey("pkcs8", pkcs8[algorithmName],
211195
{name: algorithmName},
212196
false, ["deriveKey"])
@@ -216,8 +200,8 @@ function define_tests() {
216200
noDeriveBitsKeys[algorithmName] = null;
217201
});
218202
promises.push(operation);
219-
});
220-
Object.keys(spki).forEach(function(algorithmName) {
203+
}
204+
{
221205
var operation = subtle.importKey("spki", spki[algorithmName],
222206
{name: algorithmName},
223207
false, [])
@@ -227,10 +211,17 @@ function define_tests() {
227211
publicKeys[algorithmName] = null;
228212
});
229213
promises.push(operation);
230-
});
231-
214+
}
215+
{
216+
var operation = subtle.importKey("spki", ecSPKI,
217+
{name: "ECDH", namedCurve: "P-256"},
218+
false, [])
219+
.then(function(key) {
220+
ecdhPublicKeys[algorithmName] = key;
221+
});
222+
}
232223
return Promise.all(promises)
233-
.then(function(results) {return {privateKeys: privateKeys, publicKeys: publicKeys, noDeriveBitsKeys: noDeriveBitsKeys}});
224+
.then(function(results) {return {privateKeys: privateKeys, publicKeys: publicKeys, noDeriveBitsKeys: noDeriveBitsKeys, ecdhKeys: ecdhPublicKeys}});
234225
}
235226

236227
// Compares two ArrayBuffer or ArrayBufferView objects. If bitCount is

‎test/fixtures/wpt/WebCryptoAPI/derive_bits_keys/cfrg_curves_bits_curve25519.https.any.js‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
// META: title=WebCryptoAPI: deriveKey() Using ECDH with CFRG Elliptic Curves
2+
// META: script=cfrg_curves_bits_fixtures.js
3+
// META: script=cfrg_curves_bits.js
4+
5+
// Define subtests from a `promise_test` to ensure the harness does not
6+
// complete before the subtests are available. `explicit_done` cannot be used
7+
// for this purpose because the global `done` function is automatically invoked
8+
// by the WPT infrastructure in dedicated worker tests defined using the
9+
// "multi-global" pattern.
10+
promise_test(define_tests_25519, 'setup - define tests');

test/fixtures/wpt/WebCryptoAPI/derive_bits_keys/cfrg_curves_bits.https.any.js renamed to test/fixtures/wpt/WebCryptoAPI/derive_bits_keys/cfrg_curves_bits_curve448.https.any.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// META: title=WebCryptoAPI: deriveBits() Using ECDH with CFRG Elliptic Curves
1+
// META: title=WebCryptoAPI: deriveKey() Using ECDH with CFRG Elliptic Curves
22
// META: script=cfrg_curves_bits_fixtures.js
33
// META: script=cfrg_curves_bits.js
44

@@ -7,4 +7,4 @@
77
// for this purpose because the global `done` function is automatically invoked
88
// by the WPT infrastructure in dedicated worker tests defined using the
99
// "multi-global" pattern.
10-
promise_test(define_tests, 'setup - define tests');
10+
promise_test(define_tests_448, 'setup - define tests');

‎test/fixtures/wpt/WebCryptoAPI/derive_bits_keys/cfrg_curves_bits_fixtures.js‎

Lines changed: 3 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)