Add credentials exposure test & fix STS + SSO

This adds a test to aws-config that looks for leaked credentials in all of our provider integration tests—since these test use AWS APIs under the hood, this also serves to test AWS services in general. To support this, `sensitive` was added to the ParseHttpResponse trait and code was generated to take action based on this change.
smithy-lang · Apr 19, 2023 · a777342 · a777342
1 parent a2d37ad
commit a777342
Show file tree

Hide file tree

Showing 19 changed files with 322 additions and 18 deletions.
diff --git a/aws/rust-runtime/aws-config/Cargo.toml b/aws/rust-runtime/aws-config/Cargo.toml
@@ -49,6 +49,7 @@ zeroize = { version = "1", optional = true }
 [dev-dependencies]
 futures-util = { version = "0.3.16", default-features = false }
 tracing-test = "0.2.1"
+tracing-subscriber = { version = "0.3.16", features = ["fmt", "json"] }
 
 tokio = { version = "1.23.1", features = ["full", "test-util"] }
 

diff --git a/aws/rust-runtime/aws-config/src/default_provider/credentials.rs b/aws/rust-runtime/aws-config/src/default_provider/credentials.rs
@@ -198,8 +198,6 @@ impl Builder {
 
 #[cfg(test)]
 mod test {
-    use tracing_test::traced_test;
-
     use aws_credential_types::provider::ProvideCredentials;
 
     use crate::default_provider::credentials::DefaultCredentialsChain;
@@ -242,7 +240,6 @@ mod test {
             make_test!($name, execute, $provider_config_builder);
         };
         ($name: ident, $func: ident, $provider_config_builder: expr) => {
-            #[traced_test]
             #[tokio::test]
             async fn $name() {
                 crate::test_case::TestEnvironment::from_dir(concat!(
@@ -324,7 +321,6 @@ mod test {
     }
 
     #[tokio::test]
-    #[traced_test]
     #[cfg(feature = "client-hyper")]
     async fn no_providers_configured_err() {
         use crate::provider_config::ProviderConfig;

diff --git a/aws/rust-runtime/aws-config/src/http_credential_provider.rs b/aws/rust-runtime/aws-config/src/http_credential_provider.rs
@@ -149,6 +149,10 @@ impl ParseStrictResponse for CredentialsResponseParser {
             )),
         }
     }
+
+    fn sensitive(&self) -> bool {
+        true
+    }
 }
 
 #[derive(Clone, Debug)]

diff --git a/aws/rust-runtime/aws-config/src/imds/client.rs b/aws/rust-runtime/aws-config/src/imds/client.rs
@@ -280,6 +280,10 @@ impl ParseStrictResponse for ImdsGetResponseHandler {
             Err(InnerImdsError::BadStatus)
         }
     }
+
+    fn sensitive(&self) -> bool {
+        true
+    }
 }
 
 /// IMDSv2 Endpoint Mode

diff --git a/aws/rust-runtime/aws-config/src/imds/client/token.rs b/aws/rust-runtime/aws-config/src/imds/client/token.rs
@@ -197,4 +197,8 @@ impl ParseStrictResponse for GetTokenResponseHandler {
             expiry: self.time.now() + Duration::from_secs(ttl),
         })
     }
+
+    fn sensitive(&self) -> bool {
+        true
+    }
 }
diff --git a/aws/rust-runtime/aws-config/src/lib.rs b/aws/rust-runtime/aws-config/src/lib.rs
@@ -374,7 +374,7 @@ mod loader {
         ///
         /// # Example: Using a custom profile file path
         ///
-        /// ```
+        /// ```no_run
         /// use aws_config::profile::{ProfileFileCredentialsProvider, ProfileFileRegionProvider};
         /// use aws_config::profile::profile_file::{ProfileFiles, ProfileFileKind};
         ///
@@ -417,7 +417,7 @@ mod loader {
         ///
         /// # Example: Using a custom profile name
         ///
-        /// ```
+        /// ```no_run
         /// use aws_config::profile::{ProfileFileCredentialsProvider, ProfileFileRegionProvider};
         /// use aws_config::profile::profile_file::{ProfileFiles, ProfileFileKind};
         ///

diff --git a/aws/rust-runtime/aws-config/src/profile/credentials.rs b/aws/rust-runtime/aws-config/src/profile/credentials.rs
@@ -455,14 +455,12 @@ async fn build_provider_chain(
 
 #[cfg(test)]
 mod test {
-    use tracing_test::traced_test;
 
     use crate::profile::credentials::Builder;
     use crate::test_case::TestEnvironment;
 
     macro_rules! make_test {
         ($name: ident) => {
-            #[traced_test]
             #[tokio::test]
             async fn $name() {
                 TestEnvironment::from_dir(concat!(

diff --git a/aws/rust-runtime/aws-config/src/profile/profile_file.rs b/aws/rust-runtime/aws-config/src/profile/profile_file.rs
@@ -20,7 +20,7 @@ use std::path::PathBuf;
 ///
 /// # Example: Using a custom profile file path
 ///
-/// ```
+/// ```no_run
 /// use aws_config::profile::{ProfileFileCredentialsProvider, ProfileFileRegionProvider};
 /// use aws_config::profile::profile_file::{ProfileFiles, ProfileFileKind};
 /// use std::sync::Arc;

diff --git a/aws/rust-runtime/aws-config/src/test_case.rs b/aws/rust-runtime/aws-config/src/test_case.rs
@@ -16,11 +16,17 @@ use serde::Deserialize;
 use crate::connector::default_connector;
 use aws_smithy_types::error::display::DisplayErrorContext;
 use std::collections::HashMap;
+use std::env;
 use std::error::Error;
 use std::fmt::Debug;
 use std::future::Future;
+use std::io::Write;
 use std::path::{Path, PathBuf};
+use std::sync::{Arc, Mutex};
 use std::time::{Duration, UNIX_EPOCH};
+use tracing::dispatcher::DefaultGuard;
+use tracing::Level;
+use tracing_subscriber::fmt::TestWriter;
 
 /// Test case credentials
 ///
@@ -84,7 +90,7 @@ impl AsyncSleep for InstantSleep {
     }
 }
 
-#[derive(Deserialize)]
+#[derive(Deserialize, Debug)]
 pub(crate) enum GenericTestResult<T> {
     Ok(T),
     ErrorContains(String),
@@ -129,6 +135,79 @@ pub(crate) struct Metadata {
     name: String,
 }
 
+struct Tee<W> {
+    buf: Arc<Mutex<Vec<u8>>>,
+    quiet: bool,
+    inner: W,
+}
+
+/// Capture logs from this test.
+///
+/// The logs will be captured until the `DefaultGuard` is dropped.
+///
+/// *Why use this instead of traced_test?*
+/// This captures _all_ logs, not just logs produced by the current crate.
+fn capture_test_logs() -> (DefaultGuard, Rx) {
+    // it may be helpful to upstream this at some point
+    let (mut writer, rx) = Tee::stdout();
+    if env::var("VERBOSE_TEST_LOGS").is_ok() {
+        writer.loud();
+    } else {
+        eprintln!("To see full logs from this test set VERBOSE_TEST_LOGS=true");
+    }
+    let subscriber = tracing_subscriber::fmt()
+        .with_max_level(Level::TRACE)
+        .with_writer(Mutex::new(writer))
+        .finish();
+    let guard = tracing::subscriber::set_default(subscriber);
+    (guard, rx)
+}
+
+struct Rx(Arc<Mutex<Vec<u8>>>);
+impl Rx {
+    pub(crate) fn contents(&self) -> String {
+        String::from_utf8(self.0.lock().unwrap().clone()).unwrap()
+    }
+}
+
+impl Tee<TestWriter> {
+    fn stdout() -> (Self, Rx) {
+        let buf: Arc<Mutex<Vec<u8>>> = Default::default();
+        (
+            Tee {
+                buf: buf.clone(),
+                quiet: true,
+                inner: TestWriter::new(),
+            },
+            Rx(buf),
+        )
+    }
+}
+
+impl<W> Tee<W> {
+    fn loud(&mut self) {
+        self.quiet = false;
+    }
+}
+
+impl<W> Write for Tee<W>
+where
+    W: Write,
+{
+    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)
+        } else {
+            Ok(buf.len())
+        }
+    }
+
+    fn flush(&mut self) -> std::io::Result<()> {
+        self.inner.flush()
+    }
+}
+
 impl TestEnvironment {
     pub(crate) async fn from_dir(dir: impl AsRef<Path>) -> Result<TestEnvironment, Box<dyn Error>> {
         let dir = dir.as_ref();
@@ -232,12 +311,26 @@ impl TestEnvironment {
         eprintln!("test case: {}. {}", self.metadata.name, self.metadata.docs);
     }
 
+    fn lines_with_secrets<'a>(&'a self, logs: &'a str) -> Vec<&'a str> {
+        logs.lines().filter(|l| self.contains_secret(l)).collect()
+    }
+
+    fn contains_secret(&self, log_line: &str) -> bool {
+        assert!(log_line.lines().count() <= 1);
+        match &self.metadata.result {
+            // NOTE: we aren't currently erroring if the session token is leaked, that is in the canonical request among other things
+            TestResult::Ok(creds) => log_line.contains(&creds.secret_access_key),
+            TestResult::ErrorContains(_) => false,
+        }
+    }
+
     /// Execute a test case. Failures lead to panics.
     pub(crate) async fn execute<F, P>(&self, make_provider: impl Fn(ProviderConfig) -> F)
     where
         F: Future<Output = P>,
         P: ProvideCredentials,
     {
+        let (_guard, rx) = capture_test_logs();
         let provider = make_provider(self.provider_config.clone()).await;
         let result = provider.provide_credentials().await;
         tokio::time::pause();
@@ -256,6 +349,14 @@ impl TestEnvironment {
             Ok(()) => {}
             Err(e) => panic!("{}", e),
         }
+        let contents = rx.contents();
+        let leaking_lines = self.lines_with_secrets(&contents);
+        assert!(
+            leaking_lines.is_empty(),
+            "secret was exposed\n{:?}\nSee the following log lines:\n  {}",
+            self.metadata.result,
+            leaking_lines.join("\n  ")
+        )
     }
 
     #[track_caller]

diff --git a/...rust-runtime/aws-config/test-data/profile-provider/credential_process/fs/home/.aws/config b/...rust-runtime/aws-config/test-data/profile-provider/credential_process/fs/home/.aws/config
@@ -1,6 +1,7 @@
 [default]
 source_profile = base
-credential_process = echo '{ "Version": 1, "AccessKeyId": "ASIARTESTID", "SecretAccessKey": "TESTSECRETKEY", "SessionToken": "TESTSESSIONTOKEN", "Expiration": "2022-05-02T18:36:00+00:00" }'
+# base64 encoded to prevent triggering the secret scanner
+credential_process = echo eyAiVmVyc2lvbiI6IDEsICJBY2Nlc3NLZXlJZCI6ICJBU0lBUlRFU1RJRCIsICJTZWNyZXRBY2Nlc3NLZXkiOiAiVEVTVFNFQ1JFVEtFWSIsICJTZXNzaW9uVG9rZW4iOiAiVEVTVFNFU1NJT05UT0tFTiIsICJFeHBpcmF0aW9uIjogIjIwMjItMDUtMDJUMTg6MzY6MDArMDA6MDAiIH0K | base64 --decode
 
 [profile base]
 region = us-east-1
diff --git a/aws/rust-runtime/aws-sigv4/src/http_request/sign.rs b/aws/rust-runtime/aws-sigv4/src/http_request/sign.rs
@@ -256,7 +256,7 @@ fn calculate_signing_headers<'a>(
     // Step 4: https://docs.aws.amazon.com/en_pv/general/latest/gr/sigv4-add-signature-to-request.html
     let values = creq.values.as_headers().expect("signing with headers");
     let mut headers = HeaderMap::new();
-    add_header(&mut headers, header::X_AMZ_DATE, &values.date_time);
+    add_header(&mut headers, header::X_AMZ_DATE, &values.date_time, false);
     headers.insert(
         "authorization",
         build_authorization_header(params.access_key, &creq, sts, &signature),
@@ -266,18 +266,26 @@ fn calculate_signing_headers<'a>(
             &mut headers,
             header::X_AMZ_CONTENT_SHA_256,
             &values.content_sha256,
+            false,
         );
     }
 
     if let Some(security_token) = params.security_token {
-        add_header(&mut headers, header::X_AMZ_SECURITY_TOKEN, security_token);
+        add_header(
+            &mut headers,
+            header::X_AMZ_SECURITY_TOKEN,
+            security_token,
+            true,
+        );
     }
 
     Ok(SigningOutput::new(headers, signature))
 }
 
-fn add_header(map: &mut HeaderMap<HeaderValue>, key: &'static str, value: &str) {
-    map.insert(key, HeaderValue::try_from(value).expect(key));
+fn add_header(map: &mut HeaderMap<HeaderValue>, key: &'static str, value: &str, sensitive: bool) {
+    let mut value = HeaderValue::try_from(value).expect(key);
+    value.set_sensitive(sensitive);
+    map.insert(key, value);
 }
 
 // add signature to authorization header

diff --git a/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/AwsCodegenDecorator.kt b/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/AwsCodegenDecorator.kt
@@ -19,6 +19,7 @@ import software.amazon.smithy.rustsdk.customize.route53.Route53Decorator
 import software.amazon.smithy.rustsdk.customize.s3.S3Decorator
 import software.amazon.smithy.rustsdk.customize.s3.S3ExtendedRequestIdDecorator
 import software.amazon.smithy.rustsdk.customize.s3control.S3ControlDecorator
+import software.amazon.smithy.rustsdk.customize.sso.SSODecorator
 import software.amazon.smithy.rustsdk.customize.sts.STSDecorator
 import software.amazon.smithy.rustsdk.endpoints.AwsEndpointsStdLib
 import software.amazon.smithy.rustsdk.endpoints.OperationInputTestDecorator
@@ -65,6 +66,7 @@ val DECORATORS: List<ClientCodegenDecorator> = listOf(
     ),
     S3ControlDecorator().onlyApplyTo("com.amazonaws.s3control#AWSS3ControlServiceV20180820"),
     STSDecorator().onlyApplyTo("com.amazonaws.sts#AWSSecurityTokenServiceV20110615"),
+    SSODecorator().onlyApplyTo("com.amazonaws.sso#SWBPortalService"),
 
     // Only build docs-rs for linux to reduce load on docs.rs
     listOf(

diff --git a/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/customize/sso/SSODecorator.kt b/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/customize/sso/SSODecorator.kt
@@ -0,0 +1,32 @@
+/*
+ * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package software.amazon.smithy.rustsdk.customize.sso
+
+import software.amazon.smithy.model.Model
+import software.amazon.smithy.model.shapes.ServiceShape
+import software.amazon.smithy.model.shapes.Shape
+import software.amazon.smithy.model.shapes.ShapeId
+import software.amazon.smithy.model.shapes.StructureShape
+import software.amazon.smithy.model.traits.SensitiveTrait
+import software.amazon.smithy.model.transform.ModelTransformer
+import software.amazon.smithy.rust.codegen.client.smithy.customize.ClientCodegenDecorator
+import software.amazon.smithy.rust.codegen.core.util.letIf
+import java.util.logging.Logger
+
+class SSODecorator : ClientCodegenDecorator {
+    override val name: String = "SSO"
+    override val order: Byte = 0
+    private val logger: Logger = Logger.getLogger(javaClass.name)
+
+    private fun isAwsCredentials(shape: Shape): Boolean = shape.id == ShapeId.from("com.amazonaws.sso#RoleCredentials")
+
+    override fun transformModel(service: ServiceShape, model: Model): Model =
+        ModelTransformer.create().mapShapes(model) { shape ->
+            shape.letIf(isAwsCredentials(shape)) {
+                (shape as StructureShape).toBuilder().addTrait(SensitiveTrait()).build()
+            }
+        }
+}
diff --git a/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/customize/sts/STSDecorator.kt b/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/customize/sts/STSDecorator.kt
@@ -7,9 +7,11 @@ package software.amazon.smithy.rustsdk.customize.sts
 import software.amazon.smithy.model.Model
 import software.amazon.smithy.model.shapes.ServiceShape
 import software.amazon.smithy.model.shapes.Shape
+import software.amazon.smithy.model.shapes.ShapeId
 import software.amazon.smithy.model.shapes.StructureShape
 import software.amazon.smithy.model.traits.ErrorTrait
 import software.amazon.smithy.model.traits.RetryableTrait
+import software.amazon.smithy.model.traits.SensitiveTrait
 import software.amazon.smithy.model.transform.ModelTransformer
 import software.amazon.smithy.rust.codegen.client.smithy.customize.ClientCodegenDecorator
 import software.amazon.smithy.rust.codegen.core.util.hasTrait
@@ -25,6 +27,8 @@ class STSDecorator : ClientCodegenDecorator {
         shape is StructureShape && shape.hasTrait<ErrorTrait>() &&
             shape.id.namespace == "com.amazonaws.sts" && shape.id.name == "IDPCommunicationErrorException"
 
+    private fun isAwsCredentials(shape: Shape): Boolean = shape.id == ShapeId.from("com.amazonaws.sts#Credentials")
+
     override fun transformModel(service: ServiceShape, model: Model): Model =
         ModelTransformer.create().mapShapes(model) { shape ->
             shape.letIf(isIdpCommunicationError(shape)) {
@@ -33,6 +37,8 @@ class STSDecorator : ClientCodegenDecorator {
                     .removeTrait(ErrorTrait.ID)
                     .addTrait(ErrorTrait("server"))
                     .addTrait(RetryableTrait.builder().build()).build()
+            }.letIf(isAwsCredentials(shape)) {
+                (shape as StructureShape).toBuilder().addTrait(SensitiveTrait()).build()
             }
         }
 }