pacific: rgw/auth: ignoring signatures for HTTP OPTIONS calls #55550
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
minor conflicts because #50550 wasn't backported to pacific |
|
qa will need to include test cases from ceph/s3-tests#523 and ceph/s3-tests#544 |
cbodley
force-pushed
the
wip-64404-pacific
branch
from
1391c7e
to
36acdd7
Compare
Before [1] we always sent all HTTP OPTIONS requests to the S3AnonymousEngine and ignored any provided AWSv4 credentials sent in the request. That PR changed so that if we got credentials in the request we instead sent it through the authentication code in order to solve HTTP OPTIONS requests on tenanted users to start working (because we need to resolve the tenant, also called bucket tenant in the code, and we can't only rely on the bucket name since it will not be found). We solved this by modifying the canonical HTTP method used when calculating the AWSv4 signature by instead using the access-control-request-method header which worked good. This change did not take into account that when you generated a presigned URL for a put_object request you can also pass in extra parameters like a canned ACL [2] to the Params variable in for example boto3's generated_presigned_url(). Doing that will cause the client to add the x-amz-acl header to x-amz-signedheaders and also use that in their signature calculation. When doing a HTTP OPTIONS calls for CORS on that presigned URL the browser will never send a x-amz-acl header with the correct data since that is something that the actual PUT request should include later, so that HTTP OPTIONS call should pass even though the signature can never be calculated correctly server-side like verified against AWS S3 in tracker [3]. This patch as a result skips the signature calculation when doing EC2 auth using the LocalEngine but we still need to pass the request there in order to lookup the user to support buckets in a tenant. For the Keystone EC2 auth we're pretty out of luck in the sense that Keystone's API itself requires us to send the AWSv4 signature in the request with the access_key in order to obtain a token, and we cannot leave the signature out, we also cannot spoof the signature from rgw -> keystone since we don't have access to the secret_key if it's not in our cache. For that approach we simply pass on to get_access_token() that if it's an HTTP OPTIONS and we find the access_key in the cache we pull that and ignore verifying signature and pass it on for validation. This means that the cache must be warm if using Keystone auth and adding extra params to a presigned URL. This partly makes some of the commits in [1] redundant for EC2 LocalEngine auth but we still need it for tenanted bucket support. [1] ceph#52673 [2] https://docs.aws.amazon.com/AmazonS3/latest/userguide/acl-overview.html#canned-acl [3] https://tracker.ceph.com/issues/64308 Fixes: https://tracker.ceph.com/issues/64308 Signed-off-by: Tobias Urdin <tobias.urdin@binero.se> (cherry picked from commit fe15b52) Conflicts: missing 'rgw/keystone: use secret key from EC2 for sigv4 streaming mode' src/rgw/rgw_auth_keystone.cc src/rgw/rgw_auth_keystone.h
cbodley
force-pushed
the
wip-64404-pacific
branch
from
36acdd7
to
34c71da
Compare
|
|
jenkins test make check |
passed qa 👍 |
verified that all of these test cases passed in http://qa-proxy.ceph.com/teuthology/cbodley-2024-02-13_12:46:04-rgw-wip-64404-pacific-distro-default-smithi/7558114/teuthology.log |
yuriw
approved these changes
Feb 13, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
backport tracker: https://tracker.ceph.com/issues/64404
backport of #55458
parent tracker: https://tracker.ceph.com/issues/64308
this backport was staged using ceph-backport.sh version 16.0.0.6848
find the latest version at https://github.com/ceph/ceph/blob/main/src/script/ceph-backport.sh