From e6e869e07451abcd63971811c5807190fb7f2972 Mon Sep 17 00:00:00 2001 From: Cody Henthorne Date: Fri, 1 Aug 2025 13:31:29 -0400 Subject: [PATCH] Fix identity key update failure when profile key verification failed. --- .../securesms/backup/v2/BackupRepository.kt | 2 +- .../conversation/v2/ConversationViewModel.kt | 2 +- .../storage/SignalBaseIdentityKeyStore.java | 2 +- .../securesms/database/RecipientTable.kt | 31 +++++++++++++- .../securesms/jobs/IndividualSendJob.java | 2 +- .../securesms/jobs/PushGroupSendJob.java | 2 +- .../securesms/jobs/RetrieveProfileJob.kt | 42 ++++++++++++------- .../messages/DataMessageProcessor.kt | 2 +- .../profiles/manage/EditProfileViewModel.java | 4 +- .../service/webrtc/SignalCallManager.java | 6 +-- .../storage/ContactRecordProcessor.kt | 2 +- .../reply/group/StoryGroupReplyFragment.kt | 2 +- .../api/profiles/ProfileRepository.kt | 12 ++++-- 13 files changed, 79 insertions(+), 32 deletions(-) diff --git a/app/src/main/java/org/thoughtcrime/securesms/backup/v2/BackupRepository.kt b/app/src/main/java/org/thoughtcrime/securesms/backup/v2/BackupRepository.kt index 88e267c779..3e1116723e 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/backup/v2/BackupRepository.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/backup/v2/BackupRepository.kt @@ -1241,7 +1241,7 @@ object BackupRepository { recipientSet } - RetrieveProfileJob.enqueue(recipientIds) + RetrieveProfileJob.enqueue(recipientIds, skipDebounce = false) AppDependencies.jobManager.add(CreateReleaseChannelJob.create()) diff --git a/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/ConversationViewModel.kt b/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/ConversationViewModel.kt index 970cafd551..2ec7e40f03 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/ConversationViewModel.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/ConversationViewModel.kt @@ -255,7 +255,7 @@ class ConversationViewModel( .conversationRecipient .filter { it.isRegistered } .take(1) - .subscribeBy { RetrieveProfileJob.enqueue(it.id) } + .subscribeBy { RetrieveProfileJob.enqueue(it.id, skipDebounce = false) } .addTo(disposables) disposables += recipientRepository diff --git a/app/src/main/java/org/thoughtcrime/securesms/crypto/storage/SignalBaseIdentityKeyStore.java b/app/src/main/java/org/thoughtcrime/securesms/crypto/storage/SignalBaseIdentityKeyStore.java index c1832b17b1..50c4db2ef2 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/crypto/storage/SignalBaseIdentityKeyStore.java +++ b/app/src/main/java/org/thoughtcrime/securesms/crypto/storage/SignalBaseIdentityKeyStore.java @@ -238,7 +238,7 @@ public class SignalBaseIdentityKeyStore { } if (!identityKey.equals(identityRecord.getIdentityKey())) { - Log.w(TAG, "Identity keys don't match... service: " + identityKey.hashCode() + " database: " + identityRecord.getIdentityKey().hashCode()); + Log.w(TAG, "Identity keys don't match... service: ***" + (identityKey.hashCode() % 100) + " database: ***" + (identityRecord.getIdentityKey().hashCode() % 100)); return false; } diff --git a/app/src/main/java/org/thoughtcrime/securesms/database/RecipientTable.kt b/app/src/main/java/org/thoughtcrime/securesms/database/RecipientTable.kt index 4b349206f4..ee05d55c6a 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/database/RecipientTable.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/database/RecipientTable.kt @@ -508,7 +508,7 @@ open class RecipientTable(context: Context, databaseHelper: SignalDatabase) : Da db.runPostSuccessfulTransaction { if (result.affectedIds.isNotEmpty()) { result.affectedIds.forEach { AppDependencies.databaseObserver.notifyRecipientChanged(it) } - RetrieveProfileJob.enqueue(result.affectedIds) + RetrieveProfileJob.enqueue(result.affectedIds, skipDebounce = true) } if (result.oldIds.isNotEmpty()) { @@ -1653,6 +1653,35 @@ open class RecipientTable(context: Context, databaseHelper: SignalDatabase) : Da return false } + /** + * Clears the profile key. + * + * It clears out the profile key credential and resets the unidentified access mode. + */ + fun clearProfileKeyData(id: RecipientId) { + val selection = "$ID = ?" + val args = arrayOf(id.serialize()) + + val valuesToCompare = contentValuesOf( + PROFILE_KEY to null + ) + + val valuesToSet = contentValuesOf( + PROFILE_KEY to null, + EXPIRING_PROFILE_KEY_CREDENTIAL to null, + SEALED_SENDER_MODE to SealedSenderAccessMode.UNKNOWN.mode, + LAST_PROFILE_FETCH to 0 + ) + + val updateQuery = SqlUtil.buildTrueUpdateQuery(selection, args, valuesToCompare) + + if (update(updateQuery, valuesToSet)) { + rotateStorageId(id) + AppDependencies.databaseObserver.notifyRecipientChanged(id) + StorageSyncHelper.scheduleSyncForDataChange() + } + } + /** * Sets the profile key iff currently null. * diff --git a/app/src/main/java/org/thoughtcrime/securesms/jobs/IndividualSendJob.java b/app/src/main/java/org/thoughtcrime/securesms/jobs/IndividualSendJob.java index 18ed469ded..c68a27bfb9 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/jobs/IndividualSendJob.java +++ b/app/src/main/java/org/thoughtcrime/securesms/jobs/IndividualSendJob.java @@ -221,7 +221,7 @@ public class IndividualSendJob extends PushSendJob { } database.addMismatchedIdentity(messageId, recipient.getId(), uie.getIdentityKey()); database.markAsSentFailed(messageId); - RetrieveProfileJob.enqueue(recipient.getId()); + RetrieveProfileJob.enqueue(recipient.getId(), true); } catch (ProofRequiredException e) { ProofRequiredExceptionHandler.Result result = ProofRequiredExceptionHandler.handle(context, e, SignalDatabase.threads().getRecipientForThreadId(threadId), threadId, messageId); if (result.isRetry()) { diff --git a/app/src/main/java/org/thoughtcrime/securesms/jobs/PushGroupSendJob.java b/app/src/main/java/org/thoughtcrime/securesms/jobs/PushGroupSendJob.java index 567a35a4a2..4d6b64767d 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/jobs/PushGroupSendJob.java +++ b/app/src/main/java/org/thoughtcrime/securesms/jobs/PushGroupSendJob.java @@ -509,7 +509,7 @@ public final class PushGroupSendJob extends PushSendJob { .map(mismatch -> mismatch.getRecipientId()) .collect(Collectors.toSet()); - RetrieveProfileJob.enqueue(mismatchRecipientIds); + RetrieveProfileJob.enqueue(mismatchRecipientIds, true); } else if (!networkFailures.isEmpty()) { long retryAfter = results.stream() .filter(r -> r.getRateLimitFailure() != null) diff --git a/app/src/main/java/org/thoughtcrime/securesms/jobs/RetrieveProfileJob.kt b/app/src/main/java/org/thoughtcrime/securesms/jobs/RetrieveProfileJob.kt index 941763dca8..b3d0243d79 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/jobs/RetrieveProfileJob.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/jobs/RetrieveProfileJob.kt @@ -53,9 +53,9 @@ import kotlin.time.Duration.Companion.minutes /** * Retrieves a users profile and sets the appropriate local fields. */ -class RetrieveProfileJob private constructor(parameters: Parameters, private val recipientIds: MutableSet) : BaseJob(parameters) { - constructor(recipientIds: Set) : this( - Parameters.Builder() +class RetrieveProfileJob private constructor(parameters: Parameters, private val recipientIds: MutableSet, private val skipDebounce: Boolean) : BaseJob(parameters) { + private constructor(recipientIds: Set, skipDebounce: Boolean) : this( + parameters = Parameters.Builder() .addConstraint(NetworkConstraint.KEY) .apply { if (recipientIds.size < 5) { @@ -65,12 +65,14 @@ class RetrieveProfileJob private constructor(parameters: Parameters, private val } .setMaxAttempts(3) .build(), - recipientIds.toMutableSet() + recipientIds = recipientIds.toMutableSet(), + skipDebounce = skipDebounce ) override fun serialize(): ByteArray? { return JsonJobData.Builder() .putStringListAsArray(KEY_RECIPIENTS, recipientIds.map { it.serialize() }) + .putBoolean(KEY_SKIP_DEBOUNCE, skipDebounce) .serialize() } @@ -99,7 +101,7 @@ class RetrieveProfileJob private constructor(parameters: Parameters, private val val currentTime = System.currentTimeMillis() val debounceThreshold = currentTime - PROFILE_FETCH_DEBOUNCE_TIME_MS val recipientsToFetch = recipients.filter { recipient -> - recipient.hasServiceId && recipient.lastProfileFetchTime < debounceThreshold + recipient.hasServiceId && (skipDebounce || recipient.lastProfileFetchTime < debounceThreshold) } if (recipientsToFetch.isEmpty()) { @@ -138,7 +140,7 @@ class RetrieveProfileJob private constructor(parameters: Parameters, private val Log.d(TAG, "Fetched ${localRecords.size} existing records.") stopwatch.split("disk-fetch") - val successIds: Set = recipientIds - response.retryableFailures + val successIds: Set = response.successes.map { it.id }.toSet() val newlyRegisteredIds: Set = response.successes .mapNotNull { recipientsById[it.id] } .filterNot { it.isRegistered } @@ -189,6 +191,14 @@ class RetrieveProfileJob private constructor(parameters: Parameters, private val } stopwatch.split("registered-update") + if (response.verificationFailures.isNotEmpty()) { + Log.i(TAG, "Removing profile keys for ${response.verificationFailures.size} users due to verification errors") + for (recipientId in response.verificationFailures) { + SignalDatabase.recipients.clearProfileKeyData(recipientId) + } + } + stopwatch.split("verification-update") + for (idProfilePair in response.successes) { setIdentityKey(recipientsById[idProfilePair.id]!!, idProfilePair.profileWithCredential.profile.identityKey) } @@ -523,8 +533,9 @@ class RetrieveProfileJob private constructor(parameters: Parameters, private val override fun create(parameters: Parameters, serializedData: ByteArray?): RetrieveProfileJob { val data = JsonJobData.deserialize(serializedData) val recipientIds: MutableSet = data.getStringArray(KEY_RECIPIENTS).map { RecipientId.from(it) }.toMutableSet() + val skipDebounce: Boolean = data.getBooleanOrDefault(KEY_SKIP_DEBOUNCE, false) - return RetrieveProfileJob(parameters, recipientIds) + return RetrieveProfileJob(parameters, recipientIds, skipDebounce) } } @@ -532,6 +543,7 @@ class RetrieveProfileJob private constructor(parameters: Parameters, private val const val KEY = "RetrieveProfileJob" private val TAG = Log.tag(RetrieveProfileJob::class.java) private const val KEY_RECIPIENTS = "recipients" + private const val KEY_SKIP_DEBOUNCE = "skip_debounce" private const val DEDUPE_KEY_RETRIEVE_AVATAR = KEY + "_RETRIEVE_PROFILE_AVATAR" private const val QUEUE_PREFIX = "RetrieveProfileJob_" @@ -546,8 +558,8 @@ class RetrieveProfileJob private constructor(parameters: Parameters, private val */ @JvmStatic @WorkerThread - fun enqueue(recipientId: RecipientId) { - forRecipients(setOf(recipientId)).firstOrNull()?.let { job -> + fun enqueue(recipientId: RecipientId, skipDebounce: Boolean) { + forRecipients(setOf(recipientId), skipDebounce).firstOrNull()?.let { job -> AppDependencies.jobManager.add(job) } } @@ -558,9 +570,9 @@ class RetrieveProfileJob private constructor(parameters: Parameters, private val */ @JvmStatic @WorkerThread - fun enqueue(recipientIds: Set) { + fun enqueue(recipientIds: Set, skipDebounce: Boolean) { val jobManager = AppDependencies.jobManager - for (job in forRecipients(recipientIds)) { + for (job in forRecipients(recipientIds, skipDebounce)) { jobManager.add(job) } } @@ -572,7 +584,7 @@ class RetrieveProfileJob private constructor(parameters: Parameters, private val */ @JvmStatic @WorkerThread - fun forRecipients(recipientIds: Set): List { + fun forRecipients(recipientIds: Set, skipDebounce: Boolean = false): List { val combined: MutableSet = HashSet(recipientIds.size) var includeSelf = false @@ -589,8 +601,8 @@ class RetrieveProfileJob private constructor(parameters: Parameters, private val if (includeSelf) { add(RefreshOwnProfileJob()) } - if (combined.size > 0) { - add(RetrieveProfileJob(combined)) + if (combined.isNotEmpty()) { + add(RetrieveProfileJob(combined, skipDebounce)) } } } @@ -622,7 +634,7 @@ class RetrieveProfileJob private constructor(parameters: Parameters, private val if (ids.isNotEmpty()) { Log.i(TAG, "Optimistically refreshing ${ids.size} eligible recipient(s).") - enqueue(ids.toSet()) + enqueue(ids.toSet(), false) } else { Log.i(TAG, "No recipients to refresh.") } diff --git a/app/src/main/java/org/thoughtcrime/securesms/messages/DataMessageProcessor.kt b/app/src/main/java/org/thoughtcrime/securesms/messages/DataMessageProcessor.kt index 397d5ab311..d470640574 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/messages/DataMessageProcessor.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/messages/DataMessageProcessor.kt @@ -256,7 +256,7 @@ object DataMessageProcessor { if (SignalDatabase.recipients.setProfileKey(senderRecipient.id, messageProfileKey)) { log(timestamp, "Profile key on message from " + senderRecipient.id + " didn't match our local store. It has been updated.") SignalDatabase.runPostSuccessfulTransaction { - RetrieveProfileJob.enqueue(senderRecipient.id) + RetrieveProfileJob.enqueue(senderRecipient.id, skipDebounce = true) } } } else { diff --git a/app/src/main/java/org/thoughtcrime/securesms/profiles/manage/EditProfileViewModel.java b/app/src/main/java/org/thoughtcrime/securesms/profiles/manage/EditProfileViewModel.java index 2b5f0ff969..3f5fee4a77 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/profiles/manage/EditProfileViewModel.java +++ b/app/src/main/java/org/thoughtcrime/securesms/profiles/manage/EditProfileViewModel.java @@ -15,7 +15,7 @@ import org.signal.core.util.concurrent.SignalExecutors; import org.signal.core.util.logging.Log; import org.thoughtcrime.securesms.badges.models.Badge; import org.thoughtcrime.securesms.dependencies.AppDependencies; -import org.thoughtcrime.securesms.jobs.RetrieveProfileJob; +import org.thoughtcrime.securesms.jobs.RefreshOwnProfileJob; import org.thoughtcrime.securesms.keyvalue.SignalStore; import org.thoughtcrime.securesms.mediasend.Media; import org.thoughtcrime.securesms.profiles.AvatarHelper; @@ -69,7 +69,7 @@ class EditProfileViewModel extends ViewModel { SignalExecutors.BOUNDED.execute(() -> { onRecipientChanged(Recipient.self().fresh()); - RetrieveProfileJob.enqueue(Recipient.self().getId()); + AppDependencies.getJobManager().add(new RefreshOwnProfileJob()); }); Recipient.self().live().observeForever(observer); diff --git a/app/src/main/java/org/thoughtcrime/securesms/service/webrtc/SignalCallManager.java b/app/src/main/java/org/thoughtcrime/securesms/service/webrtc/SignalCallManager.java index f2b89ffe5a..d5f8dd8589 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/service/webrtc/SignalCallManager.java +++ b/app/src/main/java/org/thoughtcrime/securesms/service/webrtc/SignalCallManager.java @@ -803,7 +803,7 @@ public final class SignalCallManager implements CallManager.Observer, GroupCall. callMessage); } catch (UntrustedIdentityException e) { Log.i(TAG, "onSendCallMessage onFailure: ", e); - RetrieveProfileJob.enqueue(recipient.getId()); + RetrieveProfileJob.enqueue(recipient.getId(), true); process((s, p) -> p.handleGroupMessageSentError(s, Collections.singletonList(recipient.getId()), UNTRUSTED_IDENTITY)); } catch (ProofRequiredException e) { Log.i(TAG, "onSendCallMessage onFailure: ", e); @@ -856,7 +856,7 @@ public final class SignalCallManager implements CallManager.Observer, GroupCall. if (Util.hasItems(identifyFailureRecipientIds)) { process((s, p) -> p.handleGroupMessageSentError(s, identifyFailureRecipientIds, UNTRUSTED_IDENTITY)); - RetrieveProfileJob.enqueue(identifyFailureRecipientIds); + RetrieveProfileJob.enqueue(identifyFailureRecipientIds, true); } } catch (UntrustedIdentityException | IOException | InvalidInputException e) { Log.w(TAG, "onSendCallMessageToGroup failed", e); @@ -1207,7 +1207,7 @@ public final class SignalCallManager implements CallManager.Observer, GroupCall. callMessage); process((s, p) -> p.handleMessageSentSuccess(s, remotePeer.getCallId())); } catch (UntrustedIdentityException e) { - RetrieveProfileJob.enqueue(remotePeer.getId()); + RetrieveProfileJob.enqueue(remotePeer.getId(), true); processSendMessageFailureWithChangeDetection(remotePeer, (s, p) -> p.handleMessageSentError(s, remotePeer.getCallId(), diff --git a/app/src/main/java/org/thoughtcrime/securesms/storage/ContactRecordProcessor.kt b/app/src/main/java/org/thoughtcrime/securesms/storage/ContactRecordProcessor.kt index 383ae41f90..ef3e9b0ddc 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/storage/ContactRecordProcessor.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/storage/ContactRecordProcessor.kt @@ -174,7 +174,7 @@ class ContactRecordProcessor( if (localAci != null && mergedIdentityKey != null && remote.proto.identityKey.isNotEmpty() && !mergedIdentityKey.contentEquals(remote.proto.identityKey.toByteArray())) { Log.w(TAG, "The local and remote identity keys do not match for " + localAci + ". Enqueueing a profile fetch.") - enqueue(trustedPush(localAci, localPni, local.proto.e164).id) + enqueue(trustedPush(localAci, localPni, local.proto.e164).id, true) } val mergedPni: PNI? diff --git a/app/src/main/java/org/thoughtcrime/securesms/stories/viewer/reply/group/StoryGroupReplyFragment.kt b/app/src/main/java/org/thoughtcrime/securesms/stories/viewer/reply/group/StoryGroupReplyFragment.kt index 0e9ddfe782..6d3a456812 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/stories/viewer/reply/group/StoryGroupReplyFragment.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/stories/viewer/reply/group/StoryGroupReplyFragment.kt @@ -150,7 +150,7 @@ class StoryGroupReplyFragment : override fun onViewCreated(view: View, savedInstanceState: Bundle?) { SignalExecutors.BOUNDED.execute { - RetrieveProfileJob.enqueue(groupRecipientId) + RetrieveProfileJob.enqueue(groupRecipientId, skipDebounce = false) } recyclerView = view.findViewById(R.id.recycler) diff --git a/libsignal-service/src/main/java/org/whispersystems/signalservice/api/profiles/ProfileRepository.kt b/libsignal-service/src/main/java/org/whispersystems/signalservice/api/profiles/ProfileRepository.kt index 0a34cee038..a3bce7cdff 100644 --- a/libsignal-service/src/main/java/org/whispersystems/signalservice/api/profiles/ProfileRepository.kt +++ b/libsignal-service/src/main/java/org/whispersystems/signalservice/api/profiles/ProfileRepository.kt @@ -38,6 +38,7 @@ class ProfileRepository(private val profileApi: ProfileApi) { val successes: MutableList> = mutableListOf() val unregistered: MutableList = mutableListOf() val retryableFailures: MutableSet = requests.map { it.id }.toMutableSet() + val verificationFailures: MutableSet = mutableSetOf() var retryAfter: Duration? = null val mutex = Mutex() @@ -82,12 +83,15 @@ class ProfileRepository(private val profileApi: ProfileApi) { } is NetworkResult.NetworkError -> Unit is NetworkResult.ApplicationError -> { - mutex.withLock { - retryableFailures -= request.id - } if (response.throwable is VerificationFailedException) { Log.w(TAG, "Failed to verify ZK profile operation for ${request.id}. Continuing with other lookups.") + mutex.withLock { + verificationFailures += request.id + } } else { + mutex.withLock { + retryableFailures -= request.id + } throw response.throwable } } @@ -105,6 +109,7 @@ class ProfileRepository(private val profileApi: ProfileApi) { successes = successes, unregistered = unregistered.toSet(), retryableFailures = retryableFailures, + verificationFailures = verificationFailures, retryAfter = retryAfter ) } @@ -113,6 +118,7 @@ class ProfileRepository(private val profileApi: ProfileApi) { val successes: List>, val unregistered: Set, val retryableFailures: Set, + val verificationFailures: Set, val retryAfter: Duration? )