Skip to content

Commit

Permalink
p2p: fix gater blocklist setup (#5511)
Browse files Browse the repository at this point in the history
## 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
  • Loading branch information
ivan4th committed Jan 30, 2024
1 parent e1d634b commit 9dde045
Show file tree
Hide file tree
Showing 7 changed files with 83 additions and 14 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
1 change: 1 addition & 0 deletions fetch/fetch_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{})
Expand Down
1 change: 1 addition & 0 deletions node/adminservice_api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions node/bad_peer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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)
Expand Down
18 changes: 4 additions & 14 deletions p2p/host.go
Original file line number Diff line number Diff line change
Expand Up @@ -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...),
Expand Down Expand Up @@ -402,7 +392,7 @@ func New(
WithConfig(cfg),
WithLog(logger),
WithBootnodes(bootnodesMap),
WithDirectNodes(directMap),
WithDirectNodes(g.direct),
)
return Upgrade(h, opts...)
}
Expand Down
71 changes: 71 additions & 0 deletions p2p/host_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
}
})
}
}
2 changes: 2 additions & 0 deletions p2p/ping_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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()
Expand Down

0 comments on commit 9dde045

Please sign in to comment.