crypto: replace rand<T>()%N idiom with unbiased rand_idx(N)

5 files changed, 36 insertions, 12 deletions

diff --git a/src/common/dns_utils.cpp b/src/common/dns_utils.cpp
index 1a1155c7c..5e03bf897 100644
--- a/src/common/dns_utils.cpp
+++ b/src/common/dns_utils.cpp
@@ -33,7 +33,7 @@
 #include <stdlib.h>
 #include "include_base_utils.h"
 #include "common/threadpool.h"
-#include <random>
+#include "crypto/crypto.h"
 #include <boost/thread/mutex.hpp>
 #include <boost/algorithm/string/join.hpp>
 #include <boost/optional.hpp>
@@ -517,10 +517,7 @@ bool load_txt_records_from_dns(std::vector<std::string> &good_records, const std
   std::vector<std::vector<std::string> > records;
   records.resize(dns_urls.size());
 
-  std::random_device rd;
-  std::mt19937 gen(rd());
-  std::uniform_int_distribution<int> dis(0, dns_urls.size() - 1);
-  size_t first_index = dis(gen);
+  size_t first_index = crypto::rand_idx(dns_urls.size());
 
   // send all requests in parallel
   std::deque<bool> avail(dns_urls.size(), false), valid(dns_urls.size(), false);
diff --git a/src/crypto/crypto.h b/src/crypto/crypto.h
index 22b182ab0..bac456f60 100644
--- a/src/crypto/crypto.h
+++ b/src/crypto/crypto.h
@@ -35,6 +35,7 @@
 #include <boost/optional.hpp>
 #include <type_traits>
 #include <vector>
+#include <random>
 
 #include "common/pod-class.h"
 #include "memwipe.h"
@@ -162,6 +163,32 @@ namespace crypto {
     return res;
   }
 
+  /* UniformRandomBitGenerator using crypto::rand<uint64_t>()
+   */
+  struct random_device
+  {
+    typedef uint64_t result_type;
+    static constexpr result_type min() { return 0; }
+    static constexpr result_type max() { return result_type(-1); }
+    result_type operator()() const { return crypto::rand<result_type>(); }
+  };
+
+  /* Generate a random value between range_min and range_max
+   */
+  template<typename T>
+  typename std::enable_if<std::is_integral<T>::value, T>::type rand_range(T range_min, T range_max) {
+    crypto::random_device rd;
+    std::uniform_int_distribution<T> dis(range_min, range_max);
+    return dis(rd);
+  }
+
+  /* Generate a random index between 0 and sz-1
+   */
+  template<typename T>
+  typename std::enable_if<std::is_unsigned<T>::value, T>::type rand_idx(T sz) {
+    return crypto::rand_range<T>(0, sz-1);
+  }
+
   /* Generate a new key pair
    */
   inline secret_key generate_keys(public_key &pub, secret_key &sec, const secret_key& recovery_key = secret_key(), bool recover = false) {
diff --git a/src/p2p/net_node.inl b/src/p2p/net_node.inl
index f0aef384f..7d13b3216 100644
--- a/src/p2p/net_node.inl
+++ b/src/p2p/net_node.inl
@@ -1259,7 +1259,7 @@ namespace nodetool
         }
       }
       else
-        random_index = crypto::rand<size_t>() % filtered.size();
+        random_index = crypto::rand_idx(filtered.size());
 
       CHECK_AND_ASSERT_MES(random_index < filtered.size(), false, "random_index < filtered.size() failed!!");
       random_index = filtered[random_index];
@@ -1313,7 +1313,7 @@ namespace nodetool
         return true;
 
       size_t try_count = 0;
-      size_t current_index = crypto::rand<size_t>()%m_seed_nodes.size();
+      size_t current_index = crypto::rand_idx(m_seed_nodes.size());
       const net_server& server = m_network_zones.at(epee::net_utils::zone::public_).m_net_server;
       while(true)
       {
diff --git a/src/p2p/net_peerlist.h b/src/p2p/net_peerlist.h
index ebe0268d8..52814af94 100644
--- a/src/p2p/net_peerlist.h
+++ b/src/p2p/net_peerlist.h
@@ -398,7 +398,7 @@ namespace nodetool
       return false;
     }
 
-    size_t random_index = crypto::rand<size_t>() % m_peers_gray.size();
+    size_t random_index = crypto::rand_idx(m_peers_gray.size());
 
     peers_indexed::index<by_time>::type& by_time_index = m_peers_gray.get<by_time>();
     pe = *epee::misc_utils::move_it_backward(--by_time_index.end(), random_index);
diff --git a/src/wallet/wallet2.cpp b/src/wallet/wallet2.cpp
index 0e82f1a91..a4b063541 100644
--- a/src/wallet/wallet2.cpp
+++ b/src/wallet/wallet2.cpp
@@ -5719,7 +5719,7 @@ namespace
   {
     CHECK_AND_ASSERT_MES(!vec.empty(), T(), "Vector must be non-empty");
 
-    size_t idx = crypto::rand<size_t>() % vec.size();
+    size_t idx = crypto::rand_idx(vec.size());
     return pop_index (vec, idx);
   }
 
@@ -5822,7 +5822,7 @@ size_t wallet2::pop_best_value_from(const transfer_container &transfers, std::ve
   }
   else
   {
-    idx = crypto::rand<size_t>() % candidates.size();
+    idx = crypto::rand_idx(candidates.size());
   }
   return pop_index (unused_indices, candidates[idx]);
 }
@@ -7474,7 +7474,7 @@ void wallet2::get_outs(std::vector<std::vector<tools::wallet2::get_outs_entry>>
       if (n_rct == 0)
         return rct_offsets[block_offset] ? rct_offsets[block_offset] - 1 : 0;
       MDEBUG("Picking 1/" << n_rct << " in " << (last_block_offset - first_block_offset + 1) << " blocks centered around " << block_offset + rct_start_height);
-      return first_rct + crypto::rand<uint64_t>() % n_rct;
+      return first_rct + crypto::rand_idx(n_rct);
     };
 
     size_t num_selected_transfers = 0;
@@ -9603,7 +9603,7 @@ std::vector<wallet2::pending_tx> wallet2::create_transactions_all(uint64_t below
     if (unused_transfer_dust_indices_per_subaddr.count(0) == 1 && unused_transfer_dust_indices_per_subaddr.size() > 1)
       unused_transfer_dust_indices_per_subaddr.erase(0);
     auto i = unused_transfer_dust_indices_per_subaddr.begin();
-    std::advance(i, crypto::rand<size_t>() % unused_transfer_dust_indices_per_subaddr.size());
+    std::advance(i, crypto::rand_idx(unused_transfer_dust_indices_per_subaddr.size()));
     unused_transfers_indices = i->second.first;
     unused_dust_indices = i->second.second;
     LOG_PRINT_L2("Spending from subaddress index " << i->first);