From e9363a5094c308d834d6cdb8e4d68fb4d554f095 Mon Sep 17 00:00:00 2001 From: Airat Arifullin Date: Wed, 14 Feb 2024 14:14:07 +0300 Subject: [PATCH 1/3] [#986] object: Introduce soft ape checks * Soft APE check means that APE should allow request even it gets status NoRuleFound for a request. Otherwise, it is interpreted as Deny. * Soft APE check is performed if basic ACL mask is not set. Signed-off-by: Airat Arifullin --- pkg/services/object/acl/v2/request.go | 7 ++ pkg/services/object/acl/v2/service.go | 107 ++++++++++-------- pkg/services/object/ape/checker.go | 7 +- pkg/services/object/ape/checker_test.go | 18 +++ pkg/services/object/ape/service.go | 143 ++++++++++++++---------- pkg/services/object/request_context.go | 2 + 6 files changed, 176 insertions(+), 108 deletions(-) diff --git a/pkg/services/object/acl/v2/request.go b/pkg/services/object/acl/v2/request.go index c58c00e0e..74279e453 100644 --- a/pkg/services/object/acl/v2/request.go +++ b/pkg/services/object/acl/v2/request.go @@ -104,6 +104,13 @@ func (r RequestInfo) RequestRole() acl.Role { return r.requestRole } +// IsSoftAPECheck states if APE should perform soft checks. +// Soft APE check allows a request if CheckAPE returns NoRuleFound for it, +// otherwise it denies the request. +func (r RequestInfo) IsSoftAPECheck() bool { + return r.BasicACL().Bits() != 0 +} + // MetaWithToken groups session and bearer tokens, // verification header and raw API request. type MetaWithToken struct { diff --git a/pkg/services/object/acl/v2/service.go b/pkg/services/object/acl/v2/service.go index af26a6fd1..b51386e0c 100644 --- a/pkg/services/object/acl/v2/service.go +++ b/pkg/services/object/acl/v2/service.go @@ -113,9 +113,10 @@ type wrappedGetObjectStream struct { func (w *wrappedGetObjectStream) Context() context.Context { return context.WithValue(w.GetObjectStream.Context(), object.RequestContextKey, &object.RequestContext{ - Namespace: w.requestInfo.ContainerNamespace(), - SenderKey: w.requestInfo.SenderKey(), - Role: w.requestInfo.RequestRole(), + Namespace: w.requestInfo.ContainerNamespace(), + SenderKey: w.requestInfo.SenderKey(), + Role: w.requestInfo.RequestRole(), + SoftAPECheck: w.requestInfo.IsSoftAPECheck(), }) } @@ -136,9 +137,10 @@ type wrappedRangeStream struct { func (w *wrappedRangeStream) Context() context.Context { return context.WithValue(w.GetObjectRangeStream.Context(), object.RequestContextKey, &object.RequestContext{ - Namespace: w.requestInfo.ContainerNamespace(), - SenderKey: w.requestInfo.SenderKey(), - Role: w.requestInfo.RequestRole(), + Namespace: w.requestInfo.ContainerNamespace(), + SenderKey: w.requestInfo.SenderKey(), + Role: w.requestInfo.RequestRole(), + SoftAPECheck: w.requestInfo.IsSoftAPECheck(), }) } @@ -159,9 +161,10 @@ type wrappedSearchStream struct { func (w *wrappedSearchStream) Context() context.Context { return context.WithValue(w.SearchStream.Context(), object.RequestContextKey, &object.RequestContext{ - Namespace: w.requestInfo.ContainerNamespace(), - SenderKey: w.requestInfo.SenderKey(), - Role: w.requestInfo.RequestRole(), + Namespace: w.requestInfo.ContainerNamespace(), + SenderKey: w.requestInfo.SenderKey(), + Role: w.requestInfo.RequestRole(), + SoftAPECheck: w.requestInfo.IsSoftAPECheck(), }) } @@ -216,10 +219,12 @@ func (b Service) Get(request *objectV2.GetRequest, stream object.GetObjectStream reqInfo.obj = obj - if !b.checker.CheckBasicACL(reqInfo) { - return basicACLErr(reqInfo) - } else if err := b.checker.CheckEACL(request, reqInfo); err != nil { - return eACLErr(reqInfo, err) + if reqInfo.IsSoftAPECheck() { + if !b.checker.CheckBasicACL(reqInfo) { + return basicACLErr(reqInfo) + } else if err := b.checker.CheckEACL(request, reqInfo); err != nil { + return eACLErr(reqInfo, err) + } } return b.next.Get(request, &getStreamBasicChecker{ @@ -283,10 +288,12 @@ func (b Service) Head( reqInfo.obj = obj - if !b.checker.CheckBasicACL(reqInfo) { - return nil, basicACLErr(reqInfo) - } else if err := b.checker.CheckEACL(request, reqInfo); err != nil { - return nil, eACLErr(reqInfo, err) + if reqInfo.IsSoftAPECheck() { + if !b.checker.CheckBasicACL(reqInfo) { + return nil, basicACLErr(reqInfo) + } else if err := b.checker.CheckEACL(request, reqInfo); err != nil { + return nil, eACLErr(reqInfo, err) + } } resp, err := b.next.Head(requestContext(ctx, reqInfo), request) @@ -334,10 +341,12 @@ func (b Service) Search(request *objectV2.SearchRequest, stream object.SearchStr return err } - if !b.checker.CheckBasicACL(reqInfo) { - return basicACLErr(reqInfo) - } else if err := b.checker.CheckEACL(request, reqInfo); err != nil { - return eACLErr(reqInfo, err) + if reqInfo.IsSoftAPECheck() { + if !b.checker.CheckBasicACL(reqInfo) { + return basicACLErr(reqInfo) + } else if err := b.checker.CheckEACL(request, reqInfo); err != nil { + return eACLErr(reqInfo, err) + } } return b.next.Search(request, &searchStreamBasicChecker{ @@ -392,10 +401,12 @@ func (b Service) Delete( reqInfo.obj = obj - if !b.checker.CheckBasicACL(reqInfo) { - return nil, basicACLErr(reqInfo) - } else if err := b.checker.CheckEACL(request, reqInfo); err != nil { - return nil, eACLErr(reqInfo, err) + if reqInfo.IsSoftAPECheck() { + if !b.checker.CheckBasicACL(reqInfo) { + return nil, basicACLErr(reqInfo) + } else if err := b.checker.CheckEACL(request, reqInfo); err != nil { + return nil, eACLErr(reqInfo, err) + } } return b.next.Delete(requestContext(ctx, reqInfo), request) @@ -443,10 +454,12 @@ func (b Service) GetRange(request *objectV2.GetRangeRequest, stream object.GetOb reqInfo.obj = obj - if !b.checker.CheckBasicACL(reqInfo) { - return basicACLErr(reqInfo) - } else if err := b.checker.CheckEACL(request, reqInfo); err != nil { - return eACLErr(reqInfo, err) + if reqInfo.IsSoftAPECheck() { + if !b.checker.CheckBasicACL(reqInfo) { + return basicACLErr(reqInfo) + } else if err := b.checker.CheckEACL(request, reqInfo); err != nil { + return eACLErr(reqInfo, err) + } } return b.next.GetRange(request, &rangeStreamBasicChecker{ @@ -458,9 +471,10 @@ func (b Service) GetRange(request *objectV2.GetRangeRequest, stream object.GetOb func requestContext(ctx context.Context, reqInfo RequestInfo) context.Context { return context.WithValue(ctx, object.RequestContextKey, &object.RequestContext{ - Namespace: reqInfo.ContainerNamespace(), - SenderKey: reqInfo.SenderKey(), - Role: reqInfo.RequestRole(), + Namespace: reqInfo.ContainerNamespace(), + SenderKey: reqInfo.SenderKey(), + Role: reqInfo.RequestRole(), + SoftAPECheck: reqInfo.IsSoftAPECheck(), }) } @@ -509,10 +523,12 @@ func (b Service) GetRangeHash( reqInfo.obj = obj - if !b.checker.CheckBasicACL(reqInfo) { - return nil, basicACLErr(reqInfo) - } else if err := b.checker.CheckEACL(request, reqInfo); err != nil { - return nil, eACLErr(reqInfo, err) + if reqInfo.IsSoftAPECheck() { + if !b.checker.CheckBasicACL(reqInfo) { + return nil, basicACLErr(reqInfo) + } else if err := b.checker.CheckEACL(request, reqInfo); err != nil { + return nil, eACLErr(reqInfo, err) + } } return b.next.GetRangeHash(requestContext(ctx, reqInfo), request) @@ -566,12 +582,13 @@ func (b Service) PutSingle(ctx context.Context, request *objectV2.PutSingleReque reqInfo.obj = obj - if !b.checker.CheckBasicACL(reqInfo) || !b.checker.StickyBitCheck(reqInfo, idOwner) { - return nil, basicACLErr(reqInfo) - } - - if err := b.checker.CheckEACL(request, reqInfo); err != nil { - return nil, eACLErr(reqInfo, err) + if reqInfo.IsSoftAPECheck() { + if !b.checker.CheckBasicACL(reqInfo) || !b.checker.StickyBitCheck(reqInfo, idOwner) { + return nil, basicACLErr(reqInfo) + } + if err := b.checker.CheckEACL(request, reqInfo); err != nil { + return nil, eACLErr(reqInfo, err) + } } return b.next.PutSingle(requestContext(ctx, reqInfo), request) @@ -639,8 +656,10 @@ func (p putStreamBasicChecker) Send(ctx context.Context, request *objectV2.PutRe reqInfo.obj = obj - if !p.source.checker.CheckBasicACL(reqInfo) || !p.source.checker.StickyBitCheck(reqInfo, idOwner) { - return basicACLErr(reqInfo) + if reqInfo.IsSoftAPECheck() { + if !p.source.checker.CheckBasicACL(reqInfo) || !p.source.checker.StickyBitCheck(reqInfo, idOwner) { + return basicACLErr(reqInfo) + } } ctx = requestContext(ctx, reqInfo) diff --git a/pkg/services/object/ape/checker.go b/pkg/services/object/ape/checker.go index 76ada3d2b..13b2729e9 100644 --- a/pkg/services/object/ape/checker.go +++ b/pkg/services/object/ape/checker.go @@ -45,6 +45,9 @@ type Prm struct { // An encoded sender's public key string. SenderKey string + + // If SoftAPECheck is set to true, then NoRuleFound is interpreted as allow. + SoftAPECheck bool } var errMissingOID = fmt.Errorf("object ID is not set") @@ -63,9 +66,9 @@ func (c *checkerImpl) CheckAPE(ctx context.Context, prm Prm) error { return err } - if !ruleFound || status == apechain.Allow { + if !ruleFound && prm.SoftAPECheck || status == apechain.Allow { return nil } - return fmt.Errorf("found denying rule for %s: %s", prm.Method, status) + return fmt.Errorf("method %s: %s", prm.Method, status) } diff --git a/pkg/services/object/ape/checker_test.go b/pkg/services/object/ape/checker_test.go index 08d097907..d7e97064b 100644 --- a/pkg/services/object/ape/checker_test.go +++ b/pkg/services/object/ape/checker_test.go @@ -165,11 +165,29 @@ func TestAPECheck(t *testing.T) { container: containerID, object: stringPtr(objectID), methods: methodsRequiredOID, + containerRules: []chain.Rule{ + { + Status: chain.Allow, + Actions: chain.Actions{Names: methodsRequiredOID}, + Resources: chain.Resources{ + Names: []string{fmt.Sprintf(nativeschema.ResourceFormatRootContainerObject, containerID, objectID)}, + }, + }, + }, }, { name: "oid optional requests are allowed", container: containerID, methods: methodsOptionalOID, + containerRules: []chain.Rule{ + { + Status: chain.Allow, + Actions: chain.Actions{Names: methodsOptionalOID}, + Resources: chain.Resources{ + Names: []string{fmt.Sprintf(nativeschema.ResourceFormatRootContainerObjects, containerID)}, + }, + }, + }, }, { name: "oid required requests are denied", diff --git a/pkg/services/object/ape/service.go b/pkg/services/object/ape/service.go index 781f9df4b..f05bdaa83 100644 --- a/pkg/services/object/ape/service.go +++ b/pkg/services/object/ape/service.go @@ -60,9 +60,13 @@ type getStreamBasicChecker struct { apeChecker Checker + namespace string + senderKey []byte role string + + softAPECheck bool } func (g *getStreamBasicChecker) Send(resp *objectV2.GetResponse) error { @@ -73,12 +77,14 @@ func (g *getStreamBasicChecker) Send(resp *objectV2.GetResponse) error { } prm := Prm{ - Container: cnrID, - Object: objID, - Header: partInit.GetHeader(), - Method: nativeschema.MethodGetObject, - SenderKey: hex.EncodeToString(g.senderKey), - Role: g.role, + Namespace: g.namespace, + Container: cnrID, + Object: objID, + Header: partInit.GetHeader(), + Method: nativeschema.MethodGetObject, + SenderKey: hex.EncodeToString(g.senderKey), + Role: g.role, + SoftAPECheck: g.softAPECheck, } if err := g.apeChecker.CheckAPE(g.Context(), prm); err != nil { @@ -112,12 +118,13 @@ func (c *Service) Get(request *objectV2.GetRequest, stream objectSvc.GetObjectSt } err = c.apeChecker.CheckAPE(stream.Context(), Prm{ - Namespace: reqCtx.Namespace, - Container: cnrID, - Object: objID, - Method: nativeschema.MethodGetObject, - Role: nativeSchemaRole(reqCtx.Role), - SenderKey: hex.EncodeToString(reqCtx.SenderKey), + Namespace: reqCtx.Namespace, + Container: cnrID, + Object: objID, + Method: nativeschema.MethodGetObject, + Role: nativeSchemaRole(reqCtx.Role), + SenderKey: hex.EncodeToString(reqCtx.SenderKey), + SoftAPECheck: reqCtx.SoftAPECheck, }) if err != nil { return toStatusErr(err) @@ -126,6 +133,10 @@ func (c *Service) Get(request *objectV2.GetRequest, stream objectSvc.GetObjectSt return c.next.Get(request, &getStreamBasicChecker{ GetObjectStream: stream, apeChecker: c.apeChecker, + namespace: reqCtx.Namespace, + senderKey: reqCtx.SenderKey, + role: nativeSchemaRole(reqCtx.Role), + softAPECheck: reqCtx.SoftAPECheck, }) } @@ -148,13 +159,14 @@ func (p *putStreamBasicChecker) Send(ctx context.Context, request *objectV2.PutR } prm := Prm{ - Namespace: reqCtx.Namespace, - Container: cnrID, - Object: objID, - Header: partInit.GetHeader(), - Method: nativeschema.MethodPutObject, - SenderKey: hex.EncodeToString(reqCtx.SenderKey), - Role: nativeSchemaRole(reqCtx.Role), + Namespace: reqCtx.Namespace, + Container: cnrID, + Object: objID, + Header: partInit.GetHeader(), + Method: nativeschema.MethodPutObject, + SenderKey: hex.EncodeToString(reqCtx.SenderKey), + Role: nativeSchemaRole(reqCtx.Role), + SoftAPECheck: reqCtx.SoftAPECheck, } if err := p.apeChecker.CheckAPE(ctx, prm); err != nil { @@ -190,12 +202,13 @@ func (c *Service) Head(ctx context.Context, request *objectV2.HeadRequest) (*obj } err = c.apeChecker.CheckAPE(ctx, Prm{ - Namespace: reqCtx.Namespace, - Container: cnrID, - Object: objID, - Method: nativeschema.MethodHeadObject, - Role: nativeSchemaRole(reqCtx.Role), - SenderKey: hex.EncodeToString(reqCtx.SenderKey), + Namespace: reqCtx.Namespace, + Container: cnrID, + Object: objID, + Method: nativeschema.MethodHeadObject, + Role: nativeSchemaRole(reqCtx.Role), + SenderKey: hex.EncodeToString(reqCtx.SenderKey), + SoftAPECheck: reqCtx.SoftAPECheck, }) if err != nil { return nil, err @@ -226,13 +239,14 @@ func (c *Service) Head(ctx context.Context, request *objectV2.HeadRequest) (*obj } err = c.apeChecker.CheckAPE(ctx, Prm{ - Namespace: reqCtx.Namespace, - Container: cnrID, - Object: objID, - Header: header, - Method: nativeschema.MethodHeadObject, - Role: nativeSchemaRole(reqCtx.Role), - SenderKey: hex.EncodeToString(reqCtx.SenderKey), + Namespace: reqCtx.Namespace, + Container: cnrID, + Object: objID, + Header: header, + Method: nativeschema.MethodHeadObject, + Role: nativeSchemaRole(reqCtx.Role), + SenderKey: hex.EncodeToString(reqCtx.SenderKey), + SoftAPECheck: reqCtx.SoftAPECheck, }) if err != nil { return nil, err @@ -254,11 +268,12 @@ func (c *Service) Search(request *objectV2.SearchRequest, stream objectSvc.Searc } err = c.apeChecker.CheckAPE(stream.Context(), Prm{ - Namespace: reqCtx.Namespace, - Container: cnrID, - Method: nativeschema.MethodSearchObject, - Role: nativeSchemaRole(reqCtx.Role), - SenderKey: hex.EncodeToString(reqCtx.SenderKey), + Namespace: reqCtx.Namespace, + Container: cnrID, + Method: nativeschema.MethodSearchObject, + Role: nativeSchemaRole(reqCtx.Role), + SenderKey: hex.EncodeToString(reqCtx.SenderKey), + SoftAPECheck: reqCtx.SoftAPECheck, }) if err != nil { return toStatusErr(err) @@ -279,12 +294,13 @@ func (c *Service) Delete(ctx context.Context, request *objectV2.DeleteRequest) ( } err = c.apeChecker.CheckAPE(ctx, Prm{ - Namespace: reqCtx.Namespace, - Container: cnrID, - Object: objID, - Method: nativeschema.MethodDeleteObject, - Role: nativeSchemaRole(reqCtx.Role), - SenderKey: hex.EncodeToString(reqCtx.SenderKey), + Namespace: reqCtx.Namespace, + Container: cnrID, + Object: objID, + Method: nativeschema.MethodDeleteObject, + Role: nativeSchemaRole(reqCtx.Role), + SenderKey: hex.EncodeToString(reqCtx.SenderKey), + SoftAPECheck: reqCtx.SoftAPECheck, }) if err != nil { return nil, err @@ -310,12 +326,13 @@ func (c *Service) GetRange(request *objectV2.GetRangeRequest, stream objectSvc.G } err = c.apeChecker.CheckAPE(stream.Context(), Prm{ - Namespace: reqCtx.Namespace, - Container: cnrID, - Object: objID, - Method: nativeschema.MethodRangeObject, - Role: nativeSchemaRole(reqCtx.Role), - SenderKey: hex.EncodeToString(reqCtx.SenderKey), + Namespace: reqCtx.Namespace, + Container: cnrID, + Object: objID, + Method: nativeschema.MethodRangeObject, + Role: nativeSchemaRole(reqCtx.Role), + SenderKey: hex.EncodeToString(reqCtx.SenderKey), + SoftAPECheck: reqCtx.SoftAPECheck, }) if err != nil { return toStatusErr(err) @@ -336,12 +353,13 @@ func (c *Service) GetRangeHash(ctx context.Context, request *objectV2.GetRangeHa } prm := Prm{ - Namespace: reqCtx.Namespace, - Container: cnrID, - Object: objID, - Method: nativeschema.MethodHashObject, - Role: nativeSchemaRole(reqCtx.Role), - SenderKey: hex.EncodeToString(reqCtx.SenderKey), + Namespace: reqCtx.Namespace, + Container: cnrID, + Object: objID, + Method: nativeschema.MethodHashObject, + Role: nativeSchemaRole(reqCtx.Role), + SenderKey: hex.EncodeToString(reqCtx.SenderKey), + SoftAPECheck: reqCtx.SoftAPECheck, } if err = c.apeChecker.CheckAPE(ctx, prm); err != nil { @@ -371,13 +389,14 @@ func (c *Service) PutSingle(ctx context.Context, request *objectV2.PutSingleRequ } prm := Prm{ - Namespace: reqCtx.Namespace, - Container: cnrID, - Object: objID, - Header: request.GetBody().GetObject().GetHeader(), - Method: nativeschema.MethodPutObject, - Role: nativeSchemaRole(reqCtx.Role), - SenderKey: hex.EncodeToString(reqCtx.SenderKey), + Namespace: reqCtx.Namespace, + Container: cnrID, + Object: objID, + Header: request.GetBody().GetObject().GetHeader(), + Method: nativeschema.MethodPutObject, + Role: nativeSchemaRole(reqCtx.Role), + SenderKey: hex.EncodeToString(reqCtx.SenderKey), + SoftAPECheck: reqCtx.SoftAPECheck, } if err = c.apeChecker.CheckAPE(ctx, prm); err != nil { diff --git a/pkg/services/object/request_context.go b/pkg/services/object/request_context.go index 4b9aa04d1..e0e6c9ed9 100644 --- a/pkg/services/object/request_context.go +++ b/pkg/services/object/request_context.go @@ -13,4 +13,6 @@ type RequestContext struct { SenderKey []byte Role acl.Role + + SoftAPECheck bool } -- 2.45.2 From 88cbe2db554701b76de98046fd631ae91619aeeb Mon Sep 17 00:00:00 2001 From: Airat Arifullin Date: Wed, 14 Feb 2024 18:14:06 +0300 Subject: [PATCH 2/3] [#986] container: Interpret APE NoRuleFound as request deny * If APE check returns NoRuleFound, then it is taken for request deny. * Add more unit-test for ape container middleware. Signed-off-by: Airat Arifullin --- pkg/services/container/ape.go | 8 +- pkg/services/container/ape_test.go | 155 +++++++++++++++++++++++++++++ 2 files changed, 159 insertions(+), 4 deletions(-) diff --git a/pkg/services/container/ape.go b/pkg/services/container/ape.go index d1b490bdf..c01ad8a1d 100644 --- a/pkg/services/container/ape.go +++ b/pkg/services/container/ape.go @@ -162,7 +162,7 @@ func (ac *apeChecker) List(ctx context.Context, req *container.ListRequest) (*co return nil, err } - if !found || s == apechain.Allow { + if found && s == apechain.Allow { return ac.next.List(ctx, req) } @@ -207,7 +207,7 @@ func (ac *apeChecker) Put(ctx context.Context, req *container.PutRequest) (*cont return nil, err } - if !found || s == apechain.Allow { + if found && s == apechain.Allow { return ac.next.Put(ctx, req) } @@ -296,13 +296,13 @@ func (ac *apeChecker) validateContainerBoundedOperation(containerID *refs.Contai } s, found, err := ac.router.IsAllowed(apechain.Ingress, - policyengine.NewRequestTarget(cntNamespace, id.EncodeToString()), + policyengine.NewRequestTarget(namespace, id.EncodeToString()), request) if err != nil { return err } - if !found || s == apechain.Allow { + if found && s == apechain.Allow { return nil } diff --git a/pkg/services/container/ape_test.go b/pkg/services/container/ape_test.go index daaacb031..1af59d1c6 100644 --- a/pkg/services/container/ape_test.go +++ b/pkg/services/container/ape_test.go @@ -39,6 +39,8 @@ const ( func TestAPE(t *testing.T) { t.Parallel() + t.Run("allow then deny get container", testAllowThenDenyGetContainerRuleDefined) + t.Run("deny get container no rule found", testDenyGetContainerNoRuleFound) t.Run("deny get container for others", testDenyGetContainerForOthers) t.Run("deny set container eACL for IR", testDenySetContainerEACLForIR) t.Run("deny get container eACL for IR with session token", testDenyGetContainerEACLForIRSessionToken) @@ -49,6 +51,130 @@ func TestAPE(t *testing.T) { t.Run("deny list containers by namespace invalidation", testDenyListContainersValidationNamespaceError) } +func testAllowThenDenyGetContainerRuleDefined(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{} + frostfsIDSubjectReader := &frostfsidStub{ + subjects: map[util.Uint160]*client.Subject{}, + } + 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 + + addDefaultAllowGetPolicy(t, router, contID) + + req := &container.GetRequest{} + req.SetBody(&container.GetRequestBody{}) + var refContID refs.ContainerID + contID.WriteToV2(&refContID) + req.GetBody().SetContainerID(&refContID) + + pk, err := keys.NewPrivateKey() + require.NoError(t, err) + require.NoError(t, signature.SignServiceMessage(&pk.PrivateKey, req)) + + _, err = apeSrv.Get(context.Background(), req) + require.NoError(t, err) + + _, _, err = router.MorphRuleChainStorage().AddMorphRuleChain(chain.Ingress, engine.ContainerTarget(contID.EncodeToString()), &chain.Chain{ + Rules: []chain.Rule{ + { + Status: chain.AccessDenied, + Actions: chain.Actions{ + Names: []string{ + nativeschema.MethodGetContainer, + }, + }, + Resources: chain.Resources{ + Names: []string{ + fmt.Sprintf(nativeschema.ResourceFormatRootContainer, contID.EncodeToString()), + }, + }, + }, + }, + }) + require.NoError(t, err) + + resp, err := apeSrv.Get(context.Background(), req) + require.Nil(t, resp) + var errAccessDenied *apistatus.ObjectAccessDenied + require.ErrorAs(t, err, &errAccessDenied) + require.Contains(t, errAccessDenied.Reason(), chain.AccessDenied.String()) +} + +func testDenyGetContainerNoRuleFound(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{} + frostfsIDSubjectReader := &frostfsidStub{ + subjects: map[util.Uint160]*client.Subject{}, + } + 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) + + pk, err := keys.NewPrivateKey() + require.NoError(t, err) + require.NoError(t, signature.SignServiceMessage(&pk.PrivateKey, req)) + + resp, err := apeSrv.Get(context.Background(), req) + require.Nil(t, resp) + var errAccessDenied *apistatus.ObjectAccessDenied + require.ErrorAs(t, err, &errAccessDenied) + require.Contains(t, errAccessDenied.Reason(), chain.NoRuleFound.String()) +} + func testDenyGetContainerForOthers(t *testing.T) { t.Parallel() srv := &srvStub{ @@ -854,6 +980,8 @@ func TestValidateContainerBoundedOperation(t *testing.T) { }) require.NoError(t, err) + addDefaultAllowGetPolicy(t, components.engine, contID) + req := initTestGetContainerRequest(t, contID) err = components.apeChecker.validateContainerBoundedOperation(req.GetBody().GetContainerID(), req.GetMetaHeader(), req.GetVerificationHeader(), nativeschema.MethodGetContainer) @@ -895,6 +1023,8 @@ func TestValidateContainerBoundedOperation(t *testing.T) { }) require.NoError(t, err) + addDefaultAllowGetPolicy(t, components.engine, contID) + req := initTestGetContainerRequest(t, contID) err = components.apeChecker.validateContainerBoundedOperation(req.GetBody().GetContainerID(), req.GetMetaHeader(), req.GetVerificationHeader(), nativeschema.MethodGetContainer) @@ -936,6 +1066,8 @@ func TestValidateContainerBoundedOperation(t *testing.T) { }) require.NoError(t, err) + addDefaultAllowGetPolicy(t, components.engine, contID) + req := initTestGetContainerRequest(t, contID) err = components.apeChecker.validateContainerBoundedOperation(req.GetBody().GetContainerID(), req.GetMetaHeader(), req.GetVerificationHeader(), nativeschema.MethodGetContainer) @@ -977,6 +1109,8 @@ func TestValidateContainerBoundedOperation(t *testing.T) { }) require.NoError(t, err) + addDefaultAllowGetPolicy(t, components.engine, contID) + req := initTestGetContainerRequest(t, contID) err = components.apeChecker.validateContainerBoundedOperation(req.GetBody().GetContainerID(), req.GetMetaHeader(), req.GetVerificationHeader(), nativeschema.MethodGetContainer) @@ -1128,3 +1262,24 @@ func initListRequest(t *testing.T, actorPK *keys.PrivateKey, ownerPK *keys.Priva require.NoError(t, signature.SignServiceMessage(&actorPK.PrivateKey, req)) return req } + +func addDefaultAllowGetPolicy(t *testing.T, e engine.Engine, contID cid.ID) { + _, _, err := e.MorphRuleChainStorage().AddMorphRuleChain(chain.Ingress, engine.ContainerTarget(contID.EncodeToString()), &chain.Chain{ + Rules: []chain.Rule{ + { + Status: chain.Allow, + Actions: chain.Actions{ + Names: []string{ + nativeschema.MethodGetContainer, + }, + }, + Resources: chain.Resources{ + Names: []string{ + nativeschema.ResourceFormatAllContainers, + }, + }, + }, + }, + }) + require.NoError(t, err) +} -- 2.45.2 From 511a8527d9e1a49a71a154aef7643a856df558a5 Mon Sep 17 00:00:00 2001 From: Airat Arifullin Date: Thu, 15 Feb 2024 13:59:52 +0300 Subject: [PATCH 3/3] [#986] tree: Skip ACL checks if basicACL mask is unset Signed-off-by: Airat Arifullin --- pkg/services/tree/signature.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/pkg/services/tree/signature.go b/pkg/services/tree/signature.go index 15067d3cd..985e1ad94 100644 --- a/pkg/services/tree/signature.go +++ b/pkg/services/tree/signature.go @@ -77,6 +77,11 @@ func (s *Service) verifyClient(req message, cid cidSDK.ID, rawBearer []byte, op } basicACL := cnr.Value.BasicACL() + // Basic ACL mask can be unset, if a container operations are performed + // with strict APE checks only. + if basicACL == 0x0 { + return nil + } if !basicACL.IsOpAllowed(op, role) { return basicACLErr(op) -- 2.45.2