Skip to content

Commit 83c38a9

Browse files
javifernandezjavifernandez
committed
Bug 1793429 - Make the deriveBits's length argument Nullable r=keeler,webidl,smaug
The PR#345 [1] of the WebCrypto API specification changed the type of the deriveBits's length argument to become 'optional' and with 'null' as default value. The affected WebCrypto algorithms (HKDF, PBKDF2, ECDH and X25519) will be adapted to handle the case of a null length properly. [1] w3c/webcrypto#345 Differential Revision: https://phabricator.services.mozilla.com/D217532
1 parent 7c81dcc commit 83c38a9

File tree

7 files changed

+50
-93
lines changed

7 files changed

+50
-93
lines changed

‎dom/base/SubtleCrypto.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ already_AddRefed<Promise> SubtleCrypto::DeriveKey(
102102

103103
already_AddRefed<Promise> SubtleCrypto::DeriveBits(
104104
JSContext* cx, const ObjectOrString& algorithm, CryptoKey& baseKey,
105-
uint32_t length, ErrorResult& aRv){
105+
const Nullable<uint32_t>& length, ErrorResult& aRv){
106106
SUBTLECRYPTO_METHOD_BODY(DeriveBits, aRv, cx, algorithm, baseKey, length)}
107107

108108
already_AddRefed<Promise> SubtleCrypto::WrapKey(

‎dom/base/SubtleCrypto.h

+2-1
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,8 @@ class SubtleCrypto final : public nsISupports, public nsWrapperCache {
8787

8888
already_AddRefed<Promise> DeriveBits(JSContext* cx,
8989
const ObjectOrString& algorithm,
90-
CryptoKey& baseKey, uint32_t length,
90+
CryptoKey& baseKey,
91+
const Nullable<uint32_t>& length,
9192
ErrorResult& aRv);
9293

9394
already_AddRefed<Promise> WrapKey(JSContext* cx, const nsAString& format,

‎dom/crypto/WebCryptoTask.cpp

+44-36
Original file line numberDiff line numberDiff line change
@@ -2481,8 +2481,8 @@ class GenerateSymmetricKeyTask : public WebCryptoTask {
24812481
class DeriveX25519BitsTask : public ReturnArrayBufferViewTask {
24822482
public:
24832483
DeriveX25519BitsTask(JSContext* aCx, const ObjectOrString& aAlgorithm,
2484-
CryptoKey& aKey, uint32_t aLength)
2485-
: mLength(Some(aLength)), mPrivKey(aKey.GetPrivateKey()) {
2484+
CryptoKey& aKey, const Nullable<uint32_t>& aLength)
2485+
: mLength(aLength), mPrivKey(aKey.GetPrivateKey()) {
24862486
Init(aCx, aAlgorithm, aKey);
24872487
}
24882488

@@ -2504,12 +2504,12 @@ class DeriveX25519BitsTask : public ReturnArrayBufferViewTask {
25042504

25052505
// If specified, length must be a multiple of 8 bigger than zero
25062506
// (otherwise, the full output of the key derivation is used).
2507-
if (mLength) {
2508-
if (*mLength == 0 || *mLength % 8) {
2507+
if (!mLength.IsNull()) {
2508+
if (mLength.Value() == 0 || mLength.Value() % 8) {
25092509
mEarlyRv = NS_ERROR_DOM_DATA_ERR;
25102510
return;
25112511
}
2512-
*mLength = *mLength >> 3; // bits to bytes
2512+
mLength.SetValue(mLength.Value() >> 3); // bits to bytes
25132513
}
25142514

25152515
// Retrieve the peer's public key.
@@ -2532,7 +2532,7 @@ class DeriveX25519BitsTask : public ReturnArrayBufferViewTask {
25322532
}
25332533

25342534
private:
2535-
Maybe<size_t> mLength;
2535+
Nullable<uint32_t> mLength;
25362536
UniqueSECKEYPrivateKey mPrivKey;
25372537
UniqueSECKEYPublicKey mPubKey;
25382538

@@ -2563,11 +2563,11 @@ class DeriveX25519BitsTask : public ReturnArrayBufferViewTask {
25632563
// data, so mResult manages one copy, while symKey manages another.
25642564
ATTEMPT_BUFFER_ASSIGN(mResult, PK11_GetKeyData(symKey.get()));
25652565

2566-
if (mLength) {
2567-
if (*mLength > mResult.Length()) {
2566+
if (!mLength.IsNull()) {
2567+
if (mLength.Value() > mResult.Length()) {
25682568
return NS_ERROR_DOM_OPERATION_ERR;
25692569
}
2570-
if (!mResult.SetLength(*mLength, fallible)) {
2570+
if (!mResult.SetLength(mLength.Value(), fallible)) {
25712571
return NS_ERROR_DOM_UNKNOWN_ERR;
25722572
}
25732573
}
@@ -2785,7 +2785,7 @@ void GenerateAsymmetricKeyTask::Cleanup() { mKeyPair = nullptr; }
27852785
class DeriveHkdfBitsTask : public ReturnArrayBufferViewTask {
27862786
public:
27872787
DeriveHkdfBitsTask(JSContext* aCx, const ObjectOrString& aAlgorithm,
2788-
CryptoKey& aKey, uint32_t aLength)
2788+
CryptoKey& aKey, const Nullable<uint32_t>& aLength)
27892789
: mMechanism(CKM_INVALID_MECHANISM) {
27902790
Init(aCx, aAlgorithm, aKey, aLength);
27912791
}
@@ -2796,13 +2796,14 @@ class DeriveHkdfBitsTask : public ReturnArrayBufferViewTask {
27962796
size_t length;
27972797
mEarlyRv = GetKeyLengthForAlgorithm(aCx, aTargetAlgorithm, length);
27982798

2799+
const Nullable<uint32_t> keyLength(length);
27992800
if (NS_SUCCEEDED(mEarlyRv)) {
2800-
Init(aCx, aAlgorithm, aKey, length);
2801+
Init(aCx, aAlgorithm, aKey, keyLength);
28012802
}
28022803
}
28032804

28042805
void Init(JSContext* aCx, const ObjectOrString& aAlgorithm, CryptoKey& aKey,
2805-
uint32_t aLength) {
2806+
const Nullable<uint32_t>& aLength) {
28062807
Telemetry::Accumulate(Telemetry::WEBCRYPTO_ALG, TA_HKDF);
28072808
CHECK_KEY_ALGORITHM(aKey.Algorithm(), WEBCRYPTO_ALG_HKDF);
28082809

@@ -2818,8 +2819,8 @@ class DeriveHkdfBitsTask : public ReturnArrayBufferViewTask {
28182819
return;
28192820
}
28202821

2821-
// length must be greater than zero and multiple of eight.
2822-
if (aLength == 0 || aLength % 8 != 0) {
2822+
// length must be non-null and greater than zero and multiple of eight.
2823+
if (aLength.IsNull() || aLength.Value() == 0 || aLength.Value() % 8 != 0) {
28232824
mEarlyRv = NS_ERROR_DOM_OPERATION_ERR;
28242825
return;
28252826
}
@@ -2852,8 +2853,8 @@ class DeriveHkdfBitsTask : public ReturnArrayBufferViewTask {
28522853

28532854
ATTEMPT_BUFFER_INIT(mSalt, params.mSalt)
28542855
ATTEMPT_BUFFER_INIT(mInfo, params.mInfo)
2855-
mLengthInBytes = ceil((double)aLength / 8);
2856-
mLengthInBits = aLength;
2856+
mLengthInBytes = ceil((double)aLength.Value() / 8);
2857+
mLengthInBits = aLength.Value();
28572858
}
28582859

28592860
private:
@@ -2931,7 +2932,7 @@ class DeriveHkdfBitsTask : public ReturnArrayBufferViewTask {
29312932
class DerivePbkdfBitsTask : public ReturnArrayBufferViewTask {
29322933
public:
29332934
DerivePbkdfBitsTask(JSContext* aCx, const ObjectOrString& aAlgorithm,
2934-
CryptoKey& aKey, uint32_t aLength)
2935+
CryptoKey& aKey, const Nullable<uint32_t>& aLength)
29352936
: mHashOidTag(SEC_OID_UNKNOWN) {
29362937
Init(aCx, aAlgorithm, aKey, aLength);
29372938
}
@@ -2942,13 +2943,14 @@ class DerivePbkdfBitsTask : public ReturnArrayBufferViewTask {
29422943
size_t length;
29432944
mEarlyRv = GetKeyLengthForAlgorithm(aCx, aTargetAlgorithm, length);
29442945

2946+
const Nullable<uint32_t> keyLength(length);
29452947
if (NS_SUCCEEDED(mEarlyRv)) {
2946-
Init(aCx, aAlgorithm, aKey, length);
2948+
Init(aCx, aAlgorithm, aKey, keyLength);
29472949
}
29482950
}
29492951

29502952
void Init(JSContext* aCx, const ObjectOrString& aAlgorithm, CryptoKey& aKey,
2951-
uint32_t aLength) {
2953+
const Nullable<uint32_t>& aLength) {
29522954
Telemetry::Accumulate(Telemetry::WEBCRYPTO_ALG, TA_PBKDF2);
29532955
CHECK_KEY_ALGORITHM(aKey.Algorithm(), WEBCRYPTO_ALG_PBKDF2);
29542956

@@ -2964,8 +2966,8 @@ class DerivePbkdfBitsTask : public ReturnArrayBufferViewTask {
29642966
return;
29652967
}
29662968

2967-
// length must be a multiple of 8 bigger than zero.
2968-
if (aLength == 0 || aLength % 8) {
2969+
// length must be non-null and greater than zero and multiple of eight.
2970+
if (aLength.IsNull() || aLength.Value() == 0 || aLength.Value() % 8) {
29692971
mEarlyRv = NS_ERROR_DOM_OPERATION_ERR;
29702972
return;
29712973
}
@@ -2997,7 +2999,7 @@ class DerivePbkdfBitsTask : public ReturnArrayBufferViewTask {
29972999
}
29983000

29993001
ATTEMPT_BUFFER_INIT(mSalt, params.mSalt)
3000-
mLength = aLength >> 3; // bits to bytes
3002+
mLength = aLength.Value() >> 3; // bits to bytes
30013003
mIterations = params.mIterations;
30023004
}
30033005

@@ -3101,16 +3103,22 @@ class DeriveKeyTask : public DeriveBitsTask {
31013103
class DeriveEcdhBitsTask : public ReturnArrayBufferViewTask {
31023104
public:
31033105
DeriveEcdhBitsTask(JSContext* aCx, const ObjectOrString& aAlgorithm,
3104-
CryptoKey& aKey, uint32_t aLength)
3105-
: mLengthInBits(Some(aLength)), mPrivKey(aKey.GetPrivateKey()) {
3106+
CryptoKey& aKey, const Nullable<uint32_t>& aLength)
3107+
: mLengthInBits(aLength), mPrivKey(aKey.GetPrivateKey()) {
31063108
Init(aCx, aAlgorithm, aKey);
31073109
}
31083110

31093111
DeriveEcdhBitsTask(JSContext* aCx, const ObjectOrString& aAlgorithm,
31103112
CryptoKey& aKey, const ObjectOrString& aTargetAlgorithm)
31113113
: mPrivKey(aKey.GetPrivateKey()) {
3114+
Maybe<size_t> lengthInBits;
31123115
mEarlyRv = GetKeyLengthForAlgorithmIfSpecified(aCx, aTargetAlgorithm,
3113-
mLengthInBits);
3116+
lengthInBits);
3117+
if (lengthInBits.isNothing()) {
3118+
mLengthInBits.SetNull();
3119+
} else {
3120+
mLengthInBits.SetValue(*lengthInBits);
3121+
}
31143122
if (NS_SUCCEEDED(mEarlyRv)) {
31153123
Init(aCx, aAlgorithm, aKey);
31163124
}
@@ -3128,8 +3136,8 @@ class DeriveEcdhBitsTask : public ReturnArrayBufferViewTask {
31283136

31293137
// If specified, length must be bigger than zero
31303138
// (otherwise, the full output of the key derivation is used).
3131-
if (mLengthInBits) {
3132-
if (*mLengthInBits == 0) {
3139+
if (!mLengthInBits.IsNull()) {
3140+
if (mLengthInBits.Value() == 0) {
31333141
mEarlyRv = NS_ERROR_DOM_DATA_ERR;
31343142
return;
31353143
}
@@ -3163,7 +3171,7 @@ class DeriveEcdhBitsTask : public ReturnArrayBufferViewTask {
31633171
}
31643172

31653173
private:
3166-
Maybe<size_t> mLengthInBits;
3174+
Nullable<uint32_t> mLengthInBits;
31673175
UniqueSECKEYPrivateKey mPrivKey;
31683176
UniqueSECKEYPublicKey mPubKey;
31693177

@@ -3189,21 +3197,21 @@ class DeriveEcdhBitsTask : public ReturnArrayBufferViewTask {
31893197
// data, so mResult manages one copy, while symKey manages another.
31903198
ATTEMPT_BUFFER_ASSIGN(mResult, PK11_GetKeyData(symKey.get()));
31913199

3192-
if (mLengthInBits) {
3193-
size_t mLengthInBytes =
3194-
ceil((double)*mLengthInBits / 8); // bits to bytes
3195-
if (mLengthInBytes > mResult.Length()) {
3200+
if (!mLengthInBits.IsNull()) {
3201+
size_t length = mLengthInBits.Value();
3202+
size_t lengthInBytes = ceil((double)length / 8); // bits to bytes
3203+
if (lengthInBytes > mResult.Length()) {
31963204
return NS_ERROR_DOM_OPERATION_ERR;
31973205
}
31983206

3199-
if (!mResult.SetLength(mLengthInBytes, fallible)) {
3207+
if (!mResult.SetLength(lengthInBytes, fallible)) {
32003208
return NS_ERROR_DOM_UNKNOWN_ERR;
32013209
}
32023210

32033211
// If the number of bits to derive is not a multiple of 8 we need to
32043212
// zero out the remaining bits that were derived but not requested.
3205-
if (*mLengthInBits % 8) {
3206-
mResult[mResult.Length() - 1] &= 0xff << (8 - (*mLengthInBits % 8));
3213+
if (length % 8) {
3214+
mResult[mResult.Length() - 1] &= 0xff << (8 - (length % 8));
32073215
}
32083216
}
32093217

@@ -3558,7 +3566,7 @@ WebCryptoTask* WebCryptoTask::CreateDeriveKeyTask(
35583566

35593567
WebCryptoTask* WebCryptoTask::CreateDeriveBitsTask(
35603568
JSContext* aCx, const ObjectOrString& aAlgorithm, CryptoKey& aKey,
3561-
uint32_t aLength) {
3569+
const Nullable<uint32_t>& aLength) {
35623570
Telemetry::Accumulate(Telemetry::WEBCRYPTO_METHOD, TM_DERIVEBITS);
35633571

35643572
// Ensure baseKey is usable for this operation

‎dom/crypto/WebCryptoTask.h

+2-1
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,8 @@ class WebCryptoTask : public CancelableRunnable {
126126
const Sequence<nsString>& aKeyUsages);
127127
static WebCryptoTask* CreateDeriveBitsTask(JSContext* aCx,
128128
const ObjectOrString& aAlgorithm,
129-
CryptoKey& aKey, uint32_t aLength);
129+
CryptoKey& aKey,
130+
const Nullable<uint32_t>& aLength);
130131

131132
static WebCryptoTask* CreateWrapKeyTask(JSContext* aCx,
132133
const nsAString& aFormat,

‎dom/webidl/SubtleCrypto.webidl

+1-1
Original file line numberDiff line numberDiff line change
@@ -217,7 +217,7 @@ interface SubtleCrypto {
217217
[NewObject]
218218
Promise<any> deriveBits(AlgorithmIdentifier algorithm,
219219
CryptoKey baseKey,
220-
unsigned long length);
220+
optional unsigned long? length = null);
221221

222222
[NewObject]
223223
Promise<any> importKey(KeyFormat format,

‎testing/web-platform/meta/WebCryptoAPI/derive_bits_keys/derived_bits_length.https.any.js.ini

-48
Original file line numberDiff line numberDiff line change
@@ -1,62 +1,14 @@
11
[derived_bits_length.https.any.html]
2-
[HKDF derivation with omitted as 'length' parameter]
3-
expected: FAIL
4-
5-
[PBKDF2 derivation with omitted as 'length' parameter]
6-
expected: FAIL
7-
82
[ECDH derivation with 0 as 'length' parameter]
93
expected: FAIL
104

11-
[ECDH derivation with null as 'length' parameter]
12-
expected: FAIL
13-
14-
[ECDH derivation with undefined as 'length' parameter]
15-
expected: FAIL
16-
17-
[ECDH derivation with omitted as 'length' parameter]
18-
expected: FAIL
19-
205
[X25519 derivation with 0 as 'length' parameter]
216
expected: FAIL
227

23-
[X25519 derivation with null as 'length' parameter]
24-
expected: FAIL
25-
26-
[X25519 derivation with undefined as 'length' parameter]
27-
expected: FAIL
28-
29-
[X25519 derivation with omitted as 'length' parameter]
30-
expected: FAIL
31-
328

339
[derived_bits_length.https.any.worker.html]
34-
[HKDF derivation with omitted as 'length' parameter]
35-
expected: FAIL
36-
37-
[PBKDF2 derivation with omitted as 'length' parameter]
38-
expected: FAIL
39-
4010
[ECDH derivation with 0 as 'length' parameter]
4111
expected: FAIL
4212

43-
[ECDH derivation with null as 'length' parameter]
44-
expected: FAIL
45-
46-
[ECDH derivation with undefined as 'length' parameter]
47-
expected: FAIL
48-
49-
[ECDH derivation with omitted as 'length' parameter]
50-
expected: FAIL
51-
5213
[X25519 derivation with 0 as 'length' parameter]
5314
expected: FAIL
54-
55-
[X25519 derivation with null as 'length' parameter]
56-
expected: FAIL
57-
58-
[X25519 derivation with undefined as 'length' parameter]
59-
expected: FAIL
60-
61-
[X25519 derivation with omitted as 'length' parameter]
62-
expected: FAIL

‎testing/web-platform/meta/WebCryptoAPI/idlharness.https.any.js.ini

-5
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,7 @@
11
[idlharness.https.any.worker.html]
22
expected:
33
if (os == "android") and fission: [OK, TIMEOUT]
4-
[SubtleCrypto interface: operation deriveBits(AlgorithmIdentifier, CryptoKey, optional unsigned long?)]
5-
expected: FAIL
6-
74

85
[idlharness.https.any.html]
96
expected:
107
if (os == "android") and fission: [OK, TIMEOUT]
11-
[SubtleCrypto interface: operation deriveBits(AlgorithmIdentifier, CryptoKey, optional unsigned long?)]
12-
expected: FAIL

0 commit comments

Comments
 (0)