From 88cbe2db554701b76de98046fd631ae91619aeeb Mon Sep 17 00:00:00 2001 From: Airat Arifullin Date: Wed, 14 Feb 2024 18:14:06 +0300 Subject: [PATCH] [#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 d1b490bd..c01ad8a1 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 daaacb03..1af59d1c 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) +}