diff --git a/src/main/java/org/whispersystems/textsecuregcm/configuration/RateLimitsConfiguration.java b/src/main/java/org/whispersystems/textsecuregcm/configuration/RateLimitsConfiguration.java index 9ef955a58..eaa79a624 100644 --- a/src/main/java/org/whispersystems/textsecuregcm/configuration/RateLimitsConfiguration.java +++ b/src/main/java/org/whispersystems/textsecuregcm/configuration/RateLimitsConfiguration.java @@ -35,6 +35,9 @@ public class RateLimitsConfiguration { @JsonProperty private RateLimitConfiguration smsVoicePrefix = new RateLimitConfiguration(1000, 1000); + @JsonProperty + private RateLimitConfiguration autoBlock = new RateLimitConfiguration(500, 500); + @JsonProperty private RateLimitConfiguration verifyNumber = new RateLimitConfiguration(2, 2); @@ -65,6 +68,10 @@ public class RateLimitsConfiguration { @JsonProperty private RateLimitConfiguration profile = new RateLimitConfiguration(4320, 3); + public RateLimitConfiguration getAutoBlock() { + return autoBlock; + } + public RateLimitConfiguration getAllocateDevice() { return allocateDevice; } diff --git a/src/main/java/org/whispersystems/textsecuregcm/controllers/AccountController.java b/src/main/java/org/whispersystems/textsecuregcm/controllers/AccountController.java index 4cee242f7..281deffb7 100644 --- a/src/main/java/org/whispersystems/textsecuregcm/controllers/AccountController.java +++ b/src/main/java/org/whispersystems/textsecuregcm/controllers/AccountController.java @@ -71,7 +71,6 @@ import java.util.List; import java.util.Map; import java.util.Optional; import java.util.concurrent.TimeUnit; -import java.util.stream.Collectors; import static com.codahale.metrics.MetricRegistry.name; import io.dropwizard.auth.Auth; @@ -141,14 +140,19 @@ public class AccountController { throw new WebApplicationException(Response.status(400).build()); } - List requesters = Arrays.stream(forwardedFor.split(",")).map(String::trim).collect(Collectors.toList()); + String requester = Arrays.stream(forwardedFor.split(",")) + .map(String::trim) + .reduce((a, b) -> b) + .orElseThrow(); - if (requesters.size() > 10) { - logger.info("Request with more than 10 hops: " + transport + ", " + number + ", " + forwardedFor); - return Response.status(400).build(); - } + CaptchaRequirement requirement = requiresCaptcha(number, transport, forwardedFor, requester, captcha); + + if (requirement.isCaptchaRequired()) { + if (requirement.isAutoBlock() && shouldAutoBlock(requester)) { + logger.info("Auto-block: " + requester); + abusiveHostRules.setBlockedHost(requester, "Auto-Block"); + } - if (requiresCaptcha(number, transport, forwardedFor, requesters, captcha)) { return Response.status(402).build(); } @@ -385,54 +389,63 @@ public class AccountController { accounts.update(account); } - private boolean requiresCaptcha(String number, String transport, String forwardedFor, - List requesters, Optional captchaToken) + private CaptchaRequirement requiresCaptcha(String number, String transport, String forwardedFor, + String requester, Optional captchaToken) { + if (captchaToken.isPresent()) { boolean validToken = recaptchaClient.verify(captchaToken.get()); if (validToken) { captchaSuccessMeter.mark(); - return false; + return new CaptchaRequirement(false, false); } else { captchaFailureMeter.mark(); - return true; + return new CaptchaRequirement(true, false); } } - for (String requester : requesters) { - List abuseRules = abusiveHostRules.getAbusiveHostRulesFor(requester); + List abuseRules = abusiveHostRules.getAbusiveHostRulesFor(requester); - for (AbusiveHostRule abuseRule : abuseRules) { - if (abuseRule.isBlocked()) { - logger.info("Blocked host: " + transport + ", " + number + ", " + requester + " (" + forwardedFor + ")"); - blockedHostMeter.mark(); - return true; - } - - if (!abuseRule.getRegions().isEmpty()) { - if (abuseRule.getRegions().stream().noneMatch(number::startsWith)) { - logger.info("Restricted host: " + transport + ", " + number + ", " + requester + " (" + forwardedFor + ")"); - filteredHostMeter.mark(); - return true; - } - } + for (AbusiveHostRule abuseRule : abuseRules) { + if (abuseRule.isBlocked()) { + logger.info("Blocked host: " + transport + ", " + number + ", " + requester + " (" + forwardedFor + ")"); + blockedHostMeter.mark(); + return new CaptchaRequirement(true, false); } - try { - rateLimiters.getSmsVoiceIpLimiter().validate(requester); - } catch (RateLimitExceededException e) { - logger.info("Rate limited exceeded: " + transport + ", " + number + ", " + requester + " (" + forwardedFor + ")"); - rateLimitedPrefixMeter.mark(); - return true; + if (!abuseRule.getRegions().isEmpty()) { + if (abuseRule.getRegions().stream().noneMatch(number::startsWith)) { + logger.info("Restricted host: " + transport + ", " + number + ", " + requester + " (" + forwardedFor + ")"); + filteredHostMeter.mark(); + return new CaptchaRequirement(true, false); + } } } + try { + rateLimiters.getSmsVoiceIpLimiter().validate(requester); + } catch (RateLimitExceededException e) { + logger.info("Rate limited exceeded: " + transport + ", " + number + ", " + requester + " (" + forwardedFor + ")"); + rateLimitedHostMeter.mark(); + return new CaptchaRequirement(true, true); + } + try { rateLimiters.getSmsVoicePrefixLimiter().validate(Util.getNumberPrefix(number)); } catch (RateLimitExceededException e) { - logger.info("Prefix rate limit exceeded: " + transport + ", " + number + ", (" + String.join(", ", requesters) + ")"); + logger.info("Prefix rate limit exceeded: " + transport + ", " + number + ", (" + forwardedFor + ")"); rateLimitedPrefixMeter.mark(); + return new CaptchaRequirement(true, true); + } + + return new CaptchaRequirement(false, false); + } + + private boolean shouldAutoBlock(String requester) { + try { + rateLimiters.getAutoBlockLimiter().validate(requester); + } catch (RateLimitExceededException e) { return true; } @@ -482,4 +495,22 @@ public class AccountController { int randomInt = 100000 + random.nextInt(900000); return new VerificationCode(randomInt); } + + private static class CaptchaRequirement { + private final boolean captchaRequired; + private final boolean autoBlock; + + private CaptchaRequirement(boolean captchaRequired, boolean autoBlock) { + this.captchaRequired = captchaRequired; + this.autoBlock = autoBlock; + } + + boolean isCaptchaRequired() { + return captchaRequired; + } + + boolean isAutoBlock() { + return autoBlock; + } + } } diff --git a/src/main/java/org/whispersystems/textsecuregcm/limits/RateLimiters.java b/src/main/java/org/whispersystems/textsecuregcm/limits/RateLimiters.java index 2f68531ac..6bee0d934 100644 --- a/src/main/java/org/whispersystems/textsecuregcm/limits/RateLimiters.java +++ b/src/main/java/org/whispersystems/textsecuregcm/limits/RateLimiters.java @@ -27,6 +27,7 @@ public class RateLimiters { private final RateLimiter voiceDestinationDailyLimiter; private final RateLimiter smsVoiceIpLimiter; private final RateLimiter smsVoicePrefixLimiter; + private final RateLimiter autoBlockLimiter; private final RateLimiter verifyLimiter; private final RateLimiter pinLimiter; @@ -63,6 +64,10 @@ public class RateLimiters { config.getSmsVoicePrefix().getBucketSize(), config.getSmsVoicePrefix().getLeakRatePerMinute()); + this.autoBlockLimiter = new RateLimiter(cacheClient, "autoBlock", + config.getAutoBlock().getBucketSize(), + config.getAutoBlock().getLeakRatePerMinute()); + this.verifyLimiter = new RateLimiter(cacheClient, "verify", config.getVerifyNumber().getBucketSize(), config.getVerifyNumber().getLeakRatePerMinute()); @@ -140,6 +145,10 @@ public class RateLimiters { return smsVoicePrefixLimiter; } + public RateLimiter getAutoBlockLimiter() { + return autoBlockLimiter; + } + public RateLimiter getVoiceDestinationLimiter() { return voiceDestinationLimiter; } diff --git a/src/main/java/org/whispersystems/textsecuregcm/storage/AbusiveHostRules.java b/src/main/java/org/whispersystems/textsecuregcm/storage/AbusiveHostRules.java index 6de1f7b7d..dde31ee1d 100644 --- a/src/main/java/org/whispersystems/textsecuregcm/storage/AbusiveHostRules.java +++ b/src/main/java/org/whispersystems/textsecuregcm/storage/AbusiveHostRules.java @@ -16,9 +16,11 @@ public class AbusiveHostRules { public static final String HOST = "host"; public static final String BLOCKED = "blocked"; public static final String REGIONS = "regions"; + public static final String NOTES = "notes"; private final MetricRegistry metricRegistry = SharedMetricRegistries.getOrCreate(Constants.METRICS_NAME); private final Timer getTimer = metricRegistry.timer(name(AbusiveHostRules.class, "get")); + private final Timer insertTimer = metricRegistry.timer(name(AbusiveHostRules.class, "setBlockedHost")); private final FaultTolerantDatabase database; @@ -38,4 +40,16 @@ public class AbusiveHostRules { })); } + public void setBlockedHost(String host, String notes) { + database.use(jdbi -> jdbi.useHandle(handle -> { + try (Timer.Context timer = insertTimer.time()) { + handle.createUpdate("INSERT INTO abusive_host_rules(host, blocked, notes) VALUES(:host::inet, :blocked, :notes) ON CONFLICT DO NOTHING") + .bind("host", host) + .bind("blocked", 1) + .bind("notes", notes) + .execute(); + } + })); + } + } diff --git a/src/test/java/org/whispersystems/textsecuregcm/tests/controllers/AccountControllerTest.java b/src/test/java/org/whispersystems/textsecuregcm/tests/controllers/AccountControllerTest.java index 33b0d8325..41e2a9255 100644 --- a/src/test/java/org/whispersystems/textsecuregcm/tests/controllers/AccountControllerTest.java +++ b/src/test/java/org/whispersystems/textsecuregcm/tests/controllers/AccountControllerTest.java @@ -44,14 +44,18 @@ import static org.mockito.Mockito.*; public class AccountControllerTest { - private static final String SENDER = "+14152222222"; - private static final String SENDER_OLD = "+14151111111"; - private static final String SENDER_PIN = "+14153333333"; - private static final String SENDER_OVER_PIN = "+14154444444"; + private static final String SENDER = "+14152222222"; + private static final String SENDER_OLD = "+14151111111"; + private static final String SENDER_PIN = "+14153333333"; + private static final String SENDER_OVER_PIN = "+14154444444"; + private static final String SENDER_OVER_PREFIX = "+14156666666"; - private static final String ABUSIVE_HOST = "192.168.1.1"; - private static final String RESTRICTED_HOST = "192.168.1.2"; - private static final String NICE_HOST = "127.0.0.1"; + private static final String ABUSIVE_HOST = "192.168.1.1"; + private static final String RESTRICTED_HOST = "192.168.1.2"; + private static final String NICE_HOST = "127.0.0.1"; + private static final String RATE_LIMITED_IP_HOST = "10.0.0.1"; + private static final String RATE_LIMITED_PREFIX_HOST = "10.0.0.2"; + private static final String RATE_LIMITED_HOST2 = "10.0.0.3"; private static final String VALID_CAPTCHA_TOKEN = "valid_token"; private static final String INVALID_CAPTCHA_TOKEN = "invalid_token"; @@ -64,6 +68,7 @@ public class AccountControllerTest { private RateLimiter pinLimiter = mock(RateLimiter.class ); private RateLimiter smsVoiceIpLimiter = mock(RateLimiter.class ); private RateLimiter smsVoicePrefixLimiter = mock(RateLimiter.class); + private RateLimiter autoBlockLimiter = mock(RateLimiter.class); private SmsSender smsSender = mock(SmsSender.class ); private DirectoryQueue directoryQueue = mock(DirectoryQueue.class); private MessagesManager storedMessages = mock(MessagesManager.class ); @@ -100,6 +105,7 @@ public class AccountControllerTest { when(rateLimiters.getPinLimiter()).thenReturn(pinLimiter); when(rateLimiters.getSmsVoiceIpLimiter()).thenReturn(smsVoiceIpLimiter); when(rateLimiters.getSmsVoicePrefixLimiter()).thenReturn(smsVoicePrefixLimiter); + when(rateLimiters.getAutoBlockLimiter()).thenReturn(autoBlockLimiter); when(timeProvider.getCurrentTimeMillis()).thenReturn(System.currentTimeMillis()); @@ -124,6 +130,13 @@ public class AccountControllerTest { when(recaptchaClient.verify(eq(VALID_CAPTCHA_TOKEN))).thenReturn(true); doThrow(new RateLimitExceededException(SENDER_OVER_PIN)).when(pinLimiter).validate(eq(SENDER_OVER_PIN)); + + doThrow(new RateLimitExceededException(RATE_LIMITED_PREFIX_HOST)).when(autoBlockLimiter).validate(eq(RATE_LIMITED_PREFIX_HOST)); + doThrow(new RateLimitExceededException(RATE_LIMITED_IP_HOST)).when(autoBlockLimiter).validate(eq(RATE_LIMITED_IP_HOST)); + + doThrow(new RateLimitExceededException(SENDER_OVER_PREFIX)).when(smsVoicePrefixLimiter).validate(SENDER_OVER_PREFIX.substring(0, 4+2)); + doThrow(new RateLimitExceededException(RATE_LIMITED_IP_HOST)).when(smsVoiceIpLimiter).validate(RATE_LIMITED_IP_HOST); + doThrow(new RateLimitExceededException(RATE_LIMITED_HOST2)).when(smsVoiceIpLimiter).validate(RATE_LIMITED_HOST2); } @Test @@ -220,6 +233,63 @@ public class AccountControllerTest { verifyNoMoreInteractions(smsSender); } + @Test + public void testSendRateLimitedHostAutoBlock() { + Response response = + resources.getJerseyTest() + .target(String.format("/v1/accounts/sms/code/%s", SENDER)) + .request() + .header("X-Forwarded-For", RATE_LIMITED_IP_HOST) + .get(); + + assertThat(response.getStatus()).isEqualTo(402); + + verify(abusiveHostRules).getAbusiveHostRulesFor(eq(RATE_LIMITED_IP_HOST)); + verify(abusiveHostRules).setBlockedHost(eq(RATE_LIMITED_IP_HOST), eq("Auto-Block")); + verifyNoMoreInteractions(abusiveHostRules); + + verifyNoMoreInteractions(recaptchaClient); + verifyNoMoreInteractions(smsSender); + } + + @Test + public void testSendRateLimitedPrefixAutoBlock() { + Response response = + resources.getJerseyTest() + .target(String.format("/v1/accounts/sms/code/%s", SENDER_OVER_PREFIX)) + .request() + .header("X-Forwarded-For", RATE_LIMITED_PREFIX_HOST) + .get(); + + assertThat(response.getStatus()).isEqualTo(402); + + verify(abusiveHostRules).getAbusiveHostRulesFor(eq(RATE_LIMITED_PREFIX_HOST)); + verify(abusiveHostRules).setBlockedHost(eq(RATE_LIMITED_PREFIX_HOST), eq("Auto-Block")); + verifyNoMoreInteractions(abusiveHostRules); + + verifyNoMoreInteractions(recaptchaClient); + verifyNoMoreInteractions(smsSender); + } + + @Test + public void testSendRateLimitedHostNoAutoBlock() { + Response response = + resources.getJerseyTest() + .target(String.format("/v1/accounts/sms/code/%s", SENDER)) + .request() + .header("X-Forwarded-For", RATE_LIMITED_HOST2) + .get(); + + assertThat(response.getStatus()).isEqualTo(402); + + verify(abusiveHostRules).getAbusiveHostRulesFor(eq(RATE_LIMITED_HOST2)); + verifyNoMoreInteractions(abusiveHostRules); + + verifyNoMoreInteractions(recaptchaClient); + verifyNoMoreInteractions(smsSender); + } + + @Test public void testSendMultipleHost() { Response response = @@ -232,7 +302,6 @@ public class AccountControllerTest { assertThat(response.getStatus()).isEqualTo(402); verify(abusiveHostRules, times(1)).getAbusiveHostRulesFor(eq(ABUSIVE_HOST)); - verify(abusiveHostRules, times(1)).getAbusiveHostRulesFor(eq(NICE_HOST)); verifyNoMoreInteractions(abusiveHostRules); verifyNoMoreInteractions(smsSender); diff --git a/src/test/java/org/whispersystems/textsecuregcm/tests/storage/AbusiveHostRulesTest.java b/src/test/java/org/whispersystems/textsecuregcm/tests/storage/AbusiveHostRulesTest.java index 36fce342c..5afdff17f 100644 --- a/src/test/java/org/whispersystems/textsecuregcm/tests/storage/AbusiveHostRulesTest.java +++ b/src/test/java/org/whispersystems/textsecuregcm/tests/storage/AbusiveHostRulesTest.java @@ -13,6 +13,7 @@ import org.whispersystems.textsecuregcm.storage.AbusiveHostRules; import org.whispersystems.textsecuregcm.storage.FaultTolerantDatabase; import java.sql.PreparedStatement; +import java.sql.ResultSet; import java.sql.SQLException; import java.util.Arrays; import java.util.List; @@ -84,4 +85,34 @@ public class AbusiveHostRulesTest { assertThat(rules.get(0).getRegions()).isEqualTo(Arrays.asList("+1", "+49")); } + @Test + public void testInsertBlocked() throws Exception { + abusiveHostRules.setBlockedHost("172.17.0.1", "Testing one two"); + + PreparedStatement statement = db.getTestDatabase().getConnection().prepareStatement("SELECT * from abusive_host_rules WHERE host = ?::inet"); + statement.setString(1, "172.17.0.1"); + + ResultSet resultSet = statement.executeQuery(); + + assertThat(resultSet.next()).isTrue(); + + assertThat(resultSet.getInt("blocked")).isEqualTo(1); + assertThat(resultSet.getString("regions")).isNullOrEmpty(); + assertThat(resultSet.getString("notes")).isEqualTo("Testing one two"); + + abusiveHostRules.setBlockedHost("172.17.0.1", "Different notes"); + + + statement = db.getTestDatabase().getConnection().prepareStatement("SELECT * from abusive_host_rules WHERE host = ?::inet"); + statement.setString(1, "172.17.0.1"); + + resultSet = statement.executeQuery(); + + assertThat(resultSet.next()).isTrue(); + + assertThat(resultSet.getInt("blocked")).isEqualTo(1); + assertThat(resultSet.getString("regions")).isNullOrEmpty(); + assertThat(resultSet.getString("notes")).isEqualTo("Testing one two"); + } + }