From cad8e983e9ea4d3cb8b589afc8231254a39d6583 Mon Sep 17 00:00:00 2001 From: Simon Noetzlin Date: Mon, 13 Nov 2023 15:27:32 +0100 Subject: [PATCH] update byzantine validators extraction --- Dockerfile | 2 +- tests/e2e/main.go | 10 +- tests/integration/misbehaviour.go | 140 +++++++++++++++++--------- testutil/crypto/evidence.go | 48 +++++---- x/ccv/provider/keeper/misbehaviour.go | 43 ++++++-- 5 files changed, 163 insertions(+), 80 deletions(-) diff --git a/Dockerfile b/Dockerfile index 7f58c49632..e8ad65a81c 100644 --- a/Dockerfile +++ b/Dockerfile @@ -36,7 +36,7 @@ FROM informalofftermatt/cometmock:latest as cometmock-builder # Get GoRelayer FROM informalofftermatt/gorelayer:nogas AS gorelayer-builder -FROM --platform=linux/amd64 fedora:36 +FROM --platform=linux/arm64 fedora:36 RUN dnf update -y RUN dnf install -y which iproute iputils procps-ng vim-minimal tmux net-tools htop jq USER root diff --git a/tests/e2e/main.go b/tests/e2e/main.go index 7428e14cb5..1f8ca6fdd4 100644 --- a/tests/e2e/main.go +++ b/tests/e2e/main.go @@ -57,11 +57,11 @@ func main() { } testRuns := []testRunWithSteps{ - {ChangeoverTestRun(), changeoverSteps}, - {DefaultTestRun(), happyPathSteps}, - {DemocracyTestRun(true), democracySteps}, - {DemocracyTestRun(false), rewardDenomConsumerSteps}, - {SlashThrottleTestRun(), slashThrottleSteps}, + // {ChangeoverTestRun(), changeoverSteps}, + // {DefaultTestRun(), happyPathSteps}, + // {DemocracyTestRun(true), democracySteps}, + // {DemocracyTestRun(false), rewardDenomConsumerSteps}, + // {SlashThrottleTestRun(), slashThrottleSteps}, {ConsumerMisbehaviourTestRun(), consumerMisbehaviourSteps}, {DefaultTestRun(), consumerDoubleSignSteps}, } diff --git a/tests/integration/misbehaviour.go b/tests/integration/misbehaviour.go index 306fb06ec4..02cd02128d 100644 --- a/tests/integration/misbehaviour.go +++ b/tests/integration/misbehaviour.go @@ -95,6 +95,15 @@ func (s *CCVTestSuite) TestGetByzantineValidators() { altSigners[clientTMValset.Validators[1].Address.String()] = clientSigners[clientTMValset.Validators[1].Address.String()] altSigners[clientTMValset.Validators[2].Address.String()] = clientSigners[clientTMValset.Validators[2].Address.String()] + // maliciousSigner := tmtypes.NewMockPV() + + // // Create a subset of the consumer client validator set + // altValset2 := tmtypes.NewValidatorSet(s.consumerChain.Vals.Validators[0:3]) + // altSigners2 := make(map[string]tmtypes.PrivValidator, 3) + // altSigners2[clientTMValset.Validators[0].Address.String()] = clientSigners[clientTMValset.Validators[0].Address.String()] + // altSigners2[clientTMValset.Validators[1].Address.String()] = clientSigners[clientTMValset.Validators[1].Address.String()] + // altSigners2[clientTMValset.Validators[2].Address.String()] = maliciousSigner + // create a consumer client header clientHeader := s.consumerChain.CreateTMClientHeader( s.consumerChain.ChainID, @@ -135,6 +144,81 @@ func (s *CCVTestSuite) TestGetByzantineValidators() { nil, false, }, + { + "incorrect valset - shouldn't pass", + func() *ibctmtypes.Misbehaviour { + clientHeader := s.consumerChain.CreateTMClientHeader( + s.consumerChain.ChainID, + int64(clientHeight.RevisionHeight+1), + clientHeight, + altTime.Add(time.Minute), + clientTMValset, + clientTMValset, + clientTMValset, + clientSigners, + ) + + // create conflicting header with 2/4 validators voting nil + clientHeaderWithNilVotes := s.consumerChain.CreateTMClientHeader( + s.consumerChain.ChainID, + int64(clientHeight.RevisionHeight+1), + clientHeight, + altTime.Add(time.Hour), + clientTMValset, + clientTMValset, + clientTMValset, + clientSigners, + ) + + testutil.CorruptValidatorPubkeyInHeader(clientHeaderWithNilVotes, clientTMValset.Validators[0].Address) + + return &ibctmtypes.Misbehaviour{ + ClientId: s.path.EndpointA.ClientID, + Header1: clientHeader, + Header2: clientHeaderWithNilVotes, + } + }, + // Expect validators who did NOT vote nil + []*tmtypes.Validator{}, + false, + }, + { + "incorrect signatures - shouldn't pass", + func() *ibctmtypes.Misbehaviour { + clientHeader := s.consumerChain.CreateTMClientHeader( + s.consumerChain.ChainID, + int64(clientHeight.RevisionHeight+1), + clientHeight, + altTime.Add(time.Minute), + clientTMValset, + clientTMValset, + clientTMValset, + clientSigners, + ) + + // create conflicting header with 2/4 validators voting nil + clientHeaderWithNilVotes := s.consumerChain.CreateTMClientHeader( + s.consumerChain.ChainID, + int64(clientHeight.RevisionHeight+1), + clientHeight, + altTime.Add(time.Hour), + clientTMValset, + clientTMValset, + clientTMValset, + clientSigners, + ) + testutil.CorruptCommitSigsInHeader(clientHeaderWithNilVotes, clientTMValset.Validators[0].Address) + + return &ibctmtypes.Misbehaviour{ + ClientId: s.path.EndpointA.ClientID, + Header1: clientHeader, + Header2: clientHeaderWithNilVotes, + } + }, + // Expect validators who did NOT vote nil + []*tmtypes.Validator{}, + false, + }, { "light client attack - lunatic attack", func() *ibctmtypes.Misbehaviour { @@ -212,43 +296,6 @@ func (s *CCVTestSuite) TestGetByzantineValidators() { []*tmtypes.Validator{}, true, }, - { - "validators who did vote nil should not be returned", - func() *ibctmtypes.Misbehaviour { - clientHeader := s.consumerChain.CreateTMClientHeader( - s.consumerChain.ChainID, - int64(clientHeight.RevisionHeight+1), - clientHeight, - altTime.Add(time.Minute), - clientTMValset, - clientTMValset, - clientTMValset, - clientSigners, - ) - - // create conflicting header with 2/4 validators voting nil - clientHeaderWithNilVotes := s.consumerChain.CreateTMClientHeader( - s.consumerChain.ChainID, - int64(clientHeight.RevisionHeight+1), - clientHeight, - altTime.Add(time.Hour), - clientTMValset, - clientTMValset, - clientTMValset, - clientSigners, - ) - testutil.UpdateHeaderCommitWithNilVotes(clientHeaderWithNilVotes, clientTMValset.Validators[:2]) - - return &ibctmtypes.Misbehaviour{ - ClientId: s.path.EndpointA.ClientID, - Header1: clientHeader, - Header2: clientHeaderWithNilVotes, - } - }, - // Expect validators who did NOT vote nil - clientTMValset.Validators[2:], - true, - }, } for _, tc := range testCases { @@ -299,10 +346,15 @@ func (s *CCVTestSuite) TestCheckMisbehaviour() { // create an alternative validator set using more than 1/3 of the trusted validator set altValset := tmtypes.NewValidatorSet(s.consumerChain.Vals.Validators[0:2]) - altSigners := make(map[string]tmtypes.PrivValidator, 1) + altSigners := make(map[string]tmtypes.PrivValidator, 2) altSigners[clientTMValset.Validators[0].Address.String()] = clientSigners[clientTMValset.Validators[0].Address.String()] altSigners[clientTMValset.Validators[1].Address.String()] = clientSigners[clientTMValset.Validators[1].Address.String()] + // create an alternative validator set using less 1/3 of the trusted validator set + altValset2 := tmtypes.NewValidatorSet(s.consumerChain.Vals.Validators[0:1]) + altSigners2 := make(map[string]tmtypes.PrivValidator, 1) + altSigners2[clientTMValset.Validators[0].Address.String()] = clientSigners[clientTMValset.Validators[0].Address.String()] + clientHeader := s.consumerChain.CreateTMClientHeader( s.consumerChain.ChainID, int64(clientHeight.RevisionHeight+1), @@ -315,19 +367,17 @@ func (s *CCVTestSuite) TestCheckMisbehaviour() { ) // create a conflicting client header with insufficient voting power - // by changing 3/4 of its validators votes to nil - clientHeaderWithNilVotes := s.consumerChain.CreateTMClientHeader( + clientHeader2 := s.consumerChain.CreateTMClientHeader( s.consumerChain.ChainID, int64(clientHeight.RevisionHeight+1), clientHeight, // use a different block time to change the header BlockID headerTs.Add(time.Hour), + altValset2, + altValset2, clientTMValset, - clientTMValset, - clientTMValset, - clientSigners, + altSigners2, ) - testutil.UpdateHeaderCommitWithNilVotes(clientHeaderWithNilVotes, clientTMValset.Validators[:4]) testCases := []struct { name string @@ -402,7 +452,7 @@ func (s *CCVTestSuite) TestCheckMisbehaviour() { &ibctmtypes.Misbehaviour{ ClientId: s.path.EndpointA.ClientID, Header1: clientHeader, - Header2: clientHeaderWithNilVotes, + Header2: clientHeader2, }, false, }, diff --git a/testutil/crypto/evidence.go b/testutil/crypto/evidence.go index 1a2c2cbc7e..6759b0e052 100644 --- a/testutil/crypto/evidence.go +++ b/testutil/crypto/evidence.go @@ -1,11 +1,11 @@ package crypto import ( - "fmt" "time" ibctmtypes "github.com/cosmos/ibc-go/v4/modules/light-clients/07-tendermint/types" "github.com/tendermint/tendermint/crypto/tmhash" + "github.com/tendermint/tendermint/libs/bytes" tmtypes "github.com/tendermint/tendermint/types" ) @@ -92,39 +92,43 @@ func MakeAndSignVoteWithForgedValAddress( return vote } -// UpdateHeaderCommitWithNilVotes updates the given light client header -// by changing the commit BlockIDFlag of the given validators to nil -// +// CorruptCommitSigsInHeader corrupts the header by changing the value of the commit signature +// given validator address // Note that this method is solely used for testing purposes -func UpdateHeaderCommitWithNilVotes(header *ibctmtypes.Header, validators []*tmtypes.Validator) { - if len(validators) > len(header.ValidatorSet.Validators) { - panic(fmt.Sprintf("cannot change more than %d validators votes: got %d", - len(header.ValidatorSet.Validators), len(header.ValidatorSet.Validators))) - } - +func CorruptCommitSigsInHeader(header *ibctmtypes.Header, valAddress bytes.HexBytes) { commit, err := tmtypes.CommitFromProto(header.Commit) if err != nil { panic(err) } + for idx, sig := range commit.Signatures { + if sig.ValidatorAddress.String() == valAddress.String() { + sig.Signature = []byte("randomsig") + commit.Signatures[idx] = sig + } + } + // update the commit in client the header + header.SignedHeader.Commit = commit.ToProto() +} + +// CorruptValidatorPubkeyInHeader corrupts the header by changing the validator pubkey +// of the given validator address in the validator set +// Note that this method is solely used for testing purposes +func CorruptValidatorPubkeyInHeader(header *ibctmtypes.Header, valAddress bytes.HexBytes) { valset, err := tmtypes.ValidatorSetFromProto(header.ValidatorSet) if err != nil { panic(err) } - for _, v := range validators { - // get validator index in valset - idx, _ := valset.GetByAddress(v.Address) - if idx != -1 { - // get validator commit sig - s := commit.Signatures[idx] - // change BlockIDFlag to nil - s.BlockIDFlag = tmtypes.BlockIDFlagNil - // update the signatures - commit.Signatures[idx] = s + for _, v := range valset.Validators { + if v.Address.String() == valAddress.String() { + v.PubKey = tmtypes.NewMockPV().PrivKey.PubKey() } } - // update the commit in client the header - header.SignedHeader.Commit = commit.ToProto() + vs, err := valset.ToProto() + if err != nil { + panic(err) + } + header.ValidatorSet = vs } diff --git a/x/ccv/provider/keeper/misbehaviour.go b/x/ccv/provider/keeper/misbehaviour.go index 7307284655..13d2b15f7a 100644 --- a/x/ccv/provider/keeper/misbehaviour.go +++ b/x/ccv/provider/keeper/misbehaviour.go @@ -101,20 +101,28 @@ func (k Keeper) GetByzantineValidators(ctx sdk.Context, misbehaviour ibctmtypes. // and return the intersection of validators who signed both // create a map with the validators' address that signed header1 - header1Signers := map[string]struct{}{} - for _, sign := range lightBlock1.Commit.Signatures { - if !sign.ForBlock() { + header1Signers := map[string]int{} + for idx, sign := range lightBlock1.Commit.Signatures { + if sign.Absent() { continue } - header1Signers[sign.ValidatorAddress.String()] = struct{}{} + header1Signers[sign.ValidatorAddress.String()] = idx } // iterate over the header2 signers and check if they signed header1 - for _, sign := range lightBlock2.Commit.Signatures { - if !sign.ForBlock() { + for sigIdxHeader2, sign := range lightBlock2.Commit.Signatures { + if sign.Absent() { continue } - if _, ok := header1Signers[sign.ValidatorAddress.String()]; ok { + if sigIdxHeader1, ok := header1Signers[sign.ValidatorAddress.String()]; ok { + if err := verifyLightBlockCommitSig(*lightBlock1, sigIdxHeader1); err != nil { + return nil, err + } + + if err := verifyLightBlockCommitSig(*lightBlock2, sigIdxHeader2); err != nil { + return nil, err + } + _, val := lightBlock1.ValidatorSet.GetByAddress(sign.ValidatorAddress) validators = append(validators, val) } @@ -192,3 +200,24 @@ func headersStateTransitionsAreConflicting(h1, h2 tmtypes.Header) bool { !bytes.Equal(h1.AppHash, h2.AppHash) || !bytes.Equal(h1.LastResultsHash, h2.LastResultsHash) } + +func verifyLightBlockCommitSig(lightBlock tmtypes.LightBlock, sigIdx int) error { + // get signature + sig := lightBlock.Commit.Signatures[sigIdx] + + // get validator + _, val := lightBlock.ValidatorSet.GetByAddress(sig.ValidatorAddress) + + // verify validator pubkey correspond to signature validator address + if !bytes.Equal(val.PubKey.Address(), sig.ValidatorAddress) { + return fmt.Errorf("validator public key doesn't correspond to signature validator address: %d!= %d", val.PubKey.Address(), sig.ValidatorAddress) + } + + // validate signature + voteSignBytes := lightBlock.Commit.VoteSignBytes(lightBlock.ChainID, int32(sigIdx)) + if !val.PubKey.VerifySignature(voteSignBytes, sig.Signature) { + return fmt.Errorf("wrong signature (#%d): %X", sigIdx, sig.Signature) + } + + return nil +}