From 3b9daec869810320fdbfe1653b346453f7ea153a Mon Sep 17 00:00:00 2001 From: Valere Date: Wed, 27 Sep 2023 15:42:05 +0200 Subject: [PATCH 1/8] Fix QR code login support in rust --- .../sdk/internal/crypto/SecretShareManager.kt | 38 +++++++++++++++++++ .../android/sdk/api/rendezvous/Rendezvous.kt | 11 +----- .../SharedSecretStorageService.kt | 6 +++ .../DefaultSharedSecretStorageService.kt | 5 +++ .../android/sdk/internal/crypto/OlmMachine.kt | 13 +++++++ .../sdk/internal/crypto/RustCryptoService.kt | 4 +- .../sdk/internal/crypto/SecretShareManager.kt | 28 +++++++++++--- 7 files changed, 89 insertions(+), 16 deletions(-) diff --git a/matrix-sdk-android/src/kotlinCrypto/java/org/matrix/android/sdk/internal/crypto/SecretShareManager.kt b/matrix-sdk-android/src/kotlinCrypto/java/org/matrix/android/sdk/internal/crypto/SecretShareManager.kt index 7f4f4aee88..56c181a306 100644 --- a/matrix-sdk-android/src/kotlinCrypto/java/org/matrix/android/sdk/internal/crypto/SecretShareManager.kt +++ b/matrix-sdk-android/src/kotlinCrypto/java/org/matrix/android/sdk/internal/crypto/SecretShareManager.kt @@ -136,6 +136,13 @@ internal class SecretShareManager @Inject constructor( .w("handleSecretRequest() : malformed request norequestingDeviceId ") } + if (request.requestingDeviceId == credentials.deviceId) { + return Unit.also { + Timber.tag(loggerTag.value) + .v("handleSecretRequest() : Ignore request from self device") + } + } + val device = cryptoStore.getUserDevice(credentials.userId, deviceId) ?: return Unit.also { Timber.tag(loggerTag.value) @@ -254,6 +261,37 @@ internal class SecretShareManager @Inject constructor( } } + suspend fun requestMissingSecrets() { + // quick implementation for backward compatibility with rust, will request all secrets to all own devices + val secretNames = listOf(MASTER_KEY_SSSS_NAME, SELF_SIGNING_KEY_SSSS_NAME, USER_SIGNING_KEY_SSSS_NAME, KEYBACKUP_SECRET_SSSS_NAME) + + secretNames.forEach { secretName -> + val toDeviceContent = SecretShareRequest( + requestingDeviceId = credentials.deviceId, + secretName = secretName, + requestId = createUniqueTxnId() + ) + + val contentMap = MXUsersDevicesMap() + contentMap.setObject(credentials.userId, "*", toDeviceContent) + + val params = SendToDeviceTask.Params( + eventType = EventType.REQUEST_SECRET, + contentMap = contentMap + ) + try { + withContext(coroutineDispatchers.io) { + sendToDeviceTask.execute(params) + } + Timber.tag(loggerTag.value) + .d("Secret request sent for $secretName") + } catch (failure: Throwable) { + Timber.tag(loggerTag.value) + .w("Failed to request secret $secretName") + } + } + } + suspend fun onSecretSendReceived(toDevice: Event, handleGossip: ((name: String, value: String) -> Boolean)) { Timber.tag(loggerTag.value) .i("onSecretSend() from ${toDevice.senderId} : onSecretSendReceived ${toDevice.content?.get("sender_key")}") diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/rendezvous/Rendezvous.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/rendezvous/Rendezvous.kt index 91f3c2a506..c2cc95855c 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/rendezvous/Rendezvous.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/rendezvous/Rendezvous.kt @@ -34,10 +34,6 @@ import org.matrix.android.sdk.api.rendezvous.model.SecureRendezvousChannelAlgori import org.matrix.android.sdk.api.rendezvous.transports.SimpleHttpRendezvousTransport import org.matrix.android.sdk.api.session.Session import org.matrix.android.sdk.api.session.crypto.crosssigning.DeviceTrustLevel -import org.matrix.android.sdk.api.session.crypto.crosssigning.KEYBACKUP_SECRET_SSSS_NAME -import org.matrix.android.sdk.api.session.crypto.crosssigning.MASTER_KEY_SSSS_NAME -import org.matrix.android.sdk.api.session.crypto.crosssigning.SELF_SIGNING_KEY_SSSS_NAME -import org.matrix.android.sdk.api.session.crypto.crosssigning.USER_SIGNING_KEY_SSSS_NAME import org.matrix.android.sdk.api.util.MatrixJsonParser import timber.log.Timber @@ -225,12 +221,7 @@ class Rendezvous( // request secrets from the verifying device Timber.tag(TAG).i("Requesting secrets from $verifyingDeviceId") - session.sharedSecretStorageService().let { - it.requestSecret(MASTER_KEY_SSSS_NAME, verifyingDeviceId) - it.requestSecret(SELF_SIGNING_KEY_SSSS_NAME, verifyingDeviceId) - it.requestSecret(USER_SIGNING_KEY_SSSS_NAME, verifyingDeviceId) - it.requestSecret(KEYBACKUP_SECRET_SSSS_NAME, verifyingDeviceId) - } + session.sharedSecretStorageService().requestMissingSecrets() } else { Timber.tag(TAG).i("Not doing verification") } diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/securestorage/SharedSecretStorageService.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/securestorage/SharedSecretStorageService.kt index bdbbd3ea84..783665f9b8 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/securestorage/SharedSecretStorageService.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/securestorage/SharedSecretStorageService.kt @@ -135,5 +135,11 @@ interface SharedSecretStorageService { fun checkShouldBeAbleToAccessSecrets(secretNames: List, keyId: String?): IntegrityResult + @Deprecated("Requesting custom secrets not yet support by rust stack, prefer requestMissingSecrets") suspend fun requestSecret(name: String, myOtherDeviceId: String) + + /** + * Request the missing local secrets to other sessions. + */ + suspend fun requestMissingSecrets() } diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/secrets/DefaultSharedSecretStorageService.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/secrets/DefaultSharedSecretStorageService.kt index ddb048a912..05b9e14b82 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/secrets/DefaultSharedSecretStorageService.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/secrets/DefaultSharedSecretStorageService.kt @@ -385,7 +385,12 @@ internal class DefaultSharedSecretStorageService @Inject constructor( return IntegrityResult.Success(keyInfo.content.passphrase != null) } + @Deprecated("Requesting custom secrets not yet support by rust stack, prefer requestMissingSecrets") override suspend fun requestSecret(name: String, myOtherDeviceId: String) { secretShareManager.requestSecretTo(myOtherDeviceId, name) } + + override suspend fun requestMissingSecrets() { + secretShareManager.requestMissingSecrets() + } } diff --git a/matrix-sdk-android/src/rustCrypto/java/org/matrix/android/sdk/internal/crypto/OlmMachine.kt b/matrix-sdk-android/src/rustCrypto/java/org/matrix/android/sdk/internal/crypto/OlmMachine.kt index 3686ab445d..4646d74c9a 100644 --- a/matrix-sdk-android/src/rustCrypto/java/org/matrix/android/sdk/internal/crypto/OlmMachine.kt +++ b/matrix-sdk-android/src/rustCrypto/java/org/matrix/android/sdk/internal/crypto/OlmMachine.kt @@ -76,6 +76,7 @@ import org.matrix.rustcomponents.sdk.crypto.DeviceLists import org.matrix.rustcomponents.sdk.crypto.EncryptionSettings import org.matrix.rustcomponents.sdk.crypto.KeyRequestPair import org.matrix.rustcomponents.sdk.crypto.KeysImportResult +import org.matrix.rustcomponents.sdk.crypto.LocalTrust import org.matrix.rustcomponents.sdk.crypto.Logger import org.matrix.rustcomponents.sdk.crypto.MegolmV1BackupKey import org.matrix.rustcomponents.sdk.crypto.Request @@ -869,6 +870,11 @@ internal class OlmMachine @Inject constructor( } } + suspend fun requestMissingSecretsFromOtherSessions(): Boolean { + return withContext(coroutineDispatchers.io) { + inner.queryMissingSecretsFromOtherSessions() + } + } @Throws(CryptoStoreException::class) suspend fun enableBackupV1(key: String, version: String) { return withContext(coroutineDispatchers.computation) { @@ -934,4 +940,11 @@ internal class OlmMachine @Inject constructor( inner.verifyBackup(serializedAuthData) } } + + @Throws(CryptoStoreException::class) + suspend fun setDeviceLocalTrust(userId: String, deviceId: String, trusted: Boolean) { + withContext(coroutineDispatchers.io) { + inner.setLocalTrust(userId, deviceId, if (trusted) LocalTrust.VERIFIED else LocalTrust.UNSET) + } + } } diff --git a/matrix-sdk-android/src/rustCrypto/java/org/matrix/android/sdk/internal/crypto/RustCryptoService.kt b/matrix-sdk-android/src/rustCrypto/java/org/matrix/android/sdk/internal/crypto/RustCryptoService.kt index 57f81ef592..d5069fe010 100755 --- a/matrix-sdk-android/src/rustCrypto/java/org/matrix/android/sdk/internal/crypto/RustCryptoService.kt +++ b/matrix-sdk-android/src/rustCrypto/java/org/matrix/android/sdk/internal/crypto/RustCryptoService.kt @@ -31,6 +31,7 @@ import org.matrix.android.sdk.api.MatrixCoroutineDispatchers import org.matrix.android.sdk.api.auth.UserInteractiveAuthInterceptor import org.matrix.android.sdk.api.crypto.MXCRYPTO_ALGORITHM_MEGOLM import org.matrix.android.sdk.api.crypto.MXCryptoConfig +import org.matrix.android.sdk.api.extensions.orFalse import org.matrix.android.sdk.api.extensions.tryOrNull import org.matrix.android.sdk.api.listeners.ProgressListener import org.matrix.android.sdk.api.logger.LoggerTag @@ -536,7 +537,8 @@ internal class RustCryptoService @Inject constructor( } override suspend fun setDeviceVerification(trustLevel: DeviceTrustLevel, userId: String, deviceId: String) { - TODO("Not yet implemented") + Timber.w("Rust stack only support API to set local trust") + olmMachine.setDeviceLocalTrust(userId, deviceId, trustLevel.isLocallyVerified().orFalse()) } /** diff --git a/matrix-sdk-android/src/rustCrypto/java/org/matrix/android/sdk/internal/crypto/SecretShareManager.kt b/matrix-sdk-android/src/rustCrypto/java/org/matrix/android/sdk/internal/crypto/SecretShareManager.kt index b4fab22677..c7b299cce2 100644 --- a/matrix-sdk-android/src/rustCrypto/java/org/matrix/android/sdk/internal/crypto/SecretShareManager.kt +++ b/matrix-sdk-android/src/rustCrypto/java/org/matrix/android/sdk/internal/crypto/SecretShareManager.kt @@ -16,15 +16,33 @@ package org.matrix.android.sdk.internal.crypto -import org.matrix.android.sdk.BuildConfig +import org.matrix.android.sdk.api.session.events.model.EventType +import org.matrix.android.sdk.internal.crypto.network.OutgoingRequestsProcessor +import org.matrix.rustcomponents.sdk.crypto.Request import timber.log.Timber import javax.inject.Inject +import javax.inject.Provider -internal class SecretShareManager @Inject constructor() { +internal class SecretShareManager @Inject constructor( + private val olmMachine: Provider, + private val outgoingRequestsProcessor: OutgoingRequestsProcessor) { suspend fun requestSecretTo(deviceId: String, secretName: String) { - // nop in rust? - if (BuildConfig.DEBUG) TODO("requestSecretTo Not implemented in Rust") - Timber.e("SecretShareManager Not supported in rust $deviceId, $secretName") + Timber.v("SecretShareManager requesting $deviceId, $secretName") + if (this.olmMachine.get().requestMissingSecretsFromOtherSessions()) { + // immediately send the requests + outgoingRequestsProcessor.processOutgoingRequests(this.olmMachine.get()) { + it is Request.ToDevice && it.eventType == EventType.REQUEST_SECRET + } + } + } + + suspend fun requestMissingSecrets() { + this.olmMachine.get().requestMissingSecretsFromOtherSessions() + + // immediately send the requests + outgoingRequestsProcessor.processOutgoingRequests(this.olmMachine.get()) { + it is Request.ToDevice && it.eventType == EventType.REQUEST_SECRET + } } } From 6ee438d7d561a8b4c5431e126f9785fd67da01ca Mon Sep 17 00:00:00 2001 From: Valere Date: Sun, 1 Oct 2023 19:25:12 +0200 Subject: [PATCH 2/8] bump crypto sdk --- matrix-sdk-android/build.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/matrix-sdk-android/build.gradle b/matrix-sdk-android/build.gradle index c7183b8365..63fb13ead8 100644 --- a/matrix-sdk-android/build.gradle +++ b/matrix-sdk-android/build.gradle @@ -216,7 +216,7 @@ dependencies { implementation libs.google.phonenumber - rustCryptoImplementation("org.matrix.rustcomponents:crypto-android:0.3.14") + rustCryptoImplementation("org.matrix.rustcomponents:crypto-android:0.3.15") // rustCryptoApi project(":library:rustCrypto") testImplementation libs.tests.junit From 42eec4b557cbdb29aaa130e01dc403cfafd70613 Mon Sep 17 00:00:00 2001 From: Valere Date: Sun, 1 Oct 2023 19:41:46 +0200 Subject: [PATCH 3/8] update changelog --- changelog.d/8653.bugfix | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/8653.bugfix diff --git a/changelog.d/8653.bugfix b/changelog.d/8653.bugfix new file mode 100644 index 0000000000..8aa0ff2e17 --- /dev/null +++ b/changelog.d/8653.bugfix @@ -0,0 +1 @@ +Fix Login with QR code not working with rust crypto. From 0d70f6eb5490486460bf49e75ef3a5982ed9a649 Mon Sep 17 00:00:00 2001 From: Valere Date: Sun, 1 Oct 2023 21:59:46 +0200 Subject: [PATCH 4/8] missing mock --- .../vector/app/test/fakes/FakeSharedSecretStorageService.kt | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/vector/src/test/java/im/vector/app/test/fakes/FakeSharedSecretStorageService.kt b/vector/src/test/java/im/vector/app/test/fakes/FakeSharedSecretStorageService.kt index 1dc36de709..cf7198469c 100644 --- a/vector/src/test/java/im/vector/app/test/fakes/FakeSharedSecretStorageService.kt +++ b/vector/src/test/java/im/vector/app/test/fakes/FakeSharedSecretStorageService.kt @@ -79,6 +79,10 @@ class FakeSharedSecretStorageService : SharedSecretStorageService by mockk() { TODO("Not yet implemented") } + override suspend fun requestMissingSecrets() { + TODO("Not yet implemented") + } + fun givenIsRecoverySetupReturns(isRecoverySetup: Boolean) { every { isRecoverySetup() } returns isRecoverySetup } From 2709cb297311b12ce9ef499c2f713cefc0e72012 Mon Sep 17 00:00:00 2001 From: Valere Date: Sun, 1 Oct 2023 22:19:54 +0200 Subject: [PATCH 5/8] missing deprecated --- .../im/vector/app/test/fakes/FakeSharedSecretStorageService.kt | 1 + 1 file changed, 1 insertion(+) diff --git a/vector/src/test/java/im/vector/app/test/fakes/FakeSharedSecretStorageService.kt b/vector/src/test/java/im/vector/app/test/fakes/FakeSharedSecretStorageService.kt index cf7198469c..9d46cc0c28 100644 --- a/vector/src/test/java/im/vector/app/test/fakes/FakeSharedSecretStorageService.kt +++ b/vector/src/test/java/im/vector/app/test/fakes/FakeSharedSecretStorageService.kt @@ -75,6 +75,7 @@ class FakeSharedSecretStorageService : SharedSecretStorageService by mockk() { override fun checkShouldBeAbleToAccessSecrets(secretNames: List, keyId: String?) = integrityResult + @Deprecated("Requesting custom secrets not yet support by rust stack, prefer requestMissingSecrets") override suspend fun requestSecret(name: String, myOtherDeviceId: String) { TODO("Not yet implemented") } From 1bd2da5c996c0fda28241e2c570c9727d85c8d6f Mon Sep 17 00:00:00 2001 From: Valere Date: Mon, 2 Oct 2023 16:39:08 +0200 Subject: [PATCH 6/8] disable flacky test on legacy crypto --- .../android/sdk/internal/crypto/E2eeShareKeysHistoryTest.kt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/internal/crypto/E2eeShareKeysHistoryTest.kt b/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/internal/crypto/E2eeShareKeysHistoryTest.kt index 700f912cf1..fc1b5bba93 100644 --- a/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/internal/crypto/E2eeShareKeysHistoryTest.kt +++ b/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/internal/crypto/E2eeShareKeysHistoryTest.kt @@ -28,6 +28,7 @@ import org.junit.Test import org.junit.runner.RunWith import org.junit.runners.JUnit4 import org.junit.runners.MethodSorters +import org.matrix.android.sdk.BuildConfig import org.matrix.android.sdk.InstrumentedTest import org.matrix.android.sdk.api.query.QueryStringValue import org.matrix.android.sdk.api.session.Session @@ -196,6 +197,7 @@ class E2eeShareKeysHistoryTest : InstrumentedTest { @Test fun testNeedsRotationFromSharedToWorldReadable() { + Assume.assumeTrue("Test is flacky on legacy crypto", BuildConfig.FLAVOR == "rustCrypto") testRotationDueToVisibilityChange(RoomHistoryVisibility.SHARED, RoomHistoryVisibilityContent("world_readable")) } From 87df8ab6f60c6466e2670e98bb6d2183b334dcdb Mon Sep 17 00:00:00 2001 From: Valere Date: Tue, 3 Oct 2023 11:24:38 +0200 Subject: [PATCH 7/8] Update matrix-sdk-android/src/kotlinCrypto/java/org/matrix/android/sdk/internal/crypto/SecretShareManager.kt Co-authored-by: Benoit Marty --- .../matrix/android/sdk/internal/crypto/SecretShareManager.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/matrix-sdk-android/src/kotlinCrypto/java/org/matrix/android/sdk/internal/crypto/SecretShareManager.kt b/matrix-sdk-android/src/kotlinCrypto/java/org/matrix/android/sdk/internal/crypto/SecretShareManager.kt index 56c181a306..24591e8bd4 100644 --- a/matrix-sdk-android/src/kotlinCrypto/java/org/matrix/android/sdk/internal/crypto/SecretShareManager.kt +++ b/matrix-sdk-android/src/kotlinCrypto/java/org/matrix/android/sdk/internal/crypto/SecretShareManager.kt @@ -136,7 +136,7 @@ internal class SecretShareManager @Inject constructor( .w("handleSecretRequest() : malformed request norequestingDeviceId ") } - if (request.requestingDeviceId == credentials.deviceId) { + if (deviceId == credentials.deviceId) { return Unit.also { Timber.tag(loggerTag.value) .v("handleSecretRequest() : Ignore request from self device") From a015eda72cbce56de8dfe47a0c5f78cfd723c0cd Mon Sep 17 00:00:00 2001 From: Valere Date: Tue, 3 Oct 2023 11:29:50 +0200 Subject: [PATCH 8/8] code review --- .../matrix/android/sdk/api/rendezvous/Rendezvous.kt | 4 ++-- .../android/sdk/internal/crypto/SecretShareManager.kt | 10 +++------- 2 files changed, 5 insertions(+), 9 deletions(-) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/rendezvous/Rendezvous.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/rendezvous/Rendezvous.kt index c2cc95855c..d5596ce56f 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/rendezvous/Rendezvous.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/rendezvous/Rendezvous.kt @@ -218,8 +218,8 @@ class Rendezvous( Timber.tag(TAG).i("No master key given by verifying device") } - // request secrets from the verifying device - Timber.tag(TAG).i("Requesting secrets from $verifyingDeviceId") + // request secrets from other sessions. + Timber.tag(TAG).i("Requesting secrets from other sessions") session.sharedSecretStorageService().requestMissingSecrets() } else { diff --git a/matrix-sdk-android/src/rustCrypto/java/org/matrix/android/sdk/internal/crypto/SecretShareManager.kt b/matrix-sdk-android/src/rustCrypto/java/org/matrix/android/sdk/internal/crypto/SecretShareManager.kt index c7b299cce2..ba46776663 100644 --- a/matrix-sdk-android/src/rustCrypto/java/org/matrix/android/sdk/internal/crypto/SecretShareManager.kt +++ b/matrix-sdk-android/src/rustCrypto/java/org/matrix/android/sdk/internal/crypto/SecretShareManager.kt @@ -28,13 +28,9 @@ internal class SecretShareManager @Inject constructor( private val outgoingRequestsProcessor: OutgoingRequestsProcessor) { suspend fun requestSecretTo(deviceId: String, secretName: String) { - Timber.v("SecretShareManager requesting $deviceId, $secretName") - if (this.olmMachine.get().requestMissingSecretsFromOtherSessions()) { - // immediately send the requests - outgoingRequestsProcessor.processOutgoingRequests(this.olmMachine.get()) { - it is Request.ToDevice && it.eventType == EventType.REQUEST_SECRET - } - } + Timber.w("SecretShareManager requesting custom secrets not supported $deviceId, $secretName") + // rust stack only support requesting secrets defined in the spec (not custom secret yet) + requestMissingSecrets() } suspend fun requestMissingSecrets() {