From: Soumya Koduri Date: Wed, 6 Mar 2019 12:28:32 +0000 (+0530) Subject: Validate bucket names as per revised s3 spec X-Git-Tag: v15.1.0~1857^2 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=eb6eddbe8dd96011fe4622b7658efd4ca7ba5f4b;p=ceph.git Validate bucket names as per revised s3 spec As per amazon s3 spec - https://docs.aws.amazon.com/AmazonS3/latest/dev/BucketRestrictions.html * The s3 bucket names should not contain upper case letters or underscore. * Name cannot end with dash or have consecutive periods, or dashes adjacent to periods. * Each label in the bucket name must start and end with a lowercase letter or a number. * Name cannot exceed 63 characters. This change is to enforce these rules if rgw_relaxed_s3_bucket_names is set to 'false' which is by default. Fixes: https://tracker.ceph.com/issues/36293 Signed-off-by: Soumya Koduri --- diff --git a/PendingReleaseNotes b/PendingReleaseNotes index 851dba5b81b..6f17a6250c0 100644 --- a/PendingReleaseNotes +++ b/PendingReleaseNotes @@ -60,6 +60,10 @@ objects and the other deletes them. Read the troubleshooting section of the dynamic resharding docs for details. +* RGW: Bucket naming restrictions have changed and likely to cause + InvalidBucketName errors. We recommend to set ``rgw_relaxed_s3_bucket_names`` + option to true as a workaround. + * In the Zabbix Mgr Module there was a typo in the key being send to Zabbix for PGs in backfill_wait state. The key that was sent was 'wait_backfill' and the correct name is 'backfill_wait'. diff --git a/doc/radosgw/s3/bucketops.rst b/doc/radosgw/s3/bucketops.rst index 0ebfc7fbb10..ffa14e8950d 100644 --- a/doc/radosgw/s3/bucketops.rst +++ b/doc/radosgw/s3/bucketops.rst @@ -12,8 +12,13 @@ Constraints In general, bucket names should follow domain name constraints. - Bucket names must be unique. -- Bucket names must begin and end with a lowercase letter. -- Bucket names may contain a dash (-). +- Bucket names cannot be formatted as IP address. +- Bucket names can be between 3 and 63 characters long. +- Bucket names must not contain uppercase characters or underscores. +- Bucket names must start with a lowercase letter or number. +- Bucket names must be a series of one or more labels. Adjacent labels are separated by a single period (.). Bucket names can contain lowercase letters, numbers, and hyphens. Each label must start and end with a lowercase letter or a number. + +.. note:: The above constraints are relaxed if the option 'rgw_relaxed_s3_bucket_names' is set to true except that the bucket names must still be unique, cannot be formatted as IP address and can contain letters, numbers, periods, dashes and underscores for up to 255 characters long. Syntax ~~~~~~ diff --git a/qa/suites/rgw/multifs/tasks/rgw_bucket_quota.yaml b/qa/suites/rgw/multifs/tasks/rgw_bucket_quota.yaml index c518d0e14ca..eb4a67ea0aa 100644 --- a/qa/suites/rgw/multifs/tasks/rgw_bucket_quota.yaml +++ b/qa/suites/rgw/multifs/tasks/rgw_bucket_quota.yaml @@ -8,3 +8,8 @@ tasks: clients: client.0: - rgw/s3_bucket_quota.pl +overrides: + ceph: + conf: + client: + rgw relaxed s3 bucket names: true diff --git a/qa/suites/rgw/multifs/tasks/rgw_multipart_upload.yaml b/qa/suites/rgw/multifs/tasks/rgw_multipart_upload.yaml index b042aa80c65..f7e561f0731 100644 --- a/qa/suites/rgw/multifs/tasks/rgw_multipart_upload.yaml +++ b/qa/suites/rgw/multifs/tasks/rgw_multipart_upload.yaml @@ -8,3 +8,8 @@ tasks: clients: client.0: - rgw/s3_multipart_upload.pl +overrides: + ceph: + conf: + client: + rgw relaxed s3 bucket names: true diff --git a/qa/suites/rgw/multifs/tasks/rgw_user_quota.yaml b/qa/suites/rgw/multifs/tasks/rgw_user_quota.yaml index ef9d6df110e..e661ff33d10 100644 --- a/qa/suites/rgw/multifs/tasks/rgw_user_quota.yaml +++ b/qa/suites/rgw/multifs/tasks/rgw_user_quota.yaml @@ -8,3 +8,8 @@ tasks: clients: client.0: - rgw/s3_user_quota.pl +overrides: + ceph: + conf: + client: + rgw relaxed s3 bucket names: true diff --git a/qa/suites/rgw/thrash/workload/rgw_bucket_quota.yaml b/qa/suites/rgw/thrash/workload/rgw_bucket_quota.yaml index 32e6af59f91..eef9ca3ec65 100644 --- a/qa/suites/rgw/thrash/workload/rgw_bucket_quota.yaml +++ b/qa/suites/rgw/thrash/workload/rgw_bucket_quota.yaml @@ -5,3 +5,8 @@ tasks: clients: client.0: - rgw/s3_bucket_quota.pl +overrides: + ceph: + conf: + client: + rgw relaxed s3 bucket names: true diff --git a/qa/suites/rgw/thrash/workload/rgw_multipart_upload.yaml b/qa/suites/rgw/thrash/workload/rgw_multipart_upload.yaml index b792336d5b5..f364f2d23bd 100644 --- a/qa/suites/rgw/thrash/workload/rgw_multipart_upload.yaml +++ b/qa/suites/rgw/thrash/workload/rgw_multipart_upload.yaml @@ -5,3 +5,8 @@ tasks: clients: client.0: - rgw/s3_multipart_upload.pl +overrides: + ceph: + conf: + client: + rgw relaxed s3 bucket names: true diff --git a/qa/suites/rgw/thrash/workload/rgw_user_quota.yaml b/qa/suites/rgw/thrash/workload/rgw_user_quota.yaml index 0a9888276fe..670ac0c170a 100644 --- a/qa/suites/rgw/thrash/workload/rgw_user_quota.yaml +++ b/qa/suites/rgw/thrash/workload/rgw_user_quota.yaml @@ -5,3 +5,8 @@ tasks: clients: client.0: - rgw/s3_user_quota.pl +overrides: + ceph: + conf: + client: + rgw relaxed s3 bucket names: true diff --git a/qa/tasks/radosgw_admin_rest.py b/qa/tasks/radosgw_admin_rest.py index 12d3ac046cf..5d8bf18687b 100644 --- a/qa/tasks/radosgw_admin_rest.py +++ b/qa/tasks/radosgw_admin_rest.py @@ -549,7 +549,7 @@ def task(ctx, config): assert out['usage']['rgw.main']['num_objects'] == 0 # create a bucket for deletion stats - useless_bucket = connection.create_bucket('useless_bucket') + useless_bucket = connection.create_bucket('useless-bucket') useless_key = useless_bucket.new_key('useless_key') useless_key.set_contents_from_string('useless string') diff --git a/src/rgw/rgw_rest_s3.cc b/src/rgw/rgw_rest_s3.cc index 95896f20e3f..e306a3cfacf 100644 --- a/src/rgw/rgw_rest_s3.cc +++ b/src/rgw/rgw_rest_s3.cc @@ -1547,8 +1547,13 @@ public: int RGWCreateBucket_ObjStore_S3::get_params() { RGWAccessControlPolicy_S3 s3policy(s->cct); + bool relaxed_names = s->cct->_conf->rgw_relaxed_s3_bucket_names; + + int r = valid_s3_bucket_name(s->bucket_name, relaxed_names); + if (r) + return r; - int r = create_s3_policy(s, store, s3policy, s->owner); + r = create_s3_policy(s, store, s3policy, s->owner); if (r < 0) return r; @@ -3907,7 +3912,6 @@ static int verify_mfa(RGWRados *store, RGWUserInfo *user, const string& mfa_str, int RGWHandler_REST_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); @@ -3920,9 +3924,6 @@ int RGWHandler_REST_S3::postauth_init() if (ret) return ret; if (!s->bucket_name.empty()) { - ret = valid_s3_bucket_name(s->bucket_name, relaxed_names); - if (ret) - return ret; ret = validate_object_name(s->object.name); if (ret) return ret; @@ -3934,9 +3935,6 @@ int RGWHandler_REST_S3::postauth_init() ret = rgw_validate_tenant_name(s->src_tenant_name); if (ret) return ret; - ret = valid_s3_bucket_name(s->src_bucket_name, relaxed_names); - if (ret) - return ret; } const char *mfa = s->info.env->get("HTTP_X_AMZ_MFA"); @@ -3957,11 +3955,7 @@ int RGWHandler_REST_S3::init(RGWRados *store, struct req_state *s, ret = rgw_validate_tenant_name(s->bucket_tenant); if (ret) return ret; - bool relaxed_names = s->cct->_conf->rgw_relaxed_s3_bucket_names; if (!s->bucket_name.empty()) { - ret = valid_s3_bucket_name(s->bucket_name, relaxed_names); - if (ret) - return ret; ret = validate_object_name(s->object.name); if (ret) return ret; diff --git a/src/rgw/rgw_rest_s3.h b/src/rgw/rgw_rest_s3.h index d87d69c7f32..f2e66673e77 100644 --- a/src/rgw/rgw_rest_s3.h +++ b/src/rgw/rgw_rest_s3.h @@ -747,15 +747,17 @@ static inline int valid_s3_bucket_name(const string& name, bool relaxed=false) // This function enforces Amazon's spec for bucket names. // (The requirements, not the recommendations.) int len = name.size(); + int max = (relaxed ? 255 : 63); + if (len < 3) { // Name too short return -ERR_INVALID_BUCKET_NAME; - } else if (len > 255) { + } else if (len > max) { // Name too long return -ERR_INVALID_BUCKET_NAME; } - // bucket names must start with a number, letter, or underscore + // bucket names must start with a number or letter if (!(isalpha(name[0]) || isdigit(name[0]))) { if (!relaxed) return -ERR_INVALID_BUCKET_NAME; @@ -763,14 +765,44 @@ static inline int valid_s3_bucket_name(const string& name, bool relaxed=false) return -ERR_INVALID_BUCKET_NAME; } + // bucket names must end with a number or letter + if (!(isalpha(name[len-1]) || isdigit(name[len-1]))) + if (!relaxed) + return -ERR_INVALID_BUCKET_NAME; + for (const char *s = name.c_str(); *s; ++s) { char c = *s; - if (isdigit(c) || (c == '.')) - continue; - if (isalpha(c)) + if (isdigit(c)) continue; - if ((c == '-') || (c == '_')) + + if (isalpha(c)) { + // name cannot contain uppercase letters + if (relaxed || islower(c)) + continue; + } + + if (c == '_') + // name cannot contain underscore + if (relaxed) + continue; + + if (c == '-') continue; + + if (c == '.') { + if (!relaxed && s && *s) { + // name cannot have consecutive periods or dashes + // adjacent to periods + // ensure s is neither the first nor the last character + char p = *(s-1); + char n = *(s+1); + if ((p != '-') && (n != '.') && (n != '-')) + continue; + } else { + continue; + } + } + // Invalid character return -ERR_INVALID_BUCKET_NAME; }