rgw: aws4: add presigned url bugfix in runtime #10160
Conversation
|
@pritha-srivastava working on my side (test suite passing, boto2 and mc manual tests also running) can you confirm it works on your side? thanks! |
| @@ -1310,6 +1310,7 @@ OPTION(rgw_keystone_verify_ssl, OPT_BOOL, true) // should we try to verify keyst | |||
| OPTION(rgw_keystone_implicit_tenants, OPT_BOOL, false) // create new users in their own tenants of the same name | |||
| OPTION(rgw_s3_auth_use_rados, OPT_BOOL, true) // should we try to use the internal credentials for s3? | |||
| OPTION(rgw_s3_auth_use_keystone, OPT_BOOL, false) // should we try to use keystone for s3? | |||
| OPTION(rgw_s3_auth_aws4_presigned_url_bugfix, OPT_BOOL, false) // aws4 auth presigned url client-side bug support? | |||
There was a problem hiding this comment.
Please remove '?' at the end of the statement
There was a problem hiding this comment.
Keeping the question mark at the end of the comment looks coherent with the comment style of the previous options (rgw_s3_auth_use_rados and rgw_s3_auth_use_keystone). Maybe it makes sense keeping it. What do you think?
There was a problem hiding this comment.
The previous options are questions, hence the question marks. The one related to rgw_s3_auth_aws4_presigned_url_bugfix is not, hence I think the question mark should be removed.
jmunhoz
force-pushed
the
bugfix-16463-presigned-url
branch
from
8f915e0
to
8f322dd
Compare
|
@yehudasa : I have run the s3 tests and they are working fine. Can you please review the PR. |
|
@mattbenjamin do you have some time to review this PR along this week? It would be great if we get it in to support Minio clients asap. thanks! |
|
@mattbenjamin ping |
|
@pritha-srivastava can you re-review this one following @jmunhoz comments? |
jmunhoz
force-pushed
the
bugfix-16463-presigned-url
branch
from
8f322dd
to
5ce9a10
Compare
|
@yehudasa renamed to '_force_boto2_compat' @cbodley added the suggested change. it makes the code more simple and compact @pritha-srivastava the new code is working on my side. feel free to re-review. txs! |
|
👍 |
|
@yehudasa ping |
|
jenkins test this please (no logs) |
Runtime bugfix to handle presigned urls computed with canonical requests using the port number once. Boto2 computes canonical requests using the port number twice although it should be used once only. This behaviour is a bug supported by AWS S3. Boto2 is used in RGW S3 as reference implementation. The client-side tools not supporting this boto2 bug will fail although they should work too. In order to support both presigned url implementations this patch adds a config option to compute a second signature. With this option disabled, the code will compute two signatures when the first signature is not valid. The aws4 auth succeed if some of the two signatures is valid. The config option rgw_s3_auth_aws4_force_boto2_compat, is enabled by default so one signature, working with boto2, is computed only. Fixes: http://tracker.ceph.com/issues/16463 Signed-off-by: Javier M. Mellid <jmunhoz@igalia.com>
jmunhoz
force-pushed
the
bugfix-16463-presigned-url
branch
from
5ce9a10
to
078c513
Compare
Runtime bugfix to handle presigned urls computed with canonical requests using
the port number once.
Boto2 computes canonical requests using the port number twice although it
should be used once only. This behaviour is a bug supported by AWS S3. Boto2 is
used in RGW S3 as reference implementation.
The client-side tools not supporting this boto2 bug will fail although they
should work too.
In order to support both presigned url implementations this patch adds a config
option to compute a second signature. With this option enabled, the code will
compute two signatures when the first signature is not valid. The aws4 auth
succeed if some of the two signatures is valid.
The config option rgw_s3_auth_aws4_presigned_url_bugfix is disabled by default
so one signature, working with boto2, is computed only.
Fixes: http://tracker.ceph.com/issues/16463
Signed-off-by: Javier M. Mellid jmunhoz@igalia.com