From d4fdb3dc3b20f91c6671e461e07fe22768c2e52e Mon Sep 17 00:00:00 2001 From: Hayden Baker Date: Mon, 2 Oct 2023 16:00:43 -0700 Subject: [PATCH 1/2] Adjust decision for chunk-encoding, fix rolling signer --- .../amazon/awssdk/spotbugs-suppressions.xml | 5 ++++ .../signer/AwsChunkedV4aPayloadSigner.java | 4 +-- .../signer/DefaultAwsCrtV4aHttpSigner.java | 26 ++++++++++++------- .../crt/internal/signer/RollingSigner.java | 10 +++++-- .../signer/DefaultAwsV4HttpSigner.java | 8 +++++- .../DefaultAwsCrtV4aHttpSignerTest.java | 6 +++-- 6 files changed, 41 insertions(+), 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 index 2f476af7d942..b107f1137d83 100644 --- 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 @@ + + + + + diff --git a/core/http-auth-aws/src/main/java/software/amazon/awssdk/http/auth/aws/crt/internal/signer/AwsChunkedV4aPayloadSigner.java b/core/http-auth-aws/src/main/java/software/amazon/awssdk/http/auth/aws/crt/internal/signer/AwsChunkedV4aPayloadSigner.java index 81a019877541..d2245364f89d 100644 --- a/core/http-auth-aws/src/main/java/software/amazon/awssdk/http/auth/aws/crt/internal/signer/AwsChunkedV4aPayloadSigner.java +++ b/core/http-auth-aws/src/main/java/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/core/http-auth-aws/src/main/java/software/amazon/awssdk/http/auth/aws/crt/internal/signer/DefaultAwsCrtV4aHttpSigner.java b/core/http-auth-aws/src/main/java/software/amazon/awssdk/http/auth/aws/crt/internal/signer/DefaultAwsCrtV4aHttpSigner.java index 66973378376f..89288d519412 100644 --- a/core/http-auth-aws/src/main/java/software/amazon/awssdk/http/auth/aws/crt/internal/signer/DefaultAwsCrtV4aHttpSigner.java +++ b/core/http-auth-aws/src/main/java/software/amazon/awssdk/http/auth/aws/crt/internal/signer/DefaultAwsCrtV4aHttpSigner.java @@ -85,9 +85,12 @@ private static V4aPayloadSigner v4aPayloadSigner( BaseSignRequest 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 request, diff --git a/core/http-auth-aws/src/main/java/software/amazon/awssdk/http/auth/aws/crt/internal/signer/RollingSigner.java b/core/http-auth-aws/src/main/java/software/amazon/awssdk/http/auth/aws/crt/internal/signer/RollingSigner.java index 028fcd8a4949..1a22cd8a427f 100644 --- a/core/http-auth-aws/src/main/java/software/amazon/awssdk/http/auth/aws/crt/internal/signer/RollingSigner.java +++ b/core/http-auth-aws/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> 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> 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> 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/core/http-auth-aws/src/main/java/software/amazon/awssdk/http/auth/aws/internal/signer/DefaultAwsV4HttpSigner.java b/core/http-auth-aws/src/main/java/software/amazon/awssdk/http/auth/aws/internal/signer/DefaultAwsV4HttpSigner.java index 5c718314fbbb..69d37c87ccc0 100644 --- a/core/http-auth-aws/src/main/java/software/amazon/awssdk/http/auth/aws/internal/signer/DefaultAwsV4HttpSigner.java +++ b/core/http-auth-aws/src/main/java/software/amazon/awssdk/http/auth/aws/internal/signer/DefaultAwsV4HttpSigner.java @@ -161,6 +161,12 @@ private static Checksummer checksummer(BaseSignRequest 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/core/http-auth-aws/src/test/java/software/amazon/awssdk/http/auth/aws/crt/internal/signer/DefaultAwsCrtV4aHttpSignerTest.java b/core/http-auth-aws/src/test/java/software/amazon/awssdk/http/auth/aws/crt/internal/signer/DefaultAwsCrtV4aHttpSignerTest.java index 868226cec4b9..9703dac8fd22 100644 --- a/core/http-auth-aws/src/test/java/software/amazon/awssdk/http/auth/aws/crt/internal/signer/DefaultAwsCrtV4aHttpSignerTest.java +++ b/core/http-auth-aws/src/test/java/software/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 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(); } } From 457d2d4490d75e87886a0df8718e97afbcfcb922 Mon Sep 17 00:00:00 2001 From: Hayden Baker Date: Mon, 2 Oct 2023 17:55:40 -0700 Subject: [PATCH 2/2] Add isPayloadSigning method --- .../signer/DefaultAwsCrtV4aHttpSigner.java | 33 ++++++++++++++++--- .../signer/DefaultAwsV4HttpSigner.java | 10 +++--- 2 files changed, 33 insertions(+), 10 deletions(-) diff --git a/core/http-auth-aws/src/main/java/software/amazon/awssdk/http/auth/aws/crt/internal/signer/DefaultAwsCrtV4aHttpSigner.java b/core/http-auth-aws/src/main/java/software/amazon/awssdk/http/auth/aws/crt/internal/signer/DefaultAwsCrtV4aHttpSigner.java index 89288d519412..c216241c7d0b 100644 --- a/core/http-auth-aws/src/main/java/software/amazon/awssdk/http/auth/aws/crt/internal/signer/DefaultAwsCrtV4aHttpSigner.java +++ b/core/http-auth-aws/src/main/java/software/amazon/awssdk/http/auth/aws/crt/internal/signer/DefaultAwsCrtV4aHttpSigner.java @@ -25,6 +25,7 @@ import static software.amazon.awssdk.http.auth.aws.crt.internal.util.CrtHttpRequestConverter.toRequest; import static software.amazon.awssdk.http.auth.aws.crt.internal.util.CrtUtils.sanitizeRequest; import static software.amazon.awssdk.http.auth.aws.crt.internal.util.CrtUtils.toCredentials; +import static software.amazon.awssdk.http.auth.aws.internal.signer.util.ChecksumUtil.checksumHeaderName; import static software.amazon.awssdk.http.auth.aws.internal.signer.util.CredentialUtils.isAnonymous; import static software.amazon.awssdk.http.auth.aws.internal.signer.util.CredentialUtils.sanitizeCredentials; import static software.amazon.awssdk.http.auth.aws.internal.signer.util.SignerConstant.PRESIGN_URL_MAX_EXPIRATION_DURATION; @@ -35,6 +36,7 @@ import java.time.Instant; import java.util.concurrent.CompletableFuture; import software.amazon.awssdk.annotations.SdkInternalApi; +import software.amazon.awssdk.checksums.spi.ChecksumAlgorithm; import software.amazon.awssdk.crt.auth.signing.AwsSigner; import software.amazon.awssdk.crt.auth.signing.AwsSigningConfig; import software.amazon.awssdk.crt.auth.signing.AwsSigningResult; @@ -85,7 +87,7 @@ private static V4aPayloadSigner v4aPayloadSigner( BaseSignRequest request, V4aProperties v4aProperties) { - boolean isPayloadSigning = request.requireProperty(PAYLOAD_SIGNING_ENABLED, true); + boolean isPayloadSigning = isPayloadSigning(request); boolean isChunkEncoding = request.requireProperty(CHUNK_ENCODING_ENABLED, false); boolean isTrailing = request.request().firstMatchingHeader(X_AMZ_TRAILER).isPresent(); boolean isFlexible = request.hasProperty(CHECKSUM_ALGORITHM); @@ -124,10 +126,10 @@ private static AwsSigningConfig signingConfig( AuthLocation authLocation = request.requireProperty(AUTH_LOCATION, AuthLocation.HEADER); Duration expirationDuration = request.property(EXPIRATION_DURATION); - boolean isPayloadSigning = request.requireProperty(PAYLOAD_SIGNING_ENABLED, true); + boolean isPayloadSigning = isPayloadSigning(request); boolean isChunkEncoding = request.requireProperty(CHUNK_ENCODING_ENABLED, false); boolean isTrailing = request.request().firstMatchingHeader(X_AMZ_TRAILER).isPresent(); - boolean isFlexible = request.hasProperty(CHECKSUM_ALGORITHM); + boolean isFlexible = request.hasProperty(CHECKSUM_ALGORITHM) && !hasChecksumHeader(request); AwsSigningConfig signingConfig = new AwsSigningConfig(); signingConfig.setCredentials(toCredentials(v4aProperties.getCredentials())); @@ -167,16 +169,26 @@ private static AwsSigningConfig signingConfig( return signingConfig; } + private static boolean isPayloadSigning(BaseSignRequest request) { + boolean isAnonymous = isAnonymous(request.identity()); + boolean isPayloadSigningEnabled = request.requireProperty(PAYLOAD_SIGNING_ENABLED, true); + boolean isEncrypted = "https".equals(request.request().protocol()); + + return !isAnonymous && (isPayloadSigningEnabled || !isEncrypted); + } + private static void configureUnsignedPayload(AwsSigningConfig signingConfig, boolean isChunkEncoding, boolean isTrailingOrFlexible) { if (isChunkEncoding && isTrailingOrFlexible) { - signingConfig.setSignedBodyValue(STREAMING_UNSIGNED_PAYLOAD_TRAILER); + signingConfig.setSignedBodyValue(STREAMING_UNSIGNED_PAYLOAD_TRAILER); } else { signingConfig.setSignedBodyValue(UNSIGNED_PAYLOAD); } } - private static void configurePayloadSigning(AwsSigningConfig signingConfig, boolean isChunkEncoding, boolean isTrailingOrFlexible) { + private static void configurePayloadSigning(AwsSigningConfig signingConfig, boolean isChunkEncoding, + boolean isTrailingOrFlexible) { + if (isChunkEncoding) { if (isTrailingOrFlexible) { signingConfig.setSignedBodyValue(STREAMING_AWS4_ECDSA_P256_SHA256_PAYLOAD_TRAILER); @@ -187,6 +199,17 @@ private static void configurePayloadSigning(AwsSigningConfig signingConfig, bool // if it's non-streaming, crt should calcualte the SHA256 body-hash } + private static boolean hasChecksumHeader(BaseSignRequest request) { + ChecksumAlgorithm checksumAlgorithm = request.property(CHECKSUM_ALGORITHM); + + if (checksumAlgorithm != null) { + String checksumHeaderName = checksumHeaderName(checksumAlgorithm); + return request.request().firstMatchingHeader(checksumHeaderName).isPresent(); + } + + return false; + } + private static SignedRequest doSign(SignRequest request, AwsSigningConfig signingConfig, V4aPayloadSigner payloadSigner) { diff --git a/core/http-auth-aws/src/main/java/software/amazon/awssdk/http/auth/aws/internal/signer/DefaultAwsV4HttpSigner.java b/core/http-auth-aws/src/main/java/software/amazon/awssdk/http/auth/aws/internal/signer/DefaultAwsV4HttpSigner.java index 69d37c87ccc0..afc5fcc3b931 100644 --- a/core/http-auth-aws/src/main/java/software/amazon/awssdk/http/auth/aws/internal/signer/DefaultAwsV4HttpSigner.java +++ b/core/http-auth-aws/src/main/java/software/amazon/awssdk/http/auth/aws/internal/signer/DefaultAwsV4HttpSigner.java @@ -121,9 +121,9 @@ private static Checksummer checksummer(BaseSignRequest