From 15d7568792d9e847cb759e9ac7f6384c6f69eedd Mon Sep 17 00:00:00 2001 From: Bill Burke Date: Mon, 7 Jul 2014 12:11:45 -0400 Subject: [PATCH] configurable hash iterations --- .../theme/admin/base/resources/js/services.js | 2 + .../resources/partials/realm-credentials.html | 2 +- .../keycloak/models/CredentialValidation.java | 53 +++++++++++++++++++ .../org/keycloak/models/PasswordPolicy.java | 31 +++++++++++ .../models/UserCredentialValueModel.java | 9 ++++ .../models/entities/CredentialEntity.java | 9 ++++ .../models/utils/Pbkdf2PasswordEncoder.java | 16 +++++- .../keycloak/models/cache/RealmAdapter.java | 9 +--- .../org/keycloak/models/jpa/RealmAdapter.java | 13 +---- .../org/keycloak/models/jpa/UserAdapter.java | 12 ++++- .../models/jpa/entities/CredentialEntity.java | 7 +++ .../mongo/keycloak/adapters/RealmAdapter.java | 8 +-- .../mongo/keycloak/adapters/UserAdapter.java | 13 ++++- .../org/keycloak/model/test/AdapterTest.java | 9 ++++ 14 files changed, 165 insertions(+), 28 deletions(-) create mode 100755 model/api/src/main/java/org/keycloak/models/CredentialValidation.java mode change 100644 => 100755 model/api/src/main/java/org/keycloak/models/PasswordPolicy.java mode change 100644 => 100755 model/api/src/main/java/org/keycloak/models/UserCredentialValueModel.java mode change 100644 => 100755 model/api/src/main/java/org/keycloak/models/entities/CredentialEntity.java diff --git a/forms/common-themes/src/main/resources/theme/admin/base/resources/js/services.js b/forms/common-themes/src/main/resources/theme/admin/base/resources/js/services.js index 415734aa44..57a3a27130 100755 --- a/forms/common-themes/src/main/resources/theme/admin/base/resources/js/services.js +++ b/forms/common-themes/src/main/resources/theme/admin/base/resources/js/services.js @@ -908,6 +908,7 @@ module.factory('PasswordPolicy', function() { var p = {}; p.policyMessages = { + hashIterations: "Number of hashing iterations. Default is 1. Recommended is 50000.", length: "Minimal password length (integer type). Default value is 8.", digits: "Minimal number (integer type) of digits in password. Default value is 1.", lowerCase: "Minimal number (integer type) of lowercase characters in password. Default value is 1.", @@ -916,6 +917,7 @@ module.factory('PasswordPolicy', function() { } p.allPolicies = [ + { name: 'hashIterations', value: 1 }, { name: 'length', value: 8 }, { name: 'digits', value: 1 }, { name: 'lowerCase', value: 1 }, diff --git a/forms/common-themes/src/main/resources/theme/admin/base/resources/partials/realm-credentials.html b/forms/common-themes/src/main/resources/theme/admin/base/resources/partials/realm-credentials.html index 75a4b7aa26..b590604849 100755 --- a/forms/common-themes/src/main/resources/theme/admin/base/resources/partials/realm-credentials.html +++ b/forms/common-themes/src/main/resources/theme/admin/base/resources/partials/realm-credentials.html @@ -50,7 +50,7 @@ - +
diff --git a/model/api/src/main/java/org/keycloak/models/CredentialValidation.java b/model/api/src/main/java/org/keycloak/models/CredentialValidation.java new file mode 100755 index 0000000000..9f8dea74d1 --- /dev/null +++ b/model/api/src/main/java/org/keycloak/models/CredentialValidation.java @@ -0,0 +1,53 @@ +package org.keycloak.models; + +import org.keycloak.models.utils.Pbkdf2PasswordEncoder; + +/** + * @author Bill Burke + * @version $Revision: 1 $ + */ +public class CredentialValidation { + + private static int hashIterations(RealmModel realm) { + PasswordPolicy policy = realm.getPasswordPolicy(); + if (policy != null) { + return policy.getHashIterations(); + } + return -1; + + } + + /** + * Will update password if hash iteration policy has changed + * + * @param realm + * @param user + * @param password + * @return + */ + public static boolean validatePassword(RealmModel realm, UserModel user, String password) { + boolean validated = false; + UserCredentialValueModel passwordCred = null; + for (UserCredentialValueModel cred : user.getCredentialsDirectly()) { + if (cred.getType().equals(UserCredentialModel.PASSWORD)) { + validated = new Pbkdf2PasswordEncoder(cred.getSalt()).verify(password, cred.getValue(), cred.getHashIterations()); + passwordCred = cred; + } + } + if (validated) { + int iterations = hashIterations(realm); + if (iterations > -1 && iterations != passwordCred.getHashIterations()) { + UserCredentialValueModel newCred = new UserCredentialValueModel(); + newCred.setType(passwordCred.getType()); + newCred.setDevice(passwordCred.getDevice()); + newCred.setSalt(passwordCred.getSalt()); + newCred.setHashIterations(iterations); + newCred.setValue(new Pbkdf2PasswordEncoder(newCred.getSalt()).encode(password, iterations)); + user.updateCredentialDirectly(newCred); + } + + } + return validated; + + } +} diff --git a/model/api/src/main/java/org/keycloak/models/PasswordPolicy.java b/model/api/src/main/java/org/keycloak/models/PasswordPolicy.java old mode 100644 new mode 100755 index 99636b95c5..bea471db0f --- a/model/api/src/main/java/org/keycloak/models/PasswordPolicy.java +++ b/model/api/src/main/java/org/keycloak/models/PasswordPolicy.java @@ -52,11 +52,28 @@ public class PasswordPolicy { list.add(new UpperCase(args)); } else if (name.equals(SpecialChars.NAME)) { list.add(new SpecialChars(args)); + } else if (name.equals(HashIterations.NAME)) { + list.add(new HashIterations(args)); } } return list; } + /** + * + * @return -1 if no hash iterations setting + */ + public int getHashIterations() { + if (policies == null) return -1; + for (Policy p : policies) { + if (p instanceof HashIterations) { + return ((HashIterations)p).iterations; + } + + } + return -1; + } + public String validate(String password) { for (Policy p : policies) { String error = p.validate(password); @@ -71,6 +88,20 @@ public class PasswordPolicy { public String validate(String password); } + private static class HashIterations implements Policy { + private static final String NAME = "hashIterations"; + private int iterations; + + public HashIterations(String[] args) { + iterations = intArg(NAME, 1, args); + } + + @Override + public String validate(String password) { + return null; + } + } + private static class Length implements Policy { private static final String NAME = "length"; private int min; diff --git a/model/api/src/main/java/org/keycloak/models/UserCredentialValueModel.java b/model/api/src/main/java/org/keycloak/models/UserCredentialValueModel.java old mode 100644 new mode 100755 index 370227460f..bd1213f32b --- a/model/api/src/main/java/org/keycloak/models/UserCredentialValueModel.java +++ b/model/api/src/main/java/org/keycloak/models/UserCredentialValueModel.java @@ -11,6 +11,7 @@ public class UserCredentialValueModel { private String value; private String device; private byte[] salt; + private int hashIterations; public String getType() { return type; @@ -43,4 +44,12 @@ public class UserCredentialValueModel { public void setSalt(byte[] salt) { this.salt = salt; } + + public int getHashIterations() { + return hashIterations; + } + + public void setHashIterations(int iterations) { + this.hashIterations = iterations; + } } diff --git a/model/api/src/main/java/org/keycloak/models/entities/CredentialEntity.java b/model/api/src/main/java/org/keycloak/models/entities/CredentialEntity.java old mode 100644 new mode 100755 index 82bc922a59..6255836a40 --- a/model/api/src/main/java/org/keycloak/models/entities/CredentialEntity.java +++ b/model/api/src/main/java/org/keycloak/models/entities/CredentialEntity.java @@ -9,6 +9,7 @@ public class CredentialEntity { private String value; private String device; private byte[] salt; + private int hashIterations; public String getType() { return type; @@ -41,4 +42,12 @@ public class CredentialEntity { public void setSalt(byte[] salt) { this.salt = salt; } + + public int getHashIterations() { + return hashIterations; + } + + public void setHashIterations(int hashIterations) { + this.hashIterations = hashIterations; + } } diff --git a/model/api/src/main/java/org/keycloak/models/utils/Pbkdf2PasswordEncoder.java b/model/api/src/main/java/org/keycloak/models/utils/Pbkdf2PasswordEncoder.java index 33c7c41b61..32f0d150be 100755 --- a/model/api/src/main/java/org/keycloak/models/utils/Pbkdf2PasswordEncoder.java +++ b/model/api/src/main/java/org/keycloak/models/utils/Pbkdf2PasswordEncoder.java @@ -43,7 +43,7 @@ public class Pbkdf2PasswordEncoder { * @param rawPassword The password used as a master key to derive into a session key * @return encoded password in Base64 */ - public String encode(String rawPassword) { + public String encode(String rawPassword, int iterations) { String encodedPassword; @@ -59,6 +59,10 @@ public class Pbkdf2PasswordEncoder { return encodedPassword; } + public String encode(String rawPassword) { + return encode(rawPassword, iterations); + } + /** * Encode the password provided and compare with the hash stored into the database * @param rawPassword The password provided @@ -69,6 +73,16 @@ public class Pbkdf2PasswordEncoder { return encode(rawPassword).equals(encodedPassword); } + /** + * Encode the password provided and compare with the hash stored into the database + * @param rawPassword The password provided + * @param encodedPassword Encoded hash stored into the database + * @return true if the password is valid, otherwise false for invalid credentials + */ + public boolean verify(String rawPassword, String encodedPassword, int iterations) { + return encode(rawPassword, iterations).equals(encodedPassword); + } + /** * Generate a salt for each password * @return cryptographically strong random number diff --git a/model/invalidation-cache/model-adapters/src/main/java/org/keycloak/models/cache/RealmAdapter.java b/model/invalidation-cache/model-adapters/src/main/java/org/keycloak/models/cache/RealmAdapter.java index 3b5158c306..2b7a41e7fe 100755 --- a/model/invalidation-cache/model-adapters/src/main/java/org/keycloak/models/cache/RealmAdapter.java +++ b/model/invalidation-cache/model-adapters/src/main/java/org/keycloak/models/cache/RealmAdapter.java @@ -3,6 +3,7 @@ package org.keycloak.models.cache; import org.keycloak.models.ApplicationModel; import org.keycloak.models.AuthenticationProviderModel; import org.keycloak.models.ClientModel; +import org.keycloak.models.CredentialValidation; import org.keycloak.models.OAuthClientModel; import org.keycloak.models.PasswordPolicy; import org.keycloak.models.RealmModel; @@ -380,13 +381,7 @@ public class RealmAdapter implements RealmModel { @Override public boolean validatePassword(UserModel user, String password) { - for (UserCredentialValueModel cred : user.getCredentialsDirectly()) { - if (cred.getType().equals(UserCredentialModel.PASSWORD)) { - return new Pbkdf2PasswordEncoder(cred.getSalt()).verify(password, cred.getValue()); - - } - } - return false; + return CredentialValidation.validatePassword(this, user, password); } @Override diff --git a/model/jpa/src/main/java/org/keycloak/models/jpa/RealmAdapter.java b/model/jpa/src/main/java/org/keycloak/models/jpa/RealmAdapter.java index a985f3d404..e774711903 100755 --- a/model/jpa/src/main/java/org/keycloak/models/jpa/RealmAdapter.java +++ b/model/jpa/src/main/java/org/keycloak/models/jpa/RealmAdapter.java @@ -3,6 +3,7 @@ package org.keycloak.models.jpa; import org.keycloak.models.AuthenticationLinkModel; import org.keycloak.models.AuthenticationProviderModel; import org.keycloak.models.ClientModel; +import org.keycloak.models.CredentialValidation; import org.keycloak.models.KeycloakSession; import org.keycloak.models.ModelDuplicateException; import org.keycloak.models.RoleContainerModel; @@ -962,19 +963,9 @@ public class RealmAdapter implements RealmModel { return role.getContainer().removeRole(role); } - - - - @Override public boolean validatePassword(UserModel user, String password) { - for (UserCredentialValueModel cred : user.getCredentialsDirectly()) { - if (cred.getType().equals(UserCredentialModel.PASSWORD)) { - return new Pbkdf2PasswordEncoder(cred.getSalt()).verify(password, cred.getValue()); - - } - } - return false; + return CredentialValidation.validatePassword(this, user, password); } @Override diff --git a/model/jpa/src/main/java/org/keycloak/models/jpa/UserAdapter.java b/model/jpa/src/main/java/org/keycloak/models/jpa/UserAdapter.java index e86b42fa73..25e6fa324a 100755 --- a/model/jpa/src/main/java/org/keycloak/models/jpa/UserAdapter.java +++ b/model/jpa/src/main/java/org/keycloak/models/jpa/UserAdapter.java @@ -3,6 +3,7 @@ package org.keycloak.models.jpa; import org.keycloak.models.ApplicationModel; import org.keycloak.models.AuthenticationLinkModel; import org.keycloak.models.KeycloakSession; +import org.keycloak.models.PasswordPolicy; import org.keycloak.models.RealmModel; import org.keycloak.models.RoleContainerModel; import org.keycloak.models.RoleModel; @@ -197,8 +198,15 @@ public class UserAdapter implements UserModel { } if (cred.getType().equals(UserCredentialModel.PASSWORD)) { byte[] salt = getSalt(); - credentialEntity.setValue(new Pbkdf2PasswordEncoder(salt).encode(cred.getValue())); + int hashIterations = 1; + PasswordPolicy policy = realm.getPasswordPolicy(); + if (policy != null) { + hashIterations = policy.getHashIterations(); + if (hashIterations == -1) hashIterations = 1; + } + credentialEntity.setValue(new Pbkdf2PasswordEncoder(salt).encode(cred.getValue(), hashIterations)); credentialEntity.setSalt(salt); + credentialEntity.setHashIterations(hashIterations); } else { credentialEntity.setValue(cred.getValue()); } @@ -228,6 +236,7 @@ public class UserAdapter implements UserModel { credModel.setDevice(credEntity.getDevice()); credModel.setValue(credEntity.getValue()); credModel.setSalt(credEntity.getSalt()); + credModel.setHashIterations(credEntity.getHashIterations()); result.add(credModel); } @@ -251,6 +260,7 @@ public class UserAdapter implements UserModel { credentialEntity.setValue(credModel.getValue()); credentialEntity.setSalt(credModel.getSalt()); credentialEntity.setDevice(credModel.getDevice()); + credentialEntity.setHashIterations(credModel.getHashIterations()); em.flush(); } diff --git a/model/jpa/src/main/java/org/keycloak/models/jpa/entities/CredentialEntity.java b/model/jpa/src/main/java/org/keycloak/models/jpa/entities/CredentialEntity.java index e18c2f860f..6d792f887c 100755 --- a/model/jpa/src/main/java/org/keycloak/models/jpa/entities/CredentialEntity.java +++ b/model/jpa/src/main/java/org/keycloak/models/jpa/entities/CredentialEntity.java @@ -28,6 +28,7 @@ public class CredentialEntity { protected String value; protected String device; protected byte[] salt; + protected int hashIterations; @ManyToOne(fetch = FetchType.LAZY) protected UserEntity user; @@ -80,5 +81,11 @@ public class CredentialEntity { this.salt = salt; } + public int getHashIterations() { + return hashIterations; + } + public void setHashIterations(int hashIterations) { + this.hashIterations = hashIterations; + } } diff --git a/model/mongo/src/main/java/org/keycloak/models/mongo/keycloak/adapters/RealmAdapter.java b/model/mongo/src/main/java/org/keycloak/models/mongo/keycloak/adapters/RealmAdapter.java index 2602222e17..0109804437 100755 --- a/model/mongo/src/main/java/org/keycloak/models/mongo/keycloak/adapters/RealmAdapter.java +++ b/model/mongo/src/main/java/org/keycloak/models/mongo/keycloak/adapters/RealmAdapter.java @@ -8,6 +8,7 @@ import org.keycloak.models.ApplicationModel; import org.keycloak.models.AuthenticationLinkModel; import org.keycloak.models.AuthenticationProviderModel; import org.keycloak.models.ClientModel; +import org.keycloak.models.CredentialValidation; import org.keycloak.models.KeycloakSession; import org.keycloak.models.OAuthClientModel; import org.keycloak.models.PasswordPolicy; @@ -808,12 +809,7 @@ public class RealmAdapter extends AbstractMongoAdapter impleme @Override public boolean validatePassword(UserModel user, String password) { - for (UserCredentialValueModel cred : user.getCredentialsDirectly()) { - if (cred.getType().equals(UserCredentialModel.PASSWORD)) { - return new Pbkdf2PasswordEncoder(cred.getSalt()).verify(password, cred.getValue()); - } - } - return false; + return CredentialValidation.validatePassword(this, user, password); } @Override diff --git a/model/mongo/src/main/java/org/keycloak/models/mongo/keycloak/adapters/UserAdapter.java b/model/mongo/src/main/java/org/keycloak/models/mongo/keycloak/adapters/UserAdapter.java index e582b9367d..3a1d14d9fd 100755 --- a/model/mongo/src/main/java/org/keycloak/models/mongo/keycloak/adapters/UserAdapter.java +++ b/model/mongo/src/main/java/org/keycloak/models/mongo/keycloak/adapters/UserAdapter.java @@ -4,6 +4,7 @@ import org.keycloak.models.ApplicationModel; import org.keycloak.models.AuthenticationLinkModel; import org.keycloak.models.ClientModel; import org.keycloak.models.KeycloakSession; +import org.keycloak.models.PasswordPolicy; import org.keycloak.models.RealmModel; import org.keycloak.models.RoleModel; import org.keycloak.models.UserCredentialModel; @@ -199,8 +200,15 @@ public class UserAdapter extends AbstractMongoAdapter implement } if (cred.getType().equals(UserCredentialModel.PASSWORD)) { byte[] salt = Pbkdf2PasswordEncoder.getSalt(); - credentialEntity.setValue(new Pbkdf2PasswordEncoder(salt).encode(cred.getValue())); + int hashIterations = 1; + PasswordPolicy policy = realm.getPasswordPolicy(); + if (policy != null) { + hashIterations = policy.getHashIterations(); + if (hashIterations == -1) hashIterations = 1; + } + credentialEntity.setValue(new Pbkdf2PasswordEncoder(salt).encode(cred.getValue(), hashIterations)); credentialEntity.setSalt(salt); + credentialEntity.setHashIterations(hashIterations); } else { credentialEntity.setValue(cred.getValue()); } @@ -229,6 +237,7 @@ public class UserAdapter extends AbstractMongoAdapter implement credModel.setDevice(credEntity.getDevice()); credModel.setValue(credEntity.getValue()); credModel.setSalt(credEntity.getSalt()); + credModel.setHashIterations(credEntity.getHashIterations()); result.add(credModel); } @@ -249,6 +258,8 @@ public class UserAdapter extends AbstractMongoAdapter implement credentialEntity.setValue(credModel.getValue()); credentialEntity.setSalt(credModel.getSalt()); credentialEntity.setDevice(credModel.getDevice()); + credentialEntity.setHashIterations(credModel.getHashIterations()); + getMongoStore().updateEntity(user, invocationContext); } diff --git a/model/tests/src/test/java/org/keycloak/model/test/AdapterTest.java b/model/tests/src/test/java/org/keycloak/model/test/AdapterTest.java index e3f29ff22e..e5b876663d 100755 --- a/model/tests/src/test/java/org/keycloak/model/test/AdapterTest.java +++ b/model/tests/src/test/java/org/keycloak/model/test/AdapterTest.java @@ -8,11 +8,13 @@ import org.keycloak.models.ApplicationModel; import org.keycloak.models.Constants; import org.keycloak.models.ModelDuplicateException; import org.keycloak.models.OAuthClientModel; +import org.keycloak.models.PasswordPolicy; import org.keycloak.models.RealmModel; import org.keycloak.models.RequiredCredentialModel; import org.keycloak.models.RoleModel; import org.keycloak.models.SocialLinkModel; import org.keycloak.models.UserCredentialModel; +import org.keycloak.models.UserCredentialValueModel; import org.keycloak.models.UserModel; import org.keycloak.models.UserSessionModel; import org.keycloak.representations.idm.CredentialRepresentation; @@ -135,6 +137,13 @@ public class AdapterTest extends AbstractModelTest { cred.setValue("geheim"); user.updateCredential(cred); Assert.assertTrue(realmModel.validatePassword(user, "geheim")); + List creds = user.getCredentialsDirectly(); + Assert.assertEquals(creds.get(0).getHashIterations(), 1); + realmModel.setPasswordPolicy( new PasswordPolicy("hashIterations(200)")); + Assert.assertTrue(realmModel.validatePassword(user, "geheim")); + creds = user.getCredentialsDirectly(); + Assert.assertEquals(creds.get(0).getHashIterations(), 200); + realmModel.setPasswordPolicy( new PasswordPolicy("hashIterations(1)")); } @Test