From e7363905fa3df241eb52e2e0b3e901fa5c838dd6 Mon Sep 17 00:00:00 2001 From: Thomas Darimont Date: Fri, 5 Jan 2024 14:47:18 +0100 Subject: [PATCH] Change password hashing defaults according to OWASP recommendations (#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 #16629 Signed-off-by: Thomas Darimont --- .../threat/password-db-compromised.adoc | 2 +- .../topics/keycloak/changes-24_0_0.adoc | 45 +++++++++ .../hash/Pbkdf2PasswordHashProvider.java | 6 ++ .../Pbkdf2PasswordHashProviderFactory.java | 18 +++- ...kdf2Sha256PasswordHashProviderFactory.java | 5 +- ...kdf2Sha512PasswordHashProviderFactory.java | 5 +- .../org/keycloak/models/PasswordPolicy.java | 5 +- .../testsuite/forms/PasswordHashingTest.java | 95 ++++++++++++++++--- .../testsuite/policy/PasswordPolicyTest.java | 10 ++ 9 files changed, 173 insertions(+), 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 index 0d19b566accd..9b1503989a1c 100644 --- 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. \ No newline at end of file +{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. \ No newline at end of file diff --git a/docs/documentation/upgrading/topics/keycloak/changes-24_0_0.adoc b/docs/documentation/upgrading/topics/keycloak/changes-24_0_0.adoc index 6c4f18fc573e..3ece2dff2cfd 100644 --- 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 ⌀ 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/server-spi-private/src/main/java/org/keycloak/credential/hash/Pbkdf2PasswordHashProvider.java b/server-spi-private/src/main/java/org/keycloak/credential/hash/Pbkdf2PasswordHashProvider.java index 4705db585b1d..756ed3811089 100644 --- a/server-spi-private/src/main/java/org/keycloak/credential/hash/Pbkdf2PasswordHashProvider.java +++ b/server-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 Kunal Kerkar */ 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/server-spi-private/src/main/java/org/keycloak/credential/hash/Pbkdf2PasswordHashProviderFactory.java b/server-spi-private/src/main/java/org/keycloak/credential/hash/Pbkdf2PasswordHashProviderFactory.java index aa796d1ae281..765afb495020 100644 --- a/server-spi-private/src/main/java/org/keycloak/credential/hash/Pbkdf2PasswordHashProviderFactory.java +++ b/server-spi-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 Kunal Kerkar + * @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 Password Storage Cheat Sheet. + */ + 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/server-spi-private/src/main/java/org/keycloak/credential/hash/Pbkdf2Sha256PasswordHashProviderFactory.java b/server-spi-private/src/main/java/org/keycloak/credential/hash/Pbkdf2Sha256PasswordHashProviderFactory.java index cee3fd9e86ba..456bb9487309 100644 --- a/server-spi-private/src/main/java/org/keycloak/credential/hash/Pbkdf2Sha256PasswordHashProviderFactory.java +++ b/server-spi-private/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 Password Storage Cheat Sheet. + */ + public static final int DEFAULT_ITERATIONS = 600_000; @Override public PasswordHashProvider create(KeycloakSession session) { diff --git a/server-spi-private/src/main/java/org/keycloak/credential/hash/Pbkdf2Sha512PasswordHashProviderFactory.java b/server-spi-private/src/main/java/org/keycloak/credential/hash/Pbkdf2Sha512PasswordHashProviderFactory.java index 09103df9edd9..4a50979241a7 100644 --- a/server-spi-private/src/main/java/org/keycloak/credential/hash/Pbkdf2Sha512PasswordHashProviderFactory.java +++ b/server-spi-private/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 Password Storage Cheat Sheet. + */ + 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 index e9e62afe8182..1e9677c04af4 100755 --- 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/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/PasswordHashingTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/PasswordHashingTest.java index 1bc396f313e3..04e686214c07 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/PasswordHashingTest.java +++ b/testsuite/integration-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,7 +239,7 @@ 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); @@ -239,7 +247,7 @@ public void testPbkdf2Sha256WithPadding() throws Exception { 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 plainTextPasswords = IntStream.rangeClosed(1, numberOfPasswords).mapToObj(i -> UUID.randomUUID().toString()).collect(Collectors.toList()); + + Function timeit = runner -> { + + long time = -System.nanoTime(); + runner.run(); + time += System.nanoTime(); + + return Duration.ofNanos((long) (time / ((double) plainTextPasswords.size()))); + }; + + BiFunction 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/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/policy/PasswordPolicyTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/policy/PasswordPolicyTest.java index 9ffbb5558a3b..d1e4901cfd17 100755 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/policy/PasswordPolicyTest.java +++ b/testsuite/integration-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 -> {