From: Yehuda Sadeh Date: Sat, 5 Jan 2019 03:41:08 +0000 (-0800) Subject: rgw: fix check for no storage_class specified in obj copy X-Git-Tag: v14.1.0~314^2~17 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=53dfdc7e36fb756d0851030d0db0afa0e292057a;p=ceph.git rgw: fix check for no storage_class specified in obj copy S3 doesn't allow copying object into itself if it doesn't modify attributes. Need to check that the object's storage class changes but we only have that info later, so code rearranged a bit to allow that. Signed-off-by: Yehuda Sadeh --- diff --git a/src/rgw/rgw_op.cc b/src/rgw/rgw_op.cc index 8e2b77be7365..c85c52ee67cd 100644 --- a/src/rgw/rgw_op.cc +++ b/src/rgw/rgw_op.cc @@ -234,6 +234,7 @@ static int get_obj_policy_from_attr(CephContext *cct, RGWBucketInfo& bucket_info, map& bucket_attrs, RGWAccessControlPolicy *policy, + string *storage_class, rgw_obj& obj) { bufferlist bl; @@ -257,6 +258,17 @@ static int get_obj_policy_from_attr(CephContext *cct, policy->create_default(bucket_info.owner, uinfo.display_name); } + + if (storage_class) { + bufferlist scbl; + int r = rop.get_attr(RGW_ATTR_STORAGE_CLASS, scbl); + if (r >= 0) { + *storage_class = scbl.to_str(); + } else { + storage_class->clear(); + } + } + return ret; } @@ -473,6 +485,7 @@ static int read_obj_policy(RGWRados *store, RGWBucketInfo& bucket_info, map& bucket_attrs, RGWAccessControlPolicy* acl, + string *storage_class, boost::optional& policy, rgw_bucket& bucket, rgw_obj_key& object) @@ -500,7 +513,7 @@ static int read_obj_policy(RGWRados *store, RGWObjectCtx *obj_ctx = static_cast(s->obj_ctx); int ret = get_obj_policy_from_attr(s->cct, store, *obj_ctx, - bucket_info, bucket_attrs, acl, obj); + bucket_info, bucket_attrs, acl, storage_class, obj); if (ret == -ENOENT) { /* object does not exist checking the bucket's ACL to make sure that we send a proper error code */ @@ -759,7 +772,7 @@ int rgw_build_object_policies(RGWRados *store, struct req_state *s, store->set_prefetch_data(s->obj_ctx, obj); } ret = read_obj_policy(store, s, s->bucket_info, s->bucket_attrs, - s->object_acl.get(), s->iam_policy, s->bucket, + s->object_acl.get(), nullptr, s->iam_policy, s->bucket, s->object); } @@ -3305,7 +3318,7 @@ int RGWPutObj::verify_permission() store->set_prefetch_data(s->obj_ctx, obj); /* check source object permissions */ - if (read_obj_policy(store, s, copy_source_bucket_info, cs_attrs, &cs_acl, + if (read_obj_policy(store, s, copy_source_bucket_info, cs_attrs, &cs_acl, nullptr, policy, cs_bucket, cs_object) < 0) { return -EACCES; } @@ -4659,13 +4672,25 @@ int RGWCopyObj::verify_permission() store->set_atomic(s->obj_ctx, src_obj); store->set_prefetch_data(s->obj_ctx, src_obj); + rgw_placement_rule src_placement; + /* check source object permissions */ - op_ret = read_obj_policy(store, s, src_bucket_info, src_attrs, &src_acl, + op_ret = read_obj_policy(store, s, src_bucket_info, src_attrs, &src_acl, &src_placement.storage_class, src_policy, src_bucket, src_object); if (op_ret < 0) { return op_ret; } + /* follow up on previous checks that required reading source object head */ + if (need_to_check_storage_class) { + src_placement.inherit_from(src_bucket_info.placement_rule); + + op_ret = check_storage_class(src_placement); + if (op_ret < 0) { + return op_ret; + } + } + /* admin request overrides permission checks */ if (!s->auth.identity->is_admin_of(src_acl.get_owner().get_id())) { if (src_policy) { diff --git a/src/rgw/rgw_op.h b/src/rgw/rgw_op.h index 5d6a73d14a12..ca68dfd4efe6 100644 --- a/src/rgw/rgw_op.h +++ b/src/rgw/rgw_op.h @@ -1324,6 +1324,8 @@ protected: boost::optional delete_at; bool copy_if_newer; + bool need_to_check_storage_class = false; + int init_common(); public: @@ -1360,6 +1362,10 @@ public: void execute() override; void progress_cb(off_t ofs); + virtual int check_storage_class(const rgw_placement_rule& src_placement) { + return 0; + } + virtual int init_dest_policy() { return 0; } virtual int get_params() = 0; virtual void send_partial_response(off_t ofs) {} diff --git a/src/rgw/rgw_rest_s3.cc b/src/rgw/rgw_rest_s3.cc index 543a801857b3..2f87e9e64496 100644 --- a/src/rgw/rgw_rest_s3.cc +++ b/src/rgw/rgw_rest_s3.cc @@ -2198,12 +2198,20 @@ int RGWCopyObj_ObjStore_S3::get_params() (dest_bucket_name.compare(src_bucket_name) == 0) && (dest_object.compare(src_object.name) == 0) && src_object.instance.empty() && - s->info.storage_class.empty() && (attrs_mod != RGWRados::ATTRSMOD_REPLACE)) { + need_to_check_storage_class = true; + } + + return 0; +} + +int RGWCopyObj_ObjStore_S3::check_storage_class(const rgw_placement_rule& src_placement) +{ + if (src_placement == s->dest_placement) { /* can only copy object into itself if replacing attrs */ s->err.message = "This copy request is illegal because it is trying to copy " - "an object to itself without changing the object's metadata, " - "storage class, website redirect location or encryption attributes."; + "an object to itself without changing the object's metadata, " + "storage class, website redirect location or encryption attributes."; ldout(s->cct, 0) << s->err.message << dendl; return -ERR_INVALID_REQUEST; } diff --git a/src/rgw/rgw_rest_s3.h b/src/rgw/rgw_rest_s3.h index 3fb60f87550d..0eaff8f7855b 100644 --- a/src/rgw/rgw_rest_s3.h +++ b/src/rgw/rgw_rest_s3.h @@ -274,6 +274,7 @@ public: int init_dest_policy() override; int get_params() override; + int check_storage_class(const rgw_placement_rule& src_placement); void send_partial_response(off_t ofs) override; void send_response() override; };