Code review

This commit is contained in:
Valere 2021-01-12 09:14:30 +01:00
parent afa1cf7d6c
commit 7eb9941f8c
3 changed files with 33 additions and 34 deletions

View file

@ -66,10 +66,9 @@ internal class MXOlmDevice @Inject constructor(
private var olmUtility: OlmUtility? = null private var olmUtility: OlmUtility? = null
// The outbound group session. // The outbound group session.
// They are not stored in 'store' to avoid to remember to which devices we sent the session key. // Caches active outbound session to avoid to sync with DB before read
// Plus, in cryptography, it is good to refresh sessions from time to time.
// The key is the session id, the value the outbound group session. // The key is the session id, the value the outbound group session.
private val outboundGroupSessionStore: MutableMap<String, OlmOutboundGroupSession> = HashMap() private val outboundGroupSessionCache: MutableMap<String, OlmOutboundGroupSession> = HashMap()
// Store a set of decrypted message indexes for each group session. // Store a set of decrypted message indexes for each group session.
// This partially mitigates a replay attack where a MITM resends a group // This partially mitigates a replay attack where a MITM resends a group
@ -137,10 +136,10 @@ internal class MXOlmDevice @Inject constructor(
*/ */
fun release() { fun release() {
olmUtility?.releaseUtility() olmUtility?.releaseUtility()
outboundGroupSessionStore.values.forEach { outboundGroupSessionCache.values.forEach {
it.releaseSession() it.releaseSession()
} }
outboundGroupSessionStore.clear() outboundGroupSessionCache.clear()
} }
/** /**
@ -416,7 +415,7 @@ internal class MXOlmDevice @Inject constructor(
var session: OlmOutboundGroupSession? = null var session: OlmOutboundGroupSession? = null
try { try {
session = OlmOutboundGroupSession() session = OlmOutboundGroupSession()
outboundGroupSessionStore[session.sessionIdentifier()] = session outboundGroupSessionCache[session.sessionIdentifier()] = session
store.storeCurrentOutboundGroupSessionForRoom(roomId, session) store.storeCurrentOutboundGroupSessionForRoom(roomId, session)
return session.sessionIdentifier() return session.sessionIdentifier()
} catch (e: Exception) { } catch (e: Exception) {
@ -429,7 +428,7 @@ internal class MXOlmDevice @Inject constructor(
} }
fun storeOutboundGroupSessionForRoom(roomId: String, sessionId: String) { fun storeOutboundGroupSessionForRoom(roomId: String, sessionId: String) {
outboundGroupSessionStore[sessionId]?.let { outboundGroupSessionCache[sessionId]?.let {
store.storeCurrentOutboundGroupSessionForRoom(roomId, it) store.storeCurrentOutboundGroupSessionForRoom(roomId, it)
} }
} }
@ -438,24 +437,25 @@ internal class MXOlmDevice @Inject constructor(
val restoredOutboundGroupSession = store.getCurrentOutboundGroupSessionForRoom(roomId) val restoredOutboundGroupSession = store.getCurrentOutboundGroupSessionForRoom(roomId)
if (restoredOutboundGroupSession != null) { if (restoredOutboundGroupSession != null) {
val sessionId = restoredOutboundGroupSession.outboundGroupSession.sessionIdentifier() val sessionId = restoredOutboundGroupSession.outboundGroupSession.sessionIdentifier()
if (!outboundGroupSessionStore.containsKey(sessionId)) { // cache it
outboundGroupSessionStore[sessionId] = restoredOutboundGroupSession.outboundGroupSession outboundGroupSessionCache[sessionId] = restoredOutboundGroupSession.outboundGroupSession
return MXOutboundSessionInfo( return MXOutboundSessionInfo(
sessionId = sessionId, sessionId = sessionId,
sharedWithHelper = SharedWithHelper(roomId, sessionId, store), sharedWithHelper = SharedWithHelper(roomId, sessionId, store),
restoredOutboundGroupSession.creationTime restoredOutboundGroupSession.creationTime
) )
} else {
restoredOutboundGroupSession.outboundGroupSession.releaseSession()
}
} }
return null return null
} }
fun discardOutboundGroupSessionForRoom(roomId: String) { fun discardOutboundGroupSessionForRoom(roomId: String) {
outboundGroupSessionStore.remove(roomId)?.releaseSession() store.getCurrentOutboundGroupSessionForRoom(roomId)?.outboundGroupSession?.sessionIdentifier()?.let { sessionId ->
outboundGroupSessionCache.remove(sessionId)?.releaseSession()
}
store.storeCurrentOutboundGroupSessionForRoom(roomId, null) store.storeCurrentOutboundGroupSessionForRoom(roomId, null)
} }
/** /**
* Get the current session key of an outbound group session. * Get the current session key of an outbound group session.
* *
@ -465,7 +465,7 @@ internal class MXOlmDevice @Inject constructor(
fun getSessionKey(sessionId: String): String? { fun getSessionKey(sessionId: String): String? {
if (sessionId.isNotEmpty()) { if (sessionId.isNotEmpty()) {
try { try {
return outboundGroupSessionStore[sessionId]!!.sessionKey() return outboundGroupSessionCache[sessionId]!!.sessionKey()
} catch (e: Exception) { } catch (e: Exception) {
Timber.e(e, "## getSessionKey() : failed") Timber.e(e, "## getSessionKey() : failed")
} }
@ -481,7 +481,7 @@ internal class MXOlmDevice @Inject constructor(
*/ */
fun getMessageIndex(sessionId: String): Int { fun getMessageIndex(sessionId: String): Int {
return if (sessionId.isNotEmpty()) { return if (sessionId.isNotEmpty()) {
outboundGroupSessionStore[sessionId]!!.messageIndex() outboundGroupSessionCache[sessionId]!!.messageIndex()
} else 0 } else 0
} }
@ -495,7 +495,7 @@ internal class MXOlmDevice @Inject constructor(
fun encryptGroupMessage(sessionId: String, payloadString: String): String? { fun encryptGroupMessage(sessionId: String, payloadString: String): String? {
if (sessionId.isNotEmpty() && payloadString.isNotEmpty()) { if (sessionId.isNotEmpty() && payloadString.isNotEmpty()) {
try { try {
return outboundGroupSessionStore[sessionId]!!.encryptMessage(payloadString) return outboundGroupSessionCache[sessionId]!!.encryptMessage(payloadString)
} catch (e: Exception) { } catch (e: Exception) {
Timber.e(e, "## encryptGroupMessage() : failed") Timber.e(e, "## encryptGroupMessage() : failed")
} }

View file

@ -24,6 +24,7 @@ internal class MXOutboundSessionInfo(
// The id of the session // The id of the session
val sessionId: String, val sessionId: String,
val sharedWithHelper: SharedWithHelper, val sharedWithHelper: SharedWithHelper,
// When the session was created
private val creationTime: Long = System.currentTimeMillis()) { private val creationTime: Long = System.currentTimeMillis()) {
// Number of times this session has been used // Number of times this session has been used

View file

@ -780,9 +780,7 @@ internal class RealmCryptoStore @Inject constructor(
// this is called for each sent message (so not high frequency), thus we can use basic realm async without // 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? // risk of reaching max async operation limit?
doRealmTransactionAsync(realmConfiguration) { realm -> doRealmTransactionAsync(realmConfiguration) { realm ->
realm.where<CryptoRoomEntity>() CryptoRoomEntity.getById(realm, roomId)?.let { entity ->
.equalTo(CryptoRoomEntityFields.ROOM_ID, roomId)
.findFirst()?.let { entity ->
// we should delete existing outbound session info if any // we should delete existing outbound session info if any
entity.outboundSessionInfo?.deleteFromRealm() entity.outboundSessionInfo?.deleteFromRealm()