1679769 - An inline signed OpenPGP message having lines with leading whitespace fails to verify

Status: RESOLVED FIXED

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

Description

[Kai Engert (:KaiE:)]

Receive an inline signed message, starting with BEGIN PGP SIGNED MESSAGE and (not PGP/MIME).

If the plain text message contains lines that begin with leading whitespace, the signature verification fails.

Comment 1

[Kai Engert (:KaiE:)]

The failure is caused by the following line

msgText = EnigmailMsgRead.trimAllLines(msgText);

found in messageParse(), in file enigmailMessengerOverlay.js

Patrick, do you remember the intention of this trim?

It seems incorrect when processing an inline message.

Should this line be removed - or should trim be limited to certain scenarios?

Comment 2

[Patrick Brunschwig]

I believe that this is only relevant for the conversion of certain HTML mails to plain text.

Comment 3

[Kai Engert (:KaiE:)]

Attachment: Bug 1679769 - Limit trimming of OpenPGP messages to the separator line. r=PatrickBrunschwig

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/cd8a00e0e3b1
Limit trimming of OpenPGP messages to the separator line. r=PatrickBrunschwig DONTBUILD

Comment 5

[Kai Engert (:KaiE:)]

I realize that the commit message no longer reflected reality.

We left-trim all lines for encrypted messages, and don't trim anything for signed messages.

Comment 6

[Kai Engert (:KaiE:)]

Comment on attachment 9192000 [details]
Bug 1679769 - Limit trimming of OpenPGP messages to the separator line. r=PatrickBrunschwig

[Approval Request Comment]
Regression caused by (bug #): no
User impact if declined: we show false security status
Testing completed (on c-c, etc.):
Risk to taking this patch (and alternatives if risky): medium, potentially some other messages will get incorrect security status, but no scenario is known yet

Comment 7

[Kai Engert (:KaiE:)]

Hmm, there was no merge yet. So apparently this will automatically be part of the next beta, without beta uplift?

Comment 8

[Kai Engert (:KaiE:)]

Attachment: Bug 1679769 - Add test for signed inline message with leading whitespace. r=mkmelin

Comment 9

[Kai Engert (:KaiE:)]

(In reply to Kai Engert (:KaiE:) from comment #7)

Hmm, there was no merge yet. So apparently this will automatically be part of the next beta, without beta uplift?

I was wrong, the merge happened already, thanks to justdave for clarifying. I'll request beta uplift.

Comment 10

[Magnus Melin [:mkmelin]]

Would be preferable to land test and fix at the same time. I'll land the test for today's nightly.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/73831e108c0a
Add test for signed inline message with leading whitespace. r=mkmelin

Comment 12

[Wayne Mery (:wsmwk)]

Comment on attachment 9193206 [details]
Bug 1679769 - Add test for signed inline message with leading whitespace. r=mkmelin

[Triage Comment]
Approved for beta

Comment 13

[Wayne Mery (:wsmwk)]

Comment on attachment 9192000 [details]
Bug 1679769 - Limit trimming of OpenPGP messages to the separator line. r=PatrickBrunschwig

[Triage Comment]
Approved for beta

Comment 14

[Rob Lemley [:rjl]]

Thunderbird 85.0b3:
https://hg.mozilla.org/releases/comm-beta/rev/44d6f6ef9434
https://hg.mozilla.org/releases/comm-beta/rev/34635409b5e5

Comment 15

[Kai Engert (:KaiE:)]

Comment on attachment 9192000 [details]
Bug 1679769 - Limit trimming of OpenPGP messages to the separator line. r=PatrickBrunschwig

[Approval Request Comment]
Regression caused by (bug #): no
User impact if declined: we show false security status
Testing completed (on c-c, etc.):
Risk to taking this patch (and alternatives if risky): medium, potentially some other messages will get incorrect security status, but no scenario is known yet

Comment 16

[Wayne Mery (:wsmwk)]

Comment on attachment 9192000 [details]
Bug 1679769 - Limit trimming of OpenPGP messages to the separator line. r=PatrickBrunschwig

[Triage Comment]
Approved for esr78

Comment 17

[Dave Miller [:justdave]]

Thunderbird 78.7.0:
https://hg.mozilla.org/releases/comm-esr78/rev/c79fb1150adc
https://hg.mozilla.org/releases/comm-esr78/rev/73c10048ccbc