From 0973b2ebe5d0bf30dfed3319e0c002c8340b469c 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 4e9cd42b8b1..172ad5b67f2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ Changelog for NeoFS Node ### Fixed - Inability to deploy contract with non-standard zone via neofs-adm (#2740) - Container session token's `wildcard` field support (#2741) +- 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 298b1a8f7ba..2839dc264bf 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 bb5641bbc53..38143007f19 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 29512195604..03f7e286452 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: {