1673241 - (CVE-2021-29957) RNP-01-008 WP3 Thunderbird: Partially unencrypted email insufficiently detected (Low)

Status: RESOLVED FIXED

(MailNews Core :: Security: OpenPGP, defect)

Description

[Wayne Mery (:wsmwk)]

It was found that a partially unencrypted email has been insufficiently detected as such by Thunderbird. This introduces the risk of users erroneously assuming the entire email was encrypted and therefore ascribing it with an incorrect security status.

*Proof-of-Concept .eml file:
refer to Attachment 9182160 [details]

It is recommended that the logic for detecting encrypted content receives improvements by design. If parts of the message were decrypted, the whole message should be checked for unencrypted parts. If any parts of the parts were not decrypted, the application should alert the user about partially unprotected information.

Comment 1

[Kai Engert (:KaiE:)]

Alessandro: This is the bug I mentioned in bug 1702582, FYI.

Comment 2

[Kai Engert (:KaiE:)]

Adding Patrick to CC.
I'm working on several changes to the handling of inline messages.

Comment 3

[Kai Engert (:KaiE:)]

Regarding the past EFAIL security issue, one of the remedies was to limit processing of encrypted messages to the top level of a MIME message.

Apparently this wasn't applied to messages that are a mix of MIME and inline PGP. In the provided sample, there are three MIME parts. One of them contains an inline PGP message, and our current code is willing to process it.

I think it is OK if we continue to allow processing of exactly one inline PGP message, even if it is contained in the second level of a MIME message. However, if we do, we should give it the "partial PGP" treatment. We should show the notification that it's partial. We shouldn't process it automatically, but require the user to click the button to process the partial content. And after the user clicks, we should hide all other MIME parts. (This is what we currently do for a single-part plain text message, which contains an PGP block.)

There is some existing code (which refers to a bug 983), which has this comment:

          // for safety reasons, we replace the complete visible message with
          // the decrypted or signed part (bug 983)

But that code doesn't work. For the given example, additional rendered content is kept.
Also, currently the code will always replace the first rendered MIME part with the decrypted/decoded contents - even if that isn't the PGP block.

We should change the code to walk the DOM and remove all unrelated DIV nodes.

Also, there is code that recursively processes the full MIME tree, and will decode any parts at any nesting level. We should remove that code for at least two reasons. (1) Our new code (post Enigmail) will use the result of decoding to update the top level message status and show status indicators, and we don't support displaying status for sub content. (2) Because of the risks described around the EFAIL security issues, we shoudn't automatically process sub content.

While testing, I discovered that for messages that we cannot process (cannot decrypt, e.g. because of a broken digest part), we will not show any OpenPGP failure status (because RNP doesn't give us an helpful error code). I suggest two changes here. (3) If we don't get helpful failure information, let's check if the message we're trying to process is an encrypted message or a signed message (according to its BEGIN PGP header), and show the respective general OpenPGP error status. (4) If we're processing partial contents, let's always reduce the shown contents. In other words, remove the code around it, and keep the PGP message that we cannot process. I've suggested this also in bug 1702582.

Comment 4

[Kai Engert (:KaiE:)]

Attachment: Bug 1673241 - Improve handling of mixed MIME and inline OpenPGP. r=PatrickBrunschwig,mkmelin

Comment 5

[Kai Engert (:KaiE:)]

I have test messages for interactive testing, I'll work on automating in a separate patch.

Comment 6

[Ronald Tse]

Kai, RNP is happy to provide helpful response for a clean implementation on this case. If you remember, we do have a pending ticket that we need to discuss with your side on (1) the needs of TB from a compliance profile and (3) the interface between TB/RNP to describe the fit of a compliance profile. This in scope in the original project i.e. comes with a deadline from MOSS.

Comment 7

[Kai Engert (:KaiE:)]

Ronald, most of this bug is about PGP/MIME. We don't use RNP to process the MIME structure of an email, we're using code in Thunderbird (formerly Enigmail code) to do that. I'm not sure how comment 6 is related to this bug.

Comment 8

[Ronald Tse]

Got it, thanks for the clarification Kai!

Comment 9

[Kai Engert (:KaiE:)]

https://hg.mozilla.org/comm-central/rev/34b4c229c416a707188eb2ddf0698c7d1cd8fe11

Comment 10

[Kai Engert (:KaiE:)]

lint fix:
https://hg.mozilla.org/comm-central/rev/1536dedb52da7b63fd7509d909a108504dd58bf8

Comment 11

[Kai Engert (:KaiE:)]

Comment on attachment 9213156 [details]
Bug 1673241 - Improve handling of mixed MIME and inline OpenPGP. r=PatrickBrunschwig,mkmelin

Needs testing on beta, because we should consider to uplift to 78.11

[Approval Request Comment]
Regression caused by (bug #): no
User impact if declined: imprecise OpenPGP status reports for some message
Testing completed (on c-c, etc.): yes
Risk to taking this patch (and alternatives if risky): mild risk for regressions, but passes existing tests, and we add new tests

Comment 12

[Kai Engert (:KaiE:)]

The fix assumes that the fixes from bug 1701908, bug 1702582, bug 1701924 are present, so adding dependencies.

Comment 13

[Kai Engert (:KaiE:)]

Comment on attachment 9213156 [details]
Bug 1673241 - Improve handling of mixed MIME and inline OpenPGP. r=PatrickBrunschwig,mkmelin

See above beta approval request for details.

Requesting esr78 approval, however, should wait for at least 2 weeks beta testing.

Comment 14

[Wayne Mery (:wsmwk)]

Comment on attachment 9213156 [details]
Bug 1673241 - Improve handling of mixed MIME and inline OpenPGP. r=PatrickBrunschwig,mkmelin

[Triage Comment]
Approved for beta

Comment 15

[Kai Engert (:KaiE:)]

https://hg.mozilla.org/releases/comm-beta/rev/beceab875bd723113c4bd0f3315fac584fd5062f
89.0b3

Comment 16

[Wayne Mery (:wsmwk)]

Comment on attachment 9213156 [details]
Bug 1673241 - Improve handling of mixed MIME and inline OpenPGP. r=PatrickBrunschwig,mkmelin

[Triage Comment]
Approved for esr78

Comment 17

[Kai Engert (:KaiE:)]

https://hg.mozilla.org/releases/comm-esr78/rev/d2f01d2934ed8aa63dafa946a90008f68c6daf0a
78.10.2

Comment 18

[Wayne Mery (:wsmwk)]

Kai, if you plan to do a CVE for this issue, please coordinate with [e-mail] - we intend to release 78.10.2 on Monday or Tuesday.

Comment 19

[Kai Engert (:KaiE:)]

cve requested