search

element-hq / element-meta

Shared/meta documentation and project artefacts for Element clients
70 stars 12 forks source link

Invisible crypto: display a warning when a verified user changes identity #2492

Open richvdh opened 1 month ago

richvdh commented 1 month ago

Part of https://github.com/element-hq/element-meta/issues/2491, itself part of Invisible crypto.

We need to display a warning when a user which we have previously verified changes their cryptographic identity.

See also https://github.com/element-hq/element-meta/issues/2500 and https://github.com/element-hq/element-web/issues/27943, which concern showing an error if we get as far as sending a message despite this warning (eg, there is a race).

Note also: previously we discussed showing a notice if an unverified (tofu-trusted) user changes identity. We now believe that is unnecessary and that a message in the timeline (#2493) is sufficient EDIT 2024/08/28: we still believe this is the case in the long term, but since #2493 looks like a reasonable amount of work, we need a warning banner in the meantime. #2513 tracks this work.

Figma designs:

richvdh commented 1 month ago

What happens if there are 2, or 5, or 50, affected users? (Likely if you reopen a session that has been closed for a while)

richvdh commented 1 month ago

XXX: not sure we actually need to lock out the composer, since the attempt to send the message will fail (https://github.com/matrix-org/matrix-rust-sdk/issues/3793)?

It's better UX, rather than waiting for the user to send the message and then getting an error.

richvdh commented 1 month ago

XXX: isn't the fact that it doesn't lock out the composer counter to https://github.com/matrix-org/matrix-rust-sdk/issues/3565?

The banner needs to have been on screen for some seconds before it is considered that the user has seen it; if it's been on screen for more than N seconds, then the UI updates pinned key in the backend; so then there will be no error (and the banner will not be shown in future rooms).

richvdh commented 1 month ago

The banner needs to have been on screen for some seconds before it is considered that the user has seen it; if it's been on screen for more than N seconds, then the UI updates pinned key in the backend; so then there will be no error (and the banner will not be shown in future rooms).

@pmaier1 says we don't need to block sending at all in tofu mode -- if you care about who receives your messages, you need to verify.

pmaier1 commented 1 month ago

The TOFU identity change design needs another iteration, I think. At least we discussed to not block the user in this scenario.

mxandreas commented 3 weeks ago

To make sure that there's no ambiguity in the overall model, and it is formally sound, I'd like to confirm the following before we go into the details of UI. Especially, as it can affect quite a bit some of the more specific scenarios.

a) My current understanding is that as soon as Alice's identity changes, Bob implicitly pins the new identity, regardless of what of what Bob sees or does on the UI. For example, if Bob has verified Alice and Alice changes their identity 2 times (without Bob doing anything on UI yet), then this counts as 2 identity changes - the first one is a verified identity change, and the second is a pinned identity change.

b) An alternative to this model is that there is an in-between state v-changed or p-changed which lasts until Bob takes some action. For example, if Bob has verified Alice and Alice changes their identity 2 times (without Bob doing anything on UI yet), then this counts as 1 identity change. Because after the first change, Alice landed in this v-changed stated and in this state we're ignoring all further identity changes until Bob takes action (e.g. withdraws/pins or verifies).

In my discussions with @pmaier1 I thought that a) is what we're using but some of the questions in here lead me to think that may be it is not the case.

Here's a model for a) in which case the Changed states generate a warning but are virtual in the sense that automatic/instant transition to the Pinned state follows without any user action. image

richvdh commented 3 weeks ago

a) My current understanding is that as soon as Alice's identity changes, Bob implicitly pins the new identity, regardless of what of what Bob sees or does on the UI.

I don't think this is correct. First of all, per https://github.com/element-hq/element-meta/issues/2492#issuecomment-2269146688, we try very hard to make sure that Bob gets a chance to see the warning before we update his "pinned" copy of Alice's identity. Secondly, if he had previously verified Alice, then he would actually have to go through the verification process again (or click "withdraw verification") before updating the pinned identity.

For example, if Bob has verified Alice and Alice changes their identity 2 times (without Bob doing anything on UI yet), then this counts as 2 identity changes - the first one is a verified identity change, and the second is a pinned identity change.

Worth noting that we don't store a separate "verified identity" -- we store (a) the most-recently-seen identity, and (b) the pinned identity, each of which may or may not be verified.

In this example, yes there are two identity changes, but they are both identity changes for a verified user, and therefore qualify for the stronger, blocking, "you must re-verify or withdraw verification before proceeding" warning.

mxandreas commented 3 weeks ago

Worth noting that we don't store a separate "verified identity" -- we store (a) the most-recently-seen identity, and (b) the pinned identity, each of which may or may not be verified.

Good to know. Also, good example why we should have an agreed model that guides implementation and UI. Very hard to tell which parts of this implementation logic (e.g. in terms of what gets stored) is just a way to realize the desired model, and which are the model we desire.

In this example, yes there are two identity changes, but they are both identity changes for a verified user

What qualifies as a verified user (given the storage mechanism that was mentioned)?

mxandreas commented 3 weeks ago

@richvdh I created a another state diagram based on what you said. I did not validate if it can be unambiguously mapped to the implementation/storage that you highlighted but perhaps we can agree first if that is what we want and need to drive the measures of trust and the UX.

image

americanrefugee commented 3 weeks ago

Identity change screens for all platforms:

richvdh commented 3 weeks ago

Identity change screens for all platforms:

Just for reference: these designs span both this issue ("show a warning above the composer") and #2493.

richvdh commented 3 weeks ago

See also my notes from the meeting earlier, at https://github.com/element-hq/element-meta/issues/2491#issuecomment-2301947967

richvdh commented 2 weeks ago

I think maybe the best way to implement this is for matrix-sdk-crypto to expose a list of "previously-verified users who have changed identity" (and a Stream for updates to the list), and then the UI can filter that list according to the list of encryption target members for the room.

richvdh commented 2 weeks ago

currently, the designs for this include the wording "\’s verified identity has changed and their messages are hidden". However, that won't actually be true until we do https://github.com/element-hq/crypto-internal/issues/353, https://github.com/element-hq/crypto-internal/issues/354 and https://github.com/element-hq/crypto-internal/issues/362

richvdh commented 2 weeks ago

currently, the designs for this include the wording "’s verified identity has changed and their messages are hidden".

Per the updated designs above, this is no longer the case.

andybalaam commented 2 days ago

To clarify:

richvdh commented 1 day ago

for this task, the messages will appear, but with no content, just a placeholder error about the user's verified identity being changed.

I think that's a separate task (https://github.com/element-hq/crypto-internal/issues/362)