diff --git a/CHANGELOG.next.toml b/CHANGELOG.next.toml index c5c9f7adff055733b56c1088e2d8ca284b9868e8..f74058dd8dd566b9d7e02952df5d1585ff4aaf7c 100644 --- a/CHANGELOG.next.toml +++ b/CHANGELOG.next.toml @@ -427,6 +427,18 @@ references = ["smithy-rs#3043", "smithy-rs#3078"] meta = { "breaking" = true, "tada" = false, "bug" = false, "target" = "client" } author = "rcoh" +[[aws-sdk-rust]] +message = "A bug was fixed where the credentials-process provider was executing the subprocess in the worker thread, potentially stalling the runtime." +references = ["smithy-rs#3052"] +meta = { "breaking" = false, "tada" = false, "bug" = true } +author = "rcoh" + +[[aws-sdk-rust]] +message = "The `credentials-process` feature was added to `aws-config`. If you currently use `no-default-features` for `aws-config`, you MUST enable this feature to use the [`CredentialProcessProvider`](https://docs.rs/aws-config/latest/aws_config/credential_process/struct.CredentialProcessProvider.html) provider directly or via `~/.aws/config`." +references = ["smithy-rs#3052"] +meta = { "breaking" = true, "tada" = false, "bug" = false } +author = "rcoh" + [[smithy-rs]] message = """ `aws_smithy_http::operation::error::{BuildError, SerializationError}` have been moved to `aws_smithy_types::error::operation::{BuildError, SerializationError}`. Type aliases for them are left in `aws_smithy_http` for backwards compatibility but are deprecated. diff --git a/aws/rust-runtime/aws-config/Cargo.toml b/aws/rust-runtime/aws-config/Cargo.toml index 8e40611d28980ec7dfb992068118209f2929c108..c008b7651c0b8726b6fa9e6ccefdf9857d72ffa5 100644 --- a/aws/rust-runtime/aws-config/Cargo.toml +++ b/aws/rust-runtime/aws-config/Cargo.toml @@ -14,8 +14,9 @@ rustls = ["aws-smithy-runtime/tls-rustls", "client-hyper"] allow-compilation = [] # our tests use `cargo test --all-features` and native-tls breaks CI rt-tokio = ["aws-smithy-async/rt-tokio", "aws-smithy-runtime/rt-tokio", "tokio/rt"] sso = ["dep:aws-sdk-sso", "dep:aws-sdk-ssooidc", "dep:ring", "dep:hex", "dep:zeroize", "aws-smithy-runtime-api/http-auth"] +credentials-process = ["tokio/process"] -default = ["client-hyper", "rustls", "rt-tokio"] +default = ["client-hyper", "rustls", "rt-tokio", "credentials-process", "sso"] [dependencies] aws-credential-types = { path = "../../sdk/build/aws-sdk/sdk/aws-credential-types" } diff --git a/aws/rust-runtime/aws-config/src/credential_process.rs b/aws/rust-runtime/aws-config/src/credential_process.rs index c91e6a7ec8c787316ea1677ddd496f47f625c684..a46c6298c6bfff57ea1411d2ea1188abcb81fca7 100644 --- a/aws/rust-runtime/aws-config/src/credential_process.rs +++ b/aws/rust-runtime/aws-config/src/credential_process.rs @@ -3,57 +3,20 @@ * SPDX-License-Identifier: Apache-2.0 */ +#![cfg(feature = "credentials-process")] + //! Credentials Provider for external process use crate::json_credentials::{json_parse_loop, InvalidJsonCredentials, RefreshableCredentials}; +use crate::sensitive_command::CommandWithSensitiveArgs; use aws_credential_types::provider::{self, error::CredentialsError, future, ProvideCredentials}; use aws_credential_types::Credentials; use aws_smithy_json::deserialize::Token; -use std::fmt; use std::process::Command; use std::time::SystemTime; use time::format_description::well_known::Rfc3339; use time::OffsetDateTime; -#[derive(Clone)] -pub(crate) struct CommandWithSensitiveArgs(T); - -impl CommandWithSensitiveArgs -where - T: AsRef, -{ - pub(crate) fn new(value: T) -> Self { - Self(value) - } - - pub(crate) fn unredacted(&self) -> &str { - self.0.as_ref() - } -} - -impl fmt::Display for CommandWithSensitiveArgs -where - T: AsRef, -{ - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - // Security: The arguments for command must be redacted since they can be sensitive - let command = self.0.as_ref(); - match command.find(char::is_whitespace) { - Some(index) => write!(f, "{} ** arguments redacted **", &command[0..index]), - None => write!(f, "{}", command), - } - } -} - -impl fmt::Debug for CommandWithSensitiveArgs -where - T: AsRef, -{ - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "{:?}", format!("{}", self)) - } -} - /// External process credentials provider /// /// This credentials provider runs a configured external process and parses @@ -109,11 +72,17 @@ impl CredentialProcessProvider { } } + pub(crate) fn from_command(command: &CommandWithSensitiveArgs<&str>) -> Self { + Self { + command: command.to_owned_string(), + } + } + async fn credentials(&self) -> provider::Result { // Security: command arguments must be redacted at debug level tracing::debug!(command = %self.command, "loading credentials from external process"); - let mut command = if cfg!(windows) { + let command = if cfg!(windows) { let mut command = Command::new("cmd.exe"); command.args(["/C", self.command.unredacted()]); command @@ -122,16 +91,18 @@ impl CredentialProcessProvider { command.args(["-c", self.command.unredacted()]); command }; - - let output = command.output().map_err(|e| { - CredentialsError::provider_error(format!( - "Error retrieving credentials from external process: {}", - e - )) - })?; + let output = tokio::process::Command::from(command) + .output() + .await + .map_err(|e| { + CredentialsError::provider_error(format!( + "Error retrieving credentials from external process: {}", + e + )) + })?; // Security: command arguments can be logged at trace level - tracing::trace!(command = ?command, status = ?output.status, "executed command (unredacted)"); + tracing::trace!(command = ?self.command, status = ?output.status, "executed command (unredacted)"); if !output.status.success() { let reason = @@ -261,9 +232,10 @@ pub(crate) fn parse_credential_process_json_credentials( mod test { use crate::credential_process::CredentialProcessProvider; use aws_credential_types::provider::ProvideCredentials; - use std::time::SystemTime; + use std::time::{Duration, SystemTime}; use time::format_description::well_known::Rfc3339; use time::OffsetDateTime; + use tokio::time::timeout; #[tokio::test] async fn test_credential_process() { @@ -285,4 +257,12 @@ mod test { ) ); } + + #[tokio::test] + async fn credentials_process_timeouts() { + let provider = CredentialProcessProvider::new(String::from("sleep 1000")); + let _creds = timeout(Duration::from_millis(1), provider.provide_credentials()) + .await + .expect_err("timeout forced"); + } } diff --git a/aws/rust-runtime/aws-config/src/lib.rs b/aws/rust-runtime/aws-config/src/lib.rs index c0cec0cfcdf15cacf0121ef1c1b29bfe684dcde8..59e3a06a14276149b905a4c7dd7156aa43e5e478 100644 --- a/aws/rust-runtime/aws-config/src/lib.rs +++ b/aws/rust-runtime/aws-config/src/lib.rs @@ -127,6 +127,7 @@ pub mod meta; pub mod profile; pub mod provider_config; pub mod retry; +mod sensitive_command; #[cfg(feature = "sso")] pub mod sso; pub(crate) mod standard_property; diff --git a/aws/rust-runtime/aws-config/src/profile/credentials.rs b/aws/rust-runtime/aws-config/src/profile/credentials.rs index 6469ececbcdb64ee8dd9450152a6ebfabd19aa3d..e050f2d9628803db8b95d26ea43c3b7d2ddfc090 100644 --- a/aws/rust-runtime/aws-config/src/profile/credentials.rs +++ b/aws/rust-runtime/aws-config/src/profile/credentials.rs @@ -262,6 +262,8 @@ pub enum ProfileFileError { FeatureNotEnabled { /// The feature or comma delimited list of features that must be enabled feature: Cow<'static, str>, + /// Additional information about the missing feature + message: Option>, }, } @@ -315,10 +317,11 @@ impl Display for ProfileFileError { "profile `{}` did not contain credential information", profile ), - ProfileFileError::FeatureNotEnabled { feature: message } => { + ProfileFileError::FeatureNotEnabled { feature, message } => { + let message = message.as_deref().unwrap_or_default(); write!( f, - "This behavior requires following cargo feature(s) enabled: {message}", + "This behavior requires following cargo feature(s) enabled: {feature}. {message}", ) } } @@ -488,7 +491,10 @@ mod test { make_test!(retry_on_error); make_test!(invalid_config); make_test!(region_override); + #[cfg(feature = "credentials-process")] make_test!(credential_process); + #[cfg(feature = "credentials-process")] make_test!(credential_process_failure); + #[cfg(feature = "credentials-process")] make_test!(credential_process_invalid); } diff --git a/aws/rust-runtime/aws-config/src/profile/credentials/exec.rs b/aws/rust-runtime/aws-config/src/profile/credentials/exec.rs index d393e9469942f6a6d377843d4429fc260c82dc02..6a84c6d7d3dbd10402e8df63e558604364473234 100644 --- a/aws/rust-runtime/aws-config/src/profile/credentials/exec.rs +++ b/aws/rust-runtime/aws-config/src/profile/credentials/exec.rs @@ -4,6 +4,7 @@ */ use super::repr::{self, BaseProvider}; +#[cfg(feature = "credentials-process")] use crate::credential_process::CredentialProcessProvider; use crate::profile::credentials::ProfileFileError; use crate::provider_config::ProviderConfig; @@ -85,9 +86,22 @@ impl ProviderChain { })? } BaseProvider::AccessKey(key) => Arc::new(key.clone()), - BaseProvider::CredentialProcess(credential_process) => Arc::new( - CredentialProcessProvider::new(credential_process.unredacted().into()), - ), + BaseProvider::CredentialProcess(_credential_process) => { + #[cfg(feature = "credentials-process")] + { + Arc::new(CredentialProcessProvider::from_command(_credential_process)) + } + #[cfg(not(feature = "credentials-process"))] + { + Err(ProfileFileError::FeatureNotEnabled { + feature: "credentials-process".into(), + message: Some( + "In order to spawn a subprocess, the `credentials-process` feature must be enabled." + .into(), + ), + })? + } + } BaseProvider::WebIdentityTokenRole { role_arn, web_identity_token_file, @@ -136,6 +150,7 @@ impl ProviderChain { { Err(ProfileFileError::FeatureNotEnabled { feature: "sso".into(), + message: None, })? } } diff --git a/aws/rust-runtime/aws-config/src/profile/credentials/repr.rs b/aws/rust-runtime/aws-config/src/profile/credentials/repr.rs index 666d74ff437eb59e33a1a765e2cc18761dfd444a..4bd044b3d35ee6435cb7bf8328ee80547e1ebf75 100644 --- a/aws/rust-runtime/aws-config/src/profile/credentials/repr.rs +++ b/aws/rust-runtime/aws-config/src/profile/credentials/repr.rs @@ -12,9 +12,9 @@ //! 1-credential-per row (as opposed to a direct profile file representation which can combine //! multiple actions into the same profile). -use crate::credential_process::CommandWithSensitiveArgs; use crate::profile::credentials::ProfileFileError; use crate::profile::{Profile, ProfileSet}; +use crate::sensitive_command::CommandWithSensitiveArgs; use aws_credential_types::Credentials; /// Chain of Profile Providers @@ -408,9 +408,9 @@ fn credential_process_from_profile( #[cfg(test)] mod tests { - use crate::credential_process::CommandWithSensitiveArgs; use crate::profile::credentials::repr::{resolve_chain, BaseProvider, ProfileChain}; use crate::profile::ProfileSet; + use crate::sensitive_command::CommandWithSensitiveArgs; use serde::Deserialize; use std::collections::HashMap; use std::error::Error; diff --git a/aws/rust-runtime/aws-config/src/sensitive_command.rs b/aws/rust-runtime/aws-config/src/sensitive_command.rs new file mode 100644 index 0000000000000000000000000000000000000000..84f87584a3c4f3b989ef7628acf88a44d57e69a6 --- /dev/null +++ b/aws/rust-runtime/aws-config/src/sensitive_command.rs @@ -0,0 +1,51 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +use std::fmt; + +#[derive(Clone)] +pub(crate) struct CommandWithSensitiveArgs(T); + +impl CommandWithSensitiveArgs +where + T: AsRef, +{ + pub(crate) fn new(value: T) -> Self { + Self(value) + } + + #[allow(dead_code)] + pub(crate) fn to_owned_string(&self) -> CommandWithSensitiveArgs { + CommandWithSensitiveArgs(self.0.as_ref().to_string()) + } + + #[allow(dead_code)] + pub(crate) fn unredacted(&self) -> &str { + self.0.as_ref() + } +} + +impl fmt::Display for CommandWithSensitiveArgs +where + T: AsRef, +{ + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + // Security: The arguments for command must be redacted since they can be sensitive + let command = self.0.as_ref(); + match command.find(char::is_whitespace) { + Some(index) => write!(f, "{} ** arguments redacted **", &command[0..index]), + None => write!(f, "{}", command), + } + } +} + +impl fmt::Debug for CommandWithSensitiveArgs +where + T: AsRef, +{ + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{:?}", format!("{}", self)) + } +} diff --git a/rust-runtime/aws-smithy-runtime/src/test_util/capture_test_logs.rs b/rust-runtime/aws-smithy-runtime/src/test_util/capture_test_logs.rs index 85730d00bc585c54faccf1ef4f44fefcd2877f20..86d31d43bbc972c643206e3baff35b916b2a885b 100644 --- a/rust-runtime/aws-smithy-runtime/src/test_util/capture_test_logs.rs +++ b/rust-runtime/aws-smithy-runtime/src/test_util/capture_test_logs.rs @@ -83,7 +83,8 @@ where fn write(&mut self, buf: &[u8]) -> std::io::Result { self.buf.lock().unwrap().extend_from_slice(buf); if !self.quiet { - self.inner.write(buf) + self.inner.write_all(buf)?; + Ok(buf.len()) } else { Ok(buf.len()) }