From 82f28a8fe3d1a8e1133d9b1d2db436a318f82f90 Mon Sep 17 00:00:00 2001 From: Pete Zaitcev Date: Wed, 6 Jan 2016 00:02:11 -0700 Subject: [PATCH] rgw: fix up the use of tenant before it's available A big problem that cropped up with the multi-tenancy as it went in was that it used tenant to initialize bucket names before tenant was actually available by parsing a token. Thus, although multi-tenancy worked, accessing anything across tenants was actually impossible. This patch fixes that oversight. Unfortunately, a new method postauth_init() was needed for it. We also fix small bugs: - validate_tenant_name() used a proper-looking C++ but was incorrect. We simply use C for now, which may be not as pretty but is correct. - We now make a distinction between empty tenant syntax that means "the logged-in tenant" ("bucket") and the syntax that means "the legacy compatibility tenant with no name (":bucket"). - We make rgw_make_bucket_entry_name() more robust against empty inputs. It's not used for work, but produces odd-looking debugging logs. Signed-off-by: Pete Zaitcev --- src/rgw/rgw_bucket.cc | 21 ++++++--- src/rgw/rgw_bucket.h | 3 +- src/rgw/rgw_common.h | 10 ++++- src/rgw/rgw_http_errors.h | 1 + src/rgw/rgw_main.cc | 10 ++++- src/rgw/rgw_op.h | 1 + src/rgw/rgw_rest.cc | 14 +++--- src/rgw/rgw_rest.h | 1 + src/rgw/rgw_rest_s3.cc | 78 +++++++++++++++++---------------- src/rgw/rgw_rest_s3.h | 6 ++- src/rgw/rgw_rest_swift.cc | 91 +++++++++++++++++++++++---------------- src/rgw/rgw_rest_swift.h | 3 +- src/rgw/rgw_swift_auth.h | 1 + 13 files changed, 148 insertions(+), 92 deletions(-) diff --git a/src/rgw/rgw_bucket.cc b/src/rgw/rgw_bucket.cc index 248dc83f8152a..3e4e85435e7ce 100644 --- a/src/rgw/rgw_bucket.cc +++ b/src/rgw/rgw_bucket.cc @@ -46,7 +46,9 @@ void rgw_get_buckets_obj(const rgw_user& user_id, string& buckets_obj_id) * with the legacy or S3 buckets. */ void rgw_make_bucket_entry_name(const string& tenant_name, const string& bucket_name, string& bucket_entry) { - if (tenant_name.empty()) { + if (bucket_name.empty()) { + bucket_entry.clear(); + } else if (tenant_name.empty()) { bucket_entry = bucket_name; } else { bucket_entry = tenant_name + "/" + bucket_name; @@ -56,7 +58,9 @@ void rgw_make_bucket_entry_name(const string& tenant_name, const string& bucket_ string rgw_make_bucket_entry_name(const string& tenant_name, const string& bucket_name) { string bucket_entry; - if (tenant_name.empty()) { + if (bucket_name.empty()) { + bucket_entry.clear(); + } else if (tenant_name.empty()) { bucket_entry = bucket_name; } else { bucket_entry = tenant_name + "/" + bucket_name; @@ -69,15 +73,22 @@ string rgw_make_bucket_entry_name(const string& tenant_name, const string& bucke * Tenants are separated from buckets in URLs by a colon in S3. * This function is not to be used on Swift URLs, not even for COPY arguments. */ -void rgw_parse_url_bucket(const string &bucket, +void rgw_parse_url_bucket(const string &bucket, const string& auth_tenant, string &tenant_name, string &bucket_name) { + int pos = bucket.find(':'); if (pos >= 0) { + /* + * N.B.: We allow ":bucket" syntax with explicit empty tenant in order + * to refer to the legacy tenant, in case users in new named tenants + * want to access old global buckets. + */ tenant_name = bucket.substr(0, pos); + bucket_name = bucket.substr(pos + 1); } else { - tenant_name.clear(); + tenant_name = auth_tenant; + bucket_name = bucket; } - bucket_name = bucket.substr(pos + 1); } /** diff --git a/src/rgw/rgw_bucket.h b/src/rgw/rgw_bucket.h index a905010e9a1a4..61f5c6218b05b 100644 --- a/src/rgw/rgw_bucket.h +++ b/src/rgw/rgw_bucket.h @@ -49,7 +49,8 @@ extern void rgw_make_bucket_entry_name(const string& tenant_name, string& bucket_entry); extern string rgw_make_bucket_entry_name(const string& tenant_name, const string& bucket_name); -extern void rgw_parse_url_bucket(const string &bucket, +extern void rgw_parse_url_bucket(const string& bucket, + const string& auth_tenant, string &tenant_name, string &bucket_name); /** diff --git a/src/rgw/rgw_common.h b/src/rgw/rgw_common.h index 59893527d03b4..585facd1079dc 100644 --- a/src/rgw/rgw_common.h +++ b/src/rgw/rgw_common.h @@ -161,6 +161,7 @@ using ceph::crypto::MD5; #define ERR_INVALID_SECRET_KEY 2034 #define ERR_INVALID_KEY_TYPE 2035 #define ERR_INVALID_CAP 2036 +#define ERR_INVALID_TENANT_NAME 2037 #define ERR_USER_SUSPENDED 2100 #define ERR_INTERNAL_ERROR 2200 #define ERR_NOT_IMPLEMENTED 2201 @@ -1034,6 +1035,12 @@ inline ostream& operator<<(ostream& out, const rgw_obj_key &o) { } } +struct req_init_state { + /* Keeps [[tenant]:]bucket until we parse the token. */ + string url_bucket; + string src_bucket; +}; + /** Store all the state necessary to complete and respond to an HTTP request*/ struct req_state { CephContext *cct; @@ -1057,7 +1064,7 @@ struct req_state { uint32_t perm_mask; utime_t header_time; - /* Set once when req_state is initialized and not violated thereafter */ + /* Set once when url_bucket is parsed and not violated thereafter. */ string bucket_tenant; string bucket_name; @@ -1106,6 +1113,7 @@ struct req_state { string trans_id; req_info info; + req_init_state init_state; req_state(CephContext *_cct, class RGWEnv *e); ~req_state(); diff --git a/src/rgw/rgw_http_errors.h b/src/rgw/rgw_http_errors.h index fbe805f41de2f..31b41b573c9b6 100644 --- a/src/rgw/rgw_http_errors.h +++ b/src/rgw/rgw_http_errors.h @@ -56,6 +56,7 @@ const static struct rgw_http_errors RGW_HTTP_ERRORS[] = { { ERR_INVALID_SECRET_KEY, 400, "InvalidSecretKey"}, { ERR_INVALID_KEY_TYPE, 400, "InvalidKeyType"}, { ERR_INVALID_CAP, 400, "InvalidCapability"}, + { ERR_INVALID_TENANT_NAME, 400, "InvalidTenantName" }, { ENOTEMPTY, 409, "BucketNotEmpty" }, { ERR_PRECONDITION_FAILED, 412, "PreconditionFailed" }, { ERANGE, 416, "InvalidRange" }, diff --git a/src/rgw/rgw_main.cc b/src/rgw/rgw_main.cc index a19b462f3eb52..3a4ad478e9332 100644 --- a/src/rgw/rgw_main.cc +++ b/src/rgw/rgw_main.cc @@ -573,7 +573,7 @@ static int process_request(RGWRados *store, RGWREST *rest, RGWRequest *req, RGWC should_log = mgr->get_logging(); - req->log(s, "getting op"); + req->log_format(s, "getting op %d", s->op); op = handler->get_op(store); if (!op) { abort_early(s, NULL, -ERR_METHOD_NOT_ALLOWED); @@ -589,6 +589,14 @@ static int process_request(RGWRados *store, RGWREST *rest, RGWRequest *req, RGWC goto done; } + req->log(s, "normalizing buckets and tenants"); + ret = handler->postauth_init(); + if (ret < 0) { + dout(10) << "failed to run post-auth init" << dendl; + abort_early(s, op, ret); + goto done; + } + if (s->user.suspended) { dout(10) << "user is suspended, uid=" << s->user.user_id << dendl; abort_early(s, op, -ERR_USER_SUSPENDED); diff --git a/src/rgw/rgw_op.h b/src/rgw/rgw_op.h index 880c634e016ed..7051e79f96f79 100644 --- a/src/rgw/rgw_op.h +++ b/src/rgw/rgw_op.h @@ -1304,6 +1304,7 @@ public: virtual void put_op(RGWOp *op); virtual int read_permissions(RGWOp *op) = 0; virtual int authorize() = 0; + virtual int postauth_init() = 0; }; #endif diff --git a/src/rgw/rgw_rest.cc b/src/rgw/rgw_rest.cc index d7f6b6072f718..9d84add1e4580 100644 --- a/src/rgw/rgw_rest.cc +++ b/src/rgw/rgw_rest.cc @@ -1203,13 +1203,13 @@ int RGWHandler_ObjStore::allocate_formatter(struct req_state *s, int default_typ int RGWHandler_ObjStore::validate_tenant_name(string const& t) { - struct tench { - static bool is_good(char ch) { - return isalnum(ch) || ch == '_'; - } - }; - std::string::const_iterator it = std::find_if(t.begin(), t.end(), tench::is_good); - return (it == t.end())? 0: -ERR_INVALID_BUCKET_NAME; + const char *p = t.c_str(); + for (int i = 0; i < t.size(); i++) { + char ch = p[i]; + if (!(isalnum(ch) || ch == '_')) + return -ERR_INVALID_TENANT_NAME; + } + return 0; } // This function enforces Amazon's spec for bucket names. diff --git a/src/rgw/rgw_rest.h b/src/rgw/rgw_rest.h index 4ba3c96f09ab6..188022db64667 100644 --- a/src/rgw/rgw_rest.h +++ b/src/rgw/rgw_rest.h @@ -359,6 +359,7 @@ public: int read_permissions(RGWOp *op); virtual int authorize() = 0; + // virtual int postauth_init(struct req_init_state *t) = 0; }; class RGWHandler_ObjStore_SWIFT; diff --git a/src/rgw/rgw_rest_s3.cc b/src/rgw/rgw_rest_s3.cc index 8c00e192c03fd..5ea703c429298 100644 --- a/src/rgw/rgw_rest_s3.cc +++ b/src/rgw/rgw_rest_s3.cc @@ -2206,7 +2206,7 @@ RGWOp *RGWHandler_ObjStore_Obj_S3::op_put() if (is_acl_op()) { return new RGWPutACLs_ObjStore_S3; } - if (s->src_bucket_name.empty()) + if (s->init_state.src_bucket.empty()) return new RGWPutObj_ObjStore_S3; else return new RGWCopyObj_ObjStore_S3; @@ -2286,12 +2286,9 @@ int RGWHandler_ObjStore_S3::init_from_header(struct req_state *s, int default_fo * the bucket (and its tenant) from DNS and Host: header (HTTP_HOST) * into req_status.bucket_name directly. */ - if (s->bucket_name.empty()) { - rgw_parse_url_bucket(first, s->bucket_tenant, s->bucket_name); - if (s->bucket_tenant.empty()) - s->bucket_tenant = s->user.user_id.tenant; - - ldout(s->cct, 20) << "s->user.user_id=" << s->user.user_id << " s->bucket_tenant=" << s->bucket_tenant << " s->bucket_name=" << s->bucket_name << dendl; + if (s->init_state.url_bucket.empty()) { + // Save bucket to tide us over until token is parsed. + s->init_state.url_bucket = first; if (pos >= 0) { string encoded_obj_str = req.substr(pos+1); @@ -2303,6 +2300,39 @@ int RGWHandler_ObjStore_S3::init_from_header(struct req_state *s, int default_fo return 0; } +int RGWHandler_ObjStore_S3::postauth_init() +{ + struct req_init_state *t = &s->init_state; + bool relaxed_names = s->cct->_conf->rgw_relaxed_s3_bucket_names; + + rgw_parse_url_bucket(t->url_bucket, s->user.user_id.tenant, s->bucket_tenant, s->bucket_name); + + dout(10) << "s->object=" << (!s->object.empty() ? s->object : rgw_obj_key("")) + << " s->bucket=" << rgw_make_bucket_entry_name(s->bucket_tenant, s->bucket_name) << dendl; + + int ret; + ret = validate_tenant_name(s->bucket_tenant); + if (ret) + return ret; + ret = validate_bucket_name(s->bucket_name, relaxed_names); + if (ret) + return ret; + ret = validate_object_name(s->object.name); + if (ret) + return ret; + + if (!t->src_bucket.empty()) { + rgw_parse_url_bucket(t->src_bucket, s->user.user_id.tenant, s->src_tenant_name, s->src_bucket_name); + ret = validate_tenant_name(s->src_tenant_name); + if (ret) + return ret; + ret = validate_bucket_name(s->src_bucket_name, relaxed_names); + if (ret) + return ret; + } + return 0; +} + static bool looks_like_ip_address(const char *bucket) { int num_periods = 0; @@ -2363,20 +2393,9 @@ int RGWHandler_ObjStore_S3::validate_bucket_name(const string& bucket, bool rela int RGWHandler_ObjStore_S3::init(RGWRados *store, struct req_state *s, RGWClientIO *cio) { - dout(10) << "s->object=" << (!s->object.empty() ? s->object : rgw_obj_key("")) - << " s->bucket=" << rgw_make_bucket_entry_name(s->bucket_tenant, s->bucket_name) << dendl; - int ret; - ret = validate_tenant_name(s->bucket_tenant); - if (ret) - return ret; - bool relaxed_names = s->cct->_conf->rgw_relaxed_s3_bucket_names; - ret = validate_bucket_name(s->bucket_name, relaxed_names); - if (ret) - return ret; - ret = validate_object_name(s->object.name); - if (ret) - return ret; + + s->dialect = "s3"; const char *cacl = s->info.env->get("HTTP_X_AMZ_ACL"); if (cacl) @@ -2386,19 +2405,13 @@ int RGWHandler_ObjStore_S3::init(RGWRados *store, struct req_state *s, RGWClient const char *copy_source = s->info.env->get("HTTP_X_AMZ_COPY_SOURCE"); if (copy_source) { - string src_bucket_str; - ret = RGWCopyObj::parse_copy_location(copy_source, src_bucket_str, s->src_object); + ret = RGWCopyObj::parse_copy_location(copy_source, s->init_state.src_bucket, s->src_object); if (!ret) { ldout(s->cct, 0) << "failed to parse copy location" << dendl; return -EINVAL; // XXX why not -ERR_INVALID_BUCKET_NAME or -ERR_BAD_URL? } - rgw_parse_url_bucket(src_bucket_str, s->src_tenant_name, s->src_bucket_name); - if (s->src_tenant_name.empty()) - s->src_tenant_name = s->user.user_id.tenant; } - s->dialect = "s3"; - return RGWHandler_ObjStore::init(store, s, cio); } @@ -2565,10 +2578,6 @@ int RGW_Auth_S3::authorize(RGWRados *store, struct req_state *s) } s->perm_mask = RGW_PERM_FULL_CONTROL; - - if (s->bucket_tenant.empty()) { - s->bucket_tenant = s->user.user_id.tenant; - } } } } @@ -2586,10 +2595,6 @@ int RGW_Auth_S3::authorize(RGWRados *store, struct req_state *s) return -ERR_INVALID_ACCESS_KEY; } - if (s->bucket_tenant.empty()) { - s->bucket_tenant = s->user.user_id.tenant; - } - /* now verify signature */ string auth_hdr; @@ -2662,7 +2667,6 @@ int RGW_Auth_S3::authorize(RGWRados *store, struct req_state *s) s->owner.set_id(s->user.user_id); s->owner.set_name(s->user.display_name); - return 0; } @@ -2681,7 +2685,7 @@ RGWHandler *RGWRESTMgr_S3::get_handler(struct req_state *s) if (ret < 0) return NULL; - if (s->bucket_name.empty()) + if (s->init_state.url_bucket.empty()) return new RGWHandler_ObjStore_Service_S3; if (s->object.empty()) diff --git a/src/rgw/rgw_rest_s3.h b/src/rgw/rgw_rest_s3.h index 1c2d5290f3e90..e3b6030e6f408 100644 --- a/src/rgw/rgw_rest_s3.h +++ b/src/rgw/rgw_rest_s3.h @@ -375,10 +375,11 @@ public: virtual int validate_object_name(const string& bucket) { return 0; } - virtual int init(RGWRados *store, struct req_state *state, RGWClientIO *cio); + virtual int init(RGWRados *store, struct req_state *s, RGWClientIO *cio); virtual int authorize() { return RGW_Auth_S3::authorize(store, s); } + int postauth_init() { return 0; } }; class RGWHandler_ObjStore_S3 : public RGWHandler_ObjStore { @@ -392,10 +393,11 @@ public: int validate_bucket_name(const string& bucket, bool relaxed_names); using RGWHandler_ObjStore::validate_bucket_name; - virtual int init(RGWRados *store, struct req_state *state, RGWClientIO *cio); + virtual int init(RGWRados *store, struct req_state *s, RGWClientIO *cio); virtual int authorize() { return RGW_Auth_S3::authorize(store, s); } + int postauth_init(); }; class RGWHandler_ObjStore_Service_S3 : public RGWHandler_ObjStore_S3 { diff --git a/src/rgw/rgw_rest_swift.cc b/src/rgw/rgw_rest_swift.cc index b170272bdea6a..54c06e40d11da 100644 --- a/src/rgw/rgw_rest_swift.cc +++ b/src/rgw/rgw_rest_swift.cc @@ -893,7 +893,6 @@ int RGWCopyObj_ObjStore_SWIFT::get_params() if_match = s->info.env->get("HTTP_COPY_IF_MATCH"); if_nomatch = s->info.env->get("HTTP_COPY_IF_NONE_MATCH"); - /* XXX why copy this? just use req_state in rgw_op.cc:verify_permission */ src_tenant_name = s->src_tenant_name; src_bucket_name = s->src_bucket_name; src_object = s->src_object; @@ -1231,7 +1230,7 @@ RGWOp *RGWHandler_ObjStore_Obj_SWIFT::op_put() if (is_acl_op()) { return new RGWPutACLs_ObjStore_SWIFT; } - if (s->src_bucket_name.empty()) + if (s->init_state.src_bucket.empty()) return new RGWPutObj_ObjStore_SWIFT; else return new RGWCopyObj_ObjStore_SWIFT; @@ -1274,6 +1273,49 @@ int RGWHandler_ObjStore_SWIFT::authorize() return 0; } +int RGWHandler_ObjStore_SWIFT::postauth_init() +{ + struct req_init_state *t = &s->init_state; + + /* XXX Stub this until Swift Auth sets account into URL. */ + s->bucket_tenant = s->user.user_id.tenant; + s->bucket_name = t->url_bucket; + + dout(10) << "s->object=" << (!s->object.empty() ? s->object : rgw_obj_key("")) + << " s->bucket=" << rgw_make_bucket_entry_name(s->bucket_tenant, s->bucket_name) << dendl; + + int ret; + ret = validate_tenant_name(s->bucket_tenant); + if (ret) + return ret; + ret = validate_bucket_name(s->bucket_name); + if (ret) + return ret; + ret = validate_object_name(s->object.name); + if (ret) + return ret; + + if (!t->src_bucket.empty()) { + /* + * We don't allow cross-tenant copy at present. It requires account + * names in the URL for Swift. + */ + s->src_tenant_name = s->user.user_id.tenant; + s->src_bucket_name = t->src_bucket; + + ret = validate_bucket_name(s->src_bucket_name); + if (ret < 0) { + return ret; + } + ret = validate_object_name(s->src_object.name); + if (ret < 0) { + return ret; + } + } + + return 0; +} + int RGWHandler_ObjStore_SWIFT::validate_bucket_name(const string& bucket) { int ret = RGWHandler_ObjStore::validate_bucket_name(bucket); @@ -1399,9 +1441,8 @@ int RGWHandler_ObjStore_SWIFT::init_from_header(struct req_state *s) s->info.effective_uri = "/" + first; - /* XXX Temporarily not parsing URL until Auth puts something in there. */ - s->bucket_tenant = s->user.user_id.tenant; - s->bucket_name = first; + // Save bucket to tide us over until token is parsed. + s->init_state.url_bucket = first; if (req.size()) { s->object = rgw_obj_key(req, s->info.env->get("HTTP_X_OBJECT_VERSION_ID", "")); /* rgw swift extension */ @@ -1413,59 +1454,34 @@ int RGWHandler_ObjStore_SWIFT::init_from_header(struct req_state *s) int RGWHandler_ObjStore_SWIFT::init(RGWRados *store, struct req_state *s, RGWClientIO *cio) { - dout(10) << "s->object=" << (!s->object.empty() ? s->object : rgw_obj_key("")) - << " s->bucket=" << rgw_make_bucket_entry_name(s->bucket_tenant, s->bucket_name) << dendl; + struct req_init_state *t = &s->init_state; - int ret; - ret = validate_tenant_name(s->bucket_tenant); - if (ret) - return ret; - ret = validate_bucket_name(s->bucket_name); - if (ret) - return ret; - ret = validate_object_name(s->object.name); - if (ret) - return ret; + s->dialect = "swift"; const char *copy_source = s->info.env->get("HTTP_X_COPY_FROM"); if (copy_source) { - bool result = RGWCopyObj::parse_copy_location(copy_source, s->src_bucket_name, s->src_object); + bool result = RGWCopyObj::parse_copy_location(copy_source, t->src_bucket, s->src_object); if (!result) return -ERR_BAD_URL; - s->src_tenant_name = s->user.user_id.tenant; } - s->dialect = "swift"; - if (s->op == OP_COPY) { const char *req_dest = s->info.env->get("HTTP_DESTINATION"); if (!req_dest) return -ERR_BAD_URL; - string dest_tenant_name, dest_bucket_name; + string dest_bucket_name; rgw_obj_key dest_obj_key; bool result = RGWCopyObj::parse_copy_location(req_dest, dest_bucket_name, dest_obj_key); if (!result) return -ERR_BAD_URL; - dest_tenant_name = s->user.user_id.tenant; string dest_object = dest_obj_key.name; - if (dest_bucket_name != s->bucket_name) { - ret = validate_bucket_name(dest_bucket_name); - if (ret < 0) - return ret; - } - - ret = validate_tenant_name(dest_tenant_name); - if (ret < 0) - return ret; /* convert COPY operation into PUT */ - s->src_tenant_name = s->bucket_tenant; - s->src_bucket_name = s->bucket_name; + t->src_bucket = t->url_bucket; s->src_object = s->object; - s->bucket_tenant = dest_tenant_name; - s->bucket_name = dest_bucket_name; + t->url_bucket = dest_bucket_name; s->object = rgw_obj_key(dest_object); s->op = OP_PUT; } @@ -1480,8 +1496,9 @@ RGWHandler *RGWRESTMgr_SWIFT::get_handler(struct req_state *s) if (ret < 0) return NULL; - if (s->bucket_name.empty()) + if (s->init_state.url_bucket.empty()) return new RGWHandler_ObjStore_Service_SWIFT; + if (s->object.empty()) return new RGWHandler_ObjStore_Bucket_SWIFT; diff --git a/src/rgw/rgw_rest_swift.h b/src/rgw/rgw_rest_swift.h index a64c9966d9935..1d483b37cda6d 100644 --- a/src/rgw/rgw_rest_swift.h +++ b/src/rgw/rgw_rest_swift.h @@ -192,8 +192,9 @@ public: int validate_bucket_name(const string& bucket); - int init(RGWRados *store, struct req_state *state, RGWClientIO *cio); + int init(RGWRados *store, struct req_state *s, RGWClientIO *cio); int authorize(); + int postauth_init(); RGWAccessControlPolicy *alloc_policy() { return NULL; /* return new RGWAccessControlPolicy_SWIFT; */ } void free_policy(RGWAccessControlPolicy *policy) { delete policy; } diff --git a/src/rgw/rgw_swift_auth.h b/src/rgw/rgw_swift_auth.h index 2fe5d344f1fe8..6b19d04523d6a 100644 --- a/src/rgw/rgw_swift_auth.h +++ b/src/rgw/rgw_swift_auth.h @@ -29,6 +29,7 @@ public: int init(RGWRados *store, struct req_state *state, RGWClientIO *cio); int authorize(); + int postauth_init() { return 0; } int read_permissions(RGWOp *op) { return 0; } virtual RGWAccessControlPolicy *alloc_policy() { return NULL; } -- 2.39.5