From dd76bae853f1a5d4e804c78971ba49540d9c3dcc Mon Sep 17 00:00:00 2001 From: Greyson Parrelli Date: Tue, 4 Aug 2026 15:57:33 -0400 Subject: [PATCH] Ensure that backupSecretRestoreRequired is properly set. --- .../v2/AppRegistrationStorageController.kt | 10 ++++++ .../AppRegistrationStorageControllerTest.kt | 33 +++++++++++++++++-- .../registration/RegistrationRepository.kt | 1 + .../src/main/protowire/Registration.proto | 3 ++ .../registration/RegistrationEndToEndTest.kt | 2 ++ 5 files changed, 47 insertions(+), 2 deletions(-) diff --git a/app/src/main/java/org/thoughtcrime/securesms/registration/v2/AppRegistrationStorageController.kt b/app/src/main/java/org/thoughtcrime/securesms/registration/v2/AppRegistrationStorageController.kt index c46aaed71e..5afcef510d 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/registration/v2/AppRegistrationStorageController.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/registration/v2/AppRegistrationStorageController.kt @@ -578,6 +578,9 @@ class AppRegistrationStorageController(private val context: Context) : StorageCo try { when (val result = BackupRepository.restoreRemoteBackup()) { is RemoteRestoreResult.Success -> { + // The restore adopted the account's existing SVRB chain, so there's nothing left for BackupMessagesJob to re-init. + SignalStore.backup.backupSecretRestoreRequired = false + send( RemoteBackupRestoreProgress.Complete( restoredSvrPin = SignalStore.svr.pin, @@ -761,6 +764,13 @@ class AppRegistrationStorageController(private val context: Context) : StorageCo SignalStore.backup.mediaRootBackupKey = MediaRootBackupKey(it.toByteArray()) } + if (accountData.reRegistration) { + // The account may already have SVRB data for this backup key. Until we've adopted that chain (either by restoring a backup, or by reading + // the remote metadata during the first BackupMessagesJob) we must not start a new chain, or the existing remote backup becomes unrestorable. + Log.i(TAG, "[applyAccountData] Re-registration. Marking that we need to restore SVRB secrets before backing up.") + SignalStore.backup.backupSecretRestoreRequired = true + } + SignalStore.account.setServicePassword(accountData.servicePassword) SignalStore.account.setRegistered(registered = true, isAciChanged = isAciChanged) TextSecurePreferences.setPromptedPushRegistration(context, true) diff --git a/app/src/test/java/org/thoughtcrime/securesms/registration/v2/AppRegistrationStorageControllerTest.kt b/app/src/test/java/org/thoughtcrime/securesms/registration/v2/AppRegistrationStorageControllerTest.kt index 080064e3b0..699325088f 100644 --- a/app/src/test/java/org/thoughtcrime/securesms/registration/v2/AppRegistrationStorageControllerTest.kt +++ b/app/src/test/java/org/thoughtcrime/securesms/registration/v2/AppRegistrationStorageControllerTest.kt @@ -230,6 +230,34 @@ class AppRegistrationStorageControllerTest { assertThat(SignalStore.svr.hasOptedOut()).isTrue() } + @Test + fun `commit - re-registration - requires svrb secret restore before backing up`() = runBlocking { + seedInProgressData( + RegistrationData( + accountData = accountData(reRegistration = true), + accountEntropyPool = aep.value + ) + ) + + controller.commitRegistrationData() + + assertThat(SignalStore.backup.backupSecretRestoreRequired).isTrue() + } + + @Test + fun `commit - new account - does not require svrb secret restore`() = runBlocking { + seedInProgressData( + RegistrationData( + accountData = accountData(reRegistration = false), + accountEntropyPool = aep.value + ) + ) + + controller.commitRegistrationData() + + assertThat(SignalStore.backup.backupSecretRestoreRequired).isFalse() + } + @Test fun `commit - called twice - only applies account data once`() = runBlocking { seedInProgressData( @@ -396,7 +424,7 @@ class AppRegistrationStorageControllerTest { private fun readInProgressData(): RegistrationData = runBlocking { controller.readInProgressRegistrationData() } - private fun accountData(servicePassword: String = SERVICE_PASSWORD, linkedDeviceData: LinkedDeviceData? = null): AccountData { + private fun accountData(servicePassword: String = SERVICE_PASSWORD, linkedDeviceData: LinkedDeviceData? = null, reRegistration: Boolean = false): AccountData { return AccountData( aciIdentityKeyPair = aciIdentity.serialize().toByteString(), pniIdentityKeyPair = pniIdentity.serialize().toByteString(), @@ -410,7 +438,8 @@ class AppRegistrationStorageControllerTest { pni = pni.toString(), e164 = E164, servicePassword = servicePassword, - linkedDeviceData = linkedDeviceData + linkedDeviceData = linkedDeviceData, + reRegistration = reRegistration ) } } diff --git a/feature/registration/src/main/java/org/signal/registration/RegistrationRepository.kt b/feature/registration/src/main/java/org/signal/registration/RegistrationRepository.kt index fbb4dc2fa8..d6912607a0 100644 --- a/feature/registration/src/main/java/org/signal/registration/RegistrationRepository.kt +++ b/feature/registration/src/main/java/org/signal/registration/RegistrationRepository.kt @@ -683,6 +683,7 @@ class RegistrationRepository(val context: Context, val networkController: Networ this.aci = result.result.aci this.pni = result.result.pni this.servicePassword = keyMaterial.servicePassword + this.reRegistration = result.result.reregistration } storageController.commitRegistrationData() } diff --git a/feature/registration/src/main/protowire/Registration.proto b/feature/registration/src/main/protowire/Registration.proto index cc25c0ed9b..272e4aa864 100644 --- a/feature/registration/src/main/protowire/Registration.proto +++ b/feature/registration/src/main/protowire/Registration.proto @@ -45,6 +45,9 @@ message AccountData { bool fetchesMessages = 14; LinkedDeviceData linkedDeviceData = 15; + + // Whether the service reported this registration as a re-registration of an existing account. + bool reRegistration = 16; } message SvrCredential { diff --git a/feature/registration/src/test/java/org/signal/registration/RegistrationEndToEndTest.kt b/feature/registration/src/test/java/org/signal/registration/RegistrationEndToEndTest.kt index a179f5fcb6..1da8dc3a2d 100644 --- a/feature/registration/src/test/java/org/signal/registration/RegistrationEndToEndTest.kt +++ b/feature/registration/src/test/java/org/signal/registration/RegistrationEndToEndTest.kt @@ -141,6 +141,7 @@ class RegistrationEndToEndTest { assert(committed.accountData?.pni?.isNotEmpty() == true) { "Expected committed PNI to be populated" } assert(committed.pin == PIN) { "Expected committed pin $PIN but was ${committed.pin}" } assert(committed.accountEntropyPool.isNotEmpty()) { "Expected committed AEP to be populated" } + assert(committed.accountData?.reRegistration == false) { "Expected a new registration to not be flagged as a re-registration" } assert(networkController.lastCreateSessionE164 == E164) { "Expected a session for $E164 but was ${networkController.lastCreateSessionE164}" } assert(networkController.lastRegisterAccountRequest?.e164 == E164) { "Expected registration for $E164 but was ${networkController.lastRegisterAccountRequest}" } @@ -643,6 +644,7 @@ class RegistrationEndToEndTest { assert(committed != null) { "Expected registration data to be committed" } assert(committed!!.accountData?.e164 == E164) { "Expected committed e164 $E164 but was ${committed.accountData?.e164}" } assert(committed.pin == PIN) { "Expected committed pin $PIN but was ${committed.pin}" } + assert(committed.accountData?.reRegistration == true) { "Expected the committed account data to be flagged as a re-registration" } } @Test