From 3702ccd2bafa9458f140f6398f7642718b8622ad Mon Sep 17 00:00:00 2001 From: Valere Date: Wed, 2 Feb 2022 19:05:03 +0100 Subject: [PATCH] Defensive coding to ensure encryption when room was once e2e --- .../sdk/api/session/crypto/CryptoService.kt | 8 +++++ .../crypto/CryptoSessionInfoProvider.kt | 2 ++ .../internal/crypto/DefaultCryptoService.kt | 4 +++ .../internal/crypto/store/IMXCryptoStore.kt | 2 ++ .../crypto/store/db/RealmCryptoStore.kt | 16 ++++++++- .../store/db/RealmCryptoStoreMigration.kt | 4 ++- .../store/db/migration/MigrateCryptoTo015.kt | 36 +++++++++++++++++++ .../crypto/store/db/model/CryptoRoomEntity.kt | 5 ++- .../room/relation/DefaultRelationService.kt | 6 ++-- .../session/room/relation/EventEditor.kt | 16 ++++----- .../session/room/send/DefaultSendService.kt | 8 ++--- .../queue/EventSenderProcessorCoroutine.kt | 2 +- .../room/summary/RoomSummaryUpdater.kt | 3 +- 13 files changed, 89 insertions(+), 23 deletions(-) create mode 100644 matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/store/db/migration/MigrateCryptoTo015.kt diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/crypto/CryptoService.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/crypto/CryptoService.kt index e3f00a24b6..9b617f3e4f 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/crypto/CryptoService.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/crypto/CryptoService.kt @@ -113,6 +113,14 @@ interface CryptoService { fun isRoomEncrypted(roomId: String): Boolean + /** + * 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 shouldEncryptInRoom(roomId: String?): Boolean + fun encryptEventContent(eventContent: Content, eventType: String, roomId: String, 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 82eced4371..2a58d731e5 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 @@ -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) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/DefaultCryptoService.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/DefaultCryptoService.kt index 0646e4d2b8..f10cde6759 100755 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/DefaultCryptoService.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/DefaultCryptoService.kt @@ -634,6 +634,10 @@ internal class DefaultCryptoService @Inject constructor( return cryptoSessionInfoProvider.isRoomEncrypted(roomId) } + override fun shouldEncryptInRoom(roomId: String?): Boolean { + return roomId?.let { cryptoStore.roomWasOnceEncrypted(it) } ?: false + } + /** * @return the stored device keys for a user. */ 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 82fb565377..46f94bc3bc 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 @@ -240,6 +240,8 @@ internal interface IMXCryptoStore { */ fun getRoomAlgorithm(roomId: String): String? + fun roomWasOnceEncrypted(roomId: String): Boolean + fun shouldEncryptForInvitedMembers(roomId: String): Boolean fun setShouldEncryptForInvitedMembers(roomId: String, shouldEncryptForInvitedMembers: Boolean) 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 33578ba06a..a07827c033 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 @@ -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 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 685b2d2967..cac6499486 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 @@ -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() } } diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/store/db/migration/MigrateCryptoTo015.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/store/db/migration/MigrateCryptoTo015.kt new file mode 100644 index 0000000000..7bfea7d72f --- /dev/null +++ b/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) + } + } +} 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 711b698464..6167314b5a 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 @@ -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() { diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/relation/DefaultRelationService.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/relation/DefaultRelationService.kt index 3abf28fdd4..848e14ff57 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/relation/DefaultRelationService.kt +++ b/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 { diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/relation/EventEditor.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/relation/EventEditor.kt index 4551f390e7..b54cd71e50 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/relation/EventEditor.kt +++ b/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") diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/send/DefaultSendService.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/send/DefaultSendService.kt index 8c0ea0ec4c..28c17f38b6 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/send/DefaultSendService.kt +++ b/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, 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) { diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/send/queue/EventSenderProcessorCoroutine.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/send/queue/EventSenderProcessorCoroutine.kt index eb69161614..f892dcee55 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/send/queue/EventSenderProcessorCoroutine.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/send/queue/EventSenderProcessorCoroutine.kt @@ -92,7 +92,7 @@ internal class EventSenderProcessorCoroutine @Inject constructor( } override fun postEvent(event: Event): Cancelable { - return postEvent(event, event.roomId?.let { cryptoService.isRoomEncrypted(it) } ?: false) + return postEvent(event, cryptoService.shouldEncryptInRoom(event.roomId)) } override fun postEvent(event: Event, encrypt: Boolean): Cancelable { 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 a7887d77f8..1c1d59fb3d 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 @@ -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)