Make services GroupIDs must also be target of APE checks #1193

Merged
fyrchik merged 4 commits from aarifullin/frostfs-node:feat/groupids_target into master 2024-06-25 08:49:27 +00:00
6 changed files with 225 additions and 8 deletions

View file

@ -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())
Review

We have cache in frostfsIDClient that's why we can freely invoke Groups function along with FormFrostfsIDRequestProperties ?

We have cache in `frostfsIDClient` that's why we can freely invoke `Groups` function along with `FormFrostfsIDRequestProperties` ?
Review

we can freely

Sorry, I don't get your point. Do you mean we can retrieve groups after FormFrostfsIDRequestProperties? Yes, we can but this requires some hurtful refactor in all services. For me that was easier to perform some duplication as the cache is used

> we can freely Sorry, I don't get your point. Do you mean we can retrieve groups after `FormFrostfsIDRequestProperties`? Yes, we can but this requires some hurtful refactor in all services. For me that was easier to perform some duplication as the cache is used
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
}

View file

@ -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

View file

@ -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{

View file

@ -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

View file

@ -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)

View file

@ -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)