This is an automated email from the ASF dual-hosted git repository. gancho pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/trafficserver.git
The following commit(s) were added to refs/heads/master by this push: new 30b75a3 Avoid AWS auth v4 path/query param double encoding 30b75a3 is described below commit 30b75a3d5cae94bcd105464122a253eb9bbf4a1c Author: Gancho Tenev <gan...@apache.org> AuthorDate: Tue Aug 13 11:35:39 2019 -0700 Avoid AWS auth v4 path/query param double encoding AWS signature validation differs from the spec published here (or spec not detailed enough): http://docs.aws.amazon.com/AmazonS3/latest/API/sig-v4-header-based-auth.html ("Task 1: Create a Canonical Request”) During signature calculation (CanonicalURI and CanonicalQueryString inside the CanonicalRequest) AWS avoids URI encoding of already encoded path or query parameters which is nowhere mentioned in the specification but it is likely done according to rfc3986#section-2.4 which says "implementations must not percent-encode or decode the same string more than once ..." We already had a fix for query parementer values. Added missing checks to be consistent with AWS behavior while still waiting for response/confirmation from AWS. --- plugins/s3_auth/aws_auth_v4.cc | 31 ++++++++++++++++---------- plugins/s3_auth/unit_tests/test_aws_auth_v4.cc | 11 ++++----- 2 files changed, 25 insertions(+), 17 deletions(-) diff --git a/plugins/s3_auth/aws_auth_v4.cc b/plugins/s3_auth/aws_auth_v4.cc index 1df1a61..22e90ad 100644 --- a/plugins/s3_auth/aws_auth_v4.cc +++ b/plugins/s3_auth/aws_auth_v4.cc @@ -151,6 +151,22 @@ isUriEncoded(const String &in, bool isObjectName) return false; } +String +canonicalEncode(const String &in, bool isObjectName) +{ + String canonical; + if (!isUriEncoded(in, isObjectName)) { + /* Not URI-encoded */ + canonical = uriEncode(in, isObjectName); + } else { + /* URI-encoded, then don't encode since AWS does not encode which is not mentioned in the spec, + * asked AWS, still waiting for confirmation */ + canonical = in; + } + + return canonical; +} + /** * @brief trim the white-space character from the beginning and the end of the string ("in-place", just moving pointers around) * @@ -287,7 +303,7 @@ getCanonicalRequestSha256Hash(TsInterface &api, bool signPayload, const StringSe str = api.getPath(&length); String path("/"); path.append(str, length); - String canonicalUri = uriEncode(path, /* isObjectName */ true); + String canonicalUri = canonicalEncode(path, /* isObjectName */ true); sha256Update(&canonicalRequestSha256Ctx, canonicalUri); sha256Update(&canonicalRequestSha256Ctx, "\n"); @@ -306,18 +322,9 @@ getCanonicalRequestSha256Hash(TsInterface &api, bool signPayload, const StringSe String param(token.substr(0, pos == String::npos ? token.size() : pos)); String value(pos == String::npos ? "" : token.substr(pos + 1, token.size())); - String encodedParam = uriEncode(param, /* isObjectName */ false); - + String encodedParam = canonicalEncode(param, /* isObjectName */ false); paramNames.insert(encodedParam); - - if (!isUriEncoded(value, /* isObjectName */ false)) { - /* Not URI-encoded */ - paramsMap[encodedParam] = uriEncode(value, /* isObjectName */ false); - } else { - /* URI-encoded, then don't encode since AWS does not encode which is not mentioned in the spec, - * asked AWS, still waiting for confirmation */ - paramsMap[encodedParam] = value; - } + paramsMap[encodedParam] = canonicalEncode(value, /* isObjectName */ false); } String queryStr; diff --git a/plugins/s3_auth/unit_tests/test_aws_auth_v4.cc b/plugins/s3_auth/unit_tests/test_aws_auth_v4.cc index 408ce21..4bf58af 100644 --- a/plugins/s3_auth/unit_tests/test_aws_auth_v4.cc +++ b/plugins/s3_auth/unit_tests/test_aws_auth_v4.cc @@ -631,7 +631,7 @@ TEST_CASE("AWSAuthSpecByExample: GET Bucket List Objects, query param value alre MockTsInterface api; api._method.assign("GET"); api._host.assign("examplebucket.s3.amazonaws.com"); - api._path.assign(""); + api._path.assign("PATH=="); api._query.assign("key=TEST=="); api._headers["Host"] = "examplebucket.s3.amazonaws.com"; api._headers["x-amz-content-sha256"] = "UNSIGNED-PAYLOAD"; @@ -642,18 +642,18 @@ TEST_CASE("AWSAuthSpecByExample: GET Bucket List Objects, query param value alre "AWS4-HMAC-SHA256 " "Credential=AKIAIOSFODNN7EXAMPLE/20130524/us-east-1/s3/aws4_request," "SignedHeaders=host;x-amz-content-sha256;x-amz-date," - "Signature=60b410f6a0ffe09b91c2aef1f179945916b45ea215278e6b8f6cfb8d461e3706", + "Signature=3b195c74eaa89790c596114a9fcb8515d6a3cc78577f8941c46b09ee7f501194", /* Canonical Request sha256 */ - "1035b1d75dad9e94fa99fa6edc2cf7d489f38796109a132721621977737a41cc", + "7b3c74369013172ce9cb3da99b5d14e358382953e7b560b4c9bf0a46e73933cb", /* Date and time*/ "20130524T000000Z", /* String to sign */ "AWS4-HMAC-SHA256\n" "20130524T000000Z\n" "20130524/us-east-1/s3/aws4_request\n" - "1035b1d75dad9e94fa99fa6edc2cf7d489f38796109a132721621977737a41cc", + "7b3c74369013172ce9cb3da99b5d14e358382953e7b560b4c9bf0a46e73933cb", /* Signature */ - "60b410f6a0ffe09b91c2aef1f179945916b45ea215278e6b8f6cfb8d461e3706", + "3b195c74eaa89790c596114a9fcb8515d6a3cc78577f8941c46b09ee7f501194", /* Payload hash */ "UNSIGNED-PAYLOAD", /* Signed Headers */ @@ -664,6 +664,7 @@ TEST_CASE("AWSAuthSpecByExample: GET Bucket List Objects, query param value alre /* Now make query param value encoded beforehand and expect the same result, * it should not URI encode it twice according to AWS real behavior undocumented in the specification.*/ + api._path.assign("PATH%3D%3D"); api._query.assign("key=TEST%3D%3D"); ValidateBench(api, /*signePayload */ false, &now, bench, defaultIncludeHeaders, defaultExcludeHeaders); }