Fix / Error management and clear keys

matrix-sdk-android/src/androidTest/java/im/vector/matrix/android/internal/crypto/crosssigning/XSigningTest.kt

@@ -3,12 +3,19 @@ package im.vector.matrix.android.internal.crypto.crosssigning
 import androidx.test.ext.junit.runners.AndroidJUnit4
 import im.vector.matrix.android.InstrumentedTest
 import im.vector.matrix.android.api.MatrixCallback
-import im.vector.matrix.android.common.*
+import im.vector.matrix.android.common.CommonTestHelper
+import im.vector.matrix.android.common.CryptoTestHelper
+import im.vector.matrix.android.common.SessionTestParams
+import im.vector.matrix.android.common.TestConstants
+import im.vector.matrix.android.common.TestMatrixCallback
 import im.vector.matrix.android.internal.crypto.model.CryptoDeviceInfo
 import im.vector.matrix.android.internal.crypto.model.MXUsersDevicesMap
-import im.vector.matrix.android.internal.crypto.model.rest.SignatureUploadResponse
 import im.vector.matrix.android.internal.crypto.model.rest.UserPasswordAuth
-import org.junit.Assert.*
+import org.junit.Assert.assertEquals
+import org.junit.Assert.assertNotNull
+import org.junit.Assert.assertNull
+import org.junit.Assert.assertTrue
+import org.junit.Assert.fail
 import org.junit.FixMethodOrder
 import org.junit.Test
 import org.junit.runner.RunWith
@@ -43,7 +50,7 @@ class XSigningTest : InstrumentedTest {
         val userKey = myCrossSigningKeys?.userKey()
         assertNotNull("User key should be stored", userKey?.unpaddedBase64PublicKey)
 
-        assertTrue("Signing Keys should be trusted", myCrossSigningKeys?.isTrusted == true)
+        assertTrue("Signing Keys should be trusted", myCrossSigningKeys?.isTrusted() == true)
 
         assertTrue("Signing Keys should be trusted", aliceSession.getCrossSigningService().checkUserTrust(aliceSession.myUserId).isVerified())
 
@@ -88,7 +95,7 @@ class XSigningTest : InstrumentedTest {
         assertEquals("Bob keys from alice pov should match", bobKeysFromAlicePOV?.masterKey()?.unpaddedBase64PublicKey, bobSession.getCrossSigningService().getMyCrossSigningKeys()?.masterKey()?.unpaddedBase64PublicKey)
         assertEquals("Bob keys from alice pov should match", bobKeysFromAlicePOV?.selfSigningKey()?.unpaddedBase64PublicKey, bobSession.getCrossSigningService().getMyCrossSigningKeys()?.selfSigningKey()?.unpaddedBase64PublicKey)
 
-        assertTrue("Bob keys from alice pov should not be trusted", bobKeysFromAlicePOV?.isTrusted == false)
+        assertTrue("Bob keys from alice pov should not be trusted", bobKeysFromAlicePOV?.isTrusted() == false)
 
         mTestHelper.signout(aliceSession)
         mTestHelper.signout(bobSession)
@@ -126,11 +133,11 @@ class XSigningTest : InstrumentedTest {
         mTestHelper.await(downloadLatch)
 
         val bobKeysFromAlicePOV = aliceSession.getCrossSigningService().getUserCrossSigningKeys(bobUserId)
-        assertTrue("Bob keys from alice pov should not be trusted", bobKeysFromAlicePOV?.isTrusted == false)
+        assertTrue("Bob keys from alice pov should not be trusted", bobKeysFromAlicePOV?.isTrusted() == false)
 
         val trustLatch = CountDownLatch(1)
-        aliceSession.getCrossSigningService().trustUser(bobUserId, object : MatrixCallback<SignatureUploadResponse> {
+        aliceSession.getCrossSigningService().trustUser(bobUserId, object : MatrixCallback<Unit> {
-            override fun onSuccess(data: SignatureUploadResponse) {
+            override fun onSuccess(data: Unit) {
                 trustLatch.countDown()
             }
 
@@ -167,8 +174,8 @@ class XSigningTest : InstrumentedTest {
 
         // Manually mark it as trusted from first session
         val bobSignLatch = CountDownLatch(1)
-        bobSession.getCrossSigningService().signDevice(bobSecondDeviceId!!, object : MatrixCallback<SignatureUploadResponse> {
+        bobSession.getCrossSigningService().signDevice(bobSecondDeviceId!!, object : MatrixCallback<Unit> {
-            override fun onSuccess(data: SignatureUploadResponse) {
+            override fun onSuccess(data: Unit) {
                 bobSignLatch.countDown()
             }
 

matrix-sdk-android/src/main/java/im/vector/matrix/android/api/session/crypto/crosssigning/CrossSigningService.kt

@@ -47,12 +47,12 @@ interface CrossSigningService {
     fun getMyCrossSigningKeys(): MXCrossSigningInfo?
     fun canCrossSign(): Boolean
 
-    fun trustUser(userId: String, callback: MatrixCallback<SignatureUploadResponse>)
+    fun trustUser(userId: String, callback: MatrixCallback<Unit>)
 
     /**
      * Sign one of your devices and upload the signature
      */
-    fun signDevice(deviceId: String, callback: MatrixCallback<SignatureUploadResponse>)
+    fun signDevice(deviceId: String, callback: MatrixCallback<Unit>)
 
     fun checkDeviceTrust(userId: String, deviceId: String, locallyTrusted: Boolean?) : DeviceTrustResult
 }

matrix-sdk-android/src/main/java/im/vector/matrix/android/api/session/crypto/crosssigning/CrossSigningState.kt

@@ -1,38 +0,0 @@
-/*
- * Copyright 2020 New Vector Ltd
- *
- * 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 im.vector.matrix.android.api.session.crypto.crosssigning
-
-/**
- * Defines the account cross signing state.
- *
- */
-enum class CrossSigningState {
-    /** Current state is unknown, need to download user keys from server to resolve */
-    Unknown,
-    /** Currently dowloading user keys*/
-    CheckingState,
-    /** No Cross signing keys are defined on the server */
-    Disabled,
-    /** CrossSigning keys are beeing created and uploaded to the server */
-    Enabling,
-    /** Cross signing keys exists and are trusted*/
-    Trusted,
-    /** Cross signing keys exists but are not yet trusted*/
-    Untrusted,
-    /** The local cross signing keys do not match with the server keys*/
-    Conflicted
-}

matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/crosssigning/DefaultCrossSigningService.kt

@@ -21,7 +21,6 @@ import dagger.Lazy
 import im.vector.matrix.android.api.MatrixCallback
 import im.vector.matrix.android.api.auth.data.Credentials
 import im.vector.matrix.android.api.session.crypto.crosssigning.CrossSigningService
-import im.vector.matrix.android.api.session.crypto.crosssigning.CrossSigningState
 import im.vector.matrix.android.api.session.crypto.crosssigning.MXCrossSigningInfo
 import im.vector.matrix.android.api.util.Optional
 import im.vector.matrix.android.internal.crypto.DeviceListManager
@@ -30,16 +29,12 @@ import im.vector.matrix.android.internal.crypto.MyDeviceInfoHolder
 import im.vector.matrix.android.internal.crypto.model.CryptoCrossSigningKey
 import im.vector.matrix.android.internal.crypto.model.KeyUsage
 import im.vector.matrix.android.internal.crypto.model.rest.KeysQueryResponse
-import im.vector.matrix.android.internal.crypto.model.rest.SignatureUploadResponse
-import im.vector.matrix.android.internal.crypto.model.rest.UploadResponseFailure
 import im.vector.matrix.android.internal.crypto.model.rest.UploadSignatureQueryBuilder
 import im.vector.matrix.android.internal.crypto.model.rest.UserPasswordAuth
 import im.vector.matrix.android.internal.crypto.store.IMXCryptoStore
 import im.vector.matrix.android.internal.crypto.tasks.UploadSignaturesTask
 import im.vector.matrix.android.internal.crypto.tasks.UploadSigningKeysTask
-import im.vector.matrix.android.internal.di.MoshiProvider
 import im.vector.matrix.android.internal.di.UserId
-import im.vector.matrix.android.internal.extensions.foldToCallback
 import im.vector.matrix.android.internal.session.SessionScope
 import im.vector.matrix.android.internal.task.TaskConstraints
 import im.vector.matrix.android.internal.task.TaskExecutor
@@ -47,7 +42,6 @@ import im.vector.matrix.android.internal.task.configureWith
 import im.vector.matrix.android.internal.util.JsonCanonicalizer
 import im.vector.matrix.android.internal.util.MatrixCoroutineDispatchers
 import kotlinx.coroutines.CoroutineScope
-import kotlinx.coroutines.launch
 import org.matrix.olm.OlmPkSigning
 import org.matrix.olm.OlmUtility
 import timber.log.Timber
@@ -69,8 +63,6 @@ internal class DefaultCrossSigningService @Inject constructor(
 
     private var olmUtility: OlmUtility? = null
 
-    private var crossSigningState: CrossSigningState = CrossSigningState.Unknown
-
     private var masterPkSigning: OlmPkSigning? = null
     private var userPkSigning: OlmPkSigning? = null
     private var selfSigningPkSigning: OlmPkSigning? = null
@@ -152,8 +144,6 @@ internal class DefaultCrossSigningService @Inject constructor(
      */
     override fun initializeCrossSigning(authParams: UserPasswordAuth?, callback: MatrixCallback<Unit>?) {
         Timber.d("## CrossSigning  initializeCrossSigning")
-        // TODO sync that
-        crossSigningState = CrossSigningState.Enabling
 
         val myUserID = credentials.userId
 
@@ -222,13 +212,10 @@ internal class DefaultCrossSigningService @Inject constructor(
         cryptoStore.setUserKeysAsTrusted(myUserID)
         cryptoStore.storePrivateKeysInfo(masterKeyPrivateKey?.toBase64NoPadding(), uskPrivateKey?.toBase64NoPadding(), sskPrivateKey?.toBase64NoPadding())
 
-        // TODO we should ensure that they are sent
-        // TODO error handling?
         uploadSigningKeysTask.configureWith(params) {
-            this.retryCount = 3
             this.constraints = TaskConstraints(true)
-            this.callback = object : MatrixCallback<KeysQueryResponse> {
+            this.callback = object : MatrixCallback<Unit> {
-                override fun onSuccess(data: KeysQueryResponse) {
+                override fun onSuccess(data: Unit) {
                     Timber.i("## CrossSigning - Keys succesfully uploaded")
 
                     //  Sign the current device with SSK
@@ -261,44 +248,44 @@ internal class DefaultCrossSigningService @Inject constructor(
 
                     resetTrustOnKeyChange()
                     uploadSignaturesTask.configureWith(UploadSignaturesTask.Params(uploadSignatureQueryBuilder.build())) {
-                        this.retryCount = 3
+                        //this.retryCount = 3
                         this.constraints = TaskConstraints(true)
-                        this.callback = object : MatrixCallback<SignatureUploadResponse> {
+                        this.callback = object : MatrixCallback<Unit> {
-                            override fun onSuccess(data: SignatureUploadResponse) {
+                            override fun onSuccess(data: Unit) {
                                 Timber.i("## CrossSigning - signatures succesfuly uploaded")
-                                // Force download of my keys now
+                                callback?.onSuccess(Unit)
-                                kotlin.runCatching {
-                                    cryptoCoroutineScope.launch(coroutineDispatchers.crypto) {
-                                        deviceListManager.downloadKeys(listOf(myUserID), true)
-                                    }
-                                }.foldToCallback(object : MatrixCallback<Any> {
-                                    override fun onSuccess(data: Any) {
-                                        callback?.onSuccess(Unit)
-                                    }
-
-                                    override fun onFailure(failure: Throwable) {
-                                        callback?.onFailure(failure)
-                                    }
-                                })
                             }
 
                             override fun onFailure(failure: Throwable) {
+                                //Clear
                                 Timber.e(failure, "## CrossSigning - Failed to upload signatures")
+                                clearSigningKeys()
                             }
                         }
                     }.executeBy(taskExecutor)
-
-                    crossSigningState = CrossSigningState.Trusted
                 }
 
                 override fun onFailure(failure: Throwable) {
                     Timber.e(failure, "## CrossSigning - Failed to upload signing keys")
+                    clearSigningKeys()
                     callback?.onFailure(failure)
                 }
             }
         }.executeBy(taskExecutor)
     }
 
+    private fun clearSigningKeys() {
+        this@DefaultCrossSigningService.masterPkSigning?.releaseSigning()
+        this@DefaultCrossSigningService.userPkSigning?.releaseSigning()
+        this@DefaultCrossSigningService.selfSigningPkSigning?.releaseSigning()
+
+        this@DefaultCrossSigningService.masterPkSigning = null
+        this@DefaultCrossSigningService.userPkSigning = null
+        this@DefaultCrossSigningService.selfSigningPkSigning = null
+
+        cryptoStore.setMyCrossSigningInfo(null)
+        cryptoStore.storePrivateKeysInfo(null, null, null)
+    }
 
     private fun resetTrustOnKeyChange() {
         Timber.i("## CrossSigning - Clear all other user trust")
@@ -481,7 +468,7 @@ internal class DefaultCrossSigningService @Inject constructor(
         return checkSelfTrust().isVerified() && cryptoStore.getCrossSigningPrivateKeys()?.selfSigned != null
     }
 
-    override fun trustUser(userId: String, callback: MatrixCallback<SignatureUploadResponse>) {
+    override fun trustUser(userId: String, callback: MatrixCallback<Unit>) {
         Timber.d("## CrossSigning - Mark user $userId as trusted ")
         // We should have this user keys
         val otherMasterKeys = getUserCrossSigningKeys(userId)?.masterKey()
@@ -513,42 +500,16 @@ internal class DefaultCrossSigningService @Inject constructor(
         cryptoStore.setUserKeysAsTrusted(userId, true)
         // TODO update local copy with new signature directly here? kind of local echo of trust?
 
-
         Timber.d("## CrossSigning - Upload signature of $userId MSK signed by USK")
         val uploadQuery = UploadSignatureQueryBuilder()
                 .withSigningKeyInfo(otherMasterKeys.copyForSignature(credentials.userId, userPubKey, newSignature))
                 .build()
         uploadSignaturesTask.configureWith(UploadSignaturesTask.Params(uploadQuery)) {
-            this.callback = object: MatrixCallback<SignatureUploadResponse> {
+            this.callback = callback
-                override fun onSuccess(data: SignatureUploadResponse) {
-                    //force a key download to refresh trust?
-                    val uldata = data
-                    kotlin.runCatching {
-                        Timber.d("## CrossSigning - Force download of user keys")
-                        cryptoCoroutineScope.launch(coroutineDispatchers.crypto) {
-                            deviceListManager.downloadKeys(listOf(userId), true)
-                        }
-                    }.foldToCallback(object : MatrixCallback<Any> {
-                        override fun onSuccess(data: Any) {
-                            callback.onSuccess(uldata)
-                        }
-
-                        override fun onFailure(failure: Throwable) {
-                            Timber.e("## CrossSigning - fail to download keys", failure)
-                            callback.onFailure(failure)
-                        }
-                    })
-                }
-
-                override fun onFailure(failure: Throwable) {
-                    Timber.e("## CrossSigning - fail to upload signature", failure)
-                    callback.onFailure(failure)
-                }
-            }
         }.executeBy(taskExecutor)
     }
 
-    override fun signDevice(deviceId: String, callback: MatrixCallback<SignatureUploadResponse>) {
+    override fun signDevice(deviceId: String, callback: MatrixCallback<Unit>) {
         // This device should be yours
         val device = cryptoStore.getUserDevice(credentials.userId, deviceId)
         if (device == null) {
@@ -590,22 +551,7 @@ internal class DefaultCrossSigningService @Inject constructor(
                 .withDeviceInfo(toUpload)
                 .build()
         uploadSignaturesTask.configureWith(UploadSignaturesTask.Params(uploadQuery)) {
-            this.callback = object : MatrixCallback<SignatureUploadResponse> {
+            this.callback = callback
-                override fun onFailure(failure: Throwable) {
-                    callback.onFailure(failure)
-                }
-
-                override fun onSuccess(data: SignatureUploadResponse) {
-                    val watchedFailure = data.failures?.get(userId)?.get(deviceId)
-                    if (watchedFailure == null) {
-                        callback.onSuccess(data)
-                    } else {
-                        val failure = MoshiProvider.providesMoshi().adapter(UploadResponseFailure::class.java).fromJson(watchedFailure.toString())?.message
-                                ?: watchedFailure.toString()
-                        callback.onFailure(Throwable(failure))
-                    }
-                }
-            }
         }.executeBy(taskExecutor)
     }
 

matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/tasks/UploadSignaturesTask.kt

@@ -23,7 +23,7 @@ import im.vector.matrix.android.internal.task.Task
 import org.greenrobot.eventbus.EventBus
 import javax.inject.Inject
 
-internal interface UploadSignaturesTask : Task<UploadSignaturesTask.Params, SignatureUploadResponse> {
+internal interface UploadSignaturesTask : Task<UploadSignaturesTask.Params, Unit> {
     data class Params(
             val signatures: Map<String, Map<String, Any>>
     )
@@ -34,14 +34,18 @@ internal class DefaultUploadSignaturesTask @Inject constructor(
         private val eventBus: EventBus
 ) : UploadSignaturesTask {
 
-    override suspend fun execute(params: UploadSignaturesTask.Params): SignatureUploadResponse {
+    override suspend fun execute(params: UploadSignaturesTask.Params): Unit {
-        val executeRequest = executeRequest<SignatureUploadResponse>(eventBus) {
+
-            apiCall = cryptoApi.uploadSignatures(params.signatures)
+        try {
+            val response = executeRequest<SignatureUploadResponse>(eventBus) {
+                apiCall = cryptoApi.uploadSignatures(params.signatures)
+            }
+            if (response.failures?.isNotEmpty() == true) {
+                throw Throwable(response.failures.toString())
+            }
+            return
+        } catch (f: Failure) {
+            throw f
         }
-        if (executeRequest.failures?.isNotEmpty() == true) {
-            // TODO better
-            throw Failure.OtherServerError(executeRequest.toString(), 400)
-        }
-        return executeRequest
     }
 }

matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/tasks/UploadSigningKeysTask.kt

@@ -30,7 +30,7 @@ import im.vector.matrix.android.internal.task.Task
 import org.greenrobot.eventbus.EventBus
 import javax.inject.Inject
 
-internal interface UploadSigningKeysTask : Task<UploadSigningKeysTask.Params, KeysQueryResponse> {
+internal interface UploadSigningKeysTask : Task<UploadSigningKeysTask.Params, Unit> {
     data class Params(
             // the device keys to send.
             val masterKey: CryptoCrossSigningKey,
@@ -42,11 +42,13 @@ internal interface UploadSigningKeysTask : Task<UploadSigningKeysTask.Params, Ke
     )
 }
 
+data class UploadSigningKeys(val failures: Map<String, Any>?) : Failure.FeatureFailure()
+
 internal class DefaultUploadSigningKeysTask @Inject constructor(
         private val cryptoApi: CryptoApi,
         private val eventBus: EventBus
 ) : UploadSigningKeysTask {
-    override suspend fun execute(params: UploadSigningKeysTask.Params): KeysQueryResponse {
+    override suspend fun execute(params: UploadSigningKeysTask.Params) {
         val uploadQuery = UploadSigningKeysBody(
                 masterKey = params.masterKey.toRest(),
                 userSigningKey = params.userKey.toRest(),
@@ -55,9 +57,13 @@ internal class DefaultUploadSigningKeysTask @Inject constructor(
         )
         try {
             // Make a first request to start user-interactive authentication
-            return executeRequest(eventBus) {
+            val request = executeRequest<KeysQueryResponse>(eventBus) {
                 apiCall = cryptoApi.uploadSigningKeys(uploadQuery)
             }
+            if (request.failures?.isNotEmpty() == true) {
+                throw UploadSigningKeys(request.failures)
+            }
+            return
         } catch (throwable: Throwable) {
             if (throwable is Failure.OtherServerError
                     && throwable.httpCode == 401
@@ -73,10 +79,18 @@ internal class DefaultUploadSigningKeysTask @Inject constructor(
                     null
                 }?.let {
                     // Retry with authentication
-                    return executeRequest(eventBus) {
+                    try {
-                        apiCall = cryptoApi.uploadSigningKeys(
+                        val req = executeRequest<KeysQueryResponse>(eventBus) {
-                                uploadQuery.copy(auth =  params.userPasswordAuth.copy(session = it.session))
+                            apiCall = cryptoApi.uploadSigningKeys(
-                        )
+                                    uploadQuery.copy(auth = params.userPasswordAuth.copy(session = it.session))
+                            )
+                        }
+                        if (req.failures?.isNotEmpty() == true) {
+                            throw UploadSigningKeys(req.failures)
+                        }
+                        return
+                    } catch (failure: Throwable) {
+                        throw failure
                     }
                 }
             }

matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/verification/SASDefaultVerificationTransaction.kt

@@ -308,7 +308,7 @@ internal abstract class SASDefaultVerificationTransaction(
         if (otherMasterKeyIsVerified && otherUserId != credentials.userId) {
             // we should trust this master key
             // And check verification MSK -> SSK?
-            crossSigningService.trustUser(otherUserId, object : MatrixCallback<SignatureUploadResponse> {
+            crossSigningService.trustUser(otherUserId, object : MatrixCallback<Unit> {
                 override fun onFailure(failure: Throwable) {
                     Timber.e(failure, "## SAS Verification: Failed to trust User $otherUserId")
                 }
@@ -318,7 +318,7 @@ internal abstract class SASDefaultVerificationTransaction(
         if (otherUserId == credentials.userId) {
             // If me it's reasonable to sign and upload the device signature
             // Notice that i might not have the private keys, so may ot be able to do it
-            crossSigningService.signDevice(otherDeviceId!!, object : MatrixCallback<SignatureUploadResponse> {
+            crossSigningService.signDevice(otherDeviceId!!, object : MatrixCallback<Unit> {
                 override fun onFailure(failure: Throwable) {
                     Timber.w(failure, "## SAS Verification: Failed to sign new device $otherDeviceId")
                 }

vector/src/main/java/im/vector/riotx/features/settings/crosssigning/CrossSigningSettingsViewModel.kt

@@ -23,7 +23,6 @@ import com.airbnb.mvrx.FragmentViewModelContext
 import com.airbnb.mvrx.MvRxState
 import com.airbnb.mvrx.MvRxViewModelFactory
 import com.airbnb.mvrx.Success
-import com.airbnb.mvrx.Uninitialized
 import com.airbnb.mvrx.ViewModelContext
 import com.squareup.inject.assisted.Assisted
 import com.squareup.inject.assisted.AssistedInject
@@ -129,7 +128,14 @@ class CrossSigningSettingsViewModel @AssistedInject constructor(@Assisted privat
                         }
                     }
                 }
-                _requestLiveData.postValue(LiveEvent(Fail(Throwable("You cannot do that from mobile"))))
+                when (failure) {
+                    is Failure.ServerError -> {
+                        _requestLiveData.postValue(LiveEvent(Fail(Throwable(failure.error.message))))
+                    }
+                    else                   -> {
+                        _requestLiveData.postValue(LiveEvent(Fail(failure)))
+                    }
+                }
                 setState {
                     copy(isUploadingKeys = false)
                 }