From f998cb6b18f9cdbc68113529bac07f67fe0ca1a0 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Mon, 6 Jul 2020 17:12:47 +0200 Subject: [PATCH] Upload device keys only once to the homeserver and fix crash when no network (#1629) --- CHANGES.md | 2 +- .../internal/crypto/DefaultCryptoService.kt | 22 +++++++++++++------ .../internal/crypto/store/IMXCryptoStore.kt | 3 +++ .../crypto/store/db/RealmCryptoStore.kt | 12 ++++++++++ .../store/db/RealmCryptoStoreMigration.kt | 10 ++++++++- .../store/db/model/CryptoMetadataEntity.kt | 3 +++ 6 files changed, 43 insertions(+), 9 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 20b3b34375..a8313cc722 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -5,7 +5,7 @@ Features ✨: - Improvements 🙌: - - + - Upload device keys only once to the homeserver and fix crash when no network (#1629) Bugfix 🐛: - Fix crash when coming from a notification (#1601) diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/DefaultCryptoService.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/DefaultCryptoService.kt index 02197fb5f3..01dd0ca17d 100755 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/DefaultCryptoService.kt +++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/DefaultCryptoService.kt @@ -70,7 +70,6 @@ import im.vector.matrix.android.internal.crypto.model.event.RoomKeyWithHeldConte import im.vector.matrix.android.internal.crypto.model.event.SecretSendEventContent import im.vector.matrix.android.internal.crypto.model.rest.DeviceInfo import im.vector.matrix.android.internal.crypto.model.rest.DevicesListResponse -import im.vector.matrix.android.internal.crypto.model.rest.KeysUploadResponse import im.vector.matrix.android.internal.crypto.model.rest.RoomKeyRequestBody import im.vector.matrix.android.internal.crypto.model.toRest import im.vector.matrix.android.internal.crypto.repository.WarnOnUnknownDeviceRepository @@ -343,8 +342,11 @@ internal class DefaultCryptoService @Inject constructor( cryptoCoroutineScope.launch(coroutineDispatchers.crypto) { // Open the store cryptoStore.open() - // TODO why do that everytime? we should mark that it was done - uploadDeviceKeys() + // this can throw if no network + tryThis { + uploadDeviceKeys() + } + oneTimeKeysUploader.maybeUploadOneTimeKeys() // this can throw if no backup tryThis { @@ -389,7 +391,7 @@ internal class DefaultCryptoService @Inject constructor( // } else { // Why would we do that? it will be called at end of syn - incomingGossipingRequestManager.processReceivedGossipingRequests() + incomingGossipingRequestManager.processReceivedGossipingRequests() // } }.fold( { @@ -888,7 +890,7 @@ internal class DefaultCryptoService @Inject constructor( */ private fun handleSDKLevelGossip(secretName: String?, secretValue: String): Boolean { return when (secretName) { - MASTER_KEY_SSSS_NAME -> { + MASTER_KEY_SSSS_NAME -> { crossSigningService.onSecretMSKGossip(secretValue) true } @@ -980,7 +982,11 @@ internal class DefaultCryptoService @Inject constructor( /** * Upload my user's device keys. */ - private suspend fun uploadDeviceKeys(): KeysUploadResponse { + private suspend fun uploadDeviceKeys() { + if (cryptoStore.getDeviceKeysUploaded()) { + Timber.d("Keys already uploaded, nothing to do") + return + } // Prepare the device keys data to send // Sign it val canonicalJson = JsonCanonicalizer.getCanonicalJson(Map::class.java, getMyDevice().signalableJSONDictionary()) @@ -991,7 +997,9 @@ internal class DefaultCryptoService @Inject constructor( ) val uploadDeviceKeysParams = UploadKeysTask.Params(rest, null) - return uploadKeysTask.execute(uploadDeviceKeysParams) + uploadKeysTask.execute(uploadDeviceKeysParams) + + cryptoStore.setDeviceKeysUploaded(true) } /** diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/store/IMXCryptoStore.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/store/IMXCryptoStore.kt index 3540f57608..f518332298 100644 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/store/IMXCryptoStore.kt +++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/store/IMXCryptoStore.kt @@ -433,4 +433,7 @@ internal interface IMXCryptoStore { fun getOutgoingSecretRequest(secretName: String): OutgoingSecretRequest? fun getIncomingRoomKeyRequests(): List fun getGossipingEventsTrail(): List + + fun setDeviceKeysUploaded(uploaded: Boolean) + fun getDeviceKeysUploaded(): Boolean } diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/store/db/RealmCryptoStore.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/store/db/RealmCryptoStore.kt index ec5e64dde8..0e67850304 100644 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/store/db/RealmCryptoStore.kt +++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/store/db/RealmCryptoStore.kt @@ -842,6 +842,18 @@ internal class RealmCryptoStore @Inject constructor( } ?: false } + override fun setDeviceKeysUploaded(uploaded: Boolean) { + doRealmTransaction(realmConfiguration) { + it.where().findFirst()?.deviceKeysSentToServer = uploaded + } + } + + override fun getDeviceKeysUploaded(): Boolean { + return doWithRealm(realmConfiguration) { + it.where().findFirst()?.deviceKeysSentToServer + } ?: false + } + override fun setRoomsListBlacklistUnverifiedDevices(roomIds: List) { doRealmTransaction(realmConfiguration) { // Reset all diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/store/db/RealmCryptoStoreMigration.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/store/db/RealmCryptoStoreMigration.kt index 7bf8e2478f..0cca430799 100644 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/store/db/RealmCryptoStoreMigration.kt +++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/store/db/RealmCryptoStoreMigration.kt @@ -54,7 +54,7 @@ internal class RealmCryptoStoreMigration @Inject constructor(private val crossSi // 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) - const val CRYPTO_STORE_SCHEMA_VERSION = 10L + const val CRYPTO_STORE_SCHEMA_VERSION = 11L } override fun migrate(realm: DynamicRealm, oldVersion: Long, newVersion: Long) { @@ -70,6 +70,7 @@ internal class RealmCryptoStoreMigration @Inject constructor(private val crossSi if (oldVersion <= 7) migrateTo8(realm) if (oldVersion <= 8) migrateTo9(realm) if (oldVersion <= 9) migrateTo10(realm) + if (oldVersion <= 10) migrateTo11(realm) } private fun migrateTo1Legacy(realm: DynamicRealm) { @@ -446,4 +447,11 @@ internal class RealmCryptoStoreMigration @Inject constructor(private val crossSi .addField(SharedSessionEntityFields.CHAIN_INDEX, Long::class.java) .setNullable(SharedSessionEntityFields.CHAIN_INDEX, true) } + + // Version 11L added deviceKeysSentToServer boolean to CryptoMetadataEntity + private fun migrateTo11(realm: DynamicRealm) { + Timber.d("Step 10 -> 11") + realm.schema.get("CryptoMetadataEntity") + ?.addField(CryptoMetadataEntityFields.DEVICE_KEYS_SENT_TO_SERVER, Boolean::class.java) + } } diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/store/db/model/CryptoMetadataEntity.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/store/db/model/CryptoMetadataEntity.kt index 2d4706ba76..f3c34af586 100644 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/store/db/model/CryptoMetadataEntity.kt +++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/store/db/model/CryptoMetadataEntity.kt @@ -36,6 +36,9 @@ internal open class CryptoMetadataEntity( // The keys backup version currently used. Null means no backup. var backupVersion: String? = null, + // The device keys has been sent to the homeserver + var deviceKeysSentToServer: Boolean = false, + var xSignMasterPrivateKey: String? = null, var xSignUserPrivateKey: String? = null, var xSignSelfSignedPrivateKey: String? = null,