From a7eb6ecd40d3d5757c8255ddbdec5b77e18ab741 Mon Sep 17 00:00:00 2001 From: Leonard Lyubich Date: Thu, 22 Feb 2024 01:29:09 +0400 Subject: [PATCH] acl: Perceive tombstone saving as delete operation Previously, storage nodes handled `ObjectService.Put` requests regardless of object types. Since writing tombstones is essentially an operation of removing objects from their context, this opened up a security issue: granting the right to write objects into the container automatically granted the right to delete any objects from it. This, on the one hand, implicit, on the other hand, strange behavior would raise obvious questions about practical access control in the system. From now, `PUT` ops of `TOMBSTONE` objects are treated as `DELETE`. The only exclusion is intra-container replication that still should be performed in `PUT` manner (container nodes have no removal rights). Closes #2261. Signed-off-by: Leonard Lyubich --- CHANGELOG.md | 1 + pkg/services/object/acl/v2/service.go | 19 ++++++++++++++++++- pkg/services/object/acl/v2/util.go | 2 +- pkg/services/object/acl/v2/util_test.go | 2 +- 4 files changed, 21 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 64d54213e0..2de4044b22 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ Changelog for NeoFS Node ### Added ### Fixed +- Access to `PUT` objects no longer grants `DELETE` rights (#2261) ### Changed diff --git a/pkg/services/object/acl/v2/service.go b/pkg/services/object/acl/v2/service.go index 298b1a8f7b..89f0b9f434 100644 --- a/pkg/services/object/acl/v2/service.go +++ b/pkg/services/object/acl/v2/service.go @@ -511,11 +511,28 @@ func (p putStreamBasicChecker) Send(request *objectV2.PutRequest) error { src: request, } - reqInfo, err := p.source.findRequestInfo(req, cnr, acl.OpObjectPut) + verb := acl.OpObjectPut + tombstone := part.GetHeader().GetObjectType() == objectV2.TypeTombstone + if tombstone { + // such objects are specific - saving them is essentially the removal of other + // objects + verb = acl.OpObjectDelete + } + + reqInfo, err := p.source.findRequestInfo(req, cnr, verb) if err != nil { return err } + if tombstone { + // the only exception when writing tombstone should not be treated as deletion + // is intra-container replication: container nodes must be able to replicate + // such objects while deleting is prohibited + if reqInfo.requestRole == acl.RoleContainer && request.GetMetaHeader().GetTTL() == 1 { + reqInfo.operation = acl.OpObjectPut + } + } + reqInfo.obj = obj if !p.source.checker.CheckBasicACL(reqInfo) || !p.source.checker.StickyBitCheck(reqInfo, idOwner) { diff --git a/pkg/services/object/acl/v2/util.go b/pkg/services/object/acl/v2/util.go index bb5641bbc5..38143007f1 100644 --- a/pkg/services/object/acl/v2/util.go +++ b/pkg/services/object/acl/v2/util.go @@ -159,7 +159,7 @@ func assertVerb(tok sessionSDK.Object, op acl.Op) bool { //nolint:exhaustive switch op { case acl.OpObjectPut: - return tok.AssertVerb(sessionSDK.VerbObjectPut, sessionSDK.VerbObjectDelete) + return tok.AssertVerb(sessionSDK.VerbObjectPut) case acl.OpObjectDelete: return tok.AssertVerb(sessionSDK.VerbObjectDelete) case acl.OpObjectGet: diff --git a/pkg/services/object/acl/v2/util_test.go b/pkg/services/object/acl/v2/util_test.go index 2951219560..03f7e28645 100644 --- a/pkg/services/object/acl/v2/util_test.go +++ b/pkg/services/object/acl/v2/util_test.go @@ -65,7 +65,7 @@ func testGenerateMetaHeader(depth uint32, b *acl.BearerToken, s *session.Token) func TestIsVerbCompatible(t *testing.T) { // Source: https://nspcc.ru/upload/neofs-spec-latest.pdf#page=28 table := map[aclsdk.Op][]sessionSDK.ObjectVerb{ - aclsdk.OpObjectPut: {sessionSDK.VerbObjectPut, sessionSDK.VerbObjectDelete}, + aclsdk.OpObjectPut: {sessionSDK.VerbObjectPut}, aclsdk.OpObjectDelete: {sessionSDK.VerbObjectDelete}, aclsdk.OpObjectGet: {sessionSDK.VerbObjectGet}, aclsdk.OpObjectHead: {