From c54231b2554c1df6da7290f41f6b3ae3d1303ac1 Mon Sep 17 00:00:00 2001 From: ljacqu Date: Sat, 25 Mar 2017 00:23:54 +0100 Subject: [PATCH] #1138 Show warning for hashes that will be deprecated in 5.4 - Introduce Usage.DEPRECATED to mark the hash algorithms accordingly - Log warning when such a deprecated hash algorithm is used - Update hash algorithms doc page --- docs/hash_algorithms.md | 14 ++-- src/main/java/fr/xephi/authme/AuthMe.java | 22 ++++-- .../authme/initialization/OnStartupTasks.java | 22 ++++++ .../authme/security/crypts/DoubleMd5.java | 4 + .../fr/xephi/authme/security/crypts/Md5.java | 3 + .../authme/security/crypts/PlainText.java | 4 + .../fr/xephi/authme/security/crypts/Sha1.java | 3 + .../xephi/authme/security/crypts/Sha512.java | 3 + .../authme/security/crypts/Whirlpool.java | 4 + .../security/crypts/description/Usage.java | 3 + .../initialization/OnStartupTasksTest.java | 76 +++++++++++++++++++ 11 files changed, 146 insertions(+), 12 deletions(-) create mode 100644 src/test/java/fr/xephi/authme/initialization/OnStartupTasksTest.java diff --git a/docs/hash_algorithms.md b/docs/hash_algorithms.md index 02bcafe6f..8e066dc7f 100644 --- a/docs/hash_algorithms.md +++ b/docs/hash_algorithms.md @@ -1,5 +1,5 @@ - + ## Hash Algorithms AuthMe supports the following hash algorithms for storing your passwords safely. @@ -10,11 +10,11 @@ Algorithm | Recommendation | Hash length | ASCII | | Salt type | Length | Se BCRYPT | Recommended | 60 | | | Text | | BCRYPT2Y | Recommended | 60 | | | Text | 22 | CRAZYCRYPT1 | Do not use | 128 | | | Username | | -DOUBLEMD5 | Do not use | 32 | | | None | | +DOUBLEMD5 | Deprecated | 32 | | | None | | IPB3 | Acceptable | 32 | | | Text | 5 | Y IPB4 | Does not work | 60 | | | Text | 22 | Y JOOMLA | Acceptable | 65 | | | Text | 32 | -MD5 | Do not use | 32 | | | None | | +MD5 | Deprecated | 32 | | | None | | MD5VB | Acceptable | 56 | | | Text | 16 | MYBB | Acceptable | 32 | | | Text | 8 | Y PBKDF2 | Recommended | 165 | | | Text | 16 | @@ -24,14 +24,14 @@ PHPFUSION | Do not use | 64 | Y | | | | Y ROYALAUTH | Do not use | 128 | | | None | | SALTED2MD5 | Acceptable | 32 | | | Text | | Y SALTEDSHA512 | Recommended | 128 | | | | | Y -SHA1 | Do not use | 40 | | | None | | +SHA1 | Deprecated | 40 | | | None | | SHA256 | Recommended | 86 | | | Text | 16 | -SHA512 | Do not use | 128 | | | None | | +SHA512 | Deprecated | 128 | | | None | | SMF | Do not use | 40 | | | Username | | TWO_FACTOR | Does not work | 16 | | | None | | WBB3 | Acceptable | 40 | | | Text | 40 | Y WBB4 | Recommended | 60 | | | Text | 8 | -WHIRLPOOL | Do not use | 128 | | | None | | +WHIRLPOOL | Deprecated | 128 | | | None | | WORDPRESS | Acceptable | 34 | | | Text | 9 | XAUTH | Recommended | 140 | | | Text | 12 | XFBCRYPT | | 60 | | | | | @@ -82,4 +82,4 @@ or bad. --- -This page was automatically generated on the [AuthMe/AuthMeReloaded repository](https://github.com/AuthMe/AuthMeReloaded/tree/master/docs/) on Fri Nov 25 15:48:35 CET 2016 +This page was automatically generated on the [AuthMe/AuthMeReloaded repository](https://github.com/AuthMe/AuthMeReloaded/tree/master/docs/) on Sat Mar 25 00:15:27 CET 2017 diff --git a/src/main/java/fr/xephi/authme/AuthMe.java b/src/main/java/fr/xephi/authme/AuthMe.java index b66258cb2..00beaf51d 100644 --- a/src/main/java/fr/xephi/authme/AuthMe.java +++ b/src/main/java/fr/xephi/authme/AuthMe.java @@ -26,6 +26,7 @@ import fr.xephi.authme.listener.PlayerListener19; import fr.xephi.authme.listener.ServerListener; import fr.xephi.authme.permission.PermissionsManager; import fr.xephi.authme.permission.PermissionsSystemType; +import fr.xephi.authme.security.HashAlgorithm; import fr.xephi.authme.security.crypts.Sha256; import fr.xephi.authme.service.BackupService; import fr.xephi.authme.service.BukkitService; @@ -148,7 +149,8 @@ public class AuthMe extends JavaPlugin { // If server is using PermissionsBukkit, print a warning that some features may not be supported if (PermissionsSystemType.PERMISSIONS_BUKKIT.equals(permsMan.getPermissionSystem())) { - ConsoleLogger.warning("Warning! This server uses PermissionsBukkit for permissions. Some permissions features may not be supported!"); + ConsoleLogger.warning("Warning! This server uses PermissionsBukkit for permissions. Some permissions " + + "features may not be supported!"); } // Do a backup on start @@ -159,10 +161,12 @@ public class AuthMe extends JavaPlugin { // Sponsor messages ConsoleLogger.info("Development builds are available on our jenkins, thanks to f14stelt."); - ConsoleLogger.info("Do you want a good game server? Look at our sponsor GameHosting.it leader in Italy as Game Server Provider!"); + ConsoleLogger.info("Do you want a good game server? Look at our sponsor GameHosting.it leader " + + "in Italy as Game Server Provider!"); // Successful message - ConsoleLogger.info("AuthMe " + getPluginVersion() + " build n." + getPluginBuildNumber() + " correctly enabled!"); + ConsoleLogger.info("AuthMe " + getPluginVersion() + " build n." + getPluginBuildNumber() + + " correctly enabled!"); // Purge on start if enabled PurgeService purgeService = injector.getSingleton(PurgeService.class); @@ -248,7 +252,7 @@ public class AuthMe extends JavaPlugin { * * @param injector the injector */ - protected void instantiateServices(Injector injector) { + void instantiateServices(Injector injector) { // PlayerCache is still injected statically sometimes PlayerCache playerCache = PlayerCache.getInstance(); injector.register(PlayerCache.class, playerCache); @@ -283,6 +287,14 @@ public class AuthMe extends JavaPlugin { && settings.getProperty(EmailSettings.SMTP_PORT) != 25) { ConsoleLogger.warning("Note: You have set Email.useTls to false but this only affects mail over port 25"); } + + // Unsalted hashes will be deprecated in 5.4 (see Github issue #1016). Exclude RoyalAuth from this check because + // it is needed to hook into an existing system. + HashAlgorithm hash = settings.getProperty(SecuritySettings.PASSWORD_HASH); + if (OnStartupTasks.isHashDeprecatedIn54(hash)) { + ConsoleLogger.warning("You are using an unsalted hash (" + hash + "). Support for this will be removed " + + "in 5.4 -- do you still need it? Comment on https://github.com/Xephi/AuthMeReloaded/issues/1016"); + } } /** @@ -290,7 +302,7 @@ public class AuthMe extends JavaPlugin { * * @param injector the injector */ - protected void registerEventListeners(Injector injector) { + void registerEventListeners(Injector injector) { // Get the plugin manager instance PluginManager pluginManager = getServer().getPluginManager(); diff --git a/src/main/java/fr/xephi/authme/initialization/OnStartupTasks.java b/src/main/java/fr/xephi/authme/initialization/OnStartupTasks.java index 8767e88a6..bce15af47 100644 --- a/src/main/java/fr/xephi/authme/initialization/OnStartupTasks.java +++ b/src/main/java/fr/xephi/authme/initialization/OnStartupTasks.java @@ -7,6 +7,9 @@ import fr.xephi.authme.data.auth.PlayerAuth; import fr.xephi.authme.datasource.DataSource; import fr.xephi.authme.message.MessageKey; import fr.xephi.authme.message.Messages; +import fr.xephi.authme.security.HashAlgorithm; +import fr.xephi.authme.security.crypts.description.Recommendation; +import fr.xephi.authme.security.crypts.description.Usage; import org.bstats.Metrics; import fr.xephi.authme.output.ConsoleFilter; import fr.xephi.authme.output.Log4JFilter; @@ -138,4 +141,23 @@ public class OnStartupTasks { } } } + + /** + * Returns whether the hash algorithm is deprecated and won't be able + * to be actively used anymore in 5.4. + * + * @param hash the hash algorithm to check + * @return true if the hash will be deprecated, false otherwise + * @see #1016 + */ + public static boolean isHashDeprecatedIn54(HashAlgorithm hash) { + if (hash.getClazz() == null || hash == HashAlgorithm.PLAINTEXT) { + // Exclude PLAINTEXT from this check because it already has a mandatory migration, which takes care of + // sending all the necessary messages and warnings. + return false; + } + + Recommendation recommendation = hash.getClazz().getAnnotation(Recommendation.class); + return recommendation != null && recommendation.value() == Usage.DEPRECATED; + } } diff --git a/src/main/java/fr/xephi/authme/security/crypts/DoubleMd5.java b/src/main/java/fr/xephi/authme/security/crypts/DoubleMd5.java index c28e0440a..54158419b 100644 --- a/src/main/java/fr/xephi/authme/security/crypts/DoubleMd5.java +++ b/src/main/java/fr/xephi/authme/security/crypts/DoubleMd5.java @@ -1,7 +1,11 @@ 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.md5; +@Recommendation(Usage.DEPRECATED) public class DoubleMd5 extends UnsaltedMethod { @Override diff --git a/src/main/java/fr/xephi/authme/security/crypts/Md5.java b/src/main/java/fr/xephi/authme/security/crypts/Md5.java index c2a2ba04a..a5f6d91d2 100644 --- a/src/main/java/fr/xephi/authme/security/crypts/Md5.java +++ b/src/main/java/fr/xephi/authme/security/crypts/Md5.java @@ -1,7 +1,10 @@ package fr.xephi.authme.security.crypts; import fr.xephi.authme.security.HashUtils; +import fr.xephi.authme.security.crypts.description.Recommendation; +import fr.xephi.authme.security.crypts.description.Usage; +@Recommendation(Usage.DEPRECATED) public class Md5 extends UnsaltedMethod { @Override diff --git a/src/main/java/fr/xephi/authme/security/crypts/PlainText.java b/src/main/java/fr/xephi/authme/security/crypts/PlainText.java index add333d69..43d6e8200 100644 --- a/src/main/java/fr/xephi/authme/security/crypts/PlainText.java +++ b/src/main/java/fr/xephi/authme/security/crypts/PlainText.java @@ -1,11 +1,15 @@ package fr.xephi.authme.security.crypts; +import fr.xephi.authme.security.crypts.description.Recommendation; +import fr.xephi.authme.security.crypts.description.Usage; + /** * Plaintext password storage. * * @deprecated Using this is no longer supported. AuthMe will migrate to SHA256 on startup. */ @Deprecated +@Recommendation(Usage.DEPRECATED) public class PlainText extends UnsaltedMethod { @Override diff --git a/src/main/java/fr/xephi/authme/security/crypts/Sha1.java b/src/main/java/fr/xephi/authme/security/crypts/Sha1.java index e3d078e7e..3430752b5 100644 --- a/src/main/java/fr/xephi/authme/security/crypts/Sha1.java +++ b/src/main/java/fr/xephi/authme/security/crypts/Sha1.java @@ -1,7 +1,10 @@ package fr.xephi.authme.security.crypts; import fr.xephi.authme.security.HashUtils; +import fr.xephi.authme.security.crypts.description.Recommendation; +import fr.xephi.authme.security.crypts.description.Usage; +@Recommendation(Usage.DEPRECATED) public class Sha1 extends UnsaltedMethod { @Override diff --git a/src/main/java/fr/xephi/authme/security/crypts/Sha512.java b/src/main/java/fr/xephi/authme/security/crypts/Sha512.java index 12e51a315..4e9682ffe 100644 --- a/src/main/java/fr/xephi/authme/security/crypts/Sha512.java +++ b/src/main/java/fr/xephi/authme/security/crypts/Sha512.java @@ -1,7 +1,10 @@ package fr.xephi.authme.security.crypts; import fr.xephi.authme.security.HashUtils; +import fr.xephi.authme.security.crypts.description.Recommendation; +import fr.xephi.authme.security.crypts.description.Usage; +@Recommendation(Usage.DEPRECATED) public class Sha512 extends UnsaltedMethod { @Override diff --git a/src/main/java/fr/xephi/authme/security/crypts/Whirlpool.java b/src/main/java/fr/xephi/authme/security/crypts/Whirlpool.java index 84efae8e8..c1ea747a9 100644 --- a/src/main/java/fr/xephi/authme/security/crypts/Whirlpool.java +++ b/src/main/java/fr/xephi/authme/security/crypts/Whirlpool.java @@ -59,8 +59,12 @@ package fr.xephi.authme.security.crypts; * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. */ +import fr.xephi.authme.security.crypts.description.Recommendation; +import fr.xephi.authme.security.crypts.description.Usage; + import java.util.Arrays; +@Recommendation(Usage.DEPRECATED) public class Whirlpool extends UnsaltedMethod { /** diff --git a/src/main/java/fr/xephi/authme/security/crypts/description/Usage.java b/src/main/java/fr/xephi/authme/security/crypts/description/Usage.java index c11c335ac..9ccc52c4f 100644 --- a/src/main/java/fr/xephi/authme/security/crypts/description/Usage.java +++ b/src/main/java/fr/xephi/authme/security/crypts/description/Usage.java @@ -20,6 +20,9 @@ public enum Usage { /** Hash algorithm is not recommended to be used. Use only if required by another system. */ DO_NOT_USE, + /** Algorithm that is or will be no longer supported actively. */ + DEPRECATED, + /** The algorithm does not work properly; do not use. */ DOES_NOT_WORK diff --git a/src/test/java/fr/xephi/authme/initialization/OnStartupTasksTest.java b/src/test/java/fr/xephi/authme/initialization/OnStartupTasksTest.java new file mode 100644 index 000000000..4a95a1029 --- /dev/null +++ b/src/test/java/fr/xephi/authme/initialization/OnStartupTasksTest.java @@ -0,0 +1,76 @@ +package fr.xephi.authme.initialization; + +import ch.jalu.injector.exceptions.InjectorReflectionException; +import fr.xephi.authme.TestHelper; +import fr.xephi.authme.security.HashAlgorithm; +import org.junit.Test; + +import java.util.logging.Logger; + +import static org.hamcrest.Matchers.equalTo; +import static org.junit.Assert.assertThat; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyZeroInteractions; + +/** + * Test for {@link OnStartupTasks}. + */ +public class OnStartupTasksTest { + + @Test + public void shouldDisplayLegacyJarHint() { + // given + Logger logger = TestHelper.setupLogger(); + NoClassDefFoundError noClassDefError = new NoClassDefFoundError("Lcom/google/gson/Gson;"); + ReflectiveOperationException ex2 = new ReflectiveOperationException("", noClassDefError); + InjectorReflectionException ex = new InjectorReflectionException("", ex2); + + // when + OnStartupTasks.displayLegacyJarHint(ex); + + // then + verify(logger).warning("YOU MUST DOWNLOAD THE LEGACY JAR TO USE AUTHME ON YOUR SERVER"); + } + + @Test + public void shouldNotDisplayLegacyHintForDifferentException() { + // given + Logger logger = TestHelper.setupLogger(); + NullPointerException npe = new NullPointerException(); + + // when + OnStartupTasks.displayLegacyJarHint(npe); + + // then + verifyZeroInteractions(logger); + } + + @Test + public void shouldNotDisplayLegacyHintForWrongCause() { + // given + Logger logger = TestHelper.setupLogger(); + IllegalAccessException illegalAccessException = new IllegalAccessException("Lcom/google/gson/Gson;"); + ReflectiveOperationException ex2 = new ReflectiveOperationException("", illegalAccessException); + InjectorReflectionException ex = new InjectorReflectionException("", ex2); + + // when + OnStartupTasks.displayLegacyJarHint(ex); + + // then + verifyZeroInteractions(logger); + } + + @Test + public void shouldCheckIfHashIsDeprecatedIn54() { + // given / when / then + assertThat(OnStartupTasks.isHashDeprecatedIn54(HashAlgorithm.CUSTOM), equalTo(false)); + assertThat(OnStartupTasks.isHashDeprecatedIn54(HashAlgorithm.IPB3), equalTo(false)); + assertThat(OnStartupTasks.isHashDeprecatedIn54(HashAlgorithm.PLAINTEXT), equalTo(false)); + assertThat(OnStartupTasks.isHashDeprecatedIn54(HashAlgorithm.SHA256), equalTo(false)); + assertThat(OnStartupTasks.isHashDeprecatedIn54(HashAlgorithm.WORDPRESS), equalTo(false)); + + assertThat(OnStartupTasks.isHashDeprecatedIn54(HashAlgorithm.MD5), equalTo(true)); + assertThat(OnStartupTasks.isHashDeprecatedIn54(HashAlgorithm.SHA512), equalTo(true)); + assertThat(OnStartupTasks.isHashDeprecatedIn54(HashAlgorithm.WHIRLPOOL), equalTo(true)); + } +}