Skip to content

Commit 65c3aa4

Browse files
authored
Fixed a bug where path normalization was not being disabled when signing requests directly with AwsS3V4Signer. (#3601)
The issue was introduced with #3534. The PR did not account for users who are using the signer directly.
1 parent 328db17 commit 65c3aa4

File tree

8 files changed

+290
-5
lines changed

8 files changed

+290
-5
lines changed
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
{
2+
"category": "Amazon S3",
3+
"contributor": "",
4+
"type": "bugfix",
5+
"description": "Fixed an issue that could result in exceptions or signature validation failures when signing or presigning S3 requests using the signer directly when paths contain encoded or path traversal characters."
6+
}

core/auth/src/main/java/software/amazon/awssdk/auth/signer/AwsSignerExecutionAttribute.java

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,11 @@
1515

1616
package software.amazon.awssdk.auth.signer;
1717

18+
import java.time.Clock;
1819
import java.time.Instant;
1920
import software.amazon.awssdk.annotations.SdkProtectedApi;
2021
import software.amazon.awssdk.auth.credentials.AwsCredentials;
22+
import software.amazon.awssdk.auth.signer.params.Aws4SignerParams;
2123
import software.amazon.awssdk.core.interceptor.ExecutionAttribute;
2224
import software.amazon.awssdk.core.interceptor.ExecutionInterceptor;
2325
import software.amazon.awssdk.core.interceptor.SdkExecutionAttribute;
@@ -64,6 +66,13 @@ public final class AwsSignerExecutionAttribute extends SdkExecutionAttribute {
6466
public static final ExecutionAttribute<Boolean> SIGNER_NORMALIZE_PATH =
6567
new ExecutionAttribute<>("NormalizePath");
6668

69+
70+
/**
71+
* An override clock to use during signing.
72+
* @see Aws4SignerParams.Builder#signingClockOverride(Clock)
73+
*/
74+
public static final ExecutionAttribute<Clock> SIGNING_CLOCK = new ExecutionAttribute<>("Clock");
75+
6776
/**
6877
* The key to specify the expiration time when pre-signing aws requests.
6978
*/

core/auth/src/main/java/software/amazon/awssdk/auth/signer/internal/AbstractAws4Signer.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -414,7 +414,8 @@ protected <B extends Aws4SignerParams.Builder> B extractSignerParams(B paramsBui
414414
paramsBuilder.awsCredentials(executionAttributes.getAttribute(AwsSignerExecutionAttribute.AWS_CREDENTIALS))
415415
.signingName(executionAttributes.getAttribute(AwsSignerExecutionAttribute.SERVICE_SIGNING_NAME))
416416
.signingRegion(executionAttributes.getAttribute(AwsSignerExecutionAttribute.SIGNING_REGION))
417-
.timeOffset(executionAttributes.getAttribute(AwsSignerExecutionAttribute.TIME_OFFSET));
417+
.timeOffset(executionAttributes.getAttribute(AwsSignerExecutionAttribute.TIME_OFFSET))
418+
.signingClockOverride(executionAttributes.getAttribute(AwsSignerExecutionAttribute.SIGNING_CLOCK));
418419

419420
Boolean doubleUrlEncode = executionAttributes.getAttribute(AwsSignerExecutionAttribute.SIGNER_DOUBLE_URL_ENCODE);
420421
if (doubleUrlEncode != null) {

core/auth/src/main/java/software/amazon/awssdk/auth/signer/internal/AbstractAwsS3V4Signer.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,8 @@ public SdkHttpFullRequest presign(SdkHttpFullRequest request, Aws4PresignerParam
115115
return request;
116116
}
117117

118+
signingParams = signingParams.copy(b -> b.normalizePath(false));
119+
118120
Aws4SignerRequestParams requestParams = new Aws4SignerRequestParams(signingParams);
119121

120122
return doPresign(request, requestParams, signingParams).build();

core/auth/src/main/java/software/amazon/awssdk/auth/signer/params/Aws4PresignerParams.java

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,13 @@
1919
import java.util.Optional;
2020
import software.amazon.awssdk.annotations.SdkPublicApi;
2121
import software.amazon.awssdk.auth.signer.internal.SignerConstant;
22+
import software.amazon.awssdk.utils.builder.CopyableBuilder;
23+
import software.amazon.awssdk.utils.builder.ToCopyableBuilder;
2224

2325
@SdkPublicApi
24-
public final class Aws4PresignerParams extends Aws4SignerParams {
26+
public final class Aws4PresignerParams
27+
extends Aws4SignerParams
28+
implements ToCopyableBuilder<Aws4PresignerParams.Builder, Aws4PresignerParams> {
2529

2630
private final Instant expirationTime;
2731

@@ -38,7 +42,12 @@ public static Builder builder() {
3842
return new BuilderImpl();
3943
}
4044

41-
public interface Builder extends Aws4SignerParams.Builder<Builder> {
45+
@Override
46+
public Builder toBuilder() {
47+
return new BuilderImpl(this);
48+
}
49+
50+
public interface Builder extends Aws4SignerParams.Builder<Builder>, CopyableBuilder<Builder, Aws4PresignerParams> {
4251

4352
/**
4453
* Sets an expiration time for the presigned url. If this value is not specified,
@@ -58,6 +67,11 @@ private static final class BuilderImpl extends Aws4SignerParams.BuilderImpl<Buil
5867
private BuilderImpl() {
5968
}
6069

70+
private BuilderImpl(Aws4PresignerParams params) {
71+
super(params);
72+
this.expirationTime = params.expirationTime;
73+
}
74+
6175
@Override
6276
public Builder expirationTime(Instant expirationTime) {
6377
this.expirationTime = expirationTime;

core/auth/src/main/java/software/amazon/awssdk/auth/signer/params/Aws4SignerParams.java

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ public SignerChecksumParams checksumParams() {
8686
return checksumParams;
8787
}
8888

89-
public interface Builder<B extends Builder> {
89+
public interface Builder<B extends Builder<B>> {
9090

9191
/**
9292
* Set this value to double url-encode the resource path when constructing the
@@ -155,7 +155,7 @@ public interface Builder<B extends Builder> {
155155
Aws4SignerParams build();
156156
}
157157

158-
protected static class BuilderImpl<B extends Builder> implements Builder<B> {
158+
protected static class BuilderImpl<B extends Builder<B>> implements Builder<B> {
159159
private static final Boolean DEFAULT_DOUBLE_URL_ENCODE = Boolean.TRUE;
160160

161161
private Boolean doubleUrlEncode = DEFAULT_DOUBLE_URL_ENCODE;
@@ -168,7 +168,17 @@ protected static class BuilderImpl<B extends Builder> implements Builder<B> {
168168
private SignerChecksumParams checksumParams;
169169

170170
protected BuilderImpl() {
171+
}
171172

173+
protected BuilderImpl(Aws4SignerParams params) {
174+
doubleUrlEncode = params.doubleUrlEncode;
175+
normalizePath = params.normalizePath;
176+
awsCredentials = params.awsCredentials;
177+
signingName = params.signingName;
178+
signingRegion = params.signingRegion;
179+
timeOffset = params.timeOffset;
180+
signingClockOverride = params.signingClockOverride;
181+
checksumParams = params.checksumParams;
172182
}
173183

174184
@Override

core/auth/src/main/java/software/amazon/awssdk/auth/signer/params/AwsS3V4SignerParams.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,8 @@ private static final class BuilderImpl extends Aws4SignerParams.BuilderImpl<Buil
9494
private Boolean enablePayloadSigning = DEFAULT_PAYLOAD_SIGNING_ENABLED;
9595

9696
private BuilderImpl() {
97+
// By default, S3 should not normalize paths
98+
normalizePath(false);
9799
}
98100

99101
@Override
Lines changed: 241 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,241 @@
1+
/*
2+
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License").
5+
* You may not use this file except in compliance with the License.
6+
* A copy of the License is located at
7+
*
8+
* http://aws.amazon.com/apache2.0
9+
*
10+
* or in the "license" file accompanying this file. This file is distributed
11+
* on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
12+
* express or implied. See the License for the specific language governing
13+
* permissions and limitations under the License.
14+
*/
15+
16+
package software.amazon.awssdk.auth.signer;
17+
18+
import static org.assertj.core.api.Assertions.assertThat;
19+
import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
20+
21+
import java.net.URI;
22+
import java.nio.ByteBuffer;
23+
import java.time.Clock;
24+
import java.time.Instant;
25+
import java.time.ZoneOffset;
26+
import java.util.concurrent.ThreadLocalRandom;
27+
import org.junit.jupiter.api.Test;
28+
import software.amazon.awssdk.auth.credentials.AwsBasicCredentials;
29+
import software.amazon.awssdk.auth.signer.params.Aws4PresignerParams;
30+
import software.amazon.awssdk.auth.signer.params.AwsS3V4SignerParams;
31+
import software.amazon.awssdk.core.interceptor.ExecutionAttributes;
32+
import software.amazon.awssdk.core.sync.RequestBody;
33+
import software.amazon.awssdk.http.SdkHttpFullRequest;
34+
import software.amazon.awssdk.http.SdkHttpMethod;
35+
import software.amazon.awssdk.regions.Region;
36+
37+
class AwsS3V4SignerTest {
38+
private static final Clock UTC_EPOCH_CLOCK = Clock.fixed(Instant.EPOCH, ZoneOffset.UTC);
39+
40+
@Test
41+
public void signWithParams_urlsAreNotNormalized() {
42+
byte[] bytes = new byte[1000];
43+
ThreadLocalRandom.current().nextBytes(bytes);
44+
ByteBuffer buffer = ByteBuffer.wrap(bytes);
45+
URI target = URI.create("https://test.com/./foo");
46+
47+
SdkHttpFullRequest request = SdkHttpFullRequest.builder()
48+
.contentStreamProvider(RequestBody.fromByteBuffer(buffer)
49+
.contentStreamProvider())
50+
.method(SdkHttpMethod.GET)
51+
.uri(target)
52+
.encodedPath(target.getPath())
53+
.build();
54+
AwsS3V4Signer signer = AwsS3V4Signer.create();
55+
SdkHttpFullRequest signedRequest =
56+
signer.sign(request,
57+
AwsS3V4SignerParams.builder()
58+
.awsCredentials(AwsBasicCredentials.create("akid", "skid"))
59+
.signingRegion(Region.US_WEST_2)
60+
.signingName("s3")
61+
.signingClockOverride(UTC_EPOCH_CLOCK)
62+
.build());
63+
64+
assertThat(signedRequest.firstMatchingHeader("Authorization"))
65+
.hasValue("AWS4-HMAC-SHA256 Credential=akid/19700101/us-west-2/s3/aws4_request, "
66+
+ "SignedHeaders=host;x-amz-content-sha256;x-amz-date, "
67+
+ "Signature=a3b97f9de337ab254f3b366c3d0b3c67016d2d8d8ba7e0e4ddab0ccebe84992a");
68+
}
69+
70+
@Test
71+
public void signWithExecutionAttributes_urlsAreNotNormalized() {
72+
byte[] bytes = new byte[1000];
73+
ThreadLocalRandom.current().nextBytes(bytes);
74+
ByteBuffer buffer = ByteBuffer.wrap(bytes);
75+
URI target = URI.create("https://test.com/./foo");
76+
77+
SdkHttpFullRequest request = SdkHttpFullRequest.builder()
78+
.contentStreamProvider(RequestBody.fromByteBuffer(buffer)
79+
.contentStreamProvider())
80+
.method(SdkHttpMethod.GET)
81+
.uri(target)
82+
.encodedPath(target.getPath())
83+
.build();
84+
ExecutionAttributes attributes =
85+
ExecutionAttributes.builder()
86+
.put(AwsSignerExecutionAttribute.AWS_CREDENTIALS,
87+
AwsBasicCredentials.create("akid", "skid"))
88+
.put(AwsSignerExecutionAttribute.SIGNING_REGION, Region.US_WEST_2)
89+
.put(AwsSignerExecutionAttribute.SERVICE_SIGNING_NAME, "s3")
90+
.put(AwsSignerExecutionAttribute.SIGNING_CLOCK, UTC_EPOCH_CLOCK)
91+
.build();
92+
93+
AwsS3V4Signer signer = AwsS3V4Signer.create();
94+
SdkHttpFullRequest signedRequest = signer.sign(request, attributes);
95+
96+
assertThat(signedRequest.firstMatchingHeader("Authorization"))
97+
.hasValue("AWS4-HMAC-SHA256 Credential=akid/19700101/us-west-2/s3/aws4_request, "
98+
+ "SignedHeaders=host;x-amz-content-sha256;x-amz-date, "
99+
+ "Signature=a3b97f9de337ab254f3b366c3d0b3c67016d2d8d8ba7e0e4ddab0ccebe84992a");
100+
}
101+
102+
@Test
103+
public void presignWithParams_urlsAreNotNormalized() {
104+
byte[] bytes = new byte[1000];
105+
ThreadLocalRandom.current().nextBytes(bytes);
106+
ByteBuffer buffer = ByteBuffer.wrap(bytes);
107+
URI target = URI.create("https://test.com/./foo");
108+
109+
SdkHttpFullRequest request = SdkHttpFullRequest.builder()
110+
.contentStreamProvider(RequestBody.fromByteBuffer(buffer)
111+
.contentStreamProvider())
112+
.method(SdkHttpMethod.GET)
113+
.uri(target)
114+
.encodedPath(target.getPath())
115+
.build();
116+
AwsS3V4Signer signer = AwsS3V4Signer.create();
117+
118+
SdkHttpFullRequest signedRequest =
119+
signer.presign(request,
120+
Aws4PresignerParams.builder()
121+
.awsCredentials(AwsBasicCredentials.create("akid", "skid"))
122+
.signingRegion(Region.US_WEST_2)
123+
.signingName("s3")
124+
.signingClockOverride(UTC_EPOCH_CLOCK)
125+
.build());
126+
127+
assertThat(signedRequest.firstMatchingRawQueryParameter("X-Amz-Signature"))
128+
.hasValue("3a9d36d37e9a554b7a3803f58ee7539b5d1f52fdfe89ce6fd40fb25762a35ec3");
129+
}
130+
131+
@Test
132+
public void presignWithExecutionAttributes_urlsAreNotNormalized() {
133+
byte[] bytes = new byte[1000];
134+
ThreadLocalRandom.current().nextBytes(bytes);
135+
ByteBuffer buffer = ByteBuffer.wrap(bytes);
136+
URI target = URI.create("https://test.com/./foo");
137+
138+
SdkHttpFullRequest request = SdkHttpFullRequest.builder()
139+
.contentStreamProvider(RequestBody.fromByteBuffer(buffer)
140+
.contentStreamProvider())
141+
.method(SdkHttpMethod.GET)
142+
.uri(target)
143+
.encodedPath(target.getPath())
144+
.build();
145+
ExecutionAttributes attributes =
146+
ExecutionAttributes.builder()
147+
.put(AwsSignerExecutionAttribute.AWS_CREDENTIALS,
148+
AwsBasicCredentials.create("akid", "skid"))
149+
.put(AwsSignerExecutionAttribute.SIGNING_REGION, Region.US_WEST_2)
150+
.put(AwsSignerExecutionAttribute.SERVICE_SIGNING_NAME, "s3")
151+
.put(AwsSignerExecutionAttribute.SIGNING_CLOCK, UTC_EPOCH_CLOCK)
152+
.build();
153+
154+
AwsS3V4Signer signer = AwsS3V4Signer.create();
155+
SdkHttpFullRequest signedRequest = signer.presign(request, attributes);
156+
157+
assertThat(signedRequest.firstMatchingRawQueryParameter("X-Amz-Signature"))
158+
.hasValue("3a9d36d37e9a554b7a3803f58ee7539b5d1f52fdfe89ce6fd40fb25762a35ec3");
159+
}
160+
161+
@Test
162+
public void signWithParams_doesNotFailWithEncodedCharacters() {
163+
URI target = URI.create("https://test.com/%20foo");
164+
165+
SdkHttpFullRequest request = SdkHttpFullRequest.builder()
166+
.method(SdkHttpMethod.GET)
167+
.uri(target)
168+
.encodedPath(target.getPath())
169+
.build();
170+
AwsS3V4Signer signer = AwsS3V4Signer.create();
171+
assertDoesNotThrow(() ->
172+
signer.sign(request,
173+
AwsS3V4SignerParams.builder()
174+
.awsCredentials(AwsBasicCredentials.create("akid", "skid"))
175+
.signingRegion(Region.US_WEST_2)
176+
.signingName("s3")
177+
.build()));
178+
}
179+
180+
@Test
181+
public void signWithExecutionAttributes_doesNotFailWithEncodedCharacters() {
182+
URI target = URI.create("https://test.com/%20foo");
183+
184+
SdkHttpFullRequest request = SdkHttpFullRequest.builder()
185+
.method(SdkHttpMethod.GET)
186+
.uri(target)
187+
.encodedPath(target.getPath())
188+
.build();
189+
ExecutionAttributes attributes =
190+
ExecutionAttributes.builder()
191+
.put(AwsSignerExecutionAttribute.AWS_CREDENTIALS,
192+
AwsBasicCredentials.create("akid", "skid"))
193+
.put(AwsSignerExecutionAttribute.SIGNING_REGION, Region.US_WEST_2)
194+
.put(AwsSignerExecutionAttribute.SERVICE_SIGNING_NAME, "s3")
195+
.build();
196+
197+
AwsS3V4Signer signer = AwsS3V4Signer.create();
198+
assertDoesNotThrow(() -> signer.sign(request, attributes));
199+
}
200+
201+
@Test
202+
public void presignWithParams_doesNotFailWithEncodedCharacters() {
203+
URI target = URI.create("https://test.com/%20foo");
204+
205+
SdkHttpFullRequest request = SdkHttpFullRequest.builder()
206+
.method(SdkHttpMethod.GET)
207+
.uri(target)
208+
.encodedPath(target.getPath())
209+
.build();
210+
AwsS3V4Signer signer = AwsS3V4Signer.create();
211+
212+
assertDoesNotThrow(() ->
213+
signer.presign(request,
214+
Aws4PresignerParams.builder()
215+
.awsCredentials(AwsBasicCredentials.create("akid", "skid"))
216+
.signingRegion(Region.US_WEST_2)
217+
.signingName("s3")
218+
.build()));
219+
}
220+
221+
@Test
222+
public void presignWithExecutionAttributes_doesNotFailWithEncodedCharacters() {
223+
URI target = URI.create("https://test.com/%20foo");
224+
225+
SdkHttpFullRequest request = SdkHttpFullRequest.builder()
226+
.method(SdkHttpMethod.GET)
227+
.uri(target)
228+
.encodedPath(target.getPath())
229+
.build();
230+
ExecutionAttributes attributes =
231+
ExecutionAttributes.builder()
232+
.put(AwsSignerExecutionAttribute.AWS_CREDENTIALS,
233+
AwsBasicCredentials.create("akid", "skid"))
234+
.put(AwsSignerExecutionAttribute.SIGNING_REGION, Region.US_WEST_2)
235+
.put(AwsSignerExecutionAttribute.SERVICE_SIGNING_NAME, "s3")
236+
.build();
237+
238+
AwsS3V4Signer signer = AwsS3V4Signer.create();
239+
assertDoesNotThrow(() -> signer.presign(request, attributes));
240+
}
241+
}

0 commit comments

Comments
 (0)