QRCode: add other_device_key field and make it optional, along with other_user_key

matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/verification/DefaultVerificationService.kt

@@ -733,7 +733,7 @@ internal class DefaultVerificationService @Inject constructor(
                 // Check if other user is able to scan QR code
                 ?.takeIf { it.contains(VERIFICATION_METHOD_QR_CODE_SCAN) }
                 ?.let {
-                    createQrCodeData(existingRequest.transactionId, existingRequest.otherUserId)
+                    createQrCodeData(existingRequest.transactionId, existingRequest.otherUserId, readyReq.fromDevice)
                 }
 
         if (readyReq.methods.orEmpty().contains(VERIFICATION_METHOD_RECIPROCATE)) {
@@ -760,13 +760,25 @@ internal class DefaultVerificationService @Inject constructor(
         ))
     }
 
-    private fun createQrCodeData(transactionId: String?, otherUserId: String): QrCodeData? {
+    private fun createQrCodeData(requestEventId: String?, otherUserId: String, otherDeviceId: String?): QrCodeData? {
-        // Build the QR code URL
+        requestEventId ?: run {
-        val requestEventId = transactionId ?: run {
             Timber.w("## Unknown requestEventId")
             return null
         }
 
+        return when {
+            userId != otherUserId                        ->
+                createQrCodeDataForDistinctUser(requestEventId, otherUserId, otherDeviceId)
+            crossSigningService.isCrossSigningVerified() ->
+                // This is a self verification and I am the old device (Osborne2)
+                createQrCodeDataForVerifiedDevice(requestEventId, otherDeviceId)
+            else                                         ->
+                // This is a self verification and I am the new device (Dynabook)
+                createQrCodeDataForUnVerifiedDevice(requestEventId, otherDeviceId)
+        }
+    }
+
+    private fun createQrCodeDataForDistinctUser(requestEventId: String, otherUserId: String, otherDeviceId: String?): QrCodeData? {
         val myMasterKey = crossSigningService.getMyCrossSigningKeys()
                 ?.masterKey()
                 ?.unpaddedBase64PublicKey
@@ -789,9 +801,17 @@ internal class DefaultVerificationService @Inject constructor(
                     return null
                 }
 
-        val myDeviceKey = myDeviceInfoHolder.get().myDevice.fingerprint()!!
+        val myDeviceKey = myDeviceInfoHolder.get().myDevice.fingerprint()
+                ?: run {
+                    Timber.w("## Unable to get my fingerprint")
+                    return null
+                }
+
+        val otherDeviceKey = otherDeviceId
+                ?.let {
+                    cryptoStore.getUserDevice(userId, otherDeviceId)?.fingerprint()
+                }
 
-        val generatedSharedSecret = generateSharedSecret()
         return QrCodeData(
                 userId = userId,
                 requestEventId = requestEventId,
@@ -800,8 +820,95 @@ internal class DefaultVerificationService @Inject constructor(
                         myMasterKey to myMasterKey,
                         myDeviceId to myDeviceKey
                 ),
-                sharedSecret = generatedSharedSecret,
+                sharedSecret = generateSharedSecret(),
-                otherUserKey = otherUserMasterKey
+                otherUserKey = otherUserMasterKey,
+                otherDeviceKey = otherDeviceKey
+        )
+    }
+
+    // Create a QR code to display on the old device (Osborne2)
+    private fun createQrCodeDataForVerifiedDevice(requestEventId: String, otherDeviceId: String?): QrCodeData? {
+        val myMasterKey = crossSigningService.getMyCrossSigningKeys()
+                ?.masterKey()
+                ?.unpaddedBase64PublicKey
+                ?: run {
+                    Timber.w("## Unable to get my master key")
+                    return null
+                }
+
+        val otherDeviceKey = otherDeviceId
+                ?.let {
+                    cryptoStore.getUserDevice(userId, otherDeviceId)?.fingerprint()
+                }
+                ?: run {
+                    Timber.w("## Unable to get other device data")
+                    return null
+                }
+
+        val myDeviceId = deviceId
+                ?: run {
+                    Timber.w("## Unable to get my deviceId")
+                    return null
+                }
+
+        val myDeviceKey = myDeviceInfoHolder.get().myDevice.fingerprint()
+                ?: run {
+                    Timber.w("## Unable to get my fingerprint")
+                    return null
+                }
+
+        return QrCodeData(
+                userId = userId,
+                requestEventId = requestEventId,
+                action = QrCodeData.ACTION_VERIFY,
+                keys = hashMapOf(
+                        myMasterKey to myMasterKey,
+                        myDeviceId to myDeviceKey
+                ),
+                sharedSecret = generateSharedSecret(),
+                otherUserKey = null,
+                otherDeviceKey = otherDeviceKey
+        )
+    }
+
+    // Create a QR code to display on the new device (Dynabook)
+    private fun createQrCodeDataForUnVerifiedDevice(requestEventId: String, otherDeviceId: String?): QrCodeData? {
+        val myMasterKey = crossSigningService.getMyCrossSigningKeys()
+                ?.masterKey()
+                ?.unpaddedBase64PublicKey
+                ?: run {
+                    Timber.w("## Unable to get my master key")
+                    return null
+                }
+
+        val myDeviceId = deviceId
+                ?: run {
+                    Timber.w("## Unable to get my deviceId")
+                    return null
+                }
+
+        val myDeviceKey = myDeviceInfoHolder.get().myDevice.fingerprint()
+                ?: run {
+                    Timber.w("## Unable to get my fingerprint")
+                    return null
+                }
+
+        val otherDeviceKey = otherDeviceId
+                ?.let {
+                    cryptoStore.getUserDevice(userId, otherDeviceId)?.fingerprint()
+                }
+
+        return QrCodeData(
+                userId = userId,
+                requestEventId = requestEventId,
+                action = QrCodeData.ACTION_VERIFY,
+                keys = hashMapOf(
+                        // Note: no master key here
+                        myDeviceId to myDeviceKey
+                ),
+                sharedSecret = generateSharedSecret(),
+                otherUserKey = myMasterKey,
+                otherDeviceKey = otherDeviceKey
         )
     }
 
@@ -1128,7 +1235,7 @@ internal class DefaultVerificationService @Inject constructor(
 
         if (VERIFICATION_METHOD_QR_CODE_SCAN in otherUserMethods || VERIFICATION_METHOD_QR_CODE_SHOW in otherUserMethods) {
             // Other user wants to verify using QR code. Cross-signing has to be setup
-            val qrCodeData = createQrCodeData(transactionId, otherUserId)
+            val qrCodeData = createQrCodeData(transactionId, otherUserId, otherDeviceId)
 
             if (qrCodeData != null) {
                 if (VERIFICATION_METHOD_QR_CODE_SCAN in otherUserMethods && VerificationMethod.QR_CODE_SHOW in methods) {

matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/verification/qrcode/DefaultQrCodeVerificationTransaction.kt

@@ -88,12 +88,29 @@ internal class DefaultQrCodeVerificationTransaction(
         }
 
         // check master key
-        if (otherQrCodeData.otherUserKey != crossSigningService.getUserCrossSigningKeys(userId)?.masterKey()?.unpaddedBase64PublicKey) {
+        if (otherQrCodeData.userId != userId
+                && otherQrCodeData.otherUserKey == null) {
+            // Verification with other user, other_user_key is mandatory in this case
+            Timber.d("## Verification QR: Invalid, missing other_user_key")
+            cancel(CancelCode.QrCodeInvalid)
+            return
+        }
+
+        if (otherQrCodeData.otherUserKey != null
+                && otherQrCodeData.otherUserKey != crossSigningService.getUserCrossSigningKeys(userId)?.masterKey()?.unpaddedBase64PublicKey) {
             Timber.d("## Verification QR: Invalid other master key ${otherQrCodeData.otherUserKey}")
             cancel(CancelCode.MismatchedKeys)
             return
         }
 
+        // Check device key if available
+        if (otherQrCodeData.otherDeviceKey != null
+                && otherQrCodeData.otherDeviceKey != cryptoStore.getUserDevice(otherQrCodeData.userId, otherDeviceId ?: "")?.fingerprint()) {
+            Timber.d("## Verification QR: Invalid other device key")
+            cancel(CancelCode.MismatchedKeys)
+            return
+        }
+
         val toVerifyDeviceIds = mutableListOf<String>()
         var canTrustOtherUserMasterKey = false
 
@@ -147,8 +164,15 @@ internal class DefaultQrCodeVerificationTransaction(
         // qrCodeData.sharedSecret will be used to send the start request
         start(otherQrCodeData.sharedSecret)
 
+        val safeOtherDeviceId = otherDeviceId
+        if (!otherQrCodeData.otherDeviceKey.isNullOrBlank()
+                && safeOtherDeviceId != null) {
+            // Locally verify the device
+            toVerifyDeviceIds.add(safeOtherDeviceId)
+        }
+
         // Trust the other user
-        trust(canTrustOtherUserMasterKey, toVerifyDeviceIds)
+        trust(canTrustOtherUserMasterKey, toVerifyDeviceIds.distinct())
     }
 
     fun start(remoteSecret: String) {
@@ -240,8 +264,8 @@ internal class DefaultQrCodeVerificationTransaction(
 
         // TODO what if the otherDevice is not in this list? and should we
         toVerifyDeviceIds.forEach {
-            setDeviceVerified(otherUserId, it)
+                    setDeviceVerified(otherUserId, it)
-        }
+                }
         transport.done(transactionId)
         state = VerificationTxState.Verified
     }

matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/verification/qrcode/Extensions.kt

@@ -32,6 +32,7 @@ private const val ENCODING = "utf-8"
  *     &key_<keyid>=<key-in-base64>...
  *     &secret=<shared_secret>
  *     &other_user_key=<master-key-in-base64>
+ *     &other_device_key=<device-key-in-base64>
  *
  * Example:
  * https://matrix.to/#/@user:matrix.org?
@@ -40,7 +41,8 @@ private const val ENCODING = "utf-8"
  *     &key_VJEDVKUYTQ=DL7LWIw7Qp%2B4AREDACTEDOwy2BjygumSWAGfzaWY
  *     &key_fsh%2FfQ08N3xvh4ySXsINB%2BJ2hREDACTEDVcVOG4qqo=fsh%2FfQ08N3xvh4ySXsINB%2BJ2hREDACTEDVcVOG4qqo
  *     &secret=AjQqw51Fp6UBuPolZ2FAD5WnXc22ZhJG6iGslrVvIdw%3D
- *     &other_user_key=WqSVLkBCS%2Fi5NqR%2F%2FymC8T7K9RPxBIuqK8Usl6Y3big
+ *     &other_user_key=WqSVLkBCS%2Fi5NqRREDACTEDRPxBIuqK8Usl6Y3big
+ *     &other_device_key=WqSVLkBREDACTEDBsfszdvsdBEvefqsdcsfBvsfcsFb
  * </pre>
  */
 fun QrCodeData.toUrl(): String {
@@ -58,8 +60,15 @@ fun QrCodeData.toUrl(): String {
 
         append("&secret=")
         append(URLEncoder.encode(sharedSecret, ENCODING))
-        append("&other_user_key=")
+
-        append(URLEncoder.encode(otherUserKey, ENCODING))
+        if (!otherUserKey.isNullOrBlank()) {
+            append("&other_user_key=")
+            append(URLEncoder.encode(otherUserKey, ENCODING))
+        }
+        if (!otherDeviceKey.isNullOrBlank()) {
+            append("&other_device_key=")
+            append(URLEncoder.encode(otherDeviceKey, ENCODING))
+        }
     }
 }
 
@@ -100,7 +109,8 @@ fun String.toQrCodeData(): QrCodeData? {
 
     val requestEventId = keyValues["request"]?.takeIf { MatrixPatterns.isEventId(it) } ?: return null
     val sharedSecret = keyValues["secret"] ?: return null
-    val otherUserKey = keyValues["other_user_key"] ?: return null
+    val otherUserKey = keyValues["other_user_key"]
+    val otherDeviceKey = keyValues["other_device_key"]
 
     val keys = keyValues.keys
             .filter { it.startsWith("key_") }
@@ -115,6 +125,7 @@ fun String.toQrCodeData(): QrCodeData? {
             action,
             keys,
             sharedSecret,
-            otherUserKey
+            otherUserKey,
+            otherDeviceKey
     )
 }

matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/crypto/verification/qrcode/QrCodeData.kt

@@ -26,13 +26,18 @@ data class QrCodeData(
         // The action
         val action: String,
         // key_<key_id>: each key that the user wants verified will have an entry of this form, where the value is the key in unpadded base64.
-        // The QR code should contain at least the user's master cross-signing key.
+        // The QR code should contain at least the user's master cross-signing key. In the case where a device does not have a cross-signing key
+        // (as in the case where a user logs in to a new device, and is verifying against another device), thin the QR code should contain at
+        // least the device's key.
         val keys: Map<String, String>,
         // random single-use shared secret in unpadded base64. It must be at least 256-bits long (43 characters when base64-encoded).
         val sharedSecret: String,
         // the other user's master cross-signing key, in unpadded base64. In other words, if Alice is displaying the QR code,
         // this would be the copy of Bob's master cross-signing key that Alice has.
-        val otherUserKey: String
+        val otherUserKey: String?,
+        // The other device's key, in unpadded base64
+        // This is only needed when a user is verifying their own devices, where the other device has not yet been signed with the cross-signing key.
+        val otherDeviceKey: String?
 ) {
     companion object {
         const val ACTION_VERIFY = "verify"

matrix-sdk-android/src/test/java/im/vector/matrix/android/internal/crypto/verification/qrcode/QrCodeTest.kt

@@ -16,6 +16,7 @@
 
 package im.vector.matrix.android.internal.crypto.verification.qrcode
 
+import org.amshove.kluent.shouldBe
 import org.amshove.kluent.shouldBeEqualTo
 import org.amshove.kluent.shouldBeNull
 import org.amshove.kluent.shouldNotBeNull
@@ -36,10 +37,18 @@ class QrCodeTest {
                     "2" to "ghijql"
             ),
             sharedSecret = "sharedSecret",
-            otherUserKey = "otherUserKey"
+            otherUserKey = "otherUserKey",
+            otherDeviceKey = "otherDeviceKey"
     )
 
-    private val basicUrl = "https://matrix.to/#/@benoit:matrix.org?request=%24azertyazerty&action=verify&key_1=abcdef&key_2=ghijql&secret=sharedSecret&other_user_key=otherUserKey"
+    private val basicUrl = "https://matrix.to/#/@benoit:matrix.org" +
+            "?request=%24azertyazerty" +
+            "&action=verify" +
+            "&key_1=abcdef" +
+            "&key_2=ghijql" +
+            "&secret=sharedSecret" +
+            "&other_user_key=otherUserKey" +
+            "&other_device_key=otherDeviceKey"
 
     @Test
     fun testNominalCase() {
@@ -56,7 +65,8 @@ class QrCodeTest {
         decodedData.keys["1"]?.shouldBeEqualTo("abcdef")
         decodedData.keys["2"]?.shouldBeEqualTo("ghijql")
         decodedData.sharedSecret shouldBeEqualTo "sharedSecret"
-        decodedData.otherUserKey shouldBeEqualTo "otherUserKey"
+        decodedData.otherUserKey?.shouldBeEqualTo("otherUserKey")
+        decodedData.otherDeviceKey?.shouldBeEqualTo("otherDeviceKey")
     }
 
     @Test
@@ -81,7 +91,56 @@ class QrCodeTest {
         decodedData.keys["1"]?.shouldBeEqualTo("abcdef")
         decodedData.keys["2"]?.shouldBeEqualTo("ghijql")
         decodedData.sharedSecret shouldBeEqualTo "sharedSecret"
-        decodedData.otherUserKey shouldBeEqualTo "otherUserKey"
+        decodedData.otherUserKey!! shouldBeEqualTo "otherUserKey"
+        decodedData.otherDeviceKey!! shouldBeEqualTo "otherDeviceKey"
+    }
+
+    @Test
+    fun testNoOtherUserKey() {
+        val url = basicQrCodeData
+                .copy(
+                        otherUserKey = null
+                )
+                .toUrl()
+
+        url shouldBeEqualTo basicUrl
+                .replace("&other_user_key=otherUserKey", "")
+
+        val decodedData = url.toQrCodeData()
+
+        decodedData.shouldNotBeNull()
+
+        decodedData.userId shouldBeEqualTo "@benoit:matrix.org"
+        decodedData.requestEventId shouldBeEqualTo "\$azertyazerty"
+        decodedData.keys["1"]?.shouldBeEqualTo("abcdef")
+        decodedData.keys["2"]?.shouldBeEqualTo("ghijql")
+        decodedData.sharedSecret shouldBeEqualTo "sharedSecret"
+        decodedData.otherUserKey shouldBe null
+        decodedData.otherDeviceKey?.shouldBeEqualTo("otherDeviceKey")
+    }
+
+    @Test
+    fun testNoOtherDeviceKey() {
+        val url = basicQrCodeData
+                .copy(
+                        otherDeviceKey = null
+                )
+                .toUrl()
+
+        url shouldBeEqualTo basicUrl
+                .replace("&other_device_key=otherDeviceKey", "")
+
+        val decodedData = url.toQrCodeData()
+
+        decodedData.shouldNotBeNull()
+
+        decodedData.userId shouldBeEqualTo "@benoit:matrix.org"
+        decodedData.requestEventId shouldBeEqualTo "\$azertyazerty"
+        decodedData.keys["1"]?.shouldBeEqualTo("abcdef")
+        decodedData.keys["2"]?.shouldBeEqualTo("ghijql")
+        decodedData.sharedSecret shouldBeEqualTo "sharedSecret"
+        decodedData.otherUserKey?.shouldBeEqualTo("otherUserKey")
+        decodedData.otherDeviceKey shouldBe null
     }
 
     @Test
@@ -149,11 +208,4 @@ class QrCodeTest {
                 .toQrCodeData()
                 .shouldBeNull()
     }
-
-    @Test
-    fun testMissingOtherUserKey() {
-        basicUrl.replace("&other_user_key=otherUserKey", "")
-                .toQrCodeData()
-                .shouldBeNull()
-    }
 }