From 1893fc10de4bc6c1cf4455c637e9dd800f9f6831 Mon Sep 17 00:00:00 2001 From: Pierre Krieger Date: Sun, 10 Nov 2019 11:14:36 +0100 Subject: [PATCH] Add a --no-private-ipv4 CLI option (#4042) * Add a --no-private-ipv4 CLI option * Fix tests * Fix tests --- core/cli/src/lib.rs | 3 ++- core/cli/src/params.rs | 6 ++++++ core/network/src/behaviour.rs | 8 +++++++- core/network/src/config.rs | 6 ++++++ core/network/src/discovery.rs | 33 ++++++++++++++++++++++++++++----- core/network/src/service.rs | 6 +++++- core/service/test/src/lib.rs | 1 + node/cli/src/browser.rs | 1 + 8 files changed, 56 insertions(+), 8 deletions(-) diff --git a/core/cli/src/lib.rs b/core/cli/src/lib.rs index f24ff6eafa295..4bcffac112c44 100644 --- a/core/cli/src/lib.rs +++ b/core/cli/src/lib.rs @@ -343,7 +343,7 @@ impl<'a> ParseAndPrepareBuildSpec<'a> { ]; spec.add_boot_node(addr) } - + let json = service::chain_ops::build_spec(spec, raw_output)?; print!("{}", json); @@ -625,6 +625,7 @@ fn fill_network_configuration( config.transport = TransportConfig::Normal { enable_mdns: !is_dev && !cli.no_mdns, + allow_private_ipv4: !cli.no_private_ipv4, wasm_external_transport: None, }; diff --git a/core/cli/src/params.rs b/core/cli/src/params.rs index 7a296620da492..dc4a9f759b1d8 100644 --- a/core/cli/src/params.rs +++ b/core/cli/src/params.rs @@ -145,6 +145,12 @@ pub struct NetworkConfigurationParams { #[structopt(long = "port", value_name = "PORT")] pub port: Option, + /// Allow connecting to private IPv4 addresses (as specified in + /// [RFC1918](https://tools.ietf.org/html/rfc1918)), unless the address was passed with + /// `--reserved-nodes` or `--bootnodes`. + #[structopt(long = "no-private-ipv4")] + pub no_private_ipv4: bool, + /// Specify the number of outgoing connections we're trying to maintain. #[structopt(long = "out-peers", value_name = "COUNT", default_value = "25")] pub out_peers: u32, diff --git a/core/network/src/behaviour.rs b/core/network/src/behaviour.rs index 28830b326eaee..a0299bc340ce2 100644 --- a/core/network/src/behaviour.rs +++ b/core/network/src/behaviour.rs @@ -62,11 +62,17 @@ impl, H: ExHashT> Behaviour { local_public_key: PublicKey, known_addresses: Vec<(PeerId, Multiaddr)>, enable_mdns: bool, + allow_private_ipv4: bool, ) -> Self { Behaviour { substrate, debug_info: debug_info::DebugInfoBehaviour::new(user_agent, local_public_key.clone()), - discovery: DiscoveryBehaviour::new(local_public_key, known_addresses, enable_mdns), + discovery: DiscoveryBehaviour::new( + local_public_key, + known_addresses, + enable_mdns, + allow_private_ipv4 + ), events: Vec::new(), } } diff --git a/core/network/src/config.rs b/core/network/src/config.rs index b22e7d2790d45..d10345c2f3817 100644 --- a/core/network/src/config.rs +++ b/core/network/src/config.rs @@ -282,6 +282,7 @@ impl Default for NetworkConfiguration { node_name: "unknown".into(), transport: TransportConfig::Normal { enable_mdns: false, + allow_private_ipv4: true, wasm_external_transport: None, }, max_parallel_downloads: 5, @@ -327,6 +328,11 @@ pub enum TransportConfig { /// and connect to them if they support the same chain. enable_mdns: bool, + /// If true, allow connecting to private IPv4 addresses (as defined in + /// [RFC1918](https://tools.ietf.org/html/rfc1918)), unless the address has been passed in + /// [`NetworkConfiguration::reserved_nodes`] or [`NetworkConfiguration::boot_nodes`]. + allow_private_ipv4: bool, + /// Optional external implementation of a libp2p transport. Used in WASM contexts where we /// need some binding between the networking provided by the operating system or environment /// and libp2p. diff --git a/core/network/src/discovery.rs b/core/network/src/discovery.rs index f956875ca518f..2e0a6fe2447ea 100644 --- a/core/network/src/discovery.rs +++ b/core/network/src/discovery.rs @@ -85,6 +85,9 @@ pub struct DiscoveryBehaviour { local_peer_id: PeerId, /// Number of nodes we're currently connected to. num_connections: u64, + /// If false, `addresses_of_peer` won't return any private IPv4 address, except for the ones + /// stored in `user_defined`. + allow_private_ipv4: bool, } impl DiscoveryBehaviour { @@ -94,7 +97,8 @@ impl DiscoveryBehaviour { pub fn new( local_public_key: PublicKey, user_defined: Vec<(PeerId, Multiaddr)>, - enable_mdns: bool + enable_mdns: bool, + allow_private_ipv4: bool, ) -> Self { if enable_mdns { #[cfg(target_os = "unknown")] @@ -116,6 +120,7 @@ impl DiscoveryBehaviour { discoveries: VecDeque::new(), local_peer_id: local_public_key.into_peer_id(), num_connections: 0, + allow_private_ipv4, #[cfg(not(target_os = "unknown"))] mdns: if enable_mdns { match Mdns::new() { @@ -214,9 +219,27 @@ where let mut list = self.user_defined.iter() .filter_map(|(p, a)| if p == peer_id { Some(a.clone()) } else { None }) .collect::>(); - list.extend(self.kademlia.addresses_of_peer(peer_id)); - #[cfg(not(target_os = "unknown"))] - list.extend(self.mdns.addresses_of_peer(peer_id)); + + { + let mut list_to_filter = self.kademlia.addresses_of_peer(peer_id); + #[cfg(not(target_os = "unknown"))] + list_to_filter.extend(self.mdns.addresses_of_peer(peer_id)); + + if !self.allow_private_ipv4 { + list_to_filter.retain(|addr| { + if let Some(Protocol::Ip4(addr)) = addr.iter().next() { + if addr.is_private() { + return false; + } + } + + true + }); + } + + list.extend(list_to_filter); + } + trace!(target: "sub-libp2p", "Addresses of {:?} are {:?}", peer_id, list); if list.is_empty() { if self.kademlia.kbuckets_entries().any(|p| p == peer_id) { @@ -457,7 +480,7 @@ mod tests { upgrade::apply(stream, upgrade, endpoint, libp2p::core::upgrade::Version::V1) }); - let behaviour = DiscoveryBehaviour::new(keypair.public(), user_defined.clone(), false); + let behaviour = DiscoveryBehaviour::new(keypair.public(), user_defined.clone(), false, true); let mut swarm = Swarm::new(transport, behaviour, keypair.public().into_peer_id()); let listen_addr: Multiaddr = format!("/memory/{}", rand::random::()).parse().unwrap(); diff --git a/core/network/src/service.rs b/core/network/src/service.rs index 7ddc27995c523..437e978f4bd47 100644 --- a/core/network/src/service.rs +++ b/core/network/src/service.rs @@ -225,7 +225,11 @@ impl, H: ExHashT> NetworkWorker match params.network_config.transport { TransportConfig::MemoryOnly => false, TransportConfig::Normal { enable_mdns, .. } => enable_mdns, - } + }, + match params.network_config.transport { + TransportConfig::MemoryOnly => false, + TransportConfig::Normal { allow_private_ipv4, .. } => allow_private_ipv4, + }, ); let (transport, bandwidth) = { let (config_mem, config_wasm) = match params.network_config.transport { diff --git a/core/service/test/src/lib.rs b/core/service/test/src/lib.rs index af02c7c3aae52..878e3e3c4a592 100644 --- a/core/service/test/src/lib.rs +++ b/core/service/test/src/lib.rs @@ -158,6 +158,7 @@ fn node_config ( node_name: "unknown".to_owned(), transport: TransportConfig::Normal { enable_mdns: false, + allow_private_ipv4: true, wasm_external_transport: None, }, max_parallel_downloads: NetworkConfiguration::default().max_parallel_downloads, diff --git a/node/cli/src/browser.rs b/node/cli/src/browser.rs index 702d67b55afa5..ada8a52e1e8b3 100644 --- a/node/cli/src/browser.rs +++ b/node/cli/src/browser.rs @@ -42,6 +42,7 @@ fn start_inner(wasm_ext: wasm_ext::ffi::Transport) -> Result::default_with_spec_and_base_path(chain_spec, None); config.network.transport = network::config::TransportConfig::Normal { wasm_external_transport: Some(wasm_ext.clone()), + allow_private_ipv4: true, enable_mdns: false, }; config.telemetry_external_transport = Some(wasm_ext);