Skip to content

Commit b8a5917

Browse files
clwluvwclwluvw
committed
rgw: evaluate policies for dest object in data sync
Destination object policies are skipped, resulting in access denial when IAM policies grant access to the UID. This can be resolved by using verify_bucket_permission() instead of verify_bucket_permission_no_policy(). Fixes: https://tracker.ceph.com/issues/68884 Signed-off-by: Seena Fallah <[email protected]>
1 parent a3f40b4 commit b8a5917

File tree

6 files changed

+74
-55
lines changed

6 files changed

+74
-55
lines changed

‎src/rgw/driver/rados/rgw_data_sync.cc‎

Lines changed: 41 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -2617,6 +2617,7 @@ class RGWUserPermHandler {
26172617
rgw::IAM::Environment env;
26182618
std::unique_ptr<rgw::auth::Identity> identity;
26192619
RGWAccessControlPolicy user_acl;
2620+
std::vector<rgw::IAM::Policy> user_policies;
26202621
};
26212622

26222623
std::shared_ptr<_info> info;
@@ -2644,7 +2645,7 @@ class RGWUserPermHandler {
26442645
}
26452646

26462647
auto result = rgw::auth::transform_old_authinfo(
2647-
sync_env->dpp, null_yield, sync_env->driver, user.get());
2648+
sync_env->dpp, null_yield, sync_env->driver, user.get(), &info->user_policies);
26482649
if (!result) {
26492650
return result.error();
26502651
}
@@ -2679,16 +2680,15 @@ class RGWUserPermHandler {
26792680
std::shared_ptr<_info> info;
26802681
RGWAccessControlPolicy bucket_acl;
26812682
std::optional<perm_state> ps;
2683+
boost::optional<rgw::IAM::Policy> bucket_policy;
26822684
public:
26832685
Bucket() {}
26842686

26852687
int init(RGWUserPermHandler *handler,
26862688
const RGWBucketInfo& bucket_info,
26872689
const map<string, bufferlist>& bucket_attrs);
26882690

2689-
bool verify_bucket_permission(int perm);
2690-
bool verify_object_permission(const map<string, bufferlist>& obj_attrs,
2691-
int perm);
2691+
bool verify_bucket_permission(const rgw_obj_key& obj_key, const uint64_t op);
26922692
};
26932693

26942694
static int policy_from_attrs(CephContext *cct,
@@ -2728,6 +2728,14 @@ int RGWUserPermHandler::Bucket::init(RGWUserPermHandler *handler,
27282728
return r;
27292729
}
27302730

2731+
// load bucket policy
2732+
try {
2733+
bucket_policy = get_iam_policy_from_attr(sync_env->cct, bucket_attrs, bucket_info.bucket.tenant);
2734+
} catch (const std::exception& e) {
2735+
ldpp_dout(sync_env->dpp, 0) << "ERROR: reading IAM Policy: " << e.what() << dendl;
2736+
return -EACCES;
2737+
}
2738+
27312739
ps.emplace(sync_env->cct,
27322740
info->env,
27332741
info->identity.get(),
@@ -2740,30 +2748,35 @@ int RGWUserPermHandler::Bucket::init(RGWUserPermHandler *handler,
27402748
return 0;
27412749
}
27422750

2743-
bool RGWUserPermHandler::Bucket::verify_bucket_permission(int perm)
2744-
{
2745-
return verify_bucket_permission_no_policy(sync_env->dpp,
2746-
&(*ps),
2747-
info->user_acl,
2748-
bucket_acl,
2749-
perm);
2750-
}
2751-
2752-
bool RGWUserPermHandler::Bucket::verify_object_permission(const map<string, bufferlist>& obj_attrs,
2753-
int perm)
2751+
bool RGWUserPermHandler::Bucket::verify_bucket_permission(const rgw_obj_key& obj_key, const uint64_t op)
27542752
{
2755-
RGWAccessControlPolicy obj_acl;
2756-
2757-
int r = policy_from_attrs(sync_env->cct, obj_attrs, &obj_acl);
2758-
if (r < 0) {
2759-
return r;
2760-
}
2761-
2762-
return verify_bucket_permission_no_policy(sync_env->dpp,
2763-
&(*ps),
2764-
bucket_acl,
2765-
obj_acl,
2766-
perm);
2753+
const rgw_obj obj(ps->bucket_info.bucket, obj_key);
2754+
const auto arn = rgw::ARN(obj);
2755+
2756+
if (ps->identity->get_account()) {
2757+
const bool account_root = (ps->identity->get_identity_type() == TYPE_ROOT);
2758+
if (!ps->identity->is_owner_of(bucket_acl.get_owner().id)) {
2759+
ldpp_dout(sync_env->dpp, 4) << "cross-account request for bucket owner "
2760+
<< bucket_acl.get_owner().id << " != " << ps->identity->get_aclowner().id << dendl;
2761+
// cross-account requests evaluate the identity-based policies separately
2762+
// from the resource-based policies and require Allow from both
2763+
return ::verify_bucket_permission(sync_env->dpp, &(*ps), arn, account_root, {}, {}, {},
2764+
info->user_policies, {}, op)
2765+
&& ::verify_bucket_permission(sync_env->dpp, &(*ps), arn, false, info->user_acl,
2766+
bucket_acl, bucket_policy, {}, {}, op);
2767+
} else {
2768+
// don't consult acls for same-account access. require an Allow from
2769+
// either identity- or resource-based policy
2770+
return ::verify_bucket_permission(sync_env->dpp, &(*ps), arn, account_root, {}, {},
2771+
bucket_policy, info->user_policies,
2772+
{}, op);
2773+
}
2774+
}
2775+
constexpr bool account_root = false;
2776+
return ::verify_bucket_permission(sync_env->dpp, &(*ps), arn, account_root,
2777+
info->user_acl, bucket_acl,
2778+
bucket_policy, info->user_policies,
2779+
{}, op);
27672780
}
27682781

27692782
class RGWFetchObjFilter_Sync : public RGWFetchObjFilter_Default {
@@ -3006,7 +3019,7 @@ class RGWObjFetchCR : public RGWCoroutine {
30063019
return set_cr_error(retcode);
30073020
}
30083021

3009-
if (!dest_bucket_perms.verify_bucket_permission(RGW_PERM_WRITE)) {
3022+
if (!dest_bucket_perms.verify_bucket_permission(dest_key.value_or(key), rgw::IAM::s3PutObject)) {
30103023
ldout(cct, 0) << "ERROR: " << __func__ << ": permission check failed: user not allowed to write into bucket (bucket=" << sync_pipe.info.dest_bucket.get_key() << ")" << dendl;
30113024
return -EPERM;
30123025
}

‎src/rgw/rgw_auth.cc‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -313,7 +313,8 @@ static auto transform_old_authinfo(const RGWUserInfo& user,
313313
auto transform_old_authinfo(const DoutPrefixProvider* dpp,
314314
optional_yield y,
315315
sal::Driver* driver,
316-
sal::User* user)
316+
sal::User* user,
317+
std::vector<IAM::Policy>* policies_)
317318
-> tl::expected<std::unique_ptr<Identity>, int>
318319
{
319320
const RGWUserInfo& info = user->get_info();
@@ -328,6 +329,9 @@ auto transform_old_authinfo(const DoutPrefixProvider* dpp,
328329
return tl::unexpected(r);
329330
}
330331

332+
if (policies_) { // return policies to caller if requested
333+
*policies_ = policies;
334+
}
331335
return transform_old_authinfo(info, std::move(account), std::move(policies));
332336
}
333337

‎src/rgw/rgw_auth.h‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,8 @@ inline std::ostream& operator<<(std::ostream& out,
105105
auto transform_old_authinfo(const DoutPrefixProvider* dpp,
106106
optional_yield y,
107107
sal::Driver* driver,
108-
sal::User* user)
108+
sal::User* user,
109+
std::vector<IAM::Policy>* policies_ = nullptr)
109110
-> tl::expected<std::unique_ptr<Identity>, int>;
110111

111112
// Load the user account and all user/group policies. May throw

‎src/rgw/rgw_common.cc‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3204,3 +3204,14 @@ void RGWObjVersionTracker::generate_new_write_ver(CephContext *cct)
32043204
append_rand_alpha(cct, write_version.tag, write_version.tag, TAG_LEN);
32053205
}
32063206

3207+
boost::optional<rgw::IAM::Policy>
3208+
get_iam_policy_from_attr(CephContext* cct,
3209+
const std::map<std::string, bufferlist>& attrs,
3210+
const std::string& tenant)
3211+
{
3212+
if (auto i = attrs.find(RGW_ATTR_IAM_POLICY); i != attrs.end()) {
3213+
return Policy(cct, &tenant, i->second.to_str(), false);
3214+
} else {
3215+
return boost::none;
3216+
}
3217+
}

‎src/rgw/rgw_common.h‎

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1744,18 +1744,6 @@ rgw::IAM::Effect evaluate_iam_policies(
17441744
const std::vector<rgw::IAM::Policy>& identity_policies,
17451745
const std::vector<rgw::IAM::Policy>& session_policies);
17461746

1747-
bool verify_user_permission(const DoutPrefixProvider* dpp,
1748-
req_state * const s,
1749-
const RGWAccessControlPolicy& user_acl,
1750-
const std::vector<rgw::IAM::Policy>& user_policies,
1751-
const std::vector<rgw::IAM::Policy>& session_policies,
1752-
const rgw::ARN& res,
1753-
const uint64_t op,
1754-
bool mandatory_policy=true);
1755-
bool verify_user_permission_no_policy(const DoutPrefixProvider* dpp,
1756-
req_state * const s,
1757-
const RGWAccessControlPolicy& user_acl,
1758-
const int perm);
17591747
bool verify_user_permission(const DoutPrefixProvider* dpp,
17601748
req_state * const s,
17611749
const rgw::ARN& res,
@@ -1764,6 +1752,16 @@ bool verify_user_permission(const DoutPrefixProvider* dpp,
17641752
bool verify_user_permission_no_policy(const DoutPrefixProvider* dpp,
17651753
req_state * const s,
17661754
int perm);
1755+
bool verify_bucket_permission(const DoutPrefixProvider* dpp,
1756+
struct perm_state_base * const s,
1757+
const rgw::ARN& arn,
1758+
bool account_root,
1759+
const RGWAccessControlPolicy& user_acl,
1760+
const RGWAccessControlPolicy& bucket_acl,
1761+
const boost::optional<rgw::IAM::Policy>& bucket_policy,
1762+
const std::vector<rgw::IAM::Policy>& identity_policies,
1763+
const std::vector<rgw::IAM::Policy>& session_policies,
1764+
const uint64_t op);
17671765
bool verify_bucket_permission(
17681766
const DoutPrefixProvider* dpp,
17691767
req_state * const s,
@@ -2011,3 +2009,8 @@ struct AioCompletionDeleter {
20112009
void operator()(librados::AioCompletion* c) { c->release(); }
20122010
};
20132011
using aio_completion_ptr = std::unique_ptr<librados::AioCompletion, AioCompletionDeleter>;
2012+
2013+
extern boost::optional<rgw::IAM::Policy>
2014+
get_iam_policy_from_attr(CephContext* cct,
2015+
const std::map<std::string, bufferlist>& attrs,
2016+
const std::string& tenant);

‎src/rgw/rgw_op.cc‎

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -330,19 +330,6 @@ static int get_obj_policy_from_attr(const DoutPrefixProvider *dpp,
330330
return ret;
331331
}
332332

333-
334-
static boost::optional<Policy>
335-
get_iam_policy_from_attr(CephContext* cct,
336-
const map<string, bufferlist>& attrs,
337-
const string& tenant)
338-
{
339-
if (auto i = attrs.find(RGW_ATTR_IAM_POLICY); i != attrs.end()) {
340-
return Policy(cct, &tenant, i->second.to_str(), false);
341-
} else {
342-
return none;
343-
}
344-
}
345-
346333
static boost::optional<PublicAccessBlockConfiguration>
347334
get_public_access_conf_from_attr(const map<string, bufferlist>& attrs)
348335
{

0 commit comments

Comments
 (0)