diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java b/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java index 7cbe14ceb..2bcfdeba8 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java @@ -1,5 +1,5 @@ /* - * Copyright 2013-2020 Signal Messenger, LLC + * Copyright 2013-2021 Signal Messenger, LLC * SPDX-License-Identifier: AGPL-3.0-only */ package org.whispersystems.textsecuregcm; @@ -110,6 +110,7 @@ import org.whispersystems.textsecuregcm.mappers.InvalidWebsocketAddressException import org.whispersystems.textsecuregcm.mappers.RateLimitChallengeExceptionMapper; import org.whispersystems.textsecuregcm.mappers.RateLimitExceededExceptionMapper; import org.whispersystems.textsecuregcm.mappers.RetryLaterExceptionMapper; +import org.whispersystems.textsecuregcm.mappers.ServerRejectedExceptionMapper; import org.whispersystems.textsecuregcm.metrics.BufferPoolGauges; import org.whispersystems.textsecuregcm.metrics.CpuUsageGauge; import org.whispersystems.textsecuregcm.metrics.FileDescriptorGauge; @@ -662,6 +663,7 @@ public class WhisperServerService extends Application account, - @HeaderParam(OptionalAccess.UNIDENTIFIED) Optional accessKey, - @PathParam("identifier") AmbiguousIdentifier targetName, - @PathParam("device_id") String deviceId, - @HeaderParam("User-Agent") String userAgent) - throws RateLimitExceededException, RateLimitChallengeException { + public Response getDeviceKeys(@Auth Optional account, + @HeaderParam(OptionalAccess.UNIDENTIFIED) Optional accessKey, + @PathParam("identifier") AmbiguousIdentifier targetName, + @PathParam("device_id") String deviceId, + @HeaderParam("User-Agent") String userAgent) + throws RateLimitExceededException, RateLimitChallengeException, ServerRejectedException { targetName.incrementRequestCounter("getDeviceKeys", userAgent); @@ -152,16 +152,17 @@ public class KeysController { preKeyRateLimiter.validate(account.get()); } catch (RateLimitExceededException e) { - final boolean enforceLimit = rateLimitChallengeManager.shouldIssueRateLimitChallenge(userAgent); + final boolean legacyClient = rateLimitChallengeManager.isClientBelowMinimumVersion(userAgent); Metrics.counter(RATE_LIMITED_GET_PREKEYS_COUNTER_NAME, - SOURCE_COUNTRY_TAG_NAME, Util.getCountryCode(account.get().getNumber()), - "enforced", String.valueOf(enforceLimit)) + SOURCE_COUNTRY_TAG_NAME, Util.getCountryCode(account.get().getNumber()), + "legacyClient", String.valueOf(legacyClient)) .increment(); - if (enforceLimit) { - throw new RateLimitChallengeException(account.get(), e.getRetryDuration()); + if (legacyClient) { + throw new ServerRejectedException(); } + throw new RateLimitChallengeException(account.get(), e.getRetryDuration()); } } diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/controllers/MessageController.java b/service/src/main/java/org/whispersystems/textsecuregcm/controllers/MessageController.java index 4cffc8b53..f0afd0e00 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/controllers/MessageController.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/controllers/MessageController.java @@ -1,5 +1,5 @@ /* - * Copyright 2013-2020 Signal Messenger, LLC + * Copyright 2013-2021 Signal Messenger, LLC * SPDX-License-Identifier: AGPL-3.0-only */ package org.whispersystems.textsecuregcm.controllers; @@ -19,6 +19,7 @@ import io.dropwizard.util.DataSize; import io.lettuce.core.ScriptOutputType; import io.micrometer.core.instrument.Metrics; import io.micrometer.core.instrument.Tag; +import io.micrometer.core.instrument.Tags; import java.io.IOException; import java.security.MessageDigest; import java.time.Duration; @@ -56,7 +57,6 @@ import javax.ws.rs.WebApplicationException; import javax.ws.rs.core.MediaType; import javax.ws.rs.core.Response; import javax.ws.rs.core.Response.Status; -import io.micrometer.core.instrument.Tags; import org.apache.commons.lang3.StringUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -265,20 +265,18 @@ public class MessageController { unsealedSenderRateLimiter.validate(source.get(), destination.get()); } catch (final RateLimitExceededException e) { - final boolean enforceLimit = rateLimitChallengeManager.shouldIssueRateLimitChallenge(userAgent); + final boolean legacyClient = rateLimitChallengeManager.isClientBelowMinimumVersion(userAgent); Metrics.counter(REJECT_UNSEALED_SENDER_COUNTER_NAME, - SENDER_COUNTRY_TAG_NAME, senderCountryCode, - "enforced", String.valueOf(enforceLimit)) + SENDER_COUNTRY_TAG_NAME, senderCountryCode, + "legacyClient", String.valueOf(legacyClient)) .increment(); - if (enforceLimit) { - logger.debug("Rejected unsealed sender limit from: {}", source.get().getNumber()); - - throw new RateLimitChallengeException(source.get(), e.getRetryDuration()); - } else { + if (legacyClient) { throw e; } + + throw new RateLimitChallengeException(source.get(), e.getRetryDuration()); } final String destinationCountryCode = Util.getCountryCode(destination.get().getNumber()); diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/controllers/ServerRejectedException.java b/service/src/main/java/org/whispersystems/textsecuregcm/controllers/ServerRejectedException.java new file mode 100644 index 000000000..565d77667 --- /dev/null +++ b/service/src/main/java/org/whispersystems/textsecuregcm/controllers/ServerRejectedException.java @@ -0,0 +1,10 @@ +/* + * Copyright 2021 Signal Messenger, LLC + * SPDX-License-Identifier: AGPL-3.0-only + */ + +package org.whispersystems.textsecuregcm.controllers; + +public class ServerRejectedException extends Exception { + +} diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/limits/RateLimitChallengeException.java b/service/src/main/java/org/whispersystems/textsecuregcm/limits/RateLimitChallengeException.java index 8a0c45020..656830564 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/limits/RateLimitChallengeException.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/limits/RateLimitChallengeException.java @@ -1,7 +1,12 @@ +/* + * Copyright 2021 Signal Messenger, LLC + * SPDX-License-Identifier: AGPL-3.0-only + */ + package org.whispersystems.textsecuregcm.limits; -import org.whispersystems.textsecuregcm.storage.Account; import java.time.Duration; +import org.whispersystems.textsecuregcm.storage.Account; public class RateLimitChallengeException extends Exception { @@ -20,4 +25,5 @@ public class RateLimitChallengeException extends Exception { public Duration getRetryAfter() { return retryAfter; } + } diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/limits/RateLimitChallengeManager.java b/service/src/main/java/org/whispersystems/textsecuregcm/limits/RateLimitChallengeManager.java index d756158bc..b8707ee69 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/limits/RateLimitChallengeManager.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/limits/RateLimitChallengeManager.java @@ -95,15 +95,15 @@ public class RateLimitChallengeManager { unsealedSenderRateLimiter.handleRateLimitReset(account); } - public boolean shouldIssueRateLimitChallenge(final String userAgent) { + public boolean isClientBelowMinimumVersion(final String userAgent) { try { final UserAgent client = UserAgentUtil.parseUserAgentString(userAgent); final Optional minimumClientVersion = dynamicConfigurationManager.getConfiguration() .getRateLimitChallengeConfiguration() .getMinimumSupportedVersion(client.getPlatform()); - return minimumClientVersion.map(version -> version.isLowerThanOrEqualTo(client.getVersion())) - .orElse(false); + return minimumClientVersion.map(version -> version.isGreaterThan(client.getVersion())) + .orElse(true); } catch (final UnrecognizedUserAgentException ignored) { return false; } diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/mappers/RateLimitChallengeExceptionMapper.java b/service/src/main/java/org/whispersystems/textsecuregcm/mappers/RateLimitChallengeExceptionMapper.java index 1b9cc52ac..bf1fbbafd 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/mappers/RateLimitChallengeExceptionMapper.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/mappers/RateLimitChallengeExceptionMapper.java @@ -1,11 +1,16 @@ +/* + * Copyright 2021 Signal Messenger, LLC + * SPDX-License-Identifier: AGPL-3.0-only + */ + package org.whispersystems.textsecuregcm.mappers; -import org.whispersystems.textsecuregcm.entities.RateLimitChallenge; -import org.whispersystems.textsecuregcm.limits.RateLimitChallengeManager; -import org.whispersystems.textsecuregcm.limits.RateLimitChallengeException; +import java.util.UUID; import javax.ws.rs.core.Response; import javax.ws.rs.ext.ExceptionMapper; -import java.util.UUID; +import org.whispersystems.textsecuregcm.entities.RateLimitChallenge; +import org.whispersystems.textsecuregcm.limits.RateLimitChallengeException; +import org.whispersystems.textsecuregcm.limits.RateLimitChallengeManager; public class RateLimitChallengeExceptionMapper implements ExceptionMapper { @@ -18,8 +23,10 @@ public class RateLimitChallengeExceptionMapper implements ExceptionMapper { + + @Override + public Response toResponse(final ServerRejectedException exception) { + return Response.status(508).build(); + } +} diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/limits/RateLimitChallengeManagerTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/limits/RateLimitChallengeManagerTest.java index 77d109eef..5732f1985 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/limits/RateLimitChallengeManagerTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/limits/RateLimitChallengeManagerTest.java @@ -112,25 +112,25 @@ class RateLimitChallengeManagerTest { @ParameterizedTest @MethodSource - void shouldIssueRateLimitChallenge(final String userAgent, final boolean expectIssueChallenge) { + void isClientBelowMinimumVersion(final String userAgent, final boolean expectBelowMinimumVersion) { when(rateLimitChallengeConfiguration.getMinimumSupportedVersion(any())).thenReturn(Optional.empty()); when(rateLimitChallengeConfiguration.getMinimumSupportedVersion(ClientPlatform.ANDROID)) .thenReturn(Optional.of(new Semver("5.6.0"))); when(rateLimitChallengeConfiguration.getMinimumSupportedVersion(ClientPlatform.DESKTOP)) .thenReturn(Optional.of(new Semver("5.0.0-beta.2"))); - assertEquals(expectIssueChallenge, rateLimitChallengeManager.shouldIssueRateLimitChallenge(userAgent)); + assertEquals(expectBelowMinimumVersion, rateLimitChallengeManager.isClientBelowMinimumVersion(userAgent)); } - private static Stream shouldIssueRateLimitChallenge() { + private static Stream isClientBelowMinimumVersion() { return Stream.of( - Arguments.of("Signal-Android/5.1.2 Android/30", false), - Arguments.of("Signal-Android/5.6.0 Android/30", true), - Arguments.of("Signal-Android/5.11.1 Android/30", true), - Arguments.of("Signal-Desktop/5.0.0-beta.3 macOS/11", true), - Arguments.of("Signal-Desktop/5.0.0-beta.1 Windows/3.1", false), - Arguments.of("Signal-Desktop/5.2.0 Debian/11", true), - Arguments.of("Signal-iOS/5.1.2 iOS/12.2", false), + Arguments.of("Signal-Android/5.1.2 Android/30", true), + Arguments.of("Signal-Android/5.6.0 Android/30", false), + Arguments.of("Signal-Android/5.11.1 Android/30", false), + Arguments.of("Signal-Desktop/5.0.0-beta.3 macOS/11", false), + Arguments.of("Signal-Desktop/5.0.0-beta.1 Windows/3.1", true), + Arguments.of("Signal-Desktop/5.2.0 Debian/11", false), + Arguments.of("Signal-iOS/5.1.2 iOS/12.2", true), Arguments.of("anything-else", false) ); } diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/tests/controllers/KeysControllerTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/tests/controllers/KeysControllerTest.java index 87d644ea6..d52e813b1 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/tests/controllers/KeysControllerTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/tests/controllers/KeysControllerTest.java @@ -1,5 +1,5 @@ /* - * Copyright 2013-2020 Signal Messenger, LLC + * Copyright 2013-2021 Signal Messenger, LLC * SPDX-License-Identifier: AGPL-3.0-only */ @@ -39,6 +39,8 @@ import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; import org.mockito.ArgumentCaptor; import org.mockito.ArgumentMatcher; import org.whispersystems.textsecuregcm.auth.AmbiguousIdentifier; @@ -57,6 +59,7 @@ import org.whispersystems.textsecuregcm.limits.RateLimitChallengeManager; import org.whispersystems.textsecuregcm.limits.RateLimiter; import org.whispersystems.textsecuregcm.limits.RateLimiters; import org.whispersystems.textsecuregcm.mappers.RateLimitChallengeExceptionMapper; +import org.whispersystems.textsecuregcm.mappers.ServerRejectedExceptionMapper; import org.whispersystems.textsecuregcm.storage.Account; import org.whispersystems.textsecuregcm.storage.AccountsManager; import org.whispersystems.textsecuregcm.storage.Device; @@ -98,12 +101,15 @@ class KeysControllerTest { private static final RateLimiter rateLimiter = mock(RateLimiter.class ); private static final ResourceExtension resources = ResourceExtension.builder() - .addProvider(AuthHelper.getAuthFilter()) - .addProvider(new PolymorphicAuthValueFactoryProvider.Binder<>(ImmutableSet.of(Account.class, DisabledPermittedAccount.class))) - .setTestContainerFactory(new GrizzlyWebTestContainerFactory()) - .addResource(new RateLimitChallengeExceptionMapper(rateLimitChallengeManager)) - .addResource(new KeysController(rateLimiters, keysDynamoDb, accounts, preKeyRateLimiter, rateLimitChallengeManager)) - .build(); + .addProvider(AuthHelper.getAuthFilter()) + .addProvider(new PolymorphicAuthValueFactoryProvider.Binder<>( + ImmutableSet.of(Account.class, DisabledPermittedAccount.class))) + .setTestContainerFactory(new GrizzlyWebTestContainerFactory()) + .addResource(new RateLimitChallengeExceptionMapper(rateLimitChallengeManager)) + .addResource(new ServerRejectedExceptionMapper()) + .addResource( + new KeysController(rateLimiters, keysDynamoDb, accounts, preKeyRateLimiter, rateLimitChallengeManager)) + .build(); @BeforeEach void setup() { @@ -622,16 +628,20 @@ class KeysControllerTest { verify(accounts).update(eq(AuthHelper.DISABLED_ACCOUNT), any()); } - @Test - void testRateLimitChallenge() throws RateLimitExceededException { + @ParameterizedTest + @ValueSource(booleans = {true, false}) + void testRateLimitChallenge(boolean clientBelowMinimumVersion) throws RateLimitExceededException { Duration retryAfter = Duration.ofMinutes(1); doThrow(new RateLimitExceededException(retryAfter)) .when(preKeyRateLimiter).validate(any()); - when(rateLimitChallengeManager.shouldIssueRateLimitChallenge("Signal-Android/5.1.2 Android/30")).thenReturn(true); + when( + rateLimitChallengeManager.isClientBelowMinimumVersion("Signal-Android/5.1.2 Android/30")).thenReturn( + clientBelowMinimumVersion); when(rateLimitChallengeManager.getChallengeOptions(AuthHelper.VALID_ACCOUNT)) - .thenReturn(List.of(RateLimitChallengeManager.OPTION_PUSH_CHALLENGE, RateLimitChallengeManager.OPTION_RECAPTCHA)); + .thenReturn( + List.of(RateLimitChallengeManager.OPTION_PUSH_CHALLENGE, RateLimitChallengeManager.OPTION_RECAPTCHA)); Response result = resources.getJerseyTest() .target(String.format("/v2/keys/%s/*", EXISTS_UUID.toString())) @@ -650,14 +660,18 @@ class KeysControllerTest { .header("User-Agent", "Signal-Android/5.1.2 Android/30") .get(); - assertThat(result.getStatus()).isEqualTo(428); + if (clientBelowMinimumVersion) { + assertThat(result.getStatus()).isEqualTo(508); + } else { + assertThat(result.getStatus()).isEqualTo(428); - RateLimitChallenge rateLimitChallenge = result.readEntity(RateLimitChallenge.class); + RateLimitChallenge rateLimitChallenge = result.readEntity(RateLimitChallenge.class); - assertThat(rateLimitChallenge.getToken()).isNotBlank(); - assertThat(rateLimitChallenge.getOptions()).isNotEmpty(); - assertThat(rateLimitChallenge.getOptions()).contains("recaptcha"); - assertThat(rateLimitChallenge.getOptions()).contains("pushChallenge"); - assertThat(Long.parseLong(result.getHeaderString("Retry-After"))).isEqualTo(retryAfter.toSeconds()); + assertThat(rateLimitChallenge.getToken()).isNotBlank(); + assertThat(rateLimitChallenge.getOptions()).isNotEmpty(); + assertThat(rateLimitChallenge.getOptions()).contains("recaptcha"); + assertThat(rateLimitChallenge.getOptions()).contains("pushChallenge"); + assertThat(Long.parseLong(result.getHeaderString("Retry-After"))).isEqualTo(retryAfter.toSeconds()); + } } } diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/tests/controllers/MessageControllerTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/tests/controllers/MessageControllerTest.java index dd750c1ac..f3ff86672 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/tests/controllers/MessageControllerTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/tests/controllers/MessageControllerTest.java @@ -294,8 +294,9 @@ class MessageControllerTest { } @ParameterizedTest - @CsvSource({"true, 5.1.0, 413", "true, 5.6.4, 428", "false, 5.6.4, 200"}) - void testUnsealedSenderCardinalityRateLimited(final boolean rateLimited, final String clientVersion, final int expectedStatusCode) throws Exception { + @CsvSource({"true, true, 413", "true, false, 428", "false, false, 200"}) + void testUnsealedSenderCardinalityRateLimited(final boolean rateLimited, final boolean legacyClient, + final int expectedStatusCode) throws Exception { final DynamicConfiguration dynamicConfiguration = mock(DynamicConfiguration.class); final DynamicMessageRateConfiguration messageRateConfiguration = mock(DynamicMessageRateConfiguration.class); @@ -323,8 +324,8 @@ class MessageControllerTest { doThrow(new RateLimitExceededException(Duration.ofHours(1))) .when(unsealedSenderRateLimiter).validate(eq(AuthHelper.VALID_ACCOUNT), eq(internationalAccount)); - when(rateLimitChallengeManager.shouldIssueRateLimitChallenge(String.format("Signal-Android/%s Android/30", clientVersion))) - .thenReturn(true); + when(rateLimitChallengeManager.isClientBelowMinimumVersion(anyString())) + .thenReturn(legacyClient); } Response response = @@ -352,9 +353,10 @@ class MessageControllerTest { doThrow(new RateLimitExceededException(retryAfter)) .when(unsealedSenderRateLimiter).validate(any(), any()); - when(rateLimitChallengeManager.shouldIssueRateLimitChallenge("Signal-Android/5.1.2 Android/30")).thenReturn(true); + when(rateLimitChallengeManager.isClientBelowMinimumVersion("Signal-Android/5.1.2 Android/30")).thenReturn(false); when(rateLimitChallengeManager.getChallengeOptions(AuthHelper.VALID_ACCOUNT)) - .thenReturn(List.of(RateLimitChallengeManager.OPTION_PUSH_CHALLENGE, RateLimitChallengeManager.OPTION_RECAPTCHA)); + .thenReturn( + List.of(RateLimitChallengeManager.OPTION_PUSH_CHALLENGE, RateLimitChallengeManager.OPTION_RECAPTCHA)); Response response = resources.getJerseyTest()