fix: non-active validators can't verify evidence signatures (#865)

* fix: don't verify evidence signatures on non-validators

* fix: don't accept evidence on non-validator hosts

* chore: really drop msg

internal/evidence/pool.go

@@ -544,6 +544,13 @@ func (evpool *Pool) updateState(state sm.State) {
 	evpool.state = state
 }
 
+// hasPublicKeys returns true if we have public keys of the current validator set.
+func (evpool *Pool) hasPublicKeys() bool {
+	evpool.mtx.Lock()
+	defer evpool.mtx.Unlock()
+	return evpool.state.Validators.HasPublicKeys
+}
+
 // processConsensusBuffer converts all the duplicate votes witnessed from consensus
 // into DuplicateVoteEvidence. It sets the evidence timestamp to the block height
 // from the most recently committed block.

internal/evidence/pool_test.go

@@ -528,6 +528,7 @@ func initializeValidatorState(
 		ThresholdPublicKey: validator.PubKey,
 		QuorumType:         quorumType,
 		QuorumHash:         quorumHash,
+		HasPublicKeys:      true,
 	}
 
 	return initializeStateFromValidatorSet(t, valSet, height)

internal/evidence/reactor.go

@@ -101,6 +101,19 @@ func (r *Reactor) handleEvidenceMessage(ctx context.Context, envelope *p2p.Envel
 
 	switch msg := envelope.Message.(type) {
 	case *tmproto.Evidence:
+
+		// Only accept evidence if we are an active validator.
+		// On other hosts, signatures in evidence (if any) cannot be verified due to lack of validator public keys,
+		// and it creates risk of adding invalid evidence to the pool.
+		//
+		// TODO: We need to figure out how to handle evidence from non-validator nodes, to avoid scenarios where some
+		// evidence is lost.
+		if !r.evpool.hasPublicKeys() {
+			// silently drop the message
+			logger.Debug("dropping evidence message as we don't have validator public keys", "evidence", envelope.Message)
+			return nil
+		}
+
 		// Process the evidence received from a peer
 		// Evidence is sent and received one by one
 		ev, err := types.EvidenceFromProto(msg)

internal/evidence/verify.go

@@ -5,6 +5,7 @@ import (
 	"context"
 	"fmt"
 
+	"github.com/dashpay/tenderdash/libs/log"
 	"github.com/dashpay/tenderdash/types"
 )
 
@@ -64,7 +65,7 @@ func (evpool *Pool) verify(ctx context.Context, evidence types.Evidence) error {
 			return err
 		}
 
-		if err := VerifyDuplicateVote(ev, state.ChainID, valSet); err != nil {
+		if err := VerifyDuplicateVote(ev, state.ChainID, valSet, evpool.logger); err != nil {
 			return types.NewErrInvalidEvidence(evidence, err)
 		}
 
@@ -90,17 +91,15 @@ func (evpool *Pool) verify(ctx context.Context, evidence types.Evidence) error {
 //   - the validator is in the validator set at the height of the evidence
 //   - the height, round, type and validator address of the votes must be the same
 //   - the block ID's must be different
-//   - The signatures must both be valid
-func VerifyDuplicateVote(e *types.DuplicateVoteEvidence, chainID string, valSet *types.ValidatorSet) error {
+//   - The signatures must both be valid (only if we have public keys of the validator set)
+//
+// TODO: Double-check that this will still work for quite old heights, where validator set has already been rotated.
+func VerifyDuplicateVote(e *types.DuplicateVoteEvidence, chainID string, valSet *types.ValidatorSet, logger log.Logger) error {
 	_, val := valSet.GetByProTxHash(e.VoteA.ValidatorProTxHash)
 	if val == nil {
 		return fmt.Errorf("protxhash %X was not a validator at height %d", e.VoteA.ValidatorProTxHash, e.Height())
 	}
 	proTxHash := val.ProTxHash
-	pubKey := val.PubKey
-	if pubKey == nil {
-		return fmt.Errorf("we don't have a public key of validator %X at height %d", proTxHash, e.Height())
-	}
 
 	// H/R/S must be the same
 	if e.VoteA.Height != e.VoteB.Height ||
@@ -134,14 +133,22 @@ func VerifyDuplicateVote(e *types.DuplicateVoteEvidence, chainID string, valSet
 			voteProTxHash, proTxHash)
 	}
 
-	va := e.VoteA.ToProto()
-	vb := e.VoteB.ToProto()
-	// Signatures must be valid
-	if !pubKey.VerifySignatureDigest(types.VoteBlockSignID(chainID, va, valSet.QuorumType, valSet.QuorumHash), e.VoteA.BlockSignature) {
-		return fmt.Errorf("verifying VoteA: %w", types.ErrVoteInvalidBlockSignature)
-	}
-	if !pubKey.VerifySignatureDigest(types.VoteBlockSignID(chainID, vb, valSet.QuorumType, valSet.QuorumHash), e.VoteB.BlockSignature) {
-		return fmt.Errorf("verifying VoteB: %w", types.ErrVoteInvalidBlockSignature)
+	pubKey := val.PubKey
+
+	if pubKey != nil {
+		va := e.VoteA.ToProto()
+		vb := e.VoteB.ToProto()
+		// Signatures must be valid
+		if !pubKey.VerifySignatureDigest(types.VoteBlockSignID(chainID, va, valSet.QuorumType, valSet.QuorumHash), e.VoteA.BlockSignature) {
+			return fmt.Errorf("verifying VoteA: %w", types.ErrVoteInvalidBlockSignature)
+		}
+		if !pubKey.VerifySignatureDigest(types.VoteBlockSignID(chainID, vb, valSet.QuorumType, valSet.QuorumHash), e.VoteB.BlockSignature) {
+			return fmt.Errorf("verifying VoteB: %w", types.ErrVoteInvalidBlockSignature)
+		}
+	} else if valSet.HasPublicKeys {
+		return fmt.Errorf("we don't have a public key of validator %X at height %d", proTxHash, e.Height())
+	} else {
+		logger.Debug("skipping verification of duplicate vote evidence signatures - no access public key", "evidence", e)
 	}
 
 	return nil

internal/evidence/verify_test.go

@@ -84,9 +84,9 @@ func TestVerifyDuplicateVoteEvidence(t *testing.T) {
 			Timestamp:        defaultEvidenceTime,
 		}
 		if c.valid {
-			assert.Nil(t, evidence.VerifyDuplicateVote(ev, chainID, valSet), "evidence should be valid")
+			assert.Nil(t, evidence.VerifyDuplicateVote(ev, chainID, valSet, logger), "evidence should be valid")
 		} else {
-			assert.NotNil(t, evidence.VerifyDuplicateVote(ev, chainID, valSet), "evidence should be invalid")
+			assert.NotNil(t, evidence.VerifyDuplicateVote(ev, chainID, valSet, logger), "evidence should be invalid")
 		}
 	}