#449 Remove use of legacy settings in encryption methods

This commit is contained in:
ljacqu 2016-04-23 12:46:30 +02:00
parent ee745f153d
commit a78e0408c6
15 changed files with 203 additions and 118 deletions

View File

@ -338,7 +338,7 @@ public class AuthMe extends JavaPlugin {
} }
database.reload(); database.reload();
messages.reload(newSettings.getMessagesFile()); messages.reload(newSettings.getMessagesFile());
passwordSecurity.reload(newSettings); passwordSecurity.reload();
spawnLoader.initialize(newSettings); spawnLoader.initialize(newSettings);
} }

View File

@ -4,7 +4,6 @@ import fr.xephi.authme.AuthMe;
import fr.xephi.authme.ConsoleLogger; import fr.xephi.authme.ConsoleLogger;
import fr.xephi.authme.cache.auth.PlayerAuth; import fr.xephi.authme.cache.auth.PlayerAuth;
import fr.xephi.authme.datasource.DataSource; import fr.xephi.authme.datasource.DataSource;
import fr.xephi.authme.security.HashAlgorithm;
import fr.xephi.authme.security.PasswordSecurity; import fr.xephi.authme.security.PasswordSecurity;
import fr.xephi.authme.security.crypts.HashedPassword; import fr.xephi.authme.security.crypts.HashedPassword;
import fr.xephi.authme.settings.Settings; import fr.xephi.authme.settings.Settings;
@ -35,7 +34,6 @@ public class RakamakConverter implements Converter {
@Override @Override
// TODO ljacqu 20151229: Restructure this into smaller portions // TODO ljacqu 20151229: Restructure this into smaller portions
public void run() { public void run() {
HashAlgorithm hash = Settings.getPasswordHash;
boolean useIP = Settings.rakamakUseIp; boolean useIP = Settings.rakamakUseIp;
String fileName = Settings.rakamakUsers; String fileName = Settings.rakamakUsers;
String ipFileName = Settings.rakamakUsersIp; String ipFileName = Settings.rakamakUsersIp;
@ -64,7 +62,7 @@ public class RakamakConverter implements Converter {
while ((line = users.readLine()) != null) { while ((line = users.readLine()) != null) {
if (line.contains("=")) { if (line.contains("=")) {
String[] arguments = line.split("="); String[] arguments = line.split("=");
HashedPassword hashedPassword = passwordSecurity.computeHash(hash, arguments[1], arguments[0]); HashedPassword hashedPassword = passwordSecurity.computeHash(arguments[1], arguments[0]);
playerPSW.put(arguments[0], hashedPassword); playerPSW.put(arguments[0], hashedPassword);
} }

View File

@ -8,46 +8,75 @@ import fr.xephi.authme.settings.NewSetting;
import fr.xephi.authme.settings.properties.SecuritySettings; import fr.xephi.authme.settings.properties.SecuritySettings;
import org.bukkit.plugin.PluginManager; import org.bukkit.plugin.PluginManager;
import java.lang.reflect.Constructor;
import java.lang.reflect.InvocationTargetException;
/** /**
* Manager class for password-related operations. * Manager class for password-related operations.
*/ */
public class PasswordSecurity { public class PasswordSecurity {
private final NewSetting settings;
private HashAlgorithm algorithm; private HashAlgorithm algorithm;
private boolean supportOldAlgorithm; private boolean supportOldAlgorithm;
private final DataSource dataSource; private final DataSource dataSource;
private final PluginManager pluginManager; private final PluginManager pluginManager;
public PasswordSecurity(DataSource dataSource, NewSetting settings, PluginManager pluginManager) { public PasswordSecurity(DataSource dataSource, NewSetting settings, PluginManager pluginManager) {
this.settings = settings;
this.algorithm = settings.getProperty(SecuritySettings.PASSWORD_HASH); this.algorithm = settings.getProperty(SecuritySettings.PASSWORD_HASH);
this.supportOldAlgorithm = settings.getProperty(SecuritySettings.SUPPORT_OLD_PASSWORD_HASH); this.supportOldAlgorithm = settings.getProperty(SecuritySettings.SUPPORT_OLD_PASSWORD_HASH);
this.dataSource = dataSource; this.dataSource = dataSource;
this.pluginManager = pluginManager; this.pluginManager = pluginManager;
} }
/**
* Compute the hash of the configured algorithm for the given password and username.
*
* @param password The password to hash
* @param playerName The player's name
*
* @return The password hash
*/
public HashedPassword computeHash(String password, String playerName) { public HashedPassword computeHash(String password, String playerName) {
return computeHash(algorithm, password, playerName);
}
public HashedPassword computeHash(HashAlgorithm algorithm, String password, String playerName) {
String playerLowerCase = playerName.toLowerCase(); String playerLowerCase = playerName.toLowerCase();
EncryptionMethod method = initializeEncryptionMethod(algorithm, playerLowerCase); EncryptionMethod method = initializeEncryptionMethodWithEvent(algorithm, playerLowerCase);
return method.computeHash(password, playerLowerCase); return method.computeHash(password, playerLowerCase);
} }
/**
* Check if the given password matches the player's stored password.
*
* @param password The password to check
* @param playerName The player to check for
*
* @return True if the password is correct, false otherwise
*/
public boolean comparePassword(String password, String playerName) { public boolean comparePassword(String password, String playerName) {
HashedPassword auth = dataSource.getPassword(playerName); HashedPassword auth = dataSource.getPassword(playerName);
return auth != null && comparePassword(password, auth, playerName); return auth != null && comparePassword(password, auth, playerName);
} }
/**
* Check if the given password matches the given hashed password.
*
* @param password The password to check
* @param hashedPassword The hashed password to check against
* @param playerName The player to check for
*
* @return True if the password matches, false otherwise
*/
public boolean comparePassword(String password, HashedPassword hashedPassword, String playerName) { public boolean comparePassword(String password, HashedPassword hashedPassword, String playerName) {
EncryptionMethod method = initializeEncryptionMethod(algorithm, playerName); EncryptionMethod method = initializeEncryptionMethodWithEvent(algorithm, playerName);
String playerLowerCase = playerName.toLowerCase(); String playerLowerCase = playerName.toLowerCase();
return methodMatches(method, password, hashedPassword, playerLowerCase) return methodMatches(method, password, hashedPassword, playerLowerCase)
|| supportOldAlgorithm && compareWithAllEncryptionMethods(password, hashedPassword, playerLowerCase); || supportOldAlgorithm && compareWithAllEncryptionMethods(password, hashedPassword, playerLowerCase);
} }
public void reload(NewSetting settings) { /**
* Reload the configuration.
*/
public void reload() {
this.algorithm = settings.getProperty(SecuritySettings.PASSWORD_HASH); this.algorithm = settings.getProperty(SecuritySettings.PASSWORD_HASH);
this.supportOldAlgorithm = settings.getProperty(SecuritySettings.SUPPORT_OLD_PASSWORD_HASH); this.supportOldAlgorithm = settings.getProperty(SecuritySettings.SUPPORT_OLD_PASSWORD_HASH);
} }
@ -63,11 +92,10 @@ public class PasswordSecurity {
* *
* @return True if there was a password match with another encryption method, false otherwise * @return True if there was a password match with another encryption method, false otherwise
*/ */
private boolean compareWithAllEncryptionMethods(String password, HashedPassword hashedPassword, private boolean compareWithAllEncryptionMethods(String password, HashedPassword hashedPassword, String playerName) {
String playerName) {
for (HashAlgorithm algorithm : HashAlgorithm.values()) { for (HashAlgorithm algorithm : HashAlgorithm.values()) {
if (!HashAlgorithm.CUSTOM.equals(algorithm)) { if (!HashAlgorithm.CUSTOM.equals(algorithm)) {
EncryptionMethod method = initializeEncryptionMethodWithoutEvent(algorithm); EncryptionMethod method = initializeEncryptionMethod(algorithm, settings);
if (methodMatches(method, password, hashedPassword, playerName)) { if (methodMatches(method, password, hashedPassword, playerName)) {
hashPasswordForNewAlgorithm(password, playerName); hashPasswordForNewAlgorithm(password, playerName);
return true; return true;
@ -85,6 +113,7 @@ public class PasswordSecurity {
* @param password The password to check * @param password The password to check
* @param hashedPassword The hash to check against * @param hashedPassword The hash to check against
* @param playerName The name of the player * @param playerName The name of the player
*
* @return True if the password matched, false otherwise * @return True if the password matched, false otherwise
*/ */
private static boolean methodMatches(EncryptionMethod method, String password, private static boolean methodMatches(EncryptionMethod method, String password,
@ -103,33 +132,45 @@ public class PasswordSecurity {
* *
* @return The encryption method * @return The encryption method
*/ */
private EncryptionMethod initializeEncryptionMethod(HashAlgorithm algorithm, String playerName) { private EncryptionMethod initializeEncryptionMethodWithEvent(HashAlgorithm algorithm, String playerName) {
EncryptionMethod method = initializeEncryptionMethodWithoutEvent(algorithm); EncryptionMethod method = initializeEncryptionMethod(algorithm, settings);
PasswordEncryptionEvent event = new PasswordEncryptionEvent(method, playerName); PasswordEncryptionEvent event = new PasswordEncryptionEvent(method, playerName);
pluginManager.callEvent(event); pluginManager.callEvent(event);
return event.getMethod(); return event.getMethod();
} }
/** /**
* Initialize the encryption method corresponding to the given hash algorithm. * Initialize the encryption method associated with the given hash algorithm.
* *
* @param algorithm The algorithm to retrieve the encryption method for * @param algorithm The algorithm to retrieve the encryption method for
* @param settings The settings instance to pass to the constructor if required
* *
* @return The associated encryption method * @return The associated encryption method, or null if CUSTOM / deprecated
*/ */
private static EncryptionMethod initializeEncryptionMethodWithoutEvent(HashAlgorithm algorithm) { public static EncryptionMethod initializeEncryptionMethod(HashAlgorithm algorithm,
NewSetting settings) {
try { try {
return HashAlgorithm.CUSTOM.equals(algorithm) || HashAlgorithm.PLAINTEXT.equals(algorithm) if (HashAlgorithm.CUSTOM.equals(algorithm) || HashAlgorithm.PLAINTEXT.equals(algorithm)) {
? null return null;
: algorithm.getClazz().newInstance(); }
} catch (InstantiationException | IllegalAccessException e) { Constructor<?> constructor = algorithm.getClazz().getConstructors()[0];
Class<?>[] parameters = constructor.getParameterTypes();
if (parameters.length == 0) {
return (EncryptionMethod) constructor.newInstance();
} else if (parameters.length == 1 && parameters[0] == NewSetting.class) {
return (EncryptionMethod) constructor.newInstance(settings);
} else {
throw new UnsupportedOperationException("Did not find default constructor or constructor with settings "
+ "parameter in class " + algorithm.getClazz().getSimpleName());
}
} catch (InstantiationException | IllegalAccessException | InvocationTargetException e) {
throw new UnsupportedOperationException("Constructor for '" + algorithm.getClazz().getSimpleName() throw new UnsupportedOperationException("Constructor for '" + algorithm.getClazz().getSimpleName()
+ "' could not be invoked. (Is there no default constructor?)", e); + "' could not be invoked. (Is there no default constructor?)", e);
} }
} }
private void hashPasswordForNewAlgorithm(String password, String playerName) { private void hashPasswordForNewAlgorithm(String password, String playerName) {
HashedPassword hashedPassword = initializeEncryptionMethod(algorithm, playerName) HashedPassword hashedPassword = initializeEncryptionMethodWithEvent(algorithm, playerName)
.computeHash(password, playerName); .computeHash(password, playerName);
dataSource.updatePassword(playerName, hashedPassword); dataSource.updatePassword(playerName, hashedPassword);
} }

View File

@ -5,7 +5,8 @@ import fr.xephi.authme.security.crypts.description.HasSalt;
import fr.xephi.authme.security.crypts.description.Recommendation; import fr.xephi.authme.security.crypts.description.Recommendation;
import fr.xephi.authme.security.crypts.description.SaltType; import fr.xephi.authme.security.crypts.description.SaltType;
import fr.xephi.authme.security.crypts.description.Usage; import fr.xephi.authme.security.crypts.description.Usage;
import fr.xephi.authme.settings.Settings; import fr.xephi.authme.settings.NewSetting;
import fr.xephi.authme.settings.properties.HooksSettings;
import fr.xephi.authme.util.StringUtils; import fr.xephi.authme.util.StringUtils;
@ -13,6 +14,12 @@ import fr.xephi.authme.util.StringUtils;
@HasSalt(value = SaltType.TEXT) // length depends on Settings.bCryptLog2Rounds @HasSalt(value = SaltType.TEXT) // length depends on Settings.bCryptLog2Rounds
public class BCRYPT implements EncryptionMethod { public class BCRYPT implements EncryptionMethod {
private final int bCryptLog2Rounds;
public BCRYPT(NewSetting settings) {
this.bCryptLog2Rounds = settings.getProperty(HooksSettings.BCRYPT_LOG2_ROUND);
}
@Override @Override
public String computeHash(String password, String salt, String name) { public String computeHash(String password, String salt, String name) {
return BCryptService.hashpw(password, salt); return BCryptService.hashpw(password, salt);
@ -36,7 +43,7 @@ public class BCRYPT implements EncryptionMethod {
@Override @Override
public String generateSalt() { public String generateSalt() {
return BCryptService.gensalt(Settings.bCryptLog2Rounds); return BCryptService.gensalt(bCryptLog2Rounds);
} }
@Override @Override

View File

@ -2,6 +2,10 @@ package fr.xephi.authme.security.crypts;
/** /**
* Public interface for custom password encryption methods. * Public interface for custom password encryption methods.
* <p>
* Note that {@link fr.xephi.authme.security.PasswordSecurity} requires classes implementing this interface
* to either have the default constructor or an accessible constructor with one parameter of type
* {@link fr.xephi.authme.settings.NewSetting}.
*/ */
public interface EncryptionMethod { public interface EncryptionMethod {
@ -31,9 +35,9 @@ public interface EncryptionMethod {
/** /**
* Check whether the given hash matches the clear-text password. * Check whether the given hash matches the clear-text password.
* *
* @param password The clear-text password to verify * @param password The clear-text password to verify
* @param hashedPassword The hash to check the password against * @param hashedPassword The hash to check the password against
* @param name The player name to do the check for (sometimes required for generating the salt) * @param name The player name to do the check for (sometimes required for generating the salt)
* *
* @return True if the password matches, false otherwise * @return True if the password matches, false otherwise
*/ */

View File

@ -5,7 +5,8 @@ import fr.xephi.authme.security.crypts.description.HasSalt;
import fr.xephi.authme.security.crypts.description.Recommendation; import fr.xephi.authme.security.crypts.description.Recommendation;
import fr.xephi.authme.security.crypts.description.SaltType; import fr.xephi.authme.security.crypts.description.SaltType;
import fr.xephi.authme.security.crypts.description.Usage; import fr.xephi.authme.security.crypts.description.Usage;
import fr.xephi.authme.settings.Settings; import fr.xephi.authme.settings.NewSetting;
import fr.xephi.authme.settings.properties.SecuritySettings;
import static fr.xephi.authme.security.HashUtils.md5; import static fr.xephi.authme.security.HashUtils.md5;
@ -13,6 +14,12 @@ import static fr.xephi.authme.security.HashUtils.md5;
@HasSalt(value = SaltType.TEXT) // length defined by Settings.saltLength @HasSalt(value = SaltType.TEXT) // length defined by Settings.saltLength
public class SALTED2MD5 extends SeparateSaltMethod { public class SALTED2MD5 extends SeparateSaltMethod {
private final int saltLength;
public SALTED2MD5(NewSetting settings) {
saltLength = settings.getProperty(SecuritySettings.DOUBLE_MD5_SALT_LENGTH);
}
@Override @Override
public String computeHash(String password, String salt, String name) { public String computeHash(String password, String salt, String name) {
return md5(md5(password) + salt); return md5(md5(password) + salt);
@ -20,7 +27,7 @@ public class SALTED2MD5 extends SeparateSaltMethod {
@Override @Override
public String generateSalt() { public String generateSalt() {
return RandomString.generateHex(Settings.saltLength); return RandomString.generateHex(saltLength);
} }
} }

View File

@ -45,8 +45,7 @@ public final class Settings {
rakamakUsers, rakamakUsersIp, defaultWorld, crazyloginFileName; rakamakUsers, rakamakUsersIp, defaultWorld, crazyloginFileName;
public static int getWarnMessageInterval, getSessionTimeout, public static int getWarnMessageInterval, getSessionTimeout,
getRegistrationTimeout, getMaxNickLength, getMinNickLength, getRegistrationTimeout, getMaxNickLength, getMinNickLength,
getNonActivatedGroup, maxLoginTry, captchaLength, saltLength, getNonActivatedGroup, maxLoginTry, captchaLength, getMaxLoginPerIp;
bCryptLog2Rounds, getMaxLoginPerIp;
protected static FileConfiguration configFile; protected static FileConfiguration configFile;
/** /**
@ -108,11 +107,9 @@ public final class Settings {
removePassword = configFile.getBoolean("Security.console.removePassword", true); removePassword = configFile.getBoolean("Security.console.removePassword", true);
maxLoginTry = configFile.getInt("Security.captcha.maxLoginTry", 5); maxLoginTry = configFile.getInt("Security.captcha.maxLoginTry", 5);
captchaLength = configFile.getInt("Security.captcha.captchaLength", 5); captchaLength = configFile.getInt("Security.captcha.captchaLength", 5);
saltLength = configFile.getInt("settings.security.doubleMD5SaltLength", 8);
multiverse = load(HooksSettings.MULTIVERSE); multiverse = load(HooksSettings.MULTIVERSE);
bungee = load(HooksSettings.BUNGEECORD); bungee = load(HooksSettings.BUNGEECORD);
getForcedWorlds = configFile.getStringList("settings.restrictions.ForceSpawnOnTheseWorlds"); getForcedWorlds = configFile.getStringList("settings.restrictions.ForceSpawnOnTheseWorlds");
bCryptLog2Rounds = configFile.getInt("ExternalBoardOptions.bCryptLog2Round", 10);
defaultWorld = configFile.getString("Purge.defaultWorld", "world"); defaultWorld = configFile.getString("Purge.defaultWorld", "world");
enableProtection = configFile.getBoolean("Protection.enableProtection", false); enableProtection = configFile.getBoolean("Protection.enableProtection", false);
countries = configFile.getStringList("Protection.countries"); countries = configFile.getStringList("Protection.countries");

View File

@ -1,10 +1,11 @@
package fr.xephi.authme.security; package fr.xephi.authme.security;
import fr.xephi.authme.security.crypts.HashedPassword;
import fr.xephi.authme.security.crypts.EncryptionMethod; import fr.xephi.authme.security.crypts.EncryptionMethod;
import fr.xephi.authme.settings.Settings; import fr.xephi.authme.security.crypts.HashedPassword;
import fr.xephi.authme.settings.NewSetting;
import fr.xephi.authme.settings.properties.HooksSettings;
import fr.xephi.authme.settings.properties.SecuritySettings;
import fr.xephi.authme.util.StringUtils; import fr.xephi.authme.util.StringUtils;
import fr.xephi.authme.util.WrapperMock;
import org.junit.BeforeClass; import org.junit.BeforeClass;
import org.junit.Test; import org.junit.Test;
@ -12,19 +13,25 @@ import java.util.HashSet;
import java.util.Set; import java.util.Set;
import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.not;
import static org.hamcrest.Matchers.nullValue;
import static org.junit.Assert.assertThat; import static org.junit.Assert.assertThat;
import static org.junit.Assert.fail; import static org.junit.Assert.fail;
import static org.mockito.BDDMockito.given;
import static org.mockito.Mockito.mock;
/** /**
* Integration test for {@link HashAlgorithm}. * Integration test for {@link HashAlgorithm}.
*/ */
public class HashAlgorithmIntegrationTest { public class HashAlgorithmIntegrationTest {
private static NewSetting settings;
@BeforeClass @BeforeClass
public static void setUpWrapper() { public static void setUpWrapper() {
WrapperMock.createInstance(); settings = mock(NewSetting.class);
Settings.bCryptLog2Rounds = 8; given(settings.getProperty(HooksSettings.BCRYPT_LOG2_ROUND)).willReturn(8);
Settings.saltLength = 16; given(settings.getProperty(SecuritySettings.DOUBLE_MD5_SALT_LENGTH)).willReturn(16);
} }
@Test @Test
@ -47,8 +54,10 @@ public class HashAlgorithmIntegrationTest {
public void shouldBeAbleToInstantiateEncryptionAlgorithms() throws InstantiationException, IllegalAccessException { public void shouldBeAbleToInstantiateEncryptionAlgorithms() throws InstantiationException, IllegalAccessException {
// given / when / then // given / when / then
for (HashAlgorithm algorithm : HashAlgorithm.values()) { for (HashAlgorithm algorithm : HashAlgorithm.values()) {
if (!HashAlgorithm.CUSTOM.equals(algorithm)) { if (!HashAlgorithm.CUSTOM.equals(algorithm) && !HashAlgorithm.PLAINTEXT.equals(algorithm)) {
EncryptionMethod method = algorithm.getClazz().newInstance(); EncryptionMethod method = PasswordSecurity.initializeEncryptionMethod(algorithm, settings);
assertThat("Encryption method for algorithm '" + algorithm + "' is not null",
method, not(nullValue()));
HashedPassword hashedPassword = method.computeHash("pwd", "name"); HashedPassword hashedPassword = method.computeHash("pwd", "name");
assertThat("Salt should not be null if method.hasSeparateSalt(), and vice versa. Method: '" assertThat("Salt should not be null if method.hasSeparateSalt(), and vice versa. Method: '"
+ method + "'", StringUtils.isEmpty(hashedPassword.getSalt()), equalTo(!method.hasSeparateSalt())); + method + "'", StringUtils.isEmpty(hashedPassword.getSalt()), equalTo(!method.hasSeparateSalt()));

View File

@ -1,22 +1,25 @@
package fr.xephi.authme.security; package fr.xephi.authme.security;
import fr.xephi.authme.ReflectionTestUtils; import fr.xephi.authme.ReflectionTestUtils;
import fr.xephi.authme.cache.auth.PlayerAuth; import fr.xephi.authme.TestHelper;
import fr.xephi.authme.datasource.DataSource; import fr.xephi.authme.datasource.DataSource;
import fr.xephi.authme.events.PasswordEncryptionEvent; import fr.xephi.authme.events.PasswordEncryptionEvent;
import fr.xephi.authme.security.crypts.HashedPassword;
import fr.xephi.authme.security.crypts.EncryptionMethod; import fr.xephi.authme.security.crypts.EncryptionMethod;
import fr.xephi.authme.security.crypts.HashedPassword;
import fr.xephi.authme.security.crypts.JOOMLA; import fr.xephi.authme.security.crypts.JOOMLA;
import fr.xephi.authme.security.crypts.PHPBB;
import fr.xephi.authme.settings.NewSetting; import fr.xephi.authme.settings.NewSetting;
import fr.xephi.authme.settings.properties.HooksSettings;
import fr.xephi.authme.settings.properties.SecuritySettings; import fr.xephi.authme.settings.properties.SecuritySettings;
import fr.xephi.authme.util.WrapperMock;
import org.bukkit.event.Event; import org.bukkit.event.Event;
import org.bukkit.plugin.PluginManager; import org.bukkit.plugin.PluginManager;
import org.junit.Before; import org.junit.Before;
import org.junit.BeforeClass;
import org.junit.Test; import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.ArgumentCaptor; import org.mockito.ArgumentCaptor;
import org.mockito.Mock;
import org.mockito.invocation.InvocationOnMock; import org.mockito.invocation.InvocationOnMock;
import org.mockito.runners.MockitoJUnitRunner;
import org.mockito.stubbing.Answer; import org.mockito.stubbing.Answer;
import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.equalTo;
@ -35,19 +38,24 @@ import static org.mockito.Mockito.verify;
/** /**
* Test for {@link PasswordSecurity}. * Test for {@link PasswordSecurity}.
*/ */
@RunWith(MockitoJUnitRunner.class)
public class PasswordSecurityTest { public class PasswordSecurityTest {
@Mock
private PluginManager pluginManager; private PluginManager pluginManager;
@Mock
private DataSource dataSource; private DataSource dataSource;
@Mock
private EncryptionMethod method; private EncryptionMethod method;
private Class<?> caughtClassInEvent; private Class<?> caughtClassInEvent;
@BeforeClass
public static void setUpTest() {
TestHelper.setupLogger();
}
@Before @Before
public void setUpMocks() { public void setUpMocks() {
WrapperMock.createInstance();
pluginManager = mock(PluginManager.class);
dataSource = mock(DataSource.class);
method = mock(EncryptionMethod.class);
caughtClassInEvent = null; caughtClassInEvent = null;
// When the password encryption event is emitted, replace the encryption method with our mock. // When the password encryption event is emitted, replace the encryption method with our mock.
@ -97,7 +105,6 @@ public class PasswordSecurityTest {
String playerLowerCase = playerName.toLowerCase(); String playerLowerCase = playerName.toLowerCase();
String clearTextPass = "passw0Rd1"; String clearTextPass = "passw0Rd1";
PlayerAuth auth = mock(PlayerAuth.class);
given(dataSource.getPassword(playerName)).willReturn(password); given(dataSource.getPassword(playerName)).willReturn(password);
given(method.comparePassword(clearTextPass, password, playerLowerCase)).willReturn(false); given(method.comparePassword(clearTextPass, password, playerLowerCase)).willReturn(false);
PasswordSecurity security = PasswordSecurity security =
@ -165,6 +172,25 @@ public class PasswordSecurityTest {
verify(dataSource).updatePassword(playerLowerCase, newPassword); verify(dataSource).updatePassword(playerLowerCase, newPassword);
} }
@Test
public void shouldTryAllMethodsAndFail() {
// given
HashedPassword password = new HashedPassword("hashNotMatchingAnyMethod", "someBogusSalt");
String playerName = "asfd";
String clearTextPass = "someInvalidPassword";
given(dataSource.getPassword(playerName)).willReturn(password);
given(method.comparePassword(clearTextPass, password, playerName)).willReturn(false);
PasswordSecurity security =
new PasswordSecurity(dataSource, mockSettings(HashAlgorithm.MD5, true), pluginManager);
// when
boolean result = security.comparePassword(clearTextPass, playerName);
// then
assertThat(result, equalTo(false));
verify(dataSource, never()).updatePassword(anyString(), any(HashedPassword.class));
}
@Test @Test
public void shouldHashPassword() { public void shouldHashPassword() {
// given // given
@ -188,28 +214,6 @@ public class PasswordSecurityTest {
assertThat(event.getPlayerName(), equalTo(usernameLowerCase)); assertThat(event.getPlayerName(), equalTo(usernameLowerCase));
} }
@Test
public void shouldHashPasswordWithGivenAlgorithm() {
// given
String password = "TopSecretPass#112525";
String username = "someone12";
HashedPassword hashedPassword = new HashedPassword("~T!est#Hash", "__someSalt__");
given(method.computeHash(password, username)).willReturn(hashedPassword);
PasswordSecurity security =
new PasswordSecurity(dataSource, mockSettings(HashAlgorithm.JOOMLA, true), pluginManager);
// when
HashedPassword result = security.computeHash(HashAlgorithm.PHPBB, password, username);
// then
assertThat(result, equalTo(hashedPassword));
ArgumentCaptor<PasswordEncryptionEvent> captor = ArgumentCaptor.forClass(PasswordEncryptionEvent.class);
verify(pluginManager).callEvent(captor.capture());
PasswordEncryptionEvent event = captor.getValue();
assertThat(PHPBB.class.equals(caughtClassInEvent), equalTo(true));
assertThat(event.getPlayerName(), equalTo(username));
}
@Test @Test
public void shouldSkipCheckIfMandatorySaltIsUnavailable() { public void shouldSkipCheckIfMandatorySaltIsUnavailable() {
// given // given
@ -234,12 +238,13 @@ public class PasswordSecurityTest {
@Test @Test
public void shouldReloadSettings() { public void shouldReloadSettings() {
// given // given
PasswordSecurity passwordSecurity = NewSetting settings = mockSettings(HashAlgorithm.BCRYPT, false);
new PasswordSecurity(dataSource, mockSettings(HashAlgorithm.BCRYPT, false), pluginManager); PasswordSecurity passwordSecurity = new PasswordSecurity(dataSource, settings, pluginManager);
NewSetting updatedSettings = mockSettings(HashAlgorithm.MD5, true); given(settings.getProperty(SecuritySettings.PASSWORD_HASH)).willReturn(HashAlgorithm.MD5);
given(settings.getProperty(SecuritySettings.SUPPORT_OLD_PASSWORD_HASH)).willReturn(true);
// when // when
passwordSecurity.reload(updatedSettings); passwordSecurity.reload();
// then // then
assertThat(ReflectionTestUtils.getFieldValue(PasswordSecurity.class, passwordSecurity, "algorithm"), assertThat(ReflectionTestUtils.getFieldValue(PasswordSecurity.class, passwordSecurity, "algorithm"),
@ -252,6 +257,8 @@ public class PasswordSecurityTest {
NewSetting settings = mock(NewSetting.class); NewSetting settings = mock(NewSetting.class);
given(settings.getProperty(SecuritySettings.PASSWORD_HASH)).willReturn(algorithm); given(settings.getProperty(SecuritySettings.PASSWORD_HASH)).willReturn(algorithm);
given(settings.getProperty(SecuritySettings.SUPPORT_OLD_PASSWORD_HASH)).willReturn(supportOldPassword); given(settings.getProperty(SecuritySettings.SUPPORT_OLD_PASSWORD_HASH)).willReturn(supportOldPassword);
given(settings.getProperty(HooksSettings.BCRYPT_LOG2_ROUND)).willReturn(8);
given(settings.getProperty(SecuritySettings.DOUBLE_MD5_SALT_LENGTH)).willReturn(16);
return settings; return settings;
} }

View File

@ -1,24 +1,25 @@
package fr.xephi.authme.security.crypts; package fr.xephi.authme.security.crypts;
import fr.xephi.authme.TestHelper; import fr.xephi.authme.TestHelper;
import fr.xephi.authme.settings.Settings; import fr.xephi.authme.settings.NewSetting;
import fr.xephi.authme.util.WrapperMock; import fr.xephi.authme.settings.properties.HooksSettings;
import org.junit.BeforeClass; import org.junit.BeforeClass;
import static org.mockito.BDDMockito.given;
import static org.mockito.Mockito.mock;
/** /**
* Test for {@link BCRYPT}. * Test for {@link BCRYPT}.
*/ */
public class BcryptTest extends AbstractEncryptionMethodTest { public class BcryptTest extends AbstractEncryptionMethodTest {
@BeforeClass @BeforeClass
public static void setUpSettings() { public static void initializeLogger() {
WrapperMock.createInstance();
Settings.bCryptLog2Rounds = 8;
TestHelper.setupLogger(); TestHelper.setupLogger();
} }
public BcryptTest() { public BcryptTest() {
super(new BCRYPT(), super(new BCRYPT(mockSettings()),
"$2a$10$6iATmYgwJVc3YONhVcZFve3Cfb5GnwvKhJ20r.hMjmcNkIT9.Uh9K", // password "$2a$10$6iATmYgwJVc3YONhVcZFve3Cfb5GnwvKhJ20r.hMjmcNkIT9.Uh9K", // password
"$2a$10$LOhUxhEcS0vgDPv/jkXvCurNb7LjP9xUlEolJGk.Uhgikqc6FtIOi", // PassWord1 "$2a$10$LOhUxhEcS0vgDPv/jkXvCurNb7LjP9xUlEolJGk.Uhgikqc6FtIOi", // PassWord1
"$2a$10$j9da7SGiaakWhzIms9BtwemLUeIhSEphGUQ3XSlvYgpYsGnGCKRBa", // &^%te$t?Pw@_ "$2a$10$j9da7SGiaakWhzIms9BtwemLUeIhSEphGUQ3XSlvYgpYsGnGCKRBa", // &^%te$t?Pw@_
@ -26,4 +27,10 @@ public class BcryptTest extends AbstractEncryptionMethodTest {
); );
} }
private static NewSetting mockSettings() {
NewSetting settings = mock(NewSetting.class);
given(settings.getProperty(HooksSettings.BCRYPT_LOG2_ROUND)).willReturn(8);
return settings;
}
} }

View File

@ -1,7 +1,6 @@
package fr.xephi.authme.security.crypts; package fr.xephi.authme.security.crypts;
import fr.xephi.authme.TestHelper; import fr.xephi.authme.TestHelper;
import fr.xephi.authme.util.WrapperMock;
import org.junit.BeforeClass; import org.junit.BeforeClass;
/** /**
@ -11,7 +10,6 @@ public class IPB4Test extends AbstractEncryptionMethodTest {
@BeforeClass @BeforeClass
public static void setUpSettings() { public static void setUpSettings() {
WrapperMock.createInstance();
TestHelper.setupLogger(); TestHelper.setupLogger();
} }

View File

@ -1,26 +1,28 @@
package fr.xephi.authme.security.crypts; package fr.xephi.authme.security.crypts;
import fr.xephi.authme.settings.Settings; import fr.xephi.authme.settings.NewSetting;
import fr.xephi.authme.util.WrapperMock; import fr.xephi.authme.settings.properties.SecuritySettings;
import org.junit.Before;
import static org.mockito.BDDMockito.given;
import static org.mockito.Mockito.mock;
/** /**
* Test for {@link SALTED2MD5}. * Test for {@link SALTED2MD5}.
*/ */
public class SALTED2MD5Test extends AbstractEncryptionMethodTest { public class SALTED2MD5Test extends AbstractEncryptionMethodTest {
@Before
public void setUpAlgorithm() {
WrapperMock.createInstance();
Settings.saltLength = 8;
}
public SALTED2MD5Test() { public SALTED2MD5Test() {
super(new SALTED2MD5(), super(new SALTED2MD5(mockSettings()),
new HashedPassword("9f3d13dc01a6fe61fd669954174399f3", "9b5f5749"), // password new HashedPassword("9f3d13dc01a6fe61fd669954174399f3", "9b5f5749"), // password
new HashedPassword("b28c32f624a4eb161d6adc9acb5bfc5b", "f750ba32"), // PassWord1 new HashedPassword("b28c32f624a4eb161d6adc9acb5bfc5b", "f750ba32"), // PassWord1
new HashedPassword("38dcb83cc68424afe3cda012700c2bb1", "eb2c3394"), // &^%te$t?Pw@_ new HashedPassword("38dcb83cc68424afe3cda012700c2bb1", "eb2c3394"), // &^%te$t?Pw@_
new HashedPassword("ad25606eae5b760c8a2469d65578ac39", "04eee598")); // âË_3(íù*) new HashedPassword("ad25606eae5b760c8a2469d65578ac39", "04eee598")); // âË_3(íù*)
} }
private static NewSetting mockSettings() {
NewSetting settings = mock(NewSetting.class);
given(settings.getProperty(SecuritySettings.DOUBLE_MD5_SALT_LENGTH)).willReturn(8);
return settings;
}
} }

View File

@ -1,7 +1,6 @@
package fr.xephi.authme.security.crypts; package fr.xephi.authme.security.crypts;
import fr.xephi.authme.TestHelper; import fr.xephi.authme.TestHelper;
import fr.xephi.authme.util.WrapperMock;
import org.junit.BeforeClass; import org.junit.BeforeClass;
/** /**
@ -11,7 +10,6 @@ public class XFBCRYPTTest extends AbstractEncryptionMethodTest {
@BeforeClass @BeforeClass
public static void setup() { public static void setup() {
WrapperMock.createInstance();
TestHelper.setupLogger(); TestHelper.setupLogger();
} }

View File

@ -1,11 +1,16 @@
package hashmethods; package hashmethods;
import fr.xephi.authme.security.HashAlgorithm; import fr.xephi.authme.security.HashAlgorithm;
import fr.xephi.authme.security.PasswordSecurity;
import fr.xephi.authme.security.crypts.EncryptionMethod; import fr.xephi.authme.security.crypts.EncryptionMethod;
import fr.xephi.authme.security.crypts.HexSaltedMethod; import fr.xephi.authme.security.crypts.HexSaltedMethod;
import fr.xephi.authme.security.crypts.description.AsciiRestricted; import fr.xephi.authme.security.crypts.description.AsciiRestricted;
import fr.xephi.authme.security.crypts.description.HasSalt; import fr.xephi.authme.security.crypts.description.HasSalt;
import fr.xephi.authme.security.crypts.description.Recommendation; import fr.xephi.authme.security.crypts.description.Recommendation;
import fr.xephi.authme.settings.NewSetting;
import fr.xephi.authme.settings.properties.HooksSettings;
import fr.xephi.authme.settings.properties.SecuritySettings;
import org.mockito.BDDMockito;
import java.lang.annotation.Annotation; import java.lang.annotation.Annotation;
import java.util.HashMap; import java.util.HashMap;
@ -14,6 +19,7 @@ import java.util.Map;
import java.util.Set; import java.util.Set;
import static com.google.common.collect.Sets.newHashSet; import static com.google.common.collect.Sets.newHashSet;
import static org.mockito.Mockito.mock;
/** /**
* Gathers information on {@link EncryptionMethod} implementations based on * Gathers information on {@link EncryptionMethod} implementations based on
@ -25,6 +31,8 @@ public class EncryptionMethodInfoGatherer {
private final static Set<Class<? extends Annotation>> RELEVANT_ANNOTATIONS = private final static Set<Class<? extends Annotation>> RELEVANT_ANNOTATIONS =
newHashSet(HasSalt.class, Recommendation.class, AsciiRestricted.class); newHashSet(HasSalt.class, Recommendation.class, AsciiRestricted.class);
private static NewSetting settings = createSettings();
private Map<HashAlgorithm, MethodDescription> descriptions; private Map<HashAlgorithm, MethodDescription> descriptions;
public EncryptionMethodInfoGatherer() { public EncryptionMethodInfoGatherer() {
@ -38,16 +46,19 @@ public class EncryptionMethodInfoGatherer {
private void constructDescriptions() { private void constructDescriptions() {
for (HashAlgorithm algorithm : HashAlgorithm.values()) { for (HashAlgorithm algorithm : HashAlgorithm.values()) {
Class<? extends EncryptionMethod> methodClazz = algorithm.getClazz(); if (!HashAlgorithm.CUSTOM.equals(algorithm) && !algorithm.getClazz().isAnnotationPresent(Deprecated.class)) {
if (!HashAlgorithm.CUSTOM.equals(algorithm) && !methodClazz.isAnnotationPresent(Deprecated.class)) { MethodDescription description = createDescription(algorithm);
MethodDescription description = createDescription(methodClazz);
descriptions.put(algorithm, description); descriptions.put(algorithm, description);
} }
} }
} }
private static MethodDescription createDescription(Class<? extends EncryptionMethod> clazz) { private static MethodDescription createDescription(HashAlgorithm algorithm) {
EncryptionMethod method = instantiateMethod(clazz); Class<? extends EncryptionMethod> clazz = algorithm.getClazz();
EncryptionMethod method = PasswordSecurity.initializeEncryptionMethod(algorithm, settings);
if (method == null) {
throw new NullPointerException("Method for '" + algorithm + "' is null");
}
MethodDescription description = new MethodDescription(clazz); MethodDescription description = new MethodDescription(clazz);
description.setHashLength(method.computeHash("test", "user").getHash().length()); description.setHashLength(method.computeHash("test", "user").getHash().length());
description.setHasSeparateSalt(method.hasSeparateSalt()); description.setHasSeparateSalt(method.hasSeparateSalt());
@ -118,18 +129,25 @@ public class EncryptionMethodInfoGatherer {
} }
} }
private static EncryptionMethod instantiateMethod(Class<? extends EncryptionMethod> clazz) {
try {
return clazz.newInstance();
} catch (InstantiationException | IllegalAccessException e) {
throw new RuntimeException("Could not instantiate " + clazz, e);
}
}
// Convenience method for retrieving an annotation in a typed fashion. // Convenience method for retrieving an annotation in a typed fashion.
// We know implicitly that the key of the map always corresponds to the type of the value // We know implicitly that the key of the map always corresponds to the type of the value
private static <T> T returnTyped(Map<Class<?>, Annotation> map, Class<T> key) { private static <T> T returnTyped(Map<Class<?>, Annotation> map, Class<T> key) {
return key.cast(map.get(key)); return key.cast(map.get(key));
} }
private static NewSetting createSettings() {
// TODO #672 Don't mock settings but instantiate a NewSetting object without any validation / migration
NewSetting settings = mock(NewSetting.class);
BDDMockito.given(settings.getProperty(HooksSettings.BCRYPT_LOG2_ROUND)).willReturn(8);
BDDMockito.given(settings.getProperty(SecuritySettings.DOUBLE_MD5_SALT_LENGTH)).willReturn(8);
return settings;
/*try (InputStreamReader isr = new InputStreamReader(getClass().getResourceAsStream("config.yml"))) {
FileConfiguration configuration = YamlConfiguration.loadConfiguration(isr);
return new NewSetting(configuration, null, null, null);
} catch (IOException e) {
throw new UnsupportedOperationException(e);
}*/
}
} }

View File

@ -1,8 +1,6 @@
package hashmethods; package hashmethods;
import fr.xephi.authme.security.HashAlgorithm; import fr.xephi.authme.security.HashAlgorithm;
import fr.xephi.authme.settings.Settings;
import fr.xephi.authme.util.WrapperMock;
import utils.FileUtils; import utils.FileUtils;
import utils.TagValue.NestedTagValue; import utils.TagValue.NestedTagValue;
import utils.TagValueHolder; import utils.TagValueHolder;
@ -24,12 +22,6 @@ public class HashAlgorithmsDescriptionTask implements ToolTask {
@Override @Override
public void execute(Scanner scanner) { public void execute(Scanner scanner) {
// Unfortunately, we need the Wrapper to be around to work with Settings, and certain encryption methods
// directly read from the Settings file
WrapperMock.createInstance();
Settings.bCryptLog2Rounds = 8;
Settings.saltLength = 8;
// Gather info and construct a row for each method // Gather info and construct a row for each method
EncryptionMethodInfoGatherer infoGatherer = new EncryptionMethodInfoGatherer(); EncryptionMethodInfoGatherer infoGatherer = new EncryptionMethodInfoGatherer();
Map<HashAlgorithm, MethodDescription> descriptions = infoGatherer.getDescriptions(); Map<HashAlgorithm, MethodDescription> descriptions = infoGatherer.getDescriptions();