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

Gracefully handle lack of submission GPG private key #140

Closed
emkll opened this issue Nov 9, 2018 · 10 comments · Fixed by #1059
Closed

Gracefully handle lack of submission GPG private key #140

emkll opened this issue Nov 9, 2018 · 10 comments · Fixed by #1059
Assignees
Labels
bug Something isn't working
Milestone

Comments

@emkll
Copy link
Contributor

emkll commented Nov 9, 2018

When a file on the SecureDrop server is encrypted to a GPG key that is not in sd-gpg, the client will be stuck in a loop attempting decryption of this file. The logs contain many instances of the following error:

2018-11-09 15:21:43,045 - securedrop_client.crypto:50(decrypt_submission_or_reply) ERROR: GPG error: gpg: encrypted with RSA key, ID <KEY_ID>
gpg: decryption failed: No secret key

Once I deleted those messages from the server, I no longer experienced the qfile error described in freedomofpress/securedrop-workstation#194 (review) . Those errors might be related.

@redshiftzero redshiftzero added this to the 0.1.0beta milestone Nov 9, 2018
@redshiftzero
Copy link
Contributor

@emkll I believe you hit this error again today in another scenario?

@heartsucker
Copy link
Contributor

What is the proposed method for dealing with this? Could we look at the output for that exact failure string and cache a list of "undecryptable" message per login/session? It's possible that an admin could re-add an old key to the keyring, so we wouldn't want to persist this flag in the db.

@eloquence eloquence added the bug Something isn't working label Jan 16, 2019
@eloquence eloquence modified the milestones: 0.2.xbeta, Post-Beta Mar 19, 2020
@redshiftzero redshiftzero added this to Near Term - SD Workstation in SecureDrop Team Board Mar 24, 2020
@redshiftzero redshiftzero moved this from Near Term - SD Workstation to QA Period - 3/17-3/31 (Kanban Mode) in SecureDrop Team Board Mar 24, 2020
@redshiftzero
Copy link
Contributor

Two thoughts:

  1. If the client does not have the private key to decrypt the file, we keep the file in its encrypted state locally (such that the user does not need to re-download if they fix the key issue), and expose the error in the UI. We no longer enqueue download jobs for that item. We allow the user to attempt the decryption again via the UI once they've resolved the key issue (to @heartsucker's point above).
  2. We could allow multiple private keys for a key rotation scenario. This is a multi-repo change (key fingerprint is on-disk in sd-app in a config file provisioned via salt). In this scenario the client would try to decrypt each file using the private keys it has available.

1 should be done pretty soon I think because the behavior when you don't have the key is pretty bad: repeated spammy gpg access notifications as well as repeated attempts to download messages/replies encrypted to the wrong key.

@eloquence eloquence moved this from Sprint #48 - 4/2-4/15 to Nominated for next sprint in SecureDrop Team Board Apr 2, 2020
@eloquence eloquence moved this from Nominated for next sprint to Sprint #48 - 4/2-4/15 in SecureDrop Team Board Apr 2, 2020
@rmol rmol self-assigned this Apr 8, 2020
@eloquence
Copy link
Member

This issue decription is a bit outdated. It would be good to capture:

  • What's the current behavior in Qubes when this happens (e.g., what notifications are shown?)
  • What happens when you attempt to download a file w/o having access to the key?

Regarding messages/replies, my understanding is that they stay in the "Message not downloaded yet" placeholder pattern state.

@rmol
Copy link
Contributor

rmol commented Apr 9, 2020

Moving /home/user/.gnupg aside in sd-gpg, so the submission key is unavailable, I see:

  • Synchronization causes lots of Keyring access from domain: sd-app tray notifications.
  • Message placeholder text never updates.
  • Trying to download a file results in an error message.

140-download-failed

@ninavizz
Copy link
Member

ninavizz commented Apr 10, 2020

Ok! Big thanks to Erik for helping wrap my brain around all this, and then to being open to some new ideas as this whole design system evolves...

Immediate Recommendation

  • Must: Continue to use global/top-bar error message to communicate what happened.
    • Show <h>Cannot decrypt file.</h> Submission key used for this file is missing. as the message text.
  • Must: Show Cannot decrypt messages as italicized preview text in Source List, if all messages are tied to missing key
  • Must: Missing key message bubbles
    • Show Cannot decrypt message italicized, in each
    • Show bottom bar as #BCBFCD grey, withbubble background as 60% white
  • Should: Disable download button. Visually fade the Download button and the filesize portion of the file-widget, so that it shows as disabled w/o the Encrypted file on server and line becoming illegible.
    Stage 1: Some content, no key

Iterate Towards

  • If possible, disable files if Submission Key to decrypt upon download can be sniffed for.
    • Preferable to not require the user to initiate a dead-end action, if possible.
  • More typographic styling to call-out uniqueness to impacted Sources
  • If any files or messages are decryptable, they'll still preview in the preview area.
  • Source-list status icon to surface visibility w/o alarm.
    • Whereas Reply Fail error state in the Source List is intentionally crafted to call attention to itself for any user to immediately act upon.
    • Only an Admin can act upon this, and since it's not a security breech it should not be treated as "needs immediate attention."
    • "FYI" vs "OMGWTF!"
  • Contextual banner affixed to Conversation Pane for Source, and to remain in place until cache refresh detects key.
    • Would remain until multi-key support implemented, then wd go away only when correct key wd be detected.
    • Source sd show as "Unread" upon newly decrypted messages, but with Client's read/unread pattern, unreads shd not float to the top. List ordered by chronology.
  • If team likes semiotic of crossed-out key icon, a cleaner icon will be developed to replace blobby icon used in mox.
    Stage 2: Some content, no key

@eloquence
Copy link
Member

Awesome work Nina! Yup, I agree the "immediate recommendations" make sense to resolve this issue, and then we can open up separate issues for follow-up iterations.

It may make sense for the client to keep track of data it can't decrypt (messages, files - after the user attempted it -, replies) so that

  • we can reliably suppress decryption attempts only for those items
  • so that we can re-trigger decryption attempts once a missing key is configured (as soon as we support that)

@eloquence
Copy link
Member

eloquence commented Apr 13, 2020

Must: Show Cannot decrypt messages as italicized preview text in Source List, if all messages are tied to missing key

We agreed to de-scope this from the initial implementation, can be tracked as a follow-up issue.

@eloquence
Copy link
Member

Synchronization causes lots of Keyring access from domain: sd-app tray notifications.

Since it was not explicitly stated in Nina's list, just clarifying that suppressing these notifications is also one of the must-have goals for resolving this issue.

@redshiftzero
Copy link
Contributor

@rmol and I discussed how to go about doing this today, we decided on:

  1. add cache locally to suppress the notifications in the near-term
  2. implement Store reference to GPG key used to encrypt submission and replies in database securedrop#5195
  3. once the API change in step 2 is done, use that client side (this will mean storing the fingerprint in the database and updating on sync as we do with other file/reply/message attributes). at that time we can remove the cache. this way the user won’t need to spend potentially a long time to download a file to see if it’s decryptable

@eloquence eloquence moved this from Sprint #48 - 4/2-4/15 to In Development in SecureDrop Team Board Apr 15, 2020
@rmol rmol moved this from In Development to Ready for Review in SecureDrop Team Board Apr 23, 2020
SecureDrop Team Board automation moved this from Ready for Review to Done Apr 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

6 participants