#358 Update EncryptionMethod to new interface

- Add new methods to the EncryptionMethod interface
- Delete temporary interface (NewEncrMethod)
- Remove temporary checks and casts to NewEncrMethod
This commit is contained in:
ljacqu 2015-12-28 22:00:43 +01:00
parent 9b73475b9a
commit 47f4275225
16 changed files with 147 additions and 188 deletions

View File

@ -7,7 +7,6 @@ import fr.xephi.authme.events.PasswordEncryptionEvent;
import fr.xephi.authme.security.crypts.BCRYPT;
import fr.xephi.authme.security.crypts.EncryptionMethod;
import fr.xephi.authme.security.crypts.HashResult;
import fr.xephi.authme.security.crypts.NewEncrMethod;
import fr.xephi.authme.settings.Settings;
import org.bukkit.Bukkit;
@ -145,7 +144,18 @@ public class PasswordSecurity {
if (method == null)
throw new NoSuchAlgorithmException("Unknown hash algorithm");
if (method.comparePassword(hash, password, playerName))
String salt = null;
if (method.hasSeparateSalt()) {
PlayerAuth auth = AuthMe.getInstance().getDataSource().getAuth(playerName);
if (auth == null) {
// User is not in data source, so the result will invariably be wrong because an encryption
// method with hasSeparateSalt() == true NEEDS the salt to evaluate the password
return false;
}
salt = auth.getSalt();
}
if (method.comparePassword(hash, password, salt, playerName))
return true;
if (Settings.supportOldPassword) {
@ -159,32 +169,21 @@ public class PasswordSecurity {
}
public HashResult computeHash(HashAlgorithm algorithm, String password, String playerName) {
EncryptionMethod method1 = initializeEncryptionMethod(algorithm, playerName);
// TODO #358: Remove this check:
NewEncrMethod method;
if (method1 instanceof NewEncrMethod) {
method = (NewEncrMethod) method1;
} else {
throw new RuntimeException("TODO #358: Class not yet extended with NewEncrMethod methods");
}
EncryptionMethod method = initializeEncryptionMethod(algorithm, playerName);
return method.computeHash(password, playerName);
}
public boolean comparePassword(HashAlgorithm algorithm, String hash, String password, String playerName) {
EncryptionMethod method1 = initializeEncryptionMethod(algorithm, playerName);
// TODO #358: Remove this check:
NewEncrMethod method;
if (method1 instanceof NewEncrMethod) {
method = (NewEncrMethod) method1;
} else {
throw new RuntimeException("TODO #358: Class not yet extended with NewEncrMethod methods");
}
EncryptionMethod method = initializeEncryptionMethod(algorithm, playerName);
String salt = null;
if (method.hasSeparateSalt()) {
PlayerAuth auth = dataSource.getAuth(playerName);
salt = (auth != null) ? auth.getSalt() : null;
if (auth == null) {
// User is not in data source, so the result will invariably be wrong because an encryption
// method with hasSeparateSalt() == true NEEDS the salt to evaluate the password
return false;
}
salt = auth.getSalt();
}
return method.comparePassword(hash, password, salt, playerName);
// TODO #358: Add logic for Settings.supportOldPassword
@ -209,11 +208,19 @@ public class PasswordSecurity {
@Deprecated
private static boolean compareWithAllEncryptionMethod(String password,
String hash, String playerName) {
String salt;
PlayerAuth auth = AuthMe.getInstance().getDataSource().getAuth(playerName);
if (auth == null) {
salt = null;
} else {
salt = auth.getSalt();
}
for (HashAlgorithm algo : HashAlgorithm.values()) {
if (algo != HashAlgorithm.CUSTOM) {
try {
EncryptionMethod method = algo.getClazz().newInstance();
if (method.comparePassword(hash, password, playerName)) {
if (method.comparePassword(hash, password, salt, playerName)) {
PlayerAuth nAuth = AuthMe.getInstance().database.getAuth(playerName);
if (nAuth != null) {
nAuth.setHash(getHash(Settings.getPasswordHash, password, playerName));

View File

@ -66,7 +66,7 @@ import java.security.SecureRandom;
*/
@Recommendation(Usage.RECOMMENDED) // provided the salt length is >= 8
@HasSalt(value = SaltType.TEXT) // length depends on Settings.bCryptLog2Rounds
public class BCRYPT implements NewEncrMethod {
public class BCRYPT implements EncryptionMethod {
// BCrypt parameters
private static final int GENSALT_DEFAULT_LOG2_ROUNDS = 10;
@ -525,14 +525,9 @@ public class BCRYPT implements NewEncrMethod {
return new HashResult(hashpw(password, salt), salt);
}
@Override
public boolean comparePassword(String hash, String password, String playerName) {
return checkpw(password, hash);
}
@Override
public boolean comparePassword(String hash, String password, String salt, String name) {
return comparePassword(hash, password, name);
return checkpw(password, hash);
}
@Override

View File

@ -5,11 +5,10 @@ import fr.xephi.authme.security.crypts.description.Usage;
import fr.xephi.authme.security.pbkdf2.PBKDF2Engine;
import fr.xephi.authme.security.pbkdf2.PBKDF2Parameters;
import java.security.NoSuchAlgorithmException;
import java.util.Arrays;
@Recommendation(Usage.DOES_NOT_WORK)
public class CryptPBKDF2 implements EncryptionMethod {
public class CryptPBKDF2 extends HexSaltedMethod {
@Override
public String computeHash(String password, String salt, String name) {
@ -21,7 +20,7 @@ public class CryptPBKDF2 implements EncryptionMethod {
}
@Override
public boolean comparePassword(String hash, String password, String playerName) {
public boolean comparePassword(String hash, String password, String unusedSalt, String unusedName) {
String[] line = hash.split("\\$");
String salt = line[2];
String derivedKey = line[3];
@ -30,4 +29,9 @@ public class CryptPBKDF2 implements EncryptionMethod {
return engine.verifyKey(password);
}
@Override
public int getSaltLength() {
return 12;
}
}

View File

@ -5,28 +5,56 @@ package fr.xephi.authme.security.crypts;
*/
public interface EncryptionMethod {
/**
* Hash the given password for the given player name.
*
* @param password The password to hash
* @param name The name of the player (sometimes required to generate a salt with)
*
* @return The hash result for the password.
* @see HashResult
*/
HashResult computeHash(String password, String name);
/**
* Hash the given password with the given salt for the given player.
*
* @param password The clear-text password to hash
* @param password The password to hash
* @param salt The salt to add to the hash
* @param name The player's name (sometimes required for storing the salt separately in the database)
* @param name The player's name (sometimes required to generate a salt with)
*
* @return The hashed password
* @see #hasSeparateSalt()
*/
String computeHash(String password, String salt, String name);
/**
* Check whether a given hash matches the clear-text password.
* Check whether the given hash matches the clear-text password.
*
* @param hash The hash to verify
* @param password The clear-text password to verify the hash against
* @param playerName The player name to do the check for (sometimes required for retrieving
* the salt from the database)
* @param hash The hash to verify
* @param password The clear-text password to verify the hash against
* @param salt The salt if it is stored separately (null otherwise)
* @param name The player name to do the check for (sometimes required for generating the salt)
*
* @return True if the password matches, false otherwise
*/
@Deprecated
boolean comparePassword(String hash, String password, String playerName);
boolean comparePassword(String hash, String password, String salt, String name);
/**
* Generate a new salt to hash a password with.
*
* @return The generated salt, null if the method does not use a random text-based salt
*/
String generateSalt();
/**
* Return whether the encryption method requires the salt to be stored separately and
* passed again to {@link #comparePassword(String, String, String, String)}. Note that
* an encryption method returning {@code false} does not imply that it uses no salt; it
* may be embedded into the hash or it may use the username as salt.
*
* @return True if the salt has to be stored and retrieved separately, false otherwise
*/
boolean hasSeparateSalt();
}

View File

@ -1,20 +1,38 @@
package fr.xephi.authme.security.crypts;
/**
* The result of a hash computation.
* The result of a hash computation. See {@link #salt} for details.
*/
public class HashResult {
/** The generated hash. */
private final String hash;
/** The generated salt; may be null if no salt is used or if the salt is included in the hash output. */
/**
* The generated salt; may be null if no salt is used or if the salt is included
* in the hash output. The salt is only not null if {@link EncryptionMethod#hasSeparateSalt()}
* returns true for the associated encryption method.
* <p>
* When the field is not null, it must be stored into the salt column of the data source
* and retrieved again to compare a password with the hash.
*/
private final String salt;
/**
* Constructor.
*
* @param hash The computed hash
* @param salt The generated salt
*/
public HashResult(String hash, String salt) {
this.hash = hash;
this.salt = salt;
}
/**
* Constructor for a hash with no separate salt.
*
* @param hash The computed hash
*/
public HashResult(String hash) {
this(hash, null);
}

View File

@ -12,7 +12,7 @@ import fr.xephi.authme.security.crypts.description.Usage;
*/
@Recommendation(Usage.ACCEPTABLE)
@HasSalt(SaltType.TEXT) // See saltLength() for length
public abstract class HexSaltedMethod implements NewEncrMethod {
public abstract class HexSaltedMethod implements EncryptionMethod {
public abstract int getSaltLength();
@ -28,12 +28,6 @@ public abstract class HexSaltedMethod implements NewEncrMethod {
@Override
public abstract boolean comparePassword(String hash, String password, String salt, String name);
@Override
@Deprecated
public boolean comparePassword(String hash, String password, String playerName) {
return false;
}
@Override
public String generateSalt() {
return RandomString.generateHex(getSaltLength());

View File

@ -1,22 +1,16 @@
package fr.xephi.authme.security.crypts;
import fr.xephi.authme.AuthMe;
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;
import java.math.BigInteger;
import java.security.MessageDigest;
import java.security.NoSuchAlgorithmException;
import static fr.xephi.authme.security.HashUtils.md5;
@Recommendation(Usage.DO_NOT_USE)
@HasSalt(value = SaltType.TEXT, length = 5)
public class IPB3 implements NewEncrMethod {
public class IPB3 implements EncryptionMethod {
@Override
public String computeHash(String password, String salt, String name) {
@ -29,12 +23,6 @@ public class IPB3 implements NewEncrMethod {
return new HashResult(computeHash(password, salt, name), salt);
}
@Override
public boolean comparePassword(String hash, String password, String playerName) {
String salt = AuthMe.getInstance().database.getAuth(playerName).getSalt();
return hash.equals(computeHash(password, salt, playerName));
}
@Override
public boolean comparePassword(String hash, String password, String salt, String name) {
return hash.equals(computeHash(password, salt, name));

View File

@ -1,16 +1,10 @@
package fr.xephi.authme.security.crypts;
import fr.xephi.authme.AuthMe;
import fr.xephi.authme.security.HashUtils;
import fr.xephi.authme.security.RandomString;
import java.math.BigInteger;
import java.security.MessageDigest;
import java.security.NoSuchAlgorithmException;
import static fr.xephi.authme.security.HashUtils.md5;
public class MYBB implements NewEncrMethod {
public class MYBB implements EncryptionMethod {
@Override
public String computeHash(String password, String salt, String name) {
@ -23,12 +17,6 @@ public class MYBB implements NewEncrMethod {
return new HashResult(computeHash(password, salt, name), salt);
}
@Override
public boolean comparePassword(String hash, String password, String playerName) {
String salt = AuthMe.getInstance().database.getAuth(playerName).getSalt();
return hash.equals(computeHash(password, salt, playerName));
}
@Override
public boolean comparePassword(String hash, String password, String salt, String name) {
return hash.equals(computeHash(password, salt, name));

View File

@ -1,17 +0,0 @@
package fr.xephi.authme.security.crypts;
/**
* Temporary interface for additional methods that will be added to {@link EncryptionMethod}.
* TODO #358: Move methods to EncryptionMethod interface and delete this.
*/
public interface NewEncrMethod extends EncryptionMethod {
HashResult computeHash(String password, String name);
String generateSalt();
boolean hasSeparateSalt();
boolean comparePassword(String hash, String password, String salt, String name);
}

View File

@ -1,6 +1,5 @@
package fr.xephi.authme.security.crypts;
import fr.xephi.authme.AuthMe;
import fr.xephi.authme.security.HashUtils;
import fr.xephi.authme.security.RandomString;
import fr.xephi.authme.security.crypts.description.Recommendation;
@ -9,13 +8,11 @@ import fr.xephi.authme.security.crypts.description.Usage;
import javax.crypto.Mac;
import javax.crypto.spec.SecretKeySpec;
import java.io.UnsupportedEncodingException;
import java.math.BigInteger;
import java.security.InvalidKeyException;
import java.security.MessageDigest;
import java.security.NoSuchAlgorithmException;
@Recommendation(Usage.DO_NOT_USE)
public class PHPFUSION implements NewEncrMethod {
public class PHPFUSION implements EncryptionMethod {
@Override
public String computeHash(String password, String salt, String name) {
@ -46,13 +43,6 @@ public class PHPFUSION implements NewEncrMethod {
return new HashResult(computeHash(password, salt, name), salt);
}
@Override
public boolean comparePassword(String hash, String password,
String playerName) {
String salt = AuthMe.getInstance().database.getAuth(playerName).getSalt();
return hash.equals(computeHash(password, salt, ""));
}
@Override
public boolean comparePassword(String hash, String password, String salt, String name) {
return hash.equals(computeHash(password, salt, name));

View File

@ -3,7 +3,7 @@ package fr.xephi.authme.security.crypts;
/**
* Common supertype for encryption methods which store their salt separately from the hash.
*/
public abstract class SeparateSaltMethod implements NewEncrMethod {
public abstract class SeparateSaltMethod implements EncryptionMethod {
@Override
public abstract String computeHash(String password, String salt, String name);
@ -27,9 +27,4 @@ public abstract class SeparateSaltMethod implements NewEncrMethod {
return true;
}
@Override
@Deprecated
public boolean comparePassword(String hash, String password, String playerName) {
return false;
}
}

View File

@ -10,7 +10,7 @@ import fr.xephi.authme.security.crypts.description.Usage;
*/
@Recommendation(Usage.DO_NOT_USE)
@HasSalt(SaltType.NONE)
public abstract class UnsaltedMethod implements NewEncrMethod {
public abstract class UnsaltedMethod implements EncryptionMethod {
public abstract String computeHash(String password);
@ -24,12 +24,6 @@ public abstract class UnsaltedMethod implements NewEncrMethod {
return computeHash(password);
}
@Override
@Deprecated
public boolean comparePassword(String hash, String password, String playerName) {
return false;
}
@Override
public boolean comparePassword(String hash, String password, String salt, String name) {
return hash.equals(computeHash(password));

View File

@ -11,7 +11,7 @@ import fr.xephi.authme.security.crypts.description.Usage;
*/
@Recommendation(Usage.DO_NOT_USE)
@HasSalt(SaltType.USERNAME)
public abstract class UsernameSaltMethod implements NewEncrMethod {
public abstract class UsernameSaltMethod implements EncryptionMethod {
@Override
public abstract HashResult computeHash(String password, String name);
@ -21,12 +21,6 @@ public abstract class UsernameSaltMethod implements NewEncrMethod {
return hash.equals(computeHash(password, name).getHash());
}
@Override
@Deprecated
public boolean comparePassword(String hash, String password, String playerName) {
return false;
}
@Override
public String computeHash(String password, String salt, String name) {
return computeHash(password, name).getHash();

View File

@ -1,10 +1,10 @@
package fr.xephi.authme.security.crypts;
import java.security.NoSuchAlgorithmException;
import fr.xephi.authme.security.crypts.description.Recommendation;
import fr.xephi.authme.security.crypts.description.Usage;
/**
*/
public class WBB4 implements EncryptionMethod {
@Recommendation(Usage.DOES_NOT_WORK)
public class WBB4 extends HexSaltedMethod {
@Override
public String computeHash(String password, String salt, String name) {
@ -12,8 +12,23 @@ public class WBB4 implements EncryptionMethod {
}
@Override
public boolean comparePassword(String hash, String password, String playerName) {
public boolean comparePassword(String hash, String password, String salt, String playerName) {
return BCRYPT.checkpw(password, hash, 2);
}
@Override
public String generateSalt() {
return BCRYPT.gensalt(8);
}
/**
* Note that {@link #generateSalt()} is overridden for this class.
*
* @return The salt length
*/
@Override
public int getSaltLength() {
return 8;
}
}

View File

@ -1,7 +1,5 @@
package fr.xephi.authme.security.crypts;
import fr.xephi.authme.AuthMe;
import java.security.MessageDigest;
import java.security.NoSuchAlgorithmException;
import java.util.ArrayList;
@ -11,7 +9,7 @@ import java.util.regex.Pattern;
/**
*/
public class XF implements NewEncrMethod {
public class XF implements EncryptionMethod {
@Override
public String computeHash(String password, String salt, String name) {
@ -24,12 +22,6 @@ public class XF implements NewEncrMethod {
return new HashResult(computeHash(password, salt, null), salt);
}
@Override
public boolean comparePassword(String hash, String password, String playerName) {
String salt = AuthMe.getInstance().database.getAuth(playerName).getSalt();
return hash.equals(regmatch("\"hash\";.:..:\"(.*)\";.:.:\"salt\"", salt));
}
public boolean comparePassword(String hash, String password, String salt, String name) {
return hash.equals(regmatch("\"hash\";.:..:\"(.*)\";.:.:\"salt\"", salt));
}

View File

@ -9,7 +9,6 @@ import static org.hamcrest.Matchers.equalTo;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertThat;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
/**
* Test for implementations of {@link EncryptionMethod}.
@ -98,45 +97,30 @@ public abstract class AbstractEncryptionMethodTest {
@Test
public void testPasswordEquality() {
// TODO #358: Remove instanceof and use this code always
if (method instanceof NewEncrMethod) {
NewEncrMethod method1 = (NewEncrMethod) method;
for (String password : INTERNAL_PASSWORDS) {
final String salt = method1.generateSalt();
final String hash = method1.computeHash(password, salt, USERNAME);
for (String password : INTERNAL_PASSWORDS) {
final String salt = method.generateSalt();
final String hash = method.computeHash(password, salt, USERNAME);
// Check that the computeHash(password, salt, name) method has the same output for the returned salt
assertThat(hash, equalTo(method1.computeHash(password, salt, USERNAME)));
// Check that the computeHash(password, salt, name) method has the same output for the returned salt
assertThat(hash, equalTo(method.computeHash(password, salt, USERNAME)));
assertTrue("Generated hash for '" + password + "' should match password (hash = '" + hash + "')",
method1.comparePassword(hash, password, salt, USERNAME));
if (!password.equals(password.toLowerCase())) {
assertFalse("Lower-case of '" + password + "' should not match generated hash '" + hash + "'",
method1.comparePassword(hash, password.toLowerCase(), salt, USERNAME));
}
if (!password.equals(password.toUpperCase())) {
assertFalse("Upper-case of '" + password + "' should not match generated hash '" + hash + "'",
method1.comparePassword(hash, password.toUpperCase(), salt, USERNAME));
}
assertTrue("Generated hash for '" + password + "' should match password (hash = '" + hash + "')",
method.comparePassword(hash, password, salt, USERNAME));
if (!password.equals(password.toLowerCase())) {
assertFalse("Lower-case of '" + password + "' should not match generated hash '" + hash + "'",
method.comparePassword(hash, password.toLowerCase(), salt, USERNAME));
}
if (!password.equals(password.toUpperCase())) {
assertFalse("Upper-case of '" + password + "' should not match generated hash '" + hash + "'",
method.comparePassword(hash, password.toUpperCase(), salt, USERNAME));
}
return;
}
fail("No longer supporting old EncryptionMethod implementations");
}
private boolean doesGivenHashMatch(String password, EncryptionMethod method) {
// TODO #358: Remove casting checks and remove old code below
if (method instanceof NewEncrMethod) {
NewEncrMethod method1 = (NewEncrMethod) method;
String hash = hashes.get(password);
String salt = salts.get(password);
return method1.comparePassword(hash, password, salt, USERNAME);
}
// TODO #358: Remove line below
return method.comparePassword(hashes.get(password), password, USERNAME);
String hash = hashes.get(password);
String salt = salts.get(password);
return method.comparePassword(hash, password, salt, USERNAME);
}
// @org.junit.Test public void a() { AbstractEncryptionMethodTest.generateTest(); }
@ -148,28 +132,18 @@ public abstract class AbstractEncryptionMethodTest {
System.out.println("\n\tpublic " + className + "Test() {");
System.out.println("\t\tsuper(new " + className + "(),");
NewEncrMethod method1 = null;
if (method instanceof NewEncrMethod) {
method1 = (NewEncrMethod) method;
}
String delim = ", ";
for (String password : GIVEN_PASSWORDS) {
if (password.equals(GIVEN_PASSWORDS[GIVEN_PASSWORDS.length - 1])) {
delim = "); ";
}
if (method1 != null) {
if (method1.hasSeparateSalt()) {
HashResult hashResult = method1.computeHash(password, USERNAME);
System.out.println(String.format("\t\tnew HashResult(\"%s\", \"%s\")%s// %s",
hashResult.getHash(), hashResult.getSalt(), delim, password));
} else {
System.out.println("\t\t\"" + method1.computeHash(password, USERNAME).getHash()
+ "\"" + delim + "// " + password);
}
if (method.hasSeparateSalt()) {
HashResult hashResult = method.computeHash(password, USERNAME);
System.out.println(String.format("\t\tnew HashResult(\"%s\", \"%s\")%s// %s",
hashResult.getHash(), hashResult.getSalt(), delim, password));
} else {
System.out.println("\t\t\"" + method.computeHash(password, null, USERNAME)
System.out.println("\t\t\"" + method.computeHash(password, USERNAME).getHash()
+ "\"" + delim + "// " + password);
}
}