solo machine GetPubKey returns an error instead of panicing (#7883)

* solo machine public key now returns an error instead of panicing

* apply @fedekunze review suggestions

Co-authored-by: Federico Kunze <31522760+fedekunze@users.noreply.github.com>
This commit is contained in:
colin axnér 2020-11-11 11:09:57 +01:00 committed by GitHub
parent 0bd46574f4
commit 116d0460fc
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 116 additions and 48 deletions

View File

@ -4,6 +4,7 @@ import (
ics23 "github.com/confio/ics23/go"
"github.com/cosmos/cosmos-sdk/codec"
cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types"
sdk "github.com/cosmos/cosmos-sdk/types"
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
"github.com/cosmos/cosmos-sdk/types/tx/signing"
@ -95,7 +96,7 @@ func (cs ClientState) VerifyClientState(
proof []byte,
clientState exported.ClientState,
) error {
sigData, timestamp, sequence, err := produceVerificationArgs(cdc, cs, height, prefix, proof)
publicKey, sigData, timestamp, sequence, err := produceVerificationArgs(cdc, cs, height, prefix, proof)
if err != nil {
return err
}
@ -111,7 +112,7 @@ func (cs ClientState) VerifyClientState(
return err
}
if err := VerifySignature(cs.ConsensusState.GetPubKey(), signBz, sigData); err != nil {
if err := VerifySignature(publicKey, signBz, sigData); err != nil {
return err
}
@ -133,7 +134,7 @@ func (cs ClientState) VerifyClientConsensusState(
proof []byte,
consensusState exported.ConsensusState,
) error {
sigData, timestamp, sequence, err := produceVerificationArgs(cdc, cs, height, prefix, proof)
publicKey, sigData, timestamp, sequence, err := produceVerificationArgs(cdc, cs, height, prefix, proof)
if err != nil {
return err
}
@ -149,7 +150,7 @@ func (cs ClientState) VerifyClientConsensusState(
return err
}
if err := VerifySignature(cs.ConsensusState.GetPubKey(), signBz, sigData); err != nil {
if err := VerifySignature(publicKey, signBz, sigData); err != nil {
return err
}
@ -170,7 +171,7 @@ func (cs ClientState) VerifyConnectionState(
connectionID string,
connectionEnd exported.ConnectionI,
) error {
sigData, timestamp, sequence, err := produceVerificationArgs(cdc, cs, height, prefix, proof)
publicKey, sigData, timestamp, sequence, err := produceVerificationArgs(cdc, cs, height, prefix, proof)
if err != nil {
return err
}
@ -186,7 +187,7 @@ func (cs ClientState) VerifyConnectionState(
return err
}
if err := VerifySignature(cs.ConsensusState.GetPubKey(), signBz, sigData); err != nil {
if err := VerifySignature(publicKey, signBz, sigData); err != nil {
return err
}
@ -208,7 +209,7 @@ func (cs ClientState) VerifyChannelState(
channelID string,
channel exported.ChannelI,
) error {
sigData, timestamp, sequence, err := produceVerificationArgs(cdc, cs, height, prefix, proof)
publicKey, sigData, timestamp, sequence, err := produceVerificationArgs(cdc, cs, height, prefix, proof)
if err != nil {
return err
}
@ -224,7 +225,7 @@ func (cs ClientState) VerifyChannelState(
return err
}
if err := VerifySignature(cs.ConsensusState.GetPubKey(), signBz, sigData); err != nil {
if err := VerifySignature(publicKey, signBz, sigData); err != nil {
return err
}
@ -247,7 +248,7 @@ func (cs ClientState) VerifyPacketCommitment(
packetSequence uint64,
commitmentBytes []byte,
) error {
sigData, timestamp, sequence, err := produceVerificationArgs(cdc, cs, height, prefix, proof)
publicKey, sigData, timestamp, sequence, err := produceVerificationArgs(cdc, cs, height, prefix, proof)
if err != nil {
return err
}
@ -263,7 +264,7 @@ func (cs ClientState) VerifyPacketCommitment(
return err
}
if err := VerifySignature(cs.ConsensusState.GetPubKey(), signBz, sigData); err != nil {
if err := VerifySignature(publicKey, signBz, sigData); err != nil {
return err
}
@ -286,7 +287,7 @@ func (cs ClientState) VerifyPacketAcknowledgement(
packetSequence uint64,
acknowledgement []byte,
) error {
sigData, timestamp, sequence, err := produceVerificationArgs(cdc, cs, height, prefix, proof)
publicKey, sigData, timestamp, sequence, err := produceVerificationArgs(cdc, cs, height, prefix, proof)
if err != nil {
return err
}
@ -302,7 +303,7 @@ func (cs ClientState) VerifyPacketAcknowledgement(
return err
}
if err := VerifySignature(cs.ConsensusState.GetPubKey(), signBz, sigData); err != nil {
if err := VerifySignature(publicKey, signBz, sigData); err != nil {
return err
}
@ -325,7 +326,7 @@ func (cs ClientState) VerifyPacketReceiptAbsence(
channelID string,
packetSequence uint64,
) error {
sigData, timestamp, sequence, err := produceVerificationArgs(cdc, cs, height, prefix, proof)
publicKey, sigData, timestamp, sequence, err := produceVerificationArgs(cdc, cs, height, prefix, proof)
if err != nil {
return err
}
@ -341,7 +342,7 @@ func (cs ClientState) VerifyPacketReceiptAbsence(
return err
}
if err := VerifySignature(cs.ConsensusState.GetPubKey(), signBz, sigData); err != nil {
if err := VerifySignature(publicKey, signBz, sigData); err != nil {
return err
}
@ -363,7 +364,7 @@ func (cs ClientState) VerifyNextSequenceRecv(
channelID string,
nextSequenceRecv uint64,
) error {
sigData, timestamp, sequence, err := produceVerificationArgs(cdc, cs, height, prefix, proof)
publicKey, sigData, timestamp, sequence, err := produceVerificationArgs(cdc, cs, height, prefix, proof)
if err != nil {
return err
}
@ -379,7 +380,7 @@ func (cs ClientState) VerifyNextSequenceRecv(
return err
}
if err := VerifySignature(cs.ConsensusState.GetPubKey(), signBz, sigData); err != nil {
if err := VerifySignature(publicKey, signBz, sigData); err != nil {
return err
}
@ -390,71 +391,76 @@ func (cs ClientState) VerifyNextSequenceRecv(
}
// produceVerificationArgs perfoms the basic checks on the arguments that are
// shared between the verification functions and returns the unmarshalled
// proof representing the signature and timestamp along with the solo-machine sequence
// encoded in the proofHeight.
// shared between the verification functions and returns the public key of the
// consensus state, the unmarshalled proof representing the signature and timestamp
// along with the solo-machine sequence encoded in the proofHeight.
func produceVerificationArgs(
cdc codec.BinaryMarshaler,
cs ClientState,
height exported.Height,
prefix exported.Prefix,
proof []byte,
) (signing.SignatureData, uint64, uint64, error) {
) (cryptotypes.PubKey, signing.SignatureData, uint64, uint64, error) {
if version := height.GetVersionNumber(); version != 0 {
return nil, 0, 0, sdkerrors.Wrapf(sdkerrors.ErrInvalidHeight, "version must be 0 for solomachine, got version-number: %d", version)
return nil, nil, 0, 0, sdkerrors.Wrapf(sdkerrors.ErrInvalidHeight, "version must be 0 for solomachine, got version-number: %d", version)
}
// sequence is encoded in the version height of height struct
sequence := height.GetVersionHeight()
if cs.IsFrozen() {
return nil, 0, 0, clienttypes.ErrClientFrozen
return nil, nil, 0, 0, clienttypes.ErrClientFrozen
}
if prefix == nil {
return nil, 0, 0, sdkerrors.Wrap(commitmenttypes.ErrInvalidPrefix, "prefix cannot be empty")
return nil, nil, 0, 0, sdkerrors.Wrap(commitmenttypes.ErrInvalidPrefix, "prefix cannot be empty")
}
_, ok := prefix.(commitmenttypes.MerklePrefix)
if !ok {
return nil, 0, 0, sdkerrors.Wrapf(commitmenttypes.ErrInvalidPrefix, "invalid prefix type %T, expected MerklePrefix", prefix)
return nil, nil, 0, 0, sdkerrors.Wrapf(commitmenttypes.ErrInvalidPrefix, "invalid prefix type %T, expected MerklePrefix", prefix)
}
if proof == nil {
return nil, 0, 0, sdkerrors.Wrap(ErrInvalidProof, "proof cannot be empty")
return nil, nil, 0, 0, sdkerrors.Wrap(ErrInvalidProof, "proof cannot be empty")
}
timestampedSigData := &TimestampedSignatureData{}
if err := cdc.UnmarshalBinaryBare(proof, timestampedSigData); err != nil {
return nil, 0, 0, sdkerrors.Wrapf(err, "failed to unmarshal proof into type %T", timestampedSigData)
return nil, nil, 0, 0, sdkerrors.Wrapf(err, "failed to unmarshal proof into type %T", timestampedSigData)
}
timestamp := timestampedSigData.Timestamp
if len(timestampedSigData.SignatureData) == 0 {
return nil, 0, 0, sdkerrors.Wrap(ErrInvalidProof, "signature data cannot be empty")
return nil, nil, 0, 0, sdkerrors.Wrap(ErrInvalidProof, "signature data cannot be empty")
}
sigData, err := UnmarshalSignatureData(cdc, timestampedSigData.SignatureData)
if err != nil {
return nil, 0, 0, err
return nil, nil, 0, 0, err
}
if cs.ConsensusState == nil {
return nil, 0, 0, sdkerrors.Wrap(clienttypes.ErrInvalidConsensus, "consensus state cannot be empty")
return nil, nil, 0, 0, sdkerrors.Wrap(clienttypes.ErrInvalidConsensus, "consensus state cannot be empty")
}
latestSequence := cs.GetLatestHeight().GetVersionHeight()
if latestSequence != sequence {
return nil, 0, 0, sdkerrors.Wrapf(
return nil, nil, 0, 0, sdkerrors.Wrapf(
sdkerrors.ErrInvalidHeight,
"client state sequence != proof sequence (%d != %d)", latestSequence, sequence,
)
}
if cs.ConsensusState.GetTimestamp() > timestamp {
return nil, 0, 0, sdkerrors.Wrapf(ErrInvalidProof, "the consensus state timestamp is greater than the signature timestamp (%d >= %d)", cs.ConsensusState.GetTimestamp(), timestamp)
return nil, nil, 0, 0, sdkerrors.Wrapf(ErrInvalidProof, "the consensus state timestamp is greater than the signature timestamp (%d >= %d)", cs.ConsensusState.GetTimestamp(), timestamp)
}
return sigData, timestamp, sequence, nil
publicKey, err := cs.ConsensusState.GetPubKey()
if err != nil {
return nil, nil, 0, 0, err
}
return publicKey, sigData, timestamp, sequence, nil
}
// sets the client state to the store

View File

@ -27,13 +27,19 @@ func (cs ConsensusState) GetRoot() exported.Root {
}
// GetPubKey unmarshals the public key into a cryptotypes.PubKey type.
func (cs ConsensusState) GetPubKey() cryptotypes.PubKey {
publicKey, ok := cs.PublicKey.GetCachedValue().(cryptotypes.PubKey)
if !ok {
panic("ConsensusState PublicKey is not cryptotypes.PubKey")
// An error is returned if the public key is nil or the cached value
// is not a PubKey.
func (cs ConsensusState) GetPubKey() (cryptotypes.PubKey, error) {
if cs.PublicKey == nil {
return nil, sdkerrors.Wrap(clienttypes.ErrInvalidConsensus, "consensus state PublicKey cannot be nil")
}
return publicKey
publicKey, ok := cs.PublicKey.GetCachedValue().(cryptotypes.PubKey)
if !ok {
return nil, sdkerrors.Wrap(clienttypes.ErrInvalidConsensus, "consensus state PublicKey is not cryptotypes.PubKey")
}
return publicKey, nil
}
// ValidateBasic defines basic validation for the solo machine consensus state.
@ -44,7 +50,9 @@ func (cs ConsensusState) ValidateBasic() error {
if cs.Diversifier != "" && strings.TrimSpace(cs.Diversifier) == "" {
return sdkerrors.Wrap(clienttypes.ErrInvalidConsensus, "diversifier cannot contain only spaces")
}
if cs.PublicKey == nil || cs.GetPubKey() == nil || len(cs.GetPubKey().Bytes()) == 0 {
publicKey, err := cs.GetPubKey()
if err != nil || publicKey == nil || len(publicKey.Bytes()) == 0 {
return sdkerrors.Wrap(clienttypes.ErrInvalidConsensus, "public key cannot be empty")
}

View File

@ -24,13 +24,19 @@ func (h Header) GetHeight() exported.Height {
}
// GetPubKey unmarshals the new public key into a cryptotypes.PubKey type.
func (h Header) GetPubKey() cryptotypes.PubKey {
publicKey, ok := h.NewPublicKey.GetCachedValue().(cryptotypes.PubKey)
if !ok {
panic("Header NewPublicKey is not cryptotypes.PubKey")
// An error is returned if the new public key is nil or the cached value
// is not a PubKey.
func (h Header) GetPubKey() (cryptotypes.PubKey, error) {
if h.NewPublicKey == nil {
return nil, sdkerrors.Wrap(ErrInvalidHeader, "header NewPublicKey cannot be nil")
}
return publicKey
publicKey, ok := h.NewPublicKey.GetCachedValue().(cryptotypes.PubKey)
if !ok {
return nil, sdkerrors.Wrap(ErrInvalidHeader, "header NewPublicKey is not cryptotypes.PubKey")
}
return publicKey, nil
}
// ValidateBasic ensures that the sequence, signature and public key have all
@ -52,7 +58,8 @@ func (h Header) ValidateBasic() error {
return sdkerrors.Wrap(clienttypes.ErrInvalidHeader, "signature cannot be empty")
}
if h.NewPublicKey == nil || h.GetPubKey() == nil || len(h.GetPubKey().Bytes()) == 0 {
newPublicKey, err := h.GetPubKey()
if err != nil || newPublicKey == nil || len(newPublicKey.Bytes()) == 0 {
return sdkerrors.Wrap(clienttypes.ErrInvalidHeader, "new public key cannot be empty")
}

View File

@ -79,7 +79,12 @@ func verifySignatureAndData(cdc codec.BinaryMarshaler, clientState ClientState,
return err
}
if err := VerifySignature(clientState.ConsensusState.GetPubKey(), data, sigData); err != nil {
publicKey, err := clientState.ConsensusState.GetPubKey()
if err != nil {
return err
}
if err := VerifySignature(publicKey, data, sigData); err != nil {
return err
}

View File

@ -241,6 +241,16 @@ func (suite *SoloMachineTestSuite) TestCheckMisbehaviourAndUpdateState() {
},
false,
},
{
"consensus state pubkey is nil",
func() {
cs := solomachine.ClientState()
cs.ConsensusState.PublicKey = nil
clientState = cs
misbehaviour = solomachine.CreateMisbehaviour()
},
false,
},
}
for _, tc := range testCases {

View File

@ -33,7 +33,17 @@ func (cs ClientState) CheckProposedHeaderAndUpdateState(
)
}
if reflect.DeepEqual(cs.ConsensusState.GetPubKey(), smHeader.GetPubKey()) {
consensusPublicKey, err := cs.ConsensusState.GetPubKey()
if err != nil {
return nil, nil, sdkerrors.Wrap(err, "failed to get consensus public key")
}
headerPublicKey, err := smHeader.GetPubKey()
if err != nil {
return nil, nil, sdkerrors.Wrap(err, "failed to get header public key")
}
if reflect.DeepEqual(consensusPublicKey, headerPublicKey) {
return nil, nil, sdkerrors.Wrapf(
clienttypes.ErrInvalidHeader, "new public key in header equals current public key",
)

View File

@ -69,7 +69,14 @@ func (suite *SoloMachineTestSuite) TestCheckProposedHeaderAndUpdateState() {
suite.Require().True(ok)
suite.Require().Equal(cs.(*types.ClientState).ConsensusState, consState)
suite.Require().Equal(smHeader.GetPubKey(), smConsState.GetPubKey())
headerPubKey, err := smHeader.GetPubKey()
suite.Require().NoError(err)
consStatePubKey, err := smConsState.GetPubKey()
suite.Require().NoError(err)
suite.Require().Equal(headerPubKey, consStatePubKey)
suite.Require().Equal(smHeader.NewDiversifier, smConsState.Diversifier)
suite.Require().Equal(smHeader.Timestamp, smConsState.Timestamp)
suite.Require().Equal(smHeader.GetHeight().GetVersionHeight(), cs.(*types.ClientState).Sequence)

View File

@ -62,7 +62,12 @@ func checkHeader(cdc codec.BinaryMarshaler, clientState *ClientState, header *He
return err
}
if err := VerifySignature(clientState.ConsensusState.GetPubKey(), data, sigData); err != nil {
publicKey, err := clientState.ConsensusState.GetPubKey()
if err != nil {
return err
}
if err := VerifySignature(publicKey, data, sigData); err != nil {
return sdkerrors.Wrap(ErrInvalidHeader, err.Error())
}

View File

@ -143,6 +143,16 @@ func (suite *SoloMachineTestSuite) TestCheckHeaderAndUpdateState() {
},
false,
},
{
"consensus state public key is nil",
func() {
cs := solomachine.ClientState()
cs.ConsensusState.PublicKey = nil
clientState = cs
header = solomachine.CreateHeader()
},
false,
},
}
for _, tc := range testCases {