Skip to content

Commit

Permalink
Use tokio::process to enable timeouts for the CredentialsProcessProvider
Browse files Browse the repository at this point in the history
  • Loading branch information
rcoh committed Oct 20, 2023
1 parent 12fa4d3 commit 2331c34
Show file tree
Hide file tree
Showing 9 changed files with 123 additions and 59 deletions.
12 changes: 12 additions & 0 deletions CHANGELOG.next.toml
Original file line number Diff line number Diff line change
Expand Up @@ -426,3 +426,15 @@ message = "**This change has [detailed upgrade guidance](https://github.com/awsl
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"
3 changes: 2 additions & 1 deletion aws/rust-runtime/aws-config/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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" }
Expand Down
80 changes: 30 additions & 50 deletions aws/rust-runtime/aws-config/src/credential_process.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,57 +3,20 @@
* SPDX-License-Identifier: Apache-2.0
*/

#![cfg(feature = "rt-tokio")]

//! 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>(T);

impl<T> CommandWithSensitiveArgs<T>
where
T: AsRef<str>,
{
pub(crate) fn new(value: T) -> Self {
Self(value)
}

pub(crate) fn unredacted(&self) -> &str {
self.0.as_ref()
}
}

impl<T> fmt::Display for CommandWithSensitiveArgs<T>
where
T: AsRef<str>,
{
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<T> fmt::Debug for CommandWithSensitiveArgs<T>
where
T: AsRef<str>,
{
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
Expand Down Expand Up @@ -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
Expand All @@ -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 =
Expand Down Expand Up @@ -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() {
Expand All @@ -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");
}
}
1 change: 1 addition & 0 deletions aws/rust-runtime/aws-config/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,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;
Expand Down
7 changes: 5 additions & 2 deletions aws/rust-runtime/aws-config/src/profile/credentials.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Cow<'static, str>>,
},
}

Expand Down Expand Up @@ -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}",
)
}
}
Expand Down
21 changes: 18 additions & 3 deletions aws/rust-runtime/aws-config/src/profile/credentials/exec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
*/

use super::repr::{self, BaseProvider};
#[cfg(feature = "rt-tokio")]
use crate::credential_process::CredentialProcessProvider;
use crate::profile::credentials::ProfileFileError;
use crate::provider_config::ProviderConfig;
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -136,6 +150,7 @@ impl ProviderChain {
{
Err(ProfileFileError::FeatureNotEnabled {
feature: "sso".into(),
message: None,
})?
}
}
Expand Down
4 changes: 2 additions & 2 deletions aws/rust-runtime/aws-config/src/profile/credentials/repr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand Down
51 changes: 51 additions & 0 deletions aws/rust-runtime/aws-config/src/sensitive_command.rs
Original file line number Diff line number Diff line change
@@ -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>(T);

impl<T> CommandWithSensitiveArgs<T>
where
T: AsRef<str>,
{
pub(crate) fn new(value: T) -> Self {
Self(value)
}

#[allow(dead_code)]
pub(crate) fn to_owned_string(&self) -> CommandWithSensitiveArgs<String> {
CommandWithSensitiveArgs(self.0.as_ref().to_string())
}

#[allow(dead_code)]
pub(crate) fn unredacted(&self) -> &str {
self.0.as_ref()
}
}

impl<T> fmt::Display for CommandWithSensitiveArgs<T>
where
T: AsRef<str>,
{
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<T> fmt::Debug for CommandWithSensitiveArgs<T>
where
T: AsRef<str>,
{
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
write!(f, "{:?}", format!("{}", self))
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,8 @@ where
fn write(&mut self, buf: &[u8]) -> std::io::Result<usize> {
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())
}
Expand Down

0 comments on commit 2331c34

Please sign in to comment.