Merge pull request ceph#60685 from clwluvw/data-sync-perm

ivancich · web-flow · commit 49a44ce2d946 · 2025-01-03T12:16:39.000-05:00
rgw: respect policies in data sync in user mode

Reviewed-by: Adam Emerson &lt;aemerson@redhat.com&gt;
diff --git a/src/rgw/driver/rados/rgw_data_sync.cc b/src/rgw/driver/rados/rgw_data_sync.cc
@@ -2617,6 +2617,7 @@ class RGWUserPermHandler {
     rgw::IAM::Environment env;
     std::unique_ptr<rgw::auth::Identity> identity;
     RGWAccessControlPolicy user_acl;
+    std::vector<rgw::IAM::Policy> user_policies;
   };
 
   std::shared_ptr<_info> info;
@@ -2644,7 +2645,7 @@ class RGWUserPermHandler {
       }
 
       auto result = rgw::auth::transform_old_authinfo(
-          sync_env->dpp, null_yield, sync_env->driver, user.get());
+          sync_env->dpp, null_yield, sync_env->driver, user.get(), &info->user_policies);
       if (!result) {
         return result.error();
       }
@@ -2679,16 +2680,15 @@ class RGWUserPermHandler {
     std::shared_ptr<_info> info;
     RGWAccessControlPolicy bucket_acl;
     std::optional<perm_state> ps;
+    boost::optional<rgw::IAM::Policy> bucket_policy;
   public:
     Bucket() {}
 
     int init(RGWUserPermHandler *handler,
              const RGWBucketInfo& bucket_info,
              const map<string, bufferlist>& bucket_attrs);
 
-    bool verify_bucket_permission(int perm);
-    bool verify_object_permission(const map<string, bufferlist>& obj_attrs,
-                                  int perm);
+    bool verify_bucket_permission(const rgw_obj_key& obj_key, const uint64_t op);
   };
 
   static int policy_from_attrs(CephContext *cct,
@@ -2728,6 +2728,14 @@ int RGWUserPermHandler::Bucket::init(RGWUserPermHandler *handler,
     return r;
   }
 
+  // load bucket policy
+  try {
+    bucket_policy = get_iam_policy_from_attr(sync_env->cct, bucket_attrs, bucket_info.bucket.tenant);
+  } catch (const std::exception& e) {
+    ldpp_dout(sync_env->dpp, 0) << "ERROR: reading IAM Policy: " << e.what() << dendl;
+    return -EACCES;
+  }
+
   ps.emplace(sync_env->cct,
              info->env,
              info->identity.get(),
@@ -2740,36 +2748,40 @@ int RGWUserPermHandler::Bucket::init(RGWUserPermHandler *handler,
   return 0;
 }
 
-bool RGWUserPermHandler::Bucket::verify_bucket_permission(int perm)
-{
-  return verify_bucket_permission_no_policy(sync_env->dpp,
-                                            &(*ps),
-                                            info->user_acl,
-                                            bucket_acl,
-                                            perm);
-}
-
-bool RGWUserPermHandler::Bucket::verify_object_permission(const map<string, bufferlist>& obj_attrs,
-                                                          int perm)
+bool RGWUserPermHandler::Bucket::verify_bucket_permission(const rgw_obj_key& obj_key, const uint64_t op)
 {
-  RGWAccessControlPolicy obj_acl;
-
-  int r = policy_from_attrs(sync_env->cct, obj_attrs, &obj_acl);
-  if (r < 0) {
-    return r;
-  }
-
-  return verify_bucket_permission_no_policy(sync_env->dpp,
-                                            &(*ps),
-                                            bucket_acl,
-                                            obj_acl,
-                                            perm);
+  const rgw_obj obj(ps->bucket_info.bucket, obj_key);
+  const auto arn = rgw::ARN(obj);
+
+  if (ps->identity->get_account()) {
+    const bool account_root = (ps->identity->get_identity_type() == TYPE_ROOT);
+    if (!ps->identity->is_owner_of(bucket_acl.get_owner().id)) {
+      ldpp_dout(sync_env->dpp, 4) << "cross-account request for bucket owner "
+          << bucket_acl.get_owner().id << " != " << ps->identity->get_aclowner().id << dendl;
+      // cross-account requests evaluate the identity-based policies separately
+      // from the resource-based policies and require Allow from both
+      return ::verify_bucket_permission(sync_env->dpp, &(*ps), arn, account_root, {}, {}, {},
+                                      info->user_policies, {}, op)
+          && ::verify_bucket_permission(sync_env->dpp, &(*ps), arn, false, info->user_acl,
+                                      bucket_acl, bucket_policy, {}, {}, op);
+    } else {
+      // don't consult acls for same-account access. require an Allow from
+      // either identity- or resource-based policy
+      return ::verify_bucket_permission(sync_env->dpp, &(*ps), arn, account_root, {}, {},
+                                      bucket_policy, info->user_policies,
+                                      {}, op);
+    }
+  }
+  constexpr bool account_root = false;
+  return ::verify_bucket_permission(sync_env->dpp, &(*ps), arn, account_root,
+                                  info->user_acl, bucket_acl,
+                                  bucket_policy, info->user_policies,
+                                  {}, op);
 }
 
 class RGWFetchObjFilter_Sync : public RGWFetchObjFilter_Default {
   rgw_bucket_sync_pipe sync_pipe;
 
-  std::shared_ptr<RGWUserPermHandler::Bucket> bucket_perms;
   std::optional<rgw_sync_pipe_dest_params> verify_dest_params;
 
   std::optional<ceph::real_time> mtime;
@@ -2782,10 +2794,8 @@ class RGWFetchObjFilter_Sync : public RGWFetchObjFilter_Default {
 
 public:
   RGWFetchObjFilter_Sync(rgw_bucket_sync_pipe& _sync_pipe,
-                         std::shared_ptr<RGWUserPermHandler::Bucket>& _bucket_perms,
                          std::optional<rgw_sync_pipe_dest_params>&& _verify_dest_params,
                          std::shared_ptr<bool>& _need_retry) : sync_pipe(_sync_pipe),
-                                         bucket_perms(_bucket_perms),
                                          verify_dest_params(std::move(_verify_dest_params)),
                                          need_retry(_need_retry) {
     *need_retry = false;
@@ -2852,12 +2862,6 @@ int RGWFetchObjFilter_Sync::filter(CephContext *cct,
       *poverride_owner = acl_translation_owner;
     }
   }
-  if (params.mode == rgw_sync_pipe_params::MODE_USER) {
-    if (!bucket_perms->verify_object_permission(obj_attrs, RGW_PERM_READ)) {
-      ldout(cct, 0) << "ERROR: " << __func__ << ": permission check failed: user not allowed to fetch object" << dendl;
-      return -EPERM;
-    }
-  }
 
   if (!dest_placement_rule &&
       params.dest.storage_class) {
@@ -2900,7 +2904,6 @@ class RGWObjFetchCR : public RGWCoroutine {
   rgw_sync_pipe_params::Mode param_mode;
 
   std::optional<RGWUserPermHandler> user_perms;
-  std::shared_ptr<RGWUserPermHandler::Bucket> source_bucket_perms;
   RGWUserPermHandler::Bucket dest_bucket_perms;
 
   std::optional<rgw_sync_pipe_dest_params> dest_params;
@@ -3016,33 +3019,22 @@ class RGWObjFetchCR : public RGWCoroutine {
             return set_cr_error(retcode);
           }
 
-          if (!dest_bucket_perms.verify_bucket_permission(RGW_PERM_WRITE)) {
+          if (!dest_bucket_perms.verify_bucket_permission(dest_key.value_or(key), rgw::IAM::s3PutObject)) {
             ldout(cct, 0) << "ERROR: " << __func__ << ": permission check failed: user not allowed to write into bucket (bucket=" << sync_pipe.info.dest_bucket.get_key() << ")" << dendl;
             return -EPERM;
           }
-
-          /* init source bucket permission structure */
-          source_bucket_perms = make_shared<RGWUserPermHandler::Bucket>();
-          r = user_perms->init_bucket(sync_pipe.source_bucket_info,
-                                      sync_pipe.source_bucket_attrs,
-                                      source_bucket_perms.get());
-          if (r < 0) {
-            ldout(cct, 20) << "ERROR: " << __func__ << ": failed to init bucket perms manager for uid=" << *param_user << " bucket=" << sync_pipe.source_bucket_info.bucket.get_key() << dendl;
-            return set_cr_error(retcode);
-          }
         }
 
         yield {
           if (!need_retry) {
             need_retry = make_shared<bool>();
           }
           auto filter = make_shared<RGWFetchObjFilter_Sync>(sync_pipe,
-                                                            source_bucket_perms,
                                                             std::move(dest_params),
                                                             need_retry);
 
           call(new RGWFetchRemoteObjCR(sync_env->async_rados, sync_env->driver, sc->source_zone,
-                                       nullopt,
+                                       param_user,
                                        sync_pipe.source_bucket_info.bucket,
                                        std::nullopt, sync_pipe.dest_bucket_info,
                                        key, dest_key, versioned_epoch,
@@ -4528,7 +4520,7 @@ class RGWBucketSyncSingleEntryCR : public RGWCoroutine {
           }
           tn->set_resource_name(SSTR(bucket_str_noinstance(bs.bucket) << "/" << key));
         }
-        if (retcode == -ERR_PRECONDITION_FAILED) {
+        if (retcode == -ERR_PRECONDITION_FAILED || retcode == -EPERM) {
 	  pretty_print(sc->env, "Skipping object s3://{}/{} in sync from zone {}\n",
 		       bs.bucket.name, key, zone_name);
           set_status("Skipping object sync: precondition failed (object contains newer change or policy doesn't allow sync)");
diff --git a/src/rgw/rgw_auth.cc b/src/rgw/rgw_auth.cc
@@ -313,7 +313,8 @@ static auto transform_old_authinfo(const RGWUserInfo& user,
 auto transform_old_authinfo(const DoutPrefixProvider* dpp,
                             optional_yield y,
                             sal::Driver* driver,
-                            sal::User* user)
+                            sal::User* user,
+                            std::vector<IAM::Policy>* policies_)
   -> tl::expected<std::unique_ptr<Identity>, int>
 {
   const RGWUserInfo& info = user->get_info();
@@ -328,6 +329,9 @@ auto transform_old_authinfo(const DoutPrefixProvider* dpp,
     return tl::unexpected(r);
   }
 
+  if (policies_) { // return policies to caller if requested
+    *policies_ = policies;
+  }
   return transform_old_authinfo(info, std::move(account), std::move(policies));
 }
 
diff --git a/src/rgw/rgw_auth.h b/src/rgw/rgw_auth.h
@@ -105,7 +105,8 @@ inline std::ostream& operator<<(std::ostream& out,
 auto transform_old_authinfo(const DoutPrefixProvider* dpp,
                             optional_yield y,
                             sal::Driver* driver,
-                            sal::User* user)
+                            sal::User* user,
+                            std::vector<IAM::Policy>* policies_ = nullptr)
   -> tl::expected<std::unique_ptr<Identity>, int>;
 
 // Load the user account and all user/group policies. May throw
diff --git a/src/rgw/rgw_common.cc b/src/rgw/rgw_common.cc
@@ -3206,3 +3206,14 @@ void RGWObjVersionTracker::generate_new_write_ver(CephContext *cct)
   append_rand_alpha(cct, write_version.tag, write_version.tag, TAG_LEN);
 }
 
+boost::optional<rgw::IAM::Policy>
+get_iam_policy_from_attr(CephContext* cct,
+                         const std::map<std::string, bufferlist>& attrs,
+                         const std::string& tenant)
+{
+  if (auto i = attrs.find(RGW_ATTR_IAM_POLICY); i != attrs.end()) {
+    return Policy(cct, &tenant, i->second.to_str(), false);
+  } else {
+    return boost::none;
+  }
+}
diff --git a/src/rgw/rgw_common.h b/src/rgw/rgw_common.h
@@ -1745,18 +1745,6 @@ rgw::IAM::Effect evaluate_iam_policies(
     const std::vector<rgw::IAM::Policy>& identity_policies,
     const std::vector<rgw::IAM::Policy>& session_policies);
 
-bool verify_user_permission(const DoutPrefixProvider* dpp,
-                            req_state * const s,
-                            const RGWAccessControlPolicy& user_acl,
-                            const std::vector<rgw::IAM::Policy>& user_policies,
-                            const std::vector<rgw::IAM::Policy>& session_policies,
-                            const rgw::ARN& res,
-                            const uint64_t op,
-                            bool mandatory_policy=true);
-bool verify_user_permission_no_policy(const DoutPrefixProvider* dpp,
-                                      req_state * const s,
-                                      const RGWAccessControlPolicy& user_acl,
-                                      const int perm);
 bool verify_user_permission(const DoutPrefixProvider* dpp,
                             req_state * const s,
                             const rgw::ARN& res,
@@ -1765,6 +1753,16 @@ bool verify_user_permission(const DoutPrefixProvider* dpp,
 bool verify_user_permission_no_policy(const DoutPrefixProvider* dpp,
                                       req_state * const s,
                                       int perm);
+bool verify_bucket_permission(const DoutPrefixProvider* dpp,
+                              struct perm_state_base * const s,
+                              const rgw::ARN& arn,
+                              bool account_root,
+                              const RGWAccessControlPolicy& user_acl,
+                              const RGWAccessControlPolicy& bucket_acl,
+			      const boost::optional<rgw::IAM::Policy>& bucket_policy,
+                              const std::vector<rgw::IAM::Policy>& identity_policies,
+                              const std::vector<rgw::IAM::Policy>& session_policies,
+                              const uint64_t op);
 bool verify_bucket_permission(
   const DoutPrefixProvider* dpp,
   req_state * const s,
@@ -2012,3 +2010,8 @@ struct AioCompletionDeleter {
   void operator()(librados::AioCompletion* c) { c->release(); }
 };
 using aio_completion_ptr = std::unique_ptr<librados::AioCompletion, AioCompletionDeleter>;
+
+extern boost::optional<rgw::IAM::Policy>
+get_iam_policy_from_attr(CephContext* cct,
+                         const std::map<std::string, bufferlist>& attrs,
+                         const std::string& tenant);
diff --git a/src/rgw/rgw_op.cc b/src/rgw/rgw_op.cc
@@ -331,19 +331,6 @@ static int get_obj_policy_from_attr(const DoutPrefixProvider *dpp,
   return ret;
 }
 
-
-static boost::optional<Policy>
-get_iam_policy_from_attr(CephContext* cct,
-                         const map<string, bufferlist>& attrs,
-                         const string& tenant)
-{
-  if (auto i = attrs.find(RGW_ATTR_IAM_POLICY); i != attrs.end()) {
-    return Policy(cct, &tenant, i->second.to_str(), false);
-  } else {
-    return none;
-  }
-}
-
 static boost::optional<PublicAccessBlockConfiguration>
 get_public_access_conf_from_attr(const map<string, bufferlist>& attrs)
 {

Original file line number	Diff line number	Diff line change
`@@ -313,7 +313,8 @@ static auto transform_old_authinfo(const RGWUserInfo& user,`
`313`	`313`	`auto transform_old_authinfo(const DoutPrefixProvider* dpp,`
`314`	`314`	`optional_yield y,`
`315`	`315`	`sal::Driver* driver,`
`316`		`- sal::User* user)`
	`316`	`+ sal::User* user,`
	`317`	`+ std::vector<IAM::Policy>* policies_)`
`317`	`318`	`-> tl::expected<std::unique_ptr<Identity>, int>`
`318`	`319`	`{`
`319`	`320`	`const RGWUserInfo& info = user->get_info();`
`@@ -328,6 +329,9 @@ auto transform_old_authinfo(const DoutPrefixProvider* dpp,`
`328`	`329`	`return tl::unexpected(r);`
`329`	`330`	`}`
`330`	`331`
	`332`	`+ if (policies_) { // return policies to caller if requested`
	`333`	`+ *policies_ = policies;`
	`334`	`+ }`
`331`	`335`	`return transform_old_authinfo(info, std::move(account), std::move(policies));`
`332`	`336`	`}`
`333`	`337`