Skip to content

Commit a98df4e

Browse files
masseykemasseyke
authored
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
1 parent b830351 commit a98df4e

File tree

3 files changed

+210
-0
lines changed

3 files changed

+210
-0
lines changed

‎x-pack/plugin/deprecation/src/main/java/org/elasticsearch/xpack/deprecation/DeprecationChecks.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,8 @@ private DeprecationChecks() {
9898
NodeDeprecationChecks::checkImplicitlyDisabledSecurityOnBasicAndTrial,
9999
NodeDeprecationChecks::checkSearchRemoteSettings,
100100
NodeDeprecationChecks::checkMonitoringExporterPassword,
101+
NodeDeprecationChecks::checkSslServerEnabled,
102+
NodeDeprecationChecks::checkSslCertConfiguration,
101103
NodeDeprecationChecks::checkClusterRoutingAllocationIncludeRelocationsSetting,
102104
(settings, pluginsAndModules, clusterState, licenseState) ->
103105
NodeDeprecationChecks.checkNoPermitHandshakeFromIncompatibleBuilds(settings, pluginsAndModules, clusterState,

‎x-pack/plugin/deprecation/src/main/java/org/elasticsearch/xpack/deprecation/NodeDeprecationChecks.java

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import org.elasticsearch.common.settings.Setting;
2121
import org.elasticsearch.common.settings.Setting.Property;
2222
import org.elasticsearch.common.settings.Settings;
23+
import org.elasticsearch.common.ssl.SslConfigurationKeys;
2324
import org.elasticsearch.common.util.concurrent.EsExecutors;
2425
import org.elasticsearch.common.util.set.Sets;
2526
import org.elasticsearch.core.TimeValue;
@@ -668,6 +669,83 @@ static DeprecationIssue checkClusterRoutingAllocationIncludeRelocationsSetting(f
668669
);
669670
}
670671

672+
static DeprecationIssue checkSslServerEnabled(final Settings settings,
673+
final PluginsAndModules pluginsAndModules,
674+
final ClusterState clusterState,
675+
final XPackLicenseState licenseState) {
676+
List<String> details = new ArrayList<>();
677+
for (String prefix : new String[] {"xpack.security.transport.ssl", "xpack.security.http.ssl"}) {
678+
final String enabledSettingKey = prefix + ".enabled";
679+
String enabledSettingValue = settings.get(enabledSettingKey);
680+
Settings sslSettings = settings.filter(setting -> setting.startsWith(prefix));
681+
if (enabledSettingValue == null && sslSettings.size() > 0) {
682+
String keys = sslSettings.keySet().stream().collect(Collectors.joining(","));
683+
String detail = String.format(Locale.ROOT, "setting [%s] is unset but the following settings exist: [%s]",
684+
enabledSettingKey, keys);
685+
details.add(detail);
686+
}
687+
}
688+
if (details.isEmpty()) {
689+
return null;
690+
} else {
691+
String url = "https://www.elastic.co/guide/en/elasticsearch/reference/master/migrating-8.0.html#breaking_80_security_changes";
692+
String message = "cannot set ssl properties without explicitly enabling or disabling ssl";
693+
String detailsString = details.stream().collect(Collectors.joining("; "));
694+
return new DeprecationIssue(DeprecationIssue.Level.CRITICAL, message, url, detailsString, false, null);
695+
}
696+
}
697+
698+
static DeprecationIssue checkSslCertConfiguration(final Settings settings,
699+
final PluginsAndModules pluginsAndModules,
700+
final ClusterState clusterState,
701+
final XPackLicenseState licenseState) {
702+
List<String> details = new ArrayList<>();
703+
for (String prefix : new String[]{"xpack.security.transport.ssl", "xpack.security.http.ssl"}) {
704+
final String enabledSettingKey = prefix + ".enabled";
705+
boolean sslEnabled = settings.getAsBoolean(enabledSettingKey, false);
706+
if (sslEnabled) {
707+
String keystorePathSettingKey = prefix + "." + SslConfigurationKeys.KEYSTORE_PATH;
708+
String keyPathSettingKey = prefix + "." + SslConfigurationKeys.KEY;
709+
String certificatePathSettingKey = prefix + "." + SslConfigurationKeys.CERTIFICATE;
710+
boolean keystorePathSettingExists = settings.get(keystorePathSettingKey) != null;
711+
boolean keyPathSettingExists = settings.get(keyPathSettingKey) != null;
712+
boolean certificatePathSettingExists = settings.get(certificatePathSettingKey) != null;
713+
if (keystorePathSettingExists == false && keyPathSettingExists == false && certificatePathSettingExists == false) {
714+
String detail = String.format(Locale.ROOT, "none of [%s], [%s], or [%s] are set. If [%s] is true either [%s] must be " +
715+
"set, or [%s] and [%s] must be set", keystorePathSettingKey, keyPathSettingKey,
716+
certificatePathSettingKey, enabledSettingKey, keystorePathSettingKey, keyPathSettingKey, certificatePathSettingKey);
717+
details.add(detail);
718+
} else if (keystorePathSettingExists && keyPathSettingExists && certificatePathSettingExists) {
719+
String detail = String.format(Locale.ROOT, "all of [%s], [%s], and [%s] are set. Either [%s] must be set, or [%s] and" +
720+
" [%s] must be set", keystorePathSettingKey, keyPathSettingKey, certificatePathSettingKey,
721+
keystorePathSettingKey, keyPathSettingKey, certificatePathSettingKey);
722+
details.add(detail);
723+
} else if (keystorePathSettingExists && (keyPathSettingExists || certificatePathSettingExists)) {
724+
String detail = String.format(Locale.ROOT, "[%s] and [%s] are set. Either [%s] must be set, or [%s] and [%s] must" +
725+
" be set",
726+
keystorePathSettingKey,
727+
keyPathSettingExists ? keyPathSettingKey : certificatePathSettingKey,
728+
keystorePathSettingKey, keyPathSettingKey, certificatePathSettingKey);
729+
details.add(detail);
730+
} else if ((keyPathSettingExists && certificatePathSettingExists == false) ||
731+
(keyPathSettingExists == false && certificatePathSettingExists)) {
732+
String detail = String.format(Locale.ROOT, "[%s] is set but [%s] is not",
733+
keyPathSettingExists ? keyPathSettingKey : certificatePathSettingKey,
734+
keyPathSettingExists ? certificatePathSettingKey : keyPathSettingKey);
735+
details.add(detail);
736+
}
737+
}
738+
}
739+
if (details.isEmpty()) {
740+
return null;
741+
} else {
742+
String url = "https://www.elastic.co/guide/en/elasticsearch/reference/master/migrating-8.0.html#breaking_80_security_changes";
743+
String message = "if ssl is enabled either keystore must be set, or key path and certificate path must be set";
744+
String detailsString = details.stream().collect(Collectors.joining("; "));
745+
return new DeprecationIssue(DeprecationIssue.Level.CRITICAL, message, url, detailsString, false, null);
746+
}
747+
}
748+
671749
static DeprecationIssue checkNoPermitHandshakeFromIncompatibleBuilds(final Settings settings,
672750
final PluginsAndModules pluginsAndModules,
673751
final ClusterState clusterState,

‎x-pack/plugin/deprecation/src/test/java/org/elasticsearch/xpack/deprecation/NodeDeprecationChecksTests.java

Lines changed: 130 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -963,6 +963,136 @@ public void testImplicitlyConfiguredSecurityOnGoldPlus() {
963963
assertThat(issues, empty());
964964
}
965965

966+
public void testCheckSslServerEnabled() {
967+
String httpSslEnabledKey = "xpack.security.http.ssl.enabled";
968+
String transportSslEnabledKey = "xpack.security.transport.ssl.enabled";
969+
String problemSettingKey1 = "xpack.security.http.ssl.keystore.path";
970+
String problemSettingValue1 = "some/fake/path";
971+
String problemSettingKey2 = "xpack.security.http.ssl.truststore.path";
972+
String problemSettingValue2 = "some/other/fake/path";
973+
final Settings nodeSettings = Settings.builder()
974+
.put(transportSslEnabledKey, "true")
975+
.put(problemSettingKey1, problemSettingValue1)
976+
.put(problemSettingKey2, problemSettingValue2)
977+
.build();
978+
final XPackLicenseState licenseState = new XPackLicenseState(Settings.EMPTY, () -> 0);
979+
final ClusterState clusterState = ClusterState.EMPTY_STATE;
980+
final DeprecationIssue expectedIssue = new DeprecationIssue(DeprecationIssue.Level.CRITICAL,
981+
"cannot set ssl properties without explicitly enabling or disabling ssl",
982+
"https://www.elastic.co/guide/en/elasticsearch/reference/master/migrating-8.0.html#breaking_80_security_changes",
983+
String.format(Locale.ROOT,
984+
"setting [%s] is unset but the following settings exist: [%s,%s]",
985+
httpSslEnabledKey,
986+
problemSettingKey1,
987+
problemSettingKey2),
988+
false,null
989+
);
990+
991+
assertThat(
992+
NodeDeprecationChecks.checkSslServerEnabled(nodeSettings, null, clusterState, licenseState),
993+
equalTo(expectedIssue)
994+
);
995+
}
996+
997+
public void testCheckSslCertConfiguration() {
998+
// SSL enabled, but no keystore/key/cert properties
999+
Settings nodeSettings = Settings.builder()
1000+
.put("xpack.security.transport.ssl.enabled", "true")
1001+
.build();
1002+
final XPackLicenseState licenseState = new XPackLicenseState(Settings.EMPTY, () -> 0);
1003+
final ClusterState clusterState = ClusterState.EMPTY_STATE;
1004+
DeprecationIssue expectedIssue = new DeprecationIssue(DeprecationIssue.Level.CRITICAL,
1005+
"if ssl is enabled either keystore must be set, or key path and certificate path must be set",
1006+
"https://www.elastic.co/guide/en/elasticsearch/reference/master/migrating-8.0.html#breaking_80_security_changes",
1007+
"none of [xpack.security.transport.ssl.keystore.path], [xpack.security.transport.ssl.key], or [xpack.security.transport" +
1008+
".ssl.certificate] are set. If [xpack.security.transport.ssl.enabled] is true either [xpack.security.transport.ssl" +
1009+
".keystore.path] must be set, or [xpack.security.transport.ssl.key] and [xpack.security.transport.ssl.certificate] " +
1010+
"must be set",
1011+
false,null
1012+
);
1013+
assertThat(
1014+
NodeDeprecationChecks.checkSslCertConfiguration(nodeSettings, null, clusterState, licenseState),
1015+
equalTo(expectedIssue)
1016+
);
1017+
1018+
// SSL enabled, and keystore path give, expect no issue
1019+
nodeSettings = Settings.builder()
1020+
.put("xpack.security.transport.ssl.enabled", "true")
1021+
.put("xpack.security.transport.ssl.keystore.path", randomAlphaOfLength(10))
1022+
.build();
1023+
assertThat(
1024+
NodeDeprecationChecks.checkSslCertConfiguration(nodeSettings, null, clusterState, licenseState),
1025+
equalTo(null)
1026+
);
1027+
1028+
// SSL enabled, and key and certificate path give, expect no issue
1029+
nodeSettings = Settings.builder()
1030+
.put("xpack.security.transport.ssl.enabled", "true")
1031+
.put("xpack.security.transport.ssl.key", randomAlphaOfLength(10))
1032+
.put("xpack.security.transport.ssl.certificate", randomAlphaOfLength(10))
1033+
.build();
1034+
assertThat(
1035+
NodeDeprecationChecks.checkSslCertConfiguration(nodeSettings, null, clusterState, licenseState),
1036+
equalTo(null)
1037+
);
1038+
1039+
// SSL enabled, specify both keystore and key and certificate path
1040+
nodeSettings = Settings.builder()
1041+
.put("xpack.security.transport.ssl.enabled", "true")
1042+
.put("xpack.security.transport.ssl.keystore.path", randomAlphaOfLength(10))
1043+
.put("xpack.security.transport.ssl.key", randomAlphaOfLength(10))
1044+
.put("xpack.security.transport.ssl.certificate", randomAlphaOfLength(10))
1045+
.build();
1046+
expectedIssue = new DeprecationIssue(DeprecationIssue.Level.CRITICAL,
1047+
"if ssl is enabled either keystore must be set, or key path and certificate path must be set",
1048+
"https://www.elastic.co/guide/en/elasticsearch/reference/master/migrating-8.0.html#breaking_80_security_changes",
1049+
"all of [xpack.security.transport.ssl.keystore.path], [xpack.security.transport.ssl.key], and [xpack.security.transport.ssl" +
1050+
".certificate] are set. Either [xpack.security.transport.ssl.keystore.path] must be set, or [xpack.security.transport.ssl" +
1051+
".key] and [xpack.security.transport.ssl.certificate] must be set",
1052+
false,null
1053+
);
1054+
assertThat(
1055+
NodeDeprecationChecks.checkSslCertConfiguration(nodeSettings, null, clusterState, licenseState),
1056+
equalTo(expectedIssue)
1057+
);
1058+
1059+
// SSL enabled, specify keystore and key
1060+
nodeSettings = Settings.builder()
1061+
.put("xpack.security.transport.ssl.enabled", "true")
1062+
.put("xpack.security.transport.ssl.keystore.path", randomAlphaOfLength(10))
1063+
.put("xpack.security.transport.ssl.key", randomAlphaOfLength(10))
1064+
.build();
1065+
expectedIssue = new DeprecationIssue(DeprecationIssue.Level.CRITICAL,
1066+
"if ssl is enabled either keystore must be set, or key path and certificate path must be set",
1067+
"https://www.elastic.co/guide/en/elasticsearch/reference/master/migrating-8.0.html#breaking_80_security_changes",
1068+
"[xpack.security.transport.ssl.keystore.path] and [xpack.security.transport.ssl.key] are set. Either [xpack.security" +
1069+
".transport.ssl.keystore.path] must be set, or [xpack.security.transport.ssl.key] and [xpack.security.transport.ssl" +
1070+
".certificate] must be set",
1071+
false,null
1072+
);
1073+
assertThat(
1074+
NodeDeprecationChecks.checkSslCertConfiguration(nodeSettings, null, clusterState, licenseState),
1075+
equalTo(expectedIssue)
1076+
);
1077+
1078+
// Sanity check that it also works for http:
1079+
nodeSettings = Settings.builder()
1080+
.put("xpack.security.http.ssl.enabled", "true")
1081+
.build();
1082+
expectedIssue = new DeprecationIssue(DeprecationIssue.Level.CRITICAL,
1083+
"if ssl is enabled either keystore must be set, or key path and certificate path must be set",
1084+
"https://www.elastic.co/guide/en/elasticsearch/reference/master/migrating-8.0.html#breaking_80_security_changes",
1085+
"none of [xpack.security.http.ssl.keystore.path], [xpack.security.http.ssl.key], or [xpack.security.http.ssl.certificate] are" +
1086+
" set. If [xpack.security.http.ssl.enabled] is true either [xpack.security.http.ssl.keystore.path] must be set, or [xpack" +
1087+
".security.http.ssl.key] and [xpack.security.http.ssl.certificate] must be set",
1088+
false,null
1089+
);
1090+
assertThat(
1091+
NodeDeprecationChecks.checkSslCertConfiguration(nodeSettings, null, clusterState, licenseState),
1092+
equalTo(expectedIssue)
1093+
);
1094+
}
1095+
9661096
@SuppressForbidden(reason = "sets and unsets es.unsafely_permit_handshake_from_incompatible_builds")
9671097
public void testCheckNoPermitHandshakeFromIncompatibleBuilds() {
9681098
final DeprecationIssue expectedNullIssue =

0 commit comments

Comments
 (0)