diff --git a/.github/workflows/golangci-lint.yml b/.github/workflows/golangci-lint.yml index 11e422b923..45e3b3c1d7 100644 --- a/.github/workflows/golangci-lint.yml +++ b/.github/workflows/golangci-lint.yml @@ -19,13 +19,13 @@ jobs: steps: - uses: actions/setup-go@v4 with: - go-version: "1.20" + go-version: '1.20' - uses: actions/checkout@v3 - name: golangci-lint uses: golangci/golangci-lint-action@v3 with: # Optional: version of golangci-lint to use in form of v1.2 or v1.2.3 or `latest` to use the latest version - version: v1.52.2 + version: v1.53.3 # Optional: working directory, useful for monorepos # working-directory: somedir diff --git a/.github/workflows/proto-registry.yml b/.github/workflows/proto-registry.yml index a3291cec36..326cc566a1 100644 --- a/.github/workflows/proto-registry.yml +++ b/.github/workflows/proto-registry.yml @@ -13,7 +13,7 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@v3 - - uses: bufbuild/buf-setup-action@v1.25.1 + - uses: bufbuild/buf-setup-action@v1.26.0 - uses: bufbuild/buf-push-action@v1 with: input: "proto" diff --git a/.github/workflows/proto.yml b/.github/workflows/proto.yml index deb110dad0..f6c0984da3 100644 --- a/.github/workflows/proto.yml +++ b/.github/workflows/proto.yml @@ -14,7 +14,7 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@v3 - - uses: bufbuild/buf-setup-action@v1.25.1 + - uses: bufbuild/buf-setup-action@v1.26.0 - uses: bufbuild/buf-breaking-action@v1 with: input: "proto" diff --git a/.golangci.yml b/.golangci.yml index 5a3b555a6d..5bf3cfb98f 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -3,9 +3,8 @@ run: timeout: 10m sort-results: true allow-parallel-runners: true - skip-dirs: - - "legacy_ibc_testing" - - "testutil" + skip-dirs: + - 'testutil' linters: disable-all: true @@ -31,28 +30,24 @@ linters: - unconvert - unused - - - - issues: exclude-rules: - - text: "Use of weak random number generator" + - text: 'Use of weak random number generator' linters: - gosec - - text: "ST1003:" + - text: 'ST1003:' linters: - stylecheck # FIXME: Disabled until golangci-lint updates stylecheck with this fix: # https://github.com/dominikh/go-tools/issues/389 - - text: "ST1016:" + - text: 'ST1016:' linters: - stylecheck - - path: "migrations" - text: "SA1019:" + - path: 'migrations' + text: 'SA1019:' linters: - staticcheck - - text: "leading space" + - text: 'leading space' linters: - nolintlint diff --git a/docs/docs/adrs/adr-005-cryptographic-equivocation-verification.md b/docs/docs/adrs/adr-005-cryptographic-equivocation-verification.md new file mode 100644 index 0000000000..f0b487d2b1 --- /dev/null +++ b/docs/docs/adrs/adr-005-cryptographic-equivocation-verification.md @@ -0,0 +1,105 @@ +--- +sidebar_position: 4 +title: Cryptographic verification of equivocation evidence +--- +# ADR 005: Cryptographic verification of equivocation evidence + +## Changelog +* 5/1/2023: First draft +* 7/23/23: Add light client attacks handling + +## Status + +Accepted + +## Context + +Currently, we use a governance proposal to slash validators for equivocation (double signing and light client attacks). +Every proposal needs to go through a (two weeks) voting period before it can be approved. +Given a three-week unbonding period, this means that an equivocation proposal needs to be submitted within one week since the infraction occurred. + +This ADR proposes a system to slash validators automatically for equivocation, immediately upon the provider chain's receipt of the evidence. Another thing to note is that we intend to introduce this system in stages, since even the partial ability to slash and/or tombstone is a strict improvement in security. +For the first stage of this work, we will only handle light client attacks. + +### Light Client Attack + +In a nutshell, the light client is a process that solely verifies a specific state machine's +consensus without executing the transactions. The light clients get new headers by querying +multiple nodes, called primary and witness nodes. + +Light clients download new headers committed on chain from a primary. Headers can be verified in two ways: sequentially, +where the block height of headers is serial, or using skipping. This second verification method allows light clients to download headers +with nonconsecutive block height, where some intermediate headers are skipped (see [Tendermint Light Client, Figure 1 and Figure 3](https://arxiv.org/pdf/2010.07031.pdf)). +Additionally, light clients are cross-checking new headers obtained from a primary with witnesses to ensure all nodes share the same state. + +A light client attack occurs when a Byzantine validator sends invalid headers to a light client. +As the light client doesn't execute transactions, it can be deceived into trusting corrupted application state transitions. +For instance, if a light client receives header `A` from the primary and header `B` from a witness for the same block height `H`, +and both headers are successfully verified, it indicates a light client attack. +Note that in this case, either the primary or the witness or both are malicious. + +The types of light client attacks are defined by analyzing the differences between the conflicting headers. +There are three types of light client attacks: lunatic attack, equivocation attack, and amnesia attack. +For details, see the [CometBFT specification](https://github.com/cometbft/cometbft/blob/main/spec/light-client/attacks/notes-on-evidence-handling.md#evidence-handling). + +When a light client agent detects two conflicting headers, it will initially verify their traces (see [cometBFT detector](https://github.com/cometbft/cometbft/blob/2af25aea6cfe6ac4ddac40ceddfb8c8eee17d0e6/light/detector.go#L28)) using its primary and witness nodes. +If these headers pass successful verification, the Byzantine validators will be identified based on the header's commit signatures +and the type of light client attack. The agent will then transmit this information to its nodes using a [`LightClientAttackEvidence`](https://github.com/cometbft/cometbft/blob/feed0ddf564e113a840c4678505601256b93a8bc/docs/architecture/adr-047-handling-evidence-from-light-client.md) to be eventually voted on and added to a block. +Note that from a light client agent perspective, it is not possible to establish whether a primary or a witness node, or both, are malicious. +Therefore, it will create and send two `LightClientAttackEvidence`: one against the primary (sent to the witness), and one against the witness (sent to the primary). +Both nodes will then verify it before broadcasting it and adding it to the [evidence pool](https://github.com/cometbft/cometbft/blob/2af25aea6cfe6ac4ddac40ceddfb8c8eee17d0e6/evidence/pool.go#L28). +If a `LightClientAttackEvidence` is finally committed to a block, the chain's evidence module will execute it, resulting in the jailing and the slashing of the validators responsible for the light client attack. + + +Light clients are a core component of IBC. In the event of a light client attack, IBC relayers notify the affected chains by submitting an [IBC misbehavior message]((https://github.com/cosmos/ibc-go/blob/2b7c969066fbcb18f90c7f5bd256439ca12535c7/proto/ibc/lightclients/tendermint/v1/tendermint.proto#L79)). +A misbehavior message includes the conflicting headers that constitute a `LightClientAttackEvidence`. Upon receiving such a message, +a chain will first verify whether these headers would have convinced its light client. This verification is achieved by checking +the header states against the light client consensus states (see [IBC misbehaviour handler](https://github.com/cosmos/ibc-go/blob/2b7c969066fbcb18f90c7f5bd256439ca12535c7/modules/light-clients/07-tendermint/types/misbehaviour_handle.go#L101)). If the misbehaviour is successfully verified, the chain will then "freeze" the +light client, halting any further trust in or updating of its states. + + +## Decision + +In the first iteration of the feature, we will introduce a new endpoint: `HandleConsumerMisbehaviour(ctx sdk.Context, misbehaviour ibctmtypes.Misbehaviour)`. +The main idea is to leverage the current IBC misbehaviour handling and update it to solely jail and slash the validators that +performed a light client attack. This update will be made under the assumption that the chain connected via this light client +share the same validator set, as it is the case with Replicated Security. + +This endpoint will reuse the IBC client libraries to verify that the misbehaviour headers would have fooled the light client. +Additionally, it’s crucial that the endpoint logic result in the slashing and jailing of validators under the same conditions +as a light client agent detector. Therefore, the endpoint will ensure that the two conditions are met: +the headers in the misbehaviour message have the same block height, and +the light client isn’t expired. + +After having successfully verified a misbehaviour, the endpoint will execute the jailing and slashing of the malicious validators similarly as in the evidence module. + +### Current limitations: + +- This only handles light client attacks, not double signing. In the future, we will add the code to also verify double signing. + +- We cannot derive an infraction height from the evidence, so it is only possible to tombstone validators, not actually slash them. +To explain the technical reasons behind this limitation, let's recap the initial consumer initiated slashing logic. +In a nutshell, consumer heights are mapped to provider heights through VSCPackets, namely through the so called vscIDs. +When an infraction occurs on the consumer, a SlashPacket containing the vscID obtained from mapping the consumer infraction height +is sent to the provider. Upon receiving the packet, the provider maps the consumer infraction height to a local infraction height, +which is used to slash the misbehaving validator. In the context of untrusted consumer chains, all their states, including vscIDs, +could be corrupted and therefore cannot be used for slashing purposes. + +- Currently, the endpoint can only handle "equivocation" light client attacks. This is because the "lunatic" attacks require the endpoint to possess the ability to dissociate which header is conflicted or trusted upon receiving a misbehavior message. Without this information, it's not possible to define the Byzantine validators from the conflicting headers (see [comment](https://github.com/cosmos/interchain-security/pull/826#discussion_r1268668684)). + + +## Consequences + +### Positive + +- After this ADR is applied, it will be possible for the provider chain to tombstone validators who committed a light client attack. + +### Negative + +- N/A + + +## References + +* [ICS misbehaviour handling PR](https://github.com/cosmos/interchain-security/pull/826) +* [Architectural diagrams](https://docs.google.com/document/d/1fe1uSJl1ZIYWXoME3Yf4Aodvz7V597Ric875JH-rigM/edit#heading=h.rv4t8i6d6jfn) diff --git a/docs/docs/adrs/intro.md b/docs/docs/adrs/intro.md index d0c097fb06..0c1c722366 100644 --- a/docs/docs/adrs/intro.md +++ b/docs/docs/adrs/intro.md @@ -36,6 +36,8 @@ To suggest an ADR, please make use of the [ADR template](./adr-template.md) prov | [002](./adr-002-throttle.md) | Jail Throttling | Accepted, Implemented | | [003](./adr-003-equivocation-gov-proposal.md) | Equivocation governance proposal | Accepted, Implemented | | [004](./adr-004-denom-dos-fixes) | Denom DOS fixes | Accepted, Implemented | +| [005](./adr-005-cryptographic-equivocation-verification.md) | Cryptographic verification of equivocation evidence | Accepted, In-progress | | [007](./adr-007-pause-unbonding-on-eqv-prop.md) | Pause validator unbonding during equivocation proposal | Proposed | | [008](./adr-008-throttle-retries.md) | Throttle with retries | Accepted, In-progress | | [009](./adr-009-soft-opt-out.md) | Soft Opt-out | Accepted, Implemented | +| [009](./adr-010-standalone-changeover.md) | Standalone to Consumer Changeover | Accepted, Implemented | diff --git a/legacy_ibc_testing/core/events.go b/legacy_ibc_testing/core/events.go index daabc92d54..a01c14c685 100644 --- a/legacy_ibc_testing/core/events.go +++ b/legacy_ibc_testing/core/events.go @@ -3,10 +3,10 @@ package core import ( "strconv" - abci "github.com/cometbft/cometbft/abci/types" - clienttypes "github.com/cosmos/ibc-go/v7/modules/core/02-client/types" "github.com/cosmos/ibc-go/v7/modules/core/04-channel/types" + + abci "github.com/cometbft/cometbft/abci/types" ) /* @@ -20,7 +20,7 @@ These files will be deprecated once ICS is able to upgrade to ibc-go v5. func ReconstructPacketFromEvent(event abci.Event) (packet types.Packet, err error) { attrMap := make(map[string][]byte) for _, attr := range event.Attributes { - attrMap[string(attr.Key)] = []byte(attr.Value) + attrMap[attr.Key] = []byte(attr.Value) } sequence, err := strconv.Atoi(string(attrMap[string(types.AttributeKeySequence)])) @@ -36,7 +36,7 @@ func ReconstructPacketFromEvent(event abci.Event) (packet types.Packet, err erro return packet, err } return types.NewPacket( - attrMap[string(types.AttributeKeyData)], // data + attrMap[string(types.AttributeKeyData)], //nolint:staticcheck // data uint64(sequence), string(attrMap[string(types.AttributeKeySrcPort)]), // sourcePort, string(attrMap[string(types.AttributeKeySrcChannel)]), // sourceChannel, diff --git a/legacy_ibc_testing/simapp/test_helpers.go b/legacy_ibc_testing/simapp/test_helpers.go index 249fffb903..efdc359499 100644 --- a/legacy_ibc_testing/simapp/test_helpers.go +++ b/legacy_ibc_testing/simapp/test_helpers.go @@ -8,15 +8,19 @@ import ( "testing" "time" - tmproto "github.com/cometbft/cometbft/proto/tendermint/types" - tmtypes "github.com/cometbft/cometbft/types" + "github.com/stretchr/testify/require" + + errorsmod "cosmossdk.io/errors" + bam "github.com/cosmos/cosmos-sdk/baseapp" "github.com/cosmos/cosmos-sdk/client" "github.com/cosmos/cosmos-sdk/crypto/keys/ed25519" cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/types/errors" - "github.com/stretchr/testify/require" + + tmproto "github.com/cometbft/cometbft/proto/tendermint/types" + tmtypes "github.com/cometbft/cometbft/types" "github.com/cosmos/interchain-security/v3/legacy_ibc_testing/simapp/helpers" ) @@ -60,7 +64,7 @@ func ConvertAddrsToValAddrs(addrs []sdk.AccAddress) []sdk.ValAddress { return valAddrs } -func TestAddr(addr string, bech string) (sdk.AccAddress, error) { +func TestAddr(addr, bech string) (sdk.AccAddress, error) { res, err := sdk.AccAddressFromHexUnsafe(addr) if err != nil { return nil, err @@ -140,7 +144,7 @@ func NewPubKeyFromHex(pk string) (res cryptotypes.PubKey) { panic(err) } if len(pkBytes) != ed25519.PubKeySize { - panic(errors.Wrap(errors.ErrInvalidPubKey, "invalid pubkey size")) + panic(errorsmod.Wrap(errors.ErrInvalidPubKey, "invalid pubkey size")) } return &ed25519.PubKey{Key: pkBytes} } diff --git a/legacy_ibc_testing/testing/app.go b/legacy_ibc_testing/testing/app.go index db9da4bfb5..116d57b284 100644 --- a/legacy_ibc_testing/testing/app.go +++ b/legacy_ibc_testing/testing/app.go @@ -5,12 +5,11 @@ import ( "testing" "time" + "github.com/cosmos/ibc-go/v7/modules/core/keeper" + "github.com/stretchr/testify/require" + "cosmossdk.io/math" - "github.com/cosmos/interchain-security/v3/legacy_ibc_testing/core" - abci "github.com/cometbft/cometbft/abci/types" - tmproto "github.com/cometbft/cometbft/proto/tendermint/types" - tmtypes "github.com/cometbft/cometbft/types" "github.com/cosmos/cosmos-sdk/baseapp" "github.com/cosmos/cosmos-sdk/client" "github.com/cosmos/cosmos-sdk/codec" @@ -22,11 +21,13 @@ import ( banktypes "github.com/cosmos/cosmos-sdk/x/bank/types" capabilitykeeper "github.com/cosmos/cosmos-sdk/x/capability/keeper" stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types" - "github.com/stretchr/testify/require" - "github.com/cosmos/ibc-go/v7/modules/core/keeper" + abci "github.com/cometbft/cometbft/abci/types" + tmproto "github.com/cometbft/cometbft/proto/tendermint/types" + tmtypes "github.com/cometbft/cometbft/types" ccvtypes "github.com/cosmos/interchain-security/v3/x/ccv/types" + "github.com/cosmos/interchain-security/v3/legacy_ibc_testing/core" "github.com/cosmos/interchain-security/v3/legacy_ibc_testing/simapp" consumertypes "github.com/cosmos/interchain-security/v3/x/ccv/consumer/types" ) diff --git a/legacy_ibc_testing/testing/chain.go b/legacy_ibc_testing/testing/chain.go index 945c5b94d0..14177679b7 100644 --- a/legacy_ibc_testing/testing/chain.go +++ b/legacy_ibc_testing/testing/chain.go @@ -6,13 +6,18 @@ import ( "testing" "time" + clienttypes "github.com/cosmos/ibc-go/v7/modules/core/02-client/types" + channeltypes "github.com/cosmos/ibc-go/v7/modules/core/04-channel/types" + commitmenttypes "github.com/cosmos/ibc-go/v7/modules/core/23-commitment/types" + host "github.com/cosmos/ibc-go/v7/modules/core/24-host" + "github.com/cosmos/ibc-go/v7/modules/core/exported" + "github.com/cosmos/ibc-go/v7/modules/core/types" + ibctmtypes "github.com/cosmos/ibc-go/v7/modules/light-clients/07-tendermint" + "github.com/cosmos/ibc-go/v7/testing/mock" + "github.com/stretchr/testify/require" + errorsmod "cosmossdk.io/errors" - abci "github.com/cometbft/cometbft/abci/types" - "github.com/cometbft/cometbft/crypto/tmhash" - tmproto "github.com/cometbft/cometbft/proto/tendermint/types" - tmprotoversion "github.com/cometbft/cometbft/proto/tendermint/version" - tmtypes "github.com/cometbft/cometbft/types" - tmversion "github.com/cometbft/cometbft/version" + "github.com/cosmos/cosmos-sdk/client" "github.com/cosmos/cosmos-sdk/codec" "github.com/cosmos/cosmos-sdk/crypto/keys/secp256k1" @@ -24,16 +29,13 @@ import ( capabilitytypes "github.com/cosmos/cosmos-sdk/x/capability/types" teststaking "github.com/cosmos/cosmos-sdk/x/staking/testutil" stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types" - "github.com/stretchr/testify/require" - clienttypes "github.com/cosmos/ibc-go/v7/modules/core/02-client/types" - channeltypes "github.com/cosmos/ibc-go/v7/modules/core/04-channel/types" - commitmenttypes "github.com/cosmos/ibc-go/v7/modules/core/23-commitment/types" - host "github.com/cosmos/ibc-go/v7/modules/core/24-host" - "github.com/cosmos/ibc-go/v7/modules/core/exported" - "github.com/cosmos/ibc-go/v7/modules/core/types" - ibctmtypes "github.com/cosmos/ibc-go/v7/modules/light-clients/07-tendermint" - "github.com/cosmos/ibc-go/v7/testing/mock" + abci "github.com/cometbft/cometbft/abci/types" + "github.com/cometbft/cometbft/crypto/tmhash" + tmproto "github.com/cometbft/cometbft/proto/tendermint/types" + tmprotoversion "github.com/cometbft/cometbft/proto/tendermint/version" + tmtypes "github.com/cometbft/cometbft/types" + tmversion "github.com/cometbft/cometbft/version" ibctestingcore "github.com/cosmos/interchain-security/v3/legacy_ibc_testing/core" "github.com/cosmos/interchain-security/v3/legacy_ibc_testing/simapp" diff --git a/legacy_ibc_testing/testing/coordinator.go b/legacy_ibc_testing/testing/coordinator.go index 4aee8f8923..13f5ac1bec 100644 --- a/legacy_ibc_testing/testing/coordinator.go +++ b/legacy_ibc_testing/testing/coordinator.go @@ -6,8 +6,9 @@ import ( "testing" "time" - abci "github.com/cometbft/cometbft/abci/types" "github.com/stretchr/testify/require" + + abci "github.com/cometbft/cometbft/abci/types" ) /* diff --git a/legacy_ibc_testing/testing/endpoint.go b/legacy_ibc_testing/testing/endpoint.go index c3f2ac9abb..ef9213d222 100644 --- a/legacy_ibc_testing/testing/endpoint.go +++ b/legacy_ibc_testing/testing/endpoint.go @@ -3,9 +3,6 @@ package testing import ( "fmt" - sdk "github.com/cosmos/cosmos-sdk/types" - "github.com/stretchr/testify/require" - clienttypes "github.com/cosmos/ibc-go/v7/modules/core/02-client/types" connectiontypes "github.com/cosmos/ibc-go/v7/modules/core/03-connection/types" channeltypes "github.com/cosmos/ibc-go/v7/modules/core/04-channel/types" @@ -13,6 +10,9 @@ import ( host "github.com/cosmos/ibc-go/v7/modules/core/24-host" "github.com/cosmos/ibc-go/v7/modules/core/exported" ibctmtypes "github.com/cosmos/ibc-go/v7/modules/light-clients/07-tendermint" + "github.com/stretchr/testify/require" + + sdk "github.com/cosmos/cosmos-sdk/types" ) /* diff --git a/legacy_ibc_testing/testing/events.go b/legacy_ibc_testing/testing/events.go index 763199a695..4bab444cbb 100644 --- a/legacy_ibc_testing/testing/events.go +++ b/legacy_ibc_testing/testing/events.go @@ -3,11 +3,11 @@ package testing import ( "fmt" - sdk "github.com/cosmos/cosmos-sdk/types" - clienttypes "github.com/cosmos/ibc-go/v7/modules/core/02-client/types" connectiontypes "github.com/cosmos/ibc-go/v7/modules/core/03-connection/types" channeltypes "github.com/cosmos/ibc-go/v7/modules/core/04-channel/types" + + sdk "github.com/cosmos/cosmos-sdk/types" ) /* @@ -23,8 +23,8 @@ func ParseClientIDFromEvents(events sdk.Events) (string, error) { for _, ev := range events { if ev.Type == clienttypes.EventTypeCreateClient { for _, attr := range ev.Attributes { - if string(attr.Key) == clienttypes.AttributeKeyClientID { - return string(attr.Value), nil + if attr.Key == clienttypes.AttributeKeyClientID { + return attr.Value, nil } } } @@ -39,8 +39,8 @@ func ParseConnectionIDFromEvents(events sdk.Events) (string, error) { if ev.Type == connectiontypes.EventTypeConnectionOpenInit || ev.Type == connectiontypes.EventTypeConnectionOpenTry { for _, attr := range ev.Attributes { - if string(attr.Key) == connectiontypes.AttributeKeyConnectionID { - return string(attr.Value), nil + if attr.Key == connectiontypes.AttributeKeyConnectionID { + return attr.Value, nil } } } @@ -54,8 +54,8 @@ func ParseChannelIDFromEvents(events sdk.Events) (string, error) { for _, ev := range events { if ev.Type == channeltypes.EventTypeChannelOpenInit || ev.Type == channeltypes.EventTypeChannelOpenTry { for _, attr := range ev.Attributes { - if string(attr.Key) == channeltypes.AttributeKeyChannelID { - return string(attr.Value), nil + if attr.Key == channeltypes.AttributeKeyChannelID { + return attr.Value, nil } } } @@ -69,7 +69,7 @@ func ParseAckFromEvents(events sdk.Events) ([]byte, error) { for _, ev := range events { if ev.Type == channeltypes.EventTypeWriteAck { for _, attr := range ev.Attributes { - if string(attr.Key) == channeltypes.AttributeKeyAck { + if attr.Key == channeltypes.AttributeKeyAck { //nolint:staticcheck // data return []byte(attr.Value), nil } } diff --git a/legacy_ibc_testing/testing/utils.go b/legacy_ibc_testing/testing/utils.go index 885958f25e..047793f455 100644 --- a/legacy_ibc_testing/testing/utils.go +++ b/legacy_ibc_testing/testing/utils.go @@ -3,9 +3,10 @@ package testing import ( "testing" + "github.com/stretchr/testify/require" + abci "github.com/cometbft/cometbft/abci/types" tmtypes "github.com/cometbft/cometbft/types" - "github.com/stretchr/testify/require" ) /*