From 543e3764d7fa4f1228ed630d1aa40e4a0de396d6 Mon Sep 17 00:00:00 2001 From: Bret Ambrose Date: Tue, 3 Mar 2020 10:51:55 -0800 Subject: [PATCH] Properly use and separate high res and system time (#60) * Properly use and separate high res and system time --- include/aws/auth/credentials.h | 7 +++- source/credentials_provider_cached.c | 57 +++++++++++++++++--------- source/credentials_provider_sts.c | 10 ++--- tests/credentials_provider_sts_tests.c | 45 ++++++++++++-------- tests/credentials_provider_utils.c | 47 ++++++++++++++++----- tests/credentials_provider_utils.h | 9 ++-- tests/credentials_tests.c | 24 ++++++----- 7 files changed, 132 insertions(+), 67 deletions(-) diff --git a/include/aws/auth/credentials.h b/include/aws/auth/credentials.h index 1f6a8637..6af15696 100644 --- a/include/aws/auth/credentials.h +++ b/include/aws/auth/credentials.h @@ -140,7 +140,10 @@ struct aws_credentials_provider_cached_options { struct aws_credentials_provider_shutdown_options shutdown_options; struct aws_credentials_provider *source; uint64_t refresh_time_in_milliseconds; - aws_io_clock_fn *clock_fn; + + /* For mocking, leave NULL otherwise */ + aws_io_clock_fn *high_res_clock_fn; + aws_io_clock_fn *system_clock_fn; }; struct aws_credentials_provider_chain_options { @@ -168,7 +171,7 @@ struct aws_credentials_provider_sts_options { /* For mocking, leave NULL otherwise */ struct aws_credentials_provider_system_vtable *function_table; - aws_io_clock_fn *clock_fn; + aws_io_clock_fn *system_clock_fn; }; struct aws_credentials_provider_chain_default_options { diff --git a/source/credentials_provider_cached.c b/source/credentials_provider_cached.c index ca6f246b..349ec341 100644 --- a/source/credentials_provider_cached.c +++ b/source/credentials_provider_cached.c @@ -30,7 +30,7 @@ AWS_STATIC_STRING_FROM_LITERAL(s_credential_expiration_env_var, "AWS_CREDENTIAL_ */ -#define REFRESH_CREDENTIALS_EARLY_DURATION_SECONDS 30 +#define REFRESH_CREDENTIALS_EARLY_DURATION_SECONDS 10 struct aws_credentials_provider_cached { struct aws_credentials_provider *source; @@ -39,7 +39,8 @@ struct aws_credentials_provider_cached { struct aws_mutex lock; uint64_t refresh_interval_in_ns; uint64_t next_refresh_time; - aws_io_clock_fn *clock_fn; + aws_io_clock_fn *high_res_clock_fn; + aws_io_clock_fn *system_clock_fn; struct aws_linked_list pending_queries; }; @@ -76,22 +77,32 @@ static void s_cached_credentials_provider_get_credentials_async_callback( uint64_t next_refresh_time_in_ns = UINT64_MAX; - if (impl->refresh_interval_in_ns > 0) { - uint64_t now = 0; - if (!impl->clock_fn(&now)) { - next_refresh_time_in_ns = now + impl->refresh_interval_in_ns; + uint64_t high_res_now = 0; + if (!impl->high_res_clock_fn(&high_res_now)) { + if (impl->refresh_interval_in_ns > 0) { + next_refresh_time_in_ns = high_res_now + impl->refresh_interval_in_ns; } - } - if (credentials && credentials->expiration_timepoint_seconds < UINT64_MAX) { - if (credentials->expiration_timepoint_seconds >= REFRESH_CREDENTIALS_EARLY_DURATION_SECONDS) { - uint64_t early_refresh_time_ns = aws_timestamp_convert( - credentials->expiration_timepoint_seconds - REFRESH_CREDENTIALS_EARLY_DURATION_SECONDS, - AWS_TIMESTAMP_SECS, - AWS_TIMESTAMP_NANOS, - NULL); - if (early_refresh_time_ns < next_refresh_time_in_ns) { - next_refresh_time_in_ns = early_refresh_time_ns; + if (credentials && credentials->expiration_timepoint_seconds < UINT64_MAX) { + uint64_t system_now = 0; + if (!impl->system_clock_fn(&system_now)) { + + uint64_t system_now_seconds = + aws_timestamp_convert(system_now, AWS_TIMESTAMP_NANOS, AWS_TIMESTAMP_SECS, NULL); + if (credentials->expiration_timepoint_seconds >= + system_now_seconds + REFRESH_CREDENTIALS_EARLY_DURATION_SECONDS) { + uint64_t early_refresh_time_ns = high_res_now; + early_refresh_time_ns += aws_timestamp_convert( + credentials->expiration_timepoint_seconds - system_now_seconds - + REFRESH_CREDENTIALS_EARLY_DURATION_SECONDS, + AWS_TIMESTAMP_SECS, + AWS_TIMESTAMP_NANOS, + NULL); + + if (early_refresh_time_ns < next_refresh_time_in_ns) { + next_refresh_time_in_ns = early_refresh_time_ns; + } + } } } } @@ -140,7 +151,7 @@ static int s_cached_credentials_provider_get_credentials_async( struct aws_credentials_provider_cached *impl = provider->impl; uint64_t current_time = 0; - impl->clock_fn(¤t_time); + impl->high_res_clock_fn(¤t_time); bool should_submit_query = false; bool perform_callback = false; @@ -293,10 +304,16 @@ struct aws_credentials_provider *aws_credentials_provider_new_cached( impl->refresh_interval_in_ns = 0; } - if (options->clock_fn != NULL) { - impl->clock_fn = options->clock_fn; + if (options->high_res_clock_fn != NULL) { + impl->high_res_clock_fn = options->high_res_clock_fn; + } else { + impl->high_res_clock_fn = &aws_high_res_clock_get_ticks; + } + + if (options->system_clock_fn != NULL) { + impl->system_clock_fn = options->system_clock_fn; } else { - impl->clock_fn = &aws_high_res_clock_get_ticks; + impl->system_clock_fn = &aws_sys_clock_get_ticks; } /* diff --git a/source/credentials_provider_sts.c b/source/credentials_provider_sts.c index 4f08dc9a..9eaf4203 100644 --- a/source/credentials_provider_sts.c +++ b/source/credentials_provider_sts.c @@ -88,7 +88,7 @@ struct aws_credentials_provider_sts_impl { struct aws_credentials_provider_shutdown_options source_shutdown_options; struct aws_credentials_provider_system_vtable *function_table; bool owns_ctx; - aws_io_clock_fn *clock_fn; + aws_io_clock_fn *system_clock_fn; }; struct sts_creds_provider_user_data { @@ -264,7 +264,7 @@ static void s_on_stream_complete_fn(struct aws_http_stream *stream, int error_co aws_mem_calloc(provider_user_data->allocator, 1, sizeof(struct aws_credentials)); uint64_t now = UINT64_MAX; - if (provider_impl->clock_fn(&now) != AWS_OP_SUCCESS) { + if (provider_impl->system_clock_fn(&now) != AWS_OP_SUCCESS) { goto finish; } @@ -635,10 +635,10 @@ struct aws_credentials_provider *aws_credentials_provider_new_sts( impl->duration_seconds = options->duration_seconds; - if (options->clock_fn != NULL) { - impl->clock_fn = options->clock_fn; + if (options->system_clock_fn != NULL) { + impl->system_clock_fn = options->system_clock_fn; } else { - impl->clock_fn = aws_sys_clock_get_ticks; + impl->system_clock_fn = aws_sys_clock_get_ticks; } /* minimum for STS is 900 seconds*/ diff --git a/tests/credentials_provider_sts_tests.c b/tests/credentials_provider_sts_tests.c index 5926fe9e..108f78e8 100644 --- a/tests/credentials_provider_sts_tests.c +++ b/tests/credentials_provider_sts_tests.c @@ -334,10 +334,10 @@ static int s_credentials_provider_sts_direct_config_succeeds_fn(struct aws_alloc .session_name = s_session_name_cur, .duration_seconds = 0, .function_table = &s_mock_function_table, - .clock_fn = mock_aws_get_time, + .system_clock_fn = mock_aws_get_system_time, }; - mock_aws_set_time(0); + mock_aws_set_system_time(0); s_tester.mock_body = aws_byte_buf_from_c_str(success_creds_doc); s_tester.mock_response_code = 200; @@ -404,10 +404,10 @@ static int s_credentials_provider_sts_direct_config_invalid_doc_fn(struct aws_al .session_name = s_session_name_cur, .duration_seconds = 0, .function_table = &s_mock_function_table, - .clock_fn = mock_aws_get_time, + .system_clock_fn = mock_aws_get_system_time, }; - mock_aws_set_time(0); + mock_aws_set_system_time(0); s_tester.mock_body = aws_byte_buf_from_c_str(malformed_creds_doc); s_tester.mock_response_code = 200; @@ -465,10 +465,10 @@ static int s_credentials_provider_sts_direct_config_connection_failed_fn(struct .session_name = s_session_name_cur, .duration_seconds = 0, .function_table = &s_mock_function_table, - .clock_fn = mock_aws_get_time, + .system_clock_fn = mock_aws_get_system_time, }; - mock_aws_set_time(0); + mock_aws_set_system_time(0); s_tester.fail_connection = true; @@ -510,10 +510,10 @@ static int s_credentials_provider_sts_direct_config_service_fails_fn(struct aws_ .session_name = s_session_name_cur, .duration_seconds = 0, .function_table = &s_mock_function_table, - .clock_fn = mock_aws_get_time, + .system_clock_fn = mock_aws_get_system_time, }; - mock_aws_set_time(0); + mock_aws_set_system_time(0); s_tester.mock_response_code = 529; struct aws_credentials_provider *sts_provider = aws_credentials_provider_new_sts(allocator, &options); @@ -701,6 +701,8 @@ AWS_TEST_CASE( credentials_provider_sts_from_profile_config_environment_succeeds, s_credentials_provider_sts_from_profile_config_environment_succeeds_fn) +#define HIGH_RES_BASE_TIME_NS 101000000000ULL + /* * In this test, we set up a cached provider with a longer and out-of-sync refresh period than the sts * provider that it wraps. We verify that the cached provider factors in the shorter-lived sts credentials @@ -725,17 +727,23 @@ static int s_credentials_provider_sts_cache_expiration_conflict(struct aws_alloc .session_name = s_session_name_cur, .duration_seconds = 0, .function_table = &s_mock_function_table, - .clock_fn = mock_aws_get_time, + .system_clock_fn = mock_aws_get_system_time, }; - mock_aws_set_time(0); + /* make sure high res time and system time are sufficiently diverged that a mistake in the + * respective calculations would fail the test */ + mock_aws_set_system_time(0); + mock_aws_set_high_res_time(HIGH_RES_BASE_TIME_NS); + s_tester.mock_body = aws_byte_buf_from_c_str(success_creds_doc); s_tester.mock_response_code = 200; struct aws_credentials_provider *sts_provider = aws_credentials_provider_new_sts(allocator, &options); - struct aws_credentials_provider_cached_options cached_options = { - .clock_fn = mock_aws_get_time, .refresh_time_in_milliseconds = 1200 * 1000, .source = sts_provider}; + struct aws_credentials_provider_cached_options cached_options = {.system_clock_fn = mock_aws_get_system_time, + .high_res_clock_fn = mock_aws_get_high_res_time, + .refresh_time_in_milliseconds = 1200 * 1000, + .source = sts_provider}; struct aws_credentials_provider *cached_provider = aws_credentials_provider_new_cached(allocator, &cached_options); aws_credentials_provider_get_credentials(cached_provider, s_get_credentials_callback, NULL); @@ -764,8 +772,10 @@ static int s_credentials_provider_sts_cache_expiration_conflict(struct aws_alloc ASSERT_BIN_ARRAYS_EQUALS( s_expected_payload.ptr, s_expected_payload.len, s_tester.request_body.buffer, s_tester.request_body.len); - /* advance time to a little before expiration, verify we get creds with the same expiration */ - mock_aws_set_time(aws_timestamp_convert(800, AWS_TIMESTAMP_SECS, AWS_TIMESTAMP_NANOS, NULL)); + /* advance each time to a little before expiration, verify we get creds with the same expiration */ + uint64_t eight_hundred_seconds_in_ns = aws_timestamp_convert(800, AWS_TIMESTAMP_SECS, AWS_TIMESTAMP_NANOS, NULL); + mock_aws_set_system_time(eight_hundred_seconds_in_ns); + mock_aws_set_high_res_time(HIGH_RES_BASE_TIME_NS + eight_hundred_seconds_in_ns); s_cleanup_creds_callback_data(); @@ -774,8 +784,11 @@ static int s_credentials_provider_sts_cache_expiration_conflict(struct aws_alloc s_aws_wait_for_credentials_result(); ASSERT_TRUE(s_tester.credentials->expiration_timepoint_seconds == 900); - /* advance time to after expiration but before cached provider timeout, verify we get new creds */ - mock_aws_set_time(aws_timestamp_convert(901, AWS_TIMESTAMP_SECS, AWS_TIMESTAMP_NANOS, NULL)); + /* advance each time to after expiration but before cached provider timeout, verify we get new creds */ + uint64_t nine_hundred_and_one_seconds_in_ns = + aws_timestamp_convert(901, AWS_TIMESTAMP_SECS, AWS_TIMESTAMP_NANOS, NULL); + mock_aws_set_system_time(nine_hundred_and_one_seconds_in_ns); + mock_aws_set_high_res_time(HIGH_RES_BASE_TIME_NS + nine_hundred_and_one_seconds_in_ns); s_cleanup_creds_callback_data(); diff --git a/tests/credentials_provider_utils.c b/tests/credentials_provider_utils.c index dc40c428..905021be 100644 --- a/tests/credentials_provider_utils.c +++ b/tests/credentials_provider_utils.c @@ -386,28 +386,53 @@ struct aws_credentials_provider *aws_credentials_provider_new_mock_async( } /* - * mock clock + * mock system clock */ -static struct aws_mutex clock_sync = AWS_MUTEX_INIT; -static uint64_t clock_time = 0; +static struct aws_mutex system_clock_sync = AWS_MUTEX_INIT; +static uint64_t system_clock_time = 0; -int mock_aws_get_time(uint64_t *current_time) { - aws_mutex_lock(&clock_sync); +int mock_aws_get_system_time(uint64_t *current_time) { + aws_mutex_lock(&system_clock_sync); - *current_time = clock_time; + *current_time = system_clock_time; - aws_mutex_unlock(&clock_sync); + aws_mutex_unlock(&system_clock_sync); return AWS_OP_SUCCESS; } -void mock_aws_set_time(uint64_t current_time) { - aws_mutex_lock(&clock_sync); +void mock_aws_set_system_time(uint64_t current_time) { + aws_mutex_lock(&system_clock_sync); - clock_time = current_time; + system_clock_time = current_time; - aws_mutex_unlock(&clock_sync); + aws_mutex_unlock(&system_clock_sync); +} + +/* + * mock high res clock + */ + +static struct aws_mutex high_res_clock_sync = AWS_MUTEX_INIT; +static uint64_t high_res_clock_time = 0; + +int mock_aws_get_high_res_time(uint64_t *current_time) { + aws_mutex_lock(&high_res_clock_sync); + + *current_time = high_res_clock_time; + + aws_mutex_unlock(&high_res_clock_sync); + + return AWS_OP_SUCCESS; +} + +void mock_aws_set_high_res_time(uint64_t current_time) { + aws_mutex_lock(&high_res_clock_sync); + + high_res_clock_time = current_time; + + aws_mutex_unlock(&high_res_clock_sync); } /* diff --git a/tests/credentials_provider_utils.h b/tests/credentials_provider_utils.h index a1d9705c..acd38113 100644 --- a/tests/credentials_provider_utils.h +++ b/tests/credentials_provider_utils.h @@ -96,10 +96,13 @@ struct aws_credentials_provider *aws_credentials_provider_new_mock_async( struct aws_credentials_provider_shutdown_options *shutdown_options); /* - * Simple global clock mock + * Simple global clock mocks */ -int mock_aws_get_time(uint64_t *current_time); -void mock_aws_set_time(uint64_t current_time); +int mock_aws_get_system_time(uint64_t *current_time); +void mock_aws_set_system_time(uint64_t current_time); + +int mock_aws_get_high_res_time(uint64_t *current_time); +void mock_aws_set_high_res_time(uint64_t current_time); /* * Credentials provider that always returns NULL. Useful for chain tests. diff --git a/tests/credentials_tests.c b/tests/credentials_tests.c index f28f9256..3db2447e 100644 --- a/tests/credentials_tests.c +++ b/tests/credentials_tests.c @@ -381,7 +381,8 @@ static int s_verify_callback_status( static int s_cached_credentials_provider_elapsed_test(struct aws_allocator *allocator, void *ctx) { (void)ctx; - mock_aws_set_time(1); + mock_aws_set_system_time(0); + mock_aws_set_high_res_time(1); s_aws_credentials_shutdown_checker_init(); @@ -405,7 +406,8 @@ static int s_cached_credentials_provider_elapsed_test(struct aws_allocator *allo AWS_ZERO_STRUCT(options); options.source = mock_provider; options.refresh_time_in_milliseconds = TEST_CACHE_REFRESH_TIME_MS; - options.clock_fn = mock_aws_get_time; + options.high_res_clock_fn = mock_aws_get_high_res_time; + options.system_clock_fn = mock_aws_get_system_time; options.shutdown_options.shutdown_callback = s_on_shutdown_complete; options.shutdown_options.shutdown_user_data = NULL; @@ -442,8 +444,8 @@ static int s_cached_credentials_provider_elapsed_test(struct aws_allocator *allo uint64_t refresh_in_ns = aws_timestamp_convert(TEST_CACHE_REFRESH_TIME_MS, AWS_TIMESTAMP_MILLIS, AWS_TIMESTAMP_NANOS, NULL); uint64_t now = 0; - mock_aws_get_time(&now); - mock_aws_set_time(now + refresh_in_ns - 1); + mock_aws_get_high_res_time(&now); + mock_aws_set_high_res_time(now + refresh_in_ns - 1); aws_get_credentials_test_callback_result_clean_up(&callback_results); ASSERT_TRUE(s_invoke_get_credentials(cached_provider, &callback_results, 1) == 0); @@ -455,7 +457,7 @@ static int s_cached_credentials_provider_elapsed_test(struct aws_allocator *allo /* * Advance time enough to cause cache expiration, verify we get the second set of mocked credentials */ - mock_aws_set_time(now + refresh_in_ns); + mock_aws_set_high_res_time(now + refresh_in_ns); aws_get_credentials_test_callback_result_clean_up(&callback_results); ASSERT_TRUE(s_invoke_get_credentials(cached_provider, &callback_results, 1) == 0); @@ -484,7 +486,8 @@ static int s_cached_credentials_provider_queued_async_test(struct aws_allocator s_aws_credentials_shutdown_checker_init(); - mock_aws_set_time(1); + mock_aws_set_system_time(0); + mock_aws_set_high_res_time(1); struct aws_credentials *first_creds = aws_credentials_new(allocator, s_access_key_id_1, s_secret_access_key_1, s_session_token_1); @@ -507,7 +510,8 @@ static int s_cached_credentials_provider_queued_async_test(struct aws_allocator AWS_ZERO_STRUCT(options); options.source = mock_provider; options.refresh_time_in_milliseconds = TEST_CACHE_REFRESH_TIME_MS; - options.clock_fn = mock_aws_get_time; + options.high_res_clock_fn = mock_aws_get_high_res_time; + options.system_clock_fn = mock_aws_get_system_time; options.shutdown_options.shutdown_callback = s_on_shutdown_complete; options.shutdown_options.shutdown_user_data = NULL; @@ -545,8 +549,8 @@ static int s_cached_credentials_provider_queued_async_test(struct aws_allocator uint64_t refresh_in_ns = aws_timestamp_convert(TEST_CACHE_REFRESH_TIME_MS, AWS_TIMESTAMP_MILLIS, AWS_TIMESTAMP_NANOS, NULL); uint64_t now = 0; - mock_aws_get_time(&now); - mock_aws_set_time(now + refresh_in_ns - 1); + mock_aws_get_high_res_time(&now); + mock_aws_set_high_res_time(now + refresh_in_ns - 1); aws_get_credentials_test_callback_result_clean_up(&callback_results); ASSERT_TRUE(s_invoke_get_credentials(cached_provider, &callback_results, 2) == 0); @@ -558,7 +562,7 @@ static int s_cached_credentials_provider_queued_async_test(struct aws_allocator /* * Advance time enough to cause cache expiration, verify we get the second set of mocked credentials */ - mock_aws_set_time(now + refresh_in_ns); + mock_aws_set_high_res_time(now + refresh_in_ns); aws_get_credentials_test_callback_result_clean_up(&callback_results); ASSERT_TRUE(s_invoke_get_credentials(cached_provider, &callback_results, 2) == 0);