723086 - Invalid S/MIME Content-Description mail header - Non-encoded 8-bit data

Status: RESOLVED FIXED

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

Description

[Torsten Krah]

User Agent: Mozilla/5.0 (Ubuntu; X11; Linux x86_64; rv:9.0.1) Gecko/20100101 Firefox/9.0.1
Build ID: 20111228084940

Steps to reproduce:

Using Thunderbird 9.0.x or 10.0 i did write and text/plain email. Checked smime signature and smime encryption.
I am using the german port of thunderbird.


Actual results:

The email created does contain non-encoded 8bit data in its mail header (Content-Description):

Content-Description: S/MIME Verschlüsselte Nachricht

But the german "ü" should have beed encoded, e.g. using quoted printable.



Expected results:

The header should not show any non-encoded 8bit data, instead it should be encoded.

Comment 1

[M.J.G.]

I can CONFIRM this observation with TB 10.0.1 on Linux. But in fact, I don't think that content-description should be localised at all:

Some MUAs (mutt, horde) use content-description along with the other fields to decide whether the message is signed only or encrypted also. Decrypting a TB-encrypted message with localised content-description fails in mutt; reverting content-description back to the English version enables mutt to decrypt that message again.

So please: Do not localise content-description.

Comment 2

[Masahiko Imanaka [:marsf]]

Bug 507804 is the cause of this bug.

Workaround is to change these strings back to English:
(/mail/locales/en-US/chrome/messenger/am-smime.properties)
## Strings used by nsMsgComposeSecure
mime_multipartSignedBlurb=This is a cryptographically signed message in MIME format.
mime_smimeEncryptedContentDesc=S/MIME Encrypted Message
mime_smimeSignatureContentDesc=S/MIME Cryptographic Signature

Comment 3

[Ludovic Hirlimann [:Usul]]

(In reply to Masahiko Imanaka [:marsf] from comment #2)
> Bug 507804 is the cause of this bug.
> 
> Workaround is to change these strings back to English:
> (/mail/locales/en-US/chrome/messenger/am-smime.properties)
> ## Strings used by nsMsgComposeSecure
> mime_multipartSignedBlurb=This is a cryptographically signed message in MIME
> format.
> mime_smimeEncryptedContentDesc=S/MIME Encrypted Message
> mime_smimeSignatureContentDesc=S/MIME Cryptographic Signature

Is there a better way than this that could be created in the form of a patch ?

Comment 4

[Serge Gautherie (:sgautherie)]

This was thought of but apparently no tested until now.

(In reply to David :Bienvenu from bug 507804 comment #18)
> (In reply to comment #17)
> > David, what will happen if people actually translate these strings, and we end
> > up with non-ascii text in the ascii email headers? Couldn't that cause
> > problems?
> 
> We convert them to utf8 (two of these are message header values, the other
> goes into the message body, right?). I think the message send code should
> figure out the right encoding, unless the s/mime code inserts itself into
> the process too late. We should definitely try this and see that we're not
> generating invalid headers or message body.

Comment 6

[Mark Banner (:standard8)]

If I'm reading my standards correctly, then in http://tools.ietf.org/html/rfc5335 section 4.3, it states:

=====
To allow the use of UTF-8 in a Content-Description header field
   [RFC2045], the following syntax is used:

   description   = "Content-Description:" unstructured CRLF

   The <utext> syntax is extended above to allow UTF-8 in all
   <unstructured> header fields.
=====

So as of RFC 5335, then utf-8 strings are allowed in the Content-Description header. However, that is still officially an experimental rfc, so possibly as referenced in comment 4, we probably need this encoded in some form.

Comment 7

[M.J.G.]

There are really two issues here:

A) Should the header field be localized?
B) Is (and if: how is) UTF-8 used in the field?

If the answer to A is "no" then B is a non-issue.

And I'm still maintaining that the answer to A should be "no", as argued in comment #1.

Comment 8

[Joshua Cranmer [:jcranmer]]

(In reply to Mark Banner (:standard8) from comment #6)
> If I'm reading my standards correctly, then in
> http://tools.ietf.org/html/rfc5335 section 4.3, it states:

RFC 5335 is experimental and obsoleted by RFC 6532 (which is proposed standard), but the latter was published only 8 months ago.

> So as of RFC 5335, then utf-8 strings are allowed in the Content-Description
> header. However, that is still officially an experimental rfc, so possibly
> as referenced in comment 4, we probably need this encoded in some form.

UTF-8 headers are valid, but, strictly-speaking, only if we can guarantee that our underlying transport handles them correctly. I don't recall how our entire MIME composition-and-send pipeline works, but I'm pretty sure we don't check for 8-bit MIME support before rewriting with RFC 2047, so we probably ought to unconditionally encode as necessary for now.

Comment 9

[Magnus Melin [:mkmelin]]

Attachment: proposed fix

So according to rfc 6532 utf-8 is allowed - but obviously there's still problems.

Making a string mime encoded doesn't seem anywhere easy to me. But then again, i don't see why we would need to have this localized. This is stuff only ever seen by people looking at the message source. So, i propose to just use the raw English strings. (We have better things to spend time on.)

Comment 11

[Magnus Melin [:mkmelin]]

Comment on attachment 794221 [details] [diff] [review]
proposed fix

Review of attachment 794221 [details] [diff] [review]:
-----------------------------------------------------------------

jcranmers new encoder does seem to make it way less painful.

Btw, should fieldnamelen include colon and space?
http://mxr.mozilla.org/comm-central/source/mailnews/mime/public/nsIMimeConverter.idl#39

Comment 12

[Magnus Melin [:mkmelin]]

Attachment: bug723086_s-mime-content_description_v2.patch

Encoding then...

Comment 13

[Magnus Melin [:mkmelin]]

Comment on attachment 794790 [details] [diff] [review]
bug723086_s-mime-content_description_v2.patch

Not this one...

Comment 14

[Magnus Melin [:mkmelin]]

Comment on attachment 794790 [details] [diff] [review]
bug723086_s-mime-content_description_v2.patch

Eh, so it's the correct uploaded patch after all.

Comment 15

[Joshua Cranmer [:jcranmer]]

Comment on attachment 794790 [details] [diff] [review]
bug723086_s-mime-content_description_v2.patch

Review of attachment 794790 [details] [diff] [review]:
-----------------------------------------------------------------

Sorry for the delay.

::: mailnews/extensions/smime/src/nsMsgComposeSecure.cpp
@@ +473,5 @@
> +     do_GetService(NS_MIME_CONVERTER_CONTRACTID, &rv);
> +  NS_ENSURE_SUCCESS(rv, rv);
> +  char *encodedContentDescription = nullptr;
> +  mimeConverter->EncodeMimePartIIStr_UTF8(enc_content_desc_utf8, false,
> +      NS_LITERAL_CSTRING("UTF-8").get(),

We call this "UTF-8" :-)

@@ +476,5 @@
> +  mimeConverter->EncodeMimePartIIStr_UTF8(enc_content_desc_utf8, false,
> +      NS_LITERAL_CSTRING("UTF-8").get(),
> +      sizeof("Content-Description: "),
> +      nsIMimeConverter::MIME_ENCODED_WORD_SIZE,
> +      &encodedContentDescription);

You never free encodedContent. Sticking this in an nsCString and using getter_Copies is preferable.

Comment 16

[Magnus Melin [:mkmelin]]

Attachment: bug723086_s-mime-content_description_v2.patch

Indeed ;)

Comment 17

[Joshua Cranmer [:jcranmer]]

Comment on attachment 804876 [details] [diff] [review]
bug723086_s-mime-content_description_v2.patch

Review of attachment 804876 [details] [diff] [review]:
-----------------------------------------------------------------

::: mailnews/extensions/smime/src/nsMsgComposeSecure.cpp
@@ +624,1 @@
>            

Clean up this trailing whitespace while you're near?

Comment 18

[Magnus Melin [:mkmelin]]

https://hg.mozilla.org/comm-central/rev/61512a3e1797 -> FIXED