Merge pull request #5136 from vector-im/feature/bca/crypto_ensure_e2e_defensive

Defensive coding to ensure encryption when room was once e2e

changelog.d/5136.misc

@@ -0,0 +1 @@
+Defensive coding to ensure encryption when room was once e2e

matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/CryptoSessionInfoProvider.kt

@@ -35,6 +35,8 @@ internal class CryptoSessionInfoProvider @Inject constructor(
 ) {
 
     fun isRoomEncrypted(roomId: String): Boolean {
+        // We look at the presence at any m.room.encryption state event no matter if it's
+        // the latest one or if it is well formed
         val encryptionEvent = monarchy.fetchCopied { realm ->
             EventEntity.whereType(realm, roomId = roomId, type = EventType.STATE_ROOM_ENCRYPTION)
                     .isEmpty(EventEntityFields.STATE_KEY)

matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/store/IMXCryptoStore.kt

@@ -240,6 +240,14 @@ internal interface IMXCryptoStore {
      */
     fun getRoomAlgorithm(roomId: String): String?
 
+    /**
+     * This is a bit different than isRoomEncrypted
+     * A room is encrypted when there is a m.room.encryption state event in the room (malformed/invalid or not)
+     * But the crypto layer has additional guaranty to ensure that encryption would never been reverted
+     * It's defensive coding out of precaution (if ever state is reset)
+     */
+    fun roomWasOnceEncrypted(roomId: String): Boolean
+
     fun shouldEncryptForInvitedMembers(roomId: String): Boolean
 
     fun setShouldEncryptForInvitedMembers(roomId: String, shouldEncryptForInvitedMembers: Boolean)

matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/store/db/RealmCryptoStore.kt

@@ -631,7 +631,15 @@ internal class RealmCryptoStore @Inject constructor(
 
     override fun storeRoomAlgorithm(roomId: String, algorithm: String?) {
         doRealmTransaction(realmConfiguration) {
-            CryptoRoomEntity.getOrCreate(it, roomId).algorithm = algorithm
+            CryptoRoomEntity.getOrCreate(it, roomId).let { entity ->
+                entity.algorithm = algorithm
+                // store anyway the new algorithm, but mark the room
+                // as having been encrypted once whatever, this can never
+                // go back to false
+                if (algorithm == MXCRYPTO_ALGORITHM_MEGOLM) {
+                    entity.wasEncryptedOnce = true
+                }
+            }
         }
     }
 
@@ -641,6 +649,12 @@ internal class RealmCryptoStore @Inject constructor(
         }
     }
 
+    override fun roomWasOnceEncrypted(roomId: String): Boolean {
+        return doWithRealm(realmConfiguration) {
+            CryptoRoomEntity.getById(it, roomId)?.wasEncryptedOnce ?: false
+        }
+    }
+
     override fun shouldEncryptForInvitedMembers(roomId: String): Boolean {
         return doWithRealm(realmConfiguration) {
             CryptoRoomEntity.getById(it, roomId)?.shouldEncryptForInvitedMembers

matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/store/db/RealmCryptoStoreMigration.kt

@@ -32,6 +32,7 @@ import org.matrix.android.sdk.internal.crypto.store.db.migration.MigrateCryptoTo
 import org.matrix.android.sdk.internal.crypto.store.db.migration.MigrateCryptoTo012
 import org.matrix.android.sdk.internal.crypto.store.db.migration.MigrateCryptoTo013
 import org.matrix.android.sdk.internal.crypto.store.db.migration.MigrateCryptoTo014
+import org.matrix.android.sdk.internal.crypto.store.db.migration.MigrateCryptoTo015
 import timber.log.Timber
 import javax.inject.Inject
 
@@ -46,7 +47,7 @@ internal class RealmCryptoStoreMigration @Inject constructor() : RealmMigration
     // 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)
-    val schemaVersion = 14L
+    val schemaVersion = 15L
 
     override fun migrate(realm: DynamicRealm, oldVersion: Long, newVersion: Long) {
         Timber.d("Migrating Realm Crypto from $oldVersion to $newVersion")
@@ -65,5 +66,6 @@ internal class RealmCryptoStoreMigration @Inject constructor() : RealmMigration
         if (oldVersion < 12) MigrateCryptoTo012(realm).perform()
         if (oldVersion < 13) MigrateCryptoTo013(realm).perform()
         if (oldVersion < 14) MigrateCryptoTo014(realm).perform()
+        if (oldVersion < 15) MigrateCryptoTo015(realm).perform()
     }
 }

matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/store/db/migration/MigrateCryptoTo015.kt

@@ -0,0 +1,36 @@
+/*
+ * Copyright (c) 2022 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.migration
+
+import io.realm.DynamicRealm
+import org.matrix.android.sdk.internal.crypto.MXCRYPTO_ALGORITHM_MEGOLM
+import org.matrix.android.sdk.internal.crypto.store.db.model.CryptoRoomEntityFields
+import org.matrix.android.sdk.internal.util.database.RealmMigrator
+
+// Version 14L Update the way we remember key sharing
+class MigrateCryptoTo015(realm: DynamicRealm) : RealmMigrator(realm, 15) {
+
+    override fun doMigrate(realm: DynamicRealm) {
+        realm.schema.get("CryptoRoomEntity")
+                ?.addField(CryptoRoomEntityFields.WAS_ENCRYPTED_ONCE, Boolean::class.java)
+                ?.setNullable(CryptoRoomEntityFields.WAS_ENCRYPTED_ONCE, true)
+                ?.transform {
+                    val currentAlgorithm = it.getString(CryptoRoomEntityFields.ALGORITHM)
+                    it.set(CryptoRoomEntityFields.WAS_ENCRYPTED_ONCE, currentAlgorithm == MXCRYPTO_ALGORITHM_MEGOLM)
+                }
+    }
+}

matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/store/db/model/CryptoRoomEntity.kt

@@ -27,7 +27,10 @@ internal open class CryptoRoomEntity(
         // 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
+        var outboundSessionInfo: OutboundGroupSessionInfoEntity? = null,
+        // a security to ensure that a room will never revert to not encrypted
+        // even if a new state event with empty encryption, or state is reset somehow
+        var wasEncryptedOnce: Boolean? = false
         ) :
     RealmObject() {
 

matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/relation/DefaultRelationService.kt

@@ -30,7 +30,6 @@ import org.matrix.android.sdk.api.util.Cancelable
 import org.matrix.android.sdk.api.util.NoOpCancellable
 import org.matrix.android.sdk.api.util.Optional
 import org.matrix.android.sdk.api.util.toOptional
-import org.matrix.android.sdk.internal.crypto.CryptoSessionInfoProvider
 import org.matrix.android.sdk.internal.database.mapper.TimelineEventMapper
 import org.matrix.android.sdk.internal.database.mapper.asDomain
 import org.matrix.android.sdk.internal.database.model.EventAnnotationsSummaryEntity
@@ -48,7 +47,6 @@ internal class DefaultRelationService @AssistedInject constructor(
         private val eventEditor: EventEditor,
         private val eventSenderProcessor: EventSenderProcessor,
         private val eventFactory: LocalEchoEventFactory,
-        private val cryptoSessionInfoProvider: CryptoSessionInfoProvider,
         private val findReactionEventForUndoTask: FindReactionEventForUndoTask,
         private val fetchEditHistoryTask: FetchEditHistoryTask,
         private val fetchThreadTimelineTask: FetchThreadTimelineTask,
@@ -146,7 +144,7 @@ internal class DefaultRelationService @AssistedInject constructor(
                 ?.also { saveLocalEcho(it) }
                 ?: return null
 
-        return eventSenderProcessor.postEvent(event, cryptoSessionInfoProvider.isRoomEncrypted(roomId))
+        return eventSenderProcessor.postEvent(event)
     }
 
     override fun getEventAnnotationsSummary(eventId: String): EventAnnotationsSummary? {
@@ -202,7 +200,7 @@ internal class DefaultRelationService @AssistedInject constructor(
                         saveLocalEcho(it)
                     }
         }
-        return eventSenderProcessor.postEvent(event, cryptoSessionInfoProvider.isRoomEncrypted(roomId))
+        return eventSenderProcessor.postEvent(event)
     }
 
     override suspend fun fetchThreadTimeline(rootThreadEventId: String): Boolean {

matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/relation/EventEditor.kt

@@ -23,7 +23,6 @@ import org.matrix.android.sdk.api.session.room.send.SendState
 import org.matrix.android.sdk.api.session.room.timeline.TimelineEvent
 import org.matrix.android.sdk.api.util.Cancelable
 import org.matrix.android.sdk.api.util.NoOpCancellable
-import org.matrix.android.sdk.internal.crypto.CryptoSessionInfoProvider
 import org.matrix.android.sdk.internal.database.mapper.toEntity
 import org.matrix.android.sdk.internal.session.room.send.LocalEchoEventFactory
 import org.matrix.android.sdk.internal.session.room.send.LocalEchoRepository
@@ -33,7 +32,6 @@ import javax.inject.Inject
 
 internal class EventEditor @Inject constructor(private val eventSenderProcessor: EventSenderProcessor,
                                                private val eventFactory: LocalEchoEventFactory,
-                                               private val cryptoSessionInfoProvider: CryptoSessionInfoProvider,
                                                private val localEchoRepository: LocalEchoRepository) {
 
     fun editTextMessage(targetEvent: TimelineEvent,
@@ -51,7 +49,7 @@ internal class EventEditor @Inject constructor(private val eventSenderProcessor:
         } else if (targetEvent.root.sendState.isSent()) {
             val event = eventFactory
                     .createReplaceTextEvent(roomId, targetEvent.eventId, newBodyText, newBodyAutoMarkdown, msgType, compatibilityBodyText)
-            return sendReplaceEvent(roomId, event)
+            return sendReplaceEvent(event)
         } else {
             // Should we throw?
             Timber.w("Can't edit a sending event")
@@ -72,7 +70,7 @@ internal class EventEditor @Inject constructor(private val eventSenderProcessor:
         } else if (targetEvent.root.sendState.isSent()) {
             val event = eventFactory
                     .createPollReplaceEvent(roomId, pollType, targetEvent.eventId, question, options)
-            return sendReplaceEvent(roomId, event)
+            return sendReplaceEvent(event)
         } else {
             Timber.w("Can't edit a sending event")
             return NoOpCancellable
@@ -82,12 +80,12 @@ internal class EventEditor @Inject constructor(private val eventSenderProcessor:
     private fun sendFailedEvent(targetEvent: TimelineEvent, editedEvent: Event): Cancelable {
         val roomId = targetEvent.roomId
         updateFailedEchoWithEvent(roomId, targetEvent.eventId, editedEvent)
-        return eventSenderProcessor.postEvent(editedEvent, cryptoSessionInfoProvider.isRoomEncrypted(roomId))
+        return eventSenderProcessor.postEvent(editedEvent)
     }
 
-    private fun sendReplaceEvent(roomId: String, editedEvent: Event): Cancelable {
+    private fun sendReplaceEvent(editedEvent: Event): Cancelable {
         localEchoRepository.createLocalEcho(editedEvent)
-        return eventSenderProcessor.postEvent(editedEvent, cryptoSessionInfoProvider.isRoomEncrypted(roomId))
+        return eventSenderProcessor.postEvent(editedEvent)
     }
 
     fun editReply(replyToEdit: TimelineEvent,
@@ -107,7 +105,7 @@ internal class EventEditor @Inject constructor(private val eventSenderProcessor:
                     eventId = replyToEdit.eventId
             ) ?: return NoOpCancellable
             updateFailedEchoWithEvent(roomId, replyToEdit.eventId, editedEvent)
-            return eventSenderProcessor.postEvent(editedEvent, cryptoSessionInfoProvider.isRoomEncrypted(roomId))
+            return eventSenderProcessor.postEvent(editedEvent)
         } else if (replyToEdit.root.sendState.isSent()) {
             val event = eventFactory.createReplaceTextOfReply(
                     roomId,
@@ -119,7 +117,7 @@ internal class EventEditor @Inject constructor(private val eventSenderProcessor:
                     compatibilityBodyText
             )
                     .also { localEchoRepository.createLocalEcho(it) }
-            return eventSenderProcessor.postEvent(event, cryptoSessionInfoProvider.isRoomEncrypted(roomId))
+            return eventSenderProcessor.postEvent(event)
         } else {
             // Should we throw?
             Timber.w("Can't edit a sending event")

matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/send/DefaultSendService.kt

@@ -46,7 +46,7 @@ import org.matrix.android.sdk.api.util.Cancelable
 import org.matrix.android.sdk.api.util.CancelableBag
 import org.matrix.android.sdk.api.util.JsonDict
 import org.matrix.android.sdk.api.util.NoOpCancellable
-import org.matrix.android.sdk.internal.crypto.CryptoSessionInfoProvider
+import org.matrix.android.sdk.internal.crypto.store.IMXCryptoStore
 import org.matrix.android.sdk.internal.di.SessionId
 import org.matrix.android.sdk.internal.di.WorkManagerProvider
 import org.matrix.android.sdk.internal.session.content.UploadContentWorker
@@ -66,7 +66,7 @@ internal class DefaultSendService @AssistedInject constructor(
         private val workManagerProvider: WorkManagerProvider,
         @SessionId private val sessionId: String,
         private val localEchoEventFactory: LocalEchoEventFactory,
-        private val cryptoSessionInfoProvider: CryptoSessionInfoProvider,
+        private val cryptoStore: IMXCryptoStore,
         private val taskExecutor: TaskExecutor,
         private val localEchoRepository: LocalEchoRepository,
         private val eventSenderProcessor: EventSenderProcessor,
@@ -303,7 +303,7 @@ internal class DefaultSendService @AssistedInject constructor(
     private fun internalSendMedia(allLocalEchoes: List<Event>, attachment: ContentAttachmentData, compressBeforeSending: Boolean): Cancelable {
         val cancelableBag = CancelableBag()
 
-        allLocalEchoes.groupBy { cryptoSessionInfoProvider.isRoomEncrypted(it.roomId!!) }
+        allLocalEchoes.groupBy { cryptoStore.roomWasOnceEncrypted(it.roomId!!) }
                 .apply {
                     keys.forEach { isRoomEncrypted ->
                         // Should never be empty
@@ -334,7 +334,7 @@ internal class DefaultSendService @AssistedInject constructor(
     }
 
     private fun sendEvent(event: Event): Cancelable {
-        return eventSenderProcessor.postEvent(event, cryptoSessionInfoProvider.isRoomEncrypted(event.roomId!!))
+        return eventSenderProcessor.postEvent(event)
     }
 
     private fun createLocalEcho(event: Event) {

matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/send/queue/EventSenderProcessorCoroutine.kt

@@ -26,9 +26,9 @@ import org.matrix.android.sdk.api.failure.Failure
 import org.matrix.android.sdk.api.failure.getRetryDelay
 import org.matrix.android.sdk.api.failure.isLimitExceededError
 import org.matrix.android.sdk.api.session.Session
-import org.matrix.android.sdk.api.session.crypto.CryptoService
 import org.matrix.android.sdk.api.session.events.model.Event
 import org.matrix.android.sdk.api.util.Cancelable
+import org.matrix.android.sdk.internal.crypto.store.IMXCryptoStore
 import org.matrix.android.sdk.internal.session.SessionScope
 import org.matrix.android.sdk.internal.task.CoroutineSequencer
 import org.matrix.android.sdk.internal.task.SemaphoreCoroutineSequencer
@@ -54,7 +54,7 @@ private const val MAX_RETRY_COUNT = 3
  */
 @SessionScope
 internal class EventSenderProcessorCoroutine @Inject constructor(
-        private val cryptoService: CryptoService,
+        private val cryptoStore: IMXCryptoStore,
         private val sessionParams: SessionParams,
         private val queuedTaskFactory: QueuedTaskFactory,
         private val taskExecutor: TaskExecutor,
@@ -92,7 +92,8 @@ internal class EventSenderProcessorCoroutine @Inject constructor(
     }
 
     override fun postEvent(event: Event): Cancelable {
-        return postEvent(event, event.roomId?.let { cryptoService.isRoomEncrypted(it) } ?: false)
+        val shouldEncrypt = event.roomId?.let { cryptoStore.roomWasOnceEncrypted(it) } ?: false
+        return postEvent(event, shouldEncrypt)
     }
 
     override fun postEvent(event: Event, encrypt: Boolean): Cancelable {

matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/summary/RoomSummaryUpdater.kt

@@ -119,9 +119,8 @@ internal class RoomSummaryUpdater @Inject constructor(
         roomSummaryEntity.roomType = roomType
         Timber.v("## Space: Updating summary room [$roomId] roomType: [$roomType]")
 
-        // Don't use current state for this one as we are only interested in having MXCRYPTO_ALGORITHM_MEGOLM event in the room
         val encryptionEvent = CurrentStateEventEntity.getOrNull(realm, roomId, type = EventType.STATE_ROOM_ENCRYPTION, stateKey = "")?.root
-        Timber.v("## CRYPTO: currentEncryptionEvent is $encryptionEvent")
+        Timber.d("## CRYPTO: currentEncryptionEvent is $encryptionEvent")
 
         val latestPreviewableEvent = RoomSummaryEventsHelper.getLatestPreviewableEvent(realm, roomId)