# 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