Deprecate misconfigured SSL server config

This commit adds a deprecation warning when starting
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.

Backport of: elastic#45892

Author: tvernum

client/rest-high-level/build.gradle

@@ -123,6 +123,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'

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

@@ -14,6 +14,7 @@
 import org.elasticsearch.ElasticsearchSecurityException;
 import org.elasticsearch.common.CheckedSupplier;
 import org.elasticsearch.common.Strings;
+import org.elasticsearch.common.logging.DeprecationLogger;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.env.Environment;
 import org.elasticsearch.xpack.core.XPackSettings;
@@ -33,14 +34,13 @@
 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;
 import java.security.GeneralSecurityException;
 import java.security.KeyManagementException;
-import java.security.NoSuchAlgorithmException;
 import java.security.KeyStore;
+import java.security.NoSuchAlgorithmException;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
@@ -68,6 +68,8 @@
 public class SSLService {
 
     private static final Logger logger = LogManager.getLogger(SSLService.class);
+    private static final DeprecationLogger deprecationLogger = new DeprecationLogger(logger);
+
     /**
      * An ordered map of protocol algorithms to SSLContext algorithms. The map is ordered from most
      * secure to least secure. The names in this map are taken from the
@@ -432,6 +434,10 @@ Map<SSLConfiguration, SSLContextHolder> loadSSLConfigurations() {
         Map<String, Settings> profileSettings = getTransportProfileSSLSettings(settings);
         profileSettings.forEach((key, profileSetting) -> loadConfiguration(key, profileSetting, sslContextHolders));
 
+        for (String context : Arrays.asList("xpack.security.transport.ssl", "xpack.security.http.ssl")) {
+            validateServerConfiguration(context);
+        }
+
         return Collections.unmodifiableMap(sslContextHolders);
     }
 
@@ -450,6 +456,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) {
+                deprecationLogger.deprecated("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.toList());
+            if (sslSettingNames.isEmpty() == false) {
+                deprecationLogger.deprecated("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())
@@ -223,10 +225,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 = randomBoolean() ? null : TestEnvironment.newEnvironment(settings);
         // Create the MockWebServer once for both pre and post checks
@@ -274,7 +276,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();
@@ -323,6 +326,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())
@@ -373,6 +377,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())
@@ -420,10 +425,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 = randomBoolean() ? null : TestEnvironment.newEnvironment(settings);
         final SSLService sslService = new SSLService(settings, env);
@@ -464,7 +469,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();
@@ -502,6 +508,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);