Fix --anonymouns-inbound data leak

monero-project · Dec 19, 2024 · 9221a8d · 9221a8d
1 parent 58a1d54
commit 9221a8d
Show file tree

Hide file tree

Showing 2 changed files with 31 additions and 10 deletions.
diff --git a/src/crypto/crypto.h b/src/crypto/crypto.h
@@ -205,6 +205,14 @@ namespace crypto {
     return crypto::rand_range<T>(0, sz-1);
   }
 
+  template<typename T>
+  struct random_distribution
+  {
+    static_assert(std::is_unsigned<T>::value, "expected unsigned type");
+    typedef T result_type;
+    result_type operator()(const result_type max) const { return rand_idx(max); }
+  };
+
   /* 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
@@ -2483,6 +2483,25 @@ namespace nodetool
     std::vector<peerlist_entry> local_peerlist_new;
     zone.m_peerlist.get_peerlist_head(local_peerlist_new, true, max_peerlist_size);
 
+    /* Tor/I2P nodes receiving connections via forwarding (from tor/i2p daemon)
+    do not know the address of the connecting peer. This is relayed to them,
+    iff the node has setup an inbound hidden service. The other peer will have
+    to use the random peer_id value to link the two. My initial thought is that
+    the inbound peer should leave the other side marked as `<unknown tor host>`,
+    etc., because someone could give faulty addresses over Tor/I2P to get the
+    real peer with that identity banned/blacklisted.
+
+    \note Insert into `local_peerlist_new` so that it is only sent once like
+      the other peers. */
+    static constexpr const std::uint32_t max_random_time =
+      1 /*day*/ * 24 /*hours*/ * 60 /*minutes*/ * 60 /*seconds*/;
+    if(outgoing_to_same_zone)
+      local_peerlist_new.push_back(
+        peerlist_entry{
+          zone.m_our_address, zone.m_config.m_peer_id, std::time(nullptr) - crypto::rand_idx(max_random_time)
+        }
+      );
+
     //only include out peers we did not already send
     rsp.local_peerlist_new.reserve(local_peerlist_new.size());
     for (auto &pe: local_peerlist_new)
@@ -2493,16 +2512,10 @@ namespace nodetool
     }
     m_payload_handler.get_payload_sync_data(rsp.payload_data);
 
-    /* Tor/I2P nodes receiving connections via forwarding (from tor/i2p daemon)
-    do not know the address of the connecting peer. This is relayed to them,
-    iff the node has setup an inbound hidden service. The other peer will have
-    to use the random peer_id value to link the two. My initial thought is that
-    the inbound peer should leave the other side marked as `<unknown tor host>`,
-    etc., because someone could give faulty addresses over Tor/I2P to get the
-    real peer with that identity banned/blacklisted. */
-
-    if(outgoing_to_same_zone)
-      rsp.local_peerlist_new.push_back(peerlist_entry{zone.m_our_address, zone.m_config.m_peer_id, std::time(nullptr)});
+    // randomize so location of local inbound is not easily found
+    std::random_shuffle(
+      rsp.local_peerlist_new.begin(), rsp.local_peerlist_new.end(), crypto::random_distribution<std::size_t>{}
+    );
 
     LOG_DEBUG_CC(context, "COMMAND_TIMED_SYNC");
     return 1;