From b66fdc9764bebd195e7522a7f05a2505972bd25c Mon Sep 17 00:00:00 2001 From: Shunichi Shinohara Date: Fri, 1 May 2015 17:32:12 +0900 Subject: [PATCH] Fix URL encoding bug around AWS v4 auth AWS v4 auth defined URL encoding algorithm that is used for generating signature. Mochiweb's utility function convert space (" ") to plus ("+") but AWS spec says it should be percent encoded. i.e. "%20". This commit fixes the encoding bug for resource (URL path) and query parameters. cf. Authenticating Requests by Using the Authorization Header (Compute Checksum of the Entire Payload Prior to Transmission) - Signature Version 4 - Amazon Simple Storage Service http://docs.aws.amazon.com/ja_jp/AmazonS3/latest/API/sig-v4-header-based-auth.html --- client_tests/python/boto_tests/boto_test.py | 19 +++++++++- src/riak_cs_s3_auth.erl | 42 ++++++++++++++++++++- 2 files changed, 57 insertions(+), 4 deletions(-) diff --git a/client_tests/python/boto_tests/boto_test.py b/client_tests/python/boto_tests/boto_test.py index e6a93b05f..5b43194c0 100755 --- a/client_tests/python/boto_tests/boto_test.py +++ b/client_tests/python/boto_tests/boto_test.py @@ -164,6 +164,17 @@ def test_auth(self): conn = self.make_connection(bad_user) self.assertRaises(S3ResponseError, conn.get_canonical_user_id) + def test_auth_weird_query_param(self): + bucket = self.conn.create_bucket(self.bucket_name) + query_args = 'foo=bar%20baz' + response = bucket.connection.make_request('GET', bucket.name, "notfound", + query_args=query_args) + body = response.read() + boto.log.debug(body) + if response.status != 404: + raise bucket.connection.provider.storage_response_error( + response.status, response.reason, body) + def test_create_bucket(self): self.conn.create_bucket(self.bucket_name) self.assertIn(self.bucket_name, @@ -317,7 +328,6 @@ def test_list_japanese_key(self): for u in uploads: self.assertEqual(u.key_name, key_name) - def one_kb_string(): "Return a 1KB string of all a's" return ''.join(['a' for _ in xrange(1024)]) @@ -665,7 +675,8 @@ class ObjectMetadataTest(S3ApiVerificationTestBase): "Expires": "Tue, 19 Jan 2038 03:14:07 GMT", "mtime": "1364742057", "UID": "0", - "with-hypen": "1"} + "with-hypen": "1", + "space-in-value": "abc xyz"} updated_metadata = { "Content-Disposition": 'attachment; filename="newname.txt"', @@ -673,6 +684,7 @@ class ObjectMetadataTest(S3ApiVerificationTestBase): "Expires": "Tue, 19 Jan 2038 03:14:07 GMT", "mtime": "2222222222", "uid": "0", + "space-in-value": "ABC XYZ", "new-entry": "NEW"} def test_normal_object_metadata(self): @@ -708,12 +720,14 @@ def assert_metadata(self, bucket, key_name): self.assertEqual(key.get_metadata("mtime"), "1364742057") self.assertEqual(key.get_metadata("uid"), "0") self.assertEqual(key.get_metadata("with-hypen"), "1") + self.assertEqual(key.get_metadata("space-in-value"), "abc xyz") # x-amz-meta-* headers should be normalized to lowercase self.assertEqual(key.get_metadata("Mtime"), None) self.assertEqual(key.get_metadata("MTIME"), None) self.assertEqual(key.get_metadata("Uid"), None) self.assertEqual(key.get_metadata("UID"), None) self.assertEqual(key.get_metadata("With-Hypen"), None) + self.assertEqual(key.get_metadata("Space-In-Value"), None) def change_metadata(self, bucket, key_name): key = Key(bucket, key_name) @@ -730,6 +744,7 @@ def assert_updated_metadata(self, bucket, key_name): 'attachment; filename="newname.txt"') self.assertEqual(key.cache_control, "private") self.assertEqual(key.get_metadata("mtime"), "2222222222") + self.assertEqual(key.get_metadata("space-in-value"), "ABC XYZ") # removed self.assertEqual(key.content_encoding, None) self.assertEqual(key.get_metadata("with-hypen"), None) diff --git a/src/riak_cs_s3_auth.erl b/src/riak_cs_s3_auth.erl index f6e1da48f..8a7fa215e 100644 --- a/src/riak_cs_s3_auth.erl +++ b/src/riak_cs_s3_auth.erl @@ -38,6 +38,14 @@ -define(QS_KEYID, "AWSAccessKeyId"). -define(QS_SIGNATURE, "Signature"). +-define(PERCENT, 37). % $\% +-define(FULLSTOP, 46). % $\. +-define(QS_SAFE(C), ((C >= $a andalso C =< $z) orelse + (C >= $A andalso C =< $Z) orelse + (C >= $0 andalso C =< $9) orelse + (C =:= ?FULLSTOP orelse C =:= $- orelse C =:= $~ orelse + C =:= $_))). + %% =================================================================== %% Public API %% =================================================================== @@ -336,7 +344,7 @@ canonicalize_qs_v4([{K, V}|T], Acc) -> %% Because keys and values from raw query string that is URL-encoded at client %% side, they should be URL-decoded once and encoded back. strict_url_encode_for_qs_value(Value) -> - mochiweb_util:quote_plus(mochiweb_util:unquote(Value)). + quote_percent(mochiweb_util:unquote(Value)). %% Force string URL encoding for path part of URL. %% Contrary to query part, slashes MUST NOT encoded and left as is. @@ -344,10 +352,31 @@ strict_url_encode_for_path(Path) -> %% Use `binary:split/3' here instead of `string:tokens/2' because %% latter drops information about preceding and trailing slashes. Tokens = binary:split(list_to_binary(Path), <<"/">>, [global]), - EncodedTokens = [mochiweb_util:quote_plus(mochiweb_util:unquote(T)) || + EncodedTokens = [quote_percent(mochiweb_util:unquote(T)) || T <- Tokens], string:join(EncodedTokens, "/"). +-spec quote_percent(string()) -> string(). +%% @doc URL safe encoding of the given string, keeping with the strict +%% specification of AWS S3 because this is used for canonicalizing +%% resource for signature. +quote_percent(String) -> + quote_percent(String, []). + +quote_percent([], Acc) -> + lists:reverse(Acc); +quote_percent([C | Rest], Acc) when ?QS_SAFE(C) -> + quote_percent(Rest, [C | Acc]); +quote_percent([$\s | Rest], Acc) -> + quote_percent(Rest, [$0, $2, ?PERCENT | Acc]); +quote_percent([C | Rest], Acc) -> + <> = <>, + quote_percent(Rest, [hexdigit(Lo), hexdigit(Hi), ?PERCENT | Acc]). + +hexdigit(C) when C < 10 -> $0 + C; +hexdigit(C) when C < 16 -> $A + (C - 10). + + %% =================================================================== %% Eunit tests %% =================================================================== @@ -532,4 +561,13 @@ example_unicode_keys() -> CalculatedSignature = calculate_signature_v2(KeyData, RD), test_fun("example unicode keys test", ExpectedSignature, CalculatedSignature). +quote_percent_test() -> + ?assertEqual("", quote_percent("")), + ?assertEqual("abc", quote_percent("abc")), + ?assertEqual("%20", quote_percent(" ")), + ?assertEqual("abc%20%20xyz%20%0A", quote_percent("abc xyz \n")), + ?assertEqual("%24%2A%3F%0A", quote_percent("$*?\n")), + ?assertEqual("foo%3B%26%3D", quote_percent("foo;&=")), + ok. + -endif.