Browse Source

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.
pull/5075/head
Anton Kaliaev 4 years ago
committed by Tess Rinearson
parent
commit
480b995a31
2 changed files with 108 additions and 4 deletions
  1. +97
    -0
      consensus/invalid_test.go
  2. +11
    -4
      types/vote_set.go

+ 97
- 0
consensus/invalid_test.go View File

@ -0,0 +1,97 @@
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"
tmproto "github.com/tendermint/tendermint/proto/tendermint/types"
"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 int32) {
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 int32, 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: tmproto.PrecommitType,
BlockID: types.BlockID{
Hash: blockHash,
PartSetHeader: types.PartSetHeader{Total: 1, Hash: tmrand.Bytes(32)}},
}
p := precommit.ToProto()
cs.privValidator.SignVote(cs.state.ChainID, p)
precommit.Signature = p.Signature
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, MustEncode(&VoteMessage{precommit}))
}
}()
}

+ 11
- 4
types/vote_set.go View File

@ -549,9 +549,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 != tmproto.PrecommitType {
panic("Cannot MakeCommit() unless VoteSet.Type is PrecommitType")
@ -567,7 +569,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)


Loading…
Cancel
Save