From 046c2c07627065de3e2bfc687cf30f15abd94bf3 Mon Sep 17 00:00:00 2001 From: Yacov Manevich Date: Wed, 18 Oct 2023 01:37:18 +0200 Subject: [PATCH] Verify transactions in a block are well formed Verify that transactions in blocks appear exactly as their marshaled form after unmarshaling. Signed-off-by: Yacov Manevich --- integration/smartbft/smartbft_test.go | 8 +- internal/peer/gossip/mcs.go | 4 + orderer/common/cluster/util.go | 3 + orderer/common/cluster/util_test.go | 13 ++- orderer/consensus/smartbft/assembler_test.go | 2 + orderer/consensus/smartbft/verifier.go | 4 + protoutil/blockutils.go | 37 +++++++ protoutil/blockutils_test.go | 109 +++++++++++++++++++ 8 files changed, 173 insertions(+), 7 deletions(-) diff --git a/integration/smartbft/smartbft_test.go b/integration/smartbft/smartbft_test.go index 4123c49492f..63c7feeeaa1 100644 --- a/integration/smartbft/smartbft_test.go +++ b/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) diff --git a/internal/peer/gossip/mcs.go b/internal/peer/gossip/mcs.go index b46df8b6abe..9c3b5c8fd14 100644 --- a/internal/peer/gossip/mcs.go +++ b/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) { diff --git a/orderer/common/cluster/util.go b/orderer/common/cluster/util.go index e229bebfcf5..05b1bfaa977 100644 --- a/orderer/common/cluster/util.go +++ b/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 diff --git a/orderer/common/cluster/util_test.go b/orderer/common/cluster/util_test.go index 79d7678eb09..7f63713333f 100644 --- a/orderer/common/cluster/util_test.go +++ b/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) } diff --git a/orderer/consensus/smartbft/assembler_test.go b/orderer/consensus/smartbft/assembler_test.go index 6aa57b4e0e1..bd3e9d3aea3 100644 --- a/orderer/consensus/smartbft/assembler_test.go +++ b/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{ diff --git a/orderer/consensus/smartbft/verifier.go b/orderer/consensus/smartbft/verifier.go index 2b9fdfc4ce5..f232a1eae95 100644 --- a/orderer/consensus/smartbft/verifier.go +++ b/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 { diff --git a/protoutil/blockutils.go b/protoutil/blockutils.go index 8527869e49e..404dd397d05 100644 --- a/protoutil/blockutils.go +++ b/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 +} diff --git a/protoutil/blockutils_test.go b/protoutil/blockutils_test.go index b2159da9f55..393cf5c4dd6 100644 --- a/protoutil/blockutils_test.go +++ b/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) + } + }) + } +}