Skip to content
This repository has been archived by the owner on Oct 15, 2024. It is now read-only.

[BUG] Passphrase prompt should not be shown for keys that have none #2836

Closed
durka opened this issue Dec 25, 2023 · 5 comments · Fixed by #3069
Closed

[BUG] Passphrase prompt should not be shown for keys that have none #2836

durka opened this issue Dec 25, 2023 · 5 comments · Fixed by #3069
Assignees
Labels
A-PGPainless Area: PGPainless-backed PGP C-bug Category: This is a bug P-high Priority: high, must be resolved before next major release
Milestone

Comments

@durka
Copy link
Contributor

durka commented Dec 25, 2023

Describe the bug

When I go to decrypt a password, Password Store asks me for a password. It's not clear, but I'm assuming it wants the passphrase for my PGP key, but there isn't one.

I can enter anything, or nothing, and click OK and it still decrypts the password.

Steps to reproduce

Steps to reproduce the behavior:

  1. Import a key with no passphrase
  2. Select a password from the store
  3. Enter something in the password prompt, or don't
  4. Click OK

Expected behavior

Just decrypt the password.

Screenshots

No response

Device information

  • Device: Pixel 7
  • OS: Android 14
  • App version: 2.0.0-SNAPSHOT (broken biometric authentication workaround)

Additional context

No response

@durka durka added C-bug Category: This is a bug S-awaiting-triage Status: New issues that have not been assessed yet labels Dec 25, 2023
@github-project-automation github-project-automation bot moved this to 🆕 Unexplored in APS v2 Backlog Dec 25, 2023
@msfjarvis
Copy link
Member

The underlying PGP library accepts the passphrase as part of a separate SecretKeyProtector object which I assume just completely gets ignored when the key doesn't have any passphrase. I'll have to check if there's a way for us to detect the same from outside without having to resolve to attempt decryption twice.

@msfjarvis msfjarvis changed the title [BUG] Asks for passphrase when apparently not necessary, accepts any entry [BUG] Passphrase prompt should not be shown for keys that have none Dec 25, 2023
@msfjarvis msfjarvis added P-high Priority: high, must be resolved before next major release S-design Status: There's a problem here, but no obvious solution; or the solution raises other questions A-PGPainless Area: PGPainless-backed PGP and removed S-awaiting-triage Status: New issues that have not been assessed yet labels Dec 25, 2023
@msfjarvis msfjarvis moved this from 🆕 Unexplored to 📋 Being triaged in APS v2 Backlog Dec 25, 2023
@msfjarvis msfjarvis added this to the v2.0.0 milestone Dec 25, 2023
@msfjarvis msfjarvis self-assigned this Dec 25, 2023
@msfjarvis msfjarvis moved this from 📋 Being triaged to 🏗 In progress in APS v2 Backlog Dec 25, 2023
@msfjarvis msfjarvis added S-in-progress Status: Implementation is underway and removed S-design Status: There's a problem here, but no obvious solution; or the solution raises other questions labels Dec 25, 2023
@msfjarvis
Copy link
Member

I've pushed a change that inspects the encrypted message and skips asking for a passphrase if it's not needed, please test out the new snapshot build once it's available and let me know how it goes.

@durka
Copy link
Contributor Author

durka commented Dec 25, 2023

It's not quite working. It avoids asking me for a passphrase and reaches the password display screen, but the password is never displayed.

Note: if I enable the "passphrase cache", then it successfully decrypts and displays the password. This is a perfectly fine state of affairs for me personally, because it means even with the biometric auth disabled due to #2802, then there is still a biometric layer before the password comes up.

Attached is a logcat from trying to decrypt a password with the passphrase cache disabled:
aps-logcat.txt

@msfjarvis
Copy link
Member

Thanks for checking it out, I'll give it another go sometime today.

@msfjarvis msfjarvis added S-in-progress Status: Implementation is underway and removed S-in-progress Status: Implementation is underway labels May 23, 2024
@msfjarvis msfjarvis moved this from 🏗 In progress to 📋 Being triaged in APS v2 Backlog May 25, 2024
@msfjarvis msfjarvis removed the S-in-progress Status: Implementation is underway label May 25, 2024
@msfjarvis msfjarvis moved this from 📋 Being triaged to 👀 In review in APS v2 Backlog May 27, 2024
@msfjarvis
Copy link
Member

Finally managed to find enough time and motivation to take another look at this, once #3069 is merged this should be resolved.

@github-project-automation github-project-automation bot moved this from 👀 In review to ✅ Done in APS v2 Backlog May 27, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A-PGPainless Area: PGPainless-backed PGP C-bug Category: This is a bug P-high Priority: high, must be resolved before next major release
Projects
Archived in project
2 participants