Skip to content

Commit db8aecf

Browse files
authored
Improve message about insecure S3 settings (#116956)
* Improve message about insecure S3 settings Clarifies that insecure settings are stored in plaintext and must not be used. Also removes the mention of the (wrong) system property from the error message if insecure settings are not permitted. Backport of #116915 to `7.17` * CI poke
1 parent 3fa1e0a commit db8aecf

File tree

4 files changed

+18
-17
lines changed

4 files changed

+18
-17
lines changed

docs/changelog/116915.yaml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
pr: 116915
2+
summary: Improve message about insecure S3 settings
3+
area: Snapshot/Restore
4+
type: enhancement
5+
issues: []

plugins/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3Repository.java

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -267,8 +267,7 @@ class S3Repository extends MeteredBlobStoreRepository {
267267
deprecationLogger.critical(
268268
DeprecationCategory.SECURITY,
269269
"s3_repository_secret_settings",
270-
"Using s3 access/secret key from repository settings. Instead "
271-
+ "store these in named clients and the elasticsearch keystore for secure settings."
270+
INSECURE_CREDENTIALS_DEPRECATION_WARNING
272271
);
273272
}
274273

@@ -285,6 +284,10 @@ class S3Repository extends MeteredBlobStoreRepository {
285284
);
286285
}
287286

287+
static final String INSECURE_CREDENTIALS_DEPRECATION_WARNING =
288+
"This repository's settings include a S3 access key and secret key, but repository settings are stored in plaintext and must not "
289+
+ "be used for security-sensitive information. Instead, store all secure settings in the keystore.";
290+
288291
private static Map<String, String> buildLocation(RepositoryMetadata metadata) {
289292
return org.elasticsearch.core.Map.of(
290293
"base_path",

plugins/repository-s3/src/test/java/org/elasticsearch/repositories/s3/RepositoryCredentialsTests.java

Lines changed: 7 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -108,12 +108,11 @@ public void testRepositoryCredentialsOverrideSecureCredentials() {
108108
assertThat(credentials.getAWSSecretKey(), is("insecure_aws_secret"));
109109

110110
assertWarnings(
111+
"[access_key] setting was deprecated in Elasticsearch and will be removed in a future release!"
112+
+ " See the breaking changes documentation for the next major version.",
111113
"[secret_key] setting was deprecated in Elasticsearch and will be removed in a future release!"
112114
+ " See the breaking changes documentation for the next major version.",
113-
"Using s3 access/secret key from repository settings. Instead store these in named clients and"
114-
+ " the elasticsearch keystore for secure settings.",
115-
"[access_key] setting was deprecated in Elasticsearch and will be removed in a future release!"
116-
+ " See the breaking changes documentation for the next major version."
115+
S3Repository.INSECURE_CREDENTIALS_DEPRECATION_WARNING
117116
);
118117
}
119118

@@ -194,12 +193,11 @@ public void testReinitSecureCredentials() {
194193

195194
if (hasInsecureSettings) {
196195
assertWarnings(
196+
"[access_key] setting was deprecated in Elasticsearch and will be removed in a future release!"
197+
+ " See the breaking changes documentation for the next major version.",
197198
"[secret_key] setting was deprecated in Elasticsearch and will be removed in a future release!"
198199
+ " See the breaking changes documentation for the next major version.",
199-
"Using s3 access/secret key from repository settings. Instead store these in named clients and"
200-
+ " the elasticsearch keystore for secure settings.",
201-
"[access_key] setting was deprecated in Elasticsearch and will be removed in a future release!"
202-
+ " See the breaking changes documentation for the next major version."
200+
S3Repository.INSECURE_CREDENTIALS_DEPRECATION_WARNING
203201
);
204202
}
205203
}
@@ -239,10 +237,7 @@ public void sendResponse(RestResponse response) {
239237
throw error.get();
240238
}
241239

242-
assertWarnings(
243-
"Using s3 access/secret key from repository settings. Instead store these in named clients and"
244-
+ " the elasticsearch keystore for secure settings."
245-
);
240+
assertWarnings(S3Repository.INSECURE_CREDENTIALS_DEPRECATION_WARNING);
246241
}
247242

248243
private void createRepository(final String name, final Settings repositorySettings) {

server/src/main/java/org/elasticsearch/common/settings/SecureSetting.java

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -182,9 +182,7 @@ private InsecureStringSetting(String name) {
182182
@Override
183183
public SecureString get(Settings settings) {
184184
if (ALLOW_INSECURE_SETTINGS == false && exists(settings)) {
185-
throw new IllegalArgumentException(
186-
"Setting [" + name + "] is insecure, " + "but property [allow_insecure_settings] is not set"
187-
);
185+
throw new IllegalArgumentException("Setting [" + name + "] is insecure, use the elasticsearch keystore instead");
188186
}
189187
return super.get(settings);
190188
}

0 commit comments

Comments
 (0)