Skip to content
This repository has been archived by the owner on Apr 4, 2024. It is now read-only.

fix: add authz ante handler #1741

Merged
merged 17 commits into from
Apr 10, 2023
Merged

fix: add authz ante handler #1741

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
f3ffc53
imp: remove distribution perms
facs95 Apr 4, 2023
b015cdb
update changelog
facs95 Apr 4, 2023
ddfc15a
fix: add authz ante handler
facs95 Apr 5, 2023
68af64d
fix tests
facs95 Apr 5, 2023
7ad1e78
fix constants
facs95 Apr 5, 2023
7b1688d
remove unnecesary setup
facs95 Apr 5, 2023
2652fdf
fix commit
facs95 Apr 5, 2023
5d7c5ce
fix lint
facs95 Apr 5, 2023
999dc34
Merge branch 'main' into facs95/authz-ante
facs95 Apr 5, 2023
dbcba8e
Address changes in authz.go and partially refactor unit tests
0a1c Apr 6, 2023
f7cf7b6
Refactor duplicate test utils and improve naming
0a1c Apr 6, 2023
4b494a2
Refactor test suite to improve code quality
0a1c Apr 6, 2023
60816dd
Fix EIP-712 testutil comment header
0a1c Apr 6, 2023
63865da
Add DisabledAuthzMsgs to handler options
0a1c Apr 6, 2023
b58be7b
Add ante test
0a1c Apr 6, 2023
44da689
Merge branch 'main' into facs95/authz-ante
0a1c Apr 6, 2023
aeabde4
Add minor code-quality refactors
0a1c Apr 6, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 22 additions & 24 deletions app/ante/authz.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,39 +25,42 @@ import (
)

// maxNestedMsgs defines a cap for the number of nested messages on a MsgExec message
const maxNestedMsgs = 7
const maxNestedMsgs = 6
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why the change to 6 ?


// AuthzLimiterDecorator blocks certain msg types from being granted or executed
// within the authorization module.
type AuthzLimiterDecorator struct {
// disabledMsgTypes is the type urls of the msgs to block.
disabledMsgTypes []string
// disabledMsgs is a set that contains type urls of unauthorized msgs.
disabledMsgs map[string]struct{}
}

// NewAuthzLimiterDecorator creates a decorator to block certain msg types from being granted or executed within authz.
// NewAuthzLimiterDecorator creates a decorator to block certain msg types
// from being granted or executed within authz.
func NewAuthzLimiterDecorator(disabledMsgTypes ...string) AuthzLimiterDecorator {
disabledMsgs := make(map[string]struct{})
for _, url := range disabledMsgTypes {
disabledMsgs[url] = struct{}{}
}

return AuthzLimiterDecorator{
disabledMsgTypes: disabledMsgTypes,
disabledMsgs: disabledMsgs,
}
}

func (ald AuthzLimiterDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler) (newCtx sdk.Context, err error) {
if err := ald.checkDisabledMsgs(tx.GetMsgs(), false, 1); err != nil {
if err := ald.checkDisabledMsgs(tx.GetMsgs(), false, 0); err != nil {
return ctx, errorsmod.Wrapf(errortypes.ErrUnauthorized, err.Error())
}
return next(ctx, tx, simulate)
}

// checkDisabledMsgs iterates through the msgs and returns an error if it finds any unauthorized msgs.
//
// When searchOnlyInAuthzMsgs is enabled, only authz MsgGrant and MsgExec are blocked, if they contain unauthorized msg types.
// Otherwise any msg matching the disabled types are blocked, regardless of being in an authz msg or not.
//
// This method is recursive as MsgExec's can wrap other MsgExecs. The check for nested messages is performed up to the
// maxNestedMsgs threshold. If there are more than that limit, it returns an error
func (ald AuthzLimiterDecorator) checkDisabledMsgs(msgs []sdk.Msg, isAuthzInnerMsg bool, nestedLvl int) error {
if nestedLvl >= maxNestedMsgs {
return fmt.Errorf("found more nested msgs than permited. Limit is : %d", maxNestedMsgs)
// This method is recursive as MsgExec's can wrap other MsgExecs. The check for nested messages is capped at
// maxNestedMsgs total MsgExec messages.
func (ald AuthzLimiterDecorator) checkDisabledMsgs(msgs []sdk.Msg, isAuthzInnerMsg bool, totalMsgs int) error {
if totalMsgs >= maxNestedMsgs {
return fmt.Errorf("found more nested msgs than permitted. Limit is : %d", maxNestedMsgs)
}
for _, msg := range msgs {
switch msg := msg.(type) {
Expand All @@ -66,8 +69,8 @@ func (ald AuthzLimiterDecorator) checkDisabledMsgs(msgs []sdk.Msg, isAuthzInnerM
if err != nil {
return err
}
nestedLvl++
if err := ald.checkDisabledMsgs(innerMsgs, true, nestedLvl); err != nil {
totalMsgs++
if err := ald.checkDisabledMsgs(innerMsgs, true, totalMsgs); err != nil {
return err
}
case *authz.MsgGrant:
Expand All @@ -90,14 +93,9 @@ func (ald AuthzLimiterDecorator) checkDisabledMsgs(msgs []sdk.Msg, isAuthzInnerM
return nil
}

// isDisabledMsg returns true if the given message is in the list of restricted
// isDisabledMsg returns true if the given message is in the set of restricted
// messages from the AnteHandler.
func (ald AuthzLimiterDecorator) isDisabledMsg(msgTypeURL string) bool {
for _, disabledType := range ald.disabledMsgTypes {
if msgTypeURL == disabledType {
return true
}
}

return false
_, ok := ald.disabledMsgs[msgTypeURL]
return ok
}
127 changes: 37 additions & 90 deletions app/ante/authz_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,24 @@ func newMsgExec(grantee sdk.AccAddress, msgs []sdk.Msg) *authz.MsgExec {
return &msg
}

func createNestedExecMsgSend(testAddresses []sdk.AccAddress, numMsgs int) *authz.MsgExec {
return createNestedMsgExec(
testAddresses[1],
numMsgs,
[]sdk.Msg{
createMsgSend(testAddresses),
},
)
}

func createMsgSend(testAddresses []sdk.AccAddress) *banktypes.MsgSend {
return banktypes.NewMsgSend(
testAddresses[0],
testAddresses[3],
sdk.NewCoins(sdk.NewInt64Coin(evmtypes.DefaultEVMDenom, 1e8)),
)
}

func newMsgGrant(granter sdk.AccAddress, grantee sdk.AccAddress, a authz.Authorization, expiration *time.Time) *authz.MsgGrant {
msg, err := authz.NewMsgGrant(granter, grantee, a, expiration)
if err != nil {
Expand Down Expand Up @@ -90,6 +108,9 @@ func TestAuthzLimiterDecorator(t *testing.T) {
sdk.MsgTypeURL(&stakingtypes.MsgUndelegate{}),
)

testMsgSend := createMsgSend(testAddresses)
testMsgEthereumTx := &evmtypes.MsgEthereumTx{}

testCases := []struct {
name string
msgs []sdk.Msg
Expand All @@ -99,19 +120,15 @@ func TestAuthzLimiterDecorator(t *testing.T) {
{
"enabled msg - non blocked msg",
[]sdk.Msg{
banktypes.NewMsgSend(
testAddresses[0],
testAddresses[1],
sdk.NewCoins(sdk.NewInt64Coin(evmtypes.DefaultEVMDenom, 100e6)),
),
testMsgSend,
},
false,
nil,
},
{
"enabled msg MsgEthereumTx - blocked msg not wrapped in MsgExec",
[]sdk.Msg{
&evmtypes.MsgEthereumTx{},
testMsgEthereumTx,
},
false,
nil,
Expand Down Expand Up @@ -181,11 +198,9 @@ func TestAuthzLimiterDecorator(t *testing.T) {
[]sdk.Msg{
newMsgExec(
testAddresses[1],
[]sdk.Msg{banktypes.NewMsgSend(
testAddresses[0],
testAddresses[3],
sdk.NewCoins(sdk.NewInt64Coin(evmtypes.DefaultEVMDenom, 100e6)),
)}),
[]sdk.Msg{
testMsgSend,
}),
},
false,
nil,
Expand All @@ -196,7 +211,7 @@ func TestAuthzLimiterDecorator(t *testing.T) {
newMsgExec(
testAddresses[1],
[]sdk.Msg{
&evmtypes.MsgEthereumTx{},
testMsgEthereumTx,
},
),
},
Expand All @@ -215,12 +230,8 @@ func TestAuthzLimiterDecorator(t *testing.T) {
newMsgExec(
testAddresses[1],
[]sdk.Msg{
banktypes.NewMsgSend(
testAddresses[0],
testAddresses[3],
sdk.NewCoins(sdk.NewInt64Coin(evmtypes.DefaultEVMDenom, 100e6)),
),
&evmtypes.MsgEthereumTx{},
testMsgSend,
testMsgEthereumTx,
},
),
},
Expand All @@ -234,7 +245,7 @@ func TestAuthzLimiterDecorator(t *testing.T) {
testAddresses[1],
2,
[]sdk.Msg{
&evmtypes.MsgEthereumTx{},
testMsgEthereumTx,
},
),
},
Expand Down Expand Up @@ -262,46 +273,16 @@ func TestAuthzLimiterDecorator(t *testing.T) {
{
"disabled msg - nested MsgExec NOT containing a blocked msg but has more nesting levels than the allowed",
[]sdk.Msg{
createNestedMsgExec(
testAddresses[1],
6,
[]sdk.Msg{
banktypes.NewMsgSend(
testAddresses[0],
testAddresses[3],
sdk.NewCoins(sdk.NewInt64Coin(evmtypes.DefaultEVMDenom, 100e6)),
),
},
),
createNestedExecMsgSend(testAddresses, 6),
},
false,
sdkerrors.ErrUnauthorized,
},
{
"disabled msg - multiple two nested MsgExec messages NOT containing a blocked msg over the limit",
[]sdk.Msg{
createNestedMsgExec(
testAddresses[1],
5,
[]sdk.Msg{
banktypes.NewMsgSend(
testAddresses[0],
testAddresses[3],
sdk.NewCoins(sdk.NewInt64Coin(evmtypes.DefaultEVMDenom, 100e6)),
),
},
),
createNestedMsgExec(
testAddresses[1],
5,
[]sdk.Msg{
banktypes.NewMsgSend(
testAddresses[0],
testAddresses[3],
sdk.NewCoins(sdk.NewInt64Coin(evmtypes.DefaultEVMDenom, 100e6)),
),
},
),
createNestedExecMsgSend(testAddresses, 5),
createNestedExecMsgSend(testAddresses, 5),
},
false,
sdkerrors.ErrUnauthorized,
Expand Down Expand Up @@ -423,11 +404,7 @@ func (suite *AnteTestSuite) TestRejectMsgsInAuthz() {
newMsgExec(
testAddresses[1],
[]sdk.Msg{
banktypes.NewMsgSend(
testAddresses[0],
testAddresses[3],
sdk.NewCoins(sdk.NewInt64Coin(evmtypes.DefaultEVMDenom, 100e6)),
),
createMsgSend(testAddresses),
&evmtypes.MsgEthereumTx{},
},
),
Expand All @@ -450,45 +427,15 @@ func (suite *AnteTestSuite) TestRejectMsgsInAuthz() {
{
name: "a MsgExec with more nested MsgExec messages than allowed and with valid messages is blocked",
msgs: []sdk.Msg{
createNestedMsgExec(
testAddresses[1],
6,
[]sdk.Msg{
banktypes.NewMsgSend(
testAddresses[0],
testAddresses[3],
sdk.NewCoins(sdk.NewInt64Coin(evmtypes.DefaultEVMDenom, 100e6)),
),
},
),
createNestedExecMsgSend(testAddresses, 6),
},
expectedCode: sdkerrors.ErrUnauthorized.ABCICode(),
},
{
name: "two MsgExec messages NOT containing a blocked msg but between the two have more nesting than the allowed. Then, is blocked",
msgs: []sdk.Msg{
createNestedMsgExec(
testAddresses[1],
5,
[]sdk.Msg{
banktypes.NewMsgSend(
testAddresses[0],
testAddresses[3],
sdk.NewCoins(sdk.NewInt64Coin(evmtypes.DefaultEVMDenom, 100e6)),
),
},
),
createNestedMsgExec(
testAddresses[1],
5,
[]sdk.Msg{
banktypes.NewMsgSend(
testAddresses[0],
testAddresses[3],
sdk.NewCoins(sdk.NewInt64Coin(evmtypes.DefaultEVMDenom, 100e6)),
),
},
),
createNestedExecMsgSend(testAddresses, 5),
createNestedExecMsgSend(testAddresses, 5),
},
expectedCode: sdkerrors.ErrUnauthorized.ABCICode(),
},
Expand Down