Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FAB-17177] Config block shouldn't verify itself in block replication #355

Merged
merged 1 commit into from
Dec 3, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions orderer/common/cluster/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -201,12 +201,14 @@ func VerifyBlocks(blockBuff []*common.Block, signatureVerifier BlockVerifier) er
}

var config *common.ConfigEnvelope
var isLastBlockConfigBlock bool
// Verify all configuration blocks that are found inside the block batch,
// with the configuration that was committed (nil) or with one that is picked up
// during iteration over the block batch.
for _, block := range blockBuff {
configFromBlock, err := ConfigFromBlock(block)
if err == errNotAConfig {
isLastBlockConfigBlock = false
continue
}
if err != nil {
Expand All @@ -217,10 +219,17 @@ func VerifyBlocks(blockBuff []*common.Block, signatureVerifier BlockVerifier) er
return err
}
config = configFromBlock
isLastBlockConfigBlock = true
}

// Verify the last block's signature
lastBlock := blockBuff[len(blockBuff)-1]

// If last block is a config block, we verified it using the policy of the previous block, so it's valid.
if isLastBlockConfigBlock {
return nil
}

return VerifyBlockSignature(lastBlock, signatureVerifier, config)
}

Expand Down
61 changes: 54 additions & 7 deletions orderer/common/cluster/util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -357,10 +357,11 @@ func TestVerifyBlocks(t *testing.T) {
}

for _, testCase := range []struct {
name string
configureVerifier func(*mocks.BlockVerifier)
mutateBlockSequence func([]*common.Block) []*common.Block
expectedError string
name string
configureVerifier func(*mocks.BlockVerifier)
mutateBlockSequence func([]*common.Block) []*common.Block
expectedError string
verifierExpectedCalls int
}{
{
name: "empty sequence",
Expand Down Expand Up @@ -388,7 +389,8 @@ func TestVerifyBlocks(t *testing.T) {
var nilEnvelope *common.ConfigEnvelope
verifier.On("VerifyBlockSignature", mock.Anything, nilEnvelope).Return(errors.New("bad signature"))
},
expectedError: "bad signature",
expectedError: "bad signature",
verifierExpectedCalls: 1,
},
{
name: "block that its type cannot be classified",
Expand Down Expand Up @@ -436,10 +438,11 @@ func TestVerifyBlocks(t *testing.T) {
proto.Unmarshal(protoutil.MarshalOrPanic(configEnvelope1), confEnv1)
verifier.On("VerifyBlockSignature", sigSet2, confEnv1).Return(errors.New("bad signature")).Once()
},
expectedError: "bad signature",
expectedError: "bad signature",
verifierExpectedCalls: 2,
},
{
name: "config block in the sequence needs to be verified, and it isproperly signed",
name: "config block in the sequence needs to be verified, and it is properly signed",
mutateBlockSequence: func(blockSequence []*common.Block) []*common.Block {
var err error
// Put a config transaction in block n / 4
Expand All @@ -465,6 +468,48 @@ func TestVerifyBlocks(t *testing.T) {
verifier.On("VerifyBlockSignature", sigSet1, nilEnvelope).Return(nil).Once()
verifier.On("VerifyBlockSignature", sigSet2, confEnv1).Return(nil).Once()
},
// We have a single config block in the 'middle' of the chain, so we have 2 verifications total:
// The last block, and the config block.
verifierExpectedCalls: 2,
},
{
name: "last two blocks are config blocks, last block only verified once",
mutateBlockSequence: func(blockSequence []*common.Block) []*common.Block {
var err error
// Put a config transaction in block n-2 and in n-1
blockSequence[len(blockSequence)-2].Data = &common.BlockData{
Data: [][]byte{protoutil.MarshalOrPanic(configTransaction(configEnvelope1))},
}
blockSequence[len(blockSequence)-2].Header.DataHash = protoutil.BlockDataHash(blockSequence[len(blockSequence)-2].Data)

blockSequence[len(blockSequence)-1].Data = &common.BlockData{
Data: [][]byte{protoutil.MarshalOrPanic(configTransaction(configEnvelope2))},
}
blockSequence[len(blockSequence)-1].Header.DataHash = protoutil.BlockDataHash(blockSequence[len(blockSequence)-1].Data)

assignHashes(blockSequence)

sigSet1, err = cluster.SignatureSetFromBlock(blockSequence[len(blockSequence)-2])
assert.NoError(t, err)

sigSet2, err = cluster.SignatureSetFromBlock(blockSequence[len(blockSequence)-1])
assert.NoError(t, err)

return blockSequence
},
configureVerifier: func(verifier *mocks.BlockVerifier) {
var nilEnvelope *common.ConfigEnvelope
confEnv1 := &common.ConfigEnvelope{}
proto.Unmarshal(protoutil.MarshalOrPanic(configEnvelope1), confEnv1)
verifier.On("VerifyBlockSignature", sigSet1, nilEnvelope).Return(nil).Once()
// We ensure that the signature set of the last block is verified using the config envelope of the block
// before it.
verifier.On("VerifyBlockSignature", sigSet2, confEnv1).Return(nil).Once()
// Note that we do not record a call to verify the last block, with the config envelope extracted from the block itself.
},
// We have 2 config blocks, yet we only verify twice- the first config block, and the next config block, but no more,
// since the last block is a config block.
verifierExpectedCalls: 2,
},
} {
testCase := testCase
Expand All @@ -478,6 +523,8 @@ func TestVerifyBlocks(t *testing.T) {
err := cluster.VerifyBlocks(blockchain, verifier)
if testCase.expectedError != "" {
assert.EqualError(t, err, testCase.expectedError)
} else {
assert.NoError(t, err)
}
})
}
Expand Down
2 changes: 1 addition & 1 deletion orderer/common/server/onboarding_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -635,7 +635,7 @@ func TestReplicate(t *testing.T) {
},
{
name: "Explicit replication is requested, but the channel shouldn't be pulled",
verificationCount: 20,
verificationCount: 10,
shouldConnect: true,
systemLedgerHeight: 10,
bootBlock: &bootBlock,
Expand Down