diff --git a/protocols/identify/src/behaviour.rs b/protocols/identify/src/behaviour.rs index a50e8e64cbd..94ad4c7feb9 100644 --- a/protocols/identify/src/behaviour.rs +++ b/protocols/identify/src/behaviour.rs @@ -87,6 +87,42 @@ fn is_tcp_addr(addr: &Multiaddr) -> bool { matches!(first, Ip4(_) | Ip6(_) | Dns(_) | Dns4(_) | Dns6(_)) && matches!(second, Tcp(_)) } +/// Extract the port from a multiaddr if it contains TCP or UDP protocols. +/// This works for QUIC addresses since they use UDP underneath. +fn extract_port(addr: &Multiaddr) -> Option { + use Protocol::*; + + for protocol in addr.iter() { + match protocol { + Tcp(port) | Udp(port) => return Some(port), + _ => continue, + } + } + + None +} + +/// Check if the observed port matches any of our current listening ports. +fn observed_port_matches_listening_port( + observed: &Multiaddr, + listen_addresses: &ListenAddresses, +) -> bool { + let Some(observed_port) = extract_port(observed) else { + return false; + }; + + // Check if the observed port matches any of our listening ports + for listen_addr in listen_addresses.iter() { + if let Some(listen_port) = extract_port(listen_addr) { + if observed_port == listen_port { + return true; + } + } + } + + false +} + /// Network behaviour that automatically identifies nodes periodically, returns information /// about them, and answers identify queries from other nodes. /// @@ -100,9 +136,6 @@ pub struct Behaviour { /// The address a remote observed for us. our_observed_addresses: HashMap, - /// The outbound connections established without port reuse (require translation) - outbound_connections_with_ephemeral_port: HashSet, - /// Pending events to be emitted when polled. events: VecDeque>, /// The addresses of all peers that we have discovered. @@ -268,7 +301,6 @@ impl Behaviour { config, connected: HashMap::new(), our_observed_addresses: Default::default(), - outbound_connections_with_ephemeral_port: Default::default(), events: VecDeque::new(), discovered_peers, listen_addresses: Default::default(), @@ -332,16 +364,18 @@ impl Behaviour { fn emit_new_external_addr_candidate_event( &mut self, - connection_id: ConnectionId, + _connection_id: ConnectionId, observed: &Multiaddr, ) { - if self - .outbound_connections_with_ephemeral_port - .contains(&connection_id) - { - // Apply address translation to the candidate address. - // For TCP without port-reuse, the observed address contains an ephemeral port which - // needs to be replaced by the port of a listen address. + if observed_port_matches_listening_port(observed, &self.listen_addresses) { + // If the observed port matches any of our listening ports, + // then port reuse actually worked. Use the original observed address. + self.events + .push_back(ToSwarm::NewExternalAddrCandidate(observed.clone())); + } else { + // The observed port doesn't match any listening port, which means + // either this is an inbound connection or an outbound connection + // that used an ephemeral port. Apply address translation. let translated_addresses = { let mut addrs: Vec<_> = self .listen_addresses @@ -374,13 +408,7 @@ impl Behaviour { .push_back(ToSwarm::NewExternalAddrCandidate(addr)); } } - return; } - - // outgoing connection dialed with port reuse - // incoming connection - self.events - .push_back(ToSwarm::NewExternalAddrCandidate(observed.clone())); } } @@ -408,11 +436,11 @@ impl NetworkBehaviour for Behaviour { fn handle_established_outbound_connection( &mut self, - connection_id: ConnectionId, + _connection_id: ConnectionId, peer: PeerId, addr: &Multiaddr, _: Endpoint, - port_use: PortUse, + _port_use: PortUse, ) -> Result, ConnectionDenied> { // Contrary to inbound events, outbound events are full-p2p qualified // so we remove /p2p/ in order to be homogeneous @@ -423,11 +451,6 @@ impl NetworkBehaviour for Behaviour { addr.pop(); } - if port_use == PortUse::New { - self.outbound_connections_with_ephemeral_port - .insert(connection_id); - } - Ok(Handler::new( self.config.interval, peer, @@ -586,8 +609,6 @@ impl NetworkBehaviour for Behaviour { } self.our_observed_addresses.remove(&connection_id); - self.outbound_connections_with_ephemeral_port - .remove(&connection_id); } FromSwarm::DialFailure(DialFailure { peer_id: Some(peer_id), @@ -732,6 +753,7 @@ impl KeyType { #[cfg(test)] mod tests { use super::*; + use libp2p_swarm::behaviour::ListenAddresses; #[test] fn check_multiaddr_matches_peer_id() { @@ -754,4 +776,175 @@ mod tests { )); assert!(multiaddr_matches_peer_id(&addr_without_peer_id, &peer_id)); } + + #[test] + fn test_extract_port() { + // TCP and UDP addresses + let tcp_addr: Multiaddr = "/ip4/127.0.0.1/tcp/8080".parse().unwrap(); + assert_eq!(extract_port(&tcp_addr), Some(8080)); + + let udp_addr: Multiaddr = "/ip4/127.0.0.1/udp/9090".parse().unwrap(); + assert_eq!(extract_port(&udp_addr), Some(9090)); + + // Addresses without ports + let no_port_addr: Multiaddr = "/ip4/127.0.0.1".parse().unwrap(); + assert_eq!(extract_port(&no_port_addr), None); + } + + #[test] + fn test_observed_port_matches_listening_port() { + use libp2p_swarm::behaviour::FromSwarm; + + let mut listen_addresses = ListenAddresses::default(); + + // Add listening addresses + let listen_addr: Multiaddr = "/ip4/0.0.0.0/tcp/8080".parse().unwrap(); + listen_addresses.on_swarm_event(&FromSwarm::NewListenAddr( + libp2p_swarm::behaviour::NewListenAddr { + listener_id: libp2p_core::transport::ListenerId::next(), + addr: &listen_addr, + }, + )); + + // Test matching port + let observed_match: Multiaddr = "/ip4/192.168.1.100/tcp/8080".parse().unwrap(); + assert!(observed_port_matches_listening_port( + &observed_match, + &listen_addresses + )); + + // Test non-matching port + let observed_no_match: Multiaddr = "/ip4/192.168.1.100/tcp/8888".parse().unwrap(); + assert!(!observed_port_matches_listening_port( + &observed_no_match, + &listen_addresses + )); + + // Test with no listening addresses + let empty_listen_addresses = ListenAddresses::default(); + assert!(!observed_port_matches_listening_port( + &observed_match, + &empty_listen_addresses + )); + } + + #[test] + fn test_address_translation_when_port_matches() { + use libp2p_identity::Keypair; + use libp2p_swarm::behaviour::FromSwarm; + + // Create a behavior with some listening addresses + let keypair = Keypair::generate_ed25519(); + let config = Config::new("test/1.0.0".to_string(), keypair.public()); + let mut behaviour = Behaviour::new(config); + + // Add a listening address + let listen_addr: Multiaddr = "/ip4/0.0.0.0/tcp/8080".parse().unwrap(); + behaviour + .listen_addresses + .on_swarm_event(&FromSwarm::NewListenAddr( + libp2p_swarm::behaviour::NewListenAddr { + listener_id: libp2p_core::transport::ListenerId::next(), + addr: &listen_addr, + }, + )); + + // Clear any existing events + behaviour.events.clear(); + + // Test case: observed address has matching port (port reuse worked) + let observed_matching: Multiaddr = "/ip4/203.0.113.1/tcp/8080".parse().unwrap(); + behaviour.emit_new_external_addr_candidate_event( + libp2p_swarm::ConnectionId::new_unchecked(1), + &observed_matching, + ); + + // Should emit the original observed address without translation + assert_eq!(behaviour.events.len(), 1); + if let ToSwarm::NewExternalAddrCandidate(addr) = &behaviour.events[0] { + assert_eq!(addr, &observed_matching); + } else { + panic!("Expected NewExternalAddrCandidate event"); + } + } + + #[test] + fn test_address_translation_when_port_differs() { + use libp2p_identity::Keypair; + use libp2p_swarm::behaviour::FromSwarm; + + // Create a behavior with some listening addresses + let keypair = Keypair::generate_ed25519(); + let config = Config::new("test/1.0.0".to_string(), keypair.public()); + let mut behaviour = Behaviour::new(config); + + // Add a listening address + let listen_addr: Multiaddr = "/ip4/0.0.0.0/tcp/8080".parse().unwrap(); + behaviour + .listen_addresses + .on_swarm_event(&FromSwarm::NewListenAddr( + libp2p_swarm::behaviour::NewListenAddr { + listener_id: libp2p_core::transport::ListenerId::next(), + addr: &listen_addr, + }, + )); + + // Clear any existing events + behaviour.events.clear(); + + // Test case: observed address has different port (ephemeral port used) + let observed_different: Multiaddr = "/ip4/203.0.113.1/tcp/54321".parse().unwrap(); + behaviour.emit_new_external_addr_candidate_event( + libp2p_swarm::ConnectionId::new_unchecked(1), + &observed_different, + ); + + // Should emit translated address(es) using the listening port + assert!(!behaviour.events.is_empty()); + + // Find the translated address + let mut found_translated = false; + for event in &behaviour.events { + if let ToSwarm::NewExternalAddrCandidate(addr) = event { + // The translated address should have the same IP as observed but port from listener + if addr.to_string().contains("203.0.113.1") && addr.to_string().contains("tcp/8080") + { + found_translated = true; + break; + } + } + } + assert!( + found_translated, + "Should have found a translated address with listening port" + ); + } + + #[test] + fn test_no_listening_addresses() { + use libp2p_identity::Keypair; + + // Create a behavior with no listening addresses + let keypair = Keypair::generate_ed25519(); + let config = Config::new("test/1.0.0".to_string(), keypair.public()); + let mut behaviour = Behaviour::new(config); + + // Clear any existing events + behaviour.events.clear(); + + // Test with any observed address + let observed: Multiaddr = "/ip4/203.0.113.1/tcp/54321".parse().unwrap(); + behaviour.emit_new_external_addr_candidate_event( + libp2p_swarm::ConnectionId::new_unchecked(1), + &observed, + ); + + // Should emit the original observed address since no listening addresses to match + assert_eq!(behaviour.events.len(), 1); + if let ToSwarm::NewExternalAddrCandidate(addr) = &behaviour.events[0] { + assert_eq!(addr, &observed); + } else { + panic!("Expected NewExternalAddrCandidate event"); + } + } }