From 6da1acc5545bac4252c4cc9f69ab114fb8a04540 Mon Sep 17 00:00:00 2001 From: Alex Vanin Date: Wed, 10 Apr 2024 15:53:36 +0300 Subject: [PATCH] [#360] Use 'c' prefix for bucket policies instead of 'n' With 'c' prefix, acl chains become shorter, thus gateway receives shorter results and avoids sessions to neo-go. There is still issue with many IAM rules. Signed-off-by: Alex Vanin --- api/handler/acl.go | 2 +- api/handler/acl_test.go | 8 +-- api/handler/api.go | 2 +- api/handler/handlers_test.go | 8 +-- api/handler/put.go | 2 +- api/middleware/policy.go | 53 ++++++++++--------- api/router_test.go | 1 + .../policy/morph_rule_chain_storage.go | 16 +++--- 8 files changed, 49 insertions(+), 43 deletions(-) diff --git a/api/handler/acl.go b/api/handler/acl.go index 67133a1..6574cb4 100644 --- a/api/handler/acl.go +++ b/api/handler/acl.go @@ -450,7 +450,7 @@ func (h *handler) putBucketACLAPEHandler(w http.ResponseWriter, r *http.Request, } chainRules := bucketCannedACLToAPERules(cannedACL, reqInfo, key, bktInfo.CID) - if err = h.ape.SaveACLChains(reqInfo.Namespace, chainRules); err != nil { + if err = h.ape.SaveACLChains(bktInfo.CID.EncodeToString(), chainRules); err != nil { h.logAndSendError(w, "failed to add morph rule chains", reqInfo, err) return } diff --git a/api/handler/acl_test.go b/api/handler/acl_test.go index 59f7fd5..f0c3b3f 100644 --- a/api/handler/acl_test.go +++ b/api/handler/acl_test.go @@ -1325,7 +1325,7 @@ func TestPutBucketAPE(t *testing.T) { _, err := hc.tp.ContainerEACL(hc.Context(), layer.PrmContainerEACL{ContainerID: info.BktInfo.CID}) require.ErrorContains(t, err, "not found") - chains, err := hc.h.ape.(*apeMock).ListChains(engine.NamespaceTarget("")) + chains, err := hc.h.ape.(*apeMock).ListChains(engine.ContainerTarget(info.BktInfo.CID.EncodeToString())) require.NoError(t, err) require.Len(t, chains, 2) } @@ -1509,7 +1509,7 @@ func TestDeleteBucketWithPolicy(t *testing.T) { hc := prepareHandlerContext(t) bktName := "bucket-for-policy" - createTestBucket(hc, bktName) + bi := createTestBucket(hc, bktName) newPolicy := engineiam.Policy{ Version: "2012-10-17", @@ -1524,12 +1524,12 @@ func TestDeleteBucketWithPolicy(t *testing.T) { putBucketPolicy(hc, bktName, newPolicy) require.Len(t, hc.h.ape.(*apeMock).policyMap, 1) - require.Len(t, hc.h.ape.(*apeMock).chainMap[engine.NamespaceTarget("")], 4) + require.Len(t, hc.h.ape.(*apeMock).chainMap[engine.ContainerTarget(bi.CID.EncodeToString())], 4) deleteBucket(t, hc, bktName, http.StatusNoContent) require.Empty(t, hc.h.ape.(*apeMock).policyMap) - chains, err := hc.h.ape.(*apeMock).ListChains(engine.NamespaceTarget("")) + chains, err := hc.h.ape.(*apeMock).ListChains(engine.ContainerTarget(bi.CID.EncodeToString())) require.NoError(t, err) require.Empty(t, chains) } diff --git a/api/handler/api.go b/api/handler/api.go index 1ee1738..e779b18 100644 --- a/api/handler/api.go +++ b/api/handler/api.go @@ -59,7 +59,7 @@ type ( PutBucketPolicy(ns string, cnrID cid.ID, policy []byte, chains []*chain.Chain) error DeleteBucketPolicy(ns string, cnrID cid.ID, chainIDs []chain.ID) error GetBucketPolicy(ns string, cnrID cid.ID) ([]byte, error) - SaveACLChains(ns string, chains []*chain.Chain) error + SaveACLChains(cid string, chains []*chain.Chain) error } ) diff --git a/api/handler/handlers_test.go b/api/handler/handlers_test.go index e8200ac..9b8f3f4 100644 --- a/api/handler/handlers_test.go +++ b/api/handler/handlers_test.go @@ -270,7 +270,7 @@ func (a *apeMock) PutBucketPolicy(ns string, cnrID cid.ID, policy []byte, chain } for i := range chain { - if err := a.AddChain(engine.NamespaceTarget(ns), chain[i]); err != nil { + if err := a.AddChain(engine.ContainerTarget(cnrID.EncodeToString()), chain[i]); err != nil { return err } } @@ -283,7 +283,7 @@ func (a *apeMock) DeleteBucketPolicy(ns string, cnrID cid.ID, chainIDs []chain.I return err } for i := range chainIDs { - if err := a.RemoveChain(engine.NamespaceTarget(ns), chainIDs[i]); err != nil { + if err := a.RemoveChain(engine.ContainerTarget(cnrID.EncodeToString()), chainIDs[i]); err != nil { return err } } @@ -300,9 +300,9 @@ func (a *apeMock) GetBucketPolicy(ns string, cnrID cid.ID) ([]byte, error) { return policy, nil } -func (a *apeMock) SaveACLChains(ns string, chains []*chain.Chain) error { +func (a *apeMock) SaveACLChains(cid string, chains []*chain.Chain) error { for i := range chains { - if err := a.AddChain(engine.NamespaceTarget(ns), chains[i]); err != nil { + if err := a.AddChain(engine.ContainerTarget(cid), chains[i]); err != nil { return err } } diff --git a/api/handler/put.go b/api/handler/put.go index 30197b7..198f6b9 100644 --- a/api/handler/put.go +++ b/api/handler/put.go @@ -898,7 +898,7 @@ func (h *handler) createBucketHandlerPolicy(w http.ResponseWriter, r *http.Reque h.reqLogger(ctx).Info(logs.BucketIsCreated, zap.Stringer("container_id", bktInfo.CID)) chains := bucketCannedACLToAPERules(cannedACL, reqInfo, key, bktInfo.CID) - if err = h.ape.SaveACLChains(reqInfo.Namespace, chains); err != nil { + if err = h.ape.SaveACLChains(bktInfo.CID.EncodeToString(), chains); err != nil { h.logAndSendError(w, "failed to add morph rule chain", reqInfo, err) return } diff --git a/api/middleware/policy.go b/api/middleware/policy.go index 35d1e7a..fc58fdc 100644 --- a/api/middleware/policy.go +++ b/api/middleware/policy.go @@ -72,15 +72,34 @@ func policyCheck(r *http.Request, cfg PolicyConfig) error { return err } - reqInfo := GetReqInfo(r.Context()) - target := engine.NewRequestTargetWithNamespace(reqInfo.Namespace) - st, found, err := cfg.Storage.IsAllowed(chain.S3, target, req) - if err != nil { - return err + var bktInfo *data.BucketInfo + if reqType != noneType && !strings.HasSuffix(req.Operation(), CreateBucketOperation) { + bktInfo, err = cfg.BucketResolver(r.Context(), bktName) + if err != nil { + return err + } } - if !found { - st = chain.NoRuleFound + reqInfo := GetReqInfo(r.Context()) + targets := []engine.RequestTarget{ + engine.NewRequestTargetWithNamespace(reqInfo.Namespace), + } + if bktInfo != nil { + targets = append(targets, engine.NewRequestTargetWithContainer(bktInfo.CID.EncodeToString())) + } + + st := chain.NoRuleFound + for _, target := range targets { + status, found, err := cfg.Storage.IsAllowed(chain.S3, target, req) + if err != nil { + return err + } + if found { + st = status + if status != chain.Allow { + break + } + } } switch { @@ -90,9 +109,9 @@ func policyCheck(r *http.Request, cfg PolicyConfig) error { return apiErr.GetAPIErrorWithError(apiErr.ErrAccessDenied, fmt.Errorf("policy check: %s", st.String())) } - isAPE, err := isAPEBehavior(r.Context(), req, cfg, reqType, bktName) - if err != nil { - return err + isAPE := !cfg.Settings.ACLEnabled() + if bktInfo != nil { + isAPE = bktInfo.APEEnabled } if isAPE && cfg.Settings.PolicyDenyByDefault() { @@ -102,20 +121,6 @@ func policyCheck(r *http.Request, cfg PolicyConfig) error { return nil } -func isAPEBehavior(ctx context.Context, req *testutil.Request, cfg PolicyConfig, reqType ReqType, bktName string) (bool, error) { - if reqType == noneType || - strings.HasSuffix(req.Operation(), CreateBucketOperation) { - return !cfg.Settings.ACLEnabled(), nil - } - - bktInfo, err := cfg.BucketResolver(ctx, bktName) // we cannot use reqInfo.BucketName because it hasn't been set yet - if err != nil { - return false, err - } - - return bktInfo.APEEnabled, nil -} - func getPolicyRequest(r *http.Request, frostfsid FrostFSIDInformer, reqType ReqType, bktName string, objName string, log *zap.Logger) (*testutil.Request, error) { var ( owner string diff --git a/api/router_test.go b/api/router_test.go index bca051c..c272208 100644 --- a/api/router_test.go +++ b/api/router_test.go @@ -197,6 +197,7 @@ func TestPolicyChecker(t *testing.T) { func TestPolicyCheckerReqTypeDetermination(t *testing.T) { chiRouter := prepareRouter(t) bktName, objName := "bucket", "object" + createBucket(chiRouter, "", bktName) policy := engineiam.Policy{ Version: "2012-10-17", diff --git a/internal/frostfs/policy/morph_rule_chain_storage.go b/internal/frostfs/policy/morph_rule_chain_storage.go index 20ce82b..5684729 100644 --- a/internal/frostfs/policy/morph_rule_chain_storage.go +++ b/internal/frostfs/policy/morph_rule_chain_storage.go @@ -76,24 +76,24 @@ func (c *MorphRuleChainStorage) ListMorphRuleChains(name chain.Name, target engi } func (c *MorphRuleChainStorage) PutBucketPolicy(ns string, cnrID cid.ID, policy []byte, chains []*chain.Chain) error { - c.cache.Delete(cache.MorphPolicyCacheKey{Target: engine.NamespaceTarget(ns), Name: chain.S3}) + c.cache.Delete(cache.MorphPolicyCacheKey{Target: engine.ContainerTarget(cnrID.EncodeToString()), Name: chain.S3}) tx := c.contract.StartTx() tx.AddChain(policycontract.IAM, ns, getBucketPolicyName(cnrID), policy) for i := range chains { - tx.AddChain(policycontract.Namespace, ns, chains[i].ID, chains[i].Bytes()) + tx.AddChain(policycontract.Container, cnrID.EncodeToString(), chains[i].ID, chains[i].Bytes()) } return c.contract.SendTx(tx) } func (c *MorphRuleChainStorage) DeleteBucketPolicy(ns string, cnrID cid.ID, chainIDs []chain.ID) error { - c.cache.Delete(cache.MorphPolicyCacheKey{Target: engine.NamespaceTarget(ns), Name: chain.S3}) + c.cache.Delete(cache.MorphPolicyCacheKey{Target: engine.ContainerTarget(cnrID.EncodeToString()), Name: chain.S3}) tx := c.contract.StartTx() for _, chainID := range chainIDs { - tx.RemoveChain(policycontract.Namespace, ns, chainID) + tx.RemoveChain(policycontract.Container, cnrID.EncodeToString(), chainID) } tx.RemoveChain(policycontract.IAM, ns, getBucketPolicyName(cnrID)) @@ -104,13 +104,13 @@ func (c *MorphRuleChainStorage) GetBucketPolicy(ns string, cnrID cid.ID) ([]byte return c.contract.GetChain(policycontract.IAM, ns, getBucketPolicyName(cnrID)) } -func (c *MorphRuleChainStorage) SaveACLChains(ns string, chains []*chain.Chain) error { - c.cache.Delete(cache.MorphPolicyCacheKey{Target: engine.NamespaceTarget(ns), Name: chain.S3}) - c.cache.Delete(cache.MorphPolicyCacheKey{Target: engine.NamespaceTarget(ns), Name: chain.Ingress}) +func (c *MorphRuleChainStorage) SaveACLChains(cid string, chains []*chain.Chain) error { + c.cache.Delete(cache.MorphPolicyCacheKey{Target: engine.ContainerTarget(cid), Name: chain.S3}) + c.cache.Delete(cache.MorphPolicyCacheKey{Target: engine.ContainerTarget(cid), Name: chain.Ingress}) tx := c.contract.StartTx() for i := range chains { - tx.AddChain(policycontract.Namespace, ns, chains[i].ID, chains[i].Bytes()) + tx.AddChain(policycontract.Container, cid, chains[i].ID, chains[i].Bytes()) } return c.contract.SendTx(tx)