]> source.dussan.org Git - gitblit.git/commitdiff
Add support for PBKDF2 to PasswordHash
authorFlorian Zschocke <florian.zschocke@devolo.de>
Tue, 5 Nov 2019 12:43:06 +0000 (13:43 +0100)
committerFlorian Zschocke <florian.zschocke@devolo.de>
Tue, 5 Nov 2019 21:20:37 +0000 (22:20 +0100)
Integrate the work of pingunaut to add support for PBKDF2 password
hashing. A new class `PasswordHashPbkdf2` is added, which builds
on his `SecurePasswordHashUtils` class, but makes it a subclass
of `PasswordHash`. This will replace the original class when
integrating the new PasswordHash way into GitBlit.

src/main/java/com/gitblit/utils/PasswordHash.java
src/main/java/com/gitblit/utils/PasswordHashPbkdf2.java [new file with mode: 0644]
src/test/java/com/gitblit/utils/PasswordHashTest.java

index 5ccfab494d30512729ebde224b1b0c94323f853a..137478056ed6897b13d141fcf33b3b562ba7a2c5 100644 (file)
@@ -16,6 +16,8 @@
 
 package com.gitblit.utils;
 
+import java.util.Arrays;
+
 /**
  * This is the superclass for classes responsible for handling password hashing.
  * 
@@ -35,7 +37,8 @@ public abstract class PasswordHash {
         */
        enum Type {
                MD5,
-               CMD5
+               CMD5,
+               PBKDF2
        }
 
        /**
@@ -70,6 +73,8 @@ public abstract class PasswordHash {
                                        return new PasswordHashMD5();
                                case CMD5:
                                        return new PasswordHashCombinedMD5();
+                               case PBKDF2:
+                                       return new PasswordHashPbkdf2();
                                default:
                                        return null;
                        }
@@ -100,17 +105,31 @@ public abstract class PasswordHash {
         * Test if a given string is a hashed password entry. This method simply checks if the
         * given string is prefixed by a known hash type identifier.
         *
-        * @param password
+        * @param storedPassword
         *                      A stored user password.
         * @return      True if the given string is detected to be hashed with a known hash type,
         *                      false otherwise.
         */
-       public static boolean isHashedEntry(String password) {
-               return null != getEntryType(password);
+       public static boolean isHashedEntry(String storedPassword) {
+               return null != getEntryType(storedPassword);
        }
 
-       
-       
+
+
+       /**
+        * Convert the given password to a hashed password entry to be stored in the user table.
+        * The resulting string is prefixed by the hashing scheme type followed by a colon:
+        * TYPE:theactualhashinhex
+        *
+        * @param password
+        *                      Password to be hashed.
+        * @param username
+        *                      User name, only used for the Combined-MD5 (user+MD5) hashing type.
+        * @return
+        *                      Hashed password entry to be stored in the user table.
+        */
+       abstract public String toHashedEntry(char[] password, String username);
+
        /**
         * Convert the given password to a hashed password entry to be stored in the user table.
         * The resulting string is prefixed by the hashing scheme type followed by a colon:
@@ -123,7 +142,10 @@ public abstract class PasswordHash {
         * @return
         *                      Hashed password entry to be stored in the user table.
         */
-       abstract public String toHashedEntry(String password, String username);
+       public String toHashedEntry(String password, String username) {
+               if (password == null) throw new IllegalArgumentException("The password argument may not be null when hashing a password.");
+               return toHashedEntry(password.toCharArray(), username);
+       }
 
        /**
         * Test if a given password (and user name) match a hashed password.
@@ -145,7 +167,8 @@ public abstract class PasswordHash {
                if (hashedEntry == null || type != PasswordHash.getEntryType(hashedEntry)) return false;
                if (password == null) return false;
 
-               String hashed = toHashedEntry(String.valueOf(password), username);
+               String hashed = toHashedEntry(password, username);
+               Arrays.fill(password, Character.MIN_VALUE);
                return hashed.equalsIgnoreCase(hashedEntry);
        }
 
@@ -159,6 +182,8 @@ public abstract class PasswordHash {
                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());
                }
@@ -169,7 +194,7 @@ public abstract class PasswordHash {
        static String getEntryValue(String hashedEntry) {
                if (hashedEntry == null) return null;
                int indexOfSeparator = hashedEntry.indexOf(':');
-               return hashedEntry.substring(indexOfSeparator +1, hashedEntry.length());
+               return hashedEntry.substring(indexOfSeparator +1);
        }
 
 
@@ -184,6 +209,14 @@ public abstract class PasswordHash {
                        super(Type.MD5);
                }
 
+               // To keep the handling identical to how it was before, and therefore not risk invalidating stored passwords,
+               // for MD5 the (String,String) variant of the method is the one implementing the hashing.
+               @Override
+               public String toHashedEntry(char[] password, String username) {
+                       if (password == null) throw new IllegalArgumentException("The password argument may not be null when hashing a password.");
+                       return toHashedEntry(new String(password), username);
+               }
+
                @Override
                public String toHashedEntry(String password, String username) {
                        if (password == null) throw new IllegalArgumentException("The password argument may not be null when hashing a password.");
@@ -201,9 +234,16 @@ public abstract class PasswordHash {
                        super(Type.CMD5);
                }
 
+               // To keep the handling identical to how it was before, and therefore not risk invalidating stored passwords,
+               // for Combined-MD5 the (String,String) variant of the method is the one implementing the hashing.
+               @Override
+               public String toHashedEntry(char[] password, String username) {
+                       if (password == null) throw new IllegalArgumentException("The password argument may not be null when hashing a password.");
+                       return toHashedEntry(new String(password), username);
+               }
                @Override
                public String toHashedEntry(String password, String username) {
-                       if (password == null) throw new IllegalArgumentException("The password argument may not be null when hashing a password with Combined-MD5.");
+                       if (password == null) throw new IllegalArgumentException("The password argument may not be null when hashing a password.");
                        if (username == null) throw new IllegalArgumentException("The username argument may not be null when hashing a password with Combined-MD5.");
                        if (StringUtils.isEmpty(username)) throw new IllegalArgumentException("The username argument may not be empty when hashing a password with Combined-MD5.");
                        return type.name() + ":"
diff --git a/src/main/java/com/gitblit/utils/PasswordHashPbkdf2.java b/src/main/java/com/gitblit/utils/PasswordHashPbkdf2.java
new file mode 100644 (file)
index 0000000..1bce122
--- /dev/null
@@ -0,0 +1,276 @@
+package com.gitblit.utils;
+
+import org.apache.commons.codec.DecoderException;
+import org.apache.commons.codec.binary.Hex;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import javax.crypto.SecretKeyFactory;
+import javax.crypto.spec.PBEKeySpec;
+import java.security.NoSuchAlgorithmException;
+import java.security.spec.InvalidKeySpecException;
+import java.util.Arrays;
+
+/**
+ * The class PasswordHashPbkdf2 implements password hashing and validation
+ * with PBKDF2
+ *
+ * It uses the concept proposed by OWASP - Hashing Java:
+ * https://www.owasp.org/index.php/Hashing_Java
+ */
+class PasswordHashPbkdf2 extends PasswordHash
+{
+
+       private static final Logger LOGGER = LoggerFactory.getLogger(PasswordHashPbkdf2.class);
+
+       /**
+        * The PBKDF has some parameters that define security and workload.
+        * The Configuration class keeps these parameters.
+        */
+       private static class Configuration
+       {
+               private final String algorithm;
+               private final int iterations;
+               private final int keyLen;
+               private final int saltLen;
+
+               private Configuration(String algorithm, int iterations, int keyLen, int saltLen) {
+                       this.algorithm = algorithm;
+                       this.iterations = iterations;
+                       this.keyLen = keyLen;
+                       this.saltLen = saltLen;
+               }
+       }
+
+
+       private static final SecureRandom RANDOM = new SecureRandom();
+       /**
+        * A list of Configurations is created to list the configurations supported by
+        * this implementation. The configuration id is stored in the hashed entry,
+        * identifying the Configuration in this array.
+        * When adding a new variant with different values for these parameters, add
+        * it to this array.
+        * The code uses the last configuration in the array as the most secure, to be used
+        * when creating new hashes when no configuration is specified.
+        */
+       private static final Configuration[] configurations = {
+                       // Configuration 0, also default when none is specified in the stored hashed entry.
+                       new Configuration("PBKDF2WithHmacSHA256", 10000, 256, 32)
+       };
+
+
+       PasswordHashPbkdf2() {
+               super(Type.PBKDF2);
+       }
+
+
+       /*
+        * We return a hashed entry, where the hash part (salt+hash) itself is prefixed
+        * again by the configuration id of the configuration that was used for the PBKDF,
+        * enclosed in '$':
+        * PBKDF2:$0$thesaltThehash
+        */
+       @Override
+       public String toHashedEntry(char[] password, String username) {
+               if (password == null) {
+                       LOGGER.warn("The password argument may not be null when hashing a password.");
+                       throw new IllegalArgumentException("The password argument may not be null when hashing a password.");
+               }
+
+               int configId = getLatestConfigurationId();
+               Configuration config = configurations[configId];
+
+               byte[] salt = new byte[config.saltLen];
+               RANDOM.nextBytes(salt);
+               byte[] hash = hash(password, salt, config);
+
+               return type.name() + ":"
+                               + "$" + configId + "$"
+                               + StringUtils.toHex(salt)
+                               + StringUtils.toHex(hash);
+       }
+
+       @Override
+       public boolean matches(String hashedEntry, char[] password, String username) {
+               if (hashedEntry == null || type != PasswordHash.getEntryType(hashedEntry)) return false;
+               if (password == null) return false;
+
+               String hashedPart = getEntryValue(hashedEntry);
+               int configId = getConfigIdFromStoredPassword(hashedPart);
+
+               return isPasswordCorrect(password, hashedPart, configurations[configId]);
+       }
+
+
+
+
+
+
+
+
+       /**
+        * Return the id of the most updated configuration of parameters for the PBKDF.
+        * New password hashes should be generated with this one.
+        *
+        * @return An index into the configurations array for the latest configuration.
+        */
+       private int getLatestConfigurationId() {
+               return configurations.length-1;
+       }
+
+
+       /**
+        * Get the configuration id from the stored hashed password, that was used when the
+        * hash was created. The configuration id is the index into the configuration array,
+        * and is stored in the format $Id$ after the type identifier: TYPE:$Id$....
+        * If there is no identifier in the stored entry, id 0 is used, to keep backward
+        * compatibility.
+        * If an id is found that is not in the range of the declared configurations,
+        * 0 is returned. This may fail password validation. As of now there is only one
+        * configuration and even if there were more, chances are slim that anything else
+        * was used. So we try at least the first one instead of failing with an exception
+        * as the probability of success is high enough to save the user from a bad experience
+        * and to risk some hassle for the admin finding out in the logs why a login failed,
+        * when it does.
+        *
+        * @param hashPart
+        *                      The hash part of the stored entry, i.e. the part after the TYPE:
+        * @return The configuration id, or
+        *         0 if none was found.
+        */
+       private static int getConfigIdFromStoredPassword(String hashPart) {
+               String[] parts = hashPart.split("\\$", 3);
+               // If there are not two parts, there is no '$'-enclosed part and we have no configuration information stored.
+               // Return default 0.
+               if (parts.length <= 2) return 0;
+
+               // The first string wil be empty. Even if it isn't we ignore it because it doesn't contain our information.
+               try {
+                       int configId = Integer.parseInt(parts[1]);
+                       if (configId < 0 || configId >= configurations.length) {
+                               LOGGER.warn("A user table password entry contains a configuration id that is not valid: {}." +
+                                               "Assuming PBKDF configuration 0. This may fail to validate the password.", configId);
+                               return 0;
+                       }
+                       return configId;
+               }
+               catch (NumberFormatException e) {
+                       LOGGER.warn("A user table password entry contains a configuration id that is not a parsable number ({}${}$...)." +
+                                       "Assuming PBKDF configuration 0. This may fail to validate the password.", parts[0], parts[1], e);
+                       return 0;
+               }
+       }
+
+
+
+
+
+       /**
+        * Hash.
+        *
+        * @param password
+        *          the password
+        * @param salt
+        *          the salt
+        * @param config
+        *                      Parameter configuration to use for the PBKDF
+        * @return Hashed result
+        */
+       private static byte[] hash(char[] password, byte[] salt, Configuration config) {
+               PBEKeySpec spec = new PBEKeySpec(password, salt, config.iterations, config.keyLen);
+               Arrays.fill(password, Character.MIN_VALUE);
+               try {
+                       SecretKeyFactory skf = SecretKeyFactory.getInstance(config.algorithm);
+                       return skf.generateSecret(spec).getEncoded();
+               } catch (NoSuchAlgorithmException | InvalidKeySpecException e) {
+                       LOGGER.warn("Error while hashing password.", 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
+        */
+       private static boolean isPasswordCorrect(char[] passwordToCheck, byte[] salt, byte[] expectedHash, Configuration config) {
+               byte[] hashToCheck = hash(passwordToCheck, salt, config);
+               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;
+       }
+
+
+       /**
+        * Gets the salt from stored password.
+        *
+        * @param storedPassword
+        *            the stored password
+        * @return the salt from stored password
+        */
+       private static byte[] getSaltFromStoredPassword(String storedPassword, Configuration config) {
+               byte[] pw = getStoredHashWithStrippedPrefix(storedPassword);
+               return Arrays.copyOfRange(pw, 0, config.saltLen);
+       }
+
+       /**
+        * Gets the hash from stored password.
+        *
+        * @param storedPassword
+        *            the stored password
+        * @return the hash from stored password
+        */
+       private static byte[] getHashFromStoredPassword(String storedPassword, Configuration config) {
+               byte[] pw = getStoredHashWithStrippedPrefix(storedPassword);
+               return Arrays.copyOfRange(pw, config.saltLen, pw.length);
+       }
+
+       /**
+        * Strips the configuration id prefix ($Id$) from a stored
+        * password and returns the decoded hash
+        *
+        * @param storedPassword
+        *            the stored password
+        * @return the stored hash with stripped prefix
+        */
+       private static byte[] getStoredHashWithStrippedPrefix(String storedPassword) {
+               String[] strings = storedPassword.split("\\$", 3);
+               String saltAndHash = strings[strings.length -1];
+               try {
+                       return Hex.decodeHex(saltAndHash.toCharArray());
+               } catch (DecoderException e) {
+                       LOGGER.warn("Failed to decode stored password entry from hex to string.", e);
+                       throw new IllegalStateException("Error while reading stored credentials", e);
+               }
+       }
+
+       /**
+        * Checks if password is correct.
+        *
+        * @param password
+        *            the password to validate
+        * @param storedPassword
+        *            the stored password, i.e. the password entry value, without the leading TYPE:
+        * @return true, if password is correct, false otherwise
+        */
+       private static boolean isPasswordCorrect(char[] password, String storedPassword, Configuration config) {
+               byte[] storedSalt = getSaltFromStoredPassword(storedPassword, config);
+               byte[] storedHash = getHashFromStoredPassword(storedPassword, config);
+               return isPasswordCorrect(password, storedSalt, storedHash, config);
+       }
+}
index 12686692a3b0e43f2ef46d16c964cf83877cc5c5..c5a485dc936d8f1a830258975498a5dbfbc451ca 100644 (file)
@@ -36,6 +36,15 @@ public class PasswordHashTest {
        static final String CMD5_USERNAME_1 = "Joe Black";
        static final String CMD5_PASSWORD_1 = "ThisIsAWeirdScheme.Weird";
        static final String CMD5_HASHED_ENTRY_1 = "cmd5:5c154768287e32fa605656b98894da89";
+       static final String PBKDF2_PASSWORD_0 = "password";
+       static final String PBKDF2_HASHED_ENTRY_0 = "PBKDF2:70617373776f726450415353574f524470617373776f726450415353574f52440f17d16621b32ae1bb2b1041fcb19e294b35d514d361c08eed385766e38f6f3a";
+       static final String PBKDF2_PASSWORD_1 = "A REALLY better scheme than MD5";
+       static final String PBKDF2_HASHED_ENTRY_1 = "PBKDF2:$0$46726573682066726f6d207468652053414c54206d696e65206f6620446f6f6de8e50b035679b25ce8b6ab41440938b7b1f97fc0c797fcf59302c2916f6c8fef";
+       static final String PBKDF2_PASSWORD_2 = "passwordPASSWORDpassword";
+       static final String PBKDF2_HASHED_ENTRY_2 = "pbkdf2:$0$73616c7453414c5473616c7453414c5473616c7453414c5473616c7453414c54560d0f02b565e37695da15141044506d54cb633a5a70b41c574069ea50a1247a";
+       static final String PBKDF2_PASSWORD_3 = "foo";
+       static final String PBKDF2_HASHED_ENTRY_3 = "PBKDF2WITHHMACSHA256:2d7d3ccaa277787f288e9f929247361bfc83607c6a8447bf496267512e360ba0a97b3114937213b23230072517d65a2e00695a1cbc47a732510840817f22c1bc";
+
 
 
        /**
@@ -98,6 +107,32 @@ public class PasswordHashTest {
 
 
 
+       /**
+        * Test method for {@link com.gitblit.utils.PasswordHash#instanceOf(java.lang.String)} for PBKDF2.
+        */
+       @Test
+       public void testInstanceOfPBKDF2() {
+               PasswordHash pwdh = PasswordHash.instanceOf("PBKDF2");
+               assertNotNull(pwdh);
+               assertEquals(PasswordHash.Type.PBKDF2, pwdh.type);
+               assertTrue("Failed to match " +PBKDF2_HASHED_ENTRY_0, pwdh.matches(PBKDF2_HASHED_ENTRY_0, PBKDF2_PASSWORD_0.toCharArray(), null));
+
+               pwdh = PasswordHash.instanceOf("pbkdf2");
+               assertNotNull(pwdh);
+               assertEquals(PasswordHash.Type.PBKDF2, pwdh.type);
+               assertTrue("Failed to match " +PBKDF2_HASHED_ENTRY_1, pwdh.matches(PBKDF2_HASHED_ENTRY_1, PBKDF2_PASSWORD_1.toCharArray(), null));
+
+               pwdh = PasswordHash.instanceOf("pbKDF2");
+               assertNotNull(pwdh);
+               assertEquals(PasswordHash.Type.PBKDF2, pwdh.type);
+               assertTrue("Failed to match " +PBKDF2_HASHED_ENTRY_1, pwdh.matches(PBKDF2_HASHED_ENTRY_1, PBKDF2_PASSWORD_1.toCharArray(), null));
+
+
+               pwdh = PasswordHash.instanceOf("md5");
+               assertNotNull(pwdh);
+               assertNotEquals(PasswordHash.Type.PBKDF2, pwdh.type);
+               assertFalse("Failed to match " +PBKDF2_HASHED_ENTRY_1, pwdh.matches(PBKDF2_HASHED_ENTRY_1, PBKDF2_PASSWORD_1.toCharArray(), null));
+       }
 
 
 
@@ -165,6 +200,20 @@ public class PasswordHashTest {
                assertTrue("Failed to match " +CMD5_HASHED_ENTRY_1, pwdh.matches(CMD5_HASHED_ENTRY_1, CMD5_PASSWORD_1.toCharArray(), CMD5_USERNAME_1));
 
 
+               pwdh = PasswordHash.instanceFor(PBKDF2_HASHED_ENTRY_0);
+               assertNotNull(pwdh);
+               assertEquals(PasswordHash.Type.PBKDF2, pwdh.type);
+               assertTrue("Failed to match " +PBKDF2_HASHED_ENTRY_0, pwdh.matches(PBKDF2_HASHED_ENTRY_0, PBKDF2_PASSWORD_0.toCharArray(), null));
+
+               pwdh = PasswordHash.instanceFor(PBKDF2_HASHED_ENTRY_1);
+               assertNotNull(pwdh);
+               assertEquals(PasswordHash.Type.PBKDF2, pwdh.type);
+               assertTrue("Failed to match " +PBKDF2_HASHED_ENTRY_1, pwdh.matches(PBKDF2_HASHED_ENTRY_1, PBKDF2_PASSWORD_1.toCharArray(), null));
+
+               pwdh = PasswordHash.instanceFor(PBKDF2_HASHED_ENTRY_3);
+               assertNotNull(pwdh);
+               assertEquals(PasswordHash.Type.PBKDF2, pwdh.type);
+               assertTrue("Failed to match " +PBKDF2_HASHED_ENTRY_3, pwdh.matches(PBKDF2_HASHED_ENTRY_3, PBKDF2_PASSWORD_3.toCharArray(), null));
        }
 
        /**
@@ -210,6 +259,10 @@ public class PasswordHashTest {
                assertTrue(MD5_HASHED_ENTRY_1, PasswordHash.isHashedEntry(MD5_HASHED_ENTRY_1));
                assertTrue(CMD5_HASHED_ENTRY_0, PasswordHash.isHashedEntry(CMD5_HASHED_ENTRY_0));
                assertTrue(CMD5_HASHED_ENTRY_1, PasswordHash.isHashedEntry(CMD5_HASHED_ENTRY_1));
+               assertTrue(PBKDF2_HASHED_ENTRY_0, PasswordHash.isHashedEntry(PBKDF2_HASHED_ENTRY_0));
+               assertTrue(PBKDF2_HASHED_ENTRY_1, PasswordHash.isHashedEntry(PBKDF2_HASHED_ENTRY_1));
+               assertTrue(PBKDF2_HASHED_ENTRY_2, PasswordHash.isHashedEntry(PBKDF2_HASHED_ENTRY_2));
+               assertTrue(PBKDF2_HASHED_ENTRY_3, PasswordHash.isHashedEntry(PBKDF2_HASHED_ENTRY_3));
 
                assertFalse(MD5_PASSWORD_1, PasswordHash.isHashedEntry(MD5_PASSWORD_1));
                assertFalse("topsecret", PasswordHash.isHashedEntry("topsecret"));
@@ -228,6 +281,8 @@ public class PasswordHashTest {
        public void testIsHashedEntryCaseInsenitive() {
                assertTrue(MD5_HASHED_ENTRY_1.toLowerCase(), PasswordHash.isHashedEntry(MD5_HASHED_ENTRY_1.toLowerCase()));
                assertTrue(CMD5_HASHED_ENTRY_1.toLowerCase(), PasswordHash.isHashedEntry(CMD5_HASHED_ENTRY_1.toLowerCase()));
+               assertTrue(PBKDF2_HASHED_ENTRY_1.toLowerCase(), PasswordHash.isHashedEntry(PBKDF2_HASHED_ENTRY_1.toLowerCase()));
+               assertTrue(PBKDF2_HASHED_ENTRY_3.toLowerCase(), PasswordHash.isHashedEntry(PBKDF2_HASHED_ENTRY_3.toLowerCase()));
        }
 
        /**
@@ -248,7 +303,7 @@ public class PasswordHashTest {
         * Test creating a hashed entry for scheme MD5. In this scheme there is no salt, so a direct
         * comparison to a constant value is possible.
         *
-        * Test method for {@link com.gitblit.utils.PasswordHash#toHashedEntry(String, String)} for MD5.
+        * Test method for {@link PasswordHash#toHashedEntry(String, String)} for MD5.
         */
        @Test
        public void testToHashedEntryMD5() {
@@ -269,7 +324,7 @@ public class PasswordHashTest {
        @Test(expected = IllegalArgumentException.class)
        public void testToHashedEntryMD5NullPassword() {
                PasswordHash pwdh = PasswordHash.instanceOf("MD5");
-               pwdh.toHashedEntry(null, null);
+               pwdh.toHashedEntry((String)null, null);
        }
 
 
@@ -277,7 +332,7 @@ public class PasswordHashTest {
         * Test creating a hashed entry for scheme Combined-MD5. In this scheme there is no salt, so a direct
         * comparison to a constant value is possible.
         *
-        * Test method for {@link com.gitblit.utils.PasswordHash#toHashedEntry(String, String)} for CMD5.
+        * Test method for {@link PasswordHash#toHashedEntry(String, String)} for CMD5.
         */
        @Test
        public void testToHashedEntryCMD5() {
@@ -295,14 +350,14 @@ public class PasswordHashTest {
        @Test(expected = IllegalArgumentException.class)
        public void testToHashedEntryCMD5NullPassword() {
                PasswordHash pwdh = PasswordHash.instanceOf("CMD5");
-               pwdh.toHashedEntry(null, CMD5_USERNAME_1);
+               pwdh.toHashedEntry((String)null, CMD5_USERNAME_1);
        }
 
        /**
         * Test creating a hashed entry for scheme Combined-MD5, when no user is given.
         * This should never happen in the application, so we expect an exception to be thrown.
         *
-        * Test method for {@link com.gitblit.utils.PasswordHash#toHashedEntry(String, String)} for broken CMD5.
+        * Test method for {@link PasswordHash#toHashedEntry(String, String)} for broken CMD5.
         */
        @Test
        public void testToHashedEntryCMD5NoUsername() {
@@ -332,7 +387,40 @@ public class PasswordHashTest {
                catch (IllegalArgumentException ignored) { /*success*/ }
        }
 
+       /**
+        * Test creating a hashed entry for scheme PBKDF2.
+        * Since this scheme uses a salt, we test by running a match. This is a bit backwards,
+        * but recreating the PBKDF2 here seems a little overkill.
+        *
+        * Test method for {@link PasswordHash#toHashedEntry(String, String)} for PBKDF2.
+        */
+       @Test
+       public void testToHashedEntryPBKDF2() {
+               PasswordHash pwdh = PasswordHash.instanceOf("PBKDF2");
+               String hashedEntry = pwdh.toHashedEntry(PBKDF2_PASSWORD_1, null);
+               assertTrue("Type identifier is incorrect.", hashedEntry.startsWith(PasswordHash.Type.PBKDF2.name()));
+               PasswordHash pwdhverify = PasswordHash.instanceFor(hashedEntry);
+               assertNotNull(pwdhverify);
+               assertTrue(PBKDF2_PASSWORD_1, pwdhverify.matches(hashedEntry, PBKDF2_PASSWORD_1.toCharArray(), null));
+
+               hashedEntry = pwdh.toHashedEntry(PBKDF2_PASSWORD_2, "");
+               assertTrue("Type identifier is incorrect.", hashedEntry.startsWith(PasswordHash.Type.PBKDF2.name()));
+               pwdhverify = PasswordHash.instanceFor(hashedEntry);
+               assertNotNull(pwdhverify);
+               assertTrue(PBKDF2_PASSWORD_2, pwdhverify.matches(hashedEntry, PBKDF2_PASSWORD_2.toCharArray(), null));
+
+               hashedEntry = pwdh.toHashedEntry(PBKDF2_PASSWORD_0, "alpha");
+               assertTrue("Type identifier is incorrect.", hashedEntry.startsWith(PasswordHash.Type.PBKDF2.name()));
+               pwdhverify = PasswordHash.instanceFor(hashedEntry);
+               assertNotNull(pwdhverify);
+               assertTrue(PBKDF2_PASSWORD_0, pwdhverify.matches(hashedEntry, PBKDF2_PASSWORD_0.toCharArray(), null));
+       }
 
+       @Test(expected = IllegalArgumentException.class)
+       public void testToHashedEntryPBKDF2NullPassword() {
+               PasswordHash pwdh = PasswordHash.instanceOf("PBKDF2");
+               pwdh.toHashedEntry((String)null, null);
+       }
 
 
        /**
@@ -379,6 +467,7 @@ public class PasswordHashTest {
 
 
                assertFalse("Matched wrong scheme", pwdh.matches(CMD5_HASHED_ENTRY_0, MD5_PASSWORD_0.toCharArray(), null));
+               assertFalse("Matched wrong scheme", pwdh.matches(PBKDF2_HASHED_ENTRY_0, MD5_PASSWORD_0.toCharArray(), ""));
                assertFalse("Matched wrong scheme", pwdh.matches(CMD5_HASHED_ENTRY_0, CMD5_PASSWORD_0.toCharArray(), CMD5_USERNAME_0));
        }
 
@@ -414,7 +503,94 @@ public class PasswordHashTest {
 
 
                assertFalse("Matched wrong scheme", pwdh.matches(MD5_HASHED_ENTRY_0, CMD5_PASSWORD_0.toCharArray(), null));
+               assertFalse("Matched wrong scheme", pwdh.matches(PBKDF2_HASHED_ENTRY_0, CMD5_PASSWORD_0.toCharArray(), ""));
                assertFalse("Matched wrong scheme", pwdh.matches(MD5_HASHED_ENTRY_0, CMD5_PASSWORD_0.toCharArray(), CMD5_USERNAME_0));
                assertFalse("Matched wrong scheme", pwdh.matches(MD5_HASHED_ENTRY_0, MD5_PASSWORD_0.toCharArray(), CMD5_USERNAME_0));
        }
+
+
+
+       /**
+        * Test method for {@link com.gitblit.utils.PasswordHash#matches(String, char[], String)} for PBKDF2.
+        */
+       @Test
+       public void testMatchesPBKDF2() {
+               PasswordHash pwdh = PasswordHash.instanceOf("PBKDF2");
+
+               assertTrue("PWD0, Null user", pwdh.matches(PBKDF2_HASHED_ENTRY_0, PBKDF2_PASSWORD_0.toCharArray(), null));
+               assertTrue("PWD0, Empty user", pwdh.matches(PBKDF2_HASHED_ENTRY_0, PBKDF2_PASSWORD_0.toCharArray(), ""));
+               assertTrue("PWD0, With user", pwdh.matches(PBKDF2_HASHED_ENTRY_0, PBKDF2_PASSWORD_0.toCharArray(), "maxine"));
+
+               assertTrue("PWD1, Null user", pwdh.matches(PBKDF2_HASHED_ENTRY_1, PBKDF2_PASSWORD_1.toCharArray(), null));
+               assertTrue("PWD1, Empty user", pwdh.matches(PBKDF2_HASHED_ENTRY_1, PBKDF2_PASSWORD_1.toCharArray(), ""));
+               assertTrue("PWD1, With user", pwdh.matches(PBKDF2_HASHED_ENTRY_1, PBKDF2_PASSWORD_1.toCharArray(), "Maxim Gorki"));
+
+               assertTrue("PWD2, Null user", pwdh.matches(PBKDF2_HASHED_ENTRY_2, PBKDF2_PASSWORD_2.toCharArray(), null));
+               assertTrue("PWD2, Empty user", pwdh.matches(PBKDF2_HASHED_ENTRY_2, PBKDF2_PASSWORD_2.toCharArray(), ""));
+               assertTrue("PWD2, With user", pwdh.matches(PBKDF2_HASHED_ENTRY_2, PBKDF2_PASSWORD_2.toCharArray(), "Epson"));
+
+
+
+               assertFalse("Matched wrong password", pwdh.matches(PBKDF2_HASHED_ENTRY_1, "wrongpassword".toCharArray(), null));
+               assertFalse("Matched wrong password, with empty user", pwdh.matches(PBKDF2_HASHED_ENTRY_1, "wrongpassword".toCharArray(), "  "));
+               assertFalse("Matched wrong password, with user", pwdh.matches(PBKDF2_HASHED_ENTRY_1, "wrongpassword".toCharArray(), "someuser"));
+
+               assertFalse("Matched empty password", pwdh.matches(PBKDF2_HASHED_ENTRY_2, "".toCharArray(), null));
+               assertFalse("Matched empty password, with empty user", pwdh.matches(PBKDF2_HASHED_ENTRY_2, " ".toCharArray(), "  "));
+               assertFalse("Matched empty password, with user", pwdh.matches(PBKDF2_HASHED_ENTRY_2, "  ".toCharArray(), "someuser"));
+
+               assertFalse("Matched null password", pwdh.matches(PBKDF2_HASHED_ENTRY_0, null, null));
+               assertFalse("Matched null password, with empty user", pwdh.matches(PBKDF2_HASHED_ENTRY_0, null, "  "));
+               assertFalse("Matched null password, with user", pwdh.matches(PBKDF2_HASHED_ENTRY_0, null, "someuser"));
+
+
+               assertFalse("Matched wrong hashed entry", pwdh.matches(PBKDF2_HASHED_ENTRY_1, PBKDF2_PASSWORD_0.toCharArray(), null));
+               assertFalse("Matched wrong hashed entry, with empty user", pwdh.matches(PBKDF2_HASHED_ENTRY_1, PBKDF2_PASSWORD_0.toCharArray(), ""));
+               assertFalse("Matched wrong hashed entry, with user", pwdh.matches(PBKDF2_HASHED_ENTRY_1, PBKDF2_PASSWORD_0.toCharArray(), "someuser"));
+
+               assertFalse("Matched empty hashed entry", pwdh.matches("", PBKDF2_PASSWORD_0.toCharArray(), null));
+               assertFalse("Matched empty hashed entry, with empty user", pwdh.matches("  ", PBKDF2_PASSWORD_0.toCharArray(), ""));
+               assertFalse("Matched empty hashed entry, with user", pwdh.matches("     ", PBKDF2_PASSWORD_0.toCharArray(), "someuser"));
+
+               assertFalse("Matched null entry", pwdh.matches(null, PBKDF2_PASSWORD_0.toCharArray(), null));
+               assertFalse("Matched null entry, with empty user", pwdh.matches(null, PBKDF2_PASSWORD_0.toCharArray(), ""));
+               assertFalse("Matched null entry, with user", pwdh.matches(null, PBKDF2_PASSWORD_0.toCharArray(), "someuser"));
+
+
+               assertFalse("Matched wrong scheme", pwdh.matches(CMD5_HASHED_ENTRY_0, PBKDF2_PASSWORD_0.toCharArray(), null));
+               assertFalse("Matched wrong scheme", pwdh.matches(MD5_HASHED_ENTRY_0, PBKDF2_PASSWORD_0.toCharArray(), ""));
+               assertFalse("Matched wrong scheme", pwdh.matches(CMD5_HASHED_ENTRY_0, PBKDF2_PASSWORD_0.toCharArray(), CMD5_USERNAME_0));
+       }
+
+
+       /**
+        * Test method for {@link com.gitblit.utils.PasswordHash#matches(String, char[], String)}
+        * for old existing entries with the "PBKDF2WITHHMACSHA256" type identifier.
+        */
+       @Test
+       public void testMatchesPBKDF2Compat() {
+               PasswordHash pwdh = PasswordHash.instanceOf("PBKDF2");
+
+               assertTrue("PWD3, Null user", pwdh.matches(PBKDF2_HASHED_ENTRY_3, PBKDF2_PASSWORD_3.toCharArray(), null));
+               assertTrue("PWD3, Empty user", pwdh.matches(PBKDF2_HASHED_ENTRY_3, PBKDF2_PASSWORD_3.toCharArray(), ""));
+               assertTrue("PWD3, With user", pwdh.matches(PBKDF2_HASHED_ENTRY_3, PBKDF2_PASSWORD_3.toCharArray(), "maxine"));
+
+
+               assertFalse("Matched wrong password", pwdh.matches(PBKDF2_HASHED_ENTRY_3, "bar".toCharArray(), null));
+               assertFalse("Matched wrong password, with empty user", pwdh.matches(PBKDF2_HASHED_ENTRY_3, "bar".toCharArray(), "  "));
+               assertFalse("Matched wrong password, with user", pwdh.matches(PBKDF2_HASHED_ENTRY_3, "bar".toCharArray(), "someuser"));
+
+               assertFalse("Matched empty password", pwdh.matches(PBKDF2_HASHED_ENTRY_3, "".toCharArray(), null));
+               assertFalse("Matched empty password, with empty user", pwdh.matches(PBKDF2_HASHED_ENTRY_3, " ".toCharArray(), "  "));
+               assertFalse("Matched empty password, with user", pwdh.matches(PBKDF2_HASHED_ENTRY_3, "  ".toCharArray(), "someuser"));
+
+               assertFalse("Matched null password", pwdh.matches(PBKDF2_HASHED_ENTRY_3, null, null));
+               assertFalse("Matched null password, with empty user", pwdh.matches(PBKDF2_HASHED_ENTRY_3, null, "  "));
+               assertFalse("Matched null password, with user", pwdh.matches(PBKDF2_HASHED_ENTRY_3, null, "someuser"));
+
+
+               assertFalse("Matched wrong hashed entry", pwdh.matches(PBKDF2_HASHED_ENTRY_3, PBKDF2_PASSWORD_0.toCharArray(), null));
+               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"));
+       }
 }