From 0db171c36eeb443314f0a5e60d683417f559e83e Mon Sep 17 00:00:00 2001 From: rmg-x Date: Sun, 3 Nov 2024 13:53:00 -0600 Subject: [PATCH] LibWeb/Crypto: Fix sizes being passed into `generate_aes_key()` Previously, callers were passing the size in bytes, but the method expected bits. This caused a crash in LibCrypto when verifying the key size later on. Also make the naming of local variables and parameters a little more clear between the different AES algorithms :^) --- .../expected/Crypto/SubtleCrypto-aesgcm.txt | 3 ++ .../input/Crypto/SubtleCrypto-aesgcm.html | 22 +++++++++++++++ .../LibWeb/Crypto/CryptoAlgorithms.cpp | 28 +++++++++---------- 3 files changed, 39 insertions(+), 14 deletions(-) create mode 100644 Tests/LibWeb/Text/expected/Crypto/SubtleCrypto-aesgcm.txt create mode 100644 Tests/LibWeb/Text/input/Crypto/SubtleCrypto-aesgcm.html diff --git a/Tests/LibWeb/Text/expected/Crypto/SubtleCrypto-aesgcm.txt b/Tests/LibWeb/Text/expected/Crypto/SubtleCrypto-aesgcm.txt new file mode 100644 index 00000000000..074135ef773 --- /dev/null +++ b/Tests/LibWeb/Text/expected/Crypto/SubtleCrypto-aesgcm.txt @@ -0,0 +1,3 @@ +exported 128 bit key length: 16 +exported 192 bit key length: 24 +exported 256 bit key length: 32 \ No newline at end of file diff --git a/Tests/LibWeb/Text/input/Crypto/SubtleCrypto-aesgcm.html b/Tests/LibWeb/Text/input/Crypto/SubtleCrypto-aesgcm.html new file mode 100644 index 00000000000..9b510017d04 --- /dev/null +++ b/Tests/LibWeb/Text/input/Crypto/SubtleCrypto-aesgcm.html @@ -0,0 +1,22 @@ + + + diff --git a/Userland/Libraries/LibWeb/Crypto/CryptoAlgorithms.cpp b/Userland/Libraries/LibWeb/Crypto/CryptoAlgorithms.cpp index 6c6112ba111..80f02a06d57 100644 --- a/Userland/Libraries/LibWeb/Crypto/CryptoAlgorithms.cpp +++ b/Userland/Libraries/LibWeb/Crypto/CryptoAlgorithms.cpp @@ -240,9 +240,9 @@ static WebIDL::ExceptionOr parse_jwk_symmetric_key(JS::Realm& realm, return base64_url_bytes_decode(realm, *jwk.k); } -static WebIDL::ExceptionOr generate_aes_key(JS::VM& vm, u16 bits) +static WebIDL::ExceptionOr generate_aes_key(JS::VM& vm, u16 const size_in_bits) { - auto key_buffer = TRY_OR_THROW_OOM(vm, ByteBuffer::create_uninitialized(bits / 8)); + auto key_buffer = TRY_OR_THROW_OOM(vm, ByteBuffer::create_uninitialized(size_in_bits / 8)); fill_with_random(key_buffer); return key_buffer; } @@ -1358,13 +1358,13 @@ WebIDL::ExceptionOr, JS::NonnullGCPtr(params); // 2. If the length member of normalizedAlgorithm is not equal to one of 128, 192 or 256, then throw an OperationError. - auto bits = normalized_algorithm.length; + auto const bits = normalized_algorithm.length; if (bits != 128 && bits != 192 && bits != 256) { return WebIDL::OperationError::create(m_realm, MUST(String::formatted("Cannot create AES-CBC key with unusual amount of {} bits", bits))); } // 3. Generate an AES key of length equal to the length member of normalizedAlgorithm. - auto key_buffer = TRY(generate_aes_key(m_realm->vm(), bits / 8)); + auto key_buffer = TRY(generate_aes_key(m_realm->vm(), bits)); // 4. If the key generation step fails, then throw an OperationError. // Note: Cannot happen in our implementation; and if we OOM, then allocating the Exception is probably going to crash anyway. @@ -1689,14 +1689,14 @@ WebIDL::ExceptionOr, JS::NonnullGCPtr(params); - auto length = normalized_algorithm.length; - if (length != 128 && length != 192 && length != 256) { - return WebIDL::OperationError::create(m_realm, MUST(String::formatted("Cannot create AES-CTR key with unusual amount of {} bits", length))); + auto const bits = normalized_algorithm.length; + if (bits != 128 && bits != 192 && bits != 256) { + return WebIDL::OperationError::create(m_realm, MUST(String::formatted("Cannot create AES-CTR key with unusual amount of {} bits", bits))); } // 3. Generate an AES key of length equal to the length member of normalizedAlgorithm. // 4. If the key generation step fails, then throw an OperationError. - auto key_buffer = TRY(generate_aes_key(m_realm->vm(), length / 8)); + auto key_buffer = TRY(generate_aes_key(m_realm->vm(), bits)); // 5. Let key be a new CryptoKey object representing the generated AES key. auto key = CryptoKey::create(m_realm, CryptoKey::InternalKeyData { key_buffer }); @@ -1708,7 +1708,7 @@ WebIDL::ExceptionOr, JS::NonnullGCPtrset_name("AES-CTR"_string); // 8. Set the length attribute of algorithm to equal the length member of normalizedAlgorithm. - algorithm->set_length(length); + algorithm->set_length(bits); // 9. Set the [[type]] internal slot of key to "secret". key->set_type(Bindings::KeyType::Secret); @@ -2118,14 +2118,14 @@ WebIDL::ExceptionOr, JS::NonnullGCPtr(params); - auto length = normalized_algorithm.length; - if (length != 128 && length != 192 && length != 256) { - return WebIDL::OperationError::create(m_realm, MUST(String::formatted("Cannot create AES-CTR key with unusual amount of {} bits", length))); + auto const bits = normalized_algorithm.length; + if (bits != 128 && bits != 192 && bits != 256) { + return WebIDL::OperationError::create(m_realm, MUST(String::formatted("Cannot create AES-GCM key with unusual amount of {} bits", bits))); } // 3. Generate an AES key of length equal to the length member of normalizedAlgorithm. // 4. If the key generation step fails, then throw an OperationError. - auto key_buffer = TRY(generate_aes_key(m_realm->vm(), length / 8)); + auto key_buffer = TRY(generate_aes_key(m_realm->vm(), bits)); // 5. Let key be a new CryptoKey object representing the generated AES key. auto key = CryptoKey::create(m_realm, CryptoKey::InternalKeyData { key_buffer }); @@ -2137,7 +2137,7 @@ WebIDL::ExceptionOr, JS::NonnullGCPtrset_name("AES-GCM"_string); // 8. Set the length attribute of algorithm to equal the length member of normalizedAlgorithm. - algorithm->set_length(length); + algorithm->set_length(bits); // 9. Set the [[type]] internal slot of key to "secret". key->set_type(Bindings::KeyType::Secret);