Added possibility to use secure hashes to store passwords

src/main/distrib/data/defaults.properties

@@ -854,12 +854,14 @@ realm.userService = ${baseFolder}/users.conf
 realm.authenticationProviders =
 
 # How to store passwords.
-# Valid values are plain, md5, or combined-md5.  md5 is the hash of password.
+# Valid values are plain, md5, combined-md5 or PBKDF2WithHmacSHA256.
+# md5 is the hash of password.
 # combined-md5 is the hash of username.toLowerCase()+password.
-# Default is md5.
+# PBKDF2WithHmacSHA256 is salt+hash(salt+password)
+# Default is PBKDF2WithHmacSHA256.
 #
 # SINCE 0.5.0 
-realm.passwordStorage = md5
+realm.passwordStorage = PBKDF2WithHmacSHA256
 
 # Minimum valid length for a plain text password.
 # Default value is 5.  Absolute minimum is 4.

src/main/java/com/gitblit/client/EditUserDialog.java

@@ -58,8 +58,10 @@
 import com.gitblit.models.ServerSettings;
 import com.gitblit.models.TeamModel;
 import com.gitblit.models.UserModel;
+import com.gitblit.utils.SecurePasswordHashUtils;
 import com.gitblit.utils.StringUtils;
 
+
 public class EditUserDialog extends JDialog {
 
 	private static final long serialVersionUID = 1L;
@@ -318,7 +320,8 @@ private boolean validateFields() {
 			return false;
 		}
 		if (!password.toUpperCase().startsWith(StringUtils.MD5_TYPE)
-				&& !password.toUpperCase().startsWith(StringUtils.COMBINED_MD5_TYPE)) {
+				&& !password.toUpperCase().startsWith(StringUtils.COMBINED_MD5_TYPE)
+				&& !password.startsWith(SecurePasswordHashUtils.PBKDF2WITHHMACSHA256_TYPE)) {
 			String cpw = new String(confirmPasswordField.getPassword());
 			if (cpw == null || cpw.length() != password.length()) {
 				error("Please confirm the password!");
@@ -332,14 +335,17 @@ private boolean validateFields() {
 			// change the cookie
 			user.cookie = user.createCookie();
 
-			String type = settings.get(Keys.realm.passwordStorage).getString("md5");
+			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);
 			} else {
 				// plain-text password
 				user.password = password;

src/main/java/com/gitblit/manager/AuthenticationManager.java

@@ -52,6 +52,7 @@
 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.StringUtils;
 import com.gitblit.utils.X509Utils.X509Metadata;
 import com.google.inject.Inject;
@@ -518,6 +519,8 @@ public UserModel authenticate(String username, char[] password, String remoteIP)
 	 */
 	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));
@@ -531,11 +534,47 @@ protected UserModel authenticateLocal(UserModel user, char [] 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;
+		} 
+
+		// 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);
+		}
+
+		return returnedUser;
+	}
+
+	/**
+	 * Update stored password to a strong hash if configured.
+	 *
+	 * @param user the user to be updated
+	 * @param password the password
+	 */
+	protected void updateStoredPassword(UserModel user, char[] password) {
+		// 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);
+				userManager.updateUserModel(user);
+			}	
 		}
-		return validateAuthentication(returnedUser, AuthenticationType.CREDENTIALS);
 	}
 
 	/**

src/main/java/com/gitblit/utils/SecurePasswordHashUtils.java

@@ -0,0 +1,190 @@
+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);
+	}
+}

src/main/java/com/gitblit/wicket/pages/ChangePasswordPage.java

@@ -28,6 +28,7 @@
 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.wicket.GitBlitWebSession;
 import com.gitblit.wicket.NonTrimmedPasswordTextField;
@@ -86,14 +87,17 @@ public void onSubmit() {
 				UserModel user = GitBlitWebSession.get().getUser();
 
 				// convert to MD5 digest, if appropriate
-				String type = app().settings().getString(Keys.realm.passwordStorage, "md5");
+				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);
 				}
 
 				user.password = password;

src/site/administration.mkd

@@ -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*, or *combined-md5* formatted (see `gitblit.properties` -> *realm.passwordStorage*).
+User passwords are CASE-SENSITIVE and may be *plain*, *md5*, *combined-md5* or *PBKDF2WithHmacSHA256* formatted (see `gitblit.properties` -> *realm.passwordStorage*).
 
 ### User Roles
 There are four actual *roles* in Gitblit:

src/test/java/com/gitblit/tests/AuthenticationManagerTest.java

@@ -55,6 +55,7 @@
 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;
 
@@ -658,12 +659,17 @@ public void testAuthenticate() throws Exception {
 		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 testContenairAuthenticate() throws Exception {