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 <seenafallah@gmail.com>

diff --git a/src/rgw/rgw_common.h b/src/rgw/rgw_common.h
index 82e7dad5743c4b59a585b22efb13f8472130e0ad..01a9829783734b781be816bdbb925958501f174f 100644 (file)
--- a/src/rgw/rgw_common.h
+++ b/src/rgw/rgw_common.h
@@ -1831,13 +1831,6 @@ extern bool verify_object_permission(
   const std::vector<rgw::IAM::Policy>& 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 18a7334a5bbb0467145ec08d64d3285f0987e25b..356bd2384b8b6c7c66903fcdd670456f596945fe 100644 (file)
--- 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 c0ba34f40ee344081571063df368abde9ccda1c8..095937ee1c4cd55b214a8f258d8365c5a4c42891 100644 (file)
--- 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 75b38f09afabca18134002654626091326bd6627..a0c66e7abdad82397b0f94dcd84a3a66921ef9a0 100644 (file)
--- 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 daa0c1dfda89a1911fb870e4b5e7f482494035f6..79362a0875562e25e3c86edd243c99ff0a736301 100644 (file)
--- 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;