diff --git a/substrate/client/cli/src/params/network_params.rs b/substrate/client/cli/src/params/network_params.rs index 79e7a70024..209742f54e 100644 --- a/substrate/client/cli/src/params/network_params.rs +++ b/substrate/client/cli/src/params/network_params.rs @@ -97,6 +97,14 @@ pub struct NetworkParams { /// By default this option is true for `--dev` and false otherwise. #[structopt(long)] pub discover_local: bool, + + /// Require iterative Kademlia DHT queries to use disjoint paths for increased resiliency in the + /// presence of potentially adversarial nodes. + /// + /// See the S/Kademlia paper for more information on the high level design as well as its + /// security improvements. + #[structopt(long)] + pub kademlia_disjoint_query_paths: bool, } impl NetworkParams { @@ -156,6 +164,7 @@ impl NetworkParams { }, max_parallel_downloads: self.max_parallel_downloads, allow_non_globals_in_dht: self.discover_local || is_dev, + kademlia_disjoint_query_paths: self.kademlia_disjoint_query_paths, } } } diff --git a/substrate/client/network/src/config.rs b/substrate/client/network/src/config.rs index c144bebb5f..86450dc6e7 100644 --- a/substrate/client/network/src/config.rs +++ b/substrate/client/network/src/config.rs @@ -423,6 +423,9 @@ pub struct NetworkConfiguration { pub max_parallel_downloads: u32, /// Should we insert non-global addresses into the DHT? pub allow_non_globals_in_dht: bool, + /// Require iterative Kademlia DHT queries to use disjoint paths for increased resiliency in the + /// presence of potentially adversarial nodes. + pub kademlia_disjoint_query_paths: bool, } impl NetworkConfiguration { @@ -454,6 +457,7 @@ impl NetworkConfiguration { }, max_parallel_downloads: 5, allow_non_globals_in_dht: false, + kademlia_disjoint_query_paths: false, } } diff --git a/substrate/client/network/src/discovery.rs b/substrate/client/network/src/discovery.rs index 6ef97708c1..ab9ee2d4db 100644 --- a/substrate/client/network/src/discovery.rs +++ b/substrate/client/network/src/discovery.rs @@ -84,7 +84,8 @@ pub struct DiscoveryConfig { allow_non_globals_in_dht: bool, discovery_only_if_under_num: u64, enable_mdns: bool, - kademlias: HashMap> + kademlia_disjoint_query_paths: bool, + protocol_ids: HashSet, } impl DiscoveryConfig { @@ -97,7 +98,8 @@ impl DiscoveryConfig { allow_non_globals_in_dht: false, discovery_only_if_under_num: std::u64::MAX, enable_mdns: false, - kademlias: HashMap::new() + kademlia_disjoint_query_paths: false, + protocol_ids: HashSet::new() } } @@ -112,12 +114,7 @@ impl DiscoveryConfig { where I: IntoIterator { - for (peer_id, addr) in user_defined { - for kad in self.kademlias.values_mut() { - kad.add_address(&peer_id, addr.clone()); - } - self.user_defined.push((peer_id, addr)) - } + self.user_defined.extend(user_defined); self } @@ -144,48 +141,71 @@ impl DiscoveryConfig { /// Add discovery via Kademlia for the given protocol. pub fn add_protocol(&mut self, id: ProtocolId) -> &mut Self { - let name = protocol_name_from_protocol_id(&id); - self.add_kademlia(id, name); + if self.protocol_ids.contains(&id) { + warn!(target: "sub-libp2p", "Discovery already registered for protocol {:?}", id); + return self; + } + + self.protocol_ids.insert(id); + self } - fn add_kademlia(&mut self, id: ProtocolId, proto_name: Vec) { - if self.kademlias.contains_key(&id) { - warn!(target: "sub-libp2p", "Discovery already registered for protocol {:?}", id); - return - } - - let mut config = KademliaConfig::default(); - config.set_protocol_name(proto_name); - // By default Kademlia attempts to insert all peers into its routing table once a dialing - // attempt succeeds. In order to control which peer is added, disable the auto-insertion and - // instead add peers manually. - config.set_kbucket_inserts(KademliaBucketInserts::Manual); - - let store = MemoryStore::new(self.local_peer_id.clone()); - let mut kad = Kademlia::with_config(self.local_peer_id.clone(), store, config); - - for (peer_id, addr) in &self.user_defined { - kad.add_address(peer_id, addr.clone()); - } - - self.kademlias.insert(id, kad); + /// Require iterative Kademlia DHT queries to use disjoint paths for increased resiliency in the + /// presence of potentially adversarial nodes. + pub fn use_kademlia_disjoint_query_paths(&mut self, value: bool) -> &mut Self { + self.kademlia_disjoint_query_paths = value; + self } /// Create a `DiscoveryBehaviour` from this config. pub fn finish(self) -> DiscoveryBehaviour { + let DiscoveryConfig { + local_peer_id, + user_defined, + allow_private_ipv4, + allow_non_globals_in_dht, + discovery_only_if_under_num, + enable_mdns, + kademlia_disjoint_query_paths, + protocol_ids, + } = self; + + let kademlias = protocol_ids.into_iter() + .map(|protocol_id| { + let proto_name = protocol_name_from_protocol_id(&protocol_id); + + let mut config = KademliaConfig::default(); + config.set_protocol_name(proto_name); + // By default Kademlia attempts to insert all peers into its routing table once a + // dialing attempt succeeds. In order to control which peer is added, disable the + // auto-insertion and instead add peers manually. + config.set_kbucket_inserts(KademliaBucketInserts::Manual); + config.disjoint_query_paths(kademlia_disjoint_query_paths); + + let store = MemoryStore::new(local_peer_id.clone()); + let mut kad = Kademlia::with_config(local_peer_id.clone(), store, config); + + for (peer_id, addr) in &user_defined { + kad.add_address(peer_id, addr.clone()); + } + + (protocol_id, kad) + }) + .collect(); + DiscoveryBehaviour { - user_defined: self.user_defined, - kademlias: self.kademlias, + user_defined, + kademlias, next_kad_random_query: Delay::new(Duration::new(0, 0)), duration_to_next_kad: Duration::from_secs(1), pending_events: VecDeque::new(), - local_peer_id: self.local_peer_id, + local_peer_id, num_connections: 0, - allow_private_ipv4: self.allow_private_ipv4, - discovery_only_if_under_num: self.discovery_only_if_under_num, + allow_private_ipv4, + discovery_only_if_under_num, #[cfg(not(target_os = "unknown"))] - mdns: if self.enable_mdns { + mdns: if enable_mdns { match Mdns::new() { Ok(mdns) => Some(mdns).into(), Err(err) => { @@ -196,7 +216,7 @@ impl DiscoveryConfig { } else { None.into() }, - allow_non_globals_in_dht: self.allow_non_globals_in_dht, + allow_non_globals_in_dht, known_external_addresses: LruHashSet::new( NonZeroUsize::new(MAX_KNOWN_EXTERNAL_ADDRESSES) .expect("value is a constant; constant is non-zero; qed.") diff --git a/substrate/client/network/src/service.rs b/substrate/client/network/src/service.rs index 0139739ad2..f0cf79182b 100644 --- a/substrate/client/network/src/service.rs +++ b/substrate/client/network/src/service.rs @@ -292,6 +292,7 @@ impl NetworkWorker { config.discovery_limit(u64::from(params.network_config.out_peers) + 15); config.add_protocol(params.protocol_id.clone()); config.allow_non_globals_in_dht(params.network_config.allow_non_globals_in_dht); + config.use_kademlia_disjoint_query_paths(params.network_config.kademlia_disjoint_query_paths); match params.network_config.transport { TransportConfig::MemoryOnly => {