Merge branch 'release/1.2.2' into develop

globekeeper · Sep 13, 2021 · 470ec82 · 470ec82
2 parents e7a1961 + 0c4bee4
commit 470ec82
Show file tree

Hide file tree

Showing 10 changed files with 77 additions and 20 deletions.
diff --git a/CHANGES.md b/CHANGES.md
@@ -1,5 +1,10 @@
 Please also refer to the Changelog of Element Android: https://github.com/vector-im/element-android/blob/main/CHANGES.md
 
+Changes in Matrix-SDK 1.2.2 (2021-09-13)
+===================================================
+
+Fix a security issue with message key sharing. See https://matrix.org/blog/2021/09/13/vulnerability-disclosure-key-sharing for details.
+
 Changes in Matrix-SDK 1.2.1 (2021-09-08)
 ===================================================
 

diff --git a/gradle.properties b/gradle.properties
@@ -26,7 +26,7 @@ vector.httpLogLevel=NONE
 # Ref: https://github.com/vanniktech/gradle-maven-publish-plugin
 GROUP=org.matrix.android
 POM_ARTIFACT_ID=matrix-android-sdk2
-VERSION_NAME=1.2.1
+VERSION_NAME=1.2.2
 
 POM_PACKAGING=aar
 

diff --git a/matrix-sdk-android/build.gradle b/matrix-sdk-android/build.gradle
@@ -22,7 +22,7 @@ android {
         minSdkVersion 21
         targetSdkVersion 30
         versionCode 1
-        versionName "1.2.1"
+        versionName "1.2.2"
         // Multidex is useful for tests
         multiDexEnabled true
         testInstrumentationRunner "androidx.test.runner.AndroidJUnitRunner"

diff --git a/.../main/java/org/matrix/android/sdk/internal/crypto/algorithms/megolm/MXMegolmEncryption.kt b/.../main/java/org/matrix/android/sdk/internal/crypto/algorithms/megolm/MXMegolmEncryption.kt
@@ -170,7 +170,7 @@ internal class MXMegolmEncryption(
             val deviceIds = devicesInRoom.getUserDeviceIds(userId)
             for (deviceId in deviceIds!!) {
                 val deviceInfo = devicesInRoom.getObject(userId, deviceId)
-                if (deviceInfo != null && !cryptoStore.getSharedSessionInfo(roomId, safeSession.sessionId, userId, deviceId).found) {
+                if (deviceInfo != null && !cryptoStore.getSharedSessionInfo(roomId, safeSession.sessionId, deviceInfo).found) {
                     val devices = shareMap.getOrPut(userId) { ArrayList() }
                     devices.add(deviceInfo)
                 }
@@ -270,16 +270,16 @@ internal class MXMegolmEncryption(
         // for dead devices on every message.
         val gossipingEventBuffer = arrayListOf<Event>()
         for ((userId, devicesToShareWith) in devicesByUser) {
-            for ((deviceId) in devicesToShareWith) {
-                session.sharedWithHelper.markedSessionAsShared(userId, deviceId, chainIndex)
+            for (deviceInfo in devicesToShareWith) {
+                session.sharedWithHelper.markedSessionAsShared(deviceInfo, chainIndex)
                 gossipingEventBuffer.add(
                         Event(
                                 type = EventType.ROOM_KEY,
                                 senderId = this.userId,
                                 content = submap.apply {
                                     this["session_key"] = ""
                                     // we add a fake key for trail
-                                    this["_dest"] = "$userId|$deviceId"
+                                    this["_dest"] = "$userId|${deviceInfo.deviceId}"
                                 }
                         ))
             }
@@ -429,7 +429,7 @@ internal class MXMegolmEncryption(
                 .also { Timber.w("## Crypto reshareKey: Device not found") }
 
         // Get the chain index of the key we previously sent this device
-        val wasSessionSharedWithUser = cryptoStore.getSharedSessionInfo(roomId, sessionId, userId, deviceId)
+        val wasSessionSharedWithUser = cryptoStore.getSharedSessionInfo(roomId, sessionId, deviceInfo)
         if (!wasSessionSharedWithUser.found) {
             // This session was never shared with this user
             // Send a room key with held

diff --git a/...rc/main/java/org/matrix/android/sdk/internal/crypto/algorithms/megolm/SharedWithHelper.kt b/...rc/main/java/org/matrix/android/sdk/internal/crypto/algorithms/megolm/SharedWithHelper.kt
@@ -16,6 +16,7 @@
 
 package org.matrix.android.sdk.internal.crypto.algorithms.megolm
 
+import org.matrix.android.sdk.internal.crypto.model.CryptoDeviceInfo
 import org.matrix.android.sdk.internal.crypto.model.MXUsersDevicesMap
 import org.matrix.android.sdk.internal.crypto.store.IMXCryptoStore
 
@@ -28,7 +29,13 @@ internal class SharedWithHelper(
         return cryptoStore.getSharedWithInfo(roomId, sessionId)
     }
 
-    fun markedSessionAsShared(userId: String, deviceId: String, chainIndex: Int) {
-        cryptoStore.markedSessionAsShared(roomId, sessionId, userId, deviceId, chainIndex)
+    fun markedSessionAsShared(deviceInfo: CryptoDeviceInfo, chainIndex: Int) {
+        cryptoStore.markedSessionAsShared(
+                roomId = roomId,
+                sessionId = sessionId,
+                userId = deviceInfo.userId,
+                deviceId = deviceInfo.deviceId,
+                deviceIdentityKey = deviceInfo.identityKey() ?: "",
+                chainIndex = chainIndex)
     }
 }
diff --git a/...-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/store/IMXCryptoStore.kt b/...-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/store/IMXCryptoStore.kt
@@ -450,7 +450,8 @@ internal interface IMXCryptoStore {
     fun addWithHeldMegolmSession(withHeldContent: RoomKeyWithHeldContent)
     fun getWithHeldMegolmSession(roomId: String, sessionId: String): RoomKeyWithHeldContent?
 
-    fun markedSessionAsShared(roomId: String?, sessionId: String, userId: String, deviceId: String, chainIndex: Int)
+    fun markedSessionAsShared(roomId: String?, sessionId: String, userId: String, deviceId: String,
+                              deviceIdentityKey: String, chainIndex: Int)
 
     /**
      * Query for information on this session sharing history.
@@ -459,7 +460,7 @@ internal interface IMXCryptoStore {
      * in this case chainIndex is not nullindicates the ratchet position.
      * In found is false, chainIndex is null
      */
-    fun getSharedSessionInfo(roomId: String?, sessionId: String, userId: String, deviceId: String): SharedSessionResult
+    fun getSharedSessionInfo(roomId: String?, sessionId: String, deviceInfo: CryptoDeviceInfo): SharedSessionResult
     data class SharedSessionResult(val found: Boolean, val chainIndex: Int?)
 
     fun getSharedWithInfo(roomId: String?, sessionId: String): MXUsersDevicesMap<Int>

diff --git a/...android/src/main/java/org/matrix/android/sdk/internal/crypto/store/db/RealmCryptoStore.kt b/...android/src/main/java/org/matrix/android/sdk/internal/crypto/store/db/RealmCryptoStore.kt
@@ -1681,22 +1681,35 @@ internal class RealmCryptoStore @Inject constructor(
         }
     }
 
-    override fun markedSessionAsShared(roomId: String?, sessionId: String, userId: String, deviceId: String, chainIndex: Int) {
+    override fun markedSessionAsShared(roomId: String?,
+                                       sessionId: String,
+                                       userId: String,
+                                       deviceId: String,
+                                       deviceIdentityKey: String,
+                                       chainIndex: Int) {
         doRealmTransaction(realmConfiguration) { realm ->
             SharedSessionEntity.create(
                     realm = realm,
                     roomId = roomId,
                     sessionId = sessionId,
                     userId = userId,
                     deviceId = deviceId,
+                    deviceIdentityKey = deviceIdentityKey,
                     chainIndex = chainIndex
             )
         }
     }
 
-    override fun getSharedSessionInfo(roomId: String?, sessionId: String, userId: String, deviceId: String): IMXCryptoStore.SharedSessionResult {
+    override fun getSharedSessionInfo(roomId: String?, sessionId: String, deviceInfo: CryptoDeviceInfo): IMXCryptoStore.SharedSessionResult {
         return doWithRealm(realmConfiguration) { realm ->
-            SharedSessionEntity.get(realm, roomId, sessionId, userId, deviceId)?.let {
+            SharedSessionEntity.get(
+                    realm = realm,
+                    roomId = roomId,
+                    sessionId = sessionId,
+                    userId = deviceInfo.userId,
+                    deviceId = deviceInfo.deviceId,
+                    deviceIdentityKey = deviceInfo.identityKey()
+            )?.let {
                 IMXCryptoStore.SharedSessionResult(true, it.chainIndex)
             } ?: IMXCryptoStore.SharedSessionResult(false, null)
         }

diff --git a/...rc/main/java/org/matrix/android/sdk/internal/crypto/store/db/RealmCryptoStoreMigration.kt b/...rc/main/java/org/matrix/android/sdk/internal/crypto/store/db/RealmCryptoStoreMigration.kt
@@ -55,7 +55,7 @@ internal object RealmCryptoStoreMigration : RealmMigration {
     // 0, 1, 2: legacy Riot-Android
     // 3: migrate to RiotX schema
     // 4, 5, 6, 7, 8, 9: migrations from RiotX (which was previously 1, 2, 3, 4, 5, 6)
-    const val CRYPTO_STORE_SCHEMA_VERSION = 13L
+    const val CRYPTO_STORE_SCHEMA_VERSION = 14L
 
     private fun RealmObjectSchema.addFieldIfNotExists(fieldName: String, fieldType: Class<*>): RealmObjectSchema {
         if (!hasField(fieldName)) {
@@ -94,6 +94,7 @@ internal object RealmCryptoStoreMigration : RealmMigration {
         if (oldVersion <= 10) migrateTo11(realm)
         if (oldVersion <= 11) migrateTo12(realm)
         if (oldVersion <= 12) migrateTo13(realm)
+        if (oldVersion <= 13) migrateTo14(realm)
     }
 
     private fun migrateTo1Legacy(realm: DynamicRealm) {
@@ -554,4 +555,21 @@ internal object RealmCryptoStoreMigration : RealmMigration {
             Timber.e("TrustLevelEntity cleanup: Something is not correct...")
         }
     }
+
+    // Version 14L Update the way we remember key sharing
+    private fun migrateTo14(realm: DynamicRealm) {
+        Timber.d("Step 13 -> 14")
+        realm.schema.get("SharedSessionEntity")
+                ?.addField(SharedSessionEntityFields.DEVICE_IDENTITY_KEY, String::class.java)
+                ?.addIndex(SharedSessionEntityFields.DEVICE_IDENTITY_KEY)
+                ?.transform {
+                    val sharedUserId = it.getString(SharedSessionEntityFields.USER_ID)
+                    val sharedDeviceId = it.getString(SharedSessionEntityFields.DEVICE_ID)
+                    val knownDevice = realm.where("DeviceInfoEntity")
+                            .equalTo(DeviceInfoEntityFields.USER_ID, sharedUserId)
+                            .equalTo(DeviceInfoEntityFields.DEVICE_ID, sharedDeviceId)
+                            .findFirst()
+                    it.setString(SharedSessionEntityFields.DEVICE_IDENTITY_KEY, knownDevice?.getString(DeviceInfoEntityFields.IDENTITY_KEY))
+                }
+    }
 }
diff --git a/...rc/main/java/org/matrix/android/sdk/internal/crypto/store/db/model/SharedSessionEntity.kt b/...rc/main/java/org/matrix/android/sdk/internal/crypto/store/db/model/SharedSessionEntity.kt
@@ -30,6 +30,7 @@ internal open class SharedSessionEntity(
         @Index var sessionId: String? = null,
         @Index var userId: String? = null,
         @Index var deviceId: String? = null,
+        @Index var deviceIdentityKey: String? = null,
         var chainIndex: Int? = null
 ) : RealmObject() {
 

diff --git a/...c/main/java/org/matrix/android/sdk/internal/crypto/store/db/query/SharedSessionQueries.kt b/...c/main/java/org/matrix/android/sdk/internal/crypto/store/db/query/SharedSessionQueries.kt
@@ -16,22 +16,28 @@
 
 package org.matrix.android.sdk.internal.crypto.store.db.query
 
-import org.matrix.android.sdk.internal.crypto.MXCRYPTO_ALGORITHM_MEGOLM
-import org.matrix.android.sdk.internal.crypto.store.db.model.SharedSessionEntity
-import org.matrix.android.sdk.internal.crypto.store.db.model.SharedSessionEntityFields
 import io.realm.Realm
 import io.realm.RealmResults
 import io.realm.kotlin.createObject
 import io.realm.kotlin.where
+import org.matrix.android.sdk.internal.crypto.MXCRYPTO_ALGORITHM_MEGOLM
+import org.matrix.android.sdk.internal.crypto.store.db.model.SharedSessionEntity
+import org.matrix.android.sdk.internal.crypto.store.db.model.SharedSessionEntityFields
 
-internal fun SharedSessionEntity.Companion.get(realm: Realm, roomId: String?, sessionId: String, userId: String, deviceId: String)
+internal fun SharedSessionEntity.Companion.get(realm: Realm,
+                                               roomId: String?,
+                                               sessionId: String,
+                                               userId: String,
+                                               deviceId: String,
+                                               deviceIdentityKey: String?)
         : SharedSessionEntity? {
     return realm.where<SharedSessionEntity>()
             .equalTo(SharedSessionEntityFields.ROOM_ID, roomId)
             .equalTo(SharedSessionEntityFields.SESSION_ID, sessionId)
             .equalTo(SharedSessionEntityFields.ALGORITHM, MXCRYPTO_ALGORITHM_MEGOLM)
             .equalTo(SharedSessionEntityFields.USER_ID, userId)
             .equalTo(SharedSessionEntityFields.DEVICE_ID, deviceId)
+            .equalTo(SharedSessionEntityFields.DEVICE_IDENTITY_KEY, deviceIdentityKey)
             .findFirst()
 }
 
@@ -44,14 +50,20 @@ internal fun SharedSessionEntity.Companion.get(realm: Realm, roomId: String?, se
             .findAll()
 }
 
-internal fun SharedSessionEntity.Companion.create(realm: Realm, roomId: String?, sessionId: String, userId: String, deviceId: String, chainIndex: Int)
+internal fun SharedSessionEntity.Companion.create(realm: Realm, roomId: String?,
+                                                  sessionId: String,
+                                                  userId: String,
+                                                  deviceId: String,
+                                                  deviceIdentityKey: String,
+                                                  chainIndex: Int)
         : SharedSessionEntity {
     return realm.createObject<SharedSessionEntity>().apply {
         this.roomId = roomId
         this.algorithm = MXCRYPTO_ALGORITHM_MEGOLM
         this.sessionId = sessionId
         this.userId = userId
         this.deviceId = deviceId
+        this.deviceIdentityKey = deviceIdentityKey
         this.chainIndex = chainIndex
     }
 }