This is an automated email from the ASF dual-hosted git repository.
zwoop pushed a commit to branch 7.1.x
in repository https://gitbox.apache.org/repos/asf/trafficserver.git
The following commit(s) were added to refs/heads/7.1.x by this push:
new 6f91503 AWS auth v4: fixed query param value URI-encoding
6f91503 is described below
commit 6f91503febcc6e8b75decb7a9dc13e7f5a0cae14
Author: Gancho Tenev <gan...@apache.com>
AuthorDate: Tue Aug 29 12:28:15 2017 -0700
AWS auth v4: fixed query param value URI-encoding
It seems AWS servers do not validate the signature according the spec they
published here:
http://docs.aws.amazon.com/AmazonS3/latest/API/sig-v4-header-based-auth.html
("Task 1: Create a Canonical Request”)
When AWS servers validate the signature (CanonicalQueryString inside the
CanonicalRequest)
they seem to check if the query parameter value is already encoded and
don’t encode if it is,
which is nowhere mentioned neither in the spec nor in its examples.
Added a check to be consistent with AWS behavior while still waiting for
response/confirmation from AWS.
(cherry picked from commit 3e20b077bb96ef7eead26e48ea9e255902453a6e)
---
plugins/s3_auth/aws_auth_v4.cc | 46 ++++++++++++-
plugins/s3_auth/unit-tests/test_aws_auth_v4.cc | 91 ++++++++++++++++++++++++++
plugins/s3_auth/unit-tests/test_aws_auth_v4.h | 1 +
3 files changed, 137 insertions(+), 1 deletion(-)
diff --git a/plugins/s3_auth/aws_auth_v4.cc b/plugins/s3_auth/aws_auth_v4.cc
index 5b8708a..1475111 100644
--- a/plugins/s3_auth/aws_auth_v4.cc
+++ b/plugins/s3_auth/aws_auth_v4.cc
@@ -23,6 +23,7 @@
*/
#include <cstring> /* strlen() */
+#include <string> /* stoi() */
#include <ctime> /* strftime(), time(), gmtime_r() */
#include <iomanip> /* std::setw */
#include <sstream> /* std::stringstream */
@@ -69,6 +70,9 @@ base16Encode(const char *in, size_t inLen)
*
* @see AWS spec:
http://docs.aws.amazon.com/AmazonS3/latest/API/sig-v4-header-based-auth.html
*
+ * @todo Consider reusing / converting to TSStringPercentEncode() using a
custom map to account for the AWS specific rules.
+ * Currently we don't build a library/archive so we could link with the
unit-test binary. Also using
+ * different sets of encode/decode functions during runtime and
unit-testing did not seem as a good idea.
* @param in string to be URI encoded
* @param isObjectName if true don't encode '/', keep it as it is.
* @return encoded string.
@@ -99,6 +103,35 @@ uriEncode(const String &in, bool isObjectName)
}
/**
+ * @brief URI-decode a character string (AWS specific version, see spec)
+ *
+ * @see AWS spec:
http://docs.aws.amazon.com/AmazonS3/latest/API/sig-v4-header-based-auth.html
+ *
+ * @todo Consider reusing / converting to TSStringPercentDecode()
+ * Currently we don't build a library/archive so we could link with the
unit-test binary. Also using
+ * different sets of encode/decode functions during runtime and
unit-testing did not seem as a good idea.
+ * @param in string to be URI decoded
+ * @return encoded string.
+ */
+String
+uriDecode(const String in)
+{
+ std::string result;
+ result.reserve(in.length());
+ size_t i = 0;
+ while (i < in.length()) {
+ if (in[i] == '%') {
+ result += static_cast<char>(std::stoi(in.substr(i + 1, 2), nullptr, 16));
+ i += 3;
+ } else {
+ result += in[i];
+ i++;
+ }
+ }
+ return result;
+}
+
+/**
* @brief trim the white-space character from the beginning and the end of the
string ("in-place", just moving pointers around)
*
* @param in ptr to an input string
@@ -256,7 +289,18 @@ getCanonicalRequestSha256Hash(TsInterface &api, bool
signPayload, const StringSe
String encodedParam = uriEncode(param, /* isObjectName */ false);
paramNames.insert(encodedParam);
- paramsMap[encodedParam] = uriEncode(value, /* isObjectName */ false);
+
+ /* Look for '%' first trying to avoid as many uri-decode calls as possible.
+ * it is hard to estimate which is more likely use-case - (1) URIs with
uri-encoded query parameter
+ * values or (2) with unencoded which defines the success of this
optimization */
+ if (nullptr == memchr(value.c_str(), '%', value.length()) || 0 ==
uriDecode(value).compare(value)) {
+ /* 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;
+ }
}
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 4589aca..f325179 100644
--- a/plugins/s3_auth/unit-tests/test_aws_auth_v4.cc
+++ b/plugins/s3_auth/unit-tests/test_aws_auth_v4.cc
@@ -68,6 +68,39 @@ TEST_CASE("uriEncode(): encode reserved chars in an object
name", "[AWS][auth][u
CHECK_FALSE(encoded.compare("%20/%21%22%23%24%25%26%27%28%29%2A%2B%2C%3A%3B%3C%3D%3E%3F%40%5B%5C%5D%5E%60%7B%7C%7D"));
}
+TEST_CASE("uriDecode(): decode empty input", "[AWS][auth][utility]")
+{
+ String encoded("");
+ String decoded = uriDecode(encoded);
+ CHECK(0 == decoded.length()); /* 0 encoded because of the invalid input */
+}
+
+TEST_CASE("uriDecode(): decode unreserved chars", "[AWS][auth][utility]")
+{
+ const String encoded = "ABCDEFGHIJKLMNOPQRSTUVWXYZ"
+ "abcdefghijklmnopqrstuvwxyz"
+ "0123456789"
+ "-._~";
+ String decoded = uriDecode(encoded);
+
+ CHECK(encoded.length() == encoded.length());
+ CHECK_FALSE(decoded.compare("ABCDEFGHIJKLMNOPQRSTUVWXYZ"
+ "abcdefghijklmnopqrstuvwxyz"
+ "0123456789"
+ "-._~"));
+}
+
+TEST_CASE("uriDecode(): decode reserved chars", "[AWS][auth][utility]")
+{
+ const String encoded =
+
"%20%2F%21%22%23%24%25%26%27%28%29%2A%2B%2C%3A%3B%3C%3D%3E%3F%40%5B%5C%5D%5E%60%7B%7C%7D";
/* some printable but
+
reserved chars */
+ String decoded = uriDecode(encoded);
+
+ CHECK(3 * decoded.length() == encoded.length()); /* size of "%NN" = 3 */
+ CHECK_FALSE(decoded.compare(" /!\"#$%&'()*+,:;<=>?@[\\]^`{|}"));
+}
+
/* base16Encode()
**************************************************************************************************************
*/
TEST_CASE("base16Encode(): base16 encode empty string", "[utility]")
@@ -265,15 +298,18 @@ ValidateBench(TsInterface &api, bool signPayload, time_t
*now, const char *bench
AwsAuthV4 util(api, now, signPayload, awsAccessKeyId,
strlen(awsAccessKeyId), awsSecretAccessKey, strlen(awsSecretAccessKey),
awsService, strlen(awsService), includedHeaders,
excludedHeaders, defaultDefaultRegionMap);
String authorizationHeader = util.getAuthorizationHeader();
+ CAPTURE(authorizationHeader);
CHECK_FALSE(authorizationHeader.compare(bench[0]));
/* Test payload hash */
String payloadHash = util.getPayloadHash();
+ CAPTURE(payloadHash);
CHECK_FALSE(payloadHash.compare(bench[5]));
/* Test the date time header content */
size_t dateLen = 0;
const char *date = util.getDateTime(&dateLen);
+ CAPTURE(String(date, dateLen));
CHECK_FALSE(String(date, dateLen).compare(bench[2]));
/* Now test particular test points to pinpoint problems easier in case of
regression */
@@ -281,12 +317,15 @@ ValidateBench(TsInterface &api, bool signPayload, time_t
*now, const char *bench
/* test the canonization of the request */
String signedHeaders;
String canonicalReq = getCanonicalRequestSha256Hash(api, signPayload,
includedHeaders, excludedHeaders, signedHeaders);
+ CAPTURE(canonicalReq);
CHECK_FALSE(canonicalReq.compare(bench[1]));
+ CAPTURE(signedHeaders);
CHECK_FALSE(signedHeaders.compare(bench[6]));
/* Test the formating of the date and time */
char dateTime[sizeof("20170428T010203Z")];
size_t dateTimeLen = getIso8601Time(now, dateTime, sizeof(dateTime));
+ CAPTURE(String(dateTime, dateTimeLen));
CHECK_FALSE(String(dateTime, dateTimeLen).compare(bench[2]));
/* Test the region name */
@@ -297,6 +336,7 @@ ValidateBench(TsInterface &api, bool signPayload, time_t
*now, const char *bench
/* Test string to sign calculation */
String stringToSign = getStringToSign(host, hostLen, dateTime, dateTimeLen,
awsRegion.c_str(), awsRegion.length(), awsService,
strlen(awsService),
canonicalReq.c_str(), canonicalReq.length());
+ CAPTURE(stringToSign);
CHECK_FALSE(stringToSign.compare(bench[3]));
/* Test the signature calculation */
@@ -305,6 +345,7 @@ ValidateBench(TsInterface &api, bool signPayload, time_t
*now, const char *bench
getSignature(awsSecretAccessKey, strlen(awsSecretAccessKey),
awsRegion.c_str(), awsRegion.length(), awsService,
strlen(awsService), dateTime, 8, stringToSign.c_str(),
stringToSign.length(), signature, EVP_MAX_MD_SIZE);
String base16Signature = base16Encode(signature, signatureLen);
+ CAPTURE(base16Signature);
CHECK_FALSE(base16Signature.compare(bench[4]));
}
@@ -535,6 +576,56 @@ TEST_CASE("AWSAuthSpecByExample: GET Bucket List Objects,
unsigned pay-load, exc
ValidateBench(api, /*signePayload */ false, &now, bench,
defaultIncludeHeaders, defaultExcludeHeaders);
}
+/**
+ * Test based on
docs.aws.amazon.com/AmazonS3/latest/API/sig-v4-header-based-auth.html
+ * but this time query param value is already encoded, it should not URI
encode it twice
+ * according to AWS real behavior undocumented in the specification.
+ */
+TEST_CASE("AWSAuthSpecByExample: GET Bucket List Objects, query param value
already URI encoded", "[AWS][auth]")
+{
+ time_t now = 1369353600; /* 5/24/2013 00:00:00 GMT */
+
+ /* Define the HTTP request elements */
+ MockTsInterface api;
+ api._method.assign("GET");
+ api._host.assign("examplebucket.s3.amazonaws.com");
+ api._path.assign("");
+ api._query.assign("key=TEST==");
+ api._headers["Host"] = "examplebucket.s3.amazonaws.com";
+ api._headers["x-amz-content-sha256"] = "UNSIGNED-PAYLOAD";
+ api._headers["x-amz-date"] = "20130524T000000Z";
+
+ const char *bench[] = {
+ /* Authorization Header */
+ "AWS4-HMAC-SHA256 "
+ "Credential=AKIAIOSFODNN7EXAMPLE/20130524/us-east-1/s3/aws4_request,"
+ "SignedHeaders=host;x-amz-content-sha256;x-amz-date,"
+
"Signature=60b410f6a0ffe09b91c2aef1f179945916b45ea215278e6b8f6cfb8d461e3706",
+ /* Canonical Request sha256 */
+ "1035b1d75dad9e94fa99fa6edc2cf7d489f38796109a132721621977737a41cc",
+ /* Date and time*/
+ "20130524T000000Z",
+ /* String to sign */
+ "AWS4-HMAC-SHA256\n"
+ "20130524T000000Z\n"
+ "20130524/us-east-1/s3/aws4_request\n"
+ "1035b1d75dad9e94fa99fa6edc2cf7d489f38796109a132721621977737a41cc",
+ /* Signature */
+ "60b410f6a0ffe09b91c2aef1f179945916b45ea215278e6b8f6cfb8d461e3706",
+ /* Payload hash */
+ "UNSIGNED-PAYLOAD",
+ /* Signed Headers */
+ "host;x-amz-content-sha256;x-amz-date",
+ };
+
+ ValidateBench(api, /*signePayload */ false, &now, bench,
defaultIncludeHeaders, defaultExcludeHeaders);
+
+ /* 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._query.assign("key=TEST%3D%3D");
+ ValidateBench(api, /*signePayload */ false, &now, bench,
defaultIncludeHeaders, defaultExcludeHeaders);
+}
+
/* Utility parameters related tests
********************************************************************************
*/
void
diff --git a/plugins/s3_auth/unit-tests/test_aws_auth_v4.h
b/plugins/s3_auth/unit-tests/test_aws_auth_v4.h
index e7889c5..3306b8e 100644
--- a/plugins/s3_auth/unit-tests/test_aws_auth_v4.h
+++ b/plugins/s3_auth/unit-tests/test_aws_auth_v4.h
@@ -120,6 +120,7 @@ public:
/* Expose the following methods only to the unit tests */
String base16Encode(const char *in, size_t inLen);
String uriEncode(const String in, bool isObjectName = false);
+String uriDecode(const String in);
String lowercase(const char *in, size_t inLen);
const char *trimWhiteSpaces(const char *in, size_t inLen, size_t &newLen);
--
To stop receiving notification emails like this one, please contact
['"commits@trafficserver.apache.org" <commits@trafficserver.apache.org>'].
