Signal/Signal-Server
1
0
Fork 0
mirror of https://github.com/signalapp/Signal-Server synced 2026-05-25 01:41:52 +01:00
Code Issues Packages Projects Releases Wiki Activity

Add post-registration change number waiting period

Browse Source
This commit is contained in:
Chris Eager
2026-05-08 16:29:16 -05:00
committed by Chris Eager
parent f045e3ee0f
commit 90c27f6969
6 changed files with 142 additions and 12 deletions
Download Patch File Download Diff File Expand all files Collapse all files
service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerConfiguration.java
+9
View File
@@ -22,6 +22,7 @@ import org.whispersystems.textsecuregcm.configuration.BraintreeConfiguration;
import org.whispersystems.textsecuregcm.configuration.CallQualitySurveyConfiguration;
import org.whispersystems.textsecuregcm.configuration.Cdn3StorageManagerConfiguration;
import org.whispersystems.textsecuregcm.configuration.CdnConfiguration;
import org.whispersystems.textsecuregcm.configuration.ChangeNumberConfiguration;
import org.whispersystems.textsecuregcm.configuration.CircuitBreakerConfiguration;
import org.whispersystems.textsecuregcm.configuration.ClientReleaseConfiguration;
import org.whispersystems.textsecuregcm.configuration.DefaultAwsCredentialsFactory;
@@ -355,6 +356,11 @@ public class WhisperServerConfiguration extends Configuration {
@JsonProperty
private CallQualitySurveyConfiguration callQualitySurvey;
@Valid
@NotNull
@JsonProperty
private ChangeNumberConfiguration changeNumber = new ChangeNumberConfiguration(Duration.ofHours(1));
public TlsKeyStoreConfiguration getTlsKeyStoreConfiguration() {
return tlsKeyStore;
}
@@ -591,4 +597,7 @@ public class WhisperServerConfiguration extends Configuration {
return hlrLookup;
}
public ChangeNumberConfiguration getChangeNumber() {
return changeNumber;
}
}
service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java
+2 -1
View File
@@ -1096,7 +1096,8 @@ public class WhisperServerService extends Application<WhisperServerConfiguration
phoneNumberIdentifiers, registrationServiceClient, registrationRecoveryPasswordsManager, registrationRecoveryChecker);
final ChangeNumberManager changeNumberManager = new ChangeNumberManager(messageSender, accountsManager,
phoneVerificationTokenManager, registrationLockVerificationManager, rateLimiters, Clock.systemUTC());
phoneVerificationTokenManager, registrationLockVerificationManager, rateLimiters,
config.getChangeNumber().postRegistrationWaitingPeriod(), Clock.systemUTC());
final List<Object> commonControllers = Lists.newArrayList(
new AccountController(accountsManager, rateLimiters, registrationRecoveryPasswordsManager,
service/src/main/java/org/whispersystems/textsecuregcm/configuration/ChangeNumberConfiguration.java
+11
View File
@@ -0,0 +1,11 @@
/*
* Copyright 2026 Signal Messenger, LLC
* SPDX-License-Identifier: AGPL-3.0-only
*/
package org.whispersystems.textsecuregcm.configuration;
import java.time.Duration;
public record ChangeNumberConfiguration(Duration postRegistrationWaitingPeriod) {
}
service/src/main/java/org/whispersystems/textsecuregcm/controllers/AccountControllerV2.java
+1 -1
View File
@@ -76,7 +76,7 @@ public class AccountControllerV2 {
@ApiResponse(responseCode = "423", content = @Content(schema = @Schema(implementation = RegistrationLockFailure.class)))
@ApiResponse(responseCode = "429", description = "Too many attempts", headers = @Header(
name = "Retry-After",
description = "If present, an positive integer indicating the number of seconds before a subsequent attempt could succeed"))
description = "If present, a positive integer indicating the number of seconds before a subsequent attempt could succeed"))
public AccountIdentityResponse changeNumber(@Auth final AuthenticatedDevice authenticatedDevice,
@NotNull @Valid final ChangeNumberRequest request,
@HeaderParam(HttpHeaders.USER_AGENT) final String userAgentString,
service/src/main/java/org/whispersystems/textsecuregcm/storage/ChangeNumberManager.java
+24 -9
View File
@@ -4,17 +4,22 @@
*/
package org.whispersystems.textsecuregcm.storage;
import java.time.Clock;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.UUID;
import java.util.stream.Collectors;
import static org.whispersystems.textsecuregcm.metrics.MetricsUtil.name;
import com.google.common.net.HttpHeaders;
import io.micrometer.core.instrument.Metrics;
import io.micrometer.core.instrument.Tag;
import io.micrometer.core.instrument.Tags;
import jakarta.ws.rs.container.ContainerRequestContext;
import java.time.Clock;
import java.time.Duration;
import java.time.Instant;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.UUID;
import java.util.stream.Collectors;
import javax.annotation.Nullable;
import org.signal.libsignal.protocol.IdentityKey;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -36,9 +41,6 @@ import org.whispersystems.textsecuregcm.metrics.UserAgentTagUtil;
import org.whispersystems.textsecuregcm.push.MessageSender;
import org.whispersystems.textsecuregcm.push.MessageTooLargeException;
import org.whispersystems.textsecuregcm.util.Pair;
import javax.annotation.Nullable;
import static org.whispersystems.textsecuregcm.metrics.MetricsUtil.name;
public class ChangeNumberManager {
@@ -46,6 +48,7 @@ public class ChangeNumberManager {
private final AccountsManager accountsManager;
private final PhoneVerificationTokenManager phoneVerificationTokenManager;
private final RegistrationLockVerificationManager registrationLockVerificationManager;
private final Duration postRegistrationWaitingPeriod;
private final RateLimiters rateLimiters;
private final Clock clock;
@@ -54,6 +57,8 @@ public class ChangeNumberManager {
// Note that this counter name references another class deliberately in the interest of metric continuity
private static final String CHANGE_NUMBER_COUNTER_NAME = name(AccountControllerV2.class, "changeNumber");
private static final String VERIFICATION_TYPE_TAG_NAME = "verification";
private static final String POST_REGISTRATION_WAITING_PERIOD_NOT_MET_COUNTER_NAME = name(ChangeNumberManager.class,
"postRegistrationWaitingPeriodNotMet");
public ChangeNumberManager(
final MessageSender messageSender,
@@ -61,6 +66,7 @@ public class ChangeNumberManager {
final PhoneVerificationTokenManager phoneVerificationTokenManager,
final RegistrationLockVerificationManager registrationLockVerificationManager,
final RateLimiters rateLimiters,
final Duration postRegistrationWaitingPeriod,
final Clock clock) {
this.messageSender = messageSender;
@@ -68,6 +74,7 @@ public class ChangeNumberManager {
this.phoneVerificationTokenManager = phoneVerificationTokenManager;
this.registrationLockVerificationManager = registrationLockVerificationManager;
this.rateLimiters = rateLimiters;
this.postRegistrationWaitingPeriod = postRegistrationWaitingPeriod;
this.clock = clock;
}
@@ -93,6 +100,14 @@ public class ChangeNumberManager {
// Only verify and check reglock if there's a data change to be made...
if (!account.getNumber().equals(number)) {
final Instant registration = Instant.ofEpochMilli(account.getPrimaryDevice().getCreated());
final Duration waitingPeriodRemaining = Duration.between(clock.instant().minus(postRegistrationWaitingPeriod), registration);
if (waitingPeriodRemaining.isPositive()) {
Metrics.counter(POST_REGISTRATION_WAITING_PERIOD_NOT_MET_COUNTER_NAME).increment();
throw new RateLimitExceededException(waitingPeriodRemaining);
}
rateLimiters.getRegistrationLimiter().validate(number);
final PhoneVerificationRequest.VerificationType verificationType =
service/src/test/java/org/whispersystems/textsecuregcm/storage/ChangeNumberManagerTest.java
+95 -1
View File
@@ -4,6 +4,7 @@
*/
package org.whispersystems.textsecuregcm.storage;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyByte;
@@ -22,6 +23,8 @@ import jakarta.ws.rs.WebApplicationException;
import jakarta.ws.rs.container.ContainerRequestContext;
import java.time.Duration;
import java.time.Instant;
import java.time.temporal.ChronoUnit;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
@@ -30,6 +33,10 @@ import java.util.Optional;
import java.util.UUID;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.function.Executable;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.MethodSource;
import org.mockito.stubbing.Answer;
import org.signal.libsignal.protocol.IdentityKey;
import org.signal.libsignal.protocol.ecc.ECKeyPair;
@@ -67,6 +74,14 @@ public class ChangeNumberManagerTest {
private static final TestClock CLOCK = TestClock.pinned(Instant.now());
private static final Duration POST_REGISTRATION_WAITING_PERIOD = Duration.ofHours(2);
private static final Device DEFAULT_PRIMARY_DEVICE;
static {
DEFAULT_PRIMARY_DEVICE = new Device();
DEFAULT_PRIMARY_DEVICE.setId((byte) 1);
DEFAULT_PRIMARY_DEVICE.setCreated(CLOCK.instant().minus(POST_REGISTRATION_WAITING_PERIOD).minusSeconds(1).toEpochMilli());
}
@BeforeEach
void setUp() throws Exception {
messageSender = mock(MessageSender.class);
@@ -87,7 +102,8 @@ public class ChangeNumberManagerTest {
when(rateLimiters.getRegistrationLimiter()).thenReturn(rateLimiter);
changeNumberManager = new ChangeNumberManager(messageSender, accountsManager,
phoneVerificationTokenManager, registrationLockVerificationManager, rateLimiters, CLOCK);
phoneVerificationTokenManager, registrationLockVerificationManager, rateLimiters,
POST_REGISTRATION_WAITING_PERIOD, CLOCK);
updatedPhoneNumberIdentifiersByAccount = new HashMap<>();
@@ -100,6 +116,7 @@ public class ChangeNumberManagerTest {
final UUID uuid = account.getIdentifier(IdentityType.ACI);
final List<Device> devices = account.getDevices();
final Device primaryDevice = account.getPrimaryDevice();
final UUID updatedPni = UUID.randomUUID();
updatedPhoneNumberIdentifiersByAccount.put(account, updatedPni);
@@ -113,6 +130,7 @@ public class ChangeNumberManagerTest {
when(updatedAccount.getNumber()).thenReturn(number);
when(updatedAccount.getDevices()).thenReturn(devices);
when(updatedAccount.getDevice(anyByte())).thenReturn(Optional.empty());
when(updatedAccount.getPrimaryDevice()).thenReturn(primaryDevice);
account.getDevices().forEach(device ->
when(updatedAccount.getDevice(device.getId())).thenReturn(Optional.of(device)));
@@ -145,6 +163,7 @@ public class ChangeNumberManagerTest {
when(account.getIdentifier(IdentityType.ACI)).thenReturn(accountIdentifier);
when(account.isIdentifiedBy(any())).thenReturn(false);
when(account.isIdentifiedBy(new AciServiceIdentifier(accountIdentifier))).thenReturn(true);
when(account.getPrimaryDevice()).thenReturn(DEFAULT_PRIMARY_DEVICE);
when(accountsManager.getAccountsForChangeNumber(eq(accountIdentifier), any()))
.thenReturn(new Pair<>(account, Optional.empty()));
@@ -187,6 +206,7 @@ public class ChangeNumberManagerTest {
when(account.getDevice(primaryDeviceId)).thenReturn(Optional.of(primaryDevice));
when(account.getDevice(linkedDeviceId)).thenReturn(Optional.of(linkedDevice));
when(account.getDevices()).thenReturn(List.of(primaryDevice, linkedDevice));
when(account.getPrimaryDevice()).thenReturn(DEFAULT_PRIMARY_DEVICE);
final ECKeyPair pniIdentityKeyPair = ECKeyPair.generate();
final IdentityKey pniIdentityKey = new IdentityKey(pniIdentityKeyPair.getPublicKey());
@@ -269,6 +289,7 @@ public class ChangeNumberManagerTest {
when(account.getIdentifier(IdentityType.ACI)).thenReturn(accountIdentifier);
when(account.isIdentifiedBy(any())).thenReturn(false);
when(account.isIdentifiedBy(new AciServiceIdentifier(accountIdentifier))).thenReturn(true);
when(account.getPrimaryDevice()).thenReturn(DEFAULT_PRIMARY_DEVICE);
when(accountsManager.getAccountsForChangeNumber(eq(accountIdentifier), any()))
.thenReturn(new Pair<>(account, Optional.empty()));
@@ -308,6 +329,7 @@ public class ChangeNumberManagerTest {
when(account.getIdentifier(IdentityType.ACI)).thenReturn(accountIdentifier);
when(account.isIdentifiedBy(any())).thenReturn(false);
when(account.isIdentifiedBy(new AciServiceIdentifier(accountIdentifier))).thenReturn(true);
when(account.getPrimaryDevice()).thenReturn(DEFAULT_PRIMARY_DEVICE);
when(accountsManager.getAccountsForChangeNumber(eq(accountIdentifier), any()))
.thenReturn(new Pair<>(account, Optional.empty()));
@@ -356,6 +378,7 @@ public class ChangeNumberManagerTest {
when(account.getIdentifier(IdentityType.ACI)).thenReturn(accountIdentifier);
when(account.isIdentifiedBy(any())).thenReturn(false);
when(account.isIdentifiedBy(new AciServiceIdentifier(accountIdentifier))).thenReturn(true);
when(account.getPrimaryDevice()).thenReturn(DEFAULT_PRIMARY_DEVICE);
final Account existingAccount = mock(Account.class);
when(existingAccount.getNumber()).thenReturn(targetNumber);
@@ -381,4 +404,75 @@ public class ChangeNumberManagerTest {
verify(accountsManager, never()).changeNumber(any(), any(), any(), any(), any(), any());
verify(messageSender, never()).sendMessages(any(), any(), any(), any(), any(), any());
}
@ParameterizedTest
@MethodSource
void testRecentRegistration(final boolean expectRateLimited, final boolean sameNumber, final Instant registrationInstant) throws Throwable {
final String originalNumber = PhoneNumberUtil.getInstance().format(
PhoneNumberUtil.getInstance().getExampleNumber("DE"), PhoneNumberUtil.PhoneNumberFormat.E164);
final String targetNumber = sameNumber
? originalNumber
: PhoneNumberUtil.getInstance().format(
PhoneNumberUtil.getInstance().getExampleNumber("US"), PhoneNumberUtil.PhoneNumberFormat.E164);
final ECKeyPair pniIdentityKeyPair = ECKeyPair.generate();
final IdentityKey pniIdentityKey = new IdentityKey(ECKeyPair.generate().getPublicKey());
final Map<Byte, ECSignedPreKey> ecSignedPreKeys =
Map.of(Device.PRIMARY_ID, KeysHelper.signedECPreKey(1, pniIdentityKeyPair));
final Map<Byte, KEMSignedPreKey> kemLastResortPreKeys =
Map.of(Device.PRIMARY_ID, KeysHelper.signedKEMPreKey(2, pniIdentityKeyPair));
final UUID accountIdentifier = UUID.randomUUID();
final Account account = mock(Account.class);
when(account.getNumber()).thenReturn(originalNumber);
when(account.getIdentifier(IdentityType.ACI)).thenReturn(accountIdentifier);
when(account.isIdentifiedBy(any())).thenReturn(false);
when(account.isIdentifiedBy(new AciServiceIdentifier(accountIdentifier))).thenReturn(true);
final Device primaryDevice = mock(Device.class);
when(account.getPrimaryDevice()).thenReturn(primaryDevice);
when(primaryDevice.getCreated()).thenReturn(registrationInstant.toEpochMilli());
when(accountsManager.getAccountsForChangeNumber(eq(accountIdentifier), any()))
.thenReturn(new Pair<>(account, Optional.empty()));
final Executable changeNumberOperation = () -> changeNumberManager.changeNumber(accountIdentifier,
null,
null,
null,
targetNumber,
pniIdentityKey,
ecSignedPreKeys,
kemLastResortPreKeys,
Collections.emptyList(),
Collections.emptyMap(),
mock(ContainerRequestContext.class));
if (expectRateLimited) {
final RateLimitExceededException e = assertThrows(RateLimitExceededException.class, changeNumberOperation);
assertEquals(Duration.between(CLOCK.instant().minus(POST_REGISTRATION_WAITING_PERIOD), registrationInstant), e.getRetryDuration().orElseThrow());
} else {
changeNumberOperation.execute();
verify(accountsManager).changeNumber(accountIdentifier, targetNumber, pniIdentityKey, ecSignedPreKeys, kemLastResortPreKeys, Collections.emptyMap());
}
}
static Collection<Arguments> testRecentRegistration() {
// truncate to millis because that is the resolution for device.created
final Instant tooRecent = CLOCK.instant().minus(POST_REGISTRATION_WAITING_PERIOD).plusSeconds(1)
.truncatedTo(ChronoUnit.MILLIS);
final Instant outsideWaitingPeriod = CLOCK.instant().minus(POST_REGISTRATION_WAITING_PERIOD).minusSeconds(1)
.truncatedTo(ChronoUnit.MILLIS);
return List.of(
// expect exception, same number, registration instant
Arguments.argumentSet("waiting period elapsed", false, false, outsideWaitingPeriod),
Arguments.argumentSet("waiting period not elapsed", true, false, tooRecent),
Arguments.argumentSet("waiting period not elapsed; same number", false, true, tooRecent)
);
}
}