From 2159aa85ac409564dfd2781b6dce3d41db72b22d Mon Sep 17 00:00:00 2001 From: Ryan Carper <51676630+rccarper@users.noreply.github.com> Date: Tue, 13 Apr 2021 13:48:46 -0400 Subject: [PATCH] Fixing signing bug where additional slash could cause a signing mismatch (#112) --- source/s3_util.c | 1 - tests/CMakeLists.txt | 1 + tests/s3_data_plane_tests.c | 39 ++++++++++++++-------- tests/s3_tester.c | 64 +++++++++++++++++++++++-------------- tests/s3_tester.h | 1 + 5 files changed, 67 insertions(+), 39 deletions(-) diff --git a/source/s3_util.c b/source/s3_util.c index 81c6693c8..9e74d1703 100644 --- 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 index ac10e2d86..c545a405e 100644 --- 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 index 8c68f7ac6..cf79044bd 100644 --- 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 index 08516826a..3e868f85e 100644 --- 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 index 0f9329c1c..783616928 100644 --- 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;