From a98df4ee15c40178c3ad221fc39d1a6266b4fef5 Mon Sep 17 00:00:00 2001 From: Keith Massey Date: Fri, 3 Sep 2021 10:57:17 -0500 Subject: [PATCH] Adding deprecation info API check for misconfigured or ambiguous SSL settings (#77092) In 8.0 we prevent the server from starting up if certain SSL properties are misconfigured or ambiguous. Specifically: 1) The server lacks a certificate/key pair (i.e. neither ssl.keystore.path nor ssl.key/ssl.certificate are configured) 2) The server has some ssl configuration, but ssl.enabled is not specified. This commit adds a check to the deprecation info API for these changes. Relates #42404 #45892 --- .../xpack/deprecation/DeprecationChecks.java | 2 + .../deprecation/NodeDeprecationChecks.java | 78 +++++++++++ .../NodeDeprecationChecksTests.java | 130 ++++++++++++++++++ 3 files changed, 210 insertions(+) diff --git a/x-pack/plugin/deprecation/src/main/java/org/elasticsearch/xpack/deprecation/DeprecationChecks.java b/x-pack/plugin/deprecation/src/main/java/org/elasticsearch/xpack/deprecation/DeprecationChecks.java index a026823656c6d..86fec7cc03c02 100644 --- a/x-pack/plugin/deprecation/src/main/java/org/elasticsearch/xpack/deprecation/DeprecationChecks.java +++ b/x-pack/plugin/deprecation/src/main/java/org/elasticsearch/xpack/deprecation/DeprecationChecks.java @@ -98,6 +98,8 @@ private DeprecationChecks() { NodeDeprecationChecks::checkImplicitlyDisabledSecurityOnBasicAndTrial, NodeDeprecationChecks::checkSearchRemoteSettings, NodeDeprecationChecks::checkMonitoringExporterPassword, + NodeDeprecationChecks::checkSslServerEnabled, + NodeDeprecationChecks::checkSslCertConfiguration, NodeDeprecationChecks::checkClusterRoutingAllocationIncludeRelocationsSetting, (settings, pluginsAndModules, clusterState, licenseState) -> NodeDeprecationChecks.checkNoPermitHandshakeFromIncompatibleBuilds(settings, pluginsAndModules, clusterState, diff --git a/x-pack/plugin/deprecation/src/main/java/org/elasticsearch/xpack/deprecation/NodeDeprecationChecks.java b/x-pack/plugin/deprecation/src/main/java/org/elasticsearch/xpack/deprecation/NodeDeprecationChecks.java index 8478e97e00547..7bfe1bc75a42f 100644 --- a/x-pack/plugin/deprecation/src/main/java/org/elasticsearch/xpack/deprecation/NodeDeprecationChecks.java +++ b/x-pack/plugin/deprecation/src/main/java/org/elasticsearch/xpack/deprecation/NodeDeprecationChecks.java @@ -20,6 +20,7 @@ import org.elasticsearch.common.settings.Setting; import org.elasticsearch.common.settings.Setting.Property; import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.common.ssl.SslConfigurationKeys; import org.elasticsearch.common.util.concurrent.EsExecutors; import org.elasticsearch.common.util.set.Sets; import org.elasticsearch.core.TimeValue; @@ -668,6 +669,83 @@ static DeprecationIssue checkClusterRoutingAllocationIncludeRelocationsSetting(f ); } + static DeprecationIssue checkSslServerEnabled(final Settings settings, + final PluginsAndModules pluginsAndModules, + final ClusterState clusterState, + final XPackLicenseState licenseState) { + List details = new ArrayList<>(); + for (String prefix : new String[] {"xpack.security.transport.ssl", "xpack.security.http.ssl"}) { + final String enabledSettingKey = prefix + ".enabled"; + String enabledSettingValue = settings.get(enabledSettingKey); + Settings sslSettings = settings.filter(setting -> setting.startsWith(prefix)); + if (enabledSettingValue == null && sslSettings.size() > 0) { + String keys = sslSettings.keySet().stream().collect(Collectors.joining(",")); + String detail = String.format(Locale.ROOT, "setting [%s] is unset but the following settings exist: [%s]", + enabledSettingKey, keys); + details.add(detail); + } + } + if (details.isEmpty()) { + return null; + } else { + String url = "https://www.elastic.co/guide/en/elasticsearch/reference/master/migrating-8.0.html#breaking_80_security_changes"; + String message = "cannot set ssl properties without explicitly enabling or disabling ssl"; + String detailsString = details.stream().collect(Collectors.joining("; ")); + return new DeprecationIssue(DeprecationIssue.Level.CRITICAL, message, url, detailsString, false, null); + } + } + + static DeprecationIssue checkSslCertConfiguration(final Settings settings, + final PluginsAndModules pluginsAndModules, + final ClusterState clusterState, + final XPackLicenseState licenseState) { + List details = new ArrayList<>(); + for (String prefix : new String[]{"xpack.security.transport.ssl", "xpack.security.http.ssl"}) { + final String enabledSettingKey = prefix + ".enabled"; + boolean sslEnabled = settings.getAsBoolean(enabledSettingKey, false); + if (sslEnabled) { + String keystorePathSettingKey = prefix + "." + SslConfigurationKeys.KEYSTORE_PATH; + String keyPathSettingKey = prefix + "." + SslConfigurationKeys.KEY; + String certificatePathSettingKey = prefix + "." + SslConfigurationKeys.CERTIFICATE; + boolean keystorePathSettingExists = settings.get(keystorePathSettingKey) != null; + boolean keyPathSettingExists = settings.get(keyPathSettingKey) != null; + boolean certificatePathSettingExists = settings.get(certificatePathSettingKey) != null; + if (keystorePathSettingExists == false && keyPathSettingExists == false && certificatePathSettingExists == false) { + String detail = String.format(Locale.ROOT, "none of [%s], [%s], or [%s] are set. If [%s] is true either [%s] must be " + + "set, or [%s] and [%s] must be set", keystorePathSettingKey, keyPathSettingKey, + certificatePathSettingKey, enabledSettingKey, keystorePathSettingKey, keyPathSettingKey, certificatePathSettingKey); + details.add(detail); + } else if (keystorePathSettingExists && keyPathSettingExists && certificatePathSettingExists) { + String detail = String.format(Locale.ROOT, "all of [%s], [%s], and [%s] are set. Either [%s] must be set, or [%s] and" + + " [%s] must be set", keystorePathSettingKey, keyPathSettingKey, certificatePathSettingKey, + keystorePathSettingKey, keyPathSettingKey, certificatePathSettingKey); + details.add(detail); + } else if (keystorePathSettingExists && (keyPathSettingExists || certificatePathSettingExists)) { + String detail = String.format(Locale.ROOT, "[%s] and [%s] are set. Either [%s] must be set, or [%s] and [%s] must" + + " be set", + keystorePathSettingKey, + keyPathSettingExists ? keyPathSettingKey : certificatePathSettingKey, + keystorePathSettingKey, keyPathSettingKey, certificatePathSettingKey); + details.add(detail); + } else if ((keyPathSettingExists && certificatePathSettingExists == false) || + (keyPathSettingExists == false && certificatePathSettingExists)) { + String detail = String.format(Locale.ROOT, "[%s] is set but [%s] is not", + keyPathSettingExists ? keyPathSettingKey : certificatePathSettingKey, + keyPathSettingExists ? certificatePathSettingKey : keyPathSettingKey); + details.add(detail); + } + } + } + if (details.isEmpty()) { + return null; + } else { + String url = "https://www.elastic.co/guide/en/elasticsearch/reference/master/migrating-8.0.html#breaking_80_security_changes"; + String message = "if ssl is enabled either keystore must be set, or key path and certificate path must be set"; + String detailsString = details.stream().collect(Collectors.joining("; ")); + return new DeprecationIssue(DeprecationIssue.Level.CRITICAL, message, url, detailsString, false, null); + } + } + static DeprecationIssue checkNoPermitHandshakeFromIncompatibleBuilds(final Settings settings, final PluginsAndModules pluginsAndModules, final ClusterState clusterState, diff --git a/x-pack/plugin/deprecation/src/test/java/org/elasticsearch/xpack/deprecation/NodeDeprecationChecksTests.java b/x-pack/plugin/deprecation/src/test/java/org/elasticsearch/xpack/deprecation/NodeDeprecationChecksTests.java index c58e66b0df2f7..a928d24580ba2 100644 --- a/x-pack/plugin/deprecation/src/test/java/org/elasticsearch/xpack/deprecation/NodeDeprecationChecksTests.java +++ b/x-pack/plugin/deprecation/src/test/java/org/elasticsearch/xpack/deprecation/NodeDeprecationChecksTests.java @@ -963,6 +963,136 @@ public void testImplicitlyConfiguredSecurityOnGoldPlus() { assertThat(issues, empty()); } + public void testCheckSslServerEnabled() { + String httpSslEnabledKey = "xpack.security.http.ssl.enabled"; + String transportSslEnabledKey = "xpack.security.transport.ssl.enabled"; + String problemSettingKey1 = "xpack.security.http.ssl.keystore.path"; + String problemSettingValue1 = "some/fake/path"; + String problemSettingKey2 = "xpack.security.http.ssl.truststore.path"; + String problemSettingValue2 = "some/other/fake/path"; + final Settings nodeSettings = Settings.builder() + .put(transportSslEnabledKey, "true") + .put(problemSettingKey1, problemSettingValue1) + .put(problemSettingKey2, problemSettingValue2) + .build(); + final XPackLicenseState licenseState = new XPackLicenseState(Settings.EMPTY, () -> 0); + final ClusterState clusterState = ClusterState.EMPTY_STATE; + final DeprecationIssue expectedIssue = new DeprecationIssue(DeprecationIssue.Level.CRITICAL, + "cannot set ssl properties without explicitly enabling or disabling ssl", + "https://www.elastic.co/guide/en/elasticsearch/reference/master/migrating-8.0.html#breaking_80_security_changes", + String.format(Locale.ROOT, + "setting [%s] is unset but the following settings exist: [%s,%s]", + httpSslEnabledKey, + problemSettingKey1, + problemSettingKey2), + false,null + ); + + assertThat( + NodeDeprecationChecks.checkSslServerEnabled(nodeSettings, null, clusterState, licenseState), + equalTo(expectedIssue) + ); + } + + public void testCheckSslCertConfiguration() { + // SSL enabled, but no keystore/key/cert properties + Settings nodeSettings = Settings.builder() + .put("xpack.security.transport.ssl.enabled", "true") + .build(); + final XPackLicenseState licenseState = new XPackLicenseState(Settings.EMPTY, () -> 0); + final ClusterState clusterState = ClusterState.EMPTY_STATE; + DeprecationIssue expectedIssue = new DeprecationIssue(DeprecationIssue.Level.CRITICAL, + "if ssl is enabled either keystore must be set, or key path and certificate path must be set", + "https://www.elastic.co/guide/en/elasticsearch/reference/master/migrating-8.0.html#breaking_80_security_changes", + "none of [xpack.security.transport.ssl.keystore.path], [xpack.security.transport.ssl.key], or [xpack.security.transport" + + ".ssl.certificate] are set. If [xpack.security.transport.ssl.enabled] is true either [xpack.security.transport.ssl" + + ".keystore.path] must be set, or [xpack.security.transport.ssl.key] and [xpack.security.transport.ssl.certificate] " + + "must be set", + false,null + ); + assertThat( + NodeDeprecationChecks.checkSslCertConfiguration(nodeSettings, null, clusterState, licenseState), + equalTo(expectedIssue) + ); + + // SSL enabled, and keystore path give, expect no issue + nodeSettings = Settings.builder() + .put("xpack.security.transport.ssl.enabled", "true") + .put("xpack.security.transport.ssl.keystore.path", randomAlphaOfLength(10)) + .build(); + assertThat( + NodeDeprecationChecks.checkSslCertConfiguration(nodeSettings, null, clusterState, licenseState), + equalTo(null) + ); + + // SSL enabled, and key and certificate path give, expect no issue + nodeSettings = Settings.builder() + .put("xpack.security.transport.ssl.enabled", "true") + .put("xpack.security.transport.ssl.key", randomAlphaOfLength(10)) + .put("xpack.security.transport.ssl.certificate", randomAlphaOfLength(10)) + .build(); + assertThat( + NodeDeprecationChecks.checkSslCertConfiguration(nodeSettings, null, clusterState, licenseState), + equalTo(null) + ); + + // SSL enabled, specify both keystore and key and certificate path + nodeSettings = Settings.builder() + .put("xpack.security.transport.ssl.enabled", "true") + .put("xpack.security.transport.ssl.keystore.path", randomAlphaOfLength(10)) + .put("xpack.security.transport.ssl.key", randomAlphaOfLength(10)) + .put("xpack.security.transport.ssl.certificate", randomAlphaOfLength(10)) + .build(); + expectedIssue = new DeprecationIssue(DeprecationIssue.Level.CRITICAL, + "if ssl is enabled either keystore must be set, or key path and certificate path must be set", + "https://www.elastic.co/guide/en/elasticsearch/reference/master/migrating-8.0.html#breaking_80_security_changes", + "all of [xpack.security.transport.ssl.keystore.path], [xpack.security.transport.ssl.key], and [xpack.security.transport.ssl" + + ".certificate] are set. Either [xpack.security.transport.ssl.keystore.path] must be set, or [xpack.security.transport.ssl" + + ".key] and [xpack.security.transport.ssl.certificate] must be set", + false,null + ); + assertThat( + NodeDeprecationChecks.checkSslCertConfiguration(nodeSettings, null, clusterState, licenseState), + equalTo(expectedIssue) + ); + + // SSL enabled, specify keystore and key + nodeSettings = Settings.builder() + .put("xpack.security.transport.ssl.enabled", "true") + .put("xpack.security.transport.ssl.keystore.path", randomAlphaOfLength(10)) + .put("xpack.security.transport.ssl.key", randomAlphaOfLength(10)) + .build(); + expectedIssue = new DeprecationIssue(DeprecationIssue.Level.CRITICAL, + "if ssl is enabled either keystore must be set, or key path and certificate path must be set", + "https://www.elastic.co/guide/en/elasticsearch/reference/master/migrating-8.0.html#breaking_80_security_changes", + "[xpack.security.transport.ssl.keystore.path] and [xpack.security.transport.ssl.key] are set. Either [xpack.security" + + ".transport.ssl.keystore.path] must be set, or [xpack.security.transport.ssl.key] and [xpack.security.transport.ssl" + + ".certificate] must be set", + false,null + ); + assertThat( + NodeDeprecationChecks.checkSslCertConfiguration(nodeSettings, null, clusterState, licenseState), + equalTo(expectedIssue) + ); + + // Sanity check that it also works for http: + nodeSettings = Settings.builder() + .put("xpack.security.http.ssl.enabled", "true") + .build(); + expectedIssue = new DeprecationIssue(DeprecationIssue.Level.CRITICAL, + "if ssl is enabled either keystore must be set, or key path and certificate path must be set", + "https://www.elastic.co/guide/en/elasticsearch/reference/master/migrating-8.0.html#breaking_80_security_changes", + "none of [xpack.security.http.ssl.keystore.path], [xpack.security.http.ssl.key], or [xpack.security.http.ssl.certificate] are" + + " set. If [xpack.security.http.ssl.enabled] is true either [xpack.security.http.ssl.keystore.path] must be set, or [xpack" + + ".security.http.ssl.key] and [xpack.security.http.ssl.certificate] must be set", + false,null + ); + assertThat( + NodeDeprecationChecks.checkSslCertConfiguration(nodeSettings, null, clusterState, licenseState), + equalTo(expectedIssue) + ); + } + @SuppressForbidden(reason = "sets and unsets es.unsafely_permit_handshake_from_incompatible_builds") public void testCheckNoPermitHandshakeFromIncompatibleBuilds() { final DeprecationIssue expectedNullIssue =