matrix-org / matrix-spec

The Matrix protocol specification
Apache License 2.0
182 stars 94 forks source link

Explicit key verification signaling #722

Open Sorunome opened 3 years ago

Sorunome commented 3 years ago

This is a continuation of the discussion on https://github.com/matrix-org/matrix-doc/pull/1544

The need arose for explicit signaling so that each client knows what verification method is being presented, and to be able to switch between them. QR code verification would need modification to reciprocate in a separate package from "start".

Sorus idea would be to allow sending multiple start's. Each new start re-starts / switches to that verification method. To be able to deduplicate correctly which start to use, the package would receive an integer counter. If it is absent or malformed (not an int) the counter is to be interpreted as 0. Each client is only allowed to send the same counter once, meaning we can have a maximum of two same counters (both clients)

The start with the highest counter is used. If there are multiple, then the one with the lexographically smaller mxid is used. if they match, then lexographically smaller device ID (same as in matrix-org/matrix-spec-proposals#2366 ) The difference to matrix-org/matrix-spec-proposals#2366 is, that the method picked can be different, and the start can be sent after other packages have been exchanged already, but not after an error or a done. This way clients can still switch to a different method after verification is initialized.

Of course there are other ways to model this, this is just one suggestion.

Clients could still present multiple verification methods on-screen (e.g. qr + nfc) and listen to them. If a different method is used than is started it could just immidiately send a new start for that one. e.g. client 1 sends a start for nfc, but listens both for nfc and displays a qr code. client 2 then scans the qr code, sends a start for qr code, and then sends the reciprocate.

uhoreg commented 3 years ago

I have to say that I'm not too fond of having unlimited starts that can override each other in a single verification flow. It seems to me like it will introduce more complexity in the system, add new edge cases that need to be figured out, etc, which is something that I would like to avoid if possible.

But I think that we can satisfy both our desires with some other changes to the verification framework.

  1. When one user makes a verification request, they also list what verification methods they're willing to auto-display. Only certain verification methods will support auto-display (currently, only QR code showing)
  2. When the second user accepts the verification, they can say what one* method they're going to auto-display (if any)
  3. User 1's client will also auto-display that type. Both clients will present buttons for switching verification methods.
  4. Verification proceeds as normal; if a user selects a different verification method from the auto-displayed type, then the auto-displayed type disappears on both clients.

* (should it just be limited to one method? or let them select a list of non-conflicting methods?)

In this way, we avoid having multiple start messages with different priorities, but we still have explicit signalling of what the clients are doing. Does this sound workable?

Sorunome commented 3 years ago

While your proposed system would also work, to soru it seems more complicated and limited than the original suggestion.

A strength of the original thing was if, let's say you pick QR code but only after that notice your camera is broken you can still seemlessly switch over to NFC

So yes, that does sound like a working compromise (would love to get @deepbluev7 's feedback, too), but to soru it seems.....more complicated and like it could have more edge cases, rather than fewer.

One of the issues with QR code was also how badly it works with the existing verification flow stuff (emoji), that it is really hard to code it for clients. Multiple starts would also make that more elegant.

TL;DR that does sound like a working compromise but soru thinks it is still limited and can still be improved even more, with the original suggestion

uhoreg commented 3 years ago

I think that https://github.com/matrix-org/matrix-doc/pull/3131 should address this.