Query peerlist should contain network addresses

[guillaumemichel]

The following security issue concerns both go-libp2p-kad-dht and go-kademlia.

Each query needs to keep track of which peers it is (1) already queried, (2) couldn't connect, (3) waiting on a response, or (4) didn't query yet.

A peer should not be marked as couldn't connect, after the first dial failed.

Let's consider the following scenario. A is looking for the 20 closest peers to a specific key (e.g to allocate a provider record). As it converges, A sends a request to B replying with closer peers to the target key. B response include the peer.AddrInfo of C, but with invalid multiaddresses. A tries to query C but cannot connect, as it doesn't know the right multiaddress of C. A marks C as couldn't connect. Later, A receives a response from D containing C's actual multiaddress. A will not try to contact C again, because it considers that this peer.ID is unresponsive. If C is among the 20 closest peers to the target key, then A will miss it, even though it could have reached it.

This has security implications, because it facilitates eclipse attacks. Multiple malicious peers can make it hard to discover a victim, by spreading an incorrect multiaddress. It wouldn't prevent honest nodes to provide the actual address of the victim, but the requester will likely not even try it.

It is OK to classify PeerIDs as (1) already queried, or (4) didn't query yet (and even (3) waiting on a response). It would be better not to classify the PeerID as (2) couldn't connect, but rather the union of peer.ID and the attempted multiaddress. This would allow the query to try connecting to a new multiaddress if a new one is discovered in the future.

[guillaumemichel]

#41 addresses this issue in go-kademlia