fix(sync): load black listed addresses from the config (#1301)

config/config_test.go

@@ -7,6 +7,7 @@ import (
 	"github.com/pactus-project/pactus/util"
 	"github.com/pactus-project/pactus/util/testsuite"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func TestSaveMainnetConfig(t *testing.T) {
@@ -15,7 +16,7 @@ func TestSaveMainnetConfig(t *testing.T) {
 
 	defConf := DefaultConfigMainnet()
 	conf, err := LoadFromFile(path, true, defConf)
-	assert.NoError(t, err)
+	require.NoError(t, err)
 
 	assert.NoError(t, conf.BasicCheck())
 	assert.Equal(t, DefaultConfigMainnet(), conf)

config/example_config.toml

@@ -99,8 +99,10 @@
     # `enable` indicates whether the firewall should be enabled or not.
     # Default is `false`.
     enable = false
-    # `blacklist_addresses` list blocked faulty p2p addresses
-    blacklist_addresses = [""]
+
+    # `blacklist_addresses` contains the list of addresses that should be blacklisted.
+    # Any connection to blacklisted addresses will be terminated.
+    blacklist_addresses = []
 
 # `tx_pool` contains configuration options for the transaction pool module.
 [tx_pool]

sync/firewall/config.go

@@ -4,6 +4,7 @@ import (
 	_ "embed"
 	"encoding/json"
 	"fmt"
+
 	"github.com/pactus-project/pactus/util/addr"
 )
 
@@ -12,8 +13,8 @@ var _defaultBlackListAddresses []byte
 
 type Config struct {
 	Enabled            bool     `toml:"enable"`
-	BlackListAddresses []string `toml:"blacklist_ips"`
-	blackListAddrSet   map[string]struct{}
+	BlackListAddresses []string `toml:"blacklist_addresses"`
+	blackListAddrSet   map[string]any
 }
 
 type defaultBlackListIPs struct {
@@ -24,31 +25,37 @@ func DefaultConfig() *Config {
 	return &Config{
 		Enabled:            false,
 		BlackListAddresses: make([]string, 0),
-		blackListAddrSet:   make(map[string]struct{}),
+		blackListAddrSet:   make(map[string]any),
 	}
 }
 
 // BasicCheck performs basic checks on the configuration.
 func (conf *Config) BasicCheck() error {
 	for _, address := range conf.BlackListAddresses {
-		parsed, err := addr.Parse(address)
+		// TODO: use libp2p library (multi-address)
+		// TODO: address should only contain protocol + address like: "/ip4/1.1.1.1"
+		_, err := addr.Parse(address)
 		if err != nil {
 			return fmt.Errorf("invalid blacklist address format: %s", address)
 		}
-		conf.blackListAddrSet[parsed.Address()] = struct{}{}
 	}
 
 	return nil
 }
 
-// LoadDefaultBlackListAddresses load default blacklist addresses from black_list.json
-func (conf *Config) LoadDefaultBlackListAddresses() error {
+// LoadDefaultBlackListAddresses loads default blacklist addresses from the `black_list.json` file.
+func (conf *Config) LoadDefaultBlackListAddresses() {
 	var def defaultBlackListIPs
 
-	if err := json.Unmarshal(_defaultBlackListAddresses, &def); err != nil {
-		return err
+	_ = json.Unmarshal(_defaultBlackListAddresses, &def)
+
+	for _, a := range def.Addresses {
+		ma, _ := addr.Parse(a)
+		conf.blackListAddrSet[ma.Address()] = true
 	}
 
-	conf.BlackListAddresses = append(conf.BlackListAddresses, def.Addresses...)
-	return nil
+	for _, a := range conf.BlackListAddresses {
+		ma, _ := addr.Parse(a)
+		conf.blackListAddrSet[ma.Address()] = true
+	}
 }

sync/firewall/firewall.go

@@ -2,7 +2,6 @@ package firewall
 
 import (
 	"bytes"
-	"github.com/pactus-project/pactus/util/addr"
 	"io"
 
 	"github.com/libp2p/go-libp2p/core/peer"
@@ -11,6 +10,7 @@ import (
 	"github.com/pactus-project/pactus/state"
 	"github.com/pactus-project/pactus/sync/bundle"
 	"github.com/pactus-project/pactus/sync/peerset"
+	"github.com/pactus-project/pactus/util/addr"
 	"github.com/pactus-project/pactus/util/errors"
 	"github.com/pactus-project/pactus/util/logger"
 )
@@ -27,6 +27,8 @@ type Firewall struct {
 func NewFirewall(conf *Config, net network.Network, peerSet *peerset.PeerSet, st state.Facade,
 	log *logger.SubLogger,
 ) *Firewall {
+	conf.LoadDefaultBlackListAddresses()
+
 	return &Firewall{
 		config:  conf,
 		network: net,
@@ -55,10 +57,12 @@ func (f *Firewall) IsBlackListAddress(remoteAddr string) bool {
 	p2pRemoteAddr, err := addr.Parse(remoteAddr)
 	if err != nil {
 		f.logger.Debug("firewall: unable to parse remote address", "err", err)
+
 		return false
 	}
 
 	_, exists := f.config.blackListAddrSet[p2pRemoteAddr.Address()]
+
 	return exists
 }
 

sync/firewall/firewall_test.go

@@ -15,6 +15,7 @@ import (
 	"github.com/pactus-project/pactus/util/logger"
 	"github.com/pactus-project/pactus/util/testsuite"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 type testData struct {
@@ -39,8 +40,9 @@ func setup(t *testing.T) *testData {
 	net := network.MockingNetwork(ts, ts.RandPeerID())
 	conf := DefaultConfig()
 	conf.Enabled = true
+	// TODO: It should be like: "/ip6/2a01:4f9:4a:1d85::2"
 	conf.BlackListAddresses = []string{"/ip6/2a01:4f9:4a:1d85::2/tcp/21888"}
-	assert.NoError(t, conf.BasicCheck())
+	require.NoError(t, conf.BasicCheck())
 	firewall := NewFirewall(conf, net, peerSet, st, subLogger)
 	assert.NotNil(t, firewall)
 	badPeerID := ts.RandPeerID()
@@ -65,11 +67,6 @@ func setup(t *testing.T) *testData {
 	}
 }
 
-func TestLoadDefaultBlackListIPs(t *testing.T) {
-	td := setup(t)
-	assert.Nil(t, td.firewall.config.LoadDefaultBlackListAddresses())
-}
-
 func TestInvalidBundlesCounter(t *testing.T) {
 	td := setup(t)
 
@@ -85,16 +82,6 @@ func TestInvalidBundlesCounter(t *testing.T) {
 	assert.Equal(t, p.InvalidBundles, 3)
 }
 
-func TestBlackListAddress(t *testing.T) {
-	td := setup(t)
-
-	blackListIP := "/ip6/2a01:4f9:4a:1d85::2/tcp/21888"
-	whiteListIP := "/ip4/10.10.10.10/tcp/21888"
-
-	assert.True(t, td.firewall.IsBlackListAddress(blackListIP))
-	assert.False(t, td.firewall.IsBlackListAddress(whiteListIP))
-}
-
 func TestGossipMessage(t *testing.T) {
 	t.Run("Message  from: unknown => should NOT close the connection", func(t *testing.T) {
 		td := setup(t)
@@ -216,3 +203,46 @@ func TestNetworkFlags(t *testing.T) {
 	bdl.Flags = 1
 	assert.Error(t, td.firewall.checkBundle(bdl))
 }
+
+func TestBlackListAddress(t *testing.T) {
+	td := setup(t)
+
+	testCases := []struct {
+		addr        string
+		blacklisted bool
+	}{
+		{
+			addr:        "/ip4/115.193.157.138/tcp/21888",
+			blacklisted: true,
+		},
+		// TOD: uncomment me
+		// {
+		// 	addr:        "/ip4/115.193.157.138",
+		// 	blacklisted: true,
+		// },
+		{
+			addr:        "/ip4/10.10.10.10",
+			blacklisted: false,
+		},
+		{
+			addr:        "/ip4/10.10.10.10/udp/21888",
+			blacklisted: false,
+		},
+		{
+			addr:        "/ip6/2a01:4f9:4a:1d85::2",
+			blacklisted: false,
+		},
+	}
+
+	for i, tc := range testCases {
+		blacklisted := td.firewall.IsBlackListAddress(tc.addr)
+
+		if tc.blacklisted {
+			assert.True(t, blacklisted,
+				"test %v failed, addr %v should blacklisted", i, tc.addr)
+		} else {
+			assert.False(t, blacklisted,
+				"test %v failed, addr %v should not blacklisted", i, tc.addr)
+		}
+	}
+}

sync/sync.go

@@ -70,10 +70,6 @@ func NewSynchronizer(
 	sync.peerSet = peerset.NewPeerSet(conf.SessionTimeout)
 	sync.logger = logger.NewSubLogger("_sync", sync)
 
-	if err := conf.Firewall.LoadDefaultBlackListAddresses(); err != nil {
-		return nil, err
-	}
-
 	sync.firewall = firewall.NewFirewall(conf.Firewall, net, sync.peerSet, st, sync.logger)
 
 	cacheSize := conf.CacheSize()
@@ -321,14 +317,16 @@ func (sync *synchronizer) processStreamMessage(msg *network.StreamMessage) {
 func (sync *synchronizer) processConnectEvent(ce *network.ConnectEvent) {
 	sync.logger.Debug("processing connect event", "pid", ce.PeerID)
 
+	sync.peerSet.UpdateAddress(ce.PeerID, ce.RemoteAddress, ce.Direction)
+
 	if sync.firewall.IsBlackListAddress(ce.RemoteAddress) {
-		sync.peerSet.UpdateStatus(ce.PeerID, peerset.StatusCodeBanned)
 		sync.logger.Debug("Peer is blacklisted", "peer_id", ce.PeerID, "remote_address", ce.RemoteAddress)
+		sync.peerSet.UpdateStatus(ce.PeerID, peerset.StatusCodeBanned)
+
 		return
 	}
 
 	sync.peerSet.UpdateStatus(ce.PeerID, peerset.StatusCodeConnected)
-	sync.peerSet.UpdateAddress(ce.PeerID, ce.RemoteAddress, ce.Direction)
 }
 
 func (sync *synchronizer) processProtocolsEvent(pe *network.ProtocolsEvents) {

sync/sync_test.go

@@ -231,12 +231,15 @@ func TestStop(t *testing.T) {
 }
 
 func TestConnectEvent(t *testing.T) {
-	td := setup(t, nil)
+	conf := testConfig()
+	conf.Firewall.BlackListAddresses = []string{"/ip4/1.1.1.1/tcp/21888"}
+
+	td := setup(t, conf)
 
 	pid := td.RandPeerID()
 	ce := &network.ConnectEvent{
 		PeerID:        pid,
-		RemoteAddress: "/ip4/address_1/tcp/21888",
+		RemoteAddress: "/ip4/2.2.2.2/tcp/21888",
 	}
 	td.network.EventCh <- ce
 
@@ -245,36 +248,33 @@ func TestConnectEvent(t *testing.T) {
 		if p == nil {
 			return false
 		}
-		assert.Equal(t, p.Address, "/ip4/address_1/tcp/21888")
+		assert.Equal(t, p.Address, "/ip4/2.2.2.2/tcp/21888")
 
 		return p.Status == peerset.StatusCodeConnected
 	}, time.Second, 100*time.Millisecond)
 
-	// Receiving connect event for the second time
-	td.sync.peerSet.UpdateStatus(pid, peerset.StatusCodeKnown)
-	td.network.EventCh <- ce
-	p := td.sync.peerSet.GetPeer(pid)
-	assert.Equal(t, peerset.StatusCodeKnown, p.Status)
-
-	// Adding the address to the blacklist
-	td.config.Firewall.BlackListAddresses = []string{"/ip4/address_1/tcp/21888"}
-	assert.NoError(t, td.config.Firewall.BasicCheck())
+	p1 := td.sync.peerSet.GetPeer(pid)
+	assert.Equal(t, peerset.StatusCodeConnected, p1.Status)
 
-	// Sending connect event for the blacklisted address
+	// Receiving connect event for the blacklisted address
+	pid = td.RandPeerID()
+	ce = &network.ConnectEvent{
+		PeerID:        pid,
+		RemoteAddress: "/ip4/1.1.1.1/tcp/21888",
+	}
 	td.network.EventCh <- ce
 
 	assert.Eventually(t, func() bool {
 		p := td.sync.peerSet.GetPeer(pid)
 		if p == nil {
 			return false
 		}
+
 		return td.sync.firewall.IsBlackListAddress(p.Address)
 	}, time.Second, 100*time.Millisecond)
 
-	p = td.sync.peerSet.GetPeer(pid)
-	assert.Equal(t, peerset.StatusCodeBanned, p.Status)
-
-	td.config.Firewall.BlackListAddresses = make([]string, 0)
+	p2 := td.sync.peerSet.GetPeer(pid)
+	assert.Equal(t, peerset.StatusCodeBanned, p2.Status)
 }
 
 func TestDisconnectEvent(t *testing.T) {

util/addr/addr.go

@@ -44,22 +44,22 @@ func Parse(addr string) (P2PAddr, error) {
 	return address, nil
 }
 
-func (p P2PAddr) Transport() string {
+func (p *P2PAddr) Transport() string {
 	return p.transport
 }
 
-func (p P2PAddr) Protocol() string {
+func (p *P2PAddr) Protocol() string {
 	return p.protocol
 }
 
-func (p P2PAddr) Address() string {
+func (p *P2PAddr) Address() string {
 	return p.addr
 }
 
-func (p P2PAddr) Port() string {
+func (p *P2PAddr) Port() string {
 	return p.port
 }
 
-func (p P2PAddr) PeerID() string {
+func (p *P2PAddr) PeerID() string {
 	return p.peerID
 }