From 2951a45b65b63861a72ae510e312a6e740a7144c Mon Sep 17 00:00:00 2001 From: Matthew Miller Date: Thu, 8 Dec 2022 19:31:16 +0000 Subject: [PATCH] Fixed a bug where path normalization was not being disabled when signing requests directly with AwsS3V4Signer. The issue was introduced with https://github.com/aws/aws-sdk-java-v2/pull/3534. The PR did not account for users who are using the signer directly. --- .../next-release/bugfix-AmazonS3-ba24933.json | 6 + .../signer/AwsSignerExecutionAttribute.java | 9 + .../signer/internal/AbstractAws4Signer.java | 3 +- .../internal/AbstractAwsS3V4Signer.java | 2 + .../signer/params/Aws4PresignerParams.java | 18 +- .../auth/signer/params/Aws4SignerParams.java | 14 +- .../signer/params/AwsS3V4SignerParams.java | 2 + .../awssdk/auth/signer/AwsS3V4SignerTest.java | 241 ++++++++++++++++++ 8 files changed, 290 insertions(+), 5 deletions(-) create mode 100644 .changes/next-release/bugfix-AmazonS3-ba24933.json create mode 100644 core/auth/src/test/java/software/amazon/awssdk/auth/signer/AwsS3V4SignerTest.java diff --git a/.changes/next-release/bugfix-AmazonS3-ba24933.json b/.changes/next-release/bugfix-AmazonS3-ba24933.json new file mode 100644 index 000000000000..6453a63851dd --- /dev/null +++ b/.changes/next-release/bugfix-AmazonS3-ba24933.json @@ -0,0 +1,6 @@ +{ + "category": "Amazon S3", + "contributor": "", + "type": "bugfix", + "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." +} diff --git a/core/auth/src/main/java/software/amazon/awssdk/auth/signer/AwsSignerExecutionAttribute.java b/core/auth/src/main/java/software/amazon/awssdk/auth/signer/AwsSignerExecutionAttribute.java index f32072be4057..a90eae258660 100644 --- a/core/auth/src/main/java/software/amazon/awssdk/auth/signer/AwsSignerExecutionAttribute.java +++ b/core/auth/src/main/java/software/amazon/awssdk/auth/signer/AwsSignerExecutionAttribute.java @@ -15,9 +15,11 @@ package software.amazon.awssdk.auth.signer; +import java.time.Clock; import java.time.Instant; import software.amazon.awssdk.annotations.SdkProtectedApi; import software.amazon.awssdk.auth.credentials.AwsCredentials; +import software.amazon.awssdk.auth.signer.params.Aws4SignerParams; import software.amazon.awssdk.core.interceptor.ExecutionAttribute; import software.amazon.awssdk.core.interceptor.ExecutionInterceptor; import software.amazon.awssdk.core.interceptor.SdkExecutionAttribute; @@ -64,6 +66,13 @@ public final class AwsSignerExecutionAttribute extends SdkExecutionAttribute { public static final ExecutionAttribute SIGNER_NORMALIZE_PATH = new ExecutionAttribute<>("NormalizePath"); + + /** + * An override clock to use during signing. + * @see Aws4SignerParams.Builder#signingClockOverride(Clock) + */ + public static final ExecutionAttribute SIGNING_CLOCK = new ExecutionAttribute<>("Clock"); + /** * The key to specify the expiration time when pre-signing aws requests. */ diff --git a/core/auth/src/main/java/software/amazon/awssdk/auth/signer/internal/AbstractAws4Signer.java b/core/auth/src/main/java/software/amazon/awssdk/auth/signer/internal/AbstractAws4Signer.java index 472b6845e758..046554f1ecf1 100644 --- a/core/auth/src/main/java/software/amazon/awssdk/auth/signer/internal/AbstractAws4Signer.java +++ b/core/auth/src/main/java/software/amazon/awssdk/auth/signer/internal/AbstractAws4Signer.java @@ -414,7 +414,8 @@ protected B extractSignerParams(B paramsBui paramsBuilder.awsCredentials(executionAttributes.getAttribute(AwsSignerExecutionAttribute.AWS_CREDENTIALS)) .signingName(executionAttributes.getAttribute(AwsSignerExecutionAttribute.SERVICE_SIGNING_NAME)) .signingRegion(executionAttributes.getAttribute(AwsSignerExecutionAttribute.SIGNING_REGION)) - .timeOffset(executionAttributes.getAttribute(AwsSignerExecutionAttribute.TIME_OFFSET)); + .timeOffset(executionAttributes.getAttribute(AwsSignerExecutionAttribute.TIME_OFFSET)) + .signingClockOverride(executionAttributes.getAttribute(AwsSignerExecutionAttribute.SIGNING_CLOCK)); Boolean doubleUrlEncode = executionAttributes.getAttribute(AwsSignerExecutionAttribute.SIGNER_DOUBLE_URL_ENCODE); if (doubleUrlEncode != null) { diff --git a/core/auth/src/main/java/software/amazon/awssdk/auth/signer/internal/AbstractAwsS3V4Signer.java b/core/auth/src/main/java/software/amazon/awssdk/auth/signer/internal/AbstractAwsS3V4Signer.java index 7c8e874bfbd1..fede7b801814 100644 --- a/core/auth/src/main/java/software/amazon/awssdk/auth/signer/internal/AbstractAwsS3V4Signer.java +++ b/core/auth/src/main/java/software/amazon/awssdk/auth/signer/internal/AbstractAwsS3V4Signer.java @@ -115,6 +115,8 @@ public SdkHttpFullRequest presign(SdkHttpFullRequest request, Aws4PresignerParam return request; } + signingParams = signingParams.copy(b -> b.normalizePath(false)); + Aws4SignerRequestParams requestParams = new Aws4SignerRequestParams(signingParams); return doPresign(request, requestParams, signingParams).build(); diff --git a/core/auth/src/main/java/software/amazon/awssdk/auth/signer/params/Aws4PresignerParams.java b/core/auth/src/main/java/software/amazon/awssdk/auth/signer/params/Aws4PresignerParams.java index 4a3fff21cd92..590d959c0aa1 100644 --- a/core/auth/src/main/java/software/amazon/awssdk/auth/signer/params/Aws4PresignerParams.java +++ b/core/auth/src/main/java/software/amazon/awssdk/auth/signer/params/Aws4PresignerParams.java @@ -19,9 +19,13 @@ import java.util.Optional; import software.amazon.awssdk.annotations.SdkPublicApi; import software.amazon.awssdk.auth.signer.internal.SignerConstant; +import software.amazon.awssdk.utils.builder.CopyableBuilder; +import software.amazon.awssdk.utils.builder.ToCopyableBuilder; @SdkPublicApi -public final class Aws4PresignerParams extends Aws4SignerParams { +public final class Aws4PresignerParams + extends Aws4SignerParams + implements ToCopyableBuilder { private final Instant expirationTime; @@ -38,7 +42,12 @@ public static Builder builder() { return new BuilderImpl(); } - public interface Builder extends Aws4SignerParams.Builder { + @Override + public Builder toBuilder() { + return new BuilderImpl(this); + } + + public interface Builder extends Aws4SignerParams.Builder, CopyableBuilder { /** * 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 { + public interface Builder> { /** * Set this value to double url-encode the resource path when constructing the @@ -155,7 +155,7 @@ public interface Builder { Aws4SignerParams build(); } - protected static class BuilderImpl implements Builder { + protected static class BuilderImpl> implements Builder { private static final Boolean DEFAULT_DOUBLE_URL_ENCODE = Boolean.TRUE; private Boolean doubleUrlEncode = DEFAULT_DOUBLE_URL_ENCODE; @@ -168,7 +168,17 @@ protected static class BuilderImpl implements Builder { private SignerChecksumParams checksumParams; protected BuilderImpl() { + } + protected BuilderImpl(Aws4SignerParams params) { + doubleUrlEncode = params.doubleUrlEncode; + normalizePath = params.normalizePath; + awsCredentials = params.awsCredentials; + signingName = params.signingName; + signingRegion = params.signingRegion; + timeOffset = params.timeOffset; + signingClockOverride = params.signingClockOverride; + checksumParams = params.checksumParams; } @Override diff --git a/core/auth/src/main/java/software/amazon/awssdk/auth/signer/params/AwsS3V4SignerParams.java b/core/auth/src/main/java/software/amazon/awssdk/auth/signer/params/AwsS3V4SignerParams.java index 9eeb5a5001a7..37628d6f8a93 100644 --- a/core/auth/src/main/java/software/amazon/awssdk/auth/signer/params/AwsS3V4SignerParams.java +++ b/core/auth/src/main/java/software/amazon/awssdk/auth/signer/params/AwsS3V4SignerParams.java @@ -94,6 +94,8 @@ private static final class BuilderImpl extends Aws4SignerParams.BuilderImpl + signer.sign(request, + AwsS3V4SignerParams.builder() + .awsCredentials(AwsBasicCredentials.create("akid", "skid")) + .signingRegion(Region.US_WEST_2) + .signingName("s3") + .build())); + } + + @Test + public void signWithExecutionAttributes_doesNotFailWithEncodedCharacters() { + URI target = URI.create("https://test.com/%20foo"); + + SdkHttpFullRequest request = SdkHttpFullRequest.builder() + .method(SdkHttpMethod.GET) + .uri(target) + .encodedPath(target.getPath()) + .build(); + ExecutionAttributes attributes = + ExecutionAttributes.builder() + .put(AwsSignerExecutionAttribute.AWS_CREDENTIALS, + AwsBasicCredentials.create("akid", "skid")) + .put(AwsSignerExecutionAttribute.SIGNING_REGION, Region.US_WEST_2) + .put(AwsSignerExecutionAttribute.SERVICE_SIGNING_NAME, "s3") + .build(); + + AwsS3V4Signer signer = AwsS3V4Signer.create(); + assertDoesNotThrow(() -> signer.sign(request, attributes)); + } + + @Test + public void presignWithParams_doesNotFailWithEncodedCharacters() { + URI target = URI.create("https://test.com/%20foo"); + + SdkHttpFullRequest request = SdkHttpFullRequest.builder() + .method(SdkHttpMethod.GET) + .uri(target) + .encodedPath(target.getPath()) + .build(); + AwsS3V4Signer signer = AwsS3V4Signer.create(); + + assertDoesNotThrow(() -> + signer.presign(request, + Aws4PresignerParams.builder() + .awsCredentials(AwsBasicCredentials.create("akid", "skid")) + .signingRegion(Region.US_WEST_2) + .signingName("s3") + .build())); + } + + @Test + public void presignWithExecutionAttributes_doesNotFailWithEncodedCharacters() { + URI target = URI.create("https://test.com/%20foo"); + + SdkHttpFullRequest request = SdkHttpFullRequest.builder() + .method(SdkHttpMethod.GET) + .uri(target) + .encodedPath(target.getPath()) + .build(); + ExecutionAttributes attributes = + ExecutionAttributes.builder() + .put(AwsSignerExecutionAttribute.AWS_CREDENTIALS, + AwsBasicCredentials.create("akid", "skid")) + .put(AwsSignerExecutionAttribute.SIGNING_REGION, Region.US_WEST_2) + .put(AwsSignerExecutionAttribute.SERVICE_SIGNING_NAME, "s3") + .build(); + + AwsS3V4Signer signer = AwsS3V4Signer.create(); + assertDoesNotThrow(() -> signer.presign(request, attributes)); + } +} \ No newline at end of file