Unfair processing of events in peer discovery

[Dentosal]

From the audit report

It is possible that events by mdns might not get polled if Kademlia always returns a
Poll:Ready(...) result. An attacker might be able to exploit this behavior and block
mdns from working correctly by causing Kademlia DHT to always yield a result when polled

fuel-core/crates/services/p2p/src/discovery.rs

Lines 160 to 262 in 94a427b

	// gets polled by the swarm
	fn poll(
	&mut self,
	cx: &mut Context<'_>,
	params: &mut impl PollParameters,
	) -> Poll<NetworkBehaviourAction<Self::OutEvent, Self::ConnectionHandler>> {
	if let Some(next_event) = self.pending_events.pop_front() {
	return Poll::Ready(NetworkBehaviourAction::GenerateEvent(next_event))
	}
	// if random walk is enabled poll the stream that will fire when random walk is scheduled
	if let Some(next_kad_random_query) = self.next_kad_random_walk.as_mut() {
	while next_kad_random_query.poll_unpin(cx).is_ready() {
	if self.connected_peers.len() < self.max_peers_connected {
	let random_peer_id = PeerId::random();
	self.kademlia.get_closest_peers(random_peer_id);
	}
	*next_kad_random_query =
	Box::pin(tokio::time::sleep(self.duration_to_next_kad));
	// duration to next random walk should either be exponentially bigger than the previous
	// or at max 60 seconds
	self.duration_to_next_kad =
	std::cmp::min(self.duration_to_next_kad * 2, SIXTY_SECONDS);
	}
	}
	// poll Kademlia behaviour
	while let Poll::Ready(kad_action) = self.kademlia.poll(cx, params) {
	match kad_action {
	NetworkBehaviourAction::GenerateEvent(
	KademliaEvent::UnroutablePeer { peer },
	) => {
	return Poll::Ready(NetworkBehaviourAction::GenerateEvent(
	DiscoveryEvent::UnroutablePeer(peer),
	))
	}
	NetworkBehaviourAction::Dial { handler, opts } => {
	return Poll::Ready(NetworkBehaviourAction::Dial { handler, opts })
	}
	NetworkBehaviourAction::CloseConnection {
	peer_id,
	connection,
	} => {
	return Poll::Ready(NetworkBehaviourAction::CloseConnection {
	peer_id,
	connection,
	})
	}
	NetworkBehaviourAction::NotifyHandler {
	peer_id,
	handler,
	event,
	} => {
	return Poll::Ready(NetworkBehaviourAction::NotifyHandler {
	peer_id,
	handler,
	event,
	})
	}
	NetworkBehaviourAction::ReportObservedAddr { address, score } => {
	return Poll::Ready(NetworkBehaviourAction::ReportObservedAddr {
	address,
	score,
	})
	}
	_ => {}
	}
	}
	while let Poll::Ready(mdns_event) = self.mdns.poll(cx, params) {
	match mdns_event {
	NetworkBehaviourAction::GenerateEvent(MdnsEvent::Discovered(list)) => {
	// inform kademlia of newly discovered local peers
	// only if there aren't enough peers already connected
	if self.connected_peers.len() < self.max_peers_connected {
	for (peer_id, multiaddr) in list {
	self.kademlia.add_address(&peer_id, multiaddr);
	}
	}
	}
	NetworkBehaviourAction::ReportObservedAddr { address, score } => {
	return Poll::Ready(NetworkBehaviourAction::ReportObservedAddr {
	address,
	score,
	})
	}
	NetworkBehaviourAction::CloseConnection {
	peer_id,
	connection,
	} => {
	return Poll::Ready(NetworkBehaviourAction::CloseConnection {
	peer_id,
	connection,
	})
	}
	_ => {}
	}
	}
	Poll::Pending
	}

Recommendations

Short term, for every invocation of the poll function, switch between first polling Kademlia and mdns.
Long term, deploy metrics which watch poll queues. If an event queue is not polled regularly, then this could cause a situation like a deadlock

[xgreenx]

Oh, I missed the existence of this issue. Clsoe in favor of #1326