LaconicNetwork/cosmos-sdk
10
0
Fork 0
Code Issues Pull Requests Packages Projects Releases 2 Wiki Activity

chore: x/circuit audit changes (backport #16901) (#16953)

Browse Source 
Co-authored-by: Likhita Polavarapu <78951027+likhita-809@users.noreply.github.com>
This commit is contained in:
mergify[bot] 2023-07-12 15:03:38 +02:00 committed by GitHub
parent 9ea398e5be
commit 0028264b73
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
12 changed files with 154 additions and 64 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
api/cosmos/circuit/v1/tx.pulsar.go
View File

@ -2965,7 +2965,7 @@ func (x *MsgAuthorizeCircuitBreaker) GetPermissions() *Permissions {
return nil
}
// MsgAuthorizeCircuitBreaker defines the Msg/AuthorizeCircuitBreaker response type.
// MsgAuthorizeCircuitBreakerResponse defines the Msg/AuthorizeCircuitBreaker response type.
type MsgAuthorizeCircuitBreakerResponse struct {
state protoimpl.MessageState
sizeCache protoimpl.SizeCache
@ -3051,7 +3051,7 @@ func (x *MsgTripCircuitBreaker) GetMsgTypeUrls() []string {
return nil
}
// MsgTripCircuitBreaker defines the Msg/TripCircuitBreaker response type.
// MsgTripCircuitBreakerResponse defines the Msg/TripCircuitBreaker response type.
type MsgTripCircuitBreakerResponse struct {
state protoimpl.MessageState
sizeCache protoimpl.SizeCache

2
proto/cosmos/circuit/v1/query.proto
View File

@ -8,7 +8,7 @@ import "cosmos/circuit/v1/types.proto";
import "google/api/annotations.proto";
import "cosmos/query/v1/query.proto";
// Query defines the circuit Msg service.
// Query defines the circuit gRPC querier service.
service Query {
// Account returns account permissions.
rpc Account(QueryAccountRequest) returns (AccountResponse) {

4
proto/cosmos/circuit/v1/tx.proto
View File

@ -39,7 +39,7 @@ message MsgAuthorizeCircuitBreaker {
Permissions permissions = 3;
}
// MsgAuthorizeCircuitBreaker defines the Msg/AuthorizeCircuitBreaker response type.
// MsgAuthorizeCircuitBreakerResponse defines the Msg/AuthorizeCircuitBreaker response type.
message MsgAuthorizeCircuitBreakerResponse {
bool success = 1;
}
@ -59,7 +59,7 @@ message MsgTripCircuitBreaker {
repeated string msg_type_urls = 2;
}
// MsgTripCircuitBreaker defines the Msg/TripCircuitBreaker response type.
// MsgTripCircuitBreakerResponse defines the Msg/TripCircuitBreaker response type.
message MsgTripCircuitBreakerResponse {
bool success = 1;
}

8
x/circuit/README.md
View File

@ -59,7 +59,7 @@ Authorize, is called by the module authority (default governance module account)
### Trip
Trip, is called by an account to disable message execution for a specific msgURL.
Trip, is called by an authorized account to disable message execution for a specific msgURL. If empty, all the msgs will be disabled.
```protobuf
// TripCircuitBreaker pauses processing of Msg's in the state machine.
@ -68,7 +68,7 @@ Trip, is called by an account to disable message execution for a specific msgURL
### Reset
Reset is called to enable execution of a previously disabled message.
Reset is called by an authorized account to enable execution for a specific msgURL of previously disabled message. If empty, all the disabled messages will be enabled.
```protobuf
// ResetCircuitBreaker resumes processing of Msg's in the state machine that
@ -118,8 +118,8 @@ The circuit module emits the following events:
| Type | Attribute Key | Attribute Value |
|---------|---------------|---------------------------|
| string | granter | {granteeAddress} |
| string | grantee | {granterAddress} |
| string | granter | {granterAddress} |
| string | grantee | {granteeAddress} |
| string | permission | {granteePermissions} |
| message | module | circuit |
| message | action | authorize_circuit_breaker |

4
x/circuit/client/cli/tx.go
View File

@ -44,7 +44,7 @@ func AuthorizeCircuitBreakerCmd() *cobra.Command {
"ALL_MSGS" = 2,
"SUPER_ADMIN" = 3,`,
Example: fmt.Sprintf(`%s circuit authorize [address] 0 "cosmos.bank.v1beta1.MsgSend,cosmos.bank.v1beta1.MsgMultiSend"`, version.AppName),
Args: cobra.RangeArgs(3, 4),
Args: cobra.RangeArgs(2, 3),
RunE: func(cmd *cobra.Command, args []string) error {
clientCtx, err := client.GetClientTxContext(cmd)
if err != nil {
@ -62,7 +62,7 @@ func AuthorizeCircuitBreakerCmd() *cobra.Command {
}
var typeUrls []string
if len(args) == 4 {
if len(args) == 3 {
typeUrls = strings.Split(args[2], ",")
}

88
x/circuit/keeper/genesis_test.go Normal file
View File

@ -0,0 +1,88 @@
package keeper_test
import (
"context"
"testing"
"github.com/stretchr/testify/suite"
storetypes "cosmossdk.io/store/types"
"cosmossdk.io/x/circuit"
"cosmossdk.io/x/circuit/keeper"
"cosmossdk.io/x/circuit/types"
"github.com/cosmos/cosmos-sdk/codec"
addresscodec "github.com/cosmos/cosmos-sdk/codec/address"
"github.com/cosmos/cosmos-sdk/runtime"
"github.com/cosmos/cosmos-sdk/testutil"
sdk "github.com/cosmos/cosmos-sdk/types"
moduletestutil "github.com/cosmos/cosmos-sdk/types/module/testutil"
authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
)
type GenesisTestSuite struct {
suite.Suite
ctx context.Context
keeper keeper.Keeper
cdc *codec.ProtoCodec
addrBytes []byte
}
func TestGenesisTestSuite(t *testing.T) {
suite.Run(t, new(GenesisTestSuite))
}
func (s *GenesisTestSuite) SetupTest() {
key := storetypes.NewKVStoreKey(types.StoreKey)
testCtx := testutil.DefaultContextWithDB(s.T(), key, storetypes.NewTransientStoreKey("transient_test"))
encCfg := moduletestutil.MakeTestEncodingConfig(circuit.AppModuleBasic{})
sdkCtx := testCtx.Ctx
s.ctx = sdkCtx
s.cdc = codec.NewProtoCodec(encCfg.InterfaceRegistry)
authority := authtypes.NewModuleAddress("gov")
ac := addresscodec.NewBech32Codec("cosmos")
bz, err := ac.StringToBytes(authority.String())
s.Require().NoError(err)
s.addrBytes = bz
s.keeper = keeper.NewKeeper(s.cdc, runtime.NewKVStoreService(key), authority.String(), ac)
}
func (s *GenesisTestSuite) TestInitExportGenesis() {
perms := types.Permissions{
Level: 3,
LimitTypeUrls: []string{"test"},
}
err := s.keeper.Permissions.Set(s.ctx, s.addrBytes, perms)
s.Require().NoError(err)
var accounts []*types.GenesisAccountPermissions
genAccsPerms := types.GenesisAccountPermissions{
Address: sdk.AccAddress(s.addrBytes).String(),
Permissions: &perms,
}
accounts = append(accounts, &genAccsPerms)
url := "test_url"
genesisState := &types.GenesisState{
AccountPermissions: accounts,
DisabledTypeUrls: []string{url},
}
s.keeper.InitGenesis(s.ctx, genesisState)
exported := s.keeper.ExportGenesis(s.ctx)
bz, err := s.cdc.MarshalJSON(exported)
s.Require().NoError(err)
var exportedGenesisState types.GenesisState
err = s.cdc.UnmarshalJSON(bz, &exportedGenesisState)
s.Require().NoError(err)
s.Require().Equal(genesisState.AccountPermissions, exportedGenesisState.AccountPermissions)
s.Require().Equal(genesisState.DisabledTypeUrls, exportedGenesisState.DisabledTypeUrls)
}

1
x/circuit/keeper/keeper.go
View File

@ -69,6 +69,7 @@ func (k *Keeper) GetAuthority() []byte {
return k.authority
}
// IsAllowed returns true when msg URL is not found in the DisableList for given context, else false.
func (k *Keeper) IsAllowed(ctx context.Context, msgURL string) (bool, error) {
has, err := k.DisableList.Has(ctx, msgURL)
return !has, err

42
x/circuit/keeper/msg_server_test.go
View File

@ -12,7 +12,7 @@ import (
const msgSend = "cosmos.bank.v1beta1.MsgSend"
func Test_AuthorizeCircuitBreaker(t *testing.T) {
func TestAuthorizeCircuitBreaker(t *testing.T) {
ft := initFixture(t)
srv := keeper.NewMsgServerImpl(ft.keeper)
@ -31,7 +31,7 @@ func Test_AuthorizeCircuitBreaker(t *testing.T) {
perms, err := ft.keeper.Permissions.Get(ft.ctx, add1)
require.NoError(t, err)
require.Equal(t, adminPerms, perms, "admin perms are not the same")
require.Equal(t, adminPerms, perms, "admin perms are the same")
// add a super user
allmsgs := types.Permissions{Level: types.Permissions_LEVEL_ALL_MSGS, LimitTypeUrls: []string{""}}
@ -45,22 +45,21 @@ func Test_AuthorizeCircuitBreaker(t *testing.T) {
perms, err = ft.keeper.Permissions.Get(ft.ctx, add2)
require.NoError(t, err)
require.Equal(t, allmsgs, perms, "admin perms are not the same")
require.Equal(t, allmsgs, perms)
// unauthorized user who does not have perms trying to authorize
superPerms := &types.Permissions{Level: types.Permissions_LEVEL_SUPER_ADMIN, LimitTypeUrls: []string{}}
msg = &types.MsgAuthorizeCircuitBreaker{Granter: addresses[3], Grantee: addresses[2], Permissions: superPerms}
_, err = srv.AuthorizeCircuitBreaker(ft.ctx, msg)
require.Error(t, err, "user with no permission should fail in authorizing others")
require.Error(t, err, "user with no permission fails in authorizing others")
// user with permission level all_msgs tries to grant another user perms
somePerms := &types.Permissions{Level: types.Permissions_LEVEL_SOME_MSGS, LimitTypeUrls: []string{}}
msg = &types.MsgAuthorizeCircuitBreaker{Granter: addresses[2], Grantee: addresses[3], Permissions: somePerms}
_, err = srv.AuthorizeCircuitBreaker(ft.ctx, msg)
require.Error(t, err, "user[2] does not have permission to grant others permission")
// user successfully grants another user perms to a specific permission
require.Error(t, err, "super user[2] does not have permission to grant others permission")
// admin successfully grants another user perms to a specific permission
somemsgs := types.Permissions{Level: types.Permissions_LEVEL_SOME_MSGS, LimitTypeUrls: []string{msgSend}}
msg = &types.MsgAuthorizeCircuitBreaker{Granter: authority, Grantee: addresses[3], Permissions: &somemsgs}
_, err = srv.AuthorizeCircuitBreaker(ft.ctx, msg)
@ -72,24 +71,24 @@ func Test_AuthorizeCircuitBreaker(t *testing.T) {
perms, err = ft.keeper.Permissions.Get(ft.ctx, add3)
require.NoError(t, err)
require.Equal(t, somemsgs, perms, "admin perms are not the same")
require.Equal(t, somemsgs, perms)
add4, err := ft.ac.StringToBytes(addresses[4])
require.NoError(t, err)
perms, err = ft.keeper.Permissions.Get(ft.ctx, add4)
require.ErrorIs(t, err, collections.ErrNotFound, "user should have no perms by default")
require.ErrorIs(t, err, collections.ErrNotFound, "users have no perms by default")
require.Equal(t, types.Permissions{Level: types.Permissions_LEVEL_NONE_UNSPECIFIED, LimitTypeUrls: nil}, perms, "user should have no perms by default")
require.Equal(t, types.Permissions{Level: types.Permissions_LEVEL_NONE_UNSPECIFIED, LimitTypeUrls: nil}, perms, "users have no perms by default")
// admin tries grants another user permission ALL_MSGS with limited urls populated
invalidmsgs := types.Permissions{Level: types.Permissions_LEVEL_SOME_MSGS, LimitTypeUrls: []string{msgSend}}
msg = &types.MsgAuthorizeCircuitBreaker{Granter: authority, Grantee: addresses[4], Permissions: &invalidmsgs}
// admin tries grants another user permission SOME_MSGS with limited urls populated
permis := types.Permissions{Level: types.Permissions_LEVEL_SOME_MSGS, LimitTypeUrls: []string{msgSend}}
msg = &types.MsgAuthorizeCircuitBreaker{Granter: authority, Grantee: addresses[4], Permissions: &permis}
_, err = srv.AuthorizeCircuitBreaker(ft.ctx, msg)
require.NoError(t, err)
}
func Test_TripCircuitBreaker(t *testing.T) {
func TestTripCircuitBreaker(t *testing.T) {
ft := initFixture(t)
srv := keeper.NewMsgServerImpl(ft.keeper)
@ -124,7 +123,7 @@ func Test_TripCircuitBreaker(t *testing.T) {
require.NoError(t, err)
require.False(t, allowed, "circuit breaker should be tripped")
// user with no permission attempts to trips circuit breaker
// user with no permission attempts to trip circuit breaker
unknownTrip := &types.MsgTripCircuitBreaker{Authority: addresses[4], MsgTypeUrls: []string{url}}
_, err = srv.TripCircuitBreaker(ft.ctx, unknownTrip)
require.Error(t, err)
@ -139,16 +138,16 @@ func Test_TripCircuitBreaker(t *testing.T) {
// try to trip two messages but user only has permission for one
someTrip := &types.MsgTripCircuitBreaker{Authority: addresses[2], MsgTypeUrls: []string{url, url2}}
_, err = srv.TripCircuitBreaker(ft.ctx, someTrip)
require.ErrorContains(t, err, "MsgEditValidator")
require.ErrorContains(t, err, "MsgEditValidator: unauthorized")
// user tries to trip an already tripped circuit breaker
alreadyTripped := "cosmos.bank.v1beta1.MsgSend"
twoTrip := &types.MsgTripCircuitBreaker{Authority: addresses[1], MsgTypeUrls: []string{alreadyTripped}}
_, err = srv.TripCircuitBreaker(ft.ctx, twoTrip)
require.Error(t, err)
require.ErrorContains(t, err, "already disabled")
}
func Test_ResetCircuitBreaker(t *testing.T) {
func TestResetCircuitBreaker(t *testing.T) {
ft := initFixture(t)
authority, err := ft.ac.BytesToString(ft.mockAddr)
require.NoError(t, err)
@ -174,7 +173,6 @@ func Test_ResetCircuitBreaker(t *testing.T) {
require.NoError(t, err)
require.True(t, allowed, "circuit breaker should be reset")
// user has no permission to reset circuit breaker
// admin trips circuit breaker
_, err = srv.TripCircuitBreaker(ft.ctx, admintrip)
require.NoError(t, err)
@ -183,6 +181,7 @@ func Test_ResetCircuitBreaker(t *testing.T) {
require.NoError(t, err)
require.False(t, allowed, "circuit breaker should be tripped")
// user has no permission to reset circuit breaker
unknownUserReset := &types.MsgResetCircuitBreaker{Authority: addresses[1], MsgTypeUrls: []string{url}}
_, err = srv.ResetCircuitBreaker(ft.ctx, unknownUserReset)
require.Error(t, err)
@ -208,8 +207,7 @@ func Test_ResetCircuitBreaker(t *testing.T) {
_, err = srv.ResetCircuitBreaker(ft.ctx, allMsgsReset)
require.NoError(t, err)
// user tries to reset an message they dont have permission to reset
// user tries to reset a message they dont have permission to reset
url = "cosmos.staking.v1beta1.MsgCreateValidator"
// give restricted perms to a user
someMsgs := &types.Permissions{Level: types.Permissions_LEVEL_SOME_MSGS, LimitTypeUrls: []string{url2}}
@ -221,7 +219,7 @@ func Test_ResetCircuitBreaker(t *testing.T) {
_, err = srv.TripCircuitBreaker(ft.ctx, admintrip)
require.NoError(t, err)
// user with all messages resets circuit breaker
// user with some messages resets circuit breaker
someMsgsReset := &types.MsgResetCircuitBreaker{Authority: addresses[2], MsgTypeUrls: []string{url}}
_, err = srv.ResetCircuitBreaker(ft.ctx, someMsgsReset)
require.NoError(t, err)

2
x/circuit/keeper/query.go
View File

@ -17,7 +17,7 @@ type QueryServer struct {
keeper Keeper
}
// NewMsgServerImpl returns an implementation of the circuit MsgServer interface
// NewQueryServer returns an implementation of the circuit QueryServer interface
// for the provided Keeper.
func NewQueryServer(keeper Keeper) types.QueryServer {
return &QueryServer{keeper: keeper}

38
x/circuit/keeper/query_test.go
View File

@ -31,6 +31,18 @@ func TestQueryAccount(t *testing.T) {
require.Equal(t, res.Permission.LimitTypeUrls, []string{
"test",
})
// test invalid address
res, err = qs.Account(f.ctx, &types.QueryAccountRequest{Address: "invalid"})
require.Error(t, err)
require.ErrorContains(t, err, "invalid bech32 string")
require.Nil(t, res)
// test account not found
res, err = qs.Account(f.ctx, &types.QueryAccountRequest{Address: addresses[1]})
require.Error(t, err)
require.ErrorContains(t, err, "not found")
require.Nil(t, res)
}
func TestQueryAccounts(t *testing.T) {
@ -40,19 +52,26 @@ func TestQueryAccounts(t *testing.T) {
add, err := f.ac.StringToBytes(addresses[0])
require.NoError(t, err)
err = f.keeper.Permissions.Set(f.ctx, add, f.mockPerms)
require.NoError(t, err)
// create a new query server
qs := keeper.NewQueryServer(f.keeper)
// test the Accounts method
// test the Accounts method with no accounts
res1, err := qs.Accounts(f.ctx, &types.QueryAccountsRequest{
Pagination: &query.PageRequest{Limit: 10},
})
require.NoError(t, err)
require.Len(t, res1.Accounts, 0)
for _, a := range res1.Accounts {
err = f.keeper.Permissions.Set(f.ctx, add, f.mockPerms)
require.NoError(t, err)
// test the Accounts method
res2, err := qs.Accounts(f.ctx, &types.QueryAccountsRequest{
Pagination: &query.PageRequest{Limit: 10},
})
require.NoError(t, err)
for _, a := range res2.Accounts {
require.Equal(t, addresses[0], a.Address)
require.Equal(t, f.mockPerms, *a.Permissions)
}
@ -64,13 +83,18 @@ func TestQueryDisabledList(t *testing.T) {
t.Parallel()
f := initFixture(t)
require.NoError(t, f.keeper.DisableList.Set(f.ctx, f.mockMsgURL))
// create a new query server
qs := keeper.NewQueryServer(f.keeper)
// test the DisabledList method
disabledList, err := qs.DisabledList(f.ctx, &types.QueryDisabledListRequest{})
require.NoError(t, err)
require.Len(t, disabledList.DisabledList, 0)
require.NoError(t, f.keeper.DisableList.Set(f.ctx, f.mockMsgURL))
// test the DisabledList method
disabledList, err = qs.DisabledList(f.ctx, &types.QueryDisabledListRequest{})
require.NoError(t, err)
require.Equal(t, []string{f.mockMsgURL}, disabledList.DisabledList)
}

21
x/circuit/types/msgs.go
View File

@ -25,13 +25,6 @@ func (m MsgAuthorizeCircuitBreaker) Route() string { return sdk.MsgTypeURL(&m) }
// Type Implements Msg.
func (m MsgAuthorizeCircuitBreaker) Type() string { return sdk.MsgTypeURL(&m) }
// GetSigners returns the expected signers for a MsgAuthorizeCircuitBreaker.
func (m MsgAuthorizeCircuitBreaker) GetSigners() []sdk.AccAddress {
granter := sdk.MustAccAddressFromBech32(m.Granter)
return []sdk.AccAddress{granter}
}
// NewMsgTripCircuitBreaker creates a new MsgTripCircuitBreaker instance.
func NewMsgTripCircuitBreaker(authority string, urls []string) *MsgTripCircuitBreaker {
return &MsgTripCircuitBreaker{
@ -46,13 +39,6 @@ func (m MsgTripCircuitBreaker) Route() string { return sdk.MsgTypeURL(&m) }
// Type Implements Msg.
func (m MsgTripCircuitBreaker) Type() string { return sdk.MsgTypeURL(&m) }
// GetSigners returns the expected signers for a MsgTripCircuitBreaker.
func (m MsgTripCircuitBreaker) GetSigners() []sdk.AccAddress {
granter := sdk.MustAccAddressFromBech32(m.Authority)
return []sdk.AccAddress{granter}
}
// NewMsgResetCircuitBreaker creates a new MsgResetCircuitBreaker instance.
func NewMsgResetCircuitBreaker(authority string, urls []string) *MsgResetCircuitBreaker {
return &MsgResetCircuitBreaker{
@ -66,10 +52,3 @@ func (m MsgResetCircuitBreaker) Route() string { return sdk.MsgTypeURL(&m) }
// Type Implements Msg.
func (m MsgResetCircuitBreaker) Type() string { return sdk.MsgTypeURL(&m) }
// GetSigners returns the expected signers for a MsgResetCircuitBreaker.
func (m MsgResetCircuitBreaker) GetSigners() []sdk.AccAddress {
granter := sdk.MustAccAddressFromBech32(m.Authority)
return []sdk.AccAddress{granter}
}

4
x/circuit/types/tx.pb.go
View File

@ -95,7 +95,7 @@ func (m *MsgAuthorizeCircuitBreaker) GetPermissions() *Permissions {
return nil
}
// MsgAuthorizeCircuitBreaker defines the Msg/AuthorizeCircuitBreaker response type.
// MsgAuthorizeCircuitBreakerResponse defines the Msg/AuthorizeCircuitBreaker response type.
type MsgAuthorizeCircuitBreakerResponse struct {
Success bool `protobuf:"varint,1,opt,name=success,proto3" json:"success,omitempty"`
}
@ -199,7 +199,7 @@ func (m *MsgTripCircuitBreaker) GetMsgTypeUrls() []string {
return nil
}
// MsgTripCircuitBreaker defines the Msg/TripCircuitBreaker response type.
// MsgTripCircuitBreakerResponse defines the Msg/TripCircuitBreaker response type.
type MsgTripCircuitBreakerResponse struct {
Success bool `protobuf:"varint,1,opt,name=success,proto3" json:"success,omitempty"`
}