From 80ab41ae5a522486a98b3c632f40fbb122bc36fa Mon Sep 17 00:00:00 2001 From: ljacqu Date: Sat, 4 Nov 2017 09:58:51 +0100 Subject: [PATCH 1/2] #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 --- .../xephi/authme/security/crypts/PhpBB.java | 256 +++++++++--------- .../authme/security/crypts/PhpBBTest.java | 51 +++- 2 files changed, 177 insertions(+), 130 deletions(-) diff --git a/src/main/java/fr/xephi/authme/security/crypts/PhpBB.java b/src/main/java/fr/xephi/authme/security/crypts/PhpBB.java index e5f7e54ca..4e618e241 100644 --- a/src/main/java/fr/xephi/authme/security/crypts/PhpBB.java +++ b/src/main/java/fr/xephi/authme/security/crypts/PhpBB.java @@ -7,146 +7,150 @@ import java.io.UnsupportedEncodingException; import java.security.MessageDigest; /** - * @author stefano + * Encryption method compatible with phpBB3. + *

+ * 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 static final String itoa64 = "./0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz"; + private final BCrypt2y bCrypt2y = new BCrypt2y(); - private static String md5(String data) { - try { - byte[] bytes = data.getBytes("ISO-8859-1"); - MessageDigest md5er = HashUtils.getDigest(MessageDigestAlgorithm.MD5); - byte[] hash = md5er.digest(bytes); - return bytes2hex(hash); - } catch (UnsupportedEncodingException e) { - throw new UnsupportedOperationException(e); - } - } - - private static int hexToInt(char ch) { - if (ch >= '0' && ch <= '9') - return ch - '0'; - ch = Character.toUpperCase(ch); - if (ch >= 'A' && ch <= 'F') - return ch - 'A' + 0xA; - throw new IllegalArgumentException("Not a hex character: " + ch); - } - - private static String bytes2hex(byte[] bytes) { - StringBuilder r = new StringBuilder(32); - for (byte b : bytes) { - String x = Integer.toHexString(b & 0xff); - if (x.length() < 2) - r.append('0'); - r.append(x); - } - return r.toString(); - } - - private static String pack(String hex) { - StringBuilder buf = new StringBuilder(); - for (int i = 0; i < hex.length(); i += 2) { - char c1 = hex.charAt(i); - char c2 = hex.charAt(i + 1); - char packed = (char) (hexToInt(c1) * 16 + hexToInt(c2)); - buf.append(packed); - } - return buf.toString(); - } - - private String phpbb_hash(String password, String salt) { - 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 { - int value = input.charAt(i++); - output.append(itoa64.charAt(value & 0x3f)); - if (i < count) - value |= input.charAt(i) << 8; - output.append(itoa64.charAt((value >> 6) & 0x3f)); - if (i++ >= count) - break; - if (i < count) - value |= input.charAt(i) << 16; - output.append(itoa64.charAt((value >> 12) & 0x3f)); - if (i++ >= count) - break; - output.append(itoa64.charAt((value >> 18) & 0x3f)); - } while (i < count); - return output.toString(); - } - - private String _hash_crypt_private(String password, String setting) { - String output = "*"; - if (!setting.substring(0, 3).equals("$H$")) - return output; - int count_log2 = itoa64.indexOf(setting.charAt(3)); - if (count_log2 < 7 || count_log2 > 30) - return output; - int count = 1 << count_log2; - String salt = setting.substring(4, 12); - if (salt.length() != 8) - return output; - String m1 = md5(salt + password); - String hash = pack(m1); - do { - hash = pack(md5(hash + password)); - } while (--count > 0); - output = setting.substring(0, 12); - output += _hash_encode64(hash, 16); - return output; - } - - private 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 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 phpbb_hash(password, salt); + return bCrypt2y.computeHash(password, salt, name); } @Override public boolean comparePassword(String password, HashedPassword hashedPassword, String name) { - return phpbb_check_hash(password, hashedPassword.getHash()); + 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 int getSaltLength() { - return 16; + 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 phpBB's salted_md5.php + * @see phpass + */ + private static final class PhpassSaltedMd5 { + + private static final String itoa64 = "./0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz"; + + private static String md5(String data) { + try { + byte[] bytes = data.getBytes("ISO-8859-1"); + MessageDigest md5er = HashUtils.getDigest(MessageDigestAlgorithm.MD5); + byte[] hash = md5er.digest(bytes); + return bytes2hex(hash); + } catch (UnsupportedEncodingException e) { + throw new UnsupportedOperationException(e); + } + } + + private static int hexToInt(char ch) { + if (ch >= '0' && ch <= '9') + return ch - '0'; + ch = Character.toUpperCase(ch); + if (ch >= 'A' && ch <= 'F') + return ch - 'A' + 0xA; + throw new IllegalArgumentException("Not a hex character: " + ch); + } + + private static String bytes2hex(byte[] bytes) { + StringBuilder r = new StringBuilder(32); + for (byte b : bytes) { + String x = Integer.toHexString(b & 0xff); + if (x.length() < 2) + r.append('0'); + r.append(x); + } + return r.toString(); + } + + private static String pack(String hex) { + StringBuilder buf = new StringBuilder(); + for (int i = 0; i < hex.length(); i += 2) { + char c1 = hex.charAt(i); + char c2 = hex.charAt(i + 1); + char packed = (char) (hexToInt(c1) * 16 + hexToInt(c2)); + buf.append(packed); + } + return buf.toString(); + } + + private static String _hash_encode64(String input, int count) { + StringBuilder output = new StringBuilder(); + int i = 0; + do { + int value = input.charAt(i++); + output.append(itoa64.charAt(value & 0x3f)); + if (i < count) + value |= input.charAt(i) << 8; + output.append(itoa64.charAt((value >> 6) & 0x3f)); + if (i++ >= count) + break; + if (i < count) + value |= input.charAt(i) << 16; + output.append(itoa64.charAt((value >> 12) & 0x3f)); + if (i++ >= count) + break; + output.append(itoa64.charAt((value >> 18) & 0x3f)); + } while (i < count); + return output.toString(); + } + + private static String _hash_crypt_private(String password, String setting) { + String output = "*"; + if (!setting.substring(0, 3).equals("$H$")) + return output; + int count_log2 = itoa64.indexOf(setting.charAt(3)); + if (count_log2 < 7 || count_log2 > 30) + return output; + int count = 1 << count_log2; + String salt = setting.substring(4, 12); + if (salt.length() != 8) + return output; + String m1 = md5(salt + password); + String hash = pack(m1); + do { + hash = pack(md5(hash + password)); + } while (--count > 0); + output = setting.substring(0, 12); + output += _hash_encode64(hash, 16); + return output; + } + + 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); + } + } } diff --git a/src/test/java/fr/xephi/authme/security/crypts/PhpBBTest.java b/src/test/java/fr/xephi/authme/security/crypts/PhpBBTest.java index f495659ec..743a7b46a 100644 --- a/src/test/java/fr/xephi/authme/security/crypts/PhpBBTest.java +++ b/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 - "$H$7ESfAVjzqajC7fJFcZKZIhyds41MuW.", // PassWord1 - "$H$7G65SXRPbR69jLg.qZTjtqsw36Ciw7.", // &^%te$t?Pw@_ - "$H$7Brcg8zO9amr2SHVgz.pFxprDu40v4/"); // âË_3(íù* + "$2a$10$1rnuna3GBduBy1NQuOpnWODqBfl8CZHeULuBThNfAvkOYDRRQR1Zi", // password + "$2a$10$F6LVgXa8.t95H0Fikr6nG.aEMgIQRXlFpzMvAjbO7ag3fny9GGS3i", // PassWord1 + "$2a$10$ex57hkfuMLwYsdG8ru/4teh48kHCSv0HPLPjhhHsEB3NqXiOi7RQS", // &^%te$t?Pw@_ + "$2a$10$2B/HAJ3MeoxGQgqLM6GDlOBqd.2uzLPi1VznXlrXcayLixSaRIWqC"); // âË_3(íù* + } + + @Test + public void shouldMatchPhpassSaltedMd5Hashes() { + // given + Map 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 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 givenHashes = ImmutableMap.of( + "password", "5f4dcc3b5aa765d61d8327deb882cf99", + "PassWord1", "f2126d405f46ed603ff5b2950f062c96", + "&^%te$t?Pw@_", "0833dcd2bc741f90c46bbac5498fd08f", + "âË_3(íù*", "e7412bf1a9d312dc2901c3101a097abe"); + PhpBB phpBB = new PhpBB(); + + // when / then + for (Map.Entry hashEntry : givenHashes.entrySet()) { + if (!phpBB.comparePassword(hashEntry.getKey(), new HashedPassword(hashEntry.getValue()), null)) { + fail("Hash comparison for '" + hashEntry.getKey() + "' failed"); + } + } } } From 5c40cb3b730dcc95864900cf9f945658e3f9f2b7 Mon Sep 17 00:00:00 2001 From: ljacqu Date: Sat, 4 Nov 2017 11:29:21 +0100 Subject: [PATCH 2/2] Insert null email as DEFAULT so column default may take effect Ugly implementation to fit into AuthMe 5.4. If email on PlayerAuth is null, do not supply NULL as the email value but use DEFAULT instead so that the default value is used if present in the column configuration. --- .../fr/xephi/authme/datasource/MySQL.java | 26 ++++++++++++------- .../xephi/authme/security/crypts/PhpBB.java | 5 +--- 2 files changed, 17 insertions(+), 14 deletions(-) diff --git a/src/main/java/fr/xephi/authme/datasource/MySQL.java b/src/main/java/fr/xephi/authme/datasource/MySQL.java index 24b287774..98bbaa68b 100644 --- a/src/main/java/fr/xephi/authme/datasource/MySQL.java +++ b/src/main/java/fr/xephi/authme/datasource/MySQL.java @@ -320,22 +320,28 @@ public class MySQL implements DataSource { @Override public boolean saveAuth(PlayerAuth auth) { try (Connection con = getConnection()) { - String sql; + // TODO ljacqu 20171104: Replace with generic columns util to clean this up boolean useSalt = !col.SALT.isEmpty() || !StringUtils.isEmpty(auth.getPassword().getSalt()); - sql = "INSERT INTO " + tableName + "(" + boolean hasEmail = auth.getEmail() != null; + String emailPlaceholder = hasEmail ? "?" : "DEFAULT"; + + String sql = "INSERT INTO " + tableName + "(" + col.NAME + "," + col.PASSWORD + "," + col.REAL_NAME + "," + col.EMAIL + "," + col.REGISTRATION_DATE + "," + col.REGISTRATION_IP + (useSalt ? "," + col.SALT : "") - + ") VALUES (?,?,?,?,?,?" + (useSalt ? ",?" : "") + ");"; + + ") VALUES (?,?,?," + emailPlaceholder + ",?,?" + (useSalt ? ",?" : "") + ");"; try (PreparedStatement pst = con.prepareStatement(sql)) { - pst.setString(1, auth.getNickname()); - pst.setString(2, auth.getPassword().getHash()); - pst.setString(3, auth.getRealName()); - pst.setString(4, auth.getEmail()); - pst.setObject(5, auth.getRegistrationDate()); - pst.setString(6, auth.getRegistrationIp()); + int index = 1; + pst.setString(index++, auth.getNickname()); + pst.setString(index++, auth.getPassword().getHash()); + pst.setString(index++, auth.getRealName()); + if (hasEmail) { + pst.setString(index++, auth.getEmail()); + } + pst.setObject(index++, auth.getRegistrationDate()); + pst.setString(index++, auth.getRegistrationIp()); if (useSalt) { - pst.setString(7, auth.getPassword().getSalt()); + pst.setString(index++, auth.getPassword().getSalt()); } pst.executeUpdate(); } diff --git a/src/main/java/fr/xephi/authme/security/crypts/PhpBB.java b/src/main/java/fr/xephi/authme/security/crypts/PhpBB.java index 4e618e241..8192306fc 100644 --- a/src/main/java/fr/xephi/authme/security/crypts/PhpBB.java +++ b/src/main/java/fr/xephi/authme/security/crypts/PhpBB.java @@ -147,10 +147,7 @@ public class PhpBB implements EncryptionMethod { } 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); + return _hash_crypt_private(password, hash).equals(hash); } } }