From f14e723ebd970892c6b980c4f1bb838aec4f7aff Mon Sep 17 00:00:00 2001 From: Wesley Shillingford Date: Thu, 21 Feb 2019 17:44:02 +0000 Subject: [PATCH] Use a single synchronised random pool across all threads (#1758) * Use a single synchronised random pool across all threads * Encapsulate mutex * Remove rpc_secure.cpp changes made from clang-format * Fix qt build * Fix nano_wallet build --- nano/core_test/block_store.cpp | 2 +- nano/core_test/wallet.cpp | 2 +- nano/core_test/work_pool.cpp | 2 +- nano/lib/blocks.cpp | 2 +- nano/lib/interface.cpp | 4 ++-- nano/lib/numbers.cpp | 21 ++++++++++++++++++++- nano/lib/numbers.hpp | 27 ++++++++++++++++++++++++--- nano/lib/work.cpp | 2 +- nano/nano_wallet/entry.cpp | 4 ++-- nano/node/bootstrap.cpp | 2 +- nano/node/lmdb.cpp | 6 +++--- nano/node/node.cpp | 2 +- nano/node/nodeconfig.cpp | 2 +- nano/node/openclwork.cpp | 2 +- nano/node/peers.cpp | 8 ++++---- nano/node/testing.cpp | 14 +++++++------- nano/node/wallet.cpp | 12 ++++++------ nano/qt_system/entry.cpp | 2 +- nano/secure/common.cpp | 13 +++++++------ nano/slow_test/node.cpp | 4 ++-- 20 files changed, 87 insertions(+), 46 deletions(-) diff --git a/nano/core_test/block_store.cpp b/nano/core_test/block_store.cpp index c3b1b112..97796ce1 100644 --- a/nano/core_test/block_store.cpp +++ b/nano/core_test/block_store.cpp @@ -666,7 +666,7 @@ TEST (block_store, large_iteration) { auto transaction (store.tx_begin (true)); nano::account account; - nano::random_pool.GenerateBlock (account.bytes.data (), account.bytes.size ()); + nano::random_pool::generate_block (account.bytes.data (), account.bytes.size ()); accounts1.insert (account); store.account_put (transaction, account, nano::account_info ()); } diff --git a/nano/core_test/wallet.cpp b/nano/core_test/wallet.cpp index ef1a99d4..3b984018 100644 --- a/nano/core_test/wallet.cpp +++ b/nano/core_test/wallet.cpp @@ -827,7 +827,7 @@ TEST (wallet, version_3_upgrade) nano::keypair key; nano::raw_key seed; nano::uint256_union seed_ciphertext; - nano::random_pool.GenerateBlock (seed.data.bytes.data (), seed.data.bytes.size ()); + nano::random_pool::generate_block (seed.data.bytes.data (), seed.data.bytes.size ()); nano::raw_key password_l; nano::wallet_value value (wallet->store.entry_get_raw (transaction, nano::wallet_store::wallet_key_special)); nano::raw_key kdf; diff --git a/nano/core_test/work_pool.cpp b/nano/core_test/work_pool.cpp index 877e741e..a2cfbed1 100644 --- a/nano/core_test/work_pool.cpp +++ b/nano/core_test/work_pool.cpp @@ -76,7 +76,7 @@ TEST (work, DISABLED_opencl) nano::uint256_union root; for (auto i (0); i < 1; ++i) { - nano::random_pool.GenerateBlock (root.bytes.data (), root.bytes.size ()); + nano::random_pool::generate_block (root.bytes.data (), root.bytes.size ()); auto result (pool.generate (root)); ASSERT_FALSE (nano::work_validate (root, result)); } diff --git a/nano/lib/blocks.cpp b/nano/lib/blocks.cpp index 646769c9..260df3b1 100644 --- a/nano/lib/blocks.cpp +++ b/nano/lib/blocks.cpp @@ -1590,7 +1590,7 @@ std::shared_ptr nano::block_uniquer::unique (std::shared_ptr::max () > blocks.size ()); for (auto i (0); i < cleanup_count && blocks.size () > 0; ++i) { - auto random_offset (nano::random_pool.GenerateWord32 (0, static_cast (blocks.size () - 1))); + auto random_offset (nano::random_pool::generate_word32 (0, static_cast (blocks.size () - 1))); auto existing (std::next (blocks.begin (), random_offset)); if (existing == blocks.end ()) { diff --git a/nano/lib/interface.cpp b/nano/lib/interface.cpp index 9b0bc644..0ea23266 100644 --- a/nano/lib/interface.cpp +++ b/nano/lib/interface.cpp @@ -70,7 +70,7 @@ int xrb_valid_address (const char * account_a) void xrb_generate_random (xrb_uint256 seed) { auto & number (*reinterpret_cast (seed)); - nano::random_pool.GenerateBlock (number.bytes.data (), number.bytes.size ()); + nano::random_pool::generate_block (number.bytes.data (), number.bytes.size ()); } void xrb_seed_key (xrb_uint256 seed, int index, xrb_uint256 destination) @@ -142,7 +142,7 @@ char * xrb_work_transaction (const char * transaction) #include void ed25519_randombytes_unsafe (void * out, size_t outlen) { - nano::random_pool.GenerateBlock (reinterpret_cast (out), outlen); + nano::random_pool::generate_block (reinterpret_cast (out), outlen); } void ed25519_hash_init (ed25519_hash_context * ctx) { diff --git a/nano/lib/numbers.cpp b/nano/lib/numbers.cpp index c32eb6fe..9808a757 100644 --- a/nano/lib/numbers.cpp +++ b/nano/lib/numbers.cpp @@ -8,7 +8,26 @@ #include #include -thread_local CryptoPP::AutoSeededRandomPool nano::random_pool; +std::mutex nano::random_pool::mutex; +CryptoPP::AutoSeededRandomPool nano::random_pool::pool; + +void nano::random_pool::generate_block (unsigned char * output, size_t size) +{ + std::lock_guard lk (mutex); + pool.GenerateBlock (output, size); +} + +unsigned nano::random_pool::generate_word32 (unsigned min, unsigned max) +{ + std::lock_guard lk (mutex); + return pool.GenerateWord32 (min, max); +} + +unsigned char nano::random_pool::generate_byte () +{ + std::lock_guard lk (mutex); + return pool.GenerateByte (); +} namespace { diff --git a/nano/lib/numbers.hpp b/nano/lib/numbers.hpp index ef924917..e06be154 100644 --- a/nano/lib/numbers.hpp +++ b/nano/lib/numbers.hpp @@ -6,9 +6,30 @@ namespace nano { -// Random pool used by Nano. -// This must be thread_local as long as the AutoSeededRandomPool implementation requires it -extern thread_local CryptoPP::AutoSeededRandomPool random_pool; +/** While this uses CryptoPP do not call any of these functions from global scope, as they depend on global variables inside the CryptoPP library which may not have been initialized yet due to an undefined order for globals in different translation units. To make sure this is not an issue, there should be no ASAN warnings at startup on Mac/Clang in the CryptoPP files. */ +class random_pool +{ +public: + static void generate_block (unsigned char * output, size_t size); + static unsigned generate_word32 (unsigned min, unsigned max); + static unsigned char generate_byte (); + + template + static void shuffle (Iter begin, Iter end) + { + std::lock_guard lk (mutex); + pool.Shuffle (begin, end); + } + + random_pool () = delete; + random_pool (random_pool const &) = delete; + random_pool & operator= (random_pool const &) = delete; + +private: + static std::mutex mutex; + static CryptoPP::AutoSeededRandomPool pool; +}; + using uint128_t = boost::multiprecision::uint128_t; using uint256_t = boost::multiprecision::uint256_t; using uint512_t = boost::multiprecision::uint512_t; diff --git a/nano/lib/work.cpp b/nano/lib/work.cpp index 7f73307b..d28c2747 100644 --- a/nano/lib/work.cpp +++ b/nano/lib/work.cpp @@ -64,7 +64,7 @@ void nano::work_pool::loop (uint64_t thread) { // Quick RNG for work attempts. xorshift1024star rng; - nano::random_pool.GenerateBlock (reinterpret_cast (rng.s.data ()), rng.s.size () * sizeof (decltype (rng.s)::value_type)); + nano::random_pool::generate_block (reinterpret_cast (rng.s.data ()), rng.s.size () * sizeof (decltype (rng.s)::value_type)); uint64_t work; uint64_t output; blake2b_state hash; diff --git a/nano/nano_wallet/entry.cpp b/nano/nano_wallet/entry.cpp index 43f51d50..402a54df 100644 --- a/nano/nano_wallet/entry.cpp +++ b/nano/nano_wallet/entry.cpp @@ -21,7 +21,7 @@ public: rpc_enable (false), opencl_enable (false) { - nano::random_pool.GenerateBlock (wallet.bytes.data (), wallet.bytes.size ()); + nano::random_pool::generate_block (wallet.bytes.data (), wallet.bytes.size ()); assert (!wallet.is_zero ()); } bool upgrade_json (unsigned version_a, nano::jsonconfig & json) @@ -114,7 +114,7 @@ public: } if (wallet.is_zero ()) { - nano::random_pool.GenerateBlock (wallet.bytes.data (), wallet.bytes.size ()); + nano::random_pool::generate_block (wallet.bytes.data (), wallet.bytes.size ()); upgraded_a = true; } } diff --git a/nano/node/bootstrap.cpp b/nano/node/bootstrap.cpp index aec94c23..3ecf18e3 100644 --- a/nano/node/bootstrap.cpp +++ b/nano/node/bootstrap.cpp @@ -1058,7 +1058,7 @@ void nano::bootstrap_attempt::run () { for (auto i = static_cast (pulls.size () - 1); i > 0; --i) { - auto k = nano::random_pool.GenerateWord32 (0, i); + auto k = nano::random_pool::generate_word32 (0, i); std::swap (pulls[i], pulls[k]); } } diff --git a/nano/node/lmdb.cpp b/nano/node/lmdb.cpp index cd7b749e..feb184c9 100644 --- a/nano/node/lmdb.cpp +++ b/nano/node/lmdb.cpp @@ -880,7 +880,7 @@ nano::raw_key nano::mdb_store::get_node_id (nano::transaction const & transactio } if (error) { - nano::random_pool.GenerateBlock (node_id.data.bytes.data (), node_id.data.bytes.size ()); + nano::random_pool::generate_block (node_id.data.bytes.data (), node_id.data.bytes.size ()); error = mdb_put (env.tx (transaction_a), meta, nano::mdb_val (node_id_mdb_key), nano::mdb_val (node_id.data), 0); } assert (!error); @@ -1471,7 +1471,7 @@ template std::shared_ptr nano::mdb_store::block_random (nano::transaction const & transaction_a, MDB_dbi database) { nano::block_hash hash; - nano::random_pool.GenerateBlock (hash.bytes.data (), hash.bytes.size ()); + nano::random_pool::generate_block (hash.bytes.data (), hash.bytes.size ()); nano::store_iterator> existing (std::make_unique>> (transaction_a, database, nano::mdb_val (hash))); if (existing == nano::store_iterator> (nullptr)) { @@ -1486,7 +1486,7 @@ std::shared_ptr nano::mdb_store::block_random (nano::transaction co { auto count (block_count (transaction_a)); release_assert (std::numeric_limits::max () > count.sum ()); - auto region = static_cast (nano::random_pool.GenerateWord32 (0, static_cast (count.sum () - 1))); + auto region = static_cast (nano::random_pool::generate_word32 (0, static_cast (count.sum () - 1))); std::shared_ptr result; if (region < count.send) { diff --git a/nano/node/node.cpp b/nano/node/node.cpp index cf15cbae..e0e82b09 100644 --- a/nano/node/node.cpp +++ b/nano/node/node.cpp @@ -439,7 +439,7 @@ void nano::network::broadcast_confirm_req (std::shared_ptr block_a) * if the votes for a block have not arrived in time. */ const size_t max_endpoints = 32; - random_pool.Shuffle (list->begin (), list->end ()); + random_pool::shuffle (list->begin (), list->end ()); if (list->size () > max_endpoints) { list->erase (list->begin () + max_endpoints, list->end ()); diff --git a/nano/node/nodeconfig.cpp b/nano/node/nodeconfig.cpp index 29f2310d..37860510 100644 --- a/nano/node/nodeconfig.cpp +++ b/nano/node/nodeconfig.cpp @@ -393,7 +393,7 @@ nano::error nano::node_config::deserialize_json (bool & upgraded_a, nano::jsonco nano::account nano::node_config::random_representative () { assert (preconfigured_representatives.size () > 0); - size_t index (nano::random_pool.GenerateWord32 (0, static_cast (preconfigured_representatives.size () - 1))); + size_t index (nano::random_pool::generate_word32 (0, static_cast (preconfigured_representatives.size () - 1))); auto result (preconfigured_representatives[index]); return result; } diff --git a/nano/node/openclwork.cpp b/nano/node/openclwork.cpp index 854243fd..343ac3b6 100644 --- a/nano/node/openclwork.cpp +++ b/nano/node/openclwork.cpp @@ -546,7 +546,7 @@ logging (logging_a) error_a |= config.device >= platform.devices.size (); if (!error_a) { - nano::random_pool.GenerateBlock (reinterpret_cast (rand.s.data ()), rand.s.size () * sizeof (decltype (rand.s)::value_type)); + nano::random_pool::generate_block (reinterpret_cast (rand.s.data ()), rand.s.size () * sizeof (decltype (rand.s)::value_type)); std::array selected_devices; selected_devices[0] = platform.devices[config.device]; cl_context_properties contextProperties[] = { diff --git a/nano/node/peers.cpp b/nano/node/peers.cpp index 4cc1847f..c91f8f07 100644 --- a/nano/node/peers.cpp +++ b/nano/node/peers.cpp @@ -98,7 +98,7 @@ std::deque nano::peer_container::list () { result.push_back (i->endpoint); } - random_pool.Shuffle (result.begin (), result.end ()); + nano::random_pool::shuffle (result.begin (), result.end ()); return result; } @@ -110,7 +110,7 @@ std::vector nano::peer_container::list_vector (size_t co { result.push_back (*i); } - random_pool.Shuffle (result.begin (), result.end ()); + random_pool::shuffle (result.begin (), result.end ()); if (result.size () > count_a) { result.resize (count_a, nano::peer_information (nano::endpoint{}, 0)); @@ -153,7 +153,7 @@ boost::optional nano::peer_container::assign_syn_cookie (na if (syn_cookies.find (endpoint) == syn_cookies.end ()) { nano::uint256_union query; - random_pool.GenerateBlock (query.bytes.data (), query.bytes.size ()); + random_pool::generate_block (query.bytes.data (), query.bytes.size ()); syn_cookie_info info{ query, std::chrono::steady_clock::now () }; syn_cookies[endpoint] = info; ++ip_cookies; @@ -201,7 +201,7 @@ std::unordered_set nano::peer_container::random_set (size_t coun { for (auto i (0); i < random_cutoff && result.size () < count_a; ++i) { - auto index (random_pool.GenerateWord32 (0, static_cast (peers_size - 1))); + auto index (nano::random_pool::generate_word32 (0, static_cast (peers_size - 1))); result.insert (peers.get<3> ()[index].endpoint); } } diff --git a/nano/node/testing.cpp b/nano/node/testing.cpp index e093423f..aa9a5e27 100644 --- a/nano/node/testing.cpp +++ b/nano/node/testing.cpp @@ -36,7 +36,7 @@ work (1, nullptr) assert (!init.error ()); node->start (); nano::uint256_union wallet; - nano::random_pool.GenerateBlock (wallet.bytes.data (), wallet.bytes.size ()); + nano::random_pool::generate_block (wallet.bytes.data (), wallet.bytes.size ()); node->wallets.create (wallet); nodes.push_back (node); } @@ -166,7 +166,7 @@ void nano::system::generate_rollback (nano::node & node_a, std::vector::max () > accounts_a.size ()); - auto index (random_pool.GenerateWord32 (0, static_cast (accounts_a.size () - 1))); + auto index (random_pool::generate_word32 (0, static_cast (accounts_a.size () - 1))); auto account (accounts_a[index]); nano::account_info info; auto error (node_a.store.account_get (transaction, account, info)); @@ -189,7 +189,7 @@ void nano::system::generate_receive (nano::node & node_a) { auto transaction (node_a.store.tx_begin_read ()); nano::uint256_union random_block; - random_pool.GenerateBlock (random_block.bytes.data (), sizeof (random_block.bytes)); + random_pool::generate_block (random_block.bytes.data (), sizeof (random_block.bytes)); auto i (node_a.store.pending_begin (transaction, nano::pending_key (random_block, 0))); if (i != node_a.store.pending_end ()) { @@ -206,7 +206,7 @@ void nano::system::generate_receive (nano::node & node_a) void nano::system::generate_activity (nano::node & node_a, std::vector & accounts_a) { - auto what (random_pool.GenerateByte ()); + auto what (random_pool::generate_byte ()); if (what < 0x1) { generate_rollback (node_a, accounts_a); @@ -236,7 +236,7 @@ void nano::system::generate_activity (nano::node & node_a, std::vector & accounts_a) { assert (std::numeric_limits::max () > accounts_a.size ()); - auto index (random_pool.GenerateWord32 (0, static_cast (accounts_a.size () - 1))); + auto index (random_pool::generate_word32 (0, static_cast (accounts_a.size () - 1))); auto result (accounts_a[index]); return result; } @@ -246,7 +246,7 @@ nano::uint128_t nano::system::get_random_amount (nano::transaction const & trans nano::uint128_t balance (node_a.ledger.account_balance (transaction_a, account_a)); std::string balance_text (balance.convert_to ()); nano::uint128_union random_amount; - random_pool.GenerateBlock (random_amount.bytes.data (), sizeof (random_amount.bytes)); + nano::random_pool::generate_block (random_amount.bytes.data (), sizeof (random_amount.bytes)); auto result (((nano::uint256_t{ random_amount.number () } * balance) / nano::uint256_t{ std::numeric_limits::max () }).convert_to ()); std::string text (result.convert_to ()); return result; @@ -259,7 +259,7 @@ void nano::system::generate_send_existing (nano::node & node_a, std::vector entry (node_a.store.latest_begin (transaction, account)); if (entry == node_a.store.latest_end ()) diff --git a/nano/node/wallet.cpp b/nano/node/wallet.cpp index 784d7c2d..dc8bfcfc 100644 --- a/nano/node/wallet.cpp +++ b/nano/node/wallet.cpp @@ -212,7 +212,7 @@ nano::fan::fan (nano::uint256_union const & key, size_t count_a) for (auto i (1); i < count_a; ++i) { std::unique_ptr entry (new nano::uint256_union); - random_pool.GenerateBlock (entry->bytes.data (), entry->bytes.size ()); + nano::random_pool::generate_block (entry->bytes.data (), entry->bytes.size ()); *first ^= *entry; values.push_back (std::move (entry)); } @@ -334,11 +334,11 @@ kdf (kdf_a) { version_put (transaction_a, version_current); nano::uint256_union salt_l; - random_pool.GenerateBlock (salt_l.bytes.data (), salt_l.bytes.size ()); + random_pool::generate_block (salt_l.bytes.data (), salt_l.bytes.size ()); entry_put_raw (transaction_a, nano::wallet_store::salt_special, nano::wallet_value (salt_l, 0)); // Wallet key is a fixed random key that encrypts all entries nano::raw_key wallet_key; - random_pool.GenerateBlock (wallet_key.data.bytes.data (), sizeof (wallet_key.data.bytes)); + random_pool::generate_block (wallet_key.data.bytes.data (), sizeof (wallet_key.data.bytes)); nano::raw_key password_l; password_l.data.clear (); password.value_set (password_l); @@ -356,7 +356,7 @@ kdf (kdf_a) entry_put_raw (transaction_a, nano::wallet_store::check_special, nano::wallet_value (check, 0)); entry_put_raw (transaction_a, nano::wallet_store::representative_special, nano::wallet_value (representative_a, 0)); nano::raw_key seed; - random_pool.GenerateBlock (seed.data.bytes.data (), seed.data.bytes.size ()); + random_pool::generate_block (seed.data.bytes.data (), seed.data.bytes.size ()); seed_set (transaction_a, seed); entry_put_raw (transaction_a, nano::wallet_store::deterministic_index_special, nano::wallet_value (nano::uint256_union (0), 0)); } @@ -685,7 +685,7 @@ void nano::wallet_store::upgrade_v2_v3 (nano::transaction const & transaction_a) { assert (version (transaction_a) == 2); nano::raw_key seed; - random_pool.GenerateBlock (seed.data.bytes.data (), seed.data.bytes.size ()); + random_pool::generate_block (seed.data.bytes.data (), seed.data.bytes.size ()); seed_set (transaction_a, seed); entry_put_raw (transaction_a, nano::wallet_store::deterministic_index_special, nano::wallet_value (nano::uint256_union (0), 0)); version_put (transaction_a, 3); @@ -881,7 +881,7 @@ bool nano::wallet::import (std::string const & json_a, std::string const & passw { auto transaction (wallets.tx_begin_write ()); nano::uint256_union id; - random_pool.GenerateBlock (id.bytes.data (), id.bytes.size ()); + random_pool::generate_block (id.bytes.data (), id.bytes.size ()); temp.reset (new nano::wallet_store (error, wallets.node.wallets.kdf, transaction, 0, 1, id.to_string (), json_a)); } if (!error) diff --git a/nano/qt_system/entry.cpp b/nano/qt_system/entry.cpp index 442dab49..babc8c76 100644 --- a/nano/qt_system/entry.cpp +++ b/nano/qt_system/entry.cpp @@ -17,7 +17,7 @@ int main (int argc, char ** argv) for (auto i (0); i < count; ++i) { nano::uint256_union wallet_id; - nano::random_pool.GenerateBlock (wallet_id.bytes.data (), wallet_id.bytes.size ()); + nano::random_pool::generate_block (wallet_id.bytes.data (), wallet_id.bytes.size ()); auto wallet (system.nodes[i]->wallets.create (wallet_id)); nano::keypair key; wallet->insert_adhoc (key.prv); diff --git a/nano/secure/common.cpp b/nano/secure/common.cpp index f8d98ad4..cdf40e68 100644 --- a/nano/secure/common.cpp +++ b/nano/secure/common.cpp @@ -80,13 +80,11 @@ public: nano::account const & not_an_account () { - static bool is_initialized = false; - static std::mutex mutex; std::lock_guard lk (mutex); if (!is_initialized) { - // Randomly generating these mean no two nodes will ever have the same sentinel values which protects against some insecure algorithms - nano::random_pool.GenerateBlock (not_an_account_m.bytes.data (), not_an_account_m.bytes.size ()); + // Randomly generating this means that no two nodes will ever have the same sentinel value which protects against some insecure algorithms + nano::random_pool::generate_block (not_an_account_m.bytes.data (), not_an_account_m.bytes.size ()); is_initialized = true; } return not_an_account_m; @@ -94,6 +92,8 @@ public: private: nano::account not_an_account_m; + std::mutex mutex; + bool is_initialized{ false }; }; ledger_constants globals; } @@ -124,7 +124,7 @@ nano::account const & nano::not_an_account () // Create a new random keypair nano::keypair::keypair () { - random_pool.GenerateBlock (prv.data.bytes.data (), prv.data.bytes.size ()); + random_pool::generate_block (prv.data.bytes.data (), prv.data.bytes.size ()); ed25519_publickey (prv.data.bytes.data (), pub.bytes.data ()); } @@ -767,7 +767,8 @@ std::shared_ptr nano::vote_uniquer::unique (std::shared_ptr::max () > votes.size ()); for (auto i (0); i < cleanup_count && votes.size () > 0; ++i) { - auto random_offset (nano::random_pool.GenerateWord32 (0, static_cast (votes.size () - 1))); + auto random_offset = nano::random_pool::generate_word32 (0, static_cast (votes.size () - 1)); + auto existing (std::next (votes.begin (), random_offset)); if (existing == votes.end ()) { diff --git a/nano/slow_test/node.cpp b/nano/slow_test/node.cpp index 8683a62d..3bf832be 100644 --- a/nano/slow_test/node.cpp +++ b/nano/slow_test/node.cpp @@ -153,7 +153,7 @@ TEST (store, load) for (auto j (0); j != 10; ++j) { nano::block_hash hash; - nano::random_pool.GenerateBlock (hash.bytes.data (), hash.bytes.size ()); + nano::random_pool::generate_block (hash.bytes.data (), hash.bytes.size ()); system.nodes[0]->store.account_put (transaction, hash, nano::account_info ()); } } @@ -323,7 +323,7 @@ TEST (broadcast, sqrt_broadcast_simulate) for (auto j (0); j != broadcast_count; ++j) { ++message_count; - auto entry (nano::random_pool.GenerateWord32 (0, node_count - 1)); + auto entry (nano::random_pool::generate_word32 (0, node_count - 1)); switch (nodes[entry]) { case 0: