Browse Source

Security: remove password hash bootstrap check (#32440)

This change removes the PasswordHashingBootstrapCheck and replaces it
with validation on the setting itself. This ensures we always get a
valid value from the setting when it is used.
Jay Modi 7 years ago
parent
commit
ac5ef8c389

+ 11 - 2
x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/XPackSettings.java

@@ -14,6 +14,7 @@ import org.elasticsearch.xpack.core.ssl.SSLConfigurationSettings;
 import org.elasticsearch.xpack.core.ssl.VerificationMode;
 
 import javax.crypto.Cipher;
+import javax.crypto.SecretKeyFactory;
 
 import java.security.NoSuchAlgorithmException;
 import java.util.ArrayList;
@@ -127,8 +128,16 @@ public class XPackSettings {
     public static final Setting<String> PASSWORD_HASHING_ALGORITHM = new Setting<>(
         "xpack.security.authc.password_hashing.algorithm", "bcrypt", Function.identity(), (v, s) -> {
         if (Hasher.getAvailableAlgoStoredHash().contains(v.toLowerCase(Locale.ROOT)) == false) {
-            throw new IllegalArgumentException("Invalid algorithm: " + v + ". Only pbkdf2 or bcrypt family algorithms can be used for " +
-                "password hashing.");
+            throw new IllegalArgumentException("Invalid algorithm: " + v + ". Valid values for password hashing are " +
+                Hasher.getAvailableAlgoStoredHash().toString());
+        } else if (v.regionMatches(true, 0, "pbkdf2", 0, "pbkdf2".length())) {
+            try {
+                SecretKeyFactory.getInstance("PBKDF2withHMACSHA512");
+            } catch (NoSuchAlgorithmException e) {
+                throw new IllegalArgumentException(
+                    "Support for PBKDF2WithHMACSHA512 must be available in order to use any of the " +
+                        "PBKDF2 algorithms for the [xpack.security.authc.password_hashing.algorithm] setting.", e);
+            }
         }
     }, Setting.Property.NodeScope);
 

+ 31 - 0
x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/XPackSettingsTests.java

@@ -5,9 +5,14 @@
  */
 package org.elasticsearch.xpack.core;
 
+import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.test.ESTestCase;
 import javax.crypto.Cipher;
+import javax.crypto.SecretKeyFactory;
 
+import java.security.NoSuchAlgorithmException;
+
+import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.hasItem;
 import static org.hamcrest.Matchers.not;
 
@@ -25,4 +30,30 @@ public class XPackSettingsTests extends ESTestCase {
             assertThat(XPackSettings.DEFAULT_CIPHERS, not(hasItem("TLS_RSA_WITH_AES_256_CBC_SHA")));
         }
     }
+
+    public void testPasswordHashingAlgorithmSettingValidation() {
+        final boolean isPBKDF2Available = isSecretkeyFactoryAlgoAvailable("PBKDF2WithHMACSHA512");
+        final String pbkdf2Algo = randomFrom("PBKDF2_10000", "PBKDF2");
+        final Settings settings = Settings.builder().put(XPackSettings.PASSWORD_HASHING_ALGORITHM.getKey(), pbkdf2Algo).build();
+        if (isPBKDF2Available) {
+            assertEquals(pbkdf2Algo, XPackSettings.PASSWORD_HASHING_ALGORITHM.get(settings));
+        } else {
+            IllegalArgumentException e = expectThrows(IllegalArgumentException.class,
+                () -> XPackSettings.PASSWORD_HASHING_ALGORITHM.get(settings));
+            assertThat(e.getMessage(), containsString("Support for PBKDF2WithHMACSHA512 must be available"));
+        }
+
+        final String bcryptAlgo = randomFrom("BCRYPT", "BCRYPT11");
+        assertEquals(bcryptAlgo, XPackSettings.PASSWORD_HASHING_ALGORITHM.get(
+            Settings.builder().put(XPackSettings.PASSWORD_HASHING_ALGORITHM.getKey(), bcryptAlgo).build()));
+    }
+
+    private boolean isSecretkeyFactoryAlgoAvailable(String algorithmId) {
+        try {
+            SecretKeyFactory.getInstance(algorithmId);
+            return true;
+        } catch (NoSuchAlgorithmException e) {
+            return false;
+        }
+    }
 }

+ 0 - 41
x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/PasswordHashingAlgorithmBootstrapCheck.java

@@ -1,41 +0,0 @@
-/*
- * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
- * or more contributor license agreements. Licensed under the Elastic License;
- * you may not use this file except in compliance with the Elastic License.
- */
-package org.elasticsearch.xpack.security;
-
-import org.elasticsearch.bootstrap.BootstrapCheck;
-import org.elasticsearch.bootstrap.BootstrapContext;
-import org.elasticsearch.xpack.core.XPackSettings;
-
-import javax.crypto.SecretKeyFactory;
-import java.security.NoSuchAlgorithmException;
-import java.util.Locale;
-
-/**
- * Bootstrap check to ensure that one of the allowed password hashing algorithms is
- * selected and that it is available.
- */
-public class PasswordHashingAlgorithmBootstrapCheck implements BootstrapCheck {
-    @Override
-    public BootstrapCheckResult check(BootstrapContext context) {
-        final String selectedAlgorithm = XPackSettings.PASSWORD_HASHING_ALGORITHM.get(context.settings);
-        if (selectedAlgorithm.toLowerCase(Locale.ROOT).startsWith("pbkdf2")) {
-            try {
-                SecretKeyFactory.getInstance("PBKDF2withHMACSHA512");
-            } catch (NoSuchAlgorithmException e) {
-                final String errorMessage = String.format(Locale.ROOT,
-                    "Support for PBKDF2WithHMACSHA512 must be available in order to use any of the " +
-                        "PBKDF2 algorithms for the [%s] setting.", XPackSettings.PASSWORD_HASHING_ALGORITHM.getKey());
-                return BootstrapCheckResult.failure(errorMessage);
-            }
-        }
-        return BootstrapCheckResult.success();
-    }
-
-    @Override
-    public boolean alwaysEnforce() {
-        return true;
-    }
-}

+ 0 - 1
x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java

@@ -300,7 +300,6 @@ public class Security extends Plugin implements ActionPlugin, IngestPlugin, Netw
                 new TokenSSLBootstrapCheck(),
                 new PkiRealmBootstrapCheck(getSslService()),
                 new TLSLicenseBootstrapCheck(),
-                new PasswordHashingAlgorithmBootstrapCheck(),
                 new FIPS140SecureSettingsBootstrapCheck(settings, env),
                 new FIPS140JKSKeystoreBootstrapCheck(settings),
                 new FIPS140PasswordHashingAlgorithmBootstrapCheck(settings)));

+ 0 - 44
x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/PasswordHashingAlgorithmBootstrapCheckTests.java

@@ -1,44 +0,0 @@
-/*
- * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
- * or more contributor license agreements. Licensed under the Elastic License;
- * you may not use this file except in compliance with the Elastic License.
- */
-package org.elasticsearch.xpack.security;
-
-import org.elasticsearch.bootstrap.BootstrapContext;
-import org.elasticsearch.common.settings.Settings;
-import org.elasticsearch.test.ESTestCase;
-import org.elasticsearch.xpack.core.XPackSettings;
-
-import javax.crypto.SecretKeyFactory;
-import java.security.NoSuchAlgorithmException;
-
-public class PasswordHashingAlgorithmBootstrapCheckTests extends ESTestCase {
-
-    public void testPasswordHashingAlgorithmBootstrapCheck() {
-        Settings settings = Settings.EMPTY;
-        assertFalse(new PasswordHashingAlgorithmBootstrapCheck().check(new BootstrapContext(settings, null)).isFailure());
-        // The following two will always pass because for now we only test in environments where PBKDF2WithHMACSHA512 is supported
-        assertTrue(isSecretkeyFactoryAlgoAvailable("PBKDF2WithHMACSHA512"));
-        settings = Settings.builder().put(XPackSettings.PASSWORD_HASHING_ALGORITHM.getKey(), "PBKDF2_10000").build();
-        assertFalse(new PasswordHashingAlgorithmBootstrapCheck().check(new BootstrapContext(settings, null)).isFailure());
-
-        settings = Settings.builder().put(XPackSettings.PASSWORD_HASHING_ALGORITHM.getKey(), "PBKDF2").build();
-        assertFalse(new PasswordHashingAlgorithmBootstrapCheck().check(new BootstrapContext(settings, null)).isFailure());
-
-        settings = Settings.builder().put(XPackSettings.PASSWORD_HASHING_ALGORITHM.getKey(), "BCRYPT").build();
-        assertFalse(new PasswordHashingAlgorithmBootstrapCheck().check(new BootstrapContext(settings, null)).isFailure());
-
-        settings = Settings.builder().put(XPackSettings.PASSWORD_HASHING_ALGORITHM.getKey(), "BCRYPT11").build();
-        assertFalse(new PasswordHashingAlgorithmBootstrapCheck().check(new BootstrapContext(settings, null)).isFailure());
-    }
-
-    private boolean isSecretkeyFactoryAlgoAvailable(String algorithmId) {
-        try {
-            SecretKeyFactory.getInstance(algorithmId);
-            return true;
-        } catch (NoSuchAlgorithmException e) {
-            return false;
-        }
-    }
-}

+ 1 - 1
x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/esnative/ReservedRealmTests.java

@@ -83,7 +83,7 @@ public class ReservedRealmTests extends ESTestCase {
         IllegalArgumentException exception = expectThrows(IllegalArgumentException.class, () -> new ReservedRealm(mock(Environment.class),
             invalidSettings, usersStore, new AnonymousUser(Settings.EMPTY), securityIndex, threadPool));
         assertThat(exception.getMessage(), containsString(invalidAlgoId));
-        assertThat(exception.getMessage(), containsString("Only pbkdf2 or bcrypt family algorithms can be used for password hashing"));
+        assertThat(exception.getMessage(), containsString("Invalid algorithm"));
     }
 
     public void testReservedUserEmptyPasswordAuthenticationFails() throws Throwable {