From 6fe0002bd3812ec6cfb15c23a283f1640bc3e661 Mon Sep 17 00:00:00 2001 From: valere Date: Wed, 7 Jun 2023 19:20:24 +0200 Subject: [PATCH 1/2] Clean room shield update logic --- .../android/sdk/common/CommonTestHelper.kt | 17 +++ .../sdk/internal/crypto/RoomShieldTest.kt | 133 ++++++++++++++++++ .../DefaultCrossSigningService.kt | 6 +- .../crypto/crosssigning/UpdateTrustWorker.kt | 112 ++++++++------- .../sync/handler/ShieldSummaryUpdater.kt | 56 ++++++++ .../crypto/CryptoSessionInfoProvider.kt | 30 +++- .../UpdateTrustWorkerDataRepository.kt | 9 +- .../room/summary/RoomSummaryUpdater.kt | 23 ++- .../SyncResponsePostTreatmentAggregator.kt | 3 +- ...cResponsePostTreatmentAggregatorHandler.kt | 11 +- .../network/OutgoingRequestsProcessor.kt | 18 +-- .../sync/handler/ShieldSummaryUpdater.kt | 60 ++++++++ 12 files changed, 379 insertions(+), 99 deletions(-) create mode 100644 matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/internal/crypto/RoomShieldTest.kt create mode 100644 matrix-sdk-android/src/kotlinCrypto/java/org/matrix/android/sdk/internal/session/sync/handler/ShieldSummaryUpdater.kt create mode 100644 matrix-sdk-android/src/rustCrypto/java/org/matrix/android/sdk/internal/session/sync/handler/ShieldSummaryUpdater.kt diff --git a/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/common/CommonTestHelper.kt b/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/common/CommonTestHelper.kt index 4053d1c1c4..983e00b9ea 100644 --- a/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/common/CommonTestHelper.kt +++ b/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/common/CommonTestHelper.kt @@ -99,6 +99,23 @@ class CommonTestHelper internal constructor(context: Context, val cryptoConfig: } } } + + @OptIn(ExperimentalCoroutinesApi::class) + internal fun runLongCryptoTest(context: Context, cryptoConfig: MXCryptoConfig? = null, autoSignoutOnClose: Boolean = true, block: suspend CoroutineScope.(CryptoTestHelper, CommonTestHelper) -> Unit) { + val testHelper = CommonTestHelper(context, cryptoConfig) + val cryptoTestHelper = CryptoTestHelper(testHelper) + return runTest(dispatchTimeoutMs = TestConstants.timeOutMillis * 4) { + try { + withContext(Dispatchers.Default) { + block(cryptoTestHelper, testHelper) + } + } finally { + if (autoSignoutOnClose) { + testHelper.cleanUpOpenedSessions() + } + } + } + } } internal val matrix: TestMatrix diff --git a/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/internal/crypto/RoomShieldTest.kt b/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/internal/crypto/RoomShieldTest.kt new file mode 100644 index 0000000000..8e2284d588 --- /dev/null +++ b/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/internal/crypto/RoomShieldTest.kt @@ -0,0 +1,133 @@ +/* + * Copyright 2023 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.lifecycle.Observer +import androidx.test.filters.LargeTest +import kotlinx.coroutines.DelicateCoroutinesApi +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.GlobalScope +import kotlinx.coroutines.launch +import kotlinx.coroutines.withContext +import org.junit.FixMethodOrder +import org.junit.Test +import org.junit.runner.RunWith +import org.junit.runners.JUnit4 +import org.junit.runners.MethodSorters +import org.matrix.android.sdk.InstrumentedTest +import org.matrix.android.sdk.api.session.Session +import org.matrix.android.sdk.api.session.crypto.model.RoomEncryptionTrustLevel +import org.matrix.android.sdk.api.session.getRoom +import org.matrix.android.sdk.api.session.room.model.RoomSummary +import org.matrix.android.sdk.api.util.Optional +import org.matrix.android.sdk.common.CommonTestHelper +import org.matrix.android.sdk.common.SessionTestParams +import org.matrix.android.sdk.common.TestConstants +import java.util.concurrent.CountDownLatch +import java.util.concurrent.TimeUnit + +@RunWith(JUnit4::class) +@FixMethodOrder(MethodSorters.JVM) +@LargeTest +class RoomShieldTest : InstrumentedTest { + + @Test + fun testShieldNoVerification() = CommonTestHelper.runCryptoTest(context()) { cryptoTestHelper, _ -> + val testData = cryptoTestHelper.doE2ETestWithAliceAndBobInARoom() + + val roomId = testData.roomId + + cryptoTestHelper.initializeCrossSigning(testData.firstSession) + cryptoTestHelper.initializeCrossSigning(testData.secondSession!!) + + // Test are flaky unless I use liveData observer on main thread + // Just calling getRoomSummary() with retryWithBackOff keeps an outdated version of the value + testData.firstSession.assertRoomShieldIs(roomId, RoomEncryptionTrustLevel.Default) + testData.secondSession!!.assertRoomShieldIs(roomId, RoomEncryptionTrustLevel.Default) + } + + @Test + fun testShieldInOneOne() = CommonTestHelper.runLongCryptoTest(context()) { cryptoTestHelper, testHelper -> + val testData = cryptoTestHelper.doE2ETestWithAliceAndBobInARoom() + + val roomId = testData.roomId + + Log.v("#E2E TEST", "Initialize cross signing...") + cryptoTestHelper.initializeCrossSigning(testData.firstSession) + cryptoTestHelper.initializeCrossSigning(testData.secondSession!!) + Log.v("#E2E TEST", "... Initialized.") + + // let alive and bob verify + Log.v("#E2E TEST", "Alice and Bob verify each others...") + cryptoTestHelper.verifySASCrossSign(testData.firstSession, testData.secondSession!!, testData.roomId) + + // Add a new session for bob + // This session will be unverified for now + + Log.v("#E2E TEST", "Log in a new bob device...") + val bobSecondSession = testHelper.logIntoAccount(testData.secondSession!!.myUserId, SessionTestParams(true)) + + Log.v("#E2E TEST", "Bob session logged in ${bobSecondSession.myUserId.take(6)}") + + Log.v("#E2E TEST", "Assert room shields...") + testData.firstSession.assertRoomShieldIs(roomId, RoomEncryptionTrustLevel.Warning) + // in 1:1 we ignore our own status + testData.secondSession!!.assertRoomShieldIs(roomId, RoomEncryptionTrustLevel.Trusted) + + // Adding another user should make bob consider his devices now and see same shield as alice + Log.v("#E2E TEST", "Create Sam account") + val samSession = testHelper.createAccount(TestConstants.USER_SAM, SessionTestParams(withInitialSync = true)) + + // Let alice invite sam + Log.v("#E2E TEST", "Let alice invite sam") + testData.firstSession.getRoom(roomId)!!.membershipService().invite(samSession.myUserId) + testHelper.waitForAndAcceptInviteInRoom(samSession, roomId) + + Log.v("#E2E TEST", "Assert room shields...") + testData.firstSession.assertRoomShieldIs(roomId, RoomEncryptionTrustLevel.Warning) + testData.secondSession!!.assertRoomShieldIs(roomId, RoomEncryptionTrustLevel.Warning) + + // Now let's bob verify his session + + Log.v("#E2E TEST", "Bob verifies his new session") + cryptoTestHelper.verifyNewSession(testData.secondSession!!, bobSecondSession) + + testData.firstSession.assertRoomShieldIs(roomId, RoomEncryptionTrustLevel.Trusted) + testData.secondSession!!.assertRoomShieldIs(roomId, RoomEncryptionTrustLevel.Trusted) + } + + @OptIn(DelicateCoroutinesApi::class) + private suspend fun Session.assertRoomShieldIs(roomId: String, state: RoomEncryptionTrustLevel?) { + val lock = CountDownLatch(1) + val roomLiveData = withContext(Dispatchers.Main) { + roomService().getRoomSummaryLive(roomId) + } + val observer = object : Observer> { + override fun onChanged(value: Optional) { + Log.v("#E2E TEST ${this@assertRoomShieldIs.myUserId.take(6)}", "Shield Update ${value.getOrNull()?.roomEncryptionTrustLevel}") + if (value.getOrNull()?.roomEncryptionTrustLevel == state) { + lock.countDown() + roomLiveData.removeObserver(this) + } + } + } + GlobalScope.launch(Dispatchers.Main) { roomLiveData.observeForever(observer) } + + lock.await(40_000, TimeUnit.MILLISECONDS) + } +} diff --git a/matrix-sdk-android/src/kotlinCrypto/java/org/matrix/android/sdk/internal/crypto/crosssigning/DefaultCrossSigningService.kt b/matrix-sdk-android/src/kotlinCrypto/java/org/matrix/android/sdk/internal/crypto/crosssigning/DefaultCrossSigningService.kt index 3090bb805e..e020946484 100644 --- a/matrix-sdk-android/src/kotlinCrypto/java/org/matrix/android/sdk/internal/crypto/crosssigning/DefaultCrossSigningService.kt +++ b/matrix-sdk-android/src/kotlinCrypto/java/org/matrix/android/sdk/internal/crypto/crosssigning/DefaultCrossSigningService.kt @@ -49,7 +49,6 @@ import org.matrix.android.sdk.internal.di.SessionId import org.matrix.android.sdk.internal.di.UserId import org.matrix.android.sdk.internal.di.WorkManagerProvider import org.matrix.android.sdk.internal.session.SessionScope -import org.matrix.android.sdk.internal.task.TaskExecutor import org.matrix.android.sdk.internal.util.JsonCanonicalizer import org.matrix.android.sdk.internal.util.logLimit import org.matrix.android.sdk.internal.worker.WorkerParamsFactory @@ -66,7 +65,6 @@ internal class DefaultCrossSigningService @Inject constructor( private val deviceListManager: DeviceListManager, private val initializeCrossSigningTask: InitializeCrossSigningTask, private val uploadSignaturesTask: UploadSignaturesTask, - private val taskExecutor: TaskExecutor, private val coroutineDispatchers: MatrixCoroutineDispatchers, private val cryptoCoroutineScope: CoroutineScope, private val workManagerProvider: WorkManagerProvider, @@ -612,9 +610,7 @@ internal class DefaultCrossSigningService @Inject constructor( withContext(coroutineDispatchers.crypto) { // This device should be yours val device = cryptoStore.getUserDevice(myUserId, deviceId) - if (device == null) { - throw IllegalArgumentException("This device [$deviceId] is not known, or not yours") - } + ?: throw IllegalArgumentException("This device [$deviceId] is not known, or not yours") val myKeys = getUserCrossSigningKeys(myUserId) ?: throw Throwable("CrossSigning is not setup for this account") diff --git a/matrix-sdk-android/src/kotlinCrypto/java/org/matrix/android/sdk/internal/crypto/crosssigning/UpdateTrustWorker.kt b/matrix-sdk-android/src/kotlinCrypto/java/org/matrix/android/sdk/internal/crypto/crosssigning/UpdateTrustWorker.kt index ce6e046de5..80f37a6c57 100644 --- a/matrix-sdk-android/src/kotlinCrypto/java/org/matrix/android/sdk/internal/crypto/crosssigning/UpdateTrustWorker.kt +++ b/matrix-sdk-android/src/kotlinCrypto/java/org/matrix/android/sdk/internal/crypto/crosssigning/UpdateTrustWorker.kt @@ -31,7 +31,7 @@ import org.matrix.android.sdk.api.session.crypto.crosssigning.isCrossSignedVerif import org.matrix.android.sdk.api.session.crypto.crosssigning.isVerified import org.matrix.android.sdk.api.session.crypto.model.RoomEncryptionTrustLevel import org.matrix.android.sdk.internal.SessionManager -import org.matrix.android.sdk.internal.crypto.store.IMXCryptoStore +import org.matrix.android.sdk.internal.crypto.CryptoSessionInfoProvider import org.matrix.android.sdk.internal.crypto.store.db.mapper.CrossSigningKeysMapper import org.matrix.android.sdk.internal.crypto.store.db.model.CrossSigningInfoEntity import org.matrix.android.sdk.internal.crypto.store.db.model.CrossSigningInfoEntityFields @@ -40,10 +40,7 @@ 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 -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 @@ -83,35 +80,54 @@ internal class UpdateTrustWorker(context: Context, params: WorkerParameters, ses @Inject lateinit var myUserId: String @Inject lateinit var crossSigningKeysMapper: CrossSigningKeysMapper @Inject lateinit var updateTrustWorkerDataRepository: UpdateTrustWorkerDataRepository + @Inject lateinit var cryptoSessionInfoProvider: CryptoSessionInfoProvider // @Inject lateinit var roomSummaryUpdater: RoomSummaryUpdater - @Inject lateinit var cryptoStore: IMXCryptoStore +// @Inject lateinit var cryptoStore: IMXCryptoStore override fun injectWith(injector: SessionComponent) { injector.inject(this) } override suspend fun doSafeWork(params: Params): Result { - val userList = params.filename + val sId = myUserId.take(5) + Timber.v("## CrossSigning - UpdateTrustWorker started..") + val workerParams = params.filename ?.let { updateTrustWorkerDataRepository.getParam(it) } - ?.userIds - ?: params.updatedUserIds.orEmpty() + ?: return Result.success().also { + Timber.w("## CrossSigning - UpdateTrustWorker failed to get params") + cleanup(params) + } + + Timber.v("## CrossSigning [$sId]- UpdateTrustWorker userIds:${workerParams.userIds.logLimit()}, roomIds:${workerParams.roomIds.orEmpty().logLimit()}") + val userList = workerParams.userIds // 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.v("## CrossSigning - Updating trust for users: ${userList.logLimit()}") + Timber.v("## CrossSigning [$sId]- Updating trust for users: ${userList.logLimit()}") updateTrust(userList) } + val roomsToCheck = workerParams.roomIds ?: cryptoSessionInfoProvider.getRoomsWhereUsersAreParticipating(userList) + Timber.v("## CrossSigning [$sId]- UpdateTrustWorker roomShield to check:${roomsToCheck.logLimit()}") + var myCrossSigningInfo: MXCrossSigningInfo? + Realm.getInstance(cryptoRealmConfiguration).use { realm -> + myCrossSigningInfo = getCrossSigningInfo(realm, myUserId) + } + // So Cross Signing keys trust is updated, device trust is updated + // We can now update room shields? in the session DB? + updateRoomShieldInSummaries(roomsToCheck, myCrossSigningInfo) + cleanup(params) return Result.success() } private suspend fun updateTrust(userListParam: List) { + val sId = myUserId.take(5) var userList = userListParam - var myCrossSigningInfo: MXCrossSigningInfo? = null + var myCrossSigningInfo: MXCrossSigningInfo? // First we check that the users MSK are trusted by mine // After that we check the trust chain for each devices of each users @@ -123,7 +139,7 @@ internal class UpdateTrustWorker(context: Context, params: WorkerParameters, ses var myTrustResult: UserTrustResult? = null if (userList.contains(myUserId)) { - Timber.d("## CrossSigning - Clear all trust as a change on my user was detected") + Timber.d("## CrossSigning [$sId]- 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 ? @@ -153,7 +169,7 @@ internal class UpdateTrustWorker(context: Context, params: WorkerParameters, ses myUserId -> myTrustResult else -> { crossSigningService.checkOtherMSKTrusted(myCrossSigningInfo, entry.value).also { - Timber.v("## CrossSigning - user:${entry.key} result:$it") + Timber.v("## CrossSigning [$sId]- user:${entry.key} result:$it") } } } @@ -163,12 +179,12 @@ internal class UpdateTrustWorker(context: Context, params: WorkerParameters, ses // i have all the new trusts, update DB trusts.forEach { val verified = it.value?.isVerified() == true - Timber.v("[$myUserId] ## CrossSigning - Updating user trust: ${it.key} to $verified") + Timber.v("[$myUserId] ## CrossSigning [$sId]- Updating user trust: ${it.key} to $verified") 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()}") + Timber.v("## CrossSigning [$sId]- Updating devices cross trust users: ${trusts.keys.logLimit()}") trusts.keys.forEach { userId -> val devicesEntities = cryptoRealm.where() .equalTo(UserEntityFields.USER_ID, userId) @@ -184,9 +200,9 @@ internal class UpdateTrustWorker(context: Context, params: WorkerParameters, ses // Update trust if needed devicesEntities?.forEach { device -> val crossSignedVerified = trustMap?.get(device)?.isCrossSignedVerified() - Timber.v("## CrossSigning - Trust for ${device.userId}|${device.deviceId} : cross verified: ${trustMap?.get(device)}") + Timber.v("## CrossSigning [$sId]- 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") + Timber.d("## CrossSigning [$sId]- Trust change detected for ${device.userId}|${device.deviceId} : cross verified: $crossSignedVerified") // need to save val trustEntity = device.trustLevelEntity if (trustEntity == null) { @@ -197,50 +213,46 @@ internal class UpdateTrustWorker(context: Context, params: WorkerParameters, ses } else { trustEntity.crossSignedVerified = crossSignedVerified } + } else { + Timber.v("## CrossSigning [$sId]- Trust unchanged for ${device.userId}|${device.deviceId} : cross verified: $crossSignedVerified") } } } } - - // So Cross Signing keys trust is updated, device trust is updated - // We can now update room shields? in the session DB? - updateTrustStep2(userList, myCrossSigningInfo) } - private suspend fun updateTrustStep2(userList: List, myCrossSigningInfo: MXCrossSigningInfo?) { - Timber.d("## CrossSigning - Updating shields for impacted rooms...") + private suspend fun updateRoomShieldInSummaries(roomList: List, myCrossSigningInfo: MXCrossSigningInfo?) { + val sId = myUserId.take(5) + Timber.d("## CrossSigning [$sId]- Updating shields for impacted rooms... ${roomList.logLimit()}") awaitTransaction(sessionRealmConfiguration) { sessionRealm -> Timber.d("## CrossSigning - Updating shields for impacted rooms - in transaction") Realm.getInstance(cryptoRealmConfiguration).use { cryptoRealm -> - 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.v("## CrossSigning - Check shield state for room $roomId") - val allActiveRoomMembers = RoomMemberHelper(sessionRealm, roomId).getActiveRoomMemberIds() - try { - val updatedTrust = computeRoomShield( - myCrossSigningInfo, - cryptoRealm, - allActiveRoomMembers, - roomSummary - ) - if (roomSummary.roomEncryptionTrustLevel != updatedTrust) { - Timber.d("## CrossSigning - Shield change detected for $roomId -> $updatedTrust") - roomSummary.roomEncryptionTrustLevel = updatedTrust - } - } catch (failure: Throwable) { - Timber.e(failure) - } + roomList.forEach { roomId -> + Timber.v("## CrossSigning [$sId]- Checking room $roomId") + RoomSummaryEntity.where(sessionRealm, roomId) +// .equalTo(RoomSummaryEntityFields.IS_ENCRYPTED, true) + .findFirst() + ?.let { roomSummary -> + Timber.v("## CrossSigning [$sId]- Check shield state for room $roomId") + val allActiveRoomMembers = RoomMemberHelper(sessionRealm, roomId).getActiveRoomMemberIds() + try { + val updatedTrust = computeRoomShield( + myCrossSigningInfo, + cryptoRealm, + allActiveRoomMembers, + roomSummary + ) + if (roomSummary.roomEncryptionTrustLevel != updatedTrust) { + Timber.d("## CrossSigning [$sId]- Shield change detected for $roomId -> $updatedTrust") + roomSummary.roomEncryptionTrustLevel = updatedTrust + } else { + Timber.v("## CrossSigning [$sId]- Shield unchanged for $roomId -> $updatedTrust") } - } + } catch (failure: Throwable) { + Timber.e(failure) + } + } + } } } Timber.d("## CrossSigning - Updating shields for impacted rooms - END") diff --git a/matrix-sdk-android/src/kotlinCrypto/java/org/matrix/android/sdk/internal/session/sync/handler/ShieldSummaryUpdater.kt b/matrix-sdk-android/src/kotlinCrypto/java/org/matrix/android/sdk/internal/session/sync/handler/ShieldSummaryUpdater.kt new file mode 100644 index 0000000000..bcc078b550 --- /dev/null +++ b/matrix-sdk-android/src/kotlinCrypto/java/org/matrix/android/sdk/internal/session/sync/handler/ShieldSummaryUpdater.kt @@ -0,0 +1,56 @@ +/* + * Copyright 2023 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.session.sync.handler + +import androidx.work.BackoffPolicy +import androidx.work.ExistingWorkPolicy +import org.matrix.android.sdk.internal.crypto.crosssigning.UpdateTrustWorker +import org.matrix.android.sdk.internal.crypto.crosssigning.UpdateTrustWorkerDataRepository +import org.matrix.android.sdk.internal.di.SessionId +import org.matrix.android.sdk.internal.di.WorkManagerProvider +import org.matrix.android.sdk.internal.session.SessionScope +import org.matrix.android.sdk.internal.util.logLimit +import org.matrix.android.sdk.internal.worker.WorkerParamsFactory +import timber.log.Timber +import java.util.concurrent.TimeUnit +import javax.inject.Inject + +@SessionScope +internal class ShieldSummaryUpdater @Inject constructor( + @SessionId private val sessionId: String, + private val workManagerProvider: WorkManagerProvider, + private val updateTrustWorkerDataRepository: UpdateTrustWorkerDataRepository, +) { + + fun refreshShieldsForRoomIds(roomIds: Set) { + Timber.d("## CrossSigning - checkAffectedRoomShields for roomIds: ${roomIds.logLimit()}") + val workerParams = UpdateTrustWorker.Params( + sessionId = sessionId, + filename = updateTrustWorkerDataRepository.createParam(emptyList(), roomIds = roomIds.toList()) + ) + val workerData = WorkerParamsFactory.toData(workerParams) + + val workRequest = workManagerProvider.matrixOneTimeWorkRequestBuilder() + .setInputData(workerData) + .setBackoffCriteria(BackoffPolicy.LINEAR, WorkManagerProvider.BACKOFF_DELAY_MILLIS, TimeUnit.MILLISECONDS) + .build() + + workManagerProvider.workManager + .beginUniqueWork("TRUST_UPDATE_QUEUE", ExistingWorkPolicy.APPEND_OR_REPLACE, workRequest) + .enqueue() + } +} 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 e26ca2f86a..086d741acc 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 @@ -19,19 +19,19 @@ package org.matrix.android.sdk.internal.crypto import com.zhuinden.monarchy.Monarchy import org.matrix.android.sdk.api.session.crypto.model.RoomEncryptionTrustLevel import org.matrix.android.sdk.api.session.events.model.EventType +import org.matrix.android.sdk.api.session.room.model.Membership import org.matrix.android.sdk.internal.database.mapper.EventMapper import org.matrix.android.sdk.internal.database.model.EventEntity import org.matrix.android.sdk.internal.database.model.EventEntityFields -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.database.query.whereType import org.matrix.android.sdk.internal.di.SessionDatabase import org.matrix.android.sdk.internal.di.UserId +import org.matrix.android.sdk.internal.query.process import org.matrix.android.sdk.internal.session.room.membership.RoomMemberHelper import org.matrix.android.sdk.internal.util.fetchCopied -import org.matrix.android.sdk.internal.util.logLimit import timber.log.Timber import javax.inject.Inject @@ -89,14 +89,30 @@ internal class CryptoSessionInfoProvider @Inject constructor( } fun getRoomsWhereUsersAreParticipating(userList: List): List { + if (userList.contains(myUserId)) { + // just take all + val roomIds: List? = null + monarchy.doWithRealm { sessionRealm -> + RoomSummaryEntity.where(sessionRealm) + .process(RoomSummaryEntityFields.MEMBERSHIP_STR, Membership.activeMemberships()) + .findAll() + .map { it.roomId } + } + return roomIds.orEmpty() + } var roomIds: List? = null monarchy.doWithRealm { sessionRealm -> - roomIds = sessionRealm.where(RoomMemberSummaryEntity::class.java) - .`in`(RoomMemberSummaryEntityFields.USER_ID, userList.toTypedArray()) - .distinct(RoomMemberSummaryEntityFields.ROOM_ID) + roomIds = RoomSummaryEntity.where(sessionRealm) + .process(RoomSummaryEntityFields.MEMBERSHIP_STR, Membership.activeMemberships()) .findAll() + .filter { it.otherMemberIds.any { it in userList } } .map { it.roomId } - .also { Timber.d("## CrossSigning - ... impacted rooms ${it.logLimit()}") } +// roomIds = 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()}") } } return roomIds.orEmpty() } diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/crosssigning/UpdateTrustWorkerDataRepository.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/crosssigning/UpdateTrustWorkerDataRepository.kt index 0878a9f765..d9207d05be 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/crosssigning/UpdateTrustWorkerDataRepository.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/crosssigning/UpdateTrustWorkerDataRepository.kt @@ -28,7 +28,10 @@ import javax.inject.Inject @JsonClass(generateAdapter = true) internal data class UpdateTrustWorkerData( @Json(name = "userIds") - val userIds: List + val userIds: List, + // When we just need to refresh the room shield (no change on user keys, but a membership change) + @Json(name = "roomIds") + val roomIds: List? = null ) internal class UpdateTrustWorkerDataRepository @Inject constructor( @@ -38,12 +41,12 @@ internal class UpdateTrustWorkerDataRepository @Inject constructor( private val jsonAdapter = MoshiProvider.providesMoshi().adapter(UpdateTrustWorkerData::class.java) // Return the path of the created file - fun createParam(userIds: List): String { + fun createParam(userIds: List, roomIds: List? = null): String { val filename = "${UUID.randomUUID()}.json" workingDirectory.mkdirs() val file = File(workingDirectory, filename) - UpdateTrustWorkerData(userIds = userIds) + UpdateTrustWorkerData(userIds = userIds, roomIds = roomIds) .let { jsonAdapter.toJson(it) } .let { file.writeText(it) } 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 b251bc24f0..cbb75398c4 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 @@ -168,6 +168,9 @@ internal class RoomSummaryUpdater @Inject constructor( val roomAliases = ContentMapper.map(lastAliasesEvent?.content).toModel()?.aliases .orEmpty() roomSummaryEntity.updateAliases(roomAliases) + + val wasEncrypted = roomSummaryEntity.isEncrypted + roomSummaryEntity.isEncrypted = encryptionEvent != null roomSummaryEntity.e2eAlgorithm = ContentMapper.map(encryptionEvent?.content) @@ -197,17 +200,13 @@ internal class RoomSummaryUpdater @Inject constructor( // better to use what we know roomSummaryEntity.joinedMembersCount = otherRoomMembers.size + 1 } - if (roomSummaryEntity.isEncrypted && otherRoomMembers.isNotEmpty()) { - if (aggregator == null) { - // Do it now - // mmm maybe we could only refresh shield instead of checking trust also? - // XXX why doing this here? we don't show shield anymore and it will be refreshed - // by the sdk - // crossSigningService.checkTrustAndAffectedRoomShields(otherRoomMembers) - } else { - // Schedule it - aggregator.userIdsForCheckingTrustAndAffectedRoomShields.addAll(otherRoomMembers) - } + } + + if (roomSummaryEntity.isEncrypted) { + if (!wasEncrypted || updateMembers || roomSummaryEntity.roomEncryptionTrustLevel == null) { + // trigger a shield update + // if users add devices/keys or signatures the device list manager will trigger a refresh + aggregator?.roomsWithMembershipChangesForShieldUpdate?.add(roomId) } } } @@ -410,7 +409,7 @@ internal class RoomSummaryUpdater @Inject constructor( val relatedSpaces = lookupMap.keys .filter { it.roomType == RoomType.SPACE } .filter { - dmRoom.otherMemberIds.toList().intersect(it.otherMemberIds.toList()).isNotEmpty() + dmRoom.otherMemberIds.toList().intersect(it.otherMemberIds.toSet()).isNotEmpty() } .map { it.roomId } .distinct() diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/SyncResponsePostTreatmentAggregator.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/SyncResponsePostTreatmentAggregator.kt index af05e08da3..4532a8d418 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/SyncResponsePostTreatmentAggregator.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/SyncResponsePostTreatmentAggregator.kt @@ -29,7 +29,8 @@ internal class SyncResponsePostTreatmentAggregator { val userIdsToFetch = mutableSetOf() // Set of users to call `crossSigningService.checkTrustAndAffectedRoomShields` once per sync - val userIdsForCheckingTrustAndAffectedRoomShields = mutableSetOf() + + val roomsWithMembershipChangesForShieldUpdate = mutableSetOf() // For the crypto store val cryptoStoreAggregator = CryptoStoreAggregator() diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/handler/SyncResponsePostTreatmentAggregatorHandler.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/handler/SyncResponsePostTreatmentAggregatorHandler.kt index 948a0a2501..3700bbf46f 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/handler/SyncResponsePostTreatmentAggregatorHandler.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/handler/SyncResponsePostTreatmentAggregatorHandler.kt @@ -19,7 +19,6 @@ package org.matrix.android.sdk.internal.session.sync.handler import androidx.work.BackoffPolicy import androidx.work.ExistingWorkPolicy import org.matrix.android.sdk.api.MatrixPatterns -import org.matrix.android.sdk.api.session.crypto.crosssigning.CrossSigningService import org.matrix.android.sdk.internal.crypto.crosssigning.UpdateTrustWorker import org.matrix.android.sdk.internal.crypto.crosssigning.UpdateTrustWorkerDataRepository import org.matrix.android.sdk.internal.di.SessionId @@ -39,16 +38,16 @@ internal class SyncResponsePostTreatmentAggregatorHandler @Inject constructor( private val directChatsHelper: DirectChatsHelper, private val ephemeralTemporaryStore: RoomSyncEphemeralTemporaryStore, private val updateUserAccountDataTask: UpdateUserAccountDataTask, - private val crossSigningService: CrossSigningService, private val updateTrustWorkerDataRepository: UpdateTrustWorkerDataRepository, private val workManagerProvider: WorkManagerProvider, + private val roomShieldSummaryUpdater: ShieldSummaryUpdater, @SessionId private val sessionId: String, ) { suspend fun handle(aggregator: SyncResponsePostTreatmentAggregator) { cleanupEphemeralFiles(aggregator.ephemeralFilesToDelete) updateDirectUserIds(aggregator.directChatsToCheck) fetchAndUpdateUsers(aggregator.userIdsToFetch) - handleUserIdsForCheckingTrustAndAffectedRoomShields(aggregator.userIdsForCheckingTrustAndAffectedRoomShields) + handleRefreshRoomShieldsForRooms(aggregator.roomsWithMembershipChangesForShieldUpdate) } private fun cleanupEphemeralFiles(ephemeralFilesToDelete: List) { @@ -105,8 +104,8 @@ internal class SyncResponsePostTreatmentAggregatorHandler @Inject constructor( .enqueue() } - private suspend fun handleUserIdsForCheckingTrustAndAffectedRoomShields(userIdsWithDeviceUpdate: Collection) { - if (userIdsWithDeviceUpdate.isEmpty()) return - crossSigningService.checkTrustAndAffectedRoomShields(userIdsWithDeviceUpdate.toList()) + private fun handleRefreshRoomShieldsForRooms(roomIds: Set) { + if (roomIds.isEmpty()) return + roomShieldSummaryUpdater.refreshShieldsForRoomIds(roomIds) } } diff --git a/matrix-sdk-android/src/rustCrypto/java/org/matrix/android/sdk/internal/crypto/network/OutgoingRequestsProcessor.kt b/matrix-sdk-android/src/rustCrypto/java/org/matrix/android/sdk/internal/crypto/network/OutgoingRequestsProcessor.kt index 77fd9b3ea3..9e0301f487 100644 --- a/matrix-sdk-android/src/rustCrypto/java/org/matrix/android/sdk/internal/crypto/network/OutgoingRequestsProcessor.kt +++ b/matrix-sdk-android/src/rustCrypto/java/org/matrix/android/sdk/internal/crypto/network/OutgoingRequestsProcessor.kt @@ -27,10 +27,10 @@ import org.matrix.android.sdk.api.MatrixConfiguration import org.matrix.android.sdk.api.MatrixCoroutineDispatchers import org.matrix.android.sdk.api.logger.LoggerTag import org.matrix.android.sdk.api.session.events.model.Event -import org.matrix.android.sdk.internal.crypto.ComputeShieldForGroupUseCase import org.matrix.android.sdk.internal.crypto.CryptoSessionInfoProvider import org.matrix.android.sdk.internal.crypto.OlmMachine import org.matrix.android.sdk.internal.session.SessionScope +import org.matrix.android.sdk.internal.session.sync.handler.ShieldSummaryUpdater import org.matrix.rustcomponents.sdk.crypto.Request import org.matrix.rustcomponents.sdk.crypto.RequestType import timber.log.Timber @@ -43,7 +43,7 @@ internal class OutgoingRequestsProcessor @Inject constructor( private val requestSender: RequestSender, private val coroutineScope: CoroutineScope, private val cryptoSessionInfoProvider: CryptoSessionInfoProvider, - private val computeShieldForGroup: ComputeShieldForGroupUseCase, + private val shieldSummaryUpdater: ShieldSummaryUpdater, private val matrixConfiguration: MatrixConfiguration, private val coroutineDispatchers: MatrixCoroutineDispatchers, ) { @@ -137,7 +137,7 @@ internal class OutgoingRequestsProcessor @Inject constructor( return try { val response = requestSender.queryKeys(request) olmMachine.markRequestAsSent(request.requestId, RequestType.KEYS_QUERY, response) - coroutineScope.updateShields(olmMachine, request.users) + shieldSummaryUpdater.refreshShieldsForRoomsWithMembers(request.users) coroutineScope.markMessageVerificationStatesAsDirty(request.users) true } catch (throwable: Throwable) { @@ -146,18 +146,6 @@ internal class OutgoingRequestsProcessor @Inject constructor( } } - private fun CoroutineScope.updateShields(olmMachine: OlmMachine, userIds: List) = launch(coroutineDispatchers.computation) { - cryptoSessionInfoProvider.getRoomsWhereUsersAreParticipating(userIds).forEach { roomId -> - if (cryptoSessionInfoProvider.isRoomEncrypted(roomId)) { - val userGroup = cryptoSessionInfoProvider.getUserListForShieldComputation(roomId) - val shield = computeShieldForGroup(olmMachine, userGroup) - cryptoSessionInfoProvider.updateShieldForRoom(roomId, shield) - } else { - cryptoSessionInfoProvider.updateShieldForRoom(roomId, null) - } - } - } - private fun CoroutineScope.markMessageVerificationStatesAsDirty(userIds: List) = launch(coroutineDispatchers.computation) { cryptoSessionInfoProvider.markMessageVerificationStateAsDirty(userIds) } diff --git a/matrix-sdk-android/src/rustCrypto/java/org/matrix/android/sdk/internal/session/sync/handler/ShieldSummaryUpdater.kt b/matrix-sdk-android/src/rustCrypto/java/org/matrix/android/sdk/internal/session/sync/handler/ShieldSummaryUpdater.kt new file mode 100644 index 0000000000..9f77d7003e --- /dev/null +++ b/matrix-sdk-android/src/rustCrypto/java/org/matrix/android/sdk/internal/session/sync/handler/ShieldSummaryUpdater.kt @@ -0,0 +1,60 @@ +/* + * Copyright (c) 2023 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.session.sync.handler + +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.launch +import org.matrix.android.sdk.api.MatrixCoroutineDispatchers +import org.matrix.android.sdk.internal.crypto.ComputeShieldForGroupUseCase +import org.matrix.android.sdk.internal.crypto.CryptoSessionInfoProvider +import org.matrix.android.sdk.internal.crypto.OlmMachine +import org.matrix.android.sdk.internal.session.SessionScope +import javax.inject.Inject + +@SessionScope +internal class ShieldSummaryUpdater @Inject constructor( + private val olmMachine: dagger.Lazy, + private val coroutineScope: CoroutineScope, + private val coroutineDispatchers: MatrixCoroutineDispatchers, + private val cryptoSessionInfoProvider: CryptoSessionInfoProvider, + private val computeShieldForGroup: ComputeShieldForGroupUseCase, +) { + + fun refreshShieldsForRoomsWithMembers(userIds: List) { + coroutineScope.launch(coroutineDispatchers.computation) { + cryptoSessionInfoProvider.getRoomsWhereUsersAreParticipating(userIds).forEach { roomId -> + if (cryptoSessionInfoProvider.isRoomEncrypted(roomId)) { + val userGroup = cryptoSessionInfoProvider.getUserListForShieldComputation(roomId) + val shield = computeShieldForGroup(olmMachine.get(), userGroup) + cryptoSessionInfoProvider.updateShieldForRoom(roomId, shield) + } else { + cryptoSessionInfoProvider.updateShieldForRoom(roomId, null) + } + } + } + } + + fun refreshShieldsForRoomIds(roomIds: Set) { + coroutineScope.launch(coroutineDispatchers.computation) { + roomIds.forEach { roomId -> + val userGroup = cryptoSessionInfoProvider.getUserListForShieldComputation(roomId) + val shield = computeShieldForGroup(olmMachine.get(), userGroup) + cryptoSessionInfoProvider.updateShieldForRoom(roomId, shield) + } + } + } +} From 1d651db82ba581eac333ba980b85e82a0633f632 Mon Sep 17 00:00:00 2001 From: valere Date: Thu, 8 Jun 2023 09:07:21 +0200 Subject: [PATCH 2/2] Add change log --- changelog.d/8507.bugfix | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/8507.bugfix diff --git a/changelog.d/8507.bugfix b/changelog.d/8507.bugfix new file mode 100644 index 0000000000..eb609dd0fc --- /dev/null +++ b/changelog.d/8507.bugfix @@ -0,0 +1 @@ +In some conditions the room shield is not refreshed correctly