From 3a214d79a97c6a59254105797ff1d4f0c2dcf61d Mon Sep 17 00:00:00 2001 From: elenaf9 Date: Sun, 17 Apr 2022 21:15:31 +0200 Subject: [PATCH 01/14] protocols/autonat: add only_global_ips config --- protocols/autonat/src/behaviour.rs | 130 +++++++++++++++++-- protocols/autonat/src/behaviour/as_client.rs | 7 +- 2 files changed, 127 insertions(+), 10 deletions(-) diff --git a/protocols/autonat/src/behaviour.rs b/protocols/autonat/src/behaviour.rs index ce5b1a923cb..ac171eb2a28 100644 --- a/protocols/autonat/src/behaviour.rs +++ b/protocols/autonat/src/behaviour.rs @@ -30,6 +30,7 @@ use futures_timer::Delay; use instant::Instant; use libp2p_core::{ connection::{ConnectionId, ListenerId}, + multiaddr::Protocol, ConnectedPoint, Endpoint, Multiaddr, PeerId, }; use libp2p_request_response::{ @@ -77,6 +78,8 @@ pub struct Config { pub throttle_clients_peer_max: usize, /// Period for throttling clients requests. pub throttle_clients_period: Duration, + /// Reject probes for clients that are observed at a non-global ip address. + pub only_global_ips: bool, } impl Default for Config { @@ -93,6 +96,7 @@ impl Default for Config { throttle_clients_global_max: 30, throttle_clients_peer_max: 3, throttle_clients_period: Duration::from_secs(1), + only_global_ips: true, } } } @@ -188,7 +192,8 @@ pub struct Behaviour { ongoing_outbound: HashMap, // Connected peers with the observed address of each connection. - // If the endpoint of a connection is relayed, the observed address is `None`. + // If the endpoint of a connection is relayed or not global (in case of Config::only_global_ips), + // the observed address is `None`. connected: HashMap>>, // Used servers in recent outbound probes that are throttled through Config::throttle_server_period. @@ -313,12 +318,15 @@ impl NetworkBehaviour for Behaviour { other_established, ); let connections = self.connected.entry(*peer).or_default(); - let addr = if endpoint.is_relayed() { - None + let addr = endpoint.get_remote_address(); + let observed_addr = if !endpoint.is_relayed() + &&( !self.config.only_global_ips || addr.is_global_ip()) + { + Some(addr.clone()) } else { - Some(endpoint.get_remote_address().clone()) + None }; - connections.insert(*conn, addr); + connections.insert(*conn, observed_addr); match endpoint { ConnectedPoint::Dialer { @@ -386,12 +394,15 @@ impl NetworkBehaviour for Behaviour { return; } let connections = self.connected.get_mut(peer).expect("Peer is connected."); - let addr = if new.is_relayed() { - None + let addr = new.get_remote_address(); + let observed_addr = if !new.is_relayed() + &&( !self.config.only_global_ips || addr.is_global_ip()) + { + Some(addr.clone()) } else { - Some(new.get_remote_address().clone()) + None }; - connections.insert(*conn, addr); + connections.insert(*conn, observed_addr); } fn inject_new_listen_addr(&mut self, id: ListenerId, addr: &Multiaddr) { @@ -512,3 +523,104 @@ trait HandleInnerEvent { event: RequestResponseEvent, ) -> (VecDeque, Option); } + +trait GlobalIp { + fn is_global_ip(&self) -> bool; +} + +impl GlobalIp for Multiaddr { + fn is_global_ip(&self) -> bool { + match self.iter().next() { + Some(Protocol::Ip4(a)) => a.is_global_ip(), + Some(Protocol::Ip6(a)) => a.is_global_ip(), + _ => false, + } + } +} + +impl GlobalIp for std::net::Ipv4Addr { + // NOTE: The below logic is copied from `std::net::Ipv4Addr::is_global`, which is at the time of + // writing behind the unstable `ip` feature. + // See https://github.com/rust-lang/rust/issues/27709 for more info. + // + // TODO: Consider removing check check with the unstable methods `is_shared`, `is_reserved` and `is_benchmarking`? In case + // of the former the peer is still eligible for a dial-back, the latter cases should never occur in practice. + // + // TODO: Consider to only check for loopback, private ip and link-local. Can any other case ever happen in practice if we solely + // use this check for observed addresses? + fn is_global_ip(&self) -> bool { + // Check if this address is 192.0.0.9 or 192.0.0.10. These addresses are the only two + // globally routable addresses in the 192.0.0.0/24 range. + if u32::from_be_bytes(self.octets()) == 0xc0000009 + || u32::from_be_bytes(self.octets()) == 0xc000000a + { + return true; + } + + // Copied from the unstable method `std::net::Ipv4Addr::is_shared`. + let is_shared = + || self.octets()[0] == 100 && (self.octets()[1] & 0b1100_0000 == 0b0100_0000); + + // Copied from the unstable method `std::net::Ipv4Addr::is_reserved`. + // + // **Warning**: As IANA assigns new addresses, this logic will be + // updated. This may result in non-reserved addresses being + // treated as reserved in code that relies on an outdated version + // of this method. + let is_reserved = || self.octets()[0] & 240 == 240 && !self.is_broadcast(); + + // Copied from the unstable method `std::net::Ipv4Addr::is_benchmarking`. + let is_benchmarking = || self.octets()[0] == 198 && (self.octets()[1] & 0xfe) == 18; + + !self.is_private() + && !self.is_loopback() + && !self.is_link_local() + && !self.is_broadcast() + && !self.is_documentation() + && !is_shared() + // addresses reserved for future protocols (`192.0.0.0/24`) + && !(self.octets()[0] == 192 && self.octets()[1] == 0 && self.octets()[2] == 0) + && !is_reserved() + && !is_benchmarking() + // Make sure the address is not in 0.0.0.0/8 + && self.octets()[0] != 0 + } +} + + + +impl GlobalIp for std::net::Ipv6Addr { + // NOTE: The below logic is copied from `std::net::Ipv6Addr::is_global`, which is at the time of + // writing behind the unstable `ip` feature. + // See https://github.com/rust-lang/rust/issues/27709 for more info. + // + // Note that contrary to `Ipv4Addr::is_global_ip` this currently checks for global scope + // rather than global reachability. + // TODO: There is a PR to change this, but seems inactive. Should we help push this forward? + // See https://github.com/rust-lang/rust/pull/86634 and https://github.com/rust-lang/rust/issues/85604. + fn is_global_ip(&self) -> bool { + + let is_unicast_global = || { + + let is_unicast_link_local = (self.segments()[0] & 0xffc0) == 0xfe80; + let is_unique_local = (self.segments()[0] & 0xfe00) == 0xfc00; + let is_documentation = (self.segments()[0] == 0x2001) && (self.segments()[1] == 0xdb8); + + !self.is_loopback() + && !is_unicast_link_local + && !is_unique_local + && is_documentation + }; + + if !self.is_multicast() { + return is_unicast_global(); + } + // Check scope of the multicast address. + // Copied from unstable method [`std::net::Ipv6Addr::multicast_scope`]. + match self.segments()[0] & 0x000f { + 14 => true, // Global multicast scope. + 1..=5 | 8 => false, // Local multicast scope. + _ => is_unicast_global(), // Unknown multicast scope. + } + } +} diff --git a/protocols/autonat/src/behaviour/as_client.rs b/protocols/autonat/src/behaviour/as_client.rs index de6a00d5629..8d0097deab8 100644 --- a/protocols/autonat/src/behaviour/as_client.rs +++ b/protocols/autonat/src/behaviour/as_client.rs @@ -262,7 +262,12 @@ impl<'a> AsClient<'a> { let mut servers: Vec<&PeerId> = self.servers.iter().collect(); if self.config.use_connected { - servers.extend(self.connected.iter().map(|(id, _)| id)); + servers.extend(self.connected.iter().filter_map(|(id, addrs)| { + // Filter servers for which no qualified address is known. + // This is the case if the connection is relayed or the address is + // not global (in case of Config::only_global_ips). + addrs.values().any(|a| a.is_some()).then(|| id) + })); } servers.retain(|s| !self.throttled_servers.iter().any(|(id, _)| s == &id)); From dedb3098e6b65b4b717ca18351904f55135e89c4 Mon Sep 17 00:00:00 2001 From: elenaf9 Date: Sun, 17 Apr 2022 21:58:24 +0200 Subject: [PATCH 02/14] protocols/autonat: fix tests and fmt --- protocols/autonat/src/behaviour.rs | 39 ++++++++++---------------- protocols/autonat/tests/test_client.rs | 3 +- protocols/autonat/tests/test_server.rs | 3 +- 3 files changed, 19 insertions(+), 26 deletions(-) diff --git a/protocols/autonat/src/behaviour.rs b/protocols/autonat/src/behaviour.rs index ac171eb2a28..124a268f313 100644 --- a/protocols/autonat/src/behaviour.rs +++ b/protocols/autonat/src/behaviour.rs @@ -192,7 +192,7 @@ pub struct Behaviour { ongoing_outbound: HashMap, // Connected peers with the observed address of each connection. - // If the endpoint of a connection is relayed or not global (in case of Config::only_global_ips), + // If the endpoint of a connection is relayed or not global (in case of Config::only_global_ips), // the observed address is `None`. connected: HashMap>>, @@ -319,13 +319,12 @@ impl NetworkBehaviour for Behaviour { ); let connections = self.connected.entry(*peer).or_default(); let addr = endpoint.get_remote_address(); - let observed_addr = if !endpoint.is_relayed() - &&( !self.config.only_global_ips || addr.is_global_ip()) - { - Some(addr.clone()) - } else { - None - }; + let observed_addr = + if !endpoint.is_relayed() && (!self.config.only_global_ips || addr.is_global_ip()) { + Some(addr.clone()) + } else { + None + }; connections.insert(*conn, observed_addr); match endpoint { @@ -395,13 +394,12 @@ impl NetworkBehaviour for Behaviour { } let connections = self.connected.get_mut(peer).expect("Peer is connected."); let addr = new.get_remote_address(); - let observed_addr = if !new.is_relayed() - &&( !self.config.only_global_ips || addr.is_global_ip()) - { - Some(addr.clone()) - } else { - None - }; + let observed_addr = + if !new.is_relayed() && (!self.config.only_global_ips || addr.is_global_ip()) { + Some(addr.clone()) + } else { + None + }; connections.insert(*conn, observed_addr); } @@ -587,29 +585,22 @@ impl GlobalIp for std::net::Ipv4Addr { } } - - impl GlobalIp for std::net::Ipv6Addr { // NOTE: The below logic is copied from `std::net::Ipv6Addr::is_global`, which is at the time of // writing behind the unstable `ip` feature. // See https://github.com/rust-lang/rust/issues/27709 for more info. // - // Note that contrary to `Ipv4Addr::is_global_ip` this currently checks for global scope + // Note that contrary to `Ipv4Addr::is_global_ip` this currently checks for global scope // rather than global reachability. // TODO: There is a PR to change this, but seems inactive. Should we help push this forward? // See https://github.com/rust-lang/rust/pull/86634 and https://github.com/rust-lang/rust/issues/85604. fn is_global_ip(&self) -> bool { - let is_unicast_global = || { - let is_unicast_link_local = (self.segments()[0] & 0xffc0) == 0xfe80; let is_unique_local = (self.segments()[0] & 0xfe00) == 0xfc00; let is_documentation = (self.segments()[0] == 0x2001) && (self.segments()[1] == 0xdb8); - !self.is_loopback() - && !is_unicast_link_local - && !is_unique_local - && is_documentation + !self.is_loopback() && !is_unicast_link_local && !is_unique_local && is_documentation }; if !self.is_multicast() { diff --git a/protocols/autonat/tests/test_client.rs b/protocols/autonat/tests/test_client.rs index b557399987a..4155c84a845 100644 --- a/protocols/autonat/tests/test_client.rs +++ b/protocols/autonat/tests/test_client.rs @@ -35,10 +35,11 @@ const MAX_CONFIDENCE: usize = 3; const TEST_RETRY_INTERVAL: Duration = Duration::from_secs(1); const TEST_REFRESH_INTERVAL: Duration = Duration::from_secs(2); -async fn init_swarm(config: Config) -> Swarm { +async fn init_swarm(mut config: Config) -> Swarm { let keypair = Keypair::generate_ed25519(); let local_id = PeerId::from_public_key(&keypair.public()); let transport = development_transport(keypair).await.unwrap(); + config.only_global_ips = false; let behaviour = Behaviour::new(local_id, config); Swarm::new(transport, behaviour, local_id) } diff --git a/protocols/autonat/tests/test_server.rs b/protocols/autonat/tests/test_server.rs index 5043e715397..1979dbaf267 100644 --- a/protocols/autonat/tests/test_server.rs +++ b/protocols/autonat/tests/test_server.rs @@ -34,10 +34,11 @@ use libp2p_core::{ConnectedPoint, Endpoint}; use libp2p_swarm::DialError; use std::{num::NonZeroU32, time::Duration}; -async fn init_swarm(config: Config) -> Swarm { +async fn init_swarm(mut config: Config) -> Swarm { let keypair = Keypair::generate_ed25519(); let local_id = PeerId::from_public_key(&keypair.public()); let transport = development_transport(keypair).await.unwrap(); + config.only_global_ips = false; let behaviour = Behaviour::new(local_id, config); Swarm::new(transport, behaviour, local_id) } From 099b258892f7d9ac94d87b7244ceb960d4b9f4e9 Mon Sep 17 00:00:00 2001 From: elenaf9 Date: Wed, 4 May 2022 17:45:16 +0200 Subject: [PATCH 03/14] autonat: literal copy of fns from std::net --- protocols/autonat/src/behaviour.rs | 40 ++++++++++++++++++++++-------- 1 file changed, 29 insertions(+), 11 deletions(-) diff --git a/protocols/autonat/src/behaviour.rs b/protocols/autonat/src/behaviour.rs index 124a268f313..3b8d52670ca 100644 --- a/protocols/autonat/src/behaviour.rs +++ b/protocols/autonat/src/behaviour.rs @@ -556,8 +556,9 @@ impl GlobalIp for std::net::Ipv4Addr { } // Copied from the unstable method `std::net::Ipv4Addr::is_shared`. - let is_shared = - || self.octets()[0] == 100 && (self.octets()[1] & 0b1100_0000 == 0b0100_0000); + fn is_shared(addr: &std::net::Ipv4Addr) -> bool { + addr.octets()[0] == 100 && (addr.octets()[1] & 0b1100_0000 == 0b0100_0000) + } // Copied from the unstable method `std::net::Ipv4Addr::is_reserved`. // @@ -565,21 +566,25 @@ impl GlobalIp for std::net::Ipv4Addr { // updated. This may result in non-reserved addresses being // treated as reserved in code that relies on an outdated version // of this method. - let is_reserved = || self.octets()[0] & 240 == 240 && !self.is_broadcast(); + fn is_reserved(addr: &std::net::Ipv4Addr) -> bool { + addr.octets()[0] & 240 == 240 && !addr.is_broadcast() + } // Copied from the unstable method `std::net::Ipv4Addr::is_benchmarking`. - let is_benchmarking = || self.octets()[0] == 198 && (self.octets()[1] & 0xfe) == 18; + fn is_benchmarking(addr: &std::net::Ipv4Addr) -> bool { + addr.octets()[0] == 198 && (addr.octets()[1] & 0xfe) == 18 + } !self.is_private() && !self.is_loopback() && !self.is_link_local() && !self.is_broadcast() && !self.is_documentation() - && !is_shared() + && !is_shared(self) // addresses reserved for future protocols (`192.0.0.0/24`) && !(self.octets()[0] == 192 && self.octets()[1] == 0 && self.octets()[2] == 0) - && !is_reserved() - && !is_benchmarking() + && !is_reserved(self) + && !is_benchmarking(self) // Make sure the address is not in 0.0.0.0/8 && self.octets()[0] != 0 } @@ -596,11 +601,24 @@ impl GlobalIp for std::net::Ipv6Addr { // See https://github.com/rust-lang/rust/pull/86634 and https://github.com/rust-lang/rust/issues/85604. fn is_global_ip(&self) -> bool { let is_unicast_global = || { - let is_unicast_link_local = (self.segments()[0] & 0xffc0) == 0xfe80; - let is_unique_local = (self.segments()[0] & 0xfe00) == 0xfc00; - let is_documentation = (self.segments()[0] == 0x2001) && (self.segments()[1] == 0xdb8); + // Copied from the unstable method `std::net::Ipv6Addr::is_unicast_link_local`. + fn is_unicast_link_local(addr: &std::net::Ipv6Addr) -> bool { + (addr.segments()[0] & 0xffc0) == 0xfe80 + } + // Copied from the unstable method `std::net::Ipv6Addr::is_unique_local`. + fn is_unique_local(addr: &std::net::Ipv6Addr) -> bool { + (addr.segments()[0] & 0xfe00) == 0xfc00 + } + // Copied from the unstable method `std::net::Ipv6Addr::is_documentation`. + fn is_documentation(addr: &std::net::Ipv6Addr) -> bool { + (addr.segments()[0] == 0x2001) && (addr.segments()[1] == 0xdb8) + } - !self.is_loopback() && !is_unicast_link_local && !is_unique_local && is_documentation + !self.is_loopback() + && !is_unicast_link_local(self) + && !is_unique_local(self) + && !self.is_unspecified() + && !is_documentation(self) }; if !self.is_multicast() { From 9c2d90101caac127786cbf9225d737ee5f3c270b Mon Sep 17 00:00:00 2001 From: elenaf9 Date: Sun, 8 May 2022 19:43:00 +0200 Subject: [PATCH 04/14] autonat: literal copy of fns from std Ipv6Addr --- protocols/autonat/src/behaviour.rs | 66 ++++++++++++++++++------------ 1 file changed, 39 insertions(+), 27 deletions(-) diff --git a/protocols/autonat/src/behaviour.rs b/protocols/autonat/src/behaviour.rs index 3b8d52670ca..99dd633fa23 100644 --- a/protocols/autonat/src/behaviour.rs +++ b/protocols/autonat/src/behaviour.rs @@ -600,36 +600,48 @@ impl GlobalIp for std::net::Ipv6Addr { // TODO: There is a PR to change this, but seems inactive. Should we help push this forward? // See https://github.com/rust-lang/rust/pull/86634 and https://github.com/rust-lang/rust/issues/85604. fn is_global_ip(&self) -> bool { - let is_unicast_global = || { - // Copied from the unstable method `std::net::Ipv6Addr::is_unicast_link_local`. - fn is_unicast_link_local(addr: &std::net::Ipv6Addr) -> bool { - (addr.segments()[0] & 0xffc0) == 0xfe80 - } - // Copied from the unstable method `std::net::Ipv6Addr::is_unique_local`. - fn is_unique_local(addr: &std::net::Ipv6Addr) -> bool { - (addr.segments()[0] & 0xfe00) == 0xfc00 - } - // Copied from the unstable method `std::net::Ipv6Addr::is_documentation`. - fn is_documentation(addr: &std::net::Ipv6Addr) -> bool { - (addr.segments()[0] == 0x2001) && (addr.segments()[1] == 0xdb8) - } + // Copied from the unstable method `std::net::Ipv6Addr::is_unicast`. + fn is_unicast(addr: &std::net::Ipv6Addr) -> bool { + !addr.is_multicast() + } + // Copied from the unstable method `std::net::Ipv6Addr::is_unicast_link_local`. + fn is_unicast_link_local(addr: &std::net::Ipv6Addr) -> bool { + (addr.segments()[0] & 0xffc0) == 0xfe80 + } + // Copied from the unstable method `std::net::Ipv6Addr::is_unique_local`. + fn is_unique_local(addr: &std::net::Ipv6Addr) -> bool { + (addr.segments()[0] & 0xfe00) == 0xfc00 + } + // Copied from the unstable method `std::net::Ipv6Addr::is_documentation`. + fn is_documentation(addr: &std::net::Ipv6Addr) -> bool { + (addr.segments()[0] == 0x2001) && (addr.segments()[1] == 0xdb8) + } - !self.is_loopback() - && !is_unicast_link_local(self) - && !is_unique_local(self) - && !self.is_unspecified() - && !is_documentation(self) - }; + // Copied from the unstable method `std::net::Ipv6Addr::is_unicast_global`. + fn is_unicast_global(addr: &std::net::Ipv6Addr) -> bool { + is_unicast(addr) + && !addr.is_loopback() + && !is_unicast_link_local(addr) + && !is_unique_local(addr) + && !addr.is_unspecified() + && !is_documentation(addr) + } - if !self.is_multicast() { - return is_unicast_global(); + // Variation of unstable method [`std::net::Ipv6Addr::multicast_scope`] that instead of the + // `Ipv6MulticastScope` just return if the scope is global or not. + // Equivalent to `Ipv6Addr::multicast_scope(..).map(|scope| matches!(scope, Ipv6MulticastScope::Global))`. + fn is_multicast_scope_global(addr: &std::net::Ipv6Addr) -> Option { + match addr.segments()[0] & 0x000f { + 14 => Some(true), // Global multicast scope. + 1..=5 | 8 => Some(false), // Local multicast scope. + _ => None, // Unknown multicast scope. + } } - // Check scope of the multicast address. - // Copied from unstable method [`std::net::Ipv6Addr::multicast_scope`]. - match self.segments()[0] & 0x000f { - 14 => true, // Global multicast scope. - 1..=5 | 8 => false, // Local multicast scope. - _ => is_unicast_global(), // Unknown multicast scope. + + match is_multicast_scope_global(self) { + Some(true) => true, + None => is_unicast_global(self), + _ => false, } } } From b0f9713751db041235a8e52ea15e761745eb4107 Mon Sep 17 00:00:00 2001 From: elenaf9 Date: Sun, 8 May 2022 20:43:51 +0200 Subject: [PATCH 05/14] autonat: return DialRefused on relayed/ private ip --- protocols/autonat/src/behaviour.rs | 5 ++++- protocols/autonat/src/behaviour/as_server.rs | 4 ++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/protocols/autonat/src/behaviour.rs b/protocols/autonat/src/behaviour.rs index 99dd633fa23..a7fda6a7149 100644 --- a/protocols/autonat/src/behaviour.rs +++ b/protocols/autonat/src/behaviour.rs @@ -78,7 +78,10 @@ pub struct Config { pub throttle_clients_peer_max: usize, /// Period for throttling clients requests. pub throttle_clients_period: Duration, - /// Reject probes for clients that are observed at a non-global ip address. + /// As a server reject probes for clients that are observed at a non-global ip address. + /// Correspondingly as a client only pick peers as server that are not observed at a + /// private ip address. Note that this does not apply for servers that are added via + /// [`Behaviour::add_server`]. pub only_global_ips: bool, } diff --git a/protocols/autonat/src/behaviour/as_server.rs b/protocols/autonat/src/behaviour/as_server.rs index 24ffb443deb..4b4b7301182 100644 --- a/protocols/autonat/src/behaviour/as_server.rs +++ b/protocols/autonat/src/behaviour/as_server.rs @@ -295,8 +295,8 @@ impl<'a> AsServer<'a> { .values() .find_map(|a| a.as_ref()) .ok_or_else(|| { - let status_text = "no dial-request over relayed connections".to_string(); - (status_text, ResponseError::DialError) + let status_text = "refusing to dial peer with blocked observed address".to_string(); + (status_text, ResponseError::DialRefused) })?; let mut addrs = Self::filter_valid_addrs(sender, request.addresses, observed_addr); From 99fec62b646488d4b7ab42d6a338927fd31372a4 Mon Sep 17 00:00:00 2001 From: elenaf9 Date: Sun, 8 May 2022 20:46:38 +0200 Subject: [PATCH 06/14] autonat: test only_global_ips config --- protocols/autonat/tests/test_client.rs | 61 +++++++++++++++++++++++++- protocols/autonat/tests/test_server.rs | 38 ++++++++++++++-- 2 files changed, 94 insertions(+), 5 deletions(-) diff --git a/protocols/autonat/tests/test_client.rs b/protocols/autonat/tests/test_client.rs index 4155c84a845..1b61722d8f3 100644 --- a/protocols/autonat/tests/test_client.rs +++ b/protocols/autonat/tests/test_client.rs @@ -35,11 +35,10 @@ const MAX_CONFIDENCE: usize = 3; const TEST_RETRY_INTERVAL: Duration = Duration::from_secs(1); const TEST_REFRESH_INTERVAL: Duration = Duration::from_secs(2); -async fn init_swarm(mut config: Config) -> Swarm { +async fn init_swarm(config: Config) -> Swarm { let keypair = Keypair::generate_ed25519(); let local_id = PeerId::from_public_key(&keypair.public()); let transport = development_transport(keypair).await.unwrap(); - config.only_global_ips = false; let behaviour = Behaviour::new(local_id, config); Swarm::new(transport, behaviour, local_id) } @@ -50,6 +49,7 @@ async fn spawn_server(kill: oneshot::Receiver<()>) -> (PeerId, Multiaddr) { let mut server = init_swarm(Config { boot_delay: Duration::from_secs(60), throttle_clients_peer_max: usize::MAX, + only_global_ips: false, ..Default::default() }) .await; @@ -101,6 +101,7 @@ async fn test_auto_probe() { retry_interval: TEST_RETRY_INTERVAL, refresh_interval: TEST_REFRESH_INTERVAL, confidence_max: MAX_CONFIDENCE, + only_global_ips: false, throttle_server_period: Duration::ZERO, boot_delay: Duration::ZERO, ..Default::default() @@ -257,6 +258,7 @@ async fn test_confidence() { retry_interval: TEST_RETRY_INTERVAL, refresh_interval: TEST_REFRESH_INTERVAL, confidence_max: MAX_CONFIDENCE, + only_global_ips: false, throttle_server_period: Duration::ZERO, boot_delay: Duration::from_millis(100), ..Default::default() @@ -352,6 +354,7 @@ async fn test_throttle_server_period() { retry_interval: TEST_RETRY_INTERVAL, refresh_interval: TEST_REFRESH_INTERVAL, confidence_max: MAX_CONFIDENCE, + only_global_ips: false, // Throttle servers so they can not be re-used for dial request. throttle_server_period: Duration::from_secs(1000), boot_delay: Duration::from_millis(100), @@ -413,6 +416,7 @@ async fn test_use_connected_as_server() { retry_interval: TEST_RETRY_INTERVAL, refresh_interval: TEST_REFRESH_INTERVAL, confidence_max: MAX_CONFIDENCE, + only_global_ips: false, throttle_server_period: Duration::ZERO, boot_delay: Duration::from_millis(100), ..Default::default() @@ -467,6 +471,7 @@ async fn test_outbound_failure() { retry_interval: TEST_RETRY_INTERVAL, refresh_interval: TEST_REFRESH_INTERVAL, confidence_max: MAX_CONFIDENCE, + only_global_ips: false, throttle_server_period: Duration::ZERO, boot_delay: Duration::from_millis(100), ..Default::default() @@ -530,3 +535,55 @@ async fn test_outbound_failure() { run_test_with_timeout(test).await; } + +#[async_std::test] +async fn test_global_ips_config() { + let test = async { + let mut client = init_swarm(Config { + retry_interval: TEST_RETRY_INTERVAL, + refresh_interval: TEST_REFRESH_INTERVAL, + confidence_max: MAX_CONFIDENCE, + // Enforce that only peers outside of the local network are used as servers. + only_global_ips: true, + boot_delay: Duration::from_millis(100), + ..Default::default() + }) + .await; + + client + .listen_on("/ip4/0.0.0.0/tcp/0".parse().unwrap()) + .unwrap(); + loop { + match client.select_next_some().await { + SwarmEvent::NewListenAddr { .. } => break, + _ => {} + } + } + + let (_handle, rx) = oneshot::channel(); + let (server_id, addr) = spawn_server(rx).await; + + // Dial server instead of adding it via `Behaviour::add_server` because the + // `only_global_ips` restriction does not apply for manually added servers. + client.dial(addr).unwrap(); + loop { + if let SwarmEvent::ConnectionEstablished { peer_id, .. } = + client.select_next_some().await + { + assert_eq!(peer_id, server_id); + break; + } + } + + // Expect that the server is not qualified for dial-back because it is observed + // at a local IP. + match next_event(&mut client).await { + Event::OutboundProbe(OutboundProbeEvent::Error { error, .. }) => { + assert!(matches!(error, OutboundProbeError::NoServer)) + } + other => panic!("Unexpected behaviour event: {:?}.", other), + } + }; + + run_test_with_timeout(test).await; +} diff --git a/protocols/autonat/tests/test_server.rs b/protocols/autonat/tests/test_server.rs index 1979dbaf267..cccfa172dc5 100644 --- a/protocols/autonat/tests/test_server.rs +++ b/protocols/autonat/tests/test_server.rs @@ -34,17 +34,19 @@ use libp2p_core::{ConnectedPoint, Endpoint}; use libp2p_swarm::DialError; use std::{num::NonZeroU32, time::Duration}; -async fn init_swarm(mut config: Config) -> Swarm { +async fn init_swarm(config: Config) -> Swarm { let keypair = Keypair::generate_ed25519(); let local_id = PeerId::from_public_key(&keypair.public()); let transport = development_transport(keypair).await.unwrap(); - config.only_global_ips = false; let behaviour = Behaviour::new(local_id, config); Swarm::new(transport, behaviour, local_id) } async fn init_server(config: Option) -> (Swarm, PeerId, Multiaddr) { - let mut config = config.unwrap_or_default(); + let mut config = config.unwrap_or_else(|| Config { + only_global_ips: false, + ..Default::default() + }); // Don't do any outbound probes. config.boot_delay = Duration::from_secs(60); @@ -75,6 +77,7 @@ async fn spawn_client( boot_delay: Duration::from_secs(1), retry_interval: Duration::from_secs(1), throttle_server_period: Duration::ZERO, + only_global_ips: false, ..Default::default() }) .await; @@ -284,6 +287,7 @@ async fn test_throttle_global_max() { let (mut server, server_id, server_addr) = init_server(Some(Config { throttle_clients_global_max: 1, throttle_clients_period: Duration::from_secs(60), + only_global_ips: false, ..Default::default() })) .await; @@ -332,6 +336,7 @@ async fn test_throttle_peer_max() { let (mut server, server_id, server_addr) = init_server(Some(Config { throttle_clients_peer_max: 1, throttle_clients_period: Duration::from_secs(60), + only_global_ips: false, ..Default::default() })) .await; @@ -383,6 +388,7 @@ async fn test_dial_multiple_addr() { let (mut server, server_id, server_addr) = init_server(Some(Config { throttle_clients_peer_max: 1, throttle_clients_period: Duration::from_secs(60), + only_global_ips: false, ..Default::default() })) .await; @@ -429,3 +435,29 @@ async fn test_dial_multiple_addr() { run_test_with_timeout(test).await; } + +#[async_std::test] +async fn test_global_ips_config() { + let test = async { + let (mut server, server_id, server_addr) = init_server(Some(Config { + // Enforce that only clients outside of the local network are qualified for dial-backs. + only_global_ips: true, + ..Default::default() + })) + .await; + + let (_handle, rx) = oneshot::channel(); + spawn_client(true, false, server_id, server_addr.clone(), rx).await; + + // Expect the probe to be refused as both peers run on the same machine and thus in the same local network. + match next_event(&mut server).await { + Event::InboundProbe(InboundProbeEvent::Error { error, .. }) => assert!(matches!( + error, + InboundProbeError::Response(ResponseError::DialRefused) + )), + other => panic!("Unexpected behaviour event: {:?}.", other), + }; + }; + + run_test_with_timeout(test).await; +} From d3d90156df1ba40611ee801747366c529d144774 Mon Sep 17 00:00:00 2001 From: elenaf9 Date: Sun, 8 May 2022 20:50:49 +0200 Subject: [PATCH 07/14] autonat: remove unneeded explicit `drop` --- protocols/autonat/tests/test_client.rs | 8 -------- protocols/autonat/tests/test_server.rs | 8 -------- 2 files changed, 16 deletions(-) diff --git a/protocols/autonat/tests/test_client.rs b/protocols/autonat/tests/test_client.rs index 1b61722d8f3..8e47fc127ab 100644 --- a/protocols/autonat/tests/test_client.rs +++ b/protocols/autonat/tests/test_client.rs @@ -244,8 +244,6 @@ async fn test_auto_probe() { assert_eq!(client.behaviour().confidence(), 0); assert!(client.behaviour().nat_status().is_public()); assert!(client.behaviour().public_address().is_some()); - - drop(_handle); }; run_test_with_timeout(test).await; @@ -340,8 +338,6 @@ async fn test_confidence() { } } } - - drop(_handle); }; run_test_with_timeout(test).await; @@ -402,8 +398,6 @@ async fn test_throttle_server_period() { other => panic!("Unexpected behaviour event: {:?}.", other), } assert_eq!(client.behaviour().confidence(), 0); - - drop(_handle) }; run_test_with_timeout(test).await; @@ -455,8 +449,6 @@ async fn test_use_connected_as_server() { } other => panic!("Unexpected behaviour event: {:?}.", other), } - - drop(_handle); }; run_test_with_timeout(test).await; diff --git a/protocols/autonat/tests/test_server.rs b/protocols/autonat/tests/test_server.rs index cccfa172dc5..36328319e9f 100644 --- a/protocols/autonat/tests/test_server.rs +++ b/protocols/autonat/tests/test_server.rs @@ -228,8 +228,6 @@ async fn test_dial_back() { } other => panic!("Unexpected behaviour event: {:?}.", other), } - - drop(_handle); }; run_test_with_timeout(test).await; @@ -274,8 +272,6 @@ async fn test_dial_error() { } other => panic!("Unexpected behaviour event: {:?}.", other), } - - drop(_handle); }; run_test_with_timeout(test).await; @@ -323,8 +319,6 @@ async fn test_throttle_global_max() { other => panic!("Unexpected behaviour event: {:?}.", other), }; } - - drop(_handles); }; run_test_with_timeout(test).await; @@ -375,8 +369,6 @@ async fn test_throttle_peer_max() { } other => panic!("Unexpected behaviour event: {:?}.", other), }; - - drop(_handle); }; run_test_with_timeout(test).await; From 05d5f2dcd643e7ff81b308e5fb1fdd192265d2a4 Mon Sep 17 00:00:00 2001 From: elenaf9 Date: Sun, 8 May 2022 20:52:54 +0200 Subject: [PATCH 08/14] autonat: remove outdated comment --- protocols/autonat/src/behaviour.rs | 8 -------- 1 file changed, 8 deletions(-) diff --git a/protocols/autonat/src/behaviour.rs b/protocols/autonat/src/behaviour.rs index a7fda6a7149..8f005b9052c 100644 --- a/protocols/autonat/src/behaviour.rs +++ b/protocols/autonat/src/behaviour.rs @@ -543,12 +543,6 @@ impl GlobalIp for std::net::Ipv4Addr { // NOTE: The below logic is copied from `std::net::Ipv4Addr::is_global`, which is at the time of // writing behind the unstable `ip` feature. // See https://github.com/rust-lang/rust/issues/27709 for more info. - // - // TODO: Consider removing check check with the unstable methods `is_shared`, `is_reserved` and `is_benchmarking`? In case - // of the former the peer is still eligible for a dial-back, the latter cases should never occur in practice. - // - // TODO: Consider to only check for loopback, private ip and link-local. Can any other case ever happen in practice if we solely - // use this check for observed addresses? fn is_global_ip(&self) -> bool { // Check if this address is 192.0.0.9 or 192.0.0.10. These addresses are the only two // globally routable addresses in the 192.0.0.0/24 range. @@ -600,8 +594,6 @@ impl GlobalIp for std::net::Ipv6Addr { // // Note that contrary to `Ipv4Addr::is_global_ip` this currently checks for global scope // rather than global reachability. - // TODO: There is a PR to change this, but seems inactive. Should we help push this forward? - // See https://github.com/rust-lang/rust/pull/86634 and https://github.com/rust-lang/rust/issues/85604. fn is_global_ip(&self) -> bool { // Copied from the unstable method `std::net::Ipv6Addr::is_unicast`. fn is_unicast(addr: &std::net::Ipv6Addr) -> bool { From 5b3b90931905b5c949b594b87e82a748bfdf838f Mon Sep 17 00:00:00 2001 From: elenaf9 Date: Sun, 8 May 2022 21:03:52 +0200 Subject: [PATCH 09/14] autonat: add changelog entry --- protocols/autonat/CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/protocols/autonat/CHANGELOG.md b/protocols/autonat/CHANGELOG.md index f76cf47e123..d3ccd98876d 100644 --- a/protocols/autonat/CHANGELOG.md +++ b/protocols/autonat/CHANGELOG.md @@ -6,6 +6,11 @@ - Update to `libp2p-request-response` `v0.18.0`. +- Add `Config::only_global_ips` to skip peers that are observed at a private IP-address + (see PR 2618). + +[PR 2445]: https://github.com/libp2p/rust-libp2p/pull/2618 + # 0.3.0 - Update to `libp2p-swarm` `v0.35.0`. From f7d47dad152bc020178ad9f1cce1e59539cc9a90 Mon Sep 17 00:00:00 2001 From: Elena Frank Date: Tue, 10 May 2022 22:12:33 +0200 Subject: [PATCH 10/14] autonat: fix changelog entry Co-authored-by: Max Inden --- protocols/autonat/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/protocols/autonat/CHANGELOG.md b/protocols/autonat/CHANGELOG.md index d3ccd98876d..78c3802780d 100644 --- a/protocols/autonat/CHANGELOG.md +++ b/protocols/autonat/CHANGELOG.md @@ -7,7 +7,7 @@ - Update to `libp2p-request-response` `v0.18.0`. - Add `Config::only_global_ips` to skip peers that are observed at a private IP-address - (see PR 2618). + (see [PR 2618]). [PR 2445]: https://github.com/libp2p/rust-libp2p/pull/2618 From 2954c1a8f7d748d36587daac35d643f4ada6ab7a Mon Sep 17 00:00:00 2001 From: elenaf9 Date: Tue, 10 May 2022 22:19:30 +0200 Subject: [PATCH 11/14] autonat: remove redundant check We only add an observed addresses for a peer to Behaviour.connected if the address is not relayed. Hence there is no need to check the address again in `filter_valid_addrs`. --- protocols/autonat/src/behaviour/as_server.rs | 23 -------------------- 1 file changed, 23 deletions(-) diff --git a/protocols/autonat/src/behaviour/as_server.rs b/protocols/autonat/src/behaviour/as_server.rs index 4b4b7301182..d0f727ccd5b 100644 --- a/protocols/autonat/src/behaviour/as_server.rs +++ b/protocols/autonat/src/behaviour/as_server.rs @@ -316,10 +316,6 @@ impl<'a> AsServer<'a> { demanded: Vec, observed_remote_at: &Multiaddr, ) -> Vec { - // Skip if the observed address is a relay address. - if observed_remote_at.iter().any(|p| p == Protocol::P2pCircuit) { - return Vec::new(); - } let observed_ip = match observed_remote_at .into_iter() .find(|p| matches!(p, Protocol::Ip4(_) | Protocol::Ip6(_))) @@ -417,23 +413,4 @@ mod test { .with(Protocol::P2p(peer_id.into())); assert_eq!(filtered, vec![expected_1, expected_2]); } - - #[test] - fn skip_relayed_addr() { - let peer_id = PeerId::random(); - let observed_ip = random_ip(); - // Observed address is relayed. - let observed_addr = Multiaddr::empty() - .with(observed_ip.clone()) - .with(random_port()) - .with(Protocol::P2p(PeerId::random().into())) - .with(Protocol::P2pCircuit) - .with(Protocol::P2p(peer_id.into())); - let demanded = Multiaddr::empty() - .with(random_ip()) - .with(random_port()) - .with(Protocol::P2p(peer_id.into())); - let filtered = AsServer::filter_valid_addrs(peer_id, vec![demanded], &observed_addr); - assert!(filtered.is_empty()); - } } From 7035422cc0defcabb2f79efe01ca676225e5292e Mon Sep 17 00:00:00 2001 From: elenaf9 Date: Tue, 10 May 2022 23:33:05 +0200 Subject: [PATCH 12/14] autonat: return DialRefused if no dialable addrs --- protocols/autonat/src/behaviour/as_server.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/protocols/autonat/src/behaviour/as_server.rs b/protocols/autonat/src/behaviour/as_server.rs index d0f727ccd5b..9b045c02b4c 100644 --- a/protocols/autonat/src/behaviour/as_server.rs +++ b/protocols/autonat/src/behaviour/as_server.rs @@ -304,7 +304,7 @@ impl<'a> AsServer<'a> { if addrs.is_empty() { let status_text = "no dialable addresses".to_string(); - return Err((status_text, ResponseError::DialError)); + return Err((status_text, ResponseError::DialRefused)); } Ok(addrs) From 202e0ad49e7b226ba61d9da5f00b8f58f385b552 Mon Sep 17 00:00:00 2001 From: Elena Frank Date: Thu, 19 May 2022 22:33:07 +0200 Subject: [PATCH 13/14] autonat: fix typo --- protocols/autonat/src/behaviour.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/protocols/autonat/src/behaviour.rs b/protocols/autonat/src/behaviour.rs index 8f005b9052c..86c65bb3416 100644 --- a/protocols/autonat/src/behaviour.rs +++ b/protocols/autonat/src/behaviour.rs @@ -623,7 +623,7 @@ impl GlobalIp for std::net::Ipv6Addr { } // Variation of unstable method [`std::net::Ipv6Addr::multicast_scope`] that instead of the - // `Ipv6MulticastScope` just return if the scope is global or not. + // `Ipv6MulticastScope` just returns if the scope is global or not. // Equivalent to `Ipv6Addr::multicast_scope(..).map(|scope| matches!(scope, Ipv6MulticastScope::Global))`. fn is_multicast_scope_global(addr: &std::net::Ipv6Addr) -> Option { match addr.segments()[0] & 0x000f { From 54c968107bd50e21962f63e5509874701f04131b Mon Sep 17 00:00:00 2001 From: Elena Frank Date: Sun, 22 May 2022 21:32:00 +0200 Subject: [PATCH 14/14] protocols/autonat: fix typo in changelog --- protocols/autonat/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/protocols/autonat/CHANGELOG.md b/protocols/autonat/CHANGELOG.md index 78c3802780d..ab74a16cfb5 100644 --- a/protocols/autonat/CHANGELOG.md +++ b/protocols/autonat/CHANGELOG.md @@ -9,7 +9,7 @@ - Add `Config::only_global_ips` to skip peers that are observed at a private IP-address (see [PR 2618]). -[PR 2445]: https://github.com/libp2p/rust-libp2p/pull/2618 +[PR 2618]: https://github.com/libp2p/rust-libp2p/pull/2618 # 0.3.0