Reject misconfigured/ambiguous SSL server config (#45892)

This commit makes it an error to start a node where either of the
server contexts (xpack.security.transport.ssl and
xpack.security.http.ssl) meet either of these conditions:

1. The server lacks a certificate/key pair (i.e. neither
   ssl.keystore.path not ssl.certificate are configured)
2. The server has some ssl configuration, but ssl.enabled is not
   specified. This new validation does not care whether ssl.enabled is
   true or false (though other validation might), it simply makes it
   an error to configure server SSL without being explicit about
   whether to enable that configuration.

Author: tvernum

client/rest-high-level/build.gradle

@@ -126,6 +126,8 @@ testClusters.all {
   setting 'xpack.security.enabled', 'true'
   setting 'xpack.security.authc.token.enabled', 'true'
   setting 'xpack.security.authc.api_key.enabled', 'true'
+  setting 'xpack.security.http.ssl.enabled', 'false'
+  setting 'xpack.security.transport.ssl.enabled', 'false'
   // Truststore settings are not used since TLS is not enabled. Included for testing the get certificates API
   setting 'xpack.security.http.ssl.certificate_authorities', 'testnode.crt'
   setting 'xpack.security.transport.ssl.truststore.path', 'testnode.jks'

docs/reference/migration/migrate_8_0/security.asciidoc

@@ -41,3 +41,73 @@ realm directly.
 The `transport.profiles.*.xpack.security.type` setting has been removed since
 the Transport Client has been removed and therefore all client traffic now uses
 the HTTP transport. Transport profiles using this setting should be removed.
+
+[float]
+[[ssl-validation-changes]]
+==== SSL/TLS configuration validation
+
+[float]
+===== The `xpack.security.transport.ssl.enabled` setting may be required
+
+It is now an error to configure any SSL settings for
+`xpack.security.transport.ssl` without also configuring
+`xpack.security.transport.ssl.enabled`.
+
+For example, the following configuration is invalid:
+[source,yaml]
+--------------------------------------------------
+xpack.security.transport.ssl.keystore.path: elastic-certificates.p12
+xpack.security.transport.ssl.truststore.path: elastic-certificates.p12
+--------------------------------------------------
+
+And must be configured as:
+[source,yaml]
+--------------------------------------------------
+xpack.security.transport.ssl.enabled: true <1>
+xpack.security.transport.ssl.keystore.path: elastic-certificates.p12
+xpack.security.transport.ssl.truststore.path: elastic-certificates.p12
+--------------------------------------------------
+<1> or `false`.
+
+[float]
+===== The `xpack.security.http.ssl.enabled` setting may be required
+
+It is now an error to configure any SSL settings for
+`xpack.security.http.ssl` without also configuring
+`xpack.security.http.ssl.enabled`.
+
+For example, the following configuration is invalid:
+[source,yaml]
+--------------------------------------------------
+xpack.security.http.ssl.certificate: elasticsearch.crt 
+xpack.security.http.ssl.key: elasticsearch.key 
+xpack.security.http.ssl.certificate_authorities: [ "corporate-ca.crt" ]
+--------------------------------------------------
+
+And must be configured as either:
+[source,yaml]
+--------------------------------------------------
+xpack.security.http.ssl.enabled: true <1>
+xpack.security.http.ssl.certificate: elasticsearch.crt 
+xpack.security.http.ssl.key: elasticsearch.key 
+xpack.security.http.ssl.certificate_authorities: [ "corporate-ca.crt" ]
+--------------------------------------------------
+<1> or `false`.
+
+[float]
+===== The `xpack.security.transport.ssl` Certificate and Key may be required
+
+It is now an error to enable SSL for the transport interface without also configuring
+a certificate and key through use of the `xpack.security.transport.ssl.keystore.path`
+setting or the `xpack.security.transport.ssl.certificate` and
+`xpack.security.transport.ssl.key` settings.
+
+[float]
+===== The `xpack.security.http.ssl` Certificate and Key may be required
+
+It is now an error to enable SSL for the HTTP (Rest) server without also configuring
+a certificate and key through use of the `xpack.security.http.ssl.keystore.path`
+setting or the `xpack.security.http.ssl.certificate` and
+`xpack.security.http.ssl.key` settings.
+
+

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ssl/SSLService.java

@@ -33,7 +33,6 @@
 import javax.net.ssl.TrustManagerFactory;
 import javax.net.ssl.X509ExtendedKeyManager;
 import javax.net.ssl.X509ExtendedTrustManager;
-
 import java.io.IOException;
 import java.net.InetAddress;
 import java.net.Socket;
@@ -428,6 +427,10 @@ Map<SSLConfiguration, SSLContextHolder> loadSSLConfigurations() {
         Map<String, Settings> profileSettings = getTransportProfileSSLSettings(settings);
         profileSettings.forEach((key, profileSetting) -> loadConfiguration(key, profileSetting, sslContextHolders));
 
+        for (String context : List.of("xpack.security.transport.ssl", "xpack.security.http.ssl")) {
+            validateServerConfiguration(context);
+        }
+
         return Collections.unmodifiableMap(sslContextHolders);
     }
 
@@ -446,6 +449,33 @@ private SSLConfiguration loadConfiguration(String key, Settings settings, Map<SS
         }
     }
 
+    private void validateServerConfiguration(String prefix) {
+        assert prefix.endsWith(".ssl");
+        SSLConfiguration configuration = getSSLConfiguration(prefix);
+        final String enabledSetting = prefix + ".enabled";
+        if (settings.getAsBoolean(enabledSetting, false) == true) {
+            // Client Authentication _should_ be required, but if someone turns it off, then this check is no longer relevant
+            final SSLConfigurationSettings configurationSettings = SSLConfigurationSettings.withPrefix(prefix + ".");
+            if (isConfigurationValidForServerUsage(configuration) == false) {
+                throw new ElasticsearchSecurityException("invalid SSL configuration for " + prefix +
+                    " - server ssl configuration requires a key and certificate, but these have not been configured; you must set either " +
+                    "[" + configurationSettings.x509KeyPair.keystorePath.getKey() + "], or both [" +
+                    configurationSettings.x509KeyPair.keyPath.getKey() + "] and [" +
+                    configurationSettings.x509KeyPair.certificatePath.getKey() + "]");
+            }
+        } else if (settings.hasValue(enabledSetting) == false) {
+            final List<String> sslSettingNames = settings.keySet().stream()
+                .filter(s -> s.startsWith(prefix))
+                .sorted()
+                .collect(Collectors.toUnmodifiableList());
+            if (sslSettingNames.isEmpty() == false) {
+                throw new ElasticsearchSecurityException("invalid configuration for " + prefix + " - [" + enabledSetting +
+                    "] is not set, but the following settings have been configured in elasticsearch.yml : [" +
+                    Strings.collectionToCommaDelimitedString(sslSettingNames) + "]");
+            }
+        }
+    }
+
     private void storeSslConfiguration(String key, SSLConfiguration configuration) {
         if (key.endsWith(".")) {
             key = key.substring(0, key.length() - 1);

x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/transport/ProfileConfigurationsTests.java

@@ -6,6 +6,7 @@
 
 package org.elasticsearch.xpack.core.security.transport;
 
+import org.elasticsearch.common.settings.MockSecureSettings;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.env.Environment;
 import org.elasticsearch.env.TestEnvironment;
@@ -15,14 +16,16 @@
 import org.elasticsearch.xpack.core.ssl.VerificationMode;
 import org.hamcrest.Matchers;
 
+import java.nio.file.Path;
 import java.util.Map;
 
 public class ProfileConfigurationsTests extends ESTestCase {
 
     public void testGetSecureTransportProfileConfigurations() {
-        final Settings settings = Settings.builder()
+        final Settings settings = getBaseSettings()
             .put("path.home", createTempDir())
             .put("xpack.security.transport.ssl.verification_mode", VerificationMode.CERTIFICATE.name())
+            .put("xpack.security.transport.ssl.verification_mode", VerificationMode.CERTIFICATE.name())
             .put("transport.profiles.full.xpack.security.ssl.verification_mode", VerificationMode.FULL.name())
             .put("transport.profiles.cert.xpack.security.ssl.verification_mode", VerificationMode.CERTIFICATE.name())
             .build();
@@ -39,7 +42,7 @@ public void testGetSecureTransportProfileConfigurations() {
 
     public void testGetInsecureTransportProfileConfigurations() {
         assumeFalse("Can't run in a FIPS JVM with verification mode None", inFipsJvm());
-        final Settings settings = Settings.builder()
+        final Settings settings = getBaseSettings()
             .put("path.home", createTempDir())
             .put("xpack.security.transport.ssl.verification_mode", VerificationMode.CERTIFICATE.name())
             .put("transport.profiles.none.xpack.security.ssl.verification_mode", VerificationMode.NONE.name())
@@ -53,4 +56,19 @@ public void testGetInsecureTransportProfileConfigurations() {
         assertThat(profileConfigurations.get("none").verificationMode(), Matchers.equalTo(VerificationMode.NONE));
         assertThat(profileConfigurations.get("default"), Matchers.sameInstance(defaultConfig));
     }
+
+    private Settings.Builder getBaseSettings() {
+        final Path keystore = randomBoolean()
+            ? getDataPath("/org/elasticsearch/xpack/security/transport/ssl/certs/simple/testnode.jks")
+            : getDataPath("/org/elasticsearch/xpack/security/transport/ssl/certs/simple/testnode.p12");
+
+        MockSecureSettings secureSettings = new MockSecureSettings();
+        secureSettings.setString("xpack.security.transport.ssl.keystore.secure_password", "testnode");
+
+        return Settings.builder()
+            .setSecureSettings(secureSettings)
+            .put("xpack.security.transport.ssl.enabled", true)
+            .put("xpack.security.transport.ssl.keystore.path", keystore.toString());
+    }
+
 }

x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ssl/SSLConfigurationReloaderTests.java

@@ -110,6 +110,7 @@ public void testReloadingKeyStore() throws Exception {
         secureSettings.setString("xpack.security.transport.ssl.keystore.secure_password", "testnode");
         final Settings settings = Settings.builder()
             .put("path.home", createTempDir())
+            .put("xpack.security.transport.ssl.enabled", true)
             .put("xpack.security.transport.ssl.keystore.path", keystorePath)
             .setSecureSettings(secureSettings)
             .build();
@@ -166,6 +167,7 @@ public void testPEMKeyConfigReloading() throws Exception {
         secureSettings.setString("xpack.security.transport.ssl.secure_key_passphrase", "testnode");
         final Settings settings = Settings.builder()
             .put("path.home", createTempDir())
+            .put("xpack.security.transport.ssl.enabled", true)
             .put("xpack.security.transport.ssl.key", keyPath)
             .put("xpack.security.transport.ssl.certificate", certPath)
             .putList("xpack.security.transport.ssl.certificate_authorities", certPath.toString())
@@ -222,10 +224,10 @@ public void testReloadingTrustStore() throws Exception {
             updatedTruststorePath);
         MockSecureSettings secureSettings = new MockSecureSettings();
         secureSettings.setString("xpack.security.transport.ssl.truststore.secure_password", "testnode");
-        Settings settings = Settings.builder()
+        final Settings settings = baseKeystoreSettings(tempDir, secureSettings)
+            .put("xpack.security.transport.ssl.enabled", true)
             .put("xpack.security.transport.ssl.truststore.path", trustStorePath)
             .put("path.home", createTempDir())
-            .setSecureSettings(secureSettings)
             .build();
         Environment env = TestEnvironment.newEnvironment(settings);
         // Create the MockWebServer once for both pre and post checks
@@ -273,7 +275,8 @@ public void testReloadingPEMTrustConfig() throws Exception {
         Files.copy(getDataPath("/org/elasticsearch/xpack/security/transport/ssl/certs/simple/testnode.crt"), serverCertPath);
         Files.copy(getDataPath("/org/elasticsearch/xpack/security/transport/ssl/certs/simple/testnode.pem"), serverKeyPath);
         Files.copy(getDataPath("/org/elasticsearch/xpack/security/transport/ssl/certs/simple/testnode_updated.crt"), updatedCert);
-        Settings settings = Settings.builder()
+        Settings settings = baseKeystoreSettings(tempDir, null)
+            .put("xpack.security.transport.ssl.enabled", true)
             .putList("xpack.security.transport.ssl.certificate_authorities", serverCertPath.toString())
             .put("path.home", createTempDir())
             .build();
@@ -322,6 +325,7 @@ public void testReloadingKeyStoreException() throws Exception {
         MockSecureSettings secureSettings = new MockSecureSettings();
         secureSettings.setString("xpack.security.transport.ssl.keystore.secure_password", "testnode");
         Settings settings = Settings.builder()
+            .put("xpack.security.transport.ssl.enabled", true)
             .put("xpack.security.transport.ssl.keystore.path", keystorePath)
             .setSecureSettings(secureSettings)
             .put("path.home", createTempDir())
@@ -372,6 +376,7 @@ public void testReloadingPEMKeyConfigException() throws Exception {
         MockSecureSettings secureSettings = new MockSecureSettings();
         secureSettings.setString("xpack.security.transport.ssl.secure_key_passphrase", "testnode");
         Settings settings = Settings.builder()
+            .put("xpack.security.transport.ssl.enabled", true)
             .put("xpack.security.transport.ssl.key", keyPath)
             .put("xpack.security.transport.ssl.certificate", certPath)
             .putList("xpack.security.transport.ssl.certificate_authorities", certPath.toString(), clientCertPath.toString())
@@ -419,10 +424,10 @@ public void testTrustStoreReloadException() throws Exception {
         Files.copy(getDataPath("/org/elasticsearch/xpack/security/transport/ssl/certs/simple/testnode.jks"), trustStorePath);
         MockSecureSettings secureSettings = new MockSecureSettings();
         secureSettings.setString("xpack.security.transport.ssl.truststore.secure_password", "testnode");
-        Settings settings = Settings.builder()
+        Settings settings = baseKeystoreSettings(tempDir, secureSettings)
+            .put("xpack.security.transport.ssl.enabled", true)
             .put("xpack.security.transport.ssl.truststore.path", trustStorePath)
             .put("path.home", createTempDir())
-            .setSecureSettings(secureSettings)
             .build();
         Environment env = TestEnvironment.newEnvironment(settings);
         final SSLService sslService = new SSLService(settings, env);
@@ -463,7 +468,8 @@ public void testPEMTrustReloadException() throws Exception {
         Path tempDir = createTempDir();
         Path clientCertPath = tempDir.resolve("testclient.crt");
         Files.copy(getDataPath("/org/elasticsearch/xpack/security/transport/ssl/certs/simple/testclient.crt"), clientCertPath);
-        Settings settings = Settings.builder()
+        Settings settings = baseKeystoreSettings(tempDir, null)
+            .put("xpack.security.transport.ssl.enabled", true)
             .putList("xpack.security.transport.ssl.certificate_authorities", clientCertPath.toString())
             .put("path.home", createTempDir())
             .build();
@@ -501,6 +507,20 @@ void reloadSSLContext(SSLConfiguration configuration) {
         assertThat(sslService.sslContextHolder(config).sslContext(), sameInstance(context));
     }
 
+    private Settings.Builder baseKeystoreSettings(Path tempDir, MockSecureSettings secureSettings) throws IOException {
+        final Path keystorePath = tempDir.resolve("testclient.jks");
+        Files.copy(getDataPath("/org/elasticsearch/xpack/security/transport/ssl/certs/simple/testnode.jks"), keystorePath);
+
+        if (secureSettings == null) {
+            secureSettings = new MockSecureSettings();
+        }
+        secureSettings.setString("xpack.security.transport.ssl.keystore.secure_password", "testnode");
+
+        return Settings.builder()
+            .put("xpack.security.transport.ssl.keystore.path", keystorePath.toString())
+            .setSecureSettings(secureSettings);
+    }
+
     private void validateSSLConfigurationIsReloaded(Settings settings, Environment env, Consumer<SSLContext> preChecks,
                                                     Runnable modificationFunction, Consumer<SSLContext> postChecks) throws Exception {
         final CountDownLatch reloadLatch = new CountDownLatch(1);