From: Pritha Srivastava Date: Thu, 24 Jun 2021 12:43:28 +0000 (+0530) Subject: rgw/sts: fix read_obj_policy permission evaluation X-Git-Tag: v17.1.0~221^2 X-Git-Url: http://git.apps.os.sepia.ceph.com/?a=commitdiff_plain;h=59c46f28d1fc28c731cc9ef86336e8a9ead437a0;p=ceph-ci.git rgw/sts: fix read_obj_policy permission evaluation to pass in boost::none for the identity parameter as identity IAM policies do not have a Principal for evaluation. The Principal is the role or the identity to which the policy is attached. Also removing boost::optional id paremeter from eval_identity_or_session_policies in all places, since an identity or a session policy doesnt have a Principal element. The identity (user or role) or the session is implicitly the 'Principal' to which the policy is attached. fixes: https://tracker.ceph.com/issues/52302 Signed-off-by: Pritha Srivastava --- diff --git a/src/rgw/rgw_common.cc b/src/rgw/rgw_common.cc index 50e34f19549..18bd65ca599 100644 --- a/src/rgw/rgw_common.cc +++ b/src/rgw/rgw_common.cc @@ -1066,12 +1066,11 @@ Effect eval_or_pass(const boost::optional& policy, Effect eval_identity_or_session_policies(const vector& policies, const rgw::IAM::Environment& env, - boost::optional id, const uint64_t op, - const ARN& resource) { + const ARN& arn) { auto policy_res = Effect::Pass, prev_res = Effect::Pass; for (auto& policy : policies) { - if (policy_res = eval_or_pass(policy, env, id, op, resource); policy_res == Effect::Deny) + if (policy_res = eval_or_pass(policy, env, boost::none, op, arn); policy_res == Effect::Deny) return policy_res; else if (policy_res == Effect::Allow) prev_res = Effect::Allow; @@ -1089,13 +1088,13 @@ bool verify_user_permission(const DoutPrefixProvider* dpp, const rgw::ARN& res, const uint64_t op) { - auto identity_policy_res = eval_identity_or_session_policies(user_policies, s->env, boost::none, op, res); + auto identity_policy_res = eval_identity_or_session_policies(user_policies, s->env, op, res); if (identity_policy_res == Effect::Deny) { return false; } if (! session_policies.empty()) { - auto session_policy_res = eval_identity_or_session_policies(session_policies, s->env, boost::none, op, res); + auto session_policy_res = eval_identity_or_session_policies(session_policies, s->env, op, res); if (session_policy_res == Effect::Deny) { return false; } @@ -1187,7 +1186,7 @@ bool verify_bucket_permission(const DoutPrefixProvider* dpp, if (!verify_requester_payer_permission(s)) return false; - auto identity_policy_res = eval_identity_or_session_policies(identity_policies, s->env, boost::none, op, ARN(bucket)); + auto identity_policy_res = eval_identity_or_session_policies(identity_policies, s->env, op, ARN(bucket)); if (identity_policy_res == Effect::Deny) return false; @@ -1199,7 +1198,7 @@ bool verify_bucket_permission(const DoutPrefixProvider* dpp, //Take into account session policies, if the identity making a request is a role if (!session_policies.empty()) { - auto session_policy_res = eval_identity_or_session_policies(session_policies, s->env, boost::none, op, ARN(bucket)); + auto session_policy_res = eval_identity_or_session_policies(session_policies, s->env, op, ARN(bucket)); if (session_policy_res == Effect::Deny) { return false; } @@ -1317,7 +1316,7 @@ bool verify_bucket_permission(const DoutPrefixProvider* dpp, struct req_state * int verify_bucket_owner_or_policy(struct req_state* const s, const uint64_t op) { - auto identity_policy_res = eval_identity_or_session_policies(s->iam_user_policies, s->env, boost::none, op, ARN(s->bucket->get_key())); + auto identity_policy_res = eval_identity_or_session_policies(s->iam_user_policies, s->env, op, ARN(s->bucket->get_key())); if (identity_policy_res == Effect::Deny) { return -EACCES; } @@ -1331,7 +1330,7 @@ int verify_bucket_owner_or_policy(struct req_state* const s, } if (!s->session_policies.empty()) { - auto session_policy_res = eval_identity_or_session_policies(s->session_policies, s->env, boost::none, op, ARN(s->bucket->get_key())); + auto session_policy_res = eval_identity_or_session_policies(s->session_policies, s->env, op, ARN(s->bucket->get_key())); if (session_policy_res == Effect::Deny) { return -EACCES; } @@ -1402,7 +1401,7 @@ bool verify_object_permission(const DoutPrefixProvider* dpp, struct perm_state_b if (!verify_requester_payer_permission(s)) return false; - auto identity_policy_res = eval_identity_or_session_policies(identity_policies, s->env, boost::none, op, ARN(obj)); + auto identity_policy_res = eval_identity_or_session_policies(identity_policies, s->env, op, ARN(obj)); if (identity_policy_res == Effect::Deny) return false; @@ -1412,7 +1411,7 @@ bool verify_object_permission(const DoutPrefixProvider* dpp, struct perm_state_b return false; if (!session_policies.empty()) { - auto session_policy_res = eval_identity_or_session_policies(session_policies, s->env, boost::none, op, ARN(obj)); + auto session_policy_res = eval_identity_or_session_policies(session_policies, s->env, op, ARN(obj)); if (session_policy_res == Effect::Deny) { return false; } diff --git a/src/rgw/rgw_common.h b/src/rgw/rgw_common.h index eebf31e2dfe..d0e0244f94a 100644 --- a/src/rgw/rgw_common.h +++ b/src/rgw/rgw_common.h @@ -2125,7 +2125,6 @@ bool verify_object_permission_no_policy(const DoutPrefixProvider* dpp, * to do the requested action */ rgw::IAM::Effect eval_identity_or_session_policies(const std::vector& user_policies, const rgw::IAM::Environment& env, - boost::optional id, const uint64_t op, const rgw::ARN& arn); bool verify_user_permission(const DoutPrefixProvider* dpp, diff --git a/src/rgw/rgw_op.cc b/src/rgw/rgw_op.cc index 20ad0d985b8..e3e30ec6de2 100644 --- a/src/rgw/rgw_op.cc +++ b/src/rgw/rgw_op.cc @@ -411,8 +411,7 @@ static int read_obj_policy(const DoutPrefixProvider *dpp, if (bucket_owner.compare(s->user->get_id()) != 0 && ! s->auth.identity->is_admin_of(bucket_owner)) { auto r = eval_identity_or_session_policies(s->iam_user_policies, s->env, - *s->auth.identity, rgw::IAM::s3ListBucket, - ARN(bucket->get_key())); + rgw::IAM::s3ListBucket, ARN(bucket->get_key())); if (r == Effect::Allow) return -ENOENT; if (r == Effect::Deny) @@ -427,8 +426,7 @@ static int read_obj_policy(const DoutPrefixProvider *dpp, } if (! s->session_policies.empty()) { r = eval_identity_or_session_policies(s->session_policies, s->env, - *s->auth.identity, rgw::IAM::s3ListBucket, - ARN(bucket->get_key())); + rgw::IAM::s3ListBucket, ARN(bucket->get_key())); if (r == Effect::Allow) return -ENOENT; if (r == Effect::Deny) @@ -3647,7 +3645,6 @@ int RGWPutObj::verify_permission(optional_yield y) rgw_iam_add_buckettags(this, s); auto identity_policy_res = eval_identity_or_session_policies(s->iam_user_policies, s->env, - boost::none, rgw::IAM::s3PutObject, s->object->get_obj()); if (identity_policy_res == Effect::Deny) @@ -3668,7 +3665,6 @@ int RGWPutObj::verify_permission(optional_yield y) if (!s->session_policies.empty()) { auto session_policy_res = eval_identity_or_session_policies(s->session_policies, s->env, - boost::none, rgw::IAM::s3PutObject, s->object->get_obj()); if (session_policy_res == Effect::Deny) { @@ -4243,7 +4239,6 @@ void RGWPostObj::execute(optional_yield y) if (s->iam_policy || ! s->iam_user_policies.empty() || !s->session_policies.empty()) { auto identity_policy_res = eval_identity_or_session_policies(s->iam_user_policies, s->env, - boost::none, rgw::IAM::s3PutObject, s->object->get_obj()); if (identity_policy_res == Effect::Deny) { @@ -4267,7 +4262,6 @@ void RGWPostObj::execute(optional_yield y) if (!s->session_policies.empty()) { auto session_policy_res = eval_identity_or_session_policies(s->session_policies, s->env, - boost::none, rgw::IAM::s3PutObject, s->object->get_obj()); if (session_policy_res == Effect::Deny) { @@ -4823,7 +4817,7 @@ int RGWDeleteObj::verify_permission(optional_yield y) if (s->iam_policy || ! s->iam_user_policies.empty() || ! s->session_policies.empty()) { if (s->bucket->get_info().obj_lock_enabled() && bypass_governance_mode) { - auto r = eval_identity_or_session_policies(s->iam_user_policies, s->env, boost::none, + auto r = eval_identity_or_session_policies(s->iam_user_policies, s->env, rgw::IAM::s3BypassGovernanceRetention, ARN(s->bucket->get_key(), s->object->get_name())); if (r == Effect::Deny) { bypass_perm = false; @@ -4834,7 +4828,7 @@ int RGWDeleteObj::verify_permission(optional_yield y) bypass_perm = false; } } else if (r == Effect::Pass && !s->session_policies.empty()) { - r = eval_identity_or_session_policies(s->session_policies, s->env, boost::none, + r = eval_identity_or_session_policies(s->session_policies, s->env, rgw::IAM::s3BypassGovernanceRetention, ARN(s->bucket->get_key(), s->object->get_name())); if (r == Effect::Deny) { bypass_perm = false; @@ -4842,7 +4836,6 @@ int RGWDeleteObj::verify_permission(optional_yield y) } } auto identity_policy_res = eval_identity_or_session_policies(s->iam_user_policies, s->env, - boost::none, s->object->get_instance().empty() ? rgw::IAM::s3DeleteObject : rgw::IAM::s3DeleteObjectVersion, @@ -4867,7 +4860,6 @@ int RGWDeleteObj::verify_permission(optional_yield y) if (!s->session_policies.empty()) { auto session_policy_res = eval_identity_or_session_policies(s->session_policies, s->env, - boost::none, s->object->get_instance().empty() ? rgw::IAM::s3DeleteObject : rgw::IAM::s3DeleteObjectVersion, @@ -5162,7 +5154,6 @@ int RGWCopyObj::verify_permission(optional_yield y) ARN obj_arn(s->src_object->get_obj()); auto identity_policy_res = eval_identity_or_session_policies(s->iam_user_policies, s->env, - boost::none, s->src_object->get_instance().empty() ? rgw::IAM::s3GetObject : rgw::IAM::s3GetObjectVersion, @@ -5185,7 +5176,6 @@ int RGWCopyObj::verify_permission(optional_yield y) } if (!s->session_policies.empty()) { auto session_policy_res = eval_identity_or_session_policies(s->session_policies, s->env, - boost::none, s->src_object->get_instance().empty() ? rgw::IAM::s3GetObject : rgw::IAM::s3GetObjectVersion, @@ -5269,7 +5259,7 @@ int RGWCopyObj::verify_permission(optional_yield y) ARN obj_arn(dest_object->get_obj()); auto identity_policy_res = eval_identity_or_session_policies(s->iam_user_policies, - s->env, boost::none, + s->env, rgw::IAM::s3PutObject, obj_arn); if (identity_policy_res == Effect::Deny) { @@ -5287,7 +5277,7 @@ int RGWCopyObj::verify_permission(optional_yield y) return -EACCES; } if (!s->session_policies.empty()) { - auto session_policy_res = eval_identity_or_session_policies(s->session_policies, s->env, boost::none, rgw::IAM::s3PutObject, obj_arn); + auto session_policy_res = eval_identity_or_session_policies(s->session_policies, s->env, rgw::IAM::s3PutObject, obj_arn); if (session_policy_res == Effect::Deny) { return false; } @@ -6085,7 +6075,6 @@ int RGWInitMultipart::verify_permission(optional_yield y) if (s->iam_policy || ! s->iam_user_policies.empty() || !s->session_policies.empty()) { auto identity_policy_res = eval_identity_or_session_policies(s->iam_user_policies, s->env, - boost::none, rgw::IAM::s3PutObject, s->object->get_obj()); if (identity_policy_res == Effect::Deny) { @@ -6107,7 +6096,6 @@ int RGWInitMultipart::verify_permission(optional_yield y) if (!s->session_policies.empty()) { auto session_policy_res = eval_identity_or_session_policies(s->session_policies, s->env, - boost::none, rgw::IAM::s3PutObject, s->object->get_obj()); if (session_policy_res == Effect::Deny) { @@ -6194,7 +6182,6 @@ int RGWCompleteMultipart::verify_permission(optional_yield y) if (s->iam_policy || ! s->iam_user_policies.empty() || ! s->session_policies.empty()) { auto identity_policy_res = eval_identity_or_session_policies(s->iam_user_policies, s->env, - boost::none, rgw::IAM::s3PutObject, s->object->get_obj()); if (identity_policy_res == Effect::Deny) { @@ -6216,7 +6203,6 @@ int RGWCompleteMultipart::verify_permission(optional_yield y) if (!s->session_policies.empty()) { auto session_policy_res = eval_identity_or_session_policies(s->session_policies, s->env, - boost::none, rgw::IAM::s3PutObject, s->object->get_obj()); if (session_policy_res == Effect::Deny) { @@ -6445,7 +6431,6 @@ int RGWAbortMultipart::verify_permission(optional_yield y) if (s->iam_policy || ! s->iam_user_policies.empty() || !s->session_policies.empty()) { auto identity_policy_res = eval_identity_or_session_policies(s->iam_user_policies, s->env, - boost::none, rgw::IAM::s3AbortMultipartUpload, s->object->get_obj()); if (identity_policy_res == Effect::Deny) { @@ -6467,7 +6452,6 @@ int RGWAbortMultipart::verify_permission(optional_yield y) if (!s->session_policies.empty()) { auto session_policy_res = eval_identity_or_session_policies(s->session_policies, s->env, - boost::none, rgw::IAM::s3PutObject, s->object->get_obj()); if (session_policy_res == Effect::Deny) { @@ -6644,7 +6628,7 @@ int RGWDeleteMultiObj::verify_permission(optional_yield y) if (s->iam_policy || ! s->iam_user_policies.empty() || ! s->session_policies.empty()) { if (s->bucket->get_info().obj_lock_enabled() && bypass_governance_mode) { ARN bucket_arn(s->bucket->get_key()); - auto r = eval_identity_or_session_policies(s->iam_user_policies, s->env, boost::none, + auto r = eval_identity_or_session_policies(s->iam_user_policies, s->env, rgw::IAM::s3BypassGovernanceRetention, ARN(s->bucket->get_key())); if (r == Effect::Deny) { bypass_perm = false; @@ -6655,7 +6639,7 @@ int RGWDeleteMultiObj::verify_permission(optional_yield y) bypass_perm = false; } } else if (r == Effect::Pass && !s->session_policies.empty()) { - r = eval_identity_or_session_policies(s->session_policies, s->env, boost::none, + r = eval_identity_or_session_policies(s->session_policies, s->env, rgw::IAM::s3BypassGovernanceRetention, ARN(s->bucket->get_key())); if (r == Effect::Deny) { bypass_perm = false; @@ -6666,7 +6650,6 @@ int RGWDeleteMultiObj::verify_permission(optional_yield y) bool not_versioned = rgw::sal::Object::empty(s->object.get()) || s->object->get_instance().empty(); auto identity_policy_res = eval_identity_or_session_policies(s->iam_user_policies, s->env, - boost::none, not_versioned ? rgw::IAM::s3DeleteObject : rgw::IAM::s3DeleteObjectVersion, @@ -6691,7 +6674,6 @@ int RGWDeleteMultiObj::verify_permission(optional_yield y) if (!s->session_policies.empty()) { auto session_policy_res = eval_identity_or_session_policies(s->session_policies, s->env, - boost::none, not_versioned ? rgw::IAM::s3DeleteObject : rgw::IAM::s3DeleteObjectVersion, @@ -6804,7 +6786,6 @@ void RGWDeleteMultiObj::execute(optional_yield y) std::unique_ptr obj = bucket->get_object(*iter); if (s->iam_policy || ! s->iam_user_policies.empty() || !s->session_policies.empty()) { auto identity_policy_res = eval_identity_or_session_policies(s->iam_user_policies, s->env, - boost::none, iter->instance.empty() ? rgw::IAM::s3DeleteObject : rgw::IAM::s3DeleteObjectVersion, @@ -6833,7 +6814,6 @@ void RGWDeleteMultiObj::execute(optional_yield y) if (!s->session_policies.empty()) { auto session_policy_res = eval_identity_or_session_policies(s->session_policies, s->env, - boost::none, iter->instance.empty() ? rgw::IAM::s3DeleteObject : rgw::IAM::s3DeleteObjectVersion, @@ -7298,7 +7278,6 @@ bool RGWBulkUploadOp::handle_file_verify_permission(RGWBucketInfo& binfo, bucket_owner = bacl.get_owner(); if (policy || ! s->iam_user_policies.empty() || !s->session_policies.empty()) { auto identity_policy_res = eval_identity_or_session_policies(s->iam_user_policies, s->env, - boost::none, rgw::IAM::s3PutObject, obj); if (identity_policy_res == Effect::Deny) { return false; @@ -7314,7 +7293,6 @@ bool RGWBulkUploadOp::handle_file_verify_permission(RGWBucketInfo& binfo, if (!s->session_policies.empty()) { auto session_policy_res = eval_identity_or_session_policies(s->session_policies, s->env, - boost::none, rgw::IAM::s3PutObject, obj); if (session_policy_res == Effect::Deny) { return false;