Attempt to properly cancel the crypto module when user signs out (#724)

Attempt to properly cancel the crypto module when user signs out (#724)
This commit is contained in:
Benoit Marty 2019-12-04 14:39:47 +01:00
parent 3c6eb4bccf
commit 3623072f08
11 changed files with 94 additions and 64 deletions

View file

@ -15,6 +15,7 @@ Bugfix 🐛:
- Do not show long click help if only invitation are displayed
- Fix emoji filtering not working
- Fix issue of closing Realm in another thread (#725)
- Attempt to properly cancel the crypto module when user signs out (#724)
Translations 🗣:
-

View file

@ -17,6 +17,7 @@
package im.vector.matrix.android.api.session.file
import im.vector.matrix.android.api.MatrixCallback
import im.vector.matrix.android.api.util.Cancelable
import im.vector.matrix.android.internal.crypto.attachments.ElementToDecrypt
import java.io.File
@ -47,5 +48,5 @@ interface FileService {
fileName: String,
url: String?,
elementToDecrypt: ElementToDecrypt?,
callback: MatrixCallback<File>)
callback: MatrixCallback<File>): Cancelable
}

View file

@ -37,6 +37,8 @@ import im.vector.matrix.android.internal.session.SessionScope
import im.vector.matrix.android.internal.session.cache.ClearCacheTask
import im.vector.matrix.android.internal.session.cache.RealmClearCacheTask
import io.realm.RealmConfiguration
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.SupervisorJob
import retrofit2.Retrofit
import java.io.File
@ -66,6 +68,13 @@ internal abstract class CryptoModule {
.build()
}
@JvmStatic
@Provides
@SessionScope
fun providesCryptoCoroutineScope(): CoroutineScope {
return CoroutineScope(SupervisorJob())
}
@JvmStatic
@Provides
@CryptoDatabase

View file

@ -132,7 +132,8 @@ internal class DefaultCryptoService @Inject constructor(
private val loadRoomMembersTask: LoadRoomMembersTask,
private val monarchy: Monarchy,
private val coroutineDispatchers: MatrixCoroutineDispatchers,
private val taskExecutor: TaskExecutor
private val taskExecutor: TaskExecutor,
private val cryptoCoroutineScope: CoroutineScope
) : CryptoService {
private val uiHandler = Handler(Looper.getMainLooper())
@ -243,7 +244,8 @@ internal class DefaultCryptoService @Inject constructor(
return
}
isStarting.set(true)
GlobalScope.launch(coroutineDispatchers.crypto) {
cryptoCoroutineScope.launch(coroutineDispatchers.crypto) {
internalStart(isInitialSync)
}
}
@ -269,10 +271,9 @@ internal class DefaultCryptoService @Inject constructor(
isStarted.set(true)
},
{
Timber.e("Start failed: $it")
delay(1000)
isStarting.set(false)
internalStart(isInitialSync)
isStarted.set(false)
Timber.e(it, "Start failed")
}
)
}
@ -281,9 +282,12 @@ internal class DefaultCryptoService @Inject constructor(
* Close the crypto
*/
fun close() = runBlocking(coroutineDispatchers.crypto) {
cryptoCoroutineScope.coroutineContext.cancelChildren(CancellationException("Closing crypto module"))
outgoingRoomKeyRequestManager.stop()
olmDevice.release()
cryptoStore.close()
outgoingRoomKeyRequestManager.stop()
}
// Aways enabled on RiotX
@ -305,19 +309,21 @@ internal class DefaultCryptoService @Inject constructor(
* @param syncResponse the syncResponse
*/
fun onSyncCompleted(syncResponse: SyncResponse) {
GlobalScope.launch(coroutineDispatchers.crypto) {
if (syncResponse.deviceLists != null) {
deviceListManager.handleDeviceListsChanges(syncResponse.deviceLists.changed, syncResponse.deviceLists.left)
}
if (syncResponse.deviceOneTimeKeysCount != null) {
val currentCount = syncResponse.deviceOneTimeKeysCount.signedCurve25519 ?: 0
oneTimeKeysUploader.updateOneTimeKeyCount(currentCount)
}
if (isStarted()) {
// Make sure we process to-device messages before generating new one-time-keys #2782
deviceListManager.refreshOutdatedDeviceLists()
oneTimeKeysUploader.maybeUploadOneTimeKeys()
incomingRoomKeyRequestManager.processReceivedRoomKeyRequests()
cryptoCoroutineScope.launch(coroutineDispatchers.crypto) {
runCatching {
if (syncResponse.deviceLists != null) {
deviceListManager.handleDeviceListsChanges(syncResponse.deviceLists.changed, syncResponse.deviceLists.left)
}
if (syncResponse.deviceOneTimeKeysCount != null) {
val currentCount = syncResponse.deviceOneTimeKeysCount.signedCurve25519 ?: 0
oneTimeKeysUploader.updateOneTimeKeyCount(currentCount)
}
if (isStarted()) {
// Make sure we process to-device messages before generating new one-time-keys #2782
deviceListManager.refreshOutdatedDeviceLists()
oneTimeKeysUploader.maybeUploadOneTimeKeys()
incomingRoomKeyRequestManager.processReceivedRoomKeyRequests()
}
}
}
}
@ -511,7 +517,7 @@ internal class DefaultCryptoService @Inject constructor(
eventType: String,
roomId: String,
callback: MatrixCallback<MXEncryptEventContentResult>) {
GlobalScope.launch(coroutineDispatchers.crypto) {
cryptoCoroutineScope.launch(coroutineDispatchers.crypto) {
if (!isStarted()) {
Timber.v("## encryptEventContent() : wait after e2e init")
internalStart(false)
@ -571,7 +577,7 @@ internal class DefaultCryptoService @Inject constructor(
* @param callback the callback to return data or null
*/
override fun decryptEventAsync(event: Event, timeline: String, callback: MatrixCallback<MXEventDecryptionResult>) {
GlobalScope.launch {
cryptoCoroutineScope.launch {
val result = runCatching {
withContext(coroutineDispatchers.crypto) {
internalDecryptEvent(event, timeline)
@ -621,7 +627,7 @@ internal class DefaultCryptoService @Inject constructor(
* @param event the event
*/
fun onToDeviceEvent(event: Event) {
GlobalScope.launch(coroutineDispatchers.crypto) {
cryptoCoroutineScope.launch(coroutineDispatchers.crypto) {
when (event.getClearType()) {
EventType.ROOM_KEY, EventType.FORWARDED_ROOM_KEY -> {
onRoomKeyEvent(event)
@ -661,7 +667,7 @@ internal class DefaultCryptoService @Inject constructor(
* @param event the encryption event.
*/
private fun onRoomEncryptionEvent(roomId: String, event: Event) {
GlobalScope.launch(coroutineDispatchers.crypto) {
cryptoCoroutineScope.launch(coroutineDispatchers.crypto) {
val params = LoadRoomMembersTask.Params(roomId)
try {
loadRoomMembersTask.execute(params)
@ -753,7 +759,7 @@ internal class DefaultCryptoService @Inject constructor(
* @param callback the exported keys
*/
override fun exportRoomKeys(password: String, callback: MatrixCallback<ByteArray>) {
GlobalScope.launch(coroutineDispatchers.main) {
cryptoCoroutineScope.launch(coroutineDispatchers.main) {
runCatching {
exportRoomKeys(password, MXMegolmExportEncryption.DEFAULT_ITERATION_COUNT)
}.foldToCallback(callback)
@ -791,7 +797,7 @@ internal class DefaultCryptoService @Inject constructor(
password: String,
progressListener: ProgressListener?,
callback: MatrixCallback<ImportRoomKeysResult>) {
GlobalScope.launch(coroutineDispatchers.main) {
cryptoCoroutineScope.launch(coroutineDispatchers.main) {
runCatching {
withContext(coroutineDispatchers.crypto) {
Timber.v("## importRoomKeys starts")
@ -839,7 +845,7 @@ internal class DefaultCryptoService @Inject constructor(
*/
fun checkUnknownDevices(userIds: List<String>, callback: MatrixCallback<Unit>) {
// force the refresh to ensure that the devices list is up-to-date
GlobalScope.launch(coroutineDispatchers.crypto) {
cryptoCoroutineScope.launch(coroutineDispatchers.crypto) {
runCatching {
val keys = deviceListManager.downloadKeys(userIds, true)
val unknownDevices = getUnknownDevices(keys)
@ -999,7 +1005,7 @@ internal class DefaultCryptoService @Inject constructor(
}
override fun downloadKeys(userIds: List<String>, forceDownload: Boolean, callback: MatrixCallback<MXUsersDevicesMap<MXDeviceInfo>>) {
GlobalScope.launch(coroutineDispatchers.crypto) {
cryptoCoroutineScope.launch(coroutineDispatchers.crypto) {
runCatching {
deviceListManager.downloadKeys(userIds, forceDownload)
}.foldToCallback(callback)

View file

@ -25,7 +25,6 @@ import im.vector.matrix.android.internal.crypto.store.IMXCryptoStore
import im.vector.matrix.android.internal.session.SessionScope
import timber.log.Timber
import javax.inject.Inject
import kotlin.collections.ArrayList
@SessionScope
internal class IncomingRoomKeyRequestManager @Inject constructor(
@ -51,7 +50,7 @@ internal class IncomingRoomKeyRequestManager @Inject constructor(
*
* @param event the announcement event.
*/
suspend fun onRoomKeyRequestEvent(event: Event) {
fun onRoomKeyRequestEvent(event: Event) {
val roomKeyShare = event.getClearContent().toModel<RoomKeyShare>()
when (roomKeyShare?.action) {
RoomKeyShare.ACTION_SHARE_REQUEST -> receivedRoomKeyRequests.add(IncomingRoomKeyRequest(event))
@ -139,7 +138,7 @@ internal class IncomingRoomKeyRequestManager @Inject constructor(
if (null != receivedRoomKeyRequestCancellations) {
for (request in receivedRoomKeyRequestCancellations!!) {
Timber.v("## ## processReceivedRoomKeyRequests() : m.room_key_request cancellation for " + request.userId
+ ":" + request.deviceId + " id " + request.requestId)
+ ":" + request.deviceId + " id " + request.requestId)
// we should probably only notify the app of cancellations we told it
// about, but we don't currently have a record of that, so we just pass

View file

@ -63,6 +63,7 @@ internal class OutgoingRoomKeyRequestManager @Inject constructor(
*/
fun stop() {
isClientRunning = false
stopTimer()
}
/**
@ -171,6 +172,10 @@ internal class OutgoingRoomKeyRequestManager @Inject constructor(
}, SEND_KEY_REQUESTS_DELAY_MS.toLong())
}
private fun stopTimer() {
BACKGROUND_HANDLER.removeCallbacksAndMessages(null)
}
// look for and send any queued requests. Runs itself recursively until
// there are no more requests, or there is an error (in which case, the
// timer will be restarted before the promise resolves).

View file

@ -34,7 +34,7 @@ import im.vector.matrix.android.internal.crypto.model.rest.RoomKeyRequestBody
import im.vector.matrix.android.internal.crypto.store.IMXCryptoStore
import im.vector.matrix.android.internal.crypto.tasks.SendToDeviceTask
import im.vector.matrix.android.internal.util.MatrixCoroutineDispatchers
import kotlinx.coroutines.GlobalScope
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.launch
import timber.log.Timber
@ -46,8 +46,9 @@ internal class MXMegolmDecryption(private val userId: String,
private val ensureOlmSessionsForDevicesAction: EnsureOlmSessionsForDevicesAction,
private val cryptoStore: IMXCryptoStore,
private val sendToDeviceTask: SendToDeviceTask,
private val coroutineDispatchers: MatrixCoroutineDispatchers)
: IMXDecrypting {
private val coroutineDispatchers: MatrixCoroutineDispatchers,
private val cryptoCoroutineScope: CoroutineScope
) : IMXDecrypting {
var newSessionListener: NewSessionListener? = null
@ -61,7 +62,7 @@ internal class MXMegolmDecryption(private val userId: String,
return decryptEvent(event, timeline, true)
}
private suspend fun decryptEvent(event: Event, timeline: String, requestKeysOnFail: Boolean): MXEventDecryptionResult {
private fun decryptEvent(event: Event, timeline: String, requestKeysOnFail: Boolean): MXEventDecryptionResult {
if (event.roomId.isNullOrBlank()) {
throw MXCryptoError.Base(MXCryptoError.ErrorType.MISSING_FIELDS, MXCryptoError.MISSING_FIELDS_REASON)
}
@ -292,7 +293,7 @@ internal class MXMegolmDecryption(private val userId: String,
return
}
val userId = request.userId ?: return
GlobalScope.launch(coroutineDispatchers.crypto) {
cryptoCoroutineScope.launch(coroutineDispatchers.crypto) {
runCatching { deviceListManager.downloadKeys(listOf(userId), false) }
.mapCatching {
val deviceId = request.deviceId

View file

@ -25,17 +25,21 @@ import im.vector.matrix.android.internal.crypto.store.IMXCryptoStore
import im.vector.matrix.android.internal.crypto.tasks.SendToDeviceTask
import im.vector.matrix.android.internal.di.UserId
import im.vector.matrix.android.internal.util.MatrixCoroutineDispatchers
import kotlinx.coroutines.CoroutineScope
import javax.inject.Inject
internal class MXMegolmDecryptionFactory @Inject constructor(@UserId private val userId: String,
private val olmDevice: MXOlmDevice,
private val deviceListManager: DeviceListManager,
private val outgoingRoomKeyRequestManager: OutgoingRoomKeyRequestManager,
private val messageEncrypter: MessageEncrypter,
private val ensureOlmSessionsForDevicesAction: EnsureOlmSessionsForDevicesAction,
private val cryptoStore: IMXCryptoStore,
private val sendToDeviceTask: SendToDeviceTask,
private val coroutineDispatchers: MatrixCoroutineDispatchers) {
internal class MXMegolmDecryptionFactory @Inject constructor(
@UserId private val userId: String,
private val olmDevice: MXOlmDevice,
private val deviceListManager: DeviceListManager,
private val outgoingRoomKeyRequestManager: OutgoingRoomKeyRequestManager,
private val messageEncrypter: MessageEncrypter,
private val ensureOlmSessionsForDevicesAction: EnsureOlmSessionsForDevicesAction,
private val cryptoStore: IMXCryptoStore,
private val sendToDeviceTask: SendToDeviceTask,
private val coroutineDispatchers: MatrixCoroutineDispatchers,
private val cryptoCoroutineScope: CoroutineScope
) {
fun create(): MXMegolmDecryption {
return MXMegolmDecryption(
@ -47,6 +51,7 @@ internal class MXMegolmDecryptionFactory @Inject constructor(@UserId private val
ensureOlmSessionsForDevicesAction,
cryptoStore,
sendToDeviceTask,
coroutineDispatchers)
coroutineDispatchers,
cryptoCoroutineScope)
}
}

View file

@ -59,7 +59,7 @@ 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 im.vector.matrix.android.internal.util.awaitCallback
import kotlinx.coroutines.GlobalScope
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.launch
import kotlinx.coroutines.withContext
import org.matrix.olm.OlmException
@ -102,7 +102,8 @@ internal class KeysBackup @Inject constructor(
private val updateKeysBackupVersionTask: UpdateKeysBackupVersionTask,
// Task executor
private val taskExecutor: TaskExecutor,
private val coroutineDispatchers: MatrixCoroutineDispatchers
private val coroutineDispatchers: MatrixCoroutineDispatchers,
private val cryptoCoroutineScope: CoroutineScope
) : KeysBackupService {
private val uiHandler = Handler(Looper.getMainLooper())
@ -143,7 +144,7 @@ internal class KeysBackup @Inject constructor(
override fun prepareKeysBackupVersion(password: String?,
progressListener: ProgressListener?,
callback: MatrixCallback<MegolmBackupCreationInfo>) {
GlobalScope.launch(coroutineDispatchers.main) {
cryptoCoroutineScope.launch(coroutineDispatchers.main) {
runCatching {
withContext(coroutineDispatchers.crypto) {
val olmPkDecryption = OlmPkDecryption()
@ -233,7 +234,7 @@ internal class KeysBackup @Inject constructor(
}
override fun deleteBackup(version: String, callback: MatrixCallback<Unit>?) {
GlobalScope.launch(coroutineDispatchers.main) {
cryptoCoroutineScope.launch(coroutineDispatchers.main) {
withContext(coroutineDispatchers.crypto) {
// If we're currently backing up to this backup... stop.
// (We start using it automatically in createKeysBackupVersion so this is symmetrical).
@ -448,7 +449,7 @@ internal class KeysBackup @Inject constructor(
callback.onFailure(IllegalArgumentException("Missing element"))
} else {
GlobalScope.launch(coroutineDispatchers.main) {
cryptoCoroutineScope.launch(coroutineDispatchers.main) {
val updateKeysBackupVersionBody = withContext(coroutineDispatchers.crypto) {
// Get current signatures, or create an empty set
val myUserSignatures = authData.signatures?.get(userId)?.toMutableMap()
@ -523,7 +524,7 @@ internal class KeysBackup @Inject constructor(
callback: MatrixCallback<Unit>) {
Timber.v("trustKeysBackupVersionWithRecoveryKey: version ${keysBackupVersion.version}")
GlobalScope.launch(coroutineDispatchers.main) {
cryptoCoroutineScope.launch(coroutineDispatchers.main) {
val isValid = withContext(coroutineDispatchers.crypto) {
isValidRecoveryKeyForKeysBackupVersion(recoveryKey, keysBackupVersion)
}
@ -543,7 +544,7 @@ internal class KeysBackup @Inject constructor(
callback: MatrixCallback<Unit>) {
Timber.v("trustKeysBackupVersionWithPassphrase: version ${keysBackupVersion.version}")
GlobalScope.launch(coroutineDispatchers.main) {
cryptoCoroutineScope.launch(coroutineDispatchers.main) {
val recoveryKey = withContext(coroutineDispatchers.crypto) {
recoveryKeyFromPassword(password, keysBackupVersion, null)
}
@ -614,7 +615,7 @@ internal class KeysBackup @Inject constructor(
callback: MatrixCallback<ImportRoomKeysResult>) {
Timber.v("restoreKeysWithRecoveryKey: From backup version: ${keysVersionResult.version}")
GlobalScope.launch(coroutineDispatchers.main) {
cryptoCoroutineScope.launch(coroutineDispatchers.main) {
runCatching {
val decryption = withContext(coroutineDispatchers.crypto) {
// Check if the recovery is valid before going any further
@ -695,7 +696,7 @@ internal class KeysBackup @Inject constructor(
callback: MatrixCallback<ImportRoomKeysResult>) {
Timber.v("[MXKeyBackup] restoreKeyBackup with password: From backup version: ${keysBackupVersion.version}")
GlobalScope.launch(coroutineDispatchers.main) {
cryptoCoroutineScope.launch(coroutineDispatchers.main) {
runCatching {
val progressListener = if (stepProgressListener != null) {
object : ProgressListener {
@ -729,8 +730,8 @@ internal class KeysBackup @Inject constructor(
* parameters and always returns a KeysBackupData object through the Callback
*/
private suspend fun getKeys(sessionId: String?,
roomId: String?,
version: String): KeysBackupData {
roomId: String?,
version: String): KeysBackupData {
return if (roomId != null && sessionId != null) {
// Get key for the room and for the session
val data = getRoomSessionDataTask.execute(GetRoomSessionDataTask.Params(roomId, sessionId, version))
@ -1154,7 +1155,7 @@ internal class KeysBackup @Inject constructor(
keysBackupStateManager.state = KeysBackupState.BackingUp
GlobalScope.launch(coroutineDispatchers.main) {
cryptoCoroutineScope.launch(coroutineDispatchers.main) {
withContext(coroutineDispatchers.crypto) {
Timber.v("backupKeys: 2 - Encrypting keys")

View file

@ -53,10 +53,10 @@ internal class DefaultUploadKeysTask @Inject constructor(private val cryptoApi:
}
return executeRequest {
if (encodedDeviceId.isNullOrBlank()) {
apiCall = cryptoApi.uploadKeys(body)
apiCall = if (encodedDeviceId.isBlank()) {
cryptoApi.uploadKeys(body)
} else {
apiCall = cryptoApi.uploadKeys(encodedDeviceId, body)
cryptoApi.uploadKeys(encodedDeviceId, body)
}
}
}

View file

@ -22,12 +22,14 @@ import arrow.core.Try
import im.vector.matrix.android.api.MatrixCallback
import im.vector.matrix.android.api.session.content.ContentUrlResolver
import im.vector.matrix.android.api.session.file.FileService
import im.vector.matrix.android.api.util.Cancelable
import im.vector.matrix.android.internal.crypto.attachments.ElementToDecrypt
import im.vector.matrix.android.internal.crypto.attachments.MXEncryptedAttachments
import im.vector.matrix.android.internal.di.UserMd5
import im.vector.matrix.android.internal.extensions.foldToCallback
import im.vector.matrix.android.internal.util.MatrixCoroutineDispatchers
import im.vector.matrix.android.internal.util.md5
import im.vector.matrix.android.internal.util.toCancelable
import im.vector.matrix.android.internal.util.writeToFile
import kotlinx.coroutines.GlobalScope
import kotlinx.coroutines.launch
@ -55,8 +57,8 @@ internal class DefaultFileService @Inject constructor(private val context: Conte
fileName: String,
url: String?,
elementToDecrypt: ElementToDecrypt?,
callback: MatrixCallback<File>) {
GlobalScope.launch(coroutineDispatchers.main) {
callback: MatrixCallback<File>): Cancelable {
return GlobalScope.launch(coroutineDispatchers.main) {
withContext(coroutineDispatchers.io) {
Try {
val folder = getFolder(downloadMode, id)
@ -96,7 +98,7 @@ internal class DefaultFileService @Inject constructor(private val context: Conte
}
}
.foldToCallback(callback)
}
}.toCancelable()
}
private fun getFolder(downloadMode: FileService.DownloadMode, id: String): File {