Merge pull request #7290 from vector-im/feature/bca/hinder_verification

E2ee dos not hinder verification

changelog.d/6723.bugfix

@@ -0,0 +1 @@
+Can't verify user when option to send keys to verified devices only is selected

matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/common/CryptoTestHelper.kt

@@ -313,7 +313,7 @@ class CryptoTestHelper(val testHelper: CommonTestHelper) {
         val incomingRequest = bobVerificationService.getExistingVerificationRequests(alice.myUserId).first {
             it.requestInfo?.fromDevice == alice.sessionParams.deviceId
         }
-        bobVerificationService.readyPendingVerification(listOf(VerificationMethod.SAS), alice.myUserId, incomingRequest.transactionId!!)
+        bobVerificationService.readyPendingVerificationInDMs(listOf(VerificationMethod.SAS), alice.myUserId, roomId, incomingRequest.transactionId!!)
 
         var requestID: String? = null
         // wait for it to be readied

matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/internal/crypto/E2eeSanityTests.kt

@@ -33,6 +33,10 @@ import org.junit.runner.RunWith
 import org.junit.runners.JUnit4
 import org.junit.runners.MethodSorters
 import org.matrix.android.sdk.InstrumentedTest
+import org.matrix.android.sdk.api.auth.UIABaseAuth
+import org.matrix.android.sdk.api.auth.UserInteractiveAuthInterceptor
+import org.matrix.android.sdk.api.auth.UserPasswordAuth
+import org.matrix.android.sdk.api.auth.registration.RegistrationFlowResponse
 import org.matrix.android.sdk.api.crypto.MXCryptoConfig
 import org.matrix.android.sdk.api.session.Session
 import org.matrix.android.sdk.api.session.crypto.MXCryptoError
@@ -61,7 +65,10 @@ import org.matrix.android.sdk.common.CommonTestHelper
 import org.matrix.android.sdk.common.CommonTestHelper.Companion.runCryptoTest
 import org.matrix.android.sdk.common.CommonTestHelper.Companion.runSessionTest
 import org.matrix.android.sdk.common.SessionTestParams
+import org.matrix.android.sdk.common.TestConstants
 import org.matrix.android.sdk.mustFail
+import timber.log.Timber
+import kotlin.coroutines.Continuation
 import kotlin.coroutines.resume
 
 // @Ignore("This test fails with an unhandled exception thrown from a coroutine which terminates the entire test run.")
@@ -607,6 +614,85 @@ class E2eeSanityTests : InstrumentedTest {
         )
     }
 
+    @Test
+    fun test_EncryptionDoesNotHinderVerification() = runCryptoTest(context()) { cryptoTestHelper, testHelper ->
+        val cryptoTestData = cryptoTestHelper.doE2ETestWithAliceAndBobInARoom()
+
+        val aliceSession = cryptoTestData.firstSession
+        val bobSession = cryptoTestData.secondSession
+
+        val aliceAuthParams = UserPasswordAuth(
+                user = aliceSession.myUserId,
+                password = TestConstants.PASSWORD
+        )
+        val bobAuthParams = UserPasswordAuth(
+                user = bobSession!!.myUserId,
+                password = TestConstants.PASSWORD
+        )
+
+        testHelper.waitForCallback {
+            aliceSession.cryptoService().crossSigningService().initializeCrossSigning(object : UserInteractiveAuthInterceptor {
+                override fun performStage(flowResponse: RegistrationFlowResponse, errCode: String?, promise: Continuation<UIABaseAuth>) {
+                    promise.resume(aliceAuthParams)
+                }
+            }, it)
+        }
+
+        testHelper.waitForCallback {
+            bobSession.cryptoService().crossSigningService().initializeCrossSigning(object : UserInteractiveAuthInterceptor {
+                override fun performStage(flowResponse: RegistrationFlowResponse, errCode: String?, promise: Continuation<UIABaseAuth>) {
+                    promise.resume(bobAuthParams)
+                }
+            }, it)
+        }
+
+        // add a second session for bob but not cross signed
+
+        val secondBobSession = testHelper.logIntoAccount(bobSession.myUserId, SessionTestParams(true))
+
+        aliceSession.cryptoService().setGlobalBlacklistUnverifiedDevices(true)
+
+        // The two bob session should not be able to decrypt any message
+
+        val roomFromAlicePOV = aliceSession.getRoom(cryptoTestData.roomId)!!
+        Timber.v("#TEST: Send a first message that should be withheld")
+        val sentEvent = sendMessageInRoom(testHelper, roomFromAlicePOV, "Hello")!!
+
+        // wait for it to be synced back the other side
+        Timber.v("#TEST: Wait for message to be synced back")
+        testHelper.retryPeriodically {
+            bobSession.roomService().getRoom(cryptoTestData.roomId)?.timelineService()?.getTimelineEvent(sentEvent) != null
+        }
+
+        testHelper.retryPeriodically {
+            secondBobSession.roomService().getRoom(cryptoTestData.roomId)?.timelineService()?.getTimelineEvent(sentEvent) != null
+        }
+
+        // bob should not be able to decrypt
+        Timber.v("#TEST: Ensure cannot be decrytped")
+        cryptoTestHelper.ensureCannotDecrypt(listOf(sentEvent), bobSession, cryptoTestData.roomId)
+        cryptoTestHelper.ensureCannotDecrypt(listOf(sentEvent), secondBobSession, cryptoTestData.roomId)
+
+        // let's try to verify, it should work even if bob devices are untrusted
+        Timber.v("#TEST: Do the verification")
+        cryptoTestHelper.verifySASCrossSign(aliceSession, bobSession, cryptoTestData.roomId)
+
+        Timber.v("#TEST: Send a second message, outbound session should have rotated and only bob 1rst session should decrypt")
+
+        val secondEvent = sendMessageInRoom(testHelper, roomFromAlicePOV, "World")!!
+        Timber.v("#TEST: Wait for message to be synced back")
+        testHelper.retryPeriodically {
+            bobSession.roomService().getRoom(cryptoTestData.roomId)?.timelineService()?.getTimelineEvent(secondEvent) != null
+        }
+
+        testHelper.retryPeriodically {
+            secondBobSession.roomService().getRoom(cryptoTestData.roomId)?.timelineService()?.getTimelineEvent(secondEvent) != null
+        }
+
+        cryptoTestHelper.ensureCanDecrypt(listOf(secondEvent), bobSession, cryptoTestData.roomId, listOf("World"))
+        cryptoTestHelper.ensureCannotDecrypt(listOf(secondEvent), secondBobSession, cryptoTestData.roomId)
+    }
+
     private suspend fun VerificationService.readOldVerificationCodeAsync(scope: CoroutineScope, userId: String): Deferred<String> {
         return scope.async {
             suspendCancellableCoroutine { continuation ->

matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/events/model/EventType.kt

@@ -128,4 +128,17 @@ object EventType {
                 type == CALL_REJECT ||
                 type == CALL_REPLACES
     }
+
+    fun isVerificationEvent(type: String): Boolean {
+        return when (type) {
+            KEY_VERIFICATION_START,
+            KEY_VERIFICATION_ACCEPT,
+            KEY_VERIFICATION_KEY,
+            KEY_VERIFICATION_MAC,
+            KEY_VERIFICATION_CANCEL,
+            KEY_VERIFICATION_DONE,
+            KEY_VERIFICATION_READY -> true
+            else -> false
+        }
+    }
 }

matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/algorithms/megolm/MXMegolmEncryption.kt

@@ -31,6 +31,8 @@ import org.matrix.android.sdk.api.session.events.model.Content
 import org.matrix.android.sdk.api.session.events.model.EventType
 import org.matrix.android.sdk.api.session.events.model.content.RoomKeyWithHeldContent
 import org.matrix.android.sdk.api.session.events.model.content.WithHeldCode
+import org.matrix.android.sdk.api.session.room.model.message.MessageContent
+import org.matrix.android.sdk.api.session.room.model.message.MessageType
 import org.matrix.android.sdk.internal.crypto.DeviceListManager
 import org.matrix.android.sdk.internal.crypto.InboundGroupSessionHolder
 import org.matrix.android.sdk.internal.crypto.MXOlmDevice
@@ -92,7 +94,18 @@ internal class MXMegolmEncryption(
     ): Content {
         val ts = clock.epochMillis()
         Timber.tag(loggerTag.value).v("encryptEventContent : getDevicesInRoom")
-        val devices = getDevicesInRoom(userIds)
+
+        /**
+         * When using in-room messages and the room has encryption enabled,
+         * clients should ensure that encryption does not hinder the verification.
+         * For example, if the verification messages are encrypted, clients must ensure that all the recipient’s
+         * unverified devices receive the keys necessary to decrypt the messages,
+         * even if they would normally not be given the keys to decrypt messages in the room.
+         */
+        val shouldSendToUnverified = isVerificationEvent(eventType, eventContent)
+
+        val devices = getDevicesInRoom(userIds, forceDistributeToUnverified = shouldSendToUnverified)
+
         Timber.tag(loggerTag.value).d("encrypt event in room=$roomId - devices count in room ${devices.allowedDevices.toDebugCount()}")
         Timber.tag(loggerTag.value).v("encryptEventContent ${clock.epochMillis() - ts}: getDevicesInRoom ${devices.allowedDevices.toDebugString()}")
         val outboundSession = ensureOutboundSession(devices.allowedDevices)
@@ -107,6 +120,11 @@ internal class MXMegolmEncryption(
                 }
     }
 
+    private fun isVerificationEvent(eventType: String, eventContent: Content) =
+            EventType.isVerificationEvent(eventType) ||
+                    (eventType == EventType.MESSAGE &&
+                            eventContent.get(MessageContent.MSG_TYPE_JSON_KEY) == MessageType.MSGTYPE_VERIFICATION_REQUEST)
+
     private fun notifyWithheldForSession(devices: MXUsersDevicesMap<WithHeldCode>, outboundSession: MXOutboundSessionInfo) {
         // offload to computation thread
         cryptoCoroutineScope.launch(coroutineDispatchers.computation) {
@@ -416,8 +434,10 @@ internal class MXMegolmEncryption(
      * This method must be called in getDecryptingThreadHandler() thread.
      *
      * @param userIds the user ids whose devices must be checked.
+     * @param forceDistributeToUnverified If true the unverified devices will be included in valid recipients even if
+     * such devices are blocked in crypto settings
      */
-    private suspend fun getDevicesInRoom(userIds: List<String>): DeviceInRoomInfo {
+    private suspend fun getDevicesInRoom(userIds: List<String>, forceDistributeToUnverified: Boolean = false): DeviceInRoomInfo {
         // We are happy to use a cached version here: we assume that if we already
         // have a list of the user's devices, then we already share an e2e room
         // with them, which means that they will have announced any new devices via
@@ -444,7 +464,7 @@ internal class MXMegolmEncryption(
                     continue
                 }
 
-                if (!deviceInfo.isVerified && encryptToVerifiedDevicesOnly) {
+                if (!deviceInfo.isVerified && encryptToVerifiedDevicesOnly && !forceDistributeToUnverified) {
                     devicesInRoom.withHeldDevices.setObject(userId, deviceId, WithHeldCode.UNVERIFIED)
                     continue
                 }