From 1662cd45a4923d1e9db131d2a29a8f1b22ce85fe Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Fri, 21 Feb 2020 10:21:20 +0100 Subject: [PATCH] Add Region and Signer Algorithm Overrides to S3 Repos (#52112) (#52562) Exposes S3 SDK signing region and algorithm override settings as requested in #51861. Closes #51861 --- docs/plugins/repository-s3.asciidoc | 16 ++++++ .../repositories/s3/S3ClientSettings.java | 38 +++++++++++-- .../repositories/s3/S3RepositoryPlugin.java | 4 +- .../repositories/s3/S3Service.java | 10 +++- .../s3/S3BlobStoreRepositoryTests.java | 56 +++++++++++++++++-- .../s3/S3ClientSettingsTests.java | 25 +++++++++ 6 files changed, 135 insertions(+), 14 deletions(-) diff --git a/docs/plugins/repository-s3.asciidoc b/docs/plugins/repository-s3.asciidoc index c120c54153a..89b454ec123 100644 --- a/docs/plugins/repository-s3.asciidoc +++ b/docs/plugins/repository-s3.asciidoc @@ -184,6 +184,22 @@ pattern then you should set this setting to `true` when upgrading. https://docs.aws.amazon.com/AWSJavaSDK/latest/javadoc/com/amazonaws/services/s3/AmazonS3Builder.html#disableChunkedEncoding--[AWS Java SDK documentation] for details. Defaults to `false`. +`region`:: + + Allows specifying the signing region to use. Specificing this setting manually should not be necessary for most use cases. Generally, + the SDK will correctly guess the signing region to use. It should be considered an expert level setting to support S3-compatible APIs + that require https://docs.aws.amazon.com/general/latest/gr/signature-version-4.html[v4 signatures] and use a region other than the + default `us-east-1`. Defaults to empty string which means that the SDK will try to automatically determine the correct signing region. + +`signer_override`:: + + Allows specifying the name of the signature algorithm to use for signing requests by the S3 client. Specifying this setting should not + be necessary for most use cases. It should be considered an expert level setting to support S3-compatible APIs that do not support the + signing algorithm that the SDK automatically determines for them. + See the + https://docs.aws.amazon.com/AWSJavaSDK/latest/javadoc/com/amazonaws/ClientConfiguration.html#setSignerOverride-java.lang.String-[AWS + Java SDK documentation] for details. Defaults to empty string which means that no signing algorithm override will be used. + [float] [[repository-s3-compatible-services]] ===== S3-compatible services diff --git a/plugins/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3ClientSettings.java b/plugins/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3ClientSettings.java index fee00786a2a..e786f1b9274 100644 --- a/plugins/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3ClientSettings.java +++ b/plugins/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3ClientSettings.java @@ -35,6 +35,7 @@ import java.util.Locale; import java.util.Map; import java.util.Objects; import java.util.Set; +import java.util.function.Function; /** * A container for settings used to create an S3 client. @@ -103,6 +104,14 @@ final class S3ClientSettings { static final Setting.AffixSetting DISABLE_CHUNKED_ENCODING = Setting.affixKeySetting(PREFIX, "disable_chunked_encoding", key -> Setting.boolSetting(key, false, Property.NodeScope)); + /** An override for the s3 region to use for signing requests. */ + static final Setting.AffixSetting REGION = Setting.affixKeySetting(PREFIX, "region", + key -> new Setting<>(key, "", Function.identity(), Property.NodeScope)); + + /** An override for the signer to use. */ + static final Setting.AffixSetting SIGNER_OVERRIDE = Setting.affixKeySetting(PREFIX, "signer_override", + key -> new Setting<>(key, "", Function.identity(), Property.NodeScope)); + /** Credentials to authenticate with s3. */ final S3BasicCredentials credentials; @@ -141,10 +150,16 @@ final class S3ClientSettings { /** Whether chunked encoding should be disabled or not. */ final boolean disableChunkedEncoding; + /** Region to use for signing requests or empty string to use default. */ + final String region; + + /** Signer override to use or empty string to use default. */ + final String signerOverride; + private S3ClientSettings(S3BasicCredentials credentials, String endpoint, Protocol protocol, String proxyHost, int proxyPort, String proxyUsername, String proxyPassword, int readTimeoutMillis, int maxRetries, boolean throttleRetries, - boolean pathStyleAccess, boolean disableChunkedEncoding) { + boolean pathStyleAccess, boolean disableChunkedEncoding, String region, String signerOverride) { this.credentials = credentials; this.endpoint = endpoint; this.protocol = protocol; @@ -157,6 +172,8 @@ final class S3ClientSettings { this.throttleRetries = throttleRetries; this.pathStyleAccess = pathStyleAccess; this.disableChunkedEncoding = disableChunkedEncoding; + this.region = region; + this.signerOverride = signerOverride; } /** @@ -188,10 +205,13 @@ final class S3ClientSettings { } else { newCredentials = credentials; } + final String newRegion = getRepoSettingOrDefault(REGION, normalizedSettings, region); + final String newSignerOverride = getRepoSettingOrDefault(SIGNER_OVERRIDE, normalizedSettings, signerOverride); if (Objects.equals(endpoint, newEndpoint) && protocol == newProtocol && Objects.equals(proxyHost, newProxyHost) && proxyPort == newProxyPort && newReadTimeoutMillis == readTimeoutMillis && maxRetries == newMaxRetries && newThrottleRetries == throttleRetries && Objects.equals(credentials, newCredentials) - && newDisableChunkedEncoding == disableChunkedEncoding) { + && newDisableChunkedEncoding == disableChunkedEncoding + && Objects.equals(region, newRegion) && Objects.equals(signerOverride, newSignerOverride)) { return this; } return new S3ClientSettings( @@ -206,7 +226,9 @@ final class S3ClientSettings { newMaxRetries, newThrottleRetries, usePathStyleAccess, - newDisableChunkedEncoding + newDisableChunkedEncoding, + newRegion, + newSignerOverride ); } @@ -295,7 +317,9 @@ final class S3ClientSettings { getConfigValue(settings, clientName, MAX_RETRIES_SETTING), getConfigValue(settings, clientName, USE_THROTTLE_RETRIES_SETTING), getConfigValue(settings, clientName, USE_PATH_STYLE_ACCESS), - getConfigValue(settings, clientName, DISABLE_CHUNKED_ENCODING) + getConfigValue(settings, clientName, DISABLE_CHUNKED_ENCODING), + getConfigValue(settings, clientName, REGION), + getConfigValue(settings, clientName, SIGNER_OVERRIDE) ); } } @@ -319,13 +343,15 @@ final class S3ClientSettings { Objects.equals(proxyHost, that.proxyHost) && Objects.equals(proxyUsername, that.proxyUsername) && Objects.equals(proxyPassword, that.proxyPassword) && - Objects.equals(disableChunkedEncoding, that.disableChunkedEncoding); + Objects.equals(disableChunkedEncoding, that.disableChunkedEncoding) && + Objects.equals(region, that.region) && + Objects.equals(signerOverride, that.signerOverride); } @Override public int hashCode() { return Objects.hash(credentials, endpoint, protocol, proxyHost, proxyPort, proxyUsername, proxyPassword, - readTimeoutMillis, maxRetries, throttleRetries, disableChunkedEncoding); + readTimeoutMillis, maxRetries, throttleRetries, disableChunkedEncoding, region, signerOverride); } private static T getConfigValue(Settings settings, String clientName, diff --git a/plugins/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3RepositoryPlugin.java b/plugins/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3RepositoryPlugin.java index fda9677943e..e97114d3ebc 100644 --- a/plugins/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3RepositoryPlugin.java +++ b/plugins/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3RepositoryPlugin.java @@ -107,7 +107,9 @@ public class S3RepositoryPlugin extends Plugin implements RepositoryPlugin, Relo S3ClientSettings.USE_THROTTLE_RETRIES_SETTING, S3ClientSettings.USE_PATH_STYLE_ACCESS, S3Repository.ACCESS_KEY_SETTING, - S3Repository.SECRET_KEY_SETTING); + S3Repository.SECRET_KEY_SETTING, + S3ClientSettings.SIGNER_OVERRIDE, + S3ClientSettings.REGION); } @Override diff --git a/plugins/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3Service.java b/plugins/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3Service.java index 3ba8145f6a4..894922e9642 100644 --- a/plugins/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3Service.java +++ b/plugins/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3Service.java @@ -141,7 +141,8 @@ class S3Service implements Closeable { builder.withClientConfiguration(buildConfiguration(clientSettings)); final String endpoint = Strings.hasLength(clientSettings.endpoint) ? clientSettings.endpoint : Constants.S3_HOSTNAME; - logger.debug("using endpoint [{}]", endpoint); + final String region = Strings.hasLength(clientSettings.region) ? clientSettings.region : null; + logger.debug("using endpoint [{}] and region [{}]", endpoint, region); // If the endpoint configuration isn't set on the builder then the default behaviour is to try // and work out what region we are in and use an appropriate endpoint - see AwsClientBuilder#setRegion. @@ -151,7 +152,7 @@ class S3Service implements Closeable { // // We do this because directly constructing the client is deprecated (was already deprecated in 1.1.223 too) // so this change removes that usage of a deprecated API. - builder.withEndpointConfiguration(new AwsClientBuilder.EndpointConfiguration(endpoint, null)); + builder.withEndpointConfiguration(new AwsClientBuilder.EndpointConfiguration(endpoint, region)); if (clientSettings.pathStyleAccess) { builder.enablePathStyleAccess(); } @@ -177,6 +178,10 @@ class S3Service implements Closeable { clientConfiguration.setProxyPassword(clientSettings.proxyPassword); } + if (Strings.hasLength(clientSettings.signerOverride)) { + clientConfiguration.setSignerOverride(clientSettings.signerOverride); + } + clientConfiguration.setMaxErrorRetry(clientSettings.maxRetries); clientConfiguration.setUseThrottleRetries(clientSettings.throttleRetries); clientConfiguration.setSocketTimeout(clientSettings.readTimeoutMillis); @@ -231,5 +236,4 @@ class S3Service implements Closeable { public void close() { releaseCachedClients(); } - } diff --git a/plugins/repository-s3/src/test/java/org/elasticsearch/repositories/s3/S3BlobStoreRepositoryTests.java b/plugins/repository-s3/src/test/java/org/elasticsearch/repositories/s3/S3BlobStoreRepositoryTests.java index 81b1a42f56b..c28f6fbb66e 100644 --- a/plugins/repository-s3/src/test/java/org/elasticsearch/repositories/s3/S3BlobStoreRepositoryTests.java +++ b/plugins/repository-s3/src/test/java/org/elasticsearch/repositories/s3/S3BlobStoreRepositoryTests.java @@ -46,6 +46,7 @@ import org.elasticsearch.repositories.blobstore.ESMockAPIBasedRepositoryIntegTes import org.elasticsearch.snapshots.SnapshotId; import org.elasticsearch.snapshots.SnapshotsService; import org.elasticsearch.snapshots.mockstore.BlobStoreWrapper; +import org.elasticsearch.test.ESIntegTestCase; import org.elasticsearch.threadpool.ThreadPool; import java.io.IOException; @@ -56,14 +57,34 @@ import java.util.Collections; import java.util.List; import java.util.Map; +import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.greaterThan; import static org.hamcrest.Matchers.lessThan; +import static org.hamcrest.Matchers.startsWith; @SuppressForbidden(reason = "this test uses a HttpServer to emulate an S3 endpoint") +// Need to set up a new cluster for each test because cluster settings use randomized authentication settings +@ESIntegTestCase.ClusterScope(scope = ESIntegTestCase.Scope.TEST) public class S3BlobStoreRepositoryTests extends ESMockAPIBasedRepositoryIntegTestCase { private static final TimeValue TEST_COOLDOWN_PERIOD = TimeValue.timeValueSeconds(5L); + private String region; + private String signerOverride; + + @Override + public void setUp() throws Exception { + if (randomBoolean()) { + region = "test-region"; + } + if (region != null && randomBoolean()) { + signerOverride = randomFrom("AWS3SignerType", "AWS4SignerType"); + } else if (randomBoolean()) { + signerOverride = "AWS3SignerType"; + } + super.setUp(); + } + @Override protected String repositoryType() { return S3Repository.TYPE; @@ -101,7 +122,7 @@ public class S3BlobStoreRepositoryTests extends ESMockAPIBasedRepositoryIntegTes secureSettings.setString(S3ClientSettings.ACCESS_KEY_SETTING.getConcreteSettingForNamespace("test").getKey(), "access"); secureSettings.setString(S3ClientSettings.SECRET_KEY_SETTING.getConcreteSettingForNamespace("test").getKey(), "secret"); - return Settings.builder() + final Settings.Builder builder = Settings.builder() .put(ThreadPool.ESTIMATED_TIME_INTERVAL_SETTING.getKey(), 0) // We have tests that verify an exact wait time .put(S3ClientSettings.ENDPOINT_SETTING.getConcreteSettingForNamespace("test").getKey(), httpServerUrl()) // Disable chunked encoding as it simplifies a lot the request parsing on the httpServer side @@ -109,8 +130,15 @@ public class S3BlobStoreRepositoryTests extends ESMockAPIBasedRepositoryIntegTes // Disable request throttling because some random values in tests might generate too many failures for the S3 client .put(S3ClientSettings.USE_THROTTLE_RETRIES_SETTING.getConcreteSettingForNamespace("test").getKey(), false) .put(super.nodeSettings(nodeOrdinal)) - .setSecureSettings(secureSettings) - .build(); + .setSecureSettings(secureSettings); + + if (signerOverride != null) { + builder.put(S3ClientSettings.SIGNER_OVERRIDE.getConcreteSettingForNamespace("test").getKey(), signerOverride); + } + if (region != null) { + builder.put(S3ClientSettings.REGION.getConcreteSettingForNamespace("test").getKey(), region); + } + return builder.build(); } public void testEnforcedCooldownPeriod() throws IOException { @@ -192,11 +220,31 @@ public class S3BlobStoreRepositoryTests extends ESMockAPIBasedRepositoryIntegTes } @SuppressForbidden(reason = "this test uses a HttpHandler to emulate an S3 endpoint") - private static class S3BlobStoreHttpHandler extends S3HttpHandler implements BlobStoreHttpHandler { + private class S3BlobStoreHttpHandler extends S3HttpHandler implements BlobStoreHttpHandler { S3BlobStoreHttpHandler(final String bucket) { super(bucket); } + + @Override + public void handle(final HttpExchange exchange) throws IOException { + validateAuthHeader(exchange); + super.handle(exchange); + } + + private void validateAuthHeader(HttpExchange exchange) { + final String authorizationHeaderV4 = exchange.getRequestHeaders().getFirst("Authorization"); + final String authorizationHeaderV3 = exchange.getRequestHeaders().getFirst("X-amzn-authorization"); + + if ("AWS3SignerType".equals(signerOverride)) { + assertThat(authorizationHeaderV3, startsWith("AWS3")); + } else if ("AWS4SignerType".equals(signerOverride)) { + assertThat(authorizationHeaderV4, containsString("aws4_request")); + } + if (region != null && authorizationHeaderV4 != null) { + assertThat(authorizationHeaderV4, containsString("/" + region + "/s3/")); + } + } } /** diff --git a/plugins/repository-s3/src/test/java/org/elasticsearch/repositories/s3/S3ClientSettingsTests.java b/plugins/repository-s3/src/test/java/org/elasticsearch/repositories/s3/S3ClientSettingsTests.java index 9f18d158804..c0da7de3444 100644 --- a/plugins/repository-s3/src/test/java/org/elasticsearch/repositories/s3/S3ClientSettingsTests.java +++ b/plugins/repository-s3/src/test/java/org/elasticsearch/repositories/s3/S3ClientSettingsTests.java @@ -21,6 +21,7 @@ package org.elasticsearch.repositories.s3; import com.amazonaws.ClientConfiguration; import com.amazonaws.Protocol; +import com.amazonaws.services.s3.AmazonS3Client; import org.elasticsearch.cluster.metadata.RepositoryMetaData; import org.elasticsearch.common.settings.MockSecureSettings; import org.elasticsearch.common.settings.Settings; @@ -158,4 +159,28 @@ public class S3ClientSettingsTests extends ESTestCase { assertThat(settings.get("default").disableChunkedEncoding, is(false)); assertThat(settings.get("other").disableChunkedEncoding, is(true)); } + + public void testRegionCanBeSet() { + final String region = randomAlphaOfLength(5); + final Map settings = S3ClientSettings.load( + Settings.builder().put("s3.client.other.region", region).build()); + assertThat(settings.get("default").region, is("")); + assertThat(settings.get("other").region, is(region)); + try (S3Service s3Service = new S3Service()) { + AmazonS3Client other = (AmazonS3Client) s3Service.buildClient(settings.get("other")); + assertThat(other.getSignerRegionOverride(), is(region)); + } + } + + public void testSignerOverrideCanBeSet() { + final String signerOverride = randomAlphaOfLength(5); + final Map settings = S3ClientSettings.load( + Settings.builder().put("s3.client.other.signer_override", signerOverride).build()); + assertThat(settings.get("default").region, is("")); + assertThat(settings.get("other").signerOverride, is(signerOverride)); + ClientConfiguration defaultConfiguration = S3Service.buildConfiguration(settings.get("default")); + assertThat(defaultConfiguration.getSignerOverride(), nullValue()); + ClientConfiguration configuration = S3Service.buildConfiguration(settings.get("other")); + assertThat(configuration.getSignerOverride(), is(signerOverride)); + } }