From 513ff9a9289463d8f08f2ddf7387bcdabedee182 Mon Sep 17 00:00:00 2001 From: ljacqu Date: Sun, 27 Dec 2015 22:16:16 +0100 Subject: [PATCH] #358 Make RandomString static & generate all rand. strings with it - Remove dubious random String generator on HashUtils - Make further hash classes use HashUtils --- .../executable/captcha/CaptchaCommand.java | 2 +- .../executable/email/RecoverEmailCommand.java | 3 +- .../process/login/AsynchronousLogin.java | 13 +++--- .../fr/xephi/authme/security/HashUtils.java | 37 +++++++--------- .../xephi/authme/security/RandomString.java | 30 ++++++------- .../security/crypts/CryptPBKDF2Django.java | 6 +-- .../xephi/authme/security/crypts/JOOMLA.java | 5 ++- .../xephi/authme/security/crypts/MD5VB.java | 6 +-- .../xephi/authme/security/crypts/SHA256.java | 43 ++++++++++--------- .../xephi/authme/security/crypts/SHA512.java | 39 ++++++++--------- .../fr/xephi/authme/security/crypts/SMF.java | 37 ++++++++-------- .../authme/security/crypts/WORDPRESS.java | 2 +- .../xephi/authme/security/crypts/XAUTH.java | 4 +- .../security/crypts/description/HasSalt.java | 9 ++++ .../crypts/description/Recommendation.java | 8 ++++ .../security/crypts/description/SaltType.java | 2 +- .../security/crypts/description/Usage.java | 2 +- 17 files changed, 125 insertions(+), 123 deletions(-) diff --git a/src/main/java/fr/xephi/authme/command/executable/captcha/CaptchaCommand.java b/src/main/java/fr/xephi/authme/command/executable/captcha/CaptchaCommand.java index b9e23681..d21bac77 100644 --- a/src/main/java/fr/xephi/authme/command/executable/captcha/CaptchaCommand.java +++ b/src/main/java/fr/xephi/authme/command/executable/captcha/CaptchaCommand.java @@ -41,7 +41,7 @@ public class CaptchaCommand extends PlayerCommand { if (Settings.useCaptcha && !captcha.equals(plugin.cap.get(playerNameLowerCase))) { plugin.cap.remove(playerNameLowerCase); - String randStr = new RandomString(Settings.captchaLength).nextString(); + String randStr = RandomString.generate(Settings.captchaLength); plugin.cap.put(playerNameLowerCase, randStr); commandService.send(player, MessageKey.CAPTCHA_WRONG_ERROR, plugin.cap.get(playerNameLowerCase)); return; diff --git a/src/main/java/fr/xephi/authme/command/executable/email/RecoverEmailCommand.java b/src/main/java/fr/xephi/authme/command/executable/email/RecoverEmailCommand.java index 63a8825d..07f05bfc 100644 --- a/src/main/java/fr/xephi/authme/command/executable/email/RecoverEmailCommand.java +++ b/src/main/java/fr/xephi/authme/command/executable/email/RecoverEmailCommand.java @@ -38,8 +38,7 @@ public class RecoverEmailCommand extends PlayerCommand { return; } try { - RandomString rand = new RandomString(Settings.getRecoveryPassLength); - String thePass = rand.nextString(); + String thePass = RandomString.generate(Settings.getRecoveryPassLength); String hashNew = PasswordSecurity.getHash(Settings.getPasswordHash, thePass, playerName); PlayerAuth auth; if (PlayerCache.getInstance().isAuthenticated(playerName)) { diff --git a/src/main/java/fr/xephi/authme/process/login/AsynchronousLogin.java b/src/main/java/fr/xephi/authme/process/login/AsynchronousLogin.java index fe47be51..e0983d22 100644 --- a/src/main/java/fr/xephi/authme/process/login/AsynchronousLogin.java +++ b/src/main/java/fr/xephi/authme/process/login/AsynchronousLogin.java @@ -26,12 +26,11 @@ import java.util.List; */ public class AsynchronousLogin { - private static final RandomString rdm = new RandomString(Settings.captchaLength); - protected final Player player; - protected final String name; - protected final String realName; - protected final String password; - protected final boolean forceLogin; + private final Player player; + private final String name; + private final String realName; + private final String password; + private final boolean forceLogin; private final AuthMe plugin; private final DataSource database; private final Messages m; @@ -70,7 +69,7 @@ public class AsynchronousLogin { plugin.captcha.putIfAbsent(name, i); } if (plugin.captcha.containsKey(name) && plugin.captcha.get(name) > Settings.maxLoginTry) { - plugin.cap.putIfAbsent(name, rdm.nextString()); + plugin.cap.putIfAbsent(name, RandomString.generate(Settings.captchaLength)); m.send(player, MessageKey.USAGE_CAPTCHA, plugin.cap.get(name)); return true; } diff --git a/src/main/java/fr/xephi/authme/security/HashUtils.java b/src/main/java/fr/xephi/authme/security/HashUtils.java index 0f4f7318..c5ae4f89 100644 --- a/src/main/java/fr/xephi/authme/security/HashUtils.java +++ b/src/main/java/fr/xephi/authme/security/HashUtils.java @@ -3,40 +3,27 @@ package fr.xephi.authme.security; import java.math.BigInteger; import java.security.MessageDigest; import java.security.NoSuchAlgorithmException; -import java.security.SecureRandom; public final class HashUtils { - private static final SecureRandom RANDOM = new SecureRandom(); - private HashUtils() { } - public static String hash(String message, MessageDigestAlgorithm algorithm) { - MessageDigest md = getDigest(algorithm); - md.reset(); - md.update(message.getBytes()); - byte[] digest = md.digest(); - return String.format("%0" + (digest.length << 1) + "x", new BigInteger(1, digest)); - } - public static String sha1(String message) { return hash(message, MessageDigestAlgorithm.SHA1); } - public static String md5(String message) { - return hash(message, MessageDigestAlgorithm.MD5); + public static String sha256(String message) { + return hash(message, MessageDigestAlgorithm.SHA256); } - // Only works for length up to 40! - public static String generateSalt(int length) { - byte[] msg = new byte[40]; - RANDOM.nextBytes(msg); - MessageDigest sha1 = getDigest(MessageDigestAlgorithm.SHA1); - sha1.reset(); - byte[] digest = sha1.digest(msg); - return String.format("%0" + (digest.length << 1) + "x", new BigInteger(1, digest)).substring(0, length); + public static String sha512(String message) { + return hash(message, MessageDigestAlgorithm.SHA512); + } + + public static String md5(String message) { + return hash(message, MessageDigestAlgorithm.MD5); } public static MessageDigest getDigest(MessageDigestAlgorithm algorithm) { @@ -48,6 +35,12 @@ public final class HashUtils { } } - + private static String hash(String message, MessageDigestAlgorithm algorithm) { + MessageDigest md = getDigest(algorithm); + md.reset(); + md.update(message.getBytes()); + byte[] digest = md.digest(); + return String.format("%0" + (digest.length << 1) + "x", new BigInteger(1, digest)); + } } diff --git a/src/main/java/fr/xephi/authme/security/RandomString.java b/src/main/java/fr/xephi/authme/security/RandomString.java index 8b1b8d63..c34a6465 100644 --- a/src/main/java/fr/xephi/authme/security/RandomString.java +++ b/src/main/java/fr/xephi/authme/security/RandomString.java @@ -1,13 +1,13 @@ package fr.xephi.authme.security; import java.security.SecureRandom; -import java.util.Calendar; import java.util.Random; -public class RandomString { +public final class RandomString { private static final char[] chars = new char[36]; private static final Random RANDOM = new SecureRandom(); + private static final int HEX_MAX_INDEX = 15; static { for (int idx = 0; idx < 10; ++idx) { @@ -18,30 +18,24 @@ public class RandomString { } } - private final Random random = new Random(); - - private final char[] buf; - - public RandomString(int length) { - if (length < 1) - throw new IllegalArgumentException("length < 1: " + length); - buf = new char[length]; - random.setSeed(Calendar.getInstance().getTimeInMillis()); - } - - public String nextString() { - for (int idx = 0; idx < buf.length; ++idx) - buf[idx] = chars[random.nextInt(chars.length)]; - return new String(buf); + private RandomString() { } public static String generate(int length) { + return generate(length, chars.length); + } + + public static String generateHex(int length) { + return generate(length, HEX_MAX_INDEX); + } + + private static String generate(int length, int maxIndex) { if (length < 0) { throw new IllegalArgumentException("Length must be positive but was " + length); } StringBuilder sb = new StringBuilder(length); for (int i = 0; i < length; ++i) { - sb.append(chars[RANDOM.nextInt(chars.length)]); + sb.append(chars[RANDOM.nextInt(maxIndex)]); } return sb.toString(); } diff --git a/src/main/java/fr/xephi/authme/security/crypts/CryptPBKDF2Django.java b/src/main/java/fr/xephi/authme/security/crypts/CryptPBKDF2Django.java index c4df8ab4..ab436697 100644 --- a/src/main/java/fr/xephi/authme/security/crypts/CryptPBKDF2Django.java +++ b/src/main/java/fr/xephi/authme/security/crypts/CryptPBKDF2Django.java @@ -1,6 +1,6 @@ package fr.xephi.authme.security.crypts; -import fr.xephi.authme.security.HashUtils; +import fr.xephi.authme.security.RandomString; import fr.xephi.authme.security.crypts.description.HasSalt; import fr.xephi.authme.security.crypts.description.Recommendation; import fr.xephi.authme.security.crypts.description.SaltType; @@ -10,7 +10,7 @@ import fr.xephi.authme.security.pbkdf2.PBKDF2Parameters; import javax.xml.bind.DatatypeConverter; -@Recommendation(Usage.OK) +@Recommendation(Usage.ACCEPTABLE) @HasSalt(value = SaltType.TEXT, length = 12) public class CryptPBKDF2Django implements EncryptionMethod { @@ -38,7 +38,7 @@ public class CryptPBKDF2Django implements EncryptionMethod { } public String generateSalt() { - return HashUtils.generateSalt(12); + return RandomString.generateHex(12); } } diff --git a/src/main/java/fr/xephi/authme/security/crypts/JOOMLA.java b/src/main/java/fr/xephi/authme/security/crypts/JOOMLA.java index 36c09244..22371171 100644 --- a/src/main/java/fr/xephi/authme/security/crypts/JOOMLA.java +++ b/src/main/java/fr/xephi/authme/security/crypts/JOOMLA.java @@ -1,12 +1,13 @@ package fr.xephi.authme.security.crypts; import fr.xephi.authme.security.HashUtils; +import fr.xephi.authme.security.RandomString; 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; -@Recommendation(Usage.OK) +@Recommendation(Usage.ACCEPTABLE) @HasSalt(value = SaltType.TEXT, length = 32) public class JOOMLA implements EncryptionMethod { @@ -20,7 +21,7 @@ public class JOOMLA implements EncryptionMethod { } public String generateSalt() { - return HashUtils.generateSalt(32); + return RandomString.generateHex(32); } @Override diff --git a/src/main/java/fr/xephi/authme/security/crypts/MD5VB.java b/src/main/java/fr/xephi/authme/security/crypts/MD5VB.java index cbd30eb2..a994f7ea 100644 --- a/src/main/java/fr/xephi/authme/security/crypts/MD5VB.java +++ b/src/main/java/fr/xephi/authme/security/crypts/MD5VB.java @@ -1,6 +1,6 @@ package fr.xephi.authme.security.crypts; -import fr.xephi.authme.security.HashUtils; +import fr.xephi.authme.security.RandomString; import fr.xephi.authme.security.crypts.description.HasSalt; import fr.xephi.authme.security.crypts.description.Recommendation; import fr.xephi.authme.security.crypts.description.SaltType; @@ -8,7 +8,7 @@ import fr.xephi.authme.security.crypts.description.Usage; import static fr.xephi.authme.security.HashUtils.md5; -@Recommendation(Usage.OK) +@Recommendation(Usage.ACCEPTABLE) @HasSalt(value = SaltType.TEXT, length = 16) public class MD5VB implements EncryptionMethod { @@ -28,7 +28,7 @@ public class MD5VB implements EncryptionMethod { } public String generateSalt() { - return HashUtils.generateSalt(16); + return RandomString.generateHex(16); } } diff --git a/src/main/java/fr/xephi/authme/security/crypts/SHA256.java b/src/main/java/fr/xephi/authme/security/crypts/SHA256.java index 834b208f..5024072d 100644 --- a/src/main/java/fr/xephi/authme/security/crypts/SHA256.java +++ b/src/main/java/fr/xephi/authme/security/crypts/SHA256.java @@ -1,33 +1,34 @@ package fr.xephi.authme.security.crypts; -import java.math.BigInteger; -import java.security.MessageDigest; -import java.security.NoSuchAlgorithmException; +import fr.xephi.authme.security.RandomString; +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 static fr.xephi.authme.security.HashUtils.sha256; + +@Recommendation(Usage.RECOMMENDED) +@HasSalt(value = SaltType.TEXT, length = 16) public class SHA256 implements EncryptionMethod { - private static String getSHA256(String message) - throws NoSuchAlgorithmException { - MessageDigest sha256 = MessageDigest.getInstance("SHA-256"); - sha256.reset(); - sha256.update(message.getBytes()); - byte[] digest = sha256.digest(); - return String.format("%0" + (digest.length << 1) + "x", new BigInteger(1, digest)); + @Override + public String computeHash(String password, String salt, String name) { + return "$SHA$" + salt + "$" + sha256(sha256(password) + salt); + } + + public String computeHash(String password, String name) { + return computeHash(password, generateSalt(), name); } @Override - public String computeHash(String password, String salt, String name) - throws NoSuchAlgorithmException { - return "$SHA$" + salt + "$" + getSHA256(getSHA256(password) + salt); - } - - @Override - public boolean comparePassword(String hash, String password, - String playerName) throws NoSuchAlgorithmException { + public boolean comparePassword(String hash, String password, String playerName) { String[] line = hash.split("\\$"); - return hash.equals(computeHash(password, line[2], "")); + return line.length == 4 && hash.equals(computeHash(password, line[2], "")); + } + + public String generateSalt() { + return RandomString.generateHex(16); } } 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 888406a1..10843205 100644 --- a/src/main/java/fr/xephi/authme/security/crypts/SHA512.java +++ b/src/main/java/fr/xephi/authme/security/crypts/SHA512.java @@ -1,31 +1,30 @@ package fr.xephi.authme.security.crypts; -import java.math.BigInteger; -import java.security.MessageDigest; -import java.security.NoSuchAlgorithmException; +import fr.xephi.authme.security.HashUtils; +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; -/** - */ +@Recommendation(Usage.DO_NOT_USE) +@HasSalt(SaltType.NONE) public class SHA512 implements EncryptionMethod { - private static String getSHA512(String message) - throws NoSuchAlgorithmException { - MessageDigest sha512 = MessageDigest.getInstance("SHA-512"); - sha512.reset(); - sha512.update(message.getBytes()); - byte[] digest = sha512.digest(); - return String.format("%0" + (digest.length << 1) + "x", new BigInteger(1, digest)); + @Override + public String computeHash(String password, String salt, String name) { + return computeHash(password, name); + } + + public String computeHash(String password, String name) { + return HashUtils.sha512(password); } @Override - public String computeHash(String password, String salt, String name) - throws NoSuchAlgorithmException { - return getSHA512(password); - } - - @Override - public boolean comparePassword(String hash, String password, - String playerName) throws NoSuchAlgorithmException { + public boolean comparePassword(String hash, String password, String playerName) { return hash.equals(computeHash(password, "", "")); } + + public String generateSalt() { + return null; + } } diff --git a/src/main/java/fr/xephi/authme/security/crypts/SMF.java b/src/main/java/fr/xephi/authme/security/crypts/SMF.java index d2d4f74d..0a2346eb 100644 --- a/src/main/java/fr/xephi/authme/security/crypts/SMF.java +++ b/src/main/java/fr/xephi/authme/security/crypts/SMF.java @@ -1,31 +1,30 @@ package fr.xephi.authme.security.crypts; -import java.math.BigInteger; -import java.security.MessageDigest; -import java.security.NoSuchAlgorithmException; +import fr.xephi.authme.security.HashUtils; +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; -/** - */ +@Recommendation(Usage.DO_NOT_USE) +@HasSalt(SaltType.USERNAME) public class SMF implements EncryptionMethod { - private static String getSHA1(String message) - throws NoSuchAlgorithmException { - MessageDigest sha1 = MessageDigest.getInstance("SHA1"); - sha1.reset(); - sha1.update(message.getBytes()); - byte[] digest = sha1.digest(); - return String.format("%0" + (digest.length << 1) + "x", new BigInteger(1, digest)); + @Override + public String computeHash(String password, String salt, String name) { + return computeHash(password, name); + } + + public String computeHash(String password, String name) { + return HashUtils.sha1(name.toLowerCase() + password); } @Override - public String computeHash(String password, String salt, String name) - throws NoSuchAlgorithmException { - return getSHA1(name.toLowerCase() + password); + public boolean comparePassword(String hash, String password, String playerName) { + return hash.equals(computeHash(password, playerName)); } - @Override - public boolean comparePassword(String hash, String password, - String playerName) throws NoSuchAlgorithmException { - return hash.equals(computeHash(password, null, playerName)); + public String generateSalt() { + return null; } } diff --git a/src/main/java/fr/xephi/authme/security/crypts/WORDPRESS.java b/src/main/java/fr/xephi/authme/security/crypts/WORDPRESS.java index 65059119..f9a17632 100644 --- a/src/main/java/fr/xephi/authme/security/crypts/WORDPRESS.java +++ b/src/main/java/fr/xephi/authme/security/crypts/WORDPRESS.java @@ -11,7 +11,7 @@ import java.security.NoSuchAlgorithmException; import java.security.SecureRandom; import java.util.Arrays; -@Recommendation(Usage.OK) +@Recommendation(Usage.ACCEPTABLE) @HasSalt(value = SaltType.TEXT, length = 9) public class WORDPRESS implements EncryptionMethod { diff --git a/src/main/java/fr/xephi/authme/security/crypts/XAUTH.java b/src/main/java/fr/xephi/authme/security/crypts/XAUTH.java index da0591aa..b3f07752 100644 --- a/src/main/java/fr/xephi/authme/security/crypts/XAUTH.java +++ b/src/main/java/fr/xephi/authme/security/crypts/XAUTH.java @@ -1,6 +1,6 @@ package fr.xephi.authme.security.crypts; -import fr.xephi.authme.security.HashUtils; +import fr.xephi.authme.security.RandomString; import fr.xephi.authme.security.crypts.description.HasSalt; import fr.xephi.authme.security.crypts.description.Recommendation; import fr.xephi.authme.security.crypts.description.SaltType; @@ -38,7 +38,7 @@ public class XAUTH implements EncryptionMethod { } public String generateSalt() { - return HashUtils.generateSalt(12); + return RandomString.generateHex(12); } } diff --git a/src/main/java/fr/xephi/authme/security/crypts/description/HasSalt.java b/src/main/java/fr/xephi/authme/security/crypts/description/HasSalt.java index b903a88d..1baf1e19 100644 --- a/src/main/java/fr/xephi/authme/security/crypts/description/HasSalt.java +++ b/src/main/java/fr/xephi/authme/security/crypts/description/HasSalt.java @@ -1,13 +1,22 @@ package fr.xephi.authme.security.crypts.description; +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + /** * Describes the type of salt the encryption algorithm uses. This is purely for documentation * purposes and is ignored by the code. */ +@Target(ElementType.TYPE) +@Retention(RetentionPolicy.RUNTIME) public @interface HasSalt { + /** The type of the salt. */ SaltType value(); + /** For text salts, the length of the salt. */ int length() default 0; } diff --git a/src/main/java/fr/xephi/authme/security/crypts/description/Recommendation.java b/src/main/java/fr/xephi/authme/security/crypts/description/Recommendation.java index 06ebc22c..f37c3eac 100644 --- a/src/main/java/fr/xephi/authme/security/crypts/description/Recommendation.java +++ b/src/main/java/fr/xephi/authme/security/crypts/description/Recommendation.java @@ -1,10 +1,18 @@ package fr.xephi.authme.security.crypts.description; +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + /** * Annotation to mark a hash algorithm with the usage recommendation, see {@link Usage}. */ +@Target(ElementType.TYPE) +@Retention(RetentionPolicy.RUNTIME) public @interface Recommendation { + /** The recommendation for using the hash algorithm. */ Usage value(); } diff --git a/src/main/java/fr/xephi/authme/security/crypts/description/SaltType.java b/src/main/java/fr/xephi/authme/security/crypts/description/SaltType.java index c7d3ba1d..f91ca61c 100644 --- a/src/main/java/fr/xephi/authme/security/crypts/description/SaltType.java +++ b/src/main/java/fr/xephi/authme/security/crypts/description/SaltType.java @@ -8,7 +8,7 @@ public enum SaltType { /** Random, newly generated text. */ TEXT, - /** The username, including variations or repetitions. */ + /** Salt is based on the username, including variations and repetitions. */ USERNAME, /** No salt. */ 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 990ee298..ecf37a98 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 @@ -9,7 +9,7 @@ public enum Usage { RECOMMENDED, /** There are safer algorithms that can be chosen but using the algorithm is generally OK. */ - OK, + ACCEPTABLE, /** Hash algorithm is not recommended to be used. Use only if required by another system. */ DO_NOT_USE,