From ac4add9f54a34b2ec9e66ca10d5a5cacf18880ea Mon Sep 17 00:00:00 2001 From: Gnat008 Date: Sun, 12 Jun 2016 13:40:34 -0400 Subject: [PATCH] add ability to tempban users after x wrong logins - ref #520 #192 --- .../fr/xephi/authme/cache/TempbanManager.java | 115 ++++++++++++++++++ .../fr/xephi/authme/output/MessageKey.java | 4 +- .../process/login/AsynchronousLogin.java | 25 +++- .../settings/properties/SecuritySettings.java | 13 ++ src/main/resources/config.yml | 8 ++ src/main/resources/messages/messages_en.yml | 1 + .../authme/cache/TempbanManagerTest.java | 114 +++++++++++++++++ 7 files changed, 277 insertions(+), 3 deletions(-) create mode 100644 src/main/java/fr/xephi/authme/cache/TempbanManager.java create mode 100644 src/test/java/fr/xephi/authme/cache/TempbanManagerTest.java diff --git a/src/main/java/fr/xephi/authme/cache/TempbanManager.java b/src/main/java/fr/xephi/authme/cache/TempbanManager.java new file mode 100644 index 000000000..30f94502e --- /dev/null +++ b/src/main/java/fr/xephi/authme/cache/TempbanManager.java @@ -0,0 +1,115 @@ +package fr.xephi.authme.cache; + +import fr.xephi.authme.initialization.SettingsDependent; +import fr.xephi.authme.output.MessageKey; +import fr.xephi.authme.output.Messages; +import fr.xephi.authme.settings.NewSetting; +import fr.xephi.authme.settings.properties.SecuritySettings; +import fr.xephi.authme.util.BukkitService; +import fr.xephi.authme.util.Utils; +import org.bukkit.BanList; +import org.bukkit.Bukkit; +import org.bukkit.entity.Player; + +import javax.inject.Inject; +import java.util.Date; +import java.util.concurrent.ConcurrentHashMap; + +/** + * Manager for handling tempbans + */ +public class TempbanManager implements SettingsDependent { + + private final ConcurrentHashMap playerCounts; + + private final long MINUTE_IN_MILLISECONDS = 60000; + + @Inject + private BukkitService bukkitService; + + @Inject + private Messages messages; + + private boolean isEnabled; + private int threshold; + private int length; + + @Inject + TempbanManager(NewSetting settings) { + this.playerCounts = new ConcurrentHashMap<>(); + loadSettings(settings); + } + + /** + * Increases the failure count for the given player. + * + * @param name the player's name + */ + public void increaseCount(String name) { + if (isEnabled) { + String nameLower = name.toLowerCase(); + Integer count = playerCounts.get(nameLower); + + if (count == null) { + playerCounts.put(nameLower, 1); + } else { + playerCounts.put(nameLower, count + 1); + } + } + } + + public void resetCount(String name) { + if (isEnabled) { + playerCounts.remove(name.toLowerCase()); + } + } + + /** + * Return whether the player should be tempbanned. + * + * @param name The player's name + * @return True if the player should be tempbanned + */ + public boolean shouldTempban(String name) { + if (isEnabled) { + Integer count = playerCounts.get(name.toLowerCase()); + return count != null && count >= threshold; + } + + return false; + } + + /** + * Tempban a player for failing to log in too many times. + * This bans the player's IP address, and calculates the expire + * time based on the time the method was called. + * + * @param player The player to tempban + */ + public void tempbanPlayer(Player player) { + if (isEnabled) { + resetCount(player.getName()); + + final String ip = Utils.getPlayerIp(player); + final String reason = messages.retrieveSingle(MessageKey.TEMPBAN_MAX_LOGINS); + + final Date expires = new Date(); + long newTime = expires.getTime() + (length * MINUTE_IN_MILLISECONDS); + expires.setTime(newTime); + + bukkitService.scheduleSyncDelayedTask(new Runnable() { + @Override + public void run() { + Bukkit.getServer().getBanList(BanList.Type.IP).addBan(ip, reason, expires, "AuthMe"); + } + }); + } + } + + @Override + public void loadSettings(NewSetting settings) { + this.isEnabled = settings.getProperty(SecuritySettings.TEMPBAN_ON_MAX_LOGINS); + this.threshold = settings.getProperty(SecuritySettings.MAX_LOGIN_TEMPBAN); + this.length = settings.getProperty(SecuritySettings.TEMPBAN_LENGTH); + } +} diff --git a/src/main/java/fr/xephi/authme/output/MessageKey.java b/src/main/java/fr/xephi/authme/output/MessageKey.java index 9cfa960da..7bd276213 100644 --- a/src/main/java/fr/xephi/authme/output/MessageKey.java +++ b/src/main/java/fr/xephi/authme/output/MessageKey.java @@ -137,7 +137,9 @@ public enum MessageKey { NOT_OWNER_ERROR("not_owner_error"), - INVALID_NAME_CASE("invalid_name_case", "%valid", "%invalid"); + INVALID_NAME_CASE("invalid_name_case", "%valid", "%invalid"), + + TEMPBAN_MAX_LOGINS("tempban_max_logins"); private String key; private String[] tags; 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 2e22150ba..5905010ab 100644 --- a/src/main/java/fr/xephi/authme/process/login/AsynchronousLogin.java +++ b/src/main/java/fr/xephi/authme/process/login/AsynchronousLogin.java @@ -3,6 +3,7 @@ package fr.xephi.authme.process.login; import fr.xephi.authme.AuthMe; import fr.xephi.authme.ConsoleLogger; import fr.xephi.authme.cache.CaptchaManager; +import fr.xephi.authme.cache.TempbanManager; import fr.xephi.authme.cache.auth.PlayerAuth; import fr.xephi.authme.cache.auth.PlayerCache; import fr.xephi.authme.cache.limbo.LimboCache; @@ -69,6 +70,9 @@ public class AsynchronousLogin implements AsynchronousProcess { @Inject private CaptchaManager captchaManager; + @Inject + private TempbanManager tempbanManager; + AsynchronousLogin() { } /** @@ -84,6 +88,19 @@ public class AsynchronousLogin implements AsynchronousProcess { return captchaManager.isCaptchaRequired(playerName); } + /** + * Queries the {@link fr.xephi.authme.cache.TempbanManager} to + * see if the player has reached the tempban threshold. + * + * @param player The player to check + * @return True if the player needs to be tempbanned + */ + private boolean shouldTempban(Player player) { + final String playerName = player.getName(); + + return tempbanManager.shouldTempban(playerName); + } + /** * Checks the precondition for authentication (like user known) and returns * the playerAuth-State @@ -146,9 +163,10 @@ public class AsynchronousLogin implements AsynchronousProcess { final String name = player.getName().toLowerCase(); final String ip = Utils.getPlayerIp(player); - // Increase the count here before knowing the result of the login. + // Increase the counts here before knowing the result of the login. // If the login is successful, we clear the count for the player. captchaManager.increaseCount(name); + tempbanManager.increaseCount(name); if ("127.0.0.1".equals(pAuth.getIp()) && !pAuth.getIp().equals(ip)) { pAuth.setIp(ip); @@ -169,6 +187,7 @@ public class AsynchronousLogin implements AsynchronousProcess { database.updateSession(auth); captchaManager.resetCounts(name); + tempbanManager.resetCount(name); player.setNoDamageTicks(0); if (!forceLogin) @@ -214,7 +233,9 @@ public class AsynchronousLogin implements AsynchronousProcess { player.kickPlayer(service.retrieveSingleMessage(MessageKey.WRONG_PASSWORD)); } }); - } else { + } else if (shouldTempban(player)) { + tempbanManager.tempbanPlayer(player); + } else { service.send(player, MessageKey.WRONG_PASSWORD); // Check again if a captcha is required to log in diff --git a/src/main/java/fr/xephi/authme/settings/properties/SecuritySettings.java b/src/main/java/fr/xephi/authme/settings/properties/SecuritySettings.java index 49398d694..365f4a9a5 100644 --- a/src/main/java/fr/xephi/authme/settings/properties/SecuritySettings.java +++ b/src/main/java/fr/xephi/authme/settings/properties/SecuritySettings.java @@ -100,6 +100,19 @@ public class SecuritySettings implements SettingsClass { public static final Property> UNSAFE_PASSWORDS = newLowercaseListProperty("settings.security.unsafePasswords", "123456", "password", "qwerty", "12345", "54321"); + @Comment("Tempban users if they enter the wrong password too many times") + public static final Property TEMPBAN_ON_MAX_LOGINS = + newProperty("Security.tempban.enableTempban", false); + + @Comment("How many times a user can attempt to login before being tempbanned") + public static final Property MAX_LOGIN_TEMPBAN = + newProperty("Security.tempban.maxLoginTries", 10); + + @Comment({"The length of time a player will be tempbanned in minutes", + "Default: 480 minutes, or 8 hours"}) + public static final Property TEMPBAN_LENGTH = + newProperty("Security.tempban.tempbanLength", 480); + private SecuritySettings() { } diff --git a/src/main/resources/config.yml b/src/main/resources/config.yml index 1a9582c2e..e7c67763b 100644 --- a/src/main/resources/config.yml +++ b/src/main/resources/config.yml @@ -331,6 +331,14 @@ Security: # Kick players before stopping the server, that allow us to save position of players, and all needed # information correctly without any corruption. kickPlayersBeforeStopping: true + tempban: + # Tempban users if they enter the wrong password too many times + enableTempban: false + # How many times a user can attempt to login before being tempbanned + maxLoginTries: 10 + # The length of time a player will be tempbanned in minutes + # Default: 480 minutes, or 8 hours + tempbanLength: 480 Converter: Rakamak: # Rakamak file name diff --git a/src/main/resources/messages/messages_en.yml b/src/main/resources/messages/messages_en.yml index 1bb5d3ff6..aabbae891 100644 --- a/src/main/resources/messages/messages_en.yml +++ b/src/main/resources/messages/messages_en.yml @@ -65,3 +65,4 @@ email_already_used: '&4The email address is already being used' two_factor_create: '&2Your secret code is %code. You can scan it from here %url' not_owner_error: 'You are not the owner of this account. Please choose another name!' invalid_name_case: 'You should join using username %valid, not %invalid.' +tempban_max_logins: '&cYou have been temporarily banned for failing to log in too many times.' diff --git a/src/test/java/fr/xephi/authme/cache/TempbanManagerTest.java b/src/test/java/fr/xephi/authme/cache/TempbanManagerTest.java new file mode 100644 index 000000000..ab6fd50d6 --- /dev/null +++ b/src/test/java/fr/xephi/authme/cache/TempbanManagerTest.java @@ -0,0 +1,114 @@ +package fr.xephi.authme.cache; + +import fr.xephi.authme.ReflectionTestUtils; +import fr.xephi.authme.settings.NewSetting; +import fr.xephi.authme.settings.properties.SecuritySettings; +import org.junit.Test; + +import java.util.Map; + +import static org.hamcrest.Matchers.equalTo; +import static org.junit.Assert.assertThat; +import static org.mockito.BDDMockito.given; +import static org.mockito.Mockito.mock; + +/** + * Test for {@link TempbanManager}. + */ +public class TempbanManagerTest { + + @Test + public void shouldAddCounts() { + // given + NewSetting settings = mockSettings(3, 60); + TempbanManager manager = new TempbanManager(settings); + String player = "Tester"; + + // when + for (int i = 0; i < 2; ++i) { + manager.increaseCount(player); + } + + // then + assertThat(manager.shouldTempban(player), equalTo(false)); + manager.increaseCount(player); + assertThat(manager.shouldTempban(player), equalTo(true)); + assertThat(manager.shouldTempban("otherPlayer"), equalTo(false)); + } + + @Test + public void shouldIncreaseAndResetCount() { + // given + String player = "plaYah"; + NewSetting settings = mockSettings(3, 60); + TempbanManager manager = new TempbanManager(settings); + + // when + manager.increaseCount(player); + manager.increaseCount(player); + manager.increaseCount(player); + + // then + assertThat(manager.shouldTempban(player), equalTo(true)); + assertHasCount(manager, player, 3); + + // when 2 + manager.resetCount(player); + + // then 2 + assertThat(manager.shouldTempban(player), equalTo(false)); + assertHasCount(manager, player, null); + } + + @Test + public void shouldNotIncreaseCountForDisabledTempban() { + // given + String player = "playah"; + NewSetting settings = mockSettings(1, 5); + given(settings.getProperty(SecuritySettings.TEMPBAN_ON_MAX_LOGINS)).willReturn(false); + TempbanManager manager = new TempbanManager(settings); + + // when + manager.increaseCount(player); + + // then + assertThat(manager.shouldTempban(player), equalTo(false)); + assertHasCount(manager, player, null); + } + + @Test + public void shouldNotCheckCountIfTempbanIsDisabled() { + // given + String player = "playah"; + NewSetting settings = mockSettings(1, 5); + TempbanManager manager = new TempbanManager(settings); + given(settings.getProperty(SecuritySettings.TEMPBAN_ON_MAX_LOGINS)).willReturn(false); + + // when + manager.increaseCount(player); + // assumptions + assertThat(manager.shouldTempban(player), equalTo(true)); + assertHasCount(manager, player, 1); + // end assumptions + manager.loadSettings(settings); + boolean result = manager.shouldTempban(player); + + // then + assertThat(result, equalTo(false)); + } + + private static NewSetting mockSettings(int maxTries, int tempbanLength) { + NewSetting settings = mock(NewSetting.class); + given(settings.getProperty(SecuritySettings.TEMPBAN_ON_MAX_LOGINS)).willReturn(true); + given(settings.getProperty(SecuritySettings.MAX_LOGIN_TEMPBAN)).willReturn(maxTries); + given(settings.getProperty(SecuritySettings.TEMPBAN_LENGTH)).willReturn(tempbanLength); + return settings; + } + + private static void assertHasCount(TempbanManager manager, String player, Integer count) { + @SuppressWarnings("unchecked") + Map playerCounts = (Map) ReflectionTestUtils + .getFieldValue(TempbanManager.class, manager, "playerCounts"); + assertThat(playerCounts.get(player.toLowerCase()), equalTo(count)); + } +}