Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions docs/changelog/116915.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
pr: 116915
summary: Improve message about insecure S3 settings
area: Snapshot/Restore
type: enhancement
issues: []
Original file line number Diff line number Diff line change
Expand Up @@ -288,8 +288,7 @@ class S3Repository extends MeteredBlobStoreRepository {
deprecationLogger.critical(
DeprecationCategory.SECURITY,
"s3_repository_secret_settings",
"Using s3 access/secret key from repository settings. Instead "
+ "store these in named clients and the elasticsearch keystore for secure settings."
INSECURE_CREDENTIALS_DEPRECATION_WARNING
);
}

Expand All @@ -306,6 +305,11 @@ class S3Repository extends MeteredBlobStoreRepository {
);
}

static final String INSECURE_CREDENTIALS_DEPRECATION_WARNING = Strings.format("""
This repository's settings include a S3 access key and secret key, but repository settings are stored in plaintext and must not be \
used for security-sensitive information. Instead, store all secure settings in the keystore. See [%s] for more information.\
""", ReferenceDocs.SECURE_SETTINGS);

private static Map<String, String> buildLocation(RepositoryMetadata metadata) {
return Map.of("base_path", BASE_PATH_SETTING.get(metadata.settings()), "bucket", BUCKET_SETTING.get(metadata.settings()));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,10 +107,9 @@ public void testRepositoryCredentialsOverrideSecureCredentials() {
assertThat(credentials.getAWSSecretKey(), is("insecure_aws_secret"));

assertCriticalWarnings(
"[access_key] setting was deprecated in Elasticsearch and will be removed in a future release.",
"[secret_key] setting was deprecated in Elasticsearch and will be removed in a future release.",
"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."
S3Repository.INSECURE_CREDENTIALS_DEPRECATION_WARNING
);
}

Expand Down Expand Up @@ -194,10 +193,9 @@ public void testReinitSecureCredentials() {

if (hasInsecureSettings) {
assertCriticalWarnings(
"[access_key] setting was deprecated in Elasticsearch and will be removed in a future release.",
"[secret_key] setting was deprecated in Elasticsearch and will be removed in a future release.",
"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."
S3Repository.INSECURE_CREDENTIALS_DEPRECATION_WARNING
);
}
}
Expand Down Expand Up @@ -238,10 +236,7 @@ public void sendResponse(RestResponse response) {
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."
);
assertWarnings(S3Repository.INSECURE_CREDENTIALS_DEPRECATION_WARNING);
}

private void createRepository(final String name, final Settings repositorySettings) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ public enum ReferenceDocs {
FORMING_SINGLE_NODE_CLUSTERS,
JDK_LOCALE_DIFFERENCES,
ALLOCATION_EXPLAIN_MAX_RETRY,
SECURE_SETTINGS,
// this comment keeps the ';' on the next line so every entry above has a trailing ',' which makes the diff for adding new links cleaner
;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -192,9 +192,7 @@ private InsecureStringSetting(String 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"
);
throw new IllegalArgumentException("Setting [" + name + "] is insecure, use the elasticsearch keystore instead");
}
return super.get(settings);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,3 +44,4 @@ X_OPAQUE_ID api-conventions.
FORMING_SINGLE_NODE_CLUSTERS modules-discovery-bootstrap-cluster.html#modules-discovery-bootstrap-cluster-joining
JDK_LOCALE_DIFFERENCES mapping-date-format.html#custom-date-format-locales
ALLOCATION_EXPLAIN_MAX_RETRY cluster-allocation-explain.html#maximum-number-of-retries-exceeded
SECURE_SETTINGS secure-settings.html