Merge branch 'master' of https://github.com/AuthMe/AuthMeReloaded into 1141-optional-additional-2fa-auth

src/main/java/fr/xephi/authme/AuthMe.java

@@ -2,9 +2,7 @@ package fr.xephi.authme;
 
 import ch.jalu.injector.Injector;
 import ch.jalu.injector.InjectorBuilder;
-
 import com.google.common.annotations.VisibleForTesting;
-
 import fr.xephi.authme.api.NewAPI;
 import fr.xephi.authme.command.CommandHandler;
 import fr.xephi.authme.datasource.DataSource;
@@ -35,9 +33,6 @@ import fr.xephi.authme.settings.properties.SecuritySettings;
 import fr.xephi.authme.task.CleanupTask;
 import fr.xephi.authme.task.purge.PurgeService;
 import fr.xephi.authme.util.ExceptionUtils;
-
-import java.io.File;
-
 import org.apache.commons.lang.SystemUtils;
 import org.bukkit.Server;
 import org.bukkit.command.Command;
@@ -48,6 +43,8 @@ import org.bukkit.plugin.java.JavaPlugin;
 import org.bukkit.plugin.java.JavaPluginLoader;
 import org.bukkit.scheduler.BukkitScheduler;
 
+import java.io.File;
+
 import static fr.xephi.authme.service.BukkitService.TICKS_PER_MINUTE;
 import static fr.xephi.authme.util.Utils.isClassLoaded;
 
@@ -160,7 +157,7 @@ public class AuthMe extends JavaPlugin {
 
         // Sponsor messages
         ConsoleLogger.info("Development builds are available on our jenkins, thanks to FastVM.io");
-        ConsoleLogger.info("Do you want a good vps for your  game server? Look at our sponsor FastVM.io leader "
+        ConsoleLogger.info("Do you want a good vps for your game server? Look at our sponsor FastVM.io leader "
             + "as virtual server provider!");
 
         // Successful message

src/main/java/fr/xephi/authme/ConsoleLogger.java

@@ -5,7 +5,7 @@ import fr.xephi.authme.output.LogLevel;
 import fr.xephi.authme.settings.Settings;
 import fr.xephi.authme.settings.properties.PluginSettings;
 import fr.xephi.authme.settings.properties.SecuritySettings;
-import fr.xephi.authme.util.StringUtils;
+import fr.xephi.authme.util.ExceptionUtils;
 
 import java.io.File;
 import java.io.FileWriter;
@@ -101,7 +101,7 @@ public final class ConsoleLogger {
      * @param th      The Throwable to log
      */
     public static void logException(String message, Throwable th) {
-        warning(message + " " + StringUtils.formatException(th));
+        warning(message + " " + ExceptionUtils.formatException(th));
         writeLog(Throwables.getStackTraceAsString(th));
     }
 

src/main/java/fr/xephi/authme/command/executable/authme/UpdateHelpMessagesCommand.java

@@ -7,6 +7,7 @@ import fr.xephi.authme.service.HelpTranslationGenerator;
 import org.bukkit.command.CommandSender;
 
 import javax.inject.Inject;
+import java.io.File;
 import java.io.IOException;
 import java.util.List;
 
@@ -24,8 +25,8 @@ public class UpdateHelpMessagesCommand implements ExecutableCommand {
     @Override
     public void executeCommand(CommandSender sender, List<String> arguments) {
         try {
-            helpTranslationGenerator.updateHelpFile();
-            sender.sendMessage("Successfully updated the help file");
+            File updatedFile = helpTranslationGenerator.updateHelpFile();
+            sender.sendMessage("Successfully updated the help file '" + updatedFile.getName() + "'");
             helpMessagesService.reloadMessagesFile();
         } catch (IOException e) {
             sender.sendMessage("Could not update help file: " + e.getMessage());

src/main/java/fr/xephi/authme/data/limbo/AllowFlightRestoreType.java

@@ -32,7 +32,7 @@ public enum AllowFlightRestoreType {
         }
     },
 
-    /** Always set flight enabled to false. */
+    /** The user's flight handling is not modified. */
     NOTHING {
         @Override
         public void restoreAllowFlight(Player player, LimboPlayer limbo) {

src/main/java/fr/xephi/authme/process/register/AsyncRegister.java

@@ -14,7 +14,6 @@ import fr.xephi.authme.service.BukkitService;
 import fr.xephi.authme.service.CommonService;
 import fr.xephi.authme.service.bungeecord.BungeeSender;
 import fr.xephi.authme.service.bungeecord.MessageType;
-import fr.xephi.authme.settings.properties.PluginSettings;
 import fr.xephi.authme.settings.properties.RegistrationSettings;
 import fr.xephi.authme.settings.properties.RestrictionSettings;
 import fr.xephi.authme.util.PlayerUtils;
@@ -82,9 +81,8 @@ public class AsyncRegister implements AsynchronousProcess {
             return false;
         }
 
-        boolean isAsync = service.getProperty(PluginSettings.USE_ASYNC_TASKS);
-        AuthMeAsyncPreRegisterEvent event = new AuthMeAsyncPreRegisterEvent(player, isAsync);
-        bukkitService.callEvent(event);
+        AuthMeAsyncPreRegisterEvent event = bukkitService.createAndCallEvent(
+            isAsync -> new AuthMeAsyncPreRegisterEvent(player, isAsync));
         if (!event.canRegister()) {
             return false;
         }

src/main/java/fr/xephi/authme/security/HashUtils.java

@@ -1,6 +1,7 @@
 package fr.xephi.authme.security;
 
 import java.math.BigInteger;
+import java.nio.charset.StandardCharsets;
 import java.security.MessageDigest;
 import java.security.NoSuchAlgorithmException;
 
@@ -78,6 +79,20 @@ public final class HashUtils {
         return hash.length() > 3 && hash.substring(0, 2).equals("$2");
     }
 
+    /**
+     * Checks whether the two strings are equal to each other in a time-constant manner.
+     * This helps to avoid timing side channel attacks,
+     * cf. <a href="https://github.com/AuthMe/AuthMeReloaded/issues/1561">issue #1561</a>.
+     *
+     * @param string1 first string
+     * @param string2 second string
+     * @return true if the strings are equal to each other, false otherwise
+     */
+    public static boolean isEqual(String string1, String string2) {
+        return MessageDigest.isEqual(
+            string1.getBytes(StandardCharsets.UTF_8), string2.getBytes(StandardCharsets.UTF_8));
+    }
+
     /**
      * Hash the message with the given algorithm and return the hash in its hexadecimal notation.
      *

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

@@ -8,7 +8,7 @@ import fr.xephi.authme.security.crypts.description.SaltType;
 import fr.xephi.authme.security.crypts.description.Usage;
 import fr.xephi.authme.settings.Settings;
 import fr.xephi.authme.settings.properties.HooksSettings;
-import fr.xephi.authme.util.StringUtils;
+import fr.xephi.authme.util.ExceptionUtils;
 
 import javax.inject.Inject;
 
@@ -39,7 +39,7 @@ public class BCrypt implements EncryptionMethod {
         try {
             return HashUtils.isValidBcryptHash(hash.getHash()) && BCryptService.checkpw(password, hash.getHash());
         } catch (IllegalArgumentException e) {
-            ConsoleLogger.warning("Bcrypt checkpw() returned " + StringUtils.formatException(e));
+            ConsoleLogger.warning("Bcrypt checkpw() returned " + ExceptionUtils.formatException(e));
         }
         return false;
     }

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

@@ -3,6 +3,8 @@ package fr.xephi.authme.security.crypts;
 import fr.xephi.authme.security.crypts.description.Recommendation;
 import fr.xephi.authme.security.crypts.description.Usage;
 
+import static fr.xephi.authme.security.HashUtils.isEqual;
+
 @Recommendation(Usage.RECOMMENDED)
 public class BCrypt2y extends HexSaltedMethod {
 
@@ -23,7 +25,7 @@ public class BCrypt2y extends HexSaltedMethod {
         // The salt is the first 29 characters of the hash
 
         String salt = hash.substring(0, 29);
-        return hash.equals(computeHash(password, salt, null));
+        return isEqual(hash, computeHash(password, salt, null));
     }
 
     @Override

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

@@ -2,12 +2,12 @@ package fr.xephi.authme.security.crypts;
 
 import fr.xephi.authme.ConsoleLogger;
 import fr.xephi.authme.security.HashUtils;
-import fr.xephi.authme.util.RandomStringUtils;
 import fr.xephi.authme.security.crypts.description.HasSalt;
 import fr.xephi.authme.security.crypts.description.Recommendation;
 import fr.xephi.authme.security.crypts.description.SaltType;
 import fr.xephi.authme.security.crypts.description.Usage;
-import fr.xephi.authme.util.StringUtils;
+import fr.xephi.authme.util.ExceptionUtils;
+import fr.xephi.authme.util.RandomStringUtils;
 
 
 /**
@@ -37,7 +37,7 @@ public class Ipb4 implements EncryptionMethod {
         try {
             return HashUtils.isValidBcryptHash(hash.getHash()) && BCryptService.checkpw(password, hash.getHash());
         } catch (IllegalArgumentException e) {
-            ConsoleLogger.warning("Bcrypt checkpw() returned " + StringUtils.formatException(e));
+            ConsoleLogger.warning("Bcrypt checkpw() returned " + ExceptionUtils.formatException(e));
         }
         return false;
     }

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

@@ -4,6 +4,8 @@ import fr.xephi.authme.security.HashUtils;
 import fr.xephi.authme.security.crypts.description.Recommendation;
 import fr.xephi.authme.security.crypts.description.Usage;
 
+import static fr.xephi.authme.security.HashUtils.isEqual;
+
 @Recommendation(Usage.ACCEPTABLE)
 public class Joomla extends HexSaltedMethod {
 
@@ -16,7 +18,7 @@ public class Joomla extends HexSaltedMethod {
     public boolean comparePassword(String password, HashedPassword hashedPassword, String unusedName) {
         String hash = hashedPassword.getHash();
         String[] hashParts = hash.split(":");
-        return hashParts.length == 2 && hash.equals(computeHash(password, hashParts[1], null));
+        return hashParts.length == 2 && isEqual(hash, computeHash(password, hashParts[1], null));
     }
 
     @Override

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

@@ -1,5 +1,6 @@
 package fr.xephi.authme.security.crypts;
 
+import static fr.xephi.authme.security.HashUtils.isEqual;
 import static fr.xephi.authme.security.HashUtils.md5;
 
 public class Md5vB extends HexSaltedMethod {
@@ -13,7 +14,7 @@ public class Md5vB extends HexSaltedMethod {
     public boolean comparePassword(String password, HashedPassword hashedPassword, String name) {
         String hash = hashedPassword.getHash();
         String[] line = hash.split("\\$");
-        return line.length == 4 && hash.equals(computeHash(password, line[2], name));
+        return line.length == 4 && isEqual(hash, computeHash(password, line[2], name));
     }
 
     @Override

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

@@ -10,6 +10,8 @@ import fr.xephi.authme.security.crypts.description.Usage;
 import java.io.UnsupportedEncodingException;
 import java.security.MessageDigest;
 
+import static fr.xephi.authme.security.HashUtils.isEqual;
+
 /**
  * Encryption method compatible with phpBB3.
  * <p>
@@ -43,7 +45,7 @@ public class PhpBB implements EncryptionMethod {
         } else if (hash.length() == 34) {
             return PhpassSaltedMd5.phpbb_check_hash(password, hash);
         } else {
-            return PhpassSaltedMd5.md5(password).equals(hash);
+            return isEqual(hash, PhpassSaltedMd5.md5(password));
         }
     }
 
@@ -153,7 +155,7 @@ public class PhpBB implements EncryptionMethod {
         }
 
         private static boolean phpbb_check_hash(String password, String hash) {
-            return _hash_crypt_private(password, hash).equals(hash);
+            return isEqual(hash, _hash_crypt_private(password, hash)); // #1561: fix timing issue
         }
     }
 }

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

@@ -1,5 +1,7 @@
 package fr.xephi.authme.security.crypts;
 
+import static fr.xephi.authme.security.HashUtils.isEqual;
+
 /**
  * Common supertype for encryption methods which store their salt separately from the hash.
  */
@@ -19,7 +21,7 @@ public abstract class SeparateSaltMethod implements EncryptionMethod {
 
     @Override
     public boolean comparePassword(String password, HashedPassword hashedPassword, String name) {
-        return hashedPassword.getHash().equals(computeHash(password, hashedPassword.getSalt(), null));
+        return isEqual(hashedPassword.getHash(), computeHash(password, hashedPassword.getSalt(), null));
     }
 
     @Override

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

@@ -3,6 +3,7 @@ package fr.xephi.authme.security.crypts;
 import fr.xephi.authme.security.crypts.description.Recommendation;
 import fr.xephi.authme.security.crypts.description.Usage;
 
+import static fr.xephi.authme.security.HashUtils.isEqual;
 import static fr.xephi.authme.security.HashUtils.sha256;
 
 @Recommendation(Usage.RECOMMENDED)
@@ -14,10 +15,10 @@ public class Sha256 extends HexSaltedMethod {
     }
 
     @Override
-    public boolean comparePassword(String password, HashedPassword hashedPassword, String playerName) {
+    public boolean comparePassword(String password, HashedPassword hashedPassword, String name) {
         String hash = hashedPassword.getHash();
         String[] line = hash.split("\\$");
-        return line.length == 4 && hash.equals(computeHash(password, line[2], ""));
+        return line.length == 4 && isEqual(hash, computeHash(password, line[2], name));
     }
 
     @Override

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

@@ -7,6 +7,8 @@ import fr.xephi.authme.security.crypts.description.SaltType;
 import fr.xephi.authme.security.crypts.description.Usage;
 import fr.xephi.authme.util.RandomStringUtils;
 
+import static fr.xephi.authme.security.HashUtils.isEqual;
+
 /**
  * Hashing algorithm for SMF forums.
  * <p>
@@ -32,7 +34,7 @@ public class Smf implements EncryptionMethod {
 
     @Override
     public boolean comparePassword(String password, HashedPassword hashedPassword, String name) {
-        return computeHash(password, null, name).equals(hashedPassword.getHash());
+        return isEqual(hashedPassword.getHash(), computeHash(password, null, name));
     }
 
     @Override

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

@@ -5,6 +5,8 @@ import fr.xephi.authme.security.crypts.description.Recommendation;
 import fr.xephi.authme.security.crypts.description.SaltType;
 import fr.xephi.authme.security.crypts.description.Usage;
 
+import static fr.xephi.authme.security.HashUtils.isEqual;
+
 /**
  * Common type for encryption methods which do not use any salt whatsoever.
  */
@@ -26,7 +28,7 @@ public abstract class UnsaltedMethod implements EncryptionMethod {
 
     @Override
     public boolean comparePassword(String password, HashedPassword hashedPassword, String name) {
-        return hashedPassword.getHash().equals(computeHash(password));
+        return isEqual(hashedPassword.getHash(), computeHash(password));
     }
 
     @Override

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

@@ -5,6 +5,8 @@ import fr.xephi.authme.security.crypts.description.Recommendation;
 import fr.xephi.authme.security.crypts.description.SaltType;
 import fr.xephi.authme.security.crypts.description.Usage;
 
+import static fr.xephi.authme.security.HashUtils.isEqual;
+
 /**
  * Common supertype of encryption methods that use a player's username
  * (or something based on it) as embedded salt.
@@ -23,7 +25,7 @@ public abstract class UsernameSaltMethod implements EncryptionMethod {
 
     @Override
     public boolean comparePassword(String password, HashedPassword hashedPassword, String name) {
-        return hashedPassword.getHash().equals(computeHash(password, name).getHash());
+        return isEqual(hashedPassword.getHash(), computeHash(password, name).getHash());
     }
 
     @Override

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

@@ -3,6 +3,7 @@ package fr.xephi.authme.security.crypts;
 import fr.xephi.authme.security.crypts.description.Recommendation;
 import fr.xephi.authme.security.crypts.description.Usage;
 
+import static fr.xephi.authme.security.HashUtils.isEqual;
 import static fr.xephi.authme.security.crypts.BCryptService.hashpw;
 
 @Recommendation(Usage.RECOMMENDED)
@@ -14,12 +15,12 @@ public class Wbb4 extends HexSaltedMethod {
     }
 
     @Override
-    public boolean comparePassword(String password, HashedPassword hashedPassword, String playerName) {
+    public boolean comparePassword(String password, HashedPassword hashedPassword, String name) {
         if (hashedPassword.getHash().length() != 60) {
             return false;
         }
         String salt = hashedPassword.getHash().substring(0, 29);
-        return computeHash(password, salt, null).equals(hashedPassword.getHash());
+        return isEqual(hashedPassword.getHash(), computeHash(password, salt, name));
     }
 
     @Override

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

@@ -12,6 +12,8 @@ import java.security.MessageDigest;
 import java.security.SecureRandom;
 import java.util.Arrays;
 
+import static fr.xephi.authme.security.HashUtils.isEqual;
+
 @Recommendation(Usage.ACCEPTABLE)
 @HasSalt(value = SaltType.TEXT, length = 9)
 // Note ljacqu 20151228: Wordpress is actually a salted algorithm but salt generation is handled internally
@@ -115,7 +117,7 @@ public class Wordpress extends UnsaltedMethod {
     public boolean comparePassword(String password, HashedPassword hashedPassword, String name) {
         String hash = hashedPassword.getHash();
         String comparedHash = crypt(password, hash);
-        return comparedHash.equals(hash);
+        return isEqual(hash, comparedHash);
     }
 
 }

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

@@ -3,6 +3,8 @@ package fr.xephi.authme.security.crypts;
 import fr.xephi.authme.security.crypts.description.Recommendation;
 import fr.xephi.authme.security.crypts.description.Usage;
 
+import static fr.xephi.authme.security.HashUtils.isEqual;
+
 @Recommendation(Usage.RECOMMENDED)
 public class XAuth extends HexSaltedMethod {
 
@@ -23,14 +25,14 @@ public class XAuth extends HexSaltedMethod {
     }
 
     @Override
-    public boolean comparePassword(String password, HashedPassword hashedPassword, String playerName) {
+    public boolean comparePassword(String password, HashedPassword hashedPassword, String name) {
         String hash = hashedPassword.getHash();
         int saltPos = password.length() >= hash.length() ? hash.length() - 1 : password.length();
         if (saltPos + 12 > hash.length()) {
             return false;
         }
         String salt = hash.substring(saltPos, saltPos + 12);
-        return hash.equals(computeHash(password, salt, null));
+        return isEqual(hash, computeHash(password, salt, name));
     }
 
     @Override

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

@@ -2,7 +2,7 @@ package fr.xephi.authme.security.crypts;
 
 import fr.xephi.authme.ConsoleLogger;
 import fr.xephi.authme.security.HashUtils;
-import fr.xephi.authme.util.StringUtils;
+import fr.xephi.authme.util.ExceptionUtils;
 
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
@@ -32,7 +32,7 @@ public class XfBCrypt implements EncryptionMethod {
         try {
             return HashUtils.isValidBcryptHash(hash.getHash()) && BCryptService.checkpw(password, hash.getHash());
         } catch (IllegalArgumentException e) {
-            ConsoleLogger.warning("XfBCrypt checkpw() returned " + StringUtils.formatException(e));
+            ConsoleLogger.warning("XfBCrypt checkpw() returned " + ExceptionUtils.formatException(e));
         }
         return false;
     }

src/main/java/fr/xephi/authme/service/HelpTranslationGenerator.java

@@ -44,15 +44,17 @@ public class HelpTranslationGenerator {
     /**
      * Updates the help file to contain entries for all commands.
      *
+     * @return the help file that has been updated
      * @throws IOException if the help file cannot be written to
      */
-    public void updateHelpFile() throws IOException {
+    public File updateHelpFile() throws IOException {
         String languageCode = settings.getProperty(PluginSettings.MESSAGES_LANGUAGE);
         File helpFile = new File(dataFolder, "messages/help_" + languageCode + ".yml");
         Map<String, Object> helpEntries = generateHelpMessageEntries();
 
         String helpEntriesYaml = exportToYaml(helpEntries);
         Files.write(helpFile.toPath(), helpEntriesYaml.getBytes(), StandardOpenOption.TRUNCATE_EXISTING);
+        return helpFile;
     }
 
     private static String exportToYaml(Map<String, Object> helpEntries) {

src/main/java/fr/xephi/authme/util/ExceptionUtils.java

@@ -33,4 +33,14 @@ public final class ExceptionUtils {
         }
         return null;
     }
+
+    /**
+     * Format the information from a Throwable as string, retaining the type and its message.
+     *
+     * @param th the throwable to process
+     * @return string with the type of the Throwable and its message, e.g. "[IOException]: Could not open stream"
+     */
+    public static String formatException(Throwable th) {
+        return "[" + th.getClass().getSimpleName() + "]: " + th.getMessage();
+    }
 }

src/main/java/fr/xephi/authme/util/StringUtils.java

@@ -66,17 +66,6 @@ public final class StringUtils {
         return str == null || str.trim().isEmpty();
     }
 
-    /**
-     * Format the information from a Throwable as string, retaining the type and its message.
-     *
-     * @param th The throwable to process
-     *
-     * @return String with the type of the Throwable and its message, e.g. "[IOException]: Could not open stream"
-     */
-    public static String formatException(Throwable th) {
-        return "[" + th.getClass().getSimpleName() + "]: " + th.getMessage();
-    }
-
     /**
      * Check that the given needle is in the middle of the haystack, i.e. that the haystack
      * contains the needle and that it is not at the very start or end.