From a812932984531162648fdbfa985a6f496fdbd80e Mon Sep 17 00:00:00 2001 From: Airat Arifullin Date: Tue, 10 Sep 2024 11:15:30 +0300 Subject: [PATCH] [#1362] ape: Move common APE check logic to separate package * Tree and object service have the same log for checking APE. So, this check should be moved to common package. Signed-off-by: Airat Arifullin --- pkg/services/common/ape/checker.go | 167 +++++++++++++++++++++++++++++ pkg/services/object/ape/checker.go | 139 +++++------------------- pkg/services/tree/ape.go | 116 ++------------------ pkg/services/tree/service.go | 5 + 4 files changed, 205 insertions(+), 222 deletions(-) create mode 100644 pkg/services/common/ape/checker.go diff --git a/pkg/services/common/ape/checker.go b/pkg/services/common/ape/checker.go new file mode 100644 index 00000000..f24d2212 --- /dev/null +++ b/pkg/services/common/ape/checker.go @@ -0,0 +1,167 @@ +package ape + +import ( + "crypto/ecdsa" + "errors" + "fmt" + + aperequest "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/ape/request" + "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/ape/router" + frostfsidcore "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/core/frostfsid" + "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/core/netmap" + "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/ape" + "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/bearer" + apistatus "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/client/status" + cid "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/container/id" + "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/user" + apechain "git.frostfs.info/TrueCloudLab/policy-engine/pkg/chain" + policyengine "git.frostfs.info/TrueCloudLab/policy-engine/pkg/engine" + "github.com/nspcc-dev/neo-go/pkg/crypto/keys" +) + +var ( + errInvalidTargetType = errors.New("bearer token defines non-container target override") + errBearerExpired = errors.New("bearer token has expired") + errBearerInvalidSignature = errors.New("bearer token has invalid signature") + errBearerInvalidContainerID = errors.New("bearer token was created for another container") + errBearerNotSignedByOwner = errors.New("bearer token is not signed by the container owner") + errBearerInvalidOwner = errors.New("bearer token owner differs from the request sender") +) + +type CheckPrm struct { + // Request is an APE-request that is checked by policy engine. + Request aperequest.Request + + Namespace string + + Container cid.ID + + // An encoded container's owner user ID. + ContainerOwner user.ID + + // PublicKey is public key of the request sender. + PublicKey *keys.PublicKey + + // The request's bearer token. It is used in order to check APE overrides with the token. + BearerToken *bearer.Token + + // If SoftAPECheck is set to true, then NoRuleFound is interpreted as allow. + SoftAPECheck bool +} + +// CheckCore provides methods to perform the common logic of APE check. +type CheckCore interface { + // CheckAPE performs the common policy-engine check logic on a prepared request. + CheckAPE(prm CheckPrm) error +} + +type checkerCoreImpl struct { + LocalOverrideStorage policyengine.LocalOverrideStorage + MorphChainStorage policyengine.MorphRuleChainStorageReader + FrostFSSubjectProvider frostfsidcore.SubjectProvider + State netmap.State +} + +func New(localOverrideStorage policyengine.LocalOverrideStorage, morphChainStorage policyengine.MorphRuleChainStorageReader, + frostFSSubjectProvider frostfsidcore.SubjectProvider, state netmap.State) CheckCore { + return &checkerCoreImpl{ + LocalOverrideStorage: localOverrideStorage, + MorphChainStorage: morphChainStorage, + FrostFSSubjectProvider: frostFSSubjectProvider, + State: state, + } +} + +// CheckAPE performs the common policy-engine check logic on a prepared request. +func (c *checkerCoreImpl) CheckAPE(prm CheckPrm) error { + var cr policyengine.ChainRouter + if prm.BearerToken != nil && !prm.BearerToken.Impersonate() { + var err error + if err = isValidBearer(prm.BearerToken, prm.ContainerOwner, prm.Container, prm.PublicKey, c.State); err != nil { + return fmt.Errorf("bearer validation error: %w", err) + } + cr, err = router.BearerChainFeedRouter(c.LocalOverrideStorage, c.MorphChainStorage, prm.BearerToken.APEOverride()) + if err != nil { + return fmt.Errorf("create chain router error: %w", err) + } + } else { + cr = policyengine.NewDefaultChainRouterWithLocalOverrides(c.MorphChainStorage, c.LocalOverrideStorage) + } + + groups, err := aperequest.Groups(c.FrostFSSubjectProvider, prm.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", prm.Namespace, groups[i]) + } + + rt := policyengine.NewRequestTargetExtended(prm.Namespace, prm.Container.EncodeToString(), fmt.Sprintf("%s:%s", prm.Namespace, prm.PublicKey.Address()), groups) + status, found, err := cr.IsAllowed(apechain.Ingress, rt, prm.Request) + if err != nil { + return err + } + if !found && prm.SoftAPECheck || status == apechain.Allow { + return nil + } + err = fmt.Errorf("access to operation %s is denied by access policy engine: %s", prm.Request.Operation(), status.String()) + return apeErr(err) +} + +func apeErr(err error) error { + errAccessDenied := &apistatus.ObjectAccessDenied{} + errAccessDenied.WriteReason(err.Error()) + return errAccessDenied +} + +// isValidBearer checks whether bearer token was correctly signed by authorized +// entity. This method might be defined on whole ACL service because it will +// require fetching current epoch to check lifetime. +func isValidBearer(token *bearer.Token, ownerCnr user.ID, cntID cid.ID, publicKey *keys.PublicKey, st netmap.State) error { + if token == nil { + return nil + } + + // First check token lifetime. Simplest verification. + if token.InvalidAt(st.CurrentEpoch()) { + return errBearerExpired + } + + // Then check if bearer token is signed correctly. + if !token.VerifySignature() { + return errBearerInvalidSignature + } + + // Check for ape overrides defined in the bearer token. + apeOverride := token.APEOverride() + if len(apeOverride.Chains) > 0 && apeOverride.Target.TargetType != ape.TargetTypeContainer { + return fmt.Errorf("%w: %s", errInvalidTargetType, apeOverride.Target.TargetType.ToV2().String()) + } + + // Then check if container is either empty or equal to the container in the request. + var targetCnr cid.ID + err := targetCnr.DecodeString(apeOverride.Target.Name) + if err != nil { + return fmt.Errorf("invalid cid format: %s", apeOverride.Target.Name) + } + if !cntID.Equals(targetCnr) { + return errBearerInvalidContainerID + } + + // Then check if container owner signed this token. + if !bearer.ResolveIssuer(*token).Equals(ownerCnr) { + return errBearerNotSignedByOwner + } + + // Then check if request sender has rights to use this token. + var usrSender user.ID + user.IDFromKey(&usrSender, (ecdsa.PublicKey)(*publicKey)) + + if !token.AssertUser(usrSender) { + return errBearerInvalidOwner + } + + return nil +} diff --git a/pkg/services/object/ape/checker.go b/pkg/services/object/ape/checker.go index a1972292..3688638d 100644 --- a/pkg/services/object/ape/checker.go +++ b/pkg/services/object/ape/checker.go @@ -2,49 +2,41 @@ package ape import ( "context" - "crypto/ecdsa" "errors" "fmt" objectV2 "git.frostfs.info/TrueCloudLab/frostfs-api-go/v2/object" "git.frostfs.info/TrueCloudLab/frostfs-api-go/v2/session" - 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" "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/core/netmap" - "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/ape" + checkercore "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/services/common/ape" "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/bearer" cid "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/container/id" oid "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/object/id" "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/user" - apechain "git.frostfs.info/TrueCloudLab/policy-engine/pkg/chain" policyengine "git.frostfs.info/TrueCloudLab/policy-engine/pkg/engine" nativeschema "git.frostfs.info/TrueCloudLab/policy-engine/schema/native" "github.com/nspcc-dev/neo-go/pkg/crypto/keys" ) type checkerImpl struct { - localOverrideStorage policyengine.LocalOverrideStorage - morphChainStorage policyengine.MorphRuleChainStorageReader - headerProvider HeaderProvider - frostFSIDClient frostfsidcore.SubjectProvider - nm netmap.Source - st netmap.State - cnrSource container.Source - nodePK []byte + checkerCore checkercore.CheckCore + frostFSIDClient frostfsidcore.SubjectProvider + headerProvider HeaderProvider + nm netmap.Source + cnrSource container.Source + nodePK []byte } 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{ - localOverrideStorage: localOverrideStorage, - morphChainStorage: morphChainStorage, - headerProvider: headerProvider, - frostFSIDClient: frostFSIDClient, - nm: nm, - st: st, - cnrSource: cnrSource, - nodePK: nodePK, + checkerCore: checkercore.New(localOverrideStorage, morphChainStorage, frostFSIDClient, st), + frostFSIDClient: frostFSIDClient, + headerProvider: headerProvider, + nm: nm, + cnrSource: cnrSource, + nodePK: nodePK, } } @@ -85,68 +77,9 @@ type Prm struct { XHeaders []session.XHeader } -var ( - errMissingOID = errors.New("object ID is not set") - errInvalidTargetType = errors.New("bearer token defines non-container target override") - errBearerExpired = errors.New("bearer token has expired") - errBearerInvalidSignature = errors.New("bearer token has invalid signature") - errBearerInvalidContainerID = errors.New("bearer token was created for another container") - errBearerNotSignedByOwner = errors.New("bearer token is not signed by the container owner") - errBearerInvalidOwner = errors.New("bearer token owner differs from the request sender") -) +var errMissingOID = errors.New("object ID is not set") -// isValidBearer checks whether bearer token was correctly signed by authorized -// entity. This method might be defined on whole ACL service because it will -// require fetching current epoch to check lifetime. -func isValidBearer(token *bearer.Token, ownerCnr user.ID, containerID cid.ID, publicKey *keys.PublicKey, st netmap.State) error { - if token == nil { - return nil - } - - // First check token lifetime. Simplest verification. - if token.InvalidAt(st.CurrentEpoch()) { - return errBearerExpired - } - - // Then check if bearer token is signed correctly. - if !token.VerifySignature() { - return errBearerInvalidSignature - } - - // Check for ape overrides defined in the bearer token. - apeOverride := token.APEOverride() - if len(apeOverride.Chains) > 0 && apeOverride.Target.TargetType != ape.TargetTypeContainer { - return fmt.Errorf("%w: %s", errInvalidTargetType, apeOverride.Target.TargetType.ToV2().String()) - } - - // Then check if container is either empty or equal to the container in the request. - var targetCnr cid.ID - err := targetCnr.DecodeString(apeOverride.Target.Name) - if err != nil { - return fmt.Errorf("invalid cid format: %s", apeOverride.Target.Name) - } - if !containerID.Equals(targetCnr) { - return errBearerInvalidContainerID - } - - // Then check if container owner signed this token. - if !bearer.ResolveIssuer(*token).Equals(ownerCnr) { - return errBearerNotSignedByOwner - } - - // Then check if request sender has rights to use this token. - var usrSender user.ID - user.IDFromKey(&usrSender, (ecdsa.PublicKey)(*publicKey)) - - if !token.AssertUser(usrSender) { - return errBearerInvalidOwner - } - - return nil -} - -// CheckAPE checks if a request or a response is permitted creating an ape request and passing -// it to chain router. +// CheckAPE prepares an APE-request and checks if it is permitted by policies. func (c *checkerImpl) CheckAPE(ctx context.Context, prm Prm) error { // APE check is ignored for some inter-node requests. if prm.Role == nativeschema.PropertyValueContainerRoleContainer { @@ -171,38 +104,14 @@ func (c *checkerImpl) CheckAPE(ctx context.Context, prm Prm) error { 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]) - } - - 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) - } - cr, err = router.BearerChainFeedRouter(c.localOverrideStorage, c.morphChainStorage, prm.BearerToken.APEOverride()) - if err != nil { - 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 := cr.IsAllowed(apechain.Ingress, rt, r) - if err != nil { - return err - } - - if !ruleFound && prm.SoftAPECheck || status == apechain.Allow { - return nil - } - - return fmt.Errorf("method %s: %s", prm.Method, status) + return c.checkerCore.CheckAPE(checkercore.CheckPrm{ + Request: r, + PublicKey: pub, + Namespace: prm.Method, + Container: prm.Container, + ContainerOwner: prm.ContainerOwner, + BearerToken: prm.BearerToken, + SoftAPECheck: prm.SoftAPECheck, + }) } diff --git a/pkg/services/tree/ape.go b/pkg/services/tree/ape.go index ee468791..693b16e6 100644 --- a/pkg/services/tree/ape.go +++ b/pkg/services/tree/ape.go @@ -2,42 +2,25 @@ package tree import ( "context" - "crypto/ecdsa" "encoding/hex" - "errors" "fmt" "net" "strings" "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/ape/converter" aperequest "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/ape/request" - "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/ape/router" core "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/core/container" - "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/core/netmap" - "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/ape" + checkercore "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/services/common/ape" "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/bearer" - apistatus "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/client/status" cnrSDK "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/container" "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/container/acl" cid "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/container/id" - "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/user" - apechain "git.frostfs.info/TrueCloudLab/policy-engine/pkg/chain" - "git.frostfs.info/TrueCloudLab/policy-engine/pkg/engine" 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" "google.golang.org/grpc/peer" ) -var ( - errInvalidTargetType = errors.New("bearer token defines non-container target override") - errBearerExpired = errors.New("bearer token has expired") - errBearerInvalidSignature = errors.New("bearer token has invalid signature") - errBearerInvalidContainerID = errors.New("bearer token was created for another container") - errBearerNotSignedByOwner = errors.New("bearer token is not signed by the container owner") - errBearerInvalidOwner = errors.New("bearer token owner differs from the request sender") -) - func (s *Service) newAPERequest(ctx context.Context, namespace string, cid cid.ID, operation acl.Op, role acl.Role, publicKey *keys.PublicKey, ) (aperequest.Request, error) { @@ -77,56 +60,6 @@ func (s *Service) newAPERequest(ctx context.Context, namespace string, ), nil } -// isValidBearer checks whether bearer token was correctly signed by authorized -// entity. This method might be defined on whole ACL service because it will -// require fetching current epoch to check lifetime. -func isValidBearer(token *bearer.Token, ownerCnr user.ID, cntID cid.ID, publicKey *keys.PublicKey, st netmap.State) error { - if token == nil { - return nil - } - - // First check token lifetime. Simplest verification. - if token.InvalidAt(st.CurrentEpoch()) { - return errBearerExpired - } - - // Then check if bearer token is signed correctly. - if !token.VerifySignature() { - return errBearerInvalidSignature - } - - // Check for ape overrides defined in the bearer token. - apeOverride := token.APEOverride() - if len(apeOverride.Chains) > 0 && apeOverride.Target.TargetType != ape.TargetTypeContainer { - return fmt.Errorf("%w: %s", errInvalidTargetType, apeOverride.Target.TargetType.ToV2().String()) - } - - // Then check if container is either empty or equal to the container in the request. - var targetCnr cid.ID - err := targetCnr.DecodeString(apeOverride.Target.Name) - if err != nil { - return fmt.Errorf("invalid cid format: %s", apeOverride.Target.Name) - } - if !cntID.Equals(targetCnr) { - return errBearerInvalidContainerID - } - - // Then check if container owner signed this token. - if !bearer.ResolveIssuer(*token).Equals(ownerCnr) { - return errBearerNotSignedByOwner - } - - // Then check if request sender has rights to use this token. - var usrSender user.ID - user.IDFromKey(&usrSender, (ecdsa.PublicKey)(*publicKey)) - - if !token.AssertUser(usrSender) { - return errBearerInvalidOwner - } - - return nil -} - func (s *Service) checkAPE(ctx context.Context, bt *bearer.Token, container *core.Container, cid cid.ID, operation acl.Op, role acl.Role, publicKey *keys.PublicKey, ) error { @@ -141,45 +74,14 @@ func (s *Service) checkAPE(ctx context.Context, bt *bearer.Token, return fmt.Errorf("failed to create ape request: %w", 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) - } - cr, err = router.BearerChainFeedRouter(s.localOverrideStorage, s.morphChainStorage, bt.APEOverride()) - if err != nil { - return fmt.Errorf("create chain router error: %w", err) - } - } else { - cr = engine.NewDefaultChainRouterWithLocalOverrides(s.morphChainStorage, s.localOverrideStorage) - } - - 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 := cr.IsAllowed(apechain.Ingress, rt, request) - if err != nil { - return err - } - if found && status == apechain.Allow { - return nil - } - err = fmt.Errorf("access to operation %s is denied by access policy engine: %s", request.Operation(), status.String()) - return apeErr(err) -} - -func apeErr(err error) error { - errAccessDenied := &apistatus.ObjectAccessDenied{} - errAccessDenied.WriteReason(err.Error()) - return errAccessDenied + return s.apeChecker.CheckAPE(checkercore.CheckPrm{ + Request: request, + Namespace: namespace, + Container: cid, + PublicKey: publicKey, + BearerToken: bt, + SoftAPECheck: false, + }) } // fillWithUserClaimTags fills ape request properties with user claim tags getting them from frostfsid contract by actor public key. diff --git a/pkg/services/tree/service.go b/pkg/services/tree/service.go index 4da61617..875e47ec 100644 --- a/pkg/services/tree/service.go +++ b/pkg/services/tree/service.go @@ -10,6 +10,7 @@ import ( "sync/atomic" "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/local_object_storage/pilorama" + checkercore "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/services/common/ape" "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/util/logger" "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/container/acl" cidSDK "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/container/id" @@ -37,6 +38,8 @@ type Service struct { initialSyncDone atomic.Bool + apeChecker checkercore.CheckCore + // cnrMap contains existing (used) container IDs. cnrMap map[cidSDK.ID]struct{} // cnrMapMtx protects cnrMap @@ -72,6 +75,8 @@ func New(opts ...Option) *Service { s.syncChan = make(chan struct{}) s.syncPool, _ = ants.NewPool(defaultSyncWorkerCount) + s.apeChecker = checkercore.New(s.localOverrideStorage, s.morphChainStorage, s.frostfsidSubjectProvider, s.state) + return &s }