Closed Bug 1746579 Opened 1 year ago Closed 18 days ago

Allow decrypting nested OpenPGP parts on demand

Categories

(MailNews Core :: Security: OpenPGP, enhancement, P3)

enhancement

Tracking

(Not tracked)

RESOLVED FIXED
110 Branch

People

(Reporter: KaiE, Assigned: KaiE)

References

Details

(Whiteboard: tb-crypto-needs-design tb-crypto-feature-incomplete)

Attachments

(3 files)

For safety reasons, we don't automatically decrypt OpenPGP messages, if the message is contained in a nested structure. Because of EFAIL, it has been shown that it can be cleverly exploited to steal the encrypted contents by an attacker.

However, we should probably offer a mechanism to display those parts anyway, on demand.
It could potentially be implemented like this:

If TB detects encrypted sub parts, it could show information in the UI. It could show a list of encrypted parts, e.g. if there are multiple encrypted parts.

It could allow the user to click an encrypted part, and select "decrypt". Then Thunderbird could display only the decrypted contents of that part, or allow saving it.

Bug 1730383 has an example where this could be useful.

So, nothing changed within 7 month. The problem is still there. It seems nobody worked on it.
Will the problem be solved on day?

Severity: -- → S3
Priority: -- → P3
Whiteboard: ketb-needs-design ketb-feature-incomplete
Whiteboard: ketb-needs-design ketb-feature-incomplete → tb-crypto-needs-design tb-crypto-feature-incomplete
Blocks: 1769797
Blocks: 1594253

It would be nice to have a suggestion for a user interface for this functionality.

For messages like this, we'd need:

  • a notification somewhere that this message contains encrypted sub-parts which currently aren't shown, possibly explaining that this is hidden by default to protect the user, to ensure those parts will not be accidentally included when replying/forwarding.

(The attack scenario that we intend to prevent by this behavior: The original message could have be crafted by an attacker, with content the attacker cannot decrypt, and with the attacker hoping that the user will decrypt and reply/forward, thereby leaking the decrypted data to the attacker.)

  • a list of nested encrypted parts (could be multiple) - maybe similar to how we should the list of attachments. Optionally, integrate into the attachment list, name it accordingly, and probably with different actions.

  • an action mechanism to allow the user to pick/select an encrypted part, and request to decrypt/view it. This should be limited to one part at a time. A new message window could pop up to show that decrypted data.

Flags: needinfo?(alessandro)

I have an initial experimental patch.

For the initial demonstration of this functionality, the patch uses a UI that isn't automatic.
It adds a message command in the view menu: "view / encrypted parts".
Currently, the user has to "know" that there is a hidden encrypted part inside the message.
If the user selects the command, we'll attempt to find encrypted parts in the selected message, and open a new message window for each of them (usually probably just one).
The message window then shows the decrypted message part.

(In theory, this could be made universal, and potentially could make the patch from bug 1594253 unnecessary.)

Assignee: nobody → kaie
Attachment #9309488 - Attachment description: WIP: Bug 1746579 - Add a menu command to view nested encrypted parts. → Bug 1746579 - Add a menu command to view nested encrypted parts. r=mkmelin
Status: NEW → ASSIGNED

I don't know about this.
I need to visually test it. Could you provide an email example I could use?

Just an adjacent heads up. We're gonna merge the ash repository into comm-central early in mid-January.
That repo renamed and simplified a lot of files that you're using in your patch. If this is not an urgent fix, it would be nice to wait until ash is merged otherwise it will be difficult to handle merging conflicts.

Flags: needinfo?(alessandro)

If you're adding a temporary menu item for testing something weird like this I don't think it should be visible to all users either, should only be enabled by a pref. Otherwise there's some risk this "temporary" thing eventually finds its way into a beta and people are asking about it and trying to use it.

Thanks for your feedback.

In the meantime, I have changed the user interaction. A menu command is no longer necessary.

The new strategy is:
If initial message loading detects that there are nested encrypted parts which we skip by default,
then a notification will be shown in the message window:

(i) This message includes additional encrypted parts. [decrypt and show]

I think that's a much better solution, it avoids the menu command, it only offers the command when available, and it makes it immediately discoverable.

When clicking the button, one or multiple message windows will be opened, one for each found nested encrypted part.

What do you think of this behavior?

Reminding myself to set a needinfo for aleca once he's back.
(Bugzilla disallows me to do that while he is away.)

Flags: needinfo?(kaie)

To test this functionality:

  • Save the attached file outer-smime-inner-pgp.eml locally.
  • open openpgp manager
  • menu file, import secret key from file
  • select the following file:
    mail/test/browser/openpgp/data/keys/alice@openpgp.example-0xf231550c4f47e38e-secret.asc
  • complete the import of the key
  • in the messenger window: menu file, open, saved message
  • select the file you saved, outer-smime-inner-pgp.eml
  • a message window will open, it shows an empty mesage body, but it shows the notification "message includes additional encrypted parts"
  • click the button "decrypt and show"
  • a new message window will open that displays the decrypted contents of the nested part
Attachment #9309488 - Attachment description: Bug 1746579 - Add a menu command to view nested encrypted parts. r=mkmelin → WIP: Bug 1746579 - Notify the user if a message has nested encrypted parts, and allow viewing them.

FYI, this patch makes it possible to view nested encrypted parts, but the message windows that open aren't fully complete windows, most of the regular message headers aren't shown.

That's why I think we should continue the work on bug 1594253, which is (apparently) a common scenario for nesting. I think that common scenario can and should be treated automatically, in the primary message window, and doing so will give the user a full experience with correct message headers etc. (for that specific scenario).

Comment on attachment 9310826 [details]
test message outer-smime-inner-pgp.eml
[shall be handled automatically by bug 1594253 in the future]

Note this kind of message will be automatically handled by bug 1594253.
Which means, once bug 1594253 lands, this specific message is no longer an example for this new UI.

I'll attach an additional test message.

Flags: needinfo?(kaie)

This is an additional test message.

Even with bug 1594253 fixed, this test message will continue to show the notification and button that I'm suggesting in this bug.

Attachment #9310826 - Attachment description: outer-smime-inner-pgp.eml → test message outer-smime-inner-pgp.eml [shall be handled automatically by bug 1594253]
Flags: needinfo?(kaie)
Attachment #9310826 - Attachment description: test message outer-smime-inner-pgp.eml [shall be handled automatically by bug 1594253] → test message outer-smime-inner-pgp.eml [shall be handled automatically by bug 1594253 in the future]
Flags: needinfo?(kaie)
No longer blocks: 1769797
Duplicate of this bug: 1769797
Flags: needinfo?(alessandro)
Attachment #9309488 - Attachment description: WIP: Bug 1746579 - Notify the user if a message has nested encrypted parts, and allow viewing them. → Bug 1746579 - Notify the user if a message has nested encrypted parts, and allow viewing them. r=aleca
Target Milestone: --- → 110 Branch

Pushed by kaie@kuix.de:
https://hg.mozilla.org/comm-central/rev/8bcd8ddcd1ef
Notify the user if a message has nested encrypted parts, and allow viewing them. r=aleca

Status: ASSIGNED → RESOLVED
Closed: 18 days ago
Resolution: --- → FIXED
Flags: needinfo?(alessandro)
You need to log in before you can comment on or make changes to this bug.