[p2p] prevent dialing of private ips (#4286)

* [p2p] fix: prevent dialing of private ips

The original feature (erroneously) prevents only querying of the private
IPs. This change prevents dialing private IPs altogether when the flag
is activated.

* [p2p] do not return `nil` gater
* [p2p] remove query filter
It was overriden by connection gater

* [p2p] add test to check gater non blocking

p2p/discovery/option.go

@@ -11,10 +11,9 @@ import (
 // DHTConfig is the configurable DHT options.
 // For normal nodes, only BootNodes field need to be specified.
 type DHTConfig struct {
-	BootNodes            []string
-	DataStoreFile        *string // File path to store DHT data. Shall be only used for bootstrap nodes.
-	DiscConcurrency      int
-	DisablePrivateIPScan bool
+	BootNodes       []string
+	DataStoreFile   *string // File path to store DHT data. Shall be only used for bootstrap nodes.
+	DiscConcurrency int
 }
 
 // getLibp2pRawOptions get the raw libp2p options as a slice.
@@ -41,13 +40,6 @@ func (opt DHTConfig) getLibp2pRawOptions() ([]libp2p_dht.Option, error) {
 		opts = append(opts, libp2p_dht.Concurrency(opt.DiscConcurrency))
 	}
 
-	if opt.DisablePrivateIPScan {
-		// QueryFilter sets a function that approves which peers may be dialed in a query
-		// PublicQueryFilter returns true if the peer is suspected of being publicly accessible
-		// includes RFC1918 + some other ranges + a stricter definition for IPv6
-		opts = append(opts, libp2p_dht.QueryFilter(libp2p_dht.PublicQueryFilter))
-	}
-
 	return opts, nil
 }
 

p2p/gater.go

@@ -0,0 +1,49 @@
+package p2p
+
+import (
+	"github.com/libp2p/go-libp2p-core/connmgr"
+	"github.com/libp2p/go-libp2p-core/control"
+	"github.com/libp2p/go-libp2p-core/network"
+	"github.com/libp2p/go-libp2p-core/peer"
+	libp2p_dht "github.com/libp2p/go-libp2p-kad-dht"
+	ma "github.com/multiformats/go-multiaddr"
+)
+
+type Gater struct {
+	isGating bool
+}
+
+func NewGater(disablePrivateIPScan bool) connmgr.ConnectionGater {
+	return Gater{
+		isGating: disablePrivateIPScan,
+	}
+}
+
+func (gater Gater) InterceptPeerDial(p peer.ID) (allow bool) {
+	return true
+}
+
+// Blocking connections at this stage is typical for address filtering.
+func (gater Gater) InterceptAddrDial(p peer.ID, m ma.Multiaddr) (allow bool) {
+	if gater.isGating {
+		return libp2p_dht.PublicQueryFilter(nil, peer.AddrInfo{
+			ID:    p,
+			Addrs: []ma.Multiaddr{m},
+		})
+	} else {
+		return true
+	}
+}
+
+func (gater Gater) InterceptAccept(network.ConnMultiaddrs) (allow bool) {
+	return true
+}
+
+func (gater Gater) InterceptSecured(network.Direction, peer.ID, network.ConnMultiaddrs) (allow bool) {
+	return true
+}
+
+// NOTE: the go-libp2p implementation currently IGNORES the disconnect reason.
+func (gater Gater) InterceptUpgraded(network.Conn) (allow bool, reason control.DisconnectReason) {
+	return true, 0
+}

p2p/gater_test.go

@@ -0,0 +1,39 @@
+package p2p
+
+import (
+	"testing"
+
+	ma "github.com/multiformats/go-multiaddr"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestGaterBlocking(t *testing.T) {
+	gater := NewGater(true)
+	require.NotNil(t, &gater, "%s", &gater)
+
+	public, err := ma.NewMultiaddr("/ip4/1.1.1.1/udp/53")
+	assert.Nil(t, err, "%s", err)
+	allowed := gater.InterceptAddrDial("somePeer", public)
+	assert.True(t, allowed, "%b", allowed)
+
+	private, err := ma.NewMultiaddr("/ip4/192.168.1.1/tcp/80")
+	assert.Nil(t, err, "%s", err)
+	allowed = gater.InterceptAddrDial("somePeer", private)
+	assert.False(t, allowed, "%b", allowed)
+}
+
+func TestGaterNotBlocking(t *testing.T) {
+	gater := NewGater(false)
+	require.NotNil(t, &gater, "%s", &gater)
+
+	public, err := ma.NewMultiaddr("/ip4/1.1.1.1/udp/53")
+	assert.Nil(t, err, "%s", err)
+	allowed := gater.InterceptAddrDial("somePeer", public)
+	assert.True(t, allowed, "%b", allowed)
+
+	private, err := ma.NewMultiaddr("/ip4/192.168.1.1/tcp/80")
+	assert.Nil(t, err, "%s", err)
+	allowed = gater.InterceptAddrDial("somePeer", private)
+	assert.True(t, allowed, "%b", allowed)
+}

p2p/host.go

@@ -123,6 +123,8 @@ func NewHost(cfg HostConfig) (Host, error) {
 		libp2p.EnableNATService(),
 		libp2p.ForceReachabilityPublic(),
 		libp2p.BandwidthReporter(newCounter()),
+		// prevent dialing of public addresses
+		libp2p.ConnectionGater(NewGater(cfg.DisablePrivateIPScan)),
 	)
 	if err != nil {
 		cancel()
@@ -130,10 +132,9 @@ func NewHost(cfg HostConfig) (Host, error) {
 	}
 
 	disc, err := discovery.NewDHTDiscovery(p2pHost, discovery.DHTConfig{
-		BootNodes:            cfg.BootNodes,
-		DataStoreFile:        cfg.DataStoreFile,
-		DiscConcurrency:      cfg.DiscConcurrency,
-		DisablePrivateIPScan: cfg.DisablePrivateIPScan,
+		BootNodes:       cfg.BootNodes,
+		DataStoreFile:   cfg.DataStoreFile,
+		DiscConcurrency: cfg.DiscConcurrency,
 	})
 	if err != nil {
 		cancel()