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.

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

Add flexible checksum support for chunked-encoding cases #4383

Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -52,23 +52,24 @@
import software.amazon.awssdk.http.auth.aws.signer.V4PayloadSigner;
import software.amazon.awssdk.utils.Pair;
import software.amazon.awssdk.utils.StringInputStream;
import software.amazon.awssdk.utils.Validate;

/**
* An implementation of a V4PayloadSigner which chunk-encodes a payload, optionally adding a chunk-signature chunk-extension,
* and/or trailers representing trailing headers with their signature at the end.
*/
@SdkInternalApi
public class AwsChunkedV4PayloadSigner implements V4PayloadSigner {
public final class AwsChunkedV4PayloadSigner implements V4PayloadSigner {

private static final String EMPTY_HASH = toHex(hash(""));

private final CredentialScope credentialScope;
private final int chunkSize;
private final ChecksumAlgorithm checksumAlgorithm;

public AwsChunkedV4PayloadSigner(CredentialScope credentialScope, int chunkSize, ChecksumAlgorithm checksumAlgorithm) {
this.credentialScope = credentialScope;
this.chunkSize = chunkSize;
private AwsChunkedV4PayloadSigner(CredentialScope credentialScope, int chunkSize, ChecksumAlgorithm checksumAlgorithm) {
this.credentialScope = Validate.paramNotNull(credentialScope, "CredentialScope");
this.chunkSize = Validate.isPositive(chunkSize, "ChunkSize");
this.checksumAlgorithm = checksumAlgorithm;
}

Expand Down Expand Up @@ -115,16 +116,12 @@ public ContentStreamProvider sign(ContentStreamProvider payload, V4Context v4Con
break;
}
case STREAMING_UNSIGNED_PAYLOAD_TRAILER:
if (checksumAlgorithm != null) {
setupChecksumTrailer(chunkedEncodedInputStreamBuilder, request);
}
setupChecksumTrailerIfNeeded(chunkedEncodedInputStreamBuilder, request);
break;
case STREAMING_SIGNED_PAYLOAD_TRAILER: {
RollingSigner rollingSigner = new RollingSigner(v4Context.getSigningKey(), v4Context.getSignature());
setupSigExt(chunkedEncodedInputStreamBuilder, rollingSigner);
if (checksumAlgorithm != null) {
setupChecksumTrailer(chunkedEncodedInputStreamBuilder, request);
}
setupChecksumTrailerIfNeeded(chunkedEncodedInputStreamBuilder, request);
setupSigTrailer(chunkedEncodedInputStreamBuilder, rollingSigner);
break;
}
Expand Down Expand Up @@ -187,13 +184,16 @@ private void setupSigTrailer(ChunkedEncodedInputStream.Builder builder, RollingS
)
);

String canonicalHeadersString = getCanonicalHeadersString(getCanonicalHeaders(headers));
String canonicalHashHex = toHex(hash(canonicalHeadersString));

// build the string-to-sign template for the rolling-signer to sign
return String.join("\n",
"AWS4-HMAC-SHA256-TRAILER",
credentialScope.getDatetime(),
credentialScope.scope(),
previousSignature,
toHex(hash(getCanonicalHeadersString(getCanonicalHeaders(headers))))
canonicalHashHex
);
}

Expand All @@ -212,9 +212,9 @@ private void setupSigTrailer(ChunkedEncodedInputStream.Builder builder, RollingS
* <p>
* The checksum-algorithm MUST be set if this is called, otherwise it will throw.
*/
private void setupChecksumTrailer(ChunkedEncodedInputStream.Builder builder, SdkHttpRequest.Builder request) {
private void setupChecksumTrailerIfNeeded(ChunkedEncodedInputStream.Builder builder, SdkHttpRequest.Builder request) {
if (checksumAlgorithm == null) {
throw new IllegalArgumentException("A checksum-algorithm must be configured in order to add a checksum trailer!");
return;
}
SdkChecksum sdkChecksum = fromChecksumAlgorithm(checksumAlgorithm);
ChecksumInputStream checksumInputStream = new ChecksumInputStream(
Expand Down Expand Up @@ -252,4 +252,49 @@ private void setupPreExistingTrailers(ChunkedEncodedInputStream.Builder builder,
request.removeHeader(header);
}
}

public static Builder builder() {
return new BuilderImpl();
}

public interface Builder {
Builder credentialScope(CredentialScope credentialScope);

Builder chunkSize(int chunkSize);
Copy link
Contributor

Choose a reason for hiding this comment

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

Should probably be boxed type so that we can throw an exception or set a default if the caller doesn't set it.


Builder checksumAlgorithm(ChecksumAlgorithm checksumAlgorithm);

AwsChunkedV4PayloadSigner build();
}

private static class BuilderImpl implements Builder {
private CredentialScope credentialScope;
private int chunkSize;
private ChecksumAlgorithm checksumAlgorithm;

@Override
public Builder credentialScope(CredentialScope credentialScope) {
this.credentialScope = credentialScope;
return this;
}

@Override
public Builder chunkSize(int chunkSize) {
this.chunkSize = chunkSize;
return this;
}

@Override
public Builder checksumAlgorithm(ChecksumAlgorithm checksumAlgorithm) {
this.checksumAlgorithm = checksumAlgorithm;
return this;
}

@Override
public AwsChunkedV4PayloadSigner build() {
return new AwsChunkedV4PayloadSigner(credentialScope, chunkSize, checksumAlgorithm);
}


}
}
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,11 @@ private static V4PayloadSigner v4PayloadSigner(
}

if (isChunkEncoding) {
return new AwsChunkedV4PayloadSigner(properties.getCredentialScope(), DEFAULT_CHUNK_SIZE_IN_BYTES, null);
return AwsChunkedV4PayloadSigner.builder()
.credentialScope(properties.getCredentialScope())
.chunkSize(DEFAULT_CHUNK_SIZE_IN_BYTES)
.checksumAlgorithm(request.property(CHECKSUM_ALGORITHM))
.build();
}

return V4PayloadSigner.create();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,10 @@ public void sign_withSignedPayload_shouldChunkEncodeWithSigV4Ext() throws IOExce
canonicalRequest,
requestBuilder
);
AwsChunkedV4PayloadSigner signer = new AwsChunkedV4PayloadSigner(credentialScope, chunkSize, null);
AwsChunkedV4PayloadSigner signer = AwsChunkedV4PayloadSigner.builder()
.credentialScope(credentialScope)
.chunkSize(chunkSize)
.build();

ContentStreamProvider signedPayload = signer.sign(payload, v4Context);

Expand Down Expand Up @@ -125,7 +128,11 @@ public void sign_withSignedPayloadAndChecksum_shouldChunkEncodeWithSigV4ExtAndCh
canonicalRequest,
requestBuilder
);
AwsChunkedV4PayloadSigner signer = new AwsChunkedV4PayloadSigner(credentialScope, chunkSize, CRC32);
AwsChunkedV4PayloadSigner signer = AwsChunkedV4PayloadSigner.builder()
.credentialScope(credentialScope)
.chunkSize(chunkSize)
.checksumAlgorithm(CRC32)
.build();

ContentStreamProvider signedPayload = signer.sign(payload, v4Context);

Expand Down Expand Up @@ -162,7 +169,11 @@ public void sign_withChecksum_shouldChunkEncodeWithChecksumTrailer() throws IOEx
canonicalRequest,
requestBuilder
);
AwsChunkedV4PayloadSigner signer = new AwsChunkedV4PayloadSigner(credentialScope, chunkSize, SHA256);
AwsChunkedV4PayloadSigner signer = AwsChunkedV4PayloadSigner.builder()
.credentialScope(credentialScope)
.chunkSize(chunkSize)
.checksumAlgorithm(SHA256)
.build();

ContentStreamProvider signedPayload = signer.sign(payload, v4Context);

Expand Down Expand Up @@ -207,7 +218,10 @@ public void sign_withPreExistingTrailers_shouldChunkEncodeWithExistingTrailers()
canonicalRequest,
requestBuilder
);
AwsChunkedV4PayloadSigner signer = new AwsChunkedV4PayloadSigner(credentialScope, chunkSize, null);
AwsChunkedV4PayloadSigner signer = AwsChunkedV4PayloadSigner.builder()
.credentialScope(credentialScope)
.chunkSize(chunkSize)
.build();

ContentStreamProvider signedPayload = signer.sign(payload, v4Context);

Expand Down Expand Up @@ -255,7 +269,11 @@ public void sign_withPreExistingTrailersAndChecksum_shouldChunkEncodeWithTrailer
canonicalRequest,
requestBuilder
);
AwsChunkedV4PayloadSigner signer = new AwsChunkedV4PayloadSigner(credentialScope, chunkSize, CRC32);
AwsChunkedV4PayloadSigner signer = AwsChunkedV4PayloadSigner.builder()
.credentialScope(credentialScope)
.chunkSize(chunkSize)
.checksumAlgorithm(CRC32)
.build();

ContentStreamProvider signedPayload = signer.sign(payload, v4Context);

Expand Down Expand Up @@ -303,7 +321,11 @@ public void sign_withPreExistingTrailersAndChecksumAndSignedPayload_shouldAwsChu
canonicalRequest,
requestBuilder
);
AwsChunkedV4PayloadSigner signer = new AwsChunkedV4PayloadSigner(credentialScope, chunkSize, CRC32);
AwsChunkedV4PayloadSigner signer = AwsChunkedV4PayloadSigner.builder()
.credentialScope(credentialScope)
.chunkSize(chunkSize)
.checksumAlgorithm(CRC32)
.build();

ContentStreamProvider signedPayload = signer.sign(payload, v4Context);

Expand Down Expand Up @@ -333,14 +355,20 @@ public void sign_withoutContentLength_throws() {
requestBuilder
);
v4Context.getSignedRequest().removeHeader(Header.CONTENT_LENGTH);
AwsChunkedV4PayloadSigner signer = new AwsChunkedV4PayloadSigner(credentialScope, chunkSize, null);
AwsChunkedV4PayloadSigner signer = AwsChunkedV4PayloadSigner.builder()
.credentialScope(credentialScope)
.chunkSize(chunkSize)
.build();

assertThrows(IllegalArgumentException.class, () -> signer.sign(payload, v4Context));
}

@Test
public void signAsync_throws() {
AwsChunkedV4PayloadSigner signer = new AwsChunkedV4PayloadSigner(credentialScope, chunkSize, null);
AwsChunkedV4PayloadSigner signer = AwsChunkedV4PayloadSigner.builder()
.credentialScope(credentialScope)
.chunkSize(chunkSize)
.build();

assertThrows(UnsupportedOperationException.class, () -> signer.signAsync(null, null));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -164,25 +164,7 @@ public void sign_WithChunkEncodingTrue_DelegatesToAwsChunkedPayloadSigner() {
}

@Test
public void sign_WithChunkEncodingTrueWithout_DelegatesToAwsChunkedPayloadSigner() {
SyncSignRequest<? extends AwsCredentialsIdentity> request = generateBasicRequest(
AwsCredentialsIdentity.create("access", "secret"),
httpRequest -> httpRequest
.putHeader(Header.CONTENT_LENGTH, "20"),
signRequest -> signRequest
.putProperty(CHUNK_ENCODING_ENABLED, true)
);

SyncSignedRequest signedRequest = signer.sign(request);

assertThat(signedRequest.request().firstMatchingHeader("x-amz-content-sha256"))
.hasValue("STREAMING-AWS4-HMAC-SHA256-PAYLOAD");
assertThat(signedRequest.request().firstMatchingHeader(Header.CONTENT_LENGTH)).isNotPresent();
assertThat(signedRequest.request().firstMatchingHeader("x-amz-decoded-content-length")).hasValue("20");
}

@Test
public void sign_ChunkEncodingTrueAndChecksumAlgorithm_DelegatesToAwsChunkedPayloadSigner() {
public void sign_WithChunkEncodingTrueAndChecksumAlgorithm_DelegatesToAwsChunkedPayloadSigner() {
SyncSignRequest<? extends AwsCredentialsIdentity> request = generateBasicRequest(
AwsCredentialsIdentity.create("access", "secret"),
httpRequest -> httpRequest
Expand All @@ -198,6 +180,7 @@ public void sign_ChunkEncodingTrueAndChecksumAlgorithm_DelegatesToAwsChunkedPayl
.hasValue("STREAMING-AWS4-HMAC-SHA256-PAYLOAD-TRAILER");
assertThat(signedRequest.request().firstMatchingHeader(Header.CONTENT_LENGTH)).isNotPresent();
assertThat(signedRequest.request().firstMatchingHeader("x-amz-decoded-content-length")).hasValue("20");
assertThat(signedRequest.request().firstMatchingHeader("x-amz-trailer")).hasValue("x-amz-checksum-crc32");
}

@Test
Expand All @@ -218,6 +201,7 @@ public void sign_WithPayloadSigningFalseAndChunkEncodingTrueAndTrailer_Delegates
.hasValue("STREAMING-UNSIGNED-PAYLOAD-TRAILER");
assertThat(signedRequest.request().firstMatchingHeader(Header.CONTENT_LENGTH)).isNotPresent();
assertThat(signedRequest.request().firstMatchingHeader("x-amz-decoded-content-length")).hasValue("20");
assertThat(signedRequest.request().firstMatchingHeader("x-amz-trailer")).hasValue("x-amz-checksum-crc32");
}

@Test
Expand Down
Loading