diff --git a/CHANGELOG.next.toml b/CHANGELOG.next.toml index d85ecdf734..7267f1f4e3 100644 --- a/CHANGELOG.next.toml +++ b/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" diff --git a/aws/rust-runtime/aws-config/src/imds/credentials.rs b/aws/rust-runtime/aws-config/src/imds/credentials.rs index 665d03f801..fd7fd02bab 100644 --- a/aws/rust-runtime/aws-config/src/imds/credentials.rs +++ b/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]