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.
elastic · Aug 14, 2018 · ac5ef8c · ac5ef8c
1 parent 7d8a64d
commit ac5ef8c
Show file tree

Hide file tree

Showing 6 changed files with 43 additions and 89 deletions.
diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/XPackSettings.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/XPackSettings.java
@@ -14,6 +14,7 @@
 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 @@ private 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);
 

diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/XPackSettingsTests.java b/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 void testDefaultSSLCiphers() throws Exception {
             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;
+        }
+    }
 }
diff --git a/...rc/main/java/org/elasticsearch/xpack/security/PasswordHashingAlgorithmBootstrapCheck.java b/...rc/main/java/org/elasticsearch/xpack/security/PasswordHashingAlgorithmBootstrapCheck.java
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
@@ -300,7 +300,6 @@ public Security(Settings settings, final Path configPath) {
                 new TokenSSLBootstrapCheck(),
                 new PkiRealmBootstrapCheck(getSslService()),
                 new TLSLicenseBootstrapCheck(),
-                new PasswordHashingAlgorithmBootstrapCheck(),
                 new FIPS140SecureSettingsBootstrapCheck(settings, env),
                 new FIPS140JKSKeystoreBootstrapCheck(settings),
                 new FIPS140PasswordHashingAlgorithmBootstrapCheck(settings)));

diff --git a/...st/java/org/elasticsearch/xpack/security/PasswordHashingAlgorithmBootstrapCheckTests.java b/...st/java/org/elasticsearch/xpack/security/PasswordHashingAlgorithmBootstrapCheckTests.java
diff --git a/...ity/src/test/java/org/elasticsearch/xpack/security/authc/esnative/ReservedRealmTests.java b/...ity/src/test/java/org/elasticsearch/xpack/security/authc/esnative/ReservedRealmTests.java
@@ -83,7 +83,7 @@ public void testInvalidHashingAlgorithmFails() {
         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 {