skip dial attempts to non-public peers (#59)

skip dial attempts to non-public peers
fix https://github.com/libp2p/go-libp2p-autonat-svc/issues/33
fix #59

p2p/host/autonat/autonat.go

@@ -154,6 +154,44 @@ func ipInList(candidate ma.Multiaddr, list []ma.Multiaddr) bool {
 	return false
 }
 
+// skipDial indicates that a multiaddress isn't worth attempted dialing.
+// The same logic is used when the autonat client is considering if
+// a remote peer is worth using as a server, and when the server is
+// considering if a requested client is worth dialing back.
+func (c *config) skipDial(addr ma.Multiaddr) bool {
+	// skip relay addresses
+	_, err := addr.ValueForProtocol(ma.P_CIRCUIT)
+	if err == nil {
+		return true
+	}
+
+	if c.allowSelfDials {
+		return false
+	}
+
+	// skip private network (unroutable) addresses
+	if !manet.IsPublicAddr(addr) {
+		return true
+	}
+	candidateIP, err := manet.ToIP(addr)
+	if err != nil {
+		return true
+	}
+
+	// Skip dialing addresses we believe are the local node's
+	for _, localAddr := range c.host.Addrs() {
+		localIP, err := manet.ToIP(localAddr)
+		if err != nil {
+			continue
+		}
+		if localIP.Equal(candidateIP) {
+			return true
+		}
+	}
+
+	return false
+}
+
 func (as *AmbientAutoNAT) background() {
 	// wait a bit for the node to come online and establish some connections
 	// before starting autodetection
@@ -336,6 +374,23 @@ func (as *AmbientAutoNAT) probeNextPeer() {
 		if proto, err := as.host.Peerstore().SupportsProtocols(p, AutoNATProto); len(proto) == 0 || err != nil {
 			continue
 		}
+
+		goodAddr := false
+		for _, a := range info.Addrs {
+			if !as.config.skipDial(a) {
+				goodAddr = true
+				// if a public IP of the peer is one of ours: skip the peer.
+				aIP, _ := manet.ToIP(a)
+				aHost, _ := manet.FromIP(aIP)
+				if len(manet.AddrMatch(aHost, as.host.Addrs())) > 0 {
+					goodAddr = false
+					break
+				}
+			}
+		}
+		if !goodAddr {
+			continue
+		}
 		addrs = append(addrs, info)
 	}
 	// TODO: track and exclude recently probed peers.

p2p/host/autonat/autonat_test.go

@@ -55,6 +55,7 @@ func makeAutoNAT(ctx context.Context, t *testing.T, ash host.Host) (host.Host, A
 	h.Peerstore().AddAddrs(ash.ID(), ash.Addrs(), time.Minute)
 	h.Peerstore().AddProtocols(ash.ID(), AutoNATProto)
 	a, _ := New(ctx, h, WithSchedule(100*time.Millisecond, time.Second), WithoutStartupDelay())
+	a.(*AmbientAutoNAT).config.allowSelfDials = true
 	return h, a
 }
 

p2p/host/autonat/options.go

@@ -16,6 +16,8 @@ type config struct {
 	dialer            network.Network
 	forceReachability bool
 	reachability      network.Reachability
+	// Don't try to skip dials that might be within the lan
+	allowSelfDials bool
 
 	// client
 	bootDelay       time.Duration

p2p/host/autonat/svc.go

@@ -20,8 +20,6 @@ import (
 	manet "github.com/multiformats/go-multiaddr-net"
 )
 
-const P_CIRCUIT = 290
-
 // AutoNATService provides NAT autodetection services to other peers
 type autoNATService struct {
 	ctx          context.Context
@@ -112,7 +110,7 @@ func (as *autoNATService) handleDial(p peer.ID, obsaddr ma.Multiaddr, mpi *pb.Me
 
 	// add observed addr to the list of addresses to dial
 	var obsHost net.IP
-	if !as.skipDial(obsaddr) {
+	if !as.config.skipDial(obsaddr) {
 		addrs = append(addrs, obsaddr)
 		seen[obsaddr.String()] = struct{}{}
 		obsHost, _ = manet.ToIP(obsaddr)
@@ -125,7 +123,7 @@ func (as *autoNATService) handleDial(p peer.ID, obsaddr ma.Multiaddr, mpi *pb.Me
 			continue
 		}
 
-		if as.skipDial(addr) {
+		if as.config.skipDial(addr) {
 			continue
 		}
 
@@ -154,28 +152,6 @@ func (as *autoNATService) handleDial(p peer.ID, obsaddr ma.Multiaddr, mpi *pb.Me
 	return as.doDial(peer.AddrInfo{ID: p, Addrs: addrs})
 }
 
-func (as *autoNATService) skipDial(addr ma.Multiaddr) bool {
-	// skip relay addresses
-	_, err := addr.ValueForProtocol(P_CIRCUIT)
-	if err == nil {
-		return true
-	}
-
-	// skip private network (unroutable) addresses
-	if !manet.IsPublicAddr(addr) {
-		return true
-	}
-
-	// Skip dialing addresses we believe are the local node's
-	for _, localAddr := range as.config.host.Addrs() {
-		if localAddr.Equal(addr) {
-			return true
-		}
-	}
-
-	return false
-}
-
 func (as *autoNATService) doDial(pi peer.AddrInfo) *pb.Message_DialResponse {
 	// rate limit check
 	as.mx.Lock()

p2p/host/autonat/svc_test.go

@@ -2,7 +2,6 @@ package autonat
 
 import (
 	"context"
-	"net"
 	"testing"
 	"time"
 
@@ -13,7 +12,6 @@ import (
 	swarmt "github.com/libp2p/go-libp2p-swarm/testing"
 
 	ma "github.com/multiformats/go-multiaddr"
-	manet "github.com/multiformats/go-multiaddr-net"
 )
 
 func makeAutoNATConfig(ctx context.Context, t *testing.T) *config {
@@ -22,6 +20,7 @@ func makeAutoNATConfig(ctx context.Context, t *testing.T) *config {
 	c := config{host: h, dialer: dh.Network()}
 	_ = defaults(&c)
 	c.forceReachability = true
+	c.allowSelfDials = true
 	return &c
 }
 
@@ -48,6 +47,7 @@ func TestAutoNATServiceDialError(t *testing.T) {
 
 	c := makeAutoNATConfig(ctx, t)
 	c.dialTimeout = 1 * time.Second
+	c.allowSelfDials = false
 	_ = makeAutoNATService(ctx, t, c)
 	hc, ac := makeAutoNATClient(ctx, t)
 	connect(t, c.host, hc)
@@ -66,9 +66,6 @@ func TestAutoNATServiceDialSuccess(t *testing.T) {
 	ctx, cancel := context.WithCancel(context.Background())
 	defer cancel()
 
-	save := manet.Private4
-	manet.Private4 = []*net.IPNet{}
-
 	c := makeAutoNATConfig(ctx, t)
 	_ = makeAutoNATService(ctx, t, c)
 
@@ -79,17 +76,12 @@ func TestAutoNATServiceDialSuccess(t *testing.T) {
 	if err != nil {
 		t.Fatalf("Dial back failed: %s", err.Error())
 	}
-
-	manet.Private4 = save
 }
 
 func TestAutoNATServiceDialRateLimiter(t *testing.T) {
 	ctx, cancel := context.WithCancel(context.Background())
 	defer cancel()
 
-	save1 := manet.Private4
-	manet.Private4 = []*net.IPNet{}
-
 	c := makeAutoNATConfig(ctx, t)
 	c.dialTimeout = 1 * time.Second
 	c.throttleResetPeriod = time.Second
@@ -120,17 +112,12 @@ func TestAutoNATServiceDialRateLimiter(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-
-	manet.Private4 = save1
 }
 
 func TestAutoNATServiceGlobalLimiter(t *testing.T) {
 	ctx, cancel := context.WithCancel(context.Background())
 	defer cancel()
 
-	save1 := manet.Private4
-	manet.Private4 = []*net.IPNet{}
-
 	c := makeAutoNATConfig(ctx, t)
 	c.dialTimeout = time.Second
 	c.throttleResetPeriod = 10 * time.Second
@@ -161,8 +148,6 @@ func TestAutoNATServiceGlobalLimiter(t *testing.T) {
 	if !IsDialRefused(err) {
 		t.Fatal(err)
 	}
-
-	manet.Private4 = save1
 }
 
 func TestAutoNATServiceRateLimitJitter(t *testing.T) {
@@ -191,12 +176,10 @@ func TestAutoNATServiceStartup(t *testing.T) {
 	ctx, cancel := context.WithCancel(context.Background())
 	defer cancel()
 
-	save := manet.Private4
-	manet.Private4 = []*net.IPNet{}
-
 	h := bhost.NewBlankHost(swarmt.GenSwarm(t, ctx))
 	dh := bhost.NewBlankHost(swarmt.GenSwarm(t, ctx))
 	an, err := New(ctx, h, EnableService(dh.Network()))
+	an.(*AmbientAutoNAT).config.allowSelfDials = true
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -223,6 +206,4 @@ func TestAutoNATServiceStartup(t *testing.T) {
 	if an.Status() != network.ReachabilityPublic {
 		t.Fatalf("autonat should report public, but didn't")
 	}
-
-	manet.Private4 = save
 }