From 4a0e481032b2099c57f0a7582cddc86e9cc4ddd0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javier=20Fern=C3=A1ndez=20Garc=C3=ADa-Boente?= Date: Fri, 2 Aug 2024 07:43:30 +0000 Subject: [PATCH] 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] https://github.com/w3c/webcrypto/pull/345 Differential Revision: https://phabricator.services.mozilla.com/D217532 --- dom/base/SubtleCrypto.cpp | 2 +- dom/base/SubtleCrypto.h | 3 +- dom/crypto/WebCryptoTask.cpp | 74 ++++++++++--------- dom/crypto/WebCryptoTask.h | 3 +- dom/webidl/SubtleCrypto.webidl | 2 +- .../derived_bits_length.https.any.js.ini | 48 ------------ 6 files changed, 46 insertions(+), 86 deletions(-) diff --git a/dom/base/SubtleCrypto.cpp b/dom/base/SubtleCrypto.cpp index bfd6c1a84ee92..7cf1042712027 100644 --- a/dom/base/SubtleCrypto.cpp +++ b/dom/base/SubtleCrypto.cpp @@ -102,7 +102,7 @@ already_AddRefed SubtleCrypto::DeriveKey( already_AddRefed SubtleCrypto::DeriveBits( JSContext* cx, const ObjectOrString& algorithm, CryptoKey& baseKey, - uint32_t length, ErrorResult& aRv){ + const Nullable& length, ErrorResult& aRv){ SUBTLECRYPTO_METHOD_BODY(DeriveBits, aRv, cx, algorithm, baseKey, length)} already_AddRefed SubtleCrypto::WrapKey( diff --git a/dom/base/SubtleCrypto.h b/dom/base/SubtleCrypto.h index a4d93420c8405..da2212dc59533 100644 --- a/dom/base/SubtleCrypto.h +++ b/dom/base/SubtleCrypto.h @@ -87,7 +87,8 @@ class SubtleCrypto final : public nsISupports, public nsWrapperCache { already_AddRefed DeriveBits(JSContext* cx, const ObjectOrString& algorithm, - CryptoKey& baseKey, uint32_t length, + CryptoKey& baseKey, + const Nullable& length, ErrorResult& aRv); already_AddRefed WrapKey(JSContext* cx, const nsAString& format, diff --git a/dom/crypto/WebCryptoTask.cpp b/dom/crypto/WebCryptoTask.cpp index a23868be7d671..3307a0a84c0c2 100644 --- a/dom/crypto/WebCryptoTask.cpp +++ b/dom/crypto/WebCryptoTask.cpp @@ -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& aLength) + : mLength(aLength), mPrivKey(aKey.GetPrivateKey()) { Init(aCx, aAlgorithm, aKey); } @@ -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. @@ -2532,7 +2532,7 @@ class DeriveX25519BitsTask : public ReturnArrayBufferViewTask { } private: - Maybe mLength; + Nullable mLength; UniqueSECKEYPrivateKey mPrivKey; UniqueSECKEYPublicKey mPubKey; @@ -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; } } @@ -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& aLength) : mMechanism(CKM_INVALID_MECHANISM) { Init(aCx, aAlgorithm, aKey, aLength); } @@ -2802,7 +2802,7 @@ class DeriveHkdfBitsTask : public ReturnArrayBufferViewTask { } void Init(JSContext* aCx, const ObjectOrString& aAlgorithm, CryptoKey& aKey, - uint32_t aLength) { + Nullable aLength) { Telemetry::Accumulate(Telemetry::WEBCRYPTO_ALG, TA_HKDF); CHECK_KEY_ALGORITHM(aKey.Algorithm(), WEBCRYPTO_ALG_HKDF); @@ -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; } @@ -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: @@ -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 aLength) : mHashOidTag(SEC_OID_UNKNOWN) { Init(aCx, aAlgorithm, aKey, aLength); } @@ -2948,7 +2948,7 @@ class DerivePbkdfBitsTask : public ReturnArrayBufferViewTask { } void Init(JSContext* aCx, const ObjectOrString& aAlgorithm, CryptoKey& aKey, - uint32_t aLength) { + Nullable aLength) { Telemetry::Accumulate(Telemetry::WEBCRYPTO_ALG, TA_PBKDF2); CHECK_KEY_ALGORITHM(aKey.Algorithm(), WEBCRYPTO_ALG_PBKDF2); @@ -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; } @@ -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; } @@ -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& 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 lengthInBits; mEarlyRv = GetKeyLengthForAlgorithmIfSpecified(aCx, aTargetAlgorithm, - mLengthInBits); + lengthInBits); + if (lengthInBits.isNothing()) { + mLengthInBits.SetNull(); + } else { + mLengthInBits.SetValue(*lengthInBits); + } if (NS_SUCCEEDED(mEarlyRv)) { Init(aCx, aAlgorithm, aKey); } @@ -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; } @@ -3163,7 +3169,7 @@ class DeriveEcdhBitsTask : public ReturnArrayBufferViewTask { } private: - Maybe mLengthInBits; + Nullable mLengthInBits; UniqueSECKEYPrivateKey mPrivKey; UniqueSECKEYPublicKey mPubKey; @@ -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)); } } @@ -3558,7 +3564,7 @@ WebCryptoTask* WebCryptoTask::CreateDeriveKeyTask( WebCryptoTask* WebCryptoTask::CreateDeriveBitsTask( JSContext* aCx, const ObjectOrString& aAlgorithm, CryptoKey& aKey, - uint32_t aLength) { + const Nullable& aLength) { Telemetry::Accumulate(Telemetry::WEBCRYPTO_METHOD, TM_DERIVEBITS); // Ensure baseKey is usable for this operation diff --git a/dom/crypto/WebCryptoTask.h b/dom/crypto/WebCryptoTask.h index c8d6cdd402201..bb335877e4e21 100644 --- a/dom/crypto/WebCryptoTask.h +++ b/dom/crypto/WebCryptoTask.h @@ -126,7 +126,8 @@ class WebCryptoTask : public CancelableRunnable { const Sequence& aKeyUsages); static WebCryptoTask* CreateDeriveBitsTask(JSContext* aCx, const ObjectOrString& aAlgorithm, - CryptoKey& aKey, uint32_t aLength); + CryptoKey& aKey, + const Nullable& aLength); static WebCryptoTask* CreateWrapKeyTask(JSContext* aCx, const nsAString& aFormat, diff --git a/dom/webidl/SubtleCrypto.webidl b/dom/webidl/SubtleCrypto.webidl index 6c7842b1f245e..3c86c93c73227 100644 --- a/dom/webidl/SubtleCrypto.webidl +++ b/dom/webidl/SubtleCrypto.webidl @@ -217,7 +217,7 @@ interface SubtleCrypto { [NewObject] Promise deriveBits(AlgorithmIdentifier algorithm, CryptoKey baseKey, - unsigned long length); + optional unsigned long? length = null); [NewObject] Promise importKey(KeyFormat format, diff --git a/testing/web-platform/meta/WebCryptoAPI/derive_bits_keys/derived_bits_length.https.any.js.ini b/testing/web-platform/meta/WebCryptoAPI/derive_bits_keys/derived_bits_length.https.any.js.ini index ad6707be8f43f..03ab296bbb55d 100644 --- a/testing/web-platform/meta/WebCryptoAPI/derive_bits_keys/derived_bits_length.https.any.js.ini +++ b/testing/web-platform/meta/WebCryptoAPI/derive_bits_keys/derived_bits_length.https.any.js.ini @@ -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