From dd9e19207fb625f034f4ae0d8cd2801d8983c0c6 Mon Sep 17 00:00:00 2001 From: Ivan Shvedunov Date: Mon, 29 Jan 2024 19:51:35 +0400 Subject: [PATCH 1/3] p2p: fix gater blocklist setup The IP blockslists were not initialized properly. This change is related to #5510, but doesn't constitute a complete fix, as enabling the blocklists makes it impossible for the peers on the same LAN to talk to each other on their private IPs without involving NAT hairpinning, which may not be supported by the router or the ISP. --- p2p/host.go | 18 +++--------- p2p/host_test.go | 71 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 75 insertions(+), 14 deletions(-) diff --git a/p2p/host.go b/p2p/host.go index a0194f1058..49f73f7d2d 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 4d00e1a987..574ea4319a 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) + } + }) + } +} From 5b1db6824665171d1fec24d5b0f329bae2d92871 Mon Sep 17 00:00:00 2001 From: Ivan Shvedunov Date: Mon, 29 Jan 2024 19:58:27 +0400 Subject: [PATCH 2/3] Update CHANGELOG.md --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index c608ba2dd8..831fad24a1 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 From 49a2196c9f3eabb58947f4f5cf2464115eec1046 Mon Sep 17 00:00:00 2001 From: Ivan Shvedunov Date: Mon, 29 Jan 2024 21:12:50 +0400 Subject: [PATCH 3/3] Fix more tests that assume localhost connectivity --- fetch/fetch_test.go | 1 + node/adminservice_api_test.go | 1 + node/bad_peer_test.go | 2 ++ p2p/ping_test.go | 2 ++ 4 files changed, 6 insertions(+) diff --git a/fetch/fetch_test.go b/fetch/fetch_test.go index 7d644bf829..4837efef38 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 3e9c105d17..cc8696f214 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 2270ee720f..ff1ab26bb4 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/ping_test.go b/p2p/ping_test.go index ad53bf41fb..407db7a489 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()