Merge pull request #1374 from vector-im/feature/sas_v2

support new key agreement method for SAS

CHANGES.md

@@ -23,7 +23,7 @@ Build 🧱:
  -
 
 Other changes:
- -
+ - support new key agreement method for SAS (#1374)
 
 Changes in RiotX 0.20.0 (2020-05-15)
 ===================================================

matrix-sdk-android/src/androidTest/java/im/vector/matrix/android/internal/crypto/verification/SASTest.kt

@@ -468,15 +468,20 @@ class SASTest : InstrumentedTest {
 
         val aliceSASLatch = CountDownLatch(1)
         val aliceListener = object : VerificationService.Listener {
+            var matchOnce = true
             override fun transactionUpdated(tx: VerificationTransaction) {
                 val uxState = (tx as OutgoingSasVerificationTransaction).uxState
+                Log.v("TEST", "== aliceState ${uxState.name}")
                 when (uxState) {
                     OutgoingSasVerificationTransaction.UxState.SHOW_SAS -> {
                         tx.userHasVerifiedShortCode()
                     }
                     OutgoingSasVerificationTransaction.UxState.VERIFIED -> {
+                        if (matchOnce) {
+                            matchOnce = false
                             aliceSASLatch.countDown()
                         }
+                    }
                     else                                                -> Unit
                 }
             }
@@ -485,15 +490,24 @@ class SASTest : InstrumentedTest {
 
         val bobSASLatch = CountDownLatch(1)
         val bobListener = object : VerificationService.Listener {
+            var acceptOnce = true
+            var matchOnce = true
             override fun transactionUpdated(tx: VerificationTransaction) {
                 val uxState = (tx as IncomingSasVerificationTransaction).uxState
+                Log.v("TEST", "== bobState ${uxState.name}")
                 when (uxState) {
                     IncomingSasVerificationTransaction.UxState.SHOW_ACCEPT -> {
+                        if (acceptOnce) {
+                            acceptOnce = false
                             tx.performAccept()
                         }
+                    }
                     IncomingSasVerificationTransaction.UxState.SHOW_SAS    -> {
+                        if (matchOnce) {
+                            matchOnce = false
                             tx.userHasVerifiedShortCode()
                         }
+                    }
                     IncomingSasVerificationTransaction.UxState.VERIFIED    -> {
                         bobSASLatch.countDown()
                     }

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

@@ -198,6 +198,20 @@ internal class DefaultIncomingSASDefaultVerificationTransaction(
         // using the result as the shared secret.
 
         getSAS().setTheirPublicKey(otherKey)
+
+        shortCodeBytes = calculateSASBytes()
+
+        if (BuildConfig.LOG_PRIVATE_DATA) {
+            Timber.v("************  BOB CODE ${getDecimalCodeRepresentation(shortCodeBytes!!)}")
+            Timber.v("************  BOB EMOJI CODE ${getShortCodeRepresentation(SasMode.EMOJI)}")
+        }
+
+        state = VerificationTxState.ShortCodeReady
+    }
+
+    private fun calculateSASBytes(): ByteArray {
+        when (accepted?.keyAgreementProtocol) {
+            KEY_AGREEMENT_V1 -> {
                 // (Note: In all of the following HKDF is as defined in RFC 5869, and uses the previously agreed-on hash function as the hash function,
                 // the shared secret as the input keying material, no salt, and with the input parameter set to the concatenation of:
                 // - the string “MATRIX_KEY_VERIFICATION_SAS”,
@@ -207,16 +221,21 @@ internal class DefaultIncomingSASDefaultVerificationTransaction(
                 // - he device ID of the device that sent the m.key.verification.accept message
                 // - the transaction ID.
                 val sasInfo = "MATRIX_KEY_VERIFICATION_SAS$otherUserId$otherDeviceId$userId$deviceId$transactionId"
+
                 // decimal: generate five bytes by using HKDF.
                 // emoji: generate six bytes by using HKDF.
-        shortCodeBytes = getSAS().generateShortCode(sasInfo, 6)
+                return getSAS().generateShortCode(sasInfo, 6)
-
+            }
-        if (BuildConfig.LOG_PRIVATE_DATA) {
+            KEY_AGREEMENT_V2 -> {
-            Timber.v("************  BOB CODE ${getDecimalCodeRepresentation(shortCodeBytes!!)}")
+                // Adds the SAS public key, and separate by |
-            Timber.v("************  BOB EMOJI CODE ${getShortCodeRepresentation(SasMode.EMOJI)}")
+                val sasInfo = "MATRIX_KEY_VERIFICATION_SAS|$otherUserId|$otherDeviceId|$otherKey|$userId|$deviceId|${getSAS().publicKey}|$transactionId"
+                return getSAS().generateShortCode(sasInfo, 6)
+            }
+            else             -> {
+                // Protocol has been checked earlier
+                throw IllegalArgumentException()
+            }
         }
-
-        state = VerificationTxState.ShortCodeReady
     }
 
     override fun onKeyVerificationMac(vMac: ValidVerificationInfoMac) {

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

@@ -193,6 +193,17 @@ internal class DefaultOutgoingSASDefaultVerificationTransaction(
 
         if (accepted!!.commitment.equals(otherCommitment)) {
             getSAS().setTheirPublicKey(otherKey)
+            shortCodeBytes = calculateSASBytes()
+            state = VerificationTxState.ShortCodeReady
+        } else {
+            // bad commitment
+            cancel(CancelCode.MismatchedCommitment)
+        }
+    }
+
+    private fun calculateSASBytes(): ByteArray {
+        when (accepted?.keyAgreementProtocol) {
+            KEY_AGREEMENT_V1 -> {
                 // (Note: In all of the following HKDF is as defined in RFC 5869, and uses the previously agreed-on hash function as the hash function,
                 // the shared secret as the input keying material, no salt, and with the input parameter set to the concatenation of:
                 // - the string “MATRIX_KEY_VERIFICATION_SAS”,
@@ -202,24 +213,33 @@ internal class DefaultOutgoingSASDefaultVerificationTransaction(
                 // - he device ID of the device that sent the m.key.verification.accept message
                 // - the transaction ID.
                 val sasInfo =  "MATRIX_KEY_VERIFICATION_SAS$userId$deviceId$otherUserId$otherDeviceId$transactionId"
+
                 // decimal: generate five bytes by using HKDF.
                 // emoji: generate six bytes by using HKDF.
-            shortCodeBytes = getSAS().generateShortCode(sasInfo, 6)
+                return getSAS().generateShortCode(sasInfo, 6)
-            state = VerificationTxState.ShortCodeReady
+            }
-        } else {
+            KEY_AGREEMENT_V2 -> {
-            // bad commitment
+                // Adds the SAS public key, and separate by |
-            cancel(CancelCode.MismatchedCommitment)
+                val sasInfo =  "MATRIX_KEY_VERIFICATION_SAS|$userId|$deviceId|${getSAS().publicKey}|$otherUserId|$otherDeviceId|$otherKey|$transactionId"
+                return getSAS().generateShortCode(sasInfo, 6)
+            }
+            else             -> {
+                // Protocol has been checked earlier
+                throw IllegalArgumentException()
+            }
         }
     }
 
     override fun onKeyVerificationMac(vMac: ValidVerificationInfoMac) {
         Timber.v("## SAS O: onKeyVerificationMac id:$transactionId")
+        // There is starting to be a huge amount of state / race here :/
         if (state != VerificationTxState.OnKeyReceived
                 && state != VerificationTxState.ShortCodeReady
                 && state != VerificationTxState.ShortCodeAccepted
+                && state != VerificationTxState.KeySent
                 && state != VerificationTxState.SendingMac
                 && state != VerificationTxState.MacSent) {
-            Timber.e("## SAS O: received key from invalid state $state")
+            Timber.e("## SAS O: received mac from invalid state $state")
             cancel(CancelCode.UnexpectedMessage)
             return
         }

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

@@ -611,7 +611,7 @@ internal class DefaultVerificationService @Inject constructor(
 
         if (validCancelReq == null) {
             // ignore
-            Timber.e("## SAS Received invalid key request")
+            Timber.e("## SAS Received invalid cancel request")
             // TODO should we cancel?
             return
         }

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

@@ -26,7 +26,6 @@ import im.vector.matrix.android.api.session.events.model.EventType
 import im.vector.matrix.android.internal.crypto.IncomingGossipingRequestManager
 import im.vector.matrix.android.internal.crypto.OutgoingGossipingRequestManager
 import im.vector.matrix.android.internal.crypto.actions.SetDeviceVerificationAction
-import im.vector.matrix.android.internal.crypto.model.MXKey
 import im.vector.matrix.android.internal.crypto.store.IMXCryptoStore
 import im.vector.matrix.android.internal.extensions.toUnsignedInt
 import im.vector.matrix.android.internal.util.withoutPrefix
@@ -66,8 +65,11 @@ internal abstract class SASDefaultVerificationTransaction(
         const val SAS_MAC_SHA256_LONGKDF = "hmac-sha256"
         const val SAS_MAC_SHA256 = "hkdf-hmac-sha256"
 
+        // Deprecated maybe removed later, use V2
+        const val KEY_AGREEMENT_V1 = "curve25519"
+        const val KEY_AGREEMENT_V2 = "curve25519-hkdf-sha256"
         // ordered by preferred order
-        val KNOWN_AGREEMENT_PROTOCOLS = listOf(MXKey.KEY_CURVE_25519_TYPE)
+        val KNOWN_AGREEMENT_PROTOCOLS = listOf(KEY_AGREEMENT_V2, KEY_AGREEMENT_V1)
         // ordered by preferred order
         val KNOWN_HASHES = listOf("sha256")
         // ordered by preferred order