From ae648c3e116635c7590113312ec07e092c17131c Mon Sep 17 00:00:00 2001 From: Onuray Sahin Date: Thu, 4 Feb 2021 11:47:24 +0300 Subject: [PATCH 01/12] Make outbound key sharing strategy configurable. --- CHANGES.md | 1 + .../android/sdk/api/MatrixConfiguration.kt | 2 ++ .../OutboundSessionKeySharingStrategy.kt | 34 +++++++++++++++++++ .../sdk/api/session/crypto/CryptoService.kt | 2 ++ .../internal/crypto/DefaultCryptoService.kt | 15 ++++++++ .../crypto/algorithms/IMXEncrypting.kt | 7 ++++ .../algorithms/megolm/MXMegolmEncryption.kt | 9 +++-- .../crypto/algorithms/olm/MXOlmEncryption.kt | 4 +++ .../session/room/timeline/DefaultTimeline.kt | 16 +++++++-- .../session/room/typing/SendTypingTask.kt | 16 ++++++++- vector/build.gradle | 2 ++ .../java/im/vector/app/VectorApplication.kt | 2 +- 12 files changed, 101 insertions(+), 9 deletions(-) create mode 100644 matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/crypto/OutboundSessionKeySharingStrategy.kt diff --git a/CHANGES.md b/CHANGES.md index ba3ad08037..a3eb4bfdf9 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -41,6 +41,7 @@ Improvements 🙌: - Improve room profile UX - Upgrade Jitsi library from 2.9.3 to 3.1.0 - a11y improvements + - Pre-share session keys when opening a room or start typing (#2771) Bugfix 🐛: - VoIP : fix audio devices output diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/MatrixConfiguration.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/MatrixConfiguration.kt index 93a1b962ed..c01aeab6dd 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/MatrixConfiguration.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/MatrixConfiguration.kt @@ -17,6 +17,7 @@ package org.matrix.android.sdk.api import org.matrix.android.sdk.api.crypto.MXCryptoConfig +import org.matrix.android.sdk.api.crypto.OutboundSessionKeySharingStrategy import java.net.Proxy data class MatrixConfiguration( @@ -40,6 +41,7 @@ data class MatrixConfiguration( * True to advertise support for call transfers to other parties on Matrix calls. */ val supportsCallTransfer: Boolean = false + val outboundSessionKeySharingStrategy: OutboundSessionKeySharingStrategy = OutboundSessionKeySharingStrategy.WhenSendingEvent ) { /** diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/crypto/OutboundSessionKeySharingStrategy.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/crypto/OutboundSessionKeySharingStrategy.kt new file mode 100644 index 0000000000..9ef1f538b8 --- /dev/null +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/crypto/OutboundSessionKeySharingStrategy.kt @@ -0,0 +1,34 @@ +/* + * Copyright 2020 The Matrix.org Foundation C.I.C. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.matrix.android.sdk.api.crypto + +enum class OutboundSessionKeySharingStrategy { + /** + * Keys will be sent for the first time when the first message is sent + */ + WhenSendingEvent, + + /** + * Keys will be sent for the first time when the timeline displayed + */ + WhenEnteringRoom, + + /** + * Keys will be sent for the first time when a typing started + */ + WhenTyping +} diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/crypto/CryptoService.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/crypto/CryptoService.kt index eead9b4ab7..1c5bfb1a7b 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/crypto/CryptoService.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/crypto/CryptoService.kt @@ -156,4 +156,6 @@ interface CryptoService { fun getWithHeldMegolmSession(roomId: String, sessionId: String): RoomKeyWithHeldContent? fun logDbUsageInfo() + + fun ensureOutboundSession(roomId: String) } diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/DefaultCryptoService.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/DefaultCryptoService.kt index 67229a5eae..821ac18e3a 100755 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/DefaultCryptoService.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/DefaultCryptoService.kt @@ -1290,6 +1290,21 @@ internal class DefaultCryptoService @Inject constructor( cryptoStore.logDbUsageInfo() } + override fun ensureOutboundSession(roomId: String) { + cryptoCoroutineScope.launch(coroutineDispatchers.crypto) { + roomEncryptorsStore + .get(roomId) + ?.let { + getEncryptionAlgorithm(roomId)?.let { safeAlgorithm -> + val userIds = getRoomUserIds(roomId) + if (setEncryptionInRoom(roomId, safeAlgorithm, false, userIds)) { + roomEncryptorsStore.get(roomId)?.ensureOutboundSession(getRoomUserIds(roomId)) + } + } + } + } + } + /* ========================================================================================== * For test only * ========================================================================================== */ diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/algorithms/IMXEncrypting.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/algorithms/IMXEncrypting.kt index fc3ea08a21..008ef1b084 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/algorithms/IMXEncrypting.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/algorithms/IMXEncrypting.kt @@ -62,4 +62,11 @@ internal interface IMXEncrypting { userId: String, deviceId: String, senderKey: String): Boolean + + /** + * Ensure the outbound session + * + * @param usersInRoom the users in the room + */ + suspend fun ensureOutboundSession(usersInRoom: List) } 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 fa8acafb83..3a07dbcfb6 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 @@ -137,11 +137,10 @@ internal class MXMegolmEncryption( return MXOutboundSessionInfo(sessionId, SharedWithHelper(roomId, sessionId, cryptoStore)) } - /** - * Ensure the outbound session - * - * @param devicesInRoom the devices list - */ + override suspend fun ensureOutboundSession(usersInRoom: List) { + getDevicesInRoom(usersInRoom).allowedDevices + } + private suspend fun ensureOutboundSession(devicesInRoom: MXUsersDevicesMap): MXOutboundSessionInfo { Timber.v("## CRYPTO | ensureOutboundSession start") var session = outboundSession diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/algorithms/olm/MXOlmEncryption.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/algorithms/olm/MXOlmEncryption.kt index 9acc9bc1b2..acea6ff3fe 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/algorithms/olm/MXOlmEncryption.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/algorithms/olm/MXOlmEncryption.kt @@ -85,4 +85,8 @@ internal class MXOlmEncryption( // No need for olm return false } + + override suspend fun ensureOutboundSession(usersInRoom: List) { + // NOOP + } } diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/timeline/DefaultTimeline.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/timeline/DefaultTimeline.kt index ca272a3520..e75beb4b21 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/timeline/DefaultTimeline.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/timeline/DefaultTimeline.kt @@ -24,9 +24,13 @@ import io.realm.RealmQuery import io.realm.RealmResults import io.realm.Sort import org.matrix.android.sdk.api.MatrixCallback +import org.matrix.android.sdk.api.MatrixConfiguration import org.matrix.android.sdk.api.NoOpMatrixCallback +import org.matrix.android.sdk.api.crypto.OutboundSessionKeySharingStrategy import org.matrix.android.sdk.api.extensions.orFalse import org.matrix.android.sdk.api.extensions.tryOrNull +import org.matrix.android.sdk.api.session.Session +import org.matrix.android.sdk.api.session.crypto.CryptoService import org.matrix.android.sdk.api.session.events.model.EventType import org.matrix.android.sdk.api.session.events.model.RelationType import org.matrix.android.sdk.api.session.events.model.toModel @@ -80,8 +84,11 @@ internal class DefaultTimeline( private val timelineInput: TimelineInput, private val eventDecryptor: TimelineEventDecryptor, private val realmSessionProvider: RealmSessionProvider, - private val loadRoomMembersTask: LoadRoomMembersTask -) : Timeline, + private val loadRoomMembersTask: LoadRoomMembersTask, + private val session: Session, + private val matrixConfiguration: MatrixConfiguration, + private val cryptoService: CryptoService + ) : Timeline, TimelineHiddenReadReceipts.Delegate, TimelineInput.Listener { @@ -188,6 +195,11 @@ internal class DefaultTimeline( } .executeBy(taskExecutor) + if (session.getRoom(roomId)?.isEncrypted().orFalse() + && matrixConfiguration.outboundSessionKeySharingStrategy == OutboundSessionKeySharingStrategy.WhenEnteringRoom) { + cryptoService.ensureOutboundSession(roomId) + } + isReady.set(true) } } diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/typing/SendTypingTask.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/typing/SendTypingTask.kt index 3b56d04872..aa8dcf6013 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/typing/SendTypingTask.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/typing/SendTypingTask.kt @@ -21,6 +21,11 @@ import org.matrix.android.sdk.internal.network.executeRequest import org.matrix.android.sdk.internal.session.room.RoomAPI import org.matrix.android.sdk.internal.task.Task import kotlinx.coroutines.delay +import org.matrix.android.sdk.api.MatrixConfiguration +import org.matrix.android.sdk.api.crypto.OutboundSessionKeySharingStrategy +import org.matrix.android.sdk.api.extensions.orFalse +import org.matrix.android.sdk.api.session.Session +import org.matrix.android.sdk.api.session.crypto.CryptoService import org.matrix.android.sdk.internal.network.GlobalErrorReceiver import javax.inject.Inject @@ -38,12 +43,21 @@ internal interface SendTypingTask : Task { internal class DefaultSendTypingTask @Inject constructor( private val roomAPI: RoomAPI, @UserId private val userId: String, - private val globalErrorReceiver: GlobalErrorReceiver + private val globalErrorReceiver: GlobalErrorReceiver, + private val matrixConfiguration: MatrixConfiguration, + private val session: Session, + private val cryptoService: CryptoService ) : SendTypingTask { override suspend fun execute(params: SendTypingTask.Params) { delay(params.delay ?: -1) + if (params.isTyping + && session.getRoom(params.roomId)?.isEncrypted().orFalse() + && matrixConfiguration.outboundSessionKeySharingStrategy == OutboundSessionKeySharingStrategy.WhenTyping) { + cryptoService.ensureOutboundSession(params.roomId) + } + executeRequest(globalErrorReceiver) { apiCall = roomAPI.sendTypingState( params.roomId, diff --git a/vector/build.gradle b/vector/build.gradle index 5543d27d95..a257f6aaf6 100644 --- a/vector/build.gradle +++ b/vector/build.gradle @@ -136,6 +136,8 @@ android { buildConfigField "String", "BUILD_NUMBER", "\"${buildNumber}\"" resValue "string", "build_number", "\"${buildNumber}\"" + buildConfigField "org.matrix.android.sdk.api.crypto.OutboundSessionKeySharingStrategy", "OutboundSessionKeySharingStrategy", "org.matrix.android.sdk.api.crypto.OutboundSessionKeySharingStrategy.WhenTyping" + testInstrumentationRunner "androidx.test.runner.AndroidJUnitRunner" // Keep abiFilter for the universalApk diff --git a/vector/src/main/java/im/vector/app/VectorApplication.kt b/vector/src/main/java/im/vector/app/VectorApplication.kt index f8cda2c417..1926a35c9c 100644 --- a/vector/src/main/java/im/vector/app/VectorApplication.kt +++ b/vector/src/main/java/im/vector/app/VectorApplication.kt @@ -205,7 +205,7 @@ class VectorApplication : } } - override fun providesMatrixConfiguration() = MatrixConfiguration(BuildConfig.FLAVOR_DESCRIPTION) + override fun providesMatrixConfiguration() = MatrixConfiguration(applicationFlavor = BuildConfig.FLAVOR_DESCRIPTION, outboundSessionKeySharingStrategy = BuildConfig.OutboundSessionKeySharingStrategy) override fun getWorkManagerConfiguration(): WorkConfiguration { return WorkConfiguration.Builder() From e92edc7cb1c90566cca19df868716a9c6ade1381 Mon Sep 17 00:00:00 2001 From: Onuray Sahin Date: Thu, 4 Feb 2021 12:18:32 +0300 Subject: [PATCH 02/12] Fix remember to call the function. --- .../sdk/internal/crypto/algorithms/megolm/MXMegolmEncryption.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 3a07dbcfb6..42c19925ae 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 @@ -138,7 +138,7 @@ internal class MXMegolmEncryption( } override suspend fun ensureOutboundSession(usersInRoom: List) { - getDevicesInRoom(usersInRoom).allowedDevices + ensureOutboundSession(getDevicesInRoom(usersInRoom).allowedDevices) } private suspend fun ensureOutboundSession(devicesInRoom: MXUsersDevicesMap): MXOutboundSessionInfo { From a2de80091d3d588e029f362645b7166d2430768d Mon Sep 17 00:00:00 2001 From: Onuray Sahin Date: Fri, 5 Feb 2021 11:56:42 +0300 Subject: [PATCH 03/12] Code review fixes. --- .../internal/crypto/DefaultCryptoService.kt | 15 ++++- .../crypto/IncomingGossipingRequestManager.kt | 13 +++-- .../crypto/algorithms/IMXEncrypting.kt | 37 ------------ .../crypto/algorithms/IMXGroupEncryption.kt | 57 +++++++++++++++++++ .../algorithms/megolm/MXMegolmEncryption.kt | 3 +- .../crypto/algorithms/olm/MXOlmEncryption.kt | 13 ----- .../session/room/DefaultRoomService.kt | 23 +++++++- .../session/room/timeline/DefaultTimeline.kt | 23 +------- .../room/typing/DefaultTypingService.kt | 15 ++++- .../session/room/typing/SendTypingTask.kt | 16 +----- 10 files changed, 119 insertions(+), 96 deletions(-) create mode 100644 matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/algorithms/IMXGroupEncryption.kt diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/DefaultCryptoService.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/DefaultCryptoService.kt index 821ac18e3a..72cf8b261d 100755 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/DefaultCryptoService.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/DefaultCryptoService.kt @@ -53,6 +53,7 @@ import org.matrix.android.sdk.api.session.room.model.RoomMemberContent import org.matrix.android.sdk.internal.crypto.actions.MegolmSessionDataImporter import org.matrix.android.sdk.internal.crypto.actions.SetDeviceVerificationAction import org.matrix.android.sdk.internal.crypto.algorithms.IMXEncrypting +import org.matrix.android.sdk.internal.crypto.algorithms.IMXGroupEncryption import org.matrix.android.sdk.internal.crypto.algorithms.IMXWithHeldExtension import org.matrix.android.sdk.internal.crypto.algorithms.megolm.MXMegolmEncryptionFactory import org.matrix.android.sdk.internal.crypto.algorithms.olm.MXOlmEncryptionFactory @@ -667,7 +668,12 @@ internal class DefaultCryptoService @Inject constructor( override fun discardOutboundSession(roomId: String) { cryptoCoroutineScope.launch(coroutineDispatchers.crypto) { - roomEncryptorsStore.get(roomId)?.discardSessionKey() + val roomEncryptor = roomEncryptorsStore.get(roomId) + if (roomEncryptor is IMXGroupEncryption) { + roomEncryptor.discardSessionKey() + } else { + Timber.e("## CRYPTO | discardOutboundSession() for:$roomId: Unable to handle IMXGroupEncryption") + } } } @@ -1298,7 +1304,12 @@ internal class DefaultCryptoService @Inject constructor( getEncryptionAlgorithm(roomId)?.let { safeAlgorithm -> val userIds = getRoomUserIds(roomId) if (setEncryptionInRoom(roomId, safeAlgorithm, false, userIds)) { - roomEncryptorsStore.get(roomId)?.ensureOutboundSession(getRoomUserIds(roomId)) + val roomEncryptor = roomEncryptorsStore.get(roomId) + if (roomEncryptor is IMXGroupEncryption) { + roomEncryptor.ensureOutboundSession(getRoomUserIds(roomId)) + } else { + Timber.e("## CRYPTO | ensureOutboundSession() for:$roomId: Unable to handle IMXGroupEncryption for $safeAlgorithm") + } } } } diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/IncomingGossipingRequestManager.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/IncomingGossipingRequestManager.kt index c075c90eb9..4a0a274f4f 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/IncomingGossipingRequestManager.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/IncomingGossipingRequestManager.kt @@ -28,6 +28,7 @@ import org.matrix.android.sdk.api.session.crypto.keyshare.GossipingRequestListen import org.matrix.android.sdk.api.session.events.model.Event import org.matrix.android.sdk.api.session.events.model.EventType import org.matrix.android.sdk.api.session.events.model.toModel +import org.matrix.android.sdk.internal.crypto.algorithms.IMXGroupEncryption import org.matrix.android.sdk.internal.crypto.crosssigning.toBase64NoPadding import org.matrix.android.sdk.internal.crypto.keysbackup.util.extractCurveKeyFromRecoveryKey import org.matrix.android.sdk.internal.crypto.model.rest.GossipingDefaultContent @@ -290,12 +291,16 @@ internal class IncomingGossipingRequestManager @Inject constructor( .also { cryptoStore.updateGossipingRequestState(request, GossipingRequestState.REJECTED) } cryptoCoroutineScope.launch(coroutineDispatchers.crypto) { - val isSuccess = roomEncryptor.reshareKey(sessionId, userId, deviceId, senderKey) + if (roomEncryptor is IMXGroupEncryption) { + val isSuccess = roomEncryptor.reshareKey(sessionId, userId, deviceId, senderKey) - if (isSuccess) { - cryptoStore.updateGossipingRequestState(request, GossipingRequestState.ACCEPTED) + if (isSuccess) { + cryptoStore.updateGossipingRequestState(request, GossipingRequestState.ACCEPTED) + } else { + cryptoStore.updateGossipingRequestState(request, GossipingRequestState.UNABLE_TO_PROCESS) + } } else { - cryptoStore.updateGossipingRequestState(request, GossipingRequestState.UNABLE_TO_PROCESS) + Timber.e("## CRYPTO | handleKeyRequestFromOtherUser() from:$userId: Unable to handle IMXGroupEncryption.reshareKey for $alg") } } cryptoStore.updateGossipingRequestState(request, GossipingRequestState.RE_REQUESTED) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/algorithms/IMXEncrypting.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/algorithms/IMXEncrypting.kt index 008ef1b084..5294429198 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/algorithms/IMXEncrypting.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/algorithms/IMXEncrypting.kt @@ -32,41 +32,4 @@ internal interface IMXEncrypting { * @return the encrypted content */ suspend fun encryptEventContent(eventContent: Content, eventType: String, userIds: List): Content - - /** - * In Megolm, each recipient maintains a record of the ratchet value which allows - * them to decrypt any messages sent in the session after the corresponding point - * in the conversation. If this value is compromised, an attacker can similarly - * decrypt past messages which were encrypted by a key derived from the - * compromised or subsequent ratchet values. This gives 'partial' forward - * secrecy. - * - * To mitigate this issue, the application should offer the user the option to - * discard historical conversations, by winding forward any stored ratchet values, - * or discarding sessions altogether. - */ - fun discardSessionKey() - - /** - * Re-shares a session key with devices if the key has already been - * sent to them. - * - * @param sessionId The id of the outbound session to share. - * @param userId The id of the user who owns the target device. - * @param deviceId The id of the target device. - * @param senderKey The key of the originating device for the session. - * - * @return true in case of success - */ - suspend fun reshareKey(sessionId: String, - userId: String, - deviceId: String, - senderKey: String): Boolean - - /** - * Ensure the outbound session - * - * @param usersInRoom the users in the room - */ - suspend fun ensureOutboundSession(usersInRoom: List) } diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/algorithms/IMXGroupEncryption.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/algorithms/IMXGroupEncryption.kt new file mode 100644 index 0000000000..97b005659d --- /dev/null +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/algorithms/IMXGroupEncryption.kt @@ -0,0 +1,57 @@ +/* + * Copyright 2020 The Matrix.org Foundation C.I.C. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.matrix.android.sdk.internal.crypto.algorithms + +internal interface IMXGroupEncryption { + + /** + * In Megolm, each recipient maintains a record of the ratchet value which allows + * them to decrypt any messages sent in the session after the corresponding point + * in the conversation. If this value is compromised, an attacker can similarly + * decrypt past messages which were encrypted by a key derived from the + * compromised or subsequent ratchet values. This gives 'partial' forward + * secrecy. + * + * To mitigate this issue, the application should offer the user the option to + * discard historical conversations, by winding forward any stored ratchet values, + * or discarding sessions altogether. + */ + fun discardSessionKey() + + /** + * Re-shares a session key with devices if the key has already been + * sent to them. + * + * @param sessionId The id of the outbound session to share. + * @param userId The id of the user who owns the target device. + * @param deviceId The id of the target device. + * @param senderKey The key of the originating device for the session. + * + * @return true in case of success + */ + suspend fun reshareKey(sessionId: String, + userId: String, + deviceId: String, + senderKey: String): Boolean + + /** + * Ensure the outbound session + * + * @param usersInRoom the users in the room + */ + suspend fun ensureOutboundSession(usersInRoom: List) +} 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 42c19925ae..be85fcec8e 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 @@ -30,6 +30,7 @@ import org.matrix.android.sdk.internal.crypto.MXOlmDevice import org.matrix.android.sdk.internal.crypto.actions.EnsureOlmSessionsForDevicesAction import org.matrix.android.sdk.internal.crypto.actions.MessageEncrypter import org.matrix.android.sdk.internal.crypto.algorithms.IMXEncrypting +import org.matrix.android.sdk.internal.crypto.algorithms.IMXGroupEncryption import org.matrix.android.sdk.internal.crypto.keysbackup.DefaultKeysBackupService import org.matrix.android.sdk.internal.crypto.model.CryptoDeviceInfo import org.matrix.android.sdk.internal.crypto.model.MXUsersDevicesMap @@ -61,7 +62,7 @@ internal class MXMegolmEncryption( private val taskExecutor: TaskExecutor, private val coroutineDispatchers: MatrixCoroutineDispatchers, private val cryptoCoroutineScope: CoroutineScope -) : IMXEncrypting { +) : IMXEncrypting, IMXGroupEncryption { // OutboundSessionInfo. Null if we haven't yet started setting one up. Note // that even if this is non-null, it may not be ready for use (in which diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/algorithms/olm/MXOlmEncryption.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/algorithms/olm/MXOlmEncryption.kt index acea6ff3fe..65f78e11f0 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/algorithms/olm/MXOlmEncryption.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/algorithms/olm/MXOlmEncryption.kt @@ -76,17 +76,4 @@ internal class MXOlmEncryption( deviceListManager.downloadKeys(users, false) ensureOlmSessionsForUsersAction.handle(users) } - - override fun discardSessionKey() { - // No need for olm - } - - override suspend fun reshareKey(sessionId: String, userId: String, deviceId: String, senderKey: String): Boolean { - // No need for olm - return false - } - - override suspend fun ensureOutboundSession(usersInRoom: List) { - // NOOP - } } diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/DefaultRoomService.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/DefaultRoomService.kt index 383dd876d3..c60150004f 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/DefaultRoomService.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/DefaultRoomService.kt @@ -20,6 +20,11 @@ import androidx.lifecycle.LiveData import androidx.lifecycle.Transformations import com.zhuinden.monarchy.Monarchy import org.matrix.android.sdk.api.MatrixCallback +import org.matrix.android.sdk.api.MatrixConfiguration +import org.matrix.android.sdk.api.NoOpMatrixCallback +import org.matrix.android.sdk.api.crypto.OutboundSessionKeySharingStrategy +import org.matrix.android.sdk.api.extensions.orFalse +import org.matrix.android.sdk.api.session.crypto.CryptoService import org.matrix.android.sdk.api.session.events.model.Event import org.matrix.android.sdk.api.session.room.Room import org.matrix.android.sdk.api.session.room.RoomService @@ -39,6 +44,7 @@ import org.matrix.android.sdk.internal.session.room.alias.DeleteRoomAliasTask import org.matrix.android.sdk.internal.session.room.alias.GetRoomIdByAliasTask import org.matrix.android.sdk.internal.session.room.alias.RoomAliasDescription import org.matrix.android.sdk.internal.session.room.create.CreateRoomTask +import org.matrix.android.sdk.internal.session.room.membership.LoadRoomMembersTask import org.matrix.android.sdk.internal.session.room.membership.RoomChangeMembershipStateDataSource import org.matrix.android.sdk.internal.session.room.membership.RoomMemberHelper import org.matrix.android.sdk.internal.session.room.membership.joining.JoinRoomTask @@ -65,7 +71,10 @@ internal class DefaultRoomService @Inject constructor( private val roomGetter: RoomGetter, private val roomSummaryDataSource: RoomSummaryDataSource, private val roomChangeMembershipStateDataSource: RoomChangeMembershipStateDataSource, - private val taskExecutor: TaskExecutor + private val taskExecutor: TaskExecutor, + private val loadRoomMembersTask: LoadRoomMembersTask, + private val matrixConfiguration: MatrixConfiguration, + private val cryptoService: CryptoService ) : RoomService { override fun createRoom(createRoomParams: CreateRoomParams, callback: MatrixCallback): Cancelable { @@ -105,6 +114,18 @@ internal class DefaultRoomService @Inject constructor( } override fun onRoomDisplayed(roomId: String): Cancelable { + // Ensure to load all room members + loadRoomMembersTask + .configureWith(LoadRoomMembersTask.Params(roomId)) { + this.callback = NoOpMatrixCallback() + } + .executeBy(taskExecutor) + + // Ensure to share the outbound session keys with all members + if (roomGetter.getRoom(roomId)?.isEncrypted().orFalse() + && matrixConfiguration.outboundSessionKeySharingStrategy == OutboundSessionKeySharingStrategy.WhenEnteringRoom) { + cryptoService.ensureOutboundSession(roomId) + } return updateBreadcrumbsTask .configureWith(UpdateBreadcrumbsTask.Params(roomId)) .executeBy(taskExecutor) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/timeline/DefaultTimeline.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/timeline/DefaultTimeline.kt index e75beb4b21..1be5d55cad 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/timeline/DefaultTimeline.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/timeline/DefaultTimeline.kt @@ -24,13 +24,8 @@ import io.realm.RealmQuery import io.realm.RealmResults import io.realm.Sort import org.matrix.android.sdk.api.MatrixCallback -import org.matrix.android.sdk.api.MatrixConfiguration -import org.matrix.android.sdk.api.NoOpMatrixCallback -import org.matrix.android.sdk.api.crypto.OutboundSessionKeySharingStrategy import org.matrix.android.sdk.api.extensions.orFalse import org.matrix.android.sdk.api.extensions.tryOrNull -import org.matrix.android.sdk.api.session.Session -import org.matrix.android.sdk.api.session.crypto.CryptoService import org.matrix.android.sdk.api.session.events.model.EventType import org.matrix.android.sdk.api.session.events.model.RelationType import org.matrix.android.sdk.api.session.events.model.toModel @@ -54,7 +49,6 @@ import org.matrix.android.sdk.internal.database.query.filterEvents import org.matrix.android.sdk.internal.database.query.findAllInRoomWithSendStates import org.matrix.android.sdk.internal.database.query.where import org.matrix.android.sdk.internal.database.query.whereRoomId -import org.matrix.android.sdk.internal.session.room.membership.LoadRoomMembersTask import org.matrix.android.sdk.internal.task.TaskExecutor import org.matrix.android.sdk.internal.task.configureWith import org.matrix.android.sdk.internal.util.Debouncer @@ -83,11 +77,7 @@ internal class DefaultTimeline( private val hiddenReadReceipts: TimelineHiddenReadReceipts, private val timelineInput: TimelineInput, private val eventDecryptor: TimelineEventDecryptor, - private val realmSessionProvider: RealmSessionProvider, - private val loadRoomMembersTask: LoadRoomMembersTask, - private val session: Session, - private val matrixConfiguration: MatrixConfiguration, - private val cryptoService: CryptoService + private val realmSessionProvider: RealmSessionProvider ) : Timeline, TimelineHiddenReadReceipts.Delegate, TimelineInput.Listener { @@ -189,17 +179,6 @@ internal class DefaultTimeline( hiddenReadReceipts.start(realm, filteredEvents, nonFilteredEvents, this) } - loadRoomMembersTask - .configureWith(LoadRoomMembersTask.Params(roomId)) { - this.callback = NoOpMatrixCallback() - } - .executeBy(taskExecutor) - - if (session.getRoom(roomId)?.isEncrypted().orFalse() - && matrixConfiguration.outboundSessionKeySharingStrategy == OutboundSessionKeySharingStrategy.WhenEnteringRoom) { - cryptoService.ensureOutboundSession(roomId) - } - isReady.set(true) } } diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/typing/DefaultTypingService.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/typing/DefaultTypingService.kt index 39b7967bc1..279ab3f334 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/typing/DefaultTypingService.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/typing/DefaultTypingService.kt @@ -21,8 +21,13 @@ import dagger.assisted.Assisted import dagger.assisted.AssistedInject import dagger.assisted.AssistedFactory import org.matrix.android.sdk.api.MatrixCallback +import org.matrix.android.sdk.api.MatrixConfiguration +import org.matrix.android.sdk.api.crypto.OutboundSessionKeySharingStrategy +import org.matrix.android.sdk.api.extensions.orFalse +import org.matrix.android.sdk.api.session.crypto.CryptoService import org.matrix.android.sdk.api.session.room.typing.TypingService import org.matrix.android.sdk.api.util.Cancelable +import org.matrix.android.sdk.internal.session.room.RoomGetter import org.matrix.android.sdk.internal.task.TaskExecutor import org.matrix.android.sdk.internal.task.configureWith import timber.log.Timber @@ -36,7 +41,10 @@ import timber.log.Timber internal class DefaultTypingService @AssistedInject constructor( @Assisted private val roomId: String, private val taskExecutor: TaskExecutor, - private val sendTypingTask: SendTypingTask + private val sendTypingTask: SendTypingTask, + private val matrixConfiguration: MatrixConfiguration, + private val cryptoService: CryptoService, + private val roomGetter: RoomGetter ) : TypingService { @AssistedFactory @@ -69,6 +77,11 @@ internal class DefaultTypingService @AssistedInject constructor( currentTask?.cancel() + if (roomGetter.getRoom(roomId)?.isEncrypted().orFalse() + && matrixConfiguration.outboundSessionKeySharingStrategy == OutboundSessionKeySharingStrategy.WhenTyping) { + cryptoService.ensureOutboundSession(roomId) + } + val params = SendTypingTask.Params(roomId, true) currentTask = sendTypingTask .configureWith(params) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/typing/SendTypingTask.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/typing/SendTypingTask.kt index aa8dcf6013..3b56d04872 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/typing/SendTypingTask.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/typing/SendTypingTask.kt @@ -21,11 +21,6 @@ import org.matrix.android.sdk.internal.network.executeRequest import org.matrix.android.sdk.internal.session.room.RoomAPI import org.matrix.android.sdk.internal.task.Task import kotlinx.coroutines.delay -import org.matrix.android.sdk.api.MatrixConfiguration -import org.matrix.android.sdk.api.crypto.OutboundSessionKeySharingStrategy -import org.matrix.android.sdk.api.extensions.orFalse -import org.matrix.android.sdk.api.session.Session -import org.matrix.android.sdk.api.session.crypto.CryptoService import org.matrix.android.sdk.internal.network.GlobalErrorReceiver import javax.inject.Inject @@ -43,21 +38,12 @@ internal interface SendTypingTask : Task { internal class DefaultSendTypingTask @Inject constructor( private val roomAPI: RoomAPI, @UserId private val userId: String, - private val globalErrorReceiver: GlobalErrorReceiver, - private val matrixConfiguration: MatrixConfiguration, - private val session: Session, - private val cryptoService: CryptoService + private val globalErrorReceiver: GlobalErrorReceiver ) : SendTypingTask { override suspend fun execute(params: SendTypingTask.Params) { delay(params.delay ?: -1) - if (params.isTyping - && session.getRoom(params.roomId)?.isEncrypted().orFalse() - && matrixConfiguration.outboundSessionKeySharingStrategy == OutboundSessionKeySharingStrategy.WhenTyping) { - cryptoService.ensureOutboundSession(params.roomId) - } - executeRequest(globalErrorReceiver) { apiCall = roomAPI.sendTypingState( params.roomId, From a623395585744c0826421ffe54f506741af1bda0 Mon Sep 17 00:00:00 2001 From: Onuray Sahin Date: Mon, 8 Feb 2021 17:04:50 +0300 Subject: [PATCH 04/12] Code review fixes. --- .../android/sdk/api/MatrixConfiguration.kt | 1 - .../session/room/crypto/RoomCryptoService.kt | 7 ++++ .../internal/crypto/DefaultCryptoService.kt | 42 ++++++++++++------- .../crypto/algorithms/IMXGroupEncryption.kt | 7 ---- .../algorithms/megolm/MXMegolmEncryption.kt | 9 ++-- .../sdk/internal/session/room/DefaultRoom.kt | 4 ++ .../session/room/DefaultRoomService.kt | 23 +--------- .../session/room/timeline/DefaultTimeline.kt | 13 +++++- .../room/typing/DefaultTypingService.kt | 15 +------ vector/build.gradle | 2 +- .../java/im/vector/app/VectorApplication.kt | 2 +- .../home/room/detail/RoomDetailViewModel.kt | 11 +++++ 12 files changed, 68 insertions(+), 68 deletions(-) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/MatrixConfiguration.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/MatrixConfiguration.kt index c01aeab6dd..9f651a21fa 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/MatrixConfiguration.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/MatrixConfiguration.kt @@ -17,7 +17,6 @@ package org.matrix.android.sdk.api import org.matrix.android.sdk.api.crypto.MXCryptoConfig -import org.matrix.android.sdk.api.crypto.OutboundSessionKeySharingStrategy import java.net.Proxy data class MatrixConfiguration( diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/room/crypto/RoomCryptoService.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/room/crypto/RoomCryptoService.kt index 1251fd9857..bb157ec7ee 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/room/crypto/RoomCryptoService.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/room/crypto/RoomCryptoService.kt @@ -30,4 +30,11 @@ interface RoomCryptoService { * Enable encryption of the room */ suspend fun enableEncryption(algorithm: String = MXCRYPTO_ALGORITHM_MEGOLM) + + /** + * Ensures all members of the room are loaded and outbound session keys are shared. + * Call this method according to [OutboundSessionKeySharingStrategy]. + * If this method is not called, CryptoService will ensure it before sending events. + */ + fun ensureOutboundSession() } diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/DefaultCryptoService.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/DefaultCryptoService.kt index 72cf8b261d..1a90377664 100755 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/DefaultCryptoService.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/DefaultCryptoService.kt @@ -50,6 +50,7 @@ import org.matrix.android.sdk.api.session.room.model.Membership import org.matrix.android.sdk.api.session.room.model.RoomHistoryVisibility import org.matrix.android.sdk.api.session.room.model.RoomHistoryVisibilityContent import org.matrix.android.sdk.api.session.room.model.RoomMemberContent +import org.matrix.android.sdk.api.util.emptyJsonDict import org.matrix.android.sdk.internal.crypto.actions.MegolmSessionDataImporter import org.matrix.android.sdk.internal.crypto.actions.SetDeviceVerificationAction import org.matrix.android.sdk.internal.crypto.algorithms.IMXEncrypting @@ -709,7 +710,7 @@ internal class DefaultCryptoService @Inject constructor( */ @Throws(MXCryptoError::class) private fun internalDecryptEvent(event: Event, timeline: String): MXEventDecryptionResult { - return eventDecryptor.decryptEvent(event, timeline) + return eventDecryptor.decryptEvent(event, timeline) } /** @@ -1297,22 +1298,31 @@ internal class DefaultCryptoService @Inject constructor( } override fun ensureOutboundSession(roomId: String) { + // Ensure to load all room members + loadRoomMembersTask + .configureWith(LoadRoomMembersTask.Params(roomId)) { + this.callback = NoOpMatrixCallback() + } + .executeBy(taskExecutor) + cryptoCoroutineScope.launch(coroutineDispatchers.crypto) { - roomEncryptorsStore - .get(roomId) - ?.let { - getEncryptionAlgorithm(roomId)?.let { safeAlgorithm -> - val userIds = getRoomUserIds(roomId) - if (setEncryptionInRoom(roomId, safeAlgorithm, false, userIds)) { - val roomEncryptor = roomEncryptorsStore.get(roomId) - if (roomEncryptor is IMXGroupEncryption) { - roomEncryptor.ensureOutboundSession(getRoomUserIds(roomId)) - } else { - Timber.e("## CRYPTO | ensureOutboundSession() for:$roomId: Unable to handle IMXGroupEncryption for $safeAlgorithm") - } - } - } - } + val userIds = getRoomUserIds(roomId) + val alg = roomEncryptorsStore.get(roomId) + ?: getEncryptionAlgorithm(roomId) + ?.let { setEncryptionInRoom(roomId, it, false, userIds) } + ?.let { roomEncryptorsStore.get(roomId) } + + if (alg == null) { + val reason = String.format(MXCryptoError.UNABLE_TO_ENCRYPT_REASON, MXCryptoError.NO_MORE_ALGORITHM_REASON) + Timber.e("## CRYPTO | encryptEventContent() : $reason") + return@launch + } + + runCatching { + alg.encryptEventContent(emptyJsonDict, EventType.DUMMY, userIds) + }.onFailure { + Timber.e("## CRYPTO | encryptEventContent() failed.") + } } } diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/algorithms/IMXGroupEncryption.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/algorithms/IMXGroupEncryption.kt index 97b005659d..2d0cf4bc01 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/algorithms/IMXGroupEncryption.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/algorithms/IMXGroupEncryption.kt @@ -47,11 +47,4 @@ internal interface IMXGroupEncryption { userId: String, deviceId: String, senderKey: String): Boolean - - /** - * Ensure the outbound session - * - * @param usersInRoom the users in the room - */ - suspend fun ensureOutboundSession(usersInRoom: List) } 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 be85fcec8e..db4f7279c6 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 @@ -138,10 +138,11 @@ internal class MXMegolmEncryption( return MXOutboundSessionInfo(sessionId, SharedWithHelper(roomId, sessionId, cryptoStore)) } - override suspend fun ensureOutboundSession(usersInRoom: List) { - ensureOutboundSession(getDevicesInRoom(usersInRoom).allowedDevices) - } - + /** + * Ensure the outbound session + * + * @param devicesInRoom the devices list + */ private suspend fun ensureOutboundSession(devicesInRoom: MXUsersDevicesMap): MXOutboundSessionInfo { Timber.v("## CRYPTO | ensureOutboundSession start") var session = outboundSession diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/DefaultRoom.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/DefaultRoom.kt index 7a819250cf..a2b7a37b9b 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/DefaultRoom.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/DefaultRoom.kt @@ -104,6 +104,10 @@ internal class DefaultRoom @Inject constructor(override val roomId: String, return cryptoService.shouldEncryptForInvitedMembers(roomId) } + override fun ensureOutboundSession() { + cryptoService.ensureOutboundSession(roomId) + } + override suspend fun enableEncryption(algorithm: String) { when { isEncrypted() -> { diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/DefaultRoomService.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/DefaultRoomService.kt index c60150004f..383dd876d3 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/DefaultRoomService.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/DefaultRoomService.kt @@ -20,11 +20,6 @@ import androidx.lifecycle.LiveData import androidx.lifecycle.Transformations import com.zhuinden.monarchy.Monarchy import org.matrix.android.sdk.api.MatrixCallback -import org.matrix.android.sdk.api.MatrixConfiguration -import org.matrix.android.sdk.api.NoOpMatrixCallback -import org.matrix.android.sdk.api.crypto.OutboundSessionKeySharingStrategy -import org.matrix.android.sdk.api.extensions.orFalse -import org.matrix.android.sdk.api.session.crypto.CryptoService import org.matrix.android.sdk.api.session.events.model.Event import org.matrix.android.sdk.api.session.room.Room import org.matrix.android.sdk.api.session.room.RoomService @@ -44,7 +39,6 @@ import org.matrix.android.sdk.internal.session.room.alias.DeleteRoomAliasTask import org.matrix.android.sdk.internal.session.room.alias.GetRoomIdByAliasTask import org.matrix.android.sdk.internal.session.room.alias.RoomAliasDescription import org.matrix.android.sdk.internal.session.room.create.CreateRoomTask -import org.matrix.android.sdk.internal.session.room.membership.LoadRoomMembersTask import org.matrix.android.sdk.internal.session.room.membership.RoomChangeMembershipStateDataSource import org.matrix.android.sdk.internal.session.room.membership.RoomMemberHelper import org.matrix.android.sdk.internal.session.room.membership.joining.JoinRoomTask @@ -71,10 +65,7 @@ internal class DefaultRoomService @Inject constructor( private val roomGetter: RoomGetter, private val roomSummaryDataSource: RoomSummaryDataSource, private val roomChangeMembershipStateDataSource: RoomChangeMembershipStateDataSource, - private val taskExecutor: TaskExecutor, - private val loadRoomMembersTask: LoadRoomMembersTask, - private val matrixConfiguration: MatrixConfiguration, - private val cryptoService: CryptoService + private val taskExecutor: TaskExecutor ) : RoomService { override fun createRoom(createRoomParams: CreateRoomParams, callback: MatrixCallback): Cancelable { @@ -114,18 +105,6 @@ internal class DefaultRoomService @Inject constructor( } override fun onRoomDisplayed(roomId: String): Cancelable { - // Ensure to load all room members - loadRoomMembersTask - .configureWith(LoadRoomMembersTask.Params(roomId)) { - this.callback = NoOpMatrixCallback() - } - .executeBy(taskExecutor) - - // Ensure to share the outbound session keys with all members - if (roomGetter.getRoom(roomId)?.isEncrypted().orFalse() - && matrixConfiguration.outboundSessionKeySharingStrategy == OutboundSessionKeySharingStrategy.WhenEnteringRoom) { - cryptoService.ensureOutboundSession(roomId) - } return updateBreadcrumbsTask .configureWith(UpdateBreadcrumbsTask.Params(roomId)) .executeBy(taskExecutor) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/timeline/DefaultTimeline.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/timeline/DefaultTimeline.kt index 1be5d55cad..ca272a3520 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/timeline/DefaultTimeline.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/timeline/DefaultTimeline.kt @@ -24,6 +24,7 @@ import io.realm.RealmQuery import io.realm.RealmResults import io.realm.Sort import org.matrix.android.sdk.api.MatrixCallback +import org.matrix.android.sdk.api.NoOpMatrixCallback import org.matrix.android.sdk.api.extensions.orFalse import org.matrix.android.sdk.api.extensions.tryOrNull import org.matrix.android.sdk.api.session.events.model.EventType @@ -49,6 +50,7 @@ import org.matrix.android.sdk.internal.database.query.filterEvents import org.matrix.android.sdk.internal.database.query.findAllInRoomWithSendStates import org.matrix.android.sdk.internal.database.query.where import org.matrix.android.sdk.internal.database.query.whereRoomId +import org.matrix.android.sdk.internal.session.room.membership.LoadRoomMembersTask import org.matrix.android.sdk.internal.task.TaskExecutor import org.matrix.android.sdk.internal.task.configureWith import org.matrix.android.sdk.internal.util.Debouncer @@ -77,8 +79,9 @@ internal class DefaultTimeline( private val hiddenReadReceipts: TimelineHiddenReadReceipts, private val timelineInput: TimelineInput, private val eventDecryptor: TimelineEventDecryptor, - private val realmSessionProvider: RealmSessionProvider - ) : Timeline, + private val realmSessionProvider: RealmSessionProvider, + private val loadRoomMembersTask: LoadRoomMembersTask +) : Timeline, TimelineHiddenReadReceipts.Delegate, TimelineInput.Listener { @@ -179,6 +182,12 @@ internal class DefaultTimeline( hiddenReadReceipts.start(realm, filteredEvents, nonFilteredEvents, this) } + loadRoomMembersTask + .configureWith(LoadRoomMembersTask.Params(roomId)) { + this.callback = NoOpMatrixCallback() + } + .executeBy(taskExecutor) + isReady.set(true) } } diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/typing/DefaultTypingService.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/typing/DefaultTypingService.kt index 279ab3f334..39b7967bc1 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/typing/DefaultTypingService.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/typing/DefaultTypingService.kt @@ -21,13 +21,8 @@ import dagger.assisted.Assisted import dagger.assisted.AssistedInject import dagger.assisted.AssistedFactory import org.matrix.android.sdk.api.MatrixCallback -import org.matrix.android.sdk.api.MatrixConfiguration -import org.matrix.android.sdk.api.crypto.OutboundSessionKeySharingStrategy -import org.matrix.android.sdk.api.extensions.orFalse -import org.matrix.android.sdk.api.session.crypto.CryptoService import org.matrix.android.sdk.api.session.room.typing.TypingService import org.matrix.android.sdk.api.util.Cancelable -import org.matrix.android.sdk.internal.session.room.RoomGetter import org.matrix.android.sdk.internal.task.TaskExecutor import org.matrix.android.sdk.internal.task.configureWith import timber.log.Timber @@ -41,10 +36,7 @@ import timber.log.Timber internal class DefaultTypingService @AssistedInject constructor( @Assisted private val roomId: String, private val taskExecutor: TaskExecutor, - private val sendTypingTask: SendTypingTask, - private val matrixConfiguration: MatrixConfiguration, - private val cryptoService: CryptoService, - private val roomGetter: RoomGetter + private val sendTypingTask: SendTypingTask ) : TypingService { @AssistedFactory @@ -77,11 +69,6 @@ internal class DefaultTypingService @AssistedInject constructor( currentTask?.cancel() - if (roomGetter.getRoom(roomId)?.isEncrypted().orFalse() - && matrixConfiguration.outboundSessionKeySharingStrategy == OutboundSessionKeySharingStrategy.WhenTyping) { - cryptoService.ensureOutboundSession(roomId) - } - val params = SendTypingTask.Params(roomId, true) currentTask = sendTypingTask .configureWith(params) diff --git a/vector/build.gradle b/vector/build.gradle index a257f6aaf6..c539ebb648 100644 --- a/vector/build.gradle +++ b/vector/build.gradle @@ -136,7 +136,7 @@ android { buildConfigField "String", "BUILD_NUMBER", "\"${buildNumber}\"" resValue "string", "build_number", "\"${buildNumber}\"" - buildConfigField "org.matrix.android.sdk.api.crypto.OutboundSessionKeySharingStrategy", "OutboundSessionKeySharingStrategy", "org.matrix.android.sdk.api.crypto.OutboundSessionKeySharingStrategy.WhenTyping" + buildConfigField "org.matrix.android.sdk.api.crypto.OutboundSessionKeySharingStrategy", "outboundSessionKeySharingStrategy", "org.matrix.android.sdk.api.crypto.OutboundSessionKeySharingStrategy.WhenTyping" testInstrumentationRunner "androidx.test.runner.AndroidJUnitRunner" diff --git a/vector/src/main/java/im/vector/app/VectorApplication.kt b/vector/src/main/java/im/vector/app/VectorApplication.kt index 1926a35c9c..dd94be77ba 100644 --- a/vector/src/main/java/im/vector/app/VectorApplication.kt +++ b/vector/src/main/java/im/vector/app/VectorApplication.kt @@ -205,7 +205,7 @@ class VectorApplication : } } - override fun providesMatrixConfiguration() = MatrixConfiguration(applicationFlavor = BuildConfig.FLAVOR_DESCRIPTION, outboundSessionKeySharingStrategy = BuildConfig.OutboundSessionKeySharingStrategy) + override fun providesMatrixConfiguration() = MatrixConfiguration(applicationFlavor = BuildConfig.FLAVOR_DESCRIPTION) override fun getWorkManagerConfiguration(): WorkConfiguration { return WorkConfiguration.Builder() diff --git a/vector/src/main/java/im/vector/app/features/home/room/detail/RoomDetailViewModel.kt b/vector/src/main/java/im/vector/app/features/home/room/detail/RoomDetailViewModel.kt index 601891b15a..099f2eefff 100644 --- a/vector/src/main/java/im/vector/app/features/home/room/detail/RoomDetailViewModel.kt +++ b/vector/src/main/java/im/vector/app/features/home/room/detail/RoomDetailViewModel.kt @@ -28,6 +28,7 @@ import com.jakewharton.rxrelay2.PublishRelay import dagger.assisted.Assisted import dagger.assisted.AssistedFactory import dagger.assisted.AssistedInject +import im.vector.app.BuildConfig import im.vector.app.R import im.vector.app.core.extensions.exhaustive import im.vector.app.core.platform.VectorViewModel @@ -60,6 +61,7 @@ import org.commonmark.renderer.html.HtmlRenderer import org.matrix.android.sdk.api.MatrixCallback import org.matrix.android.sdk.api.MatrixPatterns import org.matrix.android.sdk.api.NoOpMatrixCallback +import org.matrix.android.sdk.api.crypto.OutboundSessionKeySharingStrategy import org.matrix.android.sdk.api.extensions.tryOrNull import org.matrix.android.sdk.api.query.QueryStringValue import org.matrix.android.sdk.api.raw.RawService @@ -179,6 +181,11 @@ class RoomDetailViewModel @AssistedInject constructor( callManager.addPstnSupportListener(this) callManager.checkForPSTNSupportIfNeeded() chatEffectManager.delegate = this + + // Ensure to share the outbound session keys with all members + if (room.isEncrypted() && BuildConfig.outboundSessionKeySharingStrategy == OutboundSessionKeySharingStrategy.WhenEnteringRoom) { + room.ensureOutboundSession() + } } private fun observePowerLevel() { @@ -591,6 +598,10 @@ class RoomDetailViewModel @AssistedInject constructor( room.userStopsTyping() } } + // Ensure outbound session keys + if (room.isEncrypted() && BuildConfig.outboundSessionKeySharingStrategy == OutboundSessionKeySharingStrategy.WhenTyping) { + room.ensureOutboundSession() + } } private fun handleTombstoneEvent(action: RoomDetailAction.HandleTombstoneEvent) { From 8b39eabc0fae60f7f7fa16ea6bbd09d064068688 Mon Sep 17 00:00:00 2001 From: Onuray Sahin Date: Mon, 8 Feb 2021 17:26:09 +0300 Subject: [PATCH 05/12] Code review fixes. --- .../sdk/internal/crypto/DefaultCryptoService.kt | 14 +++++++------- vector/build.gradle | 2 +- .../OutboundSessionKeySharingStrategy.kt | 2 +- .../home/room/detail/RoomDetailViewModel.kt | 4 ++-- 4 files changed, 11 insertions(+), 11 deletions(-) rename {matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/crypto => vector/src/main/java/im/vector/app/features/crypto/keysrequest}/OutboundSessionKeySharingStrategy.kt (95%) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/DefaultCryptoService.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/DefaultCryptoService.kt index 1a90377664..14542a1ea8 100755 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/DefaultCryptoService.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/DefaultCryptoService.kt @@ -1298,14 +1298,14 @@ internal class DefaultCryptoService @Inject constructor( } override fun ensureOutboundSession(roomId: String) { - // Ensure to load all room members - loadRoomMembersTask - .configureWith(LoadRoomMembersTask.Params(roomId)) { - this.callback = NoOpMatrixCallback() - } - .executeBy(taskExecutor) - cryptoCoroutineScope.launch(coroutineDispatchers.crypto) { + // Ensure to load all room members + loadRoomMembersTask + .configureWith(LoadRoomMembersTask.Params(roomId)) { + this.callback = NoOpMatrixCallback() + } + .executeBy(taskExecutor) + val userIds = getRoomUserIds(roomId) val alg = roomEncryptorsStore.get(roomId) ?: getEncryptionAlgorithm(roomId) diff --git a/vector/build.gradle b/vector/build.gradle index c539ebb648..dc2a16ccc5 100644 --- a/vector/build.gradle +++ b/vector/build.gradle @@ -136,7 +136,7 @@ android { buildConfigField "String", "BUILD_NUMBER", "\"${buildNumber}\"" resValue "string", "build_number", "\"${buildNumber}\"" - buildConfigField "org.matrix.android.sdk.api.crypto.OutboundSessionKeySharingStrategy", "outboundSessionKeySharingStrategy", "org.matrix.android.sdk.api.crypto.OutboundSessionKeySharingStrategy.WhenTyping" + buildConfigField "im.vector.app.features.crypto.keysrequest.OutboundSessionKeySharingStrategy", "outboundSessionKeySharingStrategy", "im.vector.app.features.crypto.keysrequest.OutboundSessionKeySharingStrategy.WhenTyping" testInstrumentationRunner "androidx.test.runner.AndroidJUnitRunner" diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/crypto/OutboundSessionKeySharingStrategy.kt b/vector/src/main/java/im/vector/app/features/crypto/keysrequest/OutboundSessionKeySharingStrategy.kt similarity index 95% rename from matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/crypto/OutboundSessionKeySharingStrategy.kt rename to vector/src/main/java/im/vector/app/features/crypto/keysrequest/OutboundSessionKeySharingStrategy.kt index 9ef1f538b8..e797f4622c 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/crypto/OutboundSessionKeySharingStrategy.kt +++ b/vector/src/main/java/im/vector/app/features/crypto/keysrequest/OutboundSessionKeySharingStrategy.kt @@ -14,7 +14,7 @@ * limitations under the License. */ -package org.matrix.android.sdk.api.crypto +package im.vector.app.features.crypto.keysrequest enum class OutboundSessionKeySharingStrategy { /** diff --git a/vector/src/main/java/im/vector/app/features/home/room/detail/RoomDetailViewModel.kt b/vector/src/main/java/im/vector/app/features/home/room/detail/RoomDetailViewModel.kt index 099f2eefff..ea5787de46 100644 --- a/vector/src/main/java/im/vector/app/features/home/room/detail/RoomDetailViewModel.kt +++ b/vector/src/main/java/im/vector/app/features/home/room/detail/RoomDetailViewModel.kt @@ -61,7 +61,7 @@ import org.commonmark.renderer.html.HtmlRenderer import org.matrix.android.sdk.api.MatrixCallback import org.matrix.android.sdk.api.MatrixPatterns import org.matrix.android.sdk.api.NoOpMatrixCallback -import org.matrix.android.sdk.api.crypto.OutboundSessionKeySharingStrategy +import im.vector.app.features.crypto.keysrequest.OutboundSessionKeySharingStrategy import org.matrix.android.sdk.api.extensions.tryOrNull import org.matrix.android.sdk.api.query.QueryStringValue import org.matrix.android.sdk.api.raw.RawService @@ -599,7 +599,7 @@ class RoomDetailViewModel @AssistedInject constructor( } } // Ensure outbound session keys - if (room.isEncrypted() && BuildConfig.outboundSessionKeySharingStrategy == OutboundSessionKeySharingStrategy.WhenTyping) { + if (action.isTyping && room.isEncrypted() && BuildConfig.outboundSessionKeySharingStrategy == OutboundSessionKeySharingStrategy.WhenTyping) { room.ensureOutboundSession() } } From 11dffacc485e196baa9a3a71a7f4eab4e7a980ca Mon Sep 17 00:00:00 2001 From: Onuray Sahin Date: Mon, 8 Feb 2021 17:39:25 +0300 Subject: [PATCH 06/12] Code review fixes. --- .../android/sdk/internal/crypto/DefaultCryptoService.kt | 6 +----- .../crypto/keysrequest/OutboundSessionKeySharingStrategy.kt | 2 +- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/DefaultCryptoService.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/DefaultCryptoService.kt index 14542a1ea8..08991b2d5f 100755 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/DefaultCryptoService.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/DefaultCryptoService.kt @@ -1300,11 +1300,7 @@ internal class DefaultCryptoService @Inject constructor( override fun ensureOutboundSession(roomId: String) { cryptoCoroutineScope.launch(coroutineDispatchers.crypto) { // Ensure to load all room members - loadRoomMembersTask - .configureWith(LoadRoomMembersTask.Params(roomId)) { - this.callback = NoOpMatrixCallback() - } - .executeBy(taskExecutor) + loadRoomMembersTask.execute(LoadRoomMembersTask.Params(roomId)) val userIds = getRoomUserIds(roomId) val alg = roomEncryptorsStore.get(roomId) diff --git a/vector/src/main/java/im/vector/app/features/crypto/keysrequest/OutboundSessionKeySharingStrategy.kt b/vector/src/main/java/im/vector/app/features/crypto/keysrequest/OutboundSessionKeySharingStrategy.kt index e797f4622c..19c62ed572 100644 --- a/vector/src/main/java/im/vector/app/features/crypto/keysrequest/OutboundSessionKeySharingStrategy.kt +++ b/vector/src/main/java/im/vector/app/features/crypto/keysrequest/OutboundSessionKeySharingStrategy.kt @@ -1,5 +1,5 @@ /* - * Copyright 2020 The Matrix.org Foundation C.I.C. + * Copyright (c) 2021 New Vector Ltd * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. From fae484cb9593f385fc5c868246f4e526ec621de9 Mon Sep 17 00:00:00 2001 From: Onuray Sahin Date: Tue, 9 Feb 2021 22:37:48 +0300 Subject: [PATCH 07/12] Create test for ensureOutboundSession function. --- .../sdk/internal/crypto/CryptoServiceTest.kt | 62 +++++++++++++++++++ 1 file changed, 62 insertions(+) create mode 100644 matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/internal/crypto/CryptoServiceTest.kt diff --git a/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/internal/crypto/CryptoServiceTest.kt b/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/internal/crypto/CryptoServiceTest.kt new file mode 100644 index 0000000000..6467705b01 --- /dev/null +++ b/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/internal/crypto/CryptoServiceTest.kt @@ -0,0 +1,62 @@ +/* + * Copyright 2020 The Matrix.org Foundation C.I.C. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.matrix.android.sdk.internal.crypto + +import androidx.test.ext.junit.runners.AndroidJUnit4 +import org.junit.FixMethodOrder +import org.junit.Test +import org.junit.runner.RunWith +import org.junit.runners.MethodSorters +import org.matrix.android.sdk.InstrumentedTest +import org.matrix.android.sdk.common.CommonTestHelper +import org.matrix.android.sdk.common.CryptoTestHelper +import org.matrix.android.sdk.common.SessionTestParams +import org.matrix.android.sdk.common.TestConstants +import kotlin.test.assertTrue + +@RunWith(AndroidJUnit4::class) +@FixMethodOrder(MethodSorters.JVM) +class CryptoServiceTest : InstrumentedTest { + + private val mTestHelper = CommonTestHelper(context()) + private val mCryptoTestHelper = CryptoTestHelper(mTestHelper) + + @Test + fun ensure_outbound_session_happy_path() { + val aliceSession = mTestHelper.createAccount(TestConstants.USER_ALICE, SessionTestParams(true)) + val bobSession = mTestHelper.createAccount(TestConstants.USER_ALICE, SessionTestParams(true)) + + // Initialize cross signing on both + mCryptoTestHelper.initializeCrossSigning(aliceSession) + mCryptoTestHelper.initializeCrossSigning(bobSession) + + val roomId = mCryptoTestHelper.createDM(aliceSession, bobSession) + mCryptoTestHelper.verifySASCrossSign(aliceSession, bobSession, roomId) + + aliceSession.cryptoService().ensureOutboundSession(roomId) + + assertTrue( + aliceSession + .cryptoService() + .getGossipingEvents() + .map { it.senderId } + .containsAll( + listOf(aliceSession.myUserId, bobSession.myUserId) + ) + ) + } +} From 4450f51d7846f7bd8462592c4a27f812d8782a1b Mon Sep 17 00:00:00 2001 From: Onuray Sahin Date: Wed, 10 Feb 2021 11:53:47 +0300 Subject: [PATCH 08/12] runCatching added to loadMembersTask execution. --- .../android/sdk/internal/crypto/DefaultCryptoService.kt | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/DefaultCryptoService.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/DefaultCryptoService.kt index 08991b2d5f..e254be74ed 100755 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/DefaultCryptoService.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/DefaultCryptoService.kt @@ -1300,7 +1300,9 @@ internal class DefaultCryptoService @Inject constructor( override fun ensureOutboundSession(roomId: String) { cryptoCoroutineScope.launch(coroutineDispatchers.crypto) { // Ensure to load all room members - loadRoomMembersTask.execute(LoadRoomMembersTask.Params(roomId)) + runCatching { + loadRoomMembersTask.execute(LoadRoomMembersTask.Params(roomId)) + } val userIds = getRoomUserIds(roomId) val alg = roomEncryptorsStore.get(roomId) From 533a7bb180669ee5347a17126bc23a56f1e66946 Mon Sep 17 00:00:00 2001 From: Valere Date: Wed, 17 Feb 2021 16:47:40 +0100 Subject: [PATCH 09/12] Code Review --- .../sdk/internal/crypto/CryptoServiceTest.kt | 62 ----------- .../sdk/internal/crypto/PreShareKeysTest.kt | 103 ++++++++++++++++++ .../crypto/gossiping/WithHeldTests.kt | 2 +- .../android/sdk/api/MatrixConfiguration.kt | 1 - .../sdk/api/session/crypto/CryptoService.kt | 6 +- .../session/room/crypto/RoomCryptoService.kt | 2 +- .../internal/crypto/DefaultCryptoService.kt | 26 +++-- .../sdk/internal/crypto/EventDecryptor.kt | 2 +- .../crypto/algorithms/IMXGroupEncryption.kt | 2 + .../algorithms/megolm/MXMegolmEncryption.kt | 21 +++- .../sdk/internal/session/room/DefaultRoom.kt | 7 +- .../home/room/detail/RoomDetailAction.kt | 2 + .../home/room/detail/RoomDetailFragment.kt | 7 ++ .../home/room/detail/RoomDetailViewModel.kt | 39 ++++++- 14 files changed, 197 insertions(+), 85 deletions(-) delete mode 100644 matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/internal/crypto/CryptoServiceTest.kt create mode 100644 matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/internal/crypto/PreShareKeysTest.kt diff --git a/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/internal/crypto/CryptoServiceTest.kt b/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/internal/crypto/CryptoServiceTest.kt deleted file mode 100644 index 6467705b01..0000000000 --- a/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/internal/crypto/CryptoServiceTest.kt +++ /dev/null @@ -1,62 +0,0 @@ -/* - * Copyright 2020 The Matrix.org Foundation C.I.C. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.matrix.android.sdk.internal.crypto - -import androidx.test.ext.junit.runners.AndroidJUnit4 -import org.junit.FixMethodOrder -import org.junit.Test -import org.junit.runner.RunWith -import org.junit.runners.MethodSorters -import org.matrix.android.sdk.InstrumentedTest -import org.matrix.android.sdk.common.CommonTestHelper -import org.matrix.android.sdk.common.CryptoTestHelper -import org.matrix.android.sdk.common.SessionTestParams -import org.matrix.android.sdk.common.TestConstants -import kotlin.test.assertTrue - -@RunWith(AndroidJUnit4::class) -@FixMethodOrder(MethodSorters.JVM) -class CryptoServiceTest : InstrumentedTest { - - private val mTestHelper = CommonTestHelper(context()) - private val mCryptoTestHelper = CryptoTestHelper(mTestHelper) - - @Test - fun ensure_outbound_session_happy_path() { - val aliceSession = mTestHelper.createAccount(TestConstants.USER_ALICE, SessionTestParams(true)) - val bobSession = mTestHelper.createAccount(TestConstants.USER_ALICE, SessionTestParams(true)) - - // Initialize cross signing on both - mCryptoTestHelper.initializeCrossSigning(aliceSession) - mCryptoTestHelper.initializeCrossSigning(bobSession) - - val roomId = mCryptoTestHelper.createDM(aliceSession, bobSession) - mCryptoTestHelper.verifySASCrossSign(aliceSession, bobSession, roomId) - - aliceSession.cryptoService().ensureOutboundSession(roomId) - - assertTrue( - aliceSession - .cryptoService() - .getGossipingEvents() - .map { it.senderId } - .containsAll( - listOf(aliceSession.myUserId, bobSession.myUserId) - ) - ) - } -} diff --git a/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/internal/crypto/PreShareKeysTest.kt b/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/internal/crypto/PreShareKeysTest.kt new file mode 100644 index 0000000000..122584142e --- /dev/null +++ b/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/internal/crypto/PreShareKeysTest.kt @@ -0,0 +1,103 @@ +/* + * Copyright 2020 The Matrix.org Foundation C.I.C. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.matrix.android.sdk.internal.crypto + +import android.util.Log +import androidx.test.ext.junit.runners.AndroidJUnit4 +import org.junit.FixMethodOrder +import org.junit.Test +import org.junit.runner.RunWith +import org.junit.runners.MethodSorters +import org.matrix.android.sdk.InstrumentedTest +import org.matrix.android.sdk.api.session.events.model.EventType +import org.matrix.android.sdk.api.session.events.model.toModel +import org.matrix.android.sdk.common.CommonTestHelper +import org.matrix.android.sdk.common.CryptoTestHelper +import org.matrix.android.sdk.internal.crypto.model.event.EncryptedEventContent +import org.matrix.android.sdk.internal.crypto.model.event.RoomKeyContent +import kotlin.test.assertEquals +import kotlin.test.assertNotNull + +@RunWith(AndroidJUnit4::class) +@FixMethodOrder(MethodSorters.JVM) +class PreShareKeysTest : InstrumentedTest { + + private val mTestHelper = CommonTestHelper(context()) + private val mCryptoTestHelper = CryptoTestHelper(mTestHelper) + + @Test + fun ensure_outbound_session_happy_path() { + val testData = mCryptoTestHelper.doE2ETestWithAliceAndBobInARoom(true) + val e2eRoomID = testData.roomId + val aliceSession = testData.firstSession + val bobSession = testData.secondSession!! + + // clear any outbound session + aliceSession.cryptoService().discardOutboundSession(e2eRoomID) + + val preShareCount = bobSession.cryptoService().getGossipingEvents().count { + it.senderId == aliceSession.myUserId + && it.getClearType() == EventType.ROOM_KEY + } + + assertEquals(0, preShareCount, "Bob should not have receive any key from alice at this point") + Log.d("#Test", "Room Key Received from alice $preShareCount") + + // Force presharing of new outbound key + mTestHelper.doSync { + aliceSession.cryptoService().prepareToEncrypt(e2eRoomID, it) + } + + mTestHelper.waitWithLatch { latch -> + mTestHelper.retryPeriodicallyWithLatch(latch) { + val newGossipCount = bobSession.cryptoService().getGossipingEvents().count { + it.senderId == aliceSession.myUserId + && it.getClearType() == EventType.ROOM_KEY + } + newGossipCount > preShareCount + } + } + + val latest = bobSession.cryptoService().getGossipingEvents().lastOrNull { + it.senderId == aliceSession.myUserId + && it.getClearType() == EventType.ROOM_KEY + } + + val content = latest?.getClearContent().toModel() + assertNotNull(content, "Bob should have received and decrypted a room key event from alice") + assertEquals(e2eRoomID, content.roomId, "Wrong room") + val megolmSessionId = content.sessionId!! + + val sharedIndex = aliceSession.cryptoService().getSharedWithInfo(e2eRoomID, megolmSessionId) + .getObject(bobSession.myUserId, bobSession.sessionParams.deviceId) + + assertEquals(0, sharedIndex, "The session received by bob should match what alice sent") + + // Just send a real message as test + val sentEvent = mTestHelper.sendTextMessage(aliceSession.getRoom(e2eRoomID)!!, "Allo", 1).first() + + assertEquals(megolmSessionId, sentEvent.root.content.toModel()?.sessionId, "Unexpected megolm session") + mTestHelper.waitWithLatch { latch -> + mTestHelper.retryPeriodicallyWithLatch(latch) { + bobSession.getRoom(e2eRoomID)?.getTimeLineEvent(sentEvent.eventId)?.root?.getClearType() == EventType.MESSAGE + } + } + + mTestHelper.signOutAndClose(aliceSession) + mTestHelper.signOutAndClose(bobSession) + } +} diff --git a/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/internal/crypto/gossiping/WithHeldTests.kt b/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/internal/crypto/gossiping/WithHeldTests.kt index 80cc14fcb6..b2516ea2be 100644 --- a/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/internal/crypto/gossiping/WithHeldTests.kt +++ b/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/internal/crypto/gossiping/WithHeldTests.kt @@ -51,7 +51,7 @@ class WithHeldTests : InstrumentedTest { // ============================= val aliceSession = mTestHelper.createAccount(TestConstants.USER_ALICE, SessionTestParams(true)) - val bobSession = mTestHelper.createAccount(TestConstants.USER_ALICE, SessionTestParams(true)) + val bobSession = mTestHelper.createAccount(TestConstants.USER_BOB, SessionTestParams(true)) // Initialize cross signing on both mCryptoTestHelper.initializeCrossSigning(aliceSession) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/MatrixConfiguration.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/MatrixConfiguration.kt index 9f651a21fa..93a1b962ed 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/MatrixConfiguration.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/MatrixConfiguration.kt @@ -40,7 +40,6 @@ data class MatrixConfiguration( * True to advertise support for call transfers to other parties on Matrix calls. */ val supportsCallTransfer: Boolean = false - val outboundSessionKeySharingStrategy: OutboundSessionKeySharingStrategy = OutboundSessionKeySharingStrategy.WhenSendingEvent ) { /** diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/crypto/CryptoService.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/crypto/CryptoService.kt index 1c5bfb1a7b..1b7a5243e2 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/crypto/CryptoService.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/crypto/CryptoService.kt @@ -157,5 +157,9 @@ interface CryptoService { fun logDbUsageInfo() - fun ensureOutboundSession(roomId: String) + /** + * Perform any background tasks that can be done before a message is ready to + * send, in order to speed up sending of the message. + */ + fun prepareToEncrypt(roomId: String, callback: MatrixCallback) } diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/room/crypto/RoomCryptoService.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/room/crypto/RoomCryptoService.kt index bb157ec7ee..d8bec878a1 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/room/crypto/RoomCryptoService.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/room/crypto/RoomCryptoService.kt @@ -36,5 +36,5 @@ interface RoomCryptoService { * Call this method according to [OutboundSessionKeySharingStrategy]. * If this method is not called, CryptoService will ensure it before sending events. */ - fun ensureOutboundSession() + suspend fun prepareToEncrypt() } diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/DefaultCryptoService.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/DefaultCryptoService.kt index e254be74ed..17d25736eb 100755 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/DefaultCryptoService.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/DefaultCryptoService.kt @@ -50,7 +50,6 @@ import org.matrix.android.sdk.api.session.room.model.Membership import org.matrix.android.sdk.api.session.room.model.RoomHistoryVisibility import org.matrix.android.sdk.api.session.room.model.RoomHistoryVisibilityContent import org.matrix.android.sdk.api.session.room.model.RoomMemberContent -import org.matrix.android.sdk.api.util.emptyJsonDict import org.matrix.android.sdk.internal.crypto.actions.MegolmSessionDataImporter import org.matrix.android.sdk.internal.crypto.actions.SetDeviceVerificationAction import org.matrix.android.sdk.internal.crypto.algorithms.IMXEncrypting @@ -99,7 +98,6 @@ import org.matrix.olm.OlmManager import timber.log.Timber import java.util.concurrent.atomic.AtomicBoolean import javax.inject.Inject -import kotlin.jvm.Throws import kotlin.math.max /** @@ -1297,11 +1295,16 @@ internal class DefaultCryptoService @Inject constructor( cryptoStore.logDbUsageInfo() } - override fun ensureOutboundSession(roomId: String) { + override fun prepareToEncrypt(roomId: String, callback: MatrixCallback) { cryptoCoroutineScope.launch(coroutineDispatchers.crypto) { + Timber.d("## CRYPTO | prepareToEncrypt() : Check room members up to date") // Ensure to load all room members - runCatching { + try { loadRoomMembersTask.execute(LoadRoomMembersTask.Params(roomId)) + } catch (failure: Throwable) { + Timber.e("## CRYPTO | prepareToEncrypt() : Failed to load room members") + callback.onFailure(failure) + return@launch } val userIds = getRoomUserIds(roomId) @@ -1312,15 +1315,20 @@ internal class DefaultCryptoService @Inject constructor( if (alg == null) { val reason = String.format(MXCryptoError.UNABLE_TO_ENCRYPT_REASON, MXCryptoError.NO_MORE_ALGORITHM_REASON) - Timber.e("## CRYPTO | encryptEventContent() : $reason") + Timber.e("## CRYPTO | prepareToEncrypt() : $reason") + callback.onFailure(IllegalArgumentException("Missing algorithm")) return@launch } runCatching { - alg.encryptEventContent(emptyJsonDict, EventType.DUMMY, userIds) - }.onFailure { - Timber.e("## CRYPTO | encryptEventContent() failed.") - } + (alg as? IMXGroupEncryption)?.preshareKey(userIds) + }.fold( + { callback.onSuccess(Unit) }, + { + Timber.e("## CRYPTO | prepareToEncrypt() failed.") + callback.onFailure(it) + } + ) } } diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/EventDecryptor.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/EventDecryptor.kt index 92b7728890..32324896fa 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/EventDecryptor.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/EventDecryptor.kt @@ -105,7 +105,7 @@ internal class EventDecryptor @Inject constructor( try { return alg.decryptEvent(event, timeline) } catch (mxCryptoError: MXCryptoError) { - Timber.d("## CRYPTO | internalDecryptEvent : Failed to decrypt ${event.eventId} reason: $mxCryptoError") + Timber.v("## CRYPTO | internalDecryptEvent : Failed to decrypt ${event.eventId} reason: $mxCryptoError") if (algorithm == MXCRYPTO_ALGORITHM_OLM) { if (mxCryptoError is MXCryptoError.Base && mxCryptoError.errorType == MXCryptoError.ErrorType.BAD_ENCRYPTED_MESSAGE) { diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/algorithms/IMXGroupEncryption.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/algorithms/IMXGroupEncryption.kt index 2d0cf4bc01..1fd5061a65 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/algorithms/IMXGroupEncryption.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/algorithms/IMXGroupEncryption.kt @@ -32,6 +32,8 @@ internal interface IMXGroupEncryption { */ fun discardSessionKey() + suspend fun preshareKey(userIds: List) + /** * Re-shares a session key with devices if the key has already been * sent to them. 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 db4f7279c6..bb75d014ff 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 @@ -94,6 +94,7 @@ internal class MXMegolmEncryption( // annoyingly we have to serialize again the saved outbound session to store message index :/ // if not we would see duplicate message index errors olmDevice.storeOutboundGroupSessionForRoom(roomId, outboundSession.sessionId) + Timber.v("## CRYPTO | encryptEventContent: Finished in ${System.currentTimeMillis() - ts} millis") } } @@ -118,6 +119,16 @@ internal class MXMegolmEncryption( olmDevice.discardOutboundGroupSessionForRoom(roomId) } + override suspend fun preshareKey(userIds: List) { + val ts = System.currentTimeMillis() + Timber.v("## CRYPTO | preshareKey : getDevicesInRoom") + val devices = getDevicesInRoom(userIds) + val outboundSession = ensureOutboundSession(devices.allowedDevices) + + notifyWithheldForSession(devices.withHeldDevices, outboundSession) + + Timber.v("## CRYPTO | preshareKey ${System.currentTimeMillis() - ts} millis") + } /** * Prepare a new session. * @@ -253,7 +264,7 @@ internal class MXMegolmEncryption( continue } - Timber.i("## CRYPTO | shareUserDevicesKey() : Sharing keys with device $userId:$deviceID") + Timber.i("## CRYPTO | shareUserDevicesKey() : Add to share keys contentMap for $userId:$deviceID") contentMap.setObject(userId, deviceID, messageEncrypter.encryptMessage(payload, listOf(sessionResult.deviceInfo))) haveTargets = true } @@ -264,10 +275,12 @@ internal class MXMegolmEncryption( // attempted to share with) rather than the contentMap (those we did // share with), because we don't want to try to claim a one-time-key // for dead devices on every message. + val gossipingEventBuffer = arrayListOf() for ((userId, devicesToShareWith) in devicesByUser) { for ((deviceId) in devicesToShareWith) { session.sharedWithHelper.markedSessionAsShared(userId, deviceId, chainIndex) - cryptoStore.saveGossipingEvent(Event( + gossipingEventBuffer.add( + Event( type = EventType.ROOM_KEY, senderId = credentials.userId, content = submap.apply { @@ -279,6 +292,8 @@ internal class MXMegolmEncryption( } } + cryptoStore.saveGossipingEvents(gossipingEventBuffer) + if (haveTargets) { t0 = System.currentTimeMillis() Timber.i("## CRYPTO | shareUserDevicesKey() ${session.sessionId} : has target") @@ -296,7 +311,7 @@ internal class MXMegolmEncryption( } private fun notifyKeyWithHeld(targets: List, sessionId: String, senderKey: String?, code: WithHeldCode) { - Timber.i("## CRYPTO | notifyKeyWithHeld() :sending withheld key for $targets session:$sessionId ") + Timber.i("## CRYPTO | notifyKeyWithHeld() :sending withheld key for $targets session:$sessionId and code $code ") val withHeldContent = RoomKeyWithHeldContent( roomId = roomId, senderKey = senderKey, diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/DefaultRoom.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/DefaultRoom.kt index a2b7a37b9b..8e817ec31a 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/DefaultRoom.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/DefaultRoom.kt @@ -45,6 +45,7 @@ import org.matrix.android.sdk.internal.session.room.summary.RoomSummaryDataSourc import org.matrix.android.sdk.internal.session.search.SearchTask import org.matrix.android.sdk.internal.task.TaskExecutor import org.matrix.android.sdk.internal.task.configureWith +import org.matrix.android.sdk.internal.util.awaitCallback import java.security.InvalidParameterException import javax.inject.Inject @@ -104,8 +105,10 @@ internal class DefaultRoom @Inject constructor(override val roomId: String, return cryptoService.shouldEncryptForInvitedMembers(roomId) } - override fun ensureOutboundSession() { - cryptoService.ensureOutboundSession(roomId) + override suspend fun prepareToEncrypt() { + awaitCallback { + cryptoService.prepareToEncrypt(roomId, it) + } } override suspend fun enableEncryption(algorithm: String) { diff --git a/vector/src/main/java/im/vector/app/features/home/room/detail/RoomDetailAction.kt b/vector/src/main/java/im/vector/app/features/home/room/detail/RoomDetailAction.kt index 98ad6c454c..efc495a379 100644 --- a/vector/src/main/java/im/vector/app/features/home/room/detail/RoomDetailAction.kt +++ b/vector/src/main/java/im/vector/app/features/home/room/detail/RoomDetailAction.kt @@ -104,4 +104,6 @@ sealed class RoomDetailAction : VectorViewModelAction { // Preview URL data class DoNotShowPreviewUrlFor(val eventId: String, val url: String) : RoomDetailAction() + + data class ComposerFocusChange(val focused: Boolean) : RoomDetailAction() } diff --git a/vector/src/main/java/im/vector/app/features/home/room/detail/RoomDetailFragment.kt b/vector/src/main/java/im/vector/app/features/home/room/detail/RoomDetailFragment.kt index c511c9e666..d51ed08083 100644 --- a/vector/src/main/java/im/vector/app/features/home/room/detail/RoomDetailFragment.kt +++ b/vector/src/main/java/im/vector/app/features/home/room/detail/RoomDetailFragment.kt @@ -70,6 +70,7 @@ import com.airbnb.mvrx.args import com.airbnb.mvrx.fragmentViewModel import com.airbnb.mvrx.withState import com.google.android.material.snackbar.Snackbar +import com.jakewharton.rxbinding3.view.focusChanges import com.jakewharton.rxbinding3.widget.textChanges import com.vanniktech.emoji.EmojiPopup import im.vector.app.R @@ -1144,6 +1145,12 @@ class RoomDetailFragment @Inject constructor( roomDetailViewModel.handle(RoomDetailAction.UserIsTyping(it)) } .disposeOnDestroyView() + + views.composerLayout.views.composerEditText.focusChanges() + .subscribe { + roomDetailViewModel.handle(RoomDetailAction.ComposerFocusChange(it)) + } + .disposeOnDestroyView() } private fun sendUri(uri: Uri): Boolean { diff --git a/vector/src/main/java/im/vector/app/features/home/room/detail/RoomDetailViewModel.kt b/vector/src/main/java/im/vector/app/features/home/room/detail/RoomDetailViewModel.kt index ea5787de46..977efebb38 100644 --- a/vector/src/main/java/im/vector/app/features/home/room/detail/RoomDetailViewModel.kt +++ b/vector/src/main/java/im/vector/app/features/home/room/detail/RoomDetailViewModel.kt @@ -19,9 +19,13 @@ package im.vector.app.features.home.room.detail import android.net.Uri import androidx.annotation.IdRes import androidx.lifecycle.viewModelScope +import com.airbnb.mvrx.Async +import com.airbnb.mvrx.Fail import com.airbnb.mvrx.FragmentViewModelContext +import com.airbnb.mvrx.Loading import com.airbnb.mvrx.MvRxViewModelFactory import com.airbnb.mvrx.Success +import com.airbnb.mvrx.Uninitialized import com.airbnb.mvrx.ViewModelContext import com.jakewharton.rxrelay2.BehaviorRelay import com.jakewharton.rxrelay2.PublishRelay @@ -37,6 +41,7 @@ import im.vector.app.features.call.dialpad.DialPadLookup import im.vector.app.features.call.webrtc.WebRtcCallManager import im.vector.app.features.command.CommandParser import im.vector.app.features.command.ParsedCommand +import im.vector.app.features.crypto.keysrequest.OutboundSessionKeySharingStrategy import im.vector.app.features.createdirect.DirectRoomHelper import im.vector.app.features.crypto.verification.SupportedVerificationMethodsProvider import im.vector.app.features.home.room.detail.composer.rainbow.RainbowGenerator @@ -61,7 +66,6 @@ import org.commonmark.renderer.html.HtmlRenderer import org.matrix.android.sdk.api.MatrixCallback import org.matrix.android.sdk.api.MatrixPatterns import org.matrix.android.sdk.api.NoOpMatrixCallback -import im.vector.app.features.crypto.keysrequest.OutboundSessionKeySharingStrategy import org.matrix.android.sdk.api.extensions.tryOrNull import org.matrix.android.sdk.api.query.QueryStringValue import org.matrix.android.sdk.api.raw.RawService @@ -142,6 +146,8 @@ class RoomDetailViewModel @AssistedInject constructor( private var trackUnreadMessages = AtomicBoolean(false) private var mostRecentDisplayedEvent: TimelineEvent? = null + private var prepareToEncrypt: Async = Uninitialized + @AssistedFactory interface Factory { fun create(initialState: RoomDetailViewState): RoomDetailViewModel @@ -184,7 +190,23 @@ class RoomDetailViewModel @AssistedInject constructor( // Ensure to share the outbound session keys with all members if (room.isEncrypted() && BuildConfig.outboundSessionKeySharingStrategy == OutboundSessionKeySharingStrategy.WhenEnteringRoom) { - room.ensureOutboundSession() + prepareForEncryption() + } + } + + private fun prepareForEncryption() { + // check if there is not already a call made + if (prepareToEncrypt !is Loading) { + prepareToEncrypt = Loading() + viewModelScope.launch { + kotlin.runCatching { + room.prepareToEncrypt() + }.fold({ + prepareToEncrypt = Success(Unit) + }, { + prepareToEncrypt = Fail(it) + }) + } } } @@ -241,6 +263,7 @@ class RoomDetailViewModel @AssistedInject constructor( override fun handle(action: RoomDetailAction) { when (action) { is RoomDetailAction.UserIsTyping -> handleUserIsTyping(action) + is RoomDetailAction.ComposerFocusChange -> handleComposerFocusChange(action) is RoomDetailAction.SaveDraft -> handleSaveDraft(action) is RoomDetailAction.SendMessage -> handleSendMessage(action) is RoomDetailAction.SendMedia -> handleSendMedia(action) @@ -598,9 +621,17 @@ class RoomDetailViewModel @AssistedInject constructor( room.userStopsTyping() } } + } + + private fun handleComposerFocusChange(action: RoomDetailAction.ComposerFocusChange) { // Ensure outbound session keys - if (action.isTyping && room.isEncrypted() && BuildConfig.outboundSessionKeySharingStrategy == OutboundSessionKeySharingStrategy.WhenTyping) { - room.ensureOutboundSession() + if (BuildConfig.outboundSessionKeySharingStrategy == OutboundSessionKeySharingStrategy.WhenTyping && room.isEncrypted()) { + if (action.focused) { + // Should we add some rate limit here, or do it only once per model lifecycle? + prepareForEncryption() + } else { + // we could eventually cancel here :/ + } } } From 91872fe6734c893c0b552967a77bfe0574a22958 Mon Sep 17 00:00:00 2001 From: Valere Date: Wed, 17 Feb 2021 17:08:10 +0100 Subject: [PATCH 10/12] cleaning --- .../app/features/home/room/detail/RoomDetailViewModel.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/vector/src/main/java/im/vector/app/features/home/room/detail/RoomDetailViewModel.kt b/vector/src/main/java/im/vector/app/features/home/room/detail/RoomDetailViewModel.kt index 977efebb38..7365801589 100644 --- a/vector/src/main/java/im/vector/app/features/home/room/detail/RoomDetailViewModel.kt +++ b/vector/src/main/java/im/vector/app/features/home/room/detail/RoomDetailViewModel.kt @@ -189,7 +189,7 @@ class RoomDetailViewModel @AssistedInject constructor( chatEffectManager.delegate = this // Ensure to share the outbound session keys with all members - if (room.isEncrypted() && BuildConfig.outboundSessionKeySharingStrategy == OutboundSessionKeySharingStrategy.WhenEnteringRoom) { + if (OutboundSessionKeySharingStrategy.WhenEnteringRoom == BuildConfig.outboundSessionKeySharingStrategy && room.isEncrypted()) { prepareForEncryption() } } @@ -625,7 +625,7 @@ class RoomDetailViewModel @AssistedInject constructor( private fun handleComposerFocusChange(action: RoomDetailAction.ComposerFocusChange) { // Ensure outbound session keys - if (BuildConfig.outboundSessionKeySharingStrategy == OutboundSessionKeySharingStrategy.WhenTyping && room.isEncrypted()) { + if (OutboundSessionKeySharingStrategy.WhenTyping == BuildConfig.outboundSessionKeySharingStrategy && room.isEncrypted()) { if (action.focused) { // Should we add some rate limit here, or do it only once per model lifecycle? prepareForEncryption() From 5c9750fb0741fb2db5cb818e4d3e379d85f18480 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Tue, 2 Mar 2021 19:18:19 +0100 Subject: [PATCH 11/12] Moar cleanup after rebase and before merge --- CHANGES.md | 2 +- .../session/room/crypto/RoomCryptoService.kt | 1 - .../algorithms/megolm/MXMegolmEncryption.kt | 31 ++++++++--------- .../megolm/MXMegolmEncryptionFactory.kt | 34 +++++++++---------- .../home/room/detail/RoomDetailViewModel.kt | 8 ++--- 5 files changed, 35 insertions(+), 41 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index a3eb4bfdf9..8692a2cff8 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -10,6 +10,7 @@ Improvements 🙌: - Improve initial sync performance (#983) - PIP support for Jitsi call (#2418) - Add tooltip for room quick actions + - Pre-share session keys when opening a room or start typing (#2771) Bugfix 🐛: - Try to fix crash about UrlPreview (#2640) @@ -41,7 +42,6 @@ Improvements 🙌: - Improve room profile UX - Upgrade Jitsi library from 2.9.3 to 3.1.0 - a11y improvements - - Pre-share session keys when opening a room or start typing (#2771) Bugfix 🐛: - VoIP : fix audio devices output diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/room/crypto/RoomCryptoService.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/room/crypto/RoomCryptoService.kt index d8bec878a1..6581247b90 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/room/crypto/RoomCryptoService.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/room/crypto/RoomCryptoService.kt @@ -33,7 +33,6 @@ interface RoomCryptoService { /** * Ensures all members of the room are loaded and outbound session keys are shared. - * Call this method according to [OutboundSessionKeySharingStrategy]. * If this method is not called, CryptoService will ensure it before sending events. */ suspend fun prepareToEncrypt() 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 bb75d014ff..6b91c0b859 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 @@ -18,8 +18,6 @@ package org.matrix.android.sdk.internal.crypto.algorithms.megolm import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.launch -import org.matrix.android.sdk.api.MatrixCallback -import org.matrix.android.sdk.api.auth.data.Credentials import org.matrix.android.sdk.api.session.crypto.MXCryptoError import org.matrix.android.sdk.api.session.events.model.Content import org.matrix.android.sdk.api.session.events.model.Event @@ -40,8 +38,6 @@ import org.matrix.android.sdk.internal.crypto.model.forEach import org.matrix.android.sdk.internal.crypto.repository.WarnOnUnknownDeviceRepository import org.matrix.android.sdk.internal.crypto.store.IMXCryptoStore import org.matrix.android.sdk.internal.crypto.tasks.SendToDeviceTask -import org.matrix.android.sdk.internal.task.TaskExecutor -import org.matrix.android.sdk.internal.task.configureWith import org.matrix.android.sdk.internal.util.JsonCanonicalizer import org.matrix.android.sdk.internal.util.MatrixCoroutineDispatchers import org.matrix.android.sdk.internal.util.convertToUTF8 @@ -55,11 +51,11 @@ internal class MXMegolmEncryption( private val cryptoStore: IMXCryptoStore, private val deviceListManager: DeviceListManager, private val ensureOlmSessionsForDevicesAction: EnsureOlmSessionsForDevicesAction, - private val credentials: Credentials, + private val userId: String, + private val deviceId: String, private val sendToDeviceTask: SendToDeviceTask, private val messageEncrypter: MessageEncrypter, private val warnOnUnknownDevicesRepository: WarnOnUnknownDeviceRepository, - private val taskExecutor: TaskExecutor, private val coroutineDispatchers: MatrixCoroutineDispatchers, private val cryptoCoroutineScope: CoroutineScope ) : IMXEncrypting, IMXGroupEncryption { @@ -282,7 +278,7 @@ internal class MXMegolmEncryption( gossipingEventBuffer.add( Event( type = EventType.ROOM_KEY, - senderId = credentials.userId, + senderId = this.userId, content = submap.apply { this["session_key"] = "" // we add a fake key for trail @@ -310,8 +306,11 @@ internal class MXMegolmEncryption( } } - private fun notifyKeyWithHeld(targets: List, sessionId: String, senderKey: String?, code: WithHeldCode) { - Timber.i("## CRYPTO | notifyKeyWithHeld() :sending withheld key for $targets session:$sessionId and code $code ") + private suspend fun notifyKeyWithHeld(targets: List, + sessionId: String, + senderKey: String?, + code: WithHeldCode) { + Timber.i("## CRYPTO | notifyKeyWithHeld() :sending withheld key for $targets session:$sessionId and code $code") val withHeldContent = RoomKeyWithHeldContent( roomId = roomId, senderKey = senderKey, @@ -327,13 +326,11 @@ internal class MXMegolmEncryption( } } ) - sendToDeviceTask.configureWith(params) { - callback = object : MatrixCallback { - override fun onFailure(failure: Throwable) { - Timber.e("## CRYPTO | notifyKeyWithHeld() : Failed to notify withheld key for $targets session: $sessionId ") - } - } - }.executeBy(taskExecutor) + try { + sendToDeviceTask.execute(params) + } catch (failure: Throwable) { + Timber.e("## CRYPTO | notifyKeyWithHeld() : Failed to notify withheld key for $targets session: $sessionId ") + } } /** @@ -359,7 +356,7 @@ internal class MXMegolmEncryption( // Include our device ID so that recipients can send us a // m.new_device message if they don't have our session key. - map["device_id"] = credentials.deviceId!! + map["device_id"] = deviceId session.useCount++ return map } diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/algorithms/megolm/MXMegolmEncryptionFactory.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/algorithms/megolm/MXMegolmEncryptionFactory.kt index f0cc15fb63..9f6312ea97 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/algorithms/megolm/MXMegolmEncryptionFactory.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/algorithms/megolm/MXMegolmEncryptionFactory.kt @@ -17,7 +17,6 @@ package org.matrix.android.sdk.internal.crypto.algorithms.megolm import kotlinx.coroutines.CoroutineScope -import org.matrix.android.sdk.api.auth.data.Credentials import org.matrix.android.sdk.internal.crypto.DeviceListManager import org.matrix.android.sdk.internal.crypto.MXOlmDevice import org.matrix.android.sdk.internal.crypto.actions.EnsureOlmSessionsForDevicesAction @@ -26,7 +25,8 @@ import org.matrix.android.sdk.internal.crypto.keysbackup.DefaultKeysBackupServic import org.matrix.android.sdk.internal.crypto.repository.WarnOnUnknownDeviceRepository import org.matrix.android.sdk.internal.crypto.store.IMXCryptoStore import org.matrix.android.sdk.internal.crypto.tasks.SendToDeviceTask -import org.matrix.android.sdk.internal.task.TaskExecutor +import org.matrix.android.sdk.internal.di.DeviceId +import org.matrix.android.sdk.internal.di.UserId import org.matrix.android.sdk.internal.util.MatrixCoroutineDispatchers import javax.inject.Inject @@ -36,29 +36,29 @@ internal class MXMegolmEncryptionFactory @Inject constructor( private val cryptoStore: IMXCryptoStore, private val deviceListManager: DeviceListManager, private val ensureOlmSessionsForDevicesAction: EnsureOlmSessionsForDevicesAction, - private val credentials: Credentials, + @UserId private val userId: String, + @DeviceId private val deviceId: String?, private val sendToDeviceTask: SendToDeviceTask, private val messageEncrypter: MessageEncrypter, private val warnOnUnknownDevicesRepository: WarnOnUnknownDeviceRepository, - private val taskExecutor: TaskExecutor, private val coroutineDispatchers: MatrixCoroutineDispatchers, private val cryptoCoroutineScope: CoroutineScope) { fun create(roomId: String): MXMegolmEncryption { return MXMegolmEncryption( - roomId, - olmDevice, - defaultKeysBackupService, - cryptoStore, - deviceListManager, - ensureOlmSessionsForDevicesAction, - credentials, - sendToDeviceTask, - messageEncrypter, - warnOnUnknownDevicesRepository, - taskExecutor, - coroutineDispatchers, - cryptoCoroutineScope + roomId = roomId, + olmDevice = olmDevice, + defaultKeysBackupService = defaultKeysBackupService, + cryptoStore = cryptoStore, + deviceListManager = deviceListManager, + ensureOlmSessionsForDevicesAction = ensureOlmSessionsForDevicesAction, + userId = userId, + deviceId = deviceId!!, + sendToDeviceTask = sendToDeviceTask, + messageEncrypter = messageEncrypter, + warnOnUnknownDevicesRepository = warnOnUnknownDevicesRepository, + coroutineDispatchers = coroutineDispatchers, + cryptoCoroutineScope = cryptoCoroutineScope ) } } diff --git a/vector/src/main/java/im/vector/app/features/home/room/detail/RoomDetailViewModel.kt b/vector/src/main/java/im/vector/app/features/home/room/detail/RoomDetailViewModel.kt index 7365801589..935b4ca2f8 100644 --- a/vector/src/main/java/im/vector/app/features/home/room/detail/RoomDetailViewModel.kt +++ b/vector/src/main/java/im/vector/app/features/home/room/detail/RoomDetailViewModel.kt @@ -195,11 +195,11 @@ class RoomDetailViewModel @AssistedInject constructor( } private fun prepareForEncryption() { - // check if there is not already a call made - if (prepareToEncrypt !is Loading) { + // check if there is not already a call made, or if there has been an error + if (prepareToEncrypt.shouldLoad) { prepareToEncrypt = Loading() viewModelScope.launch { - kotlin.runCatching { + runCatching { room.prepareToEncrypt() }.fold({ prepareToEncrypt = Success(Unit) @@ -629,8 +629,6 @@ class RoomDetailViewModel @AssistedInject constructor( if (action.focused) { // Should we add some rate limit here, or do it only once per model lifecycle? prepareForEncryption() - } else { - // we could eventually cancel here :/ } } } From 259ead106fe96559b9c6c00c28532f7558ead583 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Wed, 3 Mar 2021 09:28:35 +0100 Subject: [PATCH 12/12] Fix code quality issue --- tools/check/forbidden_strings_in_code.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/check/forbidden_strings_in_code.txt b/tools/check/forbidden_strings_in_code.txt index a5127dc8aa..567e24a849 100644 --- a/tools/check/forbidden_strings_in_code.txt +++ b/tools/check/forbidden_strings_in_code.txt @@ -161,7 +161,7 @@ Formatter\.formatShortFileSize===1 # android\.text\.TextUtils ### This is not a rule, but a warning: the number of "enum class" has changed. For Json classes, it is mandatory that they have `@JsonClass(generateAdapter = false)`. If the enum is not used as a Json class, change the value in file forbidden_strings_in_code.txt -enum class===89 +enum class===90 ### Do not import temporary legacy classes import org.matrix.android.sdk.internal.legacy.riot===3