From 0c2b6f3dac70c1f0e72918d3acfb9f64ec7c6f34 Mon Sep 17 00:00:00 2001 From: Airat Arifullin Date: Mon, 1 Jul 2024 18:42:15 +0300 Subject: [PATCH] [#1216] ape: Make services use bearer chains fed router * Refactor object and tree service - they should instantiate chain router cheking the bearer token. If there are no bearer token rules, then defaul chain router is used. * Fix unit-tests. Signed-off-by: Airat Arifullin --- cmd/frostfs-node/container.go | 7 +++- cmd/frostfs-node/object.go | 3 +- cmd/frostfs-node/policy_engine.go | 30 ++++----------- cmd/frostfs-node/tree.go | 3 +- pkg/services/object/ape/checker.go | 51 +++++++++++-------------- pkg/services/object/ape/checker_test.go | 13 ++----- pkg/services/tree/ape.go | 20 +++------- pkg/services/tree/options.go | 13 +++++-- 8 files changed, 60 insertions(+), 80 deletions(-) diff --git a/cmd/frostfs-node/container.go b/cmd/frostfs-node/container.go index 99831fe98..5a29aac76 100644 --- a/cmd/frostfs-node/container.go +++ b/cmd/frostfs-node/container.go @@ -19,6 +19,7 @@ import ( containerMorph "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/services/container/morph" cid "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/container/id" "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/user" + "git.frostfs.info/TrueCloudLab/policy-engine/pkg/engine" "go.uber.org/zap" "google.golang.org/grpc" ) @@ -46,9 +47,13 @@ func initContainerService(_ context.Context, c *cfg) { c.shared.frostfsidClient = frostfsIDSubjectProvider + defaultChainRouter := engine.NewDefaultChainRouterWithLocalOverrides( + c.cfgObject.cfgAccessPolicyEngine.accessPolicyEngine.MorphRuleChainStorage(), + c.cfgObject.cfgAccessPolicyEngine.accessPolicyEngine.LocalStorage(), + ) service := containerService.NewSignService( &c.key.PrivateKey, - containerService.NewAPEServer(c.cfgObject.cfgAccessPolicyEngine.accessPolicyEngine, cnrRdr, + containerService.NewAPEServer(defaultChainRouter, cnrRdr, newCachedIRFetcher(createInnerRingFetcher(c)), c.netMapSource, c.shared.frostfsidClient, containerService.NewExecutionService(containerMorph.NewExecutor(cnrRdr, cnrWrt), c.respSvc), ), diff --git a/cmd/frostfs-node/object.go b/cmd/frostfs-node/object.go index 62183d314..0124bf772 100644 --- a/cmd/frostfs-node/object.go +++ b/cmd/frostfs-node/object.go @@ -460,7 +460,8 @@ func createAPEService(c *cfg, splitSvc *objectService.TransportSplitter) *object return objectAPE.NewService( c.log, objectAPE.NewChecker( - c.cfgObject.cfgAccessPolicyEngine.accessPolicyEngine.chainRouter, + c.cfgObject.cfgAccessPolicyEngine.accessPolicyEngine.LocalStorage(), + c.cfgObject.cfgAccessPolicyEngine.accessPolicyEngine.MorphRuleChainStorage(), objectAPE.NewStorageEngineHeaderProvider(c.cfgObject.cfgLocalStorage.localStorage, c.cfgObject.getSvc), c.shared.frostfsidClient, c.netMapSource, diff --git a/cmd/frostfs-node/policy_engine.go b/cmd/frostfs-node/policy_engine.go index 13e30a17e..22fda2b4c 100644 --- a/cmd/frostfs-node/policy_engine.go +++ b/cmd/frostfs-node/policy_engine.go @@ -1,13 +1,11 @@ package main import ( - "sync" "time" "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/ape/chainbase" "git.frostfs.info/TrueCloudLab/policy-engine/pkg/chain" "git.frostfs.info/TrueCloudLab/policy-engine/pkg/engine" - "git.frostfs.info/TrueCloudLab/policy-engine/pkg/resource" "github.com/google/uuid" "github.com/hashicorp/golang-lru/v2/expirable" "github.com/nspcc-dev/neo-go/pkg/neorpc/result" @@ -15,11 +13,9 @@ import ( ) type accessPolicyEngine struct { - mtx sync.RWMutex - - chainRouter engine.ChainRouter - localOverrideDatabase chainbase.LocalOverrideDatabase + + morphChainStorage engine.MorphRuleChainStorageReader } var _ engine.MorphRuleChainStorageReader = (*morphAPEChainCache)(nil) @@ -70,32 +66,20 @@ func newAccessPolicyEngine( localOverrideDatabase chainbase.LocalOverrideDatabase, ) *accessPolicyEngine { return &accessPolicyEngine{ - chainRouter: engine.NewDefaultChainRouterWithLocalOverrides( - morphChainStorage, - localOverrideDatabase, - ), + morphChainStorage: morphChainStorage, localOverrideDatabase: localOverrideDatabase, } } -func (a *accessPolicyEngine) IsAllowed(name chain.Name, target engine.RequestTarget, r resource.Request) (status chain.Status, found bool, err error) { - a.mtx.RLock() - defer a.mtx.RUnlock() - - return a.chainRouter.IsAllowed(name, target, r) +func (a *accessPolicyEngine) LocalStorage() engine.LocalOverrideStorage { + return a.localOverrideDatabase } -func (a *accessPolicyEngine) LocalStorage() engine.LocalOverrideStorage { - a.mtx.Lock() - defer a.mtx.Unlock() - - return a.localOverrideDatabase +func (a *accessPolicyEngine) MorphRuleChainStorage() engine.MorphRuleChainStorageReader { + return a.morphChainStorage } func (a *accessPolicyEngine) LocalOverrideDatabaseCore() chainbase.DatabaseCore { - a.mtx.Lock() - defer a.mtx.Unlock() - return a.localOverrideDatabase } diff --git a/cmd/frostfs-node/tree.go b/cmd/frostfs-node/tree.go index daaaa64a2..d22e510de 100644 --- a/cmd/frostfs-node/tree.go +++ b/cmd/frostfs-node/tree.go @@ -65,7 +65,8 @@ func initTreeService(c *cfg) { tree.WithReplicationWorkerCount(treeConfig.ReplicationWorkerCount()), tree.WithAuthorizedKeys(treeConfig.AuthorizedKeys()), tree.WithMetrics(c.metricsCollector.TreeService()), - tree.WithAPERouter(c.cfgObject.cfgAccessPolicyEngine.accessPolicyEngine), + tree.WithAPELocalOverrideStorage(c.cfgObject.cfgAccessPolicyEngine.accessPolicyEngine.LocalStorage()), + tree.WithAPEMorphRuleStorage(c.cfgObject.cfgAccessPolicyEngine.accessPolicyEngine.MorphRuleChainStorage()), tree.WithNetmapState(c.cfgNetmap.state), ) diff --git a/pkg/services/object/ape/checker.go b/pkg/services/object/ape/checker.go index 7541ad116..c6f9b711d 100644 --- a/pkg/services/object/ape/checker.go +++ b/pkg/services/object/ape/checker.go @@ -24,24 +24,26 @@ import ( ) type checkerImpl struct { - chainRouter policyengine.ChainRouter - headerProvider HeaderProvider - frostFSIDClient frostfsidcore.SubjectProvider - nm netmap.Source - st netmap.State - cnrSource container.Source - nodePK []byte + localOverrideStorage policyengine.LocalOverrideStorage + morphChainStorage policyengine.MorphRuleChainStorageReader + headerProvider HeaderProvider + frostFSIDClient frostfsidcore.SubjectProvider + nm netmap.Source + st netmap.State + cnrSource container.Source + nodePK []byte } -func NewChecker(chainRouter policyengine.ChainRouter, headerProvider HeaderProvider, frostFSIDClient frostfsidcore.SubjectProvider, nm netmap.Source, st netmap.State, cnrSource container.Source, nodePK []byte) Checker { +func NewChecker(localOverrideStorage policyengine.LocalOverrideStorage, morphChainStorage policyengine.MorphRuleChainStorageReader, headerProvider HeaderProvider, frostFSIDClient frostfsidcore.SubjectProvider, nm netmap.Source, st netmap.State, cnrSource container.Source, nodePK []byte) Checker { return &checkerImpl{ - chainRouter: chainRouter, - headerProvider: headerProvider, - frostFSIDClient: frostFSIDClient, - nm: nm, - st: st, - cnrSource: cnrSource, - nodePK: nodePK, + localOverrideStorage: localOverrideStorage, + morphChainStorage: morphChainStorage, + headerProvider: headerProvider, + frostFSIDClient: frostFSIDClient, + nm: nm, + st: st, + cnrSource: cnrSource, + nodePK: nodePK, } } @@ -175,28 +177,21 @@ func (c *checkerImpl) CheckAPE(ctx context.Context, prm Prm) error { groups[i] = fmt.Sprintf("%s:%s", prm.Namespace, groups[i]) } + var cr policyengine.ChainRouter if prm.BearerToken != nil && !prm.BearerToken.Impersonate() { if err := isValidBearer(prm.BearerToken, prm.ContainerOwner, prm.Container, pub, c.st); err != nil { return fmt.Errorf("bearer token validation error: %w", err) } - btRouter, err := router.SingleUseRouterWithBearerTokenChains([]bearer.APEOverride{prm.BearerToken.APEOverride()}) + cr, err = router.BearerChainFeedRouter(c.localOverrideStorage, c.morphChainStorage, prm.BearerToken.APEOverride()) if err != nil { - return err - } - status, found, err := btRouter.IsAllowed(apechain.Ingress, policyengine.NewRequestTargetWithContainer(prm.Container.EncodeToString()), r) - if err != nil { - return err - } - if found && status == apechain.Allow { - return nil - } - if status != apechain.NoRuleFound { - return fmt.Errorf("bearer token: method %s: %s", prm.Method, status) + return fmt.Errorf("create chain router error: %w", err) } + } else { + cr = policyengine.NewDefaultChainRouterWithLocalOverrides(c.morphChainStorage, c.localOverrideStorage) } 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) + status, ruleFound, err := cr.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 5efd3669b..27c314dfb 100644 --- a/pkg/services/object/ape/checker_test.go +++ b/pkg/services/object/ape/checker_test.go @@ -469,9 +469,8 @@ func TestAPECheck_BearerTokenOverrides(t *testing.T) { ls := inmemory.NewInmemoryLocalStorage() ms := inmemory.NewInmemoryMorphRuleChainStorage() - r := policyengine.NewDefaultChainRouterWithLocalOverrides(ms, ls) - checker := NewChecker(r, headerProvider, frostfsidProvider, nil, &stMock{}, nil, nil) + checker := NewChecker(ls, ms, headerProvider, frostfsidProvider, nil, &stMock{}, nil, nil) prm := Prm{ Method: method, @@ -534,9 +533,7 @@ func TestAPECheck(t *testing.T) { }) } - router := policyengine.NewDefaultChainRouterWithLocalOverrides(ms, ls) - - checker := NewChecker(router, headerProvider, frostfsidProvider, nil, &stMock{}, nil, nil) + checker := NewChecker(ls, ms, headerProvider, frostfsidProvider, nil, &stMock{}, nil, nil) prm := Prm{ Method: method, @@ -639,8 +636,6 @@ func TestPutECChunk(t *testing.T) { MatchType: chain.MatchTypeFirstMatch, }) - router := policyengine.NewDefaultChainRouterWithLocalOverrides(ms, ls) - node1Key, err := keys.NewPrivateKey() require.NoError(t, err) node1 := netmapSDK.NodeInfo{} @@ -669,7 +664,7 @@ func TestPutECChunk(t *testing.T) { }, } - checker := NewChecker(router, headerProvider, frostfsidProvider, nm, &stMock{}, cs, node1Key.PublicKey().Bytes()) + checker := NewChecker(ls, ms, headerProvider, frostfsidProvider, nm, &stMock{}, cs, node1Key.PublicKey().Bytes()) ecParentID := oidtest.ID() chunkHeader := newHeaderObjectSDK(cnr, obj, nil).ToV2().GetHeader() @@ -711,7 +706,7 @@ func TestPutECChunk(t *testing.T) { t.Run("access allowed for non container node", func(t *testing.T) { otherKey, err := keys.NewPrivateKey() require.NoError(t, err) - checker = NewChecker(router, headerProvider, frostfsidProvider, nm, &stMock{}, cs, otherKey.PublicKey().Bytes()) + checker = NewChecker(ls, ms, headerProvider, frostfsidProvider, nm, &stMock{}, cs, otherKey.PublicKey().Bytes()) prm := Prm{ Method: nativeschema.MethodPutObject, Container: cnr, diff --git a/pkg/services/tree/ape.go b/pkg/services/tree/ape.go index a6202d1a3..6e78bf4ec 100644 --- a/pkg/services/tree/ape.go +++ b/pkg/services/tree/ape.go @@ -141,25 +141,17 @@ func (s *Service) checkAPE(ctx context.Context, bt *bearer.Token, return apeErr(err) } + var cr engine.ChainRouter if bt != nil && !bt.Impersonate() { if err := isValidBearer(bt, container.Value.Owner(), cid, publicKey, s.state); err != nil { return fmt.Errorf("bearer validation error: %w", err) } - btRouter, err := router.SingleUseRouterWithBearerTokenChains([]bearer.APEOverride{bt.APEOverride()}) + cr, err = router.BearerChainFeedRouter(s.localOverrideStorage, s.morphChainStorage, bt.APEOverride()) if err != nil { - return apeErr(err) - } - status, found, err := btRouter.IsAllowed(apechain.Ingress, engine.NewRequestTargetWithContainer(cid.EncodeToString()), request) - if err != nil { - return apeErr(err) - } - if found && status == apechain.Allow { - return nil - } - if status != apechain.NoRuleFound { - err = fmt.Errorf("access to operation %s is denied by access policy engine (bearer token): %s", request.Operation(), status.String()) - return apeErr(err) + return fmt.Errorf("create chain router error: %w", err) } + } else { + cr = engine.NewDefaultChainRouterWithLocalOverrides(s.morphChainStorage, s.localOverrideStorage) } groups, err := aperequest.Groups(s.frostfsidSubjectProvider, publicKey) @@ -173,7 +165,7 @@ func (s *Service) checkAPE(ctx context.Context, bt *bearer.Token, } rt := engine.NewRequestTargetExtended(namespace, cid.EncodeToString(), fmt.Sprintf("%s:%s", namespace, publicKey.Address()), groups) - status, found, err := s.router.IsAllowed(apechain.Ingress, rt, request) + status, found, err := cr.IsAllowed(apechain.Ingress, rt, request) if err != nil { return apeErr(err) } diff --git a/pkg/services/tree/options.go b/pkg/services/tree/options.go index ea5539938..6a20fe5cc 100644 --- a/pkg/services/tree/options.go +++ b/pkg/services/tree/options.go @@ -42,7 +42,8 @@ type cfg struct { containerCacheSize int authorizedKeys [][]byte - router policyengine.ChainRouter + localOverrideStorage policyengine.LocalOverrideStorage + morphChainStorage policyengine.MorphRuleChainStorageReader metrics MetricsRegister } @@ -152,9 +153,15 @@ func WithAuthorizedKeys(keys keys.PublicKeys) Option { } } -func WithAPERouter(router policyengine.ChainRouter) Option { +func WithAPELocalOverrideStorage(localOverrideStorage policyengine.LocalOverrideStorage) Option { return func(c *cfg) { - c.router = router + c.localOverrideStorage = localOverrideStorage + } +} + +func WithAPEMorphRuleStorage(morphRuleStorage policyengine.MorphRuleChainStorageReader) Option { + return func(c *cfg) { + c.morphChainStorage = morphRuleStorage } }