Improve message about insecure S3 settings (#116915)

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.

Author: DaveCTurner

docs/changelog/116915.yaml

@@ -0,0 +1,5 @@
+pr: 116915
+summary: Improve message about insecure S3 settings
+area: Snapshot/Restore
+type: enhancement
+issues: []

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

@@ -318,8 +318,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
             );
         }
 
@@ -336,6 +335,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()));
     }

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

@@ -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
         );
     }
 
@@ -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
             );
         }
     }
@@ -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) {

server/src/main/java/org/elasticsearch/common/ReferenceDocs.java

@@ -82,6 +82,7 @@ public enum ReferenceDocs {
     CIRCUIT_BREAKER_ERRORS,
     ALLOCATION_EXPLAIN_NO_COPIES,
     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
     ;
 

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

@@ -185,9 +185,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);
         }

server/src/main/resources/org/elasticsearch/common/reference-docs-links.txt

@@ -44,3 +44,4 @@ FORMING_SINGLE_NODE_CLUSTERS                                    modules-discover
 CIRCUIT_BREAKER_ERRORS                                          circuit-breaker-errors.html
 ALLOCATION_EXPLAIN_NO_COPIES                                    cluster-allocation-explain.html#no-valid-shard-copy
 ALLOCATION_EXPLAIN_MAX_RETRY                                    cluster-allocation-explain.html#maximum-number-of-retries-exceeded
+SECURE_SETTINGS                                                 secure-settings.html