Skip to content

Commit

Permalink
fix: warn on mismatch underlay (#2464)
Browse files Browse the repository at this point in the history
  • Loading branch information
notanatol authored Sep 7, 2021
1 parent 2376885 commit ac11ddd
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 21 deletions.
17 changes: 14 additions & 3 deletions pkg/p2p/libp2p/internal/handshake/handshake.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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
}
Expand All @@ -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),
}
Expand Down Expand Up @@ -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
Expand Down
46 changes: 29 additions & 17 deletions pkg/p2p/libp2p/internal/handshake/handshake_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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 {
Expand All @@ -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)
}
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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)
}
Expand All @@ -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)
}
Expand All @@ -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)
}
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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)
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/p2p/libp2p/libp2p.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down

0 comments on commit ac11ddd

Please sign in to comment.