From 8c2ff2f1c20e742bece94d32cc9cb347dec68ab4 Mon Sep 17 00:00:00 2001 From: Greyson Parrelli Date: Mon, 15 Jun 2026 15:59:23 -0400 Subject: [PATCH] Improve handling of unlinked device during send. --- .../org/signal/benchmark/setup/OtherClient.kt | 1 + .../SignalServiceAccountDataStoreImpl.java | 5 ++ .../BufferedSignalServiceAccountDataStore.kt | 4 ++ .../api/SignalServiceAccountDataStore.java | 5 ++ .../api/SignalServiceMessageSender.java | 27 +++++++--- .../signal/network/service/MessageService.kt | 13 ++++- .../network/service/MessageServiceTest.kt | 53 ++++++++++++++++++- 7 files changed, 97 insertions(+), 11 deletions(-) diff --git a/app/src/benchmarkShared/java/org/signal/benchmark/setup/OtherClient.kt b/app/src/benchmarkShared/java/org/signal/benchmark/setup/OtherClient.kt index 094e015b45..f5bdff0176 100644 --- a/app/src/benchmarkShared/java/org/signal/benchmark/setup/OtherClient.kt +++ b/app/src/benchmarkShared/java/org/signal/benchmark/setup/OtherClient.kt @@ -241,5 +241,6 @@ class OtherClient(val serviceId: ServiceId, val e164: String, val identityKeyPai override fun deleteAllStaleOneTimeKyberPreKeys(threshold: Long, minCount: Int) = throw UnsupportedOperationException() override fun loadLastResortKyberPreKeys(): List = throw UnsupportedOperationException() override fun isMultiDevice(): Boolean = throw UnsupportedOperationException() + override fun setMultiDevice(isMultiDevice: Boolean) = throw UnsupportedOperationException() } } diff --git a/app/src/main/java/org/thoughtcrime/securesms/crypto/storage/SignalServiceAccountDataStoreImpl.java b/app/src/main/java/org/thoughtcrime/securesms/crypto/storage/SignalServiceAccountDataStoreImpl.java index ac79582e7c..d849e1acea 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/crypto/storage/SignalServiceAccountDataStoreImpl.java +++ b/app/src/main/java/org/thoughtcrime/securesms/crypto/storage/SignalServiceAccountDataStoreImpl.java @@ -58,6 +58,11 @@ public class SignalServiceAccountDataStoreImpl implements SignalServiceAccountDa return SignalStore.account().isMultiDevice(); } + @Override + public void setMultiDevice(boolean isMultiDevice) { + SignalStore.account().setMultiDevice(isMultiDevice); + } + @Override public IdentityKeyPair getIdentityKeyPair() { return identityKeyStore.getIdentityKeyPair(); diff --git a/app/src/main/java/org/thoughtcrime/securesms/messages/protocol/BufferedSignalServiceAccountDataStore.kt b/app/src/main/java/org/thoughtcrime/securesms/messages/protocol/BufferedSignalServiceAccountDataStore.kt index 4e17f814b3..7745b6a6a2 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/messages/protocol/BufferedSignalServiceAccountDataStore.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/messages/protocol/BufferedSignalServiceAccountDataStore.kt @@ -201,6 +201,10 @@ class BufferedSignalServiceAccountDataStore(selfServiceId: ServiceId) : SignalSe error("Should not happen during the intended usage pattern of this class") } + override fun setMultiDevice(isMultiDevice: Boolean) { + error("Should not happen during the intended usage pattern of this class") + } + fun flushToDisk(persistentStore: SignalServiceAccountDataStore) { SignalDatabase.writableDatabase.withinTransaction { identityStore.flushToDisk(persistentStore) diff --git a/lib/libsignal-service/src/main/java/org/whispersystems/signalservice/api/SignalServiceAccountDataStore.java b/lib/libsignal-service/src/main/java/org/whispersystems/signalservice/api/SignalServiceAccountDataStore.java index 7ec77b399b..feac3c9c26 100644 --- a/lib/libsignal-service/src/main/java/org/whispersystems/signalservice/api/SignalServiceAccountDataStore.java +++ b/lib/libsignal-service/src/main/java/org/whispersystems/signalservice/api/SignalServiceAccountDataStore.java @@ -15,4 +15,9 @@ public interface SignalServiceAccountDataStore extends SignalProtocolStore, * @return True if the user has linked devices, otherwise false. */ boolean isMultiDevice(); + + /** + * Update whether the user has linked devices. + */ + void setMultiDevice(boolean isMultiDevice); } diff --git a/lib/libsignal-service/src/main/java/org/whispersystems/signalservice/api/SignalServiceMessageSender.java b/lib/libsignal-service/src/main/java/org/whispersystems/signalservice/api/SignalServiceMessageSender.java index 3c20c01025..ba9ff0dc57 100644 --- a/lib/libsignal-service/src/main/java/org/whispersystems/signalservice/api/SignalServiceMessageSender.java +++ b/lib/libsignal-service/src/main/java/org/whispersystems/signalservice/api/SignalServiceMessageSender.java @@ -1969,14 +1969,15 @@ public class SignalServiceMessageSender { return SendMessageResult.canceledFailure(recipient); } + OutgoingPushMessageList messages = null; try { - OutgoingPushMessageList messages = getEncryptedMessages(recipient, - sealedSenderAccess, - timestamp, - content, - online, - urgent, - story); + messages = getEncryptedMessages(recipient, + sealedSenderAccess, + timestamp, + content, + online, + urgent, + story); boolean isSentSyncTranscript = content.getContent().isPresent() && content.getContent().get().syncMessage != null && content.getContent().get().syncMessage.sent != null; if (i == 0 && sendEvents != null) { @@ -2060,8 +2061,18 @@ public class SignalServiceMessageSender { } } catch (MismatchedDevicesException mde) { Log.w(TAG, "[sendMessage][" + timestamp + "] Handling mismatched devices. (" + mde.getMessage() + ")"); + + MismatchedDevices mismatchedDevices = mde.getMismatchedDevices(); + boolean sentOnlyToSelf = recipient.matches(localAddress) && messages != null && messages.getDevices().equals(Collections.singletonList(localDeviceId)); + if (sentOnlyToSelf && mismatchedDevices.getMissingDevices().isEmpty()) { + Log.w(TAG, "[sendMessage][" + timestamp + "] Sent only to our own device and the server reports no other devices. Marking as no longer multi-device and skipping send."); + archiveSessions(recipient, mismatchedDevices.getExtraDevices()); + aciStore.setMultiDevice(false); + return SendMessageResult.success(recipient, Collections.emptyList(), false, false, System.currentTimeMillis() - startTime, content.getContent()); + } + try { - handleMismatchedDevices(recipient, mde.getMismatchedDevices()); + handleMismatchedDevices(recipient, mismatchedDevices); } catch (InvalidPreKeyException e) { return SendMessageResult.invalidPreKeyFailure(recipient); } diff --git a/lib/network/src/main/java/org/signal/network/service/MessageService.kt b/lib/network/src/main/java/org/signal/network/service/MessageService.kt index 7e695c106e..74d571cb3b 100644 --- a/lib/network/src/main/java/org/signal/network/service/MessageService.kt +++ b/lib/network/src/main/java/org/signal/network/service/MessageService.kt @@ -172,6 +172,11 @@ open class MessageService( raise(SendError.ContentTooLarge(size = contentSize, maxAllowed = maxContentSizeBytes)) } + if (!protocolStore.isMultiDevice) { + Log.d(TAG, "We do not have any linked devices. Skipping sync message send.") + return@either SendSuccess(envelopeContent = envelopeContent, sentSealedSender = false, devices = emptyList()) + } + var encryptedReported = false // Certain errors self-resolve by mutating external state, like creating new sessions. @@ -211,6 +216,12 @@ open class MessageService( when (val error = result.error) { is MismatchedDeviceException -> { handleMismatched(error, sealedSenderAccess = null) + val sentOnlyToSelf = encryptedMessages.map { it.destinationDeviceId } == listOf(localDeviceId) + if (sentOnlyToSelf && error.entries.all { it.missingDevices.isEmpty() }) { + Log.w(TAG, "Sent only to our own device and the server reports no other devices. Marking as no longer multi-device and skipping send.") + protocolStore.setMultiDevice(false) + return@either SendSuccess(envelopeContent = envelopeContent, sentSealedSender = false, devices = emptyList()) + } } is RateLimitChallengeException -> { raise(SendError.ChallengeRequired(error.token, error.options, error.retryLater?.toKotlinDuration())) @@ -331,7 +342,7 @@ open class MessageService( } suspend fun Raise.handleMismatched(error: MismatchedDeviceException, sealedSenderAccess: SealedSenderAccess?) { - Log.w(TAG, "Handling mismatched devices: ${error.entries}") + Log.w(TAG, "Handling mismatched devices: ${error.entries.contentToString()}") for (entry in error.entries) { for (staleDeviceId in entry.staleDevices) { diff --git a/lib/network/src/test/java/org/signal/network/service/MessageServiceTest.kt b/lib/network/src/test/java/org/signal/network/service/MessageServiceTest.kt index 68c53c0a15..3fc64c8a49 100644 --- a/lib/network/src/test/java/org/signal/network/service/MessageServiceTest.kt +++ b/lib/network/src/test/java/org/signal/network/service/MessageServiceTest.kt @@ -298,6 +298,54 @@ class MessageServiceTest { coVerify { keysApi.getPreKey(recipientAci.toString(), 2, null) } } + @Test + fun `sync send to self with no other devices stops instead of looping`() = runTest { + val service = newService() + every { protocolStore.isMultiDevice } returns true + every { protocolStore.getSubDeviceSessions(localAci.toString()) } returns emptyList() + every { cipher.encrypt(any(), any(), any()) } returns OutgoingPushMessage(1, 1, 100, validSerializedSignalMessageBase64()) + + coEvery { messageApi.sendSyncMessage(any(), any(), any()) } returns + RequestResult.NonSuccess(mismatchedException(extra = intArrayOf(1), account = localAci)) + + val result = service.sendSyncMessage(timestamp, envelopeContent, urgent = true, onEncrypted = null) + + val success = (result as Either.Right).value + assertThat(success.devices).isEqualTo(emptyList()) + verify { protocolStore.archiveSession(SignalProtocolAddress(localAci.libSignalServiceId, 1)) } + verify(exactly = 1) { protocolStore.setMultiDevice(false) } + coVerify(exactly = 1) { messageApi.sendSyncMessage(any(), any(), any()) } + } + + @Test + fun `sync send that reached a real linked device is not treated as no other devices`() = runTest { + val service = newService() + every { protocolStore.isMultiDevice } returns true + every { protocolStore.getSubDeviceSessions(localAci.toString()) } returns listOf(2) + every { cipher.encrypt(any(), any(), any()) } returns OutgoingPushMessage(1, 2, 100, validSerializedSignalMessageBase64()) + + coEvery { messageApi.sendSyncMessage(any(), any(), any()) } returns + RequestResult.NonSuccess(mismatchedException(extra = intArrayOf(2), account = localAci)) + + val result = service.sendSyncMessage(timestamp, envelopeContent, urgent = true, onEncrypted = null) + + assertThat(result).isInstanceOf(Either.Left::class) + verify(exactly = 0) { protocolStore.setMultiDevice(any()) } + coVerify(atLeast = 2) { messageApi.sendSyncMessage(any(), any(), any()) } + } + + @Test + fun `sync send is skipped entirely when not multi-device`() = runTest { + val service = newService() + every { protocolStore.isMultiDevice } returns false + + val result = service.sendSyncMessage(timestamp, envelopeContent, urgent = true, onEncrypted = null) + + val success = (result as Either.Right).value + assertThat(success.devices).isEqualTo(emptyList()) + coVerify(exactly = 0) { messageApi.sendSyncMessage(any(), any(), any()) } + } + @Test fun `ServiceIdNotFoundException maps to NotRegistered`() = runTest { val service = newService() @@ -449,10 +497,11 @@ class MessageServiceTest { private fun mismatchedException( missing: IntArray = intArrayOf(), extra: IntArray = intArrayOf(), - stale: IntArray = intArrayOf() + stale: IntArray = intArrayOf(), + account: ServiceId = recipientAci ): MismatchedDeviceException { val entry = MismatchedDeviceException.Entry( - account = recipientAci.libSignalServiceId, + account = account.libSignalServiceId, missingDevices = missing, extraDevices = extra, staleDevices = stale