Properly use and separate high res and system time (#60)

* Properly use and separate high res and system time

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 {

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(&current_time);
+    impl->high_res_clock_fn(&current_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;
     }
 
     /*

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*/

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();
 

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);
 }
 
 /*

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.