diff --git a/.changes/next-release/bugfix-AmazonS3-0f180ea.json b/.changes/next-release/bugfix-AmazonS3-0f180ea.json new file mode 100644 index 000000000000..e3a44a77f206 --- /dev/null +++ b/.changes/next-release/bugfix-AmazonS3-0f180ea.json @@ -0,0 +1,5 @@ +{ + "type": "bugfix", + "category": "Amazon S3", + "description": "Interacting with an access point in a different region to the one the S3 client is configured for will no longer result in the request being signed for the wrong region and rejected by S3." +} diff --git a/services/s3/src/it/java/software/amazon/awssdk/services/s3/AccessPointsIntegrationTest.java b/services/s3/src/it/java/software/amazon/awssdk/services/s3/AccessPointsIntegrationTest.java index d4968bbee621..2fee72c61c9d 100644 --- a/services/s3/src/it/java/software/amazon/awssdk/services/s3/AccessPointsIntegrationTest.java +++ b/services/s3/src/it/java/software/amazon/awssdk/services/s3/AccessPointsIntegrationTest.java @@ -84,4 +84,25 @@ public void transfer_Succeeds_UsingAccessPoint() { assertThat(objectContent).isEqualTo("helloworld"); } + + @Test + public void transfer_Succeeds_UsingAccessPoint_CrossRegion() { + S3Client s3DifferentRegion = + s3ClientBuilder().region(Region.US_EAST_1).serviceConfiguration(c -> c.useArnRegionEnabled(true)).build(); + + StringJoiner apArn = new StringJoiner(":"); + apArn.add("arn").add("aws").add("s3").add("us-west-2").add(accountId).add("accesspoint").add(AP_NAME); + + s3DifferentRegion.putObject(PutObjectRequest.builder() + .bucket(apArn.toString()) + .key(KEY) + .build(), RequestBody.fromString("helloworld")); + + String objectContent = s3DifferentRegion.getObjectAsBytes(GetObjectRequest.builder() + .bucket(apArn.toString()) + .key(KEY) + .build()).asUtf8String(); + + assertThat(objectContent).isEqualTo("helloworld"); + } } diff --git a/services/s3/src/main/java/software/amazon/awssdk/services/s3/S3Utilities.java b/services/s3/src/main/java/software/amazon/awssdk/services/s3/S3Utilities.java index 9c8c1ca05315..7e24e0d0f57f 100644 --- a/services/s3/src/main/java/software/amazon/awssdk/services/s3/S3Utilities.java +++ b/services/s3/src/main/java/software/amazon/awssdk/services/s3/S3Utilities.java @@ -19,6 +19,7 @@ import java.net.URI; import java.net.URL; import java.util.function.Consumer; + import software.amazon.awssdk.annotations.Immutable; import software.amazon.awssdk.annotations.SdkInternalApi; import software.amazon.awssdk.annotations.SdkPublicApi; @@ -154,7 +155,8 @@ public URL getUrl(GetUrlRequest getUrlRequest) { getObjectRequest, resolvedRegion, s3Configuration, - endpointOverridden); + endpointOverridden) + .sdkHttpRequest(); try { return httpRequest.getUri().toURL(); diff --git a/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/ConfiguredS3SdkHttpRequest.java b/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/ConfiguredS3SdkHttpRequest.java new file mode 100644 index 000000000000..481458c93179 --- /dev/null +++ b/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/ConfiguredS3SdkHttpRequest.java @@ -0,0 +1,103 @@ +/* + * Copyright 2010-2019 Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"). + * You may not use this file except in compliance with the License. + * A copy of the License is located at + * + * http://aws.amazon.com/apache2.0 + * + * or in the "license" file accompanying this file. This file is distributed + * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either + * express or implied. See the License for the specific language governing + * permissions and limitations under the License. + */ + +package software.amazon.awssdk.services.s3.internal; + +import java.util.Optional; + +import software.amazon.awssdk.annotations.SdkInternalApi; +import software.amazon.awssdk.http.SdkHttpRequest; +import software.amazon.awssdk.regions.Region; +import software.amazon.awssdk.utils.Validate; +import software.amazon.awssdk.utils.builder.CopyableBuilder; +import software.amazon.awssdk.utils.builder.ToCopyableBuilder; + +@SdkInternalApi +public class ConfiguredS3SdkHttpRequest + implements ToCopyableBuilder { + private final SdkHttpRequest sdkHttpRequest; + private final Region signingRegionModification; + + private ConfiguredS3SdkHttpRequest(Builder builder) { + this.sdkHttpRequest = Validate.notNull(builder.sdkHttpRequest, "sdkHttpRequest"); + this.signingRegionModification = builder.signingRegionModification; + } + + public static Builder builder() { + return new Builder(); + } + + public SdkHttpRequest sdkHttpRequest() { + return sdkHttpRequest; + } + + public Optional signingRegionModification() { + return Optional.ofNullable(signingRegionModification); + } + + @Override + public Builder toBuilder() { + return builder().sdkHttpRequest(sdkHttpRequest).signingRegionModification(signingRegionModification); + } + + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + + ConfiguredS3SdkHttpRequest that = (ConfiguredS3SdkHttpRequest) o; + + if (sdkHttpRequest != null ? ! sdkHttpRequest.equals(that.sdkHttpRequest) : that.sdkHttpRequest != null) { + return false; + } + return signingRegionModification != null ? + signingRegionModification.equals(that.signingRegionModification) + : that.signingRegionModification == null; + } + + @Override + public int hashCode() { + int result = sdkHttpRequest != null ? sdkHttpRequest.hashCode() : 0; + result = 31 * result + (signingRegionModification != null ? signingRegionModification.hashCode() : 0); + return result; + } + + public static class Builder implements CopyableBuilder { + private SdkHttpRequest sdkHttpRequest; + private Region signingRegionModification; + + private Builder() {} + + public Builder sdkHttpRequest(SdkHttpRequest sdkHttpRequest) { + this.sdkHttpRequest = sdkHttpRequest; + return this; + } + + public Builder signingRegionModification(Region signingRegionModification) { + this.signingRegionModification = signingRegionModification; + return this; + } + + @Override + public ConfiguredS3SdkHttpRequest build() { + return new ConfiguredS3SdkHttpRequest(this); + } + } + +} diff --git a/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/S3EndpointUtils.java b/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/S3EndpointUtils.java index cff7619888a9..f2668b6ffd9f 100644 --- a/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/S3EndpointUtils.java +++ b/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/S3EndpointUtils.java @@ -21,6 +21,7 @@ import java.net.URISyntaxException; import java.util.Arrays; import java.util.List; + import software.amazon.awssdk.annotations.SdkInternalApi; import software.amazon.awssdk.arns.Arn; import software.amazon.awssdk.core.SdkRequest; @@ -56,11 +57,11 @@ private S3EndpointUtils() { * Returns a new instance of the given {@link SdkHttpRequest} by applying any endpoint changes based on * the given {@link S3Configuration} options. */ - public static SdkHttpRequest applyEndpointConfiguration(SdkHttpRequest request, - SdkRequest originalRequest, - Region region, - S3Configuration serviceConfiguration, - boolean endpointOverridden) { + public static ConfiguredS3SdkHttpRequest applyEndpointConfiguration(SdkHttpRequest request, + SdkRequest originalRequest, + Region region, + S3Configuration serviceConfiguration, + boolean endpointOverridden) { String bucketName = originalRequest.getValueForField("Bucket", String.class).orElse(null); String key = originalRequest.getValueForField("Key", String.class).orElse(null); @@ -82,15 +83,19 @@ public static SdkHttpRequest applyEndpointConfiguration(SdkHttpRequest request, } } - return mutableRequest.build(); + return ConfiguredS3SdkHttpRequest.builder() + .sdkHttpRequest(mutableRequest.build()) + .build(); } - private static SdkHttpRequest applyEndpointConfigurationForAccessPointArn(SdkHttpRequest request, - Region region, - boolean endpointOverridden, - S3Configuration serviceConfiguration, - String bucketName, - String key) { + private static ConfiguredS3SdkHttpRequest applyEndpointConfigurationForAccessPointArn( + SdkHttpRequest request, + Region region, + boolean endpointOverridden, + S3Configuration serviceConfiguration, + String bucketName, + String key) { + Arn resourceArn = Arn.fromString(bucketName); S3Resource s3Resource = S3ArnConverter.create().convertArn(resourceArn); @@ -169,12 +174,17 @@ private static SdkHttpRequest applyEndpointConfigurationForAccessPointArn(SdkHtt .dualstackEnabled(dualstackEnabled) .toUri(); - return request.toBuilder() - .protocol(accessPointUri.getScheme()) - .host(accessPointUri.getHost()) - .port(accessPointUri.getPort()) - .encodedPath(key) - .build(); + SdkHttpRequest httpRequest = request.toBuilder() + .protocol(accessPointUri.getScheme()) + .host(accessPointUri.getHost()) + .port(accessPointUri.getPort()) + .encodedPath(key) + .build(); + + return ConfiguredS3SdkHttpRequest.builder() + .sdkHttpRequest(httpRequest) + .signingRegionModification(Region.of(arnRegion)) + .build(); } /** diff --git a/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/handlers/EndpointAddressInterceptor.java b/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/handlers/EndpointAddressInterceptor.java index 2bbb4b728d80..98d2d8fd0803 100644 --- a/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/handlers/EndpointAddressInterceptor.java +++ b/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/handlers/EndpointAddressInterceptor.java @@ -24,6 +24,7 @@ import software.amazon.awssdk.core.interceptor.SdkExecutionAttribute; import software.amazon.awssdk.http.SdkHttpRequest; import software.amazon.awssdk.services.s3.S3Configuration; +import software.amazon.awssdk.services.s3.internal.ConfiguredS3SdkHttpRequest; import software.amazon.awssdk.services.s3.internal.S3EndpointUtils; @SdkInternalApi @@ -32,11 +33,17 @@ public final class EndpointAddressInterceptor implements ExecutionInterceptor { @Override public SdkHttpRequest modifyHttpRequest(Context.ModifyHttpRequest context, ExecutionAttributes executionAttributes) { - return S3EndpointUtils.applyEndpointConfiguration( - context.httpRequest(), - context.request(), - executionAttributes.getAttribute(AwsExecutionAttribute.AWS_REGION), - (S3Configuration) executionAttributes.getAttribute(AwsSignerExecutionAttribute.SERVICE_CONFIG), - Boolean.TRUE.equals(executionAttributes.getAttribute(SdkExecutionAttribute.ENDPOINT_OVERRIDDEN))); + ConfiguredS3SdkHttpRequest configuredRequest = + S3EndpointUtils.applyEndpointConfiguration( + context.httpRequest(), + context.request(), + executionAttributes.getAttribute(AwsExecutionAttribute.AWS_REGION), + (S3Configuration) executionAttributes.getAttribute(AwsSignerExecutionAttribute.SERVICE_CONFIG), + Boolean.TRUE.equals(executionAttributes.getAttribute(SdkExecutionAttribute.ENDPOINT_OVERRIDDEN))); + + configuredRequest.signingRegionModification().ifPresent( + region -> executionAttributes.putAttribute(AwsSignerExecutionAttribute.SIGNING_REGION, region)); + + return configuredRequest.sdkHttpRequest(); } } diff --git a/services/s3/src/test/java/software/amazon/awssdk/services/s3/internal/handlers/EndpointAddressInterceptorTest.java b/services/s3/src/test/java/software/amazon/awssdk/services/s3/internal/handlers/EndpointAddressInterceptorTest.java index cee305b0ff4c..b6edc541ad28 100644 --- a/services/s3/src/test/java/software/amazon/awssdk/services/s3/internal/handlers/EndpointAddressInterceptorTest.java +++ b/services/s3/src/test/java/software/amazon/awssdk/services/s3/internal/handlers/EndpointAddressInterceptorTest.java @@ -17,6 +17,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static software.amazon.awssdk.auth.signer.AwsSignerExecutionAttribute.SIGNING_REGION; import static software.amazon.awssdk.awscore.AwsExecutionAttribute.AWS_REGION; import static software.amazon.awssdk.core.interceptor.SdkExecutionAttribute.SERVICE_CONFIG; import static software.amazon.awssdk.utils.http.SdkHttpUtils.urlEncode; @@ -24,6 +25,8 @@ import java.net.URI; import java.util.Optional; import org.junit.Test; + +import software.amazon.awssdk.auth.signer.AwsSignerExecutionAttribute; import software.amazon.awssdk.core.SdkRequest; import software.amazon.awssdk.core.async.AsyncRequestBody; import software.amazon.awssdk.core.interceptor.Context; @@ -131,6 +134,7 @@ public void accesspointArn_futureUnknownRegion_US_correctlyInfersPartition() { verifyAccesspointArn("http", "arn:aws:s3:us-future-1:12345678910:accesspoint:foobar", "http://foobar-12345678910.s3-accesspoint.us-future-1.amazonaws.com", + Region.of("us-future-1"), S3Configuration.builder(), Region.of("us-future-1")); } @@ -140,6 +144,7 @@ public void accesspointArn_futureUnknownRegion_crossRegion_correctlyInfersPartit verifyAccesspointArn("http", "arn:aws:s3:us-future-2:12345678910:accesspoint:foobar", "http://foobar-12345678910.s3-accesspoint.us-future-2.amazonaws.com", + Region.of("us-future-2"), S3Configuration.builder().useArnRegionEnabled(true), Region.of("us-future-1")); } @@ -149,6 +154,7 @@ public void accesspointArn_futureUnknownRegion_CN_correctlyInfersPartition() { verifyAccesspointArn("http", "arn:aws-cn:s3:cn-future-1:12345678910:accesspoint:foobar", "http://foobar-12345678910.s3-accesspoint.cn-future-1.amazonaws.com.cn", + Region.of("cn-future-1"), S3Configuration.builder(), Region.of("cn-future-1")); } @@ -158,6 +164,7 @@ public void accesspointArn_futureUnknownRegionAndPartition_defaultsToAws() { verifyAccesspointArn("http", "arn:aws:s3:unknown:12345678910:accesspoint:foobar", "http://foobar-12345678910.s3-accesspoint.unknown.amazonaws.com", + Region.of("unknown"), S3Configuration.builder(), Region.of("unknown")); } @@ -232,11 +239,13 @@ public void accesspointArn_withCnPartition_shouldConvertEndpoint() { verifyAccesspointArn("http", "arn:aws-cn:s3:cn-north-1:12345678910:accesspoint:foobar", "http://foobar-12345678910.s3-accesspoint.cn-north-1.amazonaws.com.cn", + Region.of("cn-north-1"), S3Configuration.builder(), Region.of("cn-north-1")); verifyAccesspointArn("https", "arn:aws-cn:s3:cn-north-1:12345678910:accesspoint:foobar", "https://foobar-12345678910.s3-accesspoint.cn-north-1.amazonaws.com.cn", + Region.of("cn-north-1"), S3Configuration.builder(), Region.of("cn-north-1")); } @@ -246,6 +255,7 @@ public void accesspointArn_withDifferentPartition_useArnRegionEnabled_shouldThro assertThatThrownBy(() -> verifyAccesspointArn("http", "arn:aws-cn:s3:cn-north-1:12345678910:accesspoint:foobar", "http://foobar-12345678910.s3-accesspoint.cn-north-1.amazonaws.com.cn", + Region.of("cn-north-1"), S3Configuration.builder().useArnRegionEnabled(true), Region.of("us-east-1"))) .isInstanceOf(IllegalArgumentException.class) @@ -257,6 +267,7 @@ public void accesspointArn_withFipsRegionPrefix_useArnRegionEnabled_shouldThrowI assertThatThrownBy(() -> verifyAccesspointArn("http", "arn:aws:s3:us-east-1:12345678910:accesspoint/foobar", "http://foobar-12345678910.s3-accesspoint.us-east-1.amazonaws.com", + Region.of("us-east-1"), S3Configuration.builder().useArnRegionEnabled(true), Region.of("fips-us-east-1"))) .isInstanceOf(IllegalArgumentException.class) @@ -264,6 +275,7 @@ public void accesspointArn_withFipsRegionPrefix_useArnRegionEnabled_shouldThrowI assertThatThrownBy(() -> verifyAccesspointArn("https", "arn:aws:s3:us-east-1:12345678910:accesspoint/foobar", "https://foobar-12345678910.s3-accesspoint.us-east-1.amazonaws.com", + Region.of("us-east-1"), S3Configuration.builder().useArnRegionEnabled(true), Region.of("fips-us-east-1"))) .isInstanceOf(IllegalArgumentException.class) @@ -275,6 +287,7 @@ public void accesspointArn_withFipsRegionPrefix_shouldThrowIllegalArgumentExcept assertThatThrownBy(() -> verifyAccesspointArn("http", "arn:aws:s3:us-east-1:12345678910:accesspoint/foobar", "http://foobar-12345678910.s3-accesspoint.us-east-1.amazonaws.com", + Region.of("us-east-1"), S3Configuration.builder(), Region.of("fips-us-east-1"))) .isInstanceOf(IllegalArgumentException.class) @@ -282,6 +295,7 @@ public void accesspointArn_withFipsRegionPrefix_shouldThrowIllegalArgumentExcept assertThatThrownBy(() -> verifyAccesspointArn("https", "arn:aws:s3:us-east-1:12345678910:accesspoint/foobar", "https://foobar-12345678910.s3-accesspoint.us-east-1.amazonaws.com", + Region.of("us-east-1"), S3Configuration.builder(), Region.of("fips-us-east-1"))) .isInstanceOf(IllegalArgumentException.class) @@ -293,6 +307,7 @@ public void accesspointArn_withFipsRegionSuffix_useArnRegionEnabled_shouldThrowI assertThatThrownBy(() -> verifyAccesspointArn("http", "arn:aws:s3:us-east-1:12345678910:accesspoint/foobar", "http://foobar-12345678910.s3-accesspoint.us-east-1.amazonaws.com", + Region.of("us-east-1"), S3Configuration.builder().useArnRegionEnabled(true), Region.of("us-east-1-fips"))) .isInstanceOf(IllegalArgumentException.class) @@ -300,6 +315,7 @@ public void accesspointArn_withFipsRegionSuffix_useArnRegionEnabled_shouldThrowI assertThatThrownBy(() -> verifyAccesspointArn("https", "arn:aws:s3:us-east-1:12345678910:accesspoint/foobar", "https://foobar-12345678910.s3-accesspoint.us-east-1.amazonaws.com", + Region.of("us-east-1"), S3Configuration.builder().useArnRegionEnabled(true), Region.of("us-east-1-fips"))) .isInstanceOf(IllegalArgumentException.class) @@ -311,6 +327,7 @@ public void accesspointArn_withFipsRegionSuffix_shouldThrowIllegalArgumentExcept assertThatThrownBy(() -> verifyAccesspointArn("http", "arn:aws:s3:us-east-1:12345678910:accesspoint/foobar", "http://foobar-12345678910.s3-accesspoint.us-east-1.amazonaws.com", + Region.of("us-east-1"), S3Configuration.builder(), Region.of("us-east-1-fips"))) .isInstanceOf(IllegalArgumentException.class) @@ -318,6 +335,7 @@ public void accesspointArn_withFipsRegionSuffix_shouldThrowIllegalArgumentExcept assertThatThrownBy(() -> verifyAccesspointArn("https", "arn:aws:s3:us-east-1:12345678910:accesspoint/foobar", "https://foobar-12345678910.s3-accesspoint.us-east-1.amazonaws.com", + Region.of("us-east-1"), S3Configuration.builder(), Region.of("us-east-1-fips"))) .isInstanceOf(IllegalArgumentException.class) @@ -329,6 +347,7 @@ public void accesspointArn_withAccelerateEnabled_shouldThrowIllegalArgumentExcep assertThatThrownBy(() -> verifyAccesspointArn("http", "arn:aws:s3:us-east-1:12345678910:accesspoint/foobar", "http://foobar-12345678910.s3-accesspoint.us-east-1.amazonaws.com", + Region.of("us-east-1"), S3Configuration.builder().accelerateModeEnabled(true), Region.of("us-east-1"))) .isInstanceOf(IllegalArgumentException.class) @@ -341,6 +360,7 @@ public void accesspointArn_withPathStyleAddressingEnabled_shouldThrowIllegalArgu assertThatThrownBy(() -> verifyAccesspointArn("http", "arn:aws:s3:us-east-1:12345678910:accesspoint/foobar", "http://foobar-12345678910.s3-accesspoint.us-east-1.amazonaws.com", + Region.of("us-east-1"), S3Configuration.builder().pathStyleAccessEnabled(true), Region.of("us-east-1"))) .isInstanceOf(IllegalArgumentException.class) @@ -410,6 +430,7 @@ private void verifyEndpoint(String protocol, String expectedEndpoint, } private void verifyAccesspointArn(String protocol, String accessPointArn, String expectedEndpoint, + Region expectedSigningRegion, S3Configuration.Builder builder, Region region) { String key = "test-key"; @@ -425,16 +446,19 @@ private void verifyAccesspointArn(String protocol, String accessPointArn, String executionAttributes.putAttribute(SERVICE_CONFIG, s3Configuration); executionAttributes.putAttribute(AWS_REGION, region); + executionAttributes.putAttribute(SIGNING_REGION, region); SdkHttpRequest sdkHttpFullRequest = interceptor.modifyHttpRequest(ctx, executionAttributes); + assertThat(executionAttributes.getAttribute(SIGNING_REGION)) + .isEqualTo(expectedSigningRegion); assertThat(sdkHttpFullRequest.getUri()).isEqualTo(expectedUri); } private void verifyAccesspointArn(String protocol, String accessPointArn, String expectedEndpoint, S3Configuration.Builder builder) { - verifyAccesspointArn(protocol, accessPointArn, expectedEndpoint, builder, Region.US_EAST_1); + verifyAccesspointArn(protocol, accessPointArn, expectedEndpoint, Region.US_EAST_1, builder, Region.US_EAST_1); } private Context.ModifyHttpRequest context(SdkRequest request, SdkHttpRequest sdkHttpRequest) {