Merge pull request #2630 from uhoreg/sas2

MSC2630: checking public keys in SAS verification

proposals/2630-sas-check-public-keys.md

@@ -0,0 +1,55 @@
+# MSC2630: Checking public keys in SAS verification
+
+The current SAS protocol does not ensure that the two users correctly received
+each other's public keys.  An attacker could send Alice and Bob public keys
+that he has created and, if the attacker is lucky, could obtain the same shared
+secret with both Alice and Bob, so that when they verify the SAS string, will
+believe that the exchange was secure.
+
+To mitigate against this, Alice and Bob can use the two public keys in the
+generation of the SAS string by including it in the info parameter of the HKDF.
+Thus if an attacker sends them different public keys, the info parameters will
+be different, and so the key generated by the HKDF will be different.
+
+Thanks to [David Wong](https://twitter.com/cryptodavidw) for identifying the
+issue, disclosing responsibly, and for helping to design the fix.
+
+## Proposal
+
+A new `key_agreement_protocol`, `curve25519-hkdf-sha256` is introduced, and
+will be mandatory for clients to support when performing SAS verification. It
+is the same as `curve25519` except that the info parameter for the HKDF is the
+concatenation of:
+
+  * The string `MATRIX_KEY_VERIFICATION_SAS|`.
+  * The Matrix ID of the user who sent the `m.key.verification.start` message,
+    followed by `|`.
+  * The Device ID of the device which sent the `m.key.verification.start`
+    message, followed by `|`.
+  * The public key from the `m.key.verification.key` message sent by the device
+    which sent the `m.key.verification.start` message, followed by `|`.
+  * The Matrix ID of the user who sent the `m.key.verification.accept` message,
+    followed by `|`.
+  * The Device ID of the device which sent the `m.key.verification.accept`
+    message, followed by `|`.
+  * The public key from the `m.key.verification.key` message sent by the device
+    which sent the `m.key.verification.accept` message, followed by `|`.
+  * The `transaction_id` being used.
+
+The differences from `curve25519` are the addition of the public keys, and the
+addition of `|` as delimiter between the fields.
+
+The `key_agreement_protocol` `curve25519` is deprecated and may be removed in
+the future.  It will no longer be mandatory for clients to support, and new
+implementations are discouraged from implementing it.
+
+## Implementation
+
+This has been implemented in:
+
+- Riot Web 1.6.3 (matrix-js-sdk 6.2.0)
+- Riot Android 0.9.12 (matrix-android-sdk 0.9.35)
+- RiotX 0.21
+- Riot iOS 0.11.5 (matrix-ios-sdk 0.16.5)
+- matrix-weechat and pantalaimon (matrix-nio 0.12.0)
+- famedlysdk