From 37458d41f227880af5ec5f872503b267dfa457d7 Mon Sep 17 00:00:00 2001 From: Valere Date: Tue, 4 Oct 2022 23:54:27 +0200 Subject: [PATCH 1/2] E2ee dos not hinder verification --- changelog.d/6723.bugfix | 1 + .../android/sdk/common/CryptoTestHelper.kt | 2 +- .../sdk/internal/crypto/E2eeSanityTests.kt | 86 +++++++++++++++++++ .../sdk/api/session/events/model/EventType.kt | 13 +++ .../algorithms/megolm/MXMegolmEncryption.kt | 24 +++++- 5 files changed, 122 insertions(+), 4 deletions(-) create mode 100644 changelog.d/6723.bugfix diff --git a/changelog.d/6723.bugfix b/changelog.d/6723.bugfix new file mode 100644 index 0000000000..08b1d1fe2e --- /dev/null +++ b/changelog.d/6723.bugfix @@ -0,0 +1 @@ +Can't verify user when option to send keys to verified devices only is selected diff --git a/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/common/CryptoTestHelper.kt b/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/common/CryptoTestHelper.kt index cbaa3153df..74292daf15 100644 --- a/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/common/CryptoTestHelper.kt +++ b/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 diff --git a/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/internal/crypto/E2eeSanityTests.kt b/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/internal/crypto/E2eeSanityTests.kt index 544fe90a73..a36ba8ac02 100644 --- a/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/internal/crypto/E2eeSanityTests.kt +++ b/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) { + promise.resume(aliceAuthParams) + } + }, it) + } + + testHelper.waitForCallback { + bobSession.cryptoService().crossSigningService().initializeCrossSigning(object : UserInteractiveAuthInterceptor { + override fun performStage(flowResponse: RegistrationFlowResponse, errCode: String?, promise: Continuation) { + 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 { return scope.async { suspendCancellableCoroutine { continuation -> diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/events/model/EventType.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/events/model/EventType.kt index 84c25776e7..3ad4f3a87f 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/events/model/EventType.kt +++ b/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 + } + } } diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/algorithms/megolm/MXMegolmEncryption.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/algorithms/megolm/MXMegolmEncryption.kt index fca6fab66c..2cb90eee4a 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/algorithms/megolm/MXMegolmEncryption.kt +++ b/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, outboundSession: MXOutboundSessionInfo) { // offload to computation thread cryptoCoroutineScope.launch(coroutineDispatchers.computation) { @@ -417,7 +435,7 @@ internal class MXMegolmEncryption( * * @param userIds the user ids whose devices must be checked. */ - private suspend fun getDevicesInRoom(userIds: List): DeviceInRoomInfo { + private suspend fun getDevicesInRoom(userIds: List, 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 +462,7 @@ internal class MXMegolmEncryption( continue } - if (!deviceInfo.isVerified && encryptToVerifiedDevicesOnly) { + if (!deviceInfo.isVerified && encryptToVerifiedDevicesOnly && !forceDistributeToUnverified) { devicesInRoom.withHeldDevices.setObject(userId, deviceId, WithHeldCode.UNVERIFIED) continue } From fddeddacc7e6684cef6df0a80b713b364eb0ce91 Mon Sep 17 00:00:00 2001 From: Valere Date: Wed, 5 Oct 2022 09:11:10 +0200 Subject: [PATCH 2/2] fix outdated doc --- .../sdk/internal/crypto/algorithms/megolm/MXMegolmEncryption.kt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/algorithms/megolm/MXMegolmEncryption.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/algorithms/megolm/MXMegolmEncryption.kt index 2cb90eee4a..a90eedeff9 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/algorithms/megolm/MXMegolmEncryption.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/algorithms/megolm/MXMegolmEncryption.kt @@ -434,6 +434,8 @@ 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, forceDistributeToUnverified: Boolean = false): DeviceInRoomInfo { // We are happy to use a cached version here: we assume that if we already