160029 - S/MIME failing with HTML Only Emails and MS Outlook

Status: VERIFIED FIXED

(MailNews Core :: Composition, defect)

Description

[Adam Smith]

From Bugzilla Helper:
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.1b) Gecko/20020729
BuildID:    2002072908

When sending a signed 'HTML only' email to an Outlook client, the receiving
client (outlook and OE) can not open the email.  When sending between Mozilla
clients, this works fine.  If I choose 'Plain Text' or 'HTML and Plain Text',
then the message is sent and is read correctly by Outlook.  My certificate was
issued by Thawte and appears to be a valid S/MIME cert.

I'm hesitant to file this as a bug.  I apologize if I'm misunderstaning S/MIME,
or if this is a Microsoft issue.

Reproducible: Always
Steps to Reproduce:
1. Compose HTML email.
2. Select 'Digitally sign this message' in Security.
2. Click send, when prompted for the format choose 'HTML only'.
3. Receive message on an Outlook client.


Actual Results:  Outlook express refuses to open the email, Outlook reports the
following error: "Your key set cannot be found by the underlying security system"

Expected Results:  Outlook to open the message as Mozilla does.

Comment 1

[Adam Smith]

I don't believe this is a security issue but a mail/news issue.  If I..

(1) visit a page such as the www.mozilla.org.
(2) select all the text on the page (ctrl-A), then paste that text into an email.
(3) Sign it
(4) send as either 'HTML Only' or 'Plain Text Only', then Outlook refuses to
display it.

If I perform the same steps, and send it as 'HTML and Plain Text' then Outlook
displays properly.  The Mozilla mail/news client seems to handle each incoming
message properly.  Either it's a problem with the composition of outgoing mail,
or more likely a problem with Outlook.

Comment 2

[Daniel Veditz [:dveditz]]

Agreed; over to mailnews. Raising priority back to "normal" -- there are a lot
of potential Outlook mail recipients.

Comment 3

[Daniel Veditz [:dveditz]]

argh, why isn't "S/MIME" in the mailnews product?

Comment 4

[scottputterman]

Charles, are you able to reproduce this?

Comment 5

[Charles Rosendahl]

Yes...I can reproduce it.

It seems to appear in Outlook based products when a page is composed of complex
html and sent as either html or as text, but not both.

Looking at the different mime types, it appears that outlook products have a
problem displaying signed messages that come in a multipart/related mime type,
unless we are composing the multipart/related messages incorrectly when signed.

I've attached a multipart/related message (plain text version).

Comment 6

[Charles Rosendahl]

Attachment: Plain text message that is unable to display in Outlook family

Comment 7

[scottputterman]

Stephane, can someone on your team take a look at this.  We might want to get
this into Mach V.

Comment 8

[Charles Rosendahl]

A little more info for my conclusion:

1. Simple html and simple text arrive as multipart/signed.

2. A message sent in both html and plain text formats arrives as
multipart/alternative.

3. The complex html ( aka anything with tables in it) sent as html *or* plain
text (but not both - See #2) arrives as multipart/related.

Simple html and simple text (due to the mime type) display fine in all mail
clients.  The problem only appears in Outlook in case #3.

Comment 9

[Stephane Saux]

to Kai for investigation.

Comment 10

[Kai Engert [:KaiE:]]

Without understanding yet everything, some first notes:

- I dont see logic within the security or the S/Mime logic that triggers usage
of the multipart/related type
- we seem to create an completely unnecessary wrapper around the signed contents
- our multipart/related content seems to be invalid. Reading
http://www.ietf.org/rfc/rfc2387.txt section 2 states a multipart/related mime
content type must specify its own type in addition (to give the user agent a
clue what kind of related content can be found in the message). However, we do
not include that parameter.

Jean-Francois, do you know which logic triggers the creation of that
multipart/related wrapper?

Comment 11

[Jean-Francois Ducarroz]

We generate multipart/related message when embedding an object into the message
body like an image. You need to use an html compose window in order to be able
to insert images (or any kind of object) into the body

Comment 12

[Kai Engert [:KaiE:]]

Note: All my test messages were signed.


Outlook does not like multipart/related messages where there is only one part.

We produce messages having unnecessarily type multipart/related with only one part.

Outlook does also not like messages where there are two parts, but both parts
are plain contents, i.e. I produced a test message with two text/html parts, and
Outlook showed its error message.

However, if one composes a HTML message with Mozilla, and uses Insert/Image to
embed an image, we create a multipart/related message with the first part being
HTML and the second a graphic. This message show correctly in Outlook.
I saw that the HTML content references the image, the second part of the
message. But even if I remove the reference code and the object ID from the
graphic, Outlook still displays the message (but only the text/html part).


Conclusion:

If we want compatibility, we need to avoid creating unnecessary
multipart/related wrappers, where there is only one part.

I guess this change would have to be done in the MIME code?

Comment 13

[Jean-Francois Ducarroz]

I need to understand in which situation we are generating a "blank"
multipart/related message. Would be nive if somebody can give me the exact step...

Comment 14

[Kai Engert [:KaiE:]]

I believe I found the cause.

Make sure you have HTML composing enabled.
Make sure you have NOT enabled to send mail in both html/text formats.

Compose a new message.

Open a browser window. Go to www.mozilla.org. Click on the page. Use Select all
to mark all content. Go to the open compose window. Paste into the body.

Note that the body shows a picture.

Add yourself as recipient. Send the message. No need to use S/Mime signing.

Receive the message. Note that the message does not contain the image it showed
during composition.


I believe the message sending code assumes that an image will be included and
that multipart/related is therefore needed, but somewhere attaching the image fails.

I traced a bit in nsMsgSend. The multipart/related mime gets set at
    // This is JUST multpart related!
    if ( (mMultipartRelatedAttachmentCount > 0) &&
         (m_attachment_count == mMultipartRelatedAttachmentCount) )
      status = toppart->SetType(MULTIPART_RELATED);
  
When I test that using the above steps, all three variables are "1".


When I do the same thing, but instead copy and paste the contents from
bugzilla.mozilla.org, all variables contain 2, because there are 2 images on the
page - but still the received message does not contain images.

Comment 15

[Kai Engert [:KaiE:]]

Bute note there may be additional cases. Charles says it also occurs with
plaintext messages. I was not yet able to reproduce with plaintext messages.

Comment 16

[Kai Engert [:KaiE:]]

Hmm. If you use the previous steps, but do not use signing, the produced message
will still be multipart/related, and will still have only a single part.

Oddly enough, such a message is displayed correctly in Outlook. Only if the
single-part-multipart-related-message is wrapped by multipart/signed, Outlook is
unable to display...

Comment 17

[Jean-Francois Ducarroz]

Thanks kaie for your analyze. This is not a PSM bug but rather an MS Outlook one
that we can prevent by avoiding generating "empty" multipart/related message...
Reassign to myself

Comment 18

[Jean-Francois Ducarroz]

I know what's the matter: it appends only when all the embedded object are
remote because we don't attach the source of a remote object. That's not the
problem. The problem is when we decide if we need to use multipart/related, we
don't know yet how many of the embedded object are remote!

Comment 19

[Jean-Francois Ducarroz]

another related problem: when sending a complex message (attachment + embedded
image + plaintext&HTML + signed), we generate a bogus multipart/related parts

here is the structure genarated:
 message/rfc822
  multipart/signed
   multipart/mixed
    multipart/alternative
     text/plain    (plain text version of the message body)
     multipart/related
      multipart/related     (<== this one should not be there, this is a bug)
       text/html   (html version on the message body)
      image/jpeg   (embedded image)
    text/plain     (attachment)
   application/x-pkcs-7-signature

Comment 20

[Jean-Francois Ducarroz]

Attachment: Proposed fix, v1

I fully review the way we generate message and found couple problems:
1) (the source of )remote embedded object weren't attached anymore with the
message
2) we generate multipart/related even when we don't have a related part to
attach .Append when embedded object are remote due to problem 1 or if we are
not suppose to attach the source of embedded object for security reason
3) in some case, we added an extra multipart/related part on top of the real
multipart/related part.
4) if the body as n time the same object embedded, we will attach n time the
object while one is plenty enough

This patch takes cares of all those problem and make the code much cleaner...

Comment 21

[Jean-Francois Ducarroz]

I have tested the patch v1 is various kind of message succesfully:

plain= use plain text compose
att= with one attachment
sign= message signed
text= use html compose, send format forced to plain text
html = use html compose, send format forced to html
alt= use html compose, send format forced to both plain text & html
rel= embedded a local image in the message body, send format is html
encr= message is encrypted

ok	plain
ok	plain+att
ok	plain+sign
ok	plain+att+sign

ok	text
ok	text+att
ok	text+sign
ok	text+att+sign

	html
ok	html+att
ok	html+sign
ok	html+att+sign


ok	alt
ok	alt+att
ok	alt+sign
ok	alt+att+sign
	
ok	rel
ok	rel+att
ok	rel+sign
ok	rel+att+sign
ok	rel+att (forced to plain text)

ok	alt+rel
ok	alt+rel+att
ok	alt+rel+sign
ok	alt+rel+att+sign
ok	alt+rel+att (forced to plain text)

ok	alt+rel+att+sign+encr

I have also tested that embedded object were sent with the message only if the
user put them in the message body (for security reason).

Comment 22

[varada]

Comment on attachment 93657 [details] [diff] [review]
Proposed fix, v1

This is a good cleanup. We dont have to go to the editor to structure for the
embedded objects 4 times and we can do it just once and store it. Also good
that we create the main multipart/related object in the beginning as the parent
in cases of having alternative (embedded objects in it)instead of doing it
later in the end. The reduction of the source for the multiple occurences of
the same object will help atleast a little so potentially the message generated
is always smaller in those cases.
R=varada

Comment 23

[David :Bienvenu]

+    if (NS_FAILED(image->GetName(tName)))
+      return NS_ERROR_OUT_OF_MEMORY;
+    attachment->real_name = ToNewCString(tName);
+    
+    if (NS_FAILED(image->GetLongDesc(tDesc)))
+      return NS_ERROR_OUT_OF_MEMORY;

I don't think we should assume the only possible reason these methods would fail
is NS_ERROR_OUT_OF_MEMORY. Why not just
rv = image->GetName(tName);
NS_ENSURE_SUCCESS(rv, rv);


+  PRBool shemeIsFile = PR_FALSE;
should be schemeIsFile

+        nsCOMPtr <nsISupports> isupp =
getter_AddRefs(mEmbeddedObjectList->ElementAt(i));
+        if (!isupp)
+          continue;
+      
+        node = do_QueryInterface(isupp);

Can't you use QueryElementAt here to go directly to the node, and avoid the
extra code?

Comment 24

[Jean-Francois Ducarroz]

Attachment: Proposed fix, v2

I have addressed bienvenu's comment. Davis, can you finish your review?

Comment 25

[Jean-Francois Ducarroz]

Comment on attachment 95906 [details] [diff] [review]
Proposed fix, v2

carry over r=varada

Comment 26

[David :Bienvenu]

Comment on attachment 95906 [details] [diff] [review]
Proposed fix, v2

you can use QueryElementAt here too, I think. fix that, and sr=bienvenu.      
-      if (!isupp) {
+    nsCOMPtr <nsISupports> isupp =
getter_AddRefs(mEmbeddedObjectList->ElementAt(locCount));
+    if (!isupp)
	 return NS_ERROR_MIME_MPART_ATTACHMENT_ERROR;
-      }

Comment 27

[Jean-Francois Ducarroz]

Fix checked in. I did take care of bienvenu's request in comment #26 but forgot
to post a new patch.

Comment 28

[Charles Rosendahl]

Verified a long time ago...closing.