From 72ee5aab26e1d89819d1c06abff7c92593d7ab4c Mon Sep 17 00:00:00 2001 From: Callum Waters Date: Tue, 25 May 2021 16:53:14 +0200 Subject: [PATCH] evidence: separate abci specific validation (#6473) --- evidence/verify.go | 99 +++++++++-------------------------- evidence/verify_test.go | 3 +- light/detector.go | 2 +- types/evidence.go | 112 +++++++++++++++++++++++++++++++++++++++- 4 files changed, 136 insertions(+), 80 deletions(-) diff --git a/evidence/verify.go b/evidence/verify.go index f8142add2..99c8e28e8 100644 --- a/evidence/verify.go +++ b/evidence/verify.go @@ -42,16 +42,6 @@ func (evpool *Pool) verify(evidence types.Evidence) error { // verify the time of the evidence evTime := blockMeta.Header.Time - if evidence.Time() != evTime { - return types.NewErrInvalidEvidence( - evidence, - fmt.Errorf( - "evidence has a different time to the block it is associated with (%v != %v)", - evidence.Time(), evTime, - ), - ) - } - ageDuration := state.LastBlockTime.Sub(evTime) // check that the evidence hasn't expired @@ -80,6 +70,16 @@ func (evpool *Pool) verify(evidence types.Evidence) error { return types.NewErrInvalidEvidence(evidence, err) } + _, val := valSet.GetByAddress(ev.VoteA.ValidatorAddress) + + if err := ev.ValidateABCI(val, valSet, evTime); err != nil { + ev.GenerateABCI(val, valSet, evTime) + if addErr := evpool.addPendingEvidence(ev); addErr != nil { + evpool.logger.Error("adding pending duplicate vote evidence failed", "err", addErr) + } + return err + } + return nil case *types.LightClientAttackEvidence: @@ -128,6 +128,18 @@ func (evpool *Pool) verify(evidence types.Evidence) error { if err != nil { return types.NewErrInvalidEvidence(evidence, err) } + + // validate the ABCI component of evidence. If this fails but the rest + // is valid then we regenerate the ABCI component, save the rectified + // evidence and return an error + if err := ev.ValidateABCI(commonVals, trustedHeader, evTime); err != nil { + ev.GenerateABCI(commonVals, trustedHeader, evTime) + if addErr := evpool.addPendingEvidence(ev); addErr != nil { + evpool.logger.Error("adding pending light client attack evidence failed", "err", addErr) + } + return err + + } return nil default: @@ -166,12 +178,6 @@ func VerifyLightClientAttack(e *types.LightClientAttackEvidence, commonHeader, t return fmt.Errorf("invalid commit from conflicting block: %w", err) } - // Assert the correct amount of voting power of the validator set - if evTotal, valsTotal := e.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) - } - // check in the case of a forward lunatic attack that monotonically increasing time has been violated if e.ConflictingBlock.Height > trustedHeader.Height && e.ConflictingBlock.Time.After(trustedHeader.Time) { return fmt.Errorf("conflicting block doesn't violate monotonically increasing time (%v is after %v)", @@ -184,7 +190,7 @@ func VerifyLightClientAttack(e *types.LightClientAttackEvidence, commonHeader, t trustedHeader.Hash()) } - return validateABCIEvidence(e, commonVals, trustedHeader) + return nil } // VerifyDuplicateVote verifies DuplicateVoteEvidence against the state of full node. This involves the @@ -232,16 +238,6 @@ func VerifyDuplicateVote(e *types.DuplicateVoteEvidence, chainID string, valSet addr, pubKey, pubKey.Address()) } - // validator voting power and total voting power must match - if val.VotingPower != e.ValidatorPower { - return fmt.Errorf("validator power from evidence and our validator set does not match (%d != %d)", - e.ValidatorPower, val.VotingPower) - } - if valSet.TotalVotingPower() != e.TotalVotingPower { - return fmt.Errorf("total voting power from the evidence and our validator set does not match (%d != %d)", - e.TotalVotingPower, valSet.TotalVotingPower()) - } - va := e.VoteA.ToProto() vb := e.VoteB.ToProto() // Signatures must be valid @@ -255,55 +251,6 @@ 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 { diff --git a/evidence/verify_test.go b/evidence/verify_test.go index dbccb1ced..4a1c0c451 100644 --- a/evidence/verify_test.go +++ b/evidence/verify_test.go @@ -54,7 +54,8 @@ func TestVerifyLightClientAttack_Lunatic(t *testing.T) { 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) + assert.NoError(t, err) + assert.Error(t, ev.ValidateABCI(common.ValidatorSet, trusted.SignedHeader, defaultEvidenceTime)) // evidence without enough malicious votes should fail ev, trusted, common = makeLunaticEvidence( diff --git a/light/detector.go b/light/detector.go index b63763cf2..24972d6d8 100644 --- a/light/detector.go +++ b/light/detector.go @@ -183,7 +183,7 @@ func (c *Client) compareNewHeaderWithWitness(ctx context.Context, errc chan erro return } - if !bytes.Equal(h.Hash(), lightBlock.Hash()) { + if !bytes.Equal(h.Header.Hash(), lightBlock.Header.Hash()) { errc <- errConflictingHeaders{Block: lightBlock, WitnessIndex: witnessIndex} } diff --git a/types/evidence.go b/types/evidence.go index 90d42dec1..113004996 100644 --- a/types/evidence.go +++ b/types/evidence.go @@ -140,6 +140,44 @@ func (dve *DuplicateVoteEvidence) ValidateBasic() error { return nil } +// ValidateABCI validates the ABCI component of the evidence by checking the +// timestamp, validator power and total voting power. +func (dve *DuplicateVoteEvidence) ValidateABCI( + val *Validator, + valSet *ValidatorSet, + evidenceTime time.Time, +) error { + + if dve.Timestamp != evidenceTime { + return fmt.Errorf( + "evidence has a different time to the block it is associated with (%v != %v)", + dve.Timestamp, evidenceTime) + } + + if val.VotingPower != dve.ValidatorPower { + return fmt.Errorf("validator power from evidence and our validator set does not match (%d != %d)", + dve.ValidatorPower, val.VotingPower) + } + if valSet.TotalVotingPower() != dve.TotalVotingPower { + return fmt.Errorf("total voting power from the evidence and our validator set does not match (%d != %d)", + dve.TotalVotingPower, valSet.TotalVotingPower()) + } + + return nil +} + +// GenerateABCI populates the ABCI component of the evidence. This includes the +// validator power, timestamp and total voting power. +func (dve *DuplicateVoteEvidence) GenerateABCI( + val *Validator, + valSet *ValidatorSet, + evidenceTime time.Time, +) { + dve.ValidatorPower = val.VotingPower + dve.TotalVotingPower = valSet.TotalVotingPower() + dve.Timestamp = evidenceTime +} + // ToProto encodes DuplicateVoteEvidence to protobuf func (dve *DuplicateVoteEvidence) ToProto() *tmproto.DuplicateVoteEvidence { voteB := dve.VoteB.ToProto() @@ -258,12 +296,12 @@ func (l *LightClientAttackEvidence) GetByzantineValidators(commonVals *Validator // only need a single loop to find the validators that voted twice. for i := 0; i < len(l.ConflictingBlock.Commit.Signatures); i++ { sigA := l.ConflictingBlock.Commit.Signatures[i] - if sigA.Absent() { + if !sigA.ForBlock() { continue } sigB := trusted.Commit.Signatures[i] - if sigB.Absent() { + if !sigB.ForBlock() { continue } @@ -367,6 +405,76 @@ func (l *LightClientAttackEvidence) ValidateBasic() error { return nil } +// ValidateABCI validates the ABCI component of the evidence by checking the +// timestamp, byzantine validators and total voting power all match. ABCI +// components are validated separately because they can be re generated if +// invalid. +func (l *LightClientAttackEvidence) ValidateABCI( + commonVals *ValidatorSet, + trustedHeader *SignedHeader, + evidenceTime time.Time, +) error { + + if evTotal, valsTotal := l.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) + } + + if l.Timestamp != evidenceTime { + return fmt.Errorf( + "evidence has a different time to the block it is associated with (%v != %v)", + l.Timestamp, evidenceTime) + } + + // 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 := l.GetByzantineValidators(commonVals, trustedHeader) + + // Ensure this matches the validators that are listed in the evidence. They + // should be ordered based on power. + if validators == nil && l.ByzantineValidators != nil { + return fmt.Errorf( + "expected nil validators from an amnesia light client attack but got %d", + len(l.ByzantineValidators), + ) + } + + if exp, got := len(validators), len(l.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(l.ByzantineValidators[idx].Address, val.Address) { + return fmt.Errorf( + "evidence contained an unexpected byzantine validator address; expected: %v, got: %v", + val.Address, l.ByzantineValidators[idx].Address, + ) + } + + if l.ByzantineValidators[idx].VotingPower != val.VotingPower { + return fmt.Errorf( + "evidence contained unexpected byzantine validator power; expected %d, got %d", + val.VotingPower, l.ByzantineValidators[idx].VotingPower, + ) + } + } + + return nil +} + +// GenerateABCI populates the ABCI component of the evidence: the timestamp, +// total voting power and byantine validators +func (l *LightClientAttackEvidence) GenerateABCI( + commonVals *ValidatorSet, + trustedHeader *SignedHeader, + evidenceTime time.Time, +) { + l.Timestamp = evidenceTime + l.TotalVotingPower = commonVals.TotalVotingPower() + l.ByzantineValidators = l.GetByzantineValidators(commonVals, trustedHeader) +} + // ToProto encodes LightClientAttackEvidence to protobuf func (l *LightClientAttackEvidence) ToProto() (*tmproto.LightClientAttackEvidence, error) { conflictingBlock, err := l.ConflictingBlock.ToProto()