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..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,9 +87,12 @@ private static V4aPayloadSigner v4aPayloadSigner( BaseSignRequest request, V4aProperties v4aProperties) { + 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); - if (isChunkEncoding) { + if (useChunkEncoding(isPayloadSigning, isChunkEncoding, isTrailing || isFlexible)) { return AwsChunkedV4aPayloadSigner.builder() .credentialScope(v4aProperties.getCredentialScope()) .chunkSize(DEFAULT_CHUNK_SIZE_IN_BYTES) @@ -98,6 +103,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( @@ -115,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())); @@ -158,27 +169,45 @@ private static AwsSigningConfig signingConfig( return signingConfig; } - private static void configureUnsignedPayload(AwsSigningConfig signingConfig, boolean isChunkEncoding, boolean isTrailing) { - if (isChunkEncoding) { - if (isTrailing) { - signingConfig.setSignedBodyValue(STREAMING_UNSIGNED_PAYLOAD_TRAILER); - } else { - throw new UnsupportedOperationException("Chunk-Encoding without Payload-Signing must have a trailer!"); - } + 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); } 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 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, 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..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 request, V4Properties properties) { boolean isPayloadSigning = isPayloadSigning(request); boolean isEventStreaming = isEventStreaming(request.request()); - boolean isChunkEncoding = request.requireProperty(CHUNK_ENCODING_ENABLED, false) && !hasChecksumHeader(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); if (isEventStreaming) { if (isPayloadSigning) { @@ -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(); } }