From 89d92dee29a15c5d1be71859be9a2b485236ef4b Mon Sep 17 00:00:00 2001 From: Seena Fallah Date: Mon, 24 Feb 2025 23:41:13 +0100 Subject: [PATCH] rgw: check source object replication by replication actions Check for permissions of `s3:GetObjectVersionForReplication` in addition to `s3:GetObject` and `s3:GetObjectVersion` when fetching the object for multisite. Signed-off-by: Seena Fallah --- src/rgw/rgw_common.h | 7 ----- src/rgw/rgw_iam_policy.cc | 4 +++ src/rgw/rgw_iam_policy.h | 2 ++ src/rgw/rgw_op.cc | 42 ++++++++++++++++++++++------- src/test/rgw/test_rgw_iam_policy.cc | 4 +++ 5 files changed, 42 insertions(+), 17 deletions(-) diff --git a/src/rgw/rgw_common.h b/src/rgw/rgw_common.h index 82e7dad5743c4..01a9829783734 100644 --- a/src/rgw/rgw_common.h +++ b/src/rgw/rgw_common.h @@ -1831,13 +1831,6 @@ extern bool verify_object_permission( const std::vector& session_policies, const uint64_t op); extern bool verify_object_permission(const DoutPrefixProvider* dpp, req_state *s, uint64_t op); -extern bool verify_object_permission_no_policy( - const DoutPrefixProvider* dpp, - req_state * const s, - const RGWAccessControlPolicy& user_acl, - const RGWAccessControlPolicy& bucket_acl, - const RGWAccessControlPolicy& object_acl, - int perm); extern bool verify_object_permission_no_policy(const DoutPrefixProvider* dpp, req_state *s, int perm); extern int verify_object_lock( diff --git a/src/rgw/rgw_iam_policy.cc b/src/rgw/rgw_iam_policy.cc index 18a7334a5bbb0..356bd2384b8b6 100644 --- a/src/rgw/rgw_iam_policy.cc +++ b/src/rgw/rgw_iam_policy.cc @@ -141,6 +141,7 @@ static const actpair actpairs[] = { "s3:ReplicateDelete", s3ReplicateDelete }, { "s3:ReplicateObject", s3ReplicateObject }, { "s3:ReplicateTags", s3ReplicateTags }, + { "s3:GetObjectVersionForReplication", s3GetObjectVersionForReplication }, { "s3-object-lambda:GetObject", s3objectlambdaGetObject }, { "s3-object-lambda:ListBucket", s3objectlambdaListBucket }, { "iam:PutUserPolicy", iamPutUserPolicy }, @@ -1520,6 +1521,9 @@ const char* action_bit_string(uint64_t action) { case s3ReplicateTags: return "s3:ReplicateTags"; + case s3GetObjectVersionForReplication: + return "s3:GetObjectVersionForReplication"; + case s3objectlambdaGetObject: return "s3-object-lambda:GetObject"; diff --git a/src/rgw/rgw_iam_policy.h b/src/rgw/rgw_iam_policy.h index c0ba34f40ee34..095937ee1c4cd 100644 --- a/src/rgw/rgw_iam_policy.h +++ b/src/rgw/rgw_iam_policy.h @@ -119,6 +119,7 @@ enum { s3GetObjectVersionAttributes, s3ReplicateDelete, s3ReplicateObject, + s3GetObjectVersionForReplication, s3ReplicateTags, s3All, @@ -260,6 +261,7 @@ inline int op_to_perm(std::uint64_t op) { case s3ListBucketMultipartUploads: case s3ListBucketVersions: case s3ListMultipartUploadParts: + case s3GetObjectVersionForReplication: return RGW_PERM_READ; case s3AbortMultipartUpload: diff --git a/src/rgw/rgw_op.cc b/src/rgw/rgw_op.cc index 75b38f09afabc..a0c66e7abdad8 100644 --- a/src/rgw/rgw_op.cc +++ b/src/rgw/rgw_op.cc @@ -1103,21 +1103,43 @@ int RGWGetObj::verify_permission(optional_yield y) if (has_s3_existing_tag || has_s3_resource_tag) rgw_iam_add_objtags(this, s, has_s3_existing_tag, has_s3_resource_tag); - if (get_torrent) { - if (s->object->get_instance().empty()) { - action = rgw::IAM::s3GetObjectTorrent; - } else { - action = rgw::IAM::s3GetObjectVersionTorrent; + // for system requests, assume replication context and validate replication permissions. + // non-impersonated or standard system requests will be handled in rgw_process_authenticated(). + const bool is_replication_request = s->system_request; + + if (is_replication_request) { + // check for s3:GetObject(Version)Acl permission + action = s->object->get_instance().empty() ? rgw::IAM::s3GetObjectAcl : rgw::IAM::s3GetObjectVersionAcl; + if (!verify_object_permission(this, s, action)) { + s->err.message = fmt::format("missing {} permission", rgw::IAM::action_bit_string(action)); + ldpp_dout(this, 4) << "ERROR: fetching object for replication object=" << s->object << " reason=" << s->err.message << dendl; + + return -EACCES; } - } else { - if (s->object->get_instance().empty()) { - action = rgw::IAM::s3GetObject; - } else { - action = rgw::IAM::s3GetObjectVersion; + + // check for s3:GetObjectForReplication permission + // for versioned buckets, sync requests include `versionId`; for non-versioned, they don't. + // so s3:GetObjectForReplication doesn't help to be introduced as it doesn't add any value. + action = rgw::IAM::s3GetObjectVersionForReplication; + if (verify_object_permission(this, s, action)) { + return 0; } + + // fallback to s3:GetObject(Version) permission + action = s->object->get_instance().empty() ? rgw::IAM::s3GetObject : rgw::IAM::s3GetObjectVersion; + } else if (get_torrent) { + action = s->object->get_instance().empty() ? rgw::IAM::s3GetObjectTorrent : rgw::IAM::s3GetObjectVersionTorrent; + } else { + action = s->object->get_instance().empty() ? rgw::IAM::s3GetObject : rgw::IAM::s3GetObjectVersion; } if (!verify_object_permission(this, s, action)) { + s->err.message = fmt::format("missing {} permission", rgw::IAM::action_bit_string(action)); + + if (is_replication_request) { + ldpp_dout(this, 4) << "ERROR: fetching object for replication object=" << s->object << " reason=" << s->err.message << dendl; + } + return -EACCES; } diff --git a/src/test/rgw/test_rgw_iam_policy.cc b/src/test/rgw/test_rgw_iam_policy.cc index daa0c1dfda89a..79362a0875562 100644 --- a/src/test/rgw/test_rgw_iam_policy.cc +++ b/src/test/rgw/test_rgw_iam_policy.cc @@ -80,6 +80,7 @@ using rgw::IAM::s3GetObjectAttributes; using rgw::IAM::s3GetObjectVersionAttributes; using rgw::IAM::s3GetPublicAccessBlock; using rgw::IAM::s3GetReplicationConfiguration; +using rgw::IAM::s3GetObjectVersionForReplication; using rgw::IAM::s3ListAllMyBuckets; using rgw::IAM::s3ListBucket; using rgw::IAM::s3ListBucketMultipartUploads; @@ -452,6 +453,7 @@ TEST_F(PolicyTest, Parse3) { act2[s3GetBucketPublicAccessBlock] = 1; act2[s3GetPublicAccessBlock] = 1; act2[s3GetBucketEncryption] = 1; + act2[s3GetObjectVersionForReplication] = 1; EXPECT_EQ(p->statements[2].action, act2); EXPECT_EQ(p->statements[2].notaction, None); @@ -524,6 +526,7 @@ TEST_F(PolicyTest, Eval3) { s3allow[s3GetBucketPublicAccessBlock] = 1; s3allow[s3GetPublicAccessBlock] = 1; s3allow[s3GetBucketEncryption] = 1; + s3allow[s3GetObjectVersionForReplication] = 1; ARN arn1(Partition::aws, Service::s3, "", arbitrary_tenant, "mybucket"); @@ -920,6 +923,7 @@ TEST_F(ManagedPolicyTest, AmazonS3ReadOnlyAccess) act[s3GetPublicAccessBlock] = 1; act[s3GetBucketPublicAccessBlock] = 1; act[s3GetBucketEncryption] = 1; + act[s3GetObjectVersionForReplication] = 1; // s3:List* act[s3ListMultipartUploadParts] = 1; act[s3ListBucket] = 1; -- 2.39.5