Skip to content

Commit

Permalink
Adding deprecation info API check for misconfigured or ambiguous SSL …
Browse files Browse the repository at this point in the history 
…settings (elastic#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 elastic#42404 elastic#45892
  • Loading branch information
@masseyke
masseyke authored Sep 3, 2021
1 parent b830351 commit a98df4e
Show file tree
Hide file tree
Showing 3 changed files with 210 additions and 0 deletions.
2 changes: 2 additions & 0 deletions ...ugin/deprecation/src/main/java/org/elasticsearch/xpack/deprecation/DeprecationChecks.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
78 changes: 78 additions & 0 deletions .../deprecation/src/main/java/org/elasticsearch/xpack/deprecation/NodeDeprecationChecks.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -668,6 +669,83 @@ static DeprecationIssue checkClusterRoutingAllocationIncludeRelocationsSetting(f
);
}

static DeprecationIssue checkSslServerEnabled(final Settings settings,
final PluginsAndModules pluginsAndModules,
final ClusterState clusterState,
final XPackLicenseState licenseState) {
List<String> 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<String> 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,
Expand Down
130 changes: 130 additions & 0 deletions ...ecation/src/test/java/org/elasticsearch/xpack/deprecation/NodeDeprecationChecksTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand Down

0 comments on commit a98df4e

Please sign in to comment.