Skip to content

Commit 5071d43

Browse files
tvernumjasontedor
tvernum
authored and
jasontedor
committed
Enforce transport TLS on Basic with Security (#42150)
If a basic license enables security, then we should also enforce TLS on the transport interface. This was already the case for Standard/Gold/Platinum licenses. For Basic, security defaults to disabled, so some of the process around checking whether security is actuallY enabled is more complex now that we need to account for basic licenses.
1 parent 7bd161d commit 5071d43

File tree

7 files changed

+176
-62
lines changed

7 files changed

+176
-62
lines changed

‎x-pack/plugin/core/src/main/java/org/elasticsearch/license/License.java

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -780,22 +780,4 @@ public Builder validate() {
780780
}
781781
}
782782

783-
/**
784-
* Returns <code>true</code> iff the license is a production licnese
785-
*/
786-
public boolean isProductionLicense() {
787-
switch (operationMode()) {
788-
case MISSING:
789-
case TRIAL:
790-
case BASIC:
791-
return false;
792-
case STANDARD:
793-
case GOLD:
794-
case PLATINUM:
795-
return true;
796-
default:
797-
throw new AssertionError("unknown operation mode: " + operationMode());
798-
799-
}
800-
}
801783
}

‎x-pack/plugin/core/src/main/java/org/elasticsearch/license/LicenseService.java

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -218,10 +218,13 @@ public void registerLicense(final PutLicenseRequest request, final ActionListene
218218
}
219219
}
220220

221+
// This check would be incorrect if "basic" licenses were allowed here
222+
// because the defaults there mean that security can be "off", even if the setting is "on"
223+
// BUT basic licenses are explicitly excluded earlier in this method, so we don't need to worry
221224
if (XPackSettings.SECURITY_ENABLED.get(settings)) {
222225
// TODO we should really validate that all nodes have xpack installed and are consistently configured but this
223226
// should happen on a different level and not in this code
224-
if (newLicense.isProductionLicense()
227+
if (XPackLicenseState.isTransportTlsRequired(newLicense, settings)
225228
&& XPackSettings.TRANSPORT_SSL_ENABLED.get(settings) == false
226229
&& isProductionMode(settings, clusterService.localNode())) {
227230
// security is on but TLS is not configured we gonna fail the entire request and throw an exception

‎x-pack/plugin/core/src/main/java/org/elasticsearch/license/XPackLicenseState.java

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -282,7 +282,7 @@ private static class Status {
282282
public XPackLicenseState(Settings settings) {
283283
this.listeners = new CopyOnWriteArrayList<>();
284284
this.isSecurityEnabled = XPackSettings.SECURITY_ENABLED.get(settings);
285-
this.isSecurityExplicitlyEnabled = isSecurityEnabled && settings.hasValue(XPackSettings.SECURITY_ENABLED.getKey());
285+
this.isSecurityExplicitlyEnabled = isSecurityEnabled && isSecurityExplicitlyEnabled(settings);
286286
}
287287

288288
private XPackLicenseState(XPackLicenseState xPackLicenseState) {
@@ -292,6 +292,10 @@ private XPackLicenseState(XPackLicenseState xPackLicenseState) {
292292
this.status = xPackLicenseState.status;
293293
}
294294

295+
private static boolean isSecurityExplicitlyEnabled(Settings settings) {
296+
return settings.hasValue(XPackSettings.SECURITY_ENABLED.getKey());
297+
}
298+
295299
/**
296300
* Updates the current state of the license, which will change what features are available.
297301
*
@@ -718,6 +722,25 @@ public synchronized boolean isSecurityDisabledByLicenseDefaults() {
718722
return false;
719723
}
720724

725+
public static boolean isTransportTlsRequired(License license, Settings settings) {
726+
if (license == null) {
727+
return false;
728+
}
729+
switch (license.operationMode()) {
730+
case STANDARD:
731+
case GOLD:
732+
case PLATINUM:
733+
return XPackSettings.SECURITY_ENABLED.get(settings);
734+
case BASIC:
735+
return XPackSettings.SECURITY_ENABLED.get(settings) && isSecurityExplicitlyEnabled(settings);
736+
case MISSING:
737+
case TRIAL:
738+
return false;
739+
default:
740+
throw new AssertionError("unknown operation mode [" + license.operationMode() + "]");
741+
}
742+
}
743+
721744
private static boolean isSecurityEnabled(final OperationMode mode, final boolean isSecurityExplicitlyEnabled,
722745
final boolean isSecurityEnabled) {
723746
switch (mode) {

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

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
import org.elasticsearch.bootstrap.BootstrapContext;
1010
import org.elasticsearch.license.License;
1111
import org.elasticsearch.license.LicenseService;
12+
import org.elasticsearch.license.XPackLicenseState;
1213
import org.elasticsearch.xpack.core.XPackSettings;
1314

1415
/**
@@ -19,10 +20,11 @@ public final class TLSLicenseBootstrapCheck implements BootstrapCheck {
1920
public BootstrapCheckResult check(BootstrapContext context) {
2021
if (XPackSettings.TRANSPORT_SSL_ENABLED.get(context.settings()) == false) {
2122
License license = LicenseService.getLicense(context.metaData());
22-
if (license != null && license.isProductionLicense()) {
23-
return BootstrapCheckResult.failure("Transport SSL must be enabled for setups with production licenses. Please set " +
24-
"[xpack.security.transport.ssl.enabled] to [true] or disable security by setting [xpack.security.enabled] " +
25-
"to [false]");
23+
if (XPackLicenseState.isTransportTlsRequired(license, context.settings())) {
24+
return BootstrapCheckResult.failure("Transport SSL must be enabled if security is enabled on a [" +
25+
license.operationMode().description() + "] license. " +
26+
"Please set [xpack.security.transport.ssl.enabled] to [true] or disable security by setting " +
27+
"[xpack.security.enabled] to [false]");
2628
}
2729
}
2830
return BootstrapCheckResult.success();

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

Lines changed: 99 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -5,40 +5,115 @@
55
*/
66
package org.elasticsearch.xpack.core.ssl;
77

8+
import org.elasticsearch.bootstrap.BootstrapCheck;
9+
import org.elasticsearch.bootstrap.BootstrapContext;
810
import org.elasticsearch.cluster.metadata.MetaData;
911
import org.elasticsearch.common.settings.Settings;
1012
import org.elasticsearch.common.unit.TimeValue;
1113
import org.elasticsearch.license.License;
14+
import org.elasticsearch.license.License.OperationMode;
1215
import org.elasticsearch.license.TestUtils;
1316
import org.elasticsearch.test.AbstractBootstrapCheckTestCase;
1417

15-
import java.util.EnumSet;
16-
1718
public class TLSLicenseBootstrapCheckTests extends AbstractBootstrapCheckTestCase {
18-
public void testBootstrapCheck() throws Exception {
19+
public void testBootstrapCheckOnEmptyMetadata() {
1920
assertTrue(new TLSLicenseBootstrapCheck().check(emptyContext).isSuccess());
2021
assertTrue(new TLSLicenseBootstrapCheck().check(createTestContext(Settings.builder().put("xpack.security.transport.ssl.enabled"
21-
, randomBoolean()).build(), MetaData.EMPTY_META_DATA)).isSuccess());
22-
int numIters = randomIntBetween(1,10);
23-
for (int i = 0; i < numIters; i++) {
24-
License license = TestUtils.generateSignedLicense(TimeValue.timeValueHours(24));
25-
EnumSet<License.OperationMode> productionModes = EnumSet.of(License.OperationMode.GOLD, License.OperationMode.PLATINUM,
26-
License.OperationMode.STANDARD);
27-
MetaData.Builder builder = MetaData.builder();
28-
TestUtils.putLicense(builder, license);
29-
MetaData build = builder.build();
30-
if (productionModes.contains(license.operationMode()) == false) {
31-
assertTrue(new TLSLicenseBootstrapCheck().check(createTestContext(
32-
Settings.builder().put("xpack.security.transport.ssl.enabled", true).build(), build)).isSuccess());
33-
} else {
34-
assertTrue(new TLSLicenseBootstrapCheck().check(createTestContext(
35-
Settings.builder().put("xpack.security.transport.ssl.enabled", false).build(), build)).isFailure());
36-
assertEquals("Transport SSL must be enabled for setups with production licenses. Please set " +
37-
"[xpack.security.transport.ssl.enabled] to [true] or disable security by setting " +
38-
"[xpack.security.enabled] to [false]",
39-
new TLSLicenseBootstrapCheck().check(createTestContext(
40-
Settings.builder().put("xpack.security.transport.ssl.enabled", false).build(), build)).getMessage());
41-
}
22+
, randomBoolean()).build(), MetaData.EMPTY_META_DATA)).isSuccess());
23+
}
24+
25+
public void testBootstrapCheckFailureOnPremiumLicense() throws Exception {
26+
final OperationMode mode = randomFrom(OperationMode.PLATINUM, OperationMode.GOLD, OperationMode.STANDARD);
27+
final Settings.Builder settings = Settings.builder();
28+
if (randomBoolean()) {
29+
// randomise between default-false & explicit-false
30+
settings.put("xpack.security.transport.ssl.enabled", false);
31+
}
32+
if (randomBoolean()) {
33+
// randomise between default-true & explicit-true
34+
settings.put("xpack.security.enabled", true);
35+
}
36+
37+
final BootstrapCheck.BootstrapCheckResult result = runBootstrapCheck(mode, settings);
38+
assertTrue("Expected bootstrap failure", result.isFailure());
39+
assertEquals("Transport SSL must be enabled if security is enabled on a [" + mode.description() + "] license. Please set " +
40+
"[xpack.security.transport.ssl.enabled] to [true] or disable security by setting " +
41+
"[xpack.security.enabled] to [false]",
42+
result.getMessage());
43+
}
44+
45+
public void testBootstrapCheckSucceedsWithTlsEnabledOnPremiumLicense() throws Exception {
46+
final OperationMode mode = randomFrom(OperationMode.PLATINUM, OperationMode.GOLD, OperationMode.STANDARD);
47+
final Settings.Builder settings = Settings.builder().put("xpack.security.transport.ssl.enabled", true);
48+
final BootstrapCheck.BootstrapCheckResult result = runBootstrapCheck(mode, settings);
49+
assertSuccess(result);
50+
}
51+
52+
public void testBootstrapCheckFailureOnBasicLicense() throws Exception {
53+
final Settings.Builder settings = Settings.builder().put("xpack.security.enabled", true);
54+
if (randomBoolean()) {
55+
// randomise between default-false & explicit-false
56+
settings.put("xpack.security.transport.ssl.enabled", false);
57+
}
58+
final BootstrapCheck.BootstrapCheckResult result = runBootstrapCheck(OperationMode.BASIC, settings);
59+
assertTrue("Expected bootstrap failure", result.isFailure());
60+
assertEquals("Transport SSL must be enabled if security is enabled on a [basic] license. Please set " +
61+
"[xpack.security.transport.ssl.enabled] to [true] or disable security by setting " +
62+
"[xpack.security.enabled] to [false]",
63+
result.getMessage());
64+
}
65+
66+
public void testBootstrapSucceedsIfSecurityIsNotEnabledOnBasicLicense() throws Exception {
67+
final Settings.Builder settings = Settings.builder();
68+
if (randomBoolean()) {
69+
// randomise between default-false & explicit-false
70+
settings.put("xpack.security.enabled", false);
71+
}
72+
if (randomBoolean()) {
73+
// it does not matter whether or not this is set, as security is not enabled.
74+
settings.put("xpack.security.transport.ssl.enabled", randomBoolean());
4275
}
76+
final BootstrapCheck.BootstrapCheckResult result = runBootstrapCheck(OperationMode.BASIC, settings);
77+
assertSuccess(result);
4378
}
79+
80+
public void testBootstrapSucceedsIfTlsIsEnabledOnBasicLicense() throws Exception {
81+
final Settings.Builder settings = Settings.builder().put("xpack.security.transport.ssl.enabled", true);
82+
if (randomBoolean()) {
83+
// it does not matter whether or not this is set, as TLS is enabled.
84+
settings.put("xpack.security.enabled", randomBoolean());
85+
}
86+
final BootstrapCheck.BootstrapCheckResult result = runBootstrapCheck(OperationMode.BASIC, settings);
87+
assertSuccess(result);
88+
}
89+
90+
public void testBootstrapCheckAlwaysSucceedsOnTrialLicense() throws Exception {
91+
final Settings.Builder settings = Settings.builder();
92+
if (randomBoolean()) {
93+
// it does not matter whether this is set, or to which value.
94+
settings.put("xpack.security.enabled", randomBoolean());
95+
}
96+
if (randomBoolean()) {
97+
// it does not matter whether this is set, or to which value.
98+
settings.put("xpack.security.transport.ssl.enabled", randomBoolean());
99+
}
100+
final BootstrapCheck.BootstrapCheckResult result = runBootstrapCheck(OperationMode.TRIAL, settings);
101+
assertSuccess(result);
102+
}
103+
104+
public BootstrapCheck.BootstrapCheckResult runBootstrapCheck(OperationMode mode, Settings.Builder settings) throws Exception {
105+
final License license = TestUtils.generateSignedLicense(mode.description(), TimeValue.timeValueHours(24));
106+
MetaData.Builder builder = MetaData.builder();
107+
TestUtils.putLicense(builder, license);
108+
MetaData metaData = builder.build();
109+
final BootstrapContext context = createTestContext(settings.build(), metaData);
110+
return new TLSLicenseBootstrapCheck().check(context);
111+
}
112+
113+
public void assertSuccess(BootstrapCheck.BootstrapCheckResult result) {
114+
if (result.isFailure()) {
115+
fail("Bootstrap check failed unexpectedly: " + result.getMessage());
116+
}
117+
}
118+
44119
}

‎x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -965,7 +965,7 @@ public Function<String, Predicate<String>> getFieldFilter() {
965965
public BiConsumer<DiscoveryNode, ClusterState> getJoinValidator() {
966966
if (enabled) {
967967
return new ValidateTLSOnJoin(XPackSettings.TRANSPORT_SSL_ENABLED.get(settings),
968-
DiscoveryModule.DISCOVERY_TYPE_SETTING.get(settings))
968+
DiscoveryModule.DISCOVERY_TYPE_SETTING.get(settings), settings)
969969
.andThen(new ValidateUpgradedSecurityIndex())
970970
.andThen(new ValidateLicenseCanBeDeserialized())
971971
.andThen(new ValidateLicenseForFIPS(XPackSettings.FIPS_MODE_ENABLED.get(settings)));
@@ -976,18 +976,21 @@ public BiConsumer<DiscoveryNode, ClusterState> getJoinValidator() {
976976
static final class ValidateTLSOnJoin implements BiConsumer<DiscoveryNode, ClusterState> {
977977
private final boolean isTLSEnabled;
978978
private final String discoveryType;
979+
private final Settings settings;
979980

980-
ValidateTLSOnJoin(boolean isTLSEnabled, String discoveryType) {
981+
ValidateTLSOnJoin(boolean isTLSEnabled, String discoveryType, Settings settings) {
981982
this.isTLSEnabled = isTLSEnabled;
982983
this.discoveryType = discoveryType;
984+
this.settings = settings;
983985
}
984986

985987
@Override
986988
public void accept(DiscoveryNode node, ClusterState state) {
987989
License license = LicenseService.getLicense(state.metaData());
988-
if (license != null && license.isProductionLicense() &&
989-
isTLSEnabled == false && "single-node".equals(discoveryType) == false) {
990-
throw new IllegalStateException("TLS setup is required for license type [" + license.operationMode().name() + "]");
990+
if (isTLSEnabled == false && "single-node".equals(discoveryType) == false
991+
&& XPackLicenseState.isTransportTlsRequired(license, settings)) {
992+
throw new IllegalStateException("Transport TLS ([" + XPackSettings.TRANSPORT_SSL_ENABLED.getKey() +
993+
"]) is required for license type [" + license.operationMode().description() + "] when security is enabled");
991994
}
992995
}
993996
}

‎x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/SecurityTests.java

Lines changed: 35 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,6 @@
5353
import java.util.Arrays;
5454
import java.util.Collection;
5555
import java.util.Collections;
56-
import java.util.EnumSet;
5756
import java.util.HashMap;
5857
import java.util.HashSet;
5958
import java.util.List;
@@ -66,7 +65,6 @@
6665

6766
import static org.elasticsearch.cluster.metadata.IndexMetaData.INDEX_FORMAT_SETTING;
6867
import static org.elasticsearch.discovery.DiscoveryModule.ZEN2_DISCOVERY_TYPE;
69-
import static org.elasticsearch.discovery.DiscoveryModule.ZEN_DISCOVERY_TYPE;
7068
import static org.elasticsearch.xpack.security.support.SecurityIndexManager.INTERNAL_INDEX_FORMAT;
7169
import static org.elasticsearch.xpack.security.support.SecurityIndexManager.SECURITY_INDEX_NAME;
7270
import static org.hamcrest.Matchers.containsString;
@@ -253,17 +251,45 @@ public void testTLSJoinValidator() throws Exception {
253251
int numIters = randomIntBetween(1, 10);
254252
for (int i = 0; i < numIters; i++) {
255253
boolean tlsOn = randomBoolean();
256-
String discoveryType = randomFrom("single-node", ZEN_DISCOVERY_TYPE, ZEN2_DISCOVERY_TYPE, randomAlphaOfLength(4));
257-
Security.ValidateTLSOnJoin validator = new Security.ValidateTLSOnJoin(tlsOn, discoveryType);
254+
boolean securityExplicitlyEnabled = randomBoolean();
255+
String discoveryType = randomFrom("single-node", ZEN2_DISCOVERY_TYPE, ZEN2_DISCOVERY_TYPE, randomAlphaOfLength(4));
256+
257+
final Settings settings;
258+
if (securityExplicitlyEnabled) {
259+
settings = Settings.builder().put("xpack.security.enabled", true).build();
260+
} else {
261+
settings = Settings.EMPTY;
262+
}
263+
Security.ValidateTLSOnJoin validator = new Security.ValidateTLSOnJoin(tlsOn, discoveryType, settings);
258264
MetaData.Builder builder = MetaData.builder();
259-
License license = TestUtils.generateSignedLicense(TimeValue.timeValueHours(24));
265+
License.OperationMode licenseMode = randomFrom(License.OperationMode.values());
266+
License license = TestUtils.generateSignedLicense(licenseMode.description(), TimeValue.timeValueHours(24));
260267
TestUtils.putLicense(builder, license);
261268
ClusterState state = ClusterState.builder(ClusterName.DEFAULT).metaData(builder.build()).build();
262-
EnumSet<License.OperationMode> productionModes = EnumSet.of(License.OperationMode.GOLD, License.OperationMode.PLATINUM,
263-
License.OperationMode.STANDARD);
264-
if (productionModes.contains(license.operationMode()) && tlsOn == false && "single-node".equals(discoveryType) == false) {
269+
270+
final boolean expectFailure;
271+
switch (licenseMode) {
272+
case PLATINUM:
273+
case GOLD:
274+
case STANDARD:
275+
expectFailure = tlsOn == false && "single-node".equals(discoveryType) == false;
276+
break;
277+
case BASIC:
278+
expectFailure = tlsOn == false && "single-node".equals(discoveryType) == false && securityExplicitlyEnabled;
279+
break;
280+
case MISSING:
281+
case TRIAL:
282+
expectFailure = false;
283+
break;
284+
default:
285+
throw new AssertionError("unknown operation mode [" + license.operationMode() + "]");
286+
}
287+
logger.info("Test TLS join; Lic:{} TLS:{} Disco:{} Settings:{} ; Expect Failure: {}",
288+
licenseMode, tlsOn, discoveryType, settings.toDelimitedString(','), expectFailure);
289+
if (expectFailure) {
265290
IllegalStateException ise = expectThrows(IllegalStateException.class, () -> validator.accept(node, state));
266-
assertEquals("TLS setup is required for license type [" + license.operationMode().name() + "]", ise.getMessage());
291+
assertEquals("Transport TLS ([xpack.security.transport.ssl.enabled]) is required for license type ["
292+
+ license.operationMode().description() + "] when security is enabled", ise.getMessage());
267293
} else {
268294
validator.accept(node, state);
269295
}

0 commit comments

Comments
 (0)