fix: ignore hop-by-hop headers when signing requests (#1227)

smithy-lang · Jan 28, 2025 · 9f44cdb · 9f44cdb
1 parent 03badf9
commit 9f44cdb
Show file tree

Hide file tree

Showing 5 changed files with 36 additions and 13 deletions.
diff --git a/.changes/b0646077-397d-48a8-94f2-ea2db40b1dea.json b/.changes/b0646077-397d-48a8-94f2-ea2db40b1dea.json
@@ -0,0 +1,5 @@
+{
+    "id": "b0646077-397d-48a8-94f2-ea2db40b1dea",
+    "type": "bugfix",
+    "description": "Ignore hop-by-hop headers when signing requests"
+}
diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml
@@ -13,7 +13,7 @@ okio-version = "3.9.1"
 otel-version = "1.45.0"
 slf4j-version = "2.0.16"
 slf4j-v1x-version = "1.7.36"
-crt-kotlin-version = "0.9.0"
+crt-kotlin-version = "0.9.1"
 micrometer-version = "1.14.2"
 binary-compatibility-validator-version = "0.16.3"
 

diff --git a/...aws-signing-default/common/src/aws/smithy/kotlin/runtime/auth/awssigning/Canonicalizer.kt b/...aws-signing-default/common/src/aws/smithy/kotlin/runtime/auth/awssigning/Canonicalizer.kt
@@ -58,16 +58,27 @@ internal interface Canonicalizer {
     ): CanonicalRequest
 }
 
-// Taken from https://github.com/awslabs/aws-c-auth/blob/dd505b55fd46222834f35c6e54165d8cbebbfaaa/source/aws_signing.c#L118-L156
 private val skipHeaders = setOf(
-    "connection",
     "expect", // https://github.com/awslabs/aws-sdk-kotlin/issues/862
+
+    // Taken from https://github.com/awslabs/aws-c-auth/blob/274a1d21330731cc51bb742794adc70ada5f4380/source/aws_signing.c#L121-L164
     "sec-websocket-key",
     "sec-websocket-protocol",
     "sec-websocket-version",
-    "upgrade",
     "user-agent",
     "x-amzn-trace-id",
+
+    // Taken from https://datatracker.ietf.org/doc/html/rfc2616#section-13.5.1. These are "hop-by-hop" headers which may
+    // be modified/removed by intervening proxies or caches. These are unsafe to sign because if they change they render
+    // the signature invalid.
+    "connection",
+    "keep-alive",
+    "proxy-authenticate",
+    "proxy-authorization",
+    "te",
+    "trailers",
+    "transfer-encoding",
+    "upgrade",
 )
 
 internal class DefaultCanonicalizer(private val sha256Supplier: HashSupplier = ::Sha256) : Canonicalizer {

diff --git a/...default/common/test/aws/smithy/kotlin/runtime/auth/awssigning/DefaultCanonicalizerTest.kt b/...default/common/test/aws/smithy/kotlin/runtime/auth/awssigning/DefaultCanonicalizerTest.kt
@@ -6,8 +6,12 @@ package aws.smithy.kotlin.runtime.auth.awssigning
 
 import aws.smithy.kotlin.runtime.auth.awscredentials.Credentials
 import aws.smithy.kotlin.runtime.auth.awssigning.tests.DEFAULT_TEST_CREDENTIALS
-import aws.smithy.kotlin.runtime.http.*
-import aws.smithy.kotlin.runtime.http.request.*
+import aws.smithy.kotlin.runtime.http.Headers
+import aws.smithy.kotlin.runtime.http.HttpBody
+import aws.smithy.kotlin.runtime.http.HttpMethod
+import aws.smithy.kotlin.runtime.http.request.HttpRequest
+import aws.smithy.kotlin.runtime.http.request.headers
+import aws.smithy.kotlin.runtime.http.request.url
 import aws.smithy.kotlin.runtime.net.Host
 import aws.smithy.kotlin.runtime.net.url.Url
 import aws.smithy.kotlin.runtime.time.Instant
@@ -136,6 +140,7 @@ class DefaultCanonicalizerTest {
                 // These should not be signed
                 set("Expect", "100-continue")
                 set("X-Amzn-Trace-Id", "qux")
+                set("Transfer-Encoding", "chunked")
             }
             body = HttpBody.Empty
         }

diff --git a/...th/http-auth-aws/common/test/aws/smithy/kotlin/runtime/http/auth/AwsHttpSignerTestBase.kt b/...th/http-auth-aws/common/test/aws/smithy/kotlin/runtime/http/auth/AwsHttpSignerTestBase.kt
@@ -12,7 +12,9 @@ import aws.smithy.kotlin.runtime.auth.awssigning.DefaultAwsSigner
 import aws.smithy.kotlin.runtime.auth.awssigning.internal.AWS_CHUNKED_THRESHOLD
 import aws.smithy.kotlin.runtime.collections.Attributes
 import aws.smithy.kotlin.runtime.collections.get
-import aws.smithy.kotlin.runtime.http.*
+import aws.smithy.kotlin.runtime.http.HttpBody
+import aws.smithy.kotlin.runtime.http.HttpMethod
+import aws.smithy.kotlin.runtime.http.SdkHttpClient
 import aws.smithy.kotlin.runtime.http.operation.*
 import aws.smithy.kotlin.runtime.http.request.HttpRequest
 import aws.smithy.kotlin.runtime.http.request.HttpRequestBuilder
@@ -149,8 +151,8 @@ public abstract class AwsHttpSignerTestBase(
         val op = buildOperation(streaming = true, replayable = false, requestBody = "a".repeat(AWS_CHUNKED_THRESHOLD + 1))
         val expectedDate = "20201016T195600Z"
         val expectedSig = "AWS4-HMAC-SHA256 Credential=AKID/20201016/us-east-1/demo/aws4_request, " +
-            "SignedHeaders=content-encoding;host;transfer-encoding;x-amz-archive-description;x-amz-date;x-amz-decoded-content-length;x-amz-security-token, " +
-            "Signature=ac341b9b248a0b23d2fcd9f7e805f4eb0b8a1b789bb23a8ec6adc6c48dd084ad"
+            "SignedHeaders=content-encoding;host;x-amz-archive-description;x-amz-date;x-amz-decoded-content-length;x-amz-security-token, " +
+            "Signature=ef06c95647c4d2daa6c89ac90274f1c780777cba8eaab772df6d8009def3eb8f"
 
         val signed = getSignedRequest(op)
         assertEquals(expectedDate, signed.headers["X-Amz-Date"])
@@ -162,8 +164,8 @@ public abstract class AwsHttpSignerTestBase(
         val op = buildOperation(streaming = true, replayable = true, requestBody = "a".repeat(AWS_CHUNKED_THRESHOLD + 1))
         val expectedDate = "20201016T195600Z"
         val expectedSig = "AWS4-HMAC-SHA256 Credential=AKID/20201016/us-east-1/demo/aws4_request, " +
-            "SignedHeaders=content-encoding;host;transfer-encoding;x-amz-archive-description;x-amz-date;x-amz-decoded-content-length;x-amz-security-token, " +
-            "Signature=ac341b9b248a0b23d2fcd9f7e805f4eb0b8a1b789bb23a8ec6adc6c48dd084ad"
+            "SignedHeaders=content-encoding;host;x-amz-archive-description;x-amz-date;x-amz-decoded-content-length;x-amz-security-token, " +
+            "Signature=ef06c95647c4d2daa6c89ac90274f1c780777cba8eaab772df6d8009def3eb8f"
 
         val signed = getSignedRequest(op)
         assertEquals(expectedDate, signed.headers["X-Amz-Date"])
@@ -176,8 +178,8 @@ public abstract class AwsHttpSignerTestBase(
         val expectedDate = "20201016T195600Z"
         // should have same signature as testSignAwsChunkedStreamNonReplayable(), except for the hash, since the body is different
         val expectedSig = "AWS4-HMAC-SHA256 Credential=AKID/20201016/us-east-1/demo/aws4_request, " +
-            "SignedHeaders=content-encoding;host;transfer-encoding;x-amz-archive-description;x-amz-date;x-amz-decoded-content-length;x-amz-security-token, " +
-            "Signature=3f0277123c9ed8a8858f793886a0ac0fcb457bc54401ffc22d470f373397cff0"
+            "SignedHeaders=content-encoding;host;x-amz-archive-description;x-amz-date;x-amz-decoded-content-length;x-amz-security-token, " +
+            "Signature=a902702b57057a864bf41cc22ee846a1b7bd047e22784367ec6a459f6791330e"
 
         val signed = getSignedRequest(op)
         assertEquals(expectedDate, signed.headers["X-Amz-Date"])