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.

Sign up for GitHub

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: drop amnesia attacks in consumer misbehaviour handling #1388

Merged
merged 2 commits into from
Nov 2, 2023
Merged

fix: drop amnesia attacks in consumer misbehaviour handling #1388

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
191391d
drop amnesia attacks
sainoe Nov 2, 2023
fe27b0e
add doc
sainoe Nov 2, 2023
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
181 changes: 99 additions & 82 deletions tests/integration/misbehaviour.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -83,105 +82,113 @@ 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()]

// TODO: figure out how to test an amnesia cases for "amnesia" attack
// create a consumer client header
clientHeader := s.consumerChain.CreateTMClientHeader(
s.consumerChain.ChainID,
int64(clientHeight.RevisionHeight+1),
clientHeight,
altTime,
clientTMValset,
clientTMValset,
clientTMValset,
clientSigners,
)

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(
"light client attack - equivocation",
func() *ibctmtypes.Misbehaviour {
return &ibctmtypes.Misbehaviour{
ClientId: s.path.EndpointA.ClientID,
Header1: clientHeader,
// the resulting header contains a different BlockID
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,
},
{
"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,
Expand All @@ -190,11 +197,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,
},
}
Expand All @@ -203,22 +217,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 {
Expand Down
28 changes: 24 additions & 4 deletions x/ccv/provider/keeper/misbehaviour.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package keeper

import (
"bytes"
"fmt"

"github.com/cosmos/interchain-security/v2/x/ccv/provider/types"
Expand Down Expand Up @@ -75,18 +76,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,
if !headersStateTransitionsAreConflicting(*lightBlock1.Header, *lightBlock2.Header) && lightBlock1.Commit.Round != lightBlock2.Commit.Round {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not clear to me why those header fields (i.e., ValidatorsHash, NextValidatorsHash etc.) must be the same?
I'm not sure what I'm missing but my understanding is that for this to be misbehaviour the blockIDs have to be different (e.g., see here) and blockID is a hash that incorporates among others the above fields (?)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These fields must be the same because, in all amnesia scenarios, it's assumed that correct validators validate both headers.

The BlockID hash contains other fields that are only related to the current block e.g. the time or the proposer.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Deleted my previous comment. After the offline discussion we had yesterday this makes sense and the above code checks what CometBFT already checks in GetByzantineValidators.
Thanks @sainoe !

Just for posterity: If we have 2 headers at the same height H with h1.AppHash != h2.AppHash then this is a lunatic attack because AppHash is computed as the "state after txs from the previous block" H - 1. Only a "lunatic" validator would have 2 different views of the AppHash when applying the transactions of a previous block. The same is the case for all the fields that are based on the previous block which are the ones checked against in headersStateTransitionsAreConflicting. In case of a lunatic attack we can still slash.

return
}

// compare the signatures of the headers
// and return the intersection of validators who signed both
Expand Down Expand Up @@ -160,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 {
sainoe marked this conversation as resolved.
Show resolved Hide resolved
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)
}
Loading