From cbc6fef433067a7ce3077bec191b2fef7fd372d5 Mon Sep 17 00:00:00 2001 From: Casey Bodley Date: Wed, 21 Feb 2024 18:51:44 -0500 Subject: [PATCH] rgw/auth: replace uses of verify_bucket_owner_or_policy() all of the s3 actions that we call verify_bucket_owner_or_policy() for are already covered by rgw::IAM::op_to_perm(), which maps actions to acl permissions like RGW_PERM_READ, RGW_PERM_WRITE_ACP etc that means we can call verify_bucket_permission() as most other bucket ops do, and rely on its call to verify_bucket_permission_no_policy() to find the owner's acl grant i also hadn't implemented the cross-account rules for verify_bucket_owner_or_policy() yet, and didn't want to Signed-off-by: Casey Bodley (cherry picked from commit b021d0f2f133da6ac9e5972b481094d86802e979) --- src/rgw/rgw_common.cc | 24 ---------- src/rgw/rgw_common.h | 2 - src/rgw/rgw_op.cc | 109 +++++++++++++++++++++++++++++++++++------- 3 files changed, 91 insertions(+), 44 deletions(-) diff --git a/src/rgw/rgw_common.cc b/src/rgw/rgw_common.cc index 0e030c114964b..8ba0e044b1a09 100644 --- a/src/rgw/rgw_common.cc +++ b/src/rgw/rgw_common.cc @@ -1450,30 +1450,6 @@ bool verify_bucket_permission(const DoutPrefixProvider* dpp, req_state * const s s->iam_policy, s->iam_identity_policies, s->session_policies, op); } -// Authorize anyone permitted by the bucket policy, identity policies, session policies and the bucket owner -// unless explicitly denied by the policy. - -int verify_bucket_owner_or_policy(const DoutPrefixProvider* dpp, - req_state* const s, const uint64_t op) -{ - constexpr bool account_root = false; // just match owner below - const auto arn = ARN(s->bucket->get_key()); - const auto effect = evaluate_iam_policies( - dpp, s->env, *s->auth.identity, account_root, op, arn, - s->iam_policy, s->iam_identity_policies, s->session_policies); - if (effect == Effect::Deny) { - return -EACCES; - } - if (effect == Effect::Allow) { - return 0; - } - if (s->auth.identity->is_owner_of(s->bucket_owner.id)) { - ldpp_dout(dpp, 10) << __func__ << ": granted to bucket owner" << dendl; - return 0; - } - return -EACCES; -} - static inline bool check_deferred_bucket_only_acl(const DoutPrefixProvider* dpp, struct perm_state_base * const s, diff --git a/src/rgw/rgw_common.h b/src/rgw/rgw_common.h index 6d581b25e372c..f19c90901cc7e 100644 --- a/src/rgw/rgw_common.h +++ b/src/rgw/rgw_common.h @@ -1808,8 +1808,6 @@ bool verify_bucket_permission_no_policy( bool verify_bucket_permission_no_policy(const DoutPrefixProvider* dpp, req_state * const s, const int perm); -int verify_bucket_owner_or_policy(const DoutPrefixProvider* dpp, - req_state* s, const uint64_t op); extern bool verify_object_permission( const DoutPrefixProvider* dpp, req_state * const s, diff --git a/src/rgw/rgw_op.cc b/src/rgw/rgw_op.cc index 464bba7831b7f..d08fee5aa46a1 100644 --- a/src/rgw/rgw_op.cc +++ b/src/rgw/rgw_op.cc @@ -1288,7 +1288,11 @@ int RGWPutBucketTags::verify_permission(optional_yield y) { if (has_s3_resource_tag) rgw_iam_add_buckettags(this, s); - return verify_bucket_owner_or_policy(this, s, rgw::IAM::s3PutBucketTagging); + if (!verify_bucket_permission(this, s, rgw::IAM::s3PutBucketTagging)) { + return -EACCES; + } + + return 0; } void RGWPutBucketTags::execute(optional_yield y) @@ -1324,7 +1328,11 @@ int RGWDeleteBucketTags::verify_permission(optional_yield y) if (has_s3_resource_tag) rgw_iam_add_buckettags(this, s); - return verify_bucket_owner_or_policy(this, s, rgw::IAM::s3PutBucketTagging); + if (!verify_bucket_permission(this, s, rgw::IAM::s3PutBucketTagging)) { + return -EACCES; + } + + return 0; } void RGWDeleteBucketTags::execute(optional_yield y) @@ -1376,7 +1384,12 @@ int RGWPutBucketReplication::verify_permission(optional_yield y) { auto [has_s3_existing_tag, has_s3_resource_tag] = rgw_check_policy_condition(this, s, false); if (has_s3_resource_tag) rgw_iam_add_buckettags(this, s); - return verify_bucket_owner_or_policy(this, s, rgw::IAM::s3PutReplicationConfiguration); + + if (!verify_bucket_permission(this, s, rgw::IAM::s3PutReplicationConfiguration)) { + return -EACCES; + } + + return 0; } void RGWPutBucketReplication::execute(optional_yield y) { @@ -1422,7 +1435,11 @@ int RGWDeleteBucketReplication::verify_permission(optional_yield y) if (has_s3_resource_tag) rgw_iam_add_buckettags(this, s); - return verify_bucket_owner_or_policy(this, s, rgw::IAM::s3DeleteReplicationConfiguration); + if (!verify_bucket_permission(this, s, rgw::IAM::s3DeleteReplicationConfiguration)) { + return -EACCES; + } + + return 0; } void RGWDeleteBucketReplication::execute(optional_yield y) @@ -2826,7 +2843,11 @@ int RGWGetBucketVersioning::verify_permission(optional_yield y) if (has_s3_resource_tag) rgw_iam_add_buckettags(this, s); - return verify_bucket_owner_or_policy(this, s, rgw::IAM::s3GetBucketVersioning); + if (!verify_bucket_permission(this, s, rgw::IAM::s3GetBucketVersioning)) { + return -EACCES; + } + + return 0; } void RGWGetBucketVersioning::pre_exec() @@ -2852,7 +2873,11 @@ int RGWSetBucketVersioning::verify_permission(optional_yield y) if (has_s3_resource_tag) rgw_iam_add_buckettags(this, s); - return verify_bucket_owner_or_policy(this, s, rgw::IAM::s3PutBucketVersioning); + if (!verify_bucket_permission(this, s, rgw::IAM::s3PutBucketVersioning)) { + return -EACCES; + } + + return 0; } void RGWSetBucketVersioning::pre_exec() @@ -2951,7 +2976,11 @@ int RGWGetBucketWebsite::verify_permission(optional_yield y) if (has_s3_resource_tag) rgw_iam_add_buckettags(this, s); - return verify_bucket_owner_or_policy(this, s, rgw::IAM::s3GetBucketWebsite); + if (!verify_bucket_permission(this, s, rgw::IAM::s3GetBucketWebsite)) { + return -EACCES; + } + + return 0; } void RGWGetBucketWebsite::pre_exec() @@ -2972,7 +3001,11 @@ int RGWSetBucketWebsite::verify_permission(optional_yield y) if (has_s3_resource_tag) rgw_iam_add_buckettags(this, s); - return verify_bucket_owner_or_policy(this, s, rgw::IAM::s3PutBucketWebsite); + if (!verify_bucket_permission(this, s, rgw::IAM::s3PutBucketWebsite)) { + return -EACCES; + } + + return 0; } void RGWSetBucketWebsite::pre_exec() @@ -3019,7 +3052,11 @@ int RGWDeleteBucketWebsite::verify_permission(optional_yield y) if (has_s3_resource_tag) rgw_iam_add_buckettags(this, s); - return verify_bucket_owner_or_policy(this, s, rgw::IAM::s3DeleteBucketWebsite); + if (!verify_bucket_permission(this, s, rgw::IAM::s3DeleteBucketWebsite)) { + return -EACCES; + } + + return 0; } void RGWDeleteBucketWebsite::pre_exec() @@ -3200,7 +3237,11 @@ int RGWGetBucketLogging::verify_permission(optional_yield y) if (has_s3_resource_tag) rgw_iam_add_buckettags(this, s); - return verify_bucket_owner_or_policy(this, s, rgw::IAM::s3GetBucketLogging); + if (!verify_bucket_permission(this, s, rgw::IAM::s3GetBucketLogging)) { + return -EACCES; + } + + return 0; } int RGWGetBucketLocation::verify_permission(optional_yield y) @@ -3209,7 +3250,11 @@ int RGWGetBucketLocation::verify_permission(optional_yield y) if (has_s3_resource_tag) rgw_iam_add_buckettags(this, s); - return verify_bucket_owner_or_policy(this, s, rgw::IAM::s3GetBucketLocation); + if (!verify_bucket_permission(this, s, rgw::IAM::s3GetBucketLocation)) { + return -EACCES; + } + + return 0; } static int get_account_max_buckets(const DoutPrefixProvider* dpp, @@ -6339,7 +6384,11 @@ int RGWGetCORS::verify_permission(optional_yield y) if (has_s3_resource_tag) rgw_iam_add_buckettags(this, s); - return verify_bucket_owner_or_policy(this, s, rgw::IAM::s3GetBucketCORS); + if (!verify_bucket_permission(this, s, rgw::IAM::s3GetBucketCORS)) { + return -EACCES; + } + + return 0; } void RGWGetCORS::execute(optional_yield y) @@ -6361,7 +6410,11 @@ int RGWPutCORS::verify_permission(optional_yield y) if (has_s3_resource_tag) rgw_iam_add_buckettags(this, s); - return verify_bucket_owner_or_policy(this, s, rgw::IAM::s3PutBucketCORS); + if (!verify_bucket_permission(this, s, rgw::IAM::s3PutBucketCORS)) { + return -EACCES; + } + + return 0; } void RGWPutCORS::execute(optional_yield y) @@ -6393,7 +6446,11 @@ int RGWDeleteCORS::verify_permission(optional_yield y) rgw_iam_add_buckettags(this, s); // No separate delete permission - return verify_bucket_owner_or_policy(this, s, rgw::IAM::s3PutBucketCORS); + if (!verify_bucket_permission(this, s, rgw::IAM::s3PutBucketCORS)) { + return -EACCES; + } + + return 0; } void RGWDeleteCORS::execute(optional_yield y) @@ -6487,7 +6544,11 @@ int RGWGetRequestPayment::verify_permission(optional_yield y) if (has_s3_resource_tag) rgw_iam_add_buckettags(this, s); - return verify_bucket_owner_or_policy(this, s, rgw::IAM::s3GetBucketRequestPayment); + if (!verify_bucket_permission(this, s, rgw::IAM::s3GetBucketRequestPayment)) { + return -EACCES; + } + + return 0; } void RGWGetRequestPayment::pre_exec() @@ -6506,7 +6567,11 @@ int RGWSetRequestPayment::verify_permission(optional_yield y) if (has_s3_resource_tag) rgw_iam_add_buckettags(this, s); - return verify_bucket_owner_or_policy(this, s, rgw::IAM::s3PutBucketRequestPayment); + if (!verify_bucket_permission(this, s, rgw::IAM::s3PutBucketRequestPayment)) { + return -EACCES; + } + + return 0; } void RGWSetRequestPayment::pre_exec() @@ -8679,7 +8744,11 @@ int RGWPutBucketObjectLock::verify_permission(optional_yield y) if (has_s3_resource_tag) rgw_iam_add_buckettags(this, s); - return verify_bucket_owner_or_policy(this, s, rgw::IAM::s3PutBucketObjectLockConfiguration); + if (!verify_bucket_permission(this, s, rgw::IAM::s3PutBucketObjectLockConfiguration)) { + return -EACCES; + } + + return 0; } void RGWPutBucketObjectLock::execute(optional_yield y) @@ -8746,7 +8815,11 @@ int RGWGetBucketObjectLock::verify_permission(optional_yield y) if (has_s3_resource_tag) rgw_iam_add_buckettags(this, s); - return verify_bucket_owner_or_policy(this, s, rgw::IAM::s3GetBucketObjectLockConfiguration); + if (!verify_bucket_permission(this, s, rgw::IAM::s3GetBucketObjectLockConfiguration)) { + return -EACCES; + } + + return 0; } void RGWGetBucketObjectLock::execute(optional_yield y) -- 2.39.5