[rgw]:Validate bucket names as per revised s3 spec #26787

soumyakoduri · 2019-03-06T12:43:30Z

As per amazon s3 spec -
https://docs.aws.amazon.com/awscloudtrail/latest/userguide/cloudtrail-s3-bucket-naming-requirements.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

This change is to enforce these rules if rgw_relaxed_s3_bucket_names is set to 'false'.

Fixes: https://tracker.ceph.com/issues/36293

Signed-off-by: Soumya Koduri skoduri@redhat.com

soumyakoduri · 2019-03-06T12:44:36Z

@mattbenjamin @pritha-srivastava @cbodley @yehudasa .. kindly review the changes.

stale · 2019-05-05T16:23:18Z

This pull request has been automatically marked as stale because it has not had any activity for 60 days. It will be closed if no further activity occurs for another 30 days.
If you are a maintainer or core committer, please follow-up on this pull request to identify what steps should be taken by the author to move this proposed change forward.
If you are the author of this pull request, thank you for your proposed contribution. If you believe this change is still appropriate, please ensure that any feedback has been addressed and ask for a code review.

src/rgw/rgw_rest_s3.h

mattbenjamin · 2019-05-06T13:17:24Z

unstale this

cbodley · 2019-05-06T17:27:58Z

we document our bucket name requirements in doc/radosgw/s3/bucketops.rst (see http://docs.ceph.com/docs/master/radosgw/s3/bucketops/#constraints) - could you please update that to reflect the changes? also worth mentioning which parts are conditional on rgw_relaxed_s3_bucket_names

soumyakoduri · 2019-05-06T17:51:06Z

we document our bucket name requirements in doc/radosgw/s3/bucketops.rst (see http://docs.ceph.com/docs/master/radosgw/s3/bucketops/#constraints) - could you please update that to reflect the changes? also worth mentioning which parts are conditional on rgw_relaxed_s3_bucket_names

Sure. Will make the changes.

soumyakoduri · 2019-05-06T18:22:25Z

@cbodley ..
Amazon spec states that bucket name can be between 3 and 63 characters long whereas in the code we currently seem to be allowing upto 255 characters. Do you suggest I limit the size too if the relaxed option is not set?

cbodley · 2019-05-06T18:28:30Z

yes please, we should limit to 63 as well

soumyakoduri · 2019-05-06T18:31:50Z

yes please, we should limit to 63 as well

Thanks. Will update.

cbodley · 2019-05-14T12:33:18Z

jenkins render docs

ceph-jenkins · 2019-05-14T13:34:58Z

Doc render available at http://docs.ceph.com/ceph-prs/26787/

doc/radosgw/s3/bucketops.rst

src/rgw/rgw_rest_s3.h

soumyakoduri · 2019-06-03T17:35:49Z

jenkins render docs

ceph-jenkins · 2019-06-03T17:58:16Z

Doc render available at http://docs.ceph.com/ceph-prs/26787/

cbodley · 2019-06-04T14:50:34Z

retest this please

cbodley · 2019-06-04T14:53:18Z

issue#36293

could you please replace this part of the commit message with:

Fixes: https://tracker.ceph.com/issues/36293

soumyakoduri · 2019-06-04T15:42:35Z

Fixes: https://tracker.ceph.com/issues/36293

Done.

soumyakoduri · 2019-07-04T10:04:59Z

@soumyakoduri one last piece of documentation that i'd like to see is in PendingReleaseNotes; we should make it clear that a) the bucket naming restrictions are changing, b) the changes are likely to cause InvalidBucketName errors, and c) that rgw_relaxed_s3_bucket_names=true is recommended as a workaround

Done. Please review.

monschein · 2019-07-05T15:43:27Z

Hi all. I've tested this PR locally and I see that it is missing one rule for bucket names. Per the AWS S3 specification:

Each label must start and end with a lowercase letter or a number.

https://docs.aws.amazon.com/AmazonS3/latest/dev/BucketRestrictions.html

$ s3cmd mb s3://nametest.
Bucket 's3://nametest./' created

I am able to create buckets that end with a period.

soumyakoduri · 2019-07-05T16:00:56Z

Hi all. I've tested this PR locally and I see that it is missing one rule for bucket names. Per the AWS S3 specification:

Each label must start and end with a lowercase letter or a number.

https://docs.aws.amazon.com/AmazonS3/latest/dev/BucketRestrictions.html
$ s3cmd mb s3://nametest.
Bucket 's3://nametest./' created
I am able to create buckets that end with a period.

Thanks David. I was referring to https://docs.aws.amazon.com/awscloudtrail/latest/userguide/cloudtrail-s3-bucket-naming-requirements.html while working on this. That page must not have been updated with this rule.

@cbodley .. please let me know your thoughts.

cbodley · 2019-07-08T19:37:26Z

good catch @monschein!

alimaredia · 2019-07-19T17:27:04Z

jenkins retest this please

robbat2 · 2019-07-31T19:33:00Z

@soumyakoduri One of my asking items is that RGW gets TWO toggles for bucket names: one toggle for strict bucket names on creation, and the other that still permits access to pre-existing buckets with names that can no longer be created. Can you confirm this is implemented in your PR?

cbodley · 2019-08-01T20:57:41Z

@robbat2 verifying that existing buckets with invalid names are still accessible is important, yeah. can you think of a reason, though, that an admin would want to deny access to existing buckets? what motivates that second toggle/config option?

robbat2 · 2019-08-01T22:14:47Z

@robbat2 verifying that existing buckets with invalid names are still accessible is important, yeah. can you think of a reason, though, that an admin would want to deny access to existing buckets? what motivates that second toggle/config option?

Specifically announcing a migration plan, and locking the old names at part of the enforcement stick.

soumyakoduri · 2019-08-05T09:22:00Z

@robbat2 verifying that existing buckets with invalid names are still accessible is important, yeah. can you think of a reason, though, that an admin would want to deny access to existing buckets? what motivates that second toggle/config option?

I verified this use-case -

Create relaxed bucket name (eg., bucket_underscore)
Toggle s3 relaxed bucket name option (to false)
Try to create an object in 'bucket_underscore'. The request fails now.

Seems like the s3 bucket name is validated for every request which contains bucket name in 'postauth_init()'. As of now there is no option to toggle that behavior. To let old buckets (with invalid names) to be used post migration/upgrade, admin needs to set option "rgw relaxed s3 bucket names" to true.

soumyakoduri · 2019-08-06T12:11:08Z

jenkins retest this please

soumyakoduri · 2019-08-07T15:54:23Z

@robbat2 verifying that existing buckets with invalid names are still accessible is important, yeah. can you think of a reason, though, that an admin would want to deny access to existing buckets? what motivates that second toggle/config option?

I verified this use-case -
* Create relaxed bucket name (eg., bucket_underscore)

* Toggle s3 relaxed bucket name option (to false)

* Try to create an object in 'bucket_underscore'. The request fails now.
Seems like the s3 bucket name is validated for every request which contains bucket name in 'postauth_init()'. As of now there is no option to toggle that behavior. To let old buckets (with invalid names) to be used post migration/upgrade, admin needs to set option "rgw relaxed s3 bucket names" to true.

To avoid the old names becoming inaccessible post migration, we decided to validate s3 bucket names only during bucket creation now. This way old buckets (with relaxed names) can still be operated post migration but no new buckets with those invalid names can be created by default.

If any need arises in future to enforce this validation for all requests , a new option can be added to toggle this behavior. But it is not being addressed as part of this PR.

src/rgw/rgw_rest_s3.cc

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 <skoduri@redhat.com>

soumyakoduri · 2019-08-08T15:48:49Z

jenkins retest this please

cbodley · 2019-08-09T14:42:07Z

jenkins test make check

soumyakoduri · 2019-08-12T06:53:15Z

jenkins test make check

soumyakoduri · 2019-08-12T06:53:51Z

jenkins retest

cbodley · 2019-08-16T14:54:19Z

@soumyakoduri thanks for working through all the issues in testing!

As per Amazon s3 spec, bucket names cannot contain underscore. https://docs.aws.amazon.com/awscloudtrail/latest/userguide/cloudtrail-s3-bucket-naming-requirements.html This (along with other naming convention mentioned in the doc) are being enforced via ceph/ceph#26787 This patch is to correct the bucket name generated in ragweed tests as well. Signed-off-by: Soumya Koduri <skoduri@redhat.com> (cherry picked from commit 7d4a729)

As per amazon s3 spec - https://docs.aws.amazon.com/awscloudtrail/latest/userguide/cloudtrail-s3-bucket-naming-requirements.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 Name length shouldn't exceed 63 characters. These rules are being enforced via - ceph/ceph#26787 This patch is to update the respective testcases as well. Note: check_invalid_bucket_name() seems to have been broken. It should try to create bucket using invalid name. Have addressed it in this patch. Because of this few testcases (which were incorrectly passing earlier) are failing in validate_bucket_name. Have marked them 'fails_on_rgw' as done for test_bucket_create_naming_bad_punctuation Signed-off-by: Soumya Koduri <skoduri@redhat.com>

cbodley added the rgw label Mar 6, 2019

stale bot added the stale label May 5, 2019

soumyakoduri force-pushed the bucket_name_validation branch from baa7a94 to 5cecc9a Compare May 6, 2019 11:01

stale bot removed the stale label May 6, 2019

cbodley reviewed May 6, 2019

View reviewed changes

src/rgw/rgw_rest_s3.h Show resolved Hide resolved

cbodley added the needs-doc label May 6, 2019

soumyakoduri force-pushed the bucket_name_validation branch from 5cecc9a to 4eb3ab8 Compare May 6, 2019 18:19

soumyakoduri force-pushed the bucket_name_validation branch from 4eb3ab8 to 29096ae Compare May 7, 2019 11:25

cbodley reviewed May 14, 2019

View reviewed changes

doc/radosgw/s3/bucketops.rst Outdated Show resolved Hide resolved

src/rgw/rgw_rest_s3.h Show resolved Hide resolved

soumyakoduri force-pushed the bucket_name_validation branch 2 times, most recently from ce57225 to bb855e0 Compare May 19, 2019 14:50

cbodley approved these changes Jun 4, 2019

View reviewed changes

cbodley added needs-qa and removed needs-doc labels Jun 4, 2019

soumyakoduri force-pushed the bucket_name_validation branch from bb855e0 to 9c0a698 Compare June 4, 2019 15:42

soumyakoduri force-pushed the bucket_name_validation branch 2 times, most recently from c76492b to 2b1bd35 Compare July 3, 2019 16:19

soumyakoduri force-pushed the bucket_name_validation branch from 2b1bd35 to 88cb52f Compare July 8, 2019 18:15

soumyakoduri force-pushed the bucket_name_validation branch from 88cb52f to a1109bc Compare July 18, 2019 15:17

soumyakoduri force-pushed the bucket_name_validation branch from a1109bc to 6b2126d Compare August 7, 2019 15:58

cbodley reviewed Aug 7, 2019

View reviewed changes

src/rgw/rgw_rest_s3.cc Outdated Show resolved Hide resolved

soumyakoduri force-pushed the bucket_name_validation branch from 6b2126d to eb6eddb Compare August 8, 2019 11:27

cbodley approved these changes Aug 8, 2019

View reviewed changes

cbodley merged commit f0575a7 into ceph:master Aug 16, 2019

smithfarm mentioned this pull request Jul 7, 2020

nautilus: rgw: Validate bucket names as per revised s3 spec #35583

Closed

soumyakoduri deleted the bucket_name_validation branch June 17, 2021 12:44

[rgw]:Validate bucket names as per revised s3 spec #26787

[rgw]:Validate bucket names as per revised s3 spec #26787

soumyakoduri commented Mar 6, 2019

soumyakoduri commented Mar 6, 2019

stale bot commented May 5, 2019

mattbenjamin commented May 6, 2019

cbodley commented May 6, 2019

soumyakoduri commented May 6, 2019

soumyakoduri commented May 6, 2019

cbodley commented May 6, 2019

soumyakoduri commented May 6, 2019

cbodley commented May 14, 2019

ceph-jenkins commented May 14, 2019

soumyakoduri commented Jun 3, 2019

ceph-jenkins commented Jun 3, 2019

cbodley commented Jun 4, 2019

cbodley commented Jun 4, 2019

soumyakoduri commented Jun 4, 2019

soumyakoduri commented Jul 4, 2019

monschein commented Jul 5, 2019

soumyakoduri commented Jul 5, 2019

cbodley commented Jul 8, 2019

alimaredia commented Jul 19, 2019

robbat2 commented Jul 31, 2019

cbodley commented Aug 1, 2019

robbat2 commented Aug 1, 2019

soumyakoduri commented Aug 5, 2019

soumyakoduri commented Aug 6, 2019

soumyakoduri commented Aug 7, 2019

soumyakoduri commented Aug 8, 2019

cbodley commented Aug 9, 2019

soumyakoduri commented Aug 12, 2019

soumyakoduri commented Aug 12, 2019

cbodley commented Aug 16, 2019

[rgw]:Validate bucket names as per revised s3 spec #26787

[rgw]:Validate bucket names as per revised s3 spec #26787

Conversation

soumyakoduri commented Mar 6, 2019

soumyakoduri commented Mar 6, 2019

stale bot commented May 5, 2019

mattbenjamin commented May 6, 2019

cbodley commented May 6, 2019

soumyakoduri commented May 6, 2019

soumyakoduri commented May 6, 2019

cbodley commented May 6, 2019

soumyakoduri commented May 6, 2019

cbodley commented May 14, 2019

ceph-jenkins commented May 14, 2019

soumyakoduri commented Jun 3, 2019

ceph-jenkins commented Jun 3, 2019

cbodley commented Jun 4, 2019

cbodley commented Jun 4, 2019

soumyakoduri commented Jun 4, 2019

soumyakoduri commented Jul 4, 2019

monschein commented Jul 5, 2019

soumyakoduri commented Jul 5, 2019

cbodley commented Jul 8, 2019

alimaredia commented Jul 19, 2019

robbat2 commented Jul 31, 2019

cbodley commented Aug 1, 2019

robbat2 commented Aug 1, 2019

soumyakoduri commented Aug 5, 2019

soumyakoduri commented Aug 6, 2019

soumyakoduri commented Aug 7, 2019

soumyakoduri commented Aug 8, 2019

cbodley commented Aug 9, 2019

soumyakoduri commented Aug 12, 2019

soumyakoduri commented Aug 12, 2019

cbodley commented Aug 16, 2019