From b05beb79d48c4e95bc8548bf06274e7a65df3444 Mon Sep 17 00:00:00 2001 From: Ali Mohammad Pur Date: Thu, 13 May 2021 12:13:11 +0430 Subject: [PATCH] LibCrypto: Remove all uses of VLAs This removes all uses of VLAs with either Vectors with inline capacity for the expected soft upper bound, or the occasional heap allocation. --- .../Libraries/LibCrypto/Authentication/HMAC.h | 10 ++++--- Userland/Libraries/LibCrypto/Cipher/AES.h | 4 +-- Userland/Libraries/LibCrypto/Cipher/Cipher.h | 4 +-- .../Libraries/LibCrypto/Hash/HashFunction.h | 4 +-- .../NumberTheory/ModularFunctions.cpp | 7 ++--- .../Libraries/LibCrypto/PK/Code/EMSA_PSS.h | 26 +++++++++++-------- Userland/Libraries/LibCrypto/PK/RSA.cpp | 22 ++++++++-------- 7 files changed, 41 insertions(+), 36 deletions(-) diff --git a/Userland/Libraries/LibCrypto/Authentication/HMAC.h b/Userland/Libraries/LibCrypto/Authentication/HMAC.h index ab5d287bee..6925c2cebd 100644 --- a/Userland/Libraries/LibCrypto/Authentication/HMAC.h +++ b/Userland/Libraries/LibCrypto/Authentication/HMAC.h @@ -25,7 +25,7 @@ public: using HashType = HashT; using TagType = typename HashType::DigestType; - size_t digest_size() const { return m_inner_hasher.digest_size(); } + constexpr size_t digest_size() const { return m_inner_hasher.digest_size(); } template HMAC(KeyBufferType key, Args... args) @@ -82,9 +82,11 @@ private: void derive_key(const u8* key, size_t length) { auto block_size = m_inner_hasher.block_size(); - u8 v_key[block_size]; - __builtin_memset(v_key, 0, block_size); - auto key_buffer = Bytes { v_key, block_size }; + // Note: The block size of all the current hash functions is 512 bits. + Vector v_key; + v_key.resize(block_size); + __builtin_memset(v_key.data(), 0, block_size); + auto key_buffer = v_key.span(); // m_key_data is zero'd, so copying the data in // the first few bytes leaves the rest zero, which // is exactly what we want (zero padding) diff --git a/Userland/Libraries/LibCrypto/Cipher/AES.h b/Userland/Libraries/LibCrypto/Cipher/AES.h index d6f0c80e69..236c3bbe63 100644 --- a/Userland/Libraries/LibCrypto/Cipher/AES.h +++ b/Userland/Libraries/LibCrypto/Cipher/AES.h @@ -29,7 +29,7 @@ public: CipherBlock::overwrite(data, length); } - static size_t block_size() { return BlockSizeInBits / 8; }; + constexpr static size_t block_size() { return BlockSizeInBits / 8; }; virtual ReadonlyBytes bytes() const override { return ReadonlyBytes { m_data, sizeof(m_data) }; } virtual Bytes bytes() override { return Bytes { m_data, sizeof(m_data) }; } @@ -46,7 +46,7 @@ public: String to_string() const; private: - size_t data_size() const { return sizeof(m_data); } + constexpr static size_t data_size() { return sizeof(m_data); } u8 m_data[BlockSizeInBits / 8] {}; }; diff --git a/Userland/Libraries/LibCrypto/Cipher/Cipher.h b/Userland/Libraries/LibCrypto/Cipher/Cipher.h index d10ecb281a..01739ab603 100644 --- a/Userland/Libraries/LibCrypto/Cipher/Cipher.h +++ b/Userland/Libraries/LibCrypto/Cipher/Cipher.h @@ -39,8 +39,6 @@ public: { } - static size_t block_size() { VERIFY_NOT_REACHED(); } - virtual ReadonlyBytes bytes() const = 0; virtual void overwrite(ReadonlyBytes) = 0; @@ -106,7 +104,7 @@ public: virtual const KeyType& key() const = 0; virtual KeyType& key() = 0; - static size_t block_size() { return BlockType::block_size(); } + constexpr static size_t block_size() { return BlockType::block_size(); } PaddingMode padding_mode() const { return m_padding_mode; } diff --git a/Userland/Libraries/LibCrypto/Hash/HashFunction.h b/Userland/Libraries/LibCrypto/Hash/HashFunction.h index f3aca3da12..091c252661 100644 --- a/Userland/Libraries/LibCrypto/Hash/HashFunction.h +++ b/Userland/Libraries/LibCrypto/Hash/HashFunction.h @@ -21,8 +21,8 @@ public: using DigestType = DigestT; - static size_t block_size() { return BlockSize; }; - static size_t digest_size() { return DigestSize; }; + constexpr static size_t block_size() { return BlockSize; }; + constexpr static size_t digest_size() { return DigestSize; }; virtual void update(const u8*, size_t) = 0; diff --git a/Userland/Libraries/LibCrypto/NumberTheory/ModularFunctions.cpp b/Userland/Libraries/LibCrypto/NumberTheory/ModularFunctions.cpp index 71b372fbb0..e24700c07a 100644 --- a/Userland/Libraries/LibCrypto/NumberTheory/ModularFunctions.cpp +++ b/Userland/Libraries/LibCrypto/NumberTheory/ModularFunctions.cpp @@ -273,9 +273,10 @@ UnsignedBigInteger random_number(const UnsignedBigInteger& min, const UnsignedBi UnsignedBigInteger base; auto size = range.trimmed_length() * sizeof(u32) + 2; // "+2" is intentional (see below). - // Also, if we're about to crash anyway, at least produce a nice error: - VERIFY(size < 8 * MiB); - u8 buf[size]; + ByteBuffer buffer; + buffer.grow(size); + auto* buf = buffer.data(); + fill_with_random(buf, size); UnsignedBigInteger random { buf, size }; // At this point, `random` is a large number, in the range [0, 256^size). diff --git a/Userland/Libraries/LibCrypto/PK/Code/EMSA_PSS.h b/Userland/Libraries/LibCrypto/PK/Code/EMSA_PSS.h index 689e790c14..127f3c528e 100644 --- a/Userland/Libraries/LibCrypto/PK/Code/EMSA_PSS.h +++ b/Userland/Libraries/LibCrypto/PK/Code/EMSA_PSS.h @@ -33,7 +33,7 @@ public: auto& hash_fn = this->hasher(); hash_fn.update(in); auto message_hash = hash_fn.digest(); - auto hash_length = hash_fn.DigestSize; + constexpr auto hash_length = hash_fn.DigestSize; auto em_length = (em_bits + 7) / 8; u8 salt[SaltLength]; @@ -51,8 +51,9 @@ public: hash_fn.update(m_buffer); auto hash = hash_fn.digest(); - u8 DB_data[em_length - HashFunction::DigestSize - 1]; - auto DB = Bytes { DB_data, em_length - HashFunction::DigestSize - 1 }; + Vector DB_data; + DB_data.resize(em_length - HashFunction::DigestSize - 1); + Bytes DB = DB_data; auto DB_offset = 0; for (size_t i = 0; i < em_length - SaltLength - HashFunction::DigestSize - 2; ++i) @@ -64,8 +65,9 @@ public: auto mask_length = em_length - HashFunction::DigestSize - 1; - u8 DB_mask[mask_length]; - auto DB_mask_buffer = Bytes { DB_mask, mask_length }; + Vector DB_mask; + DB_mask.resize(mask_length); + Bytes DB_mask_buffer { DB_mask }; // FIXME: we should probably allow reading from u8* MGF1(ReadonlyBytes { hash.data, HashFunction::DigestSize }, mask_length, DB_mask_buffer); @@ -102,11 +104,13 @@ public: if ((octet >> (8 - i)) & 0x01) return VerificationConsistency::Inconsistent; - u8 DB_mask[mask_length]; - auto DB_mask_buffer = Bytes { DB_mask, mask_length }; + Vector DB_mask; + DB_mask.resize(mask_length); + Bytes DB_mask_buffer { DB_mask }; MGF1(H, mask_length, DB_mask_buffer); - u8 DB[mask_length]; + Vector DB; + DB.resize(mask_length); for (size_t i = 0; i < mask_length; ++i) DB[i] = masked_DB[i] ^ DB_mask[i]; @@ -122,8 +126,8 @@ public: if (DB[check_octets + 1] != 0x01) return VerificationConsistency::Inconsistent; - auto* salt = DB + mask_length - SaltLength; - u8 m_prime[8 + HashFunction::DigestSize + SaltLength] { 0, 0, 0, 0, 0, 0, 0, 0 }; + auto* salt = DB.span().offset(mask_length - SaltLength); + u8 m_prime[8 + HashFunction::DigestSize + SaltLength] { 0 }; auto m_prime_buffer = Bytes { m_prime, sizeof(m_prime) }; @@ -133,7 +137,7 @@ public: hash_fn.update(m_prime_buffer); auto H_prime = hash_fn.digest(); - if (__builtin_memcmp(message_hash.data, H_prime.data, HashFunction::DigestSize)) + if (__builtin_memcmp(message_hash.data, H_prime.data, HashFunction::DigestSize) != 0) return VerificationConsistency::Inconsistent; return VerificationConsistency::Consistent; diff --git a/Userland/Libraries/LibCrypto/PK/RSA.cpp b/Userland/Libraries/LibCrypto/PK/RSA.cpp index c738b60d2f..e426d755ab 100644 --- a/Userland/Libraries/LibCrypto/PK/RSA.cpp +++ b/Userland/Libraries/LibCrypto/PK/RSA.cpp @@ -293,8 +293,9 @@ void RSA_EMSA_PSS::sign(ReadonlyBytes in, Bytes& out) // -- encode via EMSA_PSS auto mod_bits = m_rsa.private_key().modulus().trimmed_length() * sizeof(u32) * 8; - u8 EM[mod_bits]; - auto EM_buf = Bytes { EM, mod_bits }; + Vector EM; + EM.resize(mod_bits); + auto EM_buf = Bytes { EM }; m_emsa_pss.encode(in, EM_buf, mod_bits - 1); // -- sign via RSA @@ -308,8 +309,9 @@ VerificationConsistency RSA_EMSA_PSS::verify(ReadonlyBytes in) if (in.size() != mod_bytes) return VerificationConsistency::Inconsistent; - u8 EM[mod_bytes]; - auto EM_buf = Bytes { EM, mod_bytes }; + Vector EM; + EM.resize(mod_bytes); + auto EM_buf = Bytes { EM }; // -- verify via RSA m_rsa.verify(in, EM_buf); @@ -333,22 +335,20 @@ void RSA_PKCS1_EME::encrypt(ReadonlyBytes in, Bytes& out) } auto ps_length = mod_len - in.size() - 3; - u8 ps[ps_length]; + Vector ps; + ps.resize(ps_length); - // FIXME: Without this assertion, GCC refuses to compile due to a memcpy overflow(!?) - VERIFY(ps_length < 16384); - - fill_with_random(ps, ps_length); + fill_with_random(ps.data(), ps_length); // since arc4random can create zeros (shocking!) // we have to go through and un-zero the zeros for (size_t i = 0; i < ps_length; ++i) while (!ps[i]) - fill_with_random(ps + i, 1); + fill_with_random(ps.span().offset(i), 1); u8 paddings[] { 0x00, 0x02 }; out.overwrite(0, paddings, 2); - out.overwrite(2, ps, ps_length); + out.overwrite(2, ps.data(), ps_length); out.overwrite(2 + ps_length, paddings, 1); out.overwrite(3 + ps_length, in.data(), in.size()); out = out.trim(3 + ps_length + in.size()); // should be a single block