From 4c4ef0d73eca19e846c553456d9f9e500276328c Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Tue, 3 Jan 2023 15:57:39 +0100 Subject: [PATCH] Batch insertion of user data after downloading keys. --- .../sdk/internal/crypto/DeviceListManager.kt | 14 +- .../internal/crypto/store/IMXCryptoStore.kt | 2 + .../internal/crypto/store/UserDataToStore.kt | 25 ++ .../crypto/store/db/RealmCryptoStore.kt | 236 ++++++++++-------- 4 files changed, 165 insertions(+), 112 deletions(-) create mode 100644 matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/store/UserDataToStore.kt diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/DeviceListManager.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/DeviceListManager.kt index 7e9e156003..f6e08ce9f7 100755 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/DeviceListManager.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/DeviceListManager.kt @@ -30,6 +30,7 @@ import org.matrix.android.sdk.api.session.crypto.model.MXUsersDevicesMap import org.matrix.android.sdk.internal.crypto.model.CryptoInfoMapper import org.matrix.android.sdk.internal.crypto.model.rest.KeysQueryResponse import org.matrix.android.sdk.internal.crypto.store.IMXCryptoStore +import org.matrix.android.sdk.internal.crypto.store.UserDataToStore import org.matrix.android.sdk.internal.crypto.tasks.DownloadKeysForUsersTask import org.matrix.android.sdk.internal.session.SessionScope import org.matrix.android.sdk.internal.session.sync.SyncTokenStore @@ -371,6 +372,8 @@ internal class DeviceListManager @Inject constructor( Timber.v("## CRYPTO | doKeyDownloadForUsers() : Got keys for " + filteredUsers.size + " users") } + val userDataToStore = UserDataToStore() + for (userId in filteredUsers) { // al devices = val models = response.deviceKeys?.get(userId)?.mapValues { entry -> CryptoInfoMapper.map(entry.value) } @@ -404,7 +407,7 @@ internal class DeviceListManager @Inject constructor( } // Update the store // Note that devices which aren't in the response will be removed from the stores - cryptoStore.storeUserDevices(userId, workingCopy) + userDataToStore.userDevices[userId] = workingCopy } val masterKey = response.masterKeys?.get(userId)?.toCryptoModel().also { @@ -416,14 +419,11 @@ internal class DeviceListManager @Inject constructor( val userSigningKey = response.userSigningKeys?.get(userId)?.toCryptoModel()?.also { Timber.v("## CRYPTO | CrossSigning : Got keys for $userId : USK ${it.unpaddedBase64PublicKey}") } - cryptoStore.storeUserCrossSigningKeys( - userId, - masterKey, - selfSigningKey, - userSigningKey - ) + userDataToStore.userCrossSigningKeys[userId] = Triple(masterKey, selfSigningKey, userSigningKey) } + cryptoStore.storeUserDataToStore(userDataToStore) + // Update devices trust for these users // dispatchDeviceChange(downloadUsers) 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 a285dbec78..92a92ab36a 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 @@ -583,4 +583,6 @@ internal interface IMXCryptoStore { fun areDeviceKeysUploaded(): Boolean fun tidyUpDataBase() fun getOutgoingRoomKeyRequests(inStates: Set): List + + fun storeUserDataToStore(userDataToStore: UserDataToStore) } diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/store/UserDataToStore.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/store/UserDataToStore.kt new file mode 100644 index 0000000000..05afc75e5a --- /dev/null +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/store/UserDataToStore.kt @@ -0,0 +1,25 @@ +/* + * Copyright (c) 2023 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 + +import org.matrix.android.sdk.api.session.crypto.crosssigning.CryptoCrossSigningKey +import org.matrix.android.sdk.api.session.crypto.model.CryptoDeviceInfo + +internal data class UserDataToStore( + val userDevices: MutableMap> = mutableMapOf(), + val userCrossSigningKeys: MutableMap> = mutableMapOf(), +) 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 949043f2db..83ff960d53 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 @@ -55,6 +55,7 @@ import org.matrix.android.sdk.internal.crypto.model.MXInboundMegolmSessionWrappe 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.store.IMXCryptoStore +import org.matrix.android.sdk.internal.crypto.store.UserDataToStore import org.matrix.android.sdk.internal.crypto.store.db.mapper.CrossSigningKeysMapper import org.matrix.android.sdk.internal.crypto.store.db.mapper.MyDeviceLastSeenInfoEntityMapper import org.matrix.android.sdk.internal.crypto.store.db.model.AuditTrailEntity @@ -289,37 +290,41 @@ internal class RealmCryptoStore @Inject constructor( override fun storeUserDevices(userId: String, devices: Map?) { doRealmTransaction("storeUserDevices", realmConfiguration) { realm -> - if (devices == null) { - Timber.d("Remove user $userId") - // Remove the user - UserEntity.delete(realm, userId) - } else { - val userEntity = UserEntity.getOrCreate(realm, userId) - // First delete the removed devices - val deviceIds = devices.keys - userEntity.devices.toTypedArray().iterator().let { - while (it.hasNext()) { - val deviceInfoEntity = it.next() - if (deviceInfoEntity.deviceId !in deviceIds) { - Timber.d("Remove device ${deviceInfoEntity.deviceId} of user $userId") - deviceInfoEntity.deleteOnCascade() - } + storeUserDevices(realm, userId, devices) + } + } + + private fun storeUserDevices(realm: Realm, userId: String, devices: Map?) { + if (devices == null) { + Timber.d("Remove user $userId") + // Remove the user + UserEntity.delete(realm, userId) + } else { + val userEntity = UserEntity.getOrCreate(realm, userId) + // First delete the removed devices + val deviceIds = devices.keys + userEntity.devices.toTypedArray().iterator().let { + while (it.hasNext()) { + val deviceInfoEntity = it.next() + if (deviceInfoEntity.deviceId !in deviceIds) { + Timber.d("Remove device ${deviceInfoEntity.deviceId} of user $userId") + deviceInfoEntity.deleteOnCascade() } } - // Then update existing devices or add new one - devices.values.forEach { cryptoDeviceInfo -> - val existingDeviceInfoEntity = userEntity.devices.firstOrNull { it.deviceId == cryptoDeviceInfo.deviceId } - if (existingDeviceInfoEntity == null) { - // Add the device - Timber.d("Add device ${cryptoDeviceInfo.deviceId} of user $userId") - val newEntity = CryptoMapper.mapToEntity(cryptoDeviceInfo) - newEntity.firstTimeSeenLocalTs = clock.epochMillis() - userEntity.devices.add(newEntity) - } else { - // Update the device - Timber.d("Update device ${cryptoDeviceInfo.deviceId} of user $userId") - CryptoMapper.updateDeviceInfoEntity(existingDeviceInfoEntity, cryptoDeviceInfo) - } + } + // Then update existing devices or add new one + devices.values.forEach { cryptoDeviceInfo -> + val existingDeviceInfoEntity = userEntity.devices.firstOrNull { it.deviceId == cryptoDeviceInfo.deviceId } + if (existingDeviceInfoEntity == null) { + // Add the device + Timber.d("Add device ${cryptoDeviceInfo.deviceId} of user $userId") + val newEntity = CryptoMapper.mapToEntity(cryptoDeviceInfo) + newEntity.firstTimeSeenLocalTs = clock.epochMillis() + userEntity.devices.add(newEntity) + } else { + // Update the device + Timber.d("Update device ${cryptoDeviceInfo.deviceId} of user $userId") + CryptoMapper.updateDeviceInfoEntity(existingDeviceInfoEntity, cryptoDeviceInfo) } } } @@ -332,85 +337,95 @@ internal class RealmCryptoStore @Inject constructor( userSigningKey: CryptoCrossSigningKey? ) { doRealmTransaction("storeUserCrossSigningKeys", realmConfiguration) { realm -> - UserEntity.getOrCreate(realm, userId) - .let { userEntity -> - if (masterKey == null || selfSigningKey == null) { - // The user has disabled cross signing? - userEntity.crossSigningInfoEntity?.deleteOnCascade() - userEntity.crossSigningInfoEntity = null - } else { - var shouldResetMyDevicesLocalTrust = false - CrossSigningInfoEntity.getOrCreate(realm, userId).let { signingInfo -> - // What should we do if we detect a change of the keys? - val existingMaster = signingInfo.getMasterKey() - if (existingMaster != null && existingMaster.publicKeyBase64 == masterKey.unpaddedBase64PublicKey) { - crossSigningKeysMapper.update(existingMaster, masterKey) - } else { - Timber.d("## CrossSigning MSK change for $userId") - val keyEntity = crossSigningKeysMapper.map(masterKey) - signingInfo.setMasterKey(keyEntity) - if (userId == this.userId) { - shouldResetMyDevicesLocalTrust = true - // my msk has changed! clear my private key - // Could we have some race here? e.g I am the one that did change the keys - // could i get this update to early and clear the private keys? - // -> initializeCrossSigning is guarding for that by storing all at once - realm.where().findFirst()?.apply { - xSignMasterPrivateKey = null - } + storeUserCrossSigningKeys(realm, userId, masterKey, selfSigningKey, userSigningKey) + } + } + + private fun storeUserCrossSigningKeys( + realm: Realm, + userId: String, + masterKey: CryptoCrossSigningKey?, + selfSigningKey: CryptoCrossSigningKey?, + userSigningKey: CryptoCrossSigningKey? + ) { + UserEntity.getOrCreate(realm, userId) + .let { userEntity -> + if (masterKey == null || selfSigningKey == null) { + // The user has disabled cross signing? + userEntity.crossSigningInfoEntity?.deleteOnCascade() + userEntity.crossSigningInfoEntity = null + } else { + var shouldResetMyDevicesLocalTrust = false + CrossSigningInfoEntity.getOrCreate(realm, userId).let { signingInfo -> + // What should we do if we detect a change of the keys? + val existingMaster = signingInfo.getMasterKey() + if (existingMaster != null && existingMaster.publicKeyBase64 == masterKey.unpaddedBase64PublicKey) { + crossSigningKeysMapper.update(existingMaster, masterKey) + } else { + Timber.d("## CrossSigning MSK change for $userId") + val keyEntity = crossSigningKeysMapper.map(masterKey) + signingInfo.setMasterKey(keyEntity) + if (userId == this.userId) { + shouldResetMyDevicesLocalTrust = true + // my msk has changed! clear my private key + // Could we have some race here? e.g I am the one that did change the keys + // could i get this update to early and clear the private keys? + // -> initializeCrossSigning is guarding for that by storing all at once + realm.where().findFirst()?.apply { + xSignMasterPrivateKey = null } } - - val existingSelfSigned = signingInfo.getSelfSignedKey() - if (existingSelfSigned != null && existingSelfSigned.publicKeyBase64 == selfSigningKey.unpaddedBase64PublicKey) { - crossSigningKeysMapper.update(existingSelfSigned, selfSigningKey) - } else { - Timber.d("## CrossSigning SSK change for $userId") - val keyEntity = crossSigningKeysMapper.map(selfSigningKey) - signingInfo.setSelfSignedKey(keyEntity) - if (userId == this.userId) { - shouldResetMyDevicesLocalTrust = true - // my ssk has changed! clear my private key - realm.where().findFirst()?.apply { - xSignSelfSignedPrivateKey = null - } - } - } - - // Only for me - if (userSigningKey != null) { - val existingUSK = signingInfo.getUserSigningKey() - if (existingUSK != null && existingUSK.publicKeyBase64 == userSigningKey.unpaddedBase64PublicKey) { - crossSigningKeysMapper.update(existingUSK, userSigningKey) - } else { - Timber.d("## CrossSigning USK change for $userId") - val keyEntity = crossSigningKeysMapper.map(userSigningKey) - signingInfo.setUserSignedKey(keyEntity) - if (userId == this.userId) { - shouldResetMyDevicesLocalTrust = true - // my usk has changed! clear my private key - realm.where().findFirst()?.apply { - xSignUserPrivateKey = null - } - } - } - } - - // When my cross signing keys are reset, we consider clearing all existing device trust - if (shouldResetMyDevicesLocalTrust) { - realm.where() - .equalTo(UserEntityFields.USER_ID, this.userId) - .findFirst() - ?.devices?.forEach { - it?.trustLevelEntity?.crossSignedVerified = false - it?.trustLevelEntity?.locallyVerified = it.deviceId == deviceId - } - } - userEntity.crossSigningInfoEntity = signingInfo } + + val existingSelfSigned = signingInfo.getSelfSignedKey() + if (existingSelfSigned != null && existingSelfSigned.publicKeyBase64 == selfSigningKey.unpaddedBase64PublicKey) { + crossSigningKeysMapper.update(existingSelfSigned, selfSigningKey) + } else { + Timber.d("## CrossSigning SSK change for $userId") + val keyEntity = crossSigningKeysMapper.map(selfSigningKey) + signingInfo.setSelfSignedKey(keyEntity) + if (userId == this.userId) { + shouldResetMyDevicesLocalTrust = true + // my ssk has changed! clear my private key + realm.where().findFirst()?.apply { + xSignSelfSignedPrivateKey = null + } + } + } + + // Only for me + if (userSigningKey != null) { + val existingUSK = signingInfo.getUserSigningKey() + if (existingUSK != null && existingUSK.publicKeyBase64 == userSigningKey.unpaddedBase64PublicKey) { + crossSigningKeysMapper.update(existingUSK, userSigningKey) + } else { + Timber.d("## CrossSigning USK change for $userId") + val keyEntity = crossSigningKeysMapper.map(userSigningKey) + signingInfo.setUserSignedKey(keyEntity) + if (userId == this.userId) { + shouldResetMyDevicesLocalTrust = true + // my usk has changed! clear my private key + realm.where().findFirst()?.apply { + xSignUserPrivateKey = null + } + } + } + } + + // When my cross signing keys are reset, we consider clearing all existing device trust + if (shouldResetMyDevicesLocalTrust) { + realm.where() + .equalTo(UserEntityFields.USER_ID, this.userId) + .findFirst() + ?.devices?.forEach { + it?.trustLevelEntity?.crossSignedVerified = false + it?.trustLevelEntity?.locallyVerified = it.deviceId == deviceId + } + } + userEntity.crossSigningInfoEntity = signingInfo } } - } + } } override fun getCrossSigningPrivateKeys(): PrivateKeysInfo? { @@ -1831,13 +1846,24 @@ internal class RealmCryptoStore @Inject constructor( } doRealmTransaction("onSyncCompleted", realmConfiguration) { realm -> // setShouldShareHistory - aggregator.setShouldShareHistoryData.map { + aggregator.setShouldShareHistoryData.forEach { CryptoRoomEntity.getOrCreate(realm, it.key).shouldShareHistory = it.value } // setShouldEncryptForInvitedMembers - aggregator.setShouldEncryptForInvitedMembersData.map { + aggregator.setShouldEncryptForInvitedMembersData.forEach { CryptoRoomEntity.getOrCreate(realm, it.key).shouldEncryptForInvitedMembers = it.value } } } + + override fun storeUserDataToStore(userDataToStore: UserDataToStore) { + doRealmTransaction("storeUserDataToStore", realmConfiguration) { realm -> + userDataToStore.userDevices.forEach { + storeUserDevices(realm, it.key, it.value) + } + userDataToStore.userCrossSigningKeys.forEach { + storeUserCrossSigningKeys(realm, it.key, it.value.first, it.value.second, it.value.third) + } + } + } }