Merge branch 'main' into jdisanti-sra-default-connector

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]