mirror of
https://github.com/signalapp/Signal-Server.git
synced 2024-09-19 19:42:18 +02:00
Prevent old versions of the Android app from carrying out "change phone number" requests
This commit is contained in:
Jon Chambers
Jon Chambers
parent
c5dc01ee11
commit
4f45f23094
@ -8,7 +8,9 @@ package org.whispersystems.textsecuregcm.controllers;
|
||||
import static org.whispersystems.textsecuregcm.metrics.MetricsUtil.name;
|
||||
|
||||
import com.google.common.net.HttpHeaders;
|
||||
import com.vdurmont.semver4j.Semver;
|
||||
import io.dropwizard.auth.Auth;
|
||||
import io.micrometer.core.instrument.Counter;
|
||||
import io.micrometer.core.instrument.Metrics;
|
||||
import io.micrometer.core.instrument.Tag;
|
||||
import io.micrometer.core.instrument.Tags;
|
||||
@ -51,6 +53,10 @@ import org.whispersystems.textsecuregcm.metrics.UserAgentTagUtil;
|
||||
import org.whispersystems.textsecuregcm.storage.Account;
|
||||
import org.whispersystems.textsecuregcm.storage.AccountsManager;
|
||||
import org.whispersystems.textsecuregcm.storage.ChangeNumberManager;
|
||||
import org.whispersystems.textsecuregcm.util.ua.ClientPlatform;
|
||||
import org.whispersystems.textsecuregcm.util.ua.UnrecognizedUserAgentException;
|
||||
import org.whispersystems.textsecuregcm.util.ua.UserAgent;
|
||||
import org.whispersystems.textsecuregcm.util.ua.UserAgentUtil;
|
||||
|
||||
@Path("/v2/accounts")
|
||||
@io.swagger.v3.oas.annotations.tags.Tag(name = "Account")
|
||||
@ -59,12 +65,17 @@ public class AccountControllerV2 {
|
||||
private static final String CHANGE_NUMBER_COUNTER_NAME = name(AccountControllerV2.class, "changeNumber");
|
||||
private static final String VERIFICATION_TYPE_TAG_NAME = "verification";
|
||||
|
||||
private static final Counter ANDROID_CHANGE_NUMBER_REJECTED_COUNTER =
|
||||
Metrics.counter(name(AccountControllerV2.class, "androidChangeNumberRejected"));
|
||||
|
||||
private final AccountsManager accountsManager;
|
||||
private final ChangeNumberManager changeNumberManager;
|
||||
private final PhoneVerificationTokenManager phoneVerificationTokenManager;
|
||||
private final RegistrationLockVerificationManager registrationLockVerificationManager;
|
||||
private final RateLimiters rateLimiters;
|
||||
|
||||
private static final Semver MINIMUM_ANDROID_CHANGE_NUMBER_VERSION = new Semver("6.46.7");
|
||||
|
||||
public AccountControllerV2(final AccountsManager accountsManager, final ChangeNumberManager changeNumberManager,
|
||||
final PhoneVerificationTokenManager phoneVerificationTokenManager,
|
||||
final RegistrationLockVerificationManager registrationLockVerificationManager, final RateLimiters rateLimiters) {
|
||||
@ -91,13 +102,24 @@ public class AccountControllerV2 {
|
||||
name = "Retry-After",
|
||||
description = "If present, an positive integer indicating the number of seconds before a subsequent attempt could succeed"))
|
||||
public AccountIdentityResponse changeNumber(@Auth final AuthenticatedAccount authenticatedAccount,
|
||||
@NotNull @Valid final ChangeNumberRequest request, @HeaderParam(HttpHeaders.USER_AGENT) final String userAgent)
|
||||
@NotNull @Valid final ChangeNumberRequest request, @HeaderParam(HttpHeaders.USER_AGENT) final String userAgentString)
|
||||
throws RateLimitExceededException, InterruptedException {
|
||||
|
||||
if (!authenticatedAccount.getAuthenticatedDevice().isPrimary()) {
|
||||
throw new ForbiddenException();
|
||||
}
|
||||
|
||||
// We can remove this check after old versions of the Android app expire on or after 2024-05-08
|
||||
try {
|
||||
final UserAgent userAgent = UserAgentUtil.parseUserAgentString(userAgentString);
|
||||
|
||||
if (userAgent.getPlatform().equals(ClientPlatform.ANDROID) && userAgent.getVersion().isLowerThan(MINIMUM_ANDROID_CHANGE_NUMBER_VERSION)) {
|
||||
ANDROID_CHANGE_NUMBER_REJECTED_COUNTER.increment();
|
||||
throw new WebApplicationException(499);
|
||||
}
|
||||
} catch (final UnrecognizedUserAgentException ignored) {
|
||||
}
|
||||
|
||||
final String number = request.number();
|
||||
|
||||
// Only verify and check reglock if there's a data change to be made...
|
||||
@ -112,10 +134,10 @@ public class AccountControllerV2 {
|
||||
|
||||
if (existingAccount.isPresent()) {
|
||||
registrationLockVerificationManager.verifyRegistrationLock(existingAccount.get(), request.registrationLock(),
|
||||
userAgent, RegistrationLockVerificationManager.Flow.CHANGE_NUMBER, verificationType);
|
||||
userAgentString, RegistrationLockVerificationManager.Flow.CHANGE_NUMBER, verificationType);
|
||||
}
|
||||
|
||||
Metrics.counter(CHANGE_NUMBER_COUNTER_NAME, Tags.of(UserAgentTagUtil.getPlatformTag(userAgent),
|
||||
Metrics.counter(CHANGE_NUMBER_COUNTER_NAME, Tags.of(UserAgentTagUtil.getPlatformTag(userAgentString),
|
||||
Tag.of(VERIFICATION_TYPE_TAG_NAME, verificationType.name())))
|
||||
.increment();
|
||||
}
|
||||
|
||||
@ -56,6 +56,7 @@ 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.Arguments;
|
||||
import org.junit.jupiter.params.provider.CsvSource;
|
||||
import org.junit.jupiter.params.provider.EnumSource;
|
||||
import org.junit.jupiter.params.provider.MethodSource;
|
||||
import org.mockito.ArgumentCaptor;
|
||||
@ -88,7 +89,6 @@ import org.whispersystems.textsecuregcm.storage.Device;
|
||||
import org.whispersystems.textsecuregcm.storage.RegistrationRecoveryPasswordsManager;
|
||||
import org.whispersystems.textsecuregcm.tests.util.AccountsHelper;
|
||||
import org.whispersystems.textsecuregcm.tests.util.AuthHelper;
|
||||
import org.whispersystems.textsecuregcm.tests.util.KeysHelper;
|
||||
import org.whispersystems.textsecuregcm.util.SystemMapper;
|
||||
import org.whispersystems.textsecuregcm.util.Util;
|
||||
|
||||
@ -419,6 +419,49 @@ class AccountControllerV2Test {
|
||||
}
|
||||
}
|
||||
|
||||
@ParameterizedTest
|
||||
@CsvSource({
|
||||
"Signal-Android/4.68.3, true",
|
||||
"Signal-Android/6.46.7, false",
|
||||
"Signal-Android/6.46.8, false",
|
||||
"Signal-Desktop/1.2.3 Linux, false",
|
||||
"Signal-iOS/3.9.0 iOS/14.2, false",
|
||||
"Not a real user-agent string, false"
|
||||
})
|
||||
void changeNumberVersionEnforced(final String userAgentString, final boolean expectBlocked) throws Exception {
|
||||
|
||||
when(registrationServiceClient.getSession(any(), any()))
|
||||
.thenReturn(CompletableFuture.completedFuture(
|
||||
Optional.of(new RegistrationServiceSession(new byte[16], NEW_NUMBER, true, null, null, null,
|
||||
SESSION_EXPIRATION_SECONDS))));
|
||||
|
||||
try (final Response response = resources.getJerseyTest()
|
||||
.target("/v2/accounts/number")
|
||||
.request()
|
||||
.header(HttpHeaders.AUTHORIZATION,
|
||||
AuthHelper.getAuthHeader(AuthHelper.VALID_UUID, AuthHelper.VALID_PASSWORD))
|
||||
.header(HttpHeaders.USER_AGENT, userAgentString)
|
||||
.put(Entity.entity(
|
||||
new ChangeNumberRequest(encodeSessionId("session"), null, NEW_NUMBER, "123", new IdentityKey(Curve.generateKeyPair().getPublicKey()),
|
||||
Collections.emptyList(),
|
||||
Collections.emptyMap(), null, Collections.emptyMap()),
|
||||
MediaType.APPLICATION_JSON_TYPE))) {
|
||||
|
||||
if (expectBlocked) {
|
||||
assertEquals(499, response.getStatus());
|
||||
|
||||
verify(changeNumberManager, never())
|
||||
.changeNumber(eq(AuthHelper.VALID_ACCOUNT), eq(NEW_NUMBER), any(), any(), any(), any(), any());
|
||||
} else {
|
||||
assertEquals(200, response.getStatus());
|
||||
|
||||
verify(changeNumberManager)
|
||||
.changeNumber(eq(AuthHelper.VALID_ACCOUNT), eq(NEW_NUMBER), any(), any(), any(), any(), any());
|
||||
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Valid request JSON with the given Recovery Password
|
||||
*/
|
||||
|
||||
Loading…
Reference in New Issue
Block a user