diff options
author | anonimal <anonimal@getmonero.org> | 2019-09-07 01:35:47 +0000 |
---|---|---|
committer | anonimal <anonimal@getmonero.org> | 2019-09-08 01:14:39 +0000 |
commit | cd57a10c90134bbe67d35cbe71037d9ca42427ad (patch) | |
tree | 8d32f7dda88da1c1068f3cdcb3d9a79ac6dc39a3 | |
parent | tests: rct_mlsag: resolve CID 203914 (UNINIT_CTOR) (diff) | |
download | monero-cd57a10c90134bbe67d35cbe71037d9ca42427ad.tar.xz |
epee: abstract_tcp_server2: resolve CID 203919 (DC.WEAK_CRYPTO)
The problem actually exists in two parts:
1. When sending chunks over a connection, if the queue size is
greater than N, the seed is predictable across every monero node.
>"If rand() is used before any calls to srand(), rand() behaves as if
it was seeded with srand(1). Each time rand() is seeded with the same seed, it
must produce the same sequence of values."
2. The CID speaks for itself: "'rand' should not be used for security-related
applications, because linear congruential algorithms are too easy to break."
*But* this is an area of contention.
One could argue that a CSPRNG is warranted in order to fully mitigate any
potential timing attacks based on crafting chunk responses. Others could argue
that the existing LCG, or even an MTG, would suffice (if properly seeded). As a
compromise, I've used an MTG with a full bit space. This should give a healthy
balance of security and speed without relying on the existing crypto library
(which I'm told might break on some systems since epee is not (shouldn't be)
dependent upon the existing crypto library).
-rw-r--r-- | contrib/epee/include/net/abstract_tcp_server2.inl | 14 |
1 files changed, 13 insertions, 1 deletions
diff --git a/contrib/epee/include/net/abstract_tcp_server2.inl b/contrib/epee/include/net/abstract_tcp_server2.inl index 12a87071a..bc1898e0e 100644 --- a/contrib/epee/include/net/abstract_tcp_server2.inl +++ b/contrib/epee/include/net/abstract_tcp_server2.inl @@ -50,6 +50,8 @@ #include <sstream> #include <iomanip> #include <algorithm> +#include <functional> +#include <random> #undef MONERO_DEFAULT_LOG_CATEGORY #define MONERO_DEFAULT_LOG_CATEGORY "net" @@ -628,7 +630,17 @@ PRAGMA_WARNING_DISABLE_VS(4355) return false; // aborted }*/ - long int ms = 250 + (rand()%50); + using engine = std::mt19937; + + engine rng; + std::random_device dev; + std::seed_seq::result_type rand[engine::state_size]{}; // Use complete bit space + + std::generate_n(rand, engine::state_size, std::ref(dev)); + std::seed_seq seed(rand, rand + engine::state_size); + rng.seed(seed); + + long int ms = 250 + (rng() % 50); MDEBUG("Sleeping because QUEUE is FULL, in " << __FUNCTION__ << " for " << ms << " ms before packet_size="<<chunk.size()); // XXX debug sleep m_send_que_lock.unlock(); boost::this_thread::sleep(boost::posix_time::milliseconds( ms ) ); |