Skip to content

Commit 1083bbb

Browse files
haydenbakerhaydenbaker
committed
Fix header updates in payload signer, http fallback to signed
1 parent 9873b81 commit 1083bbb

File tree

6 files changed

+82
-34
lines changed

6 files changed

+82
-34
lines changed

‎core/http-auth-aws/src/main/java/software/amazon/awssdk/http/auth/aws/internal/signer/AwsChunkedV4PayloadSigner.java

Lines changed: 38 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626

2727
import java.nio.ByteBuffer;
2828
import java.nio.charset.StandardCharsets;
29+
import java.util.ArrayList;
2930
import java.util.Collections;
3031
import java.util.List;
3132
import org.reactivestreams.Publisher;
@@ -54,6 +55,7 @@ public final class AwsChunkedV4PayloadSigner implements V4PayloadSigner {
5455
private final CredentialScope credentialScope;
5556
private final int chunkSize;
5657
private final ChecksumAlgorithm checksumAlgorithm;
58+
private final List<Pair<String, List<String>>> preExistingTrailers = new ArrayList<>();
5759

5860
private AwsChunkedV4PayloadSigner(Builder builder) {
5961
this.credentialScope = Validate.paramNotNull(builder.credentialScope, "CredentialScope");
@@ -68,7 +70,6 @@ public static Builder builder() {
6870
@Override
6971
public ContentStreamProvider sign(ContentStreamProvider payload, V4Context v4Context) {
7072
SdkHttpRequest.Builder request = v4Context.getSignedRequest();
71-
moveContentLength(request);
7273

7374
String checksum = request.firstMatchingHeader(X_AMZ_CONTENT_SHA256).orElseThrow(
7475
() -> new IllegalArgumentException(X_AMZ_CONTENT_SHA256 + " must be set!")
@@ -79,7 +80,8 @@ public ContentStreamProvider sign(ContentStreamProvider payload, V4Context v4Con
7980
.inputStream(payload.newStream())
8081
.chunkSize(chunkSize)
8182
.header(chunk -> Integer.toHexString(chunk.length).getBytes(StandardCharsets.UTF_8));
82-
setupPreExistingTrailers(chunkedEncodedInputStreamBuilder, request);
83+
84+
preExistingTrailers.forEach(trailer -> chunkedEncodedInputStreamBuilder.addTrailer(() -> trailer));
8385

8486
switch (checksum) {
8587
case STREAMING_SIGNED_PAYLOAD: {
@@ -88,12 +90,12 @@ public ContentStreamProvider sign(ContentStreamProvider payload, V4Context v4Con
8890
break;
8991
}
9092
case STREAMING_UNSIGNED_PAYLOAD_TRAILER:
91-
setupChecksumTrailerIfNeeded(chunkedEncodedInputStreamBuilder, request);
93+
setupChecksumTrailerIfNeeded(chunkedEncodedInputStreamBuilder);
9294
break;
9395
case STREAMING_SIGNED_PAYLOAD_TRAILER: {
9496
RollingSigner rollingSigner = new RollingSigner(v4Context.getSigningKey(), v4Context.getSignature());
9597
chunkedEncodedInputStreamBuilder.addExtension(new SigV4ChunkExtensionProvider(rollingSigner, credentialScope));
96-
setupChecksumTrailerIfNeeded(chunkedEncodedInputStreamBuilder, request);
98+
setupChecksumTrailerIfNeeded(chunkedEncodedInputStreamBuilder);
9799
chunkedEncodedInputStreamBuilder.addTrailer(
98100
new SigV4TrailerProvider(chunkedEncodedInputStreamBuilder.trailers(), rollingSigner, credentialScope)
99101
);
@@ -111,49 +113,55 @@ public Publisher<ByteBuffer> signAsync(Publisher<ByteBuffer> payload, V4Context
111113
throw new UnsupportedOperationException();
112114
}
113115

116+
@Override
117+
public void beforeSigning(SdkHttpRequest.Builder request) {
118+
moveContentLength(request);
119+
setupPreExistingTrailers(request);
120+
121+
if (checksumAlgorithm != null) {
122+
String checksumHeaderName = checksumHeaderName(checksumAlgorithm);
123+
request.appendHeader(X_AMZ_TRAILER, checksumHeaderName);
124+
}
125+
}
126+
127+
/**
128+
* Set up a map of pre-existing trailer (headers) for the given request to be used when chunk-encoding the payload.
129+
* <p>
130+
* However, we need to validate that these are valid trailers. Since aws-chunked encoding adds the checksum as a trailer, it
131+
* isn't part of the request headers, but other trailers MUST be present in the request-headers.
132+
*/
133+
private void setupPreExistingTrailers(SdkHttpRequest.Builder request) {
134+
for (String header : request.matchingHeaders(X_AMZ_TRAILER)) {
135+
List<String> values = request.matchingHeaders(header);
136+
if (values.isEmpty()) {
137+
throw new IllegalArgumentException(header + " must be present in the request headers to be a valid trailer.");
138+
}
139+
preExistingTrailers.add(Pair.of(header, values));
140+
request.removeHeader(header);
141+
}
142+
}
143+
114144
/**
115-
* Add the checksum as a chunk-trailer and add it to the request's trailer header.
145+
* Add the checksum as a trailer to the chunk-encoded stream.
116146
* <p>
117-
* The checksum-algorithm MUST be set if this is called, otherwise it will throw.
147+
* If the checksum-algorithm is not present, then nothing is done.
118148
*/
119-
private void setupChecksumTrailerIfNeeded(ChunkedEncodedInputStream.Builder builder, SdkHttpRequest.Builder request) {
149+
private void setupChecksumTrailerIfNeeded(ChunkedEncodedInputStream.Builder builder) {
120150
if (checksumAlgorithm == null) {
121151
return;
122152
}
153+
String checksumHeaderName = checksumHeaderName(checksumAlgorithm);
123154
SdkChecksum sdkChecksum = fromChecksumAlgorithm(checksumAlgorithm);
124155
ChecksumInputStream checksumInputStream = new ChecksumInputStream(
125156
builder.inputStream(),
126157
Collections.singleton(sdkChecksum)
127158
);
128-
String checksumHeaderName = checksumHeaderName(checksumAlgorithm);
129159

130160
TrailerProvider checksumTrailer = new ChecksumTrailerProvider(sdkChecksum, checksumHeaderName);
131161

132-
request.appendHeader(X_AMZ_TRAILER, checksumHeaderName);
133162
builder.inputStream(checksumInputStream).addTrailer(checksumTrailer);
134163
}
135164

136-
/**
137-
* Create chunk-trailers for each pre-existing trailer given in the request.
138-
* <p>
139-
* However, we need to validate that these are valid trailers. Since aws-chunked encoding adds the checksum as a trailer, it
140-
* isn't part of the request headers, but other trailers MUST be present in the request-headers.
141-
*/
142-
private void setupPreExistingTrailers(ChunkedEncodedInputStream.Builder builder, SdkHttpRequest.Builder request) {
143-
List<String> trailerHeaders = request.matchingHeaders(X_AMZ_TRAILER);
144-
145-
for (String header : trailerHeaders) {
146-
List<String> values = request.matchingHeaders(header);
147-
if (values.isEmpty()) {
148-
throw new IllegalArgumentException(header + " must be present in the request headers to be a valid trailer.");
149-
}
150-
151-
// Add the trailer to the aws-chunked stream-builder, and remove it from the request headers
152-
builder.addTrailer(() -> Pair.of(header, values));
153-
request.removeHeader(header);
154-
}
155-
}
156-
157165
static class Builder {
158166
private CredentialScope credentialScope;
159167
private Integer chunkSize;

‎core/http-auth-aws/src/main/java/software/amazon/awssdk/http/auth/aws/internal/signer/DefaultAwsV4HttpSigner.java

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ private static V4RequestSigner v4RequestSigner(
100100
}
101101

102102
private static Checksummer checksummer(BaseSignRequest<?, ? extends AwsCredentialsIdentity> request) {
103-
boolean isPayloadSigning = request.requireProperty(PAYLOAD_SIGNING_ENABLED, true);
103+
boolean isPayloadSigning = isPayloadSigning(request);
104104
boolean isEventStreaming = isEventStreaming(request.request());
105105
boolean isChunkEncoding = request.requireProperty(CHUNK_ENCODING_ENABLED, false);
106106
boolean isTrailing = request.request().firstMatchingHeader(X_AMZ_TRAILER).isPresent();
@@ -142,7 +142,7 @@ private static V4PayloadSigner v4PayloadSigner(
142142
BaseSignRequest<?, ? extends AwsCredentialsIdentity> request,
143143
V4Properties properties) {
144144

145-
boolean isPayloadSigning = request.requireProperty(PAYLOAD_SIGNING_ENABLED, true);
145+
boolean isPayloadSigning = isPayloadSigning(request);
146146
boolean isEventStreaming = isEventStreaming(request.request());
147147
boolean isChunkEncoding = request.requireProperty(CHUNK_ENCODING_ENABLED, false);
148148

@@ -183,6 +183,8 @@ private static SignedRequest doSign(SignRequest<? extends AwsCredentialsIdentity
183183

184184
checksummer.checksum(request.payload().orElse(null), requestBuilder);
185185

186+
payloadSigner.beforeSigning(requestBuilder);
187+
186188
V4Context v4Context = requestSigner.sign(requestBuilder);
187189

188190
ContentStreamProvider payload = payloadSigner.sign(request.payload().orElse(null), v4Context);
@@ -231,6 +233,10 @@ private static Duration validateExpirationDuration(Duration expirationDuration)
231233
return expirationDuration;
232234
}
233235

236+
private static boolean isPayloadSigning(BaseSignRequest<?, ? extends AwsCredentialsIdentity> request) {
237+
return request.requireProperty(PAYLOAD_SIGNING_ENABLED, true) || !"https".equals(request.request().protocol());
238+
}
239+
234240
private static boolean isEventStreaming(SdkHttpRequest request) {
235241
return "application/vnd.amazon.eventstream".equals(request.firstMatchingHeader(Header.CONTENT_TYPE).orElse(""));
236242
}

‎core/http-auth-aws/src/main/java/software/amazon/awssdk/http/auth/aws/internal/signer/V4PayloadSigner.java

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import org.reactivestreams.Publisher;
2020
import software.amazon.awssdk.annotations.SdkInternalApi;
2121
import software.amazon.awssdk.http.ContentStreamProvider;
22+
import software.amazon.awssdk.http.SdkHttpRequest;
2223

2324
/**
2425
* An interface for defining how to sign a payload via SigV4.
@@ -41,4 +42,10 @@ static V4PayloadSigner create() {
4142
* Given a payload and v4-context, sign the payload via the SigV4 process.
4243
*/
4344
Publisher<ByteBuffer> signAsync(Publisher<ByteBuffer> payload, V4Context v4Context);
45+
46+
/**
47+
* Modify a request before it is signed, such as changing headers or query-parameters.
48+
*/
49+
default void beforeSigning(SdkHttpRequest.Builder request) {
50+
}
4451
}

‎core/http-auth-aws/src/test/java/software/amazon/awssdk/http/auth/aws/TestUtils.java

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,10 @@
2121
import software.amazon.awssdk.utils.async.SimplePublisher;
2222

2323
public final class TestUtils {
24+
25+
private TestUtils() {
26+
}
27+
2428
// Helpers for generating test requests
2529
public static <T extends AwsCredentialsIdentity> SignRequest<T> generateBasicRequest(
2630
T credentials,
@@ -33,7 +37,7 @@ public static <T extends AwsCredentialsIdentity> SignRequest<T> generateBasicReq
3337
.putHeader("Host", "demo.us-east-1.amazonaws.com")
3438
.putHeader("x-amz-archive-description", "test test")
3539
.encodedPath("/")
36-
.uri(URI.create("http://demo.us-east-1.amazonaws.com"))
40+
.uri(URI.create("https://demo.us-east-1.amazonaws.com"))
3741
.build()
3842
.copy(requestOverrides))
3943
.payload(() -> new ByteArrayInputStream("{\"TableName\": \"foo\"}".getBytes()))
@@ -57,11 +61,12 @@ public static <T extends AwsCredentialsIdentity> AsyncSignRequest<T> generateBas
5761

5862
return AsyncSignRequest.builder(credentials)
5963
.request(SdkHttpRequest.builder()
64+
.protocol("https")
6065
.method(SdkHttpMethod.POST)
6166
.putHeader("Host", "demo.us-east-1.amazonaws.com")
6267
.putHeader("x-amz-archive-description", "test test")
6368
.encodedPath("/")
64-
.uri(URI.create("http://demo.us-east-1.amazonaws.com"))
69+
.uri(URI.create("https://demo.us-east-1.amazonaws.com"))
6570
.build()
6671
.copy(requestOverrides))
6772
.payload(publisher)

‎core/http-auth-aws/src/test/java/software/amazon/awssdk/http/auth/aws/internal/signer/AwsChunkedV4PayloadSignerTest.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,7 @@ public void sign_withSignedPayload_shouldChunkEncodeWithSigV4Ext() throws IOExce
8989
.chunkSize(chunkSize)
9090
.build();
9191

92+
signer.beforeSigning(requestBuilder);
9293
ContentStreamProvider signedPayload = signer.sign(payload, v4Context);
9394

9495
assertThat(requestBuilder.firstMatchingHeader(Header.CONTENT_LENGTH)).isNotPresent();
@@ -131,6 +132,7 @@ public void sign_withSignedPayloadAndChecksum_shouldChunkEncodeWithSigV4ExtAndSi
131132
.checksumAlgorithm(CRC32)
132133
.build();
133134

135+
signer.beforeSigning(requestBuilder);
134136
ContentStreamProvider signedPayload = signer.sign(payload, v4Context);
135137

136138
assertThat(requestBuilder.firstMatchingHeader(Header.CONTENT_LENGTH)).isNotPresent();
@@ -173,6 +175,7 @@ public void sign_withChecksum_shouldChunkEncodeWithChecksumTrailer() throws IOEx
173175
.checksumAlgorithm(SHA256)
174176
.build();
175177

178+
signer.beforeSigning(requestBuilder);
176179
ContentStreamProvider signedPayload = signer.sign(payload, v4Context);
177180

178181
assertThat(requestBuilder.firstMatchingHeader(Header.CONTENT_LENGTH)).isNotPresent();
@@ -222,6 +225,7 @@ public void sign_withPreExistingTrailers_shouldChunkEncodeWithExistingTrailers()
222225
.chunkSize(chunkSize)
223226
.build();
224227

228+
signer.beforeSigning(requestBuilder);
225229
ContentStreamProvider signedPayload = signer.sign(payload, v4Context);
226230

227231
assertThat(requestBuilder.firstMatchingHeader(Header.CONTENT_LENGTH)).isNotPresent();
@@ -275,6 +279,7 @@ public void sign_withPreExistingTrailersAndChecksum_shouldChunkEncodeWithTrailer
275279
.checksumAlgorithm(CRC32)
276280
.build();
277281

282+
signer.beforeSigning(requestBuilder);
278283
ContentStreamProvider signedPayload = signer.sign(payload, v4Context);
279284

280285
assertThat(requestBuilder.firstMatchingHeader(Header.CONTENT_LENGTH)).isNotPresent();
@@ -330,6 +335,7 @@ public void sign_withPreExistingTrailersAndChecksumAndSignedPayload_shouldAwsChu
330335
.checksumAlgorithm(CRC32)
331336
.build();
332337

338+
signer.beforeSigning(requestBuilder);
333339
ContentStreamProvider signedPayload = signer.sign(payload, v4Context);
334340

335341
assertThat(requestBuilder.firstMatchingHeader(Header.CONTENT_LENGTH)).isNotPresent();

‎core/http-auth-aws/src/test/java/software/amazon/awssdk/http/auth/aws/internal/signer/DefaultAwsV4HttpSignerTest.java

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
import static software.amazon.awssdk.http.auth.aws.signer.AwsV4HttpSigner.EXPIRATION_DURATION;
2727
import static software.amazon.awssdk.http.auth.aws.signer.AwsV4HttpSigner.PAYLOAD_SIGNING_ENABLED;
2828

29+
import java.net.URI;
2930
import java.time.Duration;
3031
import org.junit.jupiter.api.Test;
3132
import org.mockito.MockedStatic;
@@ -291,4 +292,19 @@ public void sign_WithEventStreamContentTypeAndUnsignedPayload_Throws() {
291292

292293
assertThrows(UnsupportedOperationException.class, () -> signer.signAsync(request));
293294
}
295+
296+
@Test
297+
public void sign_WithPayloadSigningFalseAndHttp_FallsBackToPayloadSigning() {
298+
SignRequest<? extends AwsCredentialsIdentity> request = generateBasicRequest(
299+
AwsCredentialsIdentity.create("access", "secret"),
300+
httpRequest -> httpRequest.uri(URI.create("http://demo.us-east-1.amazonaws.com")),
301+
signRequest -> signRequest
302+
.putProperty(PAYLOAD_SIGNING_ENABLED, false)
303+
);
304+
305+
SignedRequest signedRequest = signer.sign(request);
306+
307+
assertThat(signedRequest.request().firstMatchingHeader("x-amz-content-sha256"))
308+
.hasValue("a15c8292b1d12abbbbe4148605f7872fbdf645618fee5ab0e8072a7b34f155e2");
309+
}
294310
}

0 commit comments

Comments
 (0)