From 6cfdc5b104caf9952393f9dac2a36539d964077f Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Sun, 10 Mar 2024 15:16:20 -0400 Subject: [PATCH] random: convert XoRoShiRo128PlusPlus into full RNG Convert XoRoShiRo128PlusPlus into a full RandomMixin-based RNG class, providing all utility functionality that FastRandomContext has. In doing so, it is renamed to InsecureRandomContext, highlighting its non-cryptographic nature. To do this, a fillrand fallback is added to RandomMixin (where it is used by InsecureRandomContext), but FastRandomContext still uses its own fillrand. --- src/random.h | 45 ++++++++++++------- src/test/fuzz/bip324.cpp | 8 ++-- src/test/fuzz/bitset.cpp | 14 +++--- src/test/fuzz/crypto_chacha20.cpp | 22 ++------- src/test/fuzz/p2p_transport_serialization.cpp | 16 +++---- src/test/fuzz/poolresource.cpp | 35 ++------------- src/test/fuzz/vecdeque.cpp | 20 ++++----- src/test/random_tests.cpp | 4 +- test/sanitizer_suppressions/ubsan | 6 +-- 9 files changed, 68 insertions(+), 102 deletions(-) diff --git a/src/random.h b/src/random.h index e8708671704..cdf2f3a66d2 100644 --- a/src/random.h +++ b/src/random.h @@ -147,8 +147,6 @@ template concept RandomNumberGenerator = requires(T& rng, Span s) { // A random number generator must provide rand64(). { rng.rand64() } noexcept -> std::same_as; - // A random number generator must provide randfill(Span). - { rng.fillrand(s) } noexcept; // A random number generator must derive from RandomMixin, which adds other rand* functions. requires std::derived_from, RandomMixin>>; }; @@ -261,6 +259,25 @@ public: } } + /** Fill a Span with random bytes. */ + void fillrand(Span span) noexcept + { + while (span.size() >= 8) { + uint64_t gen = Impl().rand64(); + WriteLE64(UCharCast(span.data()), gen); + span = span.subspan(8); + } + if (span.size() >= 4) { + uint32_t gen = Impl().rand32(); + WriteLE32(UCharCast(span.data()), gen); + span = span.subspan(4); + } + while (span.size()) { + span[0] = std::byte(Impl().template randbits<8>()); + span = span.subspan(1); + } + } + /** Generate random bytes. */ template std::vector randbytes(size_t len) noexcept @@ -345,19 +362,19 @@ public: return ReadLE64(UCharCast(buf.data())); } - /** Fill a byte Span with random bytes. */ + /** Fill a byte Span with random bytes. This overrides the RandomMixin version. */ void fillrand(Span output) noexcept; }; /** xoroshiro128++ PRNG. Extremely fast, not appropriate for cryptographic purposes. * - * Memory footprint is 128bit, period is 2^128 - 1. + * Memory footprint is very small, period is 2^128 - 1. * This class is not thread-safe. * * Reference implementation available at https://prng.di.unimi.it/xoroshiro128plusplus.c * See https://prng.di.unimi.it/ */ -class XoRoShiRo128PlusPlus +class InsecureRandomContext : public RandomMixin { uint64_t m_s0; uint64_t m_s1; @@ -371,21 +388,19 @@ class XoRoShiRo128PlusPlus } public: - using result_type = uint64_t; - - constexpr explicit XoRoShiRo128PlusPlus(uint64_t seedval) noexcept + constexpr explicit InsecureRandomContext(uint64_t seedval) noexcept : m_s0(SplitMix64(seedval)), m_s1(SplitMix64(seedval)) {} // no copy - that is dangerous, we don't want accidentally copy the RNG and then have two streams // with exactly the same results. - XoRoShiRo128PlusPlus(const XoRoShiRo128PlusPlus&) = delete; - XoRoShiRo128PlusPlus& operator=(const XoRoShiRo128PlusPlus&) = delete; + InsecureRandomContext(const InsecureRandomContext&) = delete; + InsecureRandomContext& operator=(const InsecureRandomContext&) = delete; // allow moves - XoRoShiRo128PlusPlus(XoRoShiRo128PlusPlus&&) = default; - XoRoShiRo128PlusPlus& operator=(XoRoShiRo128PlusPlus&&) = default; + InsecureRandomContext(InsecureRandomContext&&) = default; + InsecureRandomContext& operator=(InsecureRandomContext&&) = default; - constexpr result_type operator()() noexcept + constexpr uint64_t rand64() noexcept { uint64_t s0 = m_s0, s1 = m_s1; const uint64_t result = std::rotl(s0 + s1, 17) + s0; @@ -394,10 +409,6 @@ public: m_s1 = std::rotl(s1, 28); return result; } - - static constexpr result_type min() noexcept { return std::numeric_limits::min(); } - static constexpr result_type max() noexcept { return std::numeric_limits::max(); } - static constexpr double entropy() noexcept { return 0.0; } }; /** More efficient than using std::shuffle on a FastRandomContext. diff --git a/src/test/fuzz/bip324.cpp b/src/test/fuzz/bip324.cpp index 06199168198..9892e7a81ce 100644 --- a/src/test/fuzz/bip324.cpp +++ b/src/test/fuzz/bip324.cpp @@ -56,7 +56,7 @@ FUZZ_TARGET(bip324_cipher_roundtrip, .init=Initialize) // (potentially buggy) edge cases triggered by specific values of contents/AAD, so we can avoid // reading the actual data for those from the fuzzer input (which would need large amounts of // data). - XoRoShiRo128PlusPlus rng(provider.ConsumeIntegral()); + InsecureRandomContext rng(provider.ConsumeIntegral()); // Compare session IDs and garbage terminators. assert(initiator.GetSessionID() == responder.GetSessionID()); @@ -79,10 +79,8 @@ FUZZ_TARGET(bip324_cipher_roundtrip, .init=Initialize) unsigned length_bits = 2 * ((mode >> 5) & 7); unsigned length = provider.ConsumeIntegralInRange(0, (1 << length_bits) - 1); // Generate aad and content. - std::vector aad(aad_length); - for (auto& val : aad) val = std::byte{(uint8_t)rng()}; - std::vector contents(length); - for (auto& val : contents) val = std::byte{(uint8_t)rng()}; + auto aad = rng.randbytes(aad_length); + auto contents = rng.randbytes(length); // Pick sides. auto& sender{from_init ? initiator : responder}; diff --git a/src/test/fuzz/bitset.cpp b/src/test/fuzz/bitset.cpp index cc3ed30f625..ce6be0499cf 100644 --- a/src/test/fuzz/bitset.cpp +++ b/src/test/fuzz/bitset.cpp @@ -29,7 +29,7 @@ void TestType(FuzzBufferType buffer) * bitsets and their simulations do not matter for the purpose of detecting edge cases, thus * these are taken from a deterministically-seeded RNG instead. To provide some level of * variation however, pick the seed based on the buffer size and size of the chosen bitset. */ - XoRoShiRo128PlusPlus rng(buffer.size() + 0x10000 * S::Size()); + InsecureRandomContext rng(buffer.size() + 0x10000 * S::Size()); using Sim = std::bitset; // Up to 4 real BitSets (initially 2). @@ -124,7 +124,7 @@ void TestType(FuzzBufferType buffer) sim[dest].reset(); real[dest] = S{}; for (unsigned i = 0; i < S::Size(); ++i) { - if (rng() & 1) { + if (rng.randbool()) { sim[dest][i] = true; real[dest].Set(i); } @@ -132,9 +132,9 @@ void TestType(FuzzBufferType buffer) break; } else if (dest < sim.size() && command-- == 0) { /* Assign initializer list. */ - unsigned r1 = rng() % S::Size(); - unsigned r2 = rng() % S::Size(); - unsigned r3 = rng() % S::Size(); + unsigned r1 = rng.randrange(S::Size()); + unsigned r2 = rng.randrange(S::Size()); + unsigned r3 = rng.randrange(S::Size()); compare_fn(dest); sim[dest].reset(); real[dest] = {r1, r2, r3}; @@ -166,8 +166,8 @@ void TestType(FuzzBufferType buffer) break; } else if (sim.size() < 4 && command-- == 0) { /* Construct with initializer list. */ - unsigned r1 = rng() % S::Size(); - unsigned r2 = rng() % S::Size(); + unsigned r1 = rng.randrange(S::Size()); + unsigned r2 = rng.randrange(S::Size()); sim.emplace_back(); sim.back().set(r1); sim.back().set(r2); diff --git a/src/test/fuzz/crypto_chacha20.cpp b/src/test/fuzz/crypto_chacha20.cpp index ff3d463ca51..d115a2b7e16 100644 --- a/src/test/fuzz/crypto_chacha20.cpp +++ b/src/test/fuzz/crypto_chacha20.cpp @@ -53,7 +53,7 @@ namespace once for a large block at once, and then the same data in chunks, comparing the outcome. - If UseCrypt, seeded Xoroshiro128++ output is used as input to Crypt(). + If UseCrypt, seeded InsecureRandomContext output is used as input to Crypt(). If not, Keystream() is used directly, or sequences of 0x00 are encrypted. */ template @@ -78,25 +78,11 @@ void ChaCha20SplitFuzz(FuzzedDataProvider& provider) data1.resize(total_bytes); data2.resize(total_bytes); - // If using Crypt(), initialize data1 and data2 with the same Xoroshiro128++ based + // If using Crypt(), initialize data1 and data2 with the same InsecureRandomContext based // stream. if constexpr (UseCrypt) { - uint64_t seed = provider.ConsumeIntegral(); - XoRoShiRo128PlusPlus rng(seed); - uint64_t bytes = 0; - while (bytes < (total_bytes & ~uint64_t{7})) { - uint64_t val = rng(); - WriteLE64(UCharCast(data1.data() + bytes), val); - WriteLE64(UCharCast(data2.data() + bytes), val); - bytes += 8; - } - if (bytes < total_bytes) { - std::byte valbytes[8]; - uint64_t val = rng(); - WriteLE64(UCharCast(valbytes), val); - std::copy(valbytes, valbytes + (total_bytes - bytes), data1.data() + bytes); - std::copy(valbytes, valbytes + (total_bytes - bytes), data2.data() + bytes); - } + InsecureRandomContext(provider.ConsumeIntegral()).fillrand(data1); + std::copy(data1.begin(), data1.end(), data2.begin()); } // Whether UseCrypt is used or not, the two byte arrays must match. diff --git a/src/test/fuzz/p2p_transport_serialization.cpp b/src/test/fuzz/p2p_transport_serialization.cpp index 5a26fb86ce1..93f77b6e5be 100644 --- a/src/test/fuzz/p2p_transport_serialization.cpp +++ b/src/test/fuzz/p2p_transport_serialization.cpp @@ -103,7 +103,7 @@ FUZZ_TARGET(p2p_transport_serialization, .init = initialize_p2p_transport_serial namespace { -template +template void SimulationTest(Transport& initiator, Transport& responder, R& rng, FuzzedDataProvider& provider) { // Simulation test with two Transport objects, which send messages to each other, with @@ -164,8 +164,7 @@ void SimulationTest(Transport& initiator, Transport& responder, R& rng, FuzzedDa // Determine size of message to send (limited to 75 kB for performance reasons). size_t size = provider.ConsumeIntegralInRange(0, 75000); // Get payload of message from RNG. - msg.data.resize(size); - for (auto& v : msg.data) v = uint8_t(rng()); + msg.data = rng.randbytes(size); // Return. return msg; }; @@ -336,7 +335,7 @@ std::unique_ptr MakeV1Transport(NodeId nodeid) noexcept return std::make_unique(nodeid); } -template +template std::unique_ptr MakeV2Transport(NodeId nodeid, bool initiator, RNG& rng, FuzzedDataProvider& provider) { // Retrieve key @@ -352,8 +351,7 @@ std::unique_ptr MakeV2Transport(NodeId nodeid, bool initiator, RNG& r } else { // If it's longer, generate it from the RNG. This avoids having large amounts of // (hopefully) irrelevant data needing to be stored in the fuzzer data. - garb.resize(garb_len); - for (auto& v : garb) v = uint8_t(rng()); + garb = rng.randbytes(garb_len); } // Retrieve entropy auto ent = provider.ConsumeBytes(32); @@ -377,7 +375,7 @@ FUZZ_TARGET(p2p_transport_bidirectional, .init = initialize_p2p_transport_serial { // Test with two V1 transports talking to each other. FuzzedDataProvider provider{buffer.data(), buffer.size()}; - XoRoShiRo128PlusPlus rng(provider.ConsumeIntegral()); + InsecureRandomContext rng(provider.ConsumeIntegral()); auto t1 = MakeV1Transport(NodeId{0}); auto t2 = MakeV1Transport(NodeId{1}); if (!t1 || !t2) return; @@ -388,7 +386,7 @@ FUZZ_TARGET(p2p_transport_bidirectional_v2, .init = initialize_p2p_transport_ser { // Test with two V2 transports talking to each other. FuzzedDataProvider provider{buffer.data(), buffer.size()}; - XoRoShiRo128PlusPlus rng(provider.ConsumeIntegral()); + InsecureRandomContext rng(provider.ConsumeIntegral()); auto t1 = MakeV2Transport(NodeId{0}, true, rng, provider); auto t2 = MakeV2Transport(NodeId{1}, false, rng, provider); if (!t1 || !t2) return; @@ -399,7 +397,7 @@ FUZZ_TARGET(p2p_transport_bidirectional_v1v2, .init = initialize_p2p_transport_s { // Test with a V1 initiator talking to a V2 responder. FuzzedDataProvider provider{buffer.data(), buffer.size()}; - XoRoShiRo128PlusPlus rng(provider.ConsumeIntegral()); + InsecureRandomContext rng(provider.ConsumeIntegral()); auto t1 = MakeV1Transport(NodeId{0}); auto t2 = MakeV2Transport(NodeId{1}, false, rng, provider); if (!t1 || !t2) return; diff --git a/src/test/fuzz/poolresource.cpp b/src/test/fuzz/poolresource.cpp index a3e48994c97..28bf7175c08 100644 --- a/src/test/fuzz/poolresource.cpp +++ b/src/test/fuzz/poolresource.cpp @@ -71,41 +71,14 @@ public: void RandomContentFill(Entry& entry) { - XoRoShiRo128PlusPlus rng(entry.seed); - auto ptr = entry.span.data(); - auto size = entry.span.size(); - - while (size >= 8) { - auto r = rng(); - std::memcpy(ptr, &r, 8); - size -= 8; - ptr += 8; - } - if (size > 0) { - auto r = rng(); - std::memcpy(ptr, &r, size); - } + InsecureRandomContext(entry.seed).fillrand(entry.span); } void RandomContentCheck(const Entry& entry) { - XoRoShiRo128PlusPlus rng(entry.seed); - auto ptr = entry.span.data(); - auto size = entry.span.size(); - - std::byte buf[8]; - while (size >= 8) { - auto r = rng(); - std::memcpy(buf, &r, 8); - assert(std::memcmp(buf, ptr, 8) == 0); - size -= 8; - ptr += 8; - } - if (size > 0) { - auto r = rng(); - std::memcpy(buf, &r, size); - assert(std::memcmp(buf, ptr, size) == 0); - } + std::vector expect(entry.span.size()); + InsecureRandomContext(entry.seed).fillrand(expect); + assert(entry.span == expect); } void Deallocate(const Entry& entry) diff --git a/src/test/fuzz/vecdeque.cpp b/src/test/fuzz/vecdeque.cpp index d94e6d895a0..3bb858ee8ae 100644 --- a/src/test/fuzz/vecdeque.cpp +++ b/src/test/fuzz/vecdeque.cpp @@ -28,7 +28,7 @@ void TestType(Span buffer, uint64_t rng_tweak) { FuzzedDataProvider provider(buffer.data(), buffer.size()); // Local RNG, only used for the seeds to initialize T objects with. - XoRoShiRo128PlusPlus rng(provider.ConsumeIntegral() ^ rng_tweak); + InsecureRandomContext rng(provider.ConsumeIntegral() ^ rng_tweak); // Real circular buffers. std::vector> real; @@ -175,7 +175,7 @@ void TestType(Span buffer, uint64_t rng_tweak) } if (existing_buffer_non_full && command-- == 0) { /* push_back() (copying) */ - tmp = T(rng()); + tmp = T(rng.rand64()); size_t old_size = real[idx].size(); size_t old_cap = real[idx].capacity(); real[idx].push_back(*tmp); @@ -191,7 +191,7 @@ void TestType(Span buffer, uint64_t rng_tweak) } if (existing_buffer_non_full && command-- == 0) { /* push_back() (moving) */ - tmp = T(rng()); + tmp = T(rng.rand64()); size_t old_size = real[idx].size(); size_t old_cap = real[idx].capacity(); sim[idx].push_back(*tmp); @@ -207,7 +207,7 @@ void TestType(Span buffer, uint64_t rng_tweak) } if (existing_buffer_non_full && command-- == 0) { /* emplace_back() */ - uint64_t seed{rng()}; + uint64_t seed{rng.rand64()}; size_t old_size = real[idx].size(); size_t old_cap = real[idx].capacity(); sim[idx].emplace_back(seed); @@ -223,7 +223,7 @@ void TestType(Span buffer, uint64_t rng_tweak) } if (existing_buffer_non_full && command-- == 0) { /* push_front() (copying) */ - tmp = T(rng()); + tmp = T(rng.rand64()); size_t old_size = real[idx].size(); size_t old_cap = real[idx].capacity(); real[idx].push_front(*tmp); @@ -239,7 +239,7 @@ void TestType(Span buffer, uint64_t rng_tweak) } if (existing_buffer_non_full && command-- == 0) { /* push_front() (moving) */ - tmp = T(rng()); + tmp = T(rng.rand64()); size_t old_size = real[idx].size(); size_t old_cap = real[idx].capacity(); sim[idx].push_front(*tmp); @@ -255,7 +255,7 @@ void TestType(Span buffer, uint64_t rng_tweak) } if (existing_buffer_non_full && command-- == 0) { /* emplace_front() */ - uint64_t seed{rng()}; + uint64_t seed{rng.rand64()}; size_t old_size = real[idx].size(); size_t old_cap = real[idx].capacity(); sim[idx].emplace_front(seed); @@ -271,7 +271,7 @@ void TestType(Span buffer, uint64_t rng_tweak) } if (existing_buffer_non_empty && command-- == 0) { /* front() [modifying] */ - tmp = T(rng()); + tmp = T(rng.rand64()); size_t old_size = real[idx].size(); assert(sim[idx].front() == real[idx].front()); sim[idx].front() = *tmp; @@ -281,7 +281,7 @@ void TestType(Span buffer, uint64_t rng_tweak) } if (existing_buffer_non_empty && command-- == 0) { /* back() [modifying] */ - tmp = T(rng()); + tmp = T(rng.rand64()); size_t old_size = real[idx].size(); assert(sim[idx].back() == real[idx].back()); sim[idx].back() = *tmp; @@ -291,7 +291,7 @@ void TestType(Span buffer, uint64_t rng_tweak) } if (existing_buffer_non_empty && command-- == 0) { /* operator[] [modifying] */ - tmp = T(rng()); + tmp = T(rng.rand64()); size_t pos = provider.ConsumeIntegralInRange(0, sim[idx].size() - 1); size_t old_size = real[idx].size(); assert(sim[idx][pos] == real[idx][pos]); diff --git a/src/test/random_tests.cpp b/src/test/random_tests.cpp index 09425c7cd7e..8392a2bc5d1 100644 --- a/src/test/random_tests.cpp +++ b/src/test/random_tests.cpp @@ -222,14 +222,14 @@ BOOST_AUTO_TEST_CASE(shuffle_stat_test) BOOST_AUTO_TEST_CASE(xoroshiro128plusplus_reference_values) { // numbers generated from reference implementation - XoRoShiRo128PlusPlus rng(0); + InsecureRandomContext rng(0); BOOST_TEST(0x6f68e1e7e2646ee1 == rng()); BOOST_TEST(0xbf971b7f454094ad == rng()); BOOST_TEST(0x48f2de556f30de38 == rng()); BOOST_TEST(0x6ea7c59f89bbfc75 == rng()); // seed with a random number - rng = XoRoShiRo128PlusPlus(0x1a26f3fa8546b47a); + rng = InsecureRandomContext(0x1a26f3fa8546b47a); BOOST_TEST(0xc8dc5e08d844ac7d == rng()); BOOST_TEST(0x5b5f1f6d499dad1b == rng()); BOOST_TEST(0xbeb0031f93313d6f == rng()); diff --git a/test/sanitizer_suppressions/ubsan b/test/sanitizer_suppressions/ubsan index 7d8b0fdfd03..94303f237c5 100644 --- a/test/sanitizer_suppressions/ubsan +++ b/test/sanitizer_suppressions/ubsan @@ -57,8 +57,8 @@ unsigned-integer-overflow:CBlockPolicyEstimator::processBlockTx unsigned-integer-overflow:TxConfirmStats::EstimateMedianVal unsigned-integer-overflow:prevector.h unsigned-integer-overflow:EvalScript -unsigned-integer-overflow:XoRoShiRo128PlusPlus::operator() -unsigned-integer-overflow:XoRoShiRo128PlusPlus::SplitMix64 +unsigned-integer-overflow:InsecureRandomContext::rand64 +unsigned-integer-overflow:InsecureRandomContext::SplitMix64 unsigned-integer-overflow:bitset_detail::PopCount implicit-integer-sign-change:CBlockPolicyEstimator::processBlockTx implicit-integer-sign-change:SetStdinEcho @@ -76,6 +76,6 @@ shift-base:arith_uint256.cpp shift-base:crypto/ shift-base:streams.h shift-base:FormatHDKeypath -shift-base:XoRoShiRo128PlusPlus::operator() +shift-base:InsecureRandomContext::rand64 shift-base:RandomMixin<*>::randbits shift-base:RandomMixin<*>::randbits<*>