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

MSC1544: Key verification using QR codes #1544

Merged
merged 26 commits into from
Nov 25, 2020

Conversation

uhoreg
Copy link
Member

@uhoreg uhoreg commented Aug 20, 2018

ara4n
ara4n previously requested changes Aug 21, 2018
proposals/1543-qr_code_key_verification.md Outdated Show resolved Hide resolved
proposals/1543-qr_code_key_verification.md Outdated Show resolved Hide resolved
@richvdh richvdh changed the title [WIP] Key verification using QR codes [WIP] MSC1543: Key verification using QR codes Aug 30, 2018
@erikjohnston
Copy link
Member

Can we not reuse the "Interactive Key Verification" method, and simply have the QR code used to set the user and device that is being verified?

@uhoreg
Copy link
Member Author

uhoreg commented Dec 11, 2018

Can we not reuse the "Interactive Key Verification" method, and simply have the QR code used to set the user and device that is being verified?

Possibly, but if we're scanning a QR code, it doesn't inconvenience the user to also throw in the key data. Also, the interactive key verification is more complicated, and so people may implement this before implementing the other.

@erikjohnston
Copy link
Member

Can we not reuse the "Interactive Key Verification" method, and simply have the QR code used to set the user and device that is being verified?

Possibly, but if we're scanning a QR code, it doesn't inconvenience the user to also throw in the key data. Also, the interactive key verification is more complicated, and so people may implement this before implementing the other.

My main concern is basically being able to reuse the same method for other QR code style mechanisms, but I take your point. I was hoping that we could split up IKV into two steps or something, so that you could reuse some of it when you already had a shared secret

@anoadragon453 anoadragon453 added proposal A matrix spec change proposal and removed T-Core labels Jan 4, 2019
@uhoreg uhoreg added the e2e label Apr 1, 2019
Copy link
Member

@turt2live turt2live left a comment

Choose a reason for hiding this comment

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

seems sane - let's see what the implementation comes up with

proposals/1543-qr_code_key_verification.md Outdated Show resolved Hide resolved
@richvdh richvdh moved this from Awaiting FCP ticks to In progress in Spec Core Team Backlog Sep 10, 2020
@richvdh richvdh moved this from In progress to Awaiting FCP ticks in Spec Core Team Backlog Sep 10, 2020
@anoadragon453 anoadragon453 added this to Ready for FCP ticks in Spec Core Team Backlog Sep 11, 2020
proposals/1543-qr_code_key_verification.md Outdated Show resolved Hide resolved
proposals/1543-qr_code_key_verification.md Outdated Show resolved Hide resolved
@mscbot
Copy link
Collaborator

mscbot commented Nov 20, 2020

🔔 This is now entering its final comment period, as per the review above. 🔔

@mscbot mscbot added final-comment-period This MSC has entered a final comment period in interest to approval, postpone, or delete in 5 days. and removed proposed-final-comment-period Currently awaiting signoff of a majority of team members in order to enter the final comment period. labels Nov 20, 2020
@@ -0,0 +1,375 @@
Bi-directional Key verification using QR codes
Copy link
Contributor

@Sorunome Sorunome Nov 20, 2020

Choose a reason for hiding this comment

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

While soru knows that she isn't part of the spec core team she is concerned that this isn't addressed properly. Without having more ping-pong game going around to actually know what is going on key verification is not future-proof for when new methods come around.

Soru has discussed alternatives of how the exact same UX could be implemented, with the only difference being that, well, it is future proof.

Just re-iterating this as the MSC is enting FCP and that is something that the spec core team should probably take into consideration.

Co-authored-by: David Baker <dbkr@users.noreply.github.com>
@mscbot
Copy link
Collaborator

mscbot commented Nov 25, 2020

The final comment period, with a disposition to merge, as per the review above, is now complete.

@mscbot mscbot added finished-final-comment-period and removed disposition-merge final-comment-period This MSC has entered a final comment period in interest to approval, postpone, or delete in 5 days. labels Nov 25, 2020
@turt2live
Copy link
Member

This has finished FCP and we've talked about the contentious points extensively in the #matrix-spec:matrix.org room - see those threads for details on their conclusion.

As such, this MSC is landing as-is though can't be converted to spec until #2366 also lands. It is my intention to propose 2366 for the spec core team's focus for the coming week.

We don't really have a label state for this sort of case, so I'm just going to leave it in finished-fcp until 2366 is landed too.

@turt2live turt2live merged commit bce1bf3 into matrix-org:master Nov 25, 2020
@uhoreg uhoreg moved this from Ready for FCP ticks to Done to some definition in Spec Core Team Backlog Dec 15, 2020
@turt2live turt2live added spec-pr-missing Proposal has been implemented and is being used in the wild but hasn't yet been added to the spec and removed finished-final-comment-period labels Mar 23, 2021
@turt2live turt2live added spec-pr-in-review A proposal which has been PR'd against the spec and is in review and removed spec-pr-missing Proposal has been implemented and is being used in the wild but hasn't yet been added to the spec labels Apr 28, 2021
@uhoreg uhoreg added merged A proposal whose PR has merged into the spec! and removed spec-pr-in-review A proposal which has been PR'd against the spec and is in review labels Apr 28, 2021
@uhoreg
Copy link
Member Author

uhoreg commented Apr 28, 2021

Merged! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cross-signing-sprint e2e kind:core MSC which is critical to the protocol's success merged A proposal whose PR has merged into the spec! proposal A matrix spec change proposal proposal-pr
Projects
Archived in project
Spec Core Team Backlog
  
Done to some definition
Development

Successfully merging this pull request may close these issues.

None yet