Avoid extending IMDS credentials expiry unconditionally (#2694)

## Motivation and Context
Fixes #2687

## Description
The implementation for IMDS static stability support introduced a bug
where returned credentials from IMDS are extended unconditionally, even
though the credentials are not stale. The amount by which credentials
are extended is randomized and it can incorrectly extend the expiry
beyond what's originally set. IMDS produces credentials that last 6
hours, and extending them by at most 25 minutes usually won't be an
issue but when other tools such as Kube2iam and AWSVault are used, the
expiry can be set much shorter than that, causing the issue to occur.

This PR will conditionally extend the credentials' expiry only when the
returned credentials have been expired with respect to the current wall
clock time. Also, the constant values have been adjusted according to
our internal spec.

## Testing
- Added a new unit test for the IMDS credentials provider

## Checklist
<!--- If a checkbox below is not applicable, then please DELETE it
rather than leaving it unchecked -->
- [x] I have updated `CHANGELOG.next.toml` if I made changes to the AWS
SDK, generated SDK code, or SDK runtime crates

----

_By submitting this pull request, I confirm that you can use, modify,
copy, and redistribute this contribution, under the terms of your
choice._

---------

Co-authored-by: Yuki Saito <awsaito@amazon.com>

CHANGELOG.next.toml

@@ -23,3 +23,9 @@ message = "Implement `Ord` and `PartialOrd` for `DateTime`."
 author = "henriiik"
 references = ["smithy-rs#2653", "smithy-rs#2656"]
 meta = { "breaking" = false, "tada" = false, "bug" = false }
+
+[[aws-sdk-rust]]
+message = "Avoid extending IMDS credentials' expiry unconditionally, which may incorrectly extend it beyond what is originally defined; If returned credentials are not stale, use them as they are."
+references = ["smithy-rs#2687", "smithy-rs#2694"]
+meta = { "breaking" = false, "tada" = false, "bug" = true }
+author = "ysaito1001"

aws/rust-runtime/aws-config/src/imds/credentials.rs

@@ -23,7 +23,10 @@ use std::fmt;
 use std::sync::{Arc, RwLock};
 use std::time::{Duration, SystemTime};
 
-const CREDENTIAL_EXPIRATION_INTERVAL: Duration = Duration::from_secs(15 * 60);
+const CREDENTIAL_EXPIRATION_INTERVAL: Duration = Duration::from_secs(10 * 60);
+const WARNING_FOR_EXTENDING_CREDENTIALS_EXPIRY: &str =
+    "Attempting credential expiration extension due to a credential service availability issue. \
+    A refresh of these credentials will be attempted again within the next";
 
 #[derive(Debug)]
 struct ImdsCommunicationError {
@@ -192,25 +195,26 @@ impl ImdsCredentialsProvider {
     //
     // This allows continued use of the credentials even when IMDS returns expired ones.
     fn maybe_extend_expiration(&self, expiration: SystemTime) -> SystemTime {
+        let now = self.time_source.now();
+        // If credentials from IMDS are not stale, use them as they are.
+        if now < expiration {
+            return expiration;
+        }
+
         let rng = fastrand::Rng::with_seed(
-            self.time_source
-                .now()
-                .duration_since(SystemTime::UNIX_EPOCH)
+            now.duration_since(SystemTime::UNIX_EPOCH)
                 .expect("now should be after UNIX EPOCH")
                 .as_secs(),
         );
-        // calculate credentials' refresh offset with jitter
-        let refresh_offset =
-            CREDENTIAL_EXPIRATION_INTERVAL + Duration::from_secs(rng.u64(120..=600));
-        let new_expiry = self.time_source.now() + refresh_offset;
-
-        if new_expiry < expiration {
-            return expiration;
-        }
+        // Calculate credentials' refresh offset with jitter, which should be less than 15 minutes
+        // the smallest amount of time credentials are valid for.
+        // Setting it to something longer than that may have the risk of the credentials expiring
+        // before the next refresh.
+        let refresh_offset = CREDENTIAL_EXPIRATION_INTERVAL + Duration::from_secs(rng.u64(0..=300));
+        let new_expiry = now + refresh_offset;
 
         tracing::warn!(
-            "Attempting credential expiration extension due to a credential service availability issue. \
-            A refresh of these credentials will be attempted again within the next {:.2} minutes.",
+            "{WARNING_FOR_EXTENDING_CREDENTIALS_EXPIRY} {:.2} minutes.",
             refresh_offset.as_secs_f64() / 60.0,
         );
 
@@ -297,7 +301,9 @@ mod test {
     use crate::imds::client::test::{
         imds_request, imds_response, make_client, token_request, token_response,
     };
-    use crate::imds::credentials::ImdsCredentialsProvider;
+    use crate::imds::credentials::{
+        ImdsCredentialsProvider, WARNING_FOR_EXTENDING_CREDENTIALS_EXPIRY,
+    };
     use crate::provider_config::ProviderConfig;
     use aws_credential_types::provider::ProvideCredentials;
     use aws_credential_types::time_source::{TestingTimeSource, TimeSource};
@@ -342,6 +348,54 @@ mod test {
         connection.assert_requests_match(&[]);
     }
 
+    #[tokio::test]
+    #[traced_test]
+    async fn credentials_not_stale_should_be_used_as_they_are() {
+        let connection = TestConnection::new(vec![
+            (
+                token_request("http://169.254.169.254", 21600),
+                token_response(21600, TOKEN_A),
+            ),
+            (
+                imds_request("http://169.254.169.254/latest/meta-data/iam/security-credentials/", TOKEN_A),
+                imds_response(r#"profile-name"#),
+            ),
+            (
+                imds_request("http://169.254.169.254/latest/meta-data/iam/security-credentials/profile-name", TOKEN_A),
+                imds_response("{\n  \"Code\" : \"Success\",\n  \"LastUpdated\" : \"2021-09-20T21:42:26Z\",\n  \"Type\" : \"AWS-HMAC\",\n  \"AccessKeyId\" : \"ASIARTEST\",\n  \"SecretAccessKey\" : \"testsecret\",\n  \"Token\" : \"testtoken\",\n  \"Expiration\" : \"2021-09-21T04:16:53Z\"\n}"),
+            ),
+        ]);
+
+        // set to 2021-09-21T04:16:50Z that makes returned credentials' expiry (2021-09-21T04:16:53Z)
+        // not stale
+        let time_of_request_to_fetch_credentials = UNIX_EPOCH + Duration::from_secs(1632197810);
+        let time_source = TimeSource::testing(&TestingTimeSource::new(
+            time_of_request_to_fetch_credentials,
+        ));
+
+        tokio::time::pause();
+
+        let provider_config = ProviderConfig::no_configuration()
+            .with_http_connector(DynConnector::new(connection.clone()))
+            .with_time_source(time_source)
+            .with_sleep(TokioSleep::new());
+        let client = crate::imds::Client::builder()
+            .configure(&provider_config)
+            .build()
+            .await
+            .expect("valid client");
+        let provider = ImdsCredentialsProvider::builder()
+            .configure(&provider_config)
+            .imds_client(client)
+            .build();
+        let creds = provider.provide_credentials().await.expect("valid creds");
+        // The expiry should be equal to what is originally set (==2021-09-21T04:16:53Z).
+        assert!(creds.expiry() == UNIX_EPOCH.checked_add(Duration::from_secs(1632197813)));
+        connection.assert_requests_match(&[]);
+
+        // There should not be logs indicating credentials are extended for stability.
+        assert!(!logs_contain(WARNING_FOR_EXTENDING_CREDENTIALS_EXPIRY));
+    }
     #[tokio::test]
     #[traced_test]
     async fn expired_credentials_should_be_extended() {
@@ -386,7 +440,7 @@ mod test {
         connection.assert_requests_match(&[]);
 
         // We should inform customers that expired credentials are being used for stability.
-        assert!(logs_contain("Attempting credential expiration extension"));
+        assert!(logs_contain(WARNING_FOR_EXTENDING_CREDENTIALS_EXPIRY));
     }
 
     #[tokio::test]