Skip to content

Commit

Permalink
evidence: fix bug with hashes (backport #6375) (#6381)
Browse files Browse the repository at this point in the history
  • Loading branch information
@mergify
mergify[bot] authored Apr 22, 2021
1 parent a9b4fac commit 353e3a3
Show file tree
Hide file tree
Showing 11 changed files with 418 additions and 316 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG_PENDING.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
71 changes: 5 additions & 66 deletions evidence/pool.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"bytes"
"errors"
"fmt"
"sort"
"sync"
"sync/atomic"
"time"
Expand Down Expand Up @@ -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")}
Expand All @@ -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.
Expand Down Expand Up @@ -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 {
Expand Down
96 changes: 42 additions & 54 deletions evidence/pool_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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)
Expand All @@ -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
Expand Down Expand Up @@ -345,7 +333,7 @@ func TestRecoverPendingEvidence(t *testing.T) {
Evidence: tmproto.EvidenceParams{
MaxAgeNumBlocks: 20,
MaxAgeDuration: 20 * time.Minute,
MaxBytes: 1000,
MaxBytes: defaultEvidenceMaxBytes,
},
},
}, nil)
Expand Down
94 changes: 51 additions & 43 deletions evidence/verify.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"bytes"
"errors"
"fmt"
"sort"
"time"

"github.com/tendermint/tendermint/light"
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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")
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand All @@ -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)
}
Loading

0 comments on commit 353e3a3

Please sign in to comment.