From 8818a0cb5e699135976e057061fb8e9d99850cd3 Mon Sep 17 00:00:00 2001 From: "Adam C. Emerson" Date: Fri, 27 Oct 2017 20:17:52 -0400 Subject: [PATCH] rgw: Refactor checking of some ops Since some operations check the user against the bucket owner in the absence of a policy, rather than open-coding that everywhere, act like a proper computer scientist and abstract it. Fixes: http://tracker.ceph.com/issues/21896 Signed-off-by: Adam C. Emerson --- src/rgw/rgw_common.cc | 15 ++++++ src/rgw/rgw_common.h | 2 + src/rgw/rgw_op.cc | 116 +++++------------------------------------- 3 files changed, 30 insertions(+), 103 deletions(-) diff --git a/src/rgw/rgw_common.cc b/src/rgw/rgw_common.cc index 49d675876bd..216b56051d1 100644 --- a/src/rgw/rgw_common.cc +++ b/src/rgw/rgw_common.cc @@ -1152,6 +1152,21 @@ bool verify_bucket_permission(struct req_state * const s, const uint64_t op) op); } +int verify_bucket_owner_or_policy(struct req_state* const s, + const uint64_t op) +{ + if (s->iam_policy) { + if (s->iam_policy->eval(s->env, *s->auth.identity, op, + ARN(s->bucket)) == Effect::Allow) { + return 0; + } + } else if (s->auth.identity->is_owner_of(s->bucket_owner.get_id())) { + return 0; + } + return -EACCES; +} + + static inline bool check_deferred_bucket_perms(struct req_state * const s, const rgw_bucket& bucket, RGWAccessControlPolicy * const user_acl, diff --git a/src/rgw/rgw_common.h b/src/rgw/rgw_common.h index 4c1583fc1a3..1a7b87aded0 100644 --- a/src/rgw/rgw_common.h +++ b/src/rgw/rgw_common.h @@ -2261,6 +2261,8 @@ bool verify_bucket_permission_no_policy( const int perm); bool verify_bucket_permission_no_policy(struct req_state * const s, const int perm); +int verify_bucket_owner_or_policy(struct req_state* const s, + const uint64_t op); extern bool verify_object_permission( struct req_state * const s, const rgw_obj& obj, diff --git a/src/rgw/rgw_op.cc b/src/rgw/rgw_op.cc index ca9a6f40f70..0e901526edc 100644 --- a/src/rgw/rgw_op.cc +++ b/src/rgw/rgw_op.cc @@ -2024,16 +2024,7 @@ void RGWStatAccount::execute() int RGWGetBucketVersioning::verify_permission() { - if (s->iam_policy) { - if (s->iam_policy->eval(s->env, *s->auth.identity, - rgw::IAM::s3GetBucketVersioning, - ARN(s->bucket)) == Effect::Allow) { - return 0; - } - } else if (s->auth.identity->is_owner_of(s->bucket_owner.get_id())) { - return 0; - } - return -EACCES; + return verify_bucket_owner_or_policy(s, rgw::IAM::s3GetBucketVersioning); } void RGWGetBucketVersioning::pre_exec() @@ -2049,16 +2040,7 @@ void RGWGetBucketVersioning::execute() int RGWSetBucketVersioning::verify_permission() { - if (s->iam_policy) { - if (s->iam_policy->eval(s->env, *s->auth.identity, - rgw::IAM::s3PutBucketVersioning, - ARN(s->bucket)) == Effect::Allow) { - return 0; - } - } else if (s->auth.identity->is_owner_of(s->bucket_owner.get_id())) { - return 0; - } - return -EACCES; + return verify_bucket_owner_or_policy(s, rgw::IAM::s3PutBucketVersioning); } void RGWSetBucketVersioning::pre_exec() @@ -2100,17 +2082,7 @@ void RGWSetBucketVersioning::execute() int RGWGetBucketWebsite::verify_permission() { - if (s->iam_policy) { - if (s->iam_policy->eval(s->env, *s->auth.identity, - rgw::IAM::s3GetBucketWebsite, - ARN(s->bucket)) == Effect::Allow) { - return 0; - } - } else if (s->auth.identity->is_owner_of(s->bucket_owner.get_id())) { - return 0; - } - - return -EACCES; + return verify_bucket_owner_or_policy(s, rgw::IAM::s3GetBucketWebsite); } void RGWGetBucketWebsite::pre_exec() @@ -2127,17 +2099,7 @@ void RGWGetBucketWebsite::execute() int RGWSetBucketWebsite::verify_permission() { - if (s->iam_policy) { - if (s->iam_policy->eval(s->env, *s->auth.identity, - rgw::IAM::s3PutBucketWebsite, - ARN(s->bucket)) == Effect::Allow) { - return 0; - } - } else if (s->auth.identity->is_owner_of(s->bucket_owner.get_id())) { - return 0; - } - - return -EACCES; + return verify_bucket_owner_or_policy(s, rgw::IAM::s3PutBucketWebsite); } void RGWSetBucketWebsite::pre_exec() @@ -2172,10 +2134,7 @@ void RGWSetBucketWebsite::execute() int RGWDeleteBucketWebsite::verify_permission() { - if (s->user->user_id.compare(s->bucket_owner.get_id()) != 0) - return -EACCES; - - return 0; + return verify_bucket_owner_or_policy(s, rgw::IAM::s3DeleteBucketWebsite); } void RGWDeleteBucketWebsite::pre_exec() @@ -2326,25 +2285,12 @@ void RGWListBucket::execute() int RGWGetBucketLogging::verify_permission() { - if (false == s->auth.identity->is_owner_of(s->bucket_owner.get_id())) { - return -EACCES; - } - - return 0; + return verify_bucket_owner_or_policy(s, rgw::IAM::s3GetBucketLogging); } int RGWGetBucketLocation::verify_permission() { - if (s->iam_policy) { - if (s->iam_policy->eval(s->env, *s->auth.identity, - rgw::IAM::s3GetBucketLocation, - ARN(s->bucket)) == Effect::Allow) { - return 0; - } - } else if (s->auth.identity->is_owner_of(s->bucket_owner.get_id())) { - return 0; - } - return -EACCES; + return verify_bucket_owner_or_policy(s, rgw::IAM::s3GetBucketLocation); } int RGWCreateBucket::verify_permission() @@ -4971,16 +4917,7 @@ void RGWDeleteLC::execute() int RGWGetCORS::verify_permission() { - if (s->iam_policy) { - if (s->iam_policy->eval(s->env, *s->auth.identity, - rgw::IAM::s3PutBucketCORS, - ARN(s->bucket)) == Effect::Allow) { - return 0; - } - } else if (s->auth.identity->is_owner_of(s->bucket_owner.get_id())) { - return 0; - } - return -EACCES; + return verify_bucket_owner_or_policy(s, rgw::IAM::s3GetBucketCORS); } void RGWGetCORS::execute() @@ -4998,16 +4935,7 @@ void RGWGetCORS::execute() int RGWPutCORS::verify_permission() { - if (s->iam_policy) { - if (s->iam_policy->eval(s->env, *s->auth.identity, - rgw::IAM::s3PutBucketCORS, - ARN(s->bucket)) == Effect::Allow) { - return 0; - } - } else if (s->auth.identity->is_owner_of(s->bucket_owner.get_id())) { - return 0; - } - return -EACCES; + return verify_bucket_owner_or_policy(s, rgw::IAM::s3PutBucketCORS); } void RGWPutCORS::execute() @@ -5033,11 +4961,8 @@ void RGWPutCORS::execute() int RGWDeleteCORS::verify_permission() { - if (false == s->auth.identity->is_owner_of(s->bucket_owner.get_id())) { - return -EACCES; - } - - return 0; + // No separate delete permission + return verify_bucket_owner_or_policy(s, rgw::IAM::s3PutBucketCORS); } void RGWDeleteCORS::execute() @@ -5130,13 +5055,7 @@ void RGWOptionsCORS::execute() int RGWGetRequestPayment::verify_permission() { - if (s->iam_policy && - s->iam_policy->eval(s->env, *s->auth.identity, - rgw::IAM::s3GetBucketRequestPayment, - ARN(s->bucket)) != Effect::Allow) { - return -EACCES; - } - return 0; + return verify_bucket_owner_or_policy(s, rgw::IAM::s3GetBucketRequestPayment); } void RGWGetRequestPayment::pre_exec() @@ -5151,16 +5070,7 @@ void RGWGetRequestPayment::execute() int RGWSetRequestPayment::verify_permission() { - if (s->iam_policy) { - if (s->iam_policy->eval(s->env, *s->auth.identity, - rgw::IAM::s3PutBucketRequestPayment, - ARN(s->bucket)) == Effect::Allow) { - return 0; - } - } else if (s->auth.identity->is_owner_of(s->bucket_owner.get_id())) { - return 0; - } - return -EACCES; + return verify_bucket_owner_or_policy(s, rgw::IAM::s3PutBucketRequestPayment); } void RGWSetRequestPayment::pre_exec() -- 2.39.5