LibWeb: Correct WebCryptoAPI derive length behaviour

Fixes multiple slightly wrong behaviours of the `deriveBits` method
across various algorithms. Some of them might be due to a spec update.

Add tests related to fixes.
This commit is contained in:
devgianlu 2024-12-17 16:22:09 +01:00 committed by Jelle Raaijmakers
parent 4b87467fc2
commit f3a9bb0a91
Notes: github-actions[bot] 2024-12-18 12:19:39 +00:00
9 changed files with 217 additions and 27 deletions

View file

@ -3734,7 +3734,10 @@ WebIDL::ExceptionOr<GC::Ref<JS::ArrayBuffer>> ECDH::derive_bits(AlgorithmParams
auto secret_data = maybe_secret_data.release_value();
VERIFY(secret_data[0] == 0x04);
secret = TRY_OR_THROW_OOM(realm.vm(), secret_data.slice(1, secret_data.size() - 1));
// NOTE: Use the x-coordinate as shared secret. RFC6090 section 4.2:
// In the ECDH key exchange protocol, after the element g^(j*k) has been
// computed, the x-coordinate of that value can be used as the shared secret.
secret = TRY_OR_THROW_OOM(realm.vm(), secret_data.slice(1, secret_data.size() / 2));
} else {
// If the namedCurve property of the [[algorithm]] internal slot of key is a value specified
// in an applicable specification that specifies the use of that value with ECDH:
@ -3747,7 +3750,6 @@ WebIDL::ExceptionOr<GC::Ref<JS::ArrayBuffer>> ECDH::derive_bits(AlgorithmParams
// 8. If length is null: Return secret
if (!length_optional.has_value()) {
auto result = TRY_OR_THROW_OOM(realm.vm(), ByteBuffer::copy(secret));
return JS::ArrayBuffer::create(realm, move(secret));
}
@ -3757,7 +3759,12 @@ WebIDL::ExceptionOr<GC::Ref<JS::ArrayBuffer>> ECDH::derive_bits(AlgorithmParams
return WebIDL::OperationError::create(realm, "Secret is too short"_string);
// Otherwise: Return an octet string containing the first length bits of secret.
auto slice = TRY_OR_THROW_OOM(realm.vm(), secret.slice(0, length / 8));
auto slice = TRY_OR_THROW_OOM(realm.vm(), secret.slice(0, ceil_div(length, 8)));
if (length % 8 != 0) {
// Zero out the last bits
slice[slice.size() - 1] &= 0xFF << (8 - (length % 8));
}
return JS::ArrayBuffer::create(realm, move(slice));
}
@ -4938,11 +4945,9 @@ WebIDL::ExceptionOr<GC::Ref<JS::ArrayBuffer>> HKDF::derive_bits(AlgorithmParams
auto& realm = *m_realm;
auto const& normalized_algorithm = static_cast<HKDFParams const&>(params);
// 1. If length is null or zero, or is not a multiple of 8, then throw an OperationError.
auto length = length_optional.value_or(0);
if (length == 0 || length % 8 != 0)
return WebIDL::OperationError::create(realm, "Length must be greater than 0 and divisible by 8"_string);
// 1. If length is null or is not a multiple of 8, then throw an OperationError.
if (!length_optional.has_value() || *length_optional % 8 != 0)
return WebIDL::OperationError::create(realm, "Length must be specified and divisible by 8"_string);
// 2. Let keyDerivationKey be the secret represented by [[handle]] internal slot of key as the message.
auto key_derivation_key = key->handle().get<ByteBuffer>();
@ -4960,13 +4965,13 @@ WebIDL::ExceptionOr<GC::Ref<JS::ArrayBuffer>> HKDF::derive_bits(AlgorithmParams
auto const& hash_algorithm = TRY(normalized_algorithm.hash.name(realm.vm()));
ErrorOr<ByteBuffer> result = Error::from_string_literal("noop error");
if (hash_algorithm == "SHA-1") {
result = ::Crypto::Hash::HKDF<::Crypto::Hash::SHA1>::derive_key(Optional<ReadonlyBytes>(normalized_algorithm.salt), key_derivation_key, normalized_algorithm.info, length / 8);
result = ::Crypto::Hash::HKDF<::Crypto::Hash::SHA1>::derive_key(Optional<ReadonlyBytes>(normalized_algorithm.salt), key_derivation_key, normalized_algorithm.info, *length_optional / 8);
} else if (hash_algorithm == "SHA-256") {
result = ::Crypto::Hash::HKDF<::Crypto::Hash::SHA256>::derive_key(Optional<ReadonlyBytes>(normalized_algorithm.salt), key_derivation_key, normalized_algorithm.info, length / 8);
result = ::Crypto::Hash::HKDF<::Crypto::Hash::SHA256>::derive_key(Optional<ReadonlyBytes>(normalized_algorithm.salt), key_derivation_key, normalized_algorithm.info, *length_optional / 8);
} else if (hash_algorithm == "SHA-384") {
result = ::Crypto::Hash::HKDF<::Crypto::Hash::SHA384>::derive_key(Optional<ReadonlyBytes>(normalized_algorithm.salt), key_derivation_key, normalized_algorithm.info, length / 8);
result = ::Crypto::Hash::HKDF<::Crypto::Hash::SHA384>::derive_key(Optional<ReadonlyBytes>(normalized_algorithm.salt), key_derivation_key, normalized_algorithm.info, *length_optional / 8);
} else if (hash_algorithm == "SHA-512") {
result = ::Crypto::Hash::HKDF<::Crypto::Hash::SHA512>::derive_key(Optional<ReadonlyBytes>(normalized_algorithm.salt), key_derivation_key, normalized_algorithm.info, length / 8);
result = ::Crypto::Hash::HKDF<::Crypto::Hash::SHA512>::derive_key(Optional<ReadonlyBytes>(normalized_algorithm.salt), key_derivation_key, normalized_algorithm.info, *length_optional / 8);
} else {
return WebIDL::NotSupportedError::create(m_realm, MUST(String::formatted("Invalid hash function '{}'", hash_algorithm)));
}
@ -4991,9 +4996,8 @@ WebIDL::ExceptionOr<GC::Ref<JS::ArrayBuffer>> PBKDF2::derive_bits(AlgorithmParam
auto& realm = *m_realm;
auto const& normalized_algorithm = static_cast<PBKDF2Params const&>(params);
// 1. If length is null or zero, or is not a multiple of 8, then throw an OperationError.
auto length = length_optional.value_or(0);
if (length == 0 || length % 8 != 0)
// 1. If length is null or is not a multiple of 8, then throw an OperationError.
if (!length_optional.has_value() || *length_optional % 8 != 0)
return WebIDL::OperationError::create(realm, "Length must be greater than 0 and divisible by 8"_string);
// 2. If the iterations member of normalizedAlgorithm is zero, then throw an OperationError.
@ -5015,7 +5019,7 @@ WebIDL::ExceptionOr<GC::Ref<JS::ArrayBuffer>> PBKDF2::derive_bits(AlgorithmParam
auto salt = normalized_algorithm.salt;
auto iterations = normalized_algorithm.iterations;
auto derived_key_length_bytes = length / 8;
auto derived_key_length_bytes = *length_optional / 8;
if (hash_algorithm == "SHA-1") {
result = ::Crypto::Hash::PBKDF2::derive_key<::Crypto::Authentication::HMAC<::Crypto::Hash::SHA1>>(password, salt, iterations, derived_key_length_bytes);
@ -5141,7 +5145,12 @@ WebIDL::ExceptionOr<GC::Ref<JS::ArrayBuffer>> X25519::derive_bits(AlgorithmParam
return WebIDL::OperationError::create(realm, "Secret is too short"_string);
// Otherwise: Return an octet string containing the first length bits of secret.
auto slice = TRY_OR_THROW_OOM(realm.vm(), secret.slice(0, length / 8));
auto slice = TRY_OR_THROW_OOM(realm.vm(), secret.slice(0, ceil_div(length, 8)));
if (length % 8 != 0) {
// Zero out the last bits
slice[slice.size() - 1] &= 0xFF << (8 - (length % 8));
}
return JS::ArrayBuffer::create(realm, move(slice));
}
@ -5634,7 +5643,12 @@ WebIDL::ExceptionOr<GC::Ref<JS::ArrayBuffer>> X448::derive_bits(
if (secret.size() * 8 < length)
return WebIDL::OperationError::create(m_realm, "Secret is too short"_string);
auto slice = TRY_OR_THROW_OOM(m_realm->vm(), secret.slice(0, length / 8));
auto slice = TRY_OR_THROW_OOM(m_realm->vm(), secret.slice(0, ceil_div(length, 8)));
if (length % 8 != 0) {
// Zero out the last bits
slice[slice.size() - 1] &= 0xFF << (8 - (length % 8));
}
return JS::ArrayBuffer::create(m_realm, move(slice));
}

View file

@ -2,8 +2,7 @@ Harness status: OK
Found 18 tests
17 Pass
1 Fail
18 Pass
Pass setup - define tests
Pass X448 key derivation checks for all-zero value result with a key of order 0
Pass X448 key derivation checks for all-zero value result with a key of order 1
@ -13,7 +12,7 @@ Pass X448 key derivation checks for all-zero value result with a key of order p+
Pass X448 good parameters
Pass X448 mixed case parameters
Pass X448 short result
Fail X448 non-multiple of 8 bits
Pass X448 non-multiple of 8 bits
Pass X448 missing public property
Pass X448 public property of algorithm is not a CryptoKey
Pass X448 mismatched algorithms

View file

@ -0,0 +1,34 @@
Harness status: OK
Found 29 tests
29 Pass
Pass setup - define tests
Pass HKDF derivation with 256 as 'length' parameter
Pass HKDF derivation with 384 as 'length' parameter
Pass HKDF derivation with 230 as 'length' parameter
Pass HKDF derivation with 0 as 'length' parameter
Pass HKDF derivation with null as 'length' parameter
Pass HKDF derivation with undefined as 'length' parameter
Pass HKDF derivation with omitted as 'length' parameter
Pass PBKDF2 derivation with 256 as 'length' parameter
Pass PBKDF2 derivation with 384 as 'length' parameter
Pass PBKDF2 derivation with 230 as 'length' parameter
Pass PBKDF2 derivation with 0 as 'length' parameter
Pass PBKDF2 derivation with null as 'length' parameter
Pass PBKDF2 derivation with undefined as 'length' parameter
Pass PBKDF2 derivation with omitted as 'length' parameter
Pass ECDH derivation with 256 as 'length' parameter
Pass ECDH derivation with 384 as 'length' parameter
Pass ECDH derivation with 230 as 'length' parameter
Pass ECDH derivation with 0 as 'length' parameter
Pass ECDH derivation with null as 'length' parameter
Pass ECDH derivation with undefined as 'length' parameter
Pass ECDH derivation with omitted as 'length' parameter
Pass X25519 derivation with 256 as 'length' parameter
Pass X25519 derivation with 384 as 'length' parameter
Pass X25519 derivation with 230 as 'length' parameter
Pass X25519 derivation with 0 as 'length' parameter
Pass X25519 derivation with null as 'length' parameter
Pass X25519 derivation with undefined as 'length' parameter
Pass X25519 derivation with omitted as 'length' parameter

View file

@ -2,8 +2,8 @@ Harness status: OK
Found 40 tests
31 Pass
9 Fail
35 Pass
5 Fail
Pass setup - define tests
Fail P-521 good parameters
Fail P-521 mixed case parameters
@ -21,7 +21,7 @@ Pass P-521 asking for too many bits
Pass P-256 good parameters
Pass P-256 mixed case parameters
Pass P-256 short result
Fail P-256 non-multiple of 8 bits
Pass P-256 non-multiple of 8 bits
Pass P-256 missing public curve
Pass P-256 public property of algorithm is not a CryptoKey
Pass P-256 mismatched curves
@ -30,11 +30,11 @@ Pass P-256 no deriveBits usage for base key
Pass P-256 base key is not a private key
Pass P-256 public property value is a private key
Pass P-256 public property value is a secret key
Fail P-256 asking for too many bits
Pass P-256 asking for too many bits
Pass P-384 good parameters
Pass P-384 mixed case parameters
Pass P-384 short result
Fail P-384 non-multiple of 8 bits
Pass P-384 non-multiple of 8 bits
Pass P-384 missing public curve
Pass P-384 public property of algorithm is not a CryptoKey
Pass P-384 mismatched curves
@ -43,4 +43,4 @@ Pass P-384 no deriveBits usage for base key
Pass P-384 base key is not a private key
Pass P-384 public property value is a private key
Pass P-384 public property value is a secret key
Fail P-384 asking for too many bits
Pass P-384 asking for too many bits

View file

@ -0,0 +1,17 @@
<!doctype html>
<meta charset=utf-8>
<title>WebCryptoAPI: deriveBits() tests for the 'length' parameter</title>
<script>
self.GLOBAL = {
isWindow: function() { return true; },
isWorker: function() { return false; },
isShadowRealm: function() { return false; },
};
</script>
<script src="../../resources/testharness.js"></script>
<script src="../../resources/testharnessreport.js"></script>
<script src="derived_bits_length.js"></script>
<script src="derived_bits_length_vectors.js"></script>
<script src="derived_bits_length_testcases.js"></script>
<div id=log></div>
<script src="../../WebCryptoAPI/derive_bits_keys/derived_bits_length.https.any.js"></script>

View file

@ -0,0 +1,11 @@
// META: title=WebCryptoAPI: deriveBits() tests for the 'length' parameter
// META: script=derived_bits_length.js
// META: script=derived_bits_length_vectors.js
// META: script=derived_bits_length_testcases.js
// Define subtests from a `promise_test` to ensure the harness does not
// complete before the subtests are available. `explicit_done` cannot be used
// for this purpose because the global `done` function is automatically invoked
// by the WPT infrastructure in dedicated worker tests defined using the
// "multi-global" pattern.
promise_test(define_tests, 'setup - define tests');

View file

@ -0,0 +1,36 @@
function define_tests() {
// May want to test prefixed implementations.
var subtle = self.crypto.subtle;
Object.keys(testCases).forEach(algorithm => {
let testData = algorithms[algorithm];
testCases[algorithm].forEach(testParam => {
promise_test(async() => {
let derivedBits, privateKey, publicKey;
try {
privateKey = await subtle.importKey(testData.privateKey.format, testData.privateKey.data, testData.importAlg, false, ["deriveBits"]);
if (testData.deriveAlg.public !== undefined) {
publicKey = await subtle.importKey(testData.publicKey.format, testData.publicKey.data, testData.importAlg, false, []);
testData.deriveAlg.public = publicKey;
}
if (testParam.length === "omitted")
derivedBits = await subtle.deriveBits(testData.deriveAlg, privateKey);
else
derivedBits = await subtle.deriveBits(testData.deriveAlg, privateKey, testParam.length);
if (testParam.expected === undefined) {
assert_unreached("deriveBits should have thrown an OperationError exception.");
}
assert_array_equals(new Uint8Array(derivedBits), testParam.expected, "Derived bits do not match the expected result.");
} catch (err) {
if (err instanceof AssertionError || testParam.expected !== undefined) {
throw err;
}
assert_true(privateKey !== undefined, "Key should be valid.");
assert_equals(err.name, "OperationError", "deriveBits correctly threw OperationError: " + err.message);
}
}, algorithm + " derivation with " + testParam.length + " as 'length' parameter");
});
});
return Promise.resolve("define_tests");
}

View file

@ -0,0 +1,38 @@
var testCases = {
"HKDF": [
{length: 256, expected: algorithms["HKDF"].derivation},
{length: 384, expected: algorithms["HKDF"].derivation384},
{length: 230, expected: undefined}, // should throw an exception, not multiple of 8
{length: 0, expected: emptyArray},
{length: null, expected: undefined }, // should throw an exception
{length: undefined, expected: undefined }, // should throw an exception
{length: "omitted", expected: undefined }, // default value is null, so should throw
],
"PBKDF2": [
{length: 256, expected: algorithms["PBKDF2"].derivation},
{length: 384, expected: algorithms["PBKDF2"].derivation384},
{length: 230, expected: undefined}, // should throw an exception, not multiple of 8
{length: 0, expected: emptyArray},
{length: null, expected: undefined }, // should throw an exception
{length: undefined, expected: undefined }, // should throw an exception
{length: "omitted", expected: undefined }, // default value is null, so should throw
],
"ECDH": [
{length: 256, expected: algorithms["ECDH"].derivation},
{length: 384, expected: undefined}, // should throw an exception, bigger than the output size
{length: 230, expected: algorithms["ECDH"].derivation230},
{length: 0, expected: emptyArray},
{length: null, expected: algorithms["ECDH"].derivation},
{length: undefined, expected: algorithms["ECDH"].derivation},
{length: "omitted", expected: algorithms["ECDH"].derivation }, // default value is null
],
"X25519": [
{length: 256, expected: algorithms["X25519"].derivation},
{length: 384, expected: undefined}, // should throw an exception, bigger than the output size
{length: 230, expected: algorithms["X25519"].derivation230},
{length: 0, expected: emptyArray},
{length: null, expected: algorithms["X25519"].derivation},
{length: undefined, expected: algorithms["X25519"].derivation},
{length: "omitted", expected: algorithms["X25519"].derivation }, // default value is null
],
}

View file

@ -0,0 +1,41 @@
const emptyArray = new Uint8Array([]);
const rawKey = new Uint8Array([85, 115, 101, 114, 115, 32, 115, 104, 111, 117, 108, 100, 32, 112, 105, 99, 107, 32, 108, 111, 110, 103, 32, 112, 97, 115, 115, 112, 104, 114, 97, 115, 101, 115, 32, 40, 110, 111, 116, 32, 117, 115, 101, 32, 115, 104, 111, 114, 116, 32, 112, 97, 115, 115, 119, 111, 114, 100, 115, 41, 33]);
const salt = new Uint8Array([83, 111, 100, 105, 117, 109, 32, 67, 104, 108, 111, 114, 105, 100, 101, 32, 99, 111, 109, 112, 111, 117, 110, 100]);
const info = new Uint8Array([72, 75, 68, 70, 32, 101, 120, 116, 114, 97, 32, 105, 110, 102, 111]);
var algorithms = {
"HKDF": {
importAlg: {name: "HKDF"},
privateKey: {format: "raw", data: rawKey},
deriveAlg: {name: "HKDF", salt: salt, hash: "SHA-256", info: info},
derivation: new Uint8Array([49, 183, 214, 133, 48, 168, 99, 231, 23, 192, 129, 202, 105, 23, 182, 134, 80, 179, 221, 154, 41, 243, 6, 6, 226, 202, 209, 153, 190, 193, 77, 19]),
derivation384: new Uint8Array([49, 183, 214, 133, 48, 168, 99, 231, 23, 192, 129, 202, 105, 23, 182, 134, 80, 179, 221, 154, 41, 243, 6, 6, 226, 202, 209, 153, 190, 193, 77, 19, 165, 50, 181, 8, 254, 59, 122, 199, 25, 224,146, 248, 105, 105, 75, 84]),
derivation230: undefined,
},
"PBKDF2": {
importAlg: {name: "PBKDF2"},
privateKey: {format: "raw", data: rawKey},
deriveAlg: {name: "PBKDF2", salt: salt, hash: "SHA-256", iterations: 100000},
derivation: new Uint8Array([17, 153, 45, 139, 129, 51, 17, 36, 76, 84, 75, 98, 41, 41, 69, 226, 8, 212, 3, 206, 189, 107, 149, 82, 161, 165, 98, 6, 93, 153, 88, 234]),
derivation384: new Uint8Array([17, 153, 45, 139, 129, 51, 17, 36, 76, 84, 75, 98, 41, 41, 69, 226, 8, 212, 3, 206, 189, 107, 149, 82, 161, 165, 98, 6, 93, 153, 88, 234, 39, 104, 8, 112, 222, 57, 166, 47, 102, 146, 195, 59, 219, 239, 238, 47]),
derivation230: undefined,
},
"ECDH": {
importAlg: {name: "ECDH", namedCurve: "P-256"},
privateKey: {format: "pkcs8", data: new Uint8Array([48, 129, 135, 2, 1, 0, 48, 19, 6, 7, 42, 134, 72, 206, 61, 2, 1, 6, 8, 42, 134, 72, 206, 61, 3, 1, 7, 4, 109, 48, 107, 2, 1, 1, 4, 32, 15, 247, 79, 232, 241, 202, 175, 97, 92, 206, 241, 29, 217, 53, 114, 87, 98, 217, 216, 65, 236, 186, 185, 94, 170, 38, 68, 123, 52, 100, 245, 113, 161, 68, 3, 66, 0, 4, 140, 96, 11, 44, 102, 25, 45, 97, 158, 39, 210, 37, 107, 59, 151, 118, 178, 141, 30, 5, 246, 13, 234, 189, 98, 174, 123, 154, 211, 157, 224, 217, 59, 4, 102, 109, 199, 119, 14, 126, 207, 13, 211, 203, 203, 211, 110, 221, 107, 94, 220, 153, 81, 7, 55, 161, 237, 104, 46, 205, 112, 244, 10, 47])},
publicKey: {format: "spki", data: new Uint8Array([48, 89, 48, 19, 6, 7, 42, 134, 72, 206, 61, 2, 1, 6, 8, 42, 134, 72, 206, 61, 3, 1, 7, 3, 66, 0, 4, 154, 116, 32, 120, 126, 95, 77, 105, 211, 232, 34, 114, 115, 1, 109, 56, 224, 71, 129, 133, 223, 127, 238, 156, 142, 103, 60, 202, 211, 79, 126, 128, 254, 49, 141, 182, 221, 107, 119, 218, 99, 32, 165, 246, 151, 89, 9, 68, 23, 177, 52, 239, 138, 139, 116, 193, 101, 4, 57, 198, 115, 0, 90, 61])},
deriveAlg: {name: "ECDH", public: new Uint8Array ([])},
derivation: new Uint8Array([14, 143, 60, 77, 177, 178, 162, 131, 115, 90, 0, 220, 87, 31, 26, 232, 151, 28, 227, 35, 250, 17, 131, 137, 203, 95, 65, 196, 59, 61, 181, 161]),
derivation384: undefined,
derivation230: new Uint8Array([14, 143, 60, 77, 177, 178, 162, 131, 115, 90, 0, 220, 87, 31, 26, 232, 151, 28, 227, 35, 250, 17, 131, 137, 203, 95, 65, 196, 56]),
},
"X25519": {
importAlg: {name: "X25519"},
privateKey: {format: "pkcs8", data: new Uint8Array([48, 46, 2, 1, 0, 48, 5, 6, 3, 43, 101, 110, 4, 34, 4, 32, 200, 131, 142, 118, 208, 87, 223, 183, 216, 201, 90, 105, 225, 56, 22, 10, 221, 99, 115, 253, 113, 164, 210, 118, 187, 86, 227, 168, 27, 100, 255, 97])},
publicKey: {format: "spki", data: new Uint8Array([48, 42, 48, 5, 6, 3, 43, 101, 110, 3, 33, 0, 28, 242, 177, 230, 2, 46, 197, 55, 55, 30, 215, 245, 62, 84, 250, 17, 84, 216, 62, 152, 235, 100, 234, 81, 250, 229, 179, 48, 124, 254, 151, 6])},
deriveAlg: {name: "X25519", public: new Uint8Array ([])},
derivation: new Uint8Array([39, 104, 64, 157, 250, 185, 158, 194, 59, 140, 137, 185, 63, 245, 136, 2, 149, 247, 97, 118, 8, 143, 137, 228, 61, 254, 190, 126, 161, 149, 0, 8]),
derivation384: undefined,
derivation230: new Uint8Array([39, 104, 64, 157, 250, 185, 158, 194, 59, 140, 137, 185, 63, 245, 136, 2, 149, 247, 97, 118, 8, 143, 137, 228, 61, 254, 190, 126, 160]),
}
};