Skip to content

Commit 45eee8d

Browse files
authored
Validate Invalid Network Interface Names at Client Initialization (#456)
1 parent ab70f74 commit 45eee8d

5 files changed

Lines changed: 125 additions & 39 deletions

File tree

.gitignore

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,3 +68,6 @@ cmake-build*
6868
# js package locks irrelevant to the overall package's security
6969
benchmarks/benchmarks-stack/benchmarks-stack/package-lock.json
7070
benchmarks/dashboard-stack/package-lock.json
71+
72+
# virtual environment
73+
.venv/

source/s3_client.c

Lines changed: 55 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -346,7 +346,7 @@ struct aws_s3_client *aws_s3_client_new(
346346
client->buffer_pool = aws_s3_buffer_pool_new(allocator, part_size, mem_limit);
347347

348348
if (client->buffer_pool == NULL) {
349-
goto on_early_fail;
349+
goto on_error;
350350
}
351351

352352
struct aws_s3_buffer_pool_usage_stats pool_usage = aws_s3_buffer_pool_get_usage(client->buffer_pool);
@@ -357,15 +357,15 @@ struct aws_s3_client *aws_s3_client_new(
357357
"Cannot create client from client_config; configured max part size should not exceed memory limit."
358358
"size.");
359359
aws_raise_error(AWS_ERROR_S3_INVALID_MEMORY_LIMIT_CONFIG);
360-
goto on_early_fail;
360+
goto on_error;
361361
}
362362

363363
client->vtable = &s_s3_client_default_vtable;
364364

365365
aws_ref_count_init(&client->ref_count, client, (aws_simple_completion_callback *)s_s3_client_start_destroy);
366366

367367
if (aws_mutex_init(&client->synced_data.lock) != AWS_OP_SUCCESS) {
368-
goto on_early_fail;
368+
goto on_error;
369369
}
370370

371371
aws_linked_list_init(&client->synced_data.pending_meta_request_work);
@@ -488,6 +488,44 @@ struct aws_s3_client *aws_s3_client_new(
488488
}
489489
}
490490

491+
client->num_network_interface_names = client_config->num_network_interface_names;
492+
if (client_config->num_network_interface_names > 0) {
493+
AWS_LOGF_DEBUG(
494+
AWS_LS_S3_CLIENT,
495+
"id=%p Client received network interface names array with length %zu.",
496+
(void *)client,
497+
client->num_network_interface_names);
498+
aws_array_list_init_dynamic(
499+
&client->network_interface_names,
500+
client->allocator,
501+
client_config->num_network_interface_names,
502+
sizeof(struct aws_string *));
503+
client->network_interface_names_cursor_array = aws_mem_calloc(
504+
client->allocator, client_config->num_network_interface_names, sizeof(struct aws_byte_cursor));
505+
for (size_t i = 0; i < client_config->num_network_interface_names; i++) {
506+
struct aws_byte_cursor interface_name = client_config->network_interface_names_array[i];
507+
struct aws_string *interface_name_str = aws_string_new_from_cursor(client->allocator, &interface_name);
508+
aws_array_list_push_back(&client->network_interface_names, &interface_name_str);
509+
if (aws_is_network_interface_name_valid(aws_string_c_str(interface_name_str)) == false) {
510+
AWS_LOGF_ERROR(
511+
AWS_LS_S3_CLIENT,
512+
"id=%p network_interface_names_array[%zu]=" PRInSTR " is not valid.",
513+
(void *)client,
514+
i,
515+
AWS_BYTE_CURSOR_PRI(interface_name));
516+
aws_raise_error(AWS_ERROR_INVALID_ARGUMENT);
517+
goto on_error;
518+
}
519+
client->network_interface_names_cursor_array[i] = aws_byte_cursor_from_string(interface_name_str);
520+
AWS_LOGF_DEBUG(
521+
AWS_LS_S3_CLIENT,
522+
"id=%p network_interface_names_array[%zu]=" PRInSTR "",
523+
(void *)client,
524+
i,
525+
AWS_BYTE_CURSOR_PRI(client->network_interface_names_cursor_array[i]));
526+
}
527+
}
528+
491529
/* Set up body streaming ELG */
492530
{
493531
uint16_t num_event_loops =
@@ -579,34 +617,6 @@ struct aws_s3_client *aws_s3_client_new(
579617
*((bool *)&client->enable_read_backpressure) = client_config->enable_read_backpressure;
580618
*((size_t *)&client->initial_read_window) = client_config->initial_read_window;
581619

582-
client->num_network_interface_names = client_config->num_network_interface_names;
583-
if (client_config->num_network_interface_names > 0) {
584-
AWS_LOGF_DEBUG(
585-
AWS_LS_S3_CLIENT,
586-
"id=%p Client received network interface names array with length %zu.",
587-
(void *)client,
588-
client->num_network_interface_names);
589-
aws_array_list_init_dynamic(
590-
&client->network_interface_names,
591-
client->allocator,
592-
client_config->num_network_interface_names,
593-
sizeof(struct aws_string *));
594-
client->network_interface_names_cursor_array = aws_mem_calloc(
595-
client->allocator, client_config->num_network_interface_names, sizeof(struct aws_byte_cursor));
596-
for (size_t i = 0; i < client_config->num_network_interface_names; i++) {
597-
struct aws_byte_cursor interface_name = client_config->network_interface_names_array[i];
598-
struct aws_string *interface_name_str = aws_string_new_from_cursor(client->allocator, &interface_name);
599-
aws_array_list_push_back(&client->network_interface_names, &interface_name_str);
600-
client->network_interface_names_cursor_array[i] = aws_byte_cursor_from_string(interface_name_str);
601-
AWS_LOGF_DEBUG(
602-
AWS_LS_S3_CLIENT,
603-
"id=%p network_interface_names_array[%zu]=" PRInSTR "",
604-
(void *)client,
605-
i,
606-
AWS_BYTE_CURSOR_PRI(client->network_interface_names_cursor_array[i]));
607-
}
608-
}
609-
610620
return client;
611621

612622
on_error:
@@ -628,10 +638,22 @@ struct aws_s3_client *aws_s3_client_new(
628638
aws_mem_release(client->allocator, client->proxy_ev_settings);
629639
aws_mem_release(client->allocator, client->tcp_keep_alive_options);
630640

631-
aws_event_loop_group_release(client->client_bootstrap->event_loop_group);
641+
if (client->client_bootstrap != NULL) {
642+
aws_event_loop_group_release(client->client_bootstrap->event_loop_group);
643+
}
632644
aws_client_bootstrap_release(client->client_bootstrap);
633645
aws_mutex_clean_up(&client->synced_data.lock);
634-
on_early_fail:
646+
647+
aws_mem_release(client->allocator, client->network_interface_names_cursor_array);
648+
for (size_t i = 0; i < aws_array_list_length(&client->network_interface_names); i++) {
649+
struct aws_string *interface_name = NULL;
650+
aws_array_list_get_at(&client->network_interface_names, &interface_name, i);
651+
aws_string_destroy(interface_name);
652+
}
653+
654+
aws_array_list_clean_up(&client->network_interface_names);
655+
aws_s3_buffer_pool_destroy(client->buffer_pool);
656+
635657
aws_mem_release(client->allocator, client);
636658
return NULL;
637659
}

tests/CMakeLists.txt

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@ add_test_case(test_s3_abort_multipart_upload_message_new)
2727

2828
add_net_test_case(test_s3_client_create_destroy)
2929
add_net_test_case(test_s3_client_create_error)
30+
add_net_test_case(test_s3_client_create_error_with_invalid_memory_limit_config)
31+
add_net_test_case(test_s3_client_create_error_with_invalid_network_interface)
3032
add_net_test_case(test_s3_client_monitoring_options_override)
3133
add_net_test_case(test_s3_client_proxy_ev_settings_override)
3234
add_net_test_case(test_s3_client_tcp_keep_alive_options_override)

tests/s3_data_plane_tests.c

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,61 @@ static int s_test_s3_client_create_error(struct aws_allocator *allocator, void *
7777
return 0;
7878
}
7979

80+
AWS_TEST_CASE(
81+
test_s3_client_create_error_with_invalid_memory_limit_config,
82+
s_test_s3_client_create_error_with_invalid_memory_limit_config)
83+
static int s_test_s3_client_create_error_with_invalid_memory_limit_config(struct aws_allocator *allocator, void *ctx) {
84+
(void)ctx;
85+
86+
struct aws_s3_tester tester;
87+
AWS_ZERO_STRUCT(tester);
88+
ASSERT_SUCCESS(aws_s3_tester_init(allocator, &tester));
89+
90+
struct aws_s3_client_config client_config;
91+
AWS_ZERO_STRUCT(client_config);
92+
ASSERT_SUCCESS(aws_s3_tester_bind_client(&tester, &client_config, AWS_S3_TESTER_BIND_CLIENT_REGION));
93+
94+
client_config.memory_limit_in_bytes = 100;
95+
struct aws_s3_client *client = aws_s3_client_new(allocator, &client_config);
96+
ASSERT_TRUE(client == NULL);
97+
client_config.memory_limit_in_bytes = GB_TO_BYTES(1);
98+
client_config.max_part_size = GB_TO_BYTES(2);
99+
client = aws_s3_client_new(allocator, &client_config);
100+
ASSERT_TRUE(client == NULL);
101+
102+
tester.bound_to_client = false;
103+
aws_s3_tester_clean_up(&tester);
104+
return 0;
105+
}
106+
107+
AWS_TEST_CASE(
108+
test_s3_client_create_error_with_invalid_network_interface,
109+
s_test_s3_client_create_error_with_invalid_network_interface)
110+
static int s_test_s3_client_create_error_with_invalid_network_interface(struct aws_allocator *allocator, void *ctx) {
111+
(void)ctx;
112+
113+
struct aws_s3_tester tester;
114+
AWS_ZERO_STRUCT(tester);
115+
ASSERT_SUCCESS(aws_s3_tester_init(allocator, &tester));
116+
117+
struct aws_s3_client_config client_config;
118+
AWS_ZERO_STRUCT(client_config);
119+
ASSERT_SUCCESS(aws_s3_tester_bind_client(&tester, &client_config, AWS_S3_TESTER_BIND_CLIENT_REGION));
120+
121+
struct aws_byte_cursor *interface_names_array = aws_mem_calloc(allocator, 1, sizeof(struct aws_byte_cursor));
122+
interface_names_array[0] = aws_byte_cursor_from_c_str("invalid-nic");
123+
client_config.network_interface_names_array = interface_names_array;
124+
client_config.num_network_interface_names = 1;
125+
126+
struct aws_s3_client *client = aws_s3_client_new(allocator, &client_config);
127+
ASSERT_TRUE(client == NULL);
128+
tester.bound_to_client = false;
129+
130+
aws_s3_tester_clean_up(&tester);
131+
aws_mem_release(allocator, interface_names_array);
132+
return 0;
133+
}
134+
80135
AWS_TEST_CASE(test_s3_client_monitoring_options_override, s_test_s3_client_monitoring_options_override)
81136
static int s_test_s3_client_monitoring_options_override(struct aws_allocator *allocator, void *ctx) {
82137
(void)ctx;

tests/s3_mock_server_tests.c

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -169,16 +169,19 @@ TEST_CASE(multipart_upload_mock_server) {
169169

170170
TEST_CASE(multipart_upload_with_network_interface_names_mock_server) {
171171
(void)ctx;
172-
172+
#if defined(AWS_OS_WINDOWS)
173+
(void)allocator;
174+
return AWS_OP_SKIP;
175+
#else
173176
struct aws_s3_tester tester;
174177
ASSERT_SUCCESS(aws_s3_tester_init(allocator, &tester));
175178
struct aws_byte_cursor *interface_names_array = aws_mem_calloc(allocator, 2, sizeof(struct aws_byte_cursor));
176179
char *localhost_interface = "\0";
177-
#if defined(AWS_OS_APPLE)
180+
# if defined(AWS_OS_APPLE)
178181
localhost_interface = "lo0";
179-
#else
182+
# else
180183
localhost_interface = "lo";
181-
#endif
184+
# endif
182185
interface_names_array[0] = aws_byte_cursor_from_c_str(localhost_interface);
183186
interface_names_array[1] = aws_byte_cursor_from_c_str(localhost_interface);
184187

@@ -212,11 +215,11 @@ TEST_CASE(multipart_upload_with_network_interface_names_mock_server) {
212215
aws_s3_meta_request_test_results_init(&out_results, allocator);
213216
ASSERT_SUCCESS(aws_s3_tester_send_meta_request_with_options(&tester, &put_options, &out_results));
214217
if (out_results.finished_error_code != 0) {
215-
#if !defined(AWS_OS_APPLE) && !defined(AWS_OS_LINUX)
218+
# if !defined(AWS_OS_APPLE) && !defined(AWS_OS_LINUX)
216219
if (out_results.finished_error_code == AWS_ERROR_PLATFORM_NOT_SUPPORTED) {
217220
return AWS_OP_SKIP;
218221
}
219-
#endif
222+
# endif
220223
ASSERT_TRUE(false, "aws_s3_tester_send_meta_request_with_options(() failed");
221224
}
222225
aws_s3_meta_request_test_results_clean_up(&out_results);
@@ -225,6 +228,7 @@ TEST_CASE(multipart_upload_with_network_interface_names_mock_server) {
225228
aws_mem_release(allocator, interface_names_array);
226229

227230
return AWS_OP_SUCCESS;
231+
#endif
228232
}
229233

230234
TEST_CASE(multipart_upload_checksum_with_retry_mock_server) {

0 commit comments

Comments
 (0)