From 85b9dda0921b2aa4092287e0352cd3644136ba19 Mon Sep 17 00:00:00 2001 From: valere Date: Fri, 28 Apr 2023 18:58:38 +0200 Subject: [PATCH] Missing backup signature Ensure device keys before bootstrap cross signing --- .../crypto/keysbackup/KeysBackupTest.kt | 10 ++ .../android/sdk/internal/crypto/OlmMachine.kt | 5 +- .../crypto/RustCrossSigningService.kt | 7 ++ .../crypto/keysbackup/RustKeyBackupService.kt | 91 ++++++++++++++----- 4 files changed, 90 insertions(+), 23 deletions(-) diff --git a/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/internal/crypto/keysbackup/KeysBackupTest.kt b/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/internal/crypto/keysbackup/KeysBackupTest.kt index b176afaa29..f573acae82 100644 --- a/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/internal/crypto/keysbackup/KeysBackupTest.kt +++ b/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/internal/crypto/keysbackup/KeysBackupTest.kt @@ -26,6 +26,7 @@ import org.junit.Assert.assertEquals import org.junit.Assert.assertFalse import org.junit.Assert.assertNotNull import org.junit.Assert.assertTrue +import org.junit.Assume import org.junit.FixMethodOrder import org.junit.Ignore import org.junit.Test @@ -128,6 +129,7 @@ class KeysBackupTest : InstrumentedTest { @Test fun createKeysBackupVersionTest() = runCryptoTest(context()) { cryptoTestHelper, testHelper -> val bobSession = testHelper.createAccount(TestConstants.USER_BOB, KeysBackupTestConstants.defaultSessionParams) + Log.d("#E2E", "Initializing crosssigning for ${bobSession.myUserId.take(8)}") cryptoTestHelper.initializeCrossSigning(bobSession) val keysBackup = bobSession.cryptoService().keysBackupService() @@ -136,12 +138,14 @@ class KeysBackupTest : InstrumentedTest { assertFalse(keysBackup.isEnabled()) + Log.d("#E2E", "prepareKeysBackupVersion") val megolmBackupCreationInfo = keysBackup.prepareKeysBackupVersion(null, null) assertFalse(keysBackup.isEnabled()) // Create the version + Log.d("#E2E", "createKeysBackupVersion") val version = keysBackup.createKeysBackupVersion(megolmBackupCreationInfo) // Backup must be enable now @@ -151,6 +155,7 @@ class KeysBackupTest : InstrumentedTest { val versionResult = keysBackup.getVersion(version.version) val trust = keysBackup.getKeysBackupTrust(versionResult!!) + Log.d("#E2E", "Check backup signatures") assertEquals("Should have 2 signatures", 2, trust.signatures.size) trust.signatures @@ -672,11 +677,16 @@ class KeysBackupTest : InstrumentedTest { */ @Test fun testBackupWithPassword() = runCryptoTest(context()) { cryptoTestHelper, testHelper -> + val keysBackupTestHelper = KeysBackupTestHelper(testHelper, cryptoTestHelper) val password = "password" val testData = keysBackupTestHelper.createKeysBackupScenarioWithPassword(password) + Assume.assumeTrue( + "Can't report progress same way in rust", + testData.cryptoTestData.firstSession.cryptoService().name() != "rust-sdk" + ) // - Restore the e2e backup with the password val steps = ArrayList() 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 fc21d49d0d..2ff4d2d119 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 @@ -82,6 +82,7 @@ import org.matrix.rustcomponents.sdk.crypto.RequestType import org.matrix.rustcomponents.sdk.crypto.RoomKeyCounts import org.matrix.rustcomponents.sdk.crypto.ShieldColor import org.matrix.rustcomponents.sdk.crypto.ShieldState +import org.matrix.rustcomponents.sdk.crypto.SignatureVerification import org.matrix.rustcomponents.sdk.crypto.setLogger import timber.log.Timber import java.io.File @@ -916,7 +917,7 @@ internal class OlmMachine @Inject constructor( } @Throws(CryptoStoreException::class) - suspend fun checkAuthDataSignature(authData: KeysAlgorithmAndData): Boolean { + suspend fun checkAuthDataSignature(authData: KeysAlgorithmAndData): SignatureVerification { return withContext(coroutineDispatchers.computation) { val adapter = moshi .newBuilder() @@ -929,7 +930,7 @@ internal class OlmMachine @Inject constructor( ) ) - inner.verifyBackup(serializedAuthData).trusted + inner.verifyBackup(serializedAuthData) } } } diff --git a/matrix-sdk-android/src/rustCrypto/java/org/matrix/android/sdk/internal/crypto/RustCrossSigningService.kt b/matrix-sdk-android/src/rustCrypto/java/org/matrix/android/sdk/internal/crypto/RustCrossSigningService.kt index 1ed8b0f8b0..85c55f31c3 100644 --- a/matrix-sdk-android/src/rustCrypto/java/org/matrix/android/sdk/internal/crypto/RustCrossSigningService.kt +++ b/matrix-sdk-android/src/rustCrypto/java/org/matrix/android/sdk/internal/crypto/RustCrossSigningService.kt @@ -28,10 +28,13 @@ import org.matrix.android.sdk.api.session.crypto.crosssigning.isVerified import org.matrix.android.sdk.api.session.crypto.model.CryptoDeviceInfo import org.matrix.android.sdk.api.session.crypto.model.RoomEncryptionTrustLevel import org.matrix.android.sdk.api.util.Optional +import org.matrix.android.sdk.internal.crypto.network.OutgoingRequestsProcessor +import org.matrix.rustcomponents.sdk.crypto.Request import javax.inject.Inject internal class RustCrossSigningService @Inject constructor( private val olmMachine: OlmMachine, + private val outgoingRequestsProcessor: OutgoingRequestsProcessor, private val computeShieldForGroup: ComputeShieldForGroupUseCase ) : CrossSigningService { @@ -78,6 +81,10 @@ internal class RustCrossSigningService @Inject constructor( * Users needs to enter credentials */ override suspend fun initializeCrossSigning(uiaInterceptor: UserInteractiveAuthInterceptor?) { + // ensure our keys are sent before initialising + outgoingRequestsProcessor.processOutgoingRequests(olmMachine) { + it is Request.KeysUpload + } olmMachine.bootstrapCrossSigning(uiaInterceptor) } diff --git a/matrix-sdk-android/src/rustCrypto/java/org/matrix/android/sdk/internal/crypto/keysbackup/RustKeyBackupService.kt b/matrix-sdk-android/src/rustCrypto/java/org/matrix/android/sdk/internal/crypto/keysbackup/RustKeyBackupService.kt index 7130e9db49..c3d172835a 100644 --- a/matrix-sdk-android/src/rustCrypto/java/org/matrix/android/sdk/internal/crypto/keysbackup/RustKeyBackupService.kt +++ b/matrix-sdk-android/src/rustCrypto/java/org/matrix/android/sdk/internal/crypto/keysbackup/RustKeyBackupService.kt @@ -43,6 +43,7 @@ import org.matrix.android.sdk.api.session.crypto.keysbackup.KeysBackupService import org.matrix.android.sdk.api.session.crypto.keysbackup.KeysBackupState import org.matrix.android.sdk.api.session.crypto.keysbackup.KeysBackupStateListener import org.matrix.android.sdk.api.session.crypto.keysbackup.KeysBackupVersionTrust +import org.matrix.android.sdk.api.session.crypto.keysbackup.KeysBackupVersionTrustSignature import org.matrix.android.sdk.api.session.crypto.keysbackup.KeysVersion import org.matrix.android.sdk.api.session.crypto.keysbackup.KeysVersionResult import org.matrix.android.sdk.api.session.crypto.keysbackup.MegolmBackupAuthData @@ -67,6 +68,8 @@ import org.matrix.android.sdk.internal.util.JsonCanonicalizer import org.matrix.olm.OlmException import org.matrix.rustcomponents.sdk.crypto.Request import org.matrix.rustcomponents.sdk.crypto.RequestType +import org.matrix.rustcomponents.sdk.crypto.SignatureState +import org.matrix.rustcomponents.sdk.crypto.SignatureVerification import timber.log.Timber import java.security.InvalidParameterException import javax.inject.Inject @@ -266,14 +269,56 @@ internal class RustKeyBackupService @Inject constructor( private suspend fun checkBackupTrust(algAndData: KeysAlgorithmAndData?): KeysBackupVersionTrust { if (algAndData == null) return KeysBackupVersionTrust(usable = false) try { - val isTrusted = olmMachine.checkAuthDataSignature(algAndData) - return KeysBackupVersionTrust(isTrusted) + val authData = olmMachine.checkAuthDataSignature(algAndData) + val signatures = authData.mapRustToAPI() + return KeysBackupVersionTrust(authData.trusted, signatures) } catch (failure: Throwable) { Timber.w(failure, "Failed to trust backup") return KeysBackupVersionTrust(usable = false) } } + private suspend fun SignatureVerification.mapRustToAPI(): List { + val signatures = mutableListOf() + // signature state of own device + val ownDeviceState = this.deviceSignature + if (ownDeviceState != SignatureState.MISSING && ownDeviceState != SignatureState.INVALID) { + // we can add it + signatures.add( + KeysBackupVersionTrustSignature.DeviceSignature( + olmMachine.deviceId(), + olmMachine.getCryptoDeviceInfo(olmMachine.userId(), olmMachine.deviceId()), + ownDeviceState == SignatureState.VALID_AND_TRUSTED + ) + ) + } + // signature state of our own identity + val ownIdentityState = this.userIdentitySignature + if (ownIdentityState != SignatureState.MISSING && ownIdentityState != SignatureState.INVALID) { + // we can add it + val masterKey = olmMachine.getIdentity(olmMachine.userId())?.toMxCrossSigningInfo()?.masterKey() + signatures.add( + KeysBackupVersionTrustSignature.UserSignature( + masterKey?.unpaddedBase64PublicKey, + masterKey, + ownIdentityState == SignatureState.VALID_AND_TRUSTED + ) + ) + } + signatures.addAll( + this.otherDevicesSignatures + .filter { it.value == SignatureState.VALID_AND_TRUSTED || it.value == SignatureState.VALID_BUT_NOT_TRUSTED } + .map { + KeysBackupVersionTrustSignature.DeviceSignature( + it.key, + olmMachine.getCryptoDeviceInfo(olmMachine.userId(), it.key), + ownDeviceState == SignatureState.VALID_AND_TRUSTED + ) + } + ) + return signatures + } + override suspend fun getKeysBackupTrust(keysBackupVersion: KeysVersionResult): KeysBackupVersionTrust { return withContext(coroutineDispatchers.crypto) { checkBackupTrust(keysBackupVersion) @@ -341,7 +386,7 @@ internal class RustKeyBackupService @Inject constructor( val authData = getMegolmBackupAuthData(keysBackupData) when { - authData == null -> { + authData == null -> { Timber.w("isValidRecoveryKeyForKeysBackupVersion: Key backup is missing required data") throw IllegalArgumentException("Missing element") } @@ -349,7 +394,7 @@ internal class RustKeyBackupService @Inject constructor( Timber.w("isValidRecoveryKeyForKeysBackupVersion: Public keys mismatch") throw IllegalArgumentException("Invalid recovery key or password") } - else -> { + else -> { // This case is fine, the public key on the server matches the public key the // recovery key produced. } @@ -428,10 +473,10 @@ internal class RustKeyBackupService @Inject constructor( roomId != null && sessionId != null -> { sender.downloadBackedUpKeys(version, roomId, sessionId) } - roomId != null -> { + roomId != null -> { sender.downloadBackedUpKeys(version, roomId) } - else -> { + else -> { sender.downloadBackedUpKeys(version) } } @@ -570,20 +615,24 @@ internal class RustKeyBackupService @Inject constructor( } } - override suspend fun restoreKeysWithRecoveryKey(keysVersionResult: KeysVersionResult, - recoveryKey: IBackupRecoveryKey, - roomId: String?, - sessionId: String?, - stepProgressListener: StepProgressListener?): ImportRoomKeysResult { + override suspend fun restoreKeysWithRecoveryKey( + keysVersionResult: KeysVersionResult, + recoveryKey: IBackupRecoveryKey, + roomId: String?, + sessionId: String?, + stepProgressListener: StepProgressListener? + ): ImportRoomKeysResult { Timber.v("restoreKeysWithRecoveryKey: From backup version: ${keysVersionResult.version}") return restoreBackup(keysVersionResult, recoveryKey, roomId, sessionId, stepProgressListener) } - override suspend fun restoreKeyBackupWithPassword(keysBackupVersion: KeysVersionResult, - password: String, - roomId: String?, - sessionId: String?, - stepProgressListener: StepProgressListener?): ImportRoomKeysResult { + override suspend fun restoreKeyBackupWithPassword( + keysBackupVersion: KeysVersionResult, + password: String, + roomId: String?, + sessionId: String?, + stepProgressListener: StepProgressListener? + ): ImportRoomKeysResult { Timber.v("[MXKeyBackup] restoreKeyBackup with password: From backup version: ${keysBackupVersion.version}") val recoveryKey = withContext(coroutineDispatchers.crypto) { recoveryKeyFromPassword(password, keysBackupVersion) @@ -752,13 +801,13 @@ internal class RustKeyBackupService @Inject constructor( val authData = getMegolmBackupAuthData(keysBackupData) return when { - authData == null -> { + authData == null -> { throw IllegalArgumentException("recoveryKeyFromPassword: invalid parameter") } authData.privateKeySalt.isNullOrBlank() || authData.privateKeyIterations == null -> { throw java.lang.IllegalArgumentException("recoveryKeyFromPassword: Salt and/or iterations not found in key backup auth data") } - else -> { + else -> { BackupRecoveryKey.fromPassphrase(password, authData.privateKeySalt, authData.privateKeyIterations) } } @@ -811,7 +860,7 @@ internal class RustKeyBackupService @Inject constructor( suspend fun maybeBackupKeys() { withContext(coroutineDispatchers.crypto) { when { - isStuck() -> { + isStuck() -> { // If not already done, or in error case, check for a valid backup version on the homeserver. // If there is one, maybeBackupKeys will be called again. checkAndStartKeysBackup() @@ -829,7 +878,7 @@ internal class RustKeyBackupService @Inject constructor( tryOrNull("AUTO backup failed") { backupKeys() } } } - else -> { + else -> { Timber.v("maybeBackupKeys: Skip it because state: ${getState()}") } } @@ -907,7 +956,7 @@ internal class RustKeyBackupService @Inject constructor( // is available on the homeserver checkAndStartKeysBackup() } - else -> + else -> // Come back to the ready state so that we will retry on the next received key keysBackupStateManager.state = KeysBackupState.ReadyToBackUp }