From f84c2f0ee95f6cd62c6df6cd109b7d0b5d0e2957 Mon Sep 17 00:00:00 2001 From: Yoav Tock Date: Tue, 6 Jun 2023 13:32:18 +0300 Subject: [PATCH] Follower: check if join block different from fetched block - before fetched block is committed - don't panic - continue to retry - improve channel participation Verifier to check DataHash== hash(block.Data) Signed-off-by: Yoav Tock Change-Id: I7089df4f1bfc200a5f17582df0da7d4172f8f09b --- cmd/osnadmin/main_test.go | 69 ++++----- .../raft/channel_participation_test.go | 69 ++++----- integration/raft/config_test.go | 1 + .../common/channelparticipation/validator.go | 10 ++ .../channelparticipation/validator_test.go | 139 +++++++++++------- orderer/common/follower/follower_chain.go | 25 ++-- .../common/follower/follower_chain_test.go | 53 ++++++- 7 files changed, 231 insertions(+), 135 deletions(-) diff --git a/cmd/osnadmin/main_test.go b/cmd/osnadmin/main_test.go index 80a5be72ff1..ea372c9065e 100644 --- a/cmd/osnadmin/main_test.go +++ b/cmd/osnadmin/main_test.go @@ -47,7 +47,7 @@ var _ = Describe("osnadmin", func() { BeforeEach(func() { var err error - tempDir, err = ioutil.TempDir("", "osnadmin") + tempDir, err = os.MkdirTemp("", "osnadmin") Expect(err).NotTo(HaveOccurred()) generateCertificates(tempDir) @@ -782,46 +782,49 @@ func generateCertificates(tempDir string) { } func blockWithGroups(groups map[string]*cb.ConfigGroup, channelID string) *cb.Block { - return &cb.Block{ - Data: &cb.BlockData{ - Data: [][]byte{ - protoutil.MarshalOrPanic(&cb.Envelope{ - Payload: protoutil.MarshalOrPanic(&cb.Payload{ - Data: protoutil.MarshalOrPanic(&cb.ConfigEnvelope{ - Config: &cb.Config{ - ChannelGroup: &cb.ConfigGroup{ - Groups: groups, - Values: map[string]*cb.ConfigValue{ - "HashingAlgorithm": { - Value: protoutil.MarshalOrPanic(&cb.HashingAlgorithm{ - Name: bccsp.SHA256, - }), - }, - "BlockDataHashingStructure": { - Value: protoutil.MarshalOrPanic(&cb.BlockDataHashingStructure{ - Width: math.MaxUint32, - }), - }, - "OrdererAddresses": { - Value: protoutil.MarshalOrPanic(&cb.OrdererAddresses{ - Addresses: []string{"localhost"}, - }), - }, + block := protoutil.NewBlock(0, []byte{}) + block.Data = &cb.BlockData{ + Data: [][]byte{ + protoutil.MarshalOrPanic(&cb.Envelope{ + Payload: protoutil.MarshalOrPanic(&cb.Payload{ + Data: protoutil.MarshalOrPanic(&cb.ConfigEnvelope{ + Config: &cb.Config{ + ChannelGroup: &cb.ConfigGroup{ + Groups: groups, + Values: map[string]*cb.ConfigValue{ + "HashingAlgorithm": { + Value: protoutil.MarshalOrPanic(&cb.HashingAlgorithm{ + Name: bccsp.SHA256, + }), + }, + "BlockDataHashingStructure": { + Value: protoutil.MarshalOrPanic(&cb.BlockDataHashingStructure{ + Width: math.MaxUint32, + }), + }, + "OrdererAddresses": { + Value: protoutil.MarshalOrPanic(&cb.OrdererAddresses{ + Addresses: []string{"localhost"}, + }), }, }, }, - }), - Header: &cb.Header{ - ChannelHeader: protoutil.MarshalOrPanic(&cb.ChannelHeader{ - Type: int32(cb.HeaderType_CONFIG), - ChannelId: channelID, - }), }, }), + Header: &cb.Header{ + ChannelHeader: protoutil.MarshalOrPanic(&cb.ChannelHeader{ + Type: int32(cb.HeaderType_CONFIG), + ChannelId: channelID, + }), + }, }), - }, + }), }, } + block.Header.DataHash = protoutil.BlockDataHash(block.Data) + protoutil.InitBlockMetadata(block) + + return block } func createBlockFile(tempDir string, configBlock *cb.Block) string { diff --git a/integration/raft/channel_participation_test.go b/integration/raft/channel_participation_test.go index 41d551ff161..a53c466d783 100644 --- a/integration/raft/channel_participation_test.go +++ b/integration/raft/channel_participation_test.go @@ -1040,46 +1040,49 @@ func multiNodeEtcdRaftTwoChannels() *nwo.Config { } func createJoinBlockDefineSystemChannel(channelID string) *common.Block { - return &common.Block{ - Data: &common.BlockData{ - Data: [][]byte{ - protoutil.MarshalOrPanic(&common.Envelope{ - Payload: protoutil.MarshalOrPanic(&common.Payload{ - Data: protoutil.MarshalOrPanic(&common.ConfigEnvelope{ - Config: &common.Config{ - ChannelGroup: &common.ConfigGroup{ - Groups: map[string]*common.ConfigGroup{ - "Consortiums": {}, + block := protoutil.NewBlock(0, []byte{}) + block.Data = &common.BlockData{ + Data: [][]byte{ + protoutil.MarshalOrPanic(&common.Envelope{ + Payload: protoutil.MarshalOrPanic(&common.Payload{ + Data: protoutil.MarshalOrPanic(&common.ConfigEnvelope{ + Config: &common.Config{ + ChannelGroup: &common.ConfigGroup{ + Groups: map[string]*common.ConfigGroup{ + "Consortiums": {}, + }, + Values: map[string]*common.ConfigValue{ + "HashingAlgorithm": { + Value: protoutil.MarshalOrPanic(&common.HashingAlgorithm{ + Name: bccsp.SHA256, + }), + }, + "BlockDataHashingStructure": { + Value: protoutil.MarshalOrPanic(&common.BlockDataHashingStructure{ + Width: math.MaxUint32, + }), }, - Values: map[string]*common.ConfigValue{ - "HashingAlgorithm": { - Value: protoutil.MarshalOrPanic(&common.HashingAlgorithm{ - Name: bccsp.SHA256, - }), - }, - "BlockDataHashingStructure": { - Value: protoutil.MarshalOrPanic(&common.BlockDataHashingStructure{ - Width: math.MaxUint32, - }), - }, - "OrdererAddresses": { - Value: protoutil.MarshalOrPanic(&common.OrdererAddresses{ - Addresses: []string{"localhost"}, - }), - }, + "OrdererAddresses": { + Value: protoutil.MarshalOrPanic(&common.OrdererAddresses{ + Addresses: []string{"localhost"}, + }), }, }, }, - }), - Header: &common.Header{ - ChannelHeader: protoutil.MarshalOrPanic(&common.ChannelHeader{ - Type: int32(common.HeaderType_CONFIG), - ChannelId: channelID, - }), }, }), + Header: &common.Header{ + ChannelHeader: protoutil.MarshalOrPanic(&common.ChannelHeader{ + Type: int32(common.HeaderType_CONFIG), + ChannelId: channelID, + }), + }, }), - }, + }), }, } + block.Header.DataHash = protoutil.BlockDataHash(block.Data) + protoutil.InitBlockMetadata(block) + + return block } diff --git a/integration/raft/config_test.go b/integration/raft/config_test.go index f9847cc3f53..b48edf39bbd 100644 --- a/integration/raft/config_test.go +++ b/integration/raft/config_test.go @@ -189,6 +189,7 @@ var _ = Describe("EndToEnd reconfiguration and onboarding", func() { Expect(err).NotTo(HaveOccurred()) genesisBlock.Data.Data[0], err = protoutil.Marshal(envelope) Expect(err).NotTo(HaveOccurred()) + genesisBlock.Header.DataHash = protoutil.BlockDataHash(genesisBlock.Data) genesisBlockBytes, err := protoutil.Marshal(genesisBlock) Expect(err).NotTo(HaveOccurred()) err = ioutil.WriteFile(network.OutputBlockPath("testchannel"), genesisBlockBytes, 0o644) diff --git a/orderer/common/channelparticipation/validator.go b/orderer/common/channelparticipation/validator.go index 60aafe6f15e..655d96ab125 100644 --- a/orderer/common/channelparticipation/validator.go +++ b/orderer/common/channelparticipation/validator.go @@ -7,6 +7,8 @@ SPDX-License-Identifier: Apache-2.0 package channelparticipation import ( + "bytes" + cb "github.com/hyperledger/fabric-protos-go/common" "github.com/hyperledger/fabric/bccsp/factory" "github.com/hyperledger/fabric/common/channelconfig" @@ -25,6 +27,14 @@ func ValidateJoinBlock(configBlock *cb.Block) (channelID string, err error) { return "", errors.New("block is not a config block") } + if configBlock.Metadata == nil || len(configBlock.Metadata.Metadata) == 0 { + return "", errors.New("invalid block: does not have metadata") + } + + if !bytes.Equal(protoutil.BlockDataHash(configBlock.Data), configBlock.Header.DataHash) { + return "", errors.New("invalid block: Header.DataHash is different from Hash(block.Data)") + } + envelope, err := protoutil.ExtractEnvelope(configBlock, 0) if err != nil { return "", err diff --git a/orderer/common/channelparticipation/validator_test.go b/orderer/common/channelparticipation/validator_test.go index 086023e2aaa..8d3f02aaf7a 100644 --- a/orderer/common/channelparticipation/validator_test.go +++ b/orderer/common/channelparticipation/validator_test.go @@ -11,6 +11,7 @@ import ( "math" "testing" + "github.com/golang/protobuf/proto" cb "github.com/hyperledger/fabric-protos-go/common" "github.com/hyperledger/fabric/bccsp" "github.com/hyperledger/fabric/orderer/common/channelparticipation" @@ -19,12 +20,45 @@ import ( ) func TestValidateJoinBlock(t *testing.T) { + validJoinBlock := blockWithGroups( + map[string]*cb.ConfigGroup{ + "Application": {}, + }, + "my-channel", + ) + tests := []struct { testName string joinBlock *cb.Block expectedChannelID string expectedErr error }{ + { + testName: "Valid application channel join block", + joinBlock: validJoinBlock, + expectedChannelID: "my-channel", + expectedErr: nil, + }, + { + testName: "Invalid block data hash", + joinBlock: func() *cb.Block { + b := proto.Clone(validJoinBlock).(*cb.Block) + b.Header.DataHash = []byte("bogus") + return b + }(), + expectedChannelID: "", + expectedErr: errors.New("invalid block: Header.DataHash is different from Hash(block.Data)"), + }, + { + testName: "Invalid block metadata", + joinBlock: func() *cb.Block { + b := proto.Clone(validJoinBlock).(*cb.Block) + b.Metadata = nil + return b + }(), + expectedChannelID: "", + expectedErr: errors.New("invalid block: does not have metadata"), + }, { testName: "Not supported: system channel join block", joinBlock: blockWithGroups( @@ -36,17 +70,6 @@ func TestValidateJoinBlock(t *testing.T) { expectedChannelID: "", expectedErr: errors.New("invalid config: contains consortiums: system channel not supported"), }, - { - testName: "Valid application channel join block", - joinBlock: blockWithGroups( - map[string]*cb.ConfigGroup{ - "Application": {}, - }, - "my-channel", - ), - expectedChannelID: "my-channel", - expectedErr: nil, - }, { testName: "Join block not a config block", joinBlock: nonConfigBlock(), @@ -100,62 +123,68 @@ func TestValidateJoinBlock(t *testing.T) { } func blockWithGroups(groups map[string]*cb.ConfigGroup, channelID string) *cb.Block { - return &cb.Block{ - Data: &cb.BlockData{ - Data: [][]byte{ - protoutil.MarshalOrPanic(&cb.Envelope{ - Payload: protoutil.MarshalOrPanic(&cb.Payload{ - Data: protoutil.MarshalOrPanic(&cb.ConfigEnvelope{ - Config: &cb.Config{ - ChannelGroup: &cb.ConfigGroup{ - Groups: groups, - Values: map[string]*cb.ConfigValue{ - "HashingAlgorithm": { - Value: protoutil.MarshalOrPanic(&cb.HashingAlgorithm{ - Name: bccsp.SHA256, - }), - }, - "BlockDataHashingStructure": { - Value: protoutil.MarshalOrPanic(&cb.BlockDataHashingStructure{ - Width: math.MaxUint32, - }), - }, - "OrdererAddresses": { - Value: protoutil.MarshalOrPanic(&cb.OrdererAddresses{ - Addresses: []string{"localhost"}, - }), - }, + block := protoutil.NewBlock(0, []byte{}) + block.Data = &cb.BlockData{ + Data: [][]byte{ + protoutil.MarshalOrPanic(&cb.Envelope{ + Payload: protoutil.MarshalOrPanic(&cb.Payload{ + Data: protoutil.MarshalOrPanic(&cb.ConfigEnvelope{ + Config: &cb.Config{ + ChannelGroup: &cb.ConfigGroup{ + Groups: groups, + Values: map[string]*cb.ConfigValue{ + "HashingAlgorithm": { + Value: protoutil.MarshalOrPanic(&cb.HashingAlgorithm{ + Name: bccsp.SHA256, + }), + }, + "BlockDataHashingStructure": { + Value: protoutil.MarshalOrPanic(&cb.BlockDataHashingStructure{ + Width: math.MaxUint32, + }), + }, + "OrdererAddresses": { + Value: protoutil.MarshalOrPanic(&cb.OrdererAddresses{ + Addresses: []string{"localhost"}, + }), }, }, }, - }), - Header: &cb.Header{ - ChannelHeader: protoutil.MarshalOrPanic(&cb.ChannelHeader{ - Type: int32(cb.HeaderType_CONFIG), - ChannelId: channelID, - }), }, }), + Header: &cb.Header{ + ChannelHeader: protoutil.MarshalOrPanic(&cb.ChannelHeader{ + Type: int32(cb.HeaderType_CONFIG), + ChannelId: channelID, + }), + }, }), - }, + }), }, } + block.Header.DataHash = protoutil.BlockDataHash(block.Data) + protoutil.InitBlockMetadata(block) + + return block } func nonConfigBlock() *cb.Block { - return &cb.Block{ - Data: &cb.BlockData{ - Data: [][]byte{ - protoutil.MarshalOrPanic(&cb.Envelope{ - Payload: protoutil.MarshalOrPanic(&cb.Payload{ - Header: &cb.Header{ - ChannelHeader: protoutil.MarshalOrPanic(&cb.ChannelHeader{ - Type: int32(cb.HeaderType_ENDORSER_TRANSACTION), - }), - }, - }), + block := protoutil.NewBlock(0, []byte{}) + block.Data = &cb.BlockData{ + Data: [][]byte{ + protoutil.MarshalOrPanic(&cb.Envelope{ + Payload: protoutil.MarshalOrPanic(&cb.Payload{ + Header: &cb.Header{ + ChannelHeader: protoutil.MarshalOrPanic(&cb.ChannelHeader{ + Type: int32(cb.HeaderType_ENDORSER_TRANSACTION), + }), + }, }), - }, + }), }, } + block.Header.DataHash = protoutil.BlockDataHash(block.Data) + protoutil.InitBlockMetadata(block) + + return block } diff --git a/orderer/common/follower/follower_chain.go b/orderer/common/follower/follower_chain.go index 5d4bae31291..1138191e666 100644 --- a/orderer/common/follower/follower_chain.go +++ b/orderer/common/follower/follower_chain.go @@ -12,7 +12,6 @@ import ( "time" "github.com/golang/protobuf/proto" - "github.com/hyperledger/fabric-protos-go/common" "github.com/hyperledger/fabric/bccsp" "github.com/hyperledger/fabric/common/flogging" @@ -297,7 +296,7 @@ func (c *Chain) run() { }() if err := c.pull(); err != nil { - c.logger.Warnf("Pull failed, error: %s", err) + c.logger.Warnf("Pull failed, follower chain stopped, error: %s", err) // TODO set the status to StatusError (see FAB-18106) } } @@ -347,10 +346,6 @@ func (c *Chain) pull() error { c.logger.Info("Onboarding finished successfully, pulled blocks up to join-block") } - if c.joinBlock != nil && !proto.Equal(c.ledgerResources.Block(c.joinBlock.Header.Number).Data, c.joinBlock.Data) { - c.logger.Panicf("Join block (%d) we pulled mismatches block we joined with", c.joinBlock.Header.Number) - } - err = c.pullAfterJoin() if err != nil { return errors.WithMessage(err, "failed to pull after join block") @@ -479,8 +474,9 @@ func (c *Chain) pullUntilLatestWithRetry(latestNetworkHeight uint64, updateEndpo break } - c.logger.Debugf("Error while trying to pull to latest height: %d; going to try again in %v", - latestNetworkHeight, retryInterval) + c.logger.Debugf("Error while trying to pull to latest height: %d; going to try again in %v; error: %s", + latestNetworkHeight, retryInterval, errPull) + select { case <-c.stopChan: c.logger.Debug("Received a stop signal") @@ -527,11 +523,22 @@ func (c *Chain) pullUntilTarget(targetHeight uint64, updateEndpoints bool) (uint if nextBlock == nil { return n, errors.WithMessagef(cluster.ErrRetryCountExhausted, "failed to pull block %d", seq) } + reportedPrevHash := nextBlock.Header.PreviousHash if (nextBlock.Header.Number > 0) && !bytes.Equal(reportedPrevHash, actualPrevHash) { - return n, errors.Errorf("block header mismatch on sequence %d, expected %x, got %x", + return n, errors.Errorf("block header previous hash mismatch on sequence %d, expected %x, got %x", nextBlock.Header.Number, actualPrevHash, reportedPrevHash) } + + if c.joinBlock != nil && c.joinBlock.Header.Number == nextBlock.Header.Number { + // We don't need to verify the block.Data because we verify the join-block's DataHash against the + // hash(join-block.Data) when we verify it during the `Join` REST API call + if !proto.Equal(nextBlock.Header, c.joinBlock.Header) { + c.logger.Errorf("Block header mismatch between the block we pulled and the block we joined with, sequence %d", c.joinBlock.Header.Number) + return n, errors.Errorf("block header mismatch between the block we pulled and the block we joined with, sequence %d", c.joinBlock.Header.Number) + } + } + actualPrevHash = protoutil.BlockHeaderHash(nextBlock.Header) if err := c.ledgerResources.Append(nextBlock); err != nil { return n, errors.WithMessagef(err, "failed to append block %d to the ledger", nextBlock.Header.Number) diff --git a/orderer/common/follower/follower_chain_test.go b/orderer/common/follower/follower_chain_test.go index 2a0268f6f03..6740fe4566e 100644 --- a/orderer/common/follower/follower_chain_test.go +++ b/orderer/common/follower/follower_chain_test.go @@ -178,8 +178,7 @@ func TestFollowerNewChain(t *testing.T) { func TestFollowerPullUpToJoin(t *testing.T) { joinNum := uint64(10) - joinBlockAppRaft := makeConfigBlock(joinNum, []byte{}, 1) - require.NotNil(t, joinBlockAppRaft) + var joinBlockAppRaft *common.Block var wgChain sync.WaitGroup @@ -187,6 +186,7 @@ func TestFollowerPullUpToJoin(t *testing.T) { globalSetup(t) remoteBlockchain.fill(joinNum) remoteBlockchain.appendConfig(1) + joinBlockAppRaft = remoteBlockchain.Block(joinNum) ledgerResources.AppendCalls(localBlockchain.Append) @@ -341,6 +341,41 @@ func TestFollowerPullUpToJoin(t *testing.T) { require.Equal(t, 50, timeAfterCount.AfterCallCount()) require.Equal(t, int64(5000), atomic.LoadInt64(&maxDelay)) }) + t.Run("join block header mismatch", func(t *testing.T) { + setup() + mockClusterConsenter.IsChannelMemberCalls(amIReallyInChannel) + + joinBlockAppRaftBad := proto.Clone(joinBlockAppRaft).(*common.Block) + joinBlockAppRaftBad.Header.DataHash = []byte("bogus") + + chain, err := follower.NewChain(ledgerResources, mockClusterConsenter, joinBlockAppRaftBad, options, pullerFactory, mockChainCreator, cryptoProvider, mockChannelParticipationMetricsReporter) + require.NoError(t, err) + + consensusRelation, status := chain.StatusReport() + require.Equal(t, types.ConsensusRelationConsenter, consensusRelation) + require.Equal(t, types.StatusOnBoarding, status) + + require.NotPanics(t, chain.Start) + require.Eventually(t, + func() bool { + return puller.PullBlockCallCount() > int(joinNum*3) // it will retry forever unless we stop it + }, + 10*time.Second, time.Millisecond) + + require.NotPanics(t, chain.Halt) + require.False(t, chain.IsRunning()) + + consensusRelation, status = chain.StatusReport() + require.Equal(t, types.ConsensusRelationConsenter, consensusRelation) + require.Equal(t, types.StatusOnBoarding, status) + + require.Equal(t, 10, ledgerResources.AppendCallCount()) + require.Equal(t, uint64(10), ledgerResources.Height()) + for i := uint64(0); i < joinNum; i++ { + require.Equal(t, remoteBlockchain.Block(i).Header, localBlockchain.Block(i).Header, "failed block i=%d", i) + } + require.Equal(t, 0, mockChainCreator.SwitchFollowerToChainCallCount()) + }) } func TestFollowerPullAfterJoin(t *testing.T) { @@ -739,8 +774,10 @@ func TestFollowerPullPastJoin(t *testing.T) { }) t.Run("Configs in the middle, latest height increasing", func(t *testing.T) { setup() - localBlockchain.fill(5) - localBlockchain.appendConfig(0) + for i := uint64(0); i < 6; i++ { + localBlockchain.Append(remoteBlockchain.Block(i)) + } + ledgerResources.AppendCalls(func(block *common.Block) error { _ = localBlockchain.Append(block) @@ -908,8 +945,11 @@ func (mbc *memoryBlockChain) fill(numBlocks uint64) { var block *common.Block if i == 0 { block = makeConfigBlock(i, prevHash, 0) + block.Header.DataHash = protoutil.BlockDataHash(block.Data) } else { block = protoutil.NewBlock(i, prevHash) + block.Data.Data = [][]byte{{uint8(i)}, {uint8(i)}} + block.Header.DataHash = protoutil.BlockDataHash(block.Data) protoutil.CopyBlockMetadata(mbc.chain[i-1], block) } @@ -923,6 +963,7 @@ func (mbc *memoryBlockChain) appendConfig(isMember uint8) { h := uint64(len(mbc.chain)) configBlock := makeConfigBlock(h, protoutil.BlockHeaderHash(mbc.chain[h-1].Header), isMember) + configBlock.Header.DataHash = protoutil.BlockDataHash(configBlock.Data) mbc.chain = append(mbc.chain, configBlock) } @@ -949,7 +990,7 @@ func makeConfigBlock(num uint64, prevHash []byte, isMember uint8) *common.Block env := &common.Envelope{ Payload: protoutil.MarshalOrPanic(&common.Payload{ Header: protoutil.MakePayloadHeader( - protoutil.MakeChannelHeader(common.HeaderType_CONFIG, 0, "my-chennel", 0), + protoutil.MakeChannelHeader(common.HeaderType_CONFIG, 0, "my-channel", 0), protoutil.MakeSignatureHeader([]byte{}, []byte{}), ), Data: []byte{isMember}, @@ -957,6 +998,8 @@ func makeConfigBlock(num uint64, prevHash []byte, isMember uint8) *common.Block ), } block.Data.Data = append(block.Data.Data, protoutil.MarshalOrPanic(env)) + block.Header.DataHash = protoutil.BlockDataHash(block.Data) + protoutil.InitBlockMetadata(block) obm := &common.OrdererBlockMetadata{LastConfig: &common.LastConfig{Index: num}} block.Metadata.Metadata[common.BlockMetadataIndex_SIGNATURES] = protoutil.MarshalOrPanic(