Repository: rusoto/rusoto (language: rust)

Issue:
mini-RFC: .aws/config's "credential_process" support
botocore supports [sourcing credentials from external processes](https://docs.aws.amazon.com/cli/latest/topic/config-vars.html#sourcing-credentials-from-external-processes):

```
[profile dev]
credential_process = /opt/bin/awscreds-custom
```

The process is expected to output something like:

```json
{
  "Version": 1,
  "AccessKeyId": "<access key id>",
  "SecretAccessKey": "<secret access key>",
  "SessionToken": "<optional session token>",
  "Expiration": "<optional expiration date in ISO8601>"
}
```

It looks like the AWS Go SDK [also recently gained support for this](https://github.com/aws/aws-sdk-go/pull/2217).

I'm interested in adding support for this to rusoto_credential's `ProfileProvider`, but first I'd like to make sure project maintainers are interested in having this feature.


Discussion / hints:
Yes, support for `credential_process` would be excellent!

After reading through the comments on the Go SDK PR, I think we should also include all the warnings about sourcing from an external process. Potential for misuse is higher than other credential providers, but if official SDKs support it, we should too. 👍 

Fix patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 51e8b75a54f..e5bf4dab6e7 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -14,6 +14,7 @@
 - add [apigatewaymanagementapi](https://docs.aws.amazon.com/cli/latest/reference/apigatewaymanagementapi/index.html) service
 - add [apigatewayv2](https://docs.aws.amazon.com/cli/latest/reference/apigatewayv2/index.html) service
 - add [ram](https://docs.aws.amazon.com/cli/latest/reference/ram/index.html) service
+- Add [`credential_process`](https://docs.aws.amazon.com/cli/latest/topic/config-vars.html#sourcing-credentials-from-external-processes) support in `~/.aws/config`
 
 ## [0.36.0] - 2018-12-04
 
diff --git a/rusoto/credential/Cargo.toml b/rusoto/credential/Cargo.toml
index 3a0720fcd3c..2c30e2e1c8f 100644
--- a/rusoto/credential/Cargo.toml
+++ b/rusoto/credential/Cargo.toml
@@ -21,6 +21,8 @@ regex = "1.0.0"
 serde = "1.0.2"
 serde_json = "1.0.2"
 serde_derive = "1.0.2"
+shlex = "0.1.1"
+tokio-process = "0.2.3"
 tokio-timer = "0.2.6"
 
 [dev-dependencies]
diff --git a/rusoto/credential/src/container.rs b/rusoto/credential/src/container.rs
index 09e83fc1145..27b5c753c61 100644
--- a/rusoto/credential/src/container.rs
+++ b/rusoto/credential/src/container.rs
@@ -164,23 +164,7 @@ fn new_request(uri: &str, env_var_name: &str) -> Result<Request<Body>, Credentia
 mod tests {
     use super::*;
     use std::env;
-    use std::sync::{Mutex, MutexGuard};
-
-    // cargo runs tests in parallel, which leads to race conditions when changing
-    // environment variables. Therefore we use a global mutex for all tests which
-    // rely on environment variables.
-    lazy_static! {
-        static ref ENV_MUTEX: Mutex<()> = Mutex::new(());
-    }
-
-    // As failed (panic) tests will poisen the global mutex, we use a helper which
-    // recovers from poisoned mutex.
-    fn lock<'a, T>(mutex: &'a Mutex<T>) -> MutexGuard<'a, T> {
-        match mutex.lock() {
-            Ok(guard) => guard,
-            Err(poisoned) => poisoned.into_inner(),
-        }
-    }
+    use test_utils::{lock, ENV_MUTEX};
 
     #[test]
     fn request_from_relative_uri() {
diff --git a/rusoto/credential/src/environment.rs b/rusoto/credential/src/environment.rs
index 74730aad5c1..f8e8485c108 100644
--- a/rusoto/credential/src/environment.rs
+++ b/rusoto/credential/src/environment.rs
@@ -193,7 +193,7 @@ mod tests {
     use super::*;
     use chrono::Utc;
     use std::env;
-    use std::sync::{Mutex, MutexGuard};
+    use test_utils::{lock, ENV_MUTEX};
 
     static AWS_ACCESS_KEY_ID: &str = "AWS_ACCESS_KEY_ID";
     static AWS_SECRET_ACCESS_KEY: &str = "AWS_SECRET_ACCESS_KEY";
@@ -204,22 +204,6 @@ mod tests {
     static E_NO_SECRET_ACCESS_KEY: &str = "No (or empty) AWS_SECRET_ACCESS_KEY in environment";
     static E_INVALID_EXPIRATION: &str = "Invalid AWS_CREDENTIAL_EXPIRATION in environment";
 
-    // cargo runs tests in parallel, which leads to race conditions when changing
-    // environment variables. Therefore we use a global mutex for all tests which
-    // rely on environment variables.
-    lazy_static! {
-        static ref ENV_MUTEX: Mutex<()> = Mutex::new(());
-    }
-
-    // As failed (panic) tests will poisen the global mutex, we use a helper which
-    // recovers from poisoned mutex.
-    fn lock<'a, T>(mutex: &'a Mutex<T>) -> MutexGuard<'a, T> {
-        match mutex.lock() {
-            Ok(guard) => guard,
-            Err(poisoned) => poisoned.into_inner(),
-        }
-    }
-
     #[test]
     fn get_temporary_credentials_from_env() {
         let _guard = lock(&ENV_MUTEX);
diff --git a/rusoto/credential/src/lib.rs b/rusoto/credential/src/lib.rs
index 8d73e3f5430..0c8131b20c6 100644
--- a/rusoto/credential/src/lib.rs
+++ b/rusoto/credential/src/lib.rs
@@ -17,6 +17,8 @@ extern crate regex;
 extern crate serde_json;
 #[macro_use]
 extern crate serde_derive;
+extern crate shlex;
+extern crate tokio_process;
 extern crate tokio_timer;
 
 pub use container::{ContainerProvider, ContainerProviderFuture};
@@ -342,6 +344,16 @@ impl<P: ProvideAwsCredentials + 'static> ProvideAwsCredentials for AutoRefreshin
 ///
 /// The underlying `ChainProvider` checks multiple sources for credentials, and the `AutoRefreshingProvider`
 /// refreshes the credentials automatically when they expire.
+///
+/// # Warning
+///
+/// This provider allows the [`credential_process`][credential_process] option in the AWS config
+/// file (`~/.aws/config`), a method of sourcing credentials from an external process. This can
+/// potentially be dangerous, so proceed with caution. Other credential providers should be
+/// preferred if at all possible. If using this option, you should make sure that the config file
+/// is as locked down as possible using security best practices for your operating system.
+///
+/// [credential_process]: https://docs.aws.amazon.com/cli/latest/topic/config-vars.html#sourcing-credentials-from-external-processes
 pub struct DefaultCredentialsProvider(AutoRefreshingProvider<ChainProvider>);
 
 impl DefaultCredentialsProvider {
@@ -378,8 +390,9 @@ impl Future for DefaultCredentialsProviderFuture {
 /// The following sources are checked in order for credentials when calling `credentials`:
 ///
 /// 1. Environment variables: `AWS_ACCESS_KEY_ID` and `AWS_SECRET_ACCESS_KEY`
-/// 2. AWS credentials file. Usually located at `~/.aws/credentials`.
-/// 3. IAM instance profile. Will only work if running on an EC2 instance with an instance profile/role.
+/// 2. `credential_process` command in the AWS config file, usually located at `~/.aws/config`.
+/// 3. AWS credentials file. Usually located at `~/.aws/credentials`.
+/// 4. IAM instance profile. Will only work if running on an EC2 instance with an instance profile/role.
 ///
 /// If the sources are exhausted without finding credentials, an error is returned.
 ///
@@ -403,6 +416,16 @@ impl Future for DefaultCredentialsProviderFuture {
 ///   // ...
 /// }
 /// ```
+///
+/// # Warning
+///
+/// This provider allows the [`credential_process`][credential_process] option in the AWS config
+/// file (`~/.aws/config`), a method of sourcing credentials from an external process. This can
+/// potentially be dangerous, so proceed with caution. Other credential providers should be
+/// preferred if at all possible. If using this option, you should make sure that the config file
+/// is as locked down as possible using security best practices for your operating system.
+///
+/// [credential_process]: https://docs.aws.amazon.com/cli/latest/topic/config-vars.html#sourcing-credentials-from-external-processes
 #[derive(Debug, Clone)]
 pub struct ChainProvider {
     environment_provider: EnvironmentProvider,
@@ -517,7 +540,7 @@ mod tests {
     use std::path::Path;
 
     use futures::Future;
-    use test_utils::{is_secret_hidden_behind_asterisks, SECRET};
+    use test_utils::{is_secret_hidden_behind_asterisks, lock, ENV_MUTEX, SECRET};
 
     use super::*;
 
@@ -540,6 +563,7 @@ mod tests {
 
     #[test]
     fn profile_provider_finds_right_credentials_in_file() {
+        let _guard = lock(&ENV_MUTEX);
         let profile_provider = ProfileProvider::with_configuration(
             "tests/sample-data/multiple_profile_credentials",
             "foo",
diff --git a/rusoto/credential/src/profile.rs b/rusoto/credential/src/profile.rs
index b21ec089d67..02dce5c0b12 100644
--- a/rusoto/credential/src/profile.rs
+++ b/rusoto/credential/src/profile.rs
@@ -5,11 +5,13 @@ use std::fs;
 use std::fs::File;
 use std::io::{BufRead, BufReader};
 use std::path::{Path, PathBuf};
+use std::process::Command;
 
 use dirs::home_dir;
 use futures::future::{result, FutureResult};
 use futures::{Future, Poll};
 use regex::Regex;
+use tokio_process::{CommandExt, OutputAsync};
 
 use {non_empty_env_var, AwsCredentials, CredentialsError, ProvideAwsCredentials};
 
@@ -19,7 +21,17 @@ const AWS_SHARED_CREDENTIALS_FILE: &str = "AWS_SHARED_CREDENTIALS_FILE";
 const DEFAULT: &str = "default";
 const REGION: &str = "region";
 
-/// Provides AWS credentials from a profile in a credentials file.
+/// Provides AWS credentials from a profile in a credentials file, or from a credential process.
+///
+/// # Warning
+///
+/// This provider allows the [`credential_process`][credential_process] option, a method of
+/// sourcing credentials from an external process. This can potentially be dangerous, so proceed
+/// with caution. Other credential providers should be preferred if at all possible. If using this
+/// option, you should make sure that the config file is as locked down as possible using security
+/// best practices for your operating system.
+///
+/// [credential_process]: https://docs.aws.amazon.com/cli/latest/topic/config-vars.html#sourcing-credentials-from-external-processes
 #[derive(Clone, Debug)]
 pub struct ProfileProvider {
     /// The File Path the Credentials File is located at.
@@ -158,7 +170,12 @@ impl ProfileProvider {
 
 /// Provides AWS credentials from a profile in a credentials file as a Future.
 pub struct ProfileProviderFuture {
-    inner: FutureResult<AwsCredentials, CredentialsError>,
+    inner: ProfileProviderFutureInner,
+}
+
+enum ProfileProviderFutureInner {
+    Result(FutureResult<AwsCredentials, CredentialsError>),
+    Future(OutputAsync),
 }
 
 impl Future for ProfileProviderFuture {
@@ -166,7 +183,20 @@ impl Future for ProfileProviderFuture {
     type Error = CredentialsError;
 
     fn poll(&mut self) -> Poll<Self::Item, Self::Error> {
-        self.inner.poll()
+        match self.inner {
+            ProfileProviderFutureInner::Result(ref mut result) => result.poll(),
+            ProfileProviderFutureInner::Future(ref mut future) => {
+                let output = try_ready!(future.poll());
+                if !output.status.success() {
+                    return Err(CredentialsError::new(format!(
+                        "Credential process failed with {}: {}",
+                        output.status,
+                        String::from_utf8_lossy(&output.stderr)
+                    )));
+                }
+                Ok(parse_credential_process_output(&output.stdout)?.into())
+            }
+        }
     }
 }
 
@@ -174,18 +204,58 @@ impl ProvideAwsCredentials for ProfileProvider {
     type Future = ProfileProviderFuture;
 
     fn credentials(&self) -> Self::Future {
-        let inner = result(
-            parse_credentials_file(self.file_path()).and_then(|mut profiles| {
-                profiles
-                    .remove(self.profile())
-                    .ok_or_else(|| CredentialsError::new("profile not found"))
-            }),
-        );
+        let inner = match ProfileProvider::default_config_location().map(|location| {
+            parse_config_file(&location).and_then(|config| {
+                config
+                    .get(&ProfileProvider::default_profile_name())
+                    .and_then(|props| props.get("credential_process"))
+                    .map(|value| value.to_owned())
+            })
+        }) {
+            Ok(Some(command)) => {
+                // credential_process is set, create the future
+                match parse_command_str(&command) {
+                    Ok(mut command) => ProfileProviderFutureInner::Future(command.output_async()),
+                    Err(err) => ProfileProviderFutureInner::Result(result(Err(err))),
+                }
+            }
+            Ok(None) => {
+                // credential_process is not set, parse the credentials file
+                ProfileProviderFutureInner::Result(result(
+                    parse_credentials_file(self.file_path()).and_then(|mut profiles| {
+                        profiles
+                            .remove(self.profile())
+                            .ok_or_else(|| CredentialsError::new("profile not found"))
+                    }),
+                ))
+            }
+            Err(err) => ProfileProviderFutureInner::Result(result(Err(err))),
+        };
 
         ProfileProviderFuture { inner: inner }
     }
 }
 
+#[derive(Deserialize)]
+struct CredentialProcessOutput {
+    #[serde(flatten)]
+    creds: AwsCredentials,
+    #[serde(rename = "Version")]
+    version: u8,
+}
+
+fn parse_credential_process_output(v: &[u8]) -> Result<AwsCredentials, CredentialsError> {
+    let output: CredentialProcessOutput = serde_json::from_slice(v)?;
+    if output.version == 1 {
+        Ok(output.creds)
+    } else {
+        Err(CredentialsError::new(format!(
+            "Unsupported version '{}' for credential process provider, supported versions: 1",
+            output.version
+        )))
+    }
+}
+
 // should probably constantize with lazy_static!
 fn new_profile_regex() -> Regex {
     Regex::new(r"^\[(profile )?([^\]]+)\]$").expect("Failed to compile regex")
@@ -343,6 +413,18 @@ fn parse_credentials_file(
     Ok(profiles)
 }
 
+fn parse_command_str(s: &str) -> Result<Command, CredentialsError> {
+    let args = shlex::split(s)
+        .ok_or_else(|| CredentialsError::new("Unable to parse credential_process value."))?;
+    let mut iter = args.iter();
+    let mut command = Command::new(
+        iter.next()
+            .ok_or_else(|| CredentialsError::new("credential_process value is empty."))?,
+    );
+    command.args(iter);
+    Ok(command)
+}
+
 #[cfg(test)]
 mod tests {
 
@@ -350,25 +432,9 @@ mod tests {
     use std::path::Path;
 
     use super::*;
-    use std::sync::{Mutex, MutexGuard};
+    use test_utils::{lock, ENV_MUTEX};
     use {CredentialsError, ProvideAwsCredentials};
 
-    // cargo runs tests in parallel, which leads to race conditions when changing
-    // environment variables. Therefore we use a global mutex for all tests which
-    // rely on environment variables.
-    lazy_static! {
-        static ref ENV_MUTEX: Mutex<()> = Mutex::new(());
-    }
-
-    // As failed (panic) tests will poisen the global mutex, we use a helper which
-    // recovers from poisoned mutex.
-    fn lock<'a, T>(mutex: &'a Mutex<T>) -> MutexGuard<'a, T> {
-        match mutex.lock() {
-            Ok(guard) => guard,
-            Err(poisoned) => poisoned.into_inner(),
-        }
-    }
-
     #[test]
     fn parse_config_file_default_profile() {
         let result = super::parse_config_file(Path::new("tests/sample-data/default_config"));
@@ -404,6 +470,23 @@ mod tests {
         assert_eq!(bar_profile.get("output"), Some(&"json".to_string()));
     }
 
+    #[test]
+    fn parse_config_file_credential_process() {
+        let result =
+            super::parse_config_file(Path::new("tests/sample-data/credential_process_config"));
+        assert!(result.is_some());
+        let profiles = result.unwrap();
+        assert_eq!(profiles.len(), 1);
+        let default_profile = profiles
+            .get(DEFAULT)
+            .expect("No Default profile in default_profile_credentials");
+        assert_eq!(default_profile.get(REGION), Some(&"us-east-2".to_string()));
+        assert_eq!(
+            default_profile.get("credential_process"),
+            Some(&"cat tests/sample-data/credential_process_sample_response".to_string())
+        );
+    }
+
     #[test]
     fn parse_credentials_file_default_profile() {
         let result = super::parse_credentials_file(Path::new(
@@ -462,6 +545,7 @@ mod tests {
 
     #[test]
     fn profile_provider_happy_path() {
+        let _guard = lock(&ENV_MUTEX);
         let provider = ProfileProvider::with_configuration(
             "tests/sample-data/multiple_profile_credentials",
             "foo",
@@ -505,6 +589,7 @@ mod tests {
 
     #[test]
     fn profile_provider_bad_profile() {
+        let _guard = lock(&ENV_MUTEX);
         let provider = ProfileProvider::with_configuration(
             "tests/sample-data/multiple_profile_credentials",
             "not_a_profile",
@@ -518,6 +603,24 @@ mod tests {
         );
     }
 
+    #[test]
+    fn profile_provider_credential_process() {
+        let _guard = lock(&ENV_MUTEX);
+        env::set_var(
+            AWS_CONFIG_FILE,
+            "tests/sample-data/credential_process_config",
+        );
+        let provider = ProfileProvider::new().unwrap();
+        let result = provider.credentials().wait();
+
+        assert!(result.is_ok());
+
+        let creds = result.ok().unwrap();
+        assert_eq!(creds.aws_access_key_id(), "baz_access_key");
+        assert_eq!(creds.aws_secret_access_key(), "baz_secret_key");
+        env::remove_var(AWS_CONFIG_FILE);
+    }
+
     #[test]
     fn profile_provider_profile_name() {
         let _guard = lock(&ENV_MUTEX);


Test patch:
diff --git a/rusoto/credential/src/test_utils.rs b/rusoto/credential/src/test_utils.rs
index 9e7d876d2c7..24d14890ad8 100644
--- a/rusoto/credential/src/test_utils.rs
+++ b/rusoto/credential/src/test_utils.rs
@@ -1,6 +1,7 @@
 #![cfg(test)]
 
 use std::fmt::Debug;
+use std::sync::{Mutex, MutexGuard};
 
 pub const SECRET: &str = &"TtnuieannGt2rGuie2t8Tt7urarg5nauedRndrur";
 
@@ -11,3 +12,19 @@ where
     let debug = format!("{:?}", obj);
     !debug.contains(SECRET) && debug.contains("**********")
 }
+
+// cargo runs tests in parallel, which leads to race conditions when changing
+// environment variables. Therefore we use a global mutex for all tests which
+// rely on environment variables.
+lazy_static! {
+    pub static ref ENV_MUTEX: Mutex<()> = Mutex::new(());
+}
+
+// As failed (panic) tests will poison the global mutex, we use a helper which
+// recovers from poisoned mutex.
+pub fn lock<'a, T>(mutex: &'a Mutex<T>) -> MutexGuard<'a, T> {
+    match mutex.lock() {
+        Ok(guard) => guard,
+        Err(poisoned) => poisoned.into_inner(),
+    }
+}
diff --git a/rusoto/credential/tests/sample-data/credential_process_config b/rusoto/credential/tests/sample-data/credential_process_config
new file mode 100644
index 00000000000..b46587f4003
--- /dev/null
+++ b/rusoto/credential/tests/sample-data/credential_process_config
@@ -0,0 +1,3 @@
+[default]
+credential_process = cat tests/sample-data/credential_process_sample_response
+region = us-east-2
diff --git a/rusoto/credential/tests/sample-data/credential_process_sample_response b/rusoto/credential/tests/sample-data/credential_process_sample_response
new file mode 100644
index 00000000000..1bdf449127a
--- /dev/null
+++ b/rusoto/credential/tests/sample-data/credential_process_sample_response
@@ -0,0 +1,1 @@
+{"Version":1,"AccessKeyId":"baz_access_key","SecretAccessKey":"baz_secret_key"}