From 4e3f61b02345dc8854e0f541cb894a8aec6d38b1 Mon Sep 17 00:00:00 2001 From: Ivan Shvedunov Date: Tue, 30 Jan 2024 06:35:28 +0000 Subject: [PATCH] p2p: fix gater blocklist setup (#5511) ## Motivation The IP blockslists were not initialized properly. This results in "portscans", when the node tries to contact other nodes outside its local network on their private IP. See #5510 for more information --- CHANGELOG.md | 2 + fetch/fetch_test.go | 1 + node/adminservice_api_test.go | 1 + node/bad_peer_test.go | 2 + p2p/host.go | 18 ++------- p2p/host_test.go | 71 +++++++++++++++++++++++++++++++++++ p2p/ping_test.go | 2 + 7 files changed, 83 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c608ba2dd83..831fad24a1e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -98,6 +98,8 @@ configuration is as follows: Fix problem in POST proving where too many files were opened at the same time. * [#5494](https://github.com/spacemeshos/go-spacemesh/pull/5494) Make routing discovery more configurable and less spammy by default. +* [#5511](https://github.com/spacemeshos/go-spacemesh/pull/5511) + Fix dialing peers on their private IPs, which was causing "portscan" complaints. ## Release v1.3.5 diff --git a/fetch/fetch_test.go b/fetch/fetch_test.go index 7d644bf8297..4837efef387 100644 --- a/fetch/fetch_test.go +++ b/fetch/fetch_test.go @@ -343,6 +343,7 @@ func TestFetch_PeerDroppedWhenMessageResultsInValidationReject(t *testing.T) { p2pconf := p2p.DefaultConfig() p2pconf.Listen = p2p.MustParseAddresses("/ip4/127.0.0.1/tcp/0") p2pconf.DataDir = t.TempDir() + p2pconf.IP4Blocklist = nil // Good host h, err := p2p.New(ctx, lg, p2pconf, []byte{}, []byte{}) diff --git a/node/adminservice_api_test.go b/node/adminservice_api_test.go index 3e9c105d173..cc8696f214f 100644 --- a/node/adminservice_api_test.go +++ b/node/adminservice_api_test.go @@ -22,6 +22,7 @@ func TestPeerInfoApi(t *testing.T) { cfg.Genesis.Accounts = nil cfg.P2P.DisableNatPort = true cfg.P2P.Listen = p2p.MustParseAddresses("/ip4/127.0.0.1/tcp/0") + cfg.P2P.IP4Blocklist = nil cfg.API.PublicListener = "0.0.0.0:0" cfg.API.PrivateServices = nil diff --git a/node/bad_peer_test.go b/node/bad_peer_test.go index 2270ee720f2..ff1ab26bb42 100644 --- a/node/bad_peer_test.go +++ b/node/bad_peer_test.go @@ -34,6 +34,7 @@ func TestPeerDisconnectForMessageResultValidationReject(t *testing.T) { conf1.DataDirParent = t.TempDir() conf1.FileLock = filepath.Join(conf1.DataDirParent, "LOCK") conf1.P2P.Listen = p2p.MustParseAddresses("/ip4/127.0.0.1/tcp/0") + conf1.P2P.IP4Blocklist = nil // We setup the api to listen on an OS assigned port, which avoids the second instance getting stuck when conf1.API.PublicListener = "0.0.0.0:0" conf1.API.PrivateListener = "0.0.0.0:0" @@ -47,6 +48,7 @@ func TestPeerDisconnectForMessageResultValidationReject(t *testing.T) { conf2.DataDirParent = t.TempDir() conf2.FileLock = filepath.Join(conf2.DataDirParent, "LOCK") conf2.P2P.Listen = p2p.MustParseAddresses("/ip4/127.0.0.1/tcp/0") + conf2.P2P.IP4Blocklist = nil conf2.API.PublicListener = "0.0.0.0:0" conf2.API.PrivateListener = "0.0.0.0:0" app2 := NewApp(t, &conf2, l) diff --git a/p2p/host.go b/p2p/host.go index a0194f1058d..49f73f7d2d8 100644 --- a/p2p/host.go +++ b/p2p/host.go @@ -246,23 +246,13 @@ func New( bootnodesMap[pid.ID] = struct{}{} } - directMap := make(map[peer.ID]struct{}) - direct, err := parseIntoAddr(cfg.Direct) - if err != nil { - return nil, err - } - for _, pid := range direct { - directMap[pid.ID] = struct{}{} - } // leaves a small room for outbound connections in order to // reduce risk of network isolation - g := &gater{ - inbound: int(float64(cfg.HighPeers) * cfg.InboundFraction), - outbound: int(float64(cfg.HighPeers) * cfg.OutboundFraction), - direct: directMap, + g, err := newGater(cfg) + if err != nil { + return nil, fmt.Errorf("can't set up connection gater: %w", err) } - g.direct = directMap lopts := []libp2p.Option{ libp2p.Identity(key), libp2p.ListenAddrs(cfg.Listen...), @@ -402,7 +392,7 @@ func New( WithConfig(cfg), WithLog(logger), WithBootnodes(bootnodesMap), - WithDirectNodes(directMap), + WithDirectNodes(g.direct), ) return Upgrade(h, opts...) } diff --git a/p2p/host_test.go b/p2p/host_test.go index 4d00e1a9872..574ea4319a9 100644 --- a/p2p/host_test.go +++ b/p2p/host_test.go @@ -36,14 +36,17 @@ func TestPrologue(t *testing.T) { cfg1.DataDir = t.TempDir() cfg1.Listen = tc.listen cfg1.EnableQUICTransport = tc.enableQUIC + cfg1.IP4Blocklist = nil cfg2 := DefaultConfig() cfg2.DataDir = t.TempDir() cfg2.Listen = tc.listen cfg2.EnableQUICTransport = tc.enableQUIC + cfg2.IP4Blocklist = nil cfg3 := DefaultConfig() cfg3.DataDir = t.TempDir() cfg3.Listen = tc.listen cfg3.EnableQUICTransport = tc.enableQUIC + cfg3.IP4Blocklist = nil nc1 := []byte("red") h1, err := New(context.Background(), logtest.New(t), cfg1, nc1, nc1) @@ -74,3 +77,71 @@ func TestPrologue(t *testing.T) { }) } } + +func TestBlocklist(t *testing.T) { + type testcase struct { + desc string + address string + clearBlocklist bool + errStr string + } + + testcases := []testcase{ + { + desc: "local IPv4 disallowed", + address: "/ip4/127.0.0.1/tcp/0", + errStr: "gater disallows connection to peer", + }, + { + desc: "local IPv6 disallowed", + address: "/ip6/::/tcp/0", + errStr: "gater disallows connection to peer", + }, + { + desc: "local IPv4 allowed", + address: "/ip4/127.0.0.1/tcp/0", + clearBlocklist: true, + }, + { + desc: "local IPv4 allowed", + address: "/ip6/::/tcp/0", + clearBlocklist: true, + }, + } + + for _, tc := range testcases { + t.Run(tc.desc, func(t *testing.T) { + cfg1 := DefaultConfig() + cfg1.DataDir = t.TempDir() + cfg1.Listen = MustParseAddresses(tc.address) + if tc.clearBlocklist { + cfg1.IP4Blocklist = nil + cfg1.IP6Blocklist = nil + } + h1, err := New(context.Background(), logtest.New(t), cfg1, nil, nil) + require.NoError(t, err) + t.Cleanup(func() { h1.Stop() }) + + cfg2 := DefaultConfig() + cfg2.DataDir = t.TempDir() + cfg2.Listen = MustParseAddresses(tc.address) + if tc.clearBlocklist { + cfg2.IP4Blocklist = nil + cfg2.IP6Blocklist = nil + } + h2, err := New(context.Background(), logtest.New(t), cfg2, nil, nil) + require.NoError(t, err) + t.Cleanup(func() { h2.Stop() }) + + err = h1.Connect(context.Background(), peer.AddrInfo{ + ID: h2.ID(), + Addrs: h2.Addrs(), + }) + if tc.errStr == "" { + require.NoError(t, err) + } else { + require.ErrorContains(t, err, tc.errStr) + } + }) + } +} diff --git a/p2p/ping_test.go b/p2p/ping_test.go index ad53bf41fb2..407db7a489c 100644 --- a/p2p/ping_test.go +++ b/p2p/ping_test.go @@ -36,6 +36,7 @@ func TestPing(t *testing.T) { cfg1.DataDir = t.TempDir() cfg1.Listen = tc.listen cfg1.EnableQUICTransport = tc.enableQUIC + cfg1.IP4Blocklist = nil nc := []byte("foobar") h1, err := New(context.Background(), logtest.New(t), cfg1, nc, nc) require.NoError(t, err) @@ -47,6 +48,7 @@ func TestPing(t *testing.T) { cfg2.Listen = tc.listen cfg2.EnableQUICTransport = tc.enableQUIC cfg2.PingPeers = []string{h1.ID().String()} + cfg2.IP4Blocklist = nil h2, err := New(context.Background(), logtest.New(t), cfg2, nc, nc) require.NoError(t, err) h2.discovery.Start()