Merge "[FAB-3530] Gossip - add block seq# validation"

hyperledger · May 2, 2017 · 2499a05 · 2499a05
2 parents eb4450a + d21cd6d
commit 2499a05
Show file tree

Hide file tree

Showing 17 changed files with 50 additions and 36 deletions.
diff --git a/core/deliverservice/blocksprovider/blocksprovider.go b/core/deliverservice/blocksprovider/blocksprovider.go
@@ -135,7 +135,7 @@ func (b *blocksProviderImpl) DeliverBlocks() {
 				logger.Errorf("Error serializing block with sequence number %d, due to %s", seqNum, err)
 				continue
 			}
-			if err := b.mcs.VerifyBlock(gossipcommon.ChainID(b.chainID), marshaledBlock); err != nil {
+			if err := b.mcs.VerifyBlock(gossipcommon.ChainID(b.chainID), seqNum, marshaledBlock); err != nil {
 				logger.Errorf("Error verifying block with sequnce number %d, due to %s", seqNum, err)
 				continue
 			}

diff --git a/core/deliverservice/blocksprovider/blocksprovider_test.go b/core/deliverservice/blocksprovider/blocksprovider_test.go
@@ -39,7 +39,7 @@ func (*mockMCS) GetPKIidOfCert(peerIdentity api.PeerIdentityType) common2.PKIidT
 	return common2.PKIidType("pkiID")
 }
 
-func (m *mockMCS) VerifyBlock(chainID common2.ChainID, signedBlock []byte) error {
+func (m *mockMCS) VerifyBlock(chainID common2.ChainID, seqNum uint64, signedBlock []byte) error {
 	args := m.Called()
 	if args.Get(0) != nil {
 		return args.Get(0).(error)

diff --git a/core/deliverservice/deliveryclient_test.go b/core/deliverservice/deliveryclient_test.go
@@ -60,7 +60,7 @@ func (*mockMCS) GetPKIidOfCert(peerIdentity api.PeerIdentityType) common.PKIidTy
 	return common.PKIidType("pkiID")
 }
 
-func (*mockMCS) VerifyBlock(chainID common.ChainID, signedBlock []byte) error {
+func (*mockMCS) VerifyBlock(chainID common.ChainID, seqNum uint64, signedBlock []byte) error {
 	return nil
 }
 

diff --git a/gossip/api/crypto.go b/gossip/api/crypto.go
@@ -33,9 +33,10 @@ type MessageCryptoService interface {
 	// This validation is supposed to be done appropriately during the execution flow.
 	GetPKIidOfCert(peerIdentity PeerIdentityType) common.PKIidType
 
-	// VerifyBlock returns nil if the block is properly signed,
+	// VerifyBlock returns nil if the block is properly signed, and the claimed seqNum is the
+	// sequence number that the block's header contains.
 	// else returns error
-	VerifyBlock(chainID common.ChainID, signedBlock []byte) error
+	VerifyBlock(chainID common.ChainID, seqNum uint64, signedBlock []byte) error
 
 	// Sign signs msg with this peer's signing key and outputs
 	// the signature if no error occurred.

diff --git a/gossip/comm/comm_test.go b/gossip/comm/comm_test.go
@@ -72,7 +72,7 @@ func (*naiveSecProvider) GetPKIidOfCert(peerIdentity api.PeerIdentityType) commo
 
 // VerifyBlock returns nil if the block is properly signed,
 // else returns error
-func (*naiveSecProvider) VerifyBlock(chainID common.ChainID, signedBlock []byte) error {
+func (*naiveSecProvider) VerifyBlock(chainID common.ChainID, seqNum uint64, signedBlock []byte) error {
 	return nil
 }
 

diff --git a/gossip/gossip/channel/channel.go b/gossip/gossip/channel/channel.go
@@ -522,11 +522,14 @@ func (gc *gossipChannel) verifyBlock(msg *proto.GossipMessage, sender common.PKI
 		gc.logger.Warning("Received from ", sender, "a DataUpdate message that contains a non-block GossipMessage:", msg)
 		return false
 	}
-	if msg.GetDataMsg().Payload == nil {
+	payload := msg.GetDataMsg().Payload
+	if payload == nil {
 		gc.logger.Warning("Received empty payload from", sender)
 		return false
 	}
-	err := gc.mcs.VerifyBlock(msg.Channel, msg.GetDataMsg().Payload.Data)
+	seqNum := payload.SeqNum
+	rawBlock := payload.Data
+	err := gc.mcs.VerifyBlock(msg.Channel, seqNum, rawBlock)
 	if err != nil {
 		gc.logger.Warning("Received fabricated block from", sender, "in DataUpdate:", err)
 		return false

diff --git a/gossip/gossip/channel/channel_test.go b/gossip/gossip/channel/channel_test.go
@@ -125,7 +125,7 @@ func (cs *cryptoService) VerifyByChannel(channel common.ChainID, identity api.Pe
 	return args.Get(0).(error)
 }
 
-func (cs *cryptoService) VerifyBlock(chainID common.ChainID, signedBlock []byte) error {
+func (cs *cryptoService) VerifyBlock(chainID common.ChainID, seqNum uint64, signedBlock []byte) error {
 	args := cs.Called(signedBlock)
 	if args.Get(0) == nil {
 		return nil

diff --git a/gossip/gossip/gossip_impl.go b/gossip/gossip/gossip_impl.go
@@ -431,7 +431,9 @@ func (g *gossipServiceImpl) validateMsg(msg proto.ReceivedMessage) bool {
 			return true
 		}
 
-		if err := g.mcs.VerifyBlock(msg.GetGossipMessage().Channel, blockMsg.Payload.Data); err != nil {
+		seqNum := blockMsg.Payload.SeqNum
+		rawBlock := blockMsg.Payload.Data
+		if err := g.mcs.VerifyBlock(msg.GetGossipMessage().Channel, seqNum, rawBlock); err != nil {
 			g.logger.Warning("Could not verify block", blockMsg.Payload.SeqNum, ":", err)
 			return false
 		}

diff --git a/gossip/gossip/gossip_test.go b/gossip/gossip/gossip_test.go
@@ -158,7 +158,7 @@ func (*naiveCryptoService) GetPKIidOfCert(peerIdentity api.PeerIdentityType) com
 
 // VerifyBlock returns nil if the block is properly signed,
 // else returns error
-func (*naiveCryptoService) VerifyBlock(chainID common.ChainID, signedBlock []byte) error {
+func (*naiveCryptoService) VerifyBlock(chainID common.ChainID, seqNum uint64, signedBlock []byte) error {
 	return nil
 }
 

diff --git a/gossip/gossip/orgs_test.go b/gossip/gossip/orgs_test.go
@@ -77,7 +77,7 @@ func (*configurableCryptoService) GetPKIidOfCert(peerIdentity api.PeerIdentityTy
 
 // VerifyBlock returns nil if the block is properly signed,
 // else returns error
-func (*configurableCryptoService) VerifyBlock(chainID common.ChainID, signedBlock []byte) error {
+func (*configurableCryptoService) VerifyBlock(chainID common.ChainID, seqNum uint64, signedBlock []byte) error {
 	return nil
 }
 

diff --git a/gossip/identity/identity_test.go b/gossip/identity/identity_test.go
@@ -54,7 +54,7 @@ func (*naiveCryptoService) GetPKIidOfCert(peerIdentity api.PeerIdentityType) com
 
 // VerifyBlock returns nil if the block is properly signed,
 // else returns error
-func (*naiveCryptoService) VerifyBlock(chainID common.ChainID, signedBlock []byte) error {
+func (*naiveCryptoService) VerifyBlock(chainID common.ChainID, seqNum uint64, signedBlock []byte) error {
 	return nil
 }
 

diff --git a/gossip/integration/integration_test.go b/gossip/integration/integration_test.go
@@ -119,7 +119,7 @@ func (s *cryptoService) GetPKIidOfCert(peerIdentity api.PeerIdentityType) common
 	return common.PKIidType(peerIdentity)
 }
 
-func (s *cryptoService) VerifyBlock(chainID common.ChainID, signedBlock []byte) error {
+func (s *cryptoService) VerifyBlock(chainID common.ChainID, seqNum uint64, signedBlock []byte) error {
 	return nil
 }
 

diff --git a/gossip/service/gossip_service_test.go b/gossip/service/gossip_service_test.go
@@ -673,7 +673,7 @@ func (*naiveCryptoService) GetPKIidOfCert(peerIdentity api.PeerIdentityType) gos
 
 // VerifyBlock returns nil if the block is properly signed,
 // else returns error
-func (*naiveCryptoService) VerifyBlock(chainID gossipCommon.ChainID, signedBlock []byte) error {
+func (*naiveCryptoService) VerifyBlock(chainID gossipCommon.ChainID, seqNum uint64, signedBlock []byte) error {
 	return nil
 }
 

diff --git a/gossip/state/state.go b/gossip/state/state.go
@@ -365,7 +365,7 @@ func (s *GossipStateProviderImpl) handleStateResponse(msg proto.ReceivedMessage)
 	}
 	for _, payload := range response.GetPayloads() {
 		logger.Debugf("Received payload with sequence number %d.", payload.SeqNum)
-		if err := s.mcs.VerifyBlock(common2.ChainID(s.chainID), payload.Data); err != nil {
+		if err := s.mcs.VerifyBlock(common2.ChainID(s.chainID), payload.SeqNum, payload.Data); err != nil {
 			logger.Warningf("Error verifying block with sequence number %d, due to %s", payload.SeqNum, err)
 			return uint64(0), err
 		}

diff --git a/gossip/state/state_test.go b/gossip/state/state_test.go
@@ -106,7 +106,7 @@ func (*cryptoServiceMock) GetPKIidOfCert(peerIdentity api.PeerIdentityType) comm
 
 // VerifyBlock returns nil if the block is properly signed,
 // else returns error
-func (*cryptoServiceMock) VerifyBlock(chainID common.ChainID, signedBlock []byte) error {
+func (*cryptoServiceMock) VerifyBlock(chainID common.ChainID, seqNum uint64, signedBlock []byte) error {
 	return nil
 }
 

diff --git a/peer/gossip/mcs.go b/peer/gossip/mcs.go
@@ -113,9 +113,10 @@ func (s *mspMessageCryptoService) GetPKIidOfCert(peerIdentity api.PeerIdentityTy
 	return digest
 }
 
-// VerifyBlock returns nil if the block is properly signed,
+// VerifyBlock returns nil if the block is properly signed, and the claimed seqNum is the
+// sequence number that the block's header contains.
 // else returns error
-func (s *mspMessageCryptoService) VerifyBlock(chainID common.ChainID, signedBlock []byte) error {
+func (s *mspMessageCryptoService) VerifyBlock(chainID common.ChainID, seqNum uint64, signedBlock []byte) error {
 	// - Convert signedBlock to common.Block.
 	block, err := utils.GetBlockFromBlockBytes(signedBlock)
 	if err != nil {
@@ -126,6 +127,11 @@ func (s *mspMessageCryptoService) VerifyBlock(chainID common.ChainID, signedBloc
 		return fmt.Errorf("Invalid Block on channel [%s]. Header must be different from nil.", chainID)
 	}
 
+	blockSeqNum := block.Header.Number
+	if seqNum != blockSeqNum {
+		return fmt.Errorf("Claimed seqNum is [%d] but actual seqNum inside block is [%d]", seqNum, blockSeqNum)
+	}
+
 	// - Extract channelID and compare with chainID
 	channelID, err := utils.GetChainIDFromBlock(block)
 	if err != nil {

diff --git a/peer/gossip/mcs_test.go b/peer/gossip/mcs_test.go
@@ -17,10 +17,8 @@ limitations under the License.
 package gossip
 
 import (
-	"fmt"
-	"testing"
-
 	"reflect"
+	"testing"
 
 	"github.com/golang/protobuf/proto"
 	"github.com/hyperledger/fabric/bccsp"
@@ -152,7 +150,7 @@ func TestVerify(t *testing.T) {
 	assert.NoError(t, err)
 	err = msgCryptoService.Verify(api.PeerIdentityType("Dave"), sigma, msg)
 	assert.Error(t, err)
-	assert.Contains(t, fmt.Sprintf("%v", err), "Could not acquire policy manager")
+	assert.Contains(t, err.Error(), "Could not acquire policy manager")
 
 	// Check invalid args
 	assert.Error(t, msgCryptoService.Verify(nil, sigma, msg))
@@ -182,34 +180,38 @@ func TestVerifyBlock(t *testing.T) {
 	)
 
 	// - Prepare testing valid block, Alice signs it.
-	blockRaw, msg := mockBlock(t, "C", aliceSigner, nil)
+	blockRaw, msg := mockBlock(t, "C", 42, aliceSigner, nil)
 	policyManagerGetter.Managers["C"].(*mocks.ChannelPolicyManager).Policy.(*mocks.Policy).Deserializer.(*mocks.IdentityDeserializer).Msg = msg
-	blockRaw2, msg2 := mockBlock(t, "D", aliceSigner, nil)
+	blockRaw2, msg2 := mockBlock(t, "D", 42, aliceSigner, nil)
 	policyManagerGetter.Managers["D"].(*mocks.ChannelPolicyManager).Policy.(*mocks.Policy).Deserializer.(*mocks.IdentityDeserializer).Msg = msg2
 
 	// - Verify block
-	assert.NoError(t, msgCryptoService.VerifyBlock([]byte("C"), blockRaw))
+	assert.NoError(t, msgCryptoService.VerifyBlock([]byte("C"), 42, blockRaw))
+	// Wrong sequence number claimed
+	err := msgCryptoService.VerifyBlock([]byte("C"), 43, blockRaw)
+	assert.Error(t, err)
+	assert.Contains(t, err.Error(), "but actual seqNum inside block is")
 	delete(policyManagerGetter.Managers, "D")
-	nilPolMgrErr := msgCryptoService.VerifyBlock([]byte("D"), blockRaw2)
-	assert.Contains(t, fmt.Sprintf("%v", nilPolMgrErr), "Could not acquire policy manager")
+	nilPolMgrErr := msgCryptoService.VerifyBlock([]byte("D"), 42, blockRaw2)
+	assert.Contains(t, nilPolMgrErr.Error(), "Could not acquire policy manager")
 	assert.Error(t, nilPolMgrErr)
-	assert.Error(t, msgCryptoService.VerifyBlock([]byte("A"), blockRaw))
-	assert.Error(t, msgCryptoService.VerifyBlock([]byte("B"), blockRaw))
+	assert.Error(t, msgCryptoService.VerifyBlock([]byte("A"), 42, blockRaw))
+	assert.Error(t, msgCryptoService.VerifyBlock([]byte("B"), 42, blockRaw))
 
 	// - Prepare testing invalid block (wrong data has), Alice signs it.
-	blockRaw, msg = mockBlock(t, "C", aliceSigner, []byte{0})
+	blockRaw, msg = mockBlock(t, "C", 42, aliceSigner, []byte{0})
 	policyManagerGetter.Managers["C"].(*mocks.ChannelPolicyManager).Policy.(*mocks.Policy).Deserializer.(*mocks.IdentityDeserializer).Msg = msg
 
 	// - Verify block
-	assert.Error(t, msgCryptoService.VerifyBlock([]byte("C"), blockRaw))
+	assert.Error(t, msgCryptoService.VerifyBlock([]byte("C"), 42, blockRaw))
 
 	// Check invalid args
-	assert.Error(t, msgCryptoService.VerifyBlock([]byte("C"), []byte{0, 1, 2, 3, 4}))
-	assert.Error(t, msgCryptoService.VerifyBlock([]byte("C"), nil))
+	assert.Error(t, msgCryptoService.VerifyBlock([]byte("C"), 42, []byte{0, 1, 2, 3, 4}))
+	assert.Error(t, msgCryptoService.VerifyBlock([]byte("C"), 42, nil))
 }
 
-func mockBlock(t *testing.T, channel string, localSigner crypto.LocalSigner, dataHash []byte) ([]byte, []byte) {
-	block := common.NewBlock(0, nil)
+func mockBlock(t *testing.T, channel string, seqNum uint64, localSigner crypto.LocalSigner, dataHash []byte) ([]byte, []byte) {
+	block := common.NewBlock(seqNum, nil)
 
 	// Add a fake transaction to the block referring channel "C"
 	sProp, _ := utils.MockSignedEndorserProposalOrPanic(channel, &protospeer.ChaincodeSpec{}, []byte("transactor"), []byte("transactor's signature"))