Open Bug 1837247 Opened 1 year ago Updated 4 months ago

TB115 can no longer decrypt a forwarded encrypted OpenPGP message

Categories

(MailNews Core :: Security: OpenPGP, defect, P2)

Thunderbird 115

Tracking

(thunderbird_esr102 unaffected, thunderbird_esr115+ affected, thunderbird115 wontfix, thunderbird116 affected, thunderbird119 affected)

Tracking Status
thunderbird_esr102 --- unaffected
thunderbird_esr115 + affected
thunderbird115 --- wontfix
thunderbird116 --- affected
thunderbird119 --- affected

People

(Reporter: KaiE, Unassigned)

References

(Blocks 1 open bug, Regression)

Details

(Keywords: regression, Whiteboard: [supernova3p][regression:tb111])

Attachments

(1 file)

Use a profile that has the Alice and Bob example secret OpenPGP keys imported.

Open test file mail/test/browser/openpgp/data/eml/fwd-unsigned-encrypted.eml

With 102, it was possible to open the attached message, and its decrypted contents were shown.

This is broken in 115.

I've tested nightly build 2023-01-12, which contains the commit from bug 1746579, which touched the handling of nested encrypted messages. It still works with that version, bug 1746579 isn't the cause of the regression.

Nightly 2023-01-17 still worked.
Nightly 2023-01-19 was broken.

I blame the Ash branch merging from Jan 18

Looking at the source of the attachment, I see the data already got decrypted, but the content-type header and boundaries are no longer appropriately set.

Apparently isn't a decryption issue, but rather a MIME level breakage.

It seems the following happens:

We process a MIME part with content-type: multipart/encrypted; protocol="application/pgp-encrypted";

The decrypt the inner data, that works. We correctly return the result to the caller.

At this point, the caller should replace the whole part with the result. However, apparently the caller keeps that content-type wrapper. It simply replaces the inner data with the decrypted data.

In the next step, we apparently attempt to process that result part again. Because it still has the same outer content-type, we again run into the same PGP decryption handler - which fails, because there's no encrypted data (rather there's cleartext, which is unexpected for that content-type).

I'm still trying to find where that caller is, and where the mistake (keep original MIME type) is made.

I'm still searching for the responsible code.

I see that the decrypt code works correctly: It only returns the inner data.

I see the code in mail/modules/AttachmentInfo.sys.mjs
loads the following URL:
mailbox:///home/user/.thunderbird/..profile../Mail/Local%20Folders/fwd-attach-encrypt?number=1&part=1.2&filename=attached-message.eml

(The contents of that mailbox is one message, the message we have in tree with name: fwd-unsigned-encrypted.eml)

The data returned from that URL contains the false extra content-type wrapper.

I need to find the code that handles that mailbox:// URL, takes the decryption result, and returns the raw bytes. That code must be responsible for adding the wrong outer MIME content-type.

I already spent 12 hours searching for the place that adds/keeps that extra header, but I couldn't find it yet.

I'd greatly appreciate any hints, do we have someone who understands the data flow well enough to help?

Maybe not the answer to your question, but noting it down here as I noticed it recently:
Atm we load each url twice (yes we should rework this somehow). First one normal load and then OpenPGP notices we have a message to check, and loads the same url once again. Triggered from https://searchfox.org/comm-central/rev/fe988e6b57d63dde2a550676304716769d2bfacf/mail/extensions/openpgp/content/ui/enigmailMsgHdrViewOverlay.js#573

With the keys available, I'm not sure what's not working?
If I load the fwd-unsigned-encrypted.eml message I get a "This message includes encrypted parts" + "Decrypt and Show" button. If i click the button, the attached message from Bob opens and I can see the decrypted content.
If I just click the attachment, then it doesn't decrypt. Obviously would be better if it did show when I open it yes.

Summary: TB 115 can no longer decrypt a forwarded encrypted OpenPGP message → TB115 can no longer decrypt a forwarded encrypted OpenPGP message
Whiteboard: [supernova3p][regression:tb111]
Version: unspecified → Thunderbird 115
Priority: -- → P2
Priority: P2 → P1

(In reply to Magnus Melin [:mkmelin] from comment #9)

If I load the fwd-unsigned-encrypted.eml message I get a "This message includes encrypted parts" + "Decrypt and Show" button. If i click the button, the attached message from Bob opens and I can see the decrypted content.

Yes, that was recently added, new functionality, which allows to decrypt any nested parts at any nesting level.

If I just click the attachment, then it doesn't decrypt. Obviously would be better if it did show when I open it yes.

Right, that's the regression.

Not a blocker for 115.0, per matrix chat

Flags: needinfo?(kaie)
Flags: needinfo?(kaie)
Priority: P1 → P2

Kai's observations from comment #5 and comment #6 are correct: The (inner) decrypted data is still embedded into a defective multipart/encrypted wrapper when written out to a file, as one can also see in the subPart.eml file in the temp directory:

Content-Type: multipart/encrypted;
 protocol="application/pgp-encrypted";
 boundary="------------pDRPNQMqISsWnmd0BK2t6P1L"

Overall the file contains a broken MIME tree since this boundary is actually never used.

The way opening attachments is implemented now, that is "save to file, open file", the entire bug has become an exact duplicate of bug 1689326 where the incorrect MIME structure was already noted in the bug description. Bug 1689326 comment #2 (quote): 'best to just save the "raw" attachment' may also be the way to go here, since then, the message would (hopefully) be decrypted when being opened from file.

For a full history, read below, although that knowledge won't produce a fix:

Before the Ash/Supernova merge, attachments were opened like this:
https://hg.mozilla.org/comm-central/file/43752e8d8e55fd237c5bc78f1f73f6dd167ce4de/mail/base/content/msgHdrView.js#l1514
via messenger.openAttachment() (also in version 102).
After the Ash merge on 19-01-2023, attachments were opened like this:
https://hg.mozilla.org/comm-central/file/5abcc1359eb7d4122f958a823e29b43de5f60b2e/mail/base/content/msgHdrView.js#l1514
via top.messenger.openAttachment(). Surprisingly that already didn't work for encrypted message attachments since the regression range from comment #2 is correct.

This was later changed to the currently implemented method "save to file, open file" in March 2023:
https://hg.mozilla.org/comm-central/rev/1b437e7aa80e83ff321d020dc651498f8db157e9#l6.60
As bug 1689326 from 2021 shows, storing half-decrypted originally encrypted attachments to a file never worked.

The openAttachment() methods were later removed later in March 2023:
https://hg.mozilla.org/comm-central/rev/907ab135e85912c5c642db8879a42ad03963d3f5#l4.54

Finally the attachment handling code moved to AttachmentInfo.sys.mjs in April 2023.

Duplicate of this bug: 1858901

This bug also happens if your mail server injects an "external mail" warning into the mail body. To do this, the original encrypted mail is put into an attachment.

This attachment can't currently be opened with TB 115.

This is a pretty big issue for us.

When loading the attachment in the new window, I see we're going through the MIME decryption twice. First time, we decrypt, and second time, we try to decrypt the already decrypted data, which fails.

We must prevent the decryption on the first time, which I think is supposed to deliver only the raw message (keeping the encryption wrapper).

When loading mail URIs, the flags influence whether a message is processed (rendered for loading), or whether it's loaded in a raw format.

When opening the attached message, we arrive in nsStreamConverter::DetermineOutputFormat,
which sees the "part=" parameter, and concludes we must use nsMimeOutput::nsMimeMessageRaw.

In both scenarios, we run through the code in mimeDecrypt.jsm.

I cannot see any checks in any PGP code for the raw output format.

I think we need to prevent that we arrive in the mimeDecrypt code.
I think if we see the request for the raw output format, we must prevent routing to any PGP handlers completely.

Where could this be done?

Maybe it could be done in nsPgpMimeProxy.cpp - which instead of routing into a processing handler, could just return the raw data.

Maybe AttachmentInfo.sys.mjs, when calling NetUtil.asyncFetch, we should add a parameter to the URI, such as "&outputformat=raw", and add code to nsPgpMimeProxy.cpp to detect that parameter.

Regarding the output of the header, including the output of Content-Type: multipart/encrypted, which we earlier (above) described as wrong:

I think that output is actually correct, because we're supposed to produce the full (raw) message.

That initial header section is written by the MIME code (I'll attach a stack that documents where).
By the time we arrive in MimePgpe_init, it was already written to the stream.

Attached file Stack for comment 18

Is there any progress on this? Still stuck on 102 ESR because of this issue.

You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: