From 3fb1671520d62ce707ebc15e8f7874540b7e2aaa Mon Sep 17 00:00:00 2001 From: Seena Fallah Date: Fri, 21 Feb 2025 00:57:25 +0100 Subject: [PATCH] rgw: support s3ReplicateTags perm on destination bucket for replication Check for tag replication permission on destination bucket, so if there was an explicit deny, donot include tags in the replicated object. Signed-off-by: Seena Fallah --- src/rgw/driver/rados/rgw_cr_rados.cc | 4 +++- src/rgw/driver/rados/rgw_cr_rados.h | 16 +++++++++++----- src/rgw/driver/rados/rgw_data_sync.cc | 26 ++++++++++++++++++++++++-- src/rgw/driver/rados/rgw_data_sync.h | 1 + src/rgw/driver/rados/rgw_rados.cc | 7 ++++++- src/rgw/driver/rados/rgw_rados.h | 3 ++- src/rgw/rgw_iam_policy.cc | 4 ++++ src/rgw/rgw_iam_policy.h | 2 ++ 8 files changed, 53 insertions(+), 10 deletions(-) diff --git a/src/rgw/driver/rados/rgw_cr_rados.cc b/src/rgw/driver/rados/rgw_cr_rados.cc index 80dc958157b..8bb30f0f3e5 100644 --- a/src/rgw/driver/rados/rgw_cr_rados.cc +++ b/src/rgw/driver/rados/rgw_cr_rados.cc @@ -12,6 +12,7 @@ #include "rgw_cr_rest.h" #include "rgw_rest_conn.h" #include "rgw_rados.h" +#include "rgw_data_sync.h" #include "services/svc_zone.h" #include "services/svc_zone_utils.h" @@ -843,7 +844,8 @@ int RGWAsyncFetchRemoteObj::_send_request(const DoutPrefixProvider *dpp) stat_dest_obj, source_trace_entry, &zones_trace, - &bytes_transferred); + &bytes_transferred, + keep_tags); if (r < 0) { ldpp_dout(dpp, 0) << "store->fetch_remote_obj() returned r=" << r << dendl; diff --git a/src/rgw/driver/rados/rgw_cr_rados.h b/src/rgw/driver/rados/rgw_cr_rados.h index 470849b1680..b3b02182229 100644 --- a/src/rgw/driver/rados/rgw_cr_rados.h +++ b/src/rgw/driver/rados/rgw_cr_rados.h @@ -8,6 +8,7 @@ #include "rgw_coroutine.h" #include "rgw_sal.h" #include "rgw_sal_rados.h" +#include "rgw_bucket_sync.h" #include "common/WorkQueue.h" #include "common/Throttle.h" @@ -1129,6 +1130,7 @@ class RGWAsyncFetchRemoteObj : public RGWAsyncRadosRequest { rgw_zone_set zones_trace; PerfCounters* counters; const DoutPrefixProvider *dpp; + bool keep_tags; protected: int _send_request(const DoutPrefixProvider *dpp) override; @@ -1148,7 +1150,8 @@ public: const rgw_zone_set_entry& source_trace_entry, rgw_zone_set *_zones_trace, PerfCounters* counters, - const DoutPrefixProvider *dpp) + const DoutPrefixProvider *dpp, + bool _keep_tags) : RGWAsyncRadosRequest(caller, cn), store(_store), source_zone(_source_zone), user_id(_user_id), @@ -1163,7 +1166,8 @@ public: stat_follow_olh(_stat_follow_olh), source_trace_entry(source_trace_entry), counters(counters), - dpp(dpp) + dpp(dpp), + keep_tags(_keep_tags) { if (_zones_trace) { zones_trace = *_zones_trace; @@ -1199,6 +1203,7 @@ class RGWFetchRemoteObjCR : public RGWSimpleCoroutine { rgw_zone_set *zones_trace; PerfCounters* counters; const DoutPrefixProvider *dpp; + bool keep_tags; public: RGWFetchRemoteObjCR(RGWAsyncRadosProcessor *_async_rados, rgw::sal::RadosStore* _store, @@ -1216,7 +1221,8 @@ public: const rgw_zone_set_entry& source_trace_entry, rgw_zone_set *_zones_trace, PerfCounters* counters, - const DoutPrefixProvider *dpp) + const DoutPrefixProvider *dpp, + bool _keep_tags) : RGWSimpleCoroutine(_store->ctx()), cct(_store->ctx()), async_rados(_async_rados), store(_store), source_zone(_source_zone), @@ -1232,7 +1238,7 @@ public: req(NULL), stat_follow_olh(_stat_follow_olh), source_trace_entry(source_trace_entry), - zones_trace(_zones_trace), counters(counters), dpp(dpp) {} + zones_trace(_zones_trace), counters(counters), dpp(dpp), keep_tags(_keep_tags) {} ~RGWFetchRemoteObjCR() override { @@ -1250,7 +1256,7 @@ public: req = new RGWAsyncFetchRemoteObj(this, stack->create_completion_notifier(), store, source_zone, user_id, src_bucket, dest_placement_rule, dest_bucket_info, key, dest_key, versioned_epoch, copy_if_newer, filter, - stat_follow_olh, source_trace_entry, zones_trace, counters, dpp); + stat_follow_olh, source_trace_entry, zones_trace, counters, dpp, keep_tags); async_rados->queue(req); return 0; } diff --git a/src/rgw/driver/rados/rgw_data_sync.cc b/src/rgw/driver/rados/rgw_data_sync.cc index 7523996d9a1..ec50d0a57ba 100644 --- a/src/rgw/driver/rados/rgw_data_sync.cc +++ b/src/rgw/driver/rados/rgw_data_sync.cc @@ -2732,6 +2732,22 @@ bool RGWUserPermHandler::Bucket::verify_bucket_permission(const rgw_obj_key& obj {}, op); } +rgw::IAM::Effect RGWUserPermHandler::Bucket::evaluate_iam_policies(const rgw_obj_key& obj_key, const uint64_t op) +{ + const rgw_obj obj(ps->bucket_info.bucket, obj_key); + const auto arn = rgw::ARN(obj); + const bool account_root = (ps->identity->get_identity_type() == TYPE_ROOT); + + return ::evaluate_iam_policies(dpp, + ps->env, + *ps->identity, + account_root, + op, arn, + bucket_policy, + info->user_policies, + {}); +} + int RGWUserPermHandler::policy_from_attrs(CephContext *cct, const map& attrs, RGWAccessControlPolicy *acl) { @@ -2877,6 +2893,7 @@ class RGWObjFetchCR : public RGWCoroutine { int try_num{0}; std::shared_ptr need_retry; + bool replicate_tags{true}; public: RGWObjFetchCR(RGWDataSyncCtx *_sc, rgw_bucket_sync_pipe& _sync_pipe, @@ -2971,7 +2988,7 @@ public: } user_perms.emplace(sync_env, *param_user); - yield call(user_perms->init_cr()); + yield call(user_perms->init_cr(sync_env)); if (retcode < 0) { ldout(cct, 0) << "ERROR: " << __func__ << ": failed to init user perms manager for uid=" << *param_user << dendl; return set_cr_error(retcode); @@ -2990,6 +3007,11 @@ public: 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 set_cr_error(-EPERM); } + + // only if there is an explicit deny, we should not replicate tags + // otherwise, s3:ReplicateObject checked above already includes the permission to replicate tags + replicate_tags = dest_bucket_perms.evaluate_iam_policies(dest_key.value_or(key), rgw::IAM::s3ReplicateTags) != rgw::IAM::Effect::Deny; + ldout(cct, 20) << "replicate_tags=" << replicate_tags << dendl; } yield { @@ -3009,7 +3031,7 @@ public: std::static_pointer_cast(filter), stat_follow_olh, source_trace_entry, zones_trace, - sync_env->counters, dpp)); + sync_env->counters, dpp, replicate_tags)); } if (retcode < 0) { if (*need_retry) { diff --git a/src/rgw/driver/rados/rgw_data_sync.h b/src/rgw/driver/rados/rgw_data_sync.h index 4f863c869d8..cc4f081ed40 100644 --- a/src/rgw/driver/rados/rgw_data_sync.h +++ b/src/rgw/driver/rados/rgw_data_sync.h @@ -912,6 +912,7 @@ public: const std::map& bucket_attrs); bool verify_bucket_permission(const rgw_obj_key& obj_key, const uint64_t op); + rgw::IAM::Effect evaluate_iam_policies(const rgw_obj_key& obj_key, const uint64_t op); }; static int policy_from_attrs(CephContext *cct, diff --git a/src/rgw/driver/rados/rgw_rados.cc b/src/rgw/driver/rados/rgw_rados.cc index a612cd8abde..8a38a6bebce 100644 --- a/src/rgw/driver/rados/rgw_rados.cc +++ b/src/rgw/driver/rados/rgw_rados.cc @@ -4319,7 +4319,8 @@ int RGWRados::fetch_remote_obj(RGWObjectCtx& dest_obj_ctx, const rgw_obj& stat_dest_obj, std::optional source_trace_entry, rgw_zone_set *zones_trace, - std::optional* bytes_transferred) + std::optional* bytes_transferred, + bool keep_tags) { /* source is in a different zonegroup, copy from there */ @@ -4600,6 +4601,10 @@ int RGWRados::fetch_remote_obj(RGWObjectCtx& dest_obj_ctx, attrs = cb.get_attrs(); } + if (!keep_tags) { + attrs.erase(RGW_ATTR_TAGS); + } + if (copy_if_newer) { uint64_t pg_ver = 0; auto i = attrs.find(RGW_ATTR_PG_VER); diff --git a/src/rgw/driver/rados/rgw_rados.h b/src/rgw/driver/rados/rgw_rados.h index a084083c577..8359a0bce1b 100644 --- a/src/rgw/driver/rados/rgw_rados.h +++ b/src/rgw/driver/rados/rgw_rados.h @@ -1179,7 +1179,8 @@ public: const rgw_obj& stat_dest_obj, std::optional source_trace_entry, rgw_zone_set *zones_trace = nullptr, - std::optional* bytes_transferred = 0); + std::optional* bytes_transferred = 0, + bool keep_tags = true); /** * Copy an object. * dest_obj: the object to copy into diff --git a/src/rgw/rgw_iam_policy.cc b/src/rgw/rgw_iam_policy.cc index fda7d68a0dd..8b7658ac074 100644 --- a/src/rgw/rgw_iam_policy.cc +++ b/src/rgw/rgw_iam_policy.cc @@ -140,6 +140,7 @@ static const actpair actpairs[] = { "s3:DescribeJob", s3DescribeJob }, { "s3:ReplicateDelete", s3ReplicateDelete }, { "s3:ReplicateObject", s3ReplicateObject }, + { "s3:ReplicateTags", s3ReplicateTags }, { "s3-object-lambda:GetObject", s3objectlambdaGetObject }, { "s3-object-lambda:ListBucket", s3objectlambdaListBucket }, { "iam:PutUserPolicy", iamPutUserPolicy }, @@ -1517,6 +1518,9 @@ const char* action_bit_string(uint64_t action) { case s3ReplicateObject: return "s3:ReplicateObject"; + case s3ReplicateTags: + return "s3:ReplicateTags"; + case s3objectlambdaGetObject: return "s3-object-lambda:GetObject"; diff --git a/src/rgw/rgw_iam_policy.h b/src/rgw/rgw_iam_policy.h index f7f9b38e5c3..b09acf09d38 100644 --- a/src/rgw/rgw_iam_policy.h +++ b/src/rgw/rgw_iam_policy.h @@ -119,6 +119,7 @@ enum { s3GetObjectVersionAttributes, s3ReplicateDelete, s3ReplicateObject, + s3ReplicateTags, s3All, s3objectlambdaGetObject, @@ -277,6 +278,7 @@ inline int op_to_perm(std::uint64_t op) { case s3BypassGovernanceRetention: case s3ReplicateDelete: case s3ReplicateObject: + case s3ReplicateTags: return RGW_PERM_WRITE; case s3GetAccelerateConfiguration: -- 2.39.5