From 4ee34126bdf11327968f5d93d008082a01051fe6 Mon Sep 17 00:00:00 2001 From: Valere Date: Tue, 5 Jan 2021 09:56:48 +0100 Subject: [PATCH 1/4] Persist outbound group session --- CHANGES.md | 1 + .../crypto/gossiping/KeyShareTests.kt | 75 +++++++++++++++++++ .../crypto/IncomingGossipingRequestManager.kt | 66 +++++++++------- .../sdk/internal/crypto/MXOlmDevice.kt | 41 +++++++++- .../algorithms/megolm/MXMegolmEncryption.kt | 27 +++++-- .../megolm/MXOutboundSessionInfo.kt | 5 +- .../algorithms/megolm/SharedWithHelper.kt | 4 - .../model/OutboundGroupSessionWrapper.kt | 24 ++++++ .../internal/crypto/store/IMXCryptoStore.kt | 22 +++++- .../crypto/store/db/RealmCryptoStore.kt | 43 ++++++++++- .../store/db/RealmCryptoStoreMigration.kt | 16 +++- .../crypto/store/db/RealmCryptoStoreModule.kt | 4 +- .../crypto/store/db/model/CryptoRoomEntity.kt | 7 +- .../model/OutboundGroupSessionInfoEntity.kt | 44 +++++++++++ .../action/MessageActionsViewModel.kt | 3 +- 15 files changed, 332 insertions(+), 50 deletions(-) create mode 100644 matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/model/OutboundGroupSessionWrapper.kt create mode 100644 matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/store/db/model/OutboundGroupSessionInfoEntity.kt diff --git a/CHANGES.md b/CHANGES.md index eaaa4c1b0a..47e57c76e4 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -6,6 +6,7 @@ Features ✨: Improvements 🙌: - Add System theme option and set as default (#904) (#2387) + - Store megolm outbound session to improve send time of first message after app launch. Bugfix 🐛: - Unspecced msgType field in m.sticker (#2580) diff --git a/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/internal/crypto/gossiping/KeyShareTests.kt b/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/internal/crypto/gossiping/KeyShareTests.kt index 197e36df06..2c4d89b070 100644 --- a/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/internal/crypto/gossiping/KeyShareTests.kt +++ b/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/internal/crypto/gossiping/KeyShareTests.kt @@ -50,6 +50,8 @@ import org.junit.FixMethodOrder import org.junit.Test import org.junit.runner.RunWith import org.junit.runners.MethodSorters +import org.matrix.android.sdk.api.extensions.tryOrNull +import org.matrix.android.sdk.api.session.room.model.message.MessageContent import java.util.concurrent.CountDownLatch @RunWith(AndroidJUnit4::class) @@ -296,4 +298,77 @@ class KeyShareTests : InstrumentedTest { mTestHelper.signOutAndClose(aliceSession1) mTestHelper.signOutAndClose(aliceSession2) } + + @Test + fun test_ImproperKeyShareBug() { + val aliceSession = mTestHelper.createAccount(TestConstants.USER_ALICE, SessionTestParams(true)) + + mTestHelper.doSync { + aliceSession.cryptoService().crossSigningService() + .initializeCrossSigning(UserPasswordAuth( + user = aliceSession.myUserId, + password = TestConstants.PASSWORD + ), it) + } + + // Create an encrypted room and send a couple of messages + val roomId = mTestHelper.doSync { + aliceSession.createRoom( + CreateRoomParams().apply { + visibility = RoomDirectoryVisibility.PRIVATE + enableEncryption() + }, + it + ) + } + val roomAlicePov = aliceSession.getRoom(roomId) + assertNotNull(roomAlicePov) + Thread.sleep(1_000) + assertTrue(roomAlicePov?.isEncrypted() == true) + val secondEventId = mTestHelper.sendTextMessage(roomAlicePov!!, "Message", 3)[1].eventId + + // Create bob session + + val bobSession = mTestHelper.createAccount(TestConstants.USER_BOB, SessionTestParams(true)) + mTestHelper.doSync { + bobSession.cryptoService().crossSigningService() + .initializeCrossSigning(UserPasswordAuth( + user = bobSession.myUserId, + password = TestConstants.PASSWORD + ), it) + } + + // Let alice invite bob + mTestHelper.doSync { + roomAlicePov.invite(bobSession.myUserId, null, it) + } + + mTestHelper.doSync { + bobSession.joinRoom(roomAlicePov.roomId, null, emptyList(), it) + } + + // we want to discard alice outbound session + aliceSession.cryptoService().discardOutboundSession(roomAlicePov.roomId) + + // and now resend a new message to reset index to 0 + mTestHelper.sendTextMessage(roomAlicePov, "After", 1) + + val roomRoomBobPov = aliceSession.getRoom(roomId) + val beforeJoin = roomRoomBobPov!!.getTimeLineEvent(secondEventId) + + var dRes = tryOrNull { bobSession.cryptoService().decryptEvent(beforeJoin!!.root, "") } + + assert(dRes == null) + + // Try to re-ask the keys + + bobSession.cryptoService().reRequestRoomKeyForEvent(beforeJoin!!.root) + + Thread.sleep(3_000) + + // With the bug the first session would have improperly reshare that key :/ + dRes = tryOrNull { bobSession.cryptoService().decryptEvent(beforeJoin.root, "") } + Log.d("#TEST", "KS: sgould not decrypt that ${beforeJoin.root.getClearContent().toModel()?.body}") + assert(dRes?.clearEvent == null) + } } 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 4f94a27bbd..c075c90eb9 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 @@ -32,6 +32,7 @@ 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 import org.matrix.android.sdk.internal.crypto.model.rest.GossipingToDeviceObject +import org.matrix.android.sdk.internal.crypto.model.rest.RoomKeyRequestBody import org.matrix.android.sdk.internal.crypto.store.IMXCryptoStore import org.matrix.android.sdk.internal.di.SessionId import org.matrix.android.sdk.internal.session.SessionScope @@ -206,34 +207,7 @@ internal class IncomingGossipingRequestManager @Inject constructor( Timber.v("## CRYPTO | GOSSIP processIncomingRoomKeyRequest from $userId:$deviceId for $roomId / ${body.sessionId} id ${request.requestId}") if (credentials.userId != userId) { - Timber.w("## CRYPTO | GOSSIP processReceivedGossipingRequests() : room key request from other user") - val senderKey = body.senderKey ?: return Unit - .also { Timber.w("missing senderKey") } - .also { cryptoStore.updateGossipingRequestState(request, GossipingRequestState.REJECTED) } - val sessionId = body.sessionId ?: return Unit - .also { Timber.w("missing sessionId") } - .also { cryptoStore.updateGossipingRequestState(request, GossipingRequestState.REJECTED) } - - if (alg != MXCRYPTO_ALGORITHM_MEGOLM) { - return Unit - .also { Timber.w("Only megolm is accepted here") } - .also { cryptoStore.updateGossipingRequestState(request, GossipingRequestState.REJECTED) } - } - - val roomEncryptor = roomEncryptorsStore.get(roomId) ?: return Unit - .also { Timber.w("no room Encryptor") } - .also { cryptoStore.updateGossipingRequestState(request, GossipingRequestState.REJECTED) } - - cryptoCoroutineScope.launch(coroutineDispatchers.crypto) { - val isSuccess = roomEncryptor.reshareKey(sessionId, userId, deviceId, senderKey) - - if (isSuccess) { - cryptoStore.updateGossipingRequestState(request, GossipingRequestState.ACCEPTED) - } else { - cryptoStore.updateGossipingRequestState(request, GossipingRequestState.UNABLE_TO_PROCESS) - } - } - cryptoStore.updateGossipingRequestState(request, GossipingRequestState.RE_REQUESTED) + handleKeyRequestFromOtherUser(body, request, alg, roomId, userId, deviceId) return } // TODO: should we queue up requests we don't yet have keys for, in case they turn up later? @@ -291,6 +265,42 @@ internal class IncomingGossipingRequestManager @Inject constructor( onRoomKeyRequest(request) } + private fun handleKeyRequestFromOtherUser(body: RoomKeyRequestBody, + request: IncomingRoomKeyRequest, + alg: String, + roomId: String, + userId: String, + deviceId: String) { + Timber.w("## CRYPTO | GOSSIP processReceivedGossipingRequests() : room key request from other user") + val senderKey = body.senderKey ?: return Unit + .also { Timber.w("missing senderKey") } + .also { cryptoStore.updateGossipingRequestState(request, GossipingRequestState.REJECTED) } + val sessionId = body.sessionId ?: return Unit + .also { Timber.w("missing sessionId") } + .also { cryptoStore.updateGossipingRequestState(request, GossipingRequestState.REJECTED) } + + if (alg != MXCRYPTO_ALGORITHM_MEGOLM) { + return Unit + .also { Timber.w("Only megolm is accepted here") } + .also { cryptoStore.updateGossipingRequestState(request, GossipingRequestState.REJECTED) } + } + + val roomEncryptor = roomEncryptorsStore.get(roomId) ?: return Unit + .also { Timber.w("no room Encryptor") } + .also { cryptoStore.updateGossipingRequestState(request, GossipingRequestState.REJECTED) } + + cryptoCoroutineScope.launch(coroutineDispatchers.crypto) { + val isSuccess = roomEncryptor.reshareKey(sessionId, userId, deviceId, senderKey) + + if (isSuccess) { + cryptoStore.updateGossipingRequestState(request, GossipingRequestState.ACCEPTED) + } else { + cryptoStore.updateGossipingRequestState(request, GossipingRequestState.UNABLE_TO_PROCESS) + } + } + cryptoStore.updateGossipingRequestState(request, GossipingRequestState.RE_REQUESTED) + } + private fun processIncomingSecretShareRequest(request: IncomingSecretShareRequest) { val secretName = request.secretName ?: return Unit.also { cryptoStore.updateGossipingRequestState(request, GossipingRequestState.REJECTED) 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 c952602d93..a892d3850a 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 @@ -19,6 +19,8 @@ package org.matrix.android.sdk.internal.crypto import org.matrix.android.sdk.api.session.crypto.MXCryptoError import org.matrix.android.sdk.api.util.JSON_DICT_PARAMETERIZED_TYPE import org.matrix.android.sdk.api.util.JsonDict +import org.matrix.android.sdk.internal.crypto.algorithms.megolm.MXOutboundSessionInfo +import org.matrix.android.sdk.internal.crypto.algorithms.megolm.SharedWithHelper import org.matrix.android.sdk.internal.crypto.algorithms.olm.OlmDecryptionResult import org.matrix.android.sdk.internal.crypto.model.OlmInboundGroupSessionWrapper2 import org.matrix.android.sdk.internal.crypto.model.OlmSessionWrapper @@ -46,7 +48,7 @@ internal class MXOlmDevice @Inject constructor( */ private val store: IMXCryptoStore, private val inboundGroupSessionStore: InboundGroupSessionStore - ) { +) { /** * @return the Curve25519 key for the account. @@ -135,6 +137,10 @@ internal class MXOlmDevice @Inject constructor( */ fun release() { olmUtility?.releaseUtility() + outboundGroupSessionStore.values.forEach { + it.releaseSession() + } + outboundGroupSessionStore.clear() } /** @@ -406,11 +412,12 @@ internal class MXOlmDevice @Inject constructor( * * @return the session id for the outbound session. */ - fun createOutboundGroupSession(): String? { + fun createOutboundGroupSessionForRoom(roomId: String): String? { var session: OlmOutboundGroupSession? = null try { session = OlmOutboundGroupSession() outboundGroupSessionStore[session.sessionIdentifier()] = session + store.storeCurrentOutboundGroupSessionForRoom(roomId, session) return session.sessionIdentifier() } catch (e: Exception) { Timber.e(e, "createOutboundGroupSession") @@ -421,6 +428,34 @@ internal class MXOlmDevice @Inject constructor( return null } + fun storeOutboundGroupSessionForRoom(roomId: String, sessionId: String) { + outboundGroupSessionStore[sessionId]?.let { + store.storeCurrentOutboundGroupSessionForRoom(roomId, it) + } + } + + fun restoreOutboundGroupSessionForRoom(roomId: String): MXOutboundSessionInfo? { + val restoredOutboundGroupSession = store.getCurrentOutboundGroupSessionForRoom(roomId) + if (restoredOutboundGroupSession != null) { + val sessionId = restoredOutboundGroupSession.outboundGroupSession.sessionIdentifier() + if (!outboundGroupSessionStore.containsKey(sessionId)) { + outboundGroupSessionStore[sessionId] = restoredOutboundGroupSession.outboundGroupSession + return MXOutboundSessionInfo( + sessionId = sessionId, + sharedWithHelper = SharedWithHelper(roomId, sessionId, store), + restoredOutboundGroupSession.creationTime + ) + } else { + restoredOutboundGroupSession.outboundGroupSession.releaseSession() + } + } + return null + } + + fun discardOutboundGroupSessionForRoom(roomId: String) { + outboundGroupSessionStore.remove(roomId)?.releaseSession() + store.storeCurrentOutboundGroupSessionForRoom(roomId, null) + } /** * Get the current session key of an outbound group session. * @@ -747,7 +782,7 @@ internal class MXOlmDevice @Inject constructor( throw MXCryptoError.Base(MXCryptoError.ErrorType.MISSING_SENDER_KEY, MXCryptoError.ERROR_MISSING_PROPERTY_REASON) } - val session = inboundGroupSessionStore.getInboundGroupSession(sessionId, senderKey) + val session = inboundGroupSessionStore.getInboundGroupSession(sessionId, senderKey) if (session != null) { // Check that the room id matches the original one for the session. This stops 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 fd431ce735..fa8acafb83 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 @@ -68,6 +68,10 @@ internal class MXMegolmEncryption( // case outboundSession.shareOperation will be non-null.) private var outboundSession: MXOutboundSessionInfo? = null + init { + // restore existing outbound session if any + outboundSession = olmDevice.restoreOutboundGroupSessionForRoom(roomId) + } // Default rotation periods // TODO: Make it configurable via parameters // Session rotation periods @@ -86,6 +90,9 @@ internal class MXMegolmEncryption( return encryptContent(outboundSession, eventType, eventContent) .also { notifyWithheldForSession(devices.withHeldDevices, outboundSession) + // 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) } } @@ -107,6 +114,7 @@ internal class MXMegolmEncryption( override fun discardSessionKey() { outboundSession = null + olmDevice.discardOutboundGroupSessionForRoom(roomId) } /** @@ -116,7 +124,7 @@ internal class MXMegolmEncryption( */ private fun prepareNewSessionInRoom(): MXOutboundSessionInfo { Timber.v("## CRYPTO | prepareNewSessionInRoom() ") - val sessionId = olmDevice.createOutboundGroupSession() + val sessionId = olmDevice.createOutboundGroupSessionForRoom(roomId) val keysClaimedMap = HashMap() keysClaimedMap["ed25519"] = olmDevice.deviceEd25519Key!! @@ -152,7 +160,7 @@ internal class MXMegolmEncryption( val deviceIds = devicesInRoom.getUserDeviceIds(userId) for (deviceId in deviceIds!!) { val deviceInfo = devicesInRoom.getObject(userId, deviceId) - if (deviceInfo != null && !cryptoStore.wasSessionSharedWithUser(roomId, safeSession.sessionId, userId, deviceId).found) { + if (deviceInfo != null && !cryptoStore.getSharedSessionInfo(roomId, safeSession.sessionId, userId, deviceId).found) { val devices = shareMap.getOrPut(userId) { ArrayList() } devices.add(deviceInfo) } @@ -401,11 +409,18 @@ internal class MXMegolmEncryption( .also { Timber.w("## Crypto reshareKey: Device not found") } // Get the chain index of the key we previously sent this device - val chainIndex = outboundSession?.sharedWithHelper?.wasSharedWith(userId, deviceId) ?: return false + val wasSessionSharedWithUser = cryptoStore.getSharedSessionInfo(roomId, sessionId, userId, deviceId) + if (!wasSessionSharedWithUser.found) { + // This session was never shared with this user + // Send a room key with held + notifyKeyWithHeld(listOf(UserDevice(userId, deviceId)), sessionId, senderKey, WithHeldCode.UNAUTHORISED) + Timber.w("## Crypto reshareKey: ERROR : Never shared megolm with this device") + return false + } + // if found chain index should not be null + val chainIndex = wasSessionSharedWithUser.chainIndex ?: return false .also { - // Send a room key with held - notifyKeyWithHeld(listOf(UserDevice(userId, deviceId)), sessionId, senderKey, WithHeldCode.UNAUTHORISED) - Timber.w("## Crypto reshareKey: ERROR : Never share megolm with this device") + Timber.w("## Crypto reshareKey: Null chain index") } val devicesByUser = mapOf(userId to listOf(deviceInfo)) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/algorithms/megolm/MXOutboundSessionInfo.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/algorithms/megolm/MXOutboundSessionInfo.kt index 9244b4d5e7..c0b920b09d 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/algorithms/megolm/MXOutboundSessionInfo.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/algorithms/megolm/MXOutboundSessionInfo.kt @@ -23,9 +23,8 @@ import timber.log.Timber internal class MXOutboundSessionInfo( // The id of the session val sessionId: String, - val sharedWithHelper: SharedWithHelper) { - // When the session was created - private val creationTime = System.currentTimeMillis() + val sharedWithHelper: SharedWithHelper, + private val creationTime: Long = System.currentTimeMillis()) { // Number of times this session has been used var useCount: Int = 0 diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/algorithms/megolm/SharedWithHelper.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/algorithms/megolm/SharedWithHelper.kt index 921f9b2cdc..f17168a6d2 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/algorithms/megolm/SharedWithHelper.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/algorithms/megolm/SharedWithHelper.kt @@ -28,10 +28,6 @@ internal class SharedWithHelper( return cryptoStore.getSharedWithInfo(roomId, sessionId) } - fun wasSharedWith(userId: String, deviceId: String): Int? { - return cryptoStore.wasSessionSharedWithUser(roomId, sessionId, userId, deviceId).chainIndex - } - fun markedSessionAsShared(userId: String, deviceId: String, chainIndex: Int) { cryptoStore.markedSessionAsShared(roomId, sessionId, userId, deviceId, chainIndex) } diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/model/OutboundGroupSessionWrapper.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/model/OutboundGroupSessionWrapper.kt new file mode 100644 index 0000000000..b616284e14 --- /dev/null +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/model/OutboundGroupSessionWrapper.kt @@ -0,0 +1,24 @@ +/* + * 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.model + +import org.matrix.olm.OlmOutboundGroupSession + +data class OutboundGroupSessionWrapper( + val outboundGroupSession: OlmOutboundGroupSession, + val creationTime: Long +) 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 9a9f645b49..6f1487f80a 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 @@ -33,11 +33,13 @@ import org.matrix.android.sdk.internal.crypto.model.CryptoDeviceInfo import org.matrix.android.sdk.internal.crypto.model.MXUsersDevicesMap import org.matrix.android.sdk.internal.crypto.model.OlmInboundGroupSessionWrapper2 import org.matrix.android.sdk.internal.crypto.model.OlmSessionWrapper +import org.matrix.android.sdk.internal.crypto.model.OutboundGroupSessionWrapper import org.matrix.android.sdk.internal.crypto.model.event.RoomKeyWithHeldContent import org.matrix.android.sdk.internal.crypto.model.rest.DeviceInfo import org.matrix.android.sdk.internal.crypto.model.rest.RoomKeyRequestBody import org.matrix.android.sdk.internal.crypto.store.db.model.KeysBackupDataEntity import org.matrix.olm.OlmAccount +import org.matrix.olm.OlmOutboundGroupSession /** * the crypto data store @@ -293,6 +295,16 @@ internal interface IMXCryptoStore { */ fun getInboundGroupSession(sessionId: String, senderKey: String): OlmInboundGroupSessionWrapper2? + /** + * Get the current outbound group session for this encrypted room + */ + fun getCurrentOutboundGroupSessionForRoom(roomId: String): OutboundGroupSessionWrapper? + + /** + * Get the current outbound group session for this encrypted room + */ + fun storeCurrentOutboundGroupSessionForRoom(roomId: String, outboundGroupSession: OlmOutboundGroupSession?) + /** * Remove an inbound group session * @@ -439,7 +451,15 @@ internal interface IMXCryptoStore { fun getWithHeldMegolmSession(roomId: String, sessionId: String): RoomKeyWithHeldContent? fun markedSessionAsShared(roomId: String?, sessionId: String, userId: String, deviceId: String, chainIndex: Int) - fun wasSessionSharedWithUser(roomId: String?, sessionId: String, userId: String, deviceId: String): SharedSessionResult + + /** + * Query for information on this session sharing history. + * @return SharedSessionResult + * if found is true then this session was initialy shared with that user|device, + * in this case chainIndex is not nullindicates the ratchet position. + * In found is false, chainIndex is null + */ + fun getSharedSessionInfo(roomId: String?, sessionId: String, userId: String, deviceId: String): SharedSessionResult data class SharedSessionResult(val found: Boolean, val chainIndex: Int?) fun getSharedWithInfo(roomId: String?, sessionId: String): MXUsersDevicesMap 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 6b83c4f7d1..7912d03ffd 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 @@ -47,6 +47,7 @@ import org.matrix.android.sdk.internal.crypto.model.CryptoDeviceInfo import org.matrix.android.sdk.internal.crypto.model.MXUsersDevicesMap import org.matrix.android.sdk.internal.crypto.model.OlmInboundGroupSessionWrapper2 import org.matrix.android.sdk.internal.crypto.model.OlmSessionWrapper +import org.matrix.android.sdk.internal.crypto.model.OutboundGroupSessionWrapper import org.matrix.android.sdk.internal.crypto.model.event.RoomKeyWithHeldContent import org.matrix.android.sdk.internal.crypto.model.rest.DeviceInfo import org.matrix.android.sdk.internal.crypto.model.rest.RoomKeyRequestBody @@ -73,6 +74,7 @@ import org.matrix.android.sdk.internal.crypto.store.db.model.OlmInboundGroupSess import org.matrix.android.sdk.internal.crypto.store.db.model.OlmInboundGroupSessionEntityFields import org.matrix.android.sdk.internal.crypto.store.db.model.OlmSessionEntity import org.matrix.android.sdk.internal.crypto.store.db.model.OlmSessionEntityFields +import org.matrix.android.sdk.internal.crypto.store.db.model.OutboundGroupSessionInfoEntity import org.matrix.android.sdk.internal.crypto.store.db.model.OutgoingGossipingRequestEntity import org.matrix.android.sdk.internal.crypto.store.db.model.OutgoingGossipingRequestEntityFields import org.matrix.android.sdk.internal.crypto.store.db.model.SharedSessionEntity @@ -95,6 +97,7 @@ import org.matrix.android.sdk.internal.di.UserId import org.matrix.android.sdk.internal.session.SessionScope import org.matrix.olm.OlmAccount import org.matrix.olm.OlmException +import org.matrix.olm.OlmOutboundGroupSession import timber.log.Timber import javax.inject.Inject import kotlin.collections.set @@ -756,6 +759,44 @@ internal class RealmCryptoStore @Inject constructor( return inboundGroupSessionToRelease[key] } + override fun getCurrentOutboundGroupSessionForRoom(roomId: String): OutboundGroupSessionWrapper? { + return doWithRealm(realmConfiguration) { realm -> + realm.where() + .equalTo(CryptoRoomEntityFields.ROOM_ID, roomId) + .findFirst()?.outboundSessionInfo?.let { entity -> + entity.getOutboundGroupSession()?.let { + OutboundGroupSessionWrapper( + it, + entity.creationTime ?: 0 + ) + } + } + } + } + + override fun storeCurrentOutboundGroupSessionForRoom(roomId: String, outboundGroupSession: OlmOutboundGroupSession?) { + // we can do this async, as it's just for restoring on next launch + // the olmdevice is caching the active instance + // this is called for each sent message (so not high frequency), thus we can use basic realm async without + // risk of reaching max async operation limit? + doRealmTransactionAsync(realmConfiguration) { realm -> + realm.where() + .equalTo(CryptoRoomEntityFields.ROOM_ID, roomId) + .findFirst()?.let { entity -> + // we should delete existing outbound session info if any + entity.outboundSessionInfo?.deleteFromRealm() + + if (outboundGroupSession != null) { + val info = realm.createObject(OutboundGroupSessionInfoEntity::class.java).apply { + creationTime = System.currentTimeMillis() + putOutboundGroupSession(outboundGroupSession) + } + entity.outboundSessionInfo = info + } + } + } + } + /** * 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 @@ -1645,7 +1686,7 @@ internal class RealmCryptoStore @Inject constructor( } } - override fun wasSessionSharedWithUser(roomId: String?, sessionId: String, userId: String, deviceId: String): IMXCryptoStore.SharedSessionResult { + override fun getSharedSessionInfo(roomId: String?, sessionId: String, userId: String, deviceId: String): IMXCryptoStore.SharedSessionResult { return doWithRealm(realmConfiguration) { realm -> SharedSessionEntity.get(realm, roomId, sessionId, userId, deviceId)?.let { IMXCryptoStore.SharedSessionResult(true, it.chainIndex) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/store/db/RealmCryptoStoreMigration.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/store/db/RealmCryptoStoreMigration.kt index 08806b0627..257e70936c 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/store/db/RealmCryptoStoreMigration.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/store/db/RealmCryptoStoreMigration.kt @@ -44,6 +44,7 @@ import org.matrix.android.sdk.internal.di.SerializeNulls import io.realm.DynamicRealm import io.realm.RealmMigration import io.realm.RealmObjectSchema +import org.matrix.android.sdk.internal.crypto.store.db.model.OutboundGroupSessionInfoEntityFields import org.matrix.androidsdk.crypto.data.MXOlmInboundGroupSession2 import timber.log.Timber import javax.inject.Inject @@ -55,7 +56,7 @@ internal class RealmCryptoStoreMigration @Inject constructor(private val crossSi // 0, 1, 2: legacy Riot-Android // 3: migrate to RiotX schema // 4, 5, 6, 7, 8, 9: migrations from RiotX (which was previously 1, 2, 3, 4, 5, 6) - const val CRYPTO_STORE_SCHEMA_VERSION = 11L + const val CRYPTO_STORE_SCHEMA_VERSION = 12L } private fun RealmObjectSchema.addFieldIfNotExists(fieldName: String, fieldType: Class<*>): RealmObjectSchema { @@ -93,6 +94,7 @@ internal class RealmCryptoStoreMigration @Inject constructor(private val crossSi if (oldVersion <= 8) migrateTo9(realm) if (oldVersion <= 9) migrateTo10(realm) if (oldVersion <= 10) migrateTo11(realm) + if (oldVersion <= 11) migrateTo12(realm) } private fun migrateTo1Legacy(realm: DynamicRealm) { @@ -483,4 +485,16 @@ internal class RealmCryptoStoreMigration @Inject constructor(private val crossSi realm.schema.get("CryptoMetadataEntity") ?.addField(CryptoMetadataEntityFields.DEVICE_KEYS_SENT_TO_SERVER, Boolean::class.java) } + + // Version 12L added outbound group session persistence + private fun migrateTo12(realm: DynamicRealm) { + Timber.d("Step 11 -> 12") + val outboundEntitySchema = realm.schema.create("OutboundGroupSessionInfoEntity") + ?.addField(OutboundGroupSessionInfoEntityFields.SERIALIZED_OUTBOUND_SESSION_DATA, String::class.java) + ?.addField(OutboundGroupSessionInfoEntityFields.CREATION_TIME, Long::class.java) + ?.setNullable(OutboundGroupSessionInfoEntityFields.CREATION_TIME, true) + + realm.schema.get("CryptoRoomEntity") + ?.addRealmObjectField(CryptoRoomEntityFields.OUTBOUND_SESSION_INFO.`$`, outboundEntitySchema) + } } diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/store/db/RealmCryptoStoreModule.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/store/db/RealmCryptoStoreModule.kt index a453fc3ed0..6aae68c83e 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/store/db/RealmCryptoStoreModule.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/store/db/RealmCryptoStoreModule.kt @@ -33,6 +33,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.WithHeldSessionEntity import io.realm.annotations.RealmModule +import org.matrix.android.sdk.internal.crypto.store.db.model.OutboundGroupSessionInfoEntity /** * Realm module for Crypto store classes @@ -54,6 +55,7 @@ import io.realm.annotations.RealmModule OutgoingGossipingRequestEntity::class, MyDeviceLastSeenInfoEntity::class, WithHeldSessionEntity::class, - SharedSessionEntity::class + SharedSessionEntity::class, + OutboundGroupSessionInfoEntity::class ]) internal class RealmCryptoStoreModule diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/store/db/model/CryptoRoomEntity.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/store/db/model/CryptoRoomEntity.kt index 4c19b5eb0e..e226f3eaa8 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/store/db/model/CryptoRoomEntity.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/store/db/model/CryptoRoomEntity.kt @@ -23,7 +23,12 @@ internal open class CryptoRoomEntity( @PrimaryKey var roomId: String? = null, var algorithm: String? = null, var shouldEncryptForInvitedMembers: Boolean? = null, - var blacklistUnverifiedDevices: Boolean = false) + var blacklistUnverifiedDevices: Boolean = false, + // Store the current outbound session for this room, + // to avoid re-create and re-share at each startup (if rotation not needed..) + // This is specific to megolm but not sure how to model it better + var outboundSessionInfo: OutboundGroupSessionInfoEntity? = null + ) : RealmObject() { companion object diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/store/db/model/OutboundGroupSessionInfoEntity.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/store/db/model/OutboundGroupSessionInfoEntity.kt new file mode 100644 index 0000000000..d50db78415 --- /dev/null +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/store/db/model/OutboundGroupSessionInfoEntity.kt @@ -0,0 +1,44 @@ +/* + * 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.store.db.model + +import io.realm.RealmObject +import org.matrix.android.sdk.internal.crypto.store.db.deserializeFromRealm +import org.matrix.android.sdk.internal.crypto.store.db.serializeForRealm +import org.matrix.olm.OlmOutboundGroupSession +import timber.log.Timber + +internal open class OutboundGroupSessionInfoEntity( + var serializedOutboundSessionData: String? = null, + var creationTime: Long? = null +) : RealmObject() { + + fun getOutboundGroupSession(): OlmOutboundGroupSession? { + return try { + deserializeFromRealm(serializedOutboundSessionData) + } catch (failure: Throwable) { + Timber.e(failure, "## getOutboundGroupSession() Deserialization failure") + return null + } + } + + fun putOutboundGroupSession(olmOutboundGroupSession: OlmOutboundGroupSession?) { + serializedOutboundSessionData = serializeForRealm(olmOutboundGroupSession) + } + + companion object +} diff --git a/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/action/MessageActionsViewModel.kt b/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/action/MessageActionsViewModel.kt index 716fdca2ad..2eae58c6b9 100644 --- a/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/action/MessageActionsViewModel.kt +++ b/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/action/MessageActionsViewModel.kt @@ -305,7 +305,8 @@ class MessageActionsViewModel @AssistedInject constructor(@Assisted ) { add(EventSharedAction.UseKeyBackup) } - if (session.cryptoService().getCryptoDeviceInfo(session.myUserId).size > 1) { + if (session.cryptoService().getCryptoDeviceInfo(session.myUserId).size > 1 + || timelineEvent.senderInfo.userId != session.myUserId) { add(EventSharedAction.ReRequestKey(timelineEvent.eventId)) } } From afa1cf7d6c46c944672dda05f58143940ef97b71 Mon Sep 17 00:00:00 2001 From: Valere Date: Thu, 7 Jan 2021 11:18:05 +0100 Subject: [PATCH 2/4] Cleaning --- .../internal/crypto/store/db/RealmCryptoStoreMigration.kt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/store/db/RealmCryptoStoreMigration.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/store/db/RealmCryptoStoreMigration.kt index 257e70936c..bca7914388 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/store/db/RealmCryptoStoreMigration.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/store/db/RealmCryptoStoreMigration.kt @@ -490,9 +490,9 @@ internal class RealmCryptoStoreMigration @Inject constructor(private val crossSi private fun migrateTo12(realm: DynamicRealm) { Timber.d("Step 11 -> 12") val outboundEntitySchema = realm.schema.create("OutboundGroupSessionInfoEntity") - ?.addField(OutboundGroupSessionInfoEntityFields.SERIALIZED_OUTBOUND_SESSION_DATA, String::class.java) - ?.addField(OutboundGroupSessionInfoEntityFields.CREATION_TIME, Long::class.java) - ?.setNullable(OutboundGroupSessionInfoEntityFields.CREATION_TIME, true) + .addField(OutboundGroupSessionInfoEntityFields.SERIALIZED_OUTBOUND_SESSION_DATA, String::class.java) + .addField(OutboundGroupSessionInfoEntityFields.CREATION_TIME, Long::class.java) + .setNullable(OutboundGroupSessionInfoEntityFields.CREATION_TIME, true) realm.schema.get("CryptoRoomEntity") ?.addRealmObjectField(CryptoRoomEntityFields.OUTBOUND_SESSION_INFO.`$`, outboundEntitySchema) From 7eb9941f8c780ca3b0a0ffd2604b9c15af74d972 Mon Sep 17 00:00:00 2001 From: Valere Date: Tue, 12 Jan 2021 09:14:30 +0100 Subject: [PATCH 3/4] Code review --- .../sdk/internal/crypto/MXOlmDevice.kt | 44 +++++++++---------- .../megolm/MXOutboundSessionInfo.kt | 1 + .../crypto/store/db/RealmCryptoStore.kt | 22 +++++----- 3 files changed, 33 insertions(+), 34 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 a892d3850a..71c1cfc728 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 @@ -66,10 +66,9 @@ internal class MXOlmDevice @Inject constructor( private var olmUtility: OlmUtility? = null // The outbound group session. - // They are not stored in 'store' to avoid to remember to which devices we sent the session key. - // Plus, in cryptography, it is good to refresh sessions from time to time. + // Caches active outbound session to avoid to sync with DB before read // The key is the session id, the value the outbound group session. - private val outboundGroupSessionStore: MutableMap = HashMap() + private val outboundGroupSessionCache: MutableMap = HashMap() // Store a set of decrypted message indexes for each group session. // This partially mitigates a replay attack where a MITM resends a group @@ -137,10 +136,10 @@ internal class MXOlmDevice @Inject constructor( */ fun release() { olmUtility?.releaseUtility() - outboundGroupSessionStore.values.forEach { + outboundGroupSessionCache.values.forEach { it.releaseSession() } - outboundGroupSessionStore.clear() + outboundGroupSessionCache.clear() } /** @@ -416,7 +415,7 @@ internal class MXOlmDevice @Inject constructor( var session: OlmOutboundGroupSession? = null try { session = OlmOutboundGroupSession() - outboundGroupSessionStore[session.sessionIdentifier()] = session + outboundGroupSessionCache[session.sessionIdentifier()] = session store.storeCurrentOutboundGroupSessionForRoom(roomId, session) return session.sessionIdentifier() } catch (e: Exception) { @@ -429,33 +428,34 @@ internal class MXOlmDevice @Inject constructor( } fun storeOutboundGroupSessionForRoom(roomId: String, sessionId: String) { - outboundGroupSessionStore[sessionId]?.let { + outboundGroupSessionCache[sessionId]?.let { store.storeCurrentOutboundGroupSessionForRoom(roomId, it) } } - fun restoreOutboundGroupSessionForRoom(roomId: String): MXOutboundSessionInfo? { + fun restoreOutboundGroupSessionForRoom(roomId: String): MXOutboundSessionInfo? { val restoredOutboundGroupSession = store.getCurrentOutboundGroupSessionForRoom(roomId) if (restoredOutboundGroupSession != null) { val sessionId = restoredOutboundGroupSession.outboundGroupSession.sessionIdentifier() - if (!outboundGroupSessionStore.containsKey(sessionId)) { - outboundGroupSessionStore[sessionId] = restoredOutboundGroupSession.outboundGroupSession - return MXOutboundSessionInfo( - sessionId = sessionId, - sharedWithHelper = SharedWithHelper(roomId, sessionId, store), - restoredOutboundGroupSession.creationTime - ) - } else { - restoredOutboundGroupSession.outboundGroupSession.releaseSession() - } + // cache it + outboundGroupSessionCache[sessionId] = restoredOutboundGroupSession.outboundGroupSession + + return MXOutboundSessionInfo( + sessionId = sessionId, + sharedWithHelper = SharedWithHelper(roomId, sessionId, store), + restoredOutboundGroupSession.creationTime + ) } return null } fun discardOutboundGroupSessionForRoom(roomId: String) { - outboundGroupSessionStore.remove(roomId)?.releaseSession() + store.getCurrentOutboundGroupSessionForRoom(roomId)?.outboundGroupSession?.sessionIdentifier()?.let { sessionId -> + outboundGroupSessionCache.remove(sessionId)?.releaseSession() + } store.storeCurrentOutboundGroupSessionForRoom(roomId, null) } + /** * Get the current session key of an outbound group session. * @@ -465,7 +465,7 @@ internal class MXOlmDevice @Inject constructor( fun getSessionKey(sessionId: String): String? { if (sessionId.isNotEmpty()) { try { - return outboundGroupSessionStore[sessionId]!!.sessionKey() + return outboundGroupSessionCache[sessionId]!!.sessionKey() } catch (e: Exception) { Timber.e(e, "## getSessionKey() : failed") } @@ -481,7 +481,7 @@ internal class MXOlmDevice @Inject constructor( */ fun getMessageIndex(sessionId: String): Int { return if (sessionId.isNotEmpty()) { - outboundGroupSessionStore[sessionId]!!.messageIndex() + outboundGroupSessionCache[sessionId]!!.messageIndex() } else 0 } @@ -495,7 +495,7 @@ internal class MXOlmDevice @Inject constructor( fun encryptGroupMessage(sessionId: String, payloadString: String): String? { if (sessionId.isNotEmpty() && payloadString.isNotEmpty()) { try { - return outboundGroupSessionStore[sessionId]!!.encryptMessage(payloadString) + return outboundGroupSessionCache[sessionId]!!.encryptMessage(payloadString) } catch (e: Exception) { Timber.e(e, "## encryptGroupMessage() : failed") } diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/algorithms/megolm/MXOutboundSessionInfo.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/algorithms/megolm/MXOutboundSessionInfo.kt index c0b920b09d..5a68937868 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/algorithms/megolm/MXOutboundSessionInfo.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/algorithms/megolm/MXOutboundSessionInfo.kt @@ -24,6 +24,7 @@ internal class MXOutboundSessionInfo( // The id of the session val sessionId: String, val sharedWithHelper: SharedWithHelper, + // When the session was created private val creationTime: Long = System.currentTimeMillis()) { // Number of times this session has been used 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 7912d03ffd..369a4976c9 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 @@ -780,20 +780,18 @@ internal class RealmCryptoStore @Inject constructor( // this is called for each sent message (so not high frequency), thus we can use basic realm async without // risk of reaching max async operation limit? doRealmTransactionAsync(realmConfiguration) { realm -> - realm.where() - .equalTo(CryptoRoomEntityFields.ROOM_ID, roomId) - .findFirst()?.let { entity -> - // we should delete existing outbound session info if any - entity.outboundSessionInfo?.deleteFromRealm() + CryptoRoomEntity.getById(realm, roomId)?.let { entity -> + // we should delete existing outbound session info if any + entity.outboundSessionInfo?.deleteFromRealm() - if (outboundGroupSession != null) { - val info = realm.createObject(OutboundGroupSessionInfoEntity::class.java).apply { - creationTime = System.currentTimeMillis() - putOutboundGroupSession(outboundGroupSession) - } - entity.outboundSessionInfo = info - } + if (outboundGroupSession != null) { + val info = realm.createObject(OutboundGroupSessionInfoEntity::class.java).apply { + creationTime = System.currentTimeMillis() + putOutboundGroupSession(outboundGroupSession) } + entity.outboundSessionInfo = info + } + } } } From 426782a001f70c0a32038cde1f270e9946194000 Mon Sep 17 00:00:00 2001 From: Valere Date: Tue, 12 Jan 2021 09:48:02 +0100 Subject: [PATCH 4/4] remember groupId in session cache --- .../sdk/internal/crypto/MXOlmDevice.kt | 31 ++++++++++++------- 1 file changed, 20 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 71c1cfc728..b1e91e8d50 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 @@ -65,10 +65,15 @@ internal class MXOlmDevice @Inject constructor( // The OLM lib utility instance. private var olmUtility: OlmUtility? = null + private data class GroupSessionCacheItem( + val groupId: String, + val groupSession: OlmOutboundGroupSession + ) + // The outbound group session. // Caches active outbound session to avoid to sync with DB before read - // The key is the session id, the value the outbound group session. - private val outboundGroupSessionCache: MutableMap = HashMap() + // The key is the session id, the value the . + private val outboundGroupSessionCache: MutableMap = HashMap() // Store a set of decrypted message indexes for each group session. // This partially mitigates a replay attack where a MITM resends a group @@ -137,7 +142,7 @@ internal class MXOlmDevice @Inject constructor( fun release() { olmUtility?.releaseUtility() outboundGroupSessionCache.values.forEach { - it.releaseSession() + it.groupSession.releaseSession() } outboundGroupSessionCache.clear() } @@ -415,7 +420,7 @@ internal class MXOlmDevice @Inject constructor( var session: OlmOutboundGroupSession? = null try { session = OlmOutboundGroupSession() - outboundGroupSessionCache[session.sessionIdentifier()] = session + outboundGroupSessionCache[session.sessionIdentifier()] = GroupSessionCacheItem(roomId, session) store.storeCurrentOutboundGroupSessionForRoom(roomId, session) return session.sessionIdentifier() } catch (e: Exception) { @@ -429,7 +434,7 @@ internal class MXOlmDevice @Inject constructor( fun storeOutboundGroupSessionForRoom(roomId: String, sessionId: String) { outboundGroupSessionCache[sessionId]?.let { - store.storeCurrentOutboundGroupSessionForRoom(roomId, it) + store.storeCurrentOutboundGroupSessionForRoom(roomId, it.groupSession) } } @@ -438,7 +443,7 @@ internal class MXOlmDevice @Inject constructor( if (restoredOutboundGroupSession != null) { val sessionId = restoredOutboundGroupSession.outboundGroupSession.sessionIdentifier() // cache it - outboundGroupSessionCache[sessionId] = restoredOutboundGroupSession.outboundGroupSession + outboundGroupSessionCache[sessionId] = GroupSessionCacheItem(roomId, restoredOutboundGroupSession.outboundGroupSession) return MXOutboundSessionInfo( sessionId = sessionId, @@ -450,8 +455,12 @@ internal class MXOlmDevice @Inject constructor( } fun discardOutboundGroupSessionForRoom(roomId: String) { - store.getCurrentOutboundGroupSessionForRoom(roomId)?.outboundGroupSession?.sessionIdentifier()?.let { sessionId -> - outboundGroupSessionCache.remove(sessionId)?.releaseSession() + val toDiscard = outboundGroupSessionCache.filter { + it.value.groupId == roomId + } + toDiscard.forEach { (sessionId, cacheItem) -> + cacheItem.groupSession.releaseSession() + outboundGroupSessionCache.remove(sessionId) } store.storeCurrentOutboundGroupSessionForRoom(roomId, null) } @@ -465,7 +474,7 @@ internal class MXOlmDevice @Inject constructor( fun getSessionKey(sessionId: String): String? { if (sessionId.isNotEmpty()) { try { - return outboundGroupSessionCache[sessionId]!!.sessionKey() + return outboundGroupSessionCache[sessionId]!!.groupSession.sessionKey() } catch (e: Exception) { Timber.e(e, "## getSessionKey() : failed") } @@ -481,7 +490,7 @@ internal class MXOlmDevice @Inject constructor( */ fun getMessageIndex(sessionId: String): Int { return if (sessionId.isNotEmpty()) { - outboundGroupSessionCache[sessionId]!!.messageIndex() + outboundGroupSessionCache[sessionId]!!.groupSession.messageIndex() } else 0 } @@ -495,7 +504,7 @@ internal class MXOlmDevice @Inject constructor( fun encryptGroupMessage(sessionId: String, payloadString: String): String? { if (sessionId.isNotEmpty() && payloadString.isNotEmpty()) { try { - return outboundGroupSessionCache[sessionId]!!.encryptMessage(payloadString) + return outboundGroupSessionCache[sessionId]!!.groupSession.encryptMessage(payloadString) } catch (e: Exception) { Timber.e(e, "## encryptGroupMessage() : failed") }