Skip to content

Commit

Permalink
fix!: gate logging intermediate signing results which may contain sen…
Browse files Browse the repository at this point in the history
…sitive information (#984)
  • Loading branch information
aajtodd authored Oct 21, 2023
1 parent d3b41f4 commit ed49a46
Show file tree
Hide file tree
Showing 6 changed files with 25 additions and 2 deletions.
5 changes: 5 additions & 0 deletions .changes/b1a1bee4-e3e4-4520-ae8e-48d3b0e94ae9.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"id": "b1a1bee4-e3e4-4520-ae8e-48d3b0e94ae9",
"type": "bugfix",
"description": "Do not log intermediate signature calculations without explicit opt-in via `LogMode.LogSigning`."
}
2 changes: 1 addition & 1 deletion gradle.properties
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,4 @@ kotlin.native.ignoreDisabledTargets=true
sdkVersion=0.27.8-SNAPSHOT

# kotlin
kotlinVersion=1.8.22
kotlinVersion=1.8.22
3 changes: 3 additions & 0 deletions runtime/auth/aws-signing-common/api/aws-signing-common.api
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ public final class aws/smithy/kotlin/runtime/auth/awssigning/AwsSigningConfig {
public final fun getCredentials ()Laws/smithy/kotlin/runtime/auth/awscredentials/Credentials;
public final fun getExpiresAfter-FghU774 ()Lkotlin/time/Duration;
public final fun getHashSpecification ()Laws/smithy/kotlin/runtime/auth/awssigning/HashSpecification;
public final fun getLogRequest ()Z
public final fun getNormalizeUriPath ()Z
public final fun getOmitSessionToken ()Z
public final fun getRegion ()Ljava/lang/String;
Expand All @@ -69,6 +70,7 @@ public final class aws/smithy/kotlin/runtime/auth/awssigning/AwsSigningConfig$Bu
public final fun getCredentials ()Laws/smithy/kotlin/runtime/auth/awscredentials/Credentials;
public final fun getExpiresAfter-FghU774 ()Lkotlin/time/Duration;
public final fun getHashSpecification ()Laws/smithy/kotlin/runtime/auth/awssigning/HashSpecification;
public final fun getLogRequest ()Z
public final fun getNormalizeUriPath ()Z
public final fun getOmitSessionToken ()Z
public final fun getRegion ()Ljava/lang/String;
Expand All @@ -82,6 +84,7 @@ public final class aws/smithy/kotlin/runtime/auth/awssigning/AwsSigningConfig$Bu
public final fun setCredentials (Laws/smithy/kotlin/runtime/auth/awscredentials/Credentials;)V
public final fun setExpiresAfter-BwNAW2A (Lkotlin/time/Duration;)V
public final fun setHashSpecification (Laws/smithy/kotlin/runtime/auth/awssigning/HashSpecification;)V
public final fun setLogRequest (Z)V
public final fun setNormalizeUriPath (Z)V
public final fun setOmitSessionToken (Z)V
public final fun setRegion (Ljava/lang/String;)V
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,14 @@ public class AwsSigningConfig(builder: Builder) {
*/
public val expiresAfter: Duration? = builder.expiresAfter

/**
* Flag enabling whether detailed trace logging is enabled (if the signer implementation supports it). When true
* signers should emit intermediate logging details such as the canonical request at the trace level.
* This is an opt-in configuration because these intermediate outputs may contain sensitive fields bound
* to headers, URI, or query parameters.
*/
public val logRequest: Boolean = builder.logRequest

public fun toBuilder(): Builder = Builder().also {
it.region = region
it.service = service
Expand All @@ -178,6 +186,7 @@ public class AwsSigningConfig(builder: Builder) {
it.signedBodyHeader = signedBodyHeader
it.credentials = credentials
it.expiresAfter = expiresAfter
it.logRequest = logRequest
}

public class Builder {
Expand All @@ -194,6 +203,7 @@ public class AwsSigningConfig(builder: Builder) {
public var signedBodyHeader: AwsSignedBodyHeader = AwsSignedBodyHeader.NONE
public var credentials: Credentials? = null
public var expiresAfter: Duration? = null
public var logRequest: Boolean = false

public fun build(): AwsSigningConfig = AwsSigningConfig(this)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,9 @@ internal class DefaultAwsSignerImpl(
require(config.algorithm == AwsSigningAlgorithm.SIGV4) { "${config.algorithm} support is not yet implemented" }

val canonical = canonicalizer.canonicalRequest(request, config)
logger.trace { "Canonical request:\n${canonical.requestString}" }
if (config.logRequest) {
logger.trace { "Canonical request:\n${canonical.requestString}" }
}

val stringToSign = signatureCalculator.stringToSign(canonical.requestString, config)
logger.trace { "String to sign:\n$stringToSign" }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ import aws.smithy.kotlin.runtime.auth.awssigning.internal.isEligibleForAwsChunke
import aws.smithy.kotlin.runtime.auth.awssigning.internal.setAwsChunkedBody
import aws.smithy.kotlin.runtime.auth.awssigning.internal.setAwsChunkedHeaders
import aws.smithy.kotlin.runtime.auth.awssigning.internal.useAwsChunkedEncoding
import aws.smithy.kotlin.runtime.client.LogMode
import aws.smithy.kotlin.runtime.client.SdkClientOption
import aws.smithy.kotlin.runtime.http.HttpBody
import aws.smithy.kotlin.runtime.http.operation.HttpOperationContext
import aws.smithy.kotlin.runtime.http.request.HttpRequest
Expand Down Expand Up @@ -138,6 +140,7 @@ public class AwsHttpSigner(private val config: Config) : HttpSigner {
shouldSignHeader = config.shouldSignHeader

signedBodyHeader = contextSignedBodyHeader ?: config.signedBodyHeader
logRequest = attributes.getOrNull(SdkClientOption.LogMode)?.isEnabled(LogMode.LogRequest) == true

// SDKs are supposed to default to signed payload _always_ when possible (and when `unsignedPayload` trait
// isn't present). The only exception is when the customer explicitly disables signed payloads (via Config.isUnsignedPayload).
Expand Down

0 comments on commit ed49a46

Please sign in to comment.