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

"Connect key backup" prompts me to enter my security key after submitting my security key #27469

Open
lampholder opened this issue May 14, 2024 · 3 comments
Labels
A-E2EE A-E2EE-Key-Backup O-Occasional Affects or can be seen by some users regularly or most users rarely S-Minor Impairs non-critical functionality or suitable workarounds exist T-Defect

Comments

@lampholder
Copy link
Member

lampholder commented May 14, 2024

Steps to reproduce

  1. Create a fresh account.
  2. Create/join an encrypted room and set up key backup following the prompts. Don't use a passphrase - generate and save a key instead.
  3. Log out.
  4. Log in.
  5. Ignore all the prompts for verifying/connecting to secure backup
  6. Go to Settings -> Security and Privacy
  7. Click Connect this session to key backup. Observe this dialog:
    image
  8. Click upload even though uploading is 1000% not what you are ever doing with your secure key
  9. Choose your file and click continue
  10. Uh oh, here's an uninvited prompt to enter my security key again, this time by typing it: Screenshot 2024-05-14 at 19 11 04
  11. If you re-enter your security key, you get an error dialog:
    image

Outcome

What did you expect?

  • A more sensible verb than upload to be attached to the activity of unlocking key backup with my locally stored key
  • Not that extra prompt

Operating system

mac

Browser information

Firefox

URL for webapp

app and develop

Application version

Element version: 482b81b-react-6e31f691189e-js-d421e7f82904 Crypto version: Rust SDK 0.7.0 (61b175b), Vodozemac 0.5.1 and whatever the latest release is

Homeserver

lant.uk

Will you send logs?

No

@dosubot dosubot bot added A-E2EE A-E2EE-Key-Backup S-Minor Impairs non-critical functionality or suitable workarounds exist labels May 14, 2024
@MidhunSureshR MidhunSureshR added the O-Occasional Affects or can be seen by some users regularly or most users rarely label May 20, 2024
@richvdh
Copy link
Member

richvdh commented Jun 11, 2024

Yeeees.

The first prompt ("Use your security key to continue", with the stupid "upload" button) is the AccessSecretStorageDialog. This is shown because RestoreKeyBackupDialog (correctly) determines that the backup key is in 4S, and therefore calls SecurityManager.accessSecretStorage to get hold of the 4S key. This completes successfully, restores the backup, and enables upload of new keys to backup. However, it doesn't correctly feed this information back to RestoreKeyBackupDialog.recoverInfo.

The second prompt ("Enter security key") is fallback code meant to handle the case when the backup key is not stored in secret storage, and instead is prompting you to enter your backup key directly. Since you don't know the backup key (it is not the same as the 4S key), it's impossible to enter this key and you are shown an error. (Presumably if you did somehow know the backup key, we would attempt to restore it again.)

This is not a new issue; it also happens with legacy crypto.

@richvdh richvdh changed the title I'm prompted to type in my security key after submitting my security key "Connect key backup" prompts me to enter my security key after submitting my security key Jun 11, 2024
@richvdh
Copy link
Member

richvdh commented Jun 11, 2024

Ideally I think this whole fallback flow would be removed, but that's not actually the problem here: all we need is to make sure that recoverInfo is correctly populated when the recovery completes. And, for the love of God, some tests for this flow.

@richvdh
Copy link
Member

richvdh commented Jun 11, 2024

This is a duplicate of #26247, but this is a better description of the issue, so closing that one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-E2EE A-E2EE-Key-Backup O-Occasional Affects or can be seen by some users regularly or most users rarely S-Minor Impairs non-critical functionality or suitable workarounds exist T-Defect
Projects
None yet
Development

No branches or pull requests

3 participants