Skip to content

Commit

Permalink
Adjust decision for chunk-encoding for CRT, fix rolling signer (#4525)
Browse files Browse the repository at this point in the history 
* Adjust decision for chunk-encoding, fix rolling signer

* Add isPayloadSigning method
  • Loading branch information
@haydenbaker
haydenbaker authored Oct 3, 2023
1 parent 7017df5 commit b3b383f
Show file tree
Hide file tree
Showing 6 changed files with 72 additions and 26 deletions.
5 changes: 5 additions & 0 deletions build-tools/src/main/resources/software/amazon/awssdk/spotbugs-suppressions.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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" />
Expand Down
4 changes: 1 addition & 3 deletions .../software/amazon/awssdk/http/auth/aws/crt/internal/signer/AwsChunkedV4aPayloadSigner.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
}

/**
Expand Down
55 changes: 42 additions & 13 deletions .../software/amazon/awssdk/http/auth/aws/crt/internal/signer/DefaultAwsCrtV4aHttpSigner.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -85,9 +87,12 @@ private static V4aPayloadSigner v4aPayloadSigner(
BaseSignRequest<?, ? extends AwsCredentialsIdentity> 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)
Expand All @@ -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(
Expand All @@ -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()));
Expand Down Expand Up @@ -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<?, ? extends AwsCredentialsIdentity> 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<?, ? extends AwsCredentialsIdentity> 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<? extends AwsCredentialsIdentity> request,
Expand Down
10 changes: 8 additions & 2 deletions ...src/main/java/software/amazon/awssdk/http/auth/aws/crt/internal/signer/RollingSigner.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand All @@ -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));
}
Expand All @@ -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() {
Expand Down
18 changes: 12 additions & 6 deletions ...ain/java/software/amazon/awssdk/http/auth/aws/internal/signer/DefaultAwsV4HttpSigner.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -121,9 +121,9 @@ private static Checksummer checksummer(BaseSignRequest<?, ? extends AwsCredentia
boolean isPayloadSigning = isPayloadSigning(request);
boolean isEventStreaming = isEventStreaming(request.request());
boolean hasChecksumHeader = hasChecksumHeader(request);
boolean isChunkEncoding = request.requireProperty(CHUNK_ENCODING_ENABLED, false) && !hasChecksumHeader;
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;
boolean isAnonymous = CredentialUtils.isAnonymous(request.identity());

if (isEventStreaming) {
Expand All @@ -150,7 +150,7 @@ private static Checksummer checksummer(BaseSignRequest<?, ? extends AwsCredentia
}
}

if (isFlexible && !hasChecksumHeader) {
if (isFlexible) {
return Checksummer.forFlexibleChecksum(UNSIGNED_PAYLOAD, request.property(CHECKSUM_ALGORITHM));
}

Expand All @@ -161,15 +161,21 @@ 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) {

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) {
Expand All @@ -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)
Expand Down
6 changes: 4 additions & 2 deletions ...tware/amazon/awssdk/http/auth/aws/crt/internal/signer/DefaultAwsCrtV4aHttpSignerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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();
}
}

0 comments on commit b3b383f

Please sign in to comment.