feat: allowlist host param using msg typeURLs (#576)

* add ica params

Add new Params type to ICA. A single test is added to check defaults and validation.
Usage within the ICA keepers is still needed

* regenerate params proto into host and controller submodules

* split params implementation into host/controller

* add keeper params logic

* Apply suggestions from code review

Co-authored-by: Damian Nolan <damiannolan@gmail.com>

* add host genesis init/export params test case

* updating host proto params to include msg allowlist

* adding surrounds for new allowlist host param

* enforcing msg is present in allowlist in AuthenticateTx, updating tests

* regenerating protos post merge conflict

* applying suggestinons from review

* adding strings.Trimspace as suggested

Co-authored-by: Colin Axnér <25233464+colin-axner@users.noreply.github.com>

modules/apps/27-interchain-accounts/host/ibc_module_test.go

@@ -139,7 +139,7 @@ func (suite *InterchainAccountsTestSuite) TestOnChanOpenTry() {
 		},
 		{
 			"host submodule disabled", func() {
-				suite.chainB.GetSimApp().ICAHostKeeper.SetParams(suite.chainB.GetContext(), types.NewParams(false))
+				suite.chainB.GetSimApp().ICAHostKeeper.SetParams(suite.chainB.GetContext(), types.NewParams(false, []string{}))
 			}, false,
 		},
 		{
@@ -260,7 +260,7 @@ func (suite *InterchainAccountsTestSuite) TestOnChanOpenConfirm() {
 		},
 		{
 			"host submodule disabled", func() {
-				suite.chainB.GetSimApp().ICAHostKeeper.SetParams(suite.chainB.GetContext(), types.NewParams(false))
+				suite.chainB.GetSimApp().ICAHostKeeper.SetParams(suite.chainB.GetContext(), types.NewParams(false, []string{}))
 			}, false,
 		},
 		{
@@ -399,7 +399,7 @@ func (suite *InterchainAccountsTestSuite) TestOnRecvPacket() {
 		},
 		{
 			"host submodule disabled", func() {
-				suite.chainB.GetSimApp().ICAHostKeeper.SetParams(suite.chainB.GetContext(), types.NewParams(false))
+				suite.chainB.GetSimApp().ICAHostKeeper.SetParams(suite.chainB.GetContext(), types.NewParams(false, []string{}))
 			}, false,
 		},
 		{
@@ -452,6 +452,9 @@ func (suite *InterchainAccountsTestSuite) TestOnRecvPacket() {
 			}
 			packetData = icaPacketData.GetBytes()
 
+			params := types.NewParams(true, []string{sdk.MsgTypeURL(msg)})
+			suite.chainB.GetSimApp().ICAHostKeeper.SetParams(suite.chainB.GetContext(), params)
+
 			// malleate packetData for test cases
 			tc.malleate()
 

modules/apps/27-interchain-accounts/host/keeper/genesis_test.go

@@ -36,7 +36,7 @@ func (suite *KeeperTestSuite) TestInitGenesis() {
 	suite.Require().True(found)
 	suite.Require().Equal(TestAccAddress.String(), accountAdrr)
 
-	expParams := types.NewParams(false)
+	expParams := types.NewParams(false, nil)
 	params := suite.chainA.GetSimApp().ICAHostKeeper.GetParams(suite.chainA.GetContext())
 	suite.Require().Equal(expParams, params)
 }

modules/apps/27-interchain-accounts/host/keeper/params.go

@@ -14,9 +14,16 @@ func (k Keeper) IsHostEnabled(ctx sdk.Context) bool {
 	return res
 }
 
+// GetAllowMessages retrieves the host enabled msg types from the paramstore
+func (k Keeper) GetAllowMessages(ctx sdk.Context) []string {
+	var res []string
+	k.paramSpace.Get(ctx, types.KeyAllowMessages, &res)
+	return res
+}
+
 // GetParams returns the total set of the host submodule parameters.
 func (k Keeper) GetParams(ctx sdk.Context) types.Params {
-	return types.NewParams(k.IsHostEnabled(ctx))
+	return types.NewParams(k.IsHostEnabled(ctx), k.GetAllowMessages(ctx))
 }
 
 // SetParams sets the total set of the host submodule parameters.

modules/apps/27-interchain-accounts/host/keeper/params_test.go

@@ -9,6 +9,7 @@ func (suite *KeeperTestSuite) TestParams() {
 	suite.Require().Equal(expParams, params)
 
 	expParams.HostEnabled = false
+	expParams.AllowMessages = []string{"/cosmos.staking.v1beta1.MsgDelegate"}
 	suite.chainA.GetSimApp().ICAHostKeeper.SetParams(suite.chainA.GetContext(), expParams)
 	params = suite.chainA.GetSimApp().ICAHostKeeper.GetParams(suite.chainA.GetContext())
 	suite.Require().Equal(expParams, params)

modules/apps/27-interchain-accounts/host/keeper/relay.go

@@ -4,6 +4,7 @@ import (
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
 
+	"github.com/cosmos/ibc-go/v2/modules/apps/27-interchain-accounts/host/types"
 	icatypes "github.com/cosmos/ibc-go/v2/modules/apps/27-interchain-accounts/types"
 	channeltypes "github.com/cosmos/ibc-go/v2/modules/core/04-channel/types"
 )
@@ -16,7 +17,12 @@ func (k Keeper) AuthenticateTx(ctx sdk.Context, msgs []sdk.Msg, portID string) e
 		return sdkerrors.Wrapf(icatypes.ErrInterchainAccountNotFound, "failed to retrieve interchain account on port %s", portID)
 	}
 
+	allowMsgs := k.GetAllowMessages(ctx)
 	for _, msg := range msgs {
+		if !types.ContainsMsgType(allowMsgs, msg) {
+			return sdkerrors.Wrapf(sdkerrors.ErrUnauthorized, "message type not allowed: %s", sdk.MsgTypeURL(msg))
+		}
+
 		for _, signer := range msg.GetSigners() {
 			if interchainAccountAddr != signer.String() {
 				return sdkerrors.Wrapf(sdkerrors.ErrUnauthorized, "unexpected signer address: expected %s, got %s", interchainAccountAddr, signer.String())

modules/apps/27-interchain-accounts/host/keeper/relay_test.go

@@ -10,6 +10,7 @@ import (
 	govtypes "github.com/cosmos/cosmos-sdk/x/gov/types"
 	stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types"
 
+	"github.com/cosmos/ibc-go/v2/modules/apps/27-interchain-accounts/host/types"
 	icatypes "github.com/cosmos/ibc-go/v2/modules/apps/27-interchain-accounts/types"
 	transfertypes "github.com/cosmos/ibc-go/v2/modules/apps/transfer/types"
 	clienttypes "github.com/cosmos/ibc-go/v2/modules/core/02-client/types"
@@ -49,6 +50,9 @@ func (suite *KeeperTestSuite) TestOnRecvPacket() {
 				}
 
 				packetData = icaPacketData.GetBytes()
+
+				params := types.NewParams(true, []string{sdk.MsgTypeURL(msg)})
+				suite.chainB.GetSimApp().ICAHostKeeper.SetParams(suite.chainB.GetContext(), params)
 			},
 			true,
 		},
@@ -74,6 +78,9 @@ func (suite *KeeperTestSuite) TestOnRecvPacket() {
 				}
 
 				packetData = icaPacketData.GetBytes()
+
+				params := types.NewParams(true, []string{sdk.MsgTypeURL(msg)})
+				suite.chainB.GetSimApp().ICAHostKeeper.SetParams(suite.chainB.GetContext(), params)
 			},
 			true,
 		},
@@ -105,6 +112,9 @@ func (suite *KeeperTestSuite) TestOnRecvPacket() {
 				}
 
 				packetData = icaPacketData.GetBytes()
+
+				params := types.NewParams(true, []string{sdk.MsgTypeURL(msgDelegate), sdk.MsgTypeURL(msgUndelegate)})
+				suite.chainB.GetSimApp().ICAHostKeeper.SetParams(suite.chainB.GetContext(), params)
 			},
 			true,
 		},
@@ -137,6 +147,9 @@ func (suite *KeeperTestSuite) TestOnRecvPacket() {
 				}
 
 				packetData = icaPacketData.GetBytes()
+
+				params := types.NewParams(true, []string{sdk.MsgTypeURL(msg)})
+				suite.chainB.GetSimApp().ICAHostKeeper.SetParams(suite.chainB.GetContext(), params)
 			},
 			true,
 		},
@@ -173,6 +186,9 @@ func (suite *KeeperTestSuite) TestOnRecvPacket() {
 				}
 
 				packetData = icaPacketData.GetBytes()
+
+				params := types.NewParams(true, []string{sdk.MsgTypeURL(msg)})
+				suite.chainB.GetSimApp().ICAHostKeeper.SetParams(suite.chainB.GetContext(), params)
 			},
 			true,
 		},
@@ -196,6 +212,9 @@ func (suite *KeeperTestSuite) TestOnRecvPacket() {
 				}
 
 				packetData = icaPacketData.GetBytes()
+
+				params := types.NewParams(true, []string{sdk.MsgTypeURL(msg)})
+				suite.chainB.GetSimApp().ICAHostKeeper.SetParams(suite.chainB.GetContext(), params)
 			},
 			true,
 		},
@@ -219,6 +238,9 @@ func (suite *KeeperTestSuite) TestOnRecvPacket() {
 				}
 
 				packetData = icaPacketData.GetBytes()
+
+				params := types.NewParams(true, []string{sdk.MsgTypeURL(msg)})
+				suite.chainB.GetSimApp().ICAHostKeeper.SetParams(suite.chainB.GetContext(), params)
 			},
 			true,
 		},
@@ -253,6 +275,9 @@ func (suite *KeeperTestSuite) TestOnRecvPacket() {
 				}
 
 				packetData = icaPacketData.GetBytes()
+
+				params := types.NewParams(true, []string{sdk.MsgTypeURL(msg)})
+				suite.chainB.GetSimApp().ICAHostKeeper.SetParams(suite.chainB.GetContext(), params)
 			},
 			true,
 		},
@@ -309,6 +334,27 @@ func (suite *KeeperTestSuite) TestOnRecvPacket() {
 			},
 			false,
 		},
+		{
+			"unauthorised: message type not allowed", // NOTE: do not update params to explicitly force the error
+			func() {
+				msg := &banktypes.MsgSend{
+					FromAddress: suite.chainB.SenderAccount.GetAddress().String(),
+					ToAddress:   suite.chainB.SenderAccount.GetAddress().String(),
+					Amount:      sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(100))),
+				}
+
+				data, err := icatypes.SerializeCosmosTx(suite.chainA.GetSimApp().AppCodec(), []sdk.Msg{msg})
+				suite.Require().NoError(err)
+
+				icaPacketData := icatypes.InterchainAccountPacketData{
+					Type: icatypes.EXECUTE_TX,
+					Data: data,
+				}
+
+				packetData = icaPacketData.GetBytes()
+			},
+			false,
+		},
 		{
 			"unauthorised: signer address is not the interchain account associated with the controller portID",
 			func() {
@@ -327,6 +373,9 @@ func (suite *KeeperTestSuite) TestOnRecvPacket() {
 				}
 
 				packetData = icaPacketData.GetBytes()
+
+				params := types.NewParams(true, []string{sdk.MsgTypeURL(msg)})
+				suite.chainB.GetSimApp().ICAHostKeeper.SetParams(suite.chainB.GetContext(), params)
 			},
 			false,
 		},

modules/apps/27-interchain-accounts/host/types/keys.go

@@ -1,9 +1,24 @@
 package types
 
+import (
+	sdk "github.com/cosmos/cosmos-sdk/types"
+)
+
 const (
 	// ModuleName defines the interchain accounts host module name
 	ModuleName = "icahost"
 
 	// StoreKey is the store key string for the interchain accounts host module
 	StoreKey = ModuleName
 )
+
+// ContainsMsgType returns true if the sdk.Msg TypeURL is present in allowMsgs, otherwise false
+func ContainsMsgType(allowMsgs []string, msg sdk.Msg) bool {
+	for _, v := range allowMsgs {
+		if v == sdk.MsgTypeURL(msg) {
+			return true
+		}
+	}
+
+	return false
+}

modules/apps/27-interchain-accounts/host/types/params.go

@@ -2,6 +2,7 @@ package types
 
 import (
 	"fmt"
+	"strings"
 
 	paramtypes "github.com/cosmos/cosmos-sdk/x/params/types"
 )
@@ -14,6 +15,8 @@ const (
 var (
 	// KeyHostEnabled is the store key for HostEnabled Params
 	KeyHostEnabled = []byte("HostEnabled")
+	// KeyAllowMessages is the store key for the AllowMessages Params
+	KeyAllowMessages = []byte("AllowMessages")
 )
 
 // ParamKeyTable type declaration for parameters
@@ -22,15 +25,16 @@ func ParamKeyTable() paramtypes.KeyTable {
 }
 
 // NewParams creates a new parameter configuration for the host submodule
-func NewParams(enableHost bool) Params {
+func NewParams(enableHost bool, allowMsgs []string) Params {
 	return Params{
-		HostEnabled: enableHost,
+		HostEnabled:   enableHost,
+		AllowMessages: allowMsgs,
 	}
 }
 
 // DefaultParams is the default parameter configuration for the host submodule
 func DefaultParams() Params {
-	return NewParams(DefaultHostEnabled)
+	return NewParams(DefaultHostEnabled, nil)
 }
 
 // Validate validates all host submodule parameters
@@ -39,13 +43,18 @@ func (p Params) Validate() error {
 		return err
 	}
 
+	if err := validateAllowlist(p.AllowMessages); err != nil {
+		return err
+	}
+
 	return nil
 }
 
 // ParamSetPairs implements params.ParamSet
 func (p *Params) ParamSetPairs() paramtypes.ParamSetPairs {
 	return paramtypes.ParamSetPairs{
 		paramtypes.NewParamSetPair(KeyHostEnabled, p.HostEnabled, validateEnabled),
+		paramtypes.NewParamSetPair(KeyAllowMessages, p.AllowMessages, validateAllowlist),
 	}
 }
 
@@ -57,3 +66,18 @@ func validateEnabled(i interface{}) error {
 
 	return nil
 }
+
+func validateAllowlist(i interface{}) error {
+	allowMsgs, ok := i.([]string)
+	if !ok {
+		return fmt.Errorf("invalid parameter type: %T", i)
+	}
+
+	for _, typeURL := range allowMsgs {
+		if strings.TrimSpace(typeURL) == "" {
+			return fmt.Errorf("parameter must not contain empty strings: %s", allowMsgs)
+		}
+	}
+
+	return nil
+}

modules/apps/27-interchain-accounts/host/types/params_test.go

@@ -10,5 +10,5 @@ import (
 
 func TestValidateParams(t *testing.T) {
 	require.NoError(t, types.DefaultParams().Validate())
-	require.NoError(t, types.NewParams(false).Validate())
+	require.NoError(t, types.NewParams(false, []string{}).Validate())
 }

proto/ibc/applications/interchain_accounts/controller/v1/controller.proto

@@ -10,6 +10,6 @@ import "gogoproto/gogo.proto";
 // Params defines the set of on-chain interchain accounts parameters.
 // The following parameters may be used to disable the controller submodule.
 message Params {
-  // controller_enabled enables or disables the controller submodule. 
+  // controller_enabled enables or disables the controller submodule.
   bool controller_enabled = 1 [(gogoproto.moretags) = "yaml:\"controller_enabled\""];
 }

proto/ibc/applications/interchain_accounts/host/v1/host.proto

@@ -10,6 +10,8 @@ import "gogoproto/gogo.proto";
 // Params defines the set of on-chain interchain accounts parameters.
 // The following parameters may be used to disable the host submodule.
 message Params {
-  // host_enabled enables or disables the host submodule. 
+  // host_enabled enables or disables the host submodule.
   bool host_enabled = 1 [(gogoproto.moretags) = "yaml:\"host_enabled\""];
+  // allow_messages defines a list of sdk message typeURLs allowed to be executed on a host chain.
+  repeated string allow_messages = 2 [(gogoproto.moretags) = "yaml:\"allow_messages\""];
 }

proto/ibc/applications/interchain_accounts/v1/genesis.proto

@@ -22,7 +22,7 @@ message ControllerGenesisState {
       [(gogoproto.nullable) = false, (gogoproto.moretags) = "yaml:\"active_channels\""];
   repeated RegisteredInterchainAccount interchain_accounts = 2
       [(gogoproto.nullable) = false, (gogoproto.moretags) = "yaml:\"interchain_accounts\""];
-  repeated string ports = 3;
+  repeated string                                           ports  = 3;
   ibc.applications.interchain_accounts.controller.v1.Params params = 4 [(gogoproto.nullable) = false];
 }
 
@@ -32,8 +32,8 @@ message HostGenesisState {
       [(gogoproto.nullable) = false, (gogoproto.moretags) = "yaml:\"active_channels\""];
   repeated RegisteredInterchainAccount interchain_accounts = 2
       [(gogoproto.nullable) = false, (gogoproto.moretags) = "yaml:\"interchain_accounts\""];
-  string port = 3;
-   ibc.applications.interchain_accounts.host.v1.Params params = 4 [(gogoproto.nullable) = false];
+  string                                              port   = 3;
+  ibc.applications.interchain_accounts.host.v1.Params params = 4 [(gogoproto.nullable) = false];
 }
 
 // ActiveChannel contains a pairing of port ID and channel ID for an active interchain accounts channel

proto/ibc/applications/interchain_accounts/v1/types.proto

@@ -29,4 +29,3 @@ message InterchainAccountPacketData {
 message CosmosTx {
   repeated google.protobuf.Any messages = 1;
 }
-