From be488ae75ab7b44349eb2086c0a618a29fd30734 Mon Sep 17 00:00:00 2001 From: Benoit Marty <benoit@matrix.org> Date: Thu, 22 Jul 2021 15:24:05 +0200 Subject: [PATCH 1/6] Ensure OTK are uploaded when we upload the device keys The sync response can omit the field device_one_time_keys_count.signed_curve25519 and the SDK was waiting to know this value to upload the OTK. Now the SDK uploads the OTK when it uploads the device keys. --- .../android/sdk/internal/crypto/DefaultCryptoService.kt | 6 ++++++ 1 file changed, 6 insertions(+) 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 d170ae3dd3..1ca08dbe65 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 @@ -314,6 +314,12 @@ internal class DefaultCryptoService @Inject constructor( cryptoCoroutineScope.launchToCallback(coroutineDispatchers.crypto, NoOpMatrixCallback()) { // Open the store cryptoStore.open() + + if (!cryptoStore.getDeviceKeysUploaded()) { + // Schedule upload of OTK + oneTimeKeysUploader.updateOneTimeKeyCount(0) + } + // this can throw if no network tryOrNull { uploadDeviceKeys() From 7a7c292b3c8ad06cba782df43e259a2d10f725f1 Mon Sep 17 00:00:00 2001 From: Benoit Marty <benoit@matrix.org> Date: Thu, 22 Jul 2021 15:26:11 +0200 Subject: [PATCH 2/6] Rename store API --- .../android/sdk/internal/crypto/DefaultCryptoService.kt | 4 ++-- .../android/sdk/internal/crypto/store/IMXCryptoStore.kt | 2 +- .../android/sdk/internal/crypto/store/db/RealmCryptoStore.kt | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) 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 1ca08dbe65..563c890950 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 @@ -315,7 +315,7 @@ internal class DefaultCryptoService @Inject constructor( // Open the store cryptoStore.open() - if (!cryptoStore.getDeviceKeysUploaded()) { + if (!cryptoStore.areDeviceKeysUploaded()) { // Schedule upload of OTK oneTimeKeysUploader.updateOneTimeKeyCount(0) } @@ -911,7 +911,7 @@ internal class DefaultCryptoService @Inject constructor( * Upload my user's device keys. */ private suspend fun uploadDeviceKeys() { - if (cryptoStore.getDeviceKeysUploaded()) { + if (cryptoStore.areDeviceKeysUploaded()) { Timber.d("Keys already uploaded, nothing to do") return } 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 181bd94cc7..3d12e74fcd 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 @@ -475,7 +475,7 @@ internal interface IMXCryptoStore { fun getGossipingEvents(): List<Event> fun setDeviceKeysUploaded(uploaded: Boolean) - fun getDeviceKeysUploaded(): Boolean + fun areDeviceKeysUploaded(): Boolean fun tidyUpDataBase() fun logDbUsageInfo() } 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 9ae93d61eb..9266f8fe7d 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 @@ -937,7 +937,7 @@ internal class RealmCryptoStore @Inject constructor( } } - override fun getDeviceKeysUploaded(): Boolean { + override fun areDeviceKeysUploaded(): Boolean { return doWithRealm(realmConfiguration) { it.where<CryptoMetadataEntity>().findFirst()?.deviceKeysSentToServer } ?: false From 952a0f7c07deccfe6e28b4de8b5e373dec6a9ab5 Mon Sep 17 00:00:00 2001 From: Benoit Marty <benoit@matrix.org> Date: Thu, 22 Jul 2021 15:33:05 +0200 Subject: [PATCH 3/6] change --- changelog.d/3724.bugfix | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/3724.bugfix diff --git a/changelog.d/3724.bugfix b/changelog.d/3724.bugfix new file mode 100644 index 0000000000..8397e687bb --- /dev/null +++ b/changelog.d/3724.bugfix @@ -0,0 +1 @@ +Ensure OTKs are uploaded when the session is created \ No newline at end of file From 05988107a7187851092bf106a12d370d22abb6e9 Mon Sep 17 00:00:00 2001 From: Benoit Marty <benoit@matrix.org> Date: Thu, 22 Jul 2021 15:59:37 +0200 Subject: [PATCH 4/6] Ask the number of OTK if unknown from the sync. --- .../sdk/internal/crypto/OneTimeKeysUploader.kt | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/OneTimeKeysUploader.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/OneTimeKeysUploader.kt index 6695234d62..e3f622cb64 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/OneTimeKeysUploader.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/OneTimeKeysUploader.kt @@ -16,6 +16,7 @@ package org.matrix.android.sdk.internal.crypto +import org.matrix.android.sdk.api.extensions.tryOrNull import org.matrix.android.sdk.internal.crypto.model.MXKey import org.matrix.android.sdk.internal.crypto.model.rest.KeysUploadResponse import org.matrix.android.sdk.internal.crypto.tasks.UploadKeysTask @@ -77,6 +78,10 @@ internal class OneTimeKeysUploader @Inject constructor( // discard the oldest private keys first. This will eventually clean // out stale private keys that won't receive a message. val keyLimit = floor(maxOneTimeKeys / 2.0).toInt() + if (oneTimeKeyCount == null) { + // Ask the server how many otk he has + oneTimeKeyCount = fetchOtkNumber() + } val oneTimeKeyCountFromSync = oneTimeKeyCount if (oneTimeKeyCountFromSync != null) { // We need to keep a pool of one time public keys on the server so that @@ -103,6 +108,13 @@ internal class OneTimeKeysUploader @Inject constructor( } } + private suspend fun fetchOtkNumber(): Int? { + return tryOrNull { + val result = uploadKeysTask.execute(UploadKeysTask.Params(null, null)) + result.oneTimeKeyCountsForAlgorithm(MXKey.KEY_SIGNED_CURVE_25519_TYPE) + } + } + /** * Upload some the OTKs. * From 1d5ed46a49dc9dc0a70532dc8e87148cd8862889 Mon Sep 17 00:00:00 2001 From: Benoit Marty <benoit@matrix.org> Date: Thu, 22 Jul 2021 16:01:01 +0200 Subject: [PATCH 5/6] Small cleanup --- .../android/sdk/internal/crypto/OneTimeKeysUploader.kt | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/OneTimeKeysUploader.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/OneTimeKeysUploader.kt index e3f622cb64..d11943630e 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/OneTimeKeysUploader.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/OneTimeKeysUploader.kt @@ -95,17 +95,15 @@ internal class OneTimeKeysUploader @Inject constructor( // private keys clogging up our local storage. // So we need some kind of engineering compromise to balance all of // these factors. - try { + tryOrNull { val uploadedKeys = uploadOTK(oneTimeKeyCountFromSync, keyLimit) Timber.v("## uploadKeys() : success, $uploadedKeys key(s) sent") - } finally { - oneTimeKeyCheckInProgress = false } } else { Timber.w("maybeUploadOneTimeKeys: waiting to know the number of OTK from the sync") - oneTimeKeyCheckInProgress = false lastOneTimeKeyCheck = 0 } + oneTimeKeyCheckInProgress = false } private suspend fun fetchOtkNumber(): Int? { From ed0143c2402f6e46d90d7c4a1ecf62ef5b85cd22 Mon Sep 17 00:00:00 2001 From: Benoit Marty <benoit@matrix.org> Date: Thu, 22 Jul 2021 16:20:16 +0200 Subject: [PATCH 6/6] Log errors --- .../android/sdk/internal/crypto/OneTimeKeysUploader.kt | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/OneTimeKeysUploader.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/OneTimeKeysUploader.kt index d11943630e..c4b62fe9fe 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/OneTimeKeysUploader.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/OneTimeKeysUploader.kt @@ -80,7 +80,7 @@ internal class OneTimeKeysUploader @Inject constructor( val keyLimit = floor(maxOneTimeKeys / 2.0).toInt() if (oneTimeKeyCount == null) { // Ask the server how many otk he has - oneTimeKeyCount = fetchOtkNumber() + oneTimeKeyCount = fetchOtkCount() } val oneTimeKeyCountFromSync = oneTimeKeyCount if (oneTimeKeyCountFromSync != null) { @@ -95,7 +95,7 @@ internal class OneTimeKeysUploader @Inject constructor( // private keys clogging up our local storage. // So we need some kind of engineering compromise to balance all of // these factors. - tryOrNull { + tryOrNull("Unable to upload OTK") { val uploadedKeys = uploadOTK(oneTimeKeyCountFromSync, keyLimit) Timber.v("## uploadKeys() : success, $uploadedKeys key(s) sent") } @@ -106,8 +106,8 @@ internal class OneTimeKeysUploader @Inject constructor( oneTimeKeyCheckInProgress = false } - private suspend fun fetchOtkNumber(): Int? { - return tryOrNull { + private suspend fun fetchOtkCount(): Int? { + return tryOrNull("Unable to get OTK count") { val result = uploadKeysTask.execute(UploadKeysTask.Params(null, null)) result.oneTimeKeyCountsForAlgorithm(MXKey.KEY_SIGNED_CURVE_25519_TYPE) }