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.

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

autorelay: curtail addrsplosion #598

Merged
merged 42 commits into from
Apr 20, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
42 commits
Select commit Hold shift + click to select a range
ffd4981
increase autorelay discovery limit to 1k
vyzo Apr 13, 2019
a1ebc4d
reduce relay find peer and connect timeout to 30s
vyzo Apr 13, 2019
aa03a9b
clean up relay address sets to curtail addrsplosion
vyzo Apr 14, 2019
9863e22
rework relay selection logic
vyzo Apr 14, 2019
765ed0e
select 50 relays
vyzo Apr 14, 2019
39f2b7a
fix selectRelays to return promptly when there are query errors
vyzo Apr 14, 2019
becb89a
shuffle relay set before queries
vyzo Apr 14, 2019
1138fb6
only determine default port if it is in a public addr
vyzo Apr 14, 2019
31dfaf6
increase relay advertising boot delay to 15min
vyzo Apr 14, 2019
c9f627e
increase FindPeer timeout to 30s
vyzo Apr 14, 2019
45d2888
don't drop ephemeral ports in address set clean up
vyzo Apr 14, 2019
da65fd7
fix and reinstate autorelay test
vyzo Apr 14, 2019
8c7da83
also check private addresses when looking for 4001
vyzo Apr 14, 2019
081bb0f
small tweak in autorelay test
vyzo Apr 14, 2019
4a6e767
log and ignore relay peers with empty address sets
vyzo Apr 14, 2019
376379b
retry to find relays if we fail to connect to any
vyzo Apr 14, 2019
c8b8014
gate retry wait with the context
vyzo Apr 14, 2019
e8e2ab1
limit relay selection to 20
vyzo Apr 14, 2019
8d5c11c
fix private->public->private transition issues with address set
vyzo Apr 14, 2019
0c69da9
move the address invalidation check outside the lock
vyzo Apr 14, 2019
0830634
limit number of FindPeer queries in relay selection
vyzo Apr 14, 2019
27f465e
some better logging
vyzo Apr 14, 2019
4a4b148
add comment about eliding the lock on addrs read
vyzo Apr 18, 2019
433a0c0
extract cleanupAddrSet and implement better heuristic
vyzo Apr 18, 2019
a331f99
addrsplosion test
vyzo Apr 18, 2019
e82eabe
cover the case where we can't select a default port in addrsplosion c…
vyzo Apr 18, 2019
9d7f6b8
rewrite isDNSAddr to use ma.SplitFirst
vyzo Apr 19, 2019
528c473
filter relay addrs in address set cleanup
vyzo Apr 19, 2019
21c4e1d
test for privately bound port first when cleaning up addrsplosion
vyzo Apr 19, 2019
ff4b98a
some more addrsplosion tests
vyzo Apr 19, 2019
a8d14f9
use addresses from the peerstore if available
vyzo Apr 19, 2019
9b2731e
used named constants for dns address protocols
vyzo Apr 19, 2019
bd22c49
remove redundant private addr check when constructing our relay addre…
vyzo Apr 19, 2019
f4f924e
don't track relay addrs, use the peerstore
vyzo Apr 19, 2019
c097172
compute relay address set dynamically
vyzo Apr 19, 2019
4727d5b
don't preallocate result array, we don't know how long it will be
vyzo Apr 19, 2019
5c9299a
pacify the race detector
vyzo Apr 19, 2019
1a81119
reduce scope of the lock, pre-allocate result slice in relayAddrs
vyzo Apr 20, 2019
f9e182f
gate max number of retries in findRelays
vyzo Apr 20, 2019
4629431
some tweaks
vyzo Apr 20, 2019
35e805d
add ignore list to account for connection failures
vyzo Apr 20, 2019
8d073ce
kill the parallel query logic in selectRelays; let it be random
vyzo Apr 20, 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
167 changes: 167 additions & 0 deletions p2p/host/relay/addrsplosion.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,167 @@
package relay

import (
"encoding/binary"

circuit "github.com/libp2p/go-libp2p-circuit"
ma "github.com/multiformats/go-multiaddr"
dns "github.com/multiformats/go-multiaddr-dns"
manet "github.com/multiformats/go-multiaddr-net"
)

// This function cleans up a relay's address set to remove private addresses and curtail
// addrsplosion.
func cleanupAddressSet(addrs []ma.Multiaddr) []ma.Multiaddr {
var public, private []ma.Multiaddr

for _, a := range addrs {
if isRelayAddr(a) {
continue
}

if manet.IsPublicAddr(a) || isDNSAddr(a) {
public = append(public, a)
continue
}
vyzo marked this conversation as resolved.
Show resolved Hide resolved

// discard unroutable addrs
if manet.IsPrivateAddr(a) {
private = append(private, a)
}
}

if !hasAddrsplosion(public) {
return public
}

return sanitizeAddrsplodedSet(public, private)
}

func isRelayAddr(a ma.Multiaddr) bool {
isRelay := false

ma.ForEach(a, func(c ma.Component) bool {
vyzo marked this conversation as resolved.
Show resolved Hide resolved
switch c.Protocol().Code {
case circuit.P_CIRCUIT:
isRelay = true
return false
default:
return true
}
})

return isRelay
}

func isDNSAddr(a ma.Multiaddr) bool {
if first, _ := ma.SplitFirst(a); first != nil {
switch first.Protocol().Code {
case dns.P_DNS4, dns.P_DNS6, dns.P_DNSADDR:
return true
}
}
return false
}

// we have addrsplosion if for some protocol we advertise multiple ports on
// the same base address.
func hasAddrsplosion(addrs []ma.Multiaddr) bool {
aset := make(map[string]int)

for _, a := range addrs {
key, port := addrKeyAndPort(a)
xport, ok := aset[key]
if ok && port != xport {
return true
}
aset[key] = port
}

return false
}

func addrKeyAndPort(a ma.Multiaddr) (string, int) {
var (
key string
port int
)

ma.ForEach(a, func(c ma.Component) bool {
switch c.Protocol().Code {
case ma.P_TCP, ma.P_UDP:
port = int(binary.BigEndian.Uint16(c.RawValue()))
Copy link
Member

Choose a reason for hiding this comment

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

Ideally, this would only bother with the first port (although multiple ports isn't currently an issue except for relay addresses which we should be ignoring anyways).

key += "/" + c.Protocol().Name
default:
val := c.Value()
if val == "" {
val = c.Protocol().Name
}
key += "/" + val
}
return true
})

return key, port
}

// clean up addrsplosion
// the following heuristic is used:
// - for each base address/protocol combination, if there are multiple ports advertised then
// only accept the default port if present.
// - If the default port is not present, we check for non-standard ports by tracking
// private port bindings if present.
// - If there is no default or private port binding, then we can't infer the correct
// port and give up and return all addrs (for that base address)
func sanitizeAddrsplodedSet(public, private []ma.Multiaddr) []ma.Multiaddr {
type portAndAddr struct {
addr ma.Multiaddr
port int
}

privports := make(map[int]struct{})
pubaddrs := make(map[string][]portAndAddr)

for _, a := range private {
_, port := addrKeyAndPort(a)
privports[port] = struct{}{}
}

for _, a := range public {
key, port := addrKeyAndPort(a)
pubaddrs[key] = append(pubaddrs[key], portAndAddr{addr: a, port: port})
}

var result []ma.Multiaddr
for _, pas := range pubaddrs {
if len(pas) == 1 {
// it's not addrsploded
result = append(result, pas[0].addr)
continue
}

haveAddr := false
for _, pa := range pas {
if _, ok := privports[pa.port]; ok {
// it matches a privately bound port, use it
result = append(result, pa.addr)
haveAddr = true
continue
}

if pa.port == 4001 || pa.port == 4002 {
// it's a default port, use it
result = append(result, pa.addr)
haveAddr = true
}
vyzo marked this conversation as resolved.
Show resolved Hide resolved
}

if !haveAddr {
// we weren't able to select a port; bite the bullet and use them all
for _, pa := range pas {
result = append(result, pa.addr)
}
}
}

return result
}
122 changes: 122 additions & 0 deletions p2p/host/relay/addrsplosion_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,122 @@
package relay

import (
"testing"

ma "github.com/multiformats/go-multiaddr"
_ "github.com/multiformats/go-multiaddr-dns"
)

func TestCleanupAddrs(t *testing.T) {
// test with no addrsplosion
addrs := makeAddrList(
"/ip4/127.0.0.1/tcp/4001",
"/ip4/127.0.0.1/udp/4002/quic",
"/ip4/1.2.3.4/tcp/4001",
"/ip4/1.2.3.4/udp/4002/quic",
"/dnsaddr/somedomain.com/tcp/4002/ws",
)
clean := makeAddrList(
"/ip4/1.2.3.4/tcp/4001",
"/ip4/1.2.3.4/udp/4002/quic",
"/dnsaddr/somedomain.com/tcp/4002/ws",
)

r := cleanupAddressSet(addrs)
if !sameAddrs(clean, r) {
t.Fatal("cleaned up set doesn't match expected")
}

// test with default port addrspolosion
addrs = makeAddrList(
"/ip4/127.0.0.1/tcp/4001",
"/ip4/1.2.3.4/tcp/4001",
"/ip4/1.2.3.4/tcp/33333",
"/ip4/1.2.3.4/tcp/33334",
"/ip4/1.2.3.4/tcp/33335",
"/ip4/1.2.3.4/udp/4002/quic",
)
clean = makeAddrList(
"/ip4/1.2.3.4/tcp/4001",
"/ip4/1.2.3.4/udp/4002/quic",
)
r = cleanupAddressSet(addrs)
if !sameAddrs(clean, r) {
t.Fatal("cleaned up set doesn't match expected")
}

// test with default port addrsplosion but no private addrs
addrs = makeAddrList(
"/ip4/1.2.3.4/tcp/4001",
"/ip4/1.2.3.4/tcp/33333",
"/ip4/1.2.3.4/tcp/33334",
"/ip4/1.2.3.4/tcp/33335",
"/ip4/1.2.3.4/udp/4002/quic",
)
clean = makeAddrList(
"/ip4/1.2.3.4/tcp/4001",
"/ip4/1.2.3.4/udp/4002/quic",
)
r = cleanupAddressSet(addrs)
if !sameAddrs(clean, r) {
t.Fatal("cleaned up set doesn't match expected")
}

// test with non-standard port addrsplosion
addrs = makeAddrList(
"/ip4/127.0.0.1/tcp/12345",
"/ip4/1.2.3.4/tcp/12345",
"/ip4/1.2.3.4/tcp/33333",
"/ip4/1.2.3.4/tcp/33334",
"/ip4/1.2.3.4/tcp/33335",
)
clean = makeAddrList(
"/ip4/1.2.3.4/tcp/12345",
)
r = cleanupAddressSet(addrs)
if !sameAddrs(clean, r) {
t.Fatal("cleaned up set doesn't match expected")
}

// test with a squeaky clean address set
addrs = makeAddrList(
"/ip4/1.2.3.4/tcp/4001",
"/ip4/1.2.3.4/udp/4001/quic",
)
clean = addrs
r = cleanupAddressSet(addrs)
if !sameAddrs(clean, r) {
t.Fatal("cleaned up set doesn't match expected")
}
}

func makeAddrList(strs ...string) []ma.Multiaddr {
result := make([]ma.Multiaddr, 0, len(strs))
for _, s := range strs {
a := ma.StringCast(s)
result = append(result, a)
}
return result
}

func sameAddrs(as, bs []ma.Multiaddr) bool {
if len(as) != len(bs) {
return false
}

for _, a := range as {
if !findAddr(a, bs) {
return false
}
}
return true
}

func findAddr(a ma.Multiaddr, bs []ma.Multiaddr) bool {
for _, b := range bs {
if a.Equal(b) {
return true
}
}
return false
}
Loading