Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Optionally use only global IPs for AutoNAT #2526

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
19 changes: 19 additions & 0 deletions protocols/autonat/src/behaviour.rs
Expand Up @@ -52,6 +52,9 @@ pub struct Config {
/// Timeout for requests.
pub timeout: Duration,

/// Whether to consider only global IP addresses for NAT.
pub use_only_global_ips: bool,

// Client Config
/// Delay on init before starting the fist probe.
pub boot_delay: Duration,
Expand Down Expand Up @@ -83,6 +86,7 @@ impl Default for Config {
fn default() -> Self {
Config {
timeout: Duration::from_secs(30),
use_only_global_ips: false,
boot_delay: Duration::from_secs(15),
retry_interval: Duration::from_secs(90),
refresh_interval: Duration::from_secs(15 * 60),
Expand Down Expand Up @@ -510,3 +514,18 @@ trait HandleInnerEvent {
event: RequestResponseEvent<DialRequest, DialResponse>,
) -> (VecDeque<Event>, Option<Action>);
}

// Test for global IP address implemented on MultiAddr
trait GlobalAddress {
fn contains_global_address(&self) -> bool;
}

impl GlobalAddress for Multiaddr {
fn contains_global_address(&self) -> bool {
self.into_iter().any(|proto| match proto {
libp2p_core::multiaddr::Protocol::Ip4(addr) => addr.is_global(),
libp2p_core::multiaddr::Protocol::Ip6(addr) => addr.is_global(),
_ => false,
})
}
}
10 changes: 8 additions & 2 deletions protocols/autonat/src/behaviour/as_client.rs
Expand Up @@ -21,8 +21,8 @@
use crate::ResponseError;

use super::{
Action, AutoNatCodec, Config, DialRequest, DialResponse, Event, HandleInnerEvent, NatStatus,
ProbeId,
Action, AutoNatCodec, Config, DialRequest, DialResponse, Event, GlobalAddress,
HandleInnerEvent, NatStatus, ProbeId,
};
use futures::FutureExt;
use futures_timer::Delay;
Expand Down Expand Up @@ -201,6 +201,12 @@ impl<'a> AsClient<'a> {

let mut addresses: Vec<_> = params.external_addresses().map(|r| r.addr).collect();
addresses.extend(params.listened_addresses());
if self.config.use_only_global_ips {
addresses = addresses
.into_iter()
.filter(|a| a.contains_global_address())
.collect::<Vec<_>>();
};
Comment on lines +204 to +209
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When a client sends a dial-back request to the server, the server replaces the IP in the sent addresses with the IP at which it observes the client. So effectively, the server never tests an IP the client sends, but instead only the ports of each address. Therefore it is not necessary to filter the addresses here. On the contrary: if we filter them here we remove all of the params.listened_addresses(), since those are always private.


let probe_id = self.probe_id.next();
let event = match self.do_probe(probe_id, addresses) {
Expand Down
15 changes: 12 additions & 3 deletions protocols/autonat/src/behaviour/as_server.rs
Expand Up @@ -19,8 +19,8 @@
// DEALINGS IN THE SOFTWARE.

use super::{
Action, AutoNatCodec, Config, DialRequest, DialResponse, Event, HandleInnerEvent, ProbeId,
ResponseError,
Action, AutoNatCodec, Config, DialRequest, DialResponse, Event, GlobalAddress,
HandleInnerEvent, ProbeId, ResponseError,
};
use instant::Instant;
use libp2p_core::{connection::ConnectionId, multiaddr::Protocol, Multiaddr, PeerId};
Expand Down Expand Up @@ -114,13 +114,22 @@ impl<'a> HandleInnerEvent for AsServer<'a> {
} => {
let probe_id = self.probe_id.next();
match self.resolve_inbound_request(peer, request) {
Ok(addrs) => {
Ok(mut addrs) => {
log::debug!(
"Inbound dial request from Peer {} with dial-back addresses {:?}.",
peer,
addrs
);

if self.config.use_only_global_ips {
addrs = addrs
.into_iter()
.filter(|a| {
!self.config.use_only_global_ips || a.contains_global_address()
})
.collect();
}

Comment on lines +124 to +132
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this logic should be moved into the AsServer::resolve_inbound_request method. If the client is in the same private network (= the observed IP is private), we want to directly return an Error and not do a dial-attempt.

self.ongoing_inbound
.insert(peer, (probe_id, request_id, addrs.clone(), channel));
self.throttled_clients.push((peer, Instant::now()));
Expand Down
1 change: 1 addition & 0 deletions protocols/autonat/src/lib.rs
Expand Up @@ -19,6 +19,7 @@
// DEALINGS IN THE SOFTWARE.

//! Implementation of the AutoNAT protocol.
#![feature(ip)]
mod behaviour;
mod protocol;

Expand Down