From e3d2186c25eeaf2446f55420250c259dd454485e Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Fri, 12 Mar 2021 18:41:31 +0100 Subject: [PATCH 1/9] Rework UpdateTrustWorker, should have better perf. --- .../crypto/crosssigning/UpdateTrustWorker.kt | 326 +++++++++--------- .../android/sdk/internal/util/LogUtil.kt | 12 + 2 files changed, 176 insertions(+), 162 deletions(-) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/crosssigning/UpdateTrustWorker.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/crosssigning/UpdateTrustWorker.kt index 1660bae0b7..a2a90c49cd 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/crosssigning/UpdateTrustWorker.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/crosssigning/UpdateTrustWorker.kt @@ -36,12 +36,14 @@ import org.matrix.android.sdk.internal.crypto.store.db.model.UserEntityFields import org.matrix.android.sdk.internal.database.model.RoomMemberSummaryEntity import org.matrix.android.sdk.internal.database.model.RoomMemberSummaryEntityFields import org.matrix.android.sdk.internal.database.model.RoomSummaryEntity +import org.matrix.android.sdk.internal.database.model.RoomSummaryEntityFields import org.matrix.android.sdk.internal.database.query.where import org.matrix.android.sdk.internal.di.CryptoDatabase import org.matrix.android.sdk.internal.di.SessionDatabase import org.matrix.android.sdk.internal.di.UserId import org.matrix.android.sdk.internal.session.SessionComponent import org.matrix.android.sdk.internal.session.room.membership.RoomMemberHelper +import org.matrix.android.sdk.internal.util.logLimit import org.matrix.android.sdk.internal.worker.SessionSafeCoroutineWorker import org.matrix.android.sdk.internal.worker.SessionWorkerParams import timber.log.Timber @@ -65,11 +67,14 @@ internal class UpdateTrustWorker(context: Context, @Inject lateinit var crossSigningService: DefaultCrossSigningService // It breaks the crypto store contract, but we need to batch things :/ - @CryptoDatabase @Inject lateinit var realmConfiguration: RealmConfiguration - @UserId @Inject lateinit var myUserId: String + @CryptoDatabase + @Inject lateinit var cryptoRealmConfiguration: RealmConfiguration + @SessionDatabase + @Inject lateinit var sessionRealmConfiguration: RealmConfiguration + @UserId + @Inject lateinit var myUserId: String @Inject lateinit var crossSigningKeysMapper: CrossSigningKeysMapper @Inject lateinit var updateTrustWorkerDataRepository: UpdateTrustWorkerDataRepository - @SessionDatabase @Inject lateinit var sessionRealmConfiguration: RealmConfiguration // @Inject lateinit var roomSummaryUpdater: RoomSummaryUpdater @Inject lateinit var cryptoStore: IMXCryptoStore @@ -79,118 +84,115 @@ internal class UpdateTrustWorker(context: Context, } override suspend fun doSafeWork(params: Params): Result { - var userList = params.filename + val userList = params.filename ?.let { updateTrustWorkerDataRepository.getParam(it) } ?.userIds ?: params.updatedUserIds.orEmpty() - if (userList.isEmpty()) { - // This should not happen, but let's avoid go further in case of empty list - cleanup(params) - return Result.success() + // List should not be empty, but let's avoid go further in case of empty list + if (userList.isNotEmpty()) { + // Unfortunately we don't have much info on what did exactly changed (is it the cross signing keys of that user, + // or a new device?) So we check all again :/ + Timber.d("## CrossSigning - Updating trust for users: ${userList.logLimit()}") + + Realm.getInstance(cryptoRealmConfiguration).use { cryptoRealm -> + Realm.getInstance(sessionRealmConfiguration).use { sessionRealm -> + updateTrust(userList, cryptoRealm, sessionRealm) + } + } } - // Unfortunately we don't have much info on what did exactly changed (is it the cross signing keys of that user, - // or a new device?) So we check all again :/ - - Timber.d("## CrossSigning - Updating trust for $userList") + cleanup(params) + return Result.success() + } + private fun updateTrust(userListParam: List, + cRealm: Realm, + sRealm: Realm) { + var userList = userListParam + var myCrossSigningInfo: MXCrossSigningInfo? = null // First we check that the users MSK are trusted by mine // After that we check the trust chain for each devices of each users - Realm.getInstance(realmConfiguration).use { realm -> - realm.executeTransaction { - // By mapping here to model, this object is not live - // I should update it if needed - var myCrossSigningInfo = realm.where(CrossSigningInfoEntity::class.java) - .equalTo(CrossSigningInfoEntityFields.USER_ID, myUserId) - .findFirst()?.let { mapCrossSigningInfoEntity(it) } + cRealm.executeTransaction { cryptoRealm -> + // By mapping here to model, this object is not live + // I should update it if needed + myCrossSigningInfo = getCrossSigningInfo(cryptoRealm, myUserId) - var myTrustResult: UserTrustResult? = null + var myTrustResult: UserTrustResult? = null - if (userList.contains(myUserId)) { - Timber.d("## CrossSigning - Clear all trust as a change on my user was detected") - // i am in the list.. but i don't know exactly the delta of change :/ - // If it's my cross signing keys we should refresh all trust - // do it anyway ? - userList = realm.where(CrossSigningInfoEntity::class.java) - .findAll().mapNotNull { it.userId } - Timber.d("## CrossSigning - Updating trust for all $userList") + if (userList.contains(myUserId)) { + Timber.d("## CrossSigning - Clear all trust as a change on my user was detected") + // i am in the list.. but i don't know exactly the delta of change :/ + // If it's my cross signing keys we should refresh all trust + // do it anyway ? + userList = cryptoRealm.where(CrossSigningInfoEntity::class.java) + .findAll() + .mapNotNull { it.userId } - // check right now my keys and mark it as trusted as other trust depends on it - val myDevices = realm.where() - .equalTo(UserEntityFields.USER_ID, myUserId) - .findFirst() - ?.devices - ?.map { deviceInfo -> - CryptoMapper.mapToModel(deviceInfo) - } - myTrustResult = crossSigningService.checkSelfTrust(myCrossSigningInfo, myDevices).also { - updateCrossSigningKeysTrust(realm, myUserId, it.isVerified()) - // update model reference - myCrossSigningInfo = realm.where(CrossSigningInfoEntity::class.java) - .equalTo(CrossSigningInfoEntityFields.USER_ID, myUserId) - .findFirst()?.let { mapCrossSigningInfoEntity(it) } - } - } + // check right now my keys and mark it as trusted as other trust depends on it + val myDevices = cryptoRealm.where() + .equalTo(UserEntityFields.USER_ID, myUserId) + .findFirst() + ?.devices + ?.map { CryptoMapper.mapToModel(it) } - val otherInfos = userList.map { - it to realm.where(CrossSigningInfoEntity::class.java) - .equalTo(CrossSigningInfoEntityFields.USER_ID, it) - .findFirst()?.let { mapCrossSigningInfoEntity(it) } - } - .toMap() + myTrustResult = crossSigningService.checkSelfTrust(myCrossSigningInfo, myDevices) + updateCrossSigningKeysTrust(cryptoRealm, myUserId, myTrustResult.isVerified()) + // update model reference + myCrossSigningInfo = getCrossSigningInfo(cryptoRealm, myUserId) + } - val trusts = otherInfos.map { infoEntry -> - infoEntry.key to when (infoEntry.key) { - myUserId -> myTrustResult - else -> { - crossSigningService.checkOtherMSKTrusted(myCrossSigningInfo, infoEntry.value).also { - Timber.d("## CrossSigning - user:${infoEntry.key} result:$it") - } + val otherInfos = userList.associateWith { userId -> + getCrossSigningInfo(cryptoRealm, userId) + } + + val trusts = otherInfos.mapValues { entry -> + when (entry.key) { + myUserId -> myTrustResult + else -> { + crossSigningService.checkOtherMSKTrusted(myCrossSigningInfo, entry.value).also { + Timber.d("## CrossSigning - user:${entry.key} result:$it") } } - }.toMap() + } + } - // TODO! if it's me and my keys has changed... I have to reset trust for everyone! - // i have all the new trusts, update DB - trusts.forEach { - val verified = it.value?.isVerified() == true - updateCrossSigningKeysTrust(realm, it.key, verified) + // TODO! if it's me and my keys has changed... I have to reset trust for everyone! + // i have all the new trusts, update DB + trusts.forEach { + val verified = it.value?.isVerified() == true + updateCrossSigningKeysTrust(cryptoRealm, it.key, verified) + } + + // Ok so now we have to check device trust for all these users.. + Timber.v("## CrossSigning - Updating devices cross trust users: ${trusts.keys.logLimit()}") + trusts.keys.forEach { userId -> + val devicesEntities = cryptoRealm.where() + .equalTo(UserEntityFields.USER_ID, userId) + .findFirst() + ?.devices + + val trustMap = devicesEntities?.associateWith { device -> + // get up to date from DB has could have been updated + val otherInfo = getCrossSigningInfo(cryptoRealm, userId) + crossSigningService.checkDeviceTrust(myCrossSigningInfo, otherInfo, CryptoMapper.mapToModel(device)) } - // Ok so now we have to check device trust for all these users.. - Timber.v("## CrossSigning - Updating devices cross trust users ${trusts.keys}") - trusts.keys.forEach { - val devicesEntities = realm.where() - .equalTo(UserEntityFields.USER_ID, it) - .findFirst() - ?.devices - - val trustMap = devicesEntities?.map { device -> - // get up to date from DB has could have been updated - val otherInfo = realm.where(CrossSigningInfoEntity::class.java) - .equalTo(CrossSigningInfoEntityFields.USER_ID, it) - .findFirst()?.let { mapCrossSigningInfoEntity(it) } - device to crossSigningService.checkDeviceTrust(myCrossSigningInfo, otherInfo, CryptoMapper.mapToModel(device)) - }?.toMap() - - // Update trust if needed - devicesEntities?.forEach { device -> - val crossSignedVerified = trustMap?.get(device)?.isCrossSignedVerified() - Timber.d("## CrossSigning - Trust for ${device.userId}|${device.deviceId} : cross verified: ${trustMap?.get(device)}") - if (device.trustLevelEntity?.crossSignedVerified != crossSignedVerified) { - Timber.d("## CrossSigning - Trust change detected for ${device.userId}|${device.deviceId} : cross verified: $crossSignedVerified") - // need to save - val trustEntity = device.trustLevelEntity - if (trustEntity == null) { - realm.createObject(TrustLevelEntity::class.java).let { - it.locallyVerified = false - it.crossSignedVerified = crossSignedVerified - device.trustLevelEntity = it - } - } else { - trustEntity.crossSignedVerified = crossSignedVerified + // Update trust if needed + devicesEntities?.forEach { device -> + val crossSignedVerified = trustMap?.get(device)?.isCrossSignedVerified() + Timber.d("## CrossSigning - Trust for ${device.userId}|${device.deviceId} : cross verified: ${trustMap?.get(device)}") + if (device.trustLevelEntity?.crossSignedVerified != crossSignedVerified) { + Timber.d("## CrossSigning - Trust change detected for ${device.userId}|${device.deviceId} : cross verified: $crossSignedVerified") + // need to save + val trustEntity = device.trustLevelEntity + if (trustEntity == null) { + device.trustLevelEntity = cryptoRealm.createObject(TrustLevelEntity::class.java).also { + it.locallyVerified = false + it.crossSignedVerified = crossSignedVerified } + } else { + trustEntity.crossSignedVerified = crossSignedVerified } } } @@ -201,35 +203,44 @@ internal class UpdateTrustWorker(context: Context, // We can now update room shields? in the session DB? Timber.d("## CrossSigning - Updating shields for impacted rooms...") - Realm.getInstance(sessionRealmConfiguration).use { it -> - it.executeTransaction { realm -> - val distinctRoomIds = realm.where(RoomMemberSummaryEntity::class.java) - .`in`(RoomMemberSummaryEntityFields.USER_ID, userList.toTypedArray()) - .distinct(RoomMemberSummaryEntityFields.ROOM_ID) - .findAll() - .map { it.roomId } - Timber.d("## CrossSigning - ... impacted rooms $distinctRoomIds") - distinctRoomIds.forEach { roomId -> - val roomSummary = RoomSummaryEntity.where(realm, roomId).findFirst() - if (roomSummary?.isEncrypted == true) { - Timber.d("## CrossSigning - Check shield state for room $roomId") - val allActiveRoomMembers = RoomMemberHelper(realm, roomId).getActiveRoomMemberIds() - try { - val updatedTrust = computeRoomShield(allActiveRoomMembers, roomSummary) - if (roomSummary.roomEncryptionTrustLevel != updatedTrust) { - Timber.d("## CrossSigning - Shield change detected for $roomId -> $updatedTrust") - roomSummary.roomEncryptionTrustLevel = updatedTrust - } - } catch (failure: Throwable) { - Timber.e(failure) - } + sRealm.executeTransaction { sessionRealm -> + sessionRealm.where(RoomMemberSummaryEntity::class.java) + .`in`(RoomMemberSummaryEntityFields.USER_ID, userList.toTypedArray()) + .distinct(RoomMemberSummaryEntityFields.ROOM_ID) + .findAll() + .map { it.roomId } + .also { Timber.d("## CrossSigning - ... impacted rooms ${it.logLimit()}") } + .forEach { roomId -> + RoomSummaryEntity.where(sessionRealm, roomId) + .equalTo(RoomSummaryEntityFields.IS_ENCRYPTED, true) + .findFirst() + ?.let { roomSummary -> + Timber.d("## CrossSigning - Check shield state for room $roomId") + val allActiveRoomMembers = RoomMemberHelper(sessionRealm, roomId).getActiveRoomMemberIds() + try { + val updatedTrust = computeRoomShield( + myCrossSigningInfo, + cRealm, + allActiveRoomMembers, + roomSummary + ) + if (roomSummary.roomEncryptionTrustLevel != updatedTrust) { + Timber.d("## CrossSigning - Shield change detected for $roomId -> $updatedTrust") + roomSummary.roomEncryptionTrustLevel = updatedTrust + } + } catch (failure: Throwable) { + Timber.e(failure) + } + } } - } - } } + } - cleanup(params) - return Result.success() + private fun getCrossSigningInfo(cryptoRealm: Realm, userId: String): MXCrossSigningInfo? { + return cryptoRealm.where(CrossSigningInfoEntity::class.java) + .equalTo(CrossSigningInfoEntityFields.USER_ID, userId) + .findFirst() + ?.let { mapCrossSigningInfoEntity(it) } } private fun cleanup(params: Params) { @@ -237,30 +248,34 @@ internal class UpdateTrustWorker(context: Context, ?.let { updateTrustWorkerDataRepository.delete(it) } } - private fun updateCrossSigningKeysTrust(realm: Realm, userId: String, verified: Boolean) { - val xInfoEntity = realm.where(CrossSigningInfoEntity::class.java) + private fun updateCrossSigningKeysTrust(cryptoRealm: Realm, userId: String, verified: Boolean) { + cryptoRealm.where(CrossSigningInfoEntity::class.java) .equalTo(CrossSigningInfoEntityFields.USER_ID, userId) .findFirst() - xInfoEntity?.crossSigningKeys?.forEach { info -> - // optimization to avoid trigger updates when there is no change.. - if (info.trustLevelEntity?.isVerified() != verified) { - Timber.d("## CrossSigning - Trust change for $userId : $verified") - val level = info.trustLevelEntity - if (level == null) { - val newLevel = realm.createObject(TrustLevelEntity::class.java) - newLevel.locallyVerified = verified - newLevel.crossSignedVerified = verified - info.trustLevelEntity = newLevel - } else { - level.locallyVerified = verified - level.crossSignedVerified = verified + ?.crossSigningKeys + ?.forEach { info -> + // optimization to avoid trigger updates when there is no change.. + if (info.trustLevelEntity?.isVerified() != verified) { + Timber.d("## CrossSigning - Trust change for $userId : $verified") + val level = info.trustLevelEntity + if (level == null) { + info.trustLevelEntity = cryptoRealm.createObject(TrustLevelEntity::class.java).also { + it.locallyVerified = verified + it.crossSignedVerified = verified + } + } else { + level.locallyVerified = verified + level.crossSignedVerified = verified + } + } } - } - } } - private fun computeRoomShield(activeMemberUserIds: List, roomSummaryEntity: RoomSummaryEntity): RoomEncryptionTrustLevel { - Timber.d("## CrossSigning - computeRoomShield ${roomSummaryEntity.roomId} -> $activeMemberUserIds") + private fun computeRoomShield(myCrossSigningInfo: MXCrossSigningInfo?, + cryptoRealm: Realm, + activeMemberUserIds: List, + roomSummaryEntity: RoomSummaryEntity): RoomEncryptionTrustLevel { + Timber.d("## CrossSigning - computeRoomShield ${roomSummaryEntity.roomId} -> ${activeMemberUserIds.logLimit()}") // The set of “all users” depends on the type of room: // For regular / topic rooms which have more than 2 members (including yourself) are considered when decorating a room // For 1:1 and group DM rooms, all other users (i.e. excluding yourself) are considered when decorating a room @@ -272,17 +287,8 @@ internal class UpdateTrustWorker(context: Context, val allTrustedUserIds = listToCheck .filter { userId -> - Realm.getInstance(realmConfiguration).use { - it.where(CrossSigningInfoEntity::class.java) - .equalTo(CrossSigningInfoEntityFields.USER_ID, userId) - .findFirst()?.let { mapCrossSigningInfoEntity(it) }?.isTrusted() == true - } + getCrossSigningInfo(cryptoRealm, userId)?.isTrusted() == true } - val myCrossKeys = Realm.getInstance(realmConfiguration).use { - it.where(CrossSigningInfoEntity::class.java) - .equalTo(CrossSigningInfoEntityFields.USER_ID, myUserId) - .findFirst()?.let { mapCrossSigningInfoEntity(it) } - } return if (allTrustedUserIds.isEmpty()) { RoomEncryptionTrustLevel.Default @@ -291,21 +297,17 @@ internal class UpdateTrustWorker(context: Context, // If all devices of all verified users are trusted -> green // else -> black allTrustedUserIds - .mapNotNull { uid -> - Realm.getInstance(realmConfiguration).use { - it.where() - .equalTo(UserEntityFields.USER_ID, uid) - .findFirst() - ?.devices - ?.map { - CryptoMapper.mapToModel(it) - } - } + .mapNotNull { userId -> + cryptoRealm.where() + .equalTo(UserEntityFields.USER_ID, userId) + .findFirst() + ?.devices + ?.map { CryptoMapper.mapToModel(it) } } .flatten() .let { allDevices -> - Timber.v("## CrossSigning - computeRoomShield ${roomSummaryEntity.roomId} devices ${allDevices.map { it.deviceId }}") - if (myCrossKeys != null) { + Timber.v("## CrossSigning - computeRoomShield ${roomSummaryEntity.roomId} devices ${allDevices.map { it.deviceId }.logLimit()}") + if (myCrossSigningInfo != null) { allDevices.any { !it.trustLevel?.crossSigningVerified.orFalse() } } else { // Legacy method diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/util/LogUtil.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/util/LogUtil.kt index fe68b49a5c..bfa723c160 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/util/LogUtil.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/util/LogUtil.kt @@ -19,6 +19,18 @@ package org.matrix.android.sdk.internal.util import org.matrix.android.sdk.BuildConfig import timber.log.Timber +internal fun Collection.logLimit(maxQuantity: Int = 5): String { + return buildString { + append(size) + append(" item(s)") + if (size > maxQuantity) { + append(", first $maxQuantity items") + } + append(": ") + append(this@logLimit.take(maxQuantity)) + } +} + internal suspend fun logDuration(message: String, block: suspend () -> T): T { Timber.v("$message -- BEGIN") From 0c774c098f5b4d5deb9586a1c1658648c2d9e0ff Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Fri, 12 Mar 2021 19:08:57 +0100 Subject: [PATCH 2/9] No op for empty list --- .../sdk/internal/session/room/summary/RoomSummaryUpdater.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/summary/RoomSummaryUpdater.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/summary/RoomSummaryUpdater.kt index fff780fb0c..4a89e1ccb4 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/summary/RoomSummaryUpdater.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/summary/RoomSummaryUpdater.kt @@ -144,7 +144,7 @@ internal class RoomSummaryUpdater @Inject constructor( roomSummaryEntity.otherMemberIds.clear() roomSummaryEntity.otherMemberIds.addAll(otherRoomMembers) - if (roomSummaryEntity.isEncrypted) { + if (roomSummaryEntity.isEncrypted && otherRoomMembers.isNotEmpty()) { // mmm maybe we could only refresh shield instead of checking trust also? crossSigningService.onUsersDeviceUpdate(otherRoomMembers) } From c4aadfed337e37a920c69733fb8d107e4f00ec8a Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Mon, 15 Mar 2021 14:35:21 +0100 Subject: [PATCH 3/9] Better API and avoid copying Collection --- .../android/sdk/internal/crypto/MXOlmDevice.kt | 2 +- .../crypto/algorithms/olm/MXOlmDecryption.kt | 2 +- .../sdk/internal/crypto/store/IMXCryptoStore.kt | 2 +- .../sdk/internal/crypto/store/db/RealmCryptoStore.kt | 12 ++++-------- 4 files changed, 7 insertions(+), 11 deletions(-) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/MXOlmDevice.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/MXOlmDevice.kt index b1e91e8d50..b8f1a9abea 100755 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/MXOlmDevice.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/MXOlmDevice.kt @@ -312,7 +312,7 @@ internal class MXOlmDevice @Inject constructor( * @param theirDeviceIdentityKey the Curve25519 identity key for the remote device. * @return a list of known session ids for the device. */ - fun getSessionIds(theirDeviceIdentityKey: String): Set? { + fun getSessionIds(theirDeviceIdentityKey: String): List? { return store.getDeviceSessionIds(theirDeviceIdentityKey) } diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/algorithms/olm/MXOlmDecryption.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/algorithms/olm/MXOlmDecryption.kt index 541f62de2c..082b86c9da 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/algorithms/olm/MXOlmDecryption.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/algorithms/olm/MXOlmDecryption.kt @@ -154,7 +154,7 @@ internal class MXOlmDecryption( * @return payload, if decrypted successfully. */ private fun decryptMessage(message: JsonDict, theirDeviceIdentityKey: String): String? { - val sessionIds = olmDevice.getSessionIds(theirDeviceIdentityKey) ?: emptySet() + val sessionIds = olmDevice.getSessionIds(theirDeviceIdentityKey).orEmpty() val messageBody = message["body"] as? String ?: return null val messageType = when (val typeAsVoid = message["type"]) { diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/store/IMXCryptoStore.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/store/IMXCryptoStore.kt index 6f1487f80a..181bd94cc7 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/store/IMXCryptoStore.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/store/IMXCryptoStore.kt @@ -259,7 +259,7 @@ internal interface IMXCryptoStore { * @param deviceKey the public key of the other device. * @return A set of sessionId, or null if device is not known */ - fun getDeviceSessionIds(deviceKey: String): Set? + fun getDeviceSessionIds(deviceKey: String): List? /** * Retrieve an end-to-end session between the logged-in user and another diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/store/db/RealmCryptoStore.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/store/db/RealmCryptoStore.kt index b9213ba758..9ae93d61eb 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/store/db/RealmCryptoStore.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/store/db/RealmCryptoStore.kt @@ -692,7 +692,7 @@ internal class RealmCryptoStore @Inject constructor( } } - override fun getDeviceSessionIds(deviceKey: String): MutableSet { + override fun getDeviceSessionIds(deviceKey: String): List { return doWithRealm(realmConfiguration) { it.where() .equalTo(OlmSessionEntityFields.DEVICE_KEY, deviceKey) @@ -701,7 +701,6 @@ internal class RealmCryptoStore @Inject constructor( sessionEntity.sessionId } } - .toMutableSet() } override fun storeInboundGroupSessions(sessions: List) { @@ -801,7 +800,7 @@ internal class RealmCryptoStore @Inject constructor( * Note: the result will be only use to export all the keys and not to use the OlmInboundGroupSessionWrapper2, * so there is no need to use or update `inboundGroupSessionToRelease` for native memory management */ - override fun getInboundGroupSessions(): MutableList { + override fun getInboundGroupSessions(): List { return doWithRealm(realmConfiguration) { it.where() .findAll() @@ -809,7 +808,6 @@ internal class RealmCryptoStore @Inject constructor( inboundGroupSessionEntity.getInboundGroupSession() } } - .toMutableList() } override fun removeInboundGroupSession(sessionId: String, senderKey: String) { @@ -964,7 +962,7 @@ internal class RealmCryptoStore @Inject constructor( } } - override fun getRoomsListBlacklistUnverifiedDevices(): MutableList { + override fun getRoomsListBlacklistUnverifiedDevices(): List { return doWithRealm(realmConfiguration) { it.where() .equalTo(CryptoRoomEntityFields.BLACKLIST_UNVERIFIED_DEVICES, true) @@ -973,10 +971,9 @@ internal class RealmCryptoStore @Inject constructor( cryptoRoom.roomId } } - .toMutableList() } - override fun getDeviceTrackingStatuses(): MutableMap { + override fun getDeviceTrackingStatuses(): Map { return doWithRealm(realmConfiguration) { it.where() .findAll() @@ -987,7 +984,6 @@ internal class RealmCryptoStore @Inject constructor( entry.value.deviceTrackingStatus } } - .toMutableMap() } override fun saveDeviceTrackingStatuses(deviceTrackingStatuses: Map) { From 71f2c507994bcf973a4dc1fa94da8d5c95d66bd4 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Mon, 15 Mar 2021 15:02:04 +0100 Subject: [PATCH 4/9] Do what the comment said --- .../android/sdk/internal/crypto/CryptoSessionInfoProvider.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/CryptoSessionInfoProvider.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/CryptoSessionInfoProvider.kt index e8a70615e1..5338e7e92f 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/CryptoSessionInfoProvider.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/CryptoSessionInfoProvider.kt @@ -38,7 +38,7 @@ internal class CryptoSessionInfoProvider @Inject constructor( val encryptionEvent = monarchy.fetchCopied { realm -> EventEntity.whereType(realm, roomId = roomId, type = EventType.STATE_ROOM_ENCRYPTION) .contains(EventEntityFields.CONTENT, "\"algorithm\":\"$MXCRYPTO_ALGORITHM_MEGOLM\"") - .isNotNull(EventEntityFields.STATE_KEY) // should be an empty key + .isEmpty(EventEntityFields.STATE_KEY) .findFirst() } return encryptionEvent != null From 13cbfaf5e798e5d056b81a772cd64bb33cac9591 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Mon, 15 Mar 2021 15:06:20 +0100 Subject: [PATCH 5/9] Do not load room members in e2e after init sync --- CHANGES.md | 2 +- .../internal/crypto/DefaultCryptoService.kt | 11 +---- .../sdk/internal/crypto/DeviceListManager.kt | 44 ++++++++++++------- .../room/membership/LoadRoomMembersTask.kt | 8 ++++ 4 files changed, 39 insertions(+), 26 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index e4ff049550..1e72c8c38e 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -5,7 +5,7 @@ Features ✨: - Improvements 🙌: - - + - Do not load room members in e2e after init sync Bugfix 🐛: - 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 17d25736eb..2163b2a5e0 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 @@ -856,15 +856,8 @@ internal class DefaultCryptoService @Inject constructor( return } cryptoCoroutineScope.launch(coroutineDispatchers.crypto) { - val params = LoadRoomMembersTask.Params(roomId) - try { - loadRoomMembersTask.execute(params) - } catch (throwable: Throwable) { - Timber.e(throwable, "## CRYPTO | onRoomEncryptionEvent ERROR FAILED TO SETUP CRYPTO ") - } finally { - val userIds = getRoomUserIds(roomId) - setEncryptionInRoom(roomId, event.content?.get("algorithm")?.toString(), true, userIds) - } + val userIds = getRoomUserIds(roomId) + setEncryptionInRoom(roomId, event.content?.get("algorithm")?.toString(), true, userIds) } } diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/DeviceListManager.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/DeviceListManager.kt index 42df6b354b..d9aa6d8db7 100755 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/DeviceListManager.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/DeviceListManager.kt @@ -16,6 +16,7 @@ package org.matrix.android.sdk.internal.crypto +import kotlinx.coroutines.launch import org.matrix.android.sdk.api.MatrixPatterns import org.matrix.android.sdk.api.auth.data.Credentials import org.matrix.android.sdk.internal.crypto.crosssigning.DeviceTrustLevel @@ -28,7 +29,6 @@ import org.matrix.android.sdk.internal.session.SessionScope import org.matrix.android.sdk.internal.session.sync.SyncTokenStore import org.matrix.android.sdk.internal.task.TaskExecutor import org.matrix.android.sdk.internal.util.MatrixCoroutineDispatchers -import kotlinx.coroutines.launch import timber.log.Timber import javax.inject.Inject @@ -39,8 +39,9 @@ internal class DeviceListManager @Inject constructor(private val cryptoStore: IM private val syncTokenStore: SyncTokenStore, private val credentials: Credentials, private val downloadKeysForUsersTask: DownloadKeysForUsersTask, + private val cryptoSessionInfoProvider: CryptoSessionInfoProvider, coroutineDispatchers: MatrixCoroutineDispatchers, - taskExecutor: TaskExecutor) { + private val taskExecutor: TaskExecutor) { interface UserDevicesUpdateListener { fun onUsersDeviceUpdate(userIds: List) @@ -75,8 +76,10 @@ internal class DeviceListManager @Inject constructor(private val cryptoStore: IM // HS not ready for retry private val notReadyToRetryHS = mutableSetOf() + private val cryptoCoroutineContext = coroutineDispatchers.crypto + init { - taskExecutor.executorScope.launch(coroutineDispatchers.crypto) { + taskExecutor.executorScope.launch(cryptoCoroutineContext) { var isUpdated = false val deviceTrackingStatuses = cryptoStore.getDeviceTrackingStatuses().toMutableMap() for ((userId, status) in deviceTrackingStatuses) { @@ -123,28 +126,37 @@ internal class DeviceListManager @Inject constructor(private val cryptoStore: IM } } + fun onRoomMembersLoadedFor(roomId: String) { + taskExecutor.executorScope.launch(cryptoCoroutineContext) { + if (cryptoSessionInfoProvider.isRoomEncrypted(roomId)) { + // It's OK to track also device for invited users + val userIds = cryptoSessionInfoProvider.getRoomUserIds(roomId, true) + startTrackingDeviceList(userIds) + refreshOutdatedDeviceLists() + } + } + } + /** * Mark the cached device list for the given user outdated * flag the given user for device-list tracking, if they are not already. * * @param userIds the user ids list */ - fun startTrackingDeviceList(userIds: List?) { - if (null != userIds) { - var isUpdated = false - val deviceTrackingStatuses = cryptoStore.getDeviceTrackingStatuses().toMutableMap() + fun startTrackingDeviceList(userIds: List) { + var isUpdated = false + val deviceTrackingStatuses = cryptoStore.getDeviceTrackingStatuses().toMutableMap() - for (userId in userIds) { - if (!deviceTrackingStatuses.containsKey(userId) || TRACKING_STATUS_NOT_TRACKED == deviceTrackingStatuses[userId]) { - Timber.v("## CRYPTO | startTrackingDeviceList() : Now tracking device list for $userId") - deviceTrackingStatuses[userId] = TRACKING_STATUS_PENDING_DOWNLOAD - isUpdated = true - } + for (userId in userIds) { + if (!deviceTrackingStatuses.containsKey(userId) || TRACKING_STATUS_NOT_TRACKED == deviceTrackingStatuses[userId]) { + Timber.v("## CRYPTO | startTrackingDeviceList() : Now tracking device list for $userId") + deviceTrackingStatuses[userId] = TRACKING_STATUS_PENDING_DOWNLOAD + isUpdated = true } + } - if (isUpdated) { - cryptoStore.saveDeviceTrackingStatuses(deviceTrackingStatuses) - } + if (isUpdated) { + cryptoStore.saveDeviceTrackingStatuses(deviceTrackingStatuses) } } diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/membership/LoadRoomMembersTask.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/membership/LoadRoomMembersTask.kt index 97cfcdaa44..cc491d1cd9 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/membership/LoadRoomMembersTask.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/membership/LoadRoomMembersTask.kt @@ -22,6 +22,8 @@ import io.realm.kotlin.createObject import kotlinx.coroutines.TimeoutCancellationException import org.matrix.android.sdk.api.session.room.model.Membership import org.matrix.android.sdk.api.session.room.send.SendState +import org.matrix.android.sdk.internal.crypto.CryptoSessionInfoProvider +import org.matrix.android.sdk.internal.crypto.DeviceListManager import org.matrix.android.sdk.internal.database.awaitNotEmptyResult import org.matrix.android.sdk.internal.database.mapper.toEntity import org.matrix.android.sdk.internal.database.model.CurrentStateEventEntity @@ -57,6 +59,8 @@ internal class DefaultLoadRoomMembersTask @Inject constructor( private val syncTokenStore: SyncTokenStore, private val roomSummaryUpdater: RoomSummaryUpdater, private val roomMemberEventHandler: RoomMemberEventHandler, + private val cryptoSessionInfoProvider: CryptoSessionInfoProvider, + private val deviceListManager: DeviceListManager, private val globalErrorReceiver: GlobalErrorReceiver ) : LoadRoomMembersTask { @@ -124,6 +128,10 @@ internal class DefaultLoadRoomMembersTask @Inject constructor( roomEntity.membersLoadStatus = RoomMembersLoadStatusType.LOADED roomSummaryUpdater.update(realm, roomId, updateMembers = true) } + + if (cryptoSessionInfoProvider.isRoomEncrypted(roomId)) { + deviceListManager.onRoomMembersLoadedFor(roomId) + } } private fun getRoomMembersLoadStatus(roomId: String): RoomMembersLoadStatusType { From 75ad6a640b015ed3f43dd3acafb96a17b35d05cd Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Mon, 15 Mar 2021 16:02:09 +0100 Subject: [PATCH 6/9] loglimit --- .../matrix/android/sdk/internal/crypto/DeviceListManager.kt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/DeviceListManager.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/DeviceListManager.kt index d9aa6d8db7..01b4f5b0fc 100755 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/DeviceListManager.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/DeviceListManager.kt @@ -29,6 +29,7 @@ import org.matrix.android.sdk.internal.session.SessionScope import org.matrix.android.sdk.internal.session.sync.SyncTokenStore import org.matrix.android.sdk.internal.task.TaskExecutor import org.matrix.android.sdk.internal.util.MatrixCoroutineDispatchers +import org.matrix.android.sdk.internal.util.logLimit import timber.log.Timber import javax.inject.Inject @@ -319,7 +320,7 @@ internal class DeviceListManager @Inject constructor(private val cryptoStore: IM * @param downloadUsers the user ids list */ private suspend fun doKeyDownloadForUsers(downloadUsers: List): MXUsersDevicesMap { - Timber.v("## CRYPTO | doKeyDownloadForUsers() : doKeyDownloadForUsers $downloadUsers") + Timber.v("## CRYPTO | doKeyDownloadForUsers() : doKeyDownloadForUsers ${downloadUsers.logLimit()}") // get the user ids which did not already trigger a keys download val filteredUsers = downloadUsers.filter { MatrixPatterns.isUserId(it) } if (filteredUsers.isEmpty()) { From 4302d50af93b9a11e48a30ca94a12bd8b5dd2651 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Mon, 15 Mar 2021 16:09:08 +0100 Subject: [PATCH 7/9] We do not display shield in the room list anymore Room member are lazy loaded, so the shield may not be accurate anymore. --- .../app/features/home/room/list/RoomSummaryItemFactory.kt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/vector/src/main/java/im/vector/app/features/home/room/list/RoomSummaryItemFactory.kt b/vector/src/main/java/im/vector/app/features/home/room/list/RoomSummaryItemFactory.kt index 988b3769c4..7d7ed1637f 100644 --- a/vector/src/main/java/im/vector/app/features/home/room/list/RoomSummaryItemFactory.kt +++ b/vector/src/main/java/im/vector/app/features/home/room/list/RoomSummaryItemFactory.kt @@ -92,7 +92,8 @@ class RoomSummaryItemFactory @Inject constructor(private val displayableEventFor return RoomSummaryItem_() .id(roomSummary.roomId) .avatarRenderer(avatarRenderer) - .encryptionTrustLevel(roomSummary.roomEncryptionTrustLevel) + // We do not display shield in the room list anymore + // .encryptionTrustLevel(roomSummary.roomEncryptionTrustLevel) .matrixItem(roomSummary.toMatrixItem()) .lastEventTime(latestEventTime) .typingMessage(typingMessage) From 4f3734f9328b0117e41487b4c690722ed305154e Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Mon, 15 Mar 2021 16:53:19 +0100 Subject: [PATCH 8/9] Ensure message are decrypted in the room list after a clear cache --- CHANGES.md | 2 +- .../sdk/internal/session/room/summary/RoomSummaryUpdater.kt | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 1e72c8c38e..8144e0a478 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -8,7 +8,7 @@ Improvements 🙌: - Do not load room members in e2e after init sync Bugfix 🐛: - - + - Ensure message are decrypted in the room list after a clear cache Translations 🗣: - diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/summary/RoomSummaryUpdater.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/summary/RoomSummaryUpdater.kt index 4a89e1ccb4..cd1bb69612 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/summary/RoomSummaryUpdater.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/summary/RoomSummaryUpdater.kt @@ -131,8 +131,8 @@ internal class RoomSummaryUpdater @Inject constructor( // mmm i want to decrypt now or is it ok to do it async? tryOrNull { eventDecryptor.decryptEvent(root.asDomain(), "") - // eventDecryptor.decryptEventAsync(root.asDomain(), "", NoOpMatrixCallback()) } + ?.let { root.setDecryptionResult(it) } } if (updateMembers) { From e7c9fb987cbf8f23735d49c5e9b20848e157b5e3 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Mon, 15 Mar 2021 18:08:33 +0100 Subject: [PATCH 9/9] Use AwaitTransaction (G's review) --- .../crypto/crosssigning/UpdateTrustWorker.kt | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/crosssigning/UpdateTrustWorker.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/crosssigning/UpdateTrustWorker.kt index a2a90c49cd..ad82c03913 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/crosssigning/UpdateTrustWorker.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/crosssigning/UpdateTrustWorker.kt @@ -33,6 +33,7 @@ import org.matrix.android.sdk.internal.crypto.store.db.model.CryptoMapper import org.matrix.android.sdk.internal.crypto.store.db.model.TrustLevelEntity import org.matrix.android.sdk.internal.crypto.store.db.model.UserEntity import org.matrix.android.sdk.internal.crypto.store.db.model.UserEntityFields +import org.matrix.android.sdk.internal.database.awaitTransaction import org.matrix.android.sdk.internal.database.model.RoomMemberSummaryEntity import org.matrix.android.sdk.internal.database.model.RoomMemberSummaryEntityFields import org.matrix.android.sdk.internal.database.model.RoomSummaryEntity @@ -69,8 +70,10 @@ internal class UpdateTrustWorker(context: Context, // It breaks the crypto store contract, but we need to batch things :/ @CryptoDatabase @Inject lateinit var cryptoRealmConfiguration: RealmConfiguration + @SessionDatabase @Inject lateinit var sessionRealmConfiguration: RealmConfiguration + @UserId @Inject lateinit var myUserId: String @Inject lateinit var crossSigningKeysMapper: CrossSigningKeysMapper @@ -96,8 +99,8 @@ internal class UpdateTrustWorker(context: Context, Timber.d("## CrossSigning - Updating trust for users: ${userList.logLimit()}") Realm.getInstance(cryptoRealmConfiguration).use { cryptoRealm -> - Realm.getInstance(sessionRealmConfiguration).use { sessionRealm -> - updateTrust(userList, cryptoRealm, sessionRealm) + Realm.getInstance(sessionRealmConfiguration).use { + updateTrust(userList, cryptoRealm) } } } @@ -106,14 +109,13 @@ internal class UpdateTrustWorker(context: Context, return Result.success() } - private fun updateTrust(userListParam: List, - cRealm: Realm, - sRealm: Realm) { + private suspend fun updateTrust(userListParam: List, + cRealm: Realm) { var userList = userListParam var myCrossSigningInfo: MXCrossSigningInfo? = null // First we check that the users MSK are trusted by mine // After that we check the trust chain for each devices of each users - cRealm.executeTransaction { cryptoRealm -> + awaitTransaction(cryptoRealmConfiguration) { cryptoRealm -> // By mapping here to model, this object is not live // I should update it if needed myCrossSigningInfo = getCrossSigningInfo(cryptoRealm, myUserId) @@ -203,7 +205,7 @@ internal class UpdateTrustWorker(context: Context, // We can now update room shields? in the session DB? Timber.d("## CrossSigning - Updating shields for impacted rooms...") - sRealm.executeTransaction { sessionRealm -> + awaitTransaction(sessionRealmConfiguration) { sessionRealm -> sessionRealm.where(RoomMemberSummaryEntity::class.java) .`in`(RoomMemberSummaryEntityFields.USER_ID, userList.toTypedArray()) .distinct(RoomMemberSummaryEntityFields.ROOM_ID)