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

Handling Anonymous, chunking decisions, and existing checksums #4517

Merged
merged 3 commits into from
Oct 2, 2023
Merged

Handling Anonymous, chunking decisions, and existing checksums #4517

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5491a9a
Handling Anonymous, chunking decisions, and existing checksums
haydenbaker Sep 29, 2023
697bc82
handle anonymous identity, adjust chunking decision
haydenbaker Sep 29, 2023
2657511
Test, javadoc
haydenbaker Sep 30, 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
4 changes: 4 additions & 0 deletions ...h-aws/src/main/java/software/amazon/awssdk/http/auth/aws/internal/signer/Checksummer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,10 @@ static Checksummer forFlexibleChecksum(String precomputedSha256, ChecksumAlgorit
throw new IllegalArgumentException("Checksum Algorithm cannot be null!");
}

static Checksummer forNoOp() {
return new FlexibleChecksummer();
}

/**
* Given a payload, calculate a checksum and add it to the request.
*/
Expand Down
61 changes: 38 additions & 23 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 @@ -15,6 +15,7 @@

package software.amazon.awssdk.http.auth.aws.internal.signer;

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.sanitizeCredentials;
import static software.amazon.awssdk.http.auth.aws.internal.signer.util.OptionalDependencyLoaderUtil.getEventStreamV4PayloadSigner;
import static software.amazon.awssdk.http.auth.aws.internal.signer.util.SignerConstant.PRESIGN_URL_MAX_EXPIRATION_DURATION;
Expand All @@ -31,6 +32,7 @@
import java.util.concurrent.CompletableFuture;
import java.util.function.Function;
import software.amazon.awssdk.annotations.SdkInternalApi;
import software.amazon.awssdk.checksums.spi.ChecksumAlgorithm;
import software.amazon.awssdk.http.ContentStreamProvider;
import software.amazon.awssdk.http.Header;
import software.amazon.awssdk.http.SdkHttpRequest;
Expand Down Expand Up @@ -77,6 +79,11 @@ private static V4RequestSigner v4RequestSigner(

AuthLocation authLocation = request.requireProperty(AUTH_LOCATION, AuthLocation.HEADER);
Duration expirationDuration = request.property(EXPIRATION_DURATION);
boolean isAnonymous = CredentialUtils.isAnonymous(request.identity());

if (isAnonymous) {
return V4RequestSigner.anonymous(v4Properties);
}

Function<V4Properties, V4RequestSigner> requestSigner;
switch (authLocation) {
Expand All @@ -99,12 +106,25 @@ private static V4RequestSigner v4RequestSigner(
return requestSigner.apply(v4Properties);
}

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 Checksummer checksummer(BaseSignRequest<?, ? extends AwsCredentialsIdentity> request) {
boolean isPayloadSigning = isPayloadSigning(request);
boolean isEventStreaming = isEventStreaming(request.request());
boolean isChunkEncoding = request.requireProperty(CHUNK_ENCODING_ENABLED, false);
boolean hasChecksumHeader = hasChecksumHeader(request);
boolean isChunkEncoding = request.requireProperty(CHUNK_ENCODING_ENABLED, false) && !hasChecksumHeader;
boolean isTrailing = request.request().firstMatchingHeader(X_AMZ_TRAILER).isPresent();
boolean isFlexible = request.hasProperty(CHECKSUM_ALGORITHM);
boolean isAnonymous = CredentialUtils.isAnonymous(request.identity());

if (isEventStreaming) {
return Checksummer.forPrecomputed256Checksum(STREAMING_EVENTS_PAYLOAD);
Expand All @@ -118,23 +138,26 @@ private static Checksummer checksummer(BaseSignRequest<?, ? extends AwsCredentia
return Checksummer.forPrecomputed256Checksum(STREAMING_SIGNED_PAYLOAD);
}

if (request.hasProperty(CHECKSUM_ALGORITHM)) {
if (isFlexible) {
return Checksummer.forFlexibleChecksum(request.property(CHECKSUM_ALGORITHM));
}
return Checksummer.create();
}

if (isChunkEncoding) {
if (isFlexible || isTrailing) {
if (isFlexible || isTrailing) {
if (isChunkEncoding) {
return Checksummer.forPrecomputed256Checksum(STREAMING_UNSIGNED_PAYLOAD_TRAILER);
}
throw new UnsupportedOperationException("Chunk-Encoding without Payload-Signing must have a trailer!");
}

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

if (isAnonymous) {
return Checksummer.forNoOp();
}

return Checksummer.forPrecomputed256Checksum(UNSIGNED_PAYLOAD);
}

Expand All @@ -144,7 +167,9 @@ private static V4PayloadSigner v4PayloadSigner(

boolean isPayloadSigning = isPayloadSigning(request);
boolean isEventStreaming = isEventStreaming(request.request());
boolean isChunkEncoding = request.requireProperty(CHUNK_ENCODING_ENABLED, false);
boolean isChunkEncoding = request.requireProperty(CHUNK_ENCODING_ENABLED, false) && !hasChecksumHeader(request);
boolean isTrailing = request.request().firstMatchingHeader(X_AMZ_TRAILER).isPresent();
boolean isFlexible = request.hasProperty(CHECKSUM_ALGORITHM);

if (isEventStreaming) {
if (isPayloadSigning) {
Expand All @@ -157,7 +182,7 @@ private static V4PayloadSigner v4PayloadSigner(
throw new UnsupportedOperationException("Unsigned payload is not supported with event-streaming.");
}

if (isChunkEncoding) {
if ((isChunkEncoding && isPayloadSigning) || (isChunkEncoding && (isTrailing || isFlexible))) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggesting extracting it to a separate method and add javadoc to make it more clear


private boolean useChunkedEncoding(BaseSignRequest<?, ? extends AwsCredentialsIdentity> request)

return AwsChunkedV4PayloadSigner.builder()
.credentialScope(properties.getCredentialScope())
.chunkSize(DEFAULT_CHUNK_SIZE_IN_BYTES)
Expand Down Expand Up @@ -199,12 +224,6 @@ private static SignedRequest doSign(SignRequest<? extends AwsCredentialsIdentity
Checksummer checksummer,
V4RequestSigner requestSigner,
V4PayloadSigner payloadSigner) {
if (CredentialUtils.isAnonymous(request.identity())) {
return SignedRequest.builder()
.request(request.request())
.payload(request.payload().orElse(null))
.build();
}

SdkHttpRequest.Builder requestBuilder = request.request().toBuilder();

Expand All @@ -226,14 +245,6 @@ private static CompletableFuture<AsyncSignedRequest> doSign(AsyncSignRequest<? e
Checksummer checksummer,
V4RequestSigner requestSigner,
V4PayloadSigner payloadSigner) {
if (CredentialUtils.isAnonymous(request.identity())) {
return CompletableFuture.completedFuture(
AsyncSignedRequest.builder()
.request(request.request())
.payload(request.payload().orElse(null))
.build()
);
}

SdkHttpRequest.Builder requestBuilder = request.request().toBuilder();

Expand Down Expand Up @@ -261,7 +272,11 @@ private static Duration validateExpirationDuration(Duration expirationDuration)
}

private static boolean isPayloadSigning(BaseSignRequest<?, ? extends AwsCredentialsIdentity> request) {
return request.requireProperty(PAYLOAD_SIGNING_ENABLED, true) || !"https".equals(request.request().protocol());
boolean isAnonymous = CredentialUtils.isAnonymous(request.identity());
boolean isPayloadSigningEnabled = request.requireProperty(PAYLOAD_SIGNING_ENABLED, true);
boolean isEncrypted = "https".equals(request.request().protocol());

return !isAnonymous && (isPayloadSigningEnabled || !isEncrypted);
}

private static boolean isEventStreaming(SdkHttpRequest request) {
Expand Down
9 changes: 9 additions & 0 deletions ...s/src/main/java/software/amazon/awssdk/http/auth/aws/internal/signer/V4RequestSigner.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,15 @@ static V4RequestSigner presigned(V4Properties properties, Duration expirationDur
};
}

/**
* Retrieve an implementation of a V4RequestSigner to handle the anonymous credentials case, where the request is not
* sigend at all.
*/
static V4RequestSigner anonymous(V4Properties properties) {
return requestBuilder ->
new V4Context("", new byte[] {}, null, null, requestBuilder);
}

/**
* Given a request builder, sign a request and return a v4-context containing the signed request and its properties.
*/
Expand Down
24 changes: 22 additions & 2 deletions ...java/software/amazon/awssdk/http/auth/aws/internal/signer/DefaultAwsV4HttpSignerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,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 @@ -231,7 +231,27 @@ 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();
}

@Test
public void sign_WithPayloadSigningFalseAndChunkEncodingTrueWithChecksumHeader_DelegatesToUnsignedPayload() {
SignRequest<? extends AwsCredentialsIdentity> request = generateBasicRequest(
AwsCredentialsIdentity.create("access", "secret"),
httpRequest -> httpRequest
.putHeader(Header.CONTENT_LENGTH, "20")
.putHeader("x-amz-checksum-crc32", "bogus"),
signRequest -> signRequest
.putProperty(PAYLOAD_SIGNING_ENABLED, false)
.putProperty(CHUNK_ENCODING_ENABLED, true)
);

SignedRequest signedRequest = signer.sign(request);
assertThat(signedRequest.request().firstMatchingHeader("x-amz-content-sha256")).hasValue("UNSIGNED-PAYLOAD");
assertThat(signedRequest.request().firstMatchingHeader("x-amz-checksum-crc32")).hasValue("bogus");
assertThat(signedRequest.request().firstMatchingHeader("x-amz-decoded-content-length")).isNotPresent();
}

@Test
Expand Down
Loading