From 6ab2a190887ac8a38bb751f827a4f9e174753e65 Mon Sep 17 00:00:00 2001 From: Marko Date: Wed, 9 Sep 2020 11:13:18 +0200 Subject: [PATCH] header: check block protocol (#5340) ## Description Check block protocol version in header validate basic. I tried searching for where we check the P2P protocol version but was unable to find it. When we check compatibility with a node we check we both have the same block protocol and are on the same network, but we do not check if we are on the same P2P protocol. It makes sense if there is a handshake change because we would not be able to establish a secure connection, but a p2p protocol version bump may be because of a p2p message change, which would go unnoticed until that message is sent over the wire. Is this purposeful? Closes: #4790 --- CHANGELOG_PENDING.md | 1 + blockchain/msgs_test.go | 3 ++- light/helpers_test.go | 3 +++ light/store/db/db_test.go | 3 +++ store/store_test.go | 8 +++++++- types/block.go | 4 ++++ types/block_test.go | 16 ++++++++++------ types/light_test.go | 7 +++++-- types/test_util.go | 5 ++++- 9 files changed, 39 insertions(+), 11 deletions(-) diff --git a/CHANGELOG_PENDING.md b/CHANGELOG_PENDING.md index a74cac809..f3be19468 100644 --- a/CHANGELOG_PENDING.md +++ b/CHANGELOG_PENDING.md @@ -35,6 +35,7 @@ Friendly reminder, we have a [bug bounty program](https://hackerone.com/tendermi - [blockchain] \#5278 Verify only +2/3 of the signatures in a block when fast syncing. (@marbar3778) - [rpc] \#5293 `/dial_peers` has added `private` and `unconditional` as parameters. (@marbar3778) +- [types] \#5340 Add check in `Header.ValidateBasic()` for block protocol version (@marbar3778) ## BUG FIXES diff --git a/blockchain/msgs_test.go b/blockchain/msgs_test.go index 784ef8065..097e9a195 100644 --- a/blockchain/msgs_test.go +++ b/blockchain/msgs_test.go @@ -81,6 +81,7 @@ func TestBcStatusResponseMessageValidateBasic(t *testing.T) { // nolint:lll // ignore line length in tests func TestBlockchainMessageVectors(t *testing.T) { block := types.MakeBlock(int64(3), []types.Tx{types.Tx("Hello World")}, nil, nil) + block.Version.Block = 11 // overwrite updated protocol version bpb, err := block.ToProto() require.NoError(t, err) @@ -96,7 +97,7 @@ func TestBlockchainMessageVectors(t *testing.T) { BlockRequest: &bcproto.BlockRequest{Height: math.MaxInt64}}}, "0a0a08ffffffffffffffff7f"}, {"BlockResponseMessage", &bcproto.Message{Sum: &bcproto.Message_BlockResponse{ - BlockResponse: &bcproto.BlockResponse{Block: bpb}}}, "1ab3010ab0010a590a001803220b088092b8c398feffffff012a0212003a20c4da88e876062aa1543400d50d0eaa0dac88096057949cfb7bca7f3a48c04bf96a20e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855122f0a0b48656c6c6f20576f726c641220c4da88e876062aa1543400d50d0eaa0dac88096057949cfb7bca7f3a48c04bf91a221220e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855"}, + BlockResponse: &bcproto.BlockResponse{Block: bpb}}}, "1ab5010ab2010a5b0a02080b1803220b088092b8c398feffffff012a0212003a20c4da88e876062aa1543400d50d0eaa0dac88096057949cfb7bca7f3a48c04bf96a20e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855122f0a0b48656c6c6f20576f726c641220c4da88e876062aa1543400d50d0eaa0dac88096057949cfb7bca7f3a48c04bf91a221220e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855"}, {"NoBlockResponseMessage", &bcproto.Message{Sum: &bcproto.Message_NoBlockResponse{ NoBlockResponse: &bcproto.NoBlockResponse{Height: 1}}}, "12020801"}, {"NoBlockResponseMessage", &bcproto.Message{Sum: &bcproto.Message_NoBlockResponse{ diff --git a/light/helpers_test.go b/light/helpers_test.go index 9e00f722c..5a1cb5fb5 100644 --- a/light/helpers_test.go +++ b/light/helpers_test.go @@ -7,8 +7,10 @@ import ( "github.com/tendermint/tendermint/crypto/ed25519" "github.com/tendermint/tendermint/crypto/tmhash" tmproto "github.com/tendermint/tendermint/proto/tendermint/types" + tmversion "github.com/tendermint/tendermint/proto/tendermint/version" "github.com/tendermint/tendermint/types" tmtime "github.com/tendermint/tendermint/types/time" + "github.com/tendermint/tendermint/version" ) // privKeys is a helper type for testing. @@ -126,6 +128,7 @@ func genHeader(chainID string, height int64, bTime time.Time, txs types.Txs, valset, nextValset *types.ValidatorSet, appHash, consHash, resHash []byte) *types.Header { return &types.Header{ + Version: tmversion.Consensus{Block: version.BlockProtocol, App: 0}, ChainID: chainID, Height: height, Time: bTime, diff --git a/light/store/db/db_test.go b/light/store/db/db_test.go index a0b71e6fa..ef9710694 100644 --- a/light/store/db/db_test.go +++ b/light/store/db/db_test.go @@ -13,7 +13,9 @@ import ( "github.com/tendermint/tendermint/crypto" "github.com/tendermint/tendermint/crypto/tmhash" tmrand "github.com/tendermint/tendermint/libs/rand" + tmversion "github.com/tendermint/tendermint/proto/tendermint/version" "github.com/tendermint/tendermint/types" + "github.com/tendermint/tendermint/version" ) func TestLast_FirstLightBlockHeight(t *testing.T) { @@ -173,6 +175,7 @@ func randLightBlock(height int64) *types.LightBlock { return &types.LightBlock{ SignedHeader: &types.SignedHeader{ Header: &types.Header{ + Version: tmversion.Consensus{Block: version.BlockProtocol, App: 0}, ChainID: tmrand.Str(12), Height: height, Time: time.Now(), diff --git a/store/store_test.go b/store/store_test.go index ce73aecf3..f03c6c896 100644 --- a/store/store_test.go +++ b/store/store_test.go @@ -19,9 +19,11 @@ import ( "github.com/tendermint/tendermint/libs/log" tmrand "github.com/tendermint/tendermint/libs/rand" tmstore "github.com/tendermint/tendermint/proto/tendermint/store" + tmversion "github.com/tendermint/tendermint/proto/tendermint/version" sm "github.com/tendermint/tendermint/state" "github.com/tendermint/tendermint/types" tmtime "github.com/tendermint/tendermint/types/time" + "github.com/tendermint/tendermint/version" ) // A cleanupFunc cleans up any config / test files created for a particular @@ -182,6 +184,7 @@ func TestBlockStoreSaveLoadBlock(t *testing.T) { require.Error(t, err) header1 := types.Header{ + Version: tmversion.Consensus{Block: version.BlockProtocol}, Height: 1, ChainID: "block_test", Time: tmtime.Now(), @@ -218,6 +221,7 @@ func TestBlockStoreSaveLoadBlock(t *testing.T) { { block: newBlock( // New block at height 5 in empty block store is fine types.Header{ + Version: tmversion.Consensus{Block: version.BlockProtocol}, Height: 5, ChainID: "block_test", Time: tmtime.Now(), @@ -502,7 +506,9 @@ func TestLoadBlockMeta(t *testing.T) { require.Contains(t, panicErr.Error(), "unmarshal to tmproto.BlockMeta") // 3. A good blockMeta serialized and saved to the DB should be retrievable - meta := &types.BlockMeta{Header: types.Header{Height: 1, ProposerAddress: tmrand.Bytes(crypto.AddressSize)}} + meta := &types.BlockMeta{Header: types.Header{ + Version: tmversion.Consensus{ + Block: version.BlockProtocol, App: 0}, Height: 1, ProposerAddress: tmrand.Bytes(crypto.AddressSize)}} pbm := meta.ToProto() err = db.Set(calcBlockMetaKey(height), mustEncode(pbm)) require.NoError(t, err) diff --git a/types/block.go b/types/block.go index 4c8889c48..749770768 100644 --- a/types/block.go +++ b/types/block.go @@ -19,6 +19,7 @@ import ( tmsync "github.com/tendermint/tendermint/libs/sync" tmproto "github.com/tendermint/tendermint/proto/tendermint/types" tmversion "github.com/tendermint/tendermint/proto/tendermint/version" + "github.com/tendermint/tendermint/version" ) const ( @@ -375,6 +376,9 @@ func (h *Header) Populate( // // NOTE: Timestamp validation is subtle and handled elsewhere. func (h Header) ValidateBasic() error { + if h.Version.Block != version.BlockProtocol { + return fmt.Errorf("block protocol is incorrect: got: %d, want: %d ", h.Version.Block, version.BlockProtocol) + } if len(h.ChainID) > MaxChainIDLen { return fmt.Errorf("chainID is too long; got: %d, max: %d", len(h.ChainID), MaxChainIDLen) } diff --git a/types/block_test.go b/types/block_test.go index d347bc675..d9e3aa64e 100644 --- a/types/block_test.go +++ b/types/block_test.go @@ -22,8 +22,9 @@ import ( "github.com/tendermint/tendermint/libs/bytes" tmrand "github.com/tendermint/tendermint/libs/rand" tmproto "github.com/tendermint/tendermint/proto/tendermint/types" - "github.com/tendermint/tendermint/proto/tendermint/version" + tmversion "github.com/tendermint/tendermint/proto/tendermint/version" tmtime "github.com/tendermint/tendermint/types/time" + "github.com/tendermint/tendermint/version" ) func TestMain(m *testing.M) { @@ -86,6 +87,9 @@ func TestBlockValidateBasic(t *testing.T) { {"Tampered EvidenceHash", func(blk *Block) { blk.EvidenceHash = []byte("something else") }, true}, + {"Incorrect block protocol version", func(blk *Block) { + blk.Version.Block = 1 + }, true}, } for i, tc := range testCases { tc := tc @@ -260,7 +264,7 @@ func TestHeaderHash(t *testing.T) { expectHash bytes.HexBytes }{ {"Generates expected hash", &Header{ - Version: version.Consensus{Block: 1, App: 2}, + Version: tmversion.Consensus{Block: 1, App: 2}, ChainID: "chainId", Height: 3, Time: time.Date(2019, 10, 13, 16, 14, 44, 0, time.UTC), @@ -277,7 +281,7 @@ func TestHeaderHash(t *testing.T) { }, hexBytesFromString("F740121F553B5418C3EFBD343C2DBFE9E007BB67B0D020A0741374BAB65242A4")}, {"nil header yields nil", nil, nil}, {"nil ValidatorsHash yields nil", &Header{ - Version: version.Consensus{Block: 1, App: 2}, + Version: tmversion.Consensus{Block: 1, App: 2}, ChainID: "chainId", Height: 3, Time: time.Date(2019, 10, 13, 16, 14, 44, 0, time.UTC), @@ -317,7 +321,7 @@ func TestHeaderHash(t *testing.T) { bz, err := gogotypes.StdTimeMarshal(f) require.NoError(t, err) byteSlices = append(byteSlices, bz) - case version.Consensus: + case tmversion.Consensus: bz, err := f.Marshal() require.NoError(t, err) byteSlices = append(byteSlices, bz) @@ -352,7 +356,7 @@ func TestMaxHeaderBytes(t *testing.T) { timestamp := time.Date(math.MaxInt64, 0, 0, 0, 0, 0, math.MaxInt64, time.UTC) h := Header{ - Version: version.Consensus{Block: math.MaxInt64, App: math.MaxInt64}, + Version: tmversion.Consensus{Block: math.MaxInt64, App: math.MaxInt64}, ChainID: maxChainID, Height: math.MaxInt64, Time: timestamp, @@ -696,7 +700,7 @@ func makeRandHeader() Header { randBytes := tmrand.Bytes(tmhash.Size) randAddress := tmrand.Bytes(crypto.AddressSize) h := Header{ - Version: version.Consensus{Block: 1, App: 1}, + Version: tmversion.Consensus{Block: version.BlockProtocol, App: 1}, ChainID: chainID, Height: height, Time: t, diff --git a/types/light_test.go b/types/light_test.go index 267c18961..fa04cd4cf 100644 --- a/types/light_test.go +++ b/types/light_test.go @@ -8,7 +8,8 @@ import ( "github.com/stretchr/testify/assert" "github.com/tendermint/tendermint/crypto" - "github.com/tendermint/tendermint/proto/tendermint/version" + tmversion "github.com/tendermint/tendermint/proto/tendermint/version" + "github.com/tendermint/tendermint/version" ) func TestLightBlockValidateBasic(t *testing.T) { @@ -18,6 +19,7 @@ func TestLightBlockValidateBasic(t *testing.T) { header.Height = commit.Height header.LastBlockID = commit.BlockID header.ValidatorsHash = vals.Hash() + header.Version.Block = version.BlockProtocol vals2, _ := RandValidatorSet(3, 1) vals3 := vals.Copy() vals3.Proposer = &Validator{} @@ -61,6 +63,7 @@ func TestLightBlockProtobuf(t *testing.T) { vals, _ := RandValidatorSet(5, 1) header.Height = commit.Height header.LastBlockID = commit.BlockID + header.Version.Block = version.BlockProtocol header.ValidatorsHash = vals.Hash() vals3 := vals.Copy() vals3.Proposer = &Validator{} @@ -112,7 +115,7 @@ func TestSignedHeaderValidateBasic(t *testing.T) { chainID := "𠜎" timestamp := time.Date(math.MaxInt64, 0, 0, 0, 0, 0, math.MaxInt64, time.UTC) h := Header{ - Version: version.Consensus{Block: math.MaxInt64, App: math.MaxInt64}, + Version: tmversion.Consensus{Block: version.BlockProtocol, App: math.MaxInt64}, ChainID: chainID, Height: commit.Height, Time: timestamp, diff --git a/types/test_util.go b/types/test_util.go index 9ae263fb2..0481c1388 100644 --- a/types/test_util.go +++ b/types/test_util.go @@ -5,6 +5,8 @@ import ( "time" tmproto "github.com/tendermint/tendermint/proto/tendermint/types" + tmversion "github.com/tendermint/tendermint/proto/tendermint/version" + "github.com/tendermint/tendermint/version" ) func MakeCommit(blockID BlockID, height int64, round int32, @@ -85,7 +87,8 @@ func MakeVote( func MakeBlock(height int64, txs []Tx, lastCommit *Commit, evidence []Evidence) *Block { block := &Block{ Header: Header{ - Height: height, + Version: tmversion.Consensus{Block: version.BlockProtocol, App: 0}, + Height: height, }, Data: Data{ Txs: txs,