From 9dc6e00335e2c91f14c46fd75acbac0a9bb5dc4f Mon Sep 17 00:00:00 2001 From: Hubert Chathi Date: Mon, 4 May 2020 22:49:34 -0400 Subject: [PATCH 1/8] draft of proposal for SAS --- proposals/xxxx-sas-v2.md | 50 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) create mode 100644 proposals/xxxx-sas-v2.md diff --git a/proposals/xxxx-sas-v2.md b/proposals/xxxx-sas-v2.md new file mode 100644 index 00000000..b4652ff8 --- /dev/null +++ b/proposals/xxxx-sas-v2.md @@ -0,0 +1,50 @@ +# SAS verification, v2 + +## Proposal + +A new `key_agreement_protocol`, `curve25519-hkdf-sha256` is introduced. 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. + +A new `message_authentication_code` method, `hkdf-hmac-sha256.v2` is introduced. It +is the same as `hkdf-hmac-sha256`, except that the info parameter for the HKDF +is the concatenation of: + + * The string `MATRIX_KEY_VERIFICATION_MAC|`. + * The Matrix ID of the user whose key is being MAC-ed, followed by `|`. + * The Device ID of the device sending the MAC, followed by `|`. + * The Matrix ID of the other user, followed by `|`. + * The Device ID of the device receiving the MAC, followed by `|`. + * The transaction_id being used, followed by `|`. + * The Key ID of the key being MAC-ed, or the string `KEY_IDS` if the item + being MAC-ed is the list of key IDs. + +A new `short_authentication_string` method, `emoji.v2` is introduced. It is +the same as `emoji`, but emoji number 34 is changed from 🔧 (`U+1F527` Spanner) +to ⭐ (`U+2B50` Star). + +The `key_agreement_protocol` `curve25519`, `message_authentication_code` +`hkdf-hmac-sha256`, and `short_authentication_string` `emoji` are deprecated. + +## Potential issues + +## Alternatives + +## Security considerations + +## Unstable prefix From 86d0d04ed19a3e6a15e0de5fa98874bcaa94e33a Mon Sep 17 00:00:00 2001 From: Hubert Chathi Date: Mon, 11 May 2020 11:54:50 -0400 Subject: [PATCH 2/8] don't include unrelated changes --- proposals/xxxx-sas-v2.md | 24 +++++------------------- 1 file changed, 5 insertions(+), 19 deletions(-) diff --git a/proposals/xxxx-sas-v2.md b/proposals/xxxx-sas-v2.md index b4652ff8..0e689c30 100644 --- a/proposals/xxxx-sas-v2.md +++ b/proposals/xxxx-sas-v2.md @@ -21,25 +21,11 @@ concatenation of: which sent the `m.key.verification.accept` message, followed by `|`. * The `transaction_id` being used. -A new `message_authentication_code` method, `hkdf-hmac-sha256.v2` is introduced. It -is the same as `hkdf-hmac-sha256`, except that the info parameter for the HKDF -is the concatenation of: - - * The string `MATRIX_KEY_VERIFICATION_MAC|`. - * The Matrix ID of the user whose key is being MAC-ed, followed by `|`. - * The Device ID of the device sending the MAC, followed by `|`. - * The Matrix ID of the other user, followed by `|`. - * The Device ID of the device receiving the MAC, followed by `|`. - * The transaction_id being used, followed by `|`. - * The Key ID of the key being MAC-ed, or the string `KEY_IDS` if the item - being MAC-ed is the list of key IDs. - -A new `short_authentication_string` method, `emoji.v2` is introduced. It is -the same as `emoji`, but emoji number 34 is changed from 🔧 (`U+1F527` Spanner) -to ⭐ (`U+2B50` Star). - -The `key_agreement_protocol` `curve25519`, `message_authentication_code` -`hkdf-hmac-sha256`, and `short_authentication_string` `emoji` are deprecated. +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. ## Potential issues From c196cbcf42ccdab92de5f72b641a65ddf4462c8e Mon Sep 17 00:00:00 2001 From: Hubert Chathi Date: Thu, 14 May 2020 13:01:24 -0400 Subject: [PATCH 3/8] add introduction and discourage new implementations from using old method --- proposals/xxxx-sas-v2.md | 29 +++++++++++++++++------------ 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/proposals/xxxx-sas-v2.md b/proposals/xxxx-sas-v2.md index 0e689c30..e4e4e320 100644 --- a/proposals/xxxx-sas-v2.md +++ b/proposals/xxxx-sas-v2.md @@ -1,9 +1,21 @@ -# SAS verification, v2 +# 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. ## Proposal -A new `key_agreement_protocol`, `curve25519-hkdf-sha256` is introduced. It is -the same as `curve25519` except that the info parameter for the HKDF is the +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|`. @@ -25,12 +37,5 @@ 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. - -## Potential issues - -## Alternatives - -## Security considerations - -## Unstable prefix +the future. It will no longer be mandatory for clients to support, and new +implementations are discouraged from implementing it. From f8e4bdfbaf1bdda634e45a81347fe09f96e3e676 Mon Sep 17 00:00:00 2001 From: Hubert Chathi Date: Tue, 2 Jun 2020 17:31:15 -0400 Subject: [PATCH 4/8] credit David Wong --- proposals/xxxx-sas-v2.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/proposals/xxxx-sas-v2.md b/proposals/xxxx-sas-v2.md index e4e4e320..d9d37471 100644 --- a/proposals/xxxx-sas-v2.md +++ b/proposals/xxxx-sas-v2.md @@ -11,6 +11,9 @@ 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 From 0fbb1b9bf9c18c7705263a0364663f3f1bd1aec2 Mon Sep 17 00:00:00 2001 From: Hubert Chathi Date: Thu, 11 Jun 2020 16:25:07 -0400 Subject: [PATCH 5/8] add information on fixed implementations --- proposals/xxxx-sas-v2.md | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/proposals/xxxx-sas-v2.md b/proposals/xxxx-sas-v2.md index d9d37471..d57f5119 100644 --- a/proposals/xxxx-sas-v2.md +++ b/proposals/xxxx-sas-v2.md @@ -42,3 +42,14 @@ 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) +- FluffyChat (famedlysdk) From d9dc3dc1801a7dfde7d45680c3d1191b9985abf1 Mon Sep 17 00:00:00 2001 From: Hubert Chathi Date: Thu, 11 Jun 2020 16:30:47 -0400 Subject: [PATCH 6/8] rename to match MSC number --- proposals/{xxxx-sas-v2.md => 2630-sas-check-public-keys.md} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename proposals/{xxxx-sas-v2.md => 2630-sas-check-public-keys.md} (100%) diff --git a/proposals/xxxx-sas-v2.md b/proposals/2630-sas-check-public-keys.md similarity index 100% rename from proposals/xxxx-sas-v2.md rename to proposals/2630-sas-check-public-keys.md From 1961a215e8eae5c41e1cc19301c18454b45153c6 Mon Sep 17 00:00:00 2001 From: Hubert Chathi Date: Thu, 11 Jun 2020 16:36:25 -0400 Subject: [PATCH 7/8] Fix the title --- proposals/2630-sas-check-public-keys.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proposals/2630-sas-check-public-keys.md b/proposals/2630-sas-check-public-keys.md index d57f5119..080fe795 100644 --- a/proposals/2630-sas-check-public-keys.md +++ b/proposals/2630-sas-check-public-keys.md @@ -1,4 +1,4 @@ -# checking public keys in SAS verification +# 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 From 6ca3996bef5f94a6e72e850cf33f0d091418db3d Mon Sep 17 00:00:00 2001 From: Hubert Chathi Date: Thu, 11 Jun 2020 16:39:08 -0400 Subject: [PATCH 8/8] FluffyChat doesn't include any verification yet --- proposals/2630-sas-check-public-keys.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proposals/2630-sas-check-public-keys.md b/proposals/2630-sas-check-public-keys.md index 080fe795..e42a2528 100644 --- a/proposals/2630-sas-check-public-keys.md +++ b/proposals/2630-sas-check-public-keys.md @@ -52,4 +52,4 @@ This has been implemented in: - RiotX 0.21 - Riot iOS 0.11.5 (matrix-ios-sdk 0.16.5) - matrix-weechat and pantalaimon (matrix-nio 0.12.0) -- FluffyChat (famedlysdk) +- famedlysdk