[#83] router: Ignore local overrides with Allow status

* If a target owner sets some private rules into morph, then the
  access may be intentionally/unintentionally violated by setting
  a local override that allows any operation;
* The same happens if no morph rule is found;
* Fix unit-test `TestInmemory_MultipleTargets`;
* Add unit-test `TestLocalOverrideAllowRule`.

Signed-off-by: Airat Arifullin <aarifullin@yadro.com>
This commit is contained in:
Airat Arifullin 2025-02-21 16:47:33 +03:00
parent 9c98b05066
commit d1db407c72
2 changed files with 78 additions and 3 deletions

View file

@ -28,9 +28,13 @@ func (dr *defaultChainRouter) IsAllowed(name chain.Name, rt RequestTarget, r res
status, ruleFound, err = dr.checkLocal(name, rt, r)
if err != nil {
return chain.NoRuleFound, false, err
} else if ruleFound {
} else if ruleFound && status != chain.Allow {
// The local overrides have the highest priority and thus
// morph rules are not considered if a local one is found.
//
// Any override with the "Allow" status is ignored in order
// to prevent access violations by bypassing the denying rules
// set for the target.
return
}

View file

@ -7,6 +7,7 @@ import (
"git.frostfs.info/TrueCloudLab/policy-engine/pkg/chain"
"git.frostfs.info/TrueCloudLab/policy-engine/pkg/engine"
resourcetest "git.frostfs.info/TrueCloudLab/policy-engine/pkg/resource/testutil"
nativeschema "git.frostfs.info/TrueCloudLab/policy-engine/schema/native"
"github.com/nspcc-dev/neo-go/pkg/vm/stackitem"
"github.com/stretchr/testify/require"
)
@ -31,6 +32,76 @@ func TestAddRootOverrides(t *testing.T) {
require.Equal(t, string(id), string(res[0].ID))
}
func TestLocalOverrideAllowRule(t *testing.T) {
const (
namespace = "root"
object = "native:object//73tQMTYyUkTgmvPR1HWib6pndbhSoBovbnMF7Pws8Rcy/BzQw5HH3feoxFDD5tCT87Y1726qzgLfxEE7wgtoRzB3R"
actor1 = "owner1"
)
s := NewInMemoryLocalOverrides()
res := resourcetest.NewResource(object, map[string]string{"FromS3": "true"})
reqBadIP := resourcetest.NewRequest(nativeschema.MethodPutObject, res, map[string]string{
"SourceIP": "10.2.1.12",
"Actor": actor1,
})
s.LocalStorage().AddOverride(chain.Ingress, engine.NamespaceTarget(namespace), &chain.Chain{
Rules: []chain.Rule{
{ // This allowing rule shouldn't override denying rules from morph storage.
Status: chain.Allow,
Actions: chain.Actions{Names: []string{nativeschema.MethodPutObject}},
Resources: chain.Resources{Names: []string{nativeschema.ResourceFormatRootObjects}},
Any: true,
Condition: []chain.Condition{
{
Op: chain.CondStringNotLike,
Kind: chain.KindRequest,
Key: "SourceIP",
Value: "10.1.1.*",
},
},
},
},
})
s.MorphRuleChainStorage().AddMorphRuleChain(chain.Ingress, engine.NamespaceTarget(namespace), &chain.Chain{
Rules: []chain.Rule{
{
Status: chain.AccessDenied,
Actions: chain.Actions{Names: []string{nativeschema.MethodDeleteObject}},
Resources: chain.Resources{Names: []string{nativeschema.ResourceFormatRootObjects}},
},
{
Status: chain.AccessDenied,
Actions: chain.Actions{Names: []string{nativeschema.MethodPutObject}},
Resources: chain.Resources{Names: []string{nativeschema.ResourceFormatRootObjects}},
Any: true,
Condition: []chain.Condition{
{
Op: chain.CondStringNotLike,
Kind: chain.KindRequest,
Key: "SourceIP",
Value: "10.1.1.*",
},
{
Op: chain.CondStringNotEquals,
Kind: chain.KindRequest,
Key: "Actor",
Value: actor1,
},
},
},
},
})
status, ok, _ := s.IsAllowed(chain.Ingress, engine.NewRequestTarget(namespace, container), reqBadIP)
require.Equal(t, chain.AccessDenied, status)
require.True(t, ok)
}
func TestInmemory_MultipleTargets(t *testing.T) {
const op = "ape::test::op"
@ -68,8 +139,8 @@ func TestInmemory_MultipleTargets(t *testing.T) {
status, found, err := s.IsAllowed(chain.Ingress, target, req)
require.NoError(t, err)
require.True(t, found)
require.Equal(t, chain.Allow, status)
require.False(t, found)
require.Equal(t, chain.NoRuleFound, status)
})
}
}