Skip to content

Commit

Permalink
radosgw: fix awsv4 header line sort order.
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
mdw-at-linuxbox committed Sep 29, 2017
1 parent c59efe0 commit c11485e
Showing 1 changed file with 8 additions and 9 deletions.
17 changes: 8 additions & 9 deletions src/rgw/rgw_auth_s3.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

#include <algorithm>
#include <map>
#include <set>
#include <iterator>
#include <string>
#include <vector>
Expand All @@ -15,6 +16,7 @@
#include "rgw_rest.h"
#include "rgw_crypt_sanitize.h"

#include <boost/algorithm/string/join.hpp>
#include <boost/container/small_vector.hpp>
#include <boost/utility/string_view.hpp>

Expand Down Expand Up @@ -567,7 +569,7 @@ get_v4_canonical_headers(const req_info& info,
const bool using_qs,
const bool force_boto2_compat)
{
std::map<boost::string_view, std::string> canonical_hdrs_map;
std::set<std::string> canonical_hdrs_set;
for (const auto& token : get_str_vec<5>(signedheaders, ";")) {
/* TODO(rzarzynski): we'd like to switch to sstring here but it should
* get push_back() and reserve() first. */
Expand Down Expand Up @@ -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>(
{std::string(token), rgw_trim_whitespace(token_value)} ), ":"));
}

std::string canonical_hdrs;
for (const auto& header : canonical_hdrs_map) {
const boost::string_view& name = header.first;
const std::string& value = header.second;

canonical_hdrs.append(name.data(), name.length())
.append(":", std::strlen(":"))
.append(value)
for (const auto& header : canonical_hdrs_set) {
canonical_hdrs.append(header.data(), header.length())
.append("\n", std::strlen("\n"));
}

Expand Down

0 comments on commit c11485e

Please sign in to comment.