From db77117a5b212867726b8af0043ca728fd35cde9 Mon Sep 17 00:00:00 2001 From: Hendrik Hofstadt Date: Thu, 29 Nov 2018 18:21:45 +0100 Subject: [PATCH] Only allow supported pubKey types (#2949) * Only allow supported pubKey types * Add type and supported types to error message * Add default value for ConsensusParams --- PENDING.md | 1 + baseapp/baseapp.go | 3 ++- types/context.go | 10 ++++++++++ x/stake/handler.go | 12 +++++++++++- x/stake/handler_test.go | 23 +++++++++++++++++++++++ x/stake/keeper/test_common.go | 2 ++ x/stake/stake.go | 19 ++++++++++--------- x/stake/types/errors.go | 6 ++++++ 8 files changed, 65 insertions(+), 11 deletions(-) diff --git a/PENDING.md b/PENDING.md index b9b7e28c7..bca868153 100644 --- a/PENDING.md +++ b/PENDING.md @@ -24,6 +24,7 @@ BREAKING CHANGES * [\#2801](https://github.com/cosmos/cosmos-sdk/pull/2801) Remove AppInit structure. * [\#2798](https://github.com/cosmos/cosmos-sdk/issues/2798) Governance API has miss-spelled English word in JSON response ('depositer' -> 'depositor') * [\#2943](https://github.com/cosmos/cosmos-sdk/pull/2943) Transaction action tags equal the message type. Stake EndBlocker tags are included. + * [\#2945](https://github.com/cosmos/cosmos-sdk/issues/2945) CreateValidator pubKey type is now validated against `ConsensusParams` * Tendermint - Update to Tendermint 0.27.0 diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index ffd895f64..eeeea53da 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -587,7 +587,8 @@ func validateBasicTxMsgs(msgs []sdk.Msg) sdk.Error { func (app *BaseApp) getContextForTx(mode runTxMode, txBytes []byte) (ctx sdk.Context) { ctx = app.getState(mode).ctx. WithTxBytes(txBytes). - WithVoteInfos(app.voteInfos) + WithVoteInfos(app.voteInfos). + WithConsensusParams(app.consensusParams) if mode == runTxModeSimulate { ctx, _ = ctx.CacheContext() } diff --git a/types/context.go b/types/context.go index add88bfc3..71e1f5303 100644 --- a/types/context.go +++ b/types/context.go @@ -48,6 +48,7 @@ func NewContext(ms MultiStore, header abci.Header, isCheckTx bool, logger log.Lo c = c.WithVoteInfos(nil) c = c.WithGasMeter(NewInfiniteGasMeter()) c = c.WithMinimumFees(Coins{}) + c = c.WithConsensusParams(nil) return c } @@ -141,6 +142,7 @@ const ( contextKeyGasMeter contextKeyBlockGasMeter contextKeyMinimumFees + contextKeyConsensusParams ) func (c Context) MultiStore() MultiStore { @@ -169,6 +171,10 @@ func (c Context) IsCheckTx() bool { return c.Value(contextKeyIsCheckTx).(bool) } func (c Context) MinimumFees() Coins { return c.Value(contextKeyMinimumFees).(Coins) } +func (c Context) ConsensusParams() *abci.ConsensusParams { + return c.Value(contextKeyConsensusParams).(*abci.ConsensusParams) +} + func (c Context) WithMultiStore(ms MultiStore) Context { return c.withValue(contextKeyMultiStore, ms) } @@ -220,6 +226,10 @@ func (c Context) WithMinimumFees(minFees Coins) Context { return c.withValue(contextKeyMinimumFees, minFees) } +func (c Context) WithConsensusParams(params *abci.ConsensusParams) Context { + return c.withValue(contextKeyConsensusParams, params) +} + // Cache the multistore and return a new cached context. The cached context is // written to the context when writeCache is called. func (c Context) CacheContext() (cc Context, writeCache func()) { diff --git a/x/stake/handler.go b/x/stake/handler.go index 72002ce7b..811fe7114 100644 --- a/x/stake/handler.go +++ b/x/stake/handler.go @@ -3,11 +3,14 @@ package stake import ( "bytes" + abci "github.com/tendermint/tendermint/abci/types" + "github.com/tendermint/tendermint/libs/common" + tmtypes "github.com/tendermint/tendermint/types" + sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/x/stake/keeper" "github.com/cosmos/cosmos-sdk/x/stake/tags" "github.com/cosmos/cosmos-sdk/x/stake/types" - abci "github.com/tendermint/tendermint/abci/types" ) func NewHandler(k keeper.Keeper) sdk.Handler { @@ -101,6 +104,13 @@ func handleMsgCreateValidator(ctx sdk.Context, msg types.MsgCreateValidator, k k return ErrBadDenom(k.Codespace()).Result() } + if ctx.ConsensusParams() != nil { + tmPubKey := tmtypes.TM2PB.PubKey(msg.PubKey) + if !common.StringInSlice(tmPubKey.Type, ctx.ConsensusParams().Validator.PubKeyTypes) { + return ErrValidatorPubKeyTypeUnsupported(k.Codespace(), tmPubKey.Type, ctx.ConsensusParams().Validator.PubKeyTypes).Result() + } + } + validator := NewValidator(msg.ValidatorAddr, msg.PubKey, msg.Description) commission := NewCommissionWithTime( msg.Commission.Rate, msg.Commission.MaxRate, diff --git a/x/stake/handler_test.go b/x/stake/handler_test.go index 81d03de05..2c82ee7ad 100644 --- a/x/stake/handler_test.go +++ b/x/stake/handler_test.go @@ -7,6 +7,10 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + abci "github.com/tendermint/tendermint/abci/types" + "github.com/tendermint/tendermint/crypto/secp256k1" + tmtypes "github.com/tendermint/tendermint/types" + sdk "github.com/cosmos/cosmos-sdk/types" keep "github.com/cosmos/cosmos-sdk/x/stake/keeper" "github.com/cosmos/cosmos-sdk/x/stake/types" @@ -157,6 +161,25 @@ func TestDuplicatesMsgCreateValidator(t *testing.T) { assert.Equal(t, Description{}, validator.Description) } +func TestInvalidPubKeyTypeMsgCreateValidator(t *testing.T) { + ctx, _, keeper := keep.CreateTestInput(t, false, 1000) + + addr := sdk.ValAddress(keep.Addrs[0]) + invalidPk := secp256k1.GenPrivKey().PubKey() + + // invalid pukKey type should not be allowed + msgCreateValidator := NewTestMsgCreateValidator(addr, invalidPk, 10) + got := handleMsgCreateValidator(ctx, msgCreateValidator, keeper) + require.False(t, got.IsOK(), "%v", got) + + ctx = ctx.WithConsensusParams(&abci.ConsensusParams{ + Validator: &abci.ValidatorParams{PubKeyTypes: []string{tmtypes.ABCIPubKeyTypeSecp256k1}}, + }) + + got = handleMsgCreateValidator(ctx, msgCreateValidator, keeper) + require.True(t, got.IsOK(), "%v", got) +} + func TestDuplicatesMsgCreateValidatorOnBehalfOf(t *testing.T) { ctx, _, keeper := keep.CreateTestInput(t, false, 1000) diff --git a/x/stake/keeper/test_common.go b/x/stake/keeper/test_common.go index 65445cbb1..5a501f721 100644 --- a/x/stake/keeper/test_common.go +++ b/x/stake/keeper/test_common.go @@ -14,6 +14,7 @@ import ( "github.com/tendermint/tendermint/crypto/ed25519" dbm "github.com/tendermint/tendermint/libs/db" "github.com/tendermint/tendermint/libs/log" + tmtypes "github.com/tendermint/tendermint/types" "github.com/cosmos/cosmos-sdk/codec" "github.com/cosmos/cosmos-sdk/store" @@ -93,6 +94,7 @@ func CreateTestInput(t *testing.T, isCheckTx bool, initCoins int64) (sdk.Context require.Nil(t, err) ctx := sdk.NewContext(ms, abci.Header{ChainID: "foochainid"}, isCheckTx, log.NewNopLogger()) + ctx = ctx.WithConsensusParams(&abci.ConsensusParams{Validator: &abci.ValidatorParams{PubKeyTypes: []string{tmtypes.ABCIPubKeyTypeEd25519}}}) cdc := MakeTestCodec() accountKeeper := auth.NewAccountKeeper( cdc, // amino codec diff --git a/x/stake/stake.go b/x/stake/stake.go index fc1fb55ee..2e5b8feb7 100644 --- a/x/stake/stake.go +++ b/x/stake/stake.go @@ -118,15 +118,16 @@ const ( ) var ( - ErrNilValidatorAddr = types.ErrNilValidatorAddr - ErrNoValidatorFound = types.ErrNoValidatorFound - ErrValidatorOwnerExists = types.ErrValidatorOwnerExists - ErrValidatorPubKeyExists = types.ErrValidatorPubKeyExists - ErrValidatorJailed = types.ErrValidatorJailed - ErrBadRemoveValidator = types.ErrBadRemoveValidator - ErrDescriptionLength = types.ErrDescriptionLength - ErrCommissionNegative = types.ErrCommissionNegative - ErrCommissionHuge = types.ErrCommissionHuge + ErrNilValidatorAddr = types.ErrNilValidatorAddr + ErrNoValidatorFound = types.ErrNoValidatorFound + ErrValidatorOwnerExists = types.ErrValidatorOwnerExists + ErrValidatorPubKeyExists = types.ErrValidatorPubKeyExists + ErrValidatorPubKeyTypeUnsupported = types.ErrValidatorPubKeyTypeNotSupported + ErrValidatorJailed = types.ErrValidatorJailed + ErrBadRemoveValidator = types.ErrBadRemoveValidator + ErrDescriptionLength = types.ErrDescriptionLength + ErrCommissionNegative = types.ErrCommissionNegative + ErrCommissionHuge = types.ErrCommissionHuge ErrNilDelegatorAddr = types.ErrNilDelegatorAddr ErrBadDenom = types.ErrBadDenom diff --git a/x/stake/types/errors.go b/x/stake/types/errors.go index 57dc2fb00..c944b489b 100644 --- a/x/stake/types/errors.go +++ b/x/stake/types/errors.go @@ -3,6 +3,7 @@ package types import ( "fmt" + "strings" "time" sdk "github.com/cosmos/cosmos-sdk/types" @@ -44,6 +45,11 @@ func ErrValidatorPubKeyExists(codespace sdk.CodespaceType) sdk.Error { return sdk.NewError(codespace, CodeInvalidValidator, "validator already exist for this pubkey, must use new validator pubkey") } +func ErrValidatorPubKeyTypeNotSupported(codespace sdk.CodespaceType, keyType string, supportedTypes []string) sdk.Error { + msg := fmt.Sprintf("validator pubkey type %s is not supported, must use %s", keyType, strings.Join(supportedTypes, ",")) + return sdk.NewError(codespace, CodeInvalidValidator, msg) +} + func ErrValidatorJailed(codespace sdk.CodespaceType) sdk.Error { return sdk.NewError(codespace, CodeInvalidValidator, "validator for this address is currently jailed") }