diff --git a/CHANGELOG.md b/CHANGELOG.md index b935231e16..5890240b97 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,6 @@ Add an entry to the unreleased provider section whenever merging a PR to main that is not targeted at a specific release. These entries will eventually be included in a provider release. -* (refactor) [#1294](https://github.com/cosmos/interchain-security/pull/1294) Remove the equivocation proposal handler. * (feature!) [#1280](https://github.com/cosmos/interchain-security/pull/1280) provider proposal for changing reward denoms * (feature!) [#1244](https://github.com/cosmos/interchain-security/pull/1244) Update the default consumer unbonding period to 2 weeks. * (deps) [#1259](https://github.com/cosmos/interchain-security/pull/1259) Bump [cosmos-sdk](https://github.com/cosmos/cosmos-sdk) to [v0.47.5](https://github.com/cosmos/cosmos-sdk/releases/tag/v0.47.5). @@ -12,6 +11,9 @@ Add an entry to the unreleased provider section whenever merging a PR to main th * (deps) [#1258](https://github.com/cosmos/interchain-security/pull/1258) Bump [cosmos-sdk](https://github.com/cosmos/cosmos-sdk) to [v0.47.4](https://github.com/cosmos/cosmos-sdk/releases/tag/v0.47.4). * (deps!) [#1196](https://github.com/cosmos/interchain-security/pull/1196) Bump [ibc-go](https://github.com/cosmos/ibc-go) to [v7.2.0](https://github.com/cosmos/ibc-go/releases/tag/v7.2.0). * `[x/ccv/provider]` (fix) [#1076](https://github.com/cosmos/interchain-security/pull/1076) Add `InitTimeoutTimestamps` and `ExportedVscSendTimestamps` to exported genesis. +* (fix!) [#1422](https://github.com/cosmos/interchain-security/pull/1422) Fix the misbehaviour handling by verifying the signatures of byzantine validators. + +## v2.2.0-provider-lsm ### Cryptographic verification of equivocation * New feature enabling the provider chain to verify equivocation evidence on its own instead of trusting consumer chains, see [EPIC](https://github.com/cosmos/interchain-security/issues/732). diff --git a/tests/integration/misbehaviour.go b/tests/integration/misbehaviour.go index d841cf7087..e12d12664c 100644 --- a/tests/integration/misbehaviour.go +++ b/tests/integration/misbehaviour.go @@ -9,6 +9,7 @@ import ( tmtypes "github.com/cometbft/cometbft/types" + testutil "github.com/cosmos/interchain-security/v3/testutil/crypto" "github.com/cosmos/interchain-security/v3/x/ccv/provider/types" ) @@ -137,6 +138,119 @@ 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, + ) + + clientHeaderWithCorruptedValset := s.consumerChain.CreateTMClientHeader( + s.consumerChain.ChainID, + int64(clientHeight.RevisionHeight+1), + clientHeight, + altTime.Add(time.Hour), + clientTMValset, + clientTMValset, + clientTMValset, + clientSigners, + ) + + // change a validator public key in one the second header + testutil.CorruptValidatorPubkeyInHeader(clientHeaderWithCorruptedValset, clientTMValset.Validators[0].Address) + + return &ibctmtypes.Misbehaviour{ + ClientId: s.path.EndpointA.ClientID, + Header1: clientHeader, + Header2: clientHeaderWithCorruptedValset, + } + }, + []*tmtypes.Validator{}, + false, + }, + { + "incorrect valset 2 - 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, + ) + + clientHeaderWithCorruptedSigs := s.consumerChain.CreateTMClientHeader( + s.consumerChain.ChainID, + int64(clientHeight.RevisionHeight+1), + clientHeight, + altTime.Add(time.Hour), + clientTMValset, + clientTMValset, + clientTMValset, + clientSigners, + ) + + // change the valset in the header + vs, _ := altValset.ToProto() + clientHeader.ValidatorSet.Validators = vs.Validators[:3] + clientHeaderWithCorruptedSigs.ValidatorSet.Validators = vs.Validators[:3] + + return &ibctmtypes.Misbehaviour{ + ClientId: s.path.EndpointA.ClientID, + Header1: clientHeader, + Header2: clientHeaderWithCorruptedSigs, + } + }, + []*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, + ) + + clientHeaderWithCorruptedSigs := s.consumerChain.CreateTMClientHeader( + s.consumerChain.ChainID, + int64(clientHeight.RevisionHeight+1), + clientHeight, + altTime.Add(time.Hour), + clientTMValset, + clientTMValset, + clientTMValset, + clientSigners, + ) + + // change the signature of one of the validator in the header + testutil.CorruptCommitSigsInHeader(clientHeaderWithCorruptedSigs, clientTMValset.Validators[0].Address) + + return &ibctmtypes.Misbehaviour{ + ClientId: s.path.EndpointA.ClientID, + Header1: clientHeader, + Header2: clientHeaderWithCorruptedSigs, + } + }, + []*tmtypes.Validator{}, + false, + }, { "light client attack - lunatic attack", func() *ibctmtypes.Misbehaviour { @@ -226,8 +340,8 @@ func (s *CCVTestSuite) TestGetByzantineValidators() { s.NoError(err) s.Equal(len(tc.expByzantineValidators), len(byzantineValidators)) - // For both lunatic and equivocation attacks all the validators - // who signed the bad header (Header2) should be in returned in the evidence + // For both lunatic and equivocation attacks, all the validators + // who signed both headers if len(tc.expByzantineValidators) > 0 { equivocatingVals := tc.getMisbehaviour().Header2.ValidatorSet s.Equal(len(equivocatingVals.Validators), len(byzantineValidators)) @@ -267,7 +381,7 @@ 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()] @@ -347,7 +461,16 @@ func (s *CCVTestSuite) TestCheckMisbehaviour() { }, false, }, - // TODO: Add valset signature test case here + // TODO: should pass after 1401 is merged + // { + // "one header of the misbehaviour has insufficient voting power - shouldn't pass", + // &ibctmtypes.Misbehaviour{ + // ClientId: s.path.EndpointA.ClientID, + // Header1: clientHeader, + // Header2: clientHeader2, + // }, + // false, + // }, { "valid misbehaviour - should pass", &ibctmtypes.Misbehaviour{ diff --git a/testutil/crypto/evidence.go b/testutil/crypto/evidence.go index 9bd96bdca5..653f20824e 100644 --- a/testutil/crypto/evidence.go +++ b/testutil/crypto/evidence.go @@ -3,7 +3,10 @@ package crypto import ( "time" + ibctmtypes "github.com/cosmos/ibc-go/v7/modules/light-clients/07-tendermint" + "github.com/cometbft/cometbft/crypto/tmhash" + "github.com/cometbft/cometbft/libs/bytes" tmtypes "github.com/cometbft/cometbft/types" ) @@ -66,7 +69,6 @@ func MakeAndSignVoteWithForgedValAddress( valAddressSigner tmtypes.PrivValidator, chainID string, ) *tmtypes.Vote { - // create the vote using a different key than the signing key vote, err := tmtypes.MakeVote( blockHeight, @@ -90,3 +92,44 @@ func MakeAndSignVoteWithForgedValAddress( vote.Signature = v.Signature return vote } + +// CorruptCommitSigsInHeader corrupts the header by changing the value +// of the commit signature for given validator address. +// Note that this method is solely used for testing purposes +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 valset.Validators { + if v.Address.String() == valAddress.String() { + v.PubKey = tmtypes.NewMockPV().PrivKey.PubKey() + } + } + + vs, err := valset.ToProto() + if err != nil { + panic(err) + } + header.ValidatorSet = vs +} diff --git a/testutil/keeper/expectations.go b/testutil/keeper/expectations.go index 4702e688e2..9494b66936 100644 --- a/testutil/keeper/expectations.go +++ b/testutil/keeper/expectations.go @@ -3,7 +3,6 @@ package keeper import ( time "time" - math "cosmossdk.io/math" clienttypes "github.com/cosmos/ibc-go/v7/modules/core/02-client/types" conntypes "github.com/cosmos/ibc-go/v7/modules/core/03-connection/types" channeltypes "github.com/cosmos/ibc-go/v7/modules/core/04-channel/types" @@ -12,6 +11,8 @@ import ( "github.com/golang/mock/gomock" extra "github.com/oxyno-zeta/gomock-extra-matcher" + math "cosmossdk.io/math" + sdk "github.com/cosmos/cosmos-sdk/types" capabilitytypes "github.com/cosmos/cosmos-sdk/x/capability/types" stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types" @@ -217,5 +218,4 @@ func GetMocksForSlashValidator( SlashWithInfractionReason(ctx, consAddr, expectedInfractionHeight, expectedSlashPower, slashFraction, stakingtypes.Infraction_INFRACTION_DOUBLE_SIGN). Times(1), } - } diff --git a/x/ccv/provider/keeper/misbehaviour.go b/x/ccv/provider/keeper/misbehaviour.go index 51bfd5749a..23b0caeb5a 100644 --- a/x/ccv/provider/keeper/misbehaviour.go +++ b/x/ccv/provider/keeper/misbehaviour.go @@ -104,20 +104,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 { + 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 { + 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) } @@ -182,3 +190,27 @@ 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 + idx, val := lightBlock.ValidatorSet.GetByAddress(sig.ValidatorAddress) + if idx == -1 { + return fmt.Errorf("incorrect signature: validator address %s isn't part of the validator set", sig.ValidatorAddress.String()) + } + + // verify validator pubkey corresponds 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: %s!= %s", 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 +} diff --git a/x/ccv/provider/keeper/punish_validator_test.go b/x/ccv/provider/keeper/punish_validator_test.go index 8834d0b464..63e7ab9e86 100644 --- a/x/ccv/provider/keeper/punish_validator_test.go +++ b/x/ccv/provider/keeper/punish_validator_test.go @@ -412,7 +412,7 @@ func TestSlashValidator(t *testing.T) { mocks.MockStakingKeeper.EXPECT(). SlashUnbondingDelegation(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()). DoAndReturn( - func(_ sdk.Context, undelegation stakingtypes.UnbondingDelegation, _ int64, _ sdk.Dec) sdk.Int { + func(_ sdk.Context, undelegation stakingtypes.UnbondingDelegation, _ int64, _ sdk.Dec) math.Int { sum := sdk.NewInt(0) for _, r := range undelegation.Entries { if r.IsMature(ctx.BlockTime()) { @@ -425,7 +425,7 @@ func TestSlashValidator(t *testing.T) { mocks.MockStakingKeeper.EXPECT(). SlashRedelegation(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()). DoAndReturn( - func(_ sdk.Context, _ stakingtypes.Validator, redelegation stakingtypes.Redelegation, _ int64, _ sdk.Dec) sdk.Int { + func(_ sdk.Context, _ stakingtypes.Validator, redelegation stakingtypes.Redelegation, _ int64, _ sdk.Dec) math.Int { sum := sdk.NewInt(0) for _, r := range redelegation.Entries { if r.IsMature(ctx.BlockTime()) { diff --git a/x/ccv/provider/types/proposal.go b/x/ccv/provider/types/proposal.go index aa15fffc81..a7fcbbdd73 100644 --- a/x/ccv/provider/types/proposal.go +++ b/x/ccv/provider/types/proposal.go @@ -9,10 +9,11 @@ import ( clienttypes "github.com/cosmos/ibc-go/v7/modules/core/02-client/types" errorsmod "cosmossdk.io/errors" + sdk "github.com/cosmos/cosmos-sdk/types" + evidencetypes "github.com/cosmos/cosmos-sdk/x/evidence/types" govv1beta1 "github.com/cosmos/cosmos-sdk/x/gov/types/v1beta1" - evidencetypes "github.com/cosmos/cosmos-sdk/x/evidence/types" ccvtypes "github.com/cosmos/interchain-security/v3/x/ccv/types" )