From b29bae50afb6ddc18d5bc79dba1b091607e0de3b Mon Sep 17 00:00:00 2001 From: Cody Henthorne Date: Mon, 29 Jun 2026 09:45:36 -0400 Subject: [PATCH] Attempt to recover from bad change number state. --- .../jobs/AccountConsistencyWorkerJob.kt | 19 ++++-- .../migrations/ApplicationMigrations.java | 7 +- .../FixChangeNumberErrorMigrationJob.kt | 68 +++++++++++++++++-- 3 files changed, 79 insertions(+), 15 deletions(-) diff --git a/app/src/main/java/org/thoughtcrime/securesms/jobs/AccountConsistencyWorkerJob.kt b/app/src/main/java/org/thoughtcrime/securesms/jobs/AccountConsistencyWorkerJob.kt index 9f0acffeaf..6a273cb7a2 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/jobs/AccountConsistencyWorkerJob.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/jobs/AccountConsistencyWorkerJob.kt @@ -67,9 +67,7 @@ class AccountConsistencyWorkerJob private constructor(parameters: Parameters) : if (aciProfile.identityKey != encodedAciPublicKey) { Log.w(TAG, "ACI identity key on profile differed from the one we have locally! Marking ourselves unregistered.") - SignalStore.account.setRegistered(false) - SignalStore.registration.clearRegistrationComplete() - SignalStore.registration.hasUploadedProfile = false + markUnregistered() SignalStore.misc.lastConsistencyCheckTime = System.currentTimeMillis() return @@ -79,11 +77,11 @@ class AccountConsistencyWorkerJob private constructor(parameters: Parameters) : val encodedPniPublicKey = Base64.encodeWithPadding(SignalStore.account.pniIdentityKey.publicKey.serialize()) if (pniProfile.identityKey != encodedPniPublicKey) { - Log.w(TAG, "PNI identity key on profile differed from the one we have locally!") + Log.w(TAG, "PNI identity key on profile differed from the one we have locally! Marking ourselves unregistered.") - SignalStore.account.setRegistered(false) - SignalStore.registration.clearRegistrationComplete() - SignalStore.registration.hasUploadedProfile = false + markUnregistered() + + SignalStore.misc.lastConsistencyCheckTime = System.currentTimeMillis() return } @@ -92,6 +90,13 @@ class AccountConsistencyWorkerJob private constructor(parameters: Parameters) : SignalStore.misc.lastConsistencyCheckTime = System.currentTimeMillis() } + /** Marks the account unregistered so the user is prompted to re-register. */ + private fun markUnregistered() { + SignalStore.account.setRegistered(false) + SignalStore.registration.clearRegistrationComplete() + SignalStore.registration.hasUploadedProfile = false + } + override fun onShouldRetry(e: Exception): Boolean { return e is IOException } diff --git a/app/src/main/java/org/thoughtcrime/securesms/migrations/ApplicationMigrations.java b/app/src/main/java/org/thoughtcrime/securesms/migrations/ApplicationMigrations.java index 15d7b7464b..f9521b97c7 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/migrations/ApplicationMigrations.java +++ b/app/src/main/java/org/thoughtcrime/securesms/migrations/ApplicationMigrations.java @@ -206,9 +206,10 @@ public class ApplicationMigrations { static final int NOTIFICATION_INDEX_MIGRATION = 162; static final int NOTIFICATION_STATE_CLEANUP = 163; static final int KT_USERNAME_CAPABILITY = 164; + static final int FIX_CHANGE_NUMBER_ERROR_2 = 165; } - public static final int CURRENT_VERSION = 164; + public static final int CURRENT_VERSION = 165; /** * This *must* be called after the {@link JobManager} has been instantiated, but *before* the call @@ -847,6 +848,10 @@ public class ApplicationMigrations { jobs.put(Version.FIX_CHANGE_NUMBER_ERROR, new FixChangeNumberErrorMigrationJob()); } + if (lastSeenVersion < Version.FIX_CHANGE_NUMBER_ERROR_2) { + jobs.put(Version.FIX_CHANGE_NUMBER_ERROR_2, new FixChangeNumberErrorMigrationJob()); + } + if (lastSeenVersion < Version.CHAT_FOLDER_STORAGE_SYNC) { jobs.put(Version.CHAT_FOLDER_STORAGE_SYNC, new SyncChatFoldersMigrationJob()); } diff --git a/app/src/main/java/org/thoughtcrime/securesms/migrations/FixChangeNumberErrorMigrationJob.kt b/app/src/main/java/org/thoughtcrime/securesms/migrations/FixChangeNumberErrorMigrationJob.kt index c37c775c45..94a2262d05 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/migrations/FixChangeNumberErrorMigrationJob.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/migrations/FixChangeNumberErrorMigrationJob.kt @@ -1,18 +1,27 @@ package org.thoughtcrime.securesms.migrations +import kotlinx.coroutines.runBlocking import org.signal.core.models.ServiceId +import org.signal.core.util.Base64 import org.signal.core.util.logging.Log +import org.signal.libsignal.protocol.IdentityKey +import org.signal.libsignal.protocol.IdentityKeyPair import org.signal.network.NetworkResult import org.thoughtcrime.securesms.components.settings.app.changenumber.ChangeNumberRepository +import org.thoughtcrime.securesms.database.model.databaseprotos.PendingChangeNumberMetadata +import org.thoughtcrime.securesms.dependencies.AppDependencies import org.thoughtcrime.securesms.jobmanager.Job +import org.thoughtcrime.securesms.jobs.AccountConsistencyWorkerJob import org.thoughtcrime.securesms.keyvalue.SignalStore import org.thoughtcrime.securesms.net.SignalNetwork import org.whispersystems.signalservice.internal.push.WhoAmIResponse import java.io.IOException /** - * There was a server error during change number where a number was changed but gave back a 409 response. - * We need devices to re-fetch their E164+PNI's, save them, and then get prekeys. + * A server-side bug during change number can commit the number/PNI change but return an error, leaving the client desynced + * (local still on the old number/PNI). This detects that via whoami and reconciles the local number/PNI. Before adopting + * the PNI identity from the pending metadata, it verifies that key against the server's published PNI identity, so + * a stale/overwritten metadata key is never blindly applied. */ internal class FixChangeNumberErrorMigrationJob( parameters: Parameters = Parameters.Builder().build() @@ -46,14 +55,22 @@ internal class FixChangeNumberErrorMigrationJob( when (val result = SignalNetwork.account.whoAmI()) { is NetworkResult.Success -> { - val pni = result.result.pni?.let { ServiceId.PNI.parseOrNull(it) } ?: return + val serverPni = result.result.pni?.let { ServiceId.PNI.parseOrNull(it) } ?: return - if (result.result.number != SignalStore.account.e164 || pni != SignalStore.account.pni) { - Log.w(TAG, "Detected a number or PNI mismatch! Fixing...") - ChangeNumberRepository().changeLocalNumber(result.result.number, pni) + if (result.result.number == SignalStore.account.e164 && serverPni == SignalStore.account.pni) { + Log.i(TAG, "No number or PNI mismatch detected.") + return + } + + Log.w(TAG, "Detected a number or PNI mismatch! Verifying PNI identity key against the server before fixing...") + + if (pendingPniIdentityMatchesServer(pendingChangeNumberMetadata, serverPni)) { + Log.w(TAG, "PNI identity key matches server. Fixing local number/PNI...") + ChangeNumberRepository().changeLocalNumber(result.result.number, serverPni) Log.w(TAG, "Done!") } else { - Log.i(TAG, "No number or PNI mismatch detected.") + Log.w(TAG, "Server PNI identity does not match pending metadata (or could not be verified); cannot safely reconcile. Enqueuing AccountConsistencyWorkerJob.") + AppDependencies.jobManager.add(AccountConsistencyWorkerJob()) return } } @@ -63,6 +80,43 @@ internal class FixChangeNumberErrorMigrationJob( } } + private fun pendingPniIdentityMatchesServer(metadata: PendingChangeNumberMetadata, pni: ServiceId.PNI): Boolean { + val metadataIdentityKey: IdentityKey = try { + IdentityKeyPair(metadata.pniIdentityKeyPair.toByteArray()).publicKey + } catch (e: Exception) { + Log.w(TAG, "Could not parse PNI identity key from pending metadata.", e) + return false + } + + val serverIdentityKey: IdentityKey = when (val profileResult = runBlocking { SignalNetwork.profile.getUnversionedProfile(pni, null) }) { + is NetworkResult.Success -> { + val identityKey = profileResult.result.identityKey + if (identityKey == null) { + Log.w(TAG, "Server profile for PNI has no identity key; cannot verify.") + return false + } + try { + IdentityKey(Base64.decode(identityKey), 0) + } catch (e: Exception) { + Log.w(TAG, "Could not parse server PNI identity key.", e) + return false + } + } + is NetworkResult.NetworkError -> throw profileResult.exception + is NetworkResult.StatusCodeError -> { + if (profileResult.code == 404) { + Log.w(TAG, "Could not fetch server profile for PNI (code=${profileResult.code}); cannot verify identity key.") + return false + } else { + throw profileResult.exception + } + } + is NetworkResult.ApplicationError -> throw profileResult.throwable + } + + return serverIdentityKey.serialize().contentEquals(metadataIdentityKey.serialize()) + } + override fun shouldRetry(e: Exception): Boolean = e is IOException class Factory : Job.Factory {