TrueCloudLab/policy-engine
21
0
Fork 12
generated from TrueCloudLab/basic
Code Issues 5 Pull requests 2 Projects Releases Packages Wiki Activity Actions

[#82] router: Make IsAllow receive client defined overrides
All checks were successful
DCO action / DCO (pull_request) Successful in 1m30s
Details
Tests and linters / Tests (1.21) (pull_request) Successful in 1m21s
Details
Tests and linters / Tests (1.20) (pull_request) Successful in 1m33s
Details
Tests and linters / Staticcheck (pull_request) Successful in 1m29s
Details
Tests and linters / Tests with -race (pull_request) Successful in 1m47s
Details
Tests and linters / Lint (pull_request) Successful in 2m22s
Details

Browse source 
* Change `ChainRouter` interface: `IsAllow` should also receive
  client defined overrides that are checked after local overrides
  but before morph chains. Client defined overrides are checked for
  per request.
* Fix code that uses `ChainRouter`.
* Fix unit-tests.

Signed-off-by: Airat Arifullin <aarifullin@yadro.com>
This commit is contained in:
Airat Arifullin 2024-06-28 14:38:05 +03:00
parent ac965e8d17
commit a847f28b01
6 changed files with 140 additions and 29 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

14
iam/converter_test.go
View file

@ -717,13 +717,13 @@ func TestIPConditions(t *testing.T) {
req := testutil.NewRequest("s3:CreateBucket", testutil.NewResource(fmt.Sprintf(s3.ResourceFormatS3Bucket, "bkt"), nil),
map[string]string{common.PropertyKeyFrostFSSourceIP: "203.0.113.128"})
status, _, err := s.IsAllowed(chain.S3, engine.NewRequestTargetWithNamespace(""), req)
status, _, err := s.IsAllowed(chain.S3, engine.NewRequestTargetWithNamespace(""), req, engine.NoClientDefined)
require.NoError(t, err)
require.Equal(t, chain.Allow.String(), status.String())
req = testutil.NewRequest("s3:CreateBucket", testutil.NewResource(fmt.Sprintf(s3.ResourceFormatS3Bucket, "bkt"), nil),
map[string]string{common.PropertyKeyFrostFSSourceIP: "203.0.114.0"})
status, _, err = s.IsAllowed(chain.S3, engine.NewRequestTargetWithNamespace(""), req)
status, _, err = s.IsAllowed(chain.S3, engine.NewRequestTargetWithNamespace(""), req, engine.NoClientDefined)
require.NoError(t, err)
require.Equal(t, chain.NoRuleFound.String(), status.String())
})
@ -1092,7 +1092,7 @@ func TestComplexNativeConditions(t *testing.T) {
} {
t.Run(tc.name, func(t *testing.T) {
req := testutil.NewRequest(tc.action, testutil.NewResource(tc.resource, tc.resourceMap), tc.requestMap)
status, _, err := s.IsAllowed("name", engine.NewRequestTargetWithNamespace("ns"), req)
status, _, err := s.IsAllowed("name", engine.NewRequestTargetWithNamespace("ns"), req, engine.NoClientDefined)
require.NoError(t, err)
require.Equal(t, tc.status.String(), status.String())
})
@ -1324,7 +1324,7 @@ func TestComplexS3Conditions(t *testing.T) {
} {
t.Run(tc.name, func(t *testing.T) {
req := testutil.NewRequest(tc.action, testutil.NewResource(tc.resource, tc.resourceMap), tc.requestMap)
status, _, err := s.IsAllowed("name", engine.NewRequestTargetWithNamespace("ns"), req)
status, _, err := s.IsAllowed("name", engine.NewRequestTargetWithNamespace("ns"), req, engine.NoClientDefined)
require.NoError(t, err)
require.Equal(t, tc.status.String(), status.String())
})
@ -1365,19 +1365,19 @@ func TestS3BucketResource(t *testing.T) {
// check we match just "bucket1" resource
req := testutil.NewRequest("s3:HeadBucket", testutil.NewResource(fmt.Sprintf(s3.ResourceFormatS3Bucket, bktName1), nil), nil)
status, _, err := s.IsAllowed(chainName, engine.NewRequestTargetWithNamespace(namespace), req)
status, _, err := s.IsAllowed(chainName, engine.NewRequestTargetWithNamespace(namespace), req, engine.NoClientDefined)
require.NoError(t, err)
require.Equal(t, chain.AccessDenied.String(), status.String())
// check we match just "bucket2" resource
req = testutil.NewRequest("s3:HeadBucket", testutil.NewResource(fmt.Sprintf(s3.ResourceFormatS3Bucket, bktName2), nil), nil)
status, _, err = s.IsAllowed(chainName, engine.NewRequestTargetWithNamespace(namespace), req)
status, _, err = s.IsAllowed(chainName, engine.NewRequestTargetWithNamespace(namespace), req, engine.NoClientDefined)
require.NoError(t, err)
require.Equal(t, chain.Allow.String(), status.String())
// check we also match "bucket2/object" resource
req = testutil.NewRequest("s3:PutObject", testutil.NewResource(fmt.Sprintf(s3.ResourceFormatS3BucketObject, bktName2, "object"), nil), nil)
status, _, err = s.IsAllowed(chainName, engine.NewRequestTargetWithNamespace(namespace), req)
status, _, err = s.IsAllowed(chainName, engine.NewRequestTargetWithNamespace(namespace), req, engine.NoClientDefined)
require.NoError(t, err)
require.Equal(t, chain.Allow.String(), status.String())
}

2
iam/policy_test.go
View file

@ -544,7 +544,7 @@ func TestProcessDenyFirst(t *testing.T) {
resource := testutil.NewResource("arn:aws:s3:::test-bucket/object", nil)
request := testutil.NewRequest("s3:PutObject", resource, map[string]string{s3.PropertyKeyOwner: mockResolver.users["root/user-name"]})
status, found, err := s.IsAllowed(chain.S3, engine.NewRequestTarget("ns", ""), request)
status, found, err := s.IsAllowed(chain.S3, engine.NewRequestTarget("ns", ""), request, engine.NoClientDefined)
require.NoError(t, err)
require.True(t, found)
require.Equal(t, chain.AccessDenied, status)

52
pkg/engine/chain_router.go
View file

@ -1,6 +1,8 @@
package engine
import (
"fmt"
"git.frostfs.info/TrueCloudLab/policy-engine/pkg/chain"
"git.frostfs.info/TrueCloudLab/policy-engine/pkg/resource"
)
@ -24,13 +26,21 @@ func NewDefaultChainRouterWithLocalOverrides(morph MorphRuleChainStorageReader,
}
}
func (dr *defaultChainRouter) IsAllowed(name chain.Name, rt RequestTarget, r resource.Request) (status chain.Status, ruleFound bool, err error) {
func (dr *defaultChainRouter) IsAllowed(name chain.Name, rt RequestTarget, r resource.Request, clientDefined ClientDefinedOverrides) (status chain.Status, ruleFound bool, err error) {
status, ruleFound, err = dr.checkLocal(name, rt, r)
if err != nil {
return chain.NoRuleFound, false, err
} else if ruleFound {
// The local overrides have the highest priority and thus
// morph rules are not considered if a local one is found.
// morph rules and client overrides are not considered if a local one is found.
return
}
status, ruleFound, err = checkClientDefined(name, rt, r, clientDefined)
if err != nil {
return chain.NoRuleFound, false, fmt.Errorf("client defined: %w", err)
} else if ruleFound {
// The client defined overrides have the higher priority then morph chains.
return
}
@ -61,6 +71,29 @@ func (dr *defaultChainRouter) checkLocal(name chain.Name, rt RequestTarget, r re
return
}
func checkClientDefined(name chain.Name, rt RequestTarget, r resource.Request, clientDefined ClientDefinedOverrides) (status chain.Status, ruleFound bool, err error) {
if clientDefined == NoClientDefined {
return
}
var ruleFounds []bool
for _, target := range rt.Targets() {
status, ruleFound, err = matchClientDefined(name, target, r, clientDefined)
if err != nil || ruleFound && status != chain.Allow {
return
}
ruleFounds = append(ruleFounds, ruleFound)
}
status = chain.NoRuleFound
for _, ruleFound = range ruleFounds {
if ruleFound {
status = chain.Allow
break
}
}
return
}
func (dr *defaultChainRouter) checkMorph(name chain.Name, rt RequestTarget, r resource.Request) (status chain.Status, ruleFound bool, err error) {
var ruleFounds []bool
for _, target := range rt.Targets() {
@ -86,7 +119,16 @@ func (dr *defaultChainRouter) matchLocalOverrides(name chain.Name, target Target
if err != nil {
return
}
status, ruleFound = dr.getStatusFromChains(localOverrides, r)
status, ruleFound = getStatusFromChains(localOverrides, r)
return
}
func matchClientDefined(name chain.Name, target Target, r resource.Request, clientDefined ClientDefinedOverrides) (status chain.Status, ruleFound bool, err error) {
overrides, err := clientDefined.ListOverrides(name, target)
if err != nil {
return
}
status, ruleFound = getStatusFromChains(overrides, r)
return
}
@ -95,11 +137,11 @@ func (dr *defaultChainRouter) matchMorphRuleChains(name chain.Name, target Targe
if err != nil {
return chain.NoRuleFound, false, err
}
status, ruleFound = dr.getStatusFromChains(namespaceChains, r)
status, ruleFound = getStatusFromChains(namespaceChains, r)
return
}
func (dr *defaultChainRouter) getStatusFromChains(chains []*chain.Chain, r resource.Request) (chain.Status, bool) {
func getStatusFromChains(chains []*chain.Chain, r resource.Request) (chain.Status, bool) {
var allow bool
for _, c := range chains {
if status, found := c.Match(r); found {

4
pkg/engine/inmemory/inmemory.go
View file

@ -43,6 +43,6 @@ func (im *inmemory) MorphRuleChainStorage() engine.MorphRuleChainStorage {
return im.morph
}
func (im *inmemory) IsAllowed(name chain.Name, rt engine.RequestTarget, r resource.Request) (status chain.Status, ruleFound bool, err error) {
return im.router.IsAllowed(name, rt, r)
func (im *inmemory) IsAllowed(name chain.Name, rt engine.RequestTarget, r resource.Request, clientDefined engine.ClientDefinedOverrides) (status chain.Status, ruleFound bool, err error) {
return im.router.IsAllowed(name, rt, r, clientDefined)
}

78
pkg/engine/inmemory/inmemory_test.go
View file

@ -51,7 +51,7 @@ func TestInmemory(t *testing.T) {
"Actor": actor1,
})
status, ok, _ := s.IsAllowed(chain.Ingress, engine.NewRequestTargetWithNamespace(namespace), reqGood)
status, ok, _ := s.IsAllowed(chain.Ingress, engine.NewRequestTargetWithNamespace(namespace), reqGood, engine.NoClientDefined)
require.Equal(t, chain.NoRuleFound, status)
require.False(t, ok)
@ -141,7 +141,59 @@ func TestInmemory(t *testing.T) {
"SourceIP": "10.122.1.20",
"Actor": actor1,
})
status, ok, _ := s.IsAllowed(chain.Ingress, engine.NewRequestTarget(namespace, container), reqBadIP)
status, ok, _ := s.IsAllowed(chain.Ingress, engine.NewRequestTarget(namespace, container), reqBadIP, engine.NoClientDefined)
require.Equal(t, chain.AccessDenied, status)
require.True(t, ok)
})
t.Run("allow by client defined then deny by local overrides", func(t *testing.T) {
reqBadIP := resourcetest.NewRequest("native::object::put", res, map[string]string{
"SourceIP": "10.122.1.20",
"Actor": actor1,
})
clientDef := NewInMemoryLocalOverrides()
clientDef.LocalStorage().AddOverride(chain.Ingress, engine.ContainerTarget(container), &chain.Chain{
Rules: []chain.Rule{
{
Status: chain.Allow,
Actions: chain.Actions{Names: []string{"native::object::put"}},
Resources: chain.Resources{Names: []string{"native::object::abc/*"}},
Condition: []chain.Condition{
{
Op: chain.CondStringEquals,
Kind: chain.KindRequest,
Key: "SourceIP",
Value: "10.122.1.20",
},
},
},
},
})
status, ok, _ := s.IsAllowed(chain.Ingress, engine.NewRequestTarget(namespace, container), reqBadIP, clientDef.LocalStorage())
require.Equal(t, chain.Allow, status)
require.True(t, ok)
s.LocalStorage().AddOverride(chain.Ingress, engine.ContainerTarget(container), &chain.Chain{
Rules: []chain.Rule{
{
Status: chain.AccessDenied,
Actions: chain.Actions{Names: []string{"native::object::put"}},
Resources: chain.Resources{Names: []string{"native::object::abc/*"}},
Condition: []chain.Condition{
{
Op: chain.CondStringEquals,
Kind: chain.KindRequest,
Key: "SourceIP",
Value: "10.122.1.20",
},
},
},
},
})
status, ok, _ = s.IsAllowed(chain.Ingress, engine.NewRequestTarget(namespace, container), reqBadIP, clientDef.LocalStorage())
// Local overrides have higher priority then client defined overrides.
require.Equal(t, chain.AccessDenied, status)
require.True(t, ok)
})
@ -151,7 +203,7 @@ func TestInmemory(t *testing.T) {
"SourceIP": "10.1.1.13",
"Actor": actor2,
})
status, ok, _ := s.IsAllowed(chain.Ingress, engine.NewRequestTarget(namespace, container), reqBadActor)
status, ok, _ := s.IsAllowed(chain.Ingress, engine.NewRequestTarget(namespace, container), reqBadActor, engine.NoClientDefined)
require.Equal(t, chain.AccessDenied, status)
require.True(t, ok)
})
@ -162,14 +214,14 @@ func TestInmemory(t *testing.T) {
status, ok, _ := s.IsAllowed(chain.Ingress, engine.NewRequestTarget(namespace, container), resourcetest.NewRequest("native::object::get", objGood, map[string]string{
"SourceIP": "10.1.1.14",
"Actor": actor2,
}))
}), engine.NoClientDefined)
require.Equal(t, chain.Allow, status)
require.True(t, ok)
status, ok, _ = s.IsAllowed(chain.Ingress, engine.NewRequestTarget(namespace, container), resourcetest.NewRequest("native::object::get", objBadAttr, map[string]string{
"SourceIP": "10.1.1.14",
"Actor": actor2,
}))
}), engine.NoClientDefined)
require.Equal(t, chain.NoRuleFound, status)
require.False(t, ok)
})
@ -179,28 +231,28 @@ func TestInmemory(t *testing.T) {
"SourceIP": "10.1.1.12",
"Actor": actor1,
})
status, ok, _ := s.IsAllowed(chain.Ingress, engine.NewRequestTarget(namespace, container), reqBadOperation)
status, ok, _ := s.IsAllowed(chain.Ingress, engine.NewRequestTarget(namespace, container), reqBadOperation, engine.NoClientDefined)
require.Equal(t, chain.AccessDenied, status)
require.True(t, ok)
})
t.Run("inverted rules", func(t *testing.T) {
req := resourcetest.NewRequest("native::object::put", resourcetest.NewResource(object, nil), nil)
status, ok, _ = s.IsAllowed(chain.Ingress, engine.NewRequestTarget(namespace2, container), req)
status, ok, _ = s.IsAllowed(chain.Ingress, engine.NewRequestTarget(namespace2, container), req, engine.NoClientDefined)
require.Equal(t, chain.NoRuleFound, status)
require.False(t, ok)
req = resourcetest.NewRequest("native::object::put", resourcetest.NewResource("native::object::cba/def", nil), nil)
status, ok, _ = s.IsAllowed(chain.Ingress, engine.NewRequestTarget(namespace2, container), req)
status, ok, _ = s.IsAllowed(chain.Ingress, engine.NewRequestTarget(namespace2, container), req, engine.NoClientDefined)
require.Equal(t, chain.AccessDenied, status)
require.True(t, ok)
req = resourcetest.NewRequest("native::object::get", resourcetest.NewResource("native::object::cba/def", nil), nil)
status, ok, _ = s.IsAllowed(chain.Ingress, engine.NewRequestTarget(namespace2, container), req)
status, ok, _ = s.IsAllowed(chain.Ingress, engine.NewRequestTarget(namespace2, container), req, engine.NoClientDefined)
require.Equal(t, chain.NoRuleFound, status)
require.False(t, ok)
})
t.Run("good", func(t *testing.T) {
status, ok, _ = s.IsAllowed(chain.Ingress, engine.NewRequestTarget(namespace, container), reqGood)
status, ok, _ = s.IsAllowed(chain.Ingress, engine.NewRequestTarget(namespace, container), reqGood, engine.NoClientDefined)
require.Equal(t, chain.NoRuleFound, status)
require.False(t, ok)
@ -221,7 +273,7 @@ func TestInmemory(t *testing.T) {
require.NoError(t, err)
itemStacksEqual(t, it.Values, toStackItems(container))
status, ok, _ = s.IsAllowed(chain.Ingress, engine.NewRequestTarget(namespace, container), reqGood)
status, ok, _ = s.IsAllowed(chain.Ingress, engine.NewRequestTarget(namespace, container), reqGood, engine.NoClientDefined)
require.Equal(t, chain.NoRuleFound, status)
require.False(t, ok)
})
@ -244,7 +296,7 @@ func TestInmemory(t *testing.T) {
require.NoError(t, err)
itemStacksEqual(t, it.Values, toStackItems(container))
status, ok, _ = s.IsAllowed(chain.Ingress, engine.NewRequestTarget(namespace, container), reqGood)
status, ok, _ = s.IsAllowed(chain.Ingress, engine.NewRequestTarget(namespace, container), reqGood, engine.NoClientDefined)
require.Equal(t, chain.QuotaLimitReached, status)
require.True(t, ok)
})
@ -260,7 +312,7 @@ func TestInmemory(t *testing.T) {
require.NoError(t, err)
itemStacksEqual(t, it.Values, toStackItems(container))
status, ok, _ = s.IsAllowed(chain.Ingress, engine.NewRequestTarget(namespace, container), reqGood)
status, ok, _ = s.IsAllowed(chain.Ingress, engine.NewRequestTarget(namespace, container), reqGood, engine.NoClientDefined)
require.Equal(t, chain.NoRuleFound, status)
require.False(t, ok)
})

19
pkg/engine/interface.go
View file

@ -8,10 +8,27 @@ import (
"github.com/nspcc-dev/neo-go/pkg/util"
)
type ClientDefinedOverrides interface {
ListOverrides(name chain.Name, target Target) ([]*chain.Chain, error)
}
type noClientDefinedImpl struct{}
func (n *noClientDefinedImpl) ListOverrides(_ chain.Name, _ Target) ([]*chain.Chain, error) {
return []*chain.Chain{}, nil
}
var NoClientDefined ClientDefinedOverrides = &noClientDefinedImpl{}
type ChainRouter interface {
// IsAllowed returns status for the operation after all checks.
// The second return value signifies whether a matching rule was found.
IsAllowed(name chain.Name, reqTarget RequestTarget, r resource.Request) (status chain.Status, found bool, err error)
//
// IsAllowed can be invoked with overrides defined by a client to check the request.
// This check is performed after local overrides but before checking of chains defined
// in morph storage.
// If a client defines no extra chains, NoClientDefined should be passed.
IsAllowed(name chain.Name, reqTarget RequestTarget, r resource.Request, clientDefined ClientDefinedOverrides) (status chain.Status, found bool, err error)
}
// LocalOverrideStorage is the interface to manage local overrides defined