From 5796e879b9ca85509218f3d53665c45f94ed4176 Mon Sep 17 00:00:00 2001 From: Alexander Simmerl Date: Wed, 30 May 2018 10:37:39 +0200 Subject: [PATCH] Introduce option to skip duplicate ip check In some scenarios like tests we want to disable the guard which prevents peers connecting from the same ip. Fixes #1632 Closes #1634 --- config/config.go | 4 ++ p2p/peer_set.go | 4 -- p2p/peer_set_test.go | 14 ----- p2p/pex/pex_reactor_test.go | 107 ++++++++++++++++++------------------ p2p/switch.go | 5 +- p2p/test_util.go | 5 +- 6 files changed, 62 insertions(+), 77 deletions(-) diff --git a/config/config.go b/config/config.go index 47df46264..932e1ae8e 100644 --- a/config/config.go +++ b/config/config.go @@ -292,6 +292,9 @@ type P2PConfig struct { // Comma separated list of peer IDs to keep private (will not be gossiped to other peers) PrivatePeerIDs string `mapstructure:"private_peer_ids"` + + // Toggle to disable guard against peers connecting from the same ip. + SkipDuplicatePeerIPCheck bool `mapstructure:"skip_duplicate_peer_ip_check"` } // DefaultP2PConfig returns a default configuration for the peer-to-peer layer @@ -317,6 +320,7 @@ func TestP2PConfig() *P2PConfig { cfg.ListenAddress = "tcp://0.0.0.0:36656" cfg.SkipUPNP = true cfg.FlushThrottleTimeout = 10 + cfg.SkipDuplicatePeerIPCheck = true return cfg } diff --git a/p2p/peer_set.go b/p2p/peer_set.go index 66a7fdadb..e048cf4e3 100644 --- a/p2p/peer_set.go +++ b/p2p/peer_set.go @@ -47,10 +47,6 @@ func (ps *PeerSet) Add(peer Peer) error { return ErrSwitchDuplicatePeerID{peer.ID()} } - if ps.hasIP(peer.RemoteIP()) { - return ErrSwitchDuplicatePeerIP{peer.RemoteIP()} - } - index := len(ps.list) // Appending is safe even with other goroutines // iterating over the ps.list slice. diff --git a/p2p/peer_set_test.go b/p2p/peer_set_test.go index fc3004684..172767781 100644 --- a/p2p/peer_set_test.go +++ b/p2p/peer_set_test.go @@ -143,20 +143,6 @@ func TestPeerSetAddDuplicate(t *testing.T) { assert.Equal(t, wantNilErrCount, gotNilErrCount, "invalid nil errCount") } -func TestPeerSetAddDuplicateIP(t *testing.T) { - t.Parallel() - - peerSet := NewPeerSet() - - if err := peerSet.Add(randPeer(net.IP{172, 0, 0, 1})); err != nil { - t.Fatal(err) - } - - // Add peer with same IP. - err := peerSet.Add(randPeer(net.IP{172, 0, 0, 1})) - assert.Equal(t, ErrSwitchDuplicatePeerIP{IP: net.IP{172, 0, 0, 1}}, err) -} - func TestPeerSetGet(t *testing.T) { t.Parallel() diff --git a/p2p/pex/pex_reactor_test.go b/p2p/pex/pex_reactor_test.go index 55960e6f5..06fe6174e 100644 --- a/p2p/pex/pex_reactor_test.go +++ b/p2p/pex/pex_reactor_test.go @@ -27,6 +27,7 @@ var ( func init() { config = cfg.DefaultP2PConfig() config.PexReactor = true + config.SkipDuplicatePeerIPCheck = true } func TestPEXReactorBasic(t *testing.T) { @@ -69,59 +70,59 @@ func TestPEXReactorAddRemovePeer(t *testing.T) { // peers have different IP addresses, they all have the same underlying remote // IP: 127.0.0.1. // -// func TestPEXReactorRunning(t *testing.T) { -// N := 3 -// switches := make([]*p2p.Switch, N) - -// // directory to store address books -// dir, err := ioutil.TempDir("", "pex_reactor") -// require.Nil(t, err) -// defer os.RemoveAll(dir) // nolint: errcheck - -// books := make([]*addrBook, N) -// logger := log.TestingLogger() - -// // create switches -// for i := 0; i < N; i++ { -// switches[i] = p2p.MakeSwitch(config, i, "testing", "123.123.123", func(i int, sw *p2p.Switch) *p2p.Switch { -// books[i] = NewAddrBook(filepath.Join(dir, fmt.Sprintf("addrbook%d.json", i)), false) -// books[i].SetLogger(logger.With("pex", i)) -// sw.SetAddrBook(books[i]) - -// sw.SetLogger(logger.With("pex", i)) - -// r := NewPEXReactor(books[i], &PEXReactorConfig{}) -// r.SetLogger(logger.With("pex", i)) -// r.SetEnsurePeersPeriod(250 * time.Millisecond) -// sw.AddReactor("pex", r) - -// return sw -// }) -// } - -// addOtherNodeAddrToAddrBook := func(switchIndex, otherSwitchIndex int) { -// addr := switches[otherSwitchIndex].NodeInfo().NetAddress() -// books[switchIndex].AddAddress(addr, addr) -// } - -// addOtherNodeAddrToAddrBook(0, 1) -// addOtherNodeAddrToAddrBook(1, 0) -// addOtherNodeAddrToAddrBook(2, 1) - -// for i, sw := range switches { -// sw.AddListener(p2p.NewDefaultListener("tcp", sw.NodeInfo().ListenAddr, true, logger.With("pex", i))) - -// err := sw.Start() // start switch and reactors -// require.Nil(t, err) -// } - -// assertPeersWithTimeout(t, switches, 10*time.Millisecond, 10*time.Second, N-1) - -// // stop them -// for _, s := range switches { -// s.Stop() -// } -// } +func TestPEXReactorRunning(t *testing.T) { + N := 3 + switches := make([]*p2p.Switch, N) + + // directory to store address books + dir, err := ioutil.TempDir("", "pex_reactor") + require.Nil(t, err) + defer os.RemoveAll(dir) // nolint: errcheck + + books := make([]*addrBook, N) + logger := log.TestingLogger() + + // create switches + for i := 0; i < N; i++ { + switches[i] = p2p.MakeSwitch(config, i, "testing", "123.123.123", func(i int, sw *p2p.Switch) *p2p.Switch { + books[i] = NewAddrBook(filepath.Join(dir, fmt.Sprintf("addrbook%d.json", i)), false) + books[i].SetLogger(logger.With("pex", i)) + sw.SetAddrBook(books[i]) + + sw.SetLogger(logger.With("pex", i)) + + r := NewPEXReactor(books[i], &PEXReactorConfig{}) + r.SetLogger(logger.With("pex", i)) + r.SetEnsurePeersPeriod(250 * time.Millisecond) + sw.AddReactor("pex", r) + + return sw + }) + } + + addOtherNodeAddrToAddrBook := func(switchIndex, otherSwitchIndex int) { + addr := switches[otherSwitchIndex].NodeInfo().NetAddress() + books[switchIndex].AddAddress(addr, addr) + } + + addOtherNodeAddrToAddrBook(0, 1) + addOtherNodeAddrToAddrBook(1, 0) + addOtherNodeAddrToAddrBook(2, 1) + + for i, sw := range switches { + sw.AddListener(p2p.NewDefaultListener("tcp", sw.NodeInfo().ListenAddr, true, logger.With("pex", i))) + + err := sw.Start() // start switch and reactors + require.Nil(t, err) + } + + assertPeersWithTimeout(t, switches, 10*time.Millisecond, 10*time.Second, N-1) + + // stop them + for _, s := range switches { + s.Stop() + } +} func TestPEXReactorReceive(t *testing.T) { r, book := createReactor(&PEXReactorConfig{}) diff --git a/p2p/switch.go b/p2p/switch.go index dc9b1698a..7656b9b02 100644 --- a/p2p/switch.go +++ b/p2p/switch.go @@ -577,8 +577,9 @@ func (sw *Switch) addPeer(pc peerConn) error { } // Check for duplicate connection or peer info IP. - if sw.peers.HasIP(pc.RemoteIP()) || - sw.peers.HasIP(peerNodeInfo.NetAddress().IP) { + if !sw.config.SkipDuplicatePeerIPCheck && + (sw.peers.HasIP(pc.RemoteIP()) || + sw.peers.HasIP(peerNodeInfo.NetAddress().IP)) { return ErrSwitchDuplicatePeerIP{pc.RemoteIP()} } diff --git a/p2p/test_util.go b/p2p/test_util.go index 86955f692..b5b739af9 100644 --- a/p2p/test_util.go +++ b/p2p/test_util.go @@ -3,7 +3,6 @@ package p2p import ( "fmt" "net" - "sync/atomic" crypto "github.com/tendermint/go-crypto" cmn "github.com/tendermint/tmlibs/common" @@ -132,8 +131,6 @@ func StartSwitches(switches []*Switch) error { return nil } -var listenAddrSuffix uint32 = 1 - func MakeSwitch(cfg *cfg.P2PConfig, i int, network, version string, initSwitch func(int, *Switch) *Switch) *Switch { // new switch, add reactors // TODO: let the config be passed in? @@ -148,7 +145,7 @@ func MakeSwitch(cfg *cfg.P2PConfig, i int, network, version string, initSwitch f Moniker: cmn.Fmt("switch%d", i), Network: network, Version: version, - ListenAddr: fmt.Sprintf("127.0.0.%d:%d", atomic.AddUint32(&listenAddrSuffix, 1), cmn.RandIntn(64512)+1023), + ListenAddr: fmt.Sprintf("127.0.0.1:%d", cmn.RandIntn(64512)+1023), } for ch := range sw.reactorsByCh { ni.Channels = append(ni.Channels, ch)