From 7eb9941f8c780ca3b0a0ffd2604b9c15af74d972 Mon Sep 17 00:00:00 2001 From: Valere Date: Tue, 12 Jan 2021 09:14:30 +0100 Subject: [PATCH] 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 + } + } } }