Verify transactions in a block are well formed

integration/smartbft/smartbft_test.go

@@ -1081,7 +1081,7 @@ var _ = Describe("EndToEnd Smart BFT configuration test", func() {
 
 			By("Sending TX with corrupted signature")
 			env = ordererclient.CreateBroadcastEnvelope(network, leader, channel, []byte("MESSSAGE_2"))
-			env.Signature = []byte{}
+			env.Signature = []byte{1, 2, 3}
 			resp, err = ordererclient.Broadcast(network, leader, env)
 			Expect(err).NotTo(HaveOccurred())
 			Expect(resp.Status).To(Equal(common.Status_FORBIDDEN))
@@ -1383,7 +1383,7 @@ var _ = Describe("EndToEnd Smart BFT configuration test", func() {
 				0,
 			)
 			Expect(err).NotTo(HaveOccurred())
-			env.Signature = []byte{}
+			env.Signature = []byte{1, 2, 3}
 
 			By("Authenticate to the leader step service")
 			err = client.Auth()
@@ -1466,12 +1466,12 @@ var _ = Describe("EndToEnd Smart BFT configuration test", func() {
 				common.HeaderType_ENDORSER_TRANSACTION,
 				channel,
 				signer,
-				&common.Envelope{Payload: []byte("TEST_MESSAGE")},
+				&common.Envelope{Payload: []byte("TEST_MESSAGE"), Signature: []byte{1, 2, 3}},
 				0,
 				0,
 			)
 			Expect(err).NotTo(HaveOccurred())
-			env.Signature = []byte{}
+			env.Signature = []byte{1, 2, 3}
 
 			By("Create the consensus request")
 			genesisBlock := network.LoadAppChannelGenesisBlock(channel)

internal/peer/gossip/mcs.go

@@ -150,6 +150,10 @@ func (s *MSPMessageCryptoService) VerifyBlock(chainID common.ChannelID, seqNum u
 		return fmt.Errorf("Block with id [%d] on channel [%s] does not have metadata. Block not valid.", block.Header.Number, chainID)
 	}
 
+	if err := protoutil.VerifyTransactionsAreWellFormed(block); err != nil {
+		return err
+	}
+
 	// - Verify that Header.DataHash is equal to the hash of block.Data
 	// This is to ensure that the header is consistent with the data carried by this block
 	if !bytes.Equal(protoutil.BlockDataHash(block.Data), block.Header.DataHash) {

orderer/common/cluster/util.go

@@ -260,6 +260,9 @@ func VerifyBlockHash(indexInBuffer int, blockBuff []*common.Block) error {
 	if block.Header == nil {
 		return errors.New("missing block header")
 	}
+	if err := protoutil.VerifyTransactionsAreWellFormed(block); err != nil {
+		return err
+	}
 	seq := block.Header.Number
 	dataHash := protoutil.BlockDataHash(block.Data)
 	// Verify data hash matches the hash in the header

orderer/common/cluster/util_test.go

@@ -187,7 +187,14 @@ func TestVerifyBlockBFT(t *testing.T) {
 	block, err := test.MakeGenesisBlock("mychannel")
 	require.NoError(t, err)
 
+	configTx := &common.Envelope{}
+	err = proto.Unmarshal(block.Data.Data[0], configTx)
+	require.NoError(t, err)
+
+	configTx.Signature = []byte{1, 2, 3}
+	block.Data.Data[0] = protoutil.MarshalOrPanic(configTx)
 	block.Header.Number = 2
+	block.Header.DataHash = protoutil.BlockDataHash(block.Data)
 
 	twoBlocks := createBlockChain(2, 3)
 	twoBlocks[0] = block
@@ -256,7 +263,7 @@ func TestVerifyBlockHash(t *testing.T) {
 		},
 		{
 			name: "data hash mismatch",
-			errorContains: "computed hash of block (13) (dcb2ec1c5e482e4914cb953ff8eedd12774b244b12912afbe6001ba5de9ff800)" +
+			errorContains: "computed hash of block (13) (6de668ac99645e179a4921b477d50df9295fa56cd44f8e5c94756b60ce32ce1c)" +
 				" doesn't match claimed hash (07)",
 			mutateBlockSequence: func(blockSequence []*common.Block) []*common.Block {
 				blockSequence[len(blockSequence)/2].Header.DataHash = []byte{7}
@@ -266,7 +273,7 @@ func TestVerifyBlockHash(t *testing.T) {
 		{
 			name: "prev hash mismatch",
 			errorContains: "block [12]'s hash " +
-				"(866351705f1c2f13e10d52ead9d0ca3b80689ede8cc8bf70a6d60c67578323f4) " +
+				"(72cc7ddf4d8465da95115c0a906416d23d8c74bfcb731a5ab057c213d8db62e1) " +
 				"mismatches block [13]'s prev block hash (07)",
 			mutateBlockSequence: func(blockSequence []*common.Block) []*common.Block {
 				blockSequence[len(blockSequence)/2].Header.PreviousHash = []byte{7}
@@ -316,6 +323,7 @@ func createBlockChain(start, end uint64) []*common.Block {
 		})
 
 		txn := protoutil.MarshalOrPanic(&common.Envelope{
+			Signature: []byte{1, 2, 3},
 			Payload: protoutil.MarshalOrPanic(&common.Payload{
 				Header: &common.Header{},
 			}),
@@ -326,7 +334,6 @@ func createBlockChain(start, end uint64) []*common.Block {
 	var blockchain []*common.Block
 	for seq := start; seq <= end; seq++ {
 		block := newBlock(seq)
-		block.Data.Data = append(block.Data.Data, make([]byte, 100))
 		block.Header.DataHash = protoutil.BlockDataHash(block.Data)
 		blockchain = append(blockchain, block)
 	}

orderer/consensus/smartbft/assembler_test.go

@@ -99,6 +99,7 @@ func TestAssembler(t *testing.T) {
 
 func makeTx(headerType int32) []byte {
 	return protoutil.MarshalOrPanic(&common.Envelope{
+		Signature: []byte{1, 2, 3},
 		Payload: protoutil.MarshalOrPanic(&common.Payload{
 			Header: &common.Header{
 				ChannelHeader: protoutil.MarshalOrPanic(&common.ChannelHeader{
@@ -136,6 +137,7 @@ func makeConfigBlock(seq uint64) *common.Block {
 		},
 		Data: &common.BlockData{
 			Data: [][]byte{protoutil.MarshalOrPanic(&common.Envelope{
+				Signature: []byte{1, 2, 3},
 				Payload: protoutil.MarshalOrPanic(&common.Payload{
 					Data: protoutil.MarshalOrPanic(&common.ConfigEnvelope{
 						Config: &common.Config{

orderer/consensus/smartbft/verifier.go

@@ -237,6 +237,10 @@ func verifyHashChain(block *cb.Block, prevHeaderHash string) error {
 		return errors.Errorf("previous header hash is %s but expected %s", thisHdrHashOfPrevHdr, prevHeaderHash)
 	}
 
+	if err := protoutil.VerifyTransactionsAreWellFormed(block); err != nil {
+		return err
+	}
+
 	dataHash := hex.EncodeToString(block.Header.DataHash)
 	actualHashOfData := hex.EncodeToString(protoutil.BlockDataHash(block.Data))
 	if dataHash != actualHashOfData {

protoutil/blockutils.go

@@ -10,6 +10,7 @@ import (
 	"bytes"
 	"crypto/sha256"
 	"encoding/asn1"
+	"encoding/base64"
 	"fmt"
 	"math/big"
 
@@ -298,3 +299,39 @@ func searchConsenterIdentityByID(consenters []*cb.Consenter, identifier uint32)
 	}
 	return nil
 }
+
+func VerifyTransactionsAreWellFormed(block *cb.Block) error {
+	if block == nil || block.Data == nil || len(block.Data.Data) == 0 {
+		return fmt.Errorf("empty block")
+	}
+
+	for i, rawTx := range block.Data.Data {
+		env := &cb.Envelope{}
+		if err := proto.Unmarshal(rawTx, env); err != nil {
+			return fmt.Errorf("transaction %d is invalid: %v", i, err)
+		}
+
+		if len(env.Payload) == 0 {
+			return fmt.Errorf("transaction %d has no payload", i)
+		}
+
+		if len(env.Signature) == 0 {
+			return fmt.Errorf("transaction %d has no signature", i)
+		}
+
+		expected, err := proto.Marshal(env)
+		if err != nil {
+			return fmt.Errorf("failed re-marshaling envelope: %v", err)
+		}
+
+		if len(expected) < len(rawTx) {
+			return fmt.Errorf("transaction %d has %d trailing bytes", i, len(rawTx)-len(expected))
+		}
+		if !bytes.Equal(expected, rawTx) {
+			return fmt.Errorf("transaction %d (%s) does not match its raw form (%s)", i,
+				base64.StdEncoding.EncodeToString(expected), base64.StdEncoding.EncodeToString(rawTx))
+		}
+	}
+
+	return nil
+}

protoutil/blockutils_test.go

@@ -489,3 +489,112 @@ func TestBlockSignatureVerifierByCreator(t *testing.T) {
 	require.Len(t, signatureSet, 1)
 	require.Equal(t, []byte("creator1"), signatureSet[0].Identity)
 }
+
+func TestVerifyTransactionsAreWellFormed(t *testing.T) {
+	originalBlock := &cb.Block{
+		Data: &cb.BlockData{
+			Data: [][]byte{
+				marshalOrPanic(&cb.Envelope{
+					Payload:   []byte{1, 2, 3},
+					Signature: []byte{4, 5, 6},
+				}),
+				marshalOrPanic(&cb.Envelope{
+					Payload:   []byte{7, 8, 9},
+					Signature: []byte{10, 11, 12},
+				}),
+			},
+		},
+	}
+
+	forgedBlock := proto.Clone(originalBlock).(*cb.Block)
+	tmp := make([]byte, len(forgedBlock.Data.Data[0])+len(forgedBlock.Data.Data[1]))
+	copy(tmp, forgedBlock.Data.Data[0])
+	copy(tmp[len(forgedBlock.Data.Data[0]):], forgedBlock.Data.Data[1])
+	forgedBlock.Data.Data = [][]byte{tmp} // Replace transactions {0,1} with transaction {0 || 1}
+
+	for _, tst := range []struct {
+		name          string
+		expectedError string
+		block         *cb.Block
+	}{
+		{
+			name:          "empty block",
+			expectedError: "empty block",
+		},
+		{
+			name:          "no block data",
+			block:         &cb.Block{},
+			expectedError: "empty block",
+		},
+		{
+			name:          "no transactions",
+			block:         &cb.Block{Data: &cb.BlockData{}},
+			expectedError: "empty block",
+		},
+		{
+			name: "single transaction",
+			block: &cb.Block{Data: &cb.BlockData{Data: [][]byte{marshalOrPanic(&cb.Envelope{
+				Payload:   []byte{1, 2, 3},
+				Signature: []byte{4, 5, 6},
+			})}}},
+		},
+		{
+			name:  "good block",
+			block: originalBlock,
+		},
+		{
+			name:          "forged block",
+			block:         forgedBlock,
+			expectedError: "transaction 0 has 10 trailing bytes",
+		},
+		{
+			name:          "no signature",
+			expectedError: "transaction 0 has no signature",
+			block: &cb.Block{
+				Data: &cb.BlockData{
+					Data: [][]byte{
+						marshalOrPanic(&cb.Envelope{
+							Payload: []byte{1, 2, 3},
+						}),
+					},
+				},
+			},
+		},
+		{
+			name:          "no payload",
+			expectedError: "transaction 0 has no payload",
+			block: &cb.Block{
+				Data: &cb.BlockData{
+					Data: [][]byte{
+						marshalOrPanic(&cb.Envelope{
+							Signature: []byte{4, 5, 6},
+						}),
+					},
+				},
+			},
+		},
+		{
+			name:          "transaction invalid",
+			expectedError: "cannot parse invalid wire-format data",
+			block: &cb.Block{
+				Data: &cb.BlockData{
+					Data: [][]byte{
+						marshalOrPanic(&cb.Envelope{
+							Payload:   []byte{1, 2, 3},
+							Signature: []byte{4, 5, 6},
+						})[9:],
+					},
+				},
+			},
+		},
+	} {
+		t.Run(tst.name, func(t *testing.T) {
+			err := protoutil.VerifyTransactionsAreWellFormed(tst.block)
+			if tst.expectedError == "" {
+				require.NoError(t, err)
+			} else {
+				require.Contains(t, err.Error(), tst.expectedError)
+			}
+		})
+	}
+}