#1400 Sync AuthMe's phpBB hash implementation with phpBB3's

- phpBB3 seems to favor using BCrypt $2y$ now
- Keep unsalted MD5 and phpass salted MD5 comparisons for backwards compatibility

src/main/java/fr/xephi/authme/security/crypts/PhpBB.java

@@ -7,9 +7,58 @@ import java.io.UnsupportedEncodingException;
 import java.security.MessageDigest;
 
 /**
- * @author stefano
+ * Encryption method compatible with phpBB3.
+ * <p>
+ * As tested with phpBB 3.2.1, by default new passwords are encrypted with BCrypt $2y$.
+ * For backwards compatibility, phpBB3 supports other hashes for comparison. This implementation
+ * successfully checks against phpBB's salted MD5 hashing algorithm (adaptation of phpass),
+ * as well as plain MD5.
  */
-public class PhpBB extends HexSaltedMethod {
+public class PhpBB implements EncryptionMethod {
+
+    private final BCrypt2y bCrypt2y = new BCrypt2y();
+
+    @Override
+    public HashedPassword computeHash(String password, String name) {
+        String salt = generateSalt();
+        return new HashedPassword(BCryptService.hashpw(password, salt));
+    }
+
+    @Override
+    public String computeHash(String password, String salt, String name) {
+        return bCrypt2y.computeHash(password, salt, name);
+    }
+
+    @Override
+    public boolean comparePassword(String password, HashedPassword hashedPassword, String name) {
+        final String hash = hashedPassword.getHash();
+        if (HashUtils.isValidBcryptHash(hash)) {
+            return bCrypt2y.comparePassword(password, hashedPassword, name);
+        } else if (hash.length() == 34) {
+            return PhpassSaltedMd5.phpbb_check_hash(password, hash);
+        } else {
+            return PhpassSaltedMd5.md5(password).equals(hash);
+        }
+    }
+
+    @Override
+    public String generateSalt() {
+        // Salt length 22, as seen in https://github.com/phpbb/phpbb/blob/master/phpBB/phpbb/passwords/driver/bcrypt.php
+        return BCryptService.gensalt(10);
+    }
+
+    @Override
+    public boolean hasSeparateSalt() {
+        return false;
+    }
+
+    /**
+     * Java implementation of the salted MD5 as used in phpBB (adapted from phpass).
+     *
+     * @see <a href="https://github.com/phpbb/phpbb/blob/master/phpBB/phpbb/passwords/driver/salted_md5.php">phpBB's salted_md5.php</a>
+     * @see <a href="http://www.openwall.com/phpass/">phpass</a>
+     */
+    private static final class PhpassSaltedMd5 {
 
         private static final String itoa64 = "./0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz";
 
@@ -55,37 +104,7 @@ public class PhpBB extends HexSaltedMethod {
             return buf.toString();
         }
 
-    private String phpbb_hash(String password, String salt) {
+        private static String _hash_encode64(String input, int count) {
-        String random_state = salt;
-        StringBuilder random = new StringBuilder();
-        int count = 6;
-        for (int i = 0; i < count; i += 16) {
-            random_state = md5(salt + random_state);
-            random.append(pack(md5(random_state)));
-        }
-        String hash = _hash_crypt_private(password, _hash_gensalt_private(random.substring(0, count), itoa64));
-        if (hash.length() == 34) {
-            return hash;
-        }
-        return md5(password);
-    }
-
-    private String _hash_gensalt_private(String input, String itoa64) {
-        return _hash_gensalt_private(input, itoa64, 6);
-    }
-
-    private String _hash_gensalt_private(String input, String itoa64,
-                                         int iteration_count_log2) {
-        if (iteration_count_log2 < 4 || iteration_count_log2 > 31) {
-            iteration_count_log2 = 8;
-        }
-        String output = "$H$";
-        output += itoa64.charAt(Math.min(iteration_count_log2 + 3, 30)); // PHP_VERSION >= 5 ? 5 : 3
-        output += _hash_encode64(input, 6);
-        return output;
-    }
-
-    private String _hash_encode64(String input, int count) {
             StringBuilder output = new StringBuilder();
             int i = 0;
             do {
@@ -106,7 +125,7 @@ public class PhpBB extends HexSaltedMethod {
             return output.toString();
         }
 
-    private String _hash_crypt_private(String password, String setting) {
+        private static String _hash_crypt_private(String password, String setting) {
             String output = "*";
             if (!setting.substring(0, 3).equals("$H$"))
                 return output;
@@ -127,26 +146,11 @@ public class PhpBB extends HexSaltedMethod {
             return output;
         }
 
-    private boolean phpbb_check_hash(String password, String hash) {
+        private static boolean phpbb_check_hash(String password, String hash) {
             if (hash.length() == 34) {
                 return _hash_crypt_private(password, hash).equals(hash);
             }
             return md5(password).equals(hash);
         }
-
-    @Override
-    public String computeHash(String password, String salt, String name) {
-        return phpbb_hash(password, salt);
     }
-
-    @Override
-    public boolean comparePassword(String password, HashedPassword hashedPassword, String name) {
-        return phpbb_check_hash(password, hashedPassword.getHash());
-    }
-
-    @Override
-    public int getSaltLength() {
-        return 16;
-    }
-
 }

src/test/java/fr/xephi/authme/security/crypts/PhpBBTest.java

@@ -1,5 +1,12 @@
 package fr.xephi.authme.security.crypts;
 
+import com.google.common.collect.ImmutableMap;
+import org.junit.Test;
+
+import java.util.Map;
+
+import static org.junit.Assert.fail;
+
 /**
  * Test for {@link PhpBB}.
  */
@@ -7,10 +14,46 @@ public class PhpBBTest extends AbstractEncryptionMethodTest {
 
     public PhpBBTest() {
         super(new PhpBB(),
-            "$H$7MaSGQb0xe3Fp/a.Q.Ewpw.UKfCv.t0",  // password
+            "$2a$10$1rnuna3GBduBy1NQuOpnWODqBfl8CZHeULuBThNfAvkOYDRRQR1Zi",  // password
-            "$H$7ESfAVjzqajC7fJFcZKZIhyds41MuW.",  // PassWord1
+            "$2a$10$F6LVgXa8.t95H0Fikr6nG.aEMgIQRXlFpzMvAjbO7ag3fny9GGS3i",  // PassWord1
-            "$H$7G65SXRPbR69jLg.qZTjtqsw36Ciw7.",  // &^%te$t?Pw@_
+            "$2a$10$ex57hkfuMLwYsdG8ru/4teh48kHCSv0HPLPjhhHsEB3NqXiOi7RQS",  // &^%te$t?Pw@_
-            "$H$7Brcg8zO9amr2SHVgz.pFxprDu40v4/"); // âË_3(íù*
+            "$2a$10$2B/HAJ3MeoxGQgqLM6GDlOBqd.2uzLPi1VznXlrXcayLixSaRIWqC"); // âË_3(íù*
+    }
+
+    @Test
+    public void shouldMatchPhpassSaltedMd5Hashes() {
+        // given
+        Map<String, String> givenHashes = ImmutableMap.of(
+            "password", "$H$7MaSGQb0xe3Fp/a.Q.Ewpw.UKfCv.t0",
+            "PassWord1", "$H$7ESfAVjzqajC7fJFcZKZIhyds41MuW.",
+            "&^%te$t?Pw@_", "$H$7G65SXRPbR69jLg.qZTjtqsw36Ciw7.",
+            "âË_3(íù*", "$H$7Brcg8zO9amr2SHVgz.pFxprDu40v4/");
+        PhpBB phpBB = new PhpBB();
+
+        // when / then
+        for (Map.Entry<String, String> hashEntry : givenHashes.entrySet()) {
+            if (!phpBB.comparePassword(hashEntry.getKey(), new HashedPassword(hashEntry.getValue()), null)) {
+                fail("Hash comparison for '" + hashEntry.getKey() + "' failed");
+            }
+        }
+    }
+
+    @Test
+    public void shouldMatchUnsaltedMd5Hashes() {
+        // given
+        Map<String, String> givenHashes = ImmutableMap.of(
+            "password", "5f4dcc3b5aa765d61d8327deb882cf99",
+            "PassWord1", "f2126d405f46ed603ff5b2950f062c96",
+            "&^%te$t?Pw@_", "0833dcd2bc741f90c46bbac5498fd08f",
+            "âË_3(íù*", "e7412bf1a9d312dc2901c3101a097abe");
+        PhpBB phpBB = new PhpBB();
+
+        // when / then
+        for (Map.Entry<String, String> hashEntry : givenHashes.entrySet()) {
+            if (!phpBB.comparePassword(hashEntry.getKey(), new HashedPassword(hashEntry.getValue()), null)) {
+                fail("Hash comparison for '" + hashEntry.getKey() + "' failed");
+            }
+        }
     }
 
 }