Adjust decision for chunk-encoding, fix rolling signer

aws · Oct 3, 2023 · d4fdb3d · d4fdb3d
1 parent 54d5420
commit d4fdb3d
Show file tree

Hide file tree

Showing 6 changed files with 41 additions and 18 deletions.
diff --git a/build-tools/src/main/resources/software/amazon/awssdk/spotbugs-suppressions.xml b/build-tools/src/main/resources/software/amazon/awssdk/spotbugs-suppressions.xml
@@ -141,6 +141,11 @@
         <Bug pattern="EI_EXPOSE_REP" />
     </Match>
 
+    <Match>
+        <Class name="software.amazon.awssdk.http.auth.aws.crt.internal.signer.RollingSigner" />
+        <Bug pattern="EI_EXPOSE_REP" />
+    </Match>
+
     <Match>
         <Class name="software.amazon.awssdk.protocols.json.internal.unmarshall.JsonProtocolUnmarshaller" />
         <Method name="unmarshallStructured" />

diff --git a/.../software/amazon/awssdk/http/auth/aws/crt/internal/signer/AwsChunkedV4aPayloadSigner.java b/.../software/amazon/awssdk/http/auth/aws/crt/internal/signer/AwsChunkedV4aPayloadSigner.java
@@ -17,8 +17,6 @@
 
 import static software.amazon.awssdk.http.auth.aws.internal.signer.util.ChecksumUtil.checksumHeaderName;
 import static software.amazon.awssdk.http.auth.aws.internal.signer.util.ChecksumUtil.fromChecksumAlgorithm;
-import static software.amazon.awssdk.http.auth.aws.internal.signer.util.SignerConstant.AWS_CHUNKED;
-import static software.amazon.awssdk.http.auth.aws.internal.signer.util.SignerConstant.CONTENT_ENCODING;
 import static software.amazon.awssdk.http.auth.aws.internal.signer.util.SignerConstant.STREAMING_ECDSA_SIGNED_PAYLOAD;
 import static software.amazon.awssdk.http.auth.aws.internal.signer.util.SignerConstant.STREAMING_ECDSA_SIGNED_PAYLOAD_TRAILER;
 import static software.amazon.awssdk.http.auth.aws.internal.signer.util.SignerConstant.STREAMING_UNSIGNED_PAYLOAD_TRAILER;
@@ -147,7 +145,7 @@ public void beforeSigning(SdkHttpRequest.Builder request, ContentStreamProvider
             request.appendHeader(X_AMZ_TRAILER, checksumHeaderName);
         }
         request.putHeader(Header.CONTENT_LENGTH, Long.toString(encodedContentLength));
-        request.appendHeader(CONTENT_ENCODING, AWS_CHUNKED);
+        // CRT-signed request doesn't expect 'aws-chunked' Content-Encoding, so we don't add it
     }
 
     /**

diff --git a/.../software/amazon/awssdk/http/auth/aws/crt/internal/signer/DefaultAwsCrtV4aHttpSigner.java b/.../software/amazon/awssdk/http/auth/aws/crt/internal/signer/DefaultAwsCrtV4aHttpSigner.java
@@ -85,9 +85,12 @@ private static V4aPayloadSigner v4aPayloadSigner(
         BaseSignRequest<?, ? extends AwsCredentialsIdentity> request,
         V4aProperties v4aProperties) {
 
+        boolean isPayloadSigning = request.requireProperty(PAYLOAD_SIGNING_ENABLED, true);
         boolean isChunkEncoding = request.requireProperty(CHUNK_ENCODING_ENABLED, false);
+        boolean isTrailing = request.request().firstMatchingHeader(X_AMZ_TRAILER).isPresent();
+        boolean isFlexible = request.hasProperty(CHECKSUM_ALGORITHM);
 
-        if (isChunkEncoding) {
+        if (useChunkEncoding(isPayloadSigning, isChunkEncoding, isTrailing || isFlexible)) {
             return AwsChunkedV4aPayloadSigner.builder()
                                              .credentialScope(v4aProperties.getCredentialScope())
                                              .chunkSize(DEFAULT_CHUNK_SIZE_IN_BYTES)
@@ -98,6 +101,12 @@ private static V4aPayloadSigner v4aPayloadSigner(
         return V4aPayloadSigner.create();
     }
 
+    private static boolean useChunkEncoding(boolean payloadSigningEnabled, boolean chunkEncodingEnabled,
+                                            boolean isTrailingOrFlexible) {
+
+        return (payloadSigningEnabled && chunkEncodingEnabled) || (chunkEncodingEnabled && isTrailingOrFlexible);
+    }
+
     private static Duration validateExpirationDuration(Duration expirationDuration) {
         if (expirationDuration.compareTo(PRESIGN_URL_MAX_EXPIRATION_DURATION) > 0) {
             throw new IllegalArgumentException(
@@ -158,27 +167,24 @@ private static AwsSigningConfig signingConfig(
         return signingConfig;
     }
 
-    private static void configureUnsignedPayload(AwsSigningConfig signingConfig, boolean isChunkEncoding, boolean isTrailing) {
-        if (isChunkEncoding) {
-            if (isTrailing) {
+    private static void configureUnsignedPayload(AwsSigningConfig signingConfig, boolean isChunkEncoding,
+                                                 boolean isTrailingOrFlexible) {
+        if (isChunkEncoding && isTrailingOrFlexible) {
                 signingConfig.setSignedBodyValue(STREAMING_UNSIGNED_PAYLOAD_TRAILER);
-            } else {
-                throw new UnsupportedOperationException("Chunk-Encoding without Payload-Signing must have a trailer!");
-            }
         } else {
             signingConfig.setSignedBodyValue(UNSIGNED_PAYLOAD);
         }
     }
 
-    private static void configurePayloadSigning(AwsSigningConfig signingConfig, boolean isChunkEncoding, boolean isTrailing) {
+    private static void configurePayloadSigning(AwsSigningConfig signingConfig, boolean isChunkEncoding, boolean isTrailingOrFlexible) {
         if (isChunkEncoding) {
-            if (isTrailing) {
+            if (isTrailingOrFlexible) {
                 signingConfig.setSignedBodyValue(STREAMING_AWS4_ECDSA_P256_SHA256_PAYLOAD_TRAILER);
             } else {
                 signingConfig.setSignedBodyValue(STREAMING_AWS4_ECDSA_P256_SHA256_PAYLOAD);
             }
         }
-        // if not chunked encoding, then signed-payload simply means the sha256 hash is included in the canonical request
+        // if it's non-streaming, crt should calcualte the SHA256 body-hash
     }
 
     private static SignedRequest doSign(SignRequest<? extends AwsCredentialsIdentity> request,

diff --git a/...src/main/java/software/amazon/awssdk/http/auth/aws/crt/internal/signer/RollingSigner.java b/...src/main/java/software/amazon/awssdk/http/auth/aws/crt/internal/signer/RollingSigner.java
@@ -49,6 +49,8 @@ private static byte[] signChunk(byte[] chunkBody, byte[] previousSignature, AwsS
         // All the config remains the same as signing config except the Signature Type.
         AwsSigningConfig configCopy = signingConfig.clone();
         configCopy.setSignatureType(AwsSigningConfig.AwsSignatureType.HTTP_REQUEST_CHUNK);
+        configCopy.setSignedBodyHeader(AwsSigningConfig.AwsSignedBodyHeaderType.NONE);
+        configCopy.setSignedBodyValue(null);
 
         HttpRequestBodyStream crtBody = new CrtInputStream(() -> new ByteArrayInputStream(chunkBody));
         return CompletableFutureUtils.joinLikeSync(AwsSigner.signChunk(crtBody, previousSignature, configCopy));
@@ -64,6 +66,8 @@ private static AwsSigningResult signTrailerHeaders(Map<String, List<String>> hea
         // All the config remains the same as signing config except the Signature Type.
         AwsSigningConfig configCopy = signingConfig.clone();
         configCopy.setSignatureType(AwsSigningConfig.AwsSignatureType.HTTP_REQUEST_TRAILING_HEADERS);
+        configCopy.setSignedBodyHeader(AwsSigningConfig.AwsSignedBodyHeaderType.NONE);
+        configCopy.setSignedBodyValue(null);
 
         return CompletableFutureUtils.joinLikeSync(AwsSigner.sign(httpHeaderList, previousSignature, configCopy));
     }
@@ -72,12 +76,14 @@ private static AwsSigningResult signTrailerHeaders(Map<String, List<String>> hea
      * Using a template that incorporates the previous calculated signature, sign the string and return it.
      */
     public byte[] sign(byte[] chunkBody) {
-        return signChunk(chunkBody, previousSignature, signingConfig);
+        previousSignature = signChunk(chunkBody, previousSignature, signingConfig);
+        return previousSignature;
     }
 
     public byte[] sign(Map<String, List<String>> headerMap) {
         AwsSigningResult result = signTrailerHeaders(headerMap, previousSignature, signingConfig);
-        return result != null ? result.getSignature() : new byte[0];
+        previousSignature = result != null ? result.getSignature() : new byte[0];
+        return previousSignature;
     }
 
     public void reset() {

diff --git a/...ain/java/software/amazon/awssdk/http/auth/aws/internal/signer/DefaultAwsV4HttpSigner.java b/...ain/java/software/amazon/awssdk/http/auth/aws/internal/signer/DefaultAwsV4HttpSigner.java
@@ -161,6 +161,12 @@ private static Checksummer checksummer(BaseSignRequest<?, ? extends AwsCredentia
         return Checksummer.forPrecomputed256Checksum(UNSIGNED_PAYLOAD);
     }
 
+    private static boolean useChunkEncoding(boolean payloadSigningEnabled, boolean chunkEncodingEnabled,
+                                            boolean isTrailingOrFlexible) {
+
+        return (payloadSigningEnabled && chunkEncodingEnabled) || (chunkEncodingEnabled && isTrailingOrFlexible);
+    }
+
     private static V4PayloadSigner v4PayloadSigner(
         BaseSignRequest<?, ? extends AwsCredentialsIdentity> request,
         V4Properties properties) {
@@ -182,7 +188,7 @@ private static V4PayloadSigner v4PayloadSigner(
             throw new UnsupportedOperationException("Unsigned payload is not supported with event-streaming.");
         }
 
-        if ((isChunkEncoding && isPayloadSigning) || (isChunkEncoding && (isTrailing || isFlexible))) {
+        if (useChunkEncoding(isPayloadSigning, isChunkEncoding, isTrailing || isFlexible)) {
             return AwsChunkedV4PayloadSigner.builder()
                                             .credentialScope(properties.getCredentialScope())
                                             .chunkSize(DEFAULT_CHUNK_SIZE_IN_BYTES)

diff --git a/...tware/amazon/awssdk/http/auth/aws/crt/internal/signer/DefaultAwsCrtV4aHttpSignerTest.java b/...tware/amazon/awssdk/http/auth/aws/crt/internal/signer/DefaultAwsCrtV4aHttpSignerTest.java
@@ -278,7 +278,7 @@ public void sign_WithPayloadSigningFalseAndChunkEncodingTrueAndTrailer_Delegates
     }
 
     @Test
-    public void sign_WithPayloadSigningFalseAndChunkEncodingTrueWithoutTrailer_Throws() {
+    public void sign_WithPayloadSigningFalseAndChunkEncodingTrueWithoutTrailer_DelegatesToUnsignedPayload() {
         SignRequest<? extends AwsCredentialsIdentity> request = generateBasicRequest(
             AwsCredentialsIdentity.create("access", "secret"),
             httpRequest -> httpRequest
@@ -288,6 +288,8 @@ public void sign_WithPayloadSigningFalseAndChunkEncodingTrueWithoutTrailer_Throw
                 .putProperty(CHUNK_ENCODING_ENABLED, true)
         );
 
-        assertThrows(UnsupportedOperationException.class, () -> signer.sign(request));
+        SignedRequest signedRequest = signer.sign(request);
+        assertThat(signedRequest.request().firstMatchingHeader("x-amz-content-sha256")).hasValue("UNSIGNED-PAYLOAD");
+        assertThat(signedRequest.request().firstMatchingHeader("x-amz-decoded-content-length")).isNotPresent();
     }
 }