From 1ea74b6b79d3e13b5dcbfd5a2673f33dc13c287f Mon Sep 17 00:00:00 2001 From: Airat Arifullin Date: Thu, 20 Jun 2024 14:50:17 +0300 Subject: [PATCH 1/4] [#1190] ape: Introduce `Groups` util function to retrieve actor's groupIDs Signed-off-by: Airat Arifullin --- pkg/ape/request/frostfsid.go | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/pkg/ape/request/frostfsid.go b/pkg/ape/request/frostfsid.go index fa620e58c..c0413678d 100644 --- a/pkg/ape/request/frostfsid.go +++ b/pkg/ape/request/frostfsid.go @@ -34,3 +34,19 @@ func FormFrostfsIDRequestProperties(frostFSIDClient frostfsidcore.SubjectProvide return reqProps, nil } + +// Groups return the actor's group ids from frostfsid contract. +func Groups(frostFSIDClient frostfsidcore.SubjectProvider, pk *keys.PublicKey) ([]string, error) { + subj, err := frostFSIDClient.GetSubjectExtended(pk.GetScriptHash()) + if err != nil { + if !strings.Contains(err.Error(), frostfsidcore.SubjectNotFoundErrorMessage) { + return nil, fmt.Errorf("get subject error: %w", err) + } + return []string{}, nil + } + groups := make([]string, len(subj.Groups)) + for i, group := range subj.Groups { + groups[i] = strconv.FormatInt(group.ID, 10) + } + return groups, nil +} -- 2.45.3 From fecf47f3164d3bc2ab7f983b86014d287d5d09f3 Mon Sep 17 00:00:00 2001 From: Airat Arifullin Date: Thu, 20 Jun 2024 15:20:48 +0300 Subject: [PATCH 2/4] [#1190] container: GroupIDs must also be target of APE checks * Also add new test case for ape middleware in container service. Signed-off-by: Airat Arifullin --- pkg/services/container/ape.go | 42 ++++++++++++- pkg/services/container/ape_test.go | 99 ++++++++++++++++++++++++++++++ 2 files changed, 140 insertions(+), 1 deletion(-) diff --git a/pkg/services/container/ape.go b/pkg/services/container/ape.go index fb1cc8121..3ea591c6a 100644 --- a/pkg/services/container/ape.go +++ b/pkg/services/container/ape.go @@ -164,11 +164,26 @@ func (ac *apeChecker) List(ctx context.Context, req *container.ListRequest) (*co reqProps, ) + groups, err := aperequest.Groups(ac.frostFSIDClient, pk) + if err != nil { + return nil, fmt.Errorf("failed to get group ids: %w", err) + } + + // Policy contract keeps group related chains as namespace-group pair. + for i := range groups { + groups[i] = fmt.Sprintf("%s:%s", namespace, groups[i]) + } + rt := policyengine.NewRequestTargetWithNamespace(namespace) rt.User = &policyengine.Target{ Type: policyengine.User, Name: fmt.Sprintf("%s:%s", namespace, pk.Address()), } + rt.Groups = make([]policyengine.Target, len(groups)) + for i := range groups { + rt.Groups[i] = policyengine.GroupTarget(groups[i]) + } + s, found, err := ac.router.IsAllowed(apechain.Ingress, rt, request) if err != nil { return nil, err @@ -222,11 +237,26 @@ func (ac *apeChecker) Put(ctx context.Context, req *container.PutRequest) (*cont reqProps, ) + groups, err := aperequest.Groups(ac.frostFSIDClient, pk) + if err != nil { + return nil, fmt.Errorf("failed to get group ids: %w", err) + } + + // Policy contract keeps group related chains as namespace-group pair. + for i := range groups { + groups[i] = fmt.Sprintf("%s:%s", namespace, groups[i]) + } + rt := policyengine.NewRequestTargetWithNamespace(namespace) rt.User = &policyengine.Target{ Type: policyengine.User, Name: fmt.Sprintf("%s:%s", namespace, pk.Address()), } + rt.Groups = make([]policyengine.Target, len(groups)) + for i := range groups { + rt.Groups[i] = policyengine.GroupTarget(groups[i]) + } + s, found, err := ac.router.IsAllowed(apechain.Ingress, rt, request) if err != nil { return nil, err @@ -311,6 +341,16 @@ func (ac *apeChecker) validateContainerBoundedOperation(ctx context.Context, con namespace = cntNamespace } + groups, err := aperequest.Groups(ac.frostFSIDClient, pk) + if err != nil { + return fmt.Errorf("failed to get group ids: %w", err) + } + + // Policy contract keeps group related chains as namespace-group pair. + for i := range groups { + groups[i] = fmt.Sprintf("%s:%s", namespace, groups[i]) + } + request := aperequest.NewRequest( op, aperequest.NewResource( @@ -321,7 +361,7 @@ func (ac *apeChecker) validateContainerBoundedOperation(ctx context.Context, con ) s, found, err := ac.router.IsAllowed(apechain.Ingress, - policyengine.NewRequestTargetExtended(namespace, id.EncodeToString(), fmt.Sprintf("%s:%s", namespace, pk.Address()), nil), + policyengine.NewRequestTargetExtended(namespace, id.EncodeToString(), fmt.Sprintf("%s:%s", namespace, pk.Address()), groups), request) if err != nil { return err diff --git a/pkg/services/container/ape_test.go b/pkg/services/container/ape_test.go index 90c0225dd..a6f0fb222 100644 --- a/pkg/services/container/ape_test.go +++ b/pkg/services/container/ape_test.go @@ -44,6 +44,7 @@ const ( func TestAPE(t *testing.T) { t.Parallel() t.Run("allow then deny get container", testAllowThenDenyGetContainerRuleDefined) + t.Run("allow by group id", TestAllowByGroupIDs) t.Run("deny get container no rule found", testDenyGetContainerNoRuleFound) t.Run("deny get container for others", testDenyGetContainerForOthers) t.Run("deny get container by user claim tag", testDenyGetContainerByUserClaimTag) @@ -145,6 +146,104 @@ func testAllowThenDenyGetContainerRuleDefined(t *testing.T) { require.Contains(t, errAccessDenied.Reason(), chain.AccessDenied.String()) } +func TestAllowByGroupIDs(t *testing.T) { + t.Parallel() + srv := &srvStub{ + calls: map[string]int{}, + } + router := inmemory.NewInMemory() + contRdr := &containerStub{ + c: map[cid.ID]*containercore.Container{}, + } + ir := &irStub{ + keys: [][]byte{}, + } + nm := &netmapStub{} + + pk, err := keys.NewPrivateKey() + require.NoError(t, err) + + frostfsIDSubjectReader := &frostfsidStub{ + subjects: map[util.Uint160]*client.Subject{ + pk.PublicKey().GetScriptHash(): { + KV: map[string]string{ + "tag-attr1": "value1", + "tag-attr2": "value2", + }, + }, + }, + subjectsExt: map[util.Uint160]*client.SubjectExtended{ + pk.PublicKey().GetScriptHash(): { + KV: map[string]string{ + "tag-attr1": "value1", + "tag-attr2": "value2", + }, + Groups: []*client.Group{ + { + ID: 1, + Name: "Group#1", + }, + }, + }, + }, + } + apeSrv := NewAPEServer(router, contRdr, ir, nm, frostfsIDSubjectReader, srv) + + contID := cidtest.ID() + testContainer := containertest.Container() + pp := netmap.PlacementPolicy{} + require.NoError(t, pp.DecodeString("REP 1")) + testContainer.SetPlacementPolicy(pp) + contRdr.c[contID] = &containercore.Container{Value: testContainer} + + nm.currentEpoch = 100 + nm.netmaps = map[uint64]*netmap.NetMap{} + var testNetmap netmap.NetMap + testNetmap.SetEpoch(nm.currentEpoch) + testNetmap.SetNodes([]netmap.NodeInfo{{}}) + nm.netmaps[nm.currentEpoch] = &testNetmap + nm.netmaps[nm.currentEpoch-1] = &testNetmap + + req := &container.GetRequest{} + req.SetBody(&container.GetRequestBody{}) + var refContID refs.ContainerID + contID.WriteToV2(&refContID) + req.GetBody().SetContainerID(&refContID) + + require.NoError(t, signature.SignServiceMessage(&pk.PrivateKey, req)) + + _, _, err = router.MorphRuleChainStorage().AddMorphRuleChain(chain.Ingress, engine.GroupTarget(":1"), &chain.Chain{ + Rules: []chain.Rule{ + { + Status: chain.Allow, + Actions: chain.Actions{ + Names: []string{ + nativeschema.MethodGetContainer, + }, + }, + Resources: chain.Resources{ + Names: []string{ + fmt.Sprintf(nativeschema.ResourceFormatRootContainer, contID.EncodeToString()), + }, + }, + Condition: []chain.Condition{ + { + Kind: chain.KindRequest, + Key: commonschema.PropertyKeyFrostFSIDGroupID, + Value: "1", + Op: chain.CondStringEquals, + }, + }, + }, + }, + }) + require.NoError(t, err) + + resp, err := apeSrv.Get(context.Background(), req) + require.NotNil(t, resp) + require.NoError(t, err) +} + func testDenyGetContainerNoRuleFound(t *testing.T) { t.Parallel() srv := &srvStub{ -- 2.45.3 From 76f99cfab2ecc102d1e874c4a26b4f86ae0554f9 Mon Sep 17 00:00:00 2001 From: Airat Arifullin Date: Thu, 20 Jun 2024 15:39:39 +0300 Subject: [PATCH 3/4] [#1190] object: GroupIDs must also be target of APE checks * Also add new test case for ape middleware in container service. Signed-off-by: Airat Arifullin --- pkg/services/object/ape/checker.go | 13 ++++++- pkg/services/object/ape/checker_test.go | 51 +++++++++++++++++++++++-- 2 files changed, 58 insertions(+), 6 deletions(-) diff --git a/pkg/services/object/ape/checker.go b/pkg/services/object/ape/checker.go index ee71e6e1d..ee12d7b97 100644 --- a/pkg/services/object/ape/checker.go +++ b/pkg/services/object/ape/checker.go @@ -7,6 +7,7 @@ import ( "fmt" objectV2 "git.frostfs.info/TrueCloudLab/frostfs-api-go/v2/object" + aperequest "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/ape/request" "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/ape/router" "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/core/container" frostfsidcore "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/core/frostfsid" @@ -159,11 +160,19 @@ func (c *checkerImpl) CheckAPE(ctx context.Context, prm Prm) error { if err != nil { return fmt.Errorf("failed to create ape request: %w", err) } - pub, err := keys.NewPublicKeyFromString(prm.SenderKey) if err != nil { return err } + groups, err := aperequest.Groups(c.frostFSIDClient, pub) + if err != nil { + return fmt.Errorf("failed to get group ids: %w", err) + } + + // Policy contract keeps group related chains as namespace-group pair. + for i := range groups { + groups[i] = fmt.Sprintf("%s:%s", prm.Namespace, groups[i]) + } if prm.BearerToken != nil && !prm.BearerToken.Impersonate() { if err := isValidBearer(prm.BearerToken, prm.ContainerOwner, prm.Container, pub, c.st); err != nil { @@ -185,7 +194,7 @@ func (c *checkerImpl) CheckAPE(ctx context.Context, prm Prm) error { } } - rt := policyengine.NewRequestTargetExtended(prm.Namespace, prm.Container.EncodeToString(), fmt.Sprintf("%s:%s", prm.Namespace, pub.Address()), nil) + rt := policyengine.NewRequestTargetExtended(prm.Namespace, prm.Container.EncodeToString(), fmt.Sprintf("%s:%s", prm.Namespace, pub.Address()), groups) status, ruleFound, err := c.chainRouter.IsAllowed(apechain.Ingress, rt, r) if err != nil { return err diff --git a/pkg/services/object/ape/checker_test.go b/pkg/services/object/ape/checker_test.go index 09954602c..5efd3669b 100644 --- a/pkg/services/object/ape/checker_test.go +++ b/pkg/services/object/ape/checker_test.go @@ -27,6 +27,7 @@ import ( "git.frostfs.info/TrueCloudLab/policy-engine/pkg/chain" policyengine "git.frostfs.info/TrueCloudLab/policy-engine/pkg/engine" "git.frostfs.info/TrueCloudLab/policy-engine/pkg/engine/inmemory" + commonschema "git.frostfs.info/TrueCloudLab/policy-engine/schema/common" nativeschema "git.frostfs.info/TrueCloudLab/policy-engine/schema/native" "github.com/nspcc-dev/neo-go/pkg/crypto/keys" "github.com/nspcc-dev/neo-go/pkg/util" @@ -159,6 +160,8 @@ var ( objectID = "BzQw5HH3feoxFDD5tCT87Y1726qzgLfxEE7wgtoRzB3R" + groupID = "1" + role = "Container" senderPrivateKey, _ = keys.NewPrivateKey() @@ -238,6 +241,7 @@ var apeCheckTestCases = []struct { methods []string header testHeader containerRules []chain.Rule + groupidRules []chain.Rule expectAPEErr bool }{ { @@ -393,6 +397,36 @@ var apeCheckTestCases = []struct { }, expectAPEErr: true, }, + { + name: "optional oid requests reached quota limit by group-id", + container: containerID, + methods: methodsOptionalOID, + header: testHeader{ + headerObjSDK: &headerObjectSDKParams{ + payloadSize: 1000, + }, + fromRequestResponseHeader: true, + }, + groupidRules: []chain.Rule{ + { + Status: chain.QuotaLimitReached, + Actions: chain.Actions{Names: methodsOptionalOID}, + Resources: chain.Resources{ + Names: []string{fmt.Sprintf(nativeschema.ResourceFormatRootContainerObjects, containerID)}, + }, + Any: true, + Condition: []chain.Condition{ + { + Op: chain.CondStringEquals, + Kind: chain.KindRequest, + Key: commonschema.PropertyKeyFrostFSIDGroupID, + Value: groupID, + }, + }, + }, + }, + expectAPEErr: true, + }, } type stMock struct{} @@ -486,10 +520,19 @@ func TestAPECheck(t *testing.T) { ls := inmemory.NewInmemoryLocalStorage() ms := inmemory.NewInmemoryMorphRuleChainStorage() - ls.AddOverride(chain.Ingress, policyengine.ContainerTarget(test.container), &chain.Chain{ - Rules: test.containerRules, - MatchType: chain.MatchTypeFirstMatch, - }) + if len(test.containerRules) > 0 { + ls.AddOverride(chain.Ingress, policyengine.ContainerTarget(test.container), &chain.Chain{ + Rules: test.containerRules, + MatchType: chain.MatchTypeFirstMatch, + }) + } + + if len(test.groupidRules) > 0 { + ls.AddOverride(chain.Ingress, policyengine.GroupTarget(":"+groupID), &chain.Chain{ + Rules: test.groupidRules, + MatchType: chain.MatchTypeFirstMatch, + }) + } router := policyengine.NewDefaultChainRouterWithLocalOverrides(ms, ls) -- 2.45.3 From 63c28f00524d35efe2ae35b02a78461d0ec93465 Mon Sep 17 00:00:00 2001 From: Airat Arifullin Date: Thu, 20 Jun 2024 15:49:22 +0300 Subject: [PATCH 4/4] [#1190] tree: GroupIDs must also be target of APE checks Signed-off-by: Airat Arifullin --- pkg/services/tree/ape.go | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/pkg/services/tree/ape.go b/pkg/services/tree/ape.go index 475567c5f..116adf5db 100644 --- a/pkg/services/tree/ape.go +++ b/pkg/services/tree/ape.go @@ -161,7 +161,17 @@ func (s *Service) checkAPE(ctx context.Context, bt *bearer.Token, } } - rt := engine.NewRequestTargetExtended(namespace, cid.EncodeToString(), fmt.Sprintf("%s:%s", namespace, publicKey.Address()), nil) + groups, err := aperequest.Groups(s.frostfsidSubjectProvider, publicKey) + if err != nil { + return fmt.Errorf("failed to get group ids: %w", err) + } + + // Policy contract keeps group related chains as namespace-group pair. + for i := range groups { + groups[i] = fmt.Sprintf("%s:%s", namespace, groups[i]) + } + + rt := engine.NewRequestTargetExtended(namespace, cid.EncodeToString(), fmt.Sprintf("%s:%s", namespace, publicKey.Address()), groups) status, found, err := s.router.IsAllowed(apechain.Ingress, rt, request) if err != nil { return apeErr(err) -- 2.45.3