Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

don't add peers with only private addresses to your routing table #360

Merged
merged 6 commits into from
Jun 26, 2019
Merged

don't add peers with only private addresses to your routing table #360

Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
9387b5a
don't add peers with only private addresses to your routing table
whyrusleeping Jun 25, 2019
03293e8
add more filters
whyrusleeping Jun 25, 2019
c6cd6d5
use less wasteful update method
whyrusleeping Jun 25, 2019
8bf0863
address PR review
whyrusleeping Jun 26, 2019
f34a5b1
rejig for query filtering.
raulk Jun 26, 2019
9bc949d
rejig v2.
raulk Jun 26, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 18 additions & 19 deletions dht.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -299,18 +299,29 @@ func (dht *IpfsDHT) shouldAddPeerToRoutingTable(c network.Conn) bool {
if isRelayAddr(c.RemoteMultiaddr()) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should only check this for outbound connections. If we're behind a relay and operating in client mode, we still want to use relayed peers in our DHT.

return false
}
return c.Stat().Direction == network.DirOutbound ||
dht.hasSensibleAddressesForPeer(c)
if c.Stat().Direction == network.DirOutbound {
// we established this connection, so they're definitely diallable.
return true
}
if dht.peerIsOnSameSubnet(c) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if we do this here, we don't return false for peers with no addresses at all, or peers with relay addresses.

Copy link
Member

@raulk raulk Jun 26, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

peers with relay addrs are caught in L299. let me look at the 0-addr case.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wait no, they're covered iff we're connected to them via a relay, but not if we are directly connected but they advertise relay addrs.

// TODO: for now, we can't easily tell if the peer on our subnet
// is dialable or not, so don't discriminate.
magik6k marked this conversation as resolved.
Show resolved Hide resolved

// We won't return these peers in queries unless the requester's
// remote addr is also private.
return true
}
ai := dht.host.Peerstore().PeerInfo(c.RemotePeer())
return dht.hasSensibleAddressesForPeer(ai)
}

func (dht *IpfsDHT) hasSensibleAddressesForPeer(c network.Conn) bool {
addrs := dht.host.Peerstore().Addrs(c.RemotePeer())
if len(addrs) == 0 {
func (dht *IpfsDHT) hasSensibleAddressesForPeer(ai peer.AddrInfo) bool {
if len(ai.Addrs) == 0 {
return false
}

var hasPublicAddr bool
for _, a := range addrs {
for _, a := range ai.Addrs {
if isRelayAddr(a) {
return false
}
Expand All @@ -319,19 +330,7 @@ func (dht *IpfsDHT) hasSensibleAddressesForPeer(c network.Conn) bool {
hasPublicAddr = true
}
}

if dht.peerIsOnSameSubnet(c) && !hasPublicAddr {
// TODO: for now, we can't easily tell if the peer on our subnet
// is dialable or not, so don't discriminate.
// ! It may be okay to not add these to our routing table...
return true
}

if !hasPublicAddr {
return false
}

return true
return hasPublicAddr
}

// taken from go-libp2p/p2p/host/relay
Expand Down