From 8ccfdb96d05f8e5f563dd5afa6db3c703f3afa6f Mon Sep 17 00:00:00 2001 From: Anton Kaliaev Date: Tue, 30 Jun 2020 11:43:51 +0400 Subject: [PATCH] consensus: Do not allow signatures for a wrong block in commits Closes #4926 The dump consensus state had this: "last_commit": { "votes": [ "Vote{0:04CBBF43CA3E 385085/00/2(Precommit) 1B73DA9FC4C8 42C97B86D89D @ 2020-05-27T06:46:51.042392895Z}", "Vote{1:055799E028FA 385085/00/2(Precommit) 652B08AD61EA 0D507D7FA3AB @ 2020-06-28T04:57:29.20793209Z}", "Vote{2:056024CFA910 385085/00/2(Precommit) 652B08AD61EA C8E95532A4C3 @ 2020-06-28T04:57:29.452696998Z}", "Vote{3:0741C95814DA 385085/00/2(Precommit) 652B08AD61EA 36D567615F7C @ 2020-06-28T04:57:29.279788593Z}", Note there's a precommit in there from the first val from May (2020-05-27) while the rest are from today (2020-06-28). It suggests there's a validator from an old instance of the network at this height (they're using the same chain-id!). Obviously a single bad validator shouldn't be an issue. But the Commit refactor work introduced a bug. When we propose a block, we get the block.LastCommit by calling MakeCommit on the set of precommits we saw for the last height. This set may include precommits for a different block, and hence the block.LastCommit we propose may include precommits that aren't actually for the last block (but of course +2/3 will be). Before v0.33, we just skipped over these precommits during verification. But in v0.33, we expect all signatures for a blockID to be for the same block ID! Thus we end up proposing a block that we can't verify. --- consensus/invalid_test.go | 94 +++++++++++++++++++++++++++++++++++++++ types/vote_set.go | 15 +++++-- 2 files changed, 105 insertions(+), 4 deletions(-) create mode 100644 consensus/invalid_test.go diff --git a/consensus/invalid_test.go b/consensus/invalid_test.go new file mode 100644 index 000000000..1ae0a69ec --- /dev/null +++ b/consensus/invalid_test.go @@ -0,0 +1,94 @@ +package consensus + +import ( + "testing" + + "github.com/tendermint/tendermint/libs/bytes" + "github.com/tendermint/tendermint/libs/log" + tmrand "github.com/tendermint/tendermint/libs/rand" + "github.com/tendermint/tendermint/p2p" + "github.com/tendermint/tendermint/types" +) + +//---------------------------------------------- +// byzantine failures + +// one byz val sends a precommit for a random block at each height +// Ensure a testnet makes blocks +func TestReactorInvalidPrecommit(t *testing.T) { + N := 4 + css, cleanup := randConsensusNet(N, "consensus_reactor_test", newMockTickerFunc(true), newCounter) + defer cleanup() + + for i := 0; i < 4; i++ { + ticker := NewTimeoutTicker() + ticker.SetLogger(css[i].Logger) + css[i].SetTimeoutTicker(ticker) + + } + + reactors, blocksSubs, eventBuses := startConsensusNet(t, css, N) + + // this val sends a random precommit at each height + byzValIdx := 0 + byzVal := css[byzValIdx] + byzR := reactors[byzValIdx] + + // update the doPrevote function to just send a valid precommit for a random block + // and otherwise disable the priv validator + byzVal.mtx.Lock() + pv := byzVal.privValidator + byzVal.doPrevote = func(height int64, round int) { + invalidDoPrevoteFunc(t, height, round, byzVal, byzR.Switch, pv) + } + byzVal.mtx.Unlock() + defer stopConsensusNet(log.TestingLogger(), reactors, eventBuses) + + // wait for a bunch of blocks + // TODO: make this tighter by ensuring the halt happens by block 2 + for i := 0; i < 10; i++ { + timeoutWaitGroup(t, N, func(j int) { + <-blocksSubs[j].Out() + }, css) + } +} + +func invalidDoPrevoteFunc(t *testing.T, height int64, round int, cs *State, sw *p2p.Switch, pv types.PrivValidator) { + // routine to: + // - precommit for a random block + // - send precommit to all peers + // - disable privValidator (so we don't do normal precommits) + go func() { + cs.mtx.Lock() + cs.privValidator = pv + pubKey, err := cs.privValidator.GetPubKey() + if err != nil { + panic(err) + } + addr := pubKey.Address() + valIndex, _ := cs.Validators.GetByAddress(addr) + + // precommit a random block + blockHash := bytes.HexBytes(tmrand.Bytes(32)) + precommit := &types.Vote{ + ValidatorAddress: addr, + ValidatorIndex: valIndex, + Height: cs.Height, + Round: cs.Round, + Timestamp: cs.voteTime(), + Type: types.PrecommitType, + BlockID: types.BlockID{ + Hash: blockHash, + PartsHeader: types.PartSetHeader{Total: 1, Hash: tmrand.Bytes(32)}}, + } + cs.privValidator.SignVote(cs.state.ChainID, precommit) + cs.privValidator = nil // disable priv val so we don't do normal votes + cs.mtx.Unlock() + + peers := sw.Peers().List() + for _, peer := range peers { + cs.Logger.Info("Sending bad vote", "block", blockHash, "peer", peer) + peer.Send(VoteChannel, cdc.MustMarshalBinaryBare(&VoteMessage{precommit})) + } + }() +} diff --git a/types/vote_set.go b/types/vote_set.go index 82698fe51..6ff6e02a5 100644 --- a/types/vote_set.go +++ b/types/vote_set.go @@ -547,9 +547,11 @@ func (voteSet *VoteSet) sumTotalFrac() (int64, int64, float64) { //-------------------------------------------------------------------------------- // Commit -// MakeCommit constructs a Commit from the VoteSet. -// Panics if the vote type is not PrecommitType or if -// there's no +2/3 votes for a single block. +// MakeCommit constructs a Commit from the VoteSet. It only includes precommits +// for the block, which has 2/3+ majority, and nil. +// +// Panics if the vote type is not PrecommitType or if there's no +2/3 votes for +// a single block. func (voteSet *VoteSet) MakeCommit() *Commit { if voteSet.signedMsgType != PrecommitType { panic("Cannot MakeCommit() unless VoteSet.Type is PrecommitType") @@ -565,7 +567,12 @@ func (voteSet *VoteSet) MakeCommit() *Commit { // For every validator, get the precommit commitSigs := make([]CommitSig, len(voteSet.votes)) for i, v := range voteSet.votes { - commitSigs[i] = v.CommitSig() + commitSig := v.CommitSig() + // if block ID exists but doesn't match, exclude sig + if commitSig.ForBlock() && !v.BlockID.Equals(*voteSet.maj23) { + commitSig = NewCommitSigAbsent() + } + commitSigs[i] = commitSig } return NewCommit(voteSet.GetHeight(), voteSet.GetRound(), *voteSet.maj23, commitSigs)