From 6905ca9d6cbc9b1227b07f252f7c3598fb5a6f57 Mon Sep 17 00:00:00 2001 From: Ioannis Kakavas Date: Mon, 20 Aug 2018 17:01:10 +0300 Subject: [PATCH] Use settings from the context in BootstrapChecks (#32908) Use settings from the context in BootstrapChecks instead of passing them in the constructor --- .../FIPS140JKSKeystoreBootstrapCheck.java | 10 ++-------- .../FIPS140LicenseBootstrapCheck.java | 9 ++------- ...asswordHashingAlgorithmBootstrapCheck.java | 9 +-------- .../xpack/security/Security.java | 6 +++--- ...FIPS140JKSKeystoreBootstrapCheckTests.java | 14 ++++++------- .../FIPS140LicenseBootstrapCheckTests.java | 20 ++++++++++--------- ...rdHashingAlgorithmBootstrapCheckTests.java | 6 +++--- 7 files changed, 29 insertions(+), 45 deletions(-) diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/FIPS140JKSKeystoreBootstrapCheck.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/FIPS140JKSKeystoreBootstrapCheck.java index cd5a720eef1dc..28f2756cf262c 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/FIPS140JKSKeystoreBootstrapCheck.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/FIPS140JKSKeystoreBootstrapCheck.java @@ -13,12 +13,6 @@ public class FIPS140JKSKeystoreBootstrapCheck implements BootstrapCheck { - private final boolean fipsModeEnabled; - - FIPS140JKSKeystoreBootstrapCheck(Settings settings) { - this.fipsModeEnabled = XPackSettings.FIPS_MODE_ENABLED.get(settings); - } - /** * Test if the node fails the check. * @@ -28,7 +22,7 @@ public class FIPS140JKSKeystoreBootstrapCheck implements BootstrapCheck { @Override public BootstrapCheckResult check(BootstrapContext context) { - if (fipsModeEnabled) { + if (XPackSettings.FIPS_MODE_ENABLED.get(context.settings)) { final Settings settings = context.settings; Settings keystoreTypeSettings = settings.filter(k -> k.endsWith("keystore.type")) .filter(k -> settings.get(k).equalsIgnoreCase("jks")); @@ -50,6 +44,6 @@ public BootstrapCheckResult check(BootstrapContext context) { @Override public boolean alwaysEnforce() { - return fipsModeEnabled; + return true; } } diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/FIPS140LicenseBootstrapCheck.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/FIPS140LicenseBootstrapCheck.java index d1bce0dcdd24c..957276bdad2f3 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/FIPS140LicenseBootstrapCheck.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/FIPS140LicenseBootstrapCheck.java @@ -10,6 +10,7 @@ import org.elasticsearch.bootstrap.BootstrapContext; import org.elasticsearch.license.License; import org.elasticsearch.license.LicenseService; +import org.elasticsearch.xpack.core.XPackSettings; import java.util.EnumSet; @@ -21,15 +22,9 @@ final class FIPS140LicenseBootstrapCheck implements BootstrapCheck { static final EnumSet ALLOWED_LICENSE_OPERATION_MODES = EnumSet.of(License.OperationMode.PLATINUM, License.OperationMode.TRIAL); - private final boolean isInFipsMode; - - FIPS140LicenseBootstrapCheck(boolean isInFipsMode) { - this.isInFipsMode = isInFipsMode; - } - @Override public BootstrapCheckResult check(BootstrapContext context) { - if (isInFipsMode) { + if (XPackSettings.FIPS_MODE_ENABLED.get(context.settings)) { License license = LicenseService.getLicense(context.metaData); if (license != null && ALLOWED_LICENSE_OPERATION_MODES.contains(license.operationMode()) == false) { return BootstrapCheckResult.failure("FIPS mode is only allowed with a Platinum or Trial license"); diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/FIPS140PasswordHashingAlgorithmBootstrapCheck.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/FIPS140PasswordHashingAlgorithmBootstrapCheck.java index 751d63be4fb3f..3faec3d747575 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/FIPS140PasswordHashingAlgorithmBootstrapCheck.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/FIPS140PasswordHashingAlgorithmBootstrapCheck.java @@ -7,19 +7,12 @@ import org.elasticsearch.bootstrap.BootstrapCheck; import org.elasticsearch.bootstrap.BootstrapContext; -import org.elasticsearch.common.settings.Settings; import org.elasticsearch.xpack.core.XPackSettings; import java.util.Locale; public class FIPS140PasswordHashingAlgorithmBootstrapCheck implements BootstrapCheck { - private final boolean fipsModeEnabled; - - FIPS140PasswordHashingAlgorithmBootstrapCheck(final Settings settings) { - this.fipsModeEnabled = XPackSettings.FIPS_MODE_ENABLED.get(settings); - } - /** * Test if the node fails the check. * @@ -28,7 +21,7 @@ public class FIPS140PasswordHashingAlgorithmBootstrapCheck implements BootstrapC */ @Override public BootstrapCheckResult check(final BootstrapContext context) { - if (fipsModeEnabled) { + if (XPackSettings.FIPS_MODE_ENABLED.get(context.settings)) { final String selectedAlgorithm = XPackSettings.PASSWORD_HASHING_ALGORITHM.get(context.settings); if (selectedAlgorithm.toLowerCase(Locale.ROOT).startsWith("pbkdf2") == false) { return BootstrapCheckResult.failure("Only PBKDF2 is allowed for password hashing in a FIPS-140 JVM. Please set the " + diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java index 02910b5dd74fc..2a392478cbcd9 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java @@ -300,9 +300,9 @@ public Security(Settings settings, final Path configPath) { new PkiRealmBootstrapCheck(getSslService()), new TLSLicenseBootstrapCheck(), new FIPS140SecureSettingsBootstrapCheck(settings, env), - new FIPS140JKSKeystoreBootstrapCheck(settings), - new FIPS140PasswordHashingAlgorithmBootstrapCheck(settings), - new FIPS140LicenseBootstrapCheck(XPackSettings.FIPS_MODE_ENABLED.get(settings)))); + new FIPS140JKSKeystoreBootstrapCheck(), + new FIPS140PasswordHashingAlgorithmBootstrapCheck(), + new FIPS140LicenseBootstrapCheck())); checks.addAll(InternalRealms.getBootstrapChecks(settings, env)); this.bootstrapChecks = Collections.unmodifiableList(checks); Automatons.updateMaxDeterminizedStates(settings); diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/FIPS140JKSKeystoreBootstrapCheckTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/FIPS140JKSKeystoreBootstrapCheckTests.java index 1d4da71e11b5e..b659adf22cfc3 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/FIPS140JKSKeystoreBootstrapCheckTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/FIPS140JKSKeystoreBootstrapCheckTests.java @@ -14,7 +14,7 @@ public class FIPS140JKSKeystoreBootstrapCheckTests extends ESTestCase { public void testNoKeystoreIsAllowed() { final Settings.Builder settings = Settings.builder() .put("xpack.security.fips_mode.enabled", "true"); - assertFalse(new FIPS140JKSKeystoreBootstrapCheck(settings.build()).check(new BootstrapContext(settings.build(), null)).isFailure()); + assertFalse(new FIPS140JKSKeystoreBootstrapCheck().check(new BootstrapContext(settings.build(), null)).isFailure()); } public void testSSLKeystoreTypeIsNotAllowed() { @@ -22,7 +22,7 @@ public void testSSLKeystoreTypeIsNotAllowed() { .put("xpack.security.fips_mode.enabled", "true") .put("xpack.ssl.keystore.path", "/this/is/the/path") .put("xpack.ssl.keystore.type", "JKS"); - assertTrue(new FIPS140JKSKeystoreBootstrapCheck(settings.build()).check(new BootstrapContext(settings.build(), null)).isFailure()); + assertTrue(new FIPS140JKSKeystoreBootstrapCheck().check(new BootstrapContext(settings.build(), null)).isFailure()); } public void testSSLImplicitKeystoreTypeIsNotAllowed() { @@ -30,7 +30,7 @@ public void testSSLImplicitKeystoreTypeIsNotAllowed() { .put("xpack.security.fips_mode.enabled", "true") .put("xpack.ssl.keystore.path", "/this/is/the/path") .put("xpack.ssl.keystore.type", "JKS"); - assertTrue(new FIPS140JKSKeystoreBootstrapCheck(settings.build()).check(new BootstrapContext(settings.build(), null)).isFailure()); + assertTrue(new FIPS140JKSKeystoreBootstrapCheck().check(new BootstrapContext(settings.build(), null)).isFailure()); } public void testTransportSSLKeystoreTypeIsNotAllowed() { @@ -38,7 +38,7 @@ public void testTransportSSLKeystoreTypeIsNotAllowed() { .put("xpack.security.fips_mode.enabled", "true") .put("xpack.security.transport.ssl.keystore.path", "/this/is/the/path") .put("xpack.security.transport.ssl.keystore.type", "JKS"); - assertTrue(new FIPS140JKSKeystoreBootstrapCheck(settings.build()).check(new BootstrapContext(settings.build(), null)).isFailure()); + assertTrue(new FIPS140JKSKeystoreBootstrapCheck().check(new BootstrapContext(settings.build(), null)).isFailure()); } public void testHttpSSLKeystoreTypeIsNotAllowed() { @@ -46,7 +46,7 @@ public void testHttpSSLKeystoreTypeIsNotAllowed() { .put("xpack.security.fips_mode.enabled", "true") .put("xpack.security.http.ssl.keystore.path", "/this/is/the/path") .put("xpack.security.http.ssl.keystore.type", "JKS"); - assertTrue(new FIPS140JKSKeystoreBootstrapCheck(settings.build()).check(new BootstrapContext(settings.build(), null)).isFailure()); + assertTrue(new FIPS140JKSKeystoreBootstrapCheck().check(new BootstrapContext(settings.build(), null)).isFailure()); } public void testRealmKeystoreTypeIsNotAllowed() { @@ -54,13 +54,13 @@ public void testRealmKeystoreTypeIsNotAllowed() { .put("xpack.security.fips_mode.enabled", "true") .put("xpack.security.authc.realms.ldap.ssl.keystore.path", "/this/is/the/path") .put("xpack.security.authc.realms.ldap.ssl.keystore.type", "JKS"); - assertTrue(new FIPS140JKSKeystoreBootstrapCheck(settings.build()).check(new BootstrapContext(settings.build(), null)).isFailure()); + assertTrue(new FIPS140JKSKeystoreBootstrapCheck().check(new BootstrapContext(settings.build(), null)).isFailure()); } public void testImplicitRealmKeystoreTypeIsNotAllowed() { final Settings.Builder settings = Settings.builder() .put("xpack.security.fips_mode.enabled", "true") .put("xpack.security.authc.realms.ldap.ssl.keystore.path", "/this/is/the/path"); - assertTrue(new FIPS140JKSKeystoreBootstrapCheck(settings.build()).check(new BootstrapContext(settings.build(), null)).isFailure()); + assertTrue(new FIPS140JKSKeystoreBootstrapCheck().check(new BootstrapContext(settings.build(), null)).isFailure()); } } diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/FIPS140LicenseBootstrapCheckTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/FIPS140LicenseBootstrapCheckTests.java index a2ec8f9fb2053..fb4c9e21a258f 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/FIPS140LicenseBootstrapCheckTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/FIPS140LicenseBootstrapCheckTests.java @@ -17,27 +17,29 @@ public class FIPS140LicenseBootstrapCheckTests extends ESTestCase { public void testBootstrapCheck() throws Exception { - assertTrue(new FIPS140LicenseBootstrapCheck(false) - .check(new BootstrapContext(Settings.EMPTY, MetaData.EMPTY_META_DATA)).isSuccess()); - assertTrue(new FIPS140LicenseBootstrapCheck(randomBoolean()) + assertTrue(new FIPS140LicenseBootstrapCheck() .check(new BootstrapContext(Settings.EMPTY, MetaData.EMPTY_META_DATA)).isSuccess()); + assertTrue(new FIPS140LicenseBootstrapCheck() + .check(new BootstrapContext(Settings.builder().put("xpack.security.fips_mode.enabled", randomBoolean()).build(), MetaData + .EMPTY_META_DATA)).isSuccess()); - License license = TestUtils.generateSignedLicense(TimeValue.timeValueHours(24)); MetaData.Builder builder = MetaData.builder(); + License license = TestUtils.generateSignedLicense(TimeValue.timeValueHours(24)); TestUtils.putLicense(builder, license); MetaData metaData = builder.build(); + if (FIPS140LicenseBootstrapCheck.ALLOWED_LICENSE_OPERATION_MODES.contains(license.operationMode())) { - assertTrue(new FIPS140LicenseBootstrapCheck(true).check(new BootstrapContext( + assertTrue(new FIPS140LicenseBootstrapCheck().check(new BootstrapContext( Settings.builder().put("xpack.security.fips_mode.enabled", true).build(), metaData)).isSuccess()); - assertTrue(new FIPS140LicenseBootstrapCheck(false).check(new BootstrapContext( + assertTrue(new FIPS140LicenseBootstrapCheck().check(new BootstrapContext( Settings.builder().put("xpack.security.fips_mode.enabled", false).build(), metaData)).isSuccess()); } else { - assertTrue(new FIPS140LicenseBootstrapCheck(false).check(new BootstrapContext( + assertTrue(new FIPS140LicenseBootstrapCheck().check(new BootstrapContext( Settings.builder().put("xpack.security.fips_mode.enabled", false).build(), metaData)).isSuccess()); - assertTrue(new FIPS140LicenseBootstrapCheck(true).check(new BootstrapContext( + assertTrue(new FIPS140LicenseBootstrapCheck().check(new BootstrapContext( Settings.builder().put("xpack.security.fips_mode.enabled", true).build(), metaData)).isFailure()); assertEquals("FIPS mode is only allowed with a Platinum or Trial license", - new FIPS140LicenseBootstrapCheck(true).check(new BootstrapContext( + new FIPS140LicenseBootstrapCheck().check(new BootstrapContext( Settings.builder().put("xpack.security.fips_mode.enabled", true).build(), metaData)).getMessage()); } } diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/FIPS140PasswordHashingAlgorithmBootstrapCheckTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/FIPS140PasswordHashingAlgorithmBootstrapCheckTests.java index 8632400866a07..6376ca211dcae 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/FIPS140PasswordHashingAlgorithmBootstrapCheckTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/FIPS140PasswordHashingAlgorithmBootstrapCheckTests.java @@ -25,7 +25,7 @@ public void testPBKDF2AlgorithmIsAllowed() { .put(XPackSettings.PASSWORD_HASHING_ALGORITHM.getKey(), "PBKDF2_10000") .build(); final BootstrapCheck.BootstrapCheckResult result = - new FIPS140PasswordHashingAlgorithmBootstrapCheck(settings).check(new BootstrapContext(settings, null)); + new FIPS140PasswordHashingAlgorithmBootstrapCheck().check(new BootstrapContext(settings, null)); assertFalse(result.isFailure()); } @@ -35,7 +35,7 @@ public void testPBKDF2AlgorithmIsAllowed() { .put(XPackSettings.PASSWORD_HASHING_ALGORITHM.getKey(), "PBKDF2") .build(); final BootstrapCheck.BootstrapCheckResult result = - new FIPS140PasswordHashingAlgorithmBootstrapCheck(settings).check(new BootstrapContext(settings, null)); + new FIPS140PasswordHashingAlgorithmBootstrapCheck().check(new BootstrapContext(settings, null)); assertFalse(result.isFailure()); } } @@ -55,7 +55,7 @@ private void runBCRYPTTest(final boolean fipsModeEnabled, final String passwordH } final Settings settings = builder.build(); final BootstrapCheck.BootstrapCheckResult result = - new FIPS140PasswordHashingAlgorithmBootstrapCheck(settings).check(new BootstrapContext(settings, null)); + new FIPS140PasswordHashingAlgorithmBootstrapCheck().check(new BootstrapContext(settings, null)); assertThat(result.isFailure(), equalTo(fipsModeEnabled)); }