From 15532df83da6a8bd91db1b28431e0c14d5cbce05 Mon Sep 17 00:00:00 2001 From: Timothy Flynn Date: Sun, 2 Apr 2023 13:08:43 -0400 Subject: [PATCH] AK+Everywhere: Change AK::fill_with_random to accept a Bytes object Rather than the very C-like API we currently have, accepting a void* and a length, let's take a Bytes object instead. In almost all existing cases, the compiler figures out the length. --- AK/Random.h | 23 ++++++++----------- Tests/LibC/TestSnprintf.cpp | 2 +- Tests/LibC/TestStrlcpy.cpp | 2 +- Tests/LibCompress/TestDeflate.cpp | 6 ++--- Tests/LibCompress/TestGzip.cpp | 2 +- Tests/LibCrypto/TestBigInteger.cpp | 2 +- Userland/Libraries/LibCore/Account.cpp | 2 +- .../Libraries/LibCrypto/Curves/Ed25519.cpp | 2 +- .../Libraries/LibCrypto/Curves/SECP256r1.cpp | 2 +- .../Libraries/LibCrypto/Curves/X25519.cpp | 2 +- Userland/Libraries/LibCrypto/Curves/X448.cpp | 2 +- .../NumberTheory/ModularFunctions.cpp | 2 +- .../Libraries/LibCrypto/PK/Code/EMSA_PSS.h | 2 +- Userland/Libraries/LibCrypto/PK/RSA.cpp | 7 +++--- Userland/Libraries/LibTLS/Handshake.cpp | 2 +- Userland/Libraries/LibTLS/HandshakeClient.cpp | 2 +- Userland/Libraries/LibTLS/Record.cpp | 4 ++-- Userland/Libraries/LibWeb/Crypto/Crypto.cpp | 4 ++-- Userland/Libraries/LibWebSocket/WebSocket.cpp | 4 ++-- Userland/Utilities/useradd.cpp | 2 +- 20 files changed, 37 insertions(+), 39 deletions(-) diff --git a/AK/Random.h b/AK/Random.h index 80b636b30c..5cb2054db8 100644 --- a/AK/Random.h +++ b/AK/Random.h @@ -7,6 +7,7 @@ #pragma once #include +#include #include #include #include @@ -21,36 +22,32 @@ namespace AK { -inline void fill_with_random([[maybe_unused]] void* buffer, [[maybe_unused]] size_t length) +inline void fill_with_random([[maybe_unused]] Bytes bytes) { #if defined(AK_OS_SERENITY) || defined(AK_OS_ANDROID) - arc4random_buf(buffer, length); + arc4random_buf(bytes.data(), bytes.size()); #elif defined(OSS_FUZZ) #else auto fill_with_random_fallback = [&]() { - char* char_buffer = static_cast(buffer); - for (size_t i = 0; i < length; i++) - char_buffer[i] = rand(); + for (auto& byte : bytes) + byte = rand(); }; # if defined(__unix__) or defined(AK_OS_MACOS) // The maximum permitted value for the getentropy length argument. static constexpr size_t getentropy_length_limit = 256; - - auto iterations = length / getentropy_length_limit; - auto remainder = length % getentropy_length_limit; - auto address = reinterpret_cast(buffer); + auto iterations = bytes.size() / getentropy_length_limit; for (size_t i = 0; i < iterations; ++i) { - if (getentropy(reinterpret_cast(address), getentropy_length_limit) != 0) { + if (getentropy(bytes.data(), getentropy_length_limit) != 0) { fill_with_random_fallback(); return; } - address += getentropy_length_limit; + bytes = bytes.slice(getentropy_length_limit); } - if (remainder == 0 || getentropy(reinterpret_cast(address), remainder) == 0) + if (bytes.is_empty() || getentropy(bytes.data(), bytes.size()) == 0) return; # endif @@ -62,7 +59,7 @@ template inline T get_random() { T t; - fill_with_random(&t, sizeof(T)); + fill_with_random({ &t, sizeof(T) }); return t; } diff --git a/Tests/LibC/TestSnprintf.cpp b/Tests/LibC/TestSnprintf.cpp index c7535db722..5f2bf01551 100644 --- a/Tests/LibC/TestSnprintf.cpp +++ b/Tests/LibC/TestSnprintf.cpp @@ -59,7 +59,7 @@ static bool test_single(Testcase const& testcase) // Setup ByteBuffer actual = ByteBuffer::create_uninitialized(SANDBOX_CANARY_SIZE + testcase.dest_n + SANDBOX_CANARY_SIZE).release_value(); - fill_with_random(actual.data(), actual.size()); + fill_with_random(actual); ByteBuffer expected = actual; VERIFY(actual.offset_pointer(0) != expected.offset_pointer(0)); actual.overwrite(SANDBOX_CANARY_SIZE, testcase.dest, testcase.dest_n); diff --git a/Tests/LibC/TestStrlcpy.cpp b/Tests/LibC/TestStrlcpy.cpp index 01d97e45c0..98317e13a3 100644 --- a/Tests/LibC/TestStrlcpy.cpp +++ b/Tests/LibC/TestStrlcpy.cpp @@ -56,7 +56,7 @@ static bool test_single(Testcase const& testcase) // Setup ByteBuffer actual = ByteBuffer::create_uninitialized(SANDBOX_CANARY_SIZE + testcase.dest_n + SANDBOX_CANARY_SIZE).release_value(); - fill_with_random(actual.data(), actual.size()); + fill_with_random(actual); ByteBuffer expected = actual; VERIFY(actual.offset_pointer(0) != expected.offset_pointer(0)); actual.overwrite(SANDBOX_CANARY_SIZE, testcase.dest, testcase.dest_n); diff --git a/Tests/LibCompress/TestDeflate.cpp b/Tests/LibCompress/TestDeflate.cpp index e51092e5b3..c4bd045182 100644 --- a/Tests/LibCompress/TestDeflate.cpp +++ b/Tests/LibCompress/TestDeflate.cpp @@ -115,7 +115,7 @@ TEST_CASE(deflate_decompress_zeroes) TEST_CASE(deflate_round_trip_store) { auto original = ByteBuffer::create_uninitialized(1024).release_value(); - fill_with_random(original.data(), 1024); + fill_with_random(original); auto compressed = Compress::DeflateCompressor::compress_all(original, Compress::DeflateCompressor::CompressionLevel::STORE); EXPECT(!compressed.is_error()); auto uncompressed = Compress::DeflateDecompressor::decompress_all(compressed.value()); @@ -126,7 +126,7 @@ TEST_CASE(deflate_round_trip_store) TEST_CASE(deflate_round_trip_compress) { auto original = ByteBuffer::create_zeroed(2048).release_value(); - fill_with_random(original.data(), 1024); // we pre-filled the second half with 0s to make sure we test back references as well + fill_with_random(original.bytes().trim(1024)); // we pre-filled the second half with 0s to make sure we test back references as well // Since the different levels just change how much time is spent looking for better matches, just use fast here to reduce test time auto compressed = Compress::DeflateCompressor::compress_all(original, Compress::DeflateCompressor::CompressionLevel::FAST); EXPECT(!compressed.is_error()); @@ -139,7 +139,7 @@ TEST_CASE(deflate_round_trip_compress_large) { auto size = Compress::DeflateCompressor::block_size * 2; auto original = ByteBuffer::create_uninitialized(size).release_value(); // Compress a buffer larger than the maximum block size to test the sliding window mechanism - fill_with_random(original.data(), size); + fill_with_random(original); // Since the different levels just change how much time is spent looking for better matches, just use fast here to reduce test time auto compressed = Compress::DeflateCompressor::compress_all(original, Compress::DeflateCompressor::CompressionLevel::FAST); EXPECT(!compressed.is_error()); diff --git a/Tests/LibCompress/TestGzip.cpp b/Tests/LibCompress/TestGzip.cpp index 2f90725aea..39082ca357 100644 --- a/Tests/LibCompress/TestGzip.cpp +++ b/Tests/LibCompress/TestGzip.cpp @@ -88,7 +88,7 @@ TEST_CASE(gzip_decompress_repeat_around_buffer) TEST_CASE(gzip_round_trip) { auto original = ByteBuffer::create_uninitialized(1024).release_value(); - fill_with_random(original.data(), 1024); + fill_with_random(original); auto compressed = Compress::GzipCompressor::compress_all(original); EXPECT(!compressed.is_error()); auto uncompressed = Compress::GzipDecompressor::decompress_all(compressed.value()); diff --git a/Tests/LibCrypto/TestBigInteger.cpp b/Tests/LibCrypto/TestBigInteger.cpp index 91f82e6f92..92e3e03194 100644 --- a/Tests/LibCrypto/TestBigInteger.cpp +++ b/Tests/LibCrypto/TestBigInteger.cpp @@ -398,7 +398,7 @@ TEST_CASE(test_bigint_import_big_endian_decode_encode_roundtrip) { u8 random_bytes[128]; u8 target_buffer[128]; - fill_with_random(random_bytes, 128); + fill_with_random(random_bytes); auto encoded = Crypto::UnsignedBigInteger::import_data(random_bytes, 128); encoded.export_data({ target_buffer, 128 }); EXPECT(memcmp(target_buffer, random_bytes, 128) == 0); diff --git a/Userland/Libraries/LibCore/Account.cpp b/Userland/Libraries/LibCore/Account.cpp index 7c24d07b5c..4940f02e3b 100644 --- a/Userland/Libraries/LibCore/Account.cpp +++ b/Userland/Libraries/LibCore/Account.cpp @@ -30,7 +30,7 @@ namespace Core { static DeprecatedString get_salt() { char random_data[12]; - fill_with_random(random_data, sizeof(random_data)); + fill_with_random({ random_data, sizeof(random_data) }); StringBuilder builder; builder.append("$5$"sv); diff --git a/Userland/Libraries/LibCrypto/Curves/Ed25519.cpp b/Userland/Libraries/LibCrypto/Curves/Ed25519.cpp index 79a743bed6..4f10e7cdf9 100644 --- a/Userland/Libraries/LibCrypto/Curves/Ed25519.cpp +++ b/Userland/Libraries/LibCrypto/Curves/Ed25519.cpp @@ -19,7 +19,7 @@ ErrorOr Ed25519::generate_private_key() // about randomness. auto buffer = TRY(ByteBuffer::create_uninitialized(key_size())); - fill_with_random(buffer.data(), buffer.size()); + fill_with_random(buffer); return buffer; }; diff --git a/Userland/Libraries/LibCrypto/Curves/SECP256r1.cpp b/Userland/Libraries/LibCrypto/Curves/SECP256r1.cpp index 91ff20a00a..d1ab81df10 100644 --- a/Userland/Libraries/LibCrypto/Curves/SECP256r1.cpp +++ b/Userland/Libraries/LibCrypto/Curves/SECP256r1.cpp @@ -357,7 +357,7 @@ static bool is_point_on_curve(JacobianPoint const& point) ErrorOr SECP256r1::generate_private_key() { auto buffer = TRY(ByteBuffer::create_uninitialized(32)); - fill_with_random(buffer.data(), buffer.size()); + fill_with_random(buffer); return buffer; } diff --git a/Userland/Libraries/LibCrypto/Curves/X25519.cpp b/Userland/Libraries/LibCrypto/Curves/X25519.cpp index 07c2ffd300..61b4533490 100644 --- a/Userland/Libraries/LibCrypto/Curves/X25519.cpp +++ b/Userland/Libraries/LibCrypto/Curves/X25519.cpp @@ -30,7 +30,7 @@ static void conditional_swap(u32* first, u32* second, u32 condition) ErrorOr X25519::generate_private_key() { auto buffer = TRY(ByteBuffer::create_uninitialized(BYTES)); - fill_with_random(buffer.data(), buffer.size()); + fill_with_random(buffer); return buffer; } diff --git a/Userland/Libraries/LibCrypto/Curves/X448.cpp b/Userland/Libraries/LibCrypto/Curves/X448.cpp index 6410a2d780..566a3e520d 100644 --- a/Userland/Libraries/LibCrypto/Curves/X448.cpp +++ b/Userland/Libraries/LibCrypto/Curves/X448.cpp @@ -291,7 +291,7 @@ static void modular_multiply_inverse(u32* state, u32* value) ErrorOr X448::generate_private_key() { auto buffer = TRY(ByteBuffer::create_uninitialized(BYTES)); - fill_with_random(buffer.data(), buffer.size()); + fill_with_random(buffer); return buffer; } diff --git a/Userland/Libraries/LibCrypto/NumberTheory/ModularFunctions.cpp b/Userland/Libraries/LibCrypto/NumberTheory/ModularFunctions.cpp index 0805130398..b2884c9f98 100644 --- a/Userland/Libraries/LibCrypto/NumberTheory/ModularFunctions.cpp +++ b/Userland/Libraries/LibCrypto/NumberTheory/ModularFunctions.cpp @@ -168,7 +168,7 @@ UnsignedBigInteger random_number(UnsignedBigInteger const& min, UnsignedBigInteg auto buffer = ByteBuffer::create_uninitialized(size).release_value_but_fixme_should_propagate_errors(); // FIXME: Handle possible OOM situation. auto* buf = buffer.data(); - fill_with_random(buf, size); + fill_with_random(buffer); UnsignedBigInteger random { buf, size }; // At this point, `random` is a large number, in the range [0, 256^size). // To get down to the actual range, we could just compute random % range. diff --git a/Userland/Libraries/LibCrypto/PK/Code/EMSA_PSS.h b/Userland/Libraries/LibCrypto/PK/Code/EMSA_PSS.h index 894e857c60..5b082a921d 100644 --- a/Userland/Libraries/LibCrypto/PK/Code/EMSA_PSS.h +++ b/Userland/Libraries/LibCrypto/PK/Code/EMSA_PSS.h @@ -39,7 +39,7 @@ public: auto em_length = (em_bits + 7) / 8; u8 salt[SaltLength]; - fill_with_random(salt, SaltLength); + fill_with_random(salt); if (em_length < hash_length + SaltLength + 2) { dbgln("Ooops...encoding error"); diff --git a/Userland/Libraries/LibCrypto/PK/RSA.cpp b/Userland/Libraries/LibCrypto/PK/RSA.cpp index cfa86aec73..c5343a5613 100644 --- a/Userland/Libraries/LibCrypto/PK/RSA.cpp +++ b/Userland/Libraries/LibCrypto/PK/RSA.cpp @@ -343,12 +343,13 @@ void RSA_PKCS1_EME::encrypt(ReadonlyBytes in, Bytes& out) Vector ps; ps.resize(ps_length); - fill_with_random(ps.data(), ps_length); + fill_with_random(ps); // since fill_with_random can create zeros (shocking!) // we have to go through and un-zero the zeros - for (size_t i = 0; i < ps_length; ++i) + for (size_t i = 0; i < ps_length; ++i) { while (!ps[i]) - fill_with_random(ps.span().offset(i), 1); + ps[i] = get_random(); + } u8 paddings[] { 0x00, 0x02 }; diff --git a/Userland/Libraries/LibTLS/Handshake.cpp b/Userland/Libraries/LibTLS/Handshake.cpp index 1cb0aa420c..b0ae6891a0 100644 --- a/Userland/Libraries/LibTLS/Handshake.cpp +++ b/Userland/Libraries/LibTLS/Handshake.cpp @@ -18,7 +18,7 @@ namespace TLS { ByteBuffer TLSv12::build_hello() { - fill_with_random(&m_context.local_random, 32); + fill_with_random(m_context.local_random); auto packet_version = (u16)m_context.options.version; auto version = (u16)m_context.options.version; diff --git a/Userland/Libraries/LibTLS/HandshakeClient.cpp b/Userland/Libraries/LibTLS/HandshakeClient.cpp index f86b30af7e..ec4e1a44df 100644 --- a/Userland/Libraries/LibTLS/HandshakeClient.cpp +++ b/Userland/Libraries/LibTLS/HandshakeClient.cpp @@ -157,7 +157,7 @@ void TLSv12::build_rsa_pre_master_secret(PacketBuilder& builder) u8 random_bytes[48]; size_t bytes = 48; - fill_with_random(random_bytes, bytes); + fill_with_random(random_bytes); // remove zeros from the random bytes for (size_t i = 0; i < bytes; ++i) { diff --git a/Userland/Libraries/LibTLS/Record.cpp b/Userland/Libraries/LibTLS/Record.cpp index 18a7f3b963..ab0f2e831f 100644 --- a/Userland/Libraries/LibTLS/Record.cpp +++ b/Userland/Libraries/LibTLS/Record.cpp @@ -159,7 +159,7 @@ void TLSv12::update_packet(ByteBuffer& packet) u8 iv[16]; Bytes iv_bytes { iv, 16 }; Bytes { m_context.crypto.local_aead_iv, 4 }.copy_to(iv_bytes); - fill_with_random(iv_bytes.offset(4), 8); + fill_with_random(iv_bytes.slice(4, 8)); memset(iv_bytes.offset(12), 0, 4); // write the random part of the iv out @@ -207,7 +207,7 @@ void TLSv12::update_packet(ByteBuffer& packet) VERIFY_NOT_REACHED(); } auto iv = iv_buffer_result.release_value(); - fill_with_random(iv.data(), iv.size()); + fill_with_random(iv); // write it into the ciphertext portion of the message ct.overwrite(header_size, iv.data(), iv.size()); diff --git a/Userland/Libraries/LibWeb/Crypto/Crypto.cpp b/Userland/Libraries/LibWeb/Crypto/Crypto.cpp index df08060f9c..935ea7b76e 100644 --- a/Userland/Libraries/LibWeb/Crypto/Crypto.cpp +++ b/Userland/Libraries/LibWeb/Crypto/Crypto.cpp @@ -61,7 +61,7 @@ WebIDL::ExceptionOr Crypto::get_random_values(JS::Value array) const // FIXME: Handle SharedArrayBuffers // 3. Overwrite all elements of array with cryptographically strong random values of the appropriate type. - fill_with_random(typed_array.viewed_array_buffer()->buffer().data(), typed_array.viewed_array_buffer()->buffer().size()); + fill_with_random(typed_array.viewed_array_buffer()->buffer()); // 4. Return array. return array; @@ -88,7 +88,7 @@ ErrorOr generate_random_uuid() u8 bytes[16]; // 2. Fill bytes with cryptographically secure random bytes. - fill_with_random(bytes, 16); + fill_with_random(bytes); // 3. Set the 4 most significant bits of bytes[6], which represent the UUID version, to 0100. bytes[6] &= ~(1 << 7); diff --git a/Userland/Libraries/LibWebSocket/WebSocket.cpp b/Userland/Libraries/LibWebSocket/WebSocket.cpp index 6d7ac8630c..4be5b4613e 100644 --- a/Userland/Libraries/LibWebSocket/WebSocket.cpp +++ b/Userland/Libraries/LibWebSocket/WebSocket.cpp @@ -184,7 +184,7 @@ void WebSocket::send_client_handshake() // 7. 16-byte nonce encoded as Base64 u8 nonce_data[16]; - fill_with_random(nonce_data, 16); + fill_with_random(nonce_data); // FIXME: change to TRY() and make method fallible m_websocket_key = MUST(encode_base64({ nonce_data, 16 })).to_deprecated_string(); builder.appendff("Sec-WebSocket-Key: {}\r\n", m_websocket_key); @@ -579,7 +579,7 @@ void WebSocket::send_frame(WebSocket::OpCode op_code, ReadonlyBytes payload, boo // > Clients MUST choose a new masking key for each frame, using an algorithm // > that cannot be predicted by end applications that provide data u8 masking_key[4]; - fill_with_random(masking_key, 4); + fill_with_random(masking_key); m_impl->send(ReadonlyBytes(masking_key, 4)); // don't try to send empty payload if (payload.size() == 0) diff --git a/Userland/Utilities/useradd.cpp b/Userland/Utilities/useradd.cpp index 1eb2fe26ac..1bb89a444a 100644 --- a/Userland/Utilities/useradd.cpp +++ b/Userland/Utilities/useradd.cpp @@ -136,7 +136,7 @@ ErrorOr serenity_main(Main::Arguments arguments) auto get_salt = []() -> ErrorOr { char random_data[12]; - fill_with_random(random_data, sizeof(random_data)); + fill_with_random({ random_data, sizeof(random_data) }); StringBuilder builder; builder.append("$5$"sv);