Fixing signing bug where additional slash could cause a signing misma…

…tch (#112)
awslabs · Apr 13, 2021 · 2159aa8 · 2159aa8
1 parent e0ad63b
commit 2159aa8
Show file tree

Hide file tree

Showing 5 changed files with 67 additions and 39 deletions.
diff --git a/source/s3_util.c b/source/s3_util.c
@@ -216,7 +216,6 @@ void aws_s3_init_default_signing_config(
     signing_config->service = g_s3_service_name;
     signing_config->signed_body_header = AWS_SBHT_X_AMZ_CONTENT_SHA256;
     signing_config->signed_body_value = g_aws_signed_body_value_unsigned_payload;
-    signing_config->flags.should_normalize_uri_path = true;
 }
 
 void replace_quote_entities(struct aws_allocator *allocator, struct aws_string *str, struct aws_byte_buf *out_buf) {

diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt
@@ -70,6 +70,7 @@ add_net_test_case(test_s3_put_object_sse_kms)
 add_net_test_case(test_s3_put_object_sse_kms_multipart)
 add_net_test_case(test_s3_put_object_sse_aes256)
 add_net_test_case(test_s3_put_object_sse_aes256_multipart)
+add_net_test_case(test_s3_put_object_double_slashes)
 add_net_test_case(test_s3_meta_request_default)
 add_net_test_case(test_s3_put_object_fail_headers_callback)
 add_net_test_case(test_s3_put_object_fail_body_callback)

diff --git a/tests/s3_data_plane_tests.c b/tests/s3_data_plane_tests.c
@@ -1621,20 +1621,7 @@ AWS_TEST_CASE(test_s3_put_object_tls_enabled, s_test_s3_put_object_tls_enabled)
 static int s_test_s3_put_object_tls_enabled(struct aws_allocator *allocator, void *ctx) {
     (void)ctx;
 
-    struct aws_s3_meta_request_test_results meta_request_test_results;
-    AWS_ZERO_STRUCT(meta_request_test_results);
-
-    struct aws_s3_tester_meta_request_options options = {
-        .allocator = allocator,
-        .meta_request_type = AWS_S3_META_REQUEST_TYPE_PUT_OBJECT,
-        .validate_type = AWS_S3_TESTER_VALIDATE_TYPE_EXPECT_SUCCESS,
-        .put_options =
-            {
-                .ensure_multipart = true,
-            },
-    };
-
-    ASSERT_SUCCESS(aws_s3_tester_send_meta_request_with_options(NULL, &options, NULL));
+    ASSERT_SUCCESS(s_test_s3_put_object_helper(allocator, AWS_S3_TLS_ENABLED, 0));
 
     return 0;
 }
@@ -1940,6 +1927,30 @@ static int s_test_s3_put_object_sse_aes256_multipart(struct aws_allocator *alloc
     return 0;
 }
 
+AWS_TEST_CASE(test_s3_put_object_double_slashes, s_test_s3_put_object_double_slashes)
+static int s_test_s3_put_object_double_slashes(struct aws_allocator *allocator, void *ctx) {
+    (void)ctx;
+
+    struct aws_s3_meta_request_test_results meta_request_test_results;
+    AWS_ZERO_STRUCT(meta_request_test_results);
+
+    struct aws_s3_tester_meta_request_options options = {
+        .allocator = allocator,
+        .meta_request_type = AWS_S3_META_REQUEST_TYPE_PUT_OBJECT,
+        .put_options =
+            {
+                .object_size_mb = 1,
+                .object_path_override = aws_byte_cursor_from_c_str("/prefix//test.txt"),
+            },
+    };
+
+    ASSERT_SUCCESS(aws_s3_tester_send_meta_request_with_options(NULL, &options, &meta_request_test_results));
+
+    aws_s3_meta_request_test_results_clean_up(&meta_request_test_results);
+
+    return 0;
+}
+
 AWS_TEST_CASE(test_s3_meta_request_default, s_test_s3_meta_request_default)
 static int s_test_s3_meta_request_default(struct aws_allocator *allocator, void *ctx) {
     (void)ctx;

diff --git a/tests/s3_tester.c b/tests/s3_tester.c
@@ -1102,32 +1102,46 @@ int aws_s3_tester_send_meta_request_with_options(
                 input_stream = aws_s3_test_input_stream_new(allocator, object_size_bytes);
             }
 
-            char object_path_buffer[128] = "";
-            switch (options->sse_type) {
-                case AWS_S3_TESTER_SSE_NONE:
-                    snprintf(
-                        object_path_buffer, sizeof(object_path_buffer), "/put_object_test_%uMB.txt", object_size_mb);
-                    break;
-                case AWS_S3_TESTER_SSE_KMS:
-                    snprintf(
-                        object_path_buffer,
-                        sizeof(object_path_buffer),
-                        "/put_object_test_kms_%uMB.txt",
-                        object_size_mb);
-                    break;
-                case AWS_S3_TESTER_SSE_AES256:
-                    snprintf(
-                        object_path_buffer,
-                        sizeof(object_path_buffer),
-                        "/put_object_test_aes256_%uMB.txt",
-                        object_size_mb);
-                    break;
-
-                default:
-                    break;
+            struct aws_byte_buf object_path_buffer;
+            aws_byte_buf_init(&object_path_buffer, allocator, 128);
+
+            if (options->put_options.object_path_override.ptr != NULL) {
+                aws_byte_buf_append_dynamic(&object_path_buffer, &options->put_options.object_path_override);
+            } else {
+                char object_path_sprintf_buffer[128] = "";
+
+                switch (options->sse_type) {
+                    case AWS_S3_TESTER_SSE_NONE:
+                        snprintf(
+                            object_path_sprintf_buffer,
+                            sizeof(object_path_sprintf_buffer),
+                            "/put_object_test_%uMB.txt",
+                            object_size_mb);
+                        break;
+                    case AWS_S3_TESTER_SSE_KMS:
+                        snprintf(
+                            object_path_sprintf_buffer,
+                            sizeof(object_path_sprintf_buffer),
+                            "/put_object_test_kms_%uMB.txt",
+                            object_size_mb);
+                        break;
+                    case AWS_S3_TESTER_SSE_AES256:
+                        snprintf(
+                            object_path_sprintf_buffer,
+                            sizeof(object_path_sprintf_buffer),
+                            "/put_object_test_aes256_%uMB.txt",
+                            object_size_mb);
+                        break;
+
+                    default:
+                        break;
+                }
+
+                struct aws_byte_cursor sprintf_buffer_cursor = aws_byte_cursor_from_c_str(object_path_sprintf_buffer);
+                aws_byte_buf_append_dynamic(&object_path_buffer, &sprintf_buffer_cursor);
             }
 
-            struct aws_byte_cursor test_object_path = aws_byte_cursor_from_c_str(object_path_buffer);
+            struct aws_byte_cursor test_object_path = aws_byte_cursor_from_buf(&object_path_buffer);
 
             /* Put together a simple S3 Put Object request. */
             struct aws_http_message *message = aws_s3_test_put_object_request_new(
@@ -1138,6 +1152,8 @@ int aws_s3_tester_send_meta_request_with_options(
                 input_stream,
                 options->sse_type);
 
+            aws_byte_buf_clean_up(&object_path_buffer);
+
             if (options->put_options.content_length) {
                 /* make a invalid request */
                 char content_length_buffer[64] = "";

diff --git a/tests/s3_tester.h b/tests/s3_tester.h
@@ -151,6 +151,7 @@ struct aws_s3_tester_meta_request_options {
 
     /* Put Object Meta request specific options. */
     struct {
+        struct aws_byte_cursor object_path_override;
         uint32_t object_size_mb;
         bool ensure_multipart;
         bool invalid_request;