rgw: fix return value of auth v2/v4 #19310
Conversation
|
hi, @cbodley, @rzarzynski, @adamemerson, would you help to review this? thanks! |
|
hi, @rzarzynski, would you help to review this? thanks! |
|
@qrGitHub: I've put this review on my TODO. |
|
@rzarzynski, ok, thanks. |
There was a problem hiding this comment.
@qrGitHub: thanks for bringing the fixes and apologizes for late review. Technically looks good.
src/rgw/rgw_rest_s3.h
Outdated
| @@ -767,8 +767,7 @@ class AWSEngine : public rgw::auth::Engine { | |||
| class AWSGeneralAbstractor : public AWSEngine::VersionAbstractor { | |||
| CephContext* const cct; | |||
|
|
|||
| bool is_time_skew_ok(const utime_t& header_time, | |||
| const bool qsr) const; | |||
| bool is_time_skew_ok(const utime_t& header_time) const; | |||
src/rgw/rgw_auth_s3.cc
Outdated
| @@ -408,6 +391,19 @@ static inline int parse_v4_auth_header(const req_info& info, /* in | |||
| } | |||
| date = d; | |||
|
|
|||
| uint64_t req_sec = (uint64_t)internal_timegm(&t); | |||
There was a problem hiding this comment.
It looks we're lacking this check for long time. Could you please dissect this change into a separated commit, create a bug report on the RGW tracker and put the Fixes: <bug_url> tag to the commit message? This would make the backporting much easier.
| } | ||
| qsr = true; | ||
| if (expires.empty()) { | ||
| return -EPERM; |
There was a problem hiding this comment.
In addition to the time-skew-check for v4, also the error code corrections might deserve for a separated commit and tracker ticket.
There was a problem hiding this comment.
Do you mean add an error code for AuthorizationQueryParametersError?
There was a problem hiding this comment.
Ok, got it. I will take this later.
There was a problem hiding this comment.
Sure. Beside this small housekeeping thing the change looks fine.
qrGitHub
force-pushed
the
wip-rgw-auth-retVal
branch
from
581d450
to
fc70d55
Compare
|
hi @rzarzynski, I remove "time skew check" from parse_v4_auth_header, and create a new one for this. Thanks. |
* The return value of auth v2/v4 in RGW is different from that in AWS: * 1. When 'Expires' is missing in auth v2 query string request, AWS * returns AccessDenied while RGW returns SignatureDoesNotMatch; * 2. When 'X-Amz-Expires' is missing in auth v4 query string * request, AWS returns AuthorizationQueryParametersError while RGW * returns RequestTimeTooSkewed; * Changes: * 1. When 'Expires' is missing in auth v2 query string request, * change RGW's return value to AccessDenied; * 2. When 'X-Amz-Expires' is missing in auth v4 query string * request, change RGW's return value to AccessDenied; * 3. remove time skew check from parse_v4_query_string; Fixes: http://tracker.ceph.com/issues/22439 Signed-off-by: Bingyin Zhang <zhangbingyin@cloudin.cn>
qrGitHub
force-pushed
the
wip-rgw-auth-retVal
branch
from
fc70d55
to
ce42f1e
Compare
|
Hi @rzarzynski, I add a RGW tracker for the error code corrections, and create another pull request(#19511) to optimize function 'is_time_skew_ok'. |
The return value of auth v2/v4 in RGW is different from that in AWS:
Changes: