From ac11dddec2d28fd316036dab3ba5716742b0587b Mon Sep 17 00:00:00 2001 From: Anatol <87016465+notanatol@users.noreply.github.com> Date: Tue, 7 Sep 2021 11:14:17 +0300 Subject: [PATCH] fix: warn on mismatch underlay (#2464) --- .../libp2p/internal/handshake/handshake.go | 17 +++++-- .../internal/handshake/handshake_test.go | 46 ++++++++++++------- pkg/p2p/libp2p/libp2p.go | 2 +- 3 files changed, 44 insertions(+), 21 deletions(-) diff --git a/pkg/p2p/libp2p/internal/handshake/handshake.go b/pkg/p2p/libp2p/internal/handshake/handshake.go index 8483e619f7d..091b869b85d 100644 --- a/pkg/p2p/libp2p/internal/handshake/handshake.go +++ b/pkg/p2p/libp2p/internal/handshake/handshake.go @@ -76,8 +76,8 @@ type Service struct { receivedHandshakes map[libp2ppeer.ID]struct{} receivedHandshakesMu sync.Mutex logger logging.Logger - - network.Notifiee // handshake service can be the receiver for network.Notify + libp2pID libp2ppeer.ID + network.Notifiee // handshake service can be the receiver for network.Notify } // Info contains the information received from the handshake. @@ -95,7 +95,7 @@ func (i *Info) LightString() string { } // New creates a new handshake Service. -func New(signer crypto.Signer, advertisableAddresser AdvertisableAddressResolver, isSender SenderMatcher, overlay swarm.Address, networkID uint64, fullNode bool, transaction []byte, welcomeMessage string, logger logging.Logger) (*Service, error) { +func New(signer crypto.Signer, advertisableAddresser AdvertisableAddressResolver, isSender SenderMatcher, overlay swarm.Address, networkID uint64, fullNode bool, transaction []byte, welcomeMessage string, ownPeerID libp2ppeer.ID, logger logging.Logger) (*Service, error) { if len(welcomeMessage) > MaxWelcomeMessageLength { return nil, ErrWelcomeMessageLength } @@ -109,6 +109,7 @@ func New(signer crypto.Signer, advertisableAddresser AdvertisableAddressResolver transaction: transaction, senderMatcher: isSender, receivedHandshakes: make(map[libp2ppeer.ID]struct{}), + libp2pID: ownPeerID, logger: logger, Notifiee: new(network.NoopNotifiee), } @@ -149,6 +150,16 @@ func (s *Service) Handshake(ctx context.Context, stream p2p.Stream, peerMultiadd return nil, ErrInvalidSyn } + observedUnderlayAddrInfo, err := libp2ppeer.AddrInfoFromP2pAddr(observedUnderlay) + if err != nil { + return nil, fmt.Errorf("extract addr from P2P: %w", err) + } + + if s.libp2pID != observedUnderlayAddrInfo.ID { + //NOTE eventually we will return error here, but for now we want to gather some statistics + s.logger.Warningf("received peer ID %s does not match ours: %s", observedUnderlayAddrInfo.ID, s.libp2pID) + } + advertisableUnderlay, err := s.advertisableAddresser.Resolve(observedUnderlay) if err != nil { return nil, err diff --git a/pkg/p2p/libp2p/internal/handshake/handshake_test.go b/pkg/p2p/libp2p/internal/handshake/handshake_test.go index ab1b85f8ced..2f5bc809536 100644 --- a/pkg/p2p/libp2p/internal/handshake/handshake_test.go +++ b/pkg/p2p/libp2p/internal/handshake/handshake_test.go @@ -50,6 +50,10 @@ func TestHandshake(t *testing.T) { if err != nil { t.Fatal(err) } + node1AddrInfo, err := libp2ppeer.AddrInfoFromP2pAddr(node1ma) + if err != nil { + t.Fatal(err) + } node2AddrInfo, err := libp2ppeer.AddrInfoFromP2pAddr(node2ma) if err != nil { t.Fatal(err) @@ -99,7 +103,7 @@ func TestHandshake(t *testing.T) { senderMatcher := &MockSenderMatcher{v: true, blockHash: blockhash} - handshakeService, err := handshake.New(signer1, aaddresser, senderMatcher, node1Info.BzzAddress.Overlay, networkID, true, trxHash, testWelcomeMessage, logger) + handshakeService, err := handshake.New(signer1, aaddresser, senderMatcher, node1Info.BzzAddress.Overlay, networkID, true, trxHash, testWelcomeMessage, node1AddrInfo.ID, logger) if err != nil { t.Fatal(err) } @@ -151,12 +155,20 @@ func TestHandshake(t *testing.T) { t.Fatal(err) } - if !bytes.Equal(ack.Address.Overlay, node1BzzAddress.Overlay.Bytes()) || - !bytes.Equal(ack.Address.Underlay, node1maBinary) || - !bytes.Equal(ack.Address.Signature, node1BzzAddress.Signature) || - ack.NetworkID != networkID || - ack.FullNode != true { - t.Fatal("bad ack") + if !bytes.Equal(ack.Address.Overlay, node1BzzAddress.Overlay.Bytes()) { + t.Fatal("bad ack - overlay") + } + if !bytes.Equal(ack.Address.Underlay, node1maBinary) { + t.Fatal("bad ack - underlay") + } + if !bytes.Equal(ack.Address.Signature, node1BzzAddress.Signature) { + t.Fatal("bad ack - signature") + } + if ack.NetworkID != networkID { + t.Fatal("bad ack - networkID") + } + if ack.FullNode != true { + t.Fatal("bad ack - full node") } if ack.WelcomeMessage != testWelcomeMessage { @@ -168,7 +180,7 @@ func TestHandshake(t *testing.T) { const LongMessage = "Lorem ipsum dolor sit amet, consectetur adipiscing elit. Morbi consectetur urna ut lorem sollicitudin posuere. Donec sagittis laoreet sapien." expectedErr := handshake.ErrWelcomeMessageLength - _, err := handshake.New(signer1, aaddresser, senderMatcher, node1Info.BzzAddress.Overlay, networkID, true, nil, LongMessage, logger) + _, err := handshake.New(signer1, aaddresser, senderMatcher, node1Info.BzzAddress.Overlay, networkID, true, nil, LongMessage, node1AddrInfo.ID, logger) if err == nil || err.Error() != expectedErr.Error() { t.Fatal("expected:", expectedErr, "got:", err) } @@ -376,7 +388,7 @@ func TestHandshake(t *testing.T) { }) t.Run("Handle - OK", func(t *testing.T) { - handshakeService, err := handshake.New(signer1, aaddresser, senderMatcher, node1Info.BzzAddress.Overlay, networkID, true, trxHash, "", logger) + handshakeService, err := handshake.New(signer1, aaddresser, senderMatcher, node1Info.BzzAddress.Overlay, networkID, true, trxHash, "", node1AddrInfo.ID, logger) if err != nil { t.Fatal(err) } @@ -434,7 +446,7 @@ func TestHandshake(t *testing.T) { }) t.Run("Handle - read error ", func(t *testing.T) { - handshakeService, err := handshake.New(signer1, aaddresser, senderMatcher, node1Info.BzzAddress.Overlay, networkID, true, nil, "", logger) + handshakeService, err := handshake.New(signer1, aaddresser, senderMatcher, node1Info.BzzAddress.Overlay, networkID, true, nil, "", node1AddrInfo.ID, logger) if err != nil { t.Fatal(err) } @@ -453,7 +465,7 @@ func TestHandshake(t *testing.T) { }) t.Run("Handle - write error ", func(t *testing.T) { - handshakeService, err := handshake.New(signer1, aaddresser, senderMatcher, node1Info.BzzAddress.Overlay, networkID, true, nil, "", logger) + handshakeService, err := handshake.New(signer1, aaddresser, senderMatcher, node1Info.BzzAddress.Overlay, networkID, true, nil, "", node1AddrInfo.ID, logger) if err != nil { t.Fatal(err) } @@ -480,7 +492,7 @@ func TestHandshake(t *testing.T) { }) t.Run("Handle - ack read error ", func(t *testing.T) { - handshakeService, err := handshake.New(signer1, aaddresser, senderMatcher, node1Info.BzzAddress.Overlay, networkID, true, nil, "", logger) + handshakeService, err := handshake.New(signer1, aaddresser, senderMatcher, node1Info.BzzAddress.Overlay, networkID, true, nil, "", node1AddrInfo.ID, logger) if err != nil { t.Fatal(err) } @@ -509,7 +521,7 @@ func TestHandshake(t *testing.T) { }) t.Run("Handle - networkID mismatch ", func(t *testing.T) { - handshakeService, err := handshake.New(signer1, aaddresser, senderMatcher, node1Info.BzzAddress.Overlay, networkID, true, nil, "", logger) + handshakeService, err := handshake.New(signer1, aaddresser, senderMatcher, node1Info.BzzAddress.Overlay, networkID, true, nil, "", node1AddrInfo.ID, logger) if err != nil { t.Fatal(err) } @@ -548,7 +560,7 @@ func TestHandshake(t *testing.T) { }) t.Run("Handle - duplicate handshake", func(t *testing.T) { - handshakeService, err := handshake.New(signer1, aaddresser, senderMatcher, node1Info.BzzAddress.Overlay, networkID, true, trxHash, "", logger) + handshakeService, err := handshake.New(signer1, aaddresser, senderMatcher, node1Info.BzzAddress.Overlay, networkID, true, trxHash, "", node1AddrInfo.ID, logger) if err != nil { t.Fatal(err) } @@ -610,7 +622,7 @@ func TestHandshake(t *testing.T) { }) t.Run("Handle - invalid ack", func(t *testing.T) { - handshakeService, err := handshake.New(signer1, aaddresser, senderMatcher, node1Info.BzzAddress.Overlay, networkID, true, nil, "", logger) + handshakeService, err := handshake.New(signer1, aaddresser, senderMatcher, node1Info.BzzAddress.Overlay, networkID, true, nil, "", node1AddrInfo.ID, logger) if err != nil { t.Fatal(err) } @@ -647,7 +659,7 @@ func TestHandshake(t *testing.T) { t.Run("Handle - transaction is not on the blockchain", func(t *testing.T) { sbMock := &MockSenderMatcher{v: false, blockHash: blockhash} - handshakeService, err := handshake.New(signer1, aaddresser, sbMock, node1Info.BzzAddress.Overlay, networkID, true, trxHash, "", logger) + handshakeService, err := handshake.New(signer1, aaddresser, sbMock, node1Info.BzzAddress.Overlay, networkID, true, trxHash, "", node1AddrInfo.ID, logger) if err != nil { t.Fatal(err) } @@ -683,7 +695,7 @@ func TestHandshake(t *testing.T) { }) t.Run("Handle - advertisable error", func(t *testing.T) { - handshakeService, err := handshake.New(signer1, aaddresser, senderMatcher, node1Info.BzzAddress.Overlay, networkID, true, nil, "", logger) + handshakeService, err := handshake.New(signer1, aaddresser, senderMatcher, node1Info.BzzAddress.Overlay, networkID, true, nil, "", node1AddrInfo.ID, logger) if err != nil { t.Fatal(err) } diff --git a/pkg/p2p/libp2p/libp2p.go b/pkg/p2p/libp2p/libp2p.go index 818446b9bd8..9d20e74220a 100644 --- a/pkg/p2p/libp2p/libp2p.go +++ b/pkg/p2p/libp2p/libp2p.go @@ -225,7 +225,7 @@ func New(ctx context.Context, signer beecrypto.Signer, networkID uint64, overlay advertisableAddresser = natAddrResolver } - handshakeService, err := handshake.New(signer, advertisableAddresser, swapBackend, overlay, networkID, o.FullNode, o.Transaction, o.WelcomeMessage, logger) + handshakeService, err := handshake.New(signer, advertisableAddresser, swapBackend, overlay, networkID, o.FullNode, o.Transaction, o.WelcomeMessage, h.ID(), logger) if err != nil { return nil, fmt.Errorf("handshake service: %w", err) }