Skip to content

Commit

Permalink
Do not sign Transfer-Encoding Header if present on the request (#5895)
Browse files Browse the repository at this point in the history
* Do not sign Transfer-Encoding Header  if present on the request

* Added changelogs

* Added skip in HeaderTransformsHelper too , to be consistent
  • Loading branch information
joviegas authored Feb 20, 2025
1 parent 9389cfa commit f461838
Show file tree
Hide file tree
Showing 8 changed files with 95 additions and 8 deletions.
6 changes: 6 additions & 0 deletions .changes/next-release/bugfix-AWSSDKforJavav2-ed80b40.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"type": "bugfix",
"category": "AWS SDK for Java v2",
"contributor": "",
"description": "Transfer-Encoding headers will no longer be signed during SigV4 authentication"
}
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ public abstract class AbstractAws4Signer<T extends Aws4SignerParams, U extends A
private static final FifoCache<SignerKey> SIGNER_CACHE =
new FifoCache<>(SIGNER_CACHE_MAX_SIZE);
private static final List<String> LIST_OF_HEADERS_TO_IGNORE_IN_LOWER_CASE =
Arrays.asList("connection", "x-amzn-trace-id", "user-agent", "expect");
Arrays.asList("connection", "x-amzn-trace-id", "user-agent", "expect", "transfer-encoding");

protected SdkHttpFullRequest.Builder doSign(SdkHttpFullRequest request,
Aws4SignerRequestParams requestParams,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
public final class HeaderTransformsHelper {

private static final List<String> LIST_OF_HEADERS_TO_IGNORE_IN_LOWER_CASE =
Arrays.asList("connection", "x-amzn-trace-id", "user-agent", "expect");
Arrays.asList("connection", "x-amzn-trace-id", "user-agent", "expect", "transfer-encoding");

private HeaderTransformsHelper() {
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -398,4 +398,18 @@ public void signing_with_Crc32Checksum_with__trailer_header_already_present() th
assertThat(signed.firstMatchingHeader(SignerConstant.X_AMZ_CONTENT_SHA256)).isNotPresent();
assertThat(signed.firstMatchingHeader("Authorization")).hasValue(expectedAuthorization);
}

@Test
public void TransferEncodingIsNotSigned_NotSigned() {
AwsBasicCredentials credentials = AwsBasicCredentials.create("akid", "skid");
SdkHttpFullRequest.Builder request = generateBasicRequest();
request.putHeader("Transfer-Encoding", "chunked");

SdkHttpFullRequest actual = SignerTestUtils.signRequest(signer, request.build(), credentials, "demo", signingOverrideClock, "us-east-1");

assertThat(actual.firstMatchingHeader("Authorization"))
.hasValue("AWS4-HMAC-SHA256 Credential=akid/19810216/us-east-1/demo/aws4_request, " +
"SignedHeaders=host;x-amz-archive-description;x-amz-date, " +
"Signature=581d0042389009a28d461124138f1fe8eeb8daed87611d2a2b47fd3d68d81d73");
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
/*
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License").
* You may not use this file except in compliance with the License.
* A copy of the License is located at
*
* http://aws.amazon.com/apache2.0
*
* or in the "license" file accompanying this file. This file is distributed
* on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
* express or implied. See the License for the specific language governing
* permissions and limitations under the License.
*/


package software.amazon.awssdk.auth.signer.internal.util;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.TreeMap;
import org.junit.jupiter.api.Test;

import static org.junit.jupiter.api.Assertions.*;

class HeaderTransformsHelperTest {

@Test
void shouldExcludeIgnoredHeadersWhenCanonicalizing() {
Map<String, List<String>> headers = new HashMap<>();

// Ignored headers that should be excluded from signing
headers.put("connection", Collections.singletonList("keep-alive"));
headers.put("x-amzn-trace-id", Collections.singletonList("Root=1-234567890"));
headers.put("user-agent", Collections.singletonList("md/user"));
headers.put("expect", Collections.singletonList("100-continue"));
headers.put("transfer-encoding", Collections.singletonList("chunked"));

// Headers that should be included in signing
headers.put("Content-Type", Collections.singletonList("application/json"));
headers.put("Host", Collections.singletonList("example.com"));

Map<String, List<String>> canonicalizedHeaders = HeaderTransformsHelper.canonicalizeSigningHeaders(headers);

assertEquals(2, canonicalizedHeaders.size(), "Should only contain non-ignored headers");

// Verify included headers
assertTrue(canonicalizedHeaders.containsKey("content-type"), "Should contain content-type header");
assertTrue(canonicalizedHeaders.containsKey("host"), "Should contain host header");

// Verify excluded headers
assertFalse(canonicalizedHeaders.containsKey("connection"), "Should not contain connection header");
assertFalse(canonicalizedHeaders.containsKey("x-amzn-trace-id"), "Should not contain x-amzn-trace-id header");
assertFalse(canonicalizedHeaders.containsKey("user-agent"), "Should not contain user-agent header");
assertFalse(canonicalizedHeaders.containsKey("expect"), "Should not contain expect header");
assertFalse(canonicalizedHeaders.containsKey("transfer-encoding"), "Should not contain transfer-encoding header");
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@
@Immutable
public final class V4CanonicalRequest {
private static final List<String> HEADERS_TO_IGNORE_IN_LOWER_CASE =
Arrays.asList("connection", "x-amzn-trace-id", "user-agent", "expect");
Arrays.asList("connection", "x-amzn-trace-id", "user-agent", "expect", "transfer-encoding");

private final SdkHttpRequest request;
private final String contentHash;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ public void canonicalRequest_WithForbiddenHeaders_shouldExcludeForbidden() {
.method(SdkHttpMethod.PUT)
.putHeader("foo", "bar")
.putHeader("x-amzn-trace-id", "wontBePresent")
.putHeader("Transfer-Encoding", "wontBePresent")
.build();
V4CanonicalRequest cr = new V4CanonicalRequest(request, "sha-256",
new V4CanonicalRequest.Options(true,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,8 @@
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.reactivestreams.Subscriber;
import software.amazon.awssdk.auth.credentials.AnonymousCredentialsProvider;
import software.amazon.awssdk.auth.credentials.AwsBasicCredentials;
import software.amazon.awssdk.auth.credentials.StaticCredentialsProvider;
import software.amazon.awssdk.core.SdkBytes;
import software.amazon.awssdk.core.async.AsyncRequestBody;
import software.amazon.awssdk.core.async.AsyncResponseTransformer;
Expand All @@ -44,17 +45,19 @@
public class AsyncRequestCompressionTest {
private static final String UNCOMPRESSED_BODY =
"RequestCompressionTest-RequestCompressionTest-RequestCompressionTest-RequestCompressionTest-RequestCompressionTest";
private static String TRANSFER_ENCODING_HEADER = "Transfer-Encoding";
private String compressedBody;
private int compressedLen;
private MockAsyncHttpClient mockAsyncHttpClient;
private ProtocolRestJsonAsyncClient asyncClient;
private Compressor compressor;
private static final AwsBasicCredentials CLIENT_CREDENTIALS = AwsBasicCredentials.create("akid", "skid");

@BeforeEach
public void setUp() {
mockAsyncHttpClient = new MockAsyncHttpClient();
asyncClient = ProtocolRestJsonAsyncClient.builder()
.credentialsProvider(AnonymousCredentialsProvider.create())
.credentialsProvider(StaticCredentialsProvider.create(CLIENT_CREDENTIALS))
.region(Region.US_EAST_1)
.httpClient(mockAsyncHttpClient)
.build();
Expand Down Expand Up @@ -129,7 +132,9 @@ public void asyncStreamingOperation_compressionEnabled_compressesCorrectly() {
assertThat(loggedBody).isEqualTo(compressedBody);
assertThat(loggedRequest.firstMatchingHeader("Content-encoding").get()).isEqualTo("gzip");
assertThat(loggedRequest.matchingHeaders("Content-Length")).isEmpty();
assertThat(loggedRequest.firstMatchingHeader("Transfer-Encoding").get()).isEqualTo("chunked");
assertThat(loggedRequest.firstMatchingHeader(TRANSFER_ENCODING_HEADER).get()).isEqualTo("chunked");
assertThat(loggedRequest.firstMatchingHeader("Authorization").get())
.doesNotContainIgnoringCase(TRANSFER_ENCODING_HEADER);
}

@Test
Expand Down Expand Up @@ -172,8 +177,8 @@ public void asyncStreamingOperation_compressionEnabledWithRetry_compressesCorrec

assertThat(loggedBody).isEqualTo(compressedBody);
assertThat(loggedRequest.firstMatchingHeader("Content-encoding").get()).isEqualTo("gzip");
assertThat(loggedRequest.matchingHeaders("Content-Length")).isEmpty();
assertThat(loggedRequest.firstMatchingHeader("Transfer-Encoding").get()).isEqualTo("chunked");
assertThat(loggedRequest.matchingHeaders("Content-Length")).isEmpty();assertThat(loggedRequest.firstMatchingHeader(TRANSFER_ENCODING_HEADER).get()).isEqualTo("chunked");
assertThat(loggedRequest.firstMatchingHeader("Authorization").get()).doesNotContainIgnoringCase(TRANSFER_ENCODING_HEADER);
}

private HttpExecuteResponse mockResponse() {
Expand Down

0 comments on commit f461838

Please sign in to comment.