Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MSC3906: Mechanism to allow sign in and E2EE set up via QR code #3906

Closed
wants to merge 17 commits into from

Conversation

hughns
Copy link
Member

@hughns hughns commented Oct 13, 2022

Rendered

Implementations:

Related MSCs for convenience:

  • MSC3882 to obtain a m.login.token
  • MSC3886 to provide a open non-trusted rendezvous endpoint
  • MSC3903 to secure the rendezvous channel

@hughns hughns marked this pull request as draft October 13, 2022 10:35
@hughns hughns changed the title Proposal to allow sign in and E2EE set up via QR code MSC3906: Proposal to allow sign in and E2EE set up via QR code Oct 13, 2022
@hughns hughns marked this pull request as ready for review October 13, 2022 10:55
@hughns hughns changed the title MSC3906: Proposal to allow sign in and E2EE set up via QR code MSC3906: Mechanism to allow sign in and E2EE set up via QR code Oct 13, 2022
@uhoreg uhoreg added e2e proposal A matrix spec change proposal kind:feature MSC for not-core and not-maintenance stuff needs-implementation This MSC does not have a qualifying implementation for the SCT to review. The MSC cannot enter FCP. labels Oct 13, 2022
@uhoreg
Copy link
Member

uhoreg commented Oct 13, 2022

I've added the needs-implementation label because we need to check the linked implementations.

proposals/3906-sign-in-with-qr.md Outdated Show resolved Hide resolved
proposals/3906-sign-in-with-qr.md Outdated Show resolved Hide resolved
proposals/3906-sign-in-with-qr.md Outdated Show resolved Hide resolved
@turt2live turt2live requested a review from a team February 28, 2023 02:36
@turt2live turt2live added this to Needs idea feedback / initial review in Spec Core Team Backlog via automation Feb 28, 2023
Copy link
Member

@dbkr dbkr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks generally good as far as I can see, although I echo @uhoreg 's comment about m.login.progress having a few different purposes.

Comment on lines +221 to +222
8. A 12 numerical digit confirmation code derived from the shared key used by
the rendezvous channel must be displayed on both devices.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I doubt anyone actually checks those. What prevents us from instead showing a second QR code?

In theory, since device A shared its public key securely via a QR code, it should be able to trust, that anything sent to it can be trusted. As such device B should be able to send device A the 12 digit code over that channel and device A then decides to cancel the login or not. Device A could share the 12 digit code to B using a QR code before that, although I am not sure if that is necessary, since device B has direct line of sight to A.

I am probably missing something, but relying on users to manually verify 12 digits to ensure, that the channel is secure AFTER they already scanned a QR code sounds like something nobody would do, so making that automatic should improve security imo.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In theory, since device A shared its public key securely via a QR code, it should be able to trust, that anything sent to it can be trusted.

That's not correct. Device A can trust that an encrypted message that it receives was not decrypted or tampered with in transit, but it can't verify the source of that message to begin with. It can't determine whether the message was sent directly from Device B, or if Device M intercepted Device B's message and replaced it with their own message.

That said, I think that since Device B can trust that Device A's public key is the right one, we should be able to use that to simplify things, similar to what we do in QR code-based verification.

Though I'm not sure that this step belongs in this MSC. It seems to me that it fits better in one of the MSCs that defines how the secure rendezvous channel operates, ensuring that the channel is secure should be part of the channel establishment -- we shouldn't be sending data over the channel until we know it's secure.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But since Device B can trust that Device A's public key is the right one, if Device B sends it the confirmation key A sent over the QR code, then there should be no way for Device M to intercept the message, because it is encrypted using Device A's public key, which in theory could only have been intercepted by someone in the same room. So that way Device A should be able to establish this connection as trusted.

I don't think you can really put that into the rendevouz MSC, because that works with arbitrary channels, while in this MSC you are explicitly using an asymmetric, one directional, secure channel (the QR code). However automatically verifying the channel as secure, by sending a secret from B to A using A's public key should be able to mark the channel as trusted also in the direction A to B.

In general I just don't like the manual confirm step, because I don't think people will verify those numbers after they already scanned a QR code. And I don't think it should be necessary, if you don't restrict yourself to one QR code.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But since Device B can trust that Device A's public key is the right one, if Device B sends it the confirmation key A sent over the QR code, then there should be no way for Device M to intercept the message, because it is encrypted using Device A's public key, which in theory could only have been intercepted by someone in the same room. So that way Device A should be able to establish this connection as trusted.

Right, if Device A scans a QR code generated by Device B that contains the confirmation key, then all is good. However, we this requires that both devices are able to scan QR codes -- Device B scanning Device A initially to get its public key, and then Device A scanning device B to get the confirmation key. Ideally, we would only need one device to do the scanning.

I don't think you can really put that into the rendevouz MSC, because that works with arbitrary channels, while in this MSC you are explicitly using an asymmetric, one directional, secure channel (the QR code). However automatically verifying the channel as secure, by sending a secret from B to A using A's public key should be able to mark the channel as trusted also in the direction A to B.

My understanding is that the "secure channel" is bi-directional, and the QR code is only used to help initialize the channel. Since we're reading this differently, it seems like this should be made clearer in the MSC, exactly how this works.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok so you are concerned over that users verify a 12 digit code. Why dont we just use the Emoji system we use for verification? Like UX wise numbers are unacceptable no matter what. The Emoji system is WAY more usable. Comparing 16 emojies is actually realistic compared to 12 digits (Yes its that bad i would argue).

Like this is why the NATO alphabet exists. Because using distinct words is way more clear and the same goes for showing a set of Emoji in your UI instead of digits.

Im not going to comment on the need or lack of need for this mechanism as im commenting from a prespective of someone who knows enough UX and enough matrix to conclude that dont we already have a system that solves this very type of problem. Users having to verify long blobs of data to prevent MITM and other malicious actions.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With regards to "UX wise numbers are unacceptable no matter what":

I would argue that the general human population are already much more familiar with handling a sequence of numbers in a security context - e.g. 4 digit PIN from a bank card or 6 digits from an SMS/authenticator OTP - than handling a sequence of emojis or words.

So, UX wise we can can make it easier for users by utilising something they are familiar with rather than having to educate them on an (entirely) new concept.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with previous comments that virtually nobody is going to bother comparing numbers after already scanning a QR code, and I think the same will hold true for a comparison of emojis.

I propose instead that a 4‑ or 6‑digit numeric code should be generated on one of the clients (doesn't matter which from a UX standpoint) that the user then has to actively type in on the other client.

Requiring the user to actively input a code is a far more effective mitigation against interception than a system in which the user can simply press 'accept' without actually performing any verification. Google, for example, employs a similar system where the user is asked to choose the matching number out of three choices. The proposed comparison, either of emojis or numbers, is for the average user no better than Discord's sorely inadequate accept‑or‑deny prompt that has allowed "Free Nitro" and other account-compromising scams to continue being effective.

(I do believe a similar change should be made to the cross‑signing UX, but that is out of scope for this discussion.)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would argue that the general human population are already much more familiar with handling a sequence of numbers in a security context - e.g. 4 digit PIN from a bank card or 6 digits from an SMS/authenticator OTP - than handling a sequence of emojis or words.

You are aware that even at the invention of PIN codes, this UX was questionable, which is why we have only 4 numbers in a general banking pincode? https://en.wikipedia.org/wiki/Personal_identification_number#PIN_length

de-duplicate payload types
finish=>failure+reason, declined
add flow field to specify algorithm
@hughns hughns requested a review from uhoreg March 2, 2023 09:50
@turt2live turt2live moved this from Needs idea feedback / initial review to Proposed for FCP readiness in Spec Core Team Backlog Mar 7, 2023
@turt2live
Copy link
Member

The author believes this is ready for FCP.

@matrix-org/spec-core-team : Please review the implementation and MSC with an aim of @mscbot fcp merge-ing it.

Copy link
Member

@uhoreg uhoreg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall, I think the idea is sound, but could do with some clarifications.

proposals/3906-sign-in-with-qr.md Outdated Show resolved Hide resolved
proposals/3906-sign-in-with-qr.md Outdated Show resolved Hide resolved
proposals/3906-sign-in-with-qr.md Outdated Show resolved Hide resolved
proposals/3906-sign-in-with-qr.md Show resolved Hide resolved
Comment on lines +221 to +222
8. A 12 numerical digit confirmation code derived from the shared key used by
the rendezvous channel must be displayed on both devices.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In theory, since device A shared its public key securely via a QR code, it should be able to trust, that anything sent to it can be trusted.

That's not correct. Device A can trust that an encrypted message that it receives was not decrypted or tampered with in transit, but it can't verify the source of that message to begin with. It can't determine whether the message was sent directly from Device B, or if Device M intercepted Device B's message and replaced it with their own message.

That said, I think that since Device B can trust that Device A's public key is the right one, we should be able to use that to simplify things, similar to what we do in QR code-based verification.

Though I'm not sure that this step belongs in this MSC. It seems to me that it fits better in one of the MSCs that defines how the secure rendezvous channel operates, ensuring that the channel is secure should be part of the channel establishment -- we shouldn't be sending data over the channel until we know it's secure.


## Secure channel prerequisite

This proposal relies on a secure "rendezvous" channel having been established
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need more of a description of what "secure" means here. I believe that what we need here is secrecy (an attacker can't read the messages), integrity (an attacker can't modify messages), and authentication (we can determine that the message was sent from the other device).

Edit: also, it seems like there are some extra concepts ("flow" and "intent") that are part of the rendezvous channel establishment. It would be helpful to have those explained here. If they're explained in other MSCs, it would be helpful to have pointers to those MSCs here, and maybe give a summary of it here, or even just a list.

This proposal defines the `m.setup.additional_device.v2` flow that is to be
used once a secure channel has been established.

The following `intent` values are defined:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I find it confusing that we're defining intent values before we say what the values are for. I think this can be solved by adding some more words. Maybe something along the lines of: "This proposal involves communication between two devices: one that is already logged in, and one that is not logged in. We assign intent values, which are sent as part of the rendezvous channel establishment(?), to identify the two devices. These values are..."

Or maybe it could be solved with more words about how the secure channel is established, if this is indeed part of the secure channel establishment? Will add another comment above.


## Proposal

This proposal defines the `m.setup.additional_device.v2` flow that is to be
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar to the comment below, I don't know what "flow" means in this context. "Flow" is mentioned in the example above, but it's not clear how that example relates to the general case. Is it defined in a different MSC?

proposals/3906-sign-in-with-qr.md Outdated Show resolved Hide resolved
Co-authored-by: Hubert Chathi <hubertc@matrix.org>
@turt2live turt2live removed this from Proposed for FCP readiness in Spec Core Team Backlog Mar 14, 2023
@turt2live
Copy link
Member

@hughns fyi this has entered a "not active SCT focus" state, which is to say it feels like it's back with you for the moment. When this changes (or if the status is wrong), let us know in the SCT office.

@hughns
Copy link
Member Author

hughns commented Apr 9, 2024

It is proposed that MSC4108 supersedes this MSC.

@uhoreg
Copy link
Member

uhoreg commented Apr 23, 2024

It is proposed that MSC4108 supersedes this MSC.

@hughns As the author of this MSC, you can close this MSC if it is superseded by another one.

@hughns
Copy link
Member Author

hughns commented Apr 30, 2024

Closing this PR as #4108 is now ready for review.

@hughns hughns closed this Apr 30, 2024
@turt2live turt2live added the obsolete A proposal which has been overtaken by other proposals label Apr 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
e2e hacktoberfest-accepted kind:feature MSC for not-core and not-maintenance stuff needs-implementation This MSC does not have a qualifying implementation for the SCT to review. The MSC cannot enter FCP. obsolete A proposal which has been overtaken by other proposals proposal A matrix spec change proposal
Projects
Status: Done for now
Development

Successfully merging this pull request may close these issues.

None yet

9 participants