radosgw: fix awsv4 header line sort order. #18046
Conversation
The awsv4 signature calculation includes a list of header lines, which are supposed to be sorted. The existing code sorts by header name, but it appears that in fact it is necessary to sort the whole header *line*, not just the field name. Sorting by just the field name usually works, but not always. The s3-tests teuthology suite includes s3tests.functional.test_s3.test_object_header_acl_grants s3tests.functional.test_s3.test_bucket_header_acl_grants which include the following header lines, x-amz-grant-read-acp:id=56789abcdef0123456789abcdef0123456789abcdef0123456789abcdef01234 x-amz-grant-read:id=56789abcdef0123456789abcdef0123456789abcdef0123456789abcdef01234 x-amz-grant-write-acp:id=56789abcdef0123456789abcdef0123456789abcdef0123456789abcdef01234 x-amz-grant-write:id=56789abcdef0123456789abcdef0123456789abcdef0123456789abcdef01234 in this case, note that ':' needs to sort after '-'. Fixes: http://tracker.ceph.com/issues/21607 Signed-off-by: Marcus Watts <mwatts@redhat.com>
mdw-at-linuxbox
requested review from
mattbenjamin,
yehudasa and
rzarzynski
|
http://pulpito.ceph.com/mbenjamin-2017-09-29_18:44:15-rgw-wip-mdw-awsv4-sort---basic-smithi/ ** @cbodley @oritwas this looks like a pass, but there is a complete failure of rgw multisite test |
| @@ -614,17 +616,14 @@ get_v4_canonical_headers(const req_info& info, | |||
| } | |||
| } | |||
|
|
|||
| canonical_hdrs_map[token] = rgw_trim_whitespace(token_value); | |||
| canonical_hdrs_set.insert( | |||
| boost::algorithm::join(std::vector<std::string>( | |||
There was a problem hiding this comment.
I would love to avoid std::vector and dynallocs here. Maybe boost::containers::static_vector could help. Anyway, this isn't even a minor.
There was a problem hiding this comment.
there's a string_join_reserve() in rgw_string.h that can do this without needing a container:
string_join_reserve(':', token, rgw_trim_whitespace(token_value))
|
jenkin retest this please ( |
|
this change caused a regression in boto3 requests with server-side encryption - this causes radosgw to sort the headers as: while boto3 produces: the original s3test failure with v4 in boto2 that motivated this pr appears to be a bug, reported at boto/boto#3032 relevant Amazon docs make it clear that headers should be sorted by name before appending the : and value(s): |
|
new tracker issue at http://tracker.ceph.com/issues/21832 and pr to revert at #18381 |
The awsv4 signature calculation includes a list of header lines, which
are supposed to be sorted. The existing code sorts by header name, but
it appears that in fact it is necessary to sort the whole header line,
not just the field name. Sorting by just the field name usually works,
but not always. The s3-tests teuthology suite includes
s3tests.functional.test_s3.test_object_header_acl_grants
s3tests.functional.test_s3.test_bucket_header_acl_grants
which include the following header lines,
x-amz-grant-read-acp:id=56789abcdef0123456789abcdef0123456789abcdef0123456789abcdef01234
x-amz-grant-read:id=56789abcdef0123456789abcdef0123456789abcdef0123456789abcdef01234
x-amz-grant-write-acp:id=56789abcdef0123456789abcdef0123456789abcdef0123456789abcdef01234
x-amz-grant-write:id=56789abcdef0123456789abcdef0123456789abcdef0123456789abcdef01234
in this case, note that ':' needs to sort after '-'.
Fixes: http://tracker.ceph.com/issues/21607
Signed-off-by: Marcus Watts mwatts@redhat.com