Change password hashing defaults according to OWASP recommendations (k…

…eycloak#16629) Changes according to the latest [OWASP cheat sheet for secure Password Storage](https://cheatsheetseries.owasp.org/cheatsheets/Password_Storage_Cheat_Sheet.html#pbkdf2): - Changed default password hashing algorithm from pbkdf2-sha256 to pbkdf2-sha512 - Increased number of hash iterations for pbkdf2-sha1 from 20.000 to 1.300.000 - Increased number of hash iterations for pbkdf2-sha256 from 27.500 to 600.000 - Increased number of hash iterations for pbkdf2-sha512 from 30.000 to 210.000 - Adapt PasswordHashingTest to new defaults - The test testBenchmarkPasswordHashingConfigurations can be used to compare the different hashing configurations. - Document changes in changes document with note on performance and how to keep the old behaviour. - Log a warning at the first time when Pbkdf2PasswordHashProviderFactory is used directly Fixes keycloak#16629 Signed-off-by: Thomas Darimont <[email protected]>
tnorimat · Jan 24, 2024 · e736390 · e736390
1 parent 208e3a6
commit e736390
Show file tree

Hide file tree

Showing 9 changed files with 173 additions and 18 deletions.
diff --git a/docs/documentation/server_admin/topics/threat/password-db-compromised.adoc b/docs/documentation/server_admin/topics/threat/password-db-compromised.adoc
@@ -1,4 +1,4 @@
 
 === Password database compromised
 
-{project_name} does not store passwords in raw text but as hashed text, using the PBKDF2 hashing algorithm. {project_name} performs 27,500 hashing iterations, the number of iterations recommended by the security community. This number of hashing iterations can adversely affect performance as PBKDF2 hashing uses a significant amount of CPU resources.
+{project_name} does not store passwords in raw text but as hashed text, using the `PBKDF2-HMAC-SHA512` message digest algorithm. {project_name} performs `210,000` hashing iterations, the number of iterations recommended by the security community. This number of hashing iterations can adversely affect performance as PBKDF2 hashing uses a significant amount of CPU resources.
diff --git a/docs/documentation/upgrading/topics/keycloak/changes-24_0_0.adoc b/docs/documentation/upgrading/topics/keycloak/changes-24_0_0.adoc
@@ -225,6 +225,51 @@ PUT /admin/realms/{realm}/users/{id}/execute-actions-email
 
 The compatibility mode for SAML encryption introduced in version 21 is now removed. The system property `keycloak.saml.deprecated.encryption` is not managed anymore by the server. The clients which still used the old signing key for encryption should update it from the new IDP configuration metadata.
 
+= Changes to Password Hashing
+
+In this release we adapted the password hashing defaults to match the https://cheatsheetseries.owasp.org/cheatsheets/Password_Storage_Cheat_Sheet.html#pbkdf2[OWASP recommendations for Password Storage].
+
+As part of this change, the default password hashing provider has changed from `pbkdf2-sha256` to `pbkdf2-sha512`.
+Also, the number of default hash iterations for `pbkdf2` based password hashing algorithms changed as follows:
+[%autowidth,cols="a,a,>a,>a"]
+|===
+| Provider ID | Algorithm | Old Iterations | New Iterations
+
+| `pbkdf2`   | `PBKDF2WithHmacSHA1` | 20.000 | 1.300.000
+| `pbkdf2-sha256` | `PBKDF2WithHmacSHA256` | 27.500 | 600.000
+| `pbkdf2-sha512` | `PBKDF2WithHmacSHA512` | 30.000 | 210.000
+|===
+
+If a realm does not explicitly configure a password policy with `hashAlgorithm` and `hashIterations`, then
+the new configuration will take effect on the next password based login, or when a user password is created or updated.
+
+Note that the increased iteration counts can have a significant impact on the required CPU resources.
+
+== Performance of new password hashing configuration
+
+Tests on a machine with an Intel i9-8950HK CPU (12) @ 4.800GHz yielded the following &#8960; time differences for hashing 1000 passwords (averages from 3 runs).
+Note that the average duration for the `PBKDF2WithHmacSHA1` was computed with a lower number of passwords due to the long runtime.
+[%autowidth,cols="a,a,>a,>a,>a"]
+|===
+| Provider ID | Algorithm | Old duration | New duration | Difference
+
+| `pbkdf2` | `PBKDF2WithHmacSHA1`   | 122ms | 3.114ms | +2.992ms
+| `pbkdf2-sha256` | `PBKDF2WithHmacSHA256` |  20ms |   451ms |   +431ms
+| `pbkdf2-sha512` | `PBKDF2WithHmacSHA512` |  33ms |   224ms |   +191ms
+|===
+
+Users of the `pbkdf2` provider might need to explicitly reduce the
+number of hash iterations to regain acceptable performance.
+This can be done by configuring the hash iterations explicitly in the password policy of the realm.
+
+== How to keep using the old pbkdf2-sha256 password hashing?
+
+To keep the old password hashing for a realm, specify `hashAlgorithm` and `hashIterations` explicitly in the
+realm password policy.
+
+* `Hashing Algorithm: pbkdf2-sha256`
+* `Hashing Iterations: 27500`
+
 = Renaming JPA provider configuration options for migration
 
 After removal of the Map Store the following configuration options were renamed:

diff --git a/...er-spi-private/src/main/java/org/keycloak/credential/hash/Pbkdf2PasswordHashProvider.java b/...er-spi-private/src/main/java/org/keycloak/credential/hash/Pbkdf2PasswordHashProvider.java
@@ -33,6 +33,8 @@
 import java.security.spec.KeySpec;
 
 /**
+ * Implementation PBKDF2 password hash algorithm.
+ *
  * @author <a href="mailto:[email protected]">Kunal Kerkar</a>
  */
 public class Pbkdf2PasswordHashProvider implements PasswordHashProvider {
@@ -137,4 +139,8 @@ private SecretKeyFactory getSecretKeyFactory() {
             throw new RuntimeException("PBKDF2 algorithm not found", e);
         }
     }
+
+    public String getPbkdf2Algorithm() {
+        return pbkdf2Algorithm;
+    }
 }
diff --git a/...private/src/main/java/org/keycloak/credential/hash/Pbkdf2PasswordHashProviderFactory.java b/...private/src/main/java/org/keycloak/credential/hash/Pbkdf2PasswordHashProviderFactory.java
@@ -17,21 +17,37 @@
 
 package org.keycloak.credential.hash;
 
+import org.jboss.logging.Logger;
 import org.keycloak.models.KeycloakSession;
 
 /**
+ * Provider factory for SHA1 variant of the PBKDF2 password hash algorithm.
+ *
  * @author <a href="mailto:[email protected]">Kunal Kerkar</a>
+ * @deprecated The PBKDF2 provider with SHA1 and the recommended number of 1.300.000 iterations is known to be very slow. We recommend to use the PBKDF2 variants with SHA256 or SHA512 instead.
  */
+@Deprecated
 public class Pbkdf2PasswordHashProviderFactory extends AbstractPbkdf2PasswordHashProviderFactory implements PasswordHashProviderFactory {
 
+    private static final Logger LOG = Logger.getLogger(Pbkdf2PasswordHashProviderFactory.class);
+
     public static final String ID = "pbkdf2";
 
     public static final String PBKDF2_ALGORITHM = "PBKDF2WithHmacSHA1";
 
-    public static final int DEFAULT_ITERATIONS = 20000;
+    /**
+     * Hash iterations for PBKDF2-HMAC-SHA1 according to the <a href="https://cheatsheetseries.owasp.org/cheatsheets/Password_Storage_Cheat_Sheet.html#pbkdf2">Password Storage Cheat Sheet</a>.
+     */
+    public static final int DEFAULT_ITERATIONS = 1_300_000;
+
+    private static boolean usageWarningPrinted;
 
     @Override
     public PasswordHashProvider create(KeycloakSession session) {
+        if (!usageWarningPrinted) {
+            LOG.warnf("Detected usage of password hashing provider '%s'. The provider is no longer recommended, use 'pbkdf2-sha256' or 'pbkdf2-sha512' instead.", ID);
+            usageWarningPrinted = true;
+        }
         return new Pbkdf2PasswordHashProvider(ID, PBKDF2_ALGORITHM, DEFAULT_ITERATIONS, getMaxPaddingLength());
     }
 

diff --git a/...e/src/main/java/org/keycloak/credential/hash/Pbkdf2Sha256PasswordHashProviderFactory.java b/...e/src/main/java/org/keycloak/credential/hash/Pbkdf2Sha256PasswordHashProviderFactory.java
@@ -13,7 +13,10 @@ public class Pbkdf2Sha256PasswordHashProviderFactory extends AbstractPbkdf2Passw
 
     public static final String PBKDF2_ALGORITHM = "PBKDF2WithHmacSHA256";
 
-    public static final int DEFAULT_ITERATIONS = 27500;
+    /**
+     * Hash iterations for PBKDF2-HMAC-SHA256 according to the <a href="https://cheatsheetseries.owasp.org/cheatsheets/Password_Storage_Cheat_Sheet.html#pbkdf2">Password Storage Cheat Sheet</a>.
+     */
+    public static final int DEFAULT_ITERATIONS = 600_000;
 
     @Override
     public PasswordHashProvider create(KeycloakSession session) {

diff --git a/...e/src/main/java/org/keycloak/credential/hash/Pbkdf2Sha512PasswordHashProviderFactory.java b/...e/src/main/java/org/keycloak/credential/hash/Pbkdf2Sha512PasswordHashProviderFactory.java
@@ -13,7 +13,10 @@ public class Pbkdf2Sha512PasswordHashProviderFactory extends AbstractPbkdf2Passw
 
     public static final String PBKDF2_ALGORITHM = "PBKDF2WithHmacSHA512";
 
-    public static final int DEFAULT_ITERATIONS = 30000;
+    /**
+     * Hash iterations for PBKDF2-HMAC-SHA512 according to the <a href="https://cheatsheetseries.owasp.org/cheatsheets/Password_Storage_Cheat_Sheet.html#pbkdf2">Password Storage Cheat Sheet</a>.
+     */
+    public static final int DEFAULT_ITERATIONS = 210_000;
 
     @Override
     public PasswordHashProvider create(KeycloakSession session) {

diff --git a/server-spi/src/main/java/org/keycloak/models/PasswordPolicy.java b/server-spi/src/main/java/org/keycloak/models/PasswordPolicy.java
@@ -17,7 +17,6 @@
 
 package org.keycloak.models;
 
-import org.keycloak.crypto.Algorithm;
 import org.keycloak.policy.PasswordPolicyConfigException;
 import org.keycloak.policy.PasswordPolicyProvider;
 
@@ -34,11 +33,11 @@ public class PasswordPolicy implements Serializable {
 
     public static final String HASH_ALGORITHM_ID = "hashAlgorithm";
 
-    public static final String HASH_ALGORITHM_DEFAULT = "pbkdf2-sha256";
+    public static final String HASH_ALGORITHM_DEFAULT = "pbkdf2-sha512";
 
     public static final String HASH_ITERATIONS_ID = "hashIterations";
 
-    public static final int HASH_ITERATIONS_DEFAULT = 27500;
+    public static final int HASH_ITERATIONS_DEFAULT = 210_000;
 
     public static final String PASSWORD_HISTORY_ID = "passwordHistory";
 

diff --git a/...arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/PasswordHashingTest.java b/...arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/PasswordHashingTest.java
@@ -16,11 +16,13 @@
  */
 package org.keycloak.testsuite.forms;
 
+import jakarta.ws.rs.BadRequestException;
 import org.jboss.arquillian.graphene.page.Page;
 import org.junit.Test;
 import org.keycloak.common.util.Base64;
 import org.keycloak.credential.CredentialModel;
 import org.keycloak.credential.hash.PasswordHashProvider;
+import org.keycloak.credential.hash.PasswordHashProviderFactory;
 import org.keycloak.credential.hash.Pbkdf2PasswordHashProvider;
 import org.keycloak.credential.hash.Pbkdf2PasswordHashProviderFactory;
 import org.keycloak.credential.hash.Pbkdf2Sha256PasswordHashProviderFactory;
@@ -40,8 +42,14 @@
 
 import javax.crypto.SecretKeyFactory;
 import javax.crypto.spec.PBEKeySpec;
-import jakarta.ws.rs.BadRequestException;
 import java.security.spec.KeySpec;
+import java.time.Duration;
+import java.util.List;
+import java.util.UUID;
+import java.util.function.BiFunction;
+import java.util.function.Function;
+import java.util.stream.Collectors;
+import java.util.stream.IntStream;
 
 import static org.junit.Assert.assertArrayEquals;
 import static org.junit.Assert.assertEquals;
@@ -114,7 +122,7 @@ public void testPasswordRehashedOnIterationsChanged() throws Exception {
         credential = PasswordCredentialModel.createFromCredentialModel(fetchCredentials(username));
 
         assertEquals(1, credential.getPasswordCredentialData().getHashIterations());
-        assertEncoded(credential, "password", credential.getPasswordSecretData().getSalt(), "PBKDF2WithHmacSHA256", 1);
+        assertEncoded(credential, "password", credential.getPasswordSecretData().getSalt(), "PBKDF2WithHmacSHA512", 1);
     }
 
     // KEYCLOAK-5282
@@ -139,7 +147,7 @@ public void testPasswordNotRehasedUnchangedIterations() {
         assertEquals(credentialId, credential.getId());
         assertArrayEquals(salt, credential.getPasswordSecretData().getSalt());
 
-        setPasswordPolicy("hashIterations(" + Pbkdf2Sha256PasswordHashProviderFactory.DEFAULT_ITERATIONS + ")");
+        setPasswordPolicy("hashIterations(" + Pbkdf2Sha512PasswordHashProviderFactory.DEFAULT_ITERATIONS + ")");
 
         AccountHelper.logout(adminClient.realm("test"), username);
 
@@ -154,7 +162,7 @@ public void testPasswordNotRehasedUnchangedIterations() {
 
     @Test
     public void testPasswordRehashedWhenCredentialImportedWithDifferentKeySize() {
-        setPasswordPolicy("hashAlgorithm(" + Pbkdf2Sha512PasswordHashProviderFactory.ID + ") and hashIterations("+ Pbkdf2Sha512PasswordHashProviderFactory.DEFAULT_ITERATIONS + ")");
+        setPasswordPolicy("hashAlgorithm(" + Pbkdf2Sha512PasswordHashProviderFactory.ID + ") and hashIterations(" + Pbkdf2Sha512PasswordHashProviderFactory.DEFAULT_ITERATIONS + ")");
 
         String username = "testPasswordRehashedWhenCredentialImportedWithDifferentKeySize";
         String password = "password";
@@ -169,7 +177,7 @@ public void testPasswordRehashedWhenCredentialImportedWithDifferentKeySize() {
 
         // Create a user with the encoded password, simulating a user import from a different system using a specific key size
         UserRepresentation user = UserBuilder.create().username(username).password(encodedPassword).build();
-        ApiUtil.createUserWithAdminClient(adminClient.realm("test"),user);
+        ApiUtil.createUserWithAdminClient(adminClient.realm("test"), user);
 
         loginPage.open();
         loginPage.login(username, password);
@@ -187,7 +195,7 @@ public void testPbkdf2Sha1() throws Exception {
         createUser(username);
 
         PasswordCredentialModel credential = PasswordCredentialModel.createFromCredentialModel(fetchCredentials(username));
-        assertEncoded(credential, "password", credential.getPasswordSecretData().getSalt(), "PBKDF2WithHmacSHA1", 20000);
+        assertEncoded(credential, "password", credential.getPasswordSecretData().getSalt(), "PBKDF2WithHmacSHA1", Pbkdf2PasswordHashProviderFactory.DEFAULT_ITERATIONS);
     }
 
     @Test
@@ -197,7 +205,7 @@ public void testDefault() throws Exception {
         createUser(username);
 
         PasswordCredentialModel credential = PasswordCredentialModel.createFromCredentialModel(fetchCredentials(username));
-        assertEncoded(credential, "password", credential.getPasswordSecretData().getSalt(), "PBKDF2WithHmacSHA256", 27500);
+        assertEncoded(credential, "password", credential.getPasswordSecretData().getSalt(), "PBKDF2WithHmacSHA512", Pbkdf2Sha512PasswordHashProviderFactory.DEFAULT_ITERATIONS);
     }
 
     @Test
@@ -207,7 +215,7 @@ public void testPbkdf2Sha256() throws Exception {
         createUser(username);
 
         PasswordCredentialModel credential = PasswordCredentialModel.createFromCredentialModel(fetchCredentials(username));
-        assertEncoded(credential, "password", credential.getPasswordSecretData().getSalt(), "PBKDF2WithHmacSHA256", 27500);
+        assertEncoded(credential, "password", credential.getPasswordSecretData().getSalt(), "PBKDF2WithHmacSHA256", Pbkdf2Sha256PasswordHashProviderFactory.DEFAULT_ITERATIONS);
     }
 
     @Test
@@ -217,7 +225,7 @@ public void testPbkdf2Sha512() throws Exception {
         createUser(username);
 
         PasswordCredentialModel credential = PasswordCredentialModel.createFromCredentialModel(fetchCredentials(username));
-        assertEncoded(credential, "password", credential.getPasswordSecretData().getSalt(), "PBKDF2WithHmacSHA512", 30000);
+        assertEncoded(credential, "password", credential.getPasswordSecretData().getSalt(), "PBKDF2WithHmacSHA512", Pbkdf2Sha512PasswordHashProviderFactory.DEFAULT_ITERATIONS);
     }
 
     @Test
@@ -231,15 +239,15 @@ public void testPbkdf2Sha256WithPadding() throws Exception {
             createUser(username1);
 
             PasswordCredentialModel credential = PasswordCredentialModel.createFromCredentialModel(fetchCredentials(username1));
-            assertEncoded(credential, "password", credential.getPasswordSecretData().getSalt(), "PBKDF2WithHmacSHA256", 27500);
+            assertEncoded(credential, "password", credential.getPasswordSecretData().getSalt(), "PBKDF2WithHmacSHA256", Pbkdf2Sha256PasswordHashProviderFactory.DEFAULT_ITERATIONS);
 
             // Now configure padding to bigger than 64. The verification without padding would fail as for longer padding than 64 characters, the hashes of the padded password and unpadded password would be different
             configurePaddingForKeycloak(65);
             String username2 = "test2-Pbkdf2Sha2562";
             createUser(username2);
 
             credential = PasswordCredentialModel.createFromCredentialModel(fetchCredentials(username2));
-            assertEncoded(credential, "password", credential.getPasswordSecretData().getSalt(), "PBKDF2WithHmacSHA256", 27500, false);
+            assertEncoded(credential, "password", credential.getPasswordSecretData().getSalt(), "PBKDF2WithHmacSHA256", Pbkdf2Sha256PasswordHashProviderFactory.DEFAULT_ITERATIONS, false);
 
         } finally {
             configurePaddingForKeycloak(originalPaddingLength);
@@ -295,4 +303,69 @@ private int configurePaddingForKeycloak(int paddingLength) {
         }, Integer.class);
     }
 
+    /**
+     * Simple test to compare runtimes of different password hashing configurations.
+     */
+//    @Test
+    public void testBenchmarkPasswordHashingConfigurations() {
+
+        int numberOfPasswords = 1000;
+        List<String> plainTextPasswords = IntStream.rangeClosed(1, numberOfPasswords).mapToObj(i -> UUID.randomUUID().toString()).collect(Collectors.toList());
+
+        Function<Runnable, Duration> timeit = runner -> {
+
+            long time = -System.nanoTime();
+            runner.run();
+            time += System.nanoTime();
+
+            return Duration.ofNanos((long) (time / ((double) plainTextPasswords.size())));
+        };
+
+        BiFunction<PasswordHashProvider, Integer, Long> hasher = (provider, iterations) -> {
+            long result = 0L;
+            for (String password : plainTextPasswords) {
+                String encoded = provider.encode(password, iterations);
+                result += encoded.hashCode();
+            }
+            return result;
+        };
+
+        var comparisons = List.of(
+                // this takes quite a long time. Run this with a low value numberOfPasswords, e.g. 1-10
+                // new PasswordHashComparison(new Pbkdf2PasswordHashProviderFactory(), 20_000, 1_300_000),
+
+                new PasswordHashComparison(new Pbkdf2Sha256PasswordHashProviderFactory(), 27_500, 600_000),
+                new PasswordHashComparison(new Pbkdf2Sha512PasswordHashProviderFactory(), 30_000, 210_000)
+        );
+
+        comparisons.forEach(comp -> {
+            Pbkdf2PasswordHashProvider hashProvider = (Pbkdf2PasswordHashProvider) comp.factory.create(null);
+            System.out.printf("Hashing %s password(s) with %s%n", plainTextPasswords.size(), hashProvider.getPbkdf2Algorithm());
+
+            var durationOld = timeit.apply(() -> hasher.apply(hashProvider, comp.iterationsOld));
+            System.out.printf("\tØ hashing duration with %d iterations: %sms%n", comp.iterationsOld, durationOld.toMillis());
+
+            var durationNew = timeit.apply(() -> hasher.apply(hashProvider, comp.iterationsNew));
+            System.out.printf("\tØ hashing duration with %d iterations: %sms%n", comp.iterationsNew, durationNew.toMillis());
+
+            var deltaTimeMillis = durationNew.toMillis() - durationOld.toMillis();
+            System.out.printf("\tDifference: +%s ms%n", deltaTimeMillis);
+        });
+    }
+
+
+    class PasswordHashComparison {
+
+        final PasswordHashProviderFactory factory;
+
+        final int iterationsOld;
+
+        final int iterationsNew;
+
+        public PasswordHashComparison(PasswordHashProviderFactory factory, int iterationsOld, int iterationsNew) {
+            this.factory = factory;
+            this.iterationsOld = iterationsOld;
+            this.iterationsNew = iterationsNew;
+        }
+    }
 }
diff --git a/...arquillian/tests/base/src/test/java/org/keycloak/testsuite/policy/PasswordPolicyTest.java b/...arquillian/tests/base/src/test/java/org/keycloak/testsuite/policy/PasswordPolicyTest.java
@@ -19,6 +19,7 @@
 
 import org.junit.Assert;
 import org.junit.Test;
+import org.keycloak.credential.hash.Pbkdf2Sha512PasswordHashProviderFactory;
 import org.keycloak.models.ModelException;
 import org.keycloak.models.PasswordPolicy;
 import org.keycloak.models.RealmModel;
@@ -49,6 +50,15 @@
  */
 public class PasswordPolicyTest extends AbstractKeycloakTest {
 
+    @Test
+    public void testDefaultPasswordPolicySettings() {
+        testingClient.server("passwordPolicy").run(session -> {
+            RealmModel realmModel = session.getContext().getRealm();
+            PasswordPolicy passwordPolicy = realmModel.getPasswordPolicy();
+            Assert.assertEquals(Pbkdf2Sha512PasswordHashProviderFactory.ID, passwordPolicy.getHashAlgorithm());
+        });
+    }
+
     @Test
     public void testLength() {
         testingClient.server("passwordPolicy").run(session -> {