diff --git a/plugins/repository-s3/build.gradle b/plugins/repository-s3/build.gradle index 4b996e8ec504e..dad3aecbc10e6 100644 --- a/plugins/repository-s3/build.gradle +++ b/plugins/repository-s3/build.gradle @@ -65,15 +65,8 @@ bundlePlugin { } } -task testRepositoryCreds(type: Test) { - include '**/RepositoryCredentialsTests.class' - systemProperty 'es.allow_insecure_settings', 'true' -} -check.dependsOn(testRepositoryCreds) - test { - // these are tested explicitly in separate test tasks - exclude '**/RepositoryCredentialsTests.class' + // this is tested explicitly in separate test tasks exclude '**/S3RepositoryThirdPartyTests.class' } 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 fee00786a2ab3..843208078b0e1 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 @@ -182,20 +182,14 @@ S3ClientSettings refine(RepositoryMetaData metadata) { final boolean usePathStyleAccess = getRepoSettingOrDefault(USE_PATH_STYLE_ACCESS, normalizedSettings, pathStyleAccess); final boolean newDisableChunkedEncoding = getRepoSettingOrDefault( DISABLE_CHUNKED_ENCODING, normalizedSettings, disableChunkedEncoding); - final S3BasicCredentials newCredentials; - if (checkDeprecatedCredentials(repoSettings)) { - newCredentials = loadDeprecatedCredentials(repoSettings); - } else { - newCredentials = credentials; - } if (Objects.equals(endpoint, newEndpoint) && protocol == newProtocol && Objects.equals(proxyHost, newProxyHost) && proxyPort == newProxyPort && newReadTimeoutMillis == readTimeoutMillis && maxRetries == newMaxRetries - && newThrottleRetries == throttleRetries && Objects.equals(credentials, newCredentials) + && newThrottleRetries == throttleRetries && newDisableChunkedEncoding == disableChunkedEncoding) { return this; } return new S3ClientSettings( - newCredentials, + credentials, newEndpoint, newProtocol, newProxyHost, @@ -229,29 +223,6 @@ static Map load(Settings settings) { return Collections.unmodifiableMap(clients); } - static boolean checkDeprecatedCredentials(Settings repositorySettings) { - if (S3Repository.ACCESS_KEY_SETTING.exists(repositorySettings)) { - if (S3Repository.SECRET_KEY_SETTING.exists(repositorySettings) == false) { - throw new IllegalArgumentException("Repository setting [" + S3Repository.ACCESS_KEY_SETTING.getKey() - + " must be accompanied by setting [" + S3Repository.SECRET_KEY_SETTING.getKey() + "]"); - } - return true; - } else if (S3Repository.SECRET_KEY_SETTING.exists(repositorySettings)) { - throw new IllegalArgumentException("Repository setting [" + S3Repository.SECRET_KEY_SETTING.getKey() - + " must be accompanied by setting [" + S3Repository.ACCESS_KEY_SETTING.getKey() + "]"); - } - return false; - } - - // backcompat for reading keys out of repository settings (clusterState) - private static S3BasicCredentials loadDeprecatedCredentials(Settings repositorySettings) { - assert checkDeprecatedCredentials(repositorySettings); - try (SecureString key = S3Repository.ACCESS_KEY_SETTING.get(repositorySettings); - SecureString secret = S3Repository.SECRET_KEY_SETTING.get(repositorySettings)) { - return new S3BasicCredentials(key.toString(), secret.toString()); - } - } - private static S3BasicCredentials loadCredentials(Settings settings, String clientName) { try (SecureString accessKey = getConfigValue(settings, clientName, ACCESS_KEY_SETTING); SecureString secretKey = getConfigValue(settings, clientName, SECRET_KEY_SETTING); diff --git a/plugins/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3Repository.java b/plugins/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3Repository.java index 72ff277ba4161..a164900b635e7 100644 --- a/plugins/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3Repository.java +++ b/plugins/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3Repository.java @@ -25,9 +25,6 @@ import org.elasticsearch.common.Strings; import org.elasticsearch.common.blobstore.BlobPath; import org.elasticsearch.common.blobstore.BlobStore; -import org.elasticsearch.common.logging.DeprecationLogger; -import org.elasticsearch.common.settings.SecureSetting; -import org.elasticsearch.common.settings.SecureString; import org.elasticsearch.common.settings.Setting; import org.elasticsearch.common.unit.ByteSizeUnit; import org.elasticsearch.common.unit.ByteSizeValue; @@ -54,16 +51,9 @@ */ class S3Repository extends BlobStoreRepository { private static final Logger logger = LogManager.getLogger(S3Repository.class); - private static final DeprecationLogger deprecationLogger = new DeprecationLogger(logger); static final String TYPE = "s3"; - /** The access key to authenticate with s3. This setting is insecure because cluster settings are stored in cluster state */ - static final Setting ACCESS_KEY_SETTING = SecureSetting.insecureString("access_key"); - - /** The secret key to authenticate with s3. This setting is insecure because cluster settings are stored in cluster state */ - static final Setting SECRET_KEY_SETTING = SecureSetting.insecureString("secret_key"); - /** * Default is to use 100MB (S3 defaults) for heaps above 2GB and 5% of * the available memory for smaller heaps. @@ -186,12 +176,6 @@ class S3Repository extends BlobStoreRepository { this.storageClass = STORAGE_CLASS_SETTING.get(metadata.settings()); this.cannedACL = CANNED_ACL_SETTING.get(metadata.settings()); - if (S3ClientSettings.checkDeprecatedCredentials(metadata.settings())) { - // provided repository settings - deprecationLogger.deprecated("Using s3 access/secret key from repository settings. Instead " - + "store these in named clients and the elasticsearch keystore for secure settings."); - } - logger.debug( "using bucket [{}], chunk_size [{}], server_side_encryption [{}], buffer_size [{}], cannedACL [{}], storageClass [{}]", bucket, 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 4bdbf482aeb6c..461eb9e9592cf 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 @@ -105,9 +105,7 @@ public List> getSettings() { S3ClientSettings.READ_TIMEOUT_SETTING, S3ClientSettings.MAX_RETRIES_SETTING, S3ClientSettings.USE_THROTTLE_RETRIES_SETTING, - S3ClientSettings.USE_PATH_STYLE_ACCESS, - S3Repository.ACCESS_KEY_SETTING, - S3Repository.SECRET_KEY_SETTING); + S3ClientSettings.USE_PATH_STYLE_ACCESS); } @Override diff --git a/plugins/repository-s3/src/main/plugin-metadata/plugin-security.policy b/plugins/repository-s3/src/main/plugin-metadata/plugin-security.policy index 5fd69b4c2fc3f..206335431f897 100644 --- a/plugins/repository-s3/src/main/plugin-metadata/plugin-security.policy +++ b/plugins/repository-s3/src/main/plugin-metadata/plugin-security.policy @@ -18,6 +18,7 @@ */ grant { + // needed because of problems in ClientConfiguration // TODO: get these fixed in aws sdk permission java.lang.RuntimePermission "accessDeclaredMembers"; @@ -38,6 +39,5 @@ grant { // s3 client opens socket connections for to access repository permission java.net.SocketPermission "*", "connect"; - // only for tests : org.elasticsearch.repositories.s3.S3RepositoryPlugin - permission java.util.PropertyPermission "es.allow_insecure_settings", "read,write"; + }; diff --git a/plugins/repository-s3/src/test/java/org/elasticsearch/repositories/s3/RepositoryCredentialsTests.java b/plugins/repository-s3/src/test/java/org/elasticsearch/repositories/s3/RepositoryCredentialsTests.java index 6695d3f2d61b3..f7d5ba021e25c 100644 --- a/plugins/repository-s3/src/test/java/org/elasticsearch/repositories/s3/RepositoryCredentialsTests.java +++ b/plugins/repository-s3/src/test/java/org/elasticsearch/repositories/s3/RepositoryCredentialsTests.java @@ -24,53 +24,28 @@ import com.amazonaws.services.s3.AmazonS3; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; -import org.elasticsearch.client.node.NodeClient; import org.elasticsearch.cluster.metadata.RepositoryMetaData; -import org.elasticsearch.common.SuppressForbidden; import org.elasticsearch.common.settings.MockSecureSettings; import org.elasticsearch.common.settings.Settings; -import org.elasticsearch.common.settings.SettingsFilter; import org.elasticsearch.common.xcontent.NamedXContentRegistry; import org.elasticsearch.plugins.Plugin; import org.elasticsearch.plugins.PluginsService; import org.elasticsearch.repositories.RepositoriesService; -import org.elasticsearch.rest.AbstractRestChannel; -import org.elasticsearch.rest.RestController; -import org.elasticsearch.rest.RestRequest; -import org.elasticsearch.rest.RestResponse; -import org.elasticsearch.rest.action.admin.cluster.RestGetRepositoriesAction; import org.elasticsearch.test.ESSingleNodeTestCase; -import org.elasticsearch.test.rest.FakeRestRequest; import org.elasticsearch.threadpool.ThreadPool; -import java.security.AccessController; -import java.security.PrivilegedAction; import java.util.Collection; import java.util.List; -import java.util.concurrent.CountDownLatch; -import java.util.concurrent.atomic.AtomicReference; import static org.elasticsearch.repositories.s3.S3ClientSettings.ACCESS_KEY_SETTING; import static org.elasticsearch.repositories.s3.S3ClientSettings.SECRET_KEY_SETTING; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked; -import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.instanceOf; import static org.hamcrest.Matchers.is; -import static org.hamcrest.Matchers.not; import static org.hamcrest.Matchers.notNullValue; -import static org.mockito.Mockito.mock; -@SuppressForbidden(reason = "test requires to set a System property to allow insecure settings when running in IDE") public class RepositoryCredentialsTests extends ESSingleNodeTestCase { - static { - AccessController.doPrivileged((PrivilegedAction) () -> { - // required for client settings overwriting when running in IDE - System.setProperty("es.allow_insecure_settings", "true"); - return null; - }); - } - @Override protected Collection> getPlugins() { return List.of(ProxyS3RepositoryPlugin.class); @@ -95,52 +70,11 @@ protected Settings nodeSettings() { .build(); } - public void testRepositoryCredentialsOverrideSecureCredentials() { - final String repositoryName = "repo-creds-override"; - final Settings.Builder repositorySettings = Settings.builder() - // repository settings for credentials override node secure settings - .put(S3Repository.ACCESS_KEY_SETTING.getKey(), "insecure_aws_key") - .put(S3Repository.SECRET_KEY_SETTING.getKey(), "insecure_aws_secret"); - - final String clientName = randomFrom("default", "other", null); - if (clientName != null) { - repositorySettings.put(S3Repository.CLIENT_NAME.getKey(), clientName); - } - createRepository(repositoryName, repositorySettings.build()); - - final RepositoriesService repositories = getInstanceFromNode(RepositoriesService.class); - assertThat(repositories.repository(repositoryName), notNullValue()); - assertThat(repositories.repository(repositoryName), instanceOf(S3Repository.class)); - - final S3Repository repository = (S3Repository) repositories.repository(repositoryName); - final AmazonS3 client = repository.createBlobStore().clientReference().client(); - assertThat(client, instanceOf(ProxyS3RepositoryPlugin.ClientAndCredentials.class)); - - final AWSCredentials credentials = ((ProxyS3RepositoryPlugin.ClientAndCredentials) client).credentials.getCredentials(); - assertThat(credentials.getAWSAccessKeyId(), is("insecure_aws_key")); - assertThat(credentials.getAWSSecretKey(), is("insecure_aws_secret")); - - assertWarnings( - "[secret_key] setting was deprecated in Elasticsearch and will be removed in a future release!" - + " See the breaking changes documentation for the next major version.", - "Using s3 access/secret key from repository settings. Instead store these in named clients and" - + " the elasticsearch keystore for secure settings.", - "[access_key] setting was deprecated in Elasticsearch and will be removed in a future release!" - + " See the breaking changes documentation for the next major version."); - } - public void testReinitSecureCredentials() { final String clientName = randomFrom("default", "other"); final Settings.Builder repositorySettings = Settings.builder(); - final boolean hasInsecureSettings = randomBoolean(); - if (hasInsecureSettings) { - // repository settings for credentials override node secure settings - repositorySettings.put(S3Repository.ACCESS_KEY_SETTING.getKey(), "insecure_aws_key"); - repositorySettings.put(S3Repository.SECRET_KEY_SETTING.getKey(), "insecure_aws_secret"); - } else { - repositorySettings.put(S3Repository.CLIENT_NAME.getKey(), clientName); - } + repositorySettings.put(S3Repository.CLIENT_NAME.getKey(), clientName); final String repositoryName = "repo-reinit-creds"; createRepository(repositoryName, repositorySettings.build()); @@ -155,10 +89,7 @@ public void testReinitSecureCredentials() { assertThat(client, instanceOf(ProxyS3RepositoryPlugin.ClientAndCredentials.class)); final AWSCredentials credentials = ((ProxyS3RepositoryPlugin.ClientAndCredentials) client).credentials.getCredentials(); - if (hasInsecureSettings) { - assertThat(credentials.getAWSAccessKeyId(), is("insecure_aws_key")); - assertThat(credentials.getAWSSecretKey(), is("insecure_aws_secret")); - } else if ("other".equals(clientName)) { + if ("other".equals(clientName)) { assertThat(credentials.getAWSAccessKeyId(), is("secure_other_key")); assertThat(credentials.getAWSSecretKey(), is("secure_other_secret")); } else { @@ -177,10 +108,7 @@ public void testReinitSecureCredentials() { plugin.reload(newSettings); // check the not-yet-closed client reference still has the same credentials - if (hasInsecureSettings) { - assertThat(credentials.getAWSAccessKeyId(), is("insecure_aws_key")); - assertThat(credentials.getAWSSecretKey(), is("insecure_aws_secret")); - } else if ("other".equals(clientName)) { + if ("other".equals(clientName)) { assertThat(credentials.getAWSAccessKeyId(), is("secure_other_key")); assertThat(credentials.getAWSSecretKey(), is("secure_other_secret")); } else { @@ -195,64 +123,11 @@ public void testReinitSecureCredentials() { assertThat(client, instanceOf(ProxyS3RepositoryPlugin.ClientAndCredentials.class)); final AWSCredentials newCredentials = ((ProxyS3RepositoryPlugin.ClientAndCredentials) client).credentials.getCredentials(); - if (hasInsecureSettings) { - assertThat(newCredentials.getAWSAccessKeyId(), is("insecure_aws_key")); - assertThat(newCredentials.getAWSSecretKey(), is("insecure_aws_secret")); - } else { - assertThat(newCredentials.getAWSAccessKeyId(), is("new_secret_aws_key")); - assertThat(newCredentials.getAWSSecretKey(), is("new_secret_aws_secret")); - } - } - - if (hasInsecureSettings) { - assertWarnings( - "[secret_key] setting was deprecated in Elasticsearch and will be removed in a future release!" - + " See the breaking changes documentation for the next major version.", - "Using s3 access/secret key from repository settings. Instead store these in named clients and" - + " the elasticsearch keystore for secure settings.", - "[access_key] setting was deprecated in Elasticsearch and will be removed in a future release!" - + " See the breaking changes documentation for the next major version."); + assertThat(newCredentials.getAWSAccessKeyId(), is("new_secret_aws_key")); + assertThat(newCredentials.getAWSSecretKey(), is("new_secret_aws_secret")); } } - public void testInsecureRepositoryCredentials() throws Exception { - final String repositoryName = "repo-insecure-creds"; - createRepository(repositoryName, Settings.builder() - .put(S3Repository.ACCESS_KEY_SETTING.getKey(), "insecure_aws_key") - .put(S3Repository.SECRET_KEY_SETTING.getKey(), "insecure_aws_secret") - .build()); - - final RestRequest fakeRestRequest = new FakeRestRequest(); - fakeRestRequest.params().put("repository", repositoryName); - final RestGetRepositoriesAction action = - new RestGetRepositoriesAction(mock(RestController.class), getInstanceFromNode(SettingsFilter.class)); - - final CountDownLatch latch = new CountDownLatch(1); - final AtomicReference error = new AtomicReference<>(); - action.handleRequest(fakeRestRequest, new AbstractRestChannel(fakeRestRequest, true) { - @Override - public void sendResponse(RestResponse response) { - try { - String responseAsString = response.content().utf8ToString(); - assertThat(responseAsString, containsString(repositoryName)); - assertThat(responseAsString, not(containsString("insecure_"))); - } catch (final AssertionError ex) { - error.set(ex); - } - latch.countDown(); - } - }, getInstanceFromNode(NodeClient.class)); - - latch.await(); - if (error.get() != null) { - throw error.get(); - } - - assertWarnings( - "Using s3 access/secret key from repository settings. Instead store these in named clients and" - + " the elasticsearch keystore for secure settings."); - } - private void createRepository(final String name, final Settings repositorySettings) { assertAcked(client().admin().cluster().preparePutRepository(name) .setType(S3Repository.TYPE) diff --git a/server/src/main/java/org/elasticsearch/common/settings/SecureSetting.java b/server/src/main/java/org/elasticsearch/common/settings/SecureSetting.java index e022e4e3760a5..9f902febc7730 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/SecureSetting.java +++ b/server/src/main/java/org/elasticsearch/common/settings/SecureSetting.java @@ -19,14 +19,13 @@ package org.elasticsearch.common.settings; +import org.elasticsearch.common.util.ArrayUtils; + import java.io.InputStream; import java.security.GeneralSecurityException; import java.util.EnumSet; import java.util.Set; -import org.elasticsearch.common.Booleans; -import org.elasticsearch.common.util.ArrayUtils; - /** * A secure setting. * @@ -34,9 +33,6 @@ */ public abstract class SecureSetting extends Setting { - /** Determines whether legacy settings with sensitive values should be allowed. */ - private static final boolean ALLOW_INSECURE_SETTINGS = Booleans.parseBoolean(System.getProperty("es.allow_insecure_settings", "false")); - private static final Set ALLOWED_PROPERTIES = EnumSet.of(Property.Deprecated, Property.Consistent); private static final Property[] FIXED_PROPERTIES = { @@ -139,14 +135,6 @@ public static Setting secureString(String name, Setting insecureString(String name) { - return new InsecureStringSetting(name); - } - /** * A setting which contains a file. Reading the setting opens an input stream to the file. * @@ -179,24 +167,6 @@ SecureString getFallback(Settings settings) { } } - private static class InsecureStringSetting extends Setting { - private final String name; - - private InsecureStringSetting(String name) { - super(name, "", SecureString::new, Property.Deprecated, Property.Filtered, Property.NodeScope); - this.name = name; - } - - @Override - public SecureString get(Settings settings) { - if (ALLOW_INSECURE_SETTINGS == false && exists(settings)) { - throw new IllegalArgumentException("Setting [" + name + "] is insecure, " + - "but property [allow_insecure_settings] is not set"); - } - return super.get(settings); - } - } - private static class SecureFileSetting extends SecureSetting { private final Setting fallback;