libp2p: remember wrong addresses

4 files changed, 58 insertions, 0 deletions

diff --git a/src/transport/libp2p/book.rs b/src/transport/libp2p/book.rs
index c5a73c4..8620895 100644
--- a/src/transport/libp2p/book.rs
+++ b/src/transport/libp2p/book.rs
@@ -44,6 +44,11 @@ pub(super) struct Book {
     /// Addresses of directly connected peers dialled for a better path (`explore`):
     /// not again while connected, unless our own interfaces change.
     pub(super) explored: HashMap<PeerId, std::collections::HashSet<Multiaddr>>,
+    /// Addresses where a dial for a peer reached another (`wrongPeerId`), say a NATed
+    /// peer's LAN address in a range our LAN uses too: not that peer's, whatever
+    /// identify or the member list say, so not kept, dialled or explored again. As long
+    /// as the peer is known (`prune`), `WRONG_PER_PEER` each.
+    pub(super) wrong: HashMap<PeerId, std::collections::HashSet<Multiaddr>>,
     pub(super) listening: Vec<Multiaddr>,
     /// Where the router forwards to our listen addresses (UPnP, `Libp2pConfig::upnp`).
     pub(super) mapped: Vec<Multiaddr>,
@@ -224,6 +229,7 @@ impl Book {
             self.addresses.remove(peer);
             self.identified.remove(peer);
             self.explored.remove(peer);
+            self.wrong.remove(peer);
             self.relays.remove(peer);
             self.public_relays.remove(peer);
             self.continuum.remove(peer);
@@ -467,6 +473,19 @@ impl Book {
     /// one it doesn't publish (`Libp2pConfig::ignore`). Nor one in a range this node
     /// leaves out, directly or through a relay there (`left_out`): never dialled, it
     /// would only take a place, and be passed on.
+    /// A dial for `peer` at `address` reached another peer: `address` isn't `peer`'s.
+    /// Forgotten, and not taken again (`wrong`).
+    pub(super) fn not_theirs(&mut self, peer: PeerId, address: Multiaddr) {
+        let address = without_peer(address);
+        if let Some(known) = self.addresses.get_mut(&peer) {
+            known.retain(|known| *known != address);
+        }
+        let wrong = self.wrong.entry(peer).or_default();
+        if wrong.len() < WRONG_PER_PEER {
+            wrong.insert(address);
+        }
+    }
+
     pub(super) fn remember(&mut self, peer: PeerId, address: Multiaddr) {
         let address = without_peer(address);
         if !self.addresses.contains_key(&peer) && self.addresses.len() >= MAX_PEERS {
@@ -476,6 +495,13 @@ impl Book {
             return;
         }
         if self
+            .wrong
+            .get(&peer)
+            .is_some_and(|wrong| wrong.contains(&address))
+        {
+            return;
+        }
+        if self
             .identified
             .get(&peer)
             .is_some_and(|said| is_ip(&address) && !said.contains(&address))
diff --git a/src/transport/libp2p/driver.rs b/src/transport/libp2p/driver.rs
index c89a784..39a2a5d 100644
--- a/src/transport/libp2p/driver.rs
+++ b/src/transport/libp2p/driver.rs
@@ -272,6 +272,19 @@ pub(super) async fn run(
                 }
                 SwarmEvent::OutgoingConnectionError { connection_id, peer_id, error } => {
                     count_dial_failure(&mut lock(&book).counts.dial_failures, &error);
+                    // Another peer there: not this one's address, though it said so (a
+                    // NATed peer's LAN address, the same range as ours).
+                    if let (Some(peer), ::libp2p::swarm::DialError::WrongPeerId { obtained, address }) =
+                        (peer_id, &error)
+                    {
+                        debug!(
+                            peer = %Addressed(SCHEME, &peer),
+                            address = %address,
+                            obtained = %obtained,
+                            "Another peer at an address of this one; not dialling it there again"
+                        );
+                        lock(&book).not_theirs(peer, address.clone());
+                    }
                     if let Some((_, done)) = identifying.remove(&connection_id) {
                         let _ = done.send(Err(describe_dial_error(&error)));
                     }
diff --git a/src/transport/libp2p/mod.rs b/src/transport/libp2p/mod.rs
index 6fea05a..477f1ba 100644
--- a/src/transport/libp2p/mod.rs
+++ b/src/transport/libp2p/mod.rs
@@ -146,6 +146,8 @@ const MIN_RATE: u64 = 1 << 20;
 const READS: usize = 32;
 /// Addresses kept per peer, and peers with addresses at most.
 const ADDRESSES_PER_PEER: usize = 8;
+/// Addresses known not to be a peer's (`Book::wrong`), per peer at most.
+const WRONG_PER_PEER: usize = 32;
 /// Of this node's own, the direct ones (the best first) and those through relays it
 /// publishes at most (`local_addresses`): a node listening everywhere has many direct
 /// ones, and those through a relay are what reaches it from behind a NAT.
diff --git a/src/transport/libp2p/tests.rs b/src/transport/libp2p/tests.rs
index ce9ad9f..e105761 100644
--- a/src/transport/libp2p/tests.rs
+++ b/src/transport/libp2p/tests.rs
@@ -562,6 +562,23 @@ fn public_ranges() {
     }
 }
 
+/// An address where a dial reached another peer isn't kept for the one dialled, even
+/// when it tells it again (identify) or the member list does.
+#[test]
+fn addresses_of_another_peer_are_forgotten() {
+    let mut book = Book::default();
+    let them = peer();
+    let lan: Multiaddr = "/ip4/192.168.1.5/tcp/9661".parse().unwrap();
+    let elsewhere: Multiaddr = "/ip4/203.0.114.9/tcp/9661".parse().unwrap();
+    book.remember(them, lan.clone());
+    book.remember(them, elsewhere.clone());
+    book.not_theirs(them, lan.clone().with_p2p(them).unwrap());
+    assert_eq!(book.addresses[&them], [elsewhere.clone()]);
+    book.remember(them, lan.clone());
+    book.identified(them, vec![lan, elsewhere.clone()]);
+    assert_eq!(book.addresses[&them], [elsewhere]);
+}
+
 #[test]
 fn lan_peers_become_seeds_once() {
     let mut book = Book::default();