TOB-FUEL-20: Unfair processing of events

[xgreenx]

Description

The following poll function processes events from Kademlia DHT and mdns in an unfair way. 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.

Figure 20.1: Function which polls events from the Kademlia extension and mdns. (fuel-core/crates/services/p2p/src/discovery.rs#160–262)

// gets polled by the swarm
fn poll(
    &mut self,
    cx: &mut Context<'_>,
    params: &mut impl PollParameters,
) -> Poll<NetworkBehaviourAction<Self::OutEvent, Self::ConnectionHandler>> {
    // ...
    // poll Kademlia behaviour
    while let Poll::Ready(kad_action) = self.kademlia.poll(cx, params) {
        match kad_action {
            // ...
            NetworkBehaviourAction::Dial { handler, opts } => {
                return Poll::Ready(NetworkBehaviourAction::Dial { handler, opts })
            }
// ...
_ => {} }
}
    while let Poll::Ready(mdns_event) = self.mdns.poll(cx, params) {
        match mdns_event {
// ...
            NetworkBehaviourAction::ReportObservedAddr { address, score } => {
                return Poll::Ready(NetworkBehaviourAction::ReportObservedAddr {
address,
score, })
}
// ...
_ => {} }
    }
    Poll::Pending
}

Exploit Scenario

An attacker constantly causes a NetworkBehaviourAction::Dial event to be available. This way the mdns might not get its events processed.

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]

Related to the #1065. But maybe p2p implementation of discovery service has the same problem.

[xgreenx]

We removed pending_events during work on #1065.

Regarding unfair events for mdns, we decided not to fix it because it is disabled in production. It is mostly used for local development.

Maybe later, we will remove support for MDNS entirly.