From f8ffac17b4682b3340c50e40a28d7ff9d5128506 Mon Sep 17 00:00:00 2001 From: Waqar Ahmed Khan Date: Wed, 22 Jan 2025 11:13:26 -0800 Subject: [PATCH 01/19] piping the headers --- include/aws/s3/private/s3_request_messages.h | 3 +++ .../s3express_credentials_provider_impl.h | 5 ++++- .../aws/s3/s3express_credentials_provider.h | 3 +++ source/s3_meta_request.c | 1 + source/s3_request_messages.c | 11 ++++++++++ source/s3express_credentials_provider.c | 22 ++++++++++++++----- .../s3_mock_server_s3express_provider_test.c | 2 +- 7 files changed, 40 insertions(+), 7 deletions(-) diff --git a/include/aws/s3/private/s3_request_messages.h b/include/aws/s3/private/s3_request_messages.h index f7b2b09c4..c83bb84c7 100644 --- a/include/aws/s3/private/s3_request_messages.h +++ b/include/aws/s3/private/s3_request_messages.h @@ -167,6 +167,9 @@ extern const size_t g_s3_complete_multipart_upload_excluded_headers_count; AWS_S3_API extern const struct aws_byte_cursor g_s3_abort_multipart_upload_excluded_headers[]; +AWS_S3_API +extern const struct aws_byte_cursor g_s3_create_session_allowed_headers[]; + AWS_S3_API extern const size_t g_s3_abort_multipart_upload_excluded_headers_count; diff --git a/include/aws/s3/private/s3express_credentials_provider_impl.h b/include/aws/s3/private/s3express_credentials_provider_impl.h index 31f1ef76d..4a0ebb9b1 100644 --- a/include/aws/s3/private/s3express_credentials_provider_impl.h +++ b/include/aws/s3/private/s3express_credentials_provider_impl.h @@ -26,6 +26,8 @@ struct aws_s3express_session { /* The region and host of the session */ struct aws_string *region; struct aws_string *host; + + struct aws_http_headers *headers; bool inactive; /* Only used for mock tests */ @@ -112,7 +114,8 @@ AWS_S3_API struct aws_string *aws_encode_s3express_hash_key_new( struct aws_allocator *allocator, const struct aws_credentials *original_credentials, - struct aws_byte_cursor host_value); + struct aws_byte_cursor host_value, + struct aws_http_headers *headers); AWS_EXTERN_C_END #endif /* AWS_S3EXPRESS_CREDENTIALS_PROVIDER_IMPL_H */ diff --git a/include/aws/s3/s3express_credentials_provider.h b/include/aws/s3/s3express_credentials_provider.h index 316a3e240..408a1cf90 100644 --- a/include/aws/s3/s3express_credentials_provider.h +++ b/include/aws/s3/s3express_credentials_provider.h @@ -27,6 +27,9 @@ struct aws_credentials_properties_s3express { * If empty, the region of the S3 client will be used. */ struct aws_byte_cursor region; + + + struct aws_http_headers *headers; }; struct aws_s3express_credentials_provider_vtable { diff --git a/source/s3_meta_request.c b/source/s3_meta_request.c index 22ba46f56..d9757d152 100644 --- a/source/s3_meta_request.c +++ b/source/s3_meta_request.c @@ -968,6 +968,7 @@ void aws_s3_meta_request_sign_request_default_impl( context->user_data = user_data; context->properties.host = aws_byte_cursor_from_string(meta_request->s3express_session_host); context->properties.region = signing_config.region; + context->properties.headers = aws_http_message_get_headers(meta_request->initial_request_message); if (signing_config.credentials) { context->original_credentials = signing_config.credentials; diff --git a/source/s3_request_messages.c b/source/s3_request_messages.c index 919f28de6..a3275857c 100644 --- a/source/s3_request_messages.c +++ b/source/s3_request_messages.c @@ -229,6 +229,17 @@ const struct aws_byte_cursor g_s3_abort_multipart_upload_excluded_headers[] = { AWS_BYTE_CUR_INIT_FROM_STRING_LITERAL("if-none-match"), }; +const struct aws_byte_cursor g_s3_create_session_allowed_headers[] = { + AWS_BYTE_CUR_INIT_FROM_STRING_LITERAL("x-amz-server-side-encryption"), + AWS_BYTE_CUR_INIT_FROM_STRING_LITERAL("x-amz-server-side-encryption-aws-kms-key-id"), + AWS_BYTE_CUR_INIT_FROM_STRING_LITERAL("x-amz-server-side-encryption-context"), + AWS_BYTE_CUR_INIT_FROM_STRING_LITERAL("x-amz-server-side-encryption-bucket-key-enabled"), + +}; + +const size_t g_s3_create_session_allowed_headers_count = + AWS_ARRAY_SIZE(g_s3_create_session_allowed_headers); + static const struct aws_byte_cursor s_x_amz_meta_prefix = AWS_BYTE_CUR_INIT_FROM_STRING_LITERAL("x-amz-meta-"); static const struct aws_byte_cursor s_checksum_type_header = diff --git a/source/s3express_credentials_provider.c b/source/s3express_credentials_provider.c index 7b0f10020..2b76363ba 100644 --- a/source/s3express_credentials_provider.c +++ b/source/s3express_credentials_provider.c @@ -48,6 +48,7 @@ struct aws_s3express_session_creator { /* The region and host of the session we are creating */ struct aws_string *region; struct aws_string *host; + struct aws_http_headers *headers; struct { /* Protected by the impl lock */ @@ -66,6 +67,7 @@ static struct aws_s3express_session *s_aws_s3express_session_new( const struct aws_string *hash_key, const struct aws_string *region, const struct aws_string *host, + struct aws_http_headers *headers, struct aws_credentials *credentials) { struct aws_s3express_session *session = @@ -74,6 +76,8 @@ static struct aws_s3express_session *s_aws_s3express_session_new( session->impl = provider->impl; session->hash_key = aws_string_new_from_string(provider->allocator, hash_key); session->host = aws_string_new_from_string(provider->allocator, host); + aws_http_headers_acquire(headers); + session->headers = headers; if (region) { session->region = aws_string_new_from_string(provider->allocator, region); } @@ -94,6 +98,7 @@ static void s_aws_s3express_session_destroy(struct aws_s3express_session *sessio aws_string_destroy(session->hash_key); aws_string_destroy(session->region); aws_string_destroy(session->host); + aws_http_headers_release(session->headers); aws_credentials_release(session->s3express_credentials); aws_mem_release(session->allocator, session); } @@ -368,6 +373,7 @@ static void s_on_request_finished( session_creator->hash_key, session_creator->region, session_creator->host, + session_creator->headers, credentials); aws_cache_put(impl->synced_data.cache, session->hash_key, session); } @@ -434,6 +440,7 @@ static struct aws_s3express_session_creator *s_aws_s3express_session_creator_des aws_string_destroy(session_creator->hash_key); aws_string_destroy(session_creator->region); aws_string_destroy(session_creator->host); + aws_http_headers_release(session_creator->headers); aws_byte_buf_clean_up(&session_creator->response_buf); aws_mem_release(session_creator->allocator, session_creator); @@ -447,8 +454,9 @@ static struct aws_s3express_session_creator *s_aws_s3express_session_creator_des struct aws_string *aws_encode_s3express_hash_key_new( struct aws_allocator *allocator, const struct aws_credentials *original_credentials, - struct aws_byte_cursor host_value) { - + struct aws_byte_cursor host_value, + struct aws_http_headers *headers) { + (void)headers; struct aws_byte_buf combine_key_buf; /* 1. Combine access_key and secret_access_key into one buffer */ @@ -500,6 +508,8 @@ static struct aws_s3express_session_creator *s_session_creator_new( session_creator->provider = provider; session_creator->host = aws_string_new_from_cursor(session_creator->allocator, &s3express_properties->host); session_creator->region = aws_string_new_from_cursor(session_creator->allocator, &s3express_properties->region); + aws_http_headers_acquire(s3express_properties->headers); + session_creator->headers = s3express_properties->headers; struct aws_signing_config_aws s3express_signing_config = { .credentials = original_credentials, @@ -556,8 +566,8 @@ static int s_s3express_get_creds( uint64_t current_stamp = UINT64_MAX; aws_sys_clock_get_ticks(¤t_stamp); - struct aws_string *hash_key = - aws_encode_s3express_hash_key_new(provider->allocator, original_credentials, s3express_properties->host); + struct aws_string *hash_key = aws_encode_s3express_hash_key_new( + provider->allocator, original_credentials, s3express_properties->host, s3express_properties->headers); uint64_t now_seconds = aws_timestamp_convert(current_stamp, AWS_TIMESTAMP_NANOS, AWS_TIMESTAMP_SECS, NULL); s_credentials_provider_s3express_impl_lock_synced_data(impl); @@ -764,7 +774,8 @@ static void s_refresh_session_list( struct aws_string *current_creds_hash = aws_encode_s3express_hash_key_new( provider->allocator, current_original_credentials, - aws_byte_cursor_from_string(session->host)); + aws_byte_cursor_from_string(session->host), + session->headers); bool creds_match = aws_string_eq(current_creds_hash, hash_key); aws_string_destroy(current_creds_hash); if (!creds_match) { @@ -784,6 +795,7 @@ static void s_refresh_session_list( struct aws_credentials_properties_s3express s3express_properties = { .host = aws_byte_cursor_from_string(session->host), + .headers = session->headers, }; if (session->region) { s3express_properties.region = aws_byte_cursor_from_string(session->region); diff --git a/tests/s3_mock_server_s3express_provider_test.c b/tests/s3_mock_server_s3express_provider_test.c index 7fc12efca..05c5d461c 100644 --- a/tests/s3_mock_server_s3express_provider_test.c +++ b/tests/s3_mock_server_s3express_provider_test.c @@ -446,7 +446,7 @@ static size_t s_get_index_from_s3express_cache( node = aws_linked_list_next(node); struct aws_s3express_session *session = table_node->value; struct aws_string *hash_key = - aws_encode_s3express_hash_key_new(s_s3express_tester.allocator, original_credentials, host_value); + aws_encode_s3express_hash_key_new(s_s3express_tester.allocator, original_credentials, host_value, session->headers); if (aws_string_eq(session->hash_key, hash_key)) { aws_string_destroy(hash_key); aws_mutex_unlock(&impl->synced_data.lock); From 8076511f30c8371c6196ee08d7d905a34c12014f Mon Sep 17 00:00:00 2001 From: Waqar Ahmed Khan Date: Wed, 22 Jan 2025 13:12:20 -0800 Subject: [PATCH 02/19] More fixes --- include/aws/s3/private/s3_request_messages.h | 3 ++ source/s3_request_messages.c | 1 - source/s3express_credentials_provider.c | 51 ++++++++++++++++---- 3 files changed, 44 insertions(+), 11 deletions(-) diff --git a/include/aws/s3/private/s3_request_messages.h b/include/aws/s3/private/s3_request_messages.h index c83bb84c7..c477484aa 100644 --- a/include/aws/s3/private/s3_request_messages.h +++ b/include/aws/s3/private/s3_request_messages.h @@ -167,6 +167,9 @@ extern const size_t g_s3_complete_multipart_upload_excluded_headers_count; AWS_S3_API extern const struct aws_byte_cursor g_s3_abort_multipart_upload_excluded_headers[]; +AWS_S3_API +extern const size_t g_s3_create_session_allowed_headers_count; + AWS_S3_API extern const struct aws_byte_cursor g_s3_create_session_allowed_headers[]; diff --git a/source/s3_request_messages.c b/source/s3_request_messages.c index a3275857c..3fc5317c2 100644 --- a/source/s3_request_messages.c +++ b/source/s3_request_messages.c @@ -234,7 +234,6 @@ const struct aws_byte_cursor g_s3_create_session_allowed_headers[] = { AWS_BYTE_CUR_INIT_FROM_STRING_LITERAL("x-amz-server-side-encryption-aws-kms-key-id"), AWS_BYTE_CUR_INIT_FROM_STRING_LITERAL("x-amz-server-side-encryption-context"), AWS_BYTE_CUR_INIT_FROM_STRING_LITERAL("x-amz-server-side-encryption-bucket-key-enabled"), - }; const size_t g_s3_create_session_allowed_headers_count = diff --git a/source/s3express_credentials_provider.c b/source/s3express_credentials_provider.c index 2b76363ba..1ee771f93 100644 --- a/source/s3express_credentials_provider.c +++ b/source/s3express_credentials_provider.c @@ -6,9 +6,11 @@ #include "aws/s3/private/s3_client_impl.h" #include "aws/s3/private/s3express_credentials_provider_impl.h" #include +#include #include #include +#include #include #include #include @@ -18,8 +20,6 @@ #include #include -#include - #include static struct aws_byte_cursor s_create_session_path_query = AWS_BYTE_CUR_INIT_FROM_STRING_LITERAL("/?session="); @@ -394,7 +394,8 @@ static void s_on_request_finished( static struct aws_http_message *s_create_session_request_new( struct aws_allocator *allocator, - struct aws_byte_cursor host_value) { + struct aws_byte_cursor host_value, + struct aws_http_headers *headers) { struct aws_http_message *request = aws_http_message_new_request(allocator); struct aws_http_header host_header = { @@ -414,6 +415,20 @@ static struct aws_http_message *s_create_session_request_new( goto error; } + for (size_t header_index = 0; header_index < g_s3_create_session_allowed_headers_count; ++header_index) { + struct aws_byte_cursor header_name = g_s3_create_session_allowed_headers[header_index]; + struct aws_byte_cursor header_value; + if (aws_http_headers_get(headers, header_name, &header_value) == AWS_OP_SUCCESS && header_value.len > 0) { + struct aws_http_header header = { + .name = header_name, + .value = header_value, + }; + if (aws_http_message_add_header(request, header)) { + goto error; + } + } + } + if (aws_http_message_set_request_method(request, aws_http_method_get)) { goto error; } @@ -450,6 +465,7 @@ static struct aws_s3express_session_creator *s_aws_s3express_session_creator_des /** * Encode the hash key to be [host_value][hash_of_credentials] * hash_of_credentials is the sha256 of [access_key][secret_access_key] + * TODO: Update docs **/ struct aws_string *aws_encode_s3express_hash_key_new( struct aws_allocator *allocator, @@ -457,17 +473,31 @@ struct aws_string *aws_encode_s3express_hash_key_new( struct aws_byte_cursor host_value, struct aws_http_headers *headers) { (void)headers; - struct aws_byte_buf combine_key_buf; + struct aws_byte_buf combined_hash_buf; /* 1. Combine access_key and secret_access_key into one buffer */ struct aws_byte_cursor access_key = aws_credentials_get_access_key_id(original_credentials); struct aws_byte_cursor secret_access_key = aws_credentials_get_secret_access_key(original_credentials); - aws_byte_buf_init(&combine_key_buf, allocator, access_key.len + secret_access_key.len); - aws_byte_buf_write_from_whole_cursor(&combine_key_buf, access_key); - aws_byte_buf_write_from_whole_cursor(&combine_key_buf, secret_access_key); + aws_byte_buf_init(&combined_hash_buf, allocator, access_key.len + secret_access_key.len); + aws_byte_buf_write_from_whole_cursor(&combined_hash_buf, access_key); + aws_byte_buf_write_from_whole_cursor(&combined_hash_buf, secret_access_key); + + /* Write the allowed headers into hash */ + struct aws_byte_cursor collon = aws_byte_cursor_from_c_str(":"); + struct aws_byte_cursor comma = aws_byte_cursor_from_c_str(","); + for (size_t header_index = 0; header_index < g_s3_create_session_allowed_headers_count; ++header_index) { + struct aws_byte_cursor header_name = g_s3_create_session_allowed_headers[header_index]; + struct aws_byte_cursor header_value; + if (aws_http_headers_get(headers, header_name, &header_value) == AWS_OP_SUCCESS && header_value.len > 0) { + aws_byte_buf_write_from_whole_cursor(&combined_hash_buf, comma); + aws_byte_buf_write_from_whole_cursor(&combined_hash_buf, header_name); + aws_byte_buf_write_from_whole_cursor(&combined_hash_buf, collon); + aws_byte_buf_write_from_whole_cursor(&combined_hash_buf, header_value); + } + } /* 2. Get sha256 digest from the combined key */ - struct aws_byte_cursor combine_key = aws_byte_cursor_from_buf(&combine_key_buf); + struct aws_byte_cursor combine_key = aws_byte_cursor_from_buf(&combined_hash_buf); struct aws_byte_buf digest_buf; aws_byte_buf_init(&digest_buf, allocator, AWS_SHA256_LEN); aws_sha256_compute(allocator, &combine_key, &digest_buf, 0); @@ -481,7 +511,7 @@ struct aws_string *aws_encode_s3express_hash_key_new( /* Clean up */ aws_byte_buf_clean_up(&result_buffer); - aws_byte_buf_clean_up(&combine_key_buf); + aws_byte_buf_clean_up(&combined_hash_buf); aws_byte_buf_clean_up(&digest_buf); return result; @@ -493,7 +523,8 @@ static struct aws_s3express_session_creator *s_session_creator_new( const struct aws_credentials_properties_s3express *s3express_properties) { struct aws_s3express_credentials_provider_impl *impl = provider->impl; - struct aws_http_message *request = s_create_session_request_new(provider->allocator, s3express_properties->host); + struct aws_http_message *request = + s_create_session_request_new(provider->allocator, s3express_properties->host, s3express_properties->headers); if (!request) { return NULL; } From dfc4a8d4528634b024d55f33bc72e8cbdfdd4c51 Mon Sep 17 00:00:00 2001 From: Waqar Ahmed Khan Date: Wed, 22 Jan 2025 13:41:11 -0800 Subject: [PATCH 03/19] null safety --- source/s3express_credentials_provider.c | 57 ++++++++++++++----------- 1 file changed, 32 insertions(+), 25 deletions(-) diff --git a/source/s3express_credentials_provider.c b/source/s3express_credentials_provider.c index 1ee771f93..674ce271a 100644 --- a/source/s3express_credentials_provider.c +++ b/source/s3express_credentials_provider.c @@ -76,8 +76,10 @@ static struct aws_s3express_session *s_aws_s3express_session_new( session->impl = provider->impl; session->hash_key = aws_string_new_from_string(provider->allocator, hash_key); session->host = aws_string_new_from_string(provider->allocator, host); - aws_http_headers_acquire(headers); - session->headers = headers; + if (headers != NULL) { + aws_http_headers_acquire(headers); + session->headers = headers; + } if (region) { session->region = aws_string_new_from_string(provider->allocator, region); } @@ -414,17 +416,18 @@ static struct aws_http_message *s_create_session_request_new( if (aws_http_message_add_header(request, user_agent_header)) { goto error; } - - for (size_t header_index = 0; header_index < g_s3_create_session_allowed_headers_count; ++header_index) { - struct aws_byte_cursor header_name = g_s3_create_session_allowed_headers[header_index]; - struct aws_byte_cursor header_value; - if (aws_http_headers_get(headers, header_name, &header_value) == AWS_OP_SUCCESS && header_value.len > 0) { - struct aws_http_header header = { - .name = header_name, - .value = header_value, - }; - if (aws_http_message_add_header(request, header)) { - goto error; + if (headers != NULL) { + for (size_t header_index = 0; header_index < g_s3_create_session_allowed_headers_count; ++header_index) { + struct aws_byte_cursor header_name = g_s3_create_session_allowed_headers[header_index]; + struct aws_byte_cursor header_value; + if (aws_http_headers_get(headers, header_name, &header_value) == AWS_OP_SUCCESS && header_value.len > 0) { + struct aws_http_header header = { + .name = header_name, + .value = header_value, + }; + if (aws_http_message_add_header(request, header)) { + goto error; + } } } } @@ -483,16 +486,18 @@ struct aws_string *aws_encode_s3express_hash_key_new( aws_byte_buf_write_from_whole_cursor(&combined_hash_buf, secret_access_key); /* Write the allowed headers into hash */ - struct aws_byte_cursor collon = aws_byte_cursor_from_c_str(":"); - struct aws_byte_cursor comma = aws_byte_cursor_from_c_str(","); - for (size_t header_index = 0; header_index < g_s3_create_session_allowed_headers_count; ++header_index) { - struct aws_byte_cursor header_name = g_s3_create_session_allowed_headers[header_index]; - struct aws_byte_cursor header_value; - if (aws_http_headers_get(headers, header_name, &header_value) == AWS_OP_SUCCESS && header_value.len > 0) { - aws_byte_buf_write_from_whole_cursor(&combined_hash_buf, comma); - aws_byte_buf_write_from_whole_cursor(&combined_hash_buf, header_name); - aws_byte_buf_write_from_whole_cursor(&combined_hash_buf, collon); - aws_byte_buf_write_from_whole_cursor(&combined_hash_buf, header_value); + if (headers != NULL) { + struct aws_byte_cursor collon = aws_byte_cursor_from_c_str(":"); + struct aws_byte_cursor comma = aws_byte_cursor_from_c_str(","); + for (size_t header_index = 0; header_index < g_s3_create_session_allowed_headers_count; ++header_index) { + struct aws_byte_cursor header_name = g_s3_create_session_allowed_headers[header_index]; + struct aws_byte_cursor header_value; + if (aws_http_headers_get(headers, header_name, &header_value) == AWS_OP_SUCCESS && header_value.len > 0) { + aws_byte_buf_write_from_whole_cursor(&combined_hash_buf, comma); + aws_byte_buf_write_from_whole_cursor(&combined_hash_buf, header_name); + aws_byte_buf_write_from_whole_cursor(&combined_hash_buf, collon); + aws_byte_buf_write_from_whole_cursor(&combined_hash_buf, header_value); + } } } @@ -539,8 +544,10 @@ static struct aws_s3express_session_creator *s_session_creator_new( session_creator->provider = provider; session_creator->host = aws_string_new_from_cursor(session_creator->allocator, &s3express_properties->host); session_creator->region = aws_string_new_from_cursor(session_creator->allocator, &s3express_properties->region); - aws_http_headers_acquire(s3express_properties->headers); - session_creator->headers = s3express_properties->headers; + if (s3express_properties->headers != NULL) { + aws_http_headers_acquire(s3express_properties->headers); + session_creator->headers = s3express_properties->headers; + } struct aws_signing_config_aws s3express_signing_config = { .credentials = original_credentials, From b3f48c767bc9a77a227db599431cfe6c14b82192 Mon Sep 17 00:00:00 2001 From: Waqar Ahmed Khan Date: Wed, 22 Jan 2025 13:42:50 -0800 Subject: [PATCH 04/19] lint --- include/aws/s3/s3express_credentials_provider.h | 1 - source/s3_request_messages.c | 3 +-- tests/s3_mock_server_s3express_provider_test.c | 4 ++-- 3 files changed, 3 insertions(+), 5 deletions(-) diff --git a/include/aws/s3/s3express_credentials_provider.h b/include/aws/s3/s3express_credentials_provider.h index 408a1cf90..d33bcd0c0 100644 --- a/include/aws/s3/s3express_credentials_provider.h +++ b/include/aws/s3/s3express_credentials_provider.h @@ -28,7 +28,6 @@ struct aws_credentials_properties_s3express { */ struct aws_byte_cursor region; - struct aws_http_headers *headers; }; diff --git a/source/s3_request_messages.c b/source/s3_request_messages.c index 3fc5317c2..4cc7945bf 100644 --- a/source/s3_request_messages.c +++ b/source/s3_request_messages.c @@ -236,8 +236,7 @@ const struct aws_byte_cursor g_s3_create_session_allowed_headers[] = { AWS_BYTE_CUR_INIT_FROM_STRING_LITERAL("x-amz-server-side-encryption-bucket-key-enabled"), }; -const size_t g_s3_create_session_allowed_headers_count = - AWS_ARRAY_SIZE(g_s3_create_session_allowed_headers); +const size_t g_s3_create_session_allowed_headers_count = AWS_ARRAY_SIZE(g_s3_create_session_allowed_headers); static const struct aws_byte_cursor s_x_amz_meta_prefix = AWS_BYTE_CUR_INIT_FROM_STRING_LITERAL("x-amz-meta-"); diff --git a/tests/s3_mock_server_s3express_provider_test.c b/tests/s3_mock_server_s3express_provider_test.c index 05c5d461c..004944935 100644 --- a/tests/s3_mock_server_s3express_provider_test.c +++ b/tests/s3_mock_server_s3express_provider_test.c @@ -445,8 +445,8 @@ static size_t s_get_index_from_s3express_cache( AWS_CONTAINER_OF(node, struct aws_linked_hash_table_node, node); node = aws_linked_list_next(node); struct aws_s3express_session *session = table_node->value; - struct aws_string *hash_key = - aws_encode_s3express_hash_key_new(s_s3express_tester.allocator, original_credentials, host_value, session->headers); + struct aws_string *hash_key = aws_encode_s3express_hash_key_new( + s_s3express_tester.allocator, original_credentials, host_value, session->headers); if (aws_string_eq(session->hash_key, hash_key)) { aws_string_destroy(hash_key); aws_mutex_unlock(&impl->synced_data.lock); From 7d6b98adaeafc9f588b7a64110224ea4c33b1df7 Mon Sep 17 00:00:00 2001 From: Waqar Ahmed Khan Date: Thu, 23 Jan 2025 13:44:57 -0800 Subject: [PATCH 05/19] Add test and update mock server --- source/s3express_credentials_provider.c | 30 ++++--- tests/CMakeLists.txt | 1 + .../CompleteMultipartUpload/sse_kms.json | 14 ++++ .../mock_s3_server/CreateSession/sse_kms.json | 21 +++++ tests/mock_s3_server/mock_s3_server.py | 14 +++- .../s3_mock_server_s3express_provider_test.c | 83 +++++++++++++++++++ tests/s3_tester.c | 3 + tests/s3_tester.h | 2 + 8 files changed, 156 insertions(+), 12 deletions(-) create mode 100644 tests/mock_s3_server/CompleteMultipartUpload/sse_kms.json create mode 100644 tests/mock_s3_server/CreateSession/sse_kms.json diff --git a/source/s3express_credentials_provider.c b/source/s3express_credentials_provider.c index 674ce271a..d22ff8ea4 100644 --- a/source/s3express_credentials_provider.c +++ b/source/s3express_credentials_provider.c @@ -397,7 +397,8 @@ static void s_on_request_finished( static struct aws_http_message *s_create_session_request_new( struct aws_allocator *allocator, struct aws_byte_cursor host_value, - struct aws_http_headers *headers) { + struct aws_http_headers *headers, + const struct aws_uri *endpoint_override) { struct aws_http_message *request = aws_http_message_new_request(allocator); struct aws_http_header host_header = { @@ -405,8 +406,11 @@ static struct aws_http_message *s_create_session_request_new( .value = host_value, }; - if (aws_http_message_add_header(request, host_header)) { - goto error; + /* NOTE: ONLY FOR TESTS. Don't add the host header for endpoint override. */ + if (endpoint_override == NULL) { + if (aws_http_message_add_header(request, host_header)) { + goto error; + } } struct aws_http_header user_agent_header = { @@ -436,7 +440,14 @@ static struct aws_http_message *s_create_session_request_new( goto error; } - if (aws_http_message_set_request_path(request, s_create_session_path_query)) { + struct aws_byte_cursor path_and_query = s_create_session_path_query; + if (endpoint_override != NULL) { + const struct aws_byte_cursor *override_path_query = aws_uri_path_and_query(endpoint_override); + if (override_path_query->len > 0) { + path_and_query = *override_path_query; + } + } + if (aws_http_message_set_request_path(request, path_and_query)) { goto error; } return request; @@ -528,15 +539,14 @@ static struct aws_s3express_session_creator *s_session_creator_new( const struct aws_credentials_properties_s3express *s3express_properties) { struct aws_s3express_credentials_provider_impl *impl = provider->impl; - struct aws_http_message *request = - s_create_session_request_new(provider->allocator, s3express_properties->host, s3express_properties->headers); + struct aws_http_message *request = s_create_session_request_new( + provider->allocator, + s3express_properties->host, + s3express_properties->headers, + impl->mock_test.endpoint_override); if (!request) { return NULL; } - if (impl->mock_test.endpoint_override) { - /* NOTE: ONLY FOR TESTS. Erase the host header for endpoint override. */ - aws_http_headers_erase(aws_http_message_get_headers(request), g_host_header_name); - } struct aws_s3express_session_creator *session_creator = aws_mem_calloc(provider->allocator, 1, sizeof(struct aws_s3express_session_creator)); diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index ad68c2989..a4f2c52c8 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -351,6 +351,7 @@ if(ENABLE_MOCK_SERVER_TESTS) add_net_test_case(s3express_provider_background_refresh_mock_server) add_net_test_case(s3express_provider_background_refresh_remove_inactive_creds_mock_server) add_net_test_case(s3express_provider_stress_mock_server) + add_net_test_case(s3express_provider_get_credentials_kms_headers_mock_server) add_net_test_case(s3express_client_sanity_test_mock_server) add_net_test_case(s3express_client_sanity_override_test_mock_server) diff --git a/tests/mock_s3_server/CompleteMultipartUpload/sse_kms.json b/tests/mock_s3_server/CompleteMultipartUpload/sse_kms.json new file mode 100644 index 000000000..d307c7aa6 --- /dev/null +++ b/tests/mock_s3_server/CompleteMultipartUpload/sse_kms.json @@ -0,0 +1,14 @@ +{ + "status": 200, + "headers": {"Connection": "close", + "x-amz-server-side-encryption": "aws:kms"}, + "body": [ + "", + "", + "http://default.s3.us-west-2.amazonaws.com/default", + "default", + "default", + "\"3858f62230ac3c915f300c664312c11f-9\"", + "" + ] +} diff --git a/tests/mock_s3_server/CreateSession/sse_kms.json b/tests/mock_s3_server/CreateSession/sse_kms.json new file mode 100644 index 000000000..49a5add2c --- /dev/null +++ b/tests/mock_s3_server/CreateSession/sse_kms.json @@ -0,0 +1,21 @@ +{ + "status": 200, + "headers": { + "x-amz-request-id": "12345", + "x-amz-server-side-encryption": "aws:kms" + }, + "request_headers": { + "x-amz-server-side-encryption": "aws:kms" + }, + "body": [ + "", + "", + "", + "sessionToken", + "secretKey", + "accessKeyId", + "2023-06-26T17:33:30Z", + "", + "" + ] +} diff --git a/tests/mock_s3_server/mock_s3_server.py b/tests/mock_s3_server/mock_s3_server.py index 58f36bfa4..3a7fbcd81 100644 --- a/tests/mock_s3_server/mock_s3_server.py +++ b/tests/mock_s3_server/mock_s3_server.py @@ -56,7 +56,7 @@ class ResponseConfig: json_path: str = None throttle: bool = False force_retry: bool = False - + request_headers = None def _resolve_file_path(self, wrapper, request_type): global SHOULD_THROTTLE if self.json_path is None: @@ -85,6 +85,16 @@ def resolve_response(self, wrapper, request_type, chunked=False, head_request=Fa ".\n generate_body_size: ", self.generate_body_size) with open(self.json_path, 'r') as f: data = json.load(f) + headers = wrapper.basic_headers() + + # If request_headers is present, validate that the request contains all required headers + if 'request_headers' in data: + for header in data['request_headers']: + header_bytes = header.encode('utf-8') + if not any(header_bytes == h[0] for h in self.request_headers): + response = Response(status_code=500, delay=0, headers=headers, + data=json.dumps({'error': f"Missing required header: {header}"}), chunked=chunked, head_request=head_request) + return response # if response has delay, then sleep before sending it delay = data.get('delay', 0) @@ -95,7 +105,6 @@ def resolve_response(self, wrapper, request_type, chunked=False, head_request=Fa else: body = "\n".join(data['body']) - headers = wrapper.basic_headers() content_length_set = False for header in data['headers'].items(): headers.append((header[0], str(header[1]))) @@ -499,6 +508,7 @@ async def handle_mock_s3_request(wrapper, request): if response_config is None: response_config = ResponseConfig(parsed_path.path) + response_config.request_headers = request.headers response = response_config.resolve_response( wrapper, request_type, head_request=method == "HEAD") diff --git a/tests/s3_mock_server_s3express_provider_test.c b/tests/s3_mock_server_s3express_provider_test.c index 004944935..ddc7400cf 100644 --- a/tests/s3_mock_server_s3express_provider_test.c +++ b/tests/s3_mock_server_s3express_provider_test.c @@ -45,6 +45,8 @@ struct aws_s3express_provider_tester { struct aws_uri mock_server; struct aws_s3_client *client; + aws_simple_completion_callback *on_provider_shutdown_callback; + void *shutdown_user_data; int error_code; }; @@ -252,6 +254,87 @@ TEST_CASE(s3express_provider_get_credentials_mock_server) { return AWS_OP_SUCCESS; } +struct aws_s3express_credentials_provider *s_s3express_credentials_provider_factory( + struct aws_allocator *allocator, + struct aws_s3_client *client, + aws_simple_completion_callback on_provider_shutdown_callback, + void *shutdown_user_data, + void *factory_user_data) { + (void)allocator; + (void)client; + (void)on_provider_shutdown_callback; + (void)shutdown_user_data; + (void)factory_user_data; + s_s3express_tester.on_provider_shutdown_callback = on_provider_shutdown_callback; + s_s3express_tester.shutdown_user_data = shutdown_user_data; + struct aws_s3express_credentials_provider_default_options options = { + .client = client, + // .shutdown_complete_callback = s_on_shutdown_complete, + // .shutdown_user_data = &s_s3express_tester, + .mock_test.bg_refresh_secs_override = s_bg_refresh_secs_override, + }; + struct aws_s3express_credentials_provider *provider = + aws_s3express_credentials_provider_new_default(allocator, &options); + struct aws_s3express_credentials_provider_impl *impl = provider->impl; + impl->mock_test.endpoint_override = &s_s3express_tester.mock_server; + impl->mock_test.s3express_session_is_valid_override = s_s3express_session_always_true; + + return provider; +} + +TEST_CASE(s3express_provider_get_credentials_kms_headers_mock_server) { + (void)ctx; + + struct aws_s3_tester tester; + ASSERT_SUCCESS(aws_s3_tester_init(allocator, &tester)); + ASSERT_SUCCESS(s_s3express_tester_init(allocator)); + + struct aws_s3_tester_client_options client_options = { + .part_size = MB_TO_BYTES(5), + .tls_usage = AWS_S3_TLS_DISABLED, + .s3express_provider_override_factory = s_s3express_credentials_provider_factory, + .factory_user_data = NULL, + }; + + struct aws_s3_client *client = NULL; + ASSERT_SUCCESS(aws_s3_tester_client_new(&tester, &client_options, &client)); + + struct aws_byte_cursor object_path = aws_byte_cursor_from_c_str("/sse_kms"); + char uri[1024] = {'\0'}; + snprintf(uri, sizeof(uri), "" PRInSTR "sse_kms?session=", AWS_BYTE_CURSOR_PRI(g_mock_server_uri)); + struct aws_byte_cursor uri_cursor = aws_byte_cursor_from_c_str(uri); + aws_uri_clean_up(&s_s3express_tester.mock_server); + ASSERT_SUCCESS(aws_uri_init_parse(&s_s3express_tester.mock_server, allocator, &uri_cursor)); + + struct aws_s3_tester_meta_request_options put_options = { + .allocator = allocator, + .meta_request_type = AWS_S3_META_REQUEST_TYPE_PUT_OBJECT, + .client = client, + .checksum_algorithm = AWS_SCA_CRC32, + .validate_get_response_checksum = false, + .put_options = + { + .object_size_mb = 10, + .object_path_override = object_path, + }, + .mock_server = true, + .use_s3express_signing = true, + .sse_type = AWS_S3_TESTER_SSE_KMS, + }; + struct aws_s3_meta_request_test_results out_results; + aws_s3_meta_request_test_results_init(&out_results, allocator); + ASSERT_SUCCESS(aws_s3_tester_send_meta_request_with_options(&tester, &put_options, &out_results)); + aws_s3_meta_request_test_results_clean_up(&out_results); + aws_s3_client_release(client); + + /* Call the provider shutdown callback to finish cleanup */ + s_s3express_tester.on_provider_shutdown_callback(s_s3express_tester.shutdown_user_data); + ASSERT_SUCCESS(s_s3express_tester_cleanup()); + aws_s3_tester_clean_up(&tester); + + return AWS_OP_SUCCESS; +} + TEST_CASE(s3express_provider_get_credentials_multiple_mock_server) { (void)ctx; diff --git a/tests/s3_tester.c b/tests/s3_tester.c index 5232f68bc..96c837b48 100644 --- a/tests/s3_tester.c +++ b/tests/s3_tester.c @@ -1387,6 +1387,9 @@ int aws_s3_tester_client_new( struct aws_s3_client_config client_config = { .part_size = options->part_size, .max_part_size = options->max_part_size, + .s3express_provider_override_factory = options->s3express_provider_override_factory, + .factory_user_data = options->factory_user_data, + .enable_s3express = options->s3express_provider_override_factory != NULL, }; struct aws_http_proxy_options proxy_options = { .connection_type = AWS_HPCT_HTTP_FORWARD, diff --git a/tests/s3_tester.h b/tests/s3_tester.h index 71383c56d..5d27ab6fe 100644 --- a/tests/s3_tester.h +++ b/tests/s3_tester.h @@ -140,6 +140,8 @@ struct aws_s3_tester_client_options { size_t num_network_interface_names; uint32_t setup_region : 1; uint32_t use_proxy : 1; + aws_s3express_provider_factory_fn *s3express_provider_override_factory; + void *factory_user_data; }; /* should really break this up to a client setup, and a meta_request sending */ From ff8577b1296ec7df0109476fe8292977109274b0 Mon Sep 17 00:00:00 2001 From: Waqar Ahmed Khan Date: Thu, 23 Jan 2025 13:47:17 -0800 Subject: [PATCH 06/19] update readme --- tests/mock_s3_server/README.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/mock_s3_server/README.md b/tests/mock_s3_server/README.md index f0c152309..5b8fcbbd2 100644 --- a/tests/mock_s3_server/README.md +++ b/tests/mock_s3_server/README.md @@ -25,6 +25,7 @@ The server will read from ./{OperationName}/{Key}.json. The json file is formatt { "status": 200, "headers": {"Connection": "close"}, + "request_headers:" {"HeaderA": "ValueA"} "body": [ "", "", @@ -40,6 +41,7 @@ The server will read from ./{OperationName}/{Key}.json. The json file is formatt Where you can define the expected response status, header and response body. If the {Key}.json is not found from file system, it will load the `default.json`. +The server validates that all specified headers in the "request_headers" field are present in the incoming request. If any required header is missing, the request will fail. These headers will not be part of the Response headers. If the "delay" field is present, the response will be delayed by X seconds. ### GetObject Response From bea7dc46b695d532969ac43a1b0c923af785e36d Mon Sep 17 00:00:00 2001 From: Waqar Ahmed Khan Date: Thu, 23 Jan 2025 14:10:00 -0800 Subject: [PATCH 07/19] Fix tests --- source/s3express_credentials_provider.c | 12 ++++++++---- tests/s3_mock_server_s3express_provider_test.c | 2 -- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/source/s3express_credentials_provider.c b/source/s3express_credentials_provider.c index d22ff8ea4..a8b459dbb 100644 --- a/source/s3express_credentials_provider.c +++ b/source/s3express_credentials_provider.c @@ -405,8 +405,9 @@ static struct aws_http_message *s_create_session_request_new( .name = g_host_header_name, .value = host_value, }; - - /* NOTE: ONLY FOR TESTS. Don't add the host header for endpoint override. */ + /* NOTE: Only for Tests. + * Don't add the host header for endpoint override. + */ if (endpoint_override == NULL) { if (aws_http_message_add_header(request, host_header)) { goto error; @@ -443,7 +444,10 @@ static struct aws_http_message *s_create_session_request_new( struct aws_byte_cursor path_and_query = s_create_session_path_query; if (endpoint_override != NULL) { const struct aws_byte_cursor *override_path_query = aws_uri_path_and_query(endpoint_override); - if (override_path_query->len > 0) { + /* NOTE: Only for Tests. + * path_and_query is at least 1 due to /. Only override if its length is more than 1 + */ + if (override_path_query->len > 1) { path_and_query = *override_path_query; } } @@ -486,7 +490,7 @@ struct aws_string *aws_encode_s3express_hash_key_new( const struct aws_credentials *original_credentials, struct aws_byte_cursor host_value, struct aws_http_headers *headers) { - (void)headers; + struct aws_byte_buf combined_hash_buf; /* 1. Combine access_key and secret_access_key into one buffer */ diff --git a/tests/s3_mock_server_s3express_provider_test.c b/tests/s3_mock_server_s3express_provider_test.c index ddc7400cf..39adefe2a 100644 --- a/tests/s3_mock_server_s3express_provider_test.c +++ b/tests/s3_mock_server_s3express_provider_test.c @@ -269,8 +269,6 @@ struct aws_s3express_credentials_provider *s_s3express_credentials_provider_fact s_s3express_tester.shutdown_user_data = shutdown_user_data; struct aws_s3express_credentials_provider_default_options options = { .client = client, - // .shutdown_complete_callback = s_on_shutdown_complete, - // .shutdown_user_data = &s_s3express_tester, .mock_test.bg_refresh_secs_override = s_bg_refresh_secs_override, }; struct aws_s3express_credentials_provider *provider = From 30e9a18e6c1257f232c068dfe6d6d3edb2490a23 Mon Sep 17 00:00:00 2001 From: Waqar Ahmed Khan Date: Fri, 24 Jan 2025 09:47:45 -0800 Subject: [PATCH 08/19] Add hash key test --- .../s3express_credentials_provider_impl.h | 8 ++- source/s3express_credentials_provider.c | 5 -- tests/CMakeLists.txt | 1 + tests/s3_s3express_client_test.c | 51 +++++++++++++++++++ 4 files changed, 58 insertions(+), 7 deletions(-) diff --git a/include/aws/s3/private/s3express_credentials_provider_impl.h b/include/aws/s3/private/s3express_credentials_provider_impl.h index 4a0ebb9b1..7bc48f94b 100644 --- a/include/aws/s3/private/s3express_credentials_provider_impl.h +++ b/include/aws/s3/private/s3express_credentials_provider_impl.h @@ -107,8 +107,12 @@ struct aws_s3express_credentials_provider *aws_s3express_credentials_provider_ne const struct aws_s3express_credentials_provider_default_options *options); /** - * Encode the hash key to be [host_value][hash_of_credentials] - * hash_of_credentials is the sha256 of [access_key][secret_access_key] + * Encodes the hash key in the format: [host_value][hash_of_credentials_and_headers] + * + * The hash_of_credentials_and_headers is calculated as follows: + * 1. Concatenate: [access_key][secret_access_key][headers] + * where headers = ",header_name1:header_value1,header_name2:header_value2..." + * 2. Generates SHA256 hash of the concatenated string */ AWS_S3_API struct aws_string *aws_encode_s3express_hash_key_new( diff --git a/source/s3express_credentials_provider.c b/source/s3express_credentials_provider.c index a8b459dbb..1b144aa4d 100644 --- a/source/s3express_credentials_provider.c +++ b/source/s3express_credentials_provider.c @@ -480,11 +480,6 @@ static struct aws_s3express_session_creator *s_aws_s3express_session_creator_des return NULL; } -/** - * Encode the hash key to be [host_value][hash_of_credentials] - * hash_of_credentials is the sha256 of [access_key][secret_access_key] - * TODO: Update docs - **/ struct aws_string *aws_encode_s3express_hash_key_new( struct aws_allocator *allocator, const struct aws_credentials *original_credentials, diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index a4f2c52c8..91104a07e 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -370,6 +370,7 @@ add_net_test_case(s3express_client_get_object_multiple) add_net_test_case(s3express_client_get_object_create_session_error) add_net_test_case(s3express_client_copy_object) add_net_test_case(s3express_client_copy_object_multipart) +add_net_test_case(s3express_hash_key_test) add_net_test_case(meta_request_auto_ranged_get_new_error_handling) add_net_test_case(meta_request_auto_ranged_put_new_error_handling) diff --git a/tests/s3_s3express_client_test.c b/tests/s3_s3express_client_test.c index a036edced..4c1b621e0 100644 --- a/tests/s3_s3express_client_test.c +++ b/tests/s3_s3express_client_test.c @@ -10,6 +10,7 @@ #include "s3_tester.h" #include #include +#include #include #include #include @@ -714,3 +715,53 @@ TEST_CASE(s3express_client_copy_object_multipart) { aws_s3_tester_clean_up(&tester); return AWS_OP_SUCCESS; } + +/** + * Test hash of the express cache key + */ +TEST_CASE(s3express_hash_key_test) { + (void)ctx; + + struct aws_string *access_key = aws_string_new_from_c_str(allocator, "AccessKey"); + struct aws_string *secret_access_key = aws_string_new_from_c_str(allocator, "SecretAccessKey"); + struct aws_http_headers *headers = aws_http_headers_new(allocator); + aws_http_headers_add( + headers, aws_byte_cursor_from_c_str("x-amz-server-side-encryption"), aws_byte_cursor_from_c_str("aws:kms")); + aws_http_headers_add( + headers, + aws_byte_cursor_from_c_str("x-amz-server-side-encryption-aws-kms-key-id"), + aws_byte_cursor_from_c_str("kms-key-id")); + aws_http_headers_add( + headers, + aws_byte_cursor_from_c_str("x-amz-server-side-encryption-context"), + aws_byte_cursor_from_c_str("context")); + aws_http_headers_add( + headers, + aws_byte_cursor_from_c_str("x-amz-server-side-encryption-bucket-key-enabled"), + aws_byte_cursor_from_c_str("true")); + aws_http_headers_add( + headers, aws_byte_cursor_from_c_str("header-not-allowed"), aws_byte_cursor_from_c_str("should-be-ignored")); + + struct aws_credentials *creds = + aws_credentials_new_from_string(allocator, access_key, secret_access_key, NULL, UINT64_MAX); + + struct aws_string *hash_key = + aws_encode_s3express_hash_key_new(allocator, creds, aws_byte_cursor_from_c_str("host"), headers); + struct aws_byte_cursor hash_cursor = aws_byte_cursor_from_string(hash_key); + + struct aws_byte_buf encoded_buf; + aws_byte_buf_init(&encoded_buf, allocator, 100); + aws_hex_encode_append_dynamic(&hash_cursor, &encoded_buf); + + char *expected_encoded_key = "686f737498ae6a365790707488b3e85402c9eddf422dc39f096e15eaba0d7cdd45f57ad2"; + ASSERT_BIN_ARRAYS_EQUALS(expected_encoded_key, strlen(expected_encoded_key), encoded_buf.buffer, encoded_buf.len); + + aws_byte_buf_clean_up(&encoded_buf); + aws_string_destroy(access_key); + aws_string_destroy(secret_access_key); + aws_credentials_release(creds); + aws_string_destroy(hash_key); + aws_http_headers_release(headers); + + return 0; +} From 2d74e6e0040f06c87e42da798cb0641cef9f3cef Mon Sep 17 00:00:00 2001 From: Waqar Ahmed Khan Date: Fri, 24 Jan 2025 10:00:12 -0800 Subject: [PATCH 09/19] Add support for session mode --- source/s3_request_messages.c | 8 ++++++++ source/s3express_credentials_provider.c | 22 +++++++++++----------- tests/s3_s3express_client_test.c | 14 +++++++++----- 3 files changed, 28 insertions(+), 16 deletions(-) diff --git a/source/s3_request_messages.c b/source/s3_request_messages.c index 4cc7945bf..69d8450f8 100644 --- a/source/s3_request_messages.c +++ b/source/s3_request_messages.c @@ -27,6 +27,7 @@ const struct aws_byte_cursor g_s3_create_multipart_upload_excluded_headers[] = { AWS_BYTE_CUR_INIT_FROM_STRING_LITERAL("x-amz-checksum-sha1"), AWS_BYTE_CUR_INIT_FROM_STRING_LITERAL("x-amz-checksum-sha256"), AWS_BYTE_CUR_INIT_FROM_STRING_LITERAL("if-none-match"), + AWS_BYTE_CUR_INIT_FROM_STRING_LITERAL("x-amz-create-session-mode"), }; const size_t g_s3_create_multipart_upload_excluded_headers_count = @@ -62,6 +63,7 @@ const struct aws_byte_cursor g_s3_upload_part_excluded_headers[] = { AWS_BYTE_CUR_INIT_FROM_STRING_LITERAL("x-amz-checksum-sha1"), AWS_BYTE_CUR_INIT_FROM_STRING_LITERAL("x-amz-checksum-sha256"), AWS_BYTE_CUR_INIT_FROM_STRING_LITERAL("if-none-match"), + AWS_BYTE_CUR_INIT_FROM_STRING_LITERAL("x-amz-create-session-mode"), }; const size_t g_s3_upload_part_excluded_headers_count = AWS_ARRAY_SIZE(g_s3_upload_part_excluded_headers); @@ -96,6 +98,7 @@ const struct aws_byte_cursor g_s3_complete_multipart_upload_excluded_headers[] = AWS_BYTE_CUR_INIT_FROM_STRING_LITERAL("x-amz-copy-source"), AWS_BYTE_CUR_INIT_FROM_STRING_LITERAL("x-amz-copy-source-range"), AWS_BYTE_CUR_INIT_FROM_STRING_LITERAL("x-amz-mp-object-size"), + AWS_BYTE_CUR_INIT_FROM_STRING_LITERAL("x-amz-create-session-mode"), }; const size_t g_s3_complete_multipart_upload_excluded_headers_count = @@ -131,6 +134,7 @@ const struct aws_byte_cursor g_s3_complete_multipart_upload_with_checksum_exclud AWS_BYTE_CUR_INIT_FROM_STRING_LITERAL("x-amz-copy-source-range"), AWS_BYTE_CUR_INIT_FROM_STRING_LITERAL("x-amz-sdk-checksum-algorithm"), AWS_BYTE_CUR_INIT_FROM_STRING_LITERAL("x-amz-mp-object-size"), + AWS_BYTE_CUR_INIT_FROM_STRING_LITERAL("x-amz-create-session-mode"), }; const struct aws_byte_cursor g_s3_list_parts_excluded_headers[] = { @@ -162,6 +166,7 @@ const struct aws_byte_cursor g_s3_list_parts_excluded_headers[] = { AWS_BYTE_CUR_INIT_FROM_STRING_LITERAL("x-amz-object-lock-legal-hold"), AWS_BYTE_CUR_INIT_FROM_STRING_LITERAL("x-amz-copy-source"), AWS_BYTE_CUR_INIT_FROM_STRING_LITERAL("x-amz-copy-source-range"), + AWS_BYTE_CUR_INIT_FROM_STRING_LITERAL("x-amz-create-session-mode"), }; const size_t g_s3_list_parts_excluded_headers_count = AWS_ARRAY_SIZE(g_s3_list_parts_excluded_headers); @@ -192,6 +197,7 @@ const struct aws_byte_cursor g_s3_list_parts_with_checksum_excluded_headers[] = AWS_BYTE_CUR_INIT_FROM_STRING_LITERAL("x-amz-object-lock-legal-hold"), AWS_BYTE_CUR_INIT_FROM_STRING_LITERAL("x-amz-copy-source"), AWS_BYTE_CUR_INIT_FROM_STRING_LITERAL("x-amz-copy-source-range"), + AWS_BYTE_CUR_INIT_FROM_STRING_LITERAL("x-amz-create-session-mode"), }; const size_t g_s3_list_parts_with_checksum_excluded_headers_count = @@ -227,9 +233,11 @@ const struct aws_byte_cursor g_s3_abort_multipart_upload_excluded_headers[] = { AWS_BYTE_CUR_INIT_FROM_STRING_LITERAL("x-amz-copy-source"), AWS_BYTE_CUR_INIT_FROM_STRING_LITERAL("x-amz-copy-source-range"), AWS_BYTE_CUR_INIT_FROM_STRING_LITERAL("if-none-match"), + AWS_BYTE_CUR_INIT_FROM_STRING_LITERAL("x-amz-create-session-mode"), }; const struct aws_byte_cursor g_s3_create_session_allowed_headers[] = { + AWS_BYTE_CUR_INIT_FROM_STRING_LITERAL("x-amz-create-session-mode"), AWS_BYTE_CUR_INIT_FROM_STRING_LITERAL("x-amz-server-side-encryption"), AWS_BYTE_CUR_INIT_FROM_STRING_LITERAL("x-amz-server-side-encryption-aws-kms-key-id"), AWS_BYTE_CUR_INIT_FROM_STRING_LITERAL("x-amz-server-side-encryption-context"), diff --git a/source/s3express_credentials_provider.c b/source/s3express_credentials_provider.c index 1b144aa4d..146a1bb72 100644 --- a/source/s3express_credentials_provider.c +++ b/source/s3express_credentials_provider.c @@ -486,14 +486,14 @@ struct aws_string *aws_encode_s3express_hash_key_new( struct aws_byte_cursor host_value, struct aws_http_headers *headers) { - struct aws_byte_buf combined_hash_buf; + struct aws_byte_buf combined_buf; /* 1. Combine access_key and secret_access_key into one buffer */ struct aws_byte_cursor access_key = aws_credentials_get_access_key_id(original_credentials); struct aws_byte_cursor secret_access_key = aws_credentials_get_secret_access_key(original_credentials); - aws_byte_buf_init(&combined_hash_buf, allocator, access_key.len + secret_access_key.len); - aws_byte_buf_write_from_whole_cursor(&combined_hash_buf, access_key); - aws_byte_buf_write_from_whole_cursor(&combined_hash_buf, secret_access_key); + aws_byte_buf_init(&combined_buf, allocator, access_key.len + secret_access_key.len); + aws_byte_buf_write_from_whole_cursor(&combined_buf, access_key); + aws_byte_buf_write_from_whole_cursor(&combined_buf, secret_access_key); /* Write the allowed headers into hash */ if (headers != NULL) { @@ -503,19 +503,19 @@ struct aws_string *aws_encode_s3express_hash_key_new( struct aws_byte_cursor header_name = g_s3_create_session_allowed_headers[header_index]; struct aws_byte_cursor header_value; if (aws_http_headers_get(headers, header_name, &header_value) == AWS_OP_SUCCESS && header_value.len > 0) { - aws_byte_buf_write_from_whole_cursor(&combined_hash_buf, comma); - aws_byte_buf_write_from_whole_cursor(&combined_hash_buf, header_name); - aws_byte_buf_write_from_whole_cursor(&combined_hash_buf, collon); - aws_byte_buf_write_from_whole_cursor(&combined_hash_buf, header_value); + aws_byte_buf_append_dynamic(&combined_buf, &comma); + aws_byte_buf_append_dynamic(&combined_buf, &header_name); + aws_byte_buf_append_dynamic(&combined_buf, &collon); + aws_byte_buf_append_dynamic(&combined_buf, &header_value); } } } /* 2. Get sha256 digest from the combined key */ - struct aws_byte_cursor combine_key = aws_byte_cursor_from_buf(&combined_hash_buf); + struct aws_byte_cursor combined_cursor = aws_byte_cursor_from_buf(&combined_buf); struct aws_byte_buf digest_buf; aws_byte_buf_init(&digest_buf, allocator, AWS_SHA256_LEN); - aws_sha256_compute(allocator, &combine_key, &digest_buf, 0); + aws_sha256_compute(allocator, &combined_cursor, &digest_buf, 0); /* 3. Encode the result to be [host_value][hash_of_credentials] */ struct aws_byte_buf result_buffer; @@ -526,7 +526,7 @@ struct aws_string *aws_encode_s3express_hash_key_new( /* Clean up */ aws_byte_buf_clean_up(&result_buffer); - aws_byte_buf_clean_up(&combined_hash_buf); + aws_byte_buf_clean_up(&combined_buf); aws_byte_buf_clean_up(&digest_buf); return result; diff --git a/tests/s3_s3express_client_test.c b/tests/s3_s3express_client_test.c index 4c1b621e0..625caa710 100644 --- a/tests/s3_s3express_client_test.c +++ b/tests/s3_s3express_client_test.c @@ -717,7 +717,7 @@ TEST_CASE(s3express_client_copy_object_multipart) { } /** - * Test hash of the express cache key + * Test hash of the express cache key */ TEST_CASE(s3express_hash_key_test) { (void)ctx; @@ -725,6 +725,8 @@ TEST_CASE(s3express_hash_key_test) { struct aws_string *access_key = aws_string_new_from_c_str(allocator, "AccessKey"); struct aws_string *secret_access_key = aws_string_new_from_c_str(allocator, "SecretAccessKey"); struct aws_http_headers *headers = aws_http_headers_new(allocator); + aws_http_headers_add( + headers, aws_byte_cursor_from_c_str("x-amz-create-session-mode"), aws_byte_cursor_from_c_str("ReadOnly")); aws_http_headers_add( headers, aws_byte_cursor_from_c_str("x-amz-server-side-encryption"), aws_byte_cursor_from_c_str("aws:kms")); aws_http_headers_add( @@ -740,20 +742,22 @@ TEST_CASE(s3express_hash_key_test) { aws_byte_cursor_from_c_str("x-amz-server-side-encryption-bucket-key-enabled"), aws_byte_cursor_from_c_str("true")); aws_http_headers_add( - headers, aws_byte_cursor_from_c_str("header-not-allowed"), aws_byte_cursor_from_c_str("should-be-ignored")); + headers, + aws_byte_cursor_from_c_str("header-not-in-allow-list"), + aws_byte_cursor_from_c_str("should-be-ignored")); struct aws_credentials *creds = aws_credentials_new_from_string(allocator, access_key, secret_access_key, NULL, UINT64_MAX); struct aws_string *hash_key = - aws_encode_s3express_hash_key_new(allocator, creds, aws_byte_cursor_from_c_str("host"), headers); + aws_encode_s3express_hash_key_new(allocator, creds, aws_byte_cursor_from_c_str(""), headers); struct aws_byte_cursor hash_cursor = aws_byte_cursor_from_string(hash_key); struct aws_byte_buf encoded_buf; - aws_byte_buf_init(&encoded_buf, allocator, 100); + aws_byte_buf_init(&encoded_buf, allocator, 200); aws_hex_encode_append_dynamic(&hash_cursor, &encoded_buf); - char *expected_encoded_key = "686f737498ae6a365790707488b3e85402c9eddf422dc39f096e15eaba0d7cdd45f57ad2"; + char *expected_encoded_key = "cabfefee4365e075646ba8928ed9f757481d1062ffcb0a3afe5b9c428dd45800"; ASSERT_BIN_ARRAYS_EQUALS(expected_encoded_key, strlen(expected_encoded_key), encoded_buf.buffer, encoded_buf.len); aws_byte_buf_clean_up(&encoded_buf); From 73c5fc2bb2f52bd6772fca3e103c211ed75bd0bf Mon Sep 17 00:00:00 2001 From: Waqar Ahmed Khan Date: Fri, 24 Jan 2025 10:04:04 -0800 Subject: [PATCH 10/19] Cleanup --- tests/CMakeLists.txt | 2 +- tests/s3_mock_server_s3express_provider_test.c | 6 ++---- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 91104a07e..d2f5c5af8 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -351,7 +351,7 @@ if(ENABLE_MOCK_SERVER_TESTS) add_net_test_case(s3express_provider_background_refresh_mock_server) add_net_test_case(s3express_provider_background_refresh_remove_inactive_creds_mock_server) add_net_test_case(s3express_provider_stress_mock_server) - add_net_test_case(s3express_provider_get_credentials_kms_headers_mock_server) + add_net_test_case(s3express_provider_get_credentials_sse_headers_mock_server) add_net_test_case(s3express_client_sanity_test_mock_server) add_net_test_case(s3express_client_sanity_override_test_mock_server) diff --git a/tests/s3_mock_server_s3express_provider_test.c b/tests/s3_mock_server_s3express_provider_test.c index 39adefe2a..9d9fb9f55 100644 --- a/tests/s3_mock_server_s3express_provider_test.c +++ b/tests/s3_mock_server_s3express_provider_test.c @@ -260,9 +260,7 @@ struct aws_s3express_credentials_provider *s_s3express_credentials_provider_fact aws_simple_completion_callback on_provider_shutdown_callback, void *shutdown_user_data, void *factory_user_data) { - (void)allocator; - (void)client; - (void)on_provider_shutdown_callback; + (void)shutdown_user_data; (void)factory_user_data; s_s3express_tester.on_provider_shutdown_callback = on_provider_shutdown_callback; @@ -280,7 +278,7 @@ struct aws_s3express_credentials_provider *s_s3express_credentials_provider_fact return provider; } -TEST_CASE(s3express_provider_get_credentials_kms_headers_mock_server) { +TEST_CASE(s3express_provider_get_credentials_sse_headers_mock_server) { (void)ctx; struct aws_s3_tester tester; From e74da126399c8a80e452efc5c005d8c3ec5f978d Mon Sep 17 00:00:00 2001 From: Waqar Ahmed Khan Date: Fri, 24 Jan 2025 10:07:58 -0800 Subject: [PATCH 11/19] Add experimental warning --- include/aws/s3/s3_client.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/include/aws/s3/s3_client.h b/include/aws/s3/s3_client.h index dcd2480bd..126836579 100644 --- a/include/aws/s3/s3_client.h +++ b/include/aws/s3/s3_client.h @@ -552,6 +552,9 @@ struct aws_s3_client_config { * If set, client will invoke the factory to get the provider to use, when needed. * * If not set, client will create a default S3 Express provider under the hood. + * + * NOTE: THE FOLLOWING BEHAVIOR IS EXPERIMENTAL AND UNSTABLE + * Default S3 Express provider will pass the headers allowed in `g_s3_create_session_allowed_headers` to the CreateSession call. */ aws_s3express_provider_factory_fn *s3express_provider_override_factory; void *factory_user_data; From c7975d2270ff14954f11e95b22cee8e151a109d6 Mon Sep 17 00:00:00 2001 From: Waqar Ahmed Khan Date: Fri, 24 Jan 2025 10:12:00 -0800 Subject: [PATCH 12/19] lint --- include/aws/s3/s3_client.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/include/aws/s3/s3_client.h b/include/aws/s3/s3_client.h index 126836579..41e55d3c2 100644 --- a/include/aws/s3/s3_client.h +++ b/include/aws/s3/s3_client.h @@ -554,7 +554,8 @@ struct aws_s3_client_config { * If not set, client will create a default S3 Express provider under the hood. * * NOTE: THE FOLLOWING BEHAVIOR IS EXPERIMENTAL AND UNSTABLE - * Default S3 Express provider will pass the headers allowed in `g_s3_create_session_allowed_headers` to the CreateSession call. + * Default S3 Express provider will pass the headers allowed in `g_s3_create_session_allowed_headers` to the + * CreateSession call. */ aws_s3express_provider_factory_fn *s3express_provider_override_factory; void *factory_user_data; From 0b079149defd72404cee12efb54c9d319d12ed33 Mon Sep 17 00:00:00 2001 From: Waqar Ahmed Khan Date: Fri, 24 Jan 2025 10:24:13 -0800 Subject: [PATCH 13/19] add type hint --- tests/mock_s3_server/mock_s3_server.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/mock_s3_server/mock_s3_server.py b/tests/mock_s3_server/mock_s3_server.py index 3a7fbcd81..032172b9d 100644 --- a/tests/mock_s3_server/mock_s3_server.py +++ b/tests/mock_s3_server/mock_s3_server.py @@ -8,7 +8,7 @@ from itertools import count from urllib.parse import parse_qs, urlparse import os -from typing import Optional +from typing import Optional, List, Tuple from enum import Enum import trio @@ -56,7 +56,8 @@ class ResponseConfig: json_path: str = None throttle: bool = False force_retry: bool = False - request_headers = None + request_headers: Optional[List[Tuple[bytes, bytes]]] = None + def _resolve_file_path(self, wrapper, request_type): global SHOULD_THROTTLE if self.json_path is None: From 54cecca97ad3a2c60271b6a1f6a976670accbd9d Mon Sep 17 00:00:00 2001 From: Waqar Ahmed Khan Date: Fri, 24 Jan 2025 10:49:48 -0800 Subject: [PATCH 14/19] fix codecov action --- .github/workflows/ci.yml | 2 +- .github/workflows/codecov.yml | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index b1b5ba1a4..05a6d2f5e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -11,7 +11,7 @@ concurrency: cancel-in-progress: true env: - BUILDER_VERSION: v0.9.72 + BUILDER_VERSION: v0.9.74 BUILDER_SOURCE: releases BUILDER_HOST: https://d19elf31gohf1l.cloudfront.net PACKAGE_NAME: aws-c-s3 diff --git a/.github/workflows/codecov.yml b/.github/workflows/codecov.yml index 157191388..6875f7463 100644 --- a/.github/workflows/codecov.yml +++ b/.github/workflows/codecov.yml @@ -5,7 +5,7 @@ on: env: - BUILDER_VERSION: v0.9.72 + BUILDER_VERSION: v0.9.74 BUILDER_SOURCE: releases BUILDER_HOST: https://d19elf31gohf1l.cloudfront.net PACKAGE_NAME: aws-c-s3 @@ -30,4 +30,4 @@ jobs: run: | python3 -c "from urllib.request import urlretrieve; urlretrieve('${{ env.BUILDER_HOST }}/${{ env.BUILDER_SOURCE }}/${{ env.BUILDER_VERSION }}/builder.pyz?run=${{ env.RUN }}', 'builder')" chmod a+x builder - ./builder build -p ${{ env.PACKAGE_NAME }} --compiler=gcc-12 --cmake-extra=-DASSERT_LOCK_HELD=ON --coverage --coverage-exclude=source/s3_copy_object.c + ./builder build -p ${{ env.PACKAGE_NAME }} --compiler=gcc --cmake-extra=-DASSERT_LOCK_HELD=ON --coverage --coverage-exclude=source/s3_copy_object.c From 2ccc3b295d106fcfa158026a4dd0190e9e88fc66 Mon Sep 17 00:00:00 2001 From: Waqar Ahmed Khan Date: Fri, 24 Jan 2025 13:02:21 -0800 Subject: [PATCH 15/19] init and cleanup --- tests/s3_s3express_client_test.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/s3_s3express_client_test.c b/tests/s3_s3express_client_test.c index 625caa710..eb3b6cb05 100644 --- a/tests/s3_s3express_client_test.c +++ b/tests/s3_s3express_client_test.c @@ -721,6 +721,7 @@ TEST_CASE(s3express_client_copy_object_multipart) { */ TEST_CASE(s3express_hash_key_test) { (void)ctx; + aws_s3_library_init(allocator); struct aws_string *access_key = aws_string_new_from_c_str(allocator, "AccessKey"); struct aws_string *secret_access_key = aws_string_new_from_c_str(allocator, "SecretAccessKey"); @@ -767,5 +768,7 @@ TEST_CASE(s3express_hash_key_test) { aws_string_destroy(hash_key); aws_http_headers_release(headers); - return 0; + aws_s3_library_clean_up(); + + return AWS_OP_SUCCESS; } From a9507a5cf35ed04fc1cf12303ee69be2558b23d3 Mon Sep 17 00:00:00 2001 From: Waqar Ahmed Khan Date: Fri, 24 Jan 2025 14:10:21 -0800 Subject: [PATCH 16/19] try the new host override --- source/s3express_credentials_provider.c | 18 ++++++++---------- tests/mock_s3_server/mock_s3_server.py | 6 +++--- 2 files changed, 11 insertions(+), 13 deletions(-) diff --git a/source/s3express_credentials_provider.c b/source/s3express_credentials_provider.c index 146a1bb72..47d984e68 100644 --- a/source/s3express_credentials_provider.c +++ b/source/s3express_credentials_provider.c @@ -401,17 +401,17 @@ static struct aws_http_message *s_create_session_request_new( const struct aws_uri *endpoint_override) { struct aws_http_message *request = aws_http_message_new_request(allocator); + struct aws_byte_cursor host = host_value; + /* NOTE: Only for Tests. */ + if (endpoint_override != NULL) { + host = *aws_uri_host_name(endpoint_override); + } struct aws_http_header host_header = { .name = g_host_header_name, - .value = host_value, + .value = host, }; - /* NOTE: Only for Tests. - * Don't add the host header for endpoint override. - */ - if (endpoint_override == NULL) { - if (aws_http_message_add_header(request, host_header)) { - goto error; - } + if (aws_http_message_add_header(request, host_header)) { + goto error; } struct aws_http_header user_agent_header = { @@ -571,8 +571,6 @@ static struct aws_s3express_session_creator *s_session_creator_new( .body_callback = s_on_incoming_body_fn, .finish_callback = s_on_request_finished, .signing_config = &s3express_signing_config, - /* Override endpoint only for tests. */ - .endpoint = impl->mock_test.endpoint_override ? impl->mock_test.endpoint_override : NULL, .user_data = session_creator, .operation_name = aws_byte_cursor_from_c_str("CreateSession"), }; diff --git a/tests/mock_s3_server/mock_s3_server.py b/tests/mock_s3_server/mock_s3_server.py index 032172b9d..885b54d6f 100644 --- a/tests/mock_s3_server/mock_s3_server.py +++ b/tests/mock_s3_server/mock_s3_server.py @@ -434,8 +434,8 @@ def handle_get_object(wrapper, request, parsed_path, head_request=False): else: RETRY_REQUEST_COUNT = 0 - if (parsed_path.path == "/get_object_invalid_response_missing_content_range" or - parsed_path.path == "/get_object_invalid_response_missing_etags" or + if (parsed_path.path == "/get_object_invalid_response_missing_content_range" or + parsed_path.path == "/get_object_invalid_response_missing_etags" or parsed_path.path == "/get_object_long_error"): # Don't generate the body for those requests return response_config @@ -529,4 +529,4 @@ async def serve(port): print("KeyboardInterrupt - shutting down") if __name__ == "__main__": - trio.run(serve, 8080) + trio.run(serve, 8181) From decf5f0766e3a6955feff25833a5edaa40fbbffe Mon Sep 17 00:00:00 2001 From: Waqar Ahmed Khan Date: Fri, 24 Jan 2025 14:21:57 -0800 Subject: [PATCH 17/19] fix server --- tests/mock_s3_server/mock_s3_server.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/mock_s3_server/mock_s3_server.py b/tests/mock_s3_server/mock_s3_server.py index 885b54d6f..1d9d2ff8f 100644 --- a/tests/mock_s3_server/mock_s3_server.py +++ b/tests/mock_s3_server/mock_s3_server.py @@ -529,4 +529,4 @@ async def serve(port): print("KeyboardInterrupt - shutting down") if __name__ == "__main__": - trio.run(serve, 8181) + trio.run(serve, 8080) From 2d93aefd7cd4d6af8197533c6c7bae8381fda8d7 Mon Sep 17 00:00:00 2001 From: Waqar Ahmed Khan Date: Fri, 24 Jan 2025 15:10:14 -0800 Subject: [PATCH 18/19] try setting port --- source/s3_client.c | 1 + 1 file changed, 1 insertion(+) diff --git a/source/s3_client.c b/source/s3_client.c index 590e1dd33..7bfc396f8 100644 --- a/source/s3_client.c +++ b/source/s3_client.c @@ -1030,6 +1030,7 @@ struct aws_s3_meta_request *aws_s3_client_make_meta_request( } endpoint_host_name = aws_string_new_from_cursor(client->allocator, aws_uri_host_name(&host_uri)); + port = aws_uri_port(&host_uri); aws_uri_clean_up(&host_uri); } From 94fec484d9186153a933a2e58a1daca26639f504 Mon Sep 17 00:00:00 2001 From: Waqar Ahmed Khan Date: Fri, 24 Jan 2025 15:26:37 -0800 Subject: [PATCH 19/19] authority instead of host --- source/s3express_credentials_provider.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/s3express_credentials_provider.c b/source/s3express_credentials_provider.c index 47d984e68..7afb5aca3 100644 --- a/source/s3express_credentials_provider.c +++ b/source/s3express_credentials_provider.c @@ -404,7 +404,7 @@ static struct aws_http_message *s_create_session_request_new( struct aws_byte_cursor host = host_value; /* NOTE: Only for Tests. */ if (endpoint_override != NULL) { - host = *aws_uri_host_name(endpoint_override); + host = *aws_uri_authority(endpoint_override); } struct aws_http_header host_header = { .name = g_host_header_name,