Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 4 commits into from
Aug 12, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions internal/evidence/reactor.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,18 @@ 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.state.Validators.HasPublicKeys {
// silently drop the message
logger.Debug("dropping evidence message as we are not a validator", "evidence", envelope.Message)
lklimek marked this conversation as resolved.
Show resolved Hide resolved
}

// Process the evidence received from a peer
// Evidence is sent and received one by one
ev, err := types.EvidenceFromProto(msg)
Expand Down
29 changes: 17 additions & 12 deletions internal/evidence/verify.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"context"
"fmt"

"github.com/dashpay/tenderdash/libs/log"
"github.com/dashpay/tenderdash/types"
)

Expand Down Expand Up @@ -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)
}

Expand All @@ -90,15 +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)
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 {
if valSet.HasPublicKeys && pubKey == nil {
return fmt.Errorf("we don't have a public key of validator %X at height %d", proTxHash, e.Height())
}

Expand Down Expand Up @@ -134,14 +135,18 @@ 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)
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 {
logger.Debug("skipping verification of duplicate vote evidence signatures - no access public key", "evidence", e)
}

return nil
Expand Down
4 changes: 2 additions & 2 deletions internal/evidence/verify_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
}

Expand Down
Loading