[#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 <a.arifullin@yadro.com>

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
+}

pkg/services/object/ape/checker.go

@@ -2,47 +2,39 @@ 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
+	checkerCore     checkercore.CheckCore
-	morphChainStorage    policyengine.MorphRuleChainStorageReader
-	headerProvider       HeaderProvider
 	frostFSIDClient frostfsidcore.SubjectProvider
+	headerProvider  HeaderProvider
 	nm              netmap.Source
-	st                   netmap.State
 	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,
+		checkerCore:     checkercore.New(localOverrideStorage, morphChainStorage, frostFSIDClient, st),
-		morphChainStorage:    morphChainStorage,
-		headerProvider:       headerProvider,
 		frostFSIDClient: frostFSIDClient,
+		headerProvider:  headerProvider,
 		nm:              nm,
-		st:                   st,
 		cnrSource:       cnrSource,
 		nodePK:          nodePK,
 	}
@@ -85,68 +77,9 @@ type Prm struct {
 	XHeaders []session.XHeader
 }
 
-var (
+var errMissingOID = errors.New("object ID is not set")
-	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")
-)
 
-// isValidBearer checks whether bearer token was correctly signed by authorized
+// CheckAPE prepares an APE-request and checks if it is permitted by policies.
-// 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.
 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.
+	return c.checkerCore.CheckAPE(checkercore.CheckPrm{
-	for i := range groups {
+		Request:        r,
-		groups[i] = fmt.Sprintf("%s:%s", prm.Namespace, groups[i])
+		PublicKey:      pub,
-	}
+		Namespace:      prm.Method,
-
+		Container:      prm.Container,
-	var cr policyengine.ChainRouter
+		ContainerOwner: prm.ContainerOwner,
-	if prm.BearerToken != nil && !prm.BearerToken.Impersonate() {
+		BearerToken:    prm.BearerToken,
-		if err := isValidBearer(prm.BearerToken, prm.ContainerOwner, prm.Container, pub, c.st); err != nil {
+		SoftAPECheck:   prm.SoftAPECheck,
-			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)
 }

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"
+	checkercore "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/services/common/ape"
-	"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"
 	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
+	return s.apeChecker.CheckAPE(checkercore.CheckPrm{
-	if bt != nil && !bt.Impersonate() {
+		Request:      request,
-		if err := isValidBearer(bt, container.Value.Owner(), cid, publicKey, s.state); err != nil {
+		Namespace:    namespace,
-			return fmt.Errorf("bearer validation error: %w", err)
+		Container:    cid,
-		}
+		PublicKey:    publicKey,
-		cr, err = router.BearerChainFeedRouter(s.localOverrideStorage, s.morphChainStorage, bt.APEOverride())
+		BearerToken:  bt,
-		if err != nil {
+		SoftAPECheck: false,
-			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
 }
 
 // fillWithUserClaimTags fills ape request properties with user claim tags getting them from frostfsid contract by actor public key.

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
 }