diff --git a/core/src/main/java/org/elasticsearch/common/settings/SecureSetting.java b/core/src/main/java/org/elasticsearch/common/settings/SecureSetting.java index e93240b9a32ba..7b8f8bbdff3ab 100644 --- a/core/src/main/java/org/elasticsearch/common/settings/SecureSetting.java +++ b/core/src/main/java/org/elasticsearch/common/settings/SecureSetting.java @@ -24,25 +24,25 @@ import java.util.EnumSet; import java.util.Set; +import org.elasticsearch.common.Booleans; import org.elasticsearch.common.util.ArrayUtils; - /** * A secure setting. * * This class allows access to settings from the Elasticsearch keystore. */ 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.Shared); private static final Property[] FIXED_PROPERTIES = { Property.NodeScope }; - private static final Property[] LEGACY_PROPERTIES = { - Property.NodeScope, Property.Deprecated, Property.Filtered - }; - private SecureSetting(String key, Property... properties) { super(key, (String)null, null, ArrayUtils.concat(properties, FIXED_PROPERTIES, Property.class)); assert assertAllowedProperties(properties); @@ -133,6 +133,23 @@ SecureString getFallback(Settings settings) { }; } + /** + * A setting which contains a sensitive string, but which for legacy reasons must be found outside secure settings. + * @see #secureString(String, Setting, Property...) + */ + public static Setting insecureString(String name) { + return new Setting(name, "", SecureString::new, Property.Deprecated, Property.Filtered, Property.NodeScope) { + @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); + } + }; + } + /** * A setting which contains a file. Reading the setting opens an input stream to the file. * diff --git a/plugins/repository-s3/build.gradle b/plugins/repository-s3/build.gradle index 50f2ac571a424..9db78e5cbb2ca 100644 --- a/plugins/repository-s3/build.gradle +++ b/plugins/repository-s3/build.gradle @@ -54,6 +54,16 @@ bundlePlugin { } } +additionalTest('testRepositoryCreds'){ + include '**/RepositorySettingsCredentialsTests.class' + systemProperty 'es.allow_insecure_settings', 'true' +} + +test { + // these are tested explicitly in separate test tasks + exclude '**/*CredentialsTests.class' +} + integTestCluster { keystoreSetting 's3.client.default.access_key', 'myaccesskey' keystoreSetting 's3.client.default.secret_key', 'mysecretkey' diff --git a/plugins/repository-s3/src/main/java/org/elasticsearch/repositories/s3/InternalAwsS3Service.java b/plugins/repository-s3/src/main/java/org/elasticsearch/repositories/s3/InternalAwsS3Service.java index c76280f116ca8..58a130bcd95c2 100644 --- a/plugins/repository-s3/src/main/java/org/elasticsearch/repositories/s3/InternalAwsS3Service.java +++ b/plugins/repository-s3/src/main/java/org/elasticsearch/repositories/s3/InternalAwsS3Service.java @@ -26,6 +26,7 @@ import com.amazonaws.ClientConfiguration; import com.amazonaws.auth.AWSCredentials; import com.amazonaws.auth.AWSCredentialsProvider; +import com.amazonaws.auth.BasicAWSCredentials; import com.amazonaws.auth.InstanceProfileCredentialsProvider; import com.amazonaws.http.IdleConnectionReaper; import com.amazonaws.internal.StaticCredentialsProvider; @@ -35,6 +36,8 @@ import org.elasticsearch.ElasticsearchException; import org.elasticsearch.common.Strings; import org.elasticsearch.common.component.AbstractLifecycleComponent; +import org.elasticsearch.common.logging.DeprecationLogger; +import org.elasticsearch.common.settings.SecureString; import org.elasticsearch.common.settings.Setting; import org.elasticsearch.common.settings.Settings; @@ -69,7 +72,7 @@ public synchronized AmazonS3 client(Settings repositorySettings) { logger.debug("creating S3 client with client_name [{}], endpoint [{}]", clientName, clientSettings.endpoint); - AWSCredentialsProvider credentials = buildCredentials(logger, clientSettings); + AWSCredentialsProvider credentials = buildCredentials(logger, deprecationLogger, clientSettings, repositorySettings); ClientConfiguration configuration = buildConfiguration(clientSettings, repositorySettings); client = new AmazonS3Client(credentials, configuration); @@ -106,14 +109,42 @@ static ClientConfiguration buildConfiguration(S3ClientSettings clientSettings, S } // pkg private for tests - static AWSCredentialsProvider buildCredentials(Logger logger, S3ClientSettings clientSettings) { - if (clientSettings.credentials == null) { + static AWSCredentialsProvider buildCredentials(Logger logger, DeprecationLogger deprecationLogger, + S3ClientSettings clientSettings, Settings repositorySettings) { + + + BasicAWSCredentials credentials = clientSettings.credentials; + 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() + "]"); + } + try (SecureString key = S3Repository.ACCESS_KEY_SETTING.get(repositorySettings); + SecureString secret = S3Repository.SECRET_KEY_SETTING.get(repositorySettings)) { + credentials = new BasicAWSCredentials(key.toString(), secret.toString()); + } + // backcompat for reading keys out of 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."); + } 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() + "]"); + } + if (credentials == null) { logger.debug("Using instance profile credentials"); return new PrivilegedInstanceProfileCredentialsProvider(); } else { logger.debug("Using basic key/secret credentials"); - return new StaticCredentialsProvider(clientSettings.credentials); + return new StaticCredentialsProvider(credentials); + } + } + + /** Returns the value for a given setting from the repository, or returns the fallback value. */ + private static T getRepoValue(Settings repositorySettings, Setting repositorySetting, T fallback) { + if (repositorySetting.exists(repositorySettings)) { + return repositorySetting.get(repositorySettings); } + return fallback; } @Override 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 b73848eed600f..eeca906ff4998 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 @@ -21,14 +21,14 @@ import java.io.IOException; -import com.amazonaws.ClientConfiguration; import com.amazonaws.services.s3.AmazonS3; import org.elasticsearch.cluster.metadata.RepositoryMetaData; import org.elasticsearch.common.Strings; import org.elasticsearch.common.blobstore.BlobPath; import org.elasticsearch.common.blobstore.BlobStore; +import org.elasticsearch.common.settings.SecureSetting; +import org.elasticsearch.common.settings.SecureString; import org.elasticsearch.common.settings.Setting; -import org.elasticsearch.common.settings.Setting.Property; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.unit.ByteSizeUnit; import org.elasticsearch.common.unit.ByteSizeValue; @@ -53,6 +53,12 @@ class S3Repository extends BlobStoreRepository { 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. diff --git a/plugins/repository-s3/src/test/java/org/elasticsearch/repositories/s3/AwsS3ServiceImplTests.java b/plugins/repository-s3/src/test/java/org/elasticsearch/repositories/s3/AwsS3ServiceImplTests.java index 85398571dc1d1..9863402ec37c1 100644 --- a/plugins/repository-s3/src/test/java/org/elasticsearch/repositories/s3/AwsS3ServiceImplTests.java +++ b/plugins/repository-s3/src/test/java/org/elasticsearch/repositories/s3/AwsS3ServiceImplTests.java @@ -24,10 +24,12 @@ import com.amazonaws.auth.AWSCredentials; import com.amazonaws.auth.AWSCredentialsProvider; import org.elasticsearch.common.settings.MockSecureSettings; +import org.elasticsearch.common.settings.SecureSetting; import org.elasticsearch.common.settings.Setting; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.test.ESTestCase; +import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.instanceOf; import static org.hamcrest.Matchers.is; @@ -35,7 +37,8 @@ public class AwsS3ServiceImplTests extends ESTestCase { public void testAWSCredentialsWithSystemProviders() { S3ClientSettings clientSettings = S3ClientSettings.getClientSettings(Settings.EMPTY, "default"); - AWSCredentialsProvider credentialsProvider = InternalAwsS3Service.buildCredentials(logger, clientSettings); + AWSCredentialsProvider credentialsProvider = + InternalAwsS3Service.buildCredentials(logger, deprecationLogger, clientSettings, Settings.EMPTY); assertThat(credentialsProvider, instanceOf(InternalAwsS3Service.PrivilegedInstanceProfileCredentialsProvider.class)); } @@ -44,7 +47,7 @@ public void testAwsCredsDefaultSettings() { secureSettings.setString("s3.client.default.access_key", "aws_key"); secureSettings.setString("s3.client.default.secret_key", "aws_secret"); Settings settings = Settings.builder().setSecureSettings(secureSettings).build(); - launchAWSCredentialsWithElasticsearchSettingsTest(Settings.EMPTY, settings, "aws_key", "aws_secret"); + assertCredentials(Settings.EMPTY, settings, "aws_key", "aws_secret"); } public void testAwsCredsExplicitConfigSettings() { @@ -55,14 +58,38 @@ public void testAwsCredsExplicitConfigSettings() { secureSettings.setString("s3.client.default.access_key", "wrong_key"); secureSettings.setString("s3.client.default.secret_key", "wrong_secret"); Settings settings = Settings.builder().setSecureSettings(secureSettings).build(); - launchAWSCredentialsWithElasticsearchSettingsTest(repositorySettings, settings, "aws_key", "aws_secret"); + assertCredentials(repositorySettings, settings, "aws_key", "aws_secret"); } - private void launchAWSCredentialsWithElasticsearchSettingsTest(Settings singleRepositorySettings, Settings settings, - String expectedKey, String expectedSecret) { + public void testRepositorySettingsCredentialsDisallowed() { + Settings repositorySettings = Settings.builder() + .put(S3Repository.ACCESS_KEY_SETTING.getKey(), "aws_key") + .put(S3Repository.SECRET_KEY_SETTING.getKey(), "aws_secret").build(); + IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> + assertCredentials(repositorySettings, Settings.EMPTY, "aws_key", "aws_secret")); + assertThat(e.getMessage(), containsString("Setting [access_key] is insecure")); + } + + public void testRepositorySettingsCredentialsMissingKey() { + Settings repositorySettings = Settings.builder().put(S3Repository.SECRET_KEY_SETTING.getKey(), "aws_secret").build(); + IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> + assertCredentials(repositorySettings, Settings.EMPTY, "aws_key", "aws_secret")); + assertThat(e.getMessage(), containsString("must be accompanied by setting [access_key]")); + } + + public void testRepositorySettingsCredentialsMissingSecret() { + Settings repositorySettings = Settings.builder().put(S3Repository.ACCESS_KEY_SETTING.getKey(), "aws_key").build(); + IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> + assertCredentials(repositorySettings, Settings.EMPTY, "aws_key", "aws_secret")); + assertThat(e.getMessage(), containsString("must be accompanied by setting [secret_key]")); + } + + private void assertCredentials(Settings singleRepositorySettings, Settings settings, + String expectedKey, String expectedSecret) { String configName = InternalAwsS3Service.CLIENT_NAME.get(singleRepositorySettings); S3ClientSettings clientSettings = S3ClientSettings.getClientSettings(settings, configName); - AWSCredentials credentials = InternalAwsS3Service.buildCredentials(logger, clientSettings).getCredentials(); + AWSCredentials credentials = InternalAwsS3Service.buildCredentials(logger, deprecationLogger, + clientSettings, singleRepositorySettings).getCredentials(); assertThat(credentials.getAWSAccessKeyId(), is(expectedKey)); assertThat(credentials.getAWSSecretKey(), is(expectedSecret)); } diff --git a/plugins/repository-s3/src/test/java/org/elasticsearch/repositories/s3/RepositorySettingsCredentialsTests.java b/plugins/repository-s3/src/test/java/org/elasticsearch/repositories/s3/RepositorySettingsCredentialsTests.java new file mode 100644 index 0000000000000..c3e7069fdfd65 --- /dev/null +++ b/plugins/repository-s3/src/test/java/org/elasticsearch/repositories/s3/RepositorySettingsCredentialsTests.java @@ -0,0 +1,41 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License 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 org.elasticsearch.repositories.s3; + +import com.amazonaws.auth.AWSCredentials; +import org.elasticsearch.common.settings.Setting; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.test.ESTestCase; + +public class RepositorySettingsCredentialsTests extends ESTestCase { + + public void testRepositorySettingsCredentials() { + Settings repositorySettings = Settings.builder() + .put(S3Repository.ACCESS_KEY_SETTING.getKey(), "aws_key") + .put(S3Repository.SECRET_KEY_SETTING.getKey(), "aws_secret").build(); + AWSCredentials credentials = InternalAwsS3Service.buildCredentials(logger, deprecationLogger, + S3ClientSettings.getClientSettings(Settings.EMPTY, "default"), repositorySettings).getCredentials(); + assertEquals("aws_key", credentials.getAWSAccessKeyId()); + assertEquals("aws_secret", credentials.getAWSSecretKey()); + assertSettingDeprecationsAndWarnings(new Setting[] { S3Repository.ACCESS_KEY_SETTING, S3Repository.SECRET_KEY_SETTING }, + "Using s3 access/secret key from repository settings. " + + "Instead store these in named clients and the elasticsearch keystore for secure settings."); + } +}