element-hq / element-web

A glossy Matrix collaboration client for the web.
https://element.io
GNU Affero General Public License v3.0
11.13k stars 1.98k forks source link

Reset secure backup does not check existing 4S before deleting key backup #27841

Open MarcWadai opened 2 months ago

MarcWadai commented 2 months ago

Steps to reproduce

  1. Where are you starting? What can you see?
    • Get an account with existing 4S setup
    • Open a new web session, enter security key
    • Go to security settings and reset the secure storage on this session
    • A new key backup is recreated with the version number updated
    • There is a possible lost of keys since all keys have not been download. The new key backup will only contain local keys for this session

-> This reset button behavior is different from Android !

Outcome

What did you expect?

Keeping the current key backup if the session already knows the private key of the key backup . This behavior is the one found on element-android. If 4S is well setup,the key backup is not recreated and the version number not updated. See https://github.com/element-hq/element-android/issues/8814#issuecomment-2196991876

in fact resets secret storage (which is used to store the backup key), but if backup was working on the client before the reset, the same key backup should continue to work afterwards

In matrix-js-sdk, we can see written in the code https://github.com/matrix-org/matrix-js-sdk/blob/6f63ff1711664154359bb1b998a80f4274569468/src/rust-crypto/rust-crypto.ts#L1192 that no check on 4S is done. From the front we always pass the value setupNewKeyBackup = true to the method bootstrapSecretStorage https://github.com/matrix-org/matrix-js-sdk/blob/6f63ff1711664154359bb1b998a80f4274569468/src/rust-crypto/rust-crypto.ts#L748

This check is important because there are cases where the users can lose their messages. Indeed, since we don't download a full copy of the keys https://github.com/element-hq/element-meta/issues/2446 anymore, we can have a state where the re-creation of the secret backups doesn't contain all the keys.

What happened instead?

The key backup is completely reset and recreated, even though there are probably keys from the previous backup have not been fully downloaded.

Operating system

linux

Browser information

firefox 128

URL for webapp

app.element.io

Application version

1.11.71

Homeserver

matrix.org

Will you send logs?

No

yostyle commented 2 months ago

Hi @richvdh @BillCarsonFr @giomfo. Could you take time to check this issue on Element Web ?

richvdh commented 2 months ago

@MarcWadai:

Go to security settings and reset the secure storage on this session

Can you confirm which option you are pressing under "security settings"? A screenshot would help, if possible.

MarcWadai commented 2 months ago

image

richvdh commented 2 months ago

Thanks! So yes, that button is to reset secure backup, rather than resetting 4S, though it does reset 4S as a side-effect. I don't think it would make sense for a "Reset" button within the "Secure backup" settings to retain the existing key backup.

I agree the UX is very confusing here; improving it is part of https://github.com/element-hq/element-web/issues/26468.

The fact that existing keys are lost when key backup is reset is tracked as https://github.com/element-hq/element-meta/issues/2446.

yostyle commented 2 months ago

I don't think it would make sense for a "Reset" button within the "Secure backup" settings to retain the existing key backup.

@richvdh the behaviour of this reset button should be aligned on Element clients. If a user loses his recovery key or passphrase this button is the unique way to generate a new one. This case will be more frequent when users want to migrate to Element X because they have to use the recovery key or passphrase to decrypt their messages.

The same button on Element Android doesn't destroy the existing backup and retain the existing key backup. image

richvdh commented 2 months ago

I have to say that if "Reset Secure Backup" does not actually reset the secure backup, that seems like a bug in Element-Android...

There's certainly an argument that there should be a "Reset Key Storage" "Change recovery key" option in both applications. Generally I think the Element Web settings page needs some serious design work.