Skip to content

Commit 51dc067

Browse files
committed
Bug 1793429 - Make the deriveBits's length argument Nullable r=keeler,webidl,smaug DONTBUILD
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 3b71eca commit 51dc067

File tree

6 files changed

+46
-86
lines changed

6 files changed

+46
-86
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

+40-34
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
}
@@ -2802,7 +2802,7 @@ class DeriveHkdfBitsTask : public ReturnArrayBufferViewTask {
28022802
}
28032803

28042804
void Init(JSContext* aCx, const ObjectOrString& aAlgorithm, CryptoKey& aKey,
2805-
uint32_t aLength) {
2805+
Nullable<uint32_t> aLength) {
28062806
Telemetry::Accumulate(Telemetry::WEBCRYPTO_ALG, TA_HKDF);
28072807
CHECK_KEY_ALGORITHM(aKey.Algorithm(), WEBCRYPTO_ALG_HKDF);
28082808

@@ -2818,8 +2818,8 @@ class DeriveHkdfBitsTask : public ReturnArrayBufferViewTask {
28182818
return;
28192819
}
28202820

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

28532853
ATTEMPT_BUFFER_INIT(mSalt, params.mSalt)
28542854
ATTEMPT_BUFFER_INIT(mInfo, params.mInfo)
2855-
mLengthInBytes = ceil((double)aLength / 8);
2856-
mLengthInBits = aLength;
2855+
mLengthInBytes = ceil((double)aLength.Value() / 8);
2856+
mLengthInBits = aLength.Value();
28572857
}
28582858

28592859
private:
@@ -2931,7 +2931,7 @@ class DeriveHkdfBitsTask : public ReturnArrayBufferViewTask {
29312931
class DerivePbkdfBitsTask : public ReturnArrayBufferViewTask {
29322932
public:
29332933
DerivePbkdfBitsTask(JSContext* aCx, const ObjectOrString& aAlgorithm,
2934-
CryptoKey& aKey, uint32_t aLength)
2934+
CryptoKey& aKey, Nullable<uint32_t> aLength)
29352935
: mHashOidTag(SEC_OID_UNKNOWN) {
29362936
Init(aCx, aAlgorithm, aKey, aLength);
29372937
}
@@ -2948,7 +2948,7 @@ class DerivePbkdfBitsTask : public ReturnArrayBufferViewTask {
29482948
}
29492949

29502950
void Init(JSContext* aCx, const ObjectOrString& aAlgorithm, CryptoKey& aKey,
2951-
uint32_t aLength) {
2951+
Nullable<uint32_t> aLength) {
29522952
Telemetry::Accumulate(Telemetry::WEBCRYPTO_ALG, TA_PBKDF2);
29532953
CHECK_KEY_ALGORITHM(aKey.Algorithm(), WEBCRYPTO_ALG_PBKDF2);
29542954

@@ -2964,8 +2964,8 @@ class DerivePbkdfBitsTask : public ReturnArrayBufferViewTask {
29642964
return;
29652965
}
29662966

2967-
// length must be a multiple of 8 bigger than zero.
2968-
if (aLength == 0 || aLength % 8) {
2967+
// length must be non-null and greater than zero and multiple of eight.
2968+
if (aLength.IsNull() || aLength.Value() == 0 || aLength.Value() % 8) {
29692969
mEarlyRv = NS_ERROR_DOM_OPERATION_ERR;
29702970
return;
29712971
}
@@ -2997,7 +2997,7 @@ class DerivePbkdfBitsTask : public ReturnArrayBufferViewTask {
29972997
}
29982998

29992999
ATTEMPT_BUFFER_INIT(mSalt, params.mSalt)
3000-
mLength = aLength >> 3; // bits to bytes
3000+
mLength = aLength.Value() >> 3; // bits to bytes
30013001
mIterations = params.mIterations;
30023002
}
30033003

@@ -3101,16 +3101,22 @@ class DeriveKeyTask : public DeriveBitsTask {
31013101
class DeriveEcdhBitsTask : public ReturnArrayBufferViewTask {
31023102
public:
31033103
DeriveEcdhBitsTask(JSContext* aCx, const ObjectOrString& aAlgorithm,
3104-
CryptoKey& aKey, uint32_t aLength)
3105-
: mLengthInBits(Some(aLength)), mPrivKey(aKey.GetPrivateKey()) {
3104+
CryptoKey& aKey, const Nullable<uint32_t>& aLength)
3105+
: mLengthInBits(aLength), mPrivKey(aKey.GetPrivateKey()) {
31063106
Init(aCx, aAlgorithm, aKey);
31073107
}
31083108

31093109
DeriveEcdhBitsTask(JSContext* aCx, const ObjectOrString& aAlgorithm,
31103110
CryptoKey& aKey, const ObjectOrString& aTargetAlgorithm)
31113111
: mPrivKey(aKey.GetPrivateKey()) {
3112+
Maybe<size_t> lengthInBits;
31123113
mEarlyRv = GetKeyLengthForAlgorithmIfSpecified(aCx, aTargetAlgorithm,
3113-
mLengthInBits);
3114+
lengthInBits);
3115+
if (lengthInBits.isNothing()) {
3116+
mLengthInBits.SetNull();
3117+
} else {
3118+
mLengthInBits.SetValue(*lengthInBits);
3119+
}
31143120
if (NS_SUCCEEDED(mEarlyRv)) {
31153121
Init(aCx, aAlgorithm, aKey);
31163122
}
@@ -3128,8 +3134,8 @@ class DeriveEcdhBitsTask : public ReturnArrayBufferViewTask {
31283134

31293135
// If specified, length must be bigger than zero
31303136
// (otherwise, the full output of the key derivation is used).
3131-
if (mLengthInBits) {
3132-
if (*mLengthInBits == 0) {
3137+
if (!mLengthInBits.IsNull()) {
3138+
if (mLengthInBits.Value() == 0) {
31333139
mEarlyRv = NS_ERROR_DOM_DATA_ERR;
31343140
return;
31353141
}
@@ -3163,7 +3169,7 @@ class DeriveEcdhBitsTask : public ReturnArrayBufferViewTask {
31633169
}
31643170

31653171
private:
3166-
Maybe<size_t> mLengthInBits;
3172+
Nullable<uint32_t> mLengthInBits;
31673173
UniqueSECKEYPrivateKey mPrivKey;
31683174
UniqueSECKEYPublicKey mPubKey;
31693175

@@ -3189,21 +3195,21 @@ class DeriveEcdhBitsTask : public ReturnArrayBufferViewTask {
31893195
// data, so mResult manages one copy, while symKey manages another.
31903196
ATTEMPT_BUFFER_ASSIGN(mResult, PK11_GetKeyData(symKey.get()));
31913197

3192-
if (mLengthInBits) {
3193-
size_t mLengthInBytes =
3194-
ceil((double)*mLengthInBits / 8); // bits to bytes
3195-
if (mLengthInBytes > mResult.Length()) {
3198+
if (!mLengthInBits.IsNull()) {
3199+
size_t length = mLengthInBits.Value();
3200+
size_t lengthInBytes = ceil((double)length / 8); // bits to bytes
3201+
if (lengthInBytes > mResult.Length()) {
31963202
return NS_ERROR_DOM_OPERATION_ERR;
31973203
}
31983204

3199-
if (!mResult.SetLength(mLengthInBytes, fallible)) {
3205+
if (!mResult.SetLength(lengthInBytes, fallible)) {
32003206
return NS_ERROR_DOM_UNKNOWN_ERR;
32013207
}
32023208

32033209
// If the number of bits to derive is not a multiple of 8 we need to
32043210
// zero out the remaining bits that were derived but not requested.
3205-
if (*mLengthInBits % 8) {
3206-
mResult[mResult.Length() - 1] &= 0xff << (8 - (*mLengthInBits % 8));
3211+
if (length % 8) {
3212+
mResult[mResult.Length() - 1] &= 0xff << (8 - (length % 8));
32073213
}
32083214
}
32093215

@@ -3558,7 +3564,7 @@ WebCryptoTask* WebCryptoTask::CreateDeriveKeyTask(
35583564

35593565
WebCryptoTask* WebCryptoTask::CreateDeriveBitsTask(
35603566
JSContext* aCx, const ObjectOrString& aAlgorithm, CryptoKey& aKey,
3561-
uint32_t aLength) {
3567+
const Nullable<uint32_t>& aLength) {
35623568
Telemetry::Accumulate(Telemetry::WEBCRYPTO_METHOD, TM_DERIVEBITS);
35633569

35643570
// 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,
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

0 commit comments

Comments
 (0)