Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Adjust decision for chunk-encoding for CRT, fix rolling signer #4525

Merged
merged 2 commits into from
Oct 3, 2023
Merged

Adjust decision for chunk-encoding for CRT, fix rolling signer #4525

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d4fdb3d
Adjust decision for chunk-encoding, fix rolling signer
haydenbaker Oct 2, 2023
457d2d4
Add isPayloadSigning method
haydenbaker Oct 3, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions build-tools/src/main/resources/software/amazon/awssdk/spotbugs-suppressions.xml
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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();
}
}
Loading