diff options
Diffstat (limited to 'src')
7 files changed, 266 insertions, 7 deletions
diff --git a/src/main/distrib/data/defaults.properties b/src/main/distrib/data/defaults.properties index 9bb02482..352a6750 100644 --- a/src/main/distrib/data/defaults.properties +++ b/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. diff --git a/src/main/java/com/gitblit/client/EditUserDialog.java b/src/main/java/com/gitblit/client/EditUserDialog.java index 4b01ff04..fcbb41ef 100644 --- a/src/main/java/com/gitblit/client/EditUserDialog.java +++ b/src/main/java/com/gitblit/client/EditUserDialog.java @@ -58,8 +58,10 @@ 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.StringUtils;
+
public class EditUserDialog extends JDialog {
private static final long serialVersionUID = 1L;
@@ -318,7 +320,8 @@ public class EditUserDialog extends JDialog { 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,7 +335,7 @@ public class EditUserDialog extends JDialog { // change the cookie
user.cookie = user.createCookie();
- String type = settings.get(Keys.realm.passwordStorage).getString("md5");
+ String type = settings.get(Keys.realm.passwordStorage).getString("PBKDF2WithHmacSHA256");
if (type.equalsIgnoreCase("md5")) {
// store MD5 digest of password
user.password = StringUtils.MD5_TYPE + StringUtils.getMD5(password);
@@ -340,6 +343,9 @@ public class EditUserDialog extends JDialog { // store MD5 digest of username+password
user.password = StringUtils.COMBINED_MD5_TYPE
+ StringUtils.getMD5(user.username + password);
+ } else if (type.equalsIgnoreCase("PBKDF2WithHmacSHA256")) {
+ // store PBKDF2WithHmacSHA256 digest of password
+ user.password = SecurePasswordHashUtils.get().createStoredPasswordFromPassword(password);
} else {
// plain-text password
user.password = password;
diff --git a/src/main/java/com/gitblit/manager/AuthenticationManager.java b/src/main/java/com/gitblit/manager/AuthenticationManager.java index 0a4d8ed7..7a1fd9f2 100644 --- a/src/main/java/com/gitblit/manager/AuthenticationManager.java +++ b/src/main/java/com/gitblit/manager/AuthenticationManager.java @@ -52,6 +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.StringUtils; import com.gitblit.utils.X509Utils.X509Metadata; import com.google.inject.Inject; @@ -518,6 +519,7 @@ public class AuthenticationManager implements IAuthenticationManager { */ protected UserModel authenticateLocal(UserModel user, char [] password) { UserModel returnedUser = null; + //weak password hash if (user.password.startsWith(StringUtils.MD5_TYPE)) { // password digest String md5 = StringUtils.MD5_TYPE + StringUtils.getMD5(new String(password)); @@ -534,7 +536,15 @@ public class AuthenticationManager implements IAuthenticationManager { } else if (user.password.equals(new String(password))) { // plain-text password 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; + } } + return validateAuthentication(returnedUser, AuthenticationType.CREDENTIALS); } diff --git a/src/main/java/com/gitblit/utils/SecurePasswordHashUtils.java b/src/main/java/com/gitblit/utils/SecurePasswordHashUtils.java new file mode 100644 index 00000000..de2c0084 --- /dev/null +++ b/src/main/java/com/gitblit/utils/SecurePasswordHashUtils.java @@ -0,0 +1,174 @@ +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_TYPE = "PBKDF2WITHHMACSHA256:"; + + 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) { + byte[] salt = getNextSalt(); + return String.format("%s%s%s", SecurePasswordHashUtils.PBKDF2WITHHMACSHA256_TYPE, StringUtils.toHex(salt), StringUtils.toHex(hash(password.toCharArray(), 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); + } +} diff --git a/src/main/java/com/gitblit/wicket/pages/ChangePasswordPage.java b/src/main/java/com/gitblit/wicket/pages/ChangePasswordPage.java index 259a4bf4..afc4058b 100644 --- a/src/main/java/com/gitblit/wicket/pages/ChangePasswordPage.java +++ b/src/main/java/com/gitblit/wicket/pages/ChangePasswordPage.java @@ -28,6 +28,7 @@ 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.wicket.GitBlitWebSession;
import com.gitblit.wicket.NonTrimmedPasswordTextField;
@@ -86,7 +87,7 @@ public class ChangePasswordPage extends RootSubPage { 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, "PBKDF2WithHmacSHA256");
if (type.equalsIgnoreCase("md5")) {
// store MD5 digest of password
password = StringUtils.MD5_TYPE + StringUtils.getMD5(password);
@@ -94,6 +95,9 @@ public class ChangePasswordPage extends RootSubPage { // store MD5 digest of username+password
password = StringUtils.COMBINED_MD5_TYPE
+ StringUtils.getMD5(user.username.toLowerCase() + password);
+ } else if (type.equalsIgnoreCase("PBKDF2WithHmacSHA256")) {
+ // store PBKDF2WithHmacSHA256 digest of password
+ user.password = SecurePasswordHashUtils.get().createStoredPasswordFromPassword(password);
}
user.password = password;
diff --git a/src/site/administration.mkd b/src/site/administration.mkd index 049a8273..5feedee9 100644 --- a/src/site/administration.mkd +++ b/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:
diff --git a/src/test/java/com/gitblit/utils/SecurePasswordHashUtilsTest.java b/src/test/java/com/gitblit/utils/SecurePasswordHashUtilsTest.java new file mode 100644 index 00000000..f687bda6 --- /dev/null +++ b/src/test/java/com/gitblit/utils/SecurePasswordHashUtilsTest.java @@ -0,0 +1,63 @@ +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)); + } + +} |