Use brHmacDrbgGenerate for all random operations in discovery (#272)

eth/p2p/discoveryv5/protocol.nim

@@ -73,10 +73,11 @@
 ## This might be a concern for mobile devices.
 
 import
-  std/[tables, sets, options, math, random, sequtils],
+  std/[tables, sets, options, math, sequtils],
   stew/shims/net as stewNet, json_serialization/std/net,
   stew/[byteutils, endians2], chronicles, chronos, stint, bearssl,
-  eth/[rlp, keys, async_utils], types, encoding, node, routing_table, enr
+  eth/[rlp, keys, async_utils],
+  types, encoding, node, routing_table, enr, random2
 
 import nimcrypto except toHex
 
@@ -702,9 +703,8 @@ proc revalidateNode*(d: Protocol, n: Node)
 proc revalidateLoop(d: Protocol) {.async, raises: [Exception, Defect].} =
   # TODO: General Exception raised.
   try:
-    randomize()
     while true:
-      await sleepAsync(rand(revalidateMax).milliseconds)
+      await sleepAsync(d.rng[].rand(revalidateMax).milliseconds)
       let n = d.routingTable.nodeToRevalidate()
       if not n.isNil:
         traceAsyncErrors d.revalidateNode(n)
@@ -765,7 +765,7 @@ proc newProtocol*(privKey: PrivateKey, db: Database,
     bootstrapRecords: @bootstrapRecords,
     rng: rng)
 
-  result.routingTable.init(node, 5)
+  result.routingTable.init(node, 5, rng)
 
 proc open*(d: Protocol) {.raises: [Exception, Defect].} =
   info "Starting discovery node", node = $d.localNode,

eth/p2p/discoveryv5/random2.nim

@@ -0,0 +1,22 @@
+import bearssl
+
+## Random helpers: similar as in stdlib, but with BrHmacDrbgContext rng
+# TODO: Move these somewhere else?
+const randMax = 18_446_744_073_709_551_615'u64
+
+proc rand*(rng: var BrHmacDrbgContext, max: Natural): int =
+  if max == 0: return 0
+
+  var x: uint64
+  while true:
+    brHmacDrbgGenerate(addr rng, addr x, csize_t(sizeof(x)))
+    if x < randMax - (randMax mod (uint64(max) + 1'u64)): # against modulo bias
+      return int(x mod (uint64(max) + 1'u64))
+
+proc sample*[T](rng: var BrHmacDrbgContext, a: openarray[T]): T =
+  result = a[rng.rand(a.high)]
+
+proc shuffle*[T](rng: var BrHmacDrbgContext, a: var openarray[T]) =
+  for i in countdown(a.high, 1):
+    let j = rng.rand(i)
+    swap(a[i], a[j])

eth/p2p/discoveryv5/routing_table.nim

@@ -1,7 +1,7 @@
 import
-  std/[algorithm, times, sequtils, bitops, random, sets, options],
-  stint, chronicles, metrics,
-  node, enr
+  std/[algorithm, times, sequtils, bitops, sets, options],
+  stint, chronicles, metrics, bearssl,
+  node, random2
 
 export options
 
@@ -22,6 +22,7 @@ type
     ## Setting it higher will increase the amount of splitting on a not in range
     ## branch (thus holding more nodes with a better keyspace coverage) and this
     ## will result in an improvement of log base(2^b) n hops per lookup.
+    rng: ref BrHmacDrbgContext
 
   KBucket = ref object
     istart, iend: NodeId ## Range of NodeIds this KBucket covers. This is not a
@@ -190,13 +191,14 @@ proc computeSharedPrefixBits(nodes: openarray[NodeId]): int =
   # Reaching this would mean that all node ids are equal.
   doAssert(false, "Unable to calculate number of shared prefix bits")
 
-proc init*(r: var RoutingTable, thisNode: Node, bitsPerHop = 5) {.inline.} =
+proc init*(r: var RoutingTable, thisNode: Node, bitsPerHop = 5,
+    rng: ref BrHmacDrbgContext) {.inline.} =
   ## Initialize the routing table for provided `Node` and bitsPerHop value.
   ## `bitsPerHop` is default set to 5 as recommended by original Kademlia paper.
   r.thisNode = thisNode
   r.buckets = @[newKBucket(0.u256, high(Uint256))]
   r.bitsPerHop = bitsPerHop
-  randomize() # for later `randomNodes` selection
+  r.rng = rng
 
 proc splitBucket(r: var RoutingTable, index: int) =
   let bucket = r.buckets[index]
@@ -342,7 +344,7 @@ proc nodeToRevalidate*(r: RoutingTable): Node =
   ## Return a node to revalidate. The least recently seen node from a random
   ## bucket is selected.
   var buckets = r.buckets
-  shuffle(buckets)
+  r.rng[].shuffle(buckets)
   # TODO: Should we prioritize less-recently-updated buckets instead? Could use
   # `lastUpdated` for this, but it would probably make more sense to only update
   # that value on revalidation then and rename it to `lastValidated`.
@@ -375,10 +377,9 @@ proc randomNodes*(r: RoutingTable, maxAmount: int,
   # randomLookup, the time might be wasted as all nodes are possibly seen
   # already.
   while len(seen) < maxAmount:
-    # TODO: Is it important to get a better random source for these sample calls?
-    let bucket = sample(r.buckets)
+    let bucket = r.rng[].sample(r.buckets)
     if bucket.nodes.len != 0:
-      let node = sample(bucket.nodes)
+      let node = r.rng[].sample(bucket.nodes)
       if node notin seen:
         seen.incl(node)
         if pred.isNil() or node.pred:

tests/p2p/test_routing_table.nim

@@ -12,7 +12,7 @@ suite "Routing Table Tests":
     var table: RoutingTable
 
     # bitsPerHop = 1 -> Split only the branch in range of own id
-    table.init(node, 1)
+    table.init(node, 1, rng)
 
     for j in 0..5'u32:
       for i in 0..<BUCKET_SIZE:
@@ -24,7 +24,7 @@ suite "Routing Table Tests":
     var table: RoutingTable
 
     # bitsPerHop = 1 -> Split only the branch in range of own id
-    table.init(node, 1)
+    table.init(node, 1, rng)
 
     # Add 16 nodes, distance 256
     for i in 0..<BUCKET_SIZE:
@@ -44,7 +44,7 @@ suite "Routing Table Tests":
     var table: RoutingTable
 
     # bitsPerHop = 2, allow not in range branch to split once (2 buckets).
-    table.init(node, 2)
+    table.init(node, 2, rng)
 
     # Add 16 nodes, distance 256 from `node`, but all with 2 bits shared prefix
     # among themselves.
@@ -74,7 +74,7 @@ suite "Routing Table Tests":
     var table: RoutingTable
 
     # bitsPerHop = 1 -> Split only the branch in range of own id
-    table.init(node, 1)
+    table.init(node, 1, rng)
 
     # create a full bucket
     let bucketNodes = node.nodesAtDistance(rng[], 256, BUCKET_SIZE)
@@ -108,7 +108,7 @@ suite "Routing Table Tests":
     var table: RoutingTable
 
     # bitsPerHop = 1 -> Split only the branch in range of own id
-    table.init(node, 1)
+    table.init(node, 1, rng)
 
     check table.nodeToRevalidate().isNil()
 
@@ -132,7 +132,7 @@ suite "Routing Table Tests":
     var table: RoutingTable
 
     # bitsPerHop = 1 -> Split only the branch in range of own id
-    table.init(node, 1)
+    table.init(node, 1, rng)
 
     # create a full bucket TODO: no need to store bucketNodes
     let bucketNodes = node.nodesAtDistance(rng[], 256, BUCKET_SIZE)
@@ -148,7 +148,7 @@ suite "Routing Table Tests":
     var table: RoutingTable
 
     # bitsPerHop = 1 -> Split only the branch in range of own id
-    table.init(node, 1)
+    table.init(node, 1, rng)
 
     let doubleNode = node.nodeAtDistance(rng[], 256)
     # Try to add the node twice
@@ -178,7 +178,7 @@ suite "Routing Table Tests":
     var table: RoutingTable
 
     # bitsPerHop = 1 -> Split only the branch in range of own id
-    table.init(node, 1)
+    table.init(node, 1, rng)
 
     # create a full bucket
     let bucketNodes = node.nodesAtDistance(rng[], 256, BUCKET_SIZE)
@@ -208,7 +208,7 @@ suite "Routing Table Tests":
     var table: RoutingTable
 
     # bitsPerHop = 1 -> Split only the branch in range of own id
-    table.init(node, 1)
+    table.init(node, 1, rng)
 
     # create a full bucket
     let bucketNodes = node.nodesAtDistance(rng[], 256, BUCKET_SIZE)
@@ -228,7 +228,7 @@ suite "Routing Table Tests":
     var table: RoutingTable
 
     # bitsPerHop = 1 -> Split only the branch in range of own id
-    table.init(node, 1)
+    table.init(node, 1, rng)
 
     # create a full bucket
     let bucketNodes = node.nodesAtDistance(rng[], 256, BUCKET_SIZE)