1805558 - Thunderbird 102 creates S/MIME messages where there is no newline between MIME boundaries. Some servers add newlines leading to invalid signatures.

Status: RESOLVED FIXED

(MailNews Core :: Security: S/MIME, defect)

Description

[b7]

Attachment: Test.eml

+++ This bug was initially created as a clone of Bug #1731529 +++

Thunderbird 102 creates S/MIME messages where there is no newline between MIME boundaries. Some servers add newlines leading to invalid signatures.

Bug 1731529 wasn't completely fixed. In the changeset we read:
https://hg.mozilla.org/comm-central/rev/88fa9d0345ee#l1.18

+    // Ensure there is exactly one blank line after a part and before
+    // the boundary, and exactly one blank line between boundary lines.
+    // This works around bugs in other software that erroneously remove
+    // additional blank lines, thereby causing verification failures of
+    // OpenPGP or S/MIME signatures. For example see bug 1731529.

This is not the case as the attached message created with TB shows. There are two MIME boundaries at lines 45 and 46 which are not separated by a newline. Some servers add a newline hear and break the signature.

In our testing this happens with PDF attachments, not plaintext attachments and for mails containing plaintext and HTML part.

This is related to:
https://thunderbird.topicbox.com/groups/e2ee/Te16a3b235e0f12c9-M7099e2c2556619db56d3dd2d

Comment 1

[b7]

Correction: Problem also occurs for plaintext attachment, mail source becomes:

  </body>
</html>

--------------5ZIBa7ZTc2Tip0zuOGkxzxe4--
--------------cFwJl4TpWQsNLQmOdpOYHi1l
Content-Type: text/plain; charset=UTF-8; name="text.txt"
Content-Disposition: attachment; filename="text.txt"
Content-Transfer-Encoding: base64

Comment 2

[b7]

https://github.com/Betterbird/thunderbird-patches/blob/main/102/bugs/1805558-fix-SMIME-signature-failures.patch
Also fixes confusion in writing (binary) strings and code like

foo(`${bodyString}`);

Comment 3

[Magnus Melin [:mkmelin]]

Attachment: Bug 1805558 - Terminate ending MIME boundary with additional \r\n (and fix confusion in string writing). r=kaie

Based on https://github.com/Betterbird/thunderbird-patches/blob/main/115/bugs/1805558-fix-SMIME-signature-failures.patch

Comment 4

[betterbird.project+13]

Thanks for taking this on board. Looking at the patch again, this this._writeString(``\r\n--${curPart.separator}--\r\n``); (line 581) should also be changed to use _writeBinaryString() like a few lines above (line 578).

Comment 5

[betterbird.project+13]

Actually, you deviated from the original patch, factoring out writing the \r\n separately. That's of course possible but not really efficient, that's why we had:

+      if (depth > 1) {
+        // If more separators follow, make sure there is a
+        // blank line after this one.
+        this._writeBinaryString(`\r\n--${curPart.separator}--\r\n\r\n`);
+      } else {
+        this._writeBinaryString(`\r\n--${curPart.separator}--\r\n`);
+      }

Comment 6

[Kai Engert (:KaiE:)]

I wonder, is there a specification that clearly defines what kind of newlines should be present between MIME boundaries?
Does this change bring us closer to a specification - or is this just an attempt to make some specific MTA happy - while other MTAs might expect and change it differently?

Comment 7

[Magnus Melin [:mkmelin]]

Yes, RFC 2046:
NOTE: The CRLF preceding the boundary delimiter line is conceptually
attached to the boundary so that it is possible to have a part that
does not end with a CRLF (line break). Body parts that must be
considered to end with line breaks, therefore, must have two CRLFs
preceding the boundary delimiter line, the first of which is part of
the preceding body part, and the second of which is part of the
encapsulation boundary.

But also aligns with what was intended fin the earlier fix - https://searchfox.org/comm-central/rev/ff862b7296f23639844ce46ba82a5dcd437120f3/mailnews/compose/src/MimeMessage.jsm#584

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/937e264e5ad9
Terminate ending MIME boundary with additional \r\n (and fix confusion in string writing). r=kaie

Comment 9

[Wayne Mery (:wsmwk)]

Good for next week's 115.6.1? Or do you prefer it ride a full beta period?

Comment 10

[Magnus Melin [:mkmelin]]

It could go into 115 now yes

Comment 11

[Wayne Mery (:wsmwk)]

Comment on attachment 9364563 [details]
Bug 1805558 - Terminate ending MIME boundary with additional \r\n (and fix confusion in string writing). r=kaie

[Triage Comment]
Approved for esr115

Comment 12

[Rob Lemley [:rjl]]

Thunderbird 115.6.1:
https://hg.mozilla.org/releases/comm-esr115/rev/4a010f513ab9