diff --git a/evidence/pool.go b/evidence/pool.go index bc09ea94e..0243472ec 100644 --- a/evidence/pool.go +++ b/evidence/pool.go @@ -21,12 +21,6 @@ import ( "github.com/tendermint/tendermint/types" ) -const ( - // prefixes are unique across all tm db's - prefixCommitted = int64(9) - prefixPending = int64(10) -) - // Pool maintains a pool of valid evidence to be broadcasted and committed type Pool struct { logger log.Logger @@ -152,6 +146,11 @@ func (evpool *Pool) AddEvidence(ev types.Evidence) error { return nil } + if evpool.checkForSimilarLightClientAttackEvidence(ev) { + evpool.logger.Debug("similar light client evidence already pending; ignoring", "evidence", ev) + return nil + } + // check that the evidence isn't already committed if evpool.isCommitted(ev) { // This can happen if the peer that sent us the evidence is behind so we @@ -203,7 +202,7 @@ func (evpool *Pool) CheckEvidence(evList types.EvidenceList) error { hashes := make([][]byte, len(evList)) for idx, ev := range evList { - ok := evpool.fastCheck(ev) + ok := evpool.isPending(ev) if !ok { // check that the evidence isn't already committed @@ -382,6 +381,15 @@ func (evpool *Pool) addPendingEvidence(ev types.Evidence) error { return fmt.Errorf("failed to persist evidence: %w", err) } + // if it is light client attack evidence, adds a secondary key to avoid + // submission of multiple variants + if lcae, ok := ev.(*types.LightClientAttackEvidence); ok { + if err := evpool.evidenceStore.Set(keyLightEvidence(lcae), keyPending(ev)); err != nil { + return fmt.Errorf("failed to persist secondary key for light evidence: %w", + err) + } + } + atomic.AddUint32(&evpool.evidenceSize, 1) return nil } @@ -396,7 +404,12 @@ func (evpool *Pool) markEvidenceAsCommitted(evidence types.EvidenceList, height for _, ev := range evidence { if evpool.isPending(ev) { if err := batch.Delete(keyPending(ev)); err != nil { - evpool.logger.Error("failed to batch pending evidence", "err", err) + evpool.logger.Error("failed to batch delete pending evidence", "err", err) + } + if lcae, ok := ev.(*types.LightClientAttackEvidence); ok { + if err := batch.Delete(keyLightEvidence(lcae)); err != nil { + evpool.logger.Error("failed to batch delete pending evidence", "err", err) + } } blockEvidenceMap[evMapKey(ev)] = struct{}{} } @@ -546,10 +559,18 @@ func (evpool *Pool) batchExpiredPendingEvidence(batch dbm.Batch) (int64, time.Ti // else add to the batch if err := batch.Delete(iter.Key()); err != nil { - evpool.logger.Error("failed to batch evidence", "err", err, "ev", ev) + evpool.logger.Error("failed to batch delete evidence", "err", err, "ev", ev) continue } + // if it is light client attack evidence then delete the secondary index + if lcae, ok := ev.(*types.LightClientAttackEvidence); ok { + if err := batch.Delete(keyLightEvidence(lcae)); err != nil { + evpool.logger.Error("failed to batch delete evidence", "err", err, "ev", ev) + continue + } + } + // and add to the map to remove the evidence from the clist blockEvidenceMap[evMapKey(ev)] = struct{}{} } @@ -651,6 +672,17 @@ func (evpool *Pool) processConsensusBuffer(state sm.State) { evpool.consensusBuffer = make([]duplicateVoteSet, 0) } +func (evpool *Pool) checkForSimilarLightClientAttackEvidence(ev types.Evidence) bool { + if lcae, ok := ev.(*types.LightClientAttackEvidence); ok { + ok, err := evpool.evidenceStore.Has(keyLightEvidence(lcae)) + if err != nil { + evpool.logger.Error("failed to find pending evidence", "err", err) + } + return ok + } + return false +} + type duplicateVoteSet struct { VoteA *types.Vote VoteB *types.Vote @@ -670,6 +702,24 @@ func evMapKey(ev types.Evidence) string { return string(ev.Hash()) } +// ########################### KEYS ############################## + +const ( + // prefixes are unique across all tm db's + prefixCommitted = int64(9) + prefixPending = int64(10) + + // It is very easy to manipulate LightClientAttackEvidence to + // form multiple valid versions of that evidence that all have + // different hashes i.e. change the common height or remove a + // commit. This is a potential DOS vector as this evidence is + // relatively large and a malicious node could freely fill + // blocks with it. To prevent this nodes won't verify a new + // LightClientAttackEvidence that has the same conflicting + // header + prefixLightEvidence = int64(13) +) + func prefixToBytes(prefix int64) []byte { key, err := orderedcode.Append(nil, prefix) if err != nil { @@ -680,7 +730,15 @@ func prefixToBytes(prefix int64) []byte { func keyCommitted(evidence types.Evidence) []byte { var height int64 = evidence.Height() - key, err := orderedcode.Append(nil, prefixCommitted, height, string(evidence.Hash())) + var hash string + // if it is light client attack evidence we add the header hash key to avoid + // submission of multiple variants + if lcae, ok := evidence.(*types.LightClientAttackEvidence); ok { + hash = string(lcae.ConflictingBlock.Header.Hash()) + } else { + hash = string(evidence.Hash()) + } + key, err := orderedcode.Append(nil, prefixCommitted, height, hash) if err != nil { panic(err) } @@ -695,3 +753,12 @@ func keyPending(evidence types.Evidence) []byte { } return key } + +func keyLightEvidence(evidence *types.LightClientAttackEvidence) []byte { + key, err := orderedcode.Append(nil, prefixLightEvidence, + string(evidence.ConflictingBlock.Header.Hash())) + if err != nil { + panic(err) + } + return key +} diff --git a/test/e2e/networks/simple.toml b/test/e2e/networks/simple.toml index 05cda1819..355eda960 100644 --- a/test/e2e/networks/simple.toml +++ b/test/e2e/networks/simple.toml @@ -1,3 +1,5 @@ +evidence = 5 + [node.validator01] [node.validator02] [node.validator03] diff --git a/test/e2e/runner/evidence.go b/test/e2e/runner/evidence.go index ece241cd2..c99bd8ab0 100644 --- a/test/e2e/runner/evidence.go +++ b/test/e2e/runner/evidence.go @@ -20,10 +20,7 @@ import ( ) // 1 in 11 evidence is light client evidence, the rest is duplicate vote -// FIXME: Setting to 11 disables light client attack evidence since nodes -// don't follow a minimum retention height invariant. When we fix this we -// should use a ratio of 4. -const lightClientEvidenceRatio = 11 +const lightClientEvidenceRatio = 4 // InjectEvidence takes a running testnet and generates an amount of valid // evidence and broadcasts it to a random node through the rpc endpoint `/broadcast_evidence`. @@ -74,7 +71,7 @@ func InjectEvidence(testnet *e2e.Testnet, amount int) error { duplicateVoteTime := status.SyncInfo.LatestBlockTime var ev types.Evidence - for i := 0; i < amount; i++ { + for i := 1; i <= amount; i++ { if i%lightClientEvidenceRatio == 0 { ev, err = generateLightClientAttackEvidence( privVals, lightEvidenceCommonHeight, valSet, testnet.Name, blockRes.Block.Time, diff --git a/test/e2e/tests/block_test.go b/test/e2e/tests/block_test.go index 369b49d61..615efacfa 100644 --- a/test/e2e/tests/block_test.go +++ b/test/e2e/tests/block_test.go @@ -38,7 +38,10 @@ 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 8e3c7decc..7284018c2 100644 --- a/types/evidence.go +++ b/types/evidence.go @@ -292,18 +292,20 @@ func (l *LightClientAttackEvidence) ConflictingHeaderIsInvalid(trustedHeader *He } -// Hash returns the hash of the header and the commonHeight. This is designed to cause hash collisions -// 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. +// Hash returns the SHA256 hash of the header, commit, common height, total +// voting power, byzantine validators and timestamp func (l *LightClientAttackEvidence) Hash() []byte { - buf := make([]byte, binary.MaxVarintLen64) - n := binary.PutVarint(buf, l.CommonHeight) - bz := make([]byte, tmhash.Size+n) - copy(bz[:tmhash.Size-1], l.ConflictingBlock.Hash().Bytes()) - copy(bz[tmhash.Size:], buf) - return tmhash.Sum(bz) + buf := new(bytes.Buffer) + buf.Write(l.ConflictingBlock.Header.Hash()) + buf.Write(l.ConflictingBlock.Commit.Hash()) + _ = binary.Write(buf, binary.LittleEndian, l.CommonHeight) + _ = binary.Write(buf, binary.LittleEndian, l.TotalVotingPower) + for _, val := range l.ByzantineValidators { + _, _ = buf.Write(val.Bytes()) + } + timeBytes, _ := l.Timestamp.MarshalBinary() + buf.Write(timeBytes) + return tmhash.Sum(buf.Bytes()) } // Height returns the last height at which the primary provider and witness provider had the same header. @@ -422,7 +424,7 @@ func (evl EvidenceList) Hash() []byte { // the Evidence size is capped. evidenceBzs := make([][]byte, len(evl)) for i := 0; i < len(evl); i++ { - evidenceBzs[i] = evl[i].Bytes() + evidenceBzs[i] = evl[i].Hash() } return merkle.HashFromByteSlices(evidenceBzs) } diff --git a/types/light.go b/types/light.go index 8f09d8205..30bed1386 100644 --- a/types/light.go +++ b/types/light.go @@ -5,6 +5,7 @@ import ( "errors" "fmt" + "github.com/tendermint/tendermint/crypto/tmhash" tmproto "github.com/tendermint/tendermint/proto/tendermint/types" ) @@ -149,13 +150,21 @@ 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) } return nil } +// Hash returns the SHA256 hash of both the header and the commit that signed that header +func (sh SignedHeader) Hash() []byte { + bz := make([]byte, tmhash.Size*2) + copy(bz[:tmhash.Size-1], sh.Header.Hash()) + copy(bz[tmhash.Size:], sh.Commit.Hash()) + return tmhash.Sum(bz) +} + // String returns a string representation of SignedHeader. func (sh SignedHeader) String() string { return sh.StringIndented("")