Skip to content

Commit

Permalink
Permission (CosmWasm#1050)
Browse files Browse the repository at this point in the history 
* move subset check into 'CanCreateCode'

* testing

* fix logic

* minor

* Code feedback

* clean test

* Revert "clean test"

This reverts commit a59be56.

* Revert test and make all pass

* Cover instantiation config with tests

* Add gov policy for sanity check

* Test with gov policy for sanity check

Co-authored-by: Alex Peters <[email protected]>
  • Loading branch information
@vuong177 @alpe @NoahSaso
2 people authored and NoahSaso committed Dec 2, 2022
1 parent eca060c commit 9fd0998
Show file tree
Hide file tree
Showing 4 changed files with 130 additions and 72 deletions.
21 changes: 17 additions & 4 deletions x/wasm/keeper/authz_policy.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,29 @@ import (
"github.com/CosmWasm/wasmd/x/wasm/types"
)

// ChainAccessConfigs chain settings
type ChainAccessConfigs struct {
Upload types.AccessConfig
Instantiate types.AccessConfig
}

// NewChainAccessConfigs constructor
func NewChainAccessConfigs(upload types.AccessConfig, instantiate types.AccessConfig) ChainAccessConfigs {
return ChainAccessConfigs{Upload: upload, Instantiate: instantiate}
}

type AuthorizationPolicy interface {
CanCreateCode(c types.AccessConfig, creator sdk.AccAddress) bool
CanCreateCode(chainConfigs ChainAccessConfigs, actor sdk.AccAddress, contractConfig types.AccessConfig) bool
CanInstantiateContract(c types.AccessConfig, actor sdk.AccAddress) bool
CanModifyContract(admin, actor sdk.AccAddress) bool
CanModifyCodeAccessConfig(creator, actor sdk.AccAddress, isSubset bool) bool
}

type DefaultAuthorizationPolicy struct{}

func (p DefaultAuthorizationPolicy) CanCreateCode(config types.AccessConfig, actor sdk.AccAddress) bool {
return config.Allowed(actor)
func (p DefaultAuthorizationPolicy) CanCreateCode(chainConfigs ChainAccessConfigs, actor sdk.AccAddress, contractConfig types.AccessConfig) bool {
return chainConfigs.Upload.Allowed(actor) &&
contractConfig.IsSubset(chainConfigs.Instantiate)
}

func (p DefaultAuthorizationPolicy) CanInstantiateContract(config types.AccessConfig, actor sdk.AccAddress) bool {
Expand All @@ -33,7 +45,8 @@ func (p DefaultAuthorizationPolicy) CanModifyCodeAccessConfig(creator, actor sdk

type GovAuthorizationPolicy struct{}

func (p GovAuthorizationPolicy) CanCreateCode(types.AccessConfig, sdk.AccAddress) bool {
// CanCreateCode implements AuthorizationPolicy.CanCreateCode to allow gov actions. Always returns true.
func (p GovAuthorizationPolicy) CanCreateCode(ChainAccessConfigs, sdk.AccAddress, types.AccessConfig) bool {
return true
}

Expand Down
134 changes: 84 additions & 50 deletions x/wasm/keeper/authz_policy_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,50 +13,68 @@ func TestDefaultAuthzPolicyCanCreateCode(t *testing.T) {
myActorAddress := RandomAccountAddress(t)
otherAddress := RandomAccountAddress(t)
specs := map[string]struct {
config types.AccessConfig
actor sdk.AccAddress
exp bool
panics bool
chainConfigs ChainAccessConfigs
contractInstConf types.AccessConfig
actor sdk.AccAddress
exp bool
panics bool
}{
"nobody": {
config: types.AllowNobody,
exp: false,
},
"everybody": {
config: types.AllowEverybody,
exp: true,
},
"only address - same": {
config: types.AccessTypeOnlyAddress.With(myActorAddress),
exp: true,
},
"only address - different": {
config: types.AccessTypeOnlyAddress.With(otherAddress),
exp: false,
},
"any address - included": {
config: types.AccessTypeAnyOfAddresses.With(otherAddress, myActorAddress),
exp: true,
},
"any address - not included": {
config: types.AccessTypeAnyOfAddresses.With(otherAddress),
exp: false,
},
"undefined config - panics": {
config: types.AccessConfig{},
panics: true,
"upload nobody": {
chainConfigs: NewChainAccessConfigs(types.AllowNobody, types.AllowEverybody),
contractInstConf: types.AllowEverybody,
exp: false,
},
"upload everybody": {
chainConfigs: NewChainAccessConfigs(types.AllowEverybody, types.AllowEverybody),
contractInstConf: types.AllowEverybody,
exp: true,
},
"upload only address - same": {
chainConfigs: NewChainAccessConfigs(types.AccessTypeOnlyAddress.With(myActorAddress), types.AllowEverybody),
contractInstConf: types.AllowEverybody,
exp: true,
},
"upload only address - different": {
chainConfigs: NewChainAccessConfigs(types.AccessTypeOnlyAddress.With(otherAddress), types.AllowEverybody),
contractInstConf: types.AllowEverybody,
exp: false,
},
"upload any address - included": {
chainConfigs: NewChainAccessConfigs(types.AccessTypeAnyOfAddresses.With(otherAddress, myActorAddress), types.AllowEverybody),
contractInstConf: types.AllowEverybody,
exp: true,
},
"upload any address - not included": {
chainConfigs: NewChainAccessConfigs(types.AccessTypeAnyOfAddresses.With(otherAddress), types.AllowEverybody),
contractInstConf: types.AllowEverybody,
exp: false,
},
"contract config - subtype": {
chainConfigs: NewChainAccessConfigs(types.AllowEverybody, types.AllowEverybody),
contractInstConf: types.AccessTypeAnyOfAddresses.With(myActorAddress),
exp: true,
},
"contract config - not subtype": {
chainConfigs: NewChainAccessConfigs(types.AllowEverybody, types.AllowNobody),
contractInstConf: types.AllowEverybody,
exp: false,
},
"upload undefined config - panics": {
chainConfigs: NewChainAccessConfigs(types.AccessConfig{}, types.AllowEverybody),
contractInstConf: types.AllowEverybody,
panics: true,
},
}
for name, spec := range specs {
t.Run(name, func(t *testing.T) {
policy := DefaultAuthorizationPolicy{}
if !spec.panics {
got := policy.CanCreateCode(spec.config, myActorAddress)
got := policy.CanCreateCode(spec.chainConfigs, myActorAddress, spec.contractInstConf)
assert.Equal(t, spec.exp, got)
return
}
assert.Panics(t, func() {
policy.CanCreateCode(spec.config, myActorAddress)
policy.CanCreateCode(spec.chainConfigs, myActorAddress, spec.contractInstConf)
})
})
}
Expand Down Expand Up @@ -184,35 +202,51 @@ func TestGovAuthzPolicyCanCreateCode(t *testing.T) {
myActorAddress := RandomAccountAddress(t)
otherAddress := RandomAccountAddress(t)
specs := map[string]struct {
config types.AccessConfig
actor sdk.AccAddress
chainConfigs ChainAccessConfigs
contractInstConf types.AccessConfig
actor sdk.AccAddress
}{
"nobody": {
config: types.AllowNobody,
"upload nobody": {
chainConfigs: NewChainAccessConfigs(types.AllowNobody, types.AllowEverybody),
contractInstConf: types.AllowEverybody,
},
"everybody": {
config: types.AllowEverybody,
"upload everybody": {
chainConfigs: NewChainAccessConfigs(types.AllowEverybody, types.AllowEverybody),
contractInstConf: types.AllowEverybody,
},
"only address - same": {
config: types.AccessTypeOnlyAddress.With(myActorAddress),
"upload only address - same": {
chainConfigs: NewChainAccessConfigs(types.AccessTypeOnlyAddress.With(myActorAddress), types.AllowEverybody),
contractInstConf: types.AllowEverybody,
},
"only address - different": {
config: types.AccessTypeOnlyAddress.With(otherAddress),
"upload only address - different": {
chainConfigs: NewChainAccessConfigs(types.AccessTypeOnlyAddress.With(otherAddress), types.AllowEverybody),
contractInstConf: types.AllowEverybody,
},
"any address - included": {
config: types.AccessTypeAnyOfAddresses.With(otherAddress, myActorAddress),
"upload any address - included": {
chainConfigs: NewChainAccessConfigs(types.AccessTypeAnyOfAddresses.With(otherAddress, myActorAddress), types.AllowEverybody),
contractInstConf: types.AllowEverybody,
},
"any address - not included": {
config: types.AccessTypeAnyOfAddresses.With(otherAddress),
"upload any address - not included": {
chainConfigs: NewChainAccessConfigs(types.AccessTypeAnyOfAddresses.With(otherAddress), types.AllowEverybody),
contractInstConf: types.AllowEverybody,
},
"undefined config - panics": {
config: types.AccessConfig{},
"contract config - subtype": {
chainConfigs: NewChainAccessConfigs(types.AllowEverybody, types.AllowEverybody),
contractInstConf: types.AccessTypeAnyOfAddresses.With(myActorAddress),
},
"contract config - not subtype": {
chainConfigs: NewChainAccessConfigs(types.AllowEverybody, types.AllowNobody),
contractInstConf: types.AllowEverybody,
},
"upload undefined config - not panics": {
chainConfigs: NewChainAccessConfigs(types.AccessConfig{}, types.AllowEverybody),
contractInstConf: types.AllowEverybody,
},
}
for name, spec := range specs {
t.Run(name, func(t *testing.T) {
policy := GovAuthorizationPolicy{}
got := policy.CanCreateCode(spec.config, myActorAddress)
got := policy.CanCreateCode(spec.chainConfigs, myActorAddress, spec.contractInstConf)
assert.True(t, got)
})
}
Expand Down
14 changes: 8 additions & 6 deletions x/wasm/keeper/keeper.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -187,16 +187,18 @@ func (k Keeper) create(ctx sdk.Context, creator sdk.AccAddress, wasmCode []byte,
return 0, checksum, sdkerrors.Wrap(sdkerrors.ErrInvalidAddress, "cannot be nil")
}

if !authZ.CanCreateCode(k.getUploadAccessConfig(ctx), creator) {
return 0, checksum, sdkerrors.Wrap(sdkerrors.ErrUnauthorized, "can not create code")
}
// figure out proper instantiate access
defaultAccessConfig := k.getInstantiateAccessConfig(ctx).With(creator)
if instantiateAccess == nil {
instantiateAccess = &defaultAccessConfig
} else if !instantiateAccess.IsSubset(defaultAccessConfig) {
// we enforce this must be subset of default upload access
return 0, checksum, sdkerrors.Wrap(sdkerrors.ErrUnauthorized, "instantiate access must be subset of default upload access")
}
chainConfigs := ChainAccessConfigs{
Instantiate: defaultAccessConfig,
Upload: k.getUploadAccessConfig(ctx),
}

if !authZ.CanCreateCode(chainConfigs, creator, *instantiateAccess) {
return 0, checksum, sdkerrors.Wrap(sdkerrors.ErrUnauthorized, "can not create code")
}

if ioutils.IsGzip(wasmCode) {
Expand Down
33 changes: 21 additions & 12 deletions x/wasm/keeper/keeper_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -138,39 +138,48 @@ func TestCreateStoresInstantiatePermission(t *testing.T) {

func TestCreateWithParamPermissions(t *testing.T) {
ctx, keepers := CreateTestInput(t, false, AvailableCapabilities)
keeper := keepers.ContractKeeper

deposit := sdk.NewCoins(sdk.NewInt64Coin("denom", 100000))
creator := keepers.Faucet.NewFundedRandomAccount(ctx, deposit...)
otherAddr := keepers.Faucet.NewFundedRandomAccount(ctx, deposit...)

specs := map[string]struct {
srcPermission types.AccessConfig
expError *sdkerrors.Error
policy AuthorizationPolicy
chainUpload types.AccessConfig
expError *sdkerrors.Error
}{
"default": {
srcPermission: types.DefaultUploadAccess,
policy: DefaultAuthorizationPolicy{},
chainUpload: types.DefaultUploadAccess,
},
"everybody": {
srcPermission: types.AllowEverybody,
policy: DefaultAuthorizationPolicy{},
chainUpload: types.AllowEverybody,
},
"nobody": {
srcPermission: types.AllowNobody,
expError: sdkerrors.ErrUnauthorized,
policy: DefaultAuthorizationPolicy{},
chainUpload: types.AllowNobody,
expError: sdkerrors.ErrUnauthorized,
},
"onlyAddress with matching address": {
srcPermission: types.AccessTypeOnlyAddress.With(creator),
policy: DefaultAuthorizationPolicy{},
chainUpload: types.AccessTypeOnlyAddress.With(creator),
},
"onlyAddress with non matching address": {
srcPermission: types.AccessTypeOnlyAddress.With(otherAddr),
expError: sdkerrors.ErrUnauthorized,
policy: DefaultAuthorizationPolicy{},
chainUpload: types.AccessTypeOnlyAddress.With(otherAddr),
expError: sdkerrors.ErrUnauthorized,
},
"gov: always allowed": {
policy: GovAuthorizationPolicy{},
chainUpload: types.AllowNobody,
},
}
for msg, spec := range specs {
t.Run(msg, func(t *testing.T) {
params := types.DefaultParams()
params.CodeUploadAccess = spec.srcPermission
params.CodeUploadAccess = spec.chainUpload
keepers.WasmKeeper.SetParams(ctx, params)
keeper := NewPermissionedKeeper(keepers.WasmKeeper, spec.policy)
_, _, err := keeper.Create(ctx, creator, hackatomWasm, nil)
require.True(t, spec.expError.Is(err), err)
if spec.expError != nil {
Expand Down

0 comments on commit 9fd0998

Please sign in to comment.