From 8cbd39083ec74fe71d39b2c47fbc9c9b67cf08ca Mon Sep 17 00:00:00 2001 From: Stian Thorgersen Date: Fri, 22 Mar 2024 12:44:11 +0100 Subject: [PATCH] Default password hashing algorithm should be set to default password hash provider (#28128) Closes #28120 Signed-off-by: stianst --- .../hash/Pbkdf2PasswordHashProvider.java | 2 +- .../Pbkdf2PasswordHashProviderFactory.java | 5 ++ ...kdf2Sha256PasswordHashProviderFactory.java | 5 ++ ...kdf2Sha512PasswordHashProviderFactory.java | 5 ++ ...lgorithmPasswordPolicyProviderFactory.java | 13 +++-- ...erationsPasswordPolicyProviderFactory.java | 2 +- .../org/keycloak/models/PasswordPolicy.java | 6 +-- .../AbstractUsernameFormAuthenticator.java | 25 +++------ .../PasswordCredentialProvider.java | 51 +++++++++++-------- .../BackwardsCompatibilityUserStorage.java | 12 ++--- .../keycloak/testsuite/admin/UserTest.java | 13 ++--- .../FederatedStorageExportImportTest.java | 8 +-- .../testsuite/forms/PasswordHashingTest.java | 21 ++++++++ .../testsuite/policy/PasswordPolicyTest.java | 10 ---- 14 files changed, 102 insertions(+), 76 deletions(-) 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 756ed38110..a25a570bff 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 @@ -61,7 +61,7 @@ public class Pbkdf2PasswordHashProvider implements PasswordHashProvider { @Override public boolean policyCheck(PasswordPolicy policy, PasswordCredentialModel credential) { - int policyHashIterations = policy.getHashIterations(); + int policyHashIterations = policy != null ? policy.getHashIterations() : -1; if (policyHashIterations == -1) { policyHashIterations = defaultIterations; } 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 765afb4950..aea95988ce 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 @@ -55,4 +55,9 @@ public class Pbkdf2PasswordHashProviderFactory extends AbstractPbkdf2PasswordHas public String getId() { return ID; } + + @Override + public int order() { + return -100; + } } 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 456bb94873..69f01b7901 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 @@ -27,4 +27,9 @@ public class Pbkdf2Sha256PasswordHashProviderFactory extends AbstractPbkdf2Passw public String getId() { return ID; } + + @Override + public int order() { + return 100; + } } 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 4a50979241..7bc8b56c04 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 @@ -27,4 +27,9 @@ public class Pbkdf2Sha512PasswordHashProviderFactory extends AbstractPbkdf2Passw public String getId() { return ID; } + + @Override + public int order() { + return 200; + } } diff --git a/server-spi-private/src/main/java/org/keycloak/policy/HashAlgorithmPasswordPolicyProviderFactory.java b/server-spi-private/src/main/java/org/keycloak/policy/HashAlgorithmPasswordPolicyProviderFactory.java index 3a51ee6ef1..a0e6ae595d 100644 --- a/server-spi-private/src/main/java/org/keycloak/policy/HashAlgorithmPasswordPolicyProviderFactory.java +++ b/server-spi-private/src/main/java/org/keycloak/policy/HashAlgorithmPasswordPolicyProviderFactory.java @@ -32,6 +32,8 @@ public class HashAlgorithmPasswordPolicyProviderFactory implements PasswordPolic private KeycloakSession session; + private String defaultHashAlgorithm; + @Override public PasswordPolicyProvider create(KeycloakSession session) { this.session = session; @@ -44,6 +46,7 @@ public class HashAlgorithmPasswordPolicyProviderFactory implements PasswordPolic @Override public void postInit(KeycloakSessionFactory factory) { + defaultHashAlgorithm = factory.getProviderFactory(PasswordHashProvider.class).getId(); } @Override @@ -77,7 +80,7 @@ public class HashAlgorithmPasswordPolicyProviderFactory implements PasswordPolic @Override public String getDefaultConfigValue() { - return PasswordPolicy.HASH_ALGORITHM_DEFAULT; + return defaultHashAlgorithm; } @Override @@ -87,12 +90,14 @@ public class HashAlgorithmPasswordPolicyProviderFactory implements PasswordPolic @Override public Object parseConfig(String value) { - String providerId = value != null && value.length() > 0 ? value : PasswordPolicy.HASH_ALGORITHM_DEFAULT; - PasswordHashProvider provider = session.getProvider(PasswordHashProvider.class, providerId); + if (value == null) { + throw new PasswordPolicyConfigException("Password hashing provider id must be set"); + } + PasswordHashProvider provider = session.getProvider(PasswordHashProvider.class, value); if (provider == null) { throw new PasswordPolicyConfigException("Password hashing provider not found"); } - return providerId; + return value; } } diff --git a/server-spi-private/src/main/java/org/keycloak/policy/HashIterationsPasswordPolicyProviderFactory.java b/server-spi-private/src/main/java/org/keycloak/policy/HashIterationsPasswordPolicyProviderFactory.java index be4dd454ed..9339061d05 100644 --- a/server-spi-private/src/main/java/org/keycloak/policy/HashIterationsPasswordPolicyProviderFactory.java +++ b/server-spi-private/src/main/java/org/keycloak/policy/HashIterationsPasswordPolicyProviderFactory.java @@ -75,7 +75,7 @@ public class HashIterationsPasswordPolicyProviderFactory implements PasswordPoli @Override public String getDefaultConfigValue() { - return String.valueOf(PasswordPolicy.HASH_ITERATIONS_DEFAULT); + return "-1"; } @Override 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 1e9677c04a..7bcc2bdc44 100755 --- a/server-spi/src/main/java/org/keycloak/models/PasswordPolicy.java +++ b/server-spi/src/main/java/org/keycloak/models/PasswordPolicy.java @@ -33,12 +33,8 @@ public class PasswordPolicy implements Serializable { public static final String HASH_ALGORITHM_ID = "hashAlgorithm"; - public static final String HASH_ALGORITHM_DEFAULT = "pbkdf2-sha512"; - public static final String HASH_ITERATIONS_ID = "hashIterations"; - public static final int HASH_ITERATIONS_DEFAULT = 210_000; - public static final String PASSWORD_HISTORY_ID = "passwordHistory"; public static final String FORCE_EXPIRED_ID = "forceExpiredPasswordChange"; @@ -81,7 +77,7 @@ public class PasswordPolicy implements Serializable { if (policyConfig.containsKey(HASH_ALGORITHM_ID)) { return getPolicyConfig(HASH_ALGORITHM_ID); } else { - return HASH_ALGORITHM_DEFAULT; + return null; } } diff --git a/services/src/main/java/org/keycloak/authentication/authenticators/browser/AbstractUsernameFormAuthenticator.java b/services/src/main/java/org/keycloak/authentication/authenticators/browser/AbstractUsernameFormAuthenticator.java index 6994f2fe4d..feb7bc4738 100755 --- a/services/src/main/java/org/keycloak/authentication/authenticators/browser/AbstractUsernameFormAuthenticator.java +++ b/services/src/main/java/org/keycloak/authentication/authenticators/browser/AbstractUsernameFormAuthenticator.java @@ -100,27 +100,16 @@ public abstract class AbstractUsernameFormAuthenticator extends AbstractFormAuth return challengeResponse; } - protected void runDefaultDummyHash(AuthenticationFlowContext context) { - PasswordHashProvider hash = context.getSession().getProvider(PasswordHashProvider.class, PasswordPolicy.HASH_ALGORITHM_DEFAULT); - hash.encode("SlightlyLongerDummyPassword", PasswordPolicy.HASH_ITERATIONS_DEFAULT); - } - protected void dummyHash(AuthenticationFlowContext context) { - PasswordPolicy policy = context.getRealm().getPasswordPolicy(); - if (policy == null) { - runDefaultDummyHash(context); - return; + PasswordPolicy passwordPolicy = context.getRealm().getPasswordPolicy(); + PasswordHashProvider provider; + if (passwordPolicy != null && passwordPolicy.getHashAlgorithm() != null) { + provider = context.getSession().getProvider(PasswordHashProvider.class, passwordPolicy.getHashAlgorithm()); } else { - PasswordHashProvider hash = context.getSession().getProvider(PasswordHashProvider.class, policy.getHashAlgorithm()); - if (hash == null) { - runDefaultDummyHash(context); - return; - - } else { - hash.encode("SlightlyLongerDummyPassword", policy.getHashIterations()); - } + provider = context.getSession().getProvider(PasswordHashProvider.class); } - + int iterations = passwordPolicy != null ? passwordPolicy.getHashIterations() : -1; + provider.encode("SlightlyLongerDummyPassword", iterations); } public void testInvalidUser(AuthenticationFlowContext context, UserModel user) { diff --git a/services/src/main/java/org/keycloak/credential/PasswordCredentialProvider.java b/services/src/main/java/org/keycloak/credential/PasswordCredentialProvider.java index 8cb7fc3908..95277d88a7 100644 --- a/services/src/main/java/org/keycloak/credential/PasswordCredentialProvider.java +++ b/services/src/main/java/org/keycloak/credential/PasswordCredentialProvider.java @@ -124,12 +124,16 @@ public class PasswordCredentialProvider implements CredentialProvider { - RealmModel realmModel = session.getContext().getRealm(); - PasswordPolicy passwordPolicy = realmModel.getPasswordPolicy(); - Assert.assertEquals(Pbkdf2Sha512PasswordHashProviderFactory.ID, passwordPolicy.getHashAlgorithm()); - }); - } - @Test public void testLength() { testingClient.server("passwordPolicy").run(session -> {