Integrate the `PasswordHash` class and subclass in the user and password editing and authentication. Replaces the old code and the previous `SecurePasswordHashingUtils` class.tags/r1.9.0
@@ -854,14 +854,14 @@ realm.userService = ${baseFolder}/users.conf | |||
realm.authenticationProviders = | |||
# How to store passwords. | |||
# Valid values are plain, md5, combined-md5 or PBKDF2WithHmacSHA256. | |||
# Valid values are plain, md5, combined-md5 or pbkdf2. | |||
# md5 is the hash of password. | |||
# combined-md5 is the hash of username.toLowerCase()+password. | |||
# PBKDF2WithHmacSHA256 is salt+hash(salt+password) | |||
# Default is PBKDF2WithHmacSHA256. | |||
# pbkdf2 implements the PBKDF2 algorithm, which is a secure, salted password hashing scheme. | |||
# Default is pbkdf2. Using plain, md5 or combined-md5 is deprecated, as these are insecure schemes by now. | |||
# | |||
# SINCE 0.5.0 | |||
realm.passwordStorage = PBKDF2WithHmacSHA256 | |||
realm.passwordStorage = pbkdf2 | |||
# Minimum valid length for a plain text password. | |||
# Default value is 5. Absolute minimum is 4. |
@@ -58,7 +58,7 @@ import com.gitblit.models.RepositoryModel; | |||
import com.gitblit.models.ServerSettings; | |||
import com.gitblit.models.TeamModel; | |||
import com.gitblit.models.UserModel; | |||
import com.gitblit.utils.SecurePasswordHashUtils; | |||
import com.gitblit.utils.PasswordHash; | |||
import com.gitblit.utils.StringUtils; | |||
@@ -319,9 +319,10 @@ public class EditUserDialog extends JDialog { | |||
minLength)); | |||
return false; | |||
} | |||
if (!password.toUpperCase().startsWith(StringUtils.MD5_TYPE) | |||
&& !password.toUpperCase().startsWith(StringUtils.COMBINED_MD5_TYPE) | |||
&& !password.startsWith(SecurePasswordHashUtils.PBKDF2WITHHMACSHA256_TYPE)) { | |||
// What we actually test for here, is if the password has been changed. But this also catches if the password | |||
// was not changed, but is stored in plain-text. Which is good because then editing the user will hash the | |||
// password if by now the storage has been changed to a hashed variant. | |||
if (!PasswordHash.isHashedEntry(password)) { | |||
String cpw = new String(confirmPasswordField.getPassword()); | |||
if (cpw == null || cpw.length() != password.length()) { | |||
error("Please confirm the password!"); | |||
@@ -335,22 +336,17 @@ public class EditUserDialog extends JDialog { | |||
// change the cookie | |||
user.cookie = user.createCookie(); | |||
String type = settings.get(Keys.realm.passwordStorage).getString(SecurePasswordHashUtils.PBKDF2WITHHMACSHA256); | |||
if (type.equalsIgnoreCase("md5")) { | |||
// store MD5 digest of password | |||
user.password = StringUtils.MD5_TYPE + StringUtils.getMD5(password); | |||
} else if (type.equalsIgnoreCase("combined-md5")) { | |||
// store MD5 digest of username+password | |||
user.password = StringUtils.COMBINED_MD5_TYPE | |||
+ StringUtils.getMD5(user.username + password); | |||
} else if (type.equalsIgnoreCase(SecurePasswordHashUtils.PBKDF2WITHHMACSHA256)) { | |||
// store PBKDF2WithHmacSHA256 digest of password | |||
user.password = SecurePasswordHashUtils.get().createStoredPasswordFromPassword(password); | |||
String type = settings.get(Keys.realm.passwordStorage).getString(PasswordHash.getDefaultType().name()); | |||
PasswordHash pwdHash = PasswordHash.instanceOf(type); | |||
if (pwdHash != null) { | |||
user.password = pwdHash.toHashedEntry(password, user.username); | |||
} else { | |||
// plain-text password | |||
// plain-text password. | |||
// TODO: This is also used when the "realm.passwordStorage" configuration is not a valid type. | |||
// This is a rather bad default, and should probably caught and changed to a secure default. | |||
user.password = password; | |||
} | |||
} else if (rename && password.toUpperCase().startsWith(StringUtils.COMBINED_MD5_TYPE)) { | |||
} else if (rename && password.toUpperCase().startsWith(PasswordHash.Type.CMD5.name())) { | |||
error("Gitblit is configured for combined-md5 password hashing. You must enter a new password on account rename."); | |||
return false; | |||
} else { |
@@ -52,7 +52,7 @@ import com.gitblit.models.UserModel; | |||
import com.gitblit.transport.ssh.SshKey; | |||
import com.gitblit.utils.Base64; | |||
import com.gitblit.utils.HttpUtils; | |||
import com.gitblit.utils.SecurePasswordHashUtils; | |||
import com.gitblit.utils.PasswordHash; | |||
import com.gitblit.utils.StringUtils; | |||
import com.gitblit.utils.X509Utils.X509Metadata; | |||
import com.google.inject.Inject; | |||
@@ -519,29 +519,12 @@ public class AuthenticationManager implements IAuthenticationManager { | |||
*/ | |||
protected UserModel authenticateLocal(UserModel user, char [] password) { | |||
UserModel returnedUser = null; | |||
boolean strongHashUsed = false; | |||
if (user.password.startsWith(StringUtils.MD5_TYPE)) { | |||
// password digest | |||
String md5 = StringUtils.MD5_TYPE + StringUtils.getMD5(new String(password)); | |||
if (user.password.equalsIgnoreCase(md5)) { | |||
PasswordHash pwdHash = PasswordHash.instanceFor(user.password); | |||
if (pwdHash != null) { | |||
if (pwdHash.matches(user.password, password, user.username)) { | |||
returnedUser = user; | |||
} | |||
} else if (user.password.startsWith(StringUtils.COMBINED_MD5_TYPE)) { | |||
// username+password digest | |||
String md5 = StringUtils.COMBINED_MD5_TYPE | |||
+ StringUtils.getMD5(user.username.toLowerCase() + new String(password)); | |||
if (user.password.equalsIgnoreCase(md5)) { | |||
returnedUser = user; | |||
} | |||
} else if (user.password.startsWith(SecurePasswordHashUtils.PBKDF2WITHHMACSHA256_TYPE)){ | |||
// strong hash | |||
SecurePasswordHashUtils hashUtils = SecurePasswordHashUtils.get(); | |||
boolean isPasswordValid = hashUtils.isPasswordCorrect(password, user.password); | |||
if(isPasswordValid){ | |||
returnedUser = user; | |||
strongHashUsed = true; | |||
} | |||
} else if (user.password.equals(new String(password))) { | |||
// plain-text password | |||
returnedUser = user; | |||
@@ -550,33 +533,37 @@ public class AuthenticationManager implements IAuthenticationManager { | |||
// validate user | |||
returnedUser = validateAuthentication(returnedUser, AuthenticationType.CREDENTIALS); | |||
// if no strong hash was used to store the password, try to update it based on the settings | |||
if(!strongHashUsed){ | |||
updateStoredPassword(returnedUser, password); | |||
} | |||
// try to upgrade the stored password hash to a stronger hash, if necessary | |||
upgradeStoredPassword(returnedUser, password, pwdHash); | |||
return returnedUser; | |||
} | |||
/** | |||
* Update stored password to a strong hash if configured. | |||
* Upgrade stored password to a strong hash if configured. | |||
* | |||
* @param user the user to be updated | |||
* @param password the password | |||
* @param pwdHash | |||
* Instance of PasswordHash for the stored password entry. If null, no current hashing is assumed. | |||
*/ | |||
protected void updateStoredPassword(UserModel user, char[] password) { | |||
private void upgradeStoredPassword(UserModel user, char[] password, PasswordHash pwdHash) { | |||
// check if user has successfully authenticated i.e. is not null | |||
if(user != null){ | |||
// check if strong hash algorithm is configured | |||
String algorithm = settings.getString(Keys.realm.passwordStorage, SecurePasswordHashUtils.PBKDF2WITHHMACSHA256); | |||
if(algorithm.equals(SecurePasswordHashUtils.PBKDF2WITHHMACSHA256)){ | |||
// rehash the provided correct password and update the user model | |||
user.password = SecurePasswordHashUtils.get().createStoredPasswordFromPassword(password); | |||
if (user == null) return; | |||
// check if strong hash algorithm is configured | |||
String algorithm = settings.getString(Keys.realm.passwordStorage, PasswordHash.getDefaultType().name()); | |||
if (pwdHash == null || pwdHash.needsUpgradeTo(algorithm)) { | |||
// rehash the provided correct password and update the user model | |||
pwdHash = PasswordHash.instanceOf(algorithm); | |||
if (pwdHash != null) { // necessary since the algorithm name could be something not supported. | |||
user.password = pwdHash.toHashedEntry(password, user.username); | |||
userManager.updateUserModel(user); | |||
} | |||
} | |||
} | |||
} | |||
/** | |||
* Returns the Gitlbit cookie in the request. | |||
* |
@@ -31,14 +31,27 @@ import java.util.Arrays; | |||
*/ | |||
public abstract class PasswordHash { | |||
/** | |||
* The types of implemented password hashing schemes. | |||
*/ | |||
enum Type { | |||
public enum Type { | |||
MD5, | |||
CMD5, | |||
PBKDF2 | |||
PBKDF2; | |||
static Type fromName(String name) { | |||
if (name == null) return null; | |||
for (Type type : Type.values()) { | |||
if (type.name().equalsIgnoreCase(name)) return type; | |||
} | |||
// Compatibility with type id "PBKDF2WITHHMACSHA256", which is also handled by PBKDF2 type. | |||
if (name.equalsIgnoreCase("PBKDF2WITHHMACSHA256")) return Type.PBKDF2; | |||
// Recognise the name used for CMD5 in the settings file. | |||
if (name.equalsIgnoreCase("combined-md5")) return Type.CMD5; | |||
return null; | |||
} | |||
} | |||
/** | |||
@@ -57,6 +70,14 @@ public abstract class PasswordHash { | |||
} | |||
/** | |||
* When no hash type is specified, this determines the default that should be used. | |||
*/ | |||
public static Type getDefaultType() { | |||
return Type.PBKDF2; | |||
} | |||
/** | |||
* Create an instance of a password hashing class for the given hash type. | |||
* | |||
@@ -66,21 +87,17 @@ public abstract class PasswordHash { | |||
* or null if the given hash type is not a valid one. | |||
*/ | |||
public static PasswordHash instanceOf(String type) { | |||
try { | |||
Type hashType = Type.valueOf(type.toUpperCase()); | |||
switch (hashType) { | |||
case MD5: | |||
return new PasswordHashMD5(); | |||
case CMD5: | |||
return new PasswordHashCombinedMD5(); | |||
case PBKDF2: | |||
return new PasswordHashPbkdf2(); | |||
default: | |||
return null; | |||
} | |||
} | |||
catch (Exception e) { | |||
return null; | |||
Type hashType = Type.fromName(type); | |||
if (hashType == null) return null; | |||
switch (hashType) { | |||
case MD5: | |||
return new PasswordHashMD5(); | |||
case CMD5: | |||
return new PasswordHashCombinedMD5(); | |||
case PBKDF2: | |||
return new PasswordHashPbkdf2(); | |||
default: | |||
return null; | |||
} | |||
} | |||
@@ -115,6 +132,27 @@ public abstract class PasswordHash { | |||
} | |||
/** | |||
* Some hash methods are considered more secure than others. This method determines for a certain type | |||
* of password hash if it is inferior than a given other type and stored passwords should be | |||
* upgraded to the given hashing method. | |||
* | |||
* @param algorithm | |||
* Password hashing type to be checked if it is superior than the one of this instance. | |||
* @return True, if the given type in parameter {@code algorithm} is better and stored passwords should be upgraded to it, | |||
* false, otehrwise. | |||
*/ | |||
public boolean needsUpgradeTo(String algorithm) { | |||
Type hashType = Type.fromName(algorithm); | |||
if (hashType == null) return false; | |||
if (this.type == hashType) return false; | |||
// Right now we keep it really simple. With the existing types, only PBKDF2 is considered secure, | |||
// everything else is inferior. This will need to be updated once more secure hashing algorithms | |||
// are implemented, or the workload/parameters of the PBKDF2 are changed. | |||
return hashType == Type.PBKDF2; | |||
} | |||
/** | |||
* Convert the given password to a hashed password entry to be stored in the user table. | |||
@@ -181,13 +219,7 @@ public abstract class PasswordHash { | |||
int indexOfSeparator = hashedEntry.indexOf(':'); | |||
if (indexOfSeparator <= 0) return null; | |||
String typeId = hashedEntry.substring(0, indexOfSeparator); | |||
// Compatibility with type id "PBKDF2WITHHMACSHA256", which is also handled by PBKDF2 type. | |||
if (typeId.equalsIgnoreCase("PBKDF2WITHHMACSHA256")) return Type.PBKDF2; | |||
try { | |||
return Type.valueOf(typeId.toUpperCase()); | |||
} | |||
catch (Exception e) { return null;} | |||
return Type.fromName(typeId); | |||
} | |||
@@ -1,190 +0,0 @@ | |||
package com.gitblit.utils; | |||
import javax.crypto.SecretKeyFactory; | |||
import javax.crypto.spec.PBEKeySpec; | |||
import org.apache.commons.codec.DecoderException; | |||
import org.apache.commons.codec.binary.Hex; | |||
import java.security.NoSuchAlgorithmException; | |||
import java.security.spec.InvalidKeySpecException; | |||
import java.util.Arrays; | |||
/** | |||
* The Class SecurePasswordHashUtils provides methods to create and validate | |||
* secure hashes from user passwords. | |||
* | |||
* It uses the concept proposed by OWASP - Hashing Java: | |||
* https://www.owasp.org/index.php/Hashing_Java | |||
*/ | |||
public class SecurePasswordHashUtils { | |||
public static final String PBKDF2WITHHMACSHA256 = "PBKDF2WithHmacSHA256"; | |||
public static final String PBKDF2WITHHMACSHA256_TYPE = PBKDF2WITHHMACSHA256.toUpperCase() + ":"; | |||
private static final SecureRandom RANDOM = new SecureRandom(); | |||
private static final int ITERATIONS = 10000; | |||
private static final int KEY_LENGTH = 256; | |||
private static final int SALT_LENGTH = 32; | |||
/** The instance. */ | |||
private static SecurePasswordHashUtils instance; | |||
/** | |||
* Instantiates a new secure password hash utils. | |||
*/ | |||
private SecurePasswordHashUtils() { | |||
} | |||
/** | |||
* Gets an instance of {@link SecurePasswordHashUtils}. | |||
* | |||
* @return the secure password hash utils | |||
*/ | |||
public static SecurePasswordHashUtils get() { | |||
if (instance == null) { | |||
instance = new SecurePasswordHashUtils(); | |||
} | |||
return instance; | |||
} | |||
/** | |||
* Gets the next salt. | |||
* | |||
* @return the next salt | |||
*/ | |||
protected byte[] getNextSalt() { | |||
byte[] salt = new byte[SALT_LENGTH]; | |||
RANDOM.nextBytes(salt); | |||
return salt; | |||
} | |||
/** | |||
* Hash. | |||
* | |||
* @param password | |||
* the password | |||
* @param salt | |||
* the salt | |||
* @return the byte[] | |||
*/ | |||
protected byte[] hash(char[] password, byte[] salt) { | |||
PBEKeySpec spec = new PBEKeySpec(password, salt, ITERATIONS, KEY_LENGTH); | |||
Arrays.fill(password, Character.MIN_VALUE); | |||
try { | |||
SecretKeyFactory skf = SecretKeyFactory.getInstance("PBKDF2WithHmacSHA256"); | |||
return skf.generateSecret(spec).getEncoded(); | |||
} catch (NoSuchAlgorithmException | InvalidKeySpecException e) { | |||
throw new IllegalStateException("Error while hashing password", e); | |||
} finally { | |||
spec.clearPassword(); | |||
} | |||
} | |||
/** | |||
* Checks if is password correct. | |||
* | |||
* @param passwordToCheck | |||
* the password to check | |||
* @param salt | |||
* the salt | |||
* @param expectedHash | |||
* the expected hash | |||
* @return true, if is password correct | |||
*/ | |||
protected boolean isPasswordCorrect(char[] passwordToCheck, byte[] salt, byte[] expectedHash) { | |||
byte[] hashToCheck = hash(passwordToCheck, salt); | |||
Arrays.fill(passwordToCheck, Character.MIN_VALUE); | |||
if (hashToCheck.length != expectedHash.length) { | |||
return false; | |||
} | |||
for (int i = 0; i < hashToCheck.length; i++) { | |||
if (hashToCheck[i] != expectedHash[i]) { | |||
return false; | |||
} | |||
} | |||
return true; | |||
} | |||
/** | |||
* Creates the new secure hash from a password and formats it properly to be | |||
* stored in a file. | |||
* | |||
* @param password | |||
* the password to be hashed | |||
* @return the sting to be stored in a file (users.conf) | |||
*/ | |||
public String createStoredPasswordFromPassword(String password) { | |||
return createStoredPasswordFromPassword(password.toCharArray()); | |||
} | |||
/** | |||
* Creates the new secure hash from a password and formats it properly to be | |||
* stored in a file. | |||
* | |||
* @param password | |||
* the password to be hashed | |||
* @return the sting to be stored in a file (users.conf) | |||
*/ | |||
public String createStoredPasswordFromPassword(char[] password) { | |||
byte[] salt = getNextSalt(); | |||
return String.format("%s%s%s", SecurePasswordHashUtils.PBKDF2WITHHMACSHA256_TYPE, StringUtils.toHex(salt), | |||
StringUtils.toHex(hash(password, salt))); | |||
} | |||
/** | |||
* Gets the salt from stored password. | |||
* | |||
* @param storedPassword | |||
* the stored password | |||
* @return the salt from stored password | |||
*/ | |||
protected byte[] getSaltFromStoredPassword(String storedPassword) { | |||
byte[] pw = getStoredHashWithStrippedPrefix(storedPassword); | |||
return Arrays.copyOfRange(pw, 0, SALT_LENGTH); | |||
} | |||
/** | |||
* Gets the hash from stored password. | |||
* | |||
* @param storedPassword | |||
* the stored password | |||
* @return the hash from stored password | |||
*/ | |||
protected byte[] getHashFromStoredPassword(String storedPassword) { | |||
byte[] pw = getStoredHashWithStrippedPrefix(storedPassword); | |||
return Arrays.copyOfRange(pw, SALT_LENGTH, pw.length); | |||
} | |||
/** | |||
* Strips the prefix ({@link #PBKDF2WITHHMACSHA256_TYPE}) from a stored | |||
* password and returns the decoded hash | |||
* | |||
* @param storedPassword | |||
* the stored password | |||
* @return the stored hash with stripped prefix | |||
*/ | |||
protected byte[] getStoredHashWithStrippedPrefix(String storedPassword) { | |||
String saltAndHash = storedPassword.substring(PBKDF2WITHHMACSHA256_TYPE.length()); | |||
try { | |||
return Hex.decodeHex(saltAndHash.toCharArray()); | |||
} catch (DecoderException e) { | |||
throw new IllegalStateException("Error while reading stored credentials", e); | |||
} | |||
} | |||
/** | |||
* Checks if is password correct. | |||
* | |||
* @param password | |||
* the password | |||
* @param storedPassword | |||
* the stored password | |||
* @return true, if is password correct | |||
*/ | |||
public boolean isPasswordCorrect(char[] password, String storedPassword) { | |||
byte[] storedSalt = getSaltFromStoredPassword(storedPassword); | |||
byte[] storedHash = getHashFromStoredPassword(storedPassword); | |||
return isPasswordCorrect(password, storedSalt, storedHash); | |||
} | |||
} |
@@ -46,10 +46,6 @@ import java.util.regex.PatternSyntaxException; | |||
*/ | |||
public class StringUtils { | |||
public static final String MD5_TYPE = "MD5:"; | |||
public static final String COMBINED_MD5_TYPE = "CMD5:"; | |||
/** | |||
* Returns true if the string is null or empty. | |||
* |
@@ -28,15 +28,14 @@ import org.apache.wicket.protocol.http.WebResponse; | |||
import com.gitblit.GitBlitException; | |||
import com.gitblit.Keys; | |||
import com.gitblit.models.UserModel; | |||
import com.gitblit.utils.SecurePasswordHashUtils; | |||
import com.gitblit.utils.StringUtils; | |||
import com.gitblit.utils.PasswordHash; | |||
import com.gitblit.wicket.GitBlitWebSession; | |||
import com.gitblit.wicket.NonTrimmedPasswordTextField; | |||
public class ChangePasswordPage extends RootSubPage { | |||
IModel<String> password = new Model<String>(""); | |||
IModel<String> confirmPassword = new Model<String>(""); | |||
private IModel<String> password = new Model<String>(""); | |||
private IModel<String> confirmPassword = new Model<String>(""); | |||
public ChangePasswordPage() { | |||
super(); | |||
@@ -86,18 +85,11 @@ public class ChangePasswordPage extends RootSubPage { | |||
UserModel user = GitBlitWebSession.get().getUser(); | |||
// convert to MD5 digest, if appropriate | |||
String type = app().settings().getString(Keys.realm.passwordStorage, SecurePasswordHashUtils.PBKDF2WITHHMACSHA256); | |||
if (type.equalsIgnoreCase("md5")) { | |||
// store MD5 digest of password | |||
password = StringUtils.MD5_TYPE + StringUtils.getMD5(password); | |||
} else if (type.equalsIgnoreCase("combined-md5")) { | |||
// store MD5 digest of username+password | |||
password = StringUtils.COMBINED_MD5_TYPE | |||
+ StringUtils.getMD5(user.username.toLowerCase() + password); | |||
} else if (type.equalsIgnoreCase(SecurePasswordHashUtils.PBKDF2WITHHMACSHA256)) { | |||
// store PBKDF2WithHmacSHA256 digest of password | |||
user.password = SecurePasswordHashUtils.get().createStoredPasswordFromPassword(password); | |||
// convert to digest, if appropriate | |||
String type = app().settings().getString(Keys.realm.passwordStorage, PasswordHash.getDefaultType().name()); | |||
PasswordHash pwdHash = PasswordHash.instanceOf(type); | |||
if (pwdHash != null) { | |||
password = pwdHash.toHashedEntry(password, user.username); | |||
} | |||
user.password = password; |
@@ -21,6 +21,7 @@ import java.util.Collections; | |||
import java.util.Iterator; | |||
import java.util.List; | |||
import com.gitblit.utils.PasswordHash; | |||
import org.apache.wicket.PageParameters; | |||
import org.apache.wicket.behavior.SimpleAttributeModifier; | |||
import org.apache.wicket.extensions.markup.html.form.palette.Palette; | |||
@@ -141,15 +142,14 @@ public class EditUserPage extends RootSubPage { | |||
return; | |||
} | |||
String password = userModel.password; | |||
if (!password.toUpperCase().startsWith(StringUtils.MD5_TYPE) | |||
&& !password.toUpperCase().startsWith(StringUtils.COMBINED_MD5_TYPE)) { | |||
if (!PasswordHash.isHashedEntry(password)) { | |||
// This is a plain text password. | |||
// Check length. | |||
int minLength = app().settings().getInteger(Keys.realm.minPasswordLength, 5); | |||
if (minLength < 4) { | |||
minLength = 4; | |||
} | |||
if (password.trim().length() < minLength) { | |||
if (password.trim().length() < minLength) { // TODO: Why do we trim here, but not in EditUserDialog and ChangePasswordPage? | |||
error(MessageFormat.format(getString("gb.passwordTooShort"), | |||
minLength)); | |||
return; | |||
@@ -159,18 +159,13 @@ public class EditUserPage extends RootSubPage { | |||
userModel.cookie = userModel.createCookie(); | |||
// Optionally store the password MD5 digest. | |||
String type = app().settings().getString(Keys.realm.passwordStorage, "md5"); | |||
if (type.equalsIgnoreCase("md5")) { | |||
// store MD5 digest of password | |||
userModel.password = StringUtils.MD5_TYPE | |||
+ StringUtils.getMD5(userModel.password); | |||
} else if (type.equalsIgnoreCase("combined-md5")) { | |||
// store MD5 digest of username+password | |||
userModel.password = StringUtils.COMBINED_MD5_TYPE | |||
+ StringUtils.getMD5(username + userModel.password); | |||
String type = app().settings().getString(Keys.realm.passwordStorage, PasswordHash.getDefaultType().name()); | |||
PasswordHash pwdh = PasswordHash.instanceOf(type); | |||
if (pwdh != null) { // Hash the password | |||
userModel.password = pwdh.toHashedEntry(password, username); | |||
} | |||
} else if (rename | |||
&& password.toUpperCase().startsWith(StringUtils.COMBINED_MD5_TYPE)) { | |||
&& password.toUpperCase().startsWith(PasswordHash.Type.CMD5.name())) { | |||
error(getString("gb.combinedMd5Rename")); | |||
return; | |||
} |
@@ -169,7 +169,7 @@ Usernames must be unique and are case-insensitive. | |||
Whitespace is illegal. | |||
### Passwords | |||
User passwords are CASE-SENSITIVE and may be *plain*, *md5*, *combined-md5* or *PBKDF2WithHmacSHA256* formatted (see `gitblit.properties` -> *realm.passwordStorage*). | |||
User passwords are CASE-SENSITIVE and may be *plain*, *md5*, *combined-md5* or *pbkdf2* formatted (see `gitblit.properties` -> *realm.passwordStorage*). | |||
### User Roles | |||
There are four actual *roles* in Gitblit: |
@@ -43,6 +43,7 @@ import javax.servlet.http.HttpSessionContext; | |||
import javax.servlet.http.HttpUpgradeHandler; | |||
import javax.servlet.http.Part; | |||
import com.gitblit.utils.PasswordHash; | |||
import org.junit.Test; | |||
import com.gitblit.IUserService; | |||
@@ -55,7 +56,6 @@ import com.gitblit.manager.UserManager; | |||
import com.gitblit.models.TeamModel; | |||
import com.gitblit.models.UserModel; | |||
import com.gitblit.tests.mock.MemorySettings; | |||
import com.gitblit.utils.SecurePasswordHashUtils; | |||
import com.gitblit.utils.XssFilter; | |||
import com.gitblit.utils.XssFilter.AllowXssFilter; | |||
@@ -659,17 +659,43 @@ public class AuthenticationManagerTest extends GitblitUnitTest { | |||
users.updateUserModel(user); | |||
assertNotNull(auth.authenticate(user.username, user.password.toCharArray(), null)); | |||
// validate that plaintext password was automatically updated to hashed one | |||
assertTrue(user.password.startsWith(SecurePasswordHashUtils.PBKDF2WITHHMACSHA256_TYPE)); | |||
user.disabled = true; | |||
users.updateUserModel(user); | |||
assertNull(auth.authenticate(user.username, user.password.toCharArray(), null)); | |||
users.deleteUserModel(user); | |||
} | |||
@Test | |||
public void testAuthenticateUpgradePlaintext() throws Exception { | |||
IAuthenticationManager auth = newAuthenticationManager(); | |||
UserModel user = new UserModel("sunnyjim"); | |||
user.password = "password"; | |||
users.updateUserModel(user); | |||
assertNotNull(auth.authenticate(user.username, user.password.toCharArray(), null)); | |||
// validate that plaintext password was automatically updated to hashed one | |||
assertTrue(user.password.startsWith(PasswordHash.getDefaultType().name() + ":")); | |||
} | |||
@Test | |||
public void testAuthenticateUpgradeMD5() throws Exception { | |||
IAuthenticationManager auth = newAuthenticationManager(); | |||
UserModel user = new UserModel("sunnyjim"); | |||
user.password = "MD5:5F4DCC3B5AA765D61D8327DEB882CF99"; | |||
users.updateUserModel(user); | |||
assertNotNull(auth.authenticate(user.username, "password".toCharArray(), null)); | |||
// validate that MD5 password was automatically updated to hashed one | |||
assertTrue(user.password.startsWith(PasswordHash.getDefaultType().name() + ":")); | |||
} | |||
@Test | |||
public void testContenairAuthenticate() throws Exception { |
@@ -99,6 +99,15 @@ public class PasswordHashTest { | |||
assertTrue("Failed to match " +CMD5_HASHED_ENTRY_0, pwdh.matches(CMD5_HASHED_ENTRY_0, CMD5_PASSWORD_0.toCharArray(), CMD5_USERNAME_0)); | |||
pwdh = PasswordHash.instanceOf("combined-md5"); | |||
assertNotNull(pwdh); | |||
assertEquals(PasswordHash.Type.CMD5, pwdh.type); | |||
pwdh = PasswordHash.instanceOf("COMBINED-MD5"); | |||
assertNotNull(pwdh); | |||
assertEquals(PasswordHash.Type.CMD5, pwdh.type); | |||
pwdh = PasswordHash.instanceOf("MD5"); | |||
assertNotNull(pwdh); | |||
assertNotEquals(PasswordHash.Type.CMD5, pwdh.type); | |||
@@ -593,4 +602,35 @@ public class PasswordHashTest { | |||
assertFalse("Matched wrong hashed entry, with empty user", pwdh.matches(PBKDF2_HASHED_ENTRY_3, PBKDF2_PASSWORD_0.toCharArray(), "")); | |||
assertFalse("Matched wrong hashed entry, with user", pwdh.matches(PBKDF2_HASHED_ENTRY_3, PBKDF2_PASSWORD_0.toCharArray(), "someuser")); | |||
} | |||
@Test | |||
public void getEntryType() { | |||
assertEquals(PasswordHash.Type.MD5, PasswordHash.getEntryType("MD5:blah")); | |||
assertEquals(PasswordHash.Type.MD5, PasswordHash.getEntryType("md5:blah")); | |||
assertEquals(PasswordHash.Type.MD5, PasswordHash.getEntryType("mD5:blah")); | |||
assertEquals(PasswordHash.Type.CMD5, PasswordHash.getEntryType("CMD5:blah")); | |||
assertEquals(PasswordHash.Type.CMD5, PasswordHash.getEntryType("cmd5:blah")); | |||
assertEquals(PasswordHash.Type.CMD5, PasswordHash.getEntryType("Cmd5:blah")); | |||
assertEquals(PasswordHash.Type.CMD5, PasswordHash.getEntryType("combined-md5:blah")); | |||
assertEquals(PasswordHash.Type.CMD5, PasswordHash.getEntryType("COMBINED-MD5:blah")); | |||
assertEquals(PasswordHash.Type.CMD5, PasswordHash.getEntryType("combined-MD5:blah")); | |||
assertEquals(PasswordHash.Type.PBKDF2, PasswordHash.getEntryType("PBKDF2:blah")); | |||
assertEquals(PasswordHash.Type.PBKDF2, PasswordHash.getEntryType("pbkdf2:blah")); | |||
assertEquals(PasswordHash.Type.PBKDF2, PasswordHash.getEntryType("Pbkdf2:blah")); | |||
assertEquals(PasswordHash.Type.PBKDF2, PasswordHash.getEntryType("pbKDF2:blah")); | |||
assertEquals(PasswordHash.Type.PBKDF2, PasswordHash.getEntryType("PBKDF2WithHmacSHA256:blah")); | |||
assertEquals(PasswordHash.Type.PBKDF2, PasswordHash.getEntryType("PBKDF2WITHHMACSHA256:blah")); | |||
} | |||
@Test | |||
public void getEntryValue() { | |||
assertEquals("value", PasswordHash.getEntryValue("MD5:value")); | |||
assertEquals("plain text", PasswordHash.getEntryValue("plain text")); | |||
assertEquals("what this", PasswordHash.getEntryValue(":what this")); | |||
assertEquals("", PasswordHash.getEntryValue(":")); | |||
} | |||
} |
@@ -1,63 +0,0 @@ | |||
package com.gitblit.utils; | |||
import static org.junit.Assert.*; | |||
import org.junit.Before; | |||
import org.junit.Test; | |||
public class SecurePasswordHashUtilsTest { | |||
private static final String STORED_PASSWORD = "PBKDF2WITHHMACSHA256:2d7d3ccaa277787f288e9f929247361bfc83607c6a8447bf496267512e360ba0a97b3114937213b23230072517d65a2e00695a1cbc47a732510840817f22c1bc"; | |||
private static final byte[] STORED_SALT_BYTES = new byte[]{45, 125, 60, -54, -94, 119, 120, 127, 40, -114, -97, -110, -110, 71, 54, 27, -4, -125, 96, 124, 106, -124, 71, -65, 73, 98, 103, 81, 46, 54, 11, -96}; | |||
private static final byte[] STORED_HASH_BYTES = new byte[]{-87, 123, 49, 20, -109, 114, 19, -78, 50, 48, 7, 37, 23, -42, 90, 46, 0, 105, 90, 28, -68, 71, -89, 50, 81, 8, 64, -127, 127, 34, -63, -68}; | |||
private SecurePasswordHashUtils utils; | |||
@Before | |||
public void init(){ | |||
utils = SecurePasswordHashUtils.get(); | |||
} | |||
@Test | |||
public void testGetNextSalt() { | |||
assertEquals(32, utils.getNextSalt().length); | |||
} | |||
@Test | |||
public void testHash() { | |||
byte[] hash = utils.hash("foo".toCharArray(), STORED_SALT_BYTES); | |||
assertArrayEquals(STORED_HASH_BYTES, hash); | |||
} | |||
@Test | |||
public void testIsPasswordCorrectCharArrayByteArrayByteArray() { | |||
assertTrue(utils.isPasswordCorrect("foo".toCharArray(), STORED_SALT_BYTES, STORED_HASH_BYTES)); | |||
assertFalse(utils.isPasswordCorrect("bar".toCharArray(), STORED_SALT_BYTES, STORED_HASH_BYTES)); | |||
} | |||
@Test | |||
public void testCreateNewStorableHashFromPassword() { | |||
String newPwHash = utils.createStoredPasswordFromPassword("foo"); | |||
assertTrue(newPwHash.startsWith(SecurePasswordHashUtils.PBKDF2WITHHMACSHA256_TYPE)); | |||
} | |||
@Test | |||
public void testGetSaltFromStoredPassword() { | |||
byte[] saltFromStoredPassword = utils.getSaltFromStoredPassword(STORED_PASSWORD); | |||
assertArrayEquals(STORED_SALT_BYTES, saltFromStoredPassword); | |||
} | |||
@Test | |||
public void testGetHashFromStoredPassword() { | |||
byte[] hashFromStoredPassword = utils.getHashFromStoredPassword(STORED_PASSWORD); | |||
assertArrayEquals(STORED_HASH_BYTES, hashFromStoredPassword); | |||
} | |||
@Test | |||
public void testIsPasswordCorrectCharArrayString() { | |||
assertTrue(utils.isPasswordCorrect("foo".toCharArray(), STORED_PASSWORD)); | |||
assertFalse(utils.isPasswordCorrect("bar".toCharArray(), STORED_PASSWORD)); | |||
} | |||
} |