From f5fa40962a00f1288e1a76d08f9ff24fe743de81 Mon Sep 17 00:00:00 2001 From: Russell Cohen Date: Thu, 20 Apr 2023 13:26:01 -0400 Subject: [PATCH] Add credentials exposure test & fix STS + SSO (#2603) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Motivation and Context - credentials providers may leak credentials in the HTTP body at the debug level ## Description 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. - [x] Add environment variable to force logging of the body - [x] consider if we want to suppress request body logging as well ## Testing ## Checklist - [x] I have updated `CHANGELOG.next.toml` if I made changes to the smithy-rs codegen or runtime crates - [x] I have updated `CHANGELOG.next.toml` if I made changes to the AWS SDK, generated SDK code, or SDK runtime crates ---- _By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice._ --------- Co-authored-by: John DiSanti --- CHANGELOG.next.toml | 16 +++ aws/rust-runtime/aws-config/Cargo.toml | 1 + .../src/default_provider/credentials.rs | 4 - .../src/http_credential_provider.rs | 4 + .../aws-config/src/imds/client.rs | 4 + .../aws-config/src/imds/client/token.rs | 4 + aws/rust-runtime/aws-config/src/lib.rs | 4 +- .../aws-config/src/profile/credentials.rs | 2 - .../aws-config/src/profile/profile_file.rs | 2 +- aws/rust-runtime/aws-config/src/test_case.rs | 103 +++++++++++++++++- .../credential_process/fs/home/.aws/config | 3 +- .../aws-sigv4/src/http_request/sign.rs | 16 ++- .../smithy/rustsdk/AwsCodegenDecorator.kt | 2 + .../rustsdk/customize/sso/SSODecorator.kt | 32 ++++++ .../rustsdk/customize/sts/STSDecorator.kt | 6 + .../smithy/generators/SensitiveIndex.kt | 29 +++++ .../protocols/HttpBoundProtocolGenerator.kt | 21 +++- .../smithy/generators/SensitiveIndexTest.kt | 81 ++++++++++++++ .../aws-smithy-http/src/middleware.rs | 12 +- rust-runtime/aws-smithy-http/src/response.rs | 18 +++ 20 files changed, 346 insertions(+), 18 deletions(-) create mode 100644 aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/customize/sso/SSODecorator.kt create mode 100644 codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/SensitiveIndex.kt create mode 100644 codegen-client/src/test/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/SensitiveIndexTest.kt diff --git a/CHANGELOG.next.toml b/CHANGELOG.next.toml index 7992dd6e5c..b9ca3ff1be 100644 --- a/CHANGELOG.next.toml +++ b/CHANGELOG.next.toml @@ -128,3 +128,19 @@ message = "Fix server code generation bug affecting constrained shapes bound wit references = ["smithy-rs#2583", "smithy-rs#2584"] meta = { "breaking" = false, "tada" = false, "bug" = true, "target" = "server" } author = "david-perez" + +[[aws-sdk-rust]] +message = """Reduce several instances of credential exposure in the SDK logs: +- IMDS now suppresses the body of the response from logs +- `aws-sigv4` marks the `x-amz-session-token` header as sensitive +- STS & SSO credentials have been manually marked as sensitive which suppresses logging of response bodies for relevant operations +""" +author = "rcoh" +references = ["smithy-rs#2603"] +meta = { "breaking" = false, "tada" = false, "bug" = false } + +[[smithy-rs]] +message = "Add a sensitive method to `ParseHttpResponse`. When this returns true, logging of the HTTP response body will be suppressed." +author = "rcoh" +references = ["smithy-rs#2603"] +meta = { "breaking" = false, "tada" = false, "bug" = false, "target" = "client" } diff --git a/aws/rust-runtime/aws-config/Cargo.toml b/aws/rust-runtime/aws-config/Cargo.toml index 92cacc1c52..9fb7ff39d4 100644 --- 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 index 7f04b334b5..e1cb203ebc 100644 --- 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 index df894f59ef..e868ffdac4 100644 --- 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 index 6bc8a290bf..05e7236cbd 100644 --- 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 index a4de2c0deb..4cd3f80075 100644 --- 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 index 88f55c4c3f..2d237a5b8f 100644 --- 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 index 9ce085503b..1f043c0959 100644 --- 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 index a90472c8e6..dfe9eb33ca 100644 --- 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 index 38593fcb46..28c10afd7d 100644 --- 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 { Ok(T), ErrorContains(String), @@ -129,6 +135,79 @@ pub(crate) struct Metadata { name: String, } +struct Tee { + buf: Arc>>, + 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>>); +impl Rx { + pub(crate) fn contents(&self) -> String { + String::from_utf8(self.0.lock().unwrap().clone()).unwrap() + } +} + +impl Tee { + fn stdout() -> (Self, Rx) { + let buf: Arc>> = Default::default(); + ( + Tee { + buf: buf.clone(), + quiet: true, + inner: TestWriter::new(), + }, + Rx(buf), + ) + } +} + +impl Tee { + fn loud(&mut self) { + self.quiet = false; + } +} + +impl Write for Tee +where + W: Write, +{ + fn write(&mut self, buf: &[u8]) -> std::io::Result { + 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) -> Result> { 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(&self, make_provider: impl Fn(ProviderConfig) -> F) where F: Future, 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/aws/rust-runtime/aws-config/test-data/profile-provider/credential_process/fs/home/.aws/config b/aws/rust-runtime/aws-config/test-data/profile-provider/credential_process/fs/home/.aws/config index aaec123f3f..6fee217076 100644 --- a/aws/rust-runtime/aws-config/test-data/profile-provider/credential_process/fs/home/.aws/config +++ b/aws/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" }' +# these fake credentials are base64 encoded to prevent triggering the unit test that scans logs for secrets +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 index 97a41ad724..b716b9fd40 100644 --- 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, key: &'static str, value: &str) { - map.insert(key, HeaderValue::try_from(value).expect(key)); +fn add_header(map: &mut HeaderMap, 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 index 21e113a3ca..7c245bb760 100644 --- 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 = 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 new file mode 100644 index 0000000000..f4e14cbe4b --- /dev/null +++ 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 index a332dd3035..9c83a266b5 100644 --- 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() && 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() } } } diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/SensitiveIndex.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/SensitiveIndex.kt new file mode 100644 index 0000000000..15e8d5361c --- /dev/null +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/SensitiveIndex.kt @@ -0,0 +1,29 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +package software.amazon.smithy.rust.codegen.client.smithy.generators + +import software.amazon.smithy.model.Model +import software.amazon.smithy.model.knowledge.KnowledgeIndex +import software.amazon.smithy.model.selector.Selector +import software.amazon.smithy.model.shapes.OperationShape + +class SensitiveIndex(model: Model) : KnowledgeIndex { + private val sensitiveInputSelector = Selector.parse("operation:test(-[input]-> ~> [trait|sensitive])") + private val sensitiveOutputSelector = Selector.parse("operation:test(-[output]-> ~> [trait|sensitive])") + private val sensitiveInputs = sensitiveInputSelector.select(model).map { it.id }.toSet() + private val sensitiveOutputs = sensitiveOutputSelector.select(model).map { it.id }.toSet() + + fun hasSensitiveInput(operationShape: OperationShape): Boolean = sensitiveInputs.contains(operationShape.id) + fun hasSensitiveOutput(operationShape: OperationShape): Boolean = sensitiveOutputs.contains(operationShape.id) + + companion object { + fun of(model: Model): SensitiveIndex { + return model.getKnowledge(SensitiveIndex::class.java) { + SensitiveIndex(it) + } + } + } +} diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/protocols/HttpBoundProtocolGenerator.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/protocols/HttpBoundProtocolGenerator.kt index 45ea108242..e6f3e6e1f7 100644 --- a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/protocols/HttpBoundProtocolGenerator.kt +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/protocols/HttpBoundProtocolGenerator.kt @@ -8,6 +8,7 @@ package software.amazon.smithy.rust.codegen.client.smithy.protocols import software.amazon.smithy.codegen.core.Symbol import software.amazon.smithy.model.shapes.OperationShape import software.amazon.smithy.rust.codegen.client.smithy.ClientCodegenContext +import software.amazon.smithy.rust.codegen.client.smithy.generators.SensitiveIndex import software.amazon.smithy.rust.codegen.client.smithy.generators.protocol.ClientProtocolGenerator import software.amazon.smithy.rust.codegen.client.smithy.generators.protocol.MakeOperationGenerator import software.amazon.smithy.rust.codegen.client.smithy.generators.protocol.ProtocolParserGenerator @@ -68,6 +69,8 @@ open class HttpBoundProtocolTraitImplGenerator( "SdkBody" to RuntimeType.sdkBody(runtimeConfig), ) + private val sensitiveIndex = SensitiveIndex.of(model) + open fun generateTraitImpls( operationWriter: RustWriter, operationShape: OperationShape, @@ -103,6 +106,11 @@ open class HttpBoundProtocolTraitImplGenerator( writeCustomizations(customizations, OperationSection.BeforeParseResponse(customizations, "response")) }, ) + val sensitive = writable { + if (sensitiveIndex.hasSensitiveOutput(operationShape)) { + rust("fn sensitive(&self) -> bool { true }") + } + } rustTemplate( """ impl #{ParseStrict} for $operationName { @@ -118,9 +126,11 @@ open class HttpBoundProtocolTraitImplGenerator( #{parse_response}(status, headers, body) } } + #{sensitive} }""", *codegenScope, *localScope, + "sensitive" to sensitive, ) } @@ -160,7 +170,10 @@ open class HttpBoundProtocolTraitImplGenerator( ) } - private fun parseStreamingResponse(operationShape: OperationShape, customizations: List): RuntimeType { + private fun parseStreamingResponse( + operationShape: OperationShape, + customizations: List, + ): RuntimeType { val outputShape = operationShape.outputShape(model) val outputSymbol = symbolProvider.toSymbol(outputShape) val errorSymbol = symbolProvider.symbolForOperationError(operationShape) @@ -179,7 +192,11 @@ open class HttpBoundProtocolTraitImplGenerator( """ #{parse_streaming_response}(response, &properties) """, - "parse_streaming_response" to parserGenerator.parseStreamingResponseFn(operationShape, true, customizations), + "parse_streaming_response" to parserGenerator.parseStreamingResponseFn( + operationShape, + true, + customizations, + ), ) } } diff --git a/codegen-client/src/test/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/SensitiveIndexTest.kt b/codegen-client/src/test/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/SensitiveIndexTest.kt new file mode 100644 index 0000000000..cecf423588 --- /dev/null +++ b/codegen-client/src/test/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/SensitiveIndexTest.kt @@ -0,0 +1,81 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +package software.amazon.smithy.rust.codegen.client.smithy.generators + +import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.Test +import software.amazon.smithy.rust.codegen.core.testutil.asSmithyModel +import software.amazon.smithy.rust.codegen.core.util.lookup + +class SensitiveIndexTest { + val model = """ + namespace com.example + service TestService { + operations: [ + NotSensitive, + SensitiveInput, + SensitiveOutput, + NestedSensitiveInput, + NestedSensitiveOutput + ] + } + + @sensitive + structure Credentials { + username: String, + password: String + } + + operation NotSensitive { + input := { userId: String } + + output := { response: String } + } + + operation SensitiveInput { + input := { credentials: Credentials } + } + + operation SensitiveOutput { + output := { credentials: Credentials } + } + + operation NestedSensitiveInput { + input := { nested: Nested } + } + + operation NestedSensitiveOutput { + output := { nested: Nested } + } + + structure Nested { + inner: Inner + } + + structure Inner { + credentials: Credentials + } + """.asSmithyModel(smithyVersion = "2.0") + + @Test + fun `correctly identify operations`() { + val index = SensitiveIndex.of(model) + + data class TestCase(val shape: String, val sensitiveInput: Boolean, val sensitiveOutput: Boolean) + + val testCases = listOf( + TestCase("NotSensitive", sensitiveInput = false, sensitiveOutput = false), + TestCase("SensitiveInput", sensitiveInput = true, sensitiveOutput = false), + TestCase("SensitiveOutput", sensitiveInput = false, sensitiveOutput = true), + TestCase("NestedSensitiveInput", sensitiveInput = true, sensitiveOutput = false), + TestCase("NestedSensitiveOutput", sensitiveInput = false, sensitiveOutput = true), + ) + testCases.forEach { tc -> + assertEquals(tc.sensitiveInput, index.hasSensitiveInput(model.lookup("com.example#${tc.shape}")), "input: $tc") + assertEquals(tc.sensitiveOutput, index.hasSensitiveOutput(model.lookup("com.example#${tc.shape}")), "output: $tc ") + } + } +} diff --git a/rust-runtime/aws-smithy-http/src/middleware.rs b/rust-runtime/aws-smithy-http/src/middleware.rs index b77c65b935..d3da9dac2d 100644 --- a/rust-runtime/aws-smithy-http/src/middleware.rs +++ b/rust-runtime/aws-smithy-http/src/middleware.rs @@ -21,6 +21,8 @@ use tracing::{debug_span, trace, Instrument}; type BoxError = Box; +const LOG_SENSITIVE_BODIES: &str = "LOG_SENSITIVE_BODIES"; + /// [`AsyncMapRequest`] defines an asynchronous middleware that transforms an [`operation::Request`]. /// /// Typically, these middleware will read configuration from the `PropertyBag` and use it to @@ -132,7 +134,15 @@ where }; let http_response = http::Response::from_parts(parts, Bytes::from(body)); - trace!(http_response = ?http_response, "read HTTP response body"); + if !handler.sensitive() + || std::env::var(LOG_SENSITIVE_BODIES) + .map(|v| v.eq_ignore_ascii_case("true")) + .unwrap_or_default() + { + trace!(http_response = ?http_response, "read HTTP response body"); + } else { + trace!(http_response = "** REDACTED **. To print, set LOG_SENSITIVE_BODIES=true") + } debug_span!("parse_loaded").in_scope(move || { let parsed = handler.parse_loaded(&http_response); sdk_result( diff --git a/rust-runtime/aws-smithy-http/src/response.rs b/rust-runtime/aws-smithy-http/src/response.rs index 9102fcbab4..7ba039f419 100644 --- a/rust-runtime/aws-smithy-http/src/response.rs +++ b/rust-runtime/aws-smithy-http/src/response.rs @@ -60,6 +60,13 @@ pub trait ParseHttpResponse { /// if `parse_unloaded` will never return `None`, however, it may make your code easier to test if an /// implementation is provided. fn parse_loaded(&self, response: &http::Response) -> Self::Output; + + /// Returns whether the contents of this response are sensitive + /// + /// When this is set to true, wire logging will be disabled + fn sensitive(&self) -> bool { + false + } } /// Convenience Trait for non-streaming APIs @@ -72,6 +79,13 @@ pub trait ParseStrictResponse { /// Parse an [`http::Response`] into `Self::Output`. fn parse(&self, response: &http::Response) -> Self::Output; + + /// Returns whether the contents of this response are sensitive + /// + /// When this is set to true, wire logging will be disabled + fn sensitive(&self) -> bool { + false + } } impl ParseHttpResponse for T { @@ -84,6 +98,10 @@ impl ParseHttpResponse for T { fn parse_loaded(&self, response: &http::Response) -> Self::Output { self.parse(response) } + + fn sensitive(&self) -> bool { + ParseStrictResponse::sensitive(self) + } } #[cfg(test)]