diff --git a/matrix-sdk-android/src/androidTest/java/im/vector/matrix/android/internal/crypto/crosssigning/XSigningTest.kt b/matrix-sdk-android/src/androidTest/java/im/vector/matrix/android/internal/crypto/crosssigning/XSigningTest.kt index d20df0b868..605fcd5f76 100644 --- a/matrix-sdk-android/src/androidTest/java/im/vector/matrix/android/internal/crypto/crosssigning/XSigningTest.kt +++ b/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 { - override fun onSuccess(data: SignatureUploadResponse) { + aliceSession.getCrossSigningService().trustUser(bobUserId, object : MatrixCallback { + 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 { - override fun onSuccess(data: SignatureUploadResponse) { + bobSession.getCrossSigningService().signDevice(bobSecondDeviceId!!, object : MatrixCallback { + override fun onSuccess(data: Unit) { bobSignLatch.countDown() } diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/api/session/crypto/crosssigning/CrossSigningService.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/api/session/crypto/crosssigning/CrossSigningService.kt index 034aea1160..7cf7011672 100644 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/api/session/crypto/crosssigning/CrossSigningService.kt +++ b/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) + fun trustUser(userId: String, callback: MatrixCallback) /** * Sign one of your devices and upload the signature */ - fun signDevice(deviceId: String, callback: MatrixCallback) + fun signDevice(deviceId: String, callback: MatrixCallback) fun checkDeviceTrust(userId: String, deviceId: String, locallyTrusted: Boolean?) : DeviceTrustResult } diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/api/session/crypto/crosssigning/CrossSigningState.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/api/session/crypto/crosssigning/CrossSigningState.kt deleted file mode 100644 index 774aea9f5b..0000000000 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/api/session/crypto/crosssigning/CrossSigningState.kt +++ /dev/null @@ -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 -} diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/crosssigning/DefaultCrossSigningService.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/crosssigning/DefaultCrossSigningService.kt index 5a8532a9fd..dbed2dcc44 100644 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/crosssigning/DefaultCrossSigningService.kt +++ b/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?) { 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 { - override fun onSuccess(data: KeysQueryResponse) { + this.callback = object : MatrixCallback { + 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 { - override fun onSuccess(data: SignatureUploadResponse) { + this.callback = object : MatrixCallback { + override fun onSuccess(data: Unit) { Timber.i("## CrossSigning - signatures succesfuly uploaded") - // Force download of my keys now - kotlin.runCatching { - cryptoCoroutineScope.launch(coroutineDispatchers.crypto) { - deviceListManager.downloadKeys(listOf(myUserID), true) - } - }.foldToCallback(object : MatrixCallback { - override fun onSuccess(data: Any) { - callback?.onSuccess(Unit) - } - - override fun onFailure(failure: Throwable) { - callback?.onFailure(failure) - } - }) + callback?.onSuccess(Unit) } 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) { + override fun trustUser(userId: String, callback: MatrixCallback) { 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 { - 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 { - 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) - } - } + this.callback = callback }.executeBy(taskExecutor) } - override fun signDevice(deviceId: String, callback: MatrixCallback) { + override fun signDevice(deviceId: String, callback: MatrixCallback) { // 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 { - 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)) - } - } - } + this.callback = callback }.executeBy(taskExecutor) } diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/tasks/UploadSignaturesTask.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/tasks/UploadSignaturesTask.kt index 8d4c8a9033..ce74806843 100644 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/tasks/UploadSignaturesTask.kt +++ b/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 { +internal interface UploadSignaturesTask : Task { data class Params( val signatures: Map> ) @@ -34,14 +34,18 @@ internal class DefaultUploadSignaturesTask @Inject constructor( private val eventBus: EventBus ) : UploadSignaturesTask { - override suspend fun execute(params: UploadSignaturesTask.Params): SignatureUploadResponse { - val executeRequest = executeRequest(eventBus) { - apiCall = cryptoApi.uploadSignatures(params.signatures) + override suspend fun execute(params: UploadSignaturesTask.Params): Unit { + + try { + val response = executeRequest(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 } } diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/tasks/UploadSigningKeysTask.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/tasks/UploadSigningKeysTask.kt index 6aec375878..0a69039219 100644 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/tasks/UploadSigningKeysTask.kt +++ b/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 { +internal interface UploadSigningKeysTask : Task { data class Params( // the device keys to send. val masterKey: CryptoCrossSigningKey, @@ -42,11 +42,13 @@ internal interface UploadSigningKeysTask : Task?) : 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(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) { - apiCall = cryptoApi.uploadSigningKeys( - uploadQuery.copy(auth = params.userPasswordAuth.copy(session = it.session)) - ) + try { + val req = executeRequest(eventBus) { + 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 } } } diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/verification/SASDefaultVerificationTransaction.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/verification/SASDefaultVerificationTransaction.kt index c5c296ad6e..806bbc2703 100644 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/verification/SASDefaultVerificationTransaction.kt +++ b/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 { + crossSigningService.trustUser(otherUserId, object : MatrixCallback { 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 { + crossSigningService.signDevice(otherDeviceId!!, object : MatrixCallback { override fun onFailure(failure: Throwable) { Timber.w(failure, "## SAS Verification: Failed to sign new device $otherDeviceId") } diff --git a/vector/src/main/java/im/vector/riotx/features/settings/crosssigning/CrossSigningSettingsViewModel.kt b/vector/src/main/java/im/vector/riotx/features/settings/crosssigning/CrossSigningSettingsViewModel.kt index 12d1134fe3..b7343eac7a 100644 --- a/vector/src/main/java/im/vector/riotx/features/settings/crosssigning/CrossSigningSettingsViewModel.kt +++ b/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) }