p2p: do not advertise private and non-routable addresses

network/hybridNetwork_test.go

@@ -64,7 +64,7 @@ func TestHybridNetwork_DuplicateConn(t *testing.T) {
 	// make it net address and restart the node
 	relayCfg.NetAddress = addr
 	relayCfg.PublicAddress = addr
-	relayCfg.P2PNetAddress = ":0"
+	relayCfg.P2PNetAddress = "127.0.0.1:0"
 	netA, err = NewHybridP2PNetwork(log.With("node", "netA"), relayCfg, p2pKeyDir, nil, genesisID, "net", &nopeNodeInfo{})
 	require.NoError(t, err)
 

network/p2p/p2p.go

@@ -20,6 +20,7 @@
 	"context"
 	"encoding/base32"
 	"fmt"
+	"net"
 	"runtime"
 	"strings"
 	"time"
@@ -43,6 +44,7 @@
 	"github.com/libp2p/go-libp2p/p2p/security/noise"
 	"github.com/libp2p/go-libp2p/p2p/transport/tcp"
 	"github.com/multiformats/go-multiaddr"
+	manet "github.com/multiformats/go-multiaddr/net"
 )
 
 // SubNextCancellable is an abstraction for pubsub.Subscription
@@ -108,18 +110,37 @@
 	ua := fmt.Sprintf("algod/%d.%d (%s; commit=%s; %d) %s(%s)", version.Major, version.Minor, version.Channel, version.CommitHash, version.BuildNumber, runtime.GOOS, runtime.GOARCH)
 
 	var listenAddr string
+	var needAddressFilter bool
 	if cfg.NetAddress != "" {
 		if parsedListenAddr, perr := netAddressToListenAddress(cfg.NetAddress); perr == nil {
 			listenAddr = parsedListenAddr
+
+			// check if the listen address is a specific address or a "all interfaces" address (0.0.0.0 or ::)
+			// in this case enable the address filter.
+			// this also means the address filter is not enabled for NetAddress set to
+			// a specific address including loopback and private addresses.
+			if manet.IsIPUnspecified(multiaddr.StringCast(listenAddr)) {
+				needAddressFilter = true
+			}
+		} else {
+			logging.Base().Warnf("failed to parse NetAddress %s: %v", cfg.NetAddress, perr)
 		}
 	} else {
-		// don't listen if NetAddress is not set.
+		logging.Base().Debug("p2p NetAddress is not set, not listening")
 		listenAddr = ""
 	}
 
 	var enableMetrics = func(cfg *libp2p.Config) error { cfg.DisableMetrics = false; return nil }
 	metrics.DefaultRegistry().Register(&metrics.PrometheusDefaultMetrics)
 
+	// if we are listening on a loopback address, most likely we are running tests or a goal network,
+	// so don't filter out any addresses
+	var addrFactory func(addrs []multiaddr.Multiaddr) []multiaddr.Multiaddr
+	if needAddressFilter {
+		logging.Base().Debug("private addresses filter is enabled")
+		addrFactory = addressFilter
+	}
+
 	rm, err := configureResourceManager(cfg)
 	if err != nil {
 		return nil, "", err
@@ -135,6 +156,7 @@
 		libp2p.Security(noise.ID, noise.New),
 		enableMetrics,
 		libp2p.ResourceManager(rm),
+		libp2p.AddrsFactory(addrFactory),
 	)
 	return host, listenAddr, err
 }
@@ -321,3 +343,55 @@
 		base32.StdEncoding.EncodeToString([]byte(telemetryInstance)),
 	)
 }
+
+var private6 = parseCIDR([]string{
+	"100::/64",
+	"2001:2::/48",
+	"2001:db8::/32", // multiaddr v0.13 has it
+})
+
+// parseCIDR converts string CIDRs to net.IPNet.
+// function panics on errors so that it is only called during initialization.
+func parseCIDR(cidrs []string) []*net.IPNet {
+	result := make([]*net.IPNet, 0, len(cidrs))
+	var ipnet *net.IPNet
+	var err error
+	for _, cidr := range cidrs {
+		if _, ipnet, err = net.ParseCIDR(cidr); err != nil {
+			panic(err)
+		}
+		result = append(result, ipnet)
+	}
+	return result
+}
+
+// addressFilter filters out private and unroutable addresses
+func addressFilter(addrs []multiaddr.Multiaddr) []multiaddr.Multiaddr {
+	res := make([]multiaddr.Multiaddr, 0, len(addrs))
+	for _, addr := range addrs {
+		if manet.IsPublicAddr(addr) && !manet.IsPrivateAddr(addr) {
+			if _, err := addr.ValueForProtocol(multiaddr.P_IP4); err == nil {
+				// no rules for IPv4 at the moment, accept
+				res = append(res, addr)
+				continue
+			}
+
+			isPrivate := false
+			a, err := addr.ValueForProtocol(multiaddr.P_IP6)
+			if err != nil {
+				logging.Base().Warnf("failed to get IPv6 addr from %s: %v", addr, err)
+				continue
+			}
+			addrIP := net.ParseIP(a)
+			for _, ipnet := range private6 {
+				if ipnet.Contains(addrIP) {
+					isPrivate = true
+				}
+			}
+			if !isPrivate {
+				res = append(res, addr)
+			}
+		}
+	}
+	return res
+}

network/p2p/p2p_test.go

@@ -19,14 +19,19 @@ package p2p
 import (
 	"context"
 	"fmt"
+	"net"
 	"testing"
 
 	"github.com/libp2p/go-libp2p"
 	"github.com/libp2p/go-libp2p/core/network"
 	"github.com/libp2p/go-libp2p/core/peer"
 	"github.com/libp2p/go-libp2p/core/protocol"
+	"github.com/multiformats/go-multiaddr"
+	manet "github.com/multiformats/go-multiaddr/net"
 	"github.com/stretchr/testify/require"
 
+	"github.com/algorand/go-algorand/config"
+	"github.com/algorand/go-algorand/network/p2p/peerstore"
 	"github.com/algorand/go-algorand/test/partitiontest"
 )
 
@@ -169,3 +174,158 @@ func TestP2PProtocolAsMeta(t *testing.T) {
 	require.Equal(t, h1TID, tid)
 	require.Equal(t, h1Inst, inst)
 }
+
+func TestP2PPrivateAddresses(t *testing.T) {
+	partitiontest.PartitionTest(t)
+	t.Parallel()
+
+	privAddrList := []string{
+		"/ip4/10.0.0.0/ipcidr/8",
+		"/ip4/100.64.0.0/ipcidr/10",
+		"/ip4/169.254.0.0/ipcidr/16",
+		"/ip4/172.16.0.0/ipcidr/12",
+		"/ip4/192.0.0.0/ipcidr/24",
+		"/ip4/192.0.2.0/ipcidr/24",
+		"/ip4/192.88.99.0/ipcidr/24",
+		"/ip4/192.168.0.0/ipcidr/16",
+		"/ip4/198.18.0.0/ipcidr/15",
+		"/ip4/198.51.100.0/ipcidr/24",
+		"/ip4/203.0.113.0/ipcidr/24",
+		"/ip4/224.0.0.0/ipcidr/4",
+		"/ip4/224.0.0.0/ipcidr/4",
+		"/ip4/233.252.0.0/ipcidr/4",
+		"/ip4/255.255.255.255/ipcidr/32",
+		"/ip6/fc00::/ipcidr/7",
+		"/ip6/fe80::/ipcidr/10",
+	}
+
+	// these are handled by addrFilter explicitly as a custom filter
+	extra := []string{
+		"/ip6/100::/ipcidr/64",
+		"/ip6/2001:2::/ipcidr/48",
+		"/ip6/2001:db8::/ipcidr/32", // multiaddr v0.13 has it
+	}
+
+	for _, addr := range privAddrList {
+		ma := multiaddr.StringCast(addr)
+		require.True(t, !manet.IsPublicAddr(ma) || manet.IsPrivateAddr(ma), "public/private check failed on %s", addr)
+		require.Empty(t, addressFilter([]multiaddr.Multiaddr{ma}), "addrFilter failed on %s", addr)
+	}
+
+	for _, addr := range extra {
+		ma := multiaddr.StringCast(addr)
+		require.Empty(t, addressFilter([]multiaddr.Multiaddr{ma}), "addrFilter failed on %s", addr)
+	}
+
+	// ensure addrFilter allows normal addresses
+	valid := []string{
+		"/ip4/3.4.5.6/tcp/1234",
+		"/ip6/200:11::/tcp/1234",
+	}
+
+	for _, addr := range valid {
+		ma := multiaddr.StringCast(addr)
+		require.Equal(t, []multiaddr.Multiaddr{ma}, addressFilter([]multiaddr.Multiaddr{ma}), "addrFilter failed on %s", addr)
+	}
+}
+
+func TestP2PMaNetIsIPUnspecified(t *testing.T) {
+	partitiontest.PartitionTest(t)
+	t.Parallel()
+
+	unspecified := []string{
+		":0",
+		":1234",
+		"0.0.0.0:2345",
+		"0.0.0.0:0",
+	}
+	for _, addr := range unspecified {
+		parsed, err := netAddressToListenAddress(addr)
+		require.NoError(t, err)
+		require.True(t, manet.IsIPUnspecified(multiaddr.StringCast(parsed)), "expected %s to be unspecified", addr)
+	}
+
+	specified := []string{
+		"127.0.0.1:0",
+		"127.0.0.1:1234",
+		"1.2.3.4:5678",
+		"1.2.3.4:0",
+		"192.168.0.111:0",
+		"10.0.0.1:101",
+	}
+	for _, addr := range specified {
+		parsed, err := netAddressToListenAddress(addr)
+		require.NoError(t, err)
+		require.False(t, manet.IsIPUnspecified(multiaddr.StringCast(parsed)), "expected %s to be specified", addr)
+	}
+
+	// also make sure IsIPUnspecified supports IPv6
+	unspecified6 := []string{
+		"/ip6/::/tcp/1234",
+	}
+	for _, addr := range unspecified6 {
+		require.True(t, manet.IsIPUnspecified(multiaddr.StringCast(addr)), "expected %s to be unspecified", addr)
+	}
+}
+
+// TestP2PMakeHostAddressFilter ensures that the host address filter is enabled only when the
+// NetAddress is set to "all interfaces" value (0.0.0.0:P or :P)
+func TestP2PMakeHostAddressFilter(t *testing.T) {
+	partitiontest.PartitionTest(t)
+	t.Parallel()
+
+	td := t.TempDir()
+	pstore, err := peerstore.NewPeerStore(nil, "test")
+	require.NoError(t, err)
+
+	// check "all interfaces" addr
+	for _, addr := range []string{":0", "0.0.0.0:0"} {
+		cfg := config.GetDefaultLocal()
+		cfg.NetAddress = addr
+		host, la, err := MakeHost(cfg, td, pstore)
+		require.NoError(t, err)
+		require.Equal(t, "/ip4/0.0.0.0/tcp/0", la)
+		require.Empty(t, host.Addrs())
+
+		mala, err := multiaddr.NewMultiaddr(la)
+		require.NoError(t, err)
+		host.Network().Listen(mala)
+		require.Empty(t, host.Addrs())
+		host.Close()
+	}
+
+	// check specific addresses IPv4 retrieved from the system
+	addresses := []string{}
+	ifaces, err := net.Interfaces()
+	require.NoError(t, err)
+	for _, i := range ifaces {
+		addrs, err := i.Addrs()
+		require.NoError(t, err)
+		for _, a := range addrs {
+			switch v := a.(type) {
+			case *net.IPAddr:
+				if v.IP.To4() != nil {
+					addresses = append(addresses, v.IP.String())
+				}
+			case *net.IPNet:
+				if v.IP.To4() != nil {
+					addresses = append(addresses, v.IP.String())
+				}
+			}
+		}
+	}
+	for _, addr := range addresses {
+		cfg := config.GetDefaultLocal()
+		cfg.NetAddress = addr + ":0"
+		host, la, err := MakeHost(cfg, td, pstore)
+		require.NoError(t, err)
+		require.Equal(t, "/ip4/"+addr+"/tcp/0", la)
+		require.Empty(t, host.Addrs())
+		mala, err := multiaddr.NewMultiaddr(la)
+		require.NoError(t, err)
+		err = host.Network().Listen(mala)
+		require.NoError(t, err)
+		require.NotEmpty(t, host.Addrs())
+		host.Close()
+	}
+}