From 12abca1b803e4efd8e6602881b49a5a15f057009 Mon Sep 17 00:00:00 2001 From: Valere Date: Tue, 7 Apr 2020 19:09:56 +0200 Subject: [PATCH 1/2] Fix / Send gossip request on other done received --- .../crypto/gossiping/KeyShareTests.kt | 2 +- .../DefaultVerificationService.kt | 56 ++++++++++++++++++- .../DefaultVerificationTransaction.kt | 19 +++---- .../verification/VerificationInfoDone.kt | 8 +-- 4 files changed, 67 insertions(+), 18 deletions(-) diff --git a/matrix-sdk-android/src/androidTest/java/im/vector/matrix/android/internal/crypto/gossiping/KeyShareTests.kt b/matrix-sdk-android/src/androidTest/java/im/vector/matrix/android/internal/crypto/gossiping/KeyShareTests.kt index 7ac92ed74c..bb6e020d89 100644 --- a/matrix-sdk-android/src/androidTest/java/im/vector/matrix/android/internal/crypto/gossiping/KeyShareTests.kt +++ b/matrix-sdk-android/src/androidTest/java/im/vector/matrix/android/internal/crypto/gossiping/KeyShareTests.kt @@ -282,7 +282,7 @@ class KeyShareTests : InstrumentedTest { val keysBackupService = aliceSession2.cryptoService().keysBackupService() mTestHelper.retryPeriodicallyWithLatch(latch) { Log.d("#TEST", "Recovery :${ keysBackupService.getKeyBackupRecoveryKeyInfo()?.recoveryKey}") - keysBackupService.getKeyBackupRecoveryKeyInfo()?.recoveryKey != creationInfo.recoveryKey + keysBackupService.getKeyBackupRecoveryKeyInfo()?.recoveryKey == creationInfo.recoveryKey } } } diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/verification/DefaultVerificationService.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/verification/DefaultVerificationService.kt index dc68fa6b76..db6c224019 100644 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/verification/DefaultVerificationService.kt +++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/verification/DefaultVerificationService.kt @@ -22,6 +22,9 @@ import dagger.Lazy import im.vector.matrix.android.api.MatrixCallback import im.vector.matrix.android.api.session.crypto.CryptoService import im.vector.matrix.android.api.session.crypto.crosssigning.CrossSigningService +import im.vector.matrix.android.api.session.crypto.crosssigning.KEYBACKUP_SECRET_SSSS_NAME +import im.vector.matrix.android.api.session.crypto.crosssigning.SELF_SIGNING_KEY_SSSS_NAME +import im.vector.matrix.android.api.session.crypto.crosssigning.USER_SIGNING_KEY_SSSS_NAME import im.vector.matrix.android.api.session.crypto.verification.CancelCode import im.vector.matrix.android.api.session.crypto.verification.PendingVerificationRequest import im.vector.matrix.android.api.session.crypto.verification.QrCodeVerificationTransaction @@ -60,6 +63,7 @@ import im.vector.matrix.android.internal.crypto.model.MXUsersDevicesMap import im.vector.matrix.android.internal.crypto.model.event.EncryptedEventContent import im.vector.matrix.android.internal.crypto.model.rest.KeyVerificationAccept import im.vector.matrix.android.internal.crypto.model.rest.KeyVerificationCancel +import im.vector.matrix.android.internal.crypto.model.rest.KeyVerificationDone import im.vector.matrix.android.internal.crypto.model.rest.KeyVerificationKey import im.vector.matrix.android.internal.crypto.model.rest.KeyVerificationMac import im.vector.matrix.android.internal.crypto.model.rest.KeyVerificationReady @@ -109,6 +113,10 @@ internal class DefaultVerificationService @Inject constructor( // map [sender : [transaction]] private val txMap = HashMap>() + // we need to keep track of finished transaction + // It will be used for gossiping (to send request after request is completed and 'done' by other) + private val pastTransactions = HashMap>() + /** * Map [sender: [PendingVerificationRequest]] * For now we keep all requests (even terminated ones) during the lifetime of the app. @@ -137,6 +145,9 @@ internal class DefaultVerificationService @Inject constructor( EventType.KEY_VERIFICATION_READY -> { onReadyReceived(event) } + EventType.KEY_VERIFICATION_DONE -> { + onDoneReceived(event) + } MessageType.MSGTYPE_VERIFICATION_REQUEST -> { onRequestReceived(event) } @@ -778,6 +789,31 @@ internal class DefaultVerificationService @Inject constructor( } } + private fun onDoneReceived(event: Event) { + Timber.v("## onDoneReceived") + val doneReq = event.getClearContent().toModel()?.asValidObject() + if (doneReq == null || event.senderId != userId) { + // ignore + Timber.e("## SAS Received invalid done request") + return + } + + // We only send gossiping request when the other sent us a done + // We can ask without checking too much thinks (like trust), because we will check validity of secret on reception + getExistingTransaction(userId, doneReq.transactionId) + ?: getOldTransaction(userId, doneReq.transactionId) + ?.let { vt -> + val otherDeviceId = vt.otherDeviceId + if (!crossSigningService.canCrossSign()) { + outgoingGossipingRequestManager.sendSecretShareRequest(SELF_SIGNING_KEY_SSSS_NAME, mapOf(userId to listOf(otherDeviceId + ?: "*"))) + outgoingGossipingRequestManager.sendSecretShareRequest(USER_SIGNING_KEY_SSSS_NAME, mapOf(userId to listOf(otherDeviceId + ?: "*"))) + } + outgoingGossipingRequestManager.sendSecretShareRequest(KEYBACKUP_SECRET_SSSS_NAME, mapOf(userId to listOf(otherDeviceId ?: "*"))) + } + } + private fun onRoomDoneReceived(event: Event) { val doneReq = event.getClearContent().toModel() ?.copy( @@ -1003,7 +1039,11 @@ internal class DefaultVerificationService @Inject constructor( private fun removeTransaction(otherUser: String, tid: String) { synchronized(txMap) { - txMap[otherUser]?.remove(tid)?.removeListener(this) + txMap[otherUser]?.remove(tid)?.also { + it.removeListener(this) + } + }?.let { + rememberOldTransaction(it) } } @@ -1016,6 +1056,20 @@ internal class DefaultVerificationService @Inject constructor( } } + private fun rememberOldTransaction(tx: DefaultVerificationTransaction) { + synchronized(pastTransactions) { + pastTransactions.getOrPut(tx.otherUserId) { HashMap() }[tx.transactionId] = tx + } + } + + private fun getOldTransaction(userId: String, tid: String?): DefaultVerificationTransaction? { + return tid?.let { + synchronized(pastTransactions) { + pastTransactions[userId]?.get(it) + } + } + } + override fun beginKeyVerification(method: VerificationMethod, otherUserId: String, otherDeviceId: String, transactionId: String?): String? { val txID = transactionId?.takeIf { it.isNotEmpty() } ?: createUniqueIDForTransaction(otherUserId, otherDeviceId) // should check if already one (and cancel it) diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/verification/DefaultVerificationTransaction.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/verification/DefaultVerificationTransaction.kt index 5b8191fc99..29cfcd2383 100644 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/verification/DefaultVerificationTransaction.kt +++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/verification/DefaultVerificationTransaction.kt @@ -17,9 +17,6 @@ package im.vector.matrix.android.internal.crypto.verification import im.vector.matrix.android.api.MatrixCallback import im.vector.matrix.android.api.session.crypto.crosssigning.CrossSigningService -import im.vector.matrix.android.api.session.crypto.crosssigning.KEYBACKUP_SECRET_SSSS_NAME -import im.vector.matrix.android.api.session.crypto.crosssigning.SELF_SIGNING_KEY_SSSS_NAME -import im.vector.matrix.android.api.session.crypto.crosssigning.USER_SIGNING_KEY_SSSS_NAME import im.vector.matrix.android.api.session.crypto.verification.VerificationTransaction import im.vector.matrix.android.api.session.crypto.verification.VerificationTxState import im.vector.matrix.android.internal.crypto.IncomingGossipingRequestManager @@ -100,15 +97,15 @@ internal abstract class DefaultVerificationTransaction( }) } - transport.done(transactionId) { - if (otherUserId == userId && !crossSigningService.canCrossSign()) { - outgoingGossipingRequestManager.sendSecretShareRequest(SELF_SIGNING_KEY_SSSS_NAME, mapOf(userId to listOf(otherDeviceId ?: "*"))) - outgoingGossipingRequestManager.sendSecretShareRequest(USER_SIGNING_KEY_SSSS_NAME, mapOf(userId to listOf(otherDeviceId ?: "*"))) - outgoingGossipingRequestManager.sendSecretShareRequest(KEYBACKUP_SECRET_SSSS_NAME, mapOf(userId to listOf(otherDeviceId ?: "*"))) - } - } - state = VerificationTxState.Verified + + transport.done(transactionId) { +// if (otherUserId == userId && !crossSigningService.canCrossSign()) { +// outgoingGossipingRequestManager.sendSecretShareRequest(SELF_SIGNING_KEY_SSSS_NAME, mapOf(userId to listOf(otherDeviceId ?: "*"))) +// outgoingGossipingRequestManager.sendSecretShareRequest(USER_SIGNING_KEY_SSSS_NAME, mapOf(userId to listOf(otherDeviceId ?: "*"))) +// outgoingGossipingRequestManager.sendSecretShareRequest(KEYBACKUP_SECRET_SSSS_NAME, mapOf(userId to listOf(otherDeviceId ?: "*"))) +// } + } } private fun setDeviceVerified(userId: String, deviceId: String) { diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/verification/VerificationInfoDone.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/verification/VerificationInfoDone.kt index abb1141355..2986013fca 100644 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/verification/VerificationInfoDone.kt +++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/verification/VerificationInfoDone.kt @@ -18,11 +18,9 @@ package im.vector.matrix.android.internal.crypto.verification internal interface VerificationInfoDone : VerificationInfo { override fun asValidObject(): ValidVerificationInfoDone? { - if (transactionId.isNullOrEmpty()) { - return null - } - return ValidVerificationInfoDone + val validTransactionId = transactionId?.takeIf { it.isNotEmpty() } ?: return null + return ValidVerificationInfoDone(validTransactionId) } } -internal object ValidVerificationInfoDone +internal data class ValidVerificationInfoDone(val transactionId: String) From b480eb36884c01cee41b044d16211bb3cb2d0da4 Mon Sep 17 00:00:00 2001 From: Valere Date: Tue, 7 Apr 2020 19:20:49 +0200 Subject: [PATCH 2/2] Update change log --- CHANGES.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGES.md b/CHANGES.md index 74269178dd..4dccae5ad5 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -19,6 +19,7 @@ Bugfix 🐛: - RiotX can't restore cross signing keys saved by web in SSSS (#1174) - Cross- Signing | After signin in new session, verification paper trail in DM is off (#1191) - Failed to encrypt message in room (message stays in red), [thanks to pwr22] (#925) + - Cross-Signing | web <-> riotX After QR code scan, gossiping fails (#1210) Translations 🗣: -