Skip to content

Commit a6368c4

Browse files
bmartybmarty
committed
Restart broken Olm sessions ([MSC1719](matrix-org/matrix-spec-proposals#1719))
1 parent 3615ca6 commit a6368c4

File tree

12 files changed

+202
-21
lines changed

12 files changed

+202
-21
lines changed

‎CHANGES.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ Improvements 🙌:
2222
- Emoji Verification | It's not the same butterfly! (#1220)
2323
- Cross-Signing | Composer decoration: shields (#1077)
2424
- Cross-Signing | Migrate existing keybackup to cross signing with 4S from mobile (#1197)
25+
- Restart broken Olm sessions ([MSC1719](https://github.com/matrix-org/matrix-doc/pull/1719))
2526

2627
Bugfix 🐛:
2728
- Fix summary notification staying after "mark as read"

‎matrix-sdk-android/src/androidTest/java/im/vector/matrix/android/internal/crypto/UnwedgingTest.kt

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,10 +27,12 @@ import im.vector.matrix.android.common.CryptoTestHelper
2727
import org.amshove.kluent.shouldBe
2828
import org.amshove.kluent.shouldBeEqualTo
2929
import org.junit.Before
30+
import org.junit.BeforeClass
3031
import org.junit.FixMethodOrder
3132
import org.junit.Test
3233
import org.junit.runner.RunWith
3334
import org.junit.runners.MethodSorters
35+
import timber.log.Timber
3436
import java.util.concurrent.CountDownLatch
3537

3638
/**
@@ -99,13 +101,13 @@ class UnwedgingTest : InstrumentedTest {
99101

100102
override fun onTimelineUpdated(snapshot: List<TimelineEvent>) {
101103
val decryptedEventReceivedByBob = snapshot.filter { it.root.getClearType() == EventType.MESSAGE }
104+
Timber.d("Bob can now decrypt ${decryptedEventReceivedByBob.size} messages")
102105
if (decryptedEventReceivedByBob.size == 3) {
103106
bobFinalLatch.countDown()
104107
}
105108
}
106109
}
107110
bobTimeline.addListener(bobHasThreeDecryptedEventsListener)
108-
109111

110112
var latch = CountDownLatch(1)
111113
var bobEventsListener = createEventListener(latch, 1)
@@ -128,7 +130,7 @@ class UnwedgingTest : InstrumentedTest {
128130
sessionIdsForBob!!.size shouldBe 1
129131
val olmSession = aliceCryptoStore.getDeviceSession(sessionIdsForBob.first(), bobSession.cryptoService().getMyDevice().identityKey()!!)!!
130132

131-
// Sam join the room
133+
// Sam join the room, so it will force a new session creation
132134
val samSession = mCryptoTestHelper.createSamAccountAndInviteToTheRoom(roomFromAlicePOV)
133135

134136
latch = CountDownLatch(1)

‎matrix-sdk-android/src/main/java/im/vector/matrix/android/api/session/events/model/EventType.kt

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,9 @@ object EventType {
8181
// Relation Events
8282
const val REACTION = "m.reaction"
8383

84+
// Unwedging
85+
internal const val DUMMY = "m.dummy"
86+
8487
private val STATE_EVENTS = listOf(
8588
STATE_ROOM_NAME,
8689
STATE_ROOM_TOPIC,

‎matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/DefaultCryptoService.kt

Lines changed: 51 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ import im.vector.matrix.android.api.session.room.model.RoomMemberSummary
4848
import im.vector.matrix.android.internal.crypto.actions.MegolmSessionDataImporter
4949
import im.vector.matrix.android.internal.crypto.actions.SetDeviceVerificationAction
5050
import im.vector.matrix.android.internal.crypto.algorithms.IMXEncrypting
51+
import im.vector.matrix.android.internal.crypto.algorithms.megolm.MXMegolmDecryption
5152
import im.vector.matrix.android.internal.crypto.algorithms.megolm.MXMegolmEncryptionFactory
5253
import im.vector.matrix.android.internal.crypto.algorithms.olm.MXOlmEncryptionFactory
5354
import im.vector.matrix.android.internal.crypto.crosssigning.DefaultCrossSigningService
@@ -179,6 +180,10 @@ internal class DefaultCryptoService @Inject constructor(
179180
private val isStarting = AtomicBoolean(false)
180181
private val isStarted = AtomicBoolean(false)
181182

183+
// The date of the last time we forced establishment
184+
// of a new session for each user:device.
185+
private val lastNewSessionForcedDates = MXUsersDevicesMap<Long>()
186+
182187
fun onStateEvent(roomId: String, event: Event) {
183188
when {
184189
event.getClearType() == EventType.STATE_ROOM_ENCRYPTION -> onRoomEncryptionEvent(roomId, event)
@@ -675,11 +680,52 @@ internal class DefaultCryptoService @Inject constructor(
675680
Timber.e("## decryptEvent() : $reason")
676681
throw MXCryptoError.Base(MXCryptoError.ErrorType.UNABLE_TO_DECRYPT, reason)
677682
} else {
678-
return alg.decryptEvent(event, timeline)
683+
try {
684+
return alg.decryptEvent(event, timeline)
685+
} catch (mxCryptoError: MXCryptoError) {
686+
if (mxCryptoError is MXCryptoError.Base
687+
&& mxCryptoError.errorType == MXCryptoError.ErrorType.BAD_ENCRYPTED_MESSAGE
688+
&& alg is MXMegolmDecryption) {
689+
// TODO Do it on decryption thread like on iOS?
690+
markOlmSessionForUnwedging(event, alg)
691+
}
692+
throw mxCryptoError
693+
}
679694
}
680695
}
681696
}
682697

698+
private fun markOlmSessionForUnwedging(event: Event, mxMegolmDecryption: MXMegolmDecryption) {
699+
val senderId = event.senderId ?: return
700+
val encryptedMessage = event.content.toModel<EncryptedEventContent>() ?: return
701+
val deviceKey = encryptedMessage.senderKey ?: return
702+
encryptedMessage.algorithm?.takeIf { it == MXCRYPTO_ALGORITHM_MEGOLM } ?: return
703+
704+
if (senderId == userId
705+
&& deviceKey == olmDevice.deviceCurve25519Key) {
706+
Timber.d("[MXCrypto] markOlmSessionForUnwedging: Do not unwedge ourselves")
707+
return
708+
}
709+
710+
val lastForcedDate = lastNewSessionForcedDates.getObject(senderId, deviceKey) ?: 0
711+
val now = System.currentTimeMillis()
712+
if (now - lastForcedDate < CRYPTO_MIN_FORCE_SESSION_PERIOD_MILLIS) {
713+
Timber.d("[MXCrypto] markOlmSessionForUnwedging: New session already forced with device at $lastForcedDate. Not forcing another")
714+
return
715+
}
716+
717+
// Establish a new olm session with this device since we're failing to decrypt messages
718+
// on a current session.
719+
val deviceInfo = getDeviceInfo(senderId, deviceKey) ?: return Unit.also {
720+
Timber.d("[MXCrypto] markOlmSessionForUnwedging: Couldn't find device for identity key $deviceKey: not re-establishing session")
721+
}
722+
723+
Timber.d("[MXCrypto] markOlmSessionForUnwedging from $senderId:${deviceInfo.deviceId}")
724+
lastNewSessionForcedDates.setObject(senderId, deviceKey, now)
725+
726+
mxMegolmDecryption.markOlmSessionForUnwedging(senderId, deviceInfo)
727+
}
728+
683729
/**
684730
* Reset replay attack data for the given timeline.
685731
*
@@ -1189,4 +1235,8 @@ internal class DefaultCryptoService @Inject constructor(
11891235

11901236
@VisibleForTesting
11911237
val cryptoStoreForTesting = cryptoStore
1238+
1239+
companion object {
1240+
const val CRYPTO_MIN_FORCE_SESSION_PERIOD_MILLIS = 3_600_000 // one hour
1241+
}
11921242
}

‎matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/IncomingGossipingRequestManager.kt

Lines changed: 42 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,10 @@ import im.vector.matrix.android.internal.crypto.model.rest.GossipingToDeviceObje
3232
import im.vector.matrix.android.internal.crypto.store.IMXCryptoStore
3333
import im.vector.matrix.android.internal.di.SessionId
3434
import im.vector.matrix.android.internal.session.SessionScope
35+
import im.vector.matrix.android.internal.util.MatrixCoroutineDispatchers
3536
import im.vector.matrix.android.internal.worker.WorkerParamsFactory
37+
import kotlinx.coroutines.CoroutineScope
38+
import kotlinx.coroutines.launch
3639
import timber.log.Timber
3740
import javax.inject.Inject
3841

@@ -43,7 +46,10 @@ internal class IncomingGossipingRequestManager @Inject constructor(
4346
private val cryptoStore: IMXCryptoStore,
4447
private val cryptoConfig: MXCryptoConfig,
4548
private val gossipingWorkManager: GossipingWorkManager,
46-
private val roomDecryptorProvider: RoomDecryptorProvider) {
49+
private val roomEncryptorsStore: RoomEncryptorsStore,
50+
private val roomDecryptorProvider: RoomDecryptorProvider,
51+
private val coroutineDispatchers: MatrixCoroutineDispatchers,
52+
private val cryptoCoroutineScope: CoroutineScope) {
4753

4854
// list of IncomingRoomKeyRequests/IncomingRoomKeyRequestCancellations
4955
// we received in the current sync.
@@ -178,17 +184,42 @@ internal class IncomingGossipingRequestManager @Inject constructor(
178184
}
179185

180186
private fun processIncomingRoomKeyRequest(request: IncomingRoomKeyRequest) {
181-
val userId = request.userId
182-
val deviceId = request.deviceId
183-
val body = request.requestBody
184-
val roomId = body!!.roomId
185-
val alg = body.algorithm
187+
val userId = request.userId ?: return
188+
val deviceId = request.deviceId ?: return
189+
val body = request.requestBody ?: return
190+
val roomId = body.roomId ?: return
191+
val alg = body.algorithm ?: return
186192

187193
Timber.v("## GOSSIP processIncomingRoomKeyRequest from $userId:$deviceId for $roomId / ${body.sessionId} id ${request.requestId}")
188-
if (userId == null || credentials.userId != userId) {
189-
// TODO: determine if we sent this device the keys already: in
190-
Timber.w("## GOSSIP processReceivedGossipingRequests() : Ignoring room key request from other user for now")
191-
cryptoStore.updateGossipingRequestState(request, GossipingRequestState.REJECTED)
194+
if (credentials.userId != userId) {
195+
Timber.w("## GOSSIP processReceivedGossipingRequests() : room key request from other user")
196+
val senderKey = body.senderKey ?: return Unit
197+
.also { Timber.w("missing senderKey") }
198+
.also { cryptoStore.updateGossipingRequestState(request, GossipingRequestState.REJECTED) }
199+
val sessionId = body.sessionId ?: return Unit
200+
.also { Timber.w("missing sessionId") }
201+
.also { cryptoStore.updateGossipingRequestState(request, GossipingRequestState.REJECTED) }
202+
203+
if (alg != MXCRYPTO_ALGORITHM_MEGOLM) {
204+
return Unit
205+
.also { Timber.w("Only megolm is accepted here") }
206+
.also { cryptoStore.updateGossipingRequestState(request, GossipingRequestState.REJECTED) }
207+
}
208+
209+
val roomEncryptor = roomEncryptorsStore.get(roomId) ?: return Unit
210+
.also { Timber.w("no room Encryptor") }
211+
.also { cryptoStore.updateGossipingRequestState(request, GossipingRequestState.REJECTED) }
212+
213+
cryptoCoroutineScope.launch(coroutineDispatchers.crypto) {
214+
val isSuccess = roomEncryptor.reshareKey(sessionId, userId, deviceId, senderKey)
215+
216+
if (isSuccess) {
217+
cryptoStore.updateGossipingRequestState(request, GossipingRequestState.ACCEPTED)
218+
} else {
219+
cryptoStore.updateGossipingRequestState(request, GossipingRequestState.UNABLE_TO_PROCESS)
220+
}
221+
}
222+
cryptoStore.updateGossipingRequestState(request, GossipingRequestState.RE_REQUESTED)
192223
return
193224
}
194225
// TODO: should we queue up requests we don't yet have keys for, in case they turn up later?
@@ -219,7 +250,7 @@ internal class IncomingGossipingRequestManager @Inject constructor(
219250
cryptoStore.updateGossipingRequestState(request, GossipingRequestState.REJECTED)
220251
}
221252
// if the device is verified already, share the keys
222-
val device = cryptoStore.getUserDevice(userId, deviceId!!)
253+
val device = cryptoStore.getUserDevice(userId, deviceId)
223254
if (device != null) {
224255
if (device.isVerified) {
225256
Timber.v("## GOSSIP processReceivedGossipingRequests() : device is already verified: sharing keys")

‎matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/RoomEncryptorsStore.kt

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,10 @@
1717
package im.vector.matrix.android.internal.crypto
1818

1919
import im.vector.matrix.android.internal.crypto.algorithms.IMXEncrypting
20+
import im.vector.matrix.android.internal.session.SessionScope
2021
import javax.inject.Inject
2122

23+
@SessionScope
2224
internal class RoomEncryptorsStore @Inject constructor() {
2325

2426
// MXEncrypting instance for each room.

‎matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/actions/EnsureOlmSessionsForDevicesAction.kt

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -25,10 +25,11 @@ import im.vector.matrix.android.internal.crypto.tasks.ClaimOneTimeKeysForUsersDe
2525
import timber.log.Timber
2626
import javax.inject.Inject
2727

28-
internal class EnsureOlmSessionsForDevicesAction @Inject constructor(private val olmDevice: MXOlmDevice,
29-
private val oneTimeKeysForUsersDeviceTask: ClaimOneTimeKeysForUsersDeviceTask) {
28+
internal class EnsureOlmSessionsForDevicesAction @Inject constructor(
29+
private val olmDevice: MXOlmDevice,
30+
private val oneTimeKeysForUsersDeviceTask: ClaimOneTimeKeysForUsersDeviceTask) {
3031

31-
suspend fun handle(devicesByUser: Map<String, List<CryptoDeviceInfo>>): MXUsersDevicesMap<MXOlmSessionResult> {
32+
suspend fun handle(devicesByUser: Map<String, List<CryptoDeviceInfo>>, force: Boolean = false): MXUsersDevicesMap<MXOlmSessionResult> {
3233
val devicesWithoutSession = ArrayList<CryptoDeviceInfo>()
3334

3435
val results = MXUsersDevicesMap<MXOlmSessionResult>()
@@ -40,7 +41,7 @@ internal class EnsureOlmSessionsForDevicesAction @Inject constructor(private val
4041

4142
val sessionId = olmDevice.getSessionId(key!!)
4243

43-
if (sessionId.isNullOrEmpty()) {
44+
if (sessionId.isNullOrEmpty() || force) {
4445
devicesWithoutSession.add(deviceInfo)
4546
}
4647

@@ -80,7 +81,7 @@ internal class EnsureOlmSessionsForDevicesAction @Inject constructor(private val
8081
if (null != deviceIds) {
8182
for (deviceId in deviceIds) {
8283
val olmSessionResult = results.getObject(userId, deviceId)
83-
if (olmSessionResult!!.sessionId != null) {
84+
if (olmSessionResult!!.sessionId != null && !force) {
8485
// We already have a result for this device
8586
continue
8687
}

‎matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/algorithms/IMXEncrypting.kt

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,4 +33,20 @@ internal interface IMXEncrypting {
3333
* @return the encrypted content
3434
*/
3535
suspend fun encryptEventContent(eventContent: Content, eventType: String, userIds: List<String>): Content
36+
37+
/**
38+
* Re-shares a session key with devices if the key has already been
39+
* sent to them.
40+
*
41+
* @param sessionId The id of the outbound session to share.
42+
* @param userId The id of the user who owns the target device.
43+
* @param deviceId The id of the target device.
44+
* @param senderKey The key of the originating device for the session.
45+
*
46+
* @return true in case of success
47+
*/
48+
suspend fun reshareKey(sessionId: String,
49+
userId: String,
50+
deviceId: String,
51+
senderKey: String): Boolean
3652
}

‎matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/algorithms/megolm/MXMegolmDecryption.kt

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ import im.vector.matrix.android.internal.crypto.actions.EnsureOlmSessionsForDevi
3131
import im.vector.matrix.android.internal.crypto.actions.MessageEncrypter
3232
import im.vector.matrix.android.internal.crypto.algorithms.IMXDecrypting
3333
import im.vector.matrix.android.internal.crypto.keysbackup.DefaultKeysBackupService
34+
import im.vector.matrix.android.internal.crypto.model.CryptoDeviceInfo
3435
import im.vector.matrix.android.internal.crypto.model.MXUsersDevicesMap
3536
import im.vector.matrix.android.internal.crypto.model.event.EncryptedEventContent
3637
import im.vector.matrix.android.internal.crypto.model.event.RoomKeyContent
@@ -346,4 +347,25 @@ internal class MXMegolmDecryption(private val userId: String,
346347
}
347348
}
348349
}
350+
351+
fun markOlmSessionForUnwedging(senderId: String, deviceInfo: CryptoDeviceInfo) {
352+
cryptoCoroutineScope.launch(coroutineDispatchers.crypto) {
353+
ensureOlmSessionsForDevicesAction.handle(mapOf(senderId to listOf(deviceInfo)), force = true)
354+
355+
// Now send a blank message on that session so the other side knows about it.
356+
// (The keyshare request is sent in the clear so that won't do)
357+
// We send this first such that, as long as the toDevice messages arrive in the
358+
// same order we sent them, the other end will get this first, set up the new session,
359+
// then get the keyshare request and send the key over this new session (because it
360+
// is the session it has most recently received a message on).
361+
val payloadJson = mapOf<String, Any>("type" to EventType.DUMMY)
362+
363+
val encodedPayload = messageEncrypter.encryptMessage(payloadJson, listOf(deviceInfo))
364+
val sendToDeviceMap = MXUsersDevicesMap<Any>()
365+
sendToDeviceMap.setObject(senderId, deviceInfo.deviceId, encodedPayload)
366+
Timber.v("## markOlmSessionForUnwedging() : sending to $senderId:${deviceInfo.deviceId}")
367+
val sendToDeviceParams = SendToDeviceTask.Params(EventType.ENCRYPTED, sendToDeviceMap)
368+
sendToDeviceTask.execute(sendToDeviceParams)
369+
}
370+
}
349371
}

‎matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/algorithms/megolm/MXMegolmEncryption.kt

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -305,4 +305,49 @@ internal class MXMegolmEncryption(
305305
throw MXCryptoError.UnknownDevice(unknownDevices)
306306
}
307307
}
308+
309+
override suspend fun reshareKey(sessionId: String,
310+
userId: String,
311+
deviceId: String,
312+
senderKey: String): Boolean {
313+
Timber.d("[MXMegolmEncryption] reshareKey: $sessionId to $userId:$deviceId")
314+
val deviceInfo = cryptoStore.getUserDevice(userId, deviceId) ?: return false
315+
.also { Timber.w("Device not found") }
316+
317+
// Get the chain index of the key we previously sent this device
318+
val chainIndex = outboundSession?.sharedWithDevices?.getObject(userId, deviceId)?.toLong() ?: return false
319+
.also { Timber.w("[MXMegolmEncryption] reshareKey : ERROR : Never share megolm with this device") }
320+
321+
val devicesByUser = mapOf(userId to listOf(deviceInfo))
322+
val usersDeviceMap = ensureOlmSessionsForDevicesAction.handle(devicesByUser)
323+
val olmSessionResult = usersDeviceMap.getObject(userId, deviceId)
324+
olmSessionResult?.sessionId
325+
?: // no session with this device, probably because there were no one-time keys.
326+
// ensureOlmSessionsForDevicesAction has already done the logging, so just skip it.
327+
return false
328+
329+
Timber.d("[MXMegolmEncryption] reshareKey: sharing keys for session $senderKey|$sessionId:$chainIndex with device $userId:$deviceId")
330+
331+
val payloadJson = mutableMapOf<String, Any>("type" to EventType.FORWARDED_ROOM_KEY)
332+
333+
runCatching { olmDevice.getInboundGroupSession(sessionId, senderKey, roomId) }
334+
.fold(
335+
{
336+
// TODO
337+
payloadJson["content"] = it.exportKeys(chainIndex) ?: ""
338+
},
339+
{
340+
// TODO
341+
}
342+
343+
)
344+
345+
val encodedPayload = messageEncrypter.encryptMessage(payloadJson, listOf(deviceInfo))
346+
val sendToDeviceMap = MXUsersDevicesMap<Any>()
347+
sendToDeviceMap.setObject(userId, deviceId, encodedPayload)
348+
Timber.v("## shareKeysWithDevice() : sending to $userId:$deviceId")
349+
val sendToDeviceParams = SendToDeviceTask.Params(EventType.ENCRYPTED, sendToDeviceMap)
350+
sendToDeviceTask.execute(sendToDeviceParams)
351+
return true
352+
}
308353
}

‎matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/algorithms/olm/MXOlmEncryption.kt

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,4 +78,9 @@ internal class MXOlmEncryption(
7878
deviceListManager.downloadKeys(users, false)
7979
ensureOlmSessionsForUsersAction.handle(users)
8080
}
81+
82+
override suspend fun reshareKey(sessionId: String, userId: String, deviceId: String, senderKey: String): Boolean {
83+
// No need for olm
84+
return false
85+
}
8186
}

0 commit comments

Comments
 (0)