Skip to content

Commit

Permalink
Enforce solo machine signature type uniqueness (#7394)
Browse files Browse the repository at this point in the history
* update solo machine proto types to use enum for uniqueness

* move data type to SignatureAndData

Adjusts SignatureAndData proto definition to take in a DataType. Updates misbehaviour basic validation to do checks on the data type. Adds unmarshaling tests.

* split signature bytes creation to allow for function reusing. Stuck on strange error on testing codec.go

* fix test bug

* update UnmarshalByType and refactor misbehaviour handle

Rename CanUnmarshalDataByType -> UnmarshalDataByType. Return a new interface and error. Refactor tests to work. Refactor misbehaviour_handle.go to check unmarshaling of the data and DRY code by separating signature and data checks into its own function. Update godoc.

* add tests to codec_test.go

* self review + lint

* update spec

* fix lint

* Update x/ibc/light-clients/solomachine/spec/01_concepts.md

Co-authored-by: Federico Kunze <31522760+fedekunze@users.noreply.github.com>

* increase code cov, update spec

apply most of @fedekunze comments.

* format spec

* make proto

* fix merge conflicts

* make proto

* fix conflicts

Co-authored-by: Federico Kunze <31522760+fedekunze@users.noreply.github.com>
Co-authored-by: Federico Kunze <federico.kunze94@gmail.com>
  • Loading branch information
3 people authored Oct 1, 2020
1 parent dcf3b54 commit a32e2a0
Show file tree
Hide file tree
Showing 16 changed files with 989 additions and 233 deletions.
58 changes: 44 additions & 14 deletions proto/ibc/lightclients/solomachine/v1/solomachine.proto
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,9 @@ message Misbehaviour {
// signature.
message SignatureAndData {
option (gogoproto.goproto_getters) = false;
bytes signature = 1;
bytes data = 2;
bytes signature = 1;
DataType data_type = 2 [(gogoproto.moretags) = "yaml:\"data_type\""];
bytes data = 3;
}

// TimestampedSignature contains the signature and the timestamp of the
Expand All @@ -79,11 +80,40 @@ message SignBytes {
uint64 sequence = 1;
uint64 timestamp = 2;
string diversifier = 3;
// type of the data used
DataType data_type = 4 [(gogoproto.moretags) = "yaml:\"data_type\""];
// marshaled data
bytes data = 4;
}

// HeaderData returns the SignBytes data for misbehaviour verification.
bytes data = 5;
}

// DataType defines the type of solo machine proof being created. This is done to preserve uniqueness of different
// data sign byte encodings.
enum DataType {
option (gogoproto.goproto_enum_prefix) = false;

// Default State
DATA_TYPE_UNINITIALIZED_UNSPECIFIED = 0 [(gogoproto.enumvalue_customname) = "UNSPECIFIED"];
// Data type for client state verification
DATA_TYPE_CLIENT_STATE = 1 [(gogoproto.enumvalue_customname) = "CLIENT"];
// Data type for consensus state verification
DATA_TYPE_CONSENSUS_STATE = 2 [(gogoproto.enumvalue_customname) = "CONSENSUS"];
// Data type for connection state verification
DATA_TYPE_CONNECTION_STATE = 3 [(gogoproto.enumvalue_customname) = "CONNECTION"];
// Data type for channel state verification
DATA_TYPE_CHANNEL_STATE = 4 [(gogoproto.enumvalue_customname) = "CHANNEL"];
// Data type for packet commitment verification
DATA_TYPE_PACKET_COMMITMENT = 5 [(gogoproto.enumvalue_customname) = "PACKETCOMMITMENT"];
// Data type for packet acknowledgement verification
DATA_TYPE_PACKET_ACKNOWLEDGEMENT = 6 [(gogoproto.enumvalue_customname) = "PACKETACKNOWLEDGEMENT"];
// Data type for packet receipt absence verification
DATA_TYPE_PACKET_RECEIPT_ABSENCE = 7 [(gogoproto.enumvalue_customname) = "PACKETRECEIPTABSENCE"];
// Data type for next sequence recv verification
DATA_TYPE_NEXT_SEQUENCE_RECV = 8 [(gogoproto.enumvalue_customname) = "NEXTSEQUENCERECV"];
// Data type for header verification
DATA_TYPE_HEADER = 9 [(gogoproto.enumvalue_customname) = "HEADER"];
}

// HeaderData returns the SignBytes data for update verification.
message HeaderData {
option (gogoproto.goproto_getters) = false;

Expand All @@ -101,7 +131,7 @@ message ClientStateData {
google.protobuf.Any client_state = 2 [(gogoproto.moretags) = "yaml:\"client_state\""];
}

// ConsensusStateSignBytes returns the SignBytes data for consensus state
// ConsensusStateData returns the SignBytes data for consensus state
// verification.
message ConsensusStateData {
option (gogoproto.goproto_getters) = false;
Expand All @@ -110,7 +140,7 @@ message ConsensusStateData {
google.protobuf.Any consensus_state = 2 [(gogoproto.moretags) = "yaml:\"consensus_state\""];
}

// ConnectionStateSignBytes returns the SignBytes data for connection state
// ConnectionStateData returns the SignBytes data for connection state
// verification.
message ConnectionStateData {
option (gogoproto.goproto_getters) = false;
Expand All @@ -119,7 +149,7 @@ message ConnectionStateData {
ibc.connection.ConnectionEnd connection = 2;
}

// ChannelStateSignBytes returns the SignBytes data for channel state
// ChannelStateData returns the SignBytes data for channel state
// verification.
message ChannelStateData {
option (gogoproto.goproto_getters) = false;
Expand All @@ -128,27 +158,27 @@ message ChannelStateData {
ibc.channel.Channel channel = 2;
}

// PacketCommitmentSignBytes returns the SignBytes data for packet commitment
// PacketCommitmentData returns the SignBytes data for packet commitment
// verification.
message PacketCommitmentData {
bytes path = 1;
bytes commitment = 2;
}

// PacketAcknowledgementSignBytes returns the SignBytes data for acknowledgement
// PacketAcknowledgementData returns the SignBytes data for acknowledgement
// verification.
message PacketAcknowledgementData {
bytes path = 1;
bytes acknowledgement = 2;
}

// PacketReceiptAbsenceSignBytes returns the SignBytes data for
// PacketReceiptAbsenceData returns the SignBytes data for
// packet receipt absence verification.
message PacketReceiptAbsenseData {
message PacketReceiptAbsenceData {
bytes path = 1;
}

// NextSequenceRecv returns the SignBytes data for verification of the next
// NextSequenceRecvData returns the SignBytes data for verification of the next
// sequence to be received.
message NextSequenceRecvData {
bytes path = 1;
Expand Down
29 changes: 18 additions & 11 deletions x/ibc/light-clients/06-solomachine/spec/01_concepts.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,18 @@ order: 1

## Proofs

A solo machine proof should verify that the solomachine public key signed
A solo machine proof should verify that the solomachine public key signed
over some specified data. The format for generating marshaled proofs for
the SDK's implementation of solo machine is as follows:

Construct the data using the associated protobuf definition and marshal it.

For example:

```go
data := &ClientStateData{
Path: []byte(path.String()),
ClientState: any,
Path: []byte(path.String()),
ClientState: any,
}

dataBz, err := cdc.MarshalBinaryBare(data)
Expand All @@ -25,27 +26,32 @@ dataBz, err := cdc.MarshalBinaryBare(data)
Construct the `SignBytes` and marshal it.

For example:

```go
signBytes := &SignBytes{
Sequence: sequence,
Timestamp: timestamp,
Diversifier: diversifier,
Data: dataBz,
Sequence: sequence,
Timestamp: timestamp,
Diversifier: diversifier,
DataType: CLIENT,
Data: dataBz,
}

signBz, err := cdc.MarshalBinaryBare(signBytes)
```

The helper functions in [proofs.go](../types/proofs.go) handle the above actions.
The `DataType` field is used to disambiguate what type of data was signed because
of potential proto encoding overlap.

Sign the sign bytes. Embed the signatures into either `SingleSignatureData` or
`MultiSignatureData`. Convert the `SignatureData` to proto and marshal it.
`MultiSignatureData`. Convert the `SignatureData` to proto and marshal it

For example:

```go
sig, err := key.Sign(signBz)
sigData := &signing.SingleSignatureData{
Signature: sig,
Signature: sig,
}

protoSigData := signing.SignatureDataToProto(sigData)
Expand All @@ -56,10 +62,11 @@ Construct a `TimestampedSignature` and marshal it. The marshaled result can be
passed in as the proof parameter to the verification functions.

For example:

```go
timestampedSignature := &types.TimestampedSignature{
Signature: sig,
Timestamp: solomachine.Time,
Signature: sig,
Timestamp: solomachine.Time,
}

proof, err := cdc.MarshalBinaryBare(timestampedSignature)
Expand Down
29 changes: 9 additions & 20 deletions x/ibc/light-clients/06-solomachine/types/client_state_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import (
connectiontypes "github.com/cosmos/cosmos-sdk/x/ibc/03-connection/types"
channeltypes "github.com/cosmos/cosmos-sdk/x/ibc/04-channel/types"
commitmenttypes "github.com/cosmos/cosmos-sdk/x/ibc/23-commitment/types"
host "github.com/cosmos/cosmos-sdk/x/ibc/24-host"
"github.com/cosmos/cosmos-sdk/x/ibc/exported"
"github.com/cosmos/cosmos-sdk/x/ibc/light-clients/06-solomachine/types"
ibctesting "github.com/cosmos/cosmos-sdk/x/ibc/testing"
Expand Down Expand Up @@ -85,10 +84,7 @@ func (suite *SoloMachineTestSuite) TestVerifyClientState() {
// create client for tendermint so we can use client state for verification
clientA, _ := suite.coordinator.SetupClients(suite.chainA, suite.chainB, ibctesting.Tendermint)
clientState := suite.chainA.GetClientState(clientA)

clientPrefixedPath := "clients/" + counterpartyClientIdentifier + "/" + host.ClientStatePath()
path, err := commitmenttypes.ApplyPrefix(prefix, clientPrefixedPath)
suite.Require().NoError(err)
path := suite.solomachine.GetClientStatePath(counterpartyClientIdentifier)

// test singlesig and multisig public keys
for _, solomachine := range []*ibctesting.Solomachine{suite.solomachine, suite.solomachineMulti} {
Expand Down Expand Up @@ -217,9 +213,7 @@ func (suite *SoloMachineTestSuite) TestVerifyClientConsensusState() {
consensusState, found := suite.chainA.GetConsensusState(clientA, clientState.GetLatestHeight())
suite.Require().True(found)

clientPrefixedPath := "clients/" + counterpartyClientIdentifier + "/" + host.ConsensusStatePath(consensusHeight)
path, err := commitmenttypes.ApplyPrefix(prefix, clientPrefixedPath)
suite.Require().NoError(err)
path := suite.solomachine.GetConsensusStatePath(counterpartyClientIdentifier, consensusHeight)

// test singlesig and multisig public keys
for _, solomachine := range []*ibctesting.Solomachine{suite.solomachine, suite.solomachineMulti} {
Expand Down Expand Up @@ -344,8 +338,7 @@ func (suite *SoloMachineTestSuite) TestVerifyConnectionState() {
counterparty := connectiontypes.NewCounterparty("clientB", testConnectionID, prefix)
conn := connectiontypes.NewConnectionEnd(connectiontypes.OPEN, "clientA", counterparty, []string{"1.0.0"})

path, err := commitmenttypes.ApplyPrefix(prefix, host.ConnectionPath(testConnectionID))
suite.Require().NoError(err)
path := suite.solomachine.GetConnectionStatePath(testConnectionID)

// test singlesig and multisig public keys
for _, solomachine := range []*ibctesting.Solomachine{suite.solomachine, suite.solomachineMulti} {
Expand Down Expand Up @@ -434,8 +427,7 @@ func (suite *SoloMachineTestSuite) TestVerifyChannelState() {
counterparty := channeltypes.NewCounterparty(testPortID, testChannelID)
ch := channeltypes.NewChannel(channeltypes.OPEN, channeltypes.ORDERED, counterparty, []string{testConnectionID}, "1.0.0")

path, err := commitmenttypes.ApplyPrefix(prefix, host.ChannelPath(testPortID, testChannelID))
suite.Require().NoError(err)
path := suite.solomachine.GetChannelStatePath(testPortID, testChannelID)

// test singlesig and multisig public keys
for _, solomachine := range []*ibctesting.Solomachine{suite.solomachine, suite.solomachineMulti} {
Expand Down Expand Up @@ -526,8 +518,7 @@ func (suite *SoloMachineTestSuite) TestVerifyPacketCommitment() {
// test singlesig and multisig public keys
for _, solomachine := range []*ibctesting.Solomachine{suite.solomachine, suite.solomachineMulti} {

path, err := commitmenttypes.ApplyPrefix(prefix, host.PacketCommitmentPath(testPortID, testChannelID, solomachine.Sequence))
suite.Require().NoError(err)
path := solomachine.GetPacketCommitmentPath(testPortID, testChannelID)

value, err := types.PacketCommitmentSignBytes(suite.chainA.Codec, solomachine.Sequence, solomachine.Time, solomachine.Diversifier, path, commitmentBytes)
suite.Require().NoError(err)
Expand Down Expand Up @@ -614,8 +605,7 @@ func (suite *SoloMachineTestSuite) TestVerifyPacketAcknowledgement() {
// test singlesig and multisig public keys
for _, solomachine := range []*ibctesting.Solomachine{suite.solomachine, suite.solomachineMulti} {

path, err := commitmenttypes.ApplyPrefix(prefix, host.PacketAcknowledgementPath(testPortID, testChannelID, solomachine.Sequence))
suite.Require().NoError(err)
path := solomachine.GetPacketAcknowledgementPath(testPortID, testChannelID)

value, err := types.PacketAcknowledgementSignBytes(suite.chainA.Codec, solomachine.Sequence, solomachine.Time, solomachine.Diversifier, path, ack)
suite.Require().NoError(err)
Expand Down Expand Up @@ -701,8 +691,8 @@ func (suite *SoloMachineTestSuite) TestVerifyPacketReceiptAbsence() {
// test singlesig and multisig public keys
for _, solomachine := range []*ibctesting.Solomachine{suite.solomachine, suite.solomachineMulti} {

path, err := commitmenttypes.ApplyPrefix(prefix, host.PacketReceiptPath(testPortID, testChannelID, solomachine.Sequence))
suite.Require().NoError(err)
// absence uses receipt path as well
path := solomachine.GetPacketReceiptPath(testPortID, testChannelID)

value, err := types.PacketReceiptAbsenceSignBytes(suite.chainA.Codec, solomachine.Sequence, solomachine.Time, solomachine.Diversifier, path)
suite.Require().NoError(err)
Expand Down Expand Up @@ -789,8 +779,7 @@ func (suite *SoloMachineTestSuite) TestVerifyNextSeqRecv() {
for _, solomachine := range []*ibctesting.Solomachine{suite.solomachine, suite.solomachineMulti} {

nextSeqRecv := solomachine.Sequence + 1
path, err := commitmenttypes.ApplyPrefix(prefix, host.NextSequenceRecvPath(testPortID, testChannelID))
suite.Require().NoError(err)
path := solomachine.GetNextSequenceRecvPath(testPortID, testChannelID)

value, err := types.NextSequenceRecvSignBytes(suite.chainA.Codec, solomachine.Sequence, solomachine.Time, solomachine.Diversifier, path, nextSeqRecv)
suite.Require().NoError(err)
Expand Down
89 changes: 89 additions & 0 deletions x/ibc/light-clients/06-solomachine/types/codec.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
codectypes "github.com/cosmos/cosmos-sdk/codec/types"
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
"github.com/cosmos/cosmos-sdk/types/tx/signing"
clienttypes "github.com/cosmos/cosmos-sdk/x/ibc/02-client/types"
"github.com/cosmos/cosmos-sdk/x/ibc/exported"
)

Expand Down Expand Up @@ -48,3 +49,91 @@ func UnmarshalSignatureData(cdc codec.BinaryMarshaler, data []byte) (signing.Sig

return sigData, nil
}

// UnmarshalDataByType attempts to unmarshal the data to the specified type. An error is
// return if it fails.
func UnmarshalDataByType(cdc codec.BinaryMarshaler, dataType DataType, data []byte) (Data, error) {
if len(data) == 0 {
return nil, sdkerrors.Wrap(ErrInvalidSignatureAndData, "data cannot be empty")
}

switch dataType {
case UNSPECIFIED:
return nil, sdkerrors.Wrap(ErrInvalidDataType, "data type cannot be UNSPECIFIED")

case CLIENT:
clientData := &ClientStateData{}
if err := cdc.UnmarshalBinaryBare(data, clientData); err != nil {
return nil, err
}

// unpack any
if _, err := clienttypes.UnpackClientState(clientData.ClientState); err != nil {
return nil, err
}
return clientData, nil

case CONSENSUS:
consensusData := &ConsensusStateData{}
if err := cdc.UnmarshalBinaryBare(data, consensusData); err != nil {
return nil, err
}

// unpack any
if _, err := clienttypes.UnpackConsensusState(consensusData.ConsensusState); err != nil {
return nil, err
}
return consensusData, nil

case CONNECTION:
connectionData := &ConnectionStateData{}
if err := cdc.UnmarshalBinaryBare(data, connectionData); err != nil {
return nil, err
}

return connectionData, nil

case CHANNEL:
channelData := &ChannelStateData{}
if err := cdc.UnmarshalBinaryBare(data, channelData); err != nil {
return nil, err
}

return channelData, nil

case PACKETCOMMITMENT:
commitmentData := &PacketCommitmentData{}
if err := cdc.UnmarshalBinaryBare(data, commitmentData); err != nil {
return nil, err
}

return commitmentData, nil

case PACKETACKNOWLEDGEMENT:
ackData := &PacketAcknowledgementData{}
if err := cdc.UnmarshalBinaryBare(data, ackData); err != nil {
return nil, err
}

return ackData, nil

case PACKETRECEIPTABSENCE:
receiptAbsenceData := &PacketReceiptAbsenceData{}
if err := cdc.UnmarshalBinaryBare(data, receiptAbsenceData); err != nil {
return nil, err
}

return receiptAbsenceData, nil

case NEXTSEQUENCERECV:
nextSeqRecvData := &NextSequenceRecvData{}
if err := cdc.UnmarshalBinaryBare(data, nextSeqRecvData); err != nil {
return nil, err
}

return nextSeqRecvData, nil

default:
return nil, sdkerrors.Wrapf(ErrInvalidDataType, "unsupported data type %T", dataType)
}
}
Loading

0 comments on commit a32e2a0

Please sign in to comment.