From 999932140084c3cfca882dfee0f3141d27c9f0e5 Mon Sep 17 00:00:00 2001 From: Moxie Marlinspike Date: Fri, 29 Mar 2019 11:59:37 -0700 Subject: [PATCH] Support for registration recaptcha --- .../WhisperServerConfiguration.java | 8 ++ .../textsecuregcm/WhisperServerService.java | 4 +- .../configuration/RecaptchaConfiguration.java | 16 ++++ .../controllers/AccountController.java | 93 ++++++++++++------- .../recaptcha/RecaptchaClient.java | 56 +++++++++++ .../controllers/AccountControllerTest.java | 53 ++++++++++- 6 files changed, 192 insertions(+), 38 deletions(-) create mode 100644 src/main/java/org/whispersystems/textsecuregcm/configuration/RecaptchaConfiguration.java create mode 100644 src/main/java/org/whispersystems/textsecuregcm/recaptcha/RecaptchaClient.java diff --git a/src/main/java/org/whispersystems/textsecuregcm/WhisperServerConfiguration.java b/src/main/java/org/whispersystems/textsecuregcm/WhisperServerConfiguration.java index 762359616..6e926cd23 100644 --- a/src/main/java/org/whispersystems/textsecuregcm/WhisperServerConfiguration.java +++ b/src/main/java/org/whispersystems/textsecuregcm/WhisperServerConfiguration.java @@ -146,8 +146,16 @@ public class WhisperServerConfiguration extends Configuration { @JsonProperty private VoiceVerificationConfiguration voiceVerification; + @Valid + @NotNull + @JsonProperty + private RecaptchaConfiguration recaptcha; + private Map transparentDataIndex = new HashMap<>(); + public RecaptchaConfiguration getRecaptchaConfiguration() { + return recaptcha; + } public VoiceVerificationConfiguration getVoiceVerificationConfiguration() { return voiceVerification; diff --git a/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java b/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java index f34079fe8..150194acd 100644 --- a/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java +++ b/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java @@ -59,6 +59,7 @@ import org.whispersystems.textsecuregcm.push.GCMSender; import org.whispersystems.textsecuregcm.push.PushSender; import org.whispersystems.textsecuregcm.push.ReceiptSender; import org.whispersystems.textsecuregcm.push.WebsocketSender; +import org.whispersystems.textsecuregcm.recaptcha.RecaptchaClient; import org.whispersystems.textsecuregcm.redis.ReplicatedJedisPool; import org.whispersystems.textsecuregcm.s3.UrlSigner; import org.whispersystems.textsecuregcm.sms.SmsSender; @@ -192,6 +193,7 @@ public class WhisperServerService extends Application(Account.class)); - environment.jersey().register(new AccountController(pendingAccountsManager, accountsManager, abusiveHostRules, rateLimiters, smsSender, directoryQueue, messagesManager, turnTokenGenerator, config.getTestDevices())); + environment.jersey().register(new AccountController(pendingAccountsManager, accountsManager, abusiveHostRules, rateLimiters, smsSender, directoryQueue, messagesManager, turnTokenGenerator, config.getTestDevices(), recaptchaClient)); environment.jersey().register(new DeviceController(pendingDevicesManager, accountsManager, messagesManager, directoryQueue, rateLimiters, config.getMaxDevices())); environment.jersey().register(new DirectoryController(rateLimiters, directory, directoryCredentialsGenerator)); environment.jersey().register(new ProvisioningController(rateLimiters, pushSender)); diff --git a/src/main/java/org/whispersystems/textsecuregcm/configuration/RecaptchaConfiguration.java b/src/main/java/org/whispersystems/textsecuregcm/configuration/RecaptchaConfiguration.java new file mode 100644 index 000000000..8bbbcbfb3 --- /dev/null +++ b/src/main/java/org/whispersystems/textsecuregcm/configuration/RecaptchaConfiguration.java @@ -0,0 +1,16 @@ +package org.whispersystems.textsecuregcm.configuration; + +import com.fasterxml.jackson.annotation.JsonProperty; +import org.hibernate.validator.constraints.NotEmpty; + +public class RecaptchaConfiguration { + + @JsonProperty + @NotEmpty + private String secret; + + public String getSecret() { + return secret; + } + +} diff --git a/src/main/java/org/whispersystems/textsecuregcm/controllers/AccountController.java b/src/main/java/org/whispersystems/textsecuregcm/controllers/AccountController.java index 14d17bc1a..0da089e93 100644 --- a/src/main/java/org/whispersystems/textsecuregcm/controllers/AccountController.java +++ b/src/main/java/org/whispersystems/textsecuregcm/controllers/AccountController.java @@ -36,6 +36,7 @@ import org.whispersystems.textsecuregcm.entities.GcmRegistrationId; import org.whispersystems.textsecuregcm.entities.RegistrationLock; import org.whispersystems.textsecuregcm.entities.RegistrationLockFailure; import org.whispersystems.textsecuregcm.limits.RateLimiters; +import org.whispersystems.textsecuregcm.recaptcha.RecaptchaClient; import org.whispersystems.textsecuregcm.sms.SmsSender; import org.whispersystems.textsecuregcm.sqs.DirectoryQueue; import org.whispersystems.textsecuregcm.storage.AbusiveHostRule; @@ -79,11 +80,13 @@ import io.dropwizard.auth.Auth; @Path("/v1/accounts") public class AccountController { - private final Logger logger = LoggerFactory.getLogger(AccountController.class); - private final MetricRegistry metricRegistry = SharedMetricRegistries.getOrCreate(Constants.METRICS_NAME); - private final Meter newUserMeter = metricRegistry.meter(name(AccountController.class, "brand_new_user")); - private final Meter blockedHostMeter = metricRegistry.meter(name(AccountController.class, "blocked_host")); - private final Meter filteredHostMeter = metricRegistry.meter(name(AccountController.class, "filtered_host")); + private final Logger logger = LoggerFactory.getLogger(AccountController.class); + private final MetricRegistry metricRegistry = SharedMetricRegistries.getOrCreate(Constants.METRICS_NAME); + private final Meter newUserMeter = metricRegistry.meter(name(AccountController.class, "brand_new_user" )); + private final Meter blockedHostMeter = metricRegistry.meter(name(AccountController.class, "blocked_host" )); + private final Meter filteredHostMeter = metricRegistry.meter(name(AccountController.class, "filtered_host" )); + private final Meter captchaSuccessMeter = metricRegistry.meter(name(AccountController.class, "captcha_success")); + private final Meter captchaFailureMeter = metricRegistry.meter(name(AccountController.class, "captcha_failure")); private final PendingAccountsManager pendingAccounts; private final AccountsManager accounts; @@ -94,6 +97,7 @@ public class AccountController { private final MessagesManager messagesManager; private final TurnTokenGenerator turnTokenGenerator; private final Map testDevices; + private final RecaptchaClient recaptchaClient; public AccountController(PendingAccountsManager pendingAccounts, AccountsManager accounts, @@ -103,7 +107,8 @@ public class AccountController { DirectoryQueue directoryQueue, MessagesManager messagesManager, TurnTokenGenerator turnTokenGenerator, - Map testDevices) + Map testDevices, + RecaptchaClient recaptchaClient) { this.pendingAccounts = pendingAccounts; this.accounts = accounts; @@ -114,6 +119,7 @@ public class AccountController { this.messagesManager = messagesManager; this.testDevices = testDevices; this.turnTokenGenerator = turnTokenGenerator; + this.recaptchaClient = recaptchaClient; } @Timed @@ -123,7 +129,8 @@ public class AccountController { @PathParam("number") String number, @HeaderParam("X-Forwarded-For") String forwardedFor, @HeaderParam("Accept-Language") Optional locale, - @QueryParam("client") Optional client) + @QueryParam("client") Optional client, + @QueryParam("captcha") Optional captcha) throws IOException, RateLimitExceededException { if (!Util.isValidNumber(number)) { @@ -138,31 +145,8 @@ public class AccountController { return Response.status(400).build(); } - for (String requester : requesters) { - List abuseRules = abusiveHostRules.getAbusiveHostRulesFor(requester); - - for (AbusiveHostRule abuseRule : abuseRules) { - if (abuseRule.isBlocked()) { - logger.info("Blocked host: " + transport + ", " + number + ", " + requester + " (" + forwardedFor + ")"); - blockedHostMeter.mark(); - return Response.ok().build(); - } - - if (!abuseRule.getRegions().isEmpty()) { - if (abuseRule.getRegions().stream().noneMatch(number::startsWith)) { - logger.info("Restricted host: " + transport + ", " + number + ", " + requester + " (" + forwardedFor + ")"); - filteredHostMeter.mark(); - return Response.ok().build(); - } - } - } - - try { - rateLimiters.getSmsVoiceIpLimiter().validate(requester); - } catch (RateLimitExceededException e) { - logger.info("Rate limited exceeded: " + transport + ", " + number + ", " + requester + " (" + forwardedFor + ")"); - return Response.ok().build(); - } + if (requiresCaptcha(number, transport, forwardedFor, requesters, captcha)) { + return Response.status(402).build(); } switch (transport) { @@ -398,6 +382,51 @@ public class AccountController { accounts.update(account); } + private boolean requiresCaptcha(String number, String transport, String forwardedFor, + List requesters, Optional captchaToken) + { + if (captchaToken.isPresent()) { + boolean validToken = recaptchaClient.verify(captchaToken.get()); + + if (validToken) { + captchaSuccessMeter.mark(); + return false; + } else { + captchaFailureMeter.mark(); + return true; + } + } + + for (String requester : requesters) { + 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; + } + } + } + + try { + rateLimiters.getSmsVoiceIpLimiter().validate(requester); + } catch (RateLimitExceededException e) { + logger.info("Rate limited exceeded: " + transport + ", " + number + ", " + requester + " (" + forwardedFor + ")"); + return true; + } + } + + return false; + } + private void createAccount(String number, String password, String userAgent, AccountAttributes accountAttributes) { Device device = new Device(); device.setId(Device.MASTER_ID); diff --git a/src/main/java/org/whispersystems/textsecuregcm/recaptcha/RecaptchaClient.java b/src/main/java/org/whispersystems/textsecuregcm/recaptcha/RecaptchaClient.java new file mode 100644 index 000000000..8bafd4ade --- /dev/null +++ b/src/main/java/org/whispersystems/textsecuregcm/recaptcha/RecaptchaClient.java @@ -0,0 +1,56 @@ +package org.whispersystems.textsecuregcm.recaptcha; + +import com.fasterxml.jackson.annotation.JsonProperty; +import com.fasterxml.jackson.databind.DeserializationFeature; +import com.fasterxml.jackson.jaxrs.json.JacksonJaxbJsonProvider; +import org.glassfish.jersey.client.ClientConfig; + +import javax.ws.rs.client.Client; +import javax.ws.rs.client.ClientBuilder; +import javax.ws.rs.client.Entity; +import javax.ws.rs.core.MultivaluedHashMap; +import javax.ws.rs.core.MultivaluedMap; + +public class RecaptchaClient { + + private final Client client; + private final String recaptchaSecret; + + public RecaptchaClient(String recaptchaSecret) { + this.client = ClientBuilder.newClient(new ClientConfig(new JacksonJaxbJsonProvider().configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false))); + this.recaptchaSecret = recaptchaSecret; + } + + public boolean verify(String captchaToken) { + MultivaluedMap formData = new MultivaluedHashMap<>(); + formData.add("secret", recaptchaSecret); + formData.add("response", captchaToken); + + VerifyResponse response = client.target("https://www.google.com/recaptcha/api/siteverify") + .request() + .post(Entity.form(formData), VerifyResponse.class); + + return response.success; + } + + private static class VerifyResponse { + @JsonProperty + private boolean success; + + @JsonProperty("error-codes") + private String[] errorCodes; + + @JsonProperty + private String hostname; + + @JsonProperty + private String challenge_ts; + + @Override + public String toString() { + return "success: " + success + ", errorCodes: " + String.join(", ", errorCodes == null ? new String[0] : errorCodes) + ", hostname: " + hostname + ", challenge_ts: " + challenge_ts; + } + } + +} + 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 2f110c79b..9ed1b184a 100644 --- a/src/test/java/org/whispersystems/textsecuregcm/tests/controllers/AccountControllerTest.java +++ b/src/test/java/org/whispersystems/textsecuregcm/tests/controllers/AccountControllerTest.java @@ -15,6 +15,7 @@ import org.whispersystems.textsecuregcm.limits.RateLimiter; import org.whispersystems.textsecuregcm.limits.RateLimiters; import org.whispersystems.textsecuregcm.mappers.RateLimitExceededExceptionMapper; import org.whispersystems.textsecuregcm.providers.TimeProvider; +import org.whispersystems.textsecuregcm.recaptcha.RecaptchaClient; import org.whispersystems.textsecuregcm.sms.SmsSender; import org.whispersystems.textsecuregcm.sqs.DirectoryQueue; import org.whispersystems.textsecuregcm.storage.AbusiveHostRule; @@ -29,9 +30,9 @@ import org.whispersystems.textsecuregcm.util.SystemMapper; import javax.ws.rs.client.Entity; import javax.ws.rs.core.MediaType; import javax.ws.rs.core.Response; +import java.io.IOException; import java.util.Collections; import java.util.HashMap; -import java.util.List; import java.util.Optional; import java.util.concurrent.TimeUnit; @@ -52,6 +53,9 @@ public class AccountControllerTest { private static final String RESTRICTED_HOST = "192.168.1.2"; private static final String NICE_HOST = "127.0.0.1"; + private static final String VALID_CAPTCHA_TOKEN = "valid_token"; + private static final String INVALID_CAPTCHA_TOKEN = "invalid_token"; + private PendingAccountsManager pendingAccountsManager = mock(PendingAccountsManager.class); private AccountsManager accountsManager = mock(AccountsManager.class ); private AbusiveHostRules abusiveHostRules = mock(AbusiveHostRules.class ); @@ -65,6 +69,7 @@ public class AccountControllerTest { private TimeProvider timeProvider = mock(TimeProvider.class ); private TurnTokenGenerator turnTokenGenerator = mock(TurnTokenGenerator.class); private Account senderPinAccount = mock(Account.class); + private RecaptchaClient recaptchaClient = mock(RecaptchaClient.class); @Rule public final ResourceTestRule resources = ResourceTestRule.builder() @@ -81,7 +86,8 @@ public class AccountControllerTest { directoryQueue, storedMessages, turnTokenGenerator, - new HashMap<>())) + new HashMap<>(), + recaptchaClient)) .build(); @@ -112,6 +118,9 @@ public class AccountControllerTest { when(abusiveHostRules.getAbusiveHostRulesFor(eq(RESTRICTED_HOST))).thenReturn(Collections.singletonList(new AbusiveHostRule(RESTRICTED_HOST, false, Collections.singletonList("+123")))); when(abusiveHostRules.getAbusiveHostRulesFor(eq(NICE_HOST))).thenReturn(Collections.emptyList()); + when(recaptchaClient.verify(eq(INVALID_CAPTCHA_TOKEN))).thenReturn(false); + when(recaptchaClient.verify(eq(VALID_CAPTCHA_TOKEN))).thenReturn(true); + doThrow(new RateLimitExceededException(SENDER_OVER_PIN)).when(pinLimiter).validate(eq(SENDER_OVER_PIN)); } @@ -169,12 +178,46 @@ public class AccountControllerTest { .header("X-Forwarded-For", ABUSIVE_HOST) .get(); - assertThat(response.getStatus()).isEqualTo(200); + assertThat(response.getStatus()).isEqualTo(402); verify(abusiveHostRules).getAbusiveHostRulesFor(eq(ABUSIVE_HOST)); verifyNoMoreInteractions(smsSender); } + @Test + public void testSendAbusiveHostWithValidCaptcha() throws IOException { + Response response = + resources.getJerseyTest() + .target(String.format("/v1/accounts/sms/code/%s", SENDER)) + .queryParam("captcha", VALID_CAPTCHA_TOKEN) + .request() + .header("X-Forwarded-For", ABUSIVE_HOST) + .get(); + + assertThat(response.getStatus()).isEqualTo(200); + + verifyNoMoreInteractions(abusiveHostRules); + verify(recaptchaClient).verify(eq(VALID_CAPTCHA_TOKEN)); + verify(smsSender).deliverSmsVerification(eq(SENDER), eq(Optional.empty()), anyString()); + } + + @Test + public void testSendAbusiveHostWithInvalidCaptcha() { + Response response = + resources.getJerseyTest() + .target(String.format("/v1/accounts/sms/code/%s", SENDER)) + .queryParam("captcha", INVALID_CAPTCHA_TOKEN) + .request() + .header("X-Forwarded-For", ABUSIVE_HOST) + .get(); + + assertThat(response.getStatus()).isEqualTo(402); + + verifyNoMoreInteractions(abusiveHostRules); + verify(recaptchaClient).verify(eq(INVALID_CAPTCHA_TOKEN)); + verifyNoMoreInteractions(smsSender); + } + @Test public void testSendMultipleHost() { Response response = @@ -184,7 +227,7 @@ public class AccountControllerTest { .header("X-Forwarded-For", NICE_HOST + ", " + ABUSIVE_HOST) .get(); - assertThat(response.getStatus()).isEqualTo(200); + assertThat(response.getStatus()).isEqualTo(402); verify(abusiveHostRules, times(1)).getAbusiveHostRulesFor(eq(ABUSIVE_HOST)); verify(abusiveHostRules, times(1)).getAbusiveHostRulesFor(eq(NICE_HOST)); @@ -203,7 +246,7 @@ public class AccountControllerTest { .header("X-Forwarded-For", RESTRICTED_HOST) .get(); - assertThat(response.getStatus()).isEqualTo(200); + assertThat(response.getStatus()).isEqualTo(402); verify(abusiveHostRules).getAbusiveHostRulesFor(eq(RESTRICTED_HOST)); verifyNoMoreInteractions(smsSender);