From 353e3a3243bcf0c49602259c415862a87b6a4f1a Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Thu, 22 Apr 2021 15:05:56 +0200 Subject: [PATCH] evidence: fix bug with hashes (backport #6375) (#6381) --- CHANGELOG_PENDING.md | 2 + evidence/pool.go | 71 +------- evidence/pool_test.go | 96 +++++------ evidence/verify.go | 94 +++++----- evidence/verify_test.go | 321 ++++++++++++++++++++++++----------- test/e2e/tests/block_test.go | 6 +- types/evidence.go | 25 ++- types/evidence_test.go | 100 ++++++----- types/light.go | 2 +- types/light_test.go | 6 +- types/validator_set.go | 11 +- 11 files changed, 418 insertions(+), 316 deletions(-) diff --git a/CHANGELOG_PENDING.md b/CHANGELOG_PENDING.md index cbdff5427..f94afa373 100644 --- a/CHANGELOG_PENDING.md +++ b/CHANGELOG_PENDING.md @@ -25,3 +25,5 @@ Friendly reminder, we have a [bug bounty program](https://hackerone.com/tendermi - [statesync] \#6378 Retry requests for snapshots and add a minimum discovery time (5s) for new snapshots. ### BUG FIXES + +- [evidence] \#6375 Fix bug with inconsistent LightClientAttackEvidence hashing (cmwaters) \ No newline at end of file diff --git a/evidence/pool.go b/evidence/pool.go index 5b6b9ef7e..4b49d73cf 100644 --- a/evidence/pool.go +++ b/evidence/pool.go @@ -4,7 +4,6 @@ import ( "bytes" "errors" "fmt" - "sort" "sync" "sync/atomic" "time" @@ -194,9 +193,11 @@ func (evpool *Pool) CheckEvidence(evList types.EvidenceList) error { hashes := make([][]byte, len(evList)) for idx, ev := range evList { - ok := evpool.fastCheck(ev) + _, isLightEv := ev.(*types.LightClientAttackEvidence) - if !ok { + // We must verify light client attack evidence regardless because there could be a + // different conflicting block with the same hash. + if isLightEv || !evpool.isPending(ev) { // check that the evidence isn't already committed if evpool.isCommitted(ev) { return &types.ErrInvalidEvidence{Evidence: ev, Reason: errors.New("evidence was already committed")} @@ -213,7 +214,7 @@ func (evpool *Pool) CheckEvidence(evList types.EvidenceList) error { evpool.logger.Error("Can't add evidence to pending list", "err", err, "ev", ev) } - evpool.logger.Info("Verified new evidence of byzantine behavior", "evidence", ev) + evpool.logger.Info("Check evidence: verified evidence of byzantine behavior", "evidence", ev) } // check for duplicate evidence. We cache hashes so we don't have to work them out again. @@ -255,68 +256,6 @@ func (evpool *Pool) State() sm.State { return evpool.state } -//-------------------------------------------------------------------------- - -// fastCheck leverages the fact that the evidence pool may have already verified the evidence to see if it can -// quickly conclude that the evidence is already valid. -func (evpool *Pool) fastCheck(ev types.Evidence) bool { - if lcae, ok := ev.(*types.LightClientAttackEvidence); ok { - key := keyPending(ev) - evBytes, err := evpool.evidenceStore.Get(key) - if evBytes == nil { // the evidence is not in the nodes pending list - return false - } - if err != nil { - evpool.logger.Error("Failed to load light client attack evidence", "err", err, "key(height/hash)", key) - return false - } - var trustedPb tmproto.LightClientAttackEvidence - err = trustedPb.Unmarshal(evBytes) - if err != nil { - evpool.logger.Error("Failed to convert light client attack evidence from bytes", - "err", err, "key(height/hash)", key) - return false - } - trustedEv, err := types.LightClientAttackEvidenceFromProto(&trustedPb) - if err != nil { - evpool.logger.Error("Failed to convert light client attack evidence from protobuf", - "err", err, "key(height/hash)", key) - return false - } - // ensure that all the byzantine validators that the evidence pool has match the byzantine validators - // in this evidence - if trustedEv.ByzantineValidators == nil && lcae.ByzantineValidators != nil { - return false - } - - if len(trustedEv.ByzantineValidators) != len(lcae.ByzantineValidators) { - return false - } - - byzValsCopy := make([]*types.Validator, len(lcae.ByzantineValidators)) - for i, v := range lcae.ByzantineValidators { - byzValsCopy[i] = v.Copy() - } - - // ensure that both validator arrays are in the same order - sort.Sort(types.ValidatorsByVotingPower(byzValsCopy)) - - for idx, val := range trustedEv.ByzantineValidators { - if !bytes.Equal(byzValsCopy[idx].Address, val.Address) { - return false - } - if byzValsCopy[idx].VotingPower != val.VotingPower { - return false - } - } - - return true - } - - // for all other evidence the evidence pool just checks if it is already in the pending db - return evpool.isPending(ev) -} - // IsExpired checks whether evidence or a polc is expired by checking whether a height and time is older // than set by the evidence consensus parameters func (evpool *Pool) isExpired(height int64, time time.Time) bool { diff --git a/evidence/pool_test.go b/evidence/pool_test.go index 64c6f1b03..365a3be88 100644 --- a/evidence/pool_test.go +++ b/evidence/pool_test.go @@ -174,6 +174,9 @@ func TestReportConflictingVotes(t *testing.T) { // should be able to retrieve evidence from pool evList, _ = pool.PendingEvidence(defaultEvidenceMaxBytes) require.Equal(t, []types.Evidence{ev}, evList) + + next = pool.EvidenceFront() + require.NotNil(t, next) } func TestEvidencePoolUpdate(t *testing.T) { @@ -234,62 +237,29 @@ func TestVerifyDuplicatedEvidenceFails(t *testing.T) { // check that valid light client evidence is correctly validated and stored in // evidence pool -func TestCheckEvidenceWithLightClientAttack(t *testing.T) { +func TestLightClientAttackEvidenceLifecycle(t *testing.T) { var ( - nValidators = 5 - validatorPower int64 = 10 - height int64 = 10 + height int64 = 100 + commonHeight int64 = 90 ) - conflictingVals, conflictingPrivVals := types.RandValidatorSet(nValidators, validatorPower) - trustedHeader := makeHeaderRandom(height) - trustedHeader.Time = defaultEvidenceTime - - conflictingHeader := makeHeaderRandom(height) - conflictingHeader.ValidatorsHash = conflictingVals.Hash() - - trustedHeader.ValidatorsHash = conflictingHeader.ValidatorsHash - trustedHeader.NextValidatorsHash = conflictingHeader.NextValidatorsHash - trustedHeader.ConsensusHash = conflictingHeader.ConsensusHash - trustedHeader.AppHash = conflictingHeader.AppHash - trustedHeader.LastResultsHash = conflictingHeader.LastResultsHash - - // for simplicity we are simulating a duplicate vote attack where all the validators in the - // conflictingVals set voted twice - blockID := makeBlockID(conflictingHeader.Hash(), 1000, []byte("partshash")) - voteSet := types.NewVoteSet(evidenceChainID, height, 1, tmproto.SignedMsgType(2), conflictingVals) - commit, err := types.MakeCommit(blockID, height, 1, voteSet, conflictingPrivVals, defaultEvidenceTime) - require.NoError(t, err) - ev := &types.LightClientAttackEvidence{ - ConflictingBlock: &types.LightBlock{ - SignedHeader: &types.SignedHeader{ - Header: conflictingHeader, - Commit: commit, - }, - ValidatorSet: conflictingVals, - }, - CommonHeight: 10, - TotalVotingPower: int64(nValidators) * validatorPower, - ByzantineValidators: conflictingVals.Validators, - Timestamp: defaultEvidenceTime, - } - trustedBlockID := makeBlockID(trustedHeader.Hash(), 1000, []byte("partshash")) - trustedVoteSet := types.NewVoteSet(evidenceChainID, height, 1, tmproto.SignedMsgType(2), conflictingVals) - trustedCommit, err := types.MakeCommit(trustedBlockID, height, 1, trustedVoteSet, conflictingPrivVals, - defaultEvidenceTime) - require.NoError(t, err) + ev, trusted, common := makeLunaticEvidence(t, height, commonHeight, + 10, 5, 5, defaultEvidenceTime, defaultEvidenceTime.Add(1*time.Hour)) state := sm.State{ - LastBlockTime: defaultEvidenceTime.Add(1 * time.Minute), - LastBlockHeight: 11, + LastBlockTime: defaultEvidenceTime.Add(2 * time.Hour), + LastBlockHeight: 110, ConsensusParams: *types.DefaultConsensusParams(), } stateStore := &smmocks.Store{} - stateStore.On("LoadValidators", height).Return(conflictingVals, nil) + stateStore.On("LoadValidators", height).Return(trusted.ValidatorSet, nil) + stateStore.On("LoadValidators", commonHeight).Return(common.ValidatorSet, nil) stateStore.On("Load").Return(state, nil) blockStore := &mocks.BlockStore{} - blockStore.On("LoadBlockMeta", height).Return(&types.BlockMeta{Header: *trustedHeader}) - blockStore.On("LoadBlockCommit", height).Return(trustedCommit) + blockStore.On("LoadBlockMeta", height).Return(&types.BlockMeta{Header: *trusted.Header}) + blockStore.On("LoadBlockMeta", commonHeight).Return(&types.BlockMeta{Header: *common.Header}) + blockStore.On("LoadBlockCommit", height).Return(trusted.Commit) + blockStore.On("LoadBlockCommit", commonHeight).Return(common.Commit) pool, err := evidence.NewPool(dbm.NewMemDB(), stateStore, blockStore) require.NoError(t, err) @@ -298,14 +268,32 @@ func TestCheckEvidenceWithLightClientAttack(t *testing.T) { err = pool.AddEvidence(ev) assert.NoError(t, err) - err = pool.CheckEvidence(types.EvidenceList{ev}) - assert.NoError(t, err) + hash := ev.Hash() - // take away the last signature -> there are less validators then what we have detected, - // hence this should fail - commit.Signatures = append(commit.Signatures[:nValidators-1], types.NewCommitSigAbsent()) - err = pool.CheckEvidence(types.EvidenceList{ev}) - assert.Error(t, err) + require.NoError(t, pool.AddEvidence(ev)) + require.NoError(t, pool.AddEvidence(ev)) + + pendingEv, _ := pool.PendingEvidence(state.ConsensusParams.Evidence.MaxBytes) + require.Equal(t, 1, len(pendingEv)) + require.Equal(t, ev, pendingEv[0]) + + require.NoError(t, pool.CheckEvidence(pendingEv)) + require.Equal(t, ev, pendingEv[0]) + + state.LastBlockHeight++ + state.LastBlockTime = state.LastBlockTime.Add(1 * time.Minute) + pool.Update(state, pendingEv) + require.Equal(t, hash, pendingEv[0].Hash()) + + remaindingEv, _ := pool.PendingEvidence(state.ConsensusParams.Evidence.MaxBytes) + require.Empty(t, remaindingEv) + + // evidence is already committed so it shouldn't pass + require.Error(t, pool.CheckEvidence(types.EvidenceList{ev})) + require.NoError(t, pool.AddEvidence(ev)) + + remaindingEv, _ = pool.PendingEvidence(state.ConsensusParams.Evidence.MaxBytes) + require.Empty(t, remaindingEv) } // Tests that restarting the evidence pool after a potential failure will recover the @@ -345,7 +333,7 @@ func TestRecoverPendingEvidence(t *testing.T) { Evidence: tmproto.EvidenceParams{ MaxAgeNumBlocks: 20, MaxAgeDuration: 20 * time.Minute, - MaxBytes: 1000, + MaxBytes: defaultEvidenceMaxBytes, }, }, }, nil) diff --git a/evidence/verify.go b/evidence/verify.go index cf0df8cfb..f3eba5358 100644 --- a/evidence/verify.go +++ b/evidence/verify.go @@ -4,7 +4,6 @@ import ( "bytes" "errors" "fmt" - "sort" "time" "github.com/tendermint/tendermint/light" @@ -94,34 +93,6 @@ func (evpool *Pool) verify(evidence types.Evidence) error { if err != nil { return err } - // find out what type of attack this was and thus extract the malicious validators. Note in the case of an - // Amnesia attack we don't have any malicious validators. - validators := ev.GetByzantineValidators(commonVals, trustedHeader) - // ensure this matches the validators that are listed in the evidence. They should be ordered based on power. - if validators == nil && ev.ByzantineValidators != nil { - return fmt.Errorf("expected nil validators from an amnesia light client attack but got %d", - len(ev.ByzantineValidators)) - } - - if exp, got := len(validators), len(ev.ByzantineValidators); exp != got { - return fmt.Errorf("expected %d byzantine validators from evidence but got %d", - exp, got) - } - - // ensure that both validator arrays are in the same order - sort.Sort(types.ValidatorsByVotingPower(ev.ByzantineValidators)) - - for idx, val := range validators { - if !bytes.Equal(ev.ByzantineValidators[idx].Address, val.Address) { - return fmt.Errorf("evidence contained a different byzantine validator address to the one we were expecting."+ - "Expected %v, got %v", val.Address, ev.ByzantineValidators[idx].Address) - } - if ev.ByzantineValidators[idx].VotingPower != val.VotingPower { - return fmt.Errorf("evidence contained a byzantine validator with a different power to the one we were expecting."+ - "Expected %d, got %d", val.VotingPower, ev.ByzantineValidators[idx].VotingPower) - } - } - return nil default: return fmt.Errorf("unrecognized evidence type: %T", evidence) @@ -149,7 +120,7 @@ func VerifyLightClientAttack(e *types.LightClientAttackEvidence, commonHeader, t } // In the case of equivocation and amnesia we expect all header hashes to be correctly derived - } else if isInvalidHeader(trustedHeader.Header, e.ConflictingBlock.Header) { + } else if e.ConflictingHeaderIsInvalid(trustedHeader.Header) { return errors.New("common height is the same as conflicting block height so expected the conflicting" + " block to be correctly derived yet it wasn't") } @@ -178,7 +149,7 @@ func VerifyLightClientAttack(e *types.LightClientAttackEvidence, commonHeader, t trustedHeader.Hash()) } - return nil + return validateABCIEvidence(e, commonVals, trustedHeader) } // VerifyDuplicateVote verifies DuplicateVoteEvidence against the state of full node. This involves the @@ -249,6 +220,55 @@ func VerifyDuplicateVote(e *types.DuplicateVoteEvidence, chainID string, valSet return nil } +// validateABCIEvidence validates the ABCI component of the light client attack +// evidence i.e voting power and byzantine validators +func validateABCIEvidence( + ev *types.LightClientAttackEvidence, + commonVals *types.ValidatorSet, + trustedHeader *types.SignedHeader, +) error { + if evTotal, valsTotal := ev.TotalVotingPower, commonVals.TotalVotingPower(); evTotal != valsTotal { + return fmt.Errorf("total voting power from the evidence and our validator set does not match (%d != %d)", + evTotal, valsTotal) + } + + // Find out what type of attack this was and thus extract the malicious + // validators. Note, in the case of an Amnesia attack we don't have any + // malicious validators. + validators := ev.GetByzantineValidators(commonVals, trustedHeader) + + // Ensure this matches the validators that are listed in the evidence. They + // should be ordered based on power. + if validators == nil && ev.ByzantineValidators != nil { + return fmt.Errorf( + "expected nil validators from an amnesia light client attack but got %d", + len(ev.ByzantineValidators), + ) + } + + if exp, got := len(validators), len(ev.ByzantineValidators); exp != got { + return fmt.Errorf("expected %d byzantine validators from evidence but got %d", exp, got) + } + + for idx, val := range validators { + if !bytes.Equal(ev.ByzantineValidators[idx].Address, val.Address) { + return fmt.Errorf( + "evidence contained an unexpected byzantine validator address; expected: %v, got: %v", + val.Address, ev.ByzantineValidators[idx].Address, + ) + } + + if ev.ByzantineValidators[idx].VotingPower != val.VotingPower { + return fmt.Errorf( + "evidence contained unexpected byzantine validator power; expected %d, got %d", + val.VotingPower, ev.ByzantineValidators[idx].VotingPower, + ) + } + } + + return nil +} + func getSignedHeader(blockStore BlockStore, height int64) (*types.SignedHeader, error) { blockMeta := blockStore.LoadBlockMeta(height) if blockMeta == nil { @@ -263,15 +283,3 @@ func getSignedHeader(blockStore BlockStore, height int64) (*types.SignedHeader, Commit: commit, }, nil } - -// isInvalidHeader takes a trusted header and matches it againt a conflicting header -// to determine whether the conflicting header was the product of a valid state transition -// or not. If it is then all the deterministic fields of the header should be the same. -// If not, it is an invalid header and constitutes a lunatic attack. -func isInvalidHeader(trusted, conflicting *types.Header) bool { - return !bytes.Equal(trusted.ValidatorsHash, conflicting.ValidatorsHash) || - !bytes.Equal(trusted.NextValidatorsHash, conflicting.NextValidatorsHash) || - !bytes.Equal(trusted.ConsensusHash, conflicting.ConsensusHash) || - !bytes.Equal(trusted.AppHash, conflicting.AppHash) || - !bytes.Equal(trusted.LastResultsHash, conflicting.LastResultsHash) -} diff --git a/evidence/verify_test.go b/evidence/verify_test.go index 5b3adf7f1..7825f9d43 100644 --- a/evidence/verify_test.go +++ b/evidence/verify_test.go @@ -1,6 +1,7 @@ package evidence_test import ( + "bytes" "testing" "time" @@ -22,140 +23,171 @@ import ( "github.com/tendermint/tendermint/version" ) -func TestVerifyLightClientAttack_Lunatic(t *testing.T) { - commonVals, commonPrivVals := types.RandValidatorSet(2, 10) - - newVal, newPrivVal := types.RandValidator(false, 9) - - conflictingVals, err := types.ValidatorSetFromExistingValidators(append(commonVals.Validators, newVal)) - require.NoError(t, err) - conflictingPrivVals := append(commonPrivVals, newPrivVal) - - commonHeader := makeHeaderRandom(4) - commonHeader.Time = defaultEvidenceTime - trustedHeader := makeHeaderRandom(10) - trustedHeader.Time = defaultEvidenceTime.Add(1 * time.Hour) - - conflictingHeader := makeHeaderRandom(10) - conflictingHeader.Time = defaultEvidenceTime.Add(1 * time.Hour) - conflictingHeader.ValidatorsHash = conflictingVals.Hash() - - // we are simulating a lunatic light client attack - blockID := makeBlockID(conflictingHeader.Hash(), 1000, []byte("partshash")) - voteSet := types.NewVoteSet(evidenceChainID, 10, 1, tmproto.SignedMsgType(2), conflictingVals) - commit, err := types.MakeCommit(blockID, 10, 1, voteSet, conflictingPrivVals, defaultEvidenceTime) - require.NoError(t, err) - ev := &types.LightClientAttackEvidence{ - ConflictingBlock: &types.LightBlock{ - SignedHeader: &types.SignedHeader{ - Header: conflictingHeader, - Commit: commit, - }, - ValidatorSet: conflictingVals, - }, - CommonHeight: 4, - TotalVotingPower: 20, - ByzantineValidators: commonVals.Validators, - Timestamp: defaultEvidenceTime, - } +const ( + defaultVotingPower = 10 +) - commonSignedHeader := &types.SignedHeader{ - Header: commonHeader, - Commit: &types.Commit{}, - } - trustedBlockID := makeBlockID(trustedHeader.Hash(), 1000, []byte("partshash")) - vals, privVals := types.RandValidatorSet(3, 8) - trustedVoteSet := types.NewVoteSet(evidenceChainID, 10, 1, tmproto.SignedMsgType(2), vals) - trustedCommit, err := types.MakeCommit(trustedBlockID, 10, 1, trustedVoteSet, privVals, defaultEvidenceTime) - require.NoError(t, err) - trustedSignedHeader := &types.SignedHeader{ - Header: trustedHeader, - Commit: trustedCommit, - } +func TestVerifyLightClientAttack_Lunatic(t *testing.T) { + const ( + height int64 = 10 + commonHeight int64 = 4 + totalVals = 10 + byzVals = 4 + ) + attackTime := defaultEvidenceTime.Add(1 * time.Hour) + // create valid lunatic evidence + ev, trusted, common := makeLunaticEvidence( + t, height, commonHeight, totalVals, byzVals, totalVals-byzVals, defaultEvidenceTime, attackTime) + require.NoError(t, ev.ValidateBasic()) // good pass -> no error - err = evidence.VerifyLightClientAttack(ev, commonSignedHeader, trustedSignedHeader, commonVals, + err := evidence.VerifyLightClientAttack(ev, common.SignedHeader, trusted.SignedHeader, common.ValidatorSet, defaultEvidenceTime.Add(2*time.Hour), 3*time.Hour) assert.NoError(t, err) // trusted and conflicting hashes are the same -> an error should be returned - err = evidence.VerifyLightClientAttack(ev, commonSignedHeader, ev.ConflictingBlock.SignedHeader, commonVals, + err = evidence.VerifyLightClientAttack(ev, common.SignedHeader, ev.ConflictingBlock.SignedHeader, common.ValidatorSet, defaultEvidenceTime.Add(2*time.Hour), 3*time.Hour) assert.Error(t, err) // evidence with different total validator power should fail - ev.TotalVotingPower = 1 - err = evidence.VerifyLightClientAttack(ev, commonSignedHeader, trustedSignedHeader, commonVals, + ev.TotalVotingPower = 1 * defaultVotingPower + err = evidence.VerifyLightClientAttack(ev, common.SignedHeader, trusted.SignedHeader, common.ValidatorSet, defaultEvidenceTime.Add(2*time.Hour), 3*time.Hour) assert.Error(t, err) - ev.TotalVotingPower = 20 - - forwardConflictingHeader := makeHeaderRandom(11) - forwardConflictingHeader.Time = defaultEvidenceTime.Add(30 * time.Minute) - forwardConflictingHeader.ValidatorsHash = conflictingVals.Hash() - forwardBlockID := makeBlockID(forwardConflictingHeader.Hash(), 1000, []byte("partshash")) - forwardVoteSet := types.NewVoteSet(evidenceChainID, 11, 1, tmproto.SignedMsgType(2), conflictingVals) - forwardCommit, err := types.MakeCommit(forwardBlockID, 11, 1, forwardVoteSet, conflictingPrivVals, defaultEvidenceTime) - require.NoError(t, err) - forwardLunaticEv := &types.LightClientAttackEvidence{ - ConflictingBlock: &types.LightBlock{ - SignedHeader: &types.SignedHeader{ - Header: forwardConflictingHeader, - Commit: forwardCommit, - }, - ValidatorSet: conflictingVals, - }, - CommonHeight: 4, - TotalVotingPower: 20, - ByzantineValidators: commonVals.Validators, - Timestamp: defaultEvidenceTime, - } - err = evidence.VerifyLightClientAttack(forwardLunaticEv, commonSignedHeader, trustedSignedHeader, commonVals, + + // evidence without enough malicious votes should fail + ev, trusted, common = makeLunaticEvidence( + t, height, commonHeight, totalVals, byzVals-1, totalVals-byzVals, defaultEvidenceTime, attackTime) + err = evidence.VerifyLightClientAttack(ev, common.SignedHeader, trusted.SignedHeader, common.ValidatorSet, defaultEvidenceTime.Add(2*time.Hour), 3*time.Hour) - assert.NoError(t, err) + assert.Error(t, err) +} + +func TestVerify_LunaticAttackAgainstState(t *testing.T) { + const ( + height int64 = 10 + commonHeight int64 = 4 + totalVals = 10 + byzVals = 4 + ) + attackTime := defaultEvidenceTime.Add(1 * time.Hour) + // create valid lunatic evidence + ev, trusted, common := makeLunaticEvidence( + t, height, commonHeight, totalVals, byzVals, totalVals-byzVals, defaultEvidenceTime, attackTime) + // now we try to test verification against state state := sm.State{ LastBlockTime: defaultEvidenceTime.Add(2 * time.Hour), - LastBlockHeight: 11, + LastBlockHeight: height + 1, ConsensusParams: *types.DefaultConsensusParams(), } stateStore := &smmocks.Store{} - stateStore.On("LoadValidators", int64(4)).Return(commonVals, nil) + stateStore.On("LoadValidators", commonHeight).Return(common.ValidatorSet, nil) stateStore.On("Load").Return(state, nil) blockStore := &mocks.BlockStore{} - blockStore.On("LoadBlockMeta", int64(4)).Return(&types.BlockMeta{Header: *commonHeader}) - blockStore.On("LoadBlockMeta", int64(10)).Return(&types.BlockMeta{Header: *trustedHeader}) - blockStore.On("LoadBlockMeta", int64(11)).Return(nil) - blockStore.On("LoadBlockCommit", int64(4)).Return(commit) - blockStore.On("LoadBlockCommit", int64(10)).Return(trustedCommit) - blockStore.On("Height").Return(int64(10)) - + blockStore.On("LoadBlockMeta", commonHeight).Return(&types.BlockMeta{Header: *common.Header}) + blockStore.On("LoadBlockMeta", height).Return(&types.BlockMeta{Header: *trusted.Header}) + blockStore.On("LoadBlockCommit", commonHeight).Return(common.Commit) + blockStore.On("LoadBlockCommit", height).Return(trusted.Commit) pool, err := evidence.NewPool(dbm.NewMemDB(), stateStore, blockStore) require.NoError(t, err) pool.SetLogger(log.TestingLogger()) evList := types.EvidenceList{ev} - err = pool.CheckEvidence(evList) - assert.NoError(t, err) + // check that the evidence pool correctly verifies the evidence + assert.NoError(t, pool.CheckEvidence(evList)) + // as it was not originally in the pending bucket, it should now have been added pendingEvs, _ := pool.PendingEvidence(state.ConsensusParams.Evidence.MaxBytes) assert.Equal(t, 1, len(pendingEvs)) + assert.Equal(t, ev, pendingEvs[0]) // if we submit evidence only against a single byzantine validator when we see there are more validators then this // should return an error - ev.ByzantineValidators = []*types.Validator{commonVals.Validators[0]} - err = pool.CheckEvidence(evList) - assert.Error(t, err) - ev.ByzantineValidators = commonVals.Validators // restore evidence + ev.ByzantineValidators = ev.ByzantineValidators[:1] + t.Log(evList) + assert.Error(t, pool.CheckEvidence(evList)) + // restore original byz vals + ev.ByzantineValidators = ev.GetByzantineValidators(common.ValidatorSet, trusted.SignedHeader) + + // duplicate evidence should be rejected + evList = types.EvidenceList{ev, ev} + pool, err = evidence.NewPool(dbm.NewMemDB(), stateStore, blockStore) + require.NoError(t, err) + assert.Error(t, pool.CheckEvidence(evList)) // If evidence is submitted with an altered timestamp it should return an error ev.Timestamp = defaultEvidenceTime.Add(1 * time.Minute) - err = pool.CheckEvidence(evList) - assert.Error(t, err) + pool, err = evidence.NewPool(dbm.NewMemDB(), stateStore, blockStore) + require.NoError(t, err) + assert.Error(t, pool.AddEvidence(ev)) + ev.Timestamp = defaultEvidenceTime - evList = types.EvidenceList{forwardLunaticEv} - err = pool.CheckEvidence(evList) - assert.NoError(t, err) + // Evidence submitted with a different validator power should fail + ev.TotalVotingPower = 1 + pool, err = evidence.NewPool(dbm.NewMemDB(), stateStore, blockStore) + require.NoError(t, err) + assert.Error(t, pool.AddEvidence(ev)) + ev.TotalVotingPower = common.ValidatorSet.TotalVotingPower() +} + +func TestVerify_ForwardLunaticAttack(t *testing.T) { + const ( + nodeHeight int64 = 8 + attackHeight int64 = 10 + commonHeight int64 = 4 + totalVals = 10 + byzVals = 5 + ) + attackTime := defaultEvidenceTime.Add(1 * time.Hour) + + // create a forward lunatic attack + ev, trusted, common := makeLunaticEvidence( + t, attackHeight, commonHeight, totalVals, byzVals, totalVals-byzVals, defaultEvidenceTime, attackTime) + + // now we try to test verification against state + state := sm.State{ + LastBlockTime: defaultEvidenceTime.Add(2 * time.Hour), + LastBlockHeight: nodeHeight, + ConsensusParams: *types.DefaultConsensusParams(), + } + + // modify trusted light block so that it is of a height less than the conflicting one + trusted.Header.Height = state.LastBlockHeight + trusted.Header.Time = state.LastBlockTime + + stateStore := &smmocks.Store{} + stateStore.On("LoadValidators", commonHeight).Return(common.ValidatorSet, nil) + stateStore.On("Load").Return(state, nil) + blockStore := &mocks.BlockStore{} + blockStore.On("LoadBlockMeta", commonHeight).Return(&types.BlockMeta{Header: *common.Header}) + blockStore.On("LoadBlockMeta", nodeHeight).Return(&types.BlockMeta{Header: *trusted.Header}) + blockStore.On("LoadBlockMeta", attackHeight).Return(nil) + blockStore.On("LoadBlockCommit", commonHeight).Return(common.Commit) + blockStore.On("LoadBlockCommit", nodeHeight).Return(trusted.Commit) + blockStore.On("Height").Return(nodeHeight) + pool, err := evidence.NewPool(dbm.NewMemDB(), stateStore, blockStore) + require.NoError(t, err) + + // check that the evidence pool correctly verifies the evidence + assert.NoError(t, pool.CheckEvidence(types.EvidenceList{ev})) + + // now we use a time which isn't able to contradict the FLA - thus we can't verify the evidence + oldBlockStore := &mocks.BlockStore{} + oldHeader := trusted.Header + oldHeader.Time = defaultEvidenceTime + oldBlockStore.On("LoadBlockMeta", commonHeight).Return(&types.BlockMeta{Header: *common.Header}) + oldBlockStore.On("LoadBlockMeta", nodeHeight).Return(&types.BlockMeta{Header: *oldHeader}) + oldBlockStore.On("LoadBlockMeta", attackHeight).Return(nil) + oldBlockStore.On("LoadBlockCommit", commonHeight).Return(common.Commit) + oldBlockStore.On("LoadBlockCommit", nodeHeight).Return(trusted.Commit) + oldBlockStore.On("Height").Return(nodeHeight) + require.Equal(t, defaultEvidenceTime, oldBlockStore.LoadBlockMeta(nodeHeight).Header.Time) + + pool, err = evidence.NewPool(dbm.NewMemDB(), stateStore, oldBlockStore) + require.NoError(t, err) + assert.Error(t, pool.CheckEvidence(types.EvidenceList{ev})) } func TestVerifyLightClientAttack_Equivocation(t *testing.T) { @@ -417,6 +449,84 @@ func TestVerifyDuplicateVoteEvidence(t *testing.T) { assert.Error(t, err) } +func makeLunaticEvidence( + t *testing.T, + height, commonHeight int64, + totalVals, byzVals, phantomVals int, + commonTime, attackTime time.Time, +) (ev *types.LightClientAttackEvidence, trusted *types.LightBlock, common *types.LightBlock) { + commonValSet, commonPrivVals := types.RandValidatorSet(totalVals, defaultVotingPower) + + require.Greater(t, totalVals, byzVals) + + // extract out the subset of byzantine validators in the common validator set + byzValSet, byzPrivVals := commonValSet.Validators[:byzVals], commonPrivVals[:byzVals] + + phantomValSet, phantomPrivVals := types.RandValidatorSet(phantomVals, defaultVotingPower) + + conflictingVals := phantomValSet.Copy() + require.NoError(t, conflictingVals.UpdateWithChangeSet(byzValSet)) + conflictingPrivVals := append(phantomPrivVals, byzPrivVals...) + + conflictingPrivVals = orderPrivValsByValSet(t, conflictingVals, conflictingPrivVals) + + commonHeader := makeHeaderRandom(commonHeight) + commonHeader.Time = commonTime + trustedHeader := makeHeaderRandom(height) + + conflictingHeader := makeHeaderRandom(height) + conflictingHeader.Time = attackTime + conflictingHeader.ValidatorsHash = conflictingVals.Hash() + + blockID := makeBlockID(conflictingHeader.Hash(), 1000, []byte("partshash")) + voteSet := types.NewVoteSet(evidenceChainID, height, 1, tmproto.SignedMsgType(2), conflictingVals) + commit, err := types.MakeCommit(blockID, height, 1, voteSet, conflictingPrivVals, defaultEvidenceTime) + require.NoError(t, err) + ev = &types.LightClientAttackEvidence{ + ConflictingBlock: &types.LightBlock{ + SignedHeader: &types.SignedHeader{ + Header: conflictingHeader, + Commit: commit, + }, + ValidatorSet: conflictingVals, + }, + CommonHeight: commonHeight, + TotalVotingPower: commonValSet.TotalVotingPower(), + ByzantineValidators: byzValSet, + Timestamp: commonTime, + } + + common = &types.LightBlock{ + SignedHeader: &types.SignedHeader{ + Header: commonHeader, + // we can leave this empty because we shouldn't be checking this + Commit: &types.Commit{}, + }, + ValidatorSet: commonValSet, + } + trustedBlockID := makeBlockID(trustedHeader.Hash(), 1000, []byte("partshash")) + trustedVals, privVals := types.RandValidatorSet(totalVals, defaultVotingPower) + trustedVoteSet := types.NewVoteSet(evidenceChainID, height, 1, tmproto.SignedMsgType(2), trustedVals) + trustedCommit, err := types.MakeCommit(trustedBlockID, height, 1, trustedVoteSet, privVals, defaultEvidenceTime) + require.NoError(t, err) + trusted = &types.LightBlock{ + SignedHeader: &types.SignedHeader{ + Header: trustedHeader, + Commit: trustedCommit, + }, + ValidatorSet: trustedVals, + } + return ev, trusted, common +} + +// func makeEquivocationEvidence() *types.LightClientAttackEvidence { + +// } + +// func makeAmnesiaEvidence() *types.LightClientAttackEvidence { + +// } + func makeVote( t *testing.T, val types.PrivValidator, chainID string, valIndex int32, height int64, round int32, step int, blockID types.BlockID, time time.Time) *types.Vote { @@ -475,3 +585,20 @@ func makeBlockID(hash []byte, partSetSize uint32, partSetHash []byte) types.Bloc }, } } + +func orderPrivValsByValSet( + t *testing.T, vals *types.ValidatorSet, privVals []types.PrivValidator) []types.PrivValidator { + output := make([]types.PrivValidator, len(privVals)) + for idx, v := range vals.Validators { + for _, p := range privVals { + pubKey, err := p.GetPubKey() + require.NoError(t, err) + if bytes.Equal(v.Address, pubKey.Address()) { + output[idx] = p + break + } + } + require.NotEmpty(t, output[idx]) + } + return output +} diff --git a/test/e2e/tests/block_test.go b/test/e2e/tests/block_test.go index 369b49d61..b3f4e9139 100644 --- a/test/e2e/tests/block_test.go +++ b/test/e2e/tests/block_test.go @@ -37,8 +37,12 @@ func TestBlock_Header(t *testing.T) { } resp, err := client.Block(ctx, &block.Header.Height) require.NoError(t, err) + require.Equal(t, block, resp.Block, - "block mismatch for height %v", block.Header.Height) + "block mismatch for height %d", block.Header.Height) + + require.NoError(t, resp.Block.ValidateBasic(), + "block at height %d is invalid", block.Header.Height) } }) } diff --git a/types/evidence.go b/types/evidence.go index 8007763b7..35acaaed1 100644 --- a/types/evidence.go +++ b/types/evidence.go @@ -295,7 +295,10 @@ func (l *LightClientAttackEvidence) ConflictingHeaderIsInvalid(trustedHeader *He // with evidence that have the same conflicting header and common height but different permutations // of validator commit signatures. The reason for this is that we don't want to allow several // permutations of the same evidence to be committed on chain. Ideally we commit the header with the -// most commit signatures (captures the most byzantine validators) but anything greater than 1/3 is sufficient. +// most commit signatures (captures the most byzantine validators) but anything greater than 1/3 is +// sufficient. +// TODO: We should change the hash to include the commit, header, total voting power, byzantine +// validators and timestamp func (l *LightClientAttackEvidence) Hash() []byte { buf := make([]byte, binary.MaxVarintLen64) n := binary.PutVarint(buf, l.CommonHeight) @@ -314,8 +317,14 @@ func (l *LightClientAttackEvidence) Height() int64 { // String returns a string representation of LightClientAttackEvidence func (l *LightClientAttackEvidence) String() string { - return fmt.Sprintf("LightClientAttackEvidence{ConflictingBlock: %v, CommonHeight: %d}", - l.ConflictingBlock.String(), l.CommonHeight) + return fmt.Sprintf(`LightClientAttackEvidence{ + ConflictingBlock: %v, + CommonHeight: %d, + ByzatineValidators: %v, + TotalVotingPower: %d, + Timestamp: %v}#%X`, + l.ConflictingBlock.String(), l.CommonHeight, l.ByzantineValidators, + l.TotalVotingPower, l.Timestamp, l.Hash()) } // Time returns the time of the common block where the infraction leveraged off. @@ -334,8 +343,8 @@ func (l *LightClientAttackEvidence) ValidateBasic() error { return errors.New("conflicting block missing header") } - if err := l.ConflictingBlock.ValidateBasic(l.ConflictingBlock.ChainID); err != nil { - return fmt.Errorf("invalid conflicting light block: %w", err) + if l.TotalVotingPower <= 0 { + return errors.New("negative or zero total voting power") } if l.CommonHeight <= 0 { @@ -350,6 +359,10 @@ func (l *LightClientAttackEvidence) ValidateBasic() error { l.CommonHeight, l.ConflictingBlock.Height) } + if err := l.ConflictingBlock.ValidateBasic(l.ConflictingBlock.ChainID); err != nil { + return fmt.Errorf("invalid conflicting light block: %w", err) + } + return nil } @@ -421,6 +434,8 @@ func (evl EvidenceList) Hash() []byte { // the Evidence size is capped. evidenceBzs := make([][]byte, len(evl)) for i := 0; i < len(evl); i++ { + // TODO: We should change this to the hash. Using bytes contains some unexported data that + // may cause different hashes evidenceBzs[i] = evl[i].Bytes() } return merkle.HashFromByteSlices(evidenceBzs) diff --git a/types/evidence_test.go b/types/evidence_test.go index 2e61f6a9c..946373aad 100644 --- a/types/evidence_test.go +++ b/types/evidence_test.go @@ -89,9 +89,11 @@ func TestDuplicateVoteEvidenceValidation(t *testing.T) { } } -func TestLightClientAttackEvidence(t *testing.T) { +func TestLightClientAttackEvidenceBasic(t *testing.T) { height := int64(5) - voteSet, valSet, privVals := randVoteSet(height, 1, tmproto.PrecommitType, 10, 1) + commonHeight := height - 1 + nValidators := 10 + voteSet, valSet, privVals := randVoteSet(height, 1, tmproto.PrecommitType, nValidators, 1) header := makeHeaderRandom() header.Height = height blockID := makeBlockID(tmhash.Sum([]byte("blockhash")), math.MaxInt32, tmhash.Sum([]byte("partshash"))) @@ -105,56 +107,52 @@ func TestLightClientAttackEvidence(t *testing.T) { }, ValidatorSet: valSet, }, - CommonHeight: height - 1, + CommonHeight: commonHeight, + TotalVotingPower: valSet.TotalVotingPower(), + Timestamp: header.Time, + ByzantineValidators: valSet.Validators[:nValidators/2], } assert.NotNil(t, lcae.String()) assert.NotNil(t, lcae.Hash()) - // only 7 validators sign - differentCommit, err := MakeCommit(blockID, height, 1, voteSet, privVals[:7], defaultVoteTime) - require.NoError(t, err) - differentEv := &LightClientAttackEvidence{ - ConflictingBlock: &LightBlock{ - SignedHeader: &SignedHeader{ - Header: header, - Commit: differentCommit, - }, - ValidatorSet: valSet, - }, - CommonHeight: height - 1, - } - assert.Equal(t, lcae.Hash(), differentEv.Hash()) - // different header hash - differentHeader := makeHeaderRandom() - differentEv = &LightClientAttackEvidence{ - ConflictingBlock: &LightBlock{ - SignedHeader: &SignedHeader{ - Header: differentHeader, - Commit: differentCommit, - }, - ValidatorSet: valSet, - }, - CommonHeight: height - 1, + assert.Equal(t, lcae.Height(), commonHeight) // Height should be the common Height + assert.NotNil(t, lcae.Bytes()) + + // maleate evidence to test hash uniqueness + testCases := []struct { + testName string + malleateEvidence func(*LightClientAttackEvidence) + }{ + {"Different header", func(ev *LightClientAttackEvidence) { ev.ConflictingBlock.Header = makeHeaderRandom() }}, + {"Different common height", func(ev *LightClientAttackEvidence) { + ev.CommonHeight = height + 1 + }}, } - assert.NotEqual(t, lcae.Hash(), differentEv.Hash()) - // different common height should produce a different header - differentEv = &LightClientAttackEvidence{ - ConflictingBlock: &LightBlock{ - SignedHeader: &SignedHeader{ - Header: header, - Commit: differentCommit, + + for _, tc := range testCases { + lcae := &LightClientAttackEvidence{ + ConflictingBlock: &LightBlock{ + SignedHeader: &SignedHeader{ + Header: header, + Commit: commit, + }, + ValidatorSet: valSet, }, - ValidatorSet: valSet, - }, - CommonHeight: height - 2, + CommonHeight: commonHeight, + TotalVotingPower: valSet.TotalVotingPower(), + Timestamp: header.Time, + ByzantineValidators: valSet.Validators[:nValidators/2], + } + hash := lcae.Hash() + tc.malleateEvidence(lcae) + assert.NotEqual(t, hash, lcae.Hash(), tc.testName) } - assert.NotEqual(t, lcae.Hash(), differentEv.Hash()) - assert.Equal(t, lcae.Height(), int64(4)) // Height should be the common Height - assert.NotNil(t, lcae.Bytes()) } func TestLightClientAttackEvidenceValidation(t *testing.T) { height := int64(5) - voteSet, valSet, privVals := randVoteSet(height, 1, tmproto.PrecommitType, 10, 1) + commonHeight := height - 1 + nValidators := 10 + voteSet, valSet, privVals := randVoteSet(height, 1, tmproto.PrecommitType, nValidators, 1) header := makeHeaderRandom() header.Height = height header.ValidatorsHash = valSet.Hash() @@ -169,7 +167,10 @@ func TestLightClientAttackEvidenceValidation(t *testing.T) { }, ValidatorSet: valSet, }, - CommonHeight: height - 1, + CommonHeight: commonHeight, + TotalVotingPower: valSet.TotalVotingPower(), + Timestamp: header.Time, + ByzantineValidators: valSet.Validators[:nValidators/2], } assert.NoError(t, lcae.ValidateBasic()) @@ -178,16 +179,22 @@ func TestLightClientAttackEvidenceValidation(t *testing.T) { malleateEvidence func(*LightClientAttackEvidence) expectErr bool }{ - {"Good DuplicateVoteEvidence", func(ev *LightClientAttackEvidence) {}, false}, + {"Good LightClientAttackEvidence", func(ev *LightClientAttackEvidence) {}, false}, {"Negative height", func(ev *LightClientAttackEvidence) { ev.CommonHeight = -10 }, true}, {"Height is greater than divergent block", func(ev *LightClientAttackEvidence) { ev.CommonHeight = height + 1 }, true}, + {"Height is equal to the divergent block", func(ev *LightClientAttackEvidence) { + ev.CommonHeight = height + }, false}, {"Nil conflicting header", func(ev *LightClientAttackEvidence) { ev.ConflictingBlock.Header = nil }, true}, {"Nil conflicting blocl", func(ev *LightClientAttackEvidence) { ev.ConflictingBlock = nil }, true}, {"Nil validator set", func(ev *LightClientAttackEvidence) { ev.ConflictingBlock.ValidatorSet = &ValidatorSet{} }, true}, + {"Negative total voting power", func(ev *LightClientAttackEvidence) { + ev.TotalVotingPower = -1 + }, true}, } for _, tc := range testCases { tc := tc @@ -200,7 +207,10 @@ func TestLightClientAttackEvidenceValidation(t *testing.T) { }, ValidatorSet: valSet, }, - CommonHeight: height - 1, + CommonHeight: commonHeight, + TotalVotingPower: valSet.TotalVotingPower(), + Timestamp: header.Time, + ByzantineValidators: valSet.Validators[:nValidators/2], } tc.malleateEvidence(lcae) if tc.expectErr { diff --git a/types/light.go b/types/light.go index 8f09d8205..5a650a159 100644 --- a/types/light.go +++ b/types/light.go @@ -149,7 +149,7 @@ func (sh SignedHeader) ValidateBasic(chainID string) error { if sh.Commit.Height != sh.Height { return fmt.Errorf("header and commit height mismatch: %d vs %d", sh.Height, sh.Commit.Height) } - if hhash, chash := sh.Hash(), sh.Commit.BlockID.Hash; !bytes.Equal(hhash, chash) { + if hhash, chash := sh.Header.Hash(), sh.Commit.BlockID.Hash; !bytes.Equal(hhash, chash) { return fmt.Errorf("commit signs block %X, header is block %X", chash, hhash) } diff --git a/types/light_test.go b/types/light_test.go index fa04cd4cf..68c065cde 100644 --- a/types/light_test.go +++ b/types/light_test.go @@ -153,11 +153,13 @@ func TestSignedHeaderValidateBasic(t *testing.T) { Header: tc.shHeader, Commit: tc.shCommit, } - assert.Equal( + err := sh.ValidateBasic(validSignedHeader.Header.ChainID) + assert.Equalf( t, tc.expectErr, - sh.ValidateBasic(validSignedHeader.Header.ChainID) != nil, + err != nil, "Validate Basic had an unexpected result", + err, ) }) } diff --git a/types/validator_set.go b/types/validator_set.go index c808adb0c..5b2ec85a5 100644 --- a/types/validator_set.go +++ b/types/validator_set.go @@ -946,7 +946,9 @@ func (vals *ValidatorSet) ToProto() (*tmproto.ValidatorSet, error) { } vp.Proposer = valProposer - vp.TotalVotingPower = vals.totalVotingPower + // NOTE: Sometimes we use the bytes of the proto form as a hash. This means that we need to + // be consistent with cached data + vp.TotalVotingPower = 0 return vp, nil } @@ -977,7 +979,12 @@ func ValidatorSetFromProto(vp *tmproto.ValidatorSet) (*ValidatorSet, error) { vals.Proposer = p - vals.totalVotingPower = vp.GetTotalVotingPower() + // NOTE: We can't trust the total voting power given to us by other peers. If someone were to + // inject a non-zeo value that wasn't the correct voting power we could assume a wrong total + // power hence we need to recompute it. + // FIXME: We should look to remove TotalVotingPower from proto or add it in the validators hash + // so we don't have to do this + vals.TotalVotingPower() return vals, vals.ValidateBasic() }