From 191391da5cbfb18e561a68e000b173ac0fa4585d Mon Sep 17 00:00:00 2001 From: Simon Noetzlin Date: Thu, 2 Nov 2023 15:10:11 +0100 Subject: [PATCH 1/2] drop amnesia attacks --- tests/integration/misbehaviour.go | 178 ++++++++++++++------------ x/ccv/provider/keeper/misbehaviour.go | 16 ++- 2 files changed, 110 insertions(+), 84 deletions(-) diff --git a/tests/integration/misbehaviour.go b/tests/integration/misbehaviour.go index caa0647542..ad4b15cdb4 100644 --- a/tests/integration/misbehaviour.go +++ b/tests/integration/misbehaviour.go @@ -4,9 +4,8 @@ import ( "time" sdk "github.com/cosmos/cosmos-sdk/types" - "github.com/cosmos/interchain-security/v2/x/ccv/provider/types" - ibctmtypes "github.com/cosmos/ibc-go/v4/modules/light-clients/07-tendermint/types" + "github.com/cosmos/interchain-security/v2/x/ccv/provider/types" tmtypes "github.com/tendermint/tendermint/types" ) @@ -83,105 +82,114 @@ func (s *CCVTestSuite) TestGetByzantineValidators() { altTime := s.providerCtx().BlockTime().Add(time.Minute) + // Get the consumer client validator set clientHeight := s.consumerChain.LastHeader.TrustedHeight clientTMValset := tmtypes.NewValidatorSet(s.consumerChain.Vals.Validators) clientSigners := s.consumerChain.Signers - // Create a validator set subset + // Create a subset of the consumer client validator set altValset := tmtypes.NewValidatorSet(s.consumerChain.Vals.Validators[0:3]) - altSigners := make(map[string]tmtypes.PrivValidator, 1) + altSigners := make(map[string]tmtypes.PrivValidator, 3) altSigners[clientTMValset.Validators[0].Address.String()] = clientSigners[clientTMValset.Validators[0].Address.String()] altSigners[clientTMValset.Validators[1].Address.String()] = clientSigners[clientTMValset.Validators[1].Address.String()] altSigners[clientTMValset.Validators[2].Address.String()] = clientSigners[clientTMValset.Validators[2].Address.String()] + clientHeader := s.consumerChain.CreateTMClientHeader( + s.consumerChain.ChainID, + int64(clientHeight.RevisionHeight+1), + clientHeight, + altTime, + clientTMValset, + clientTMValset, + clientTMValset, + clientSigners, + ) + // TODO: figure out how to test an amnesia cases for "amnesia" attack testCases := []struct { name string - misbehaviour *ibctmtypes.Misbehaviour + getMisbehaviour func() *ibctmtypes.Misbehaviour expByzantineValidators []*tmtypes.Validator expPass bool }{ { "invalid misbehaviour - Header1 is empty", - &ibctmtypes.Misbehaviour{ - Header1: &ibctmtypes.Header{}, - Header2: s.consumerChain.CreateTMClientHeader( - s.consumerChain.ChainID, - int64(clientHeight.RevisionHeight+1), - clientHeight, - altTime, - altValset, - altValset, - clientTMValset, - altSigners, - ), + func() *ibctmtypes.Misbehaviour { + return &ibctmtypes.Misbehaviour{ + Header1: &ibctmtypes.Header{}, + Header2: clientHeader, + } }, nil, false, }, { "invalid headers - Header2 is empty", - &ibctmtypes.Misbehaviour{ - Header1: s.consumerChain.CreateTMClientHeader( - s.consumerChain.ChainID, - int64(clientHeight.RevisionHeight+1), - clientHeight, - altTime, - clientTMValset, - clientTMValset, - clientTMValset, - clientSigners, - ), - Header2: &ibctmtypes.Header{}, + func() *ibctmtypes.Misbehaviour { + return &ibctmtypes.Misbehaviour{ + Header1: clientHeader, + Header2: &ibctmtypes.Header{}, + } }, nil, false, }, { - "invalid light client attack - lunatic attack", - &ibctmtypes.Misbehaviour{ - ClientId: s.path.EndpointA.ClientID, - Header1: s.consumerChain.CreateTMClientHeader( - s.consumerChain.ChainID, - int64(clientHeight.RevisionHeight+1), - clientHeight, - altTime, - clientTMValset, - clientTMValset, - clientTMValset, - clientSigners, - ), - Header2: s.consumerChain.CreateTMClientHeader( - s.consumerChain.ChainID, - int64(clientHeight.RevisionHeight+1), - clientHeight, - altTime, - altValset, - altValset, - clientTMValset, - altSigners, - ), + "light client attack - lunatic attack", + func() *ibctmtypes.Misbehaviour { + return &ibctmtypes.Misbehaviour{ + ClientId: s.path.EndpointA.ClientID, + Header1: clientHeader, + // the resulting header contains invalid fields + // i.e. ValidatorsHash, NextValidatorsHash. + Header2: s.consumerChain.CreateTMClientHeader( + s.consumerChain.ChainID, + int64(clientHeight.RevisionHeight+1), + clientHeight, + altTime, + altValset, + altValset, + clientTMValset, + altSigners, + ), + } }, // Expect to get only the validators - // who signed both headers are returned + // who signed both headers altValset.Validators, true, }, { "valid light client attack - equivocation", - &ibctmtypes.Misbehaviour{ - ClientId: s.path.EndpointA.ClientID, - Header1: s.consumerChain.CreateTMClientHeader( - s.consumerChain.ChainID, - int64(clientHeight.RevisionHeight+1), - clientHeight, - altTime, - clientTMValset, - clientTMValset, - clientTMValset, - clientSigners, - ), - Header2: s.consumerChain.CreateTMClientHeader( + func() *ibctmtypes.Misbehaviour { + return &ibctmtypes.Misbehaviour{ + ClientId: s.path.EndpointA.ClientID, + Header1: clientHeader, + // the resulting header contains invalid fields + // i.e. ValidatorsHash, NextValidatorsHash etc. + Header2: s.consumerChain.CreateTMClientHeader( + s.consumerChain.ChainID, + int64(clientHeight.RevisionHeight+1), + clientHeight, + altTime.Add(time.Minute), + clientTMValset, + clientTMValset, + clientTMValset, + clientSigners, + ), + } + }, + // Expect to get the entire valset since + // all validators double-signed + clientTMValset.Validators, + true, + }, + { + "valid light client attack - amnesia", + func() *ibctmtypes.Misbehaviour { + // create a valid header with a different hash + // and commit round + amnesiaHeader := s.consumerChain.CreateTMClientHeader( s.consumerChain.ChainID, int64(clientHeight.RevisionHeight+1), clientHeight, @@ -190,11 +198,18 @@ func (s *CCVTestSuite) TestGetByzantineValidators() { clientTMValset, clientTMValset, clientSigners, - ), + ) + amnesiaHeader.Commit.Round = 2 + + return &ibctmtypes.Misbehaviour{ + ClientId: s.path.EndpointA.ClientID, + Header1: clientHeader, + Header2: amnesiaHeader, + } }, - // Expect to get the entire valset since - // all validators double-signed - clientTMValset.Validators, + // Expect no validators + // since amnesia attacks are dropped + []*tmtypes.Validator{}, true, }, } @@ -203,22 +218,25 @@ func (s *CCVTestSuite) TestGetByzantineValidators() { s.Run(tc.name, func() { byzantineValidators, err := s.providerApp.GetProviderKeeper().GetByzantineValidators( s.providerCtx(), - *tc.misbehaviour, + *tc.getMisbehaviour(), ) if tc.expPass { s.NoError(err) - // For both lunatic and equivocation attack all the validators - // who signed the bad header (Header2) should be in returned in the evidence - h2Valset := tc.misbehaviour.Header2.ValidatorSet + s.Equal(len(tc.expByzantineValidators), len(byzantineValidators)) - s.Equal(len(h2Valset.Validators), len(byzantineValidators)) + // For both lunatic and equivocation attacks all the validators + // who signed the bad header (Header2) should be in returned in the evidence + if len(tc.expByzantineValidators) > 0 { + equivocatingVals := tc.getMisbehaviour().Header2.ValidatorSet + s.Equal(len(equivocatingVals.Validators), len(byzantineValidators)) - vs, err := tmtypes.ValidatorSetFromProto(tc.misbehaviour.Header2.ValidatorSet) - s.NoError(err) + vs, err := tmtypes.ValidatorSetFromProto(equivocatingVals) + s.NoError(err) - for _, v := range tc.expByzantineValidators { - idx, _ := vs.GetByAddress(v.Address) - s.True(idx >= 0) + for _, v := range tc.expByzantineValidators { + idx, _ := vs.GetByAddress(v.Address) + s.True(idx >= 0) + } } } else { diff --git a/x/ccv/provider/keeper/misbehaviour.go b/x/ccv/provider/keeper/misbehaviour.go index 3dfdcebc21..8b29f2031b 100644 --- a/x/ccv/provider/keeper/misbehaviour.go +++ b/x/ccv/provider/keeper/misbehaviour.go @@ -75,18 +75,26 @@ func (k Keeper) HandleConsumerMisbehaviour(ctx sdk.Context, misbehaviour ibctmty // GetByzantineValidators returns the validators that signed both headers. // If the misbehavior is an equivocation light client attack, then these // validators are the Byzantine validators. -func (k Keeper) GetByzantineValidators(ctx sdk.Context, misbehaviour ibctmtypes.Misbehaviour) ([]*tmtypes.Validator, error) { +func (k Keeper) GetByzantineValidators(ctx sdk.Context, misbehaviour ibctmtypes.Misbehaviour) (validators []*tmtypes.Validator, err error) { // construct the trusted and conflicted light blocks lightBlock1, err := headerToLightBlock(*misbehaviour.Header1) if err != nil { - return nil, err + return } lightBlock2, err := headerToLightBlock(*misbehaviour.Header2) if err != nil { - return nil, err + return } - var validators []*tmtypes.Validator + // Check if the misbehaviour corresponds to an Amnesia attack, + // meaning that the conflicting headers have both valid state transitions + // and different commit rounds. In this case, we return no validators as we can't identify the byzantine validators. + // + // Note that we cannot differentiate which of the headers is trusted or malicious. + ev := &tmtypes.LightClientAttackEvidence{ConflictingBlock: lightBlock1} + if !ev.ConflictingHeaderIsInvalid(lightBlock2.Header) && lightBlock1.Commit.Round != lightBlock2.Commit.Round { + return + } // compare the signatures of the headers // and return the intersection of validators who signed both From fe27b0ec81d368d4e0637d4abced30e68ced7bd5 Mon Sep 17 00:00:00 2001 From: Simon Noetzlin Date: Thu, 2 Nov 2023 15:34:58 +0100 Subject: [PATCH 2/2] add doc --- tests/integration/misbehaviour.go | 9 ++++----- x/ccv/provider/keeper/misbehaviour.go | 20 ++++++++++++++++---- 2 files changed, 20 insertions(+), 9 deletions(-) diff --git a/tests/integration/misbehaviour.go b/tests/integration/misbehaviour.go index ad4b15cdb4..766fa41e5b 100644 --- a/tests/integration/misbehaviour.go +++ b/tests/integration/misbehaviour.go @@ -94,6 +94,7 @@ 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()] + // create a consumer client header clientHeader := s.consumerChain.CreateTMClientHeader( s.consumerChain.ChainID, int64(clientHeight.RevisionHeight+1), @@ -105,7 +106,6 @@ func (s *CCVTestSuite) TestGetByzantineValidators() { clientSigners, ) - // TODO: figure out how to test an amnesia cases for "amnesia" attack testCases := []struct { name string getMisbehaviour func() *ibctmtypes.Misbehaviour @@ -160,13 +160,12 @@ func (s *CCVTestSuite) TestGetByzantineValidators() { true, }, { - "valid light client attack - equivocation", + "light client attack - equivocation", func() *ibctmtypes.Misbehaviour { return &ibctmtypes.Misbehaviour{ ClientId: s.path.EndpointA.ClientID, Header1: clientHeader, - // the resulting header contains invalid fields - // i.e. ValidatorsHash, NextValidatorsHash etc. + // the resulting header contains a different BlockID Header2: s.consumerChain.CreateTMClientHeader( s.consumerChain.ChainID, int64(clientHeight.RevisionHeight+1), @@ -185,7 +184,7 @@ func (s *CCVTestSuite) TestGetByzantineValidators() { true, }, { - "valid light client attack - amnesia", + "light client attack - amnesia", func() *ibctmtypes.Misbehaviour { // create a valid header with a different hash // and commit round diff --git a/x/ccv/provider/keeper/misbehaviour.go b/x/ccv/provider/keeper/misbehaviour.go index 8b29f2031b..c9749a1245 100644 --- a/x/ccv/provider/keeper/misbehaviour.go +++ b/x/ccv/provider/keeper/misbehaviour.go @@ -1,6 +1,7 @@ package keeper import ( + "bytes" "fmt" "github.com/cosmos/interchain-security/v2/x/ccv/provider/types" @@ -88,11 +89,11 @@ func (k Keeper) GetByzantineValidators(ctx sdk.Context, misbehaviour ibctmtypes. // Check if the misbehaviour corresponds to an Amnesia attack, // meaning that the conflicting headers have both valid state transitions - // and different commit rounds. In this case, we return no validators as we can't identify the byzantine validators. + // and different commit rounds. In this case, we return no validators as + // we can't identify the byzantine validators. // - // Note that we cannot differentiate which of the headers is trusted or malicious. - ev := &tmtypes.LightClientAttackEvidence{ConflictingBlock: lightBlock1} - if !ev.ConflictingHeaderIsInvalid(lightBlock2.Header) && lightBlock1.Commit.Round != lightBlock2.Commit.Round { + // Note that we cannot differentiate which of the headers is trusted or malicious, + if !headersStateTransitionsAreConflicting(*lightBlock1.Header, *lightBlock2.Header) && lightBlock1.Commit.Round != lightBlock2.Commit.Round { return } @@ -168,3 +169,14 @@ func (k Keeper) CheckMisbehaviour(ctx sdk.Context, misbehaviour ibctmtypes.Misbe return nil } + +// Check if the given block headers have conflicting state transitions. +// Note that this method was copied from ConflictingHeaderIsInvalid in CometBFT, +// see https://github.com/cometbft/cometbft/blob/v0.34.27/types/evidence.go#L285 +func headersStateTransitionsAreConflicting(h1, h2 tmtypes.Header) bool { + return !bytes.Equal(h1.ValidatorsHash, h2.ValidatorsHash) || + !bytes.Equal(h1.NextValidatorsHash, h2.NextValidatorsHash) || + !bytes.Equal(h1.ConsensusHash, h2.ConsensusHash) || + !bytes.Equal(h1.AppHash, h2.AppHash) || + !bytes.Equal(h1.LastResultsHash, h2.LastResultsHash) +}