From ebfa3ac60eba67e759ba425594fb4546d8c33d01 Mon Sep 17 00:00:00 2001 From: aarifullin Date: Thu, 11 Apr 2024 11:53:01 +0300 Subject: [PATCH 1/3] [#1090] go.mod: Update policy-engine version Signed-off-by: Airat Arifullin --- go.mod | 2 +- go.sum | Bin 43413 -> 43413 bytes 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/go.mod b/go.mod index b99799337..d6d461f72 100644 --- a/go.mod +++ b/go.mod @@ -9,7 +9,7 @@ require ( git.frostfs.info/TrueCloudLab/frostfs-observability v0.0.0-20231101111734-b3ad3335ff65 git.frostfs.info/TrueCloudLab/frostfs-sdk-go v0.0.0-20240329104804-ec0cb2169f92 git.frostfs.info/TrueCloudLab/hrw v1.2.1 - git.frostfs.info/TrueCloudLab/policy-engine v0.0.0-20240307151106-2ec958cbfdfd + git.frostfs.info/TrueCloudLab/policy-engine v0.0.0-20240410114823-1f190e1668ec git.frostfs.info/TrueCloudLab/tzhash v1.8.0 git.frostfs.info/TrueCloudLab/zapjournald v0.0.0-20240124114243-cb2e66427d02 github.com/cheggaaa/pb v1.0.29 diff --git a/go.sum b/go.sum index 66273b1744c79ee39c08d23bf4d757e55eaae932..23d50b5b94c39d3d836bdb3e748d3fe41c1683a5 100644 GIT binary patch delta 110 zcmbPwnQ7`}rVZI_t|o>ChK42LLIaLLfu{ delta 110 zcmbPwnQ7`}rVZI_uEqxDhNgyw24=cOsmYe67RgCzDQPJR8HQE`m7Y14X(oA|Mdqp5 uUgpN(mf5L3o`I$*1^M1d7Wrlse!2N6#SxLF+WC_W*{sB{o3x32>LLJ;7b2 Date: Thu, 11 Apr 2024 12:35:49 +0300 Subject: [PATCH 2/3] [#1090] ape: Move ape request and resource implementations to common package Signed-off-by: Airat Arifullin --- pkg/ape/converter/converter.go | 44 +++++++++++++ pkg/ape/request/request.go | 55 ++++++++++++++++ pkg/services/container/ape.go | 84 ++++++++----------------- pkg/services/object/ape/request.go | 57 ++++------------- pkg/services/object/ape/request_test.go | 18 +++--- 5 files changed, 144 insertions(+), 114 deletions(-) create mode 100644 pkg/ape/converter/converter.go create mode 100644 pkg/ape/request/request.go diff --git a/pkg/ape/converter/converter.go b/pkg/ape/converter/converter.go new file mode 100644 index 000000000..9032680af --- /dev/null +++ b/pkg/ape/converter/converter.go @@ -0,0 +1,44 @@ +package converter + +import ( + "fmt" + + "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/container/acl" + nativeschema "git.frostfs.info/TrueCloudLab/policy-engine/schema/native" +) + +func SchemaRoleFromACLRole(role acl.Role) (string, error) { + switch role { + case acl.RoleOwner: + return nativeschema.PropertyValueContainerRoleOwner, nil + case acl.RoleContainer: + return nativeschema.PropertyValueContainerRoleContainer, nil + case acl.RoleInnerRing: + return nativeschema.PropertyValueContainerRoleIR, nil + case acl.RoleOthers: + return nativeschema.PropertyValueContainerRoleOthers, nil + default: + return "", fmt.Errorf("failed to convert %s", role.String()) + } +} + +func SchemaMethodFromACLOperation(op acl.Op) (string, error) { + switch op { + case acl.OpObjectGet: + return nativeschema.MethodGetObject, nil + case acl.OpObjectHead: + return nativeschema.MethodHeadObject, nil + case acl.OpObjectPut: + return nativeschema.MethodPutObject, nil + case acl.OpObjectDelete: + return nativeschema.MethodDeleteObject, nil + case acl.OpObjectSearch: + return nativeschema.MethodSearchObject, nil + case acl.OpObjectRange: + return nativeschema.MethodRangeObject, nil + case acl.OpObjectHash: + return nativeschema.MethodHashObject, nil + default: + return "", fmt.Errorf("operation cannot be converted: %d", op) + } +} diff --git a/pkg/ape/request/request.go b/pkg/ape/request/request.go new file mode 100644 index 000000000..6d62ef3d5 --- /dev/null +++ b/pkg/ape/request/request.go @@ -0,0 +1,55 @@ +package ape + +import ( + aperesource "git.frostfs.info/TrueCloudLab/policy-engine/pkg/resource" +) + +type Request struct { + operation string + resource Resource + properties map[string]string +} + +func NewRequest(operation string, resource Resource, properties map[string]string) Request { + return Request{ + operation: operation, + resource: resource, + properties: properties, + } +} + +var _ aperesource.Request = Request{} + +func (r Request) Operation() string { + return r.operation +} + +func (r Request) Property(key string) string { + return r.properties[key] +} + +func (r Request) Resource() aperesource.Resource { + return r.resource +} + +type Resource struct { + name string + properties map[string]string +} + +var _ aperesource.Resource = Resource{} + +func NewResource(name string, properties map[string]string) Resource { + return Resource{ + name: name, + properties: properties, + } +} + +func (r Resource) Name() string { + return r.name +} + +func (r Resource) Property(key string) string { + return r.properties[key] +} diff --git a/pkg/services/container/ape.go b/pkg/services/container/ape.go index 83361257a..7622a40bc 100644 --- a/pkg/services/container/ape.go +++ b/pkg/services/container/ape.go @@ -15,6 +15,7 @@ import ( "git.frostfs.info/TrueCloudLab/frostfs-api-go/v2/refs" session "git.frostfs.info/TrueCloudLab/frostfs-api-go/v2/session" "git.frostfs.info/TrueCloudLab/frostfs-contract/frostfsid/client" + aperequest "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/ape/request" containercore "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/core/container" "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/core/netmap" "git.frostfs.info/TrueCloudLab/frostfs-observability/tracing" @@ -26,7 +27,6 @@ import ( "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" - "git.frostfs.info/TrueCloudLab/policy-engine/pkg/resource" nativeschema "git.frostfs.info/TrueCloudLab/policy-engine/schema/native" "github.com/nspcc-dev/neo-go/pkg/crypto/keys" "github.com/nspcc-dev/neo-go/pkg/util" @@ -148,14 +148,14 @@ func (ac *apeChecker) List(ctx context.Context, req *container.ListRequest) (*co return nil, err } - request := &apeRequest{ - resource: &apeResource{ - name: resourceName(namespace, ""), - props: make(map[string]string), - }, - op: nativeschema.MethodListContainers, - props: reqProps, - } + request := aperequest.NewRequest( + nativeschema.MethodListContainers, + aperequest.NewResource( + resourceName(namespace, ""), + make(map[string]string), + ), + reqProps, + ) s, found, err := ac.router.IsAllowed(apechain.Ingress, policyengine.NewRequestTargetWithNamespace(namespace), @@ -193,14 +193,14 @@ func (ac *apeChecker) Put(ctx context.Context, req *container.PutRequest) (*cont return nil, err } - request := &apeRequest{ - resource: &apeResource{ - name: resourceName(namespace, ""), - props: make(map[string]string), - }, - op: nativeschema.MethodPutContainer, - props: reqProps, - } + request := aperequest.NewRequest( + nativeschema.MethodPutContainer, + aperequest.NewResource( + resourceName(namespace, ""), + make(map[string]string), + ), + reqProps, + ) s, found, err := ac.router.IsAllowed(apechain.Ingress, policyengine.NewRequestTargetWithNamespace(namespace), @@ -288,14 +288,14 @@ func (ac *apeChecker) validateContainerBoundedOperation(containerID *refs.Contai namespace = cntNamespace } - request := &apeRequest{ - resource: &apeResource{ - name: resourceName(namespace, id.EncodeToString()), - props: ac.getContainerProps(cont), - }, - op: op, - props: reqProps, - } + request := aperequest.NewRequest( + op, + aperequest.NewResource( + resourceName(namespace, id.EncodeToString()), + ac.getContainerProps(cont), + ), + reqProps, + ) s, found, err := ac.router.IsAllowed(apechain.Ingress, policyengine.NewRequestTarget(namespace, id.EncodeToString()), @@ -329,40 +329,6 @@ func getContainerID(reqContID *refs.ContainerID) (cid.ID, error) { return id, nil } -type apeRequest struct { - resource *apeResource - op string - props map[string]string -} - -// Operation implements resource.Request. -func (r *apeRequest) Operation() string { - return r.op -} - -// Property implements resource.Request. -func (r *apeRequest) Property(key string) string { - return r.props[key] -} - -// Resource implements resource.Request. -func (r *apeRequest) Resource() resource.Resource { - return r.resource -} - -type apeResource struct { - name string - props map[string]string -} - -func (r *apeResource) Name() string { - return r.name -} - -func (r *apeResource) Property(key string) string { - return r.props[key] -} - func resourceName(namespace string, container string) string { if namespace == "" && container == "" { return nativeschema.ResourceFormatRootContainers diff --git a/pkg/services/object/ape/request.go b/pkg/services/object/ape/request.go index b81fe8c32..7bbee31c1 100644 --- a/pkg/services/object/ape/request.go +++ b/pkg/services/object/ape/request.go @@ -6,51 +6,16 @@ import ( "strconv" objectV2 "git.frostfs.info/TrueCloudLab/frostfs-api-go/v2/object" + aperequest "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/ape/request" "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/container/acl" cid "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/container/id" objectSDK "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/object" oid "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/object/id" "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/user" - aperesource "git.frostfs.info/TrueCloudLab/policy-engine/pkg/resource" nativeschema "git.frostfs.info/TrueCloudLab/policy-engine/schema/native" ) -type request struct { - operation string - resource resource - properties map[string]string -} - -var defaultRequest = request{} - -var _ aperesource.Request = request{} - -type resource struct { - name string - properties map[string]string -} - -var _ aperesource.Resource = resource{} - -func (r resource) Name() string { - return r.name -} - -func (r resource) Property(key string) string { - return r.properties[key] -} - -func (r request) Operation() string { - return r.operation -} - -func (r request) Property(key string) string { - return r.properties[key] -} - -func (r request) Resource() aperesource.Resource { - return r.resource -} +var defaultRequest = aperequest.Request{} func nativeSchemaRole(role acl.Role) string { switch role { @@ -125,7 +90,7 @@ func objectProperties(cnr cid.ID, oid *oid.ID, cnrOwner user.ID, header *objectV // newAPERequest creates an APE request to be passed to a chain router. It collects resource properties from // header provided by headerProvider. If it cannot be found in headerProvider, then properties are // initialized from header given in prm (if it is set). Otherwise, just CID and OID are set to properties. -func (c *checkerImpl) newAPERequest(ctx context.Context, prm Prm) (request, error) { +func (c *checkerImpl) newAPERequest(ctx context.Context, prm Prm) (aperequest.Request, error) { switch prm.Method { case nativeschema.MethodGetObject, nativeschema.MethodHeadObject, @@ -150,15 +115,15 @@ func (c *checkerImpl) newAPERequest(ctx context.Context, prm Prm) (request, erro } } - return request{ - operation: prm.Method, - resource: resource{ - name: resourceName(prm.Container, prm.Object, prm.Namespace), - properties: objectProperties(prm.Container, prm.Object, prm.ContainerOwner, header), - }, - properties: map[string]string{ + return aperequest.NewRequest( + prm.Method, + aperequest.NewResource( + resourceName(prm.Container, prm.Object, prm.Namespace), + objectProperties(prm.Container, prm.Object, prm.ContainerOwner, header), + ), + map[string]string{ nativeschema.PropertyKeyActorPublicKey: prm.SenderKey, nativeschema.PropertyKeyActorRole: prm.Role, }, - }, nil + ), nil } diff --git a/pkg/services/object/ape/request_test.go b/pkg/services/object/ape/request_test.go index 71e234e78..fdb7af219 100644 --- a/pkg/services/object/ape/request_test.go +++ b/pkg/services/object/ape/request_test.go @@ -6,6 +6,7 @@ import ( "testing" objectV2 "git.frostfs.info/TrueCloudLab/frostfs-api-go/v2/object" + aperequest "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/ape/request" checksumtest "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/checksum/test" objectSDK "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/object" "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/user" @@ -256,22 +257,21 @@ func TestNewAPERequest(t *testing.T) { return } - expectedRequest := request{ - operation: method, - resource: resource{ - name: resourceName(cnr, obj, prm.Namespace), - properties: objectProperties(cnr, obj, testCnrOwner, func() *objectV2.Header { + expectedRequest := aperequest.NewRequest( + method, + aperequest.NewResource( + resourceName(cnr, obj, prm.Namespace), + objectProperties(cnr, obj, testCnrOwner, func() *objectV2.Header { if headerObjSDK != nil { return headerObjSDK.ToV2().GetHeader() } return prm.Header - }()), - }, - properties: map[string]string{ + }())), + map[string]string{ nativeschema.PropertyKeyActorPublicKey: prm.SenderKey, nativeschema.PropertyKeyActorRole: prm.Role, }, - } + ) require.Equal(t, expectedRequest, r) }) -- 2.45.2 From 04aa7874b2582ec8a758bf890a02e5d1635aa3df Mon Sep 17 00:00:00 2001 From: aarifullin Date: Thu, 11 Apr 2024 18:10:33 +0300 Subject: [PATCH 3/3] [#1090] tree: Make workaround for APE checks * Make `verifyClient` method perform APE check if a container was created with zero-filled basic ACL. * Object verbs are used in APE, until tree verbs are introduced. Signed-off-by: Airat Arifullin --- cmd/frostfs-node/tree.go | 4 +- pkg/services/tree/ape.go | 69 ++++++++++++++++++++++++++++++++++ pkg/services/tree/options.go | 9 +++++ pkg/services/tree/signature.go | 13 ++++--- 4 files changed, 89 insertions(+), 6 deletions(-) create mode 100644 pkg/services/tree/ape.go diff --git a/cmd/frostfs-node/tree.go b/cmd/frostfs-node/tree.go index dced05bc2..ff00b6fac 100644 --- a/cmd/frostfs-node/tree.go +++ b/cmd/frostfs-node/tree.go @@ -63,7 +63,9 @@ func initTreeService(c *cfg) { tree.WithReplicationChannelCapacity(treeConfig.ReplicationChannelCapacity()), tree.WithReplicationWorkerCount(treeConfig.ReplicationWorkerCount()), tree.WithAuthorizedKeys(treeConfig.AuthorizedKeys()), - tree.WithMetrics(c.metricsCollector.TreeService())) + tree.WithMetrics(c.metricsCollector.TreeService()), + tree.WithAPERouter(c.cfgObject.cfgAccessPolicyEngine.accessPolicyEngine), + ) c.cfgGRPC.performAndSave(func(_ string, _ net.Listener, s *grpc.Server) { tree.RegisterTreeServiceServer(s, c.treeService) diff --git a/pkg/services/tree/ape.go b/pkg/services/tree/ape.go new file mode 100644 index 000000000..52036074a --- /dev/null +++ b/pkg/services/tree/ape.go @@ -0,0 +1,69 @@ +package tree + +import ( + "encoding/hex" + "fmt" + "strings" + + "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/ape/converter" + aperequest "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/ape/request" + core "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/core/container" + 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" + apechain "git.frostfs.info/TrueCloudLab/policy-engine/pkg/chain" + "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" +) + +func (s *Service) checkAPE(container *core.Container, cid cid.ID, operation acl.Op, role acl.Role, publicKey *keys.PublicKey) error { + namespace := "" + cntNamespace, hasNamespace := strings.CutSuffix(cnrSDK.ReadDomain(container.Value).Zone(), ".ns") + if hasNamespace { + namespace = cntNamespace + } + + schemaMethod, err := converter.SchemaMethodFromACLOperation(operation) + if err != nil { + return apeErr(err) + } + schemaRole, err := converter.SchemaRoleFromACLRole(role) + if err != nil { + return apeErr(err) + } + reqProps := map[string]string{ + nativeschema.PropertyKeyActorPublicKey: hex.EncodeToString(publicKey.Bytes()), + nativeschema.PropertyKeyActorRole: schemaRole, + } + + var resourceName string + if namespace == "root" || namespace == "" { + resourceName = fmt.Sprintf(nativeschema.ResourceFormatRootContainerObjects, cid.EncodeToString()) + } else { + resourceName = fmt.Sprintf(nativeschema.ResourceFormatNamespaceContainerObjects, namespace, cid.EncodeToString()) + } + + request := aperequest.NewRequest( + schemaMethod, + aperequest.NewResource(resourceName, make(map[string]string)), + reqProps, + ) + + status, found, err := s.router.IsAllowed(apechain.Ingress, engine.NewRequestTarget(namespace, cid.EncodeToString()), request) + if err != nil { + return apeErr(err) + } + if found && status == apechain.Allow { + return nil + } + err = fmt.Errorf("access to operation %s is denied by access policy engine: %s", schemaMethod, status.String()) + return apeErr(err) +} + +func apeErr(err error) error { + errAccessDenied := &apistatus.ObjectAccessDenied{} + errAccessDenied.WriteReason(err.Error()) + return errAccessDenied +} diff --git a/pkg/services/tree/options.go b/pkg/services/tree/options.go index 043e12cb2..0e5f64274 100644 --- a/pkg/services/tree/options.go +++ b/pkg/services/tree/options.go @@ -9,6 +9,7 @@ import ( "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/local_object_storage/pilorama" "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/util/logger" cid "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/container/id" + policyengine "git.frostfs.info/TrueCloudLab/policy-engine/pkg/engine" "github.com/nspcc-dev/neo-go/pkg/crypto/keys" ) @@ -38,6 +39,8 @@ type cfg struct { containerCacheSize int authorizedKeys [][]byte + router policyengine.ChainRouter + metrics MetricsRegister } @@ -139,3 +142,9 @@ func WithAuthorizedKeys(keys keys.PublicKeys) Option { } } } + +func WithAPERouter(router policyengine.ChainRouter) Option { + return func(c *cfg) { + c.router = router + } +} diff --git a/pkg/services/tree/signature.go b/pkg/services/tree/signature.go index 985e1ad94..162b189e3 100644 --- a/pkg/services/tree/signature.go +++ b/pkg/services/tree/signature.go @@ -71,7 +71,7 @@ func (s *Service) verifyClient(req message, cid cidSDK.ID, rawBearer []byte, op return err } - role, err := roleFromReq(cnr, req, bt) + role, pubKey, err := roleAndPubKeyFromReq(cnr, req, bt) if err != nil { return fmt.Errorf("can't get request role: %w", err) } @@ -79,8 +79,11 @@ func (s *Service) verifyClient(req message, cid cidSDK.ID, rawBearer []byte, op basicACL := cnr.Value.BasicACL() // Basic ACL mask can be unset, if a container operations are performed // with strict APE checks only. + // + // FIXME(@aarifullin): tree service temporiraly performs APE checks on + // object verbs, because tree verbs have not been introduced yet. if basicACL == 0x0 { - return nil + return s.checkAPE(cnr, cid, op, role, pubKey) } if !basicACL.IsOpAllowed(op, role) { @@ -222,7 +225,7 @@ func SignMessage(m message, key *ecdsa.PrivateKey) error { return nil } -func roleFromReq(cnr *core.Container, req message, bt *bearer.Token) (acl.Role, error) { +func roleAndPubKeyFromReq(cnr *core.Container, req message, bt *bearer.Token) (acl.Role, *keys.PublicKey, error) { role := acl.RoleOthers owner := cnr.Value.Owner() @@ -233,7 +236,7 @@ func roleFromReq(cnr *core.Container, req message, bt *bearer.Token) (acl.Role, pub, err := keys.NewPublicKeyFromBytes(rawKey, elliptic.P256()) if err != nil { - return role, fmt.Errorf("invalid public key: %w", err) + return role, nil, fmt.Errorf("invalid public key: %w", err) } var reqSigner user.ID @@ -243,7 +246,7 @@ func roleFromReq(cnr *core.Container, req message, bt *bearer.Token) (acl.Role, role = acl.RoleOwner } - return role, nil + return role, pub, nil } func eACLOp(op acl.Op) eacl.Operation { -- 2.45.2