From e37dd547b8debc08bcdd191b78b682abe0b79ade Mon Sep 17 00:00:00 2001 From: Valere Date: Wed, 22 Apr 2020 18:50:59 +0200 Subject: [PATCH] code review --- .../android/internal/crypto/UnwedgingTest.kt | 9 ++-- .../api/session/crypto/CryptoService.kt | 2 +- .../internal/crypto/DefaultCryptoService.kt | 46 ++++++------------- .../vector/riotx/features/command/Command.kt | 2 +- .../home/room/detail/RoomDetailViewModel.kt | 2 +- 5 files changed, 20 insertions(+), 41 deletions(-) diff --git a/matrix-sdk-android/src/androidTest/java/im/vector/matrix/android/internal/crypto/UnwedgingTest.kt b/matrix-sdk-android/src/androidTest/java/im/vector/matrix/android/internal/crypto/UnwedgingTest.kt index 7f75d7d6fd..6391c0392c 100644 --- a/matrix-sdk-android/src/androidTest/java/im/vector/matrix/android/internal/crypto/UnwedgingTest.kt +++ b/matrix-sdk-android/src/androidTest/java/im/vector/matrix/android/internal/crypto/UnwedgingTest.kt @@ -88,8 +88,8 @@ class UnwedgingTest : InstrumentedTest { val aliceCryptoStore = (aliceSession.cryptoService() as DefaultCryptoService).cryptoStoreForTesting - //bobSession.cryptoService().setWarnOnUnknownDevices(false) - //aliceSession.cryptoService().setWarnOnUnknownDevices(false) + // bobSession.cryptoService().setWarnOnUnknownDevices(false) + // aliceSession.cryptoService().setWarnOnUnknownDevices(false) val roomFromBobPOV = bobSession.getRoom(aliceRoomId)!! val roomFromAlicePOV = aliceSession.getRoom(aliceRoomId)!! @@ -143,7 +143,7 @@ class UnwedgingTest : InstrumentedTest { val oldSession = serializeForRealm(olmSession.olmSession) - aliceSession.cryptoService().discardOutbundSession(roomFromAlicePOV.roomId) + aliceSession.cryptoService().discardOutboundSession(roomFromAlicePOV.roomId) Thread.sleep(6_000) latch = CountDownLatch(1) @@ -171,7 +171,7 @@ class UnwedgingTest : InstrumentedTest { Thread.sleep(6_000) // Force new session, and key share - aliceSession.cryptoService().discardOutbundSession(roomFromAlicePOV.roomId) + aliceSession.cryptoService().discardOutboundSession(roomFromAlicePOV.roomId) // Wait for the message to be received by Bob mTestHelper.waitWithLatch { @@ -220,7 +220,6 @@ class UnwedgingTest : InstrumentedTest { } } - bobTimeline.dispose() cryptoTestData.cleanUp(mTestHelper) diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/api/session/crypto/CryptoService.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/api/session/crypto/CryptoService.kt index a923b2cc3d..e6fbaaf9a6 100644 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/api/session/crypto/CryptoService.kt +++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/api/session/crypto/CryptoService.kt @@ -111,7 +111,7 @@ interface CryptoService { roomId: String, callback: MatrixCallback) - fun discardOutbundSession(roomId: String) + fun discardOutboundSession(roomId: String) @Throws(MXCryptoError::class) fun decryptEvent(event: Event, timeline: String): MXEventDecryptionResult diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/DefaultCryptoService.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/DefaultCryptoService.kt index e865998fa9..a789814958 100755 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/DefaultCryptoService.kt +++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/DefaultCryptoService.kt @@ -635,7 +635,7 @@ internal class DefaultCryptoService @Inject constructor( } } - override fun discardOutbundSession(roomId: String) { + override fun discardOutboundSession(roomId: String) { cryptoCoroutineScope.launch(coroutineDispatchers.crypto) { roomEncryptorsStore.get(roomId)?.discardSessionKey() } @@ -718,38 +718,6 @@ internal class DefaultCryptoService @Inject constructor( } } -// private fun markOlmSessionForUnwedging(event: Event, mxMegolmDecryption: MXMegolmDecryption) { -// Timber.d("## CRYPTO |  markOlmSessionForUnwedging: ${event.eventId}") -// val senderId = event.senderId ?: return -// val encryptedMessage = event.content.toModel() ?: return -// val deviceKey = encryptedMessage.senderKey ?: return -// encryptedMessage.algorithm?.takeIf { it == MXCRYPTO_ALGORITHM_MEGOLM } ?: return -// -// if (senderId == userId -// && deviceKey == olmDevice.deviceCurve25519Key) { -// Timber.d("## CRYPTO |  markOlmSessionForUnwedging: Do not unwedge ourselves") -// return -// } -// -// val lastForcedDate = lastNewSessionForcedDates.getObject(senderId, deviceKey) ?: 0 -// val now = System.currentTimeMillis() -// if (now - lastForcedDate < CRYPTO_MIN_FORCE_SESSION_PERIOD_MILLIS) { -// Timber.d("## CRYPTO | markOlmSessionForUnwedging: New session already forced with device at $lastForcedDate. Not forcing another") -// return -// } -// -// // Establish a new olm session with this device since we're failing to decrypt messages -// // on a current session. -// val deviceInfo = getDeviceInfo(senderId, deviceKey) ?: return Unit.also { -// Timber.d("## CRYPTO | markOlmSessionForUnwedging: Couldn't find device for identity key $deviceKey: not re-establishing session") -// } -// -// Timber.d("## CRYPTO | markOlmSessionForUnwedging from $senderId:${deviceInfo.deviceId}") -// lastNewSessionForcedDates.setObject(senderId, deviceKey, now) -// -// mxMegolmDecryption.markOlmSessionForUnwedging(senderId, deviceInfo) -// } - /** * Reset replay attack data for the given timeline. * @@ -1197,6 +1165,18 @@ internal class DefaultCryptoService @Inject constructor( } private fun markOlmSessionForUnwedging(senderId: String, deviceInfo: CryptoDeviceInfo) { + val deviceKey = deviceInfo.identityKey() + + val lastForcedDate = lastNewSessionForcedDates.getObject(senderId, deviceKey) ?: 0 + val now = System.currentTimeMillis() + if (now - lastForcedDate < CRYPTO_MIN_FORCE_SESSION_PERIOD_MILLIS) { + Timber.d("## CRYPTO | markOlmSessionForUnwedging: New session already forced with device at $lastForcedDate. Not forcing another") + return + } + + Timber.d("## CRYPTO | markOlmSessionForUnwedging from $senderId:${deviceInfo.deviceId}") + lastNewSessionForcedDates.setObject(senderId, deviceKey, now) + cryptoCoroutineScope.launch(coroutineDispatchers.crypto) { ensureOlmSessionsForDevicesAction.handle(mapOf(senderId to listOf(deviceInfo)), force = true) diff --git a/vector/src/main/java/im/vector/riotx/features/command/Command.kt b/vector/src/main/java/im/vector/riotx/features/command/Command.kt index 4046880ebd..d98ebcfa73 100644 --- a/vector/src/main/java/im/vector/riotx/features/command/Command.kt +++ b/vector/src/main/java/im/vector/riotx/features/command/Command.kt @@ -44,7 +44,7 @@ enum class Command(val command: String, val parameters: String, @StringRes val d POLL("/poll", "Question | Option 1 | Option 2 ...", R.string.command_description_poll), SHRUG("/shrug", "", R.string.command_description_shrug), PLAIN("/plain", "", R.string.command_description_plain), - DISCARD_SESSION("/discardsession","", R.string.command_description_discard_session), + DISCARD_SESSION("/discardsession", "", R.string.command_description_discard_session), // TODO temporary command VERIFY_USER("/verify", "", R.string.command_description_verify); diff --git a/vector/src/main/java/im/vector/riotx/features/home/room/detail/RoomDetailViewModel.kt b/vector/src/main/java/im/vector/riotx/features/home/room/detail/RoomDetailViewModel.kt index 1d74751de5..d0dcac6ecc 100644 --- a/vector/src/main/java/im/vector/riotx/features/home/room/detail/RoomDetailViewModel.kt +++ b/vector/src/main/java/im/vector/riotx/features/home/room/detail/RoomDetailViewModel.kt @@ -449,7 +449,7 @@ class RoomDetailViewModel @AssistedInject constructor( } is ParsedCommand.DiscardSession -> { if (room.isEncrypted()) { - session.cryptoService().discardOutbundSession(room.roomId) + session.cryptoService().discardOutboundSession(room.roomId) _viewEvents.post(RoomDetailViewEvents.SlashCommandHandled()) popDraft() } else {