Skip to content

Commit

Permalink
Bug 1793429 - Make the deriveBits's length argument Nullable r=keeler…
Browse files Browse the repository at this point in the history 
…,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
  • Loading branch information
@javifernandez
javifernandez committed Aug 2, 2024
1 parent 3b71eca commit 51dc067
Show file tree
Hide file tree
Showing 6 changed files with 46 additions and 86 deletions.
2 changes: 1 addition & 1 deletion dom/base/SubtleCrypto.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ already_AddRefed<Promise> SubtleCrypto::DeriveKey(

already_AddRefed<Promise> SubtleCrypto::DeriveBits(
JSContext* cx, const ObjectOrString& algorithm, CryptoKey& baseKey,
uint32_t length, ErrorResult& aRv){
const Nullable<uint32_t>& length, ErrorResult& aRv){
SUBTLECRYPTO_METHOD_BODY(DeriveBits, aRv, cx, algorithm, baseKey, length)}

already_AddRefed<Promise> SubtleCrypto::WrapKey(
Expand Down
3 changes: 2 additions & 1 deletion dom/base/SubtleCrypto.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,8 @@ class SubtleCrypto final : public nsISupports, public nsWrapperCache {

already_AddRefed<Promise> DeriveBits(JSContext* cx,
const ObjectOrString& algorithm,
CryptoKey& baseKey, uint32_t length,
CryptoKey& baseKey,
const Nullable<uint32_t>& length,
ErrorResult& aRv);

already_AddRefed<Promise> WrapKey(JSContext* cx, const nsAString& format,
Expand Down
74 changes: 40 additions & 34 deletions dom/crypto/WebCryptoTask.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2481,8 +2481,8 @@ class GenerateSymmetricKeyTask : public WebCryptoTask {
class DeriveX25519BitsTask : public ReturnArrayBufferViewTask {
public:
DeriveX25519BitsTask(JSContext* aCx, const ObjectOrString& aAlgorithm,
CryptoKey& aKey, uint32_t aLength)
: mLength(Some(aLength)), mPrivKey(aKey.GetPrivateKey()) {
CryptoKey& aKey, const Nullable<uint32_t>& aLength)
: mLength(aLength), mPrivKey(aKey.GetPrivateKey()) {
Init(aCx, aAlgorithm, aKey);
}

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

// If specified, length must be a multiple of 8 bigger than zero
// (otherwise, the full output of the key derivation is used).
if (mLength) {
if (*mLength == 0 || *mLength % 8) {
if (!mLength.IsNull()) {
if (mLength.Value() == 0 || mLength.Value() % 8) {
mEarlyRv = NS_ERROR_DOM_DATA_ERR;
return;
}
*mLength = *mLength >> 3; // bits to bytes
mLength.SetValue(mLength.Value() >> 3); // bits to bytes
}

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

private:
Maybe<size_t> mLength;
Nullable<uint32_t> mLength;
UniqueSECKEYPrivateKey mPrivKey;
UniqueSECKEYPublicKey mPubKey;

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

if (mLength) {
if (*mLength > mResult.Length()) {
if (!mLength.IsNull()) {
if (mLength.Value() > mResult.Length()) {
return NS_ERROR_DOM_OPERATION_ERR;
}
if (!mResult.SetLength(*mLength, fallible)) {
if (!mResult.SetLength(mLength.Value(), fallible)) {
return NS_ERROR_DOM_UNKNOWN_ERR;
}
}
Expand Down Expand Up @@ -2785,7 +2785,7 @@ void GenerateAsymmetricKeyTask::Cleanup() { mKeyPair = nullptr; }
class DeriveHkdfBitsTask : public ReturnArrayBufferViewTask {
public:
DeriveHkdfBitsTask(JSContext* aCx, const ObjectOrString& aAlgorithm,
CryptoKey& aKey, uint32_t aLength)
CryptoKey& aKey, const Nullable<uint32_t>& aLength)
: mMechanism(CKM_INVALID_MECHANISM) {
Init(aCx, aAlgorithm, aKey, aLength);
}
Expand All @@ -2802,7 +2802,7 @@ class DeriveHkdfBitsTask : public ReturnArrayBufferViewTask {
}

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

Expand All @@ -2818,8 +2818,8 @@ class DeriveHkdfBitsTask : public ReturnArrayBufferViewTask {
return;
}

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

ATTEMPT_BUFFER_INIT(mSalt, params.mSalt)
ATTEMPT_BUFFER_INIT(mInfo, params.mInfo)
mLengthInBytes = ceil((double)aLength / 8);
mLengthInBits = aLength;
mLengthInBytes = ceil((double)aLength.Value() / 8);
mLengthInBits = aLength.Value();
}

private:
Expand Down Expand Up @@ -2931,7 +2931,7 @@ class DeriveHkdfBitsTask : public ReturnArrayBufferViewTask {
class DerivePbkdfBitsTask : public ReturnArrayBufferViewTask {
public:
DerivePbkdfBitsTask(JSContext* aCx, const ObjectOrString& aAlgorithm,
CryptoKey& aKey, uint32_t aLength)
CryptoKey& aKey, Nullable<uint32_t> aLength)
: mHashOidTag(SEC_OID_UNKNOWN) {
Init(aCx, aAlgorithm, aKey, aLength);
}
Expand All @@ -2948,7 +2948,7 @@ class DerivePbkdfBitsTask : public ReturnArrayBufferViewTask {
}

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

Expand All @@ -2964,8 +2964,8 @@ class DerivePbkdfBitsTask : public ReturnArrayBufferViewTask {
return;
}

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

ATTEMPT_BUFFER_INIT(mSalt, params.mSalt)
mLength = aLength >> 3; // bits to bytes
mLength = aLength.Value() >> 3; // bits to bytes
mIterations = params.mIterations;
}

Expand Down Expand Up @@ -3101,16 +3101,22 @@ class DeriveKeyTask : public DeriveBitsTask {
class DeriveEcdhBitsTask : public ReturnArrayBufferViewTask {
public:
DeriveEcdhBitsTask(JSContext* aCx, const ObjectOrString& aAlgorithm,
CryptoKey& aKey, uint32_t aLength)
: mLengthInBits(Some(aLength)), mPrivKey(aKey.GetPrivateKey()) {
CryptoKey& aKey, const Nullable<uint32_t>& aLength)
: mLengthInBits(aLength), mPrivKey(aKey.GetPrivateKey()) {
Init(aCx, aAlgorithm, aKey);
}

DeriveEcdhBitsTask(JSContext* aCx, const ObjectOrString& aAlgorithm,
CryptoKey& aKey, const ObjectOrString& aTargetAlgorithm)
: mPrivKey(aKey.GetPrivateKey()) {
Maybe<size_t> lengthInBits;
mEarlyRv = GetKeyLengthForAlgorithmIfSpecified(aCx, aTargetAlgorithm,
mLengthInBits);
lengthInBits);
if (lengthInBits.isNothing()) {
mLengthInBits.SetNull();
} else {
mLengthInBits.SetValue(*lengthInBits);
}
if (NS_SUCCEEDED(mEarlyRv)) {
Init(aCx, aAlgorithm, aKey);
}
Expand All @@ -3128,8 +3134,8 @@ class DeriveEcdhBitsTask : public ReturnArrayBufferViewTask {

// If specified, length must be bigger than zero
// (otherwise, the full output of the key derivation is used).
if (mLengthInBits) {
if (*mLengthInBits == 0) {
if (!mLengthInBits.IsNull()) {
if (mLengthInBits.Value() == 0) {
mEarlyRv = NS_ERROR_DOM_DATA_ERR;
return;
}
Expand Down Expand Up @@ -3163,7 +3169,7 @@ class DeriveEcdhBitsTask : public ReturnArrayBufferViewTask {
}

private:
Maybe<size_t> mLengthInBits;
Nullable<uint32_t> mLengthInBits;
UniqueSECKEYPrivateKey mPrivKey;
UniqueSECKEYPublicKey mPubKey;

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

if (mLengthInBits) {
size_t mLengthInBytes =
ceil((double)*mLengthInBits / 8); // bits to bytes
if (mLengthInBytes > mResult.Length()) {
if (!mLengthInBits.IsNull()) {
size_t length = mLengthInBits.Value();
size_t lengthInBytes = ceil((double)length / 8); // bits to bytes
if (lengthInBytes > mResult.Length()) {
return NS_ERROR_DOM_OPERATION_ERR;
}

if (!mResult.SetLength(mLengthInBytes, fallible)) {
if (!mResult.SetLength(lengthInBytes, fallible)) {
return NS_ERROR_DOM_UNKNOWN_ERR;
}

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

Expand Down Expand Up @@ -3558,7 +3564,7 @@ WebCryptoTask* WebCryptoTask::CreateDeriveKeyTask(

WebCryptoTask* WebCryptoTask::CreateDeriveBitsTask(
JSContext* aCx, const ObjectOrString& aAlgorithm, CryptoKey& aKey,
uint32_t aLength) {
const Nullable<uint32_t>& aLength) {
Telemetry::Accumulate(Telemetry::WEBCRYPTO_METHOD, TM_DERIVEBITS);

// Ensure baseKey is usable for this operation
Expand Down
3 changes: 2 additions & 1 deletion dom/crypto/WebCryptoTask.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,8 @@ class WebCryptoTask : public CancelableRunnable {
const Sequence<nsString>& aKeyUsages);
static WebCryptoTask* CreateDeriveBitsTask(JSContext* aCx,
const ObjectOrString& aAlgorithm,
CryptoKey& aKey, uint32_t aLength);
CryptoKey& aKey,
const Nullable<uint32_t>& aLength);

static WebCryptoTask* CreateWrapKeyTask(JSContext* aCx,
const nsAString& aFormat,
Expand Down
2 changes: 1 addition & 1 deletion dom/webidl/SubtleCrypto.webidl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ interface SubtleCrypto {
[NewObject]
Promise<any> deriveBits(AlgorithmIdentifier algorithm,
CryptoKey baseKey,
unsigned long length);
optional unsigned long? length = null);

[NewObject]
Promise<any> importKey(KeyFormat format,
Expand Down
48 changes: 0 additions & 48 deletions testing/web-platform/meta/WebCryptoAPI/derive_bits_keys/derived_bits_length.https.any.js.ini
View file
Original file line number Diff line number Diff line change
@@ -1,62 +1,14 @@
[derived_bits_length.https.any.html]
[HKDF derivation with omitted as 'length' parameter]
expected: FAIL

[PBKDF2 derivation with omitted as 'length' parameter]
expected: FAIL

[ECDH derivation with 0 as 'length' parameter]
expected: FAIL

[ECDH derivation with null as 'length' parameter]
expected: FAIL

[ECDH derivation with undefined as 'length' parameter]
expected: FAIL

[ECDH derivation with omitted as 'length' parameter]
expected: FAIL

[X25519 derivation with 0 as 'length' parameter]
expected: FAIL

[X25519 derivation with null as 'length' parameter]
expected: FAIL

[X25519 derivation with undefined as 'length' parameter]
expected: FAIL

[X25519 derivation with omitted as 'length' parameter]
expected: FAIL


[derived_bits_length.https.any.worker.html]
[HKDF derivation with omitted as 'length' parameter]
expected: FAIL

[PBKDF2 derivation with omitted as 'length' parameter]
expected: FAIL

[ECDH derivation with 0 as 'length' parameter]
expected: FAIL

[ECDH derivation with null as 'length' parameter]
expected: FAIL

[ECDH derivation with undefined as 'length' parameter]
expected: FAIL

[ECDH derivation with omitted as 'length' parameter]
expected: FAIL

[X25519 derivation with 0 as 'length' parameter]
expected: FAIL

[X25519 derivation with null as 'length' parameter]
expected: FAIL

[X25519 derivation with undefined as 'length' parameter]
expected: FAIL

[X25519 derivation with omitted as 'length' parameter]
expected: FAIL

0 comments on commit 51dc067

Please sign in to comment.