MirrorMirror
/
Signal-Server
mirror of https://github.com/signalapp/Signal-Server.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Update AccountsManager#changeNumber validation to check for PNI equality

This commit is contained in:
Chris Eager 2024-11-22 14:35:18 -06:00 committed by Chris Eager
parent cba56f3263
commit e4a2af67b1
2 changed files with 20 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
service/src/main/java/org/whispersystems/textsecuregcm/storage/AccountsManager.java
View File

@ -637,10 +637,10 @@ public class AccountsManager extends RedisPubSubAdapter<String, String> implemen
@Nullable final Map<Byte, KEMSignedPreKey> pniPqLastResortPreKeys, @Nullable final Map<Byte, KEMSignedPreKey> pniPqLastResortPreKeys,
@Nullable final Map<Byte, Integer> pniRegistrationIds) throws InterruptedException, MismatchedDevicesException { @Nullable final Map<Byte, Integer> pniRegistrationIds) throws InterruptedException, MismatchedDevicesException {
final String originalNumber = account.getNumber();
final UUID originalPhoneNumberIdentifier = account.getPhoneNumberIdentifier(); final UUID originalPhoneNumberIdentifier = account.getPhoneNumberIdentifier();
final UUID targetPhoneNumberIdentifier = phoneNumberIdentifiers.getPhoneNumberIdentifier(targetNumber).join();
if (originalNumber.equals(targetNumber)) { if (originalPhoneNumberIdentifier.equals(targetPhoneNumberIdentifier)) {
if (pniIdentityKey != null) { if (pniIdentityKey != null) {
throw new IllegalArgumentException("change number must supply a changed phone number; otherwise use updatePniKeys"); throw new IllegalArgumentException("change number must supply a changed phone number; otherwise use updatePniKeys");
} }
@ -650,7 +650,6 @@ public class AccountsManager extends RedisPubSubAdapter<String, String> implemen
validateDevices(account, pniSignedPreKeys, pniPqLastResortPreKeys, pniRegistrationIds); validateDevices(account, pniSignedPreKeys, pniPqLastResortPreKeys, pniRegistrationIds);
final AtomicReference<Account> updatedAccount = new AtomicReference<>(); final AtomicReference<Account> updatedAccount = new AtomicReference<>();
final UUID targetPhoneNumberIdentifier = phoneNumberIdentifiers.getPhoneNumberIdentifier(targetNumber).join();
accountLockManager.withLock(List.of(account.getPhoneNumberIdentifier(), targetPhoneNumberIdentifier), () -> { accountLockManager.withLock(List.of(account.getPhoneNumberIdentifier(), targetPhoneNumberIdentifier), () -> {
redisDelete(account); redisDelete(account);

18
service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerTest.java
View File

@ -1054,17 +1054,35 @@ class AccountsManagerTest {
final String number = "+14152222222"; final String number = "+14152222222";
Account account = AccountsHelper.generateTestAccount(number, UUID.randomUUID(), UUID.randomUUID(), new ArrayList<>(), new byte[UnidentifiedAccessUtil.UNIDENTIFIED_ACCESS_KEY_LENGTH]); Account account = AccountsHelper.generateTestAccount(number, UUID.randomUUID(), UUID.randomUUID(), new ArrayList<>(), new byte[UnidentifiedAccessUtil.UNIDENTIFIED_ACCESS_KEY_LENGTH]);
phoneNumberIdentifiersByE164.put(number, account.getPhoneNumberIdentifier());
account = accountsManager.changeNumber(account, number, null, null, null, null); account = accountsManager.changeNumber(account, number, null, null, null, null);
assertEquals(number, account.getNumber()); assertEquals(number, account.getNumber());
verify(keysManager, never()).deleteSingleUsePreKeys(any()); verify(keysManager, never()).deleteSingleUsePreKeys(any());
} }
@Test
void testChangePhoneNumberDifferentNumberSamePni() throws InterruptedException, MismatchedDevicesException {
final String originalNumber = "+22923456789";
// the canonical form of numbers may change over time, so we use PNIs as stable identifiers
final String newNumber = "+2290123456789";
Account account = AccountsHelper.generateTestAccount(originalNumber, UUID.randomUUID(), UUID.randomUUID(),
new ArrayList<>(), new byte[UnidentifiedAccessUtil.UNIDENTIFIED_ACCESS_KEY_LENGTH]);
phoneNumberIdentifiersByE164.put(originalNumber, account.getPhoneNumberIdentifier());
phoneNumberIdentifiersByE164.put(newNumber, account.getPhoneNumberIdentifier());
account = accountsManager.changeNumber(account, newNumber, null, null, null, null);
assertEquals(originalNumber, account.getNumber());
verify(keysManager, never()).deleteSingleUsePreKeys(any());
}
@Test @Test
void testChangePhoneNumberSameNumberWithPniData() { void testChangePhoneNumberSameNumberWithPniData() {
final String number = "+14152222222"; final String number = "+14152222222";
Account account = AccountsHelper.generateTestAccount(number, UUID.randomUUID(), UUID.randomUUID(), new ArrayList<>(), new byte[UnidentifiedAccessUtil.UNIDENTIFIED_ACCESS_KEY_LENGTH]); Account account = AccountsHelper.generateTestAccount(number, UUID.randomUUID(), UUID.randomUUID(), new ArrayList<>(), new byte[UnidentifiedAccessUtil.UNIDENTIFIED_ACCESS_KEY_LENGTH]);
phoneNumberIdentifiersByE164.put(number, account.getPhoneNumberIdentifier());
final ECKeyPair pniIdentityKeyPair = Curve.generateKeyPair(); final ECKeyPair pniIdentityKeyPair = Curve.generateKeyPair();
assertThrows(IllegalArgumentException.class, assertThrows(IllegalArgumentException.class,
() -> accountsManager.changeNumber( () -> accountsManager.changeNumber(