125607 - Failed to validate signed message with binary attachment

Status: VERIFIED FIXED

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

Description

[Antonio Lam]

Build used: 2002021403 trunk

If a signed message contains an MIME attachment (eg. MS-Word, Excel document), 
mozilla failed to validate the signature, and shows <invalid signature>.

Note:  if the attachment is inline (mean it can be recognized by moz, eg: a jpg 
image), then mozilla works fine and displays as a valid signature.

Comment 1

[Stephane Saux]

works for me using build Id 2002031103

Comment 2

[Stephane Saux]

wfm

Comment 3

[Antonio Lam]

Using 2002031503 trunk. I still see the problem.

Steps:

1. Send a sign message with an non-inline attachment (eg. a MS-Excel doc). Note 
that it worked fine with inline attachment such as JPG image.
2. Tried to retrieve the sign message, and it show <invalid signature> in the 
header area of the message. The security information dialog to be followed.

Reproducible: always.

The same sign message can be read from Commnicator as a valid sign message

Also, if the Attachment file size is very small (eg. less than 10k), mozilla is 
able to validate the signature.  But if the file size is greater than that, it 
showed <invalid signature>.  I will attach a sample excel document which will 
cause mozilla to failed to validate the digital signature.

Comment 4

[Antonio Lam]

Attachment: Sample Excel file that cause mozilla failed to detect the digital signature

Comment 5

[Antonio Lam]

Attachment: security dialog for the invalid signature

Comment 6

[Charles Rosendahl]

I have encountered this problem on the 2002040503 build.

Check the sent directory as well after mailing out a large attachment.  In this
case I mailed a dll as a signed attachment.  The dll size was approximately 250k.

This is serious enough to fix for beta.

Comment 7

[Charles Rosendahl]

Upping the Severity/Priority of this bug.  really nominate for nsbeta+.

Comment 8

[Stephane Saux]

kai

Comment 9

[Kai Engert (:KaiE:)]

What we have is a signed message in multipart format, i.e. the outer part is a
multipart message, where one part is the actual message, the other part the
signature.

In simple signed messages, where there is no attachment, the actual message is a
simple message.

However, when sending messages with attachments, the actual message is a
multipart message, too, where each attachments lives in its own inner part.

The signature, contained in the outer multipart message, is a signature over all
parts of the inner message.


I verified that the sent message has a correct signature, because I can verify
such messages correctly with Communicator and with openssl's smime command line
tool.


I traced what happens. Only the contents of the first part of the inner
multipart message are passed on to the hash data calculation.


The problem seems to be either in mimemsig.cpp or in mimemult.cpp.
When the boundary after the first inner part is seen, mult->state arrives at
MimeMultipartHeaders.

The correct approach should be to continue looking for additional parts, and
continue processing.

But what it does, it set sig->state to MimeMultipartSignedSignatureHeaders. This
causes that data_eof routine for signature processing will be called,
terminating data collection.

(In addition, because mult->state stays unchanged, and other parts will be skipped.)


The solution is to implement correct detection of all inner parts, and not
terminate data collection until all data has been feeded to the crypto code.

I need to learn how to do this, and I will need to look at the code more
closely. Suggestions welcome :)

Comment 10

[Kai Engert (:KaiE:)]

Only part of my previous comment is true, meanwhile I know the real reason of
the problem.

The problem is restricted to situations where all of the following come together:
- signed message
- at least one attachment with a mime type, that can not be displayed "inline"
in the message view (e.g. binary document file)
- viewed from an IMAP server
- larger than a specific minimal size (seems to be > ~32 KB)

The current behaviour of mail code is to avoid downloading of parts from
multipart messages, which can not displayed inline.

But verifying the correctness of a message will only work, if the complete
message is fed to the crypto engine, in order to compute the crypto checksum
over all message data.

What we currently see is, only some portions of the message are fed to the
crypto engine, and the result will of course not match the signature.

I looked what Communicator did in such a case. It has the same strategy to avoid
downloading of attachments. However, it does a better job of dealing with this
kind of message.

When Communicator detects a signed message with attachments, it neither displays
"valid signature" or "invalid signature". Instead it displays a special icon,
which visualizes "unconfirmed signature" - a signature icon with a red question
mark.

One solution to implement a correct behaviour in Mozilla would be, to always
download all parts of the message. But personally I think that is not
acceptable. (But we could do this as a temporary workaround, what do you think?)

In order to replicate the behaviour of Communicator, which seems more approriate
for messages with large attachments, for users on slow network connections, we
have to do several things:

First, the mime parser needs to detect that special situation, and needs to
communicate that to the crypto UI.

We need to create new icons (at least one per skin).

We need to enhance the "message security info" code, to display details.

We need to add logic, if the user clicks on the new "unconfirmed signature"
icon, to download the message again, download completely this time, verify the
signature, and update the UI.

Comment 11

[Kai Engert (:KaiE:)]

I think we are unable to implement the complete solution for beta, therefore we
should remove nsbeta1+.

What do you think, should we try to work on the workaround, as suggested in the
previous comment?

Comment 12

[Jean-Francois Ducarroz]

I would suggest as temporary solution to just make a release note telling how to
turn off "part on demand", we have a pref for that if I am right.

cc'ing bienvenu and mscott for advise

Comment 13

[David :Bienvenu]

I don't think the workaround is acceptable for the general case (turning off
mime parts on demand) - many more users care about not automatically downloading
large attachments than care about the validity of signatures for the few
messages that have them.
As far as turning of mime parts on demand is concerned, there is a per server
pref called "mime_parts_on_demand" that you can set to false to turn off mime
parts on demand. There is no UI for this, however, so you would need to edit
your prefs.js by hand.

Comment 14

[Jean-Francois Ducarroz]

Kai, if you have access to the mime object (MimeObject) from the crypto engin,
you can check the option (MimeDisplayOptions) missing_parts to know if you
have/will have a full message or not into your hand:

obj->options->missing_parts

Comment 15

[Charles Rosendahl]

Not sure what Outlook and Outlook Express are doing (pulling down the entire
body, etc), but it sure looks nice when the signature is verified and doesn't
suggest the message was tampered with (nobody swapped my haveaniceday.exe file)

If this can't be fixed by beta, then it needs to be fixed by RTM.  We have to at
least provide the same correct functionality as Outlook/OE to remain competitive
on the desktop.

Comment 16

[Jean-Francois Ducarroz]

Microsoft IMAP implementation is very basic, they alway pull the all message
making display messages with large attachment much slower than Mozilla.

Comment 17

[Kai Engert (:KaiE:)]

Summry:

For beta, we only have a limited amount of choices. For UI strings, which could
inform the user, it is too late.

If we do nothing, the user will see that the signature is invalid. Currently we
report the statement to the user, that signature does not match the message.
This statement is wrong.

The best I can think of as a temporary solution is the following:

- introduce new icons, which are shown instead of the broken signature icons.
  I created what's necessary, and just placed a red question mark on top of
  the signature icon.

- we detect the situation that parts are missing, and display the new icon.

- we introduce a new failure code for this situation. In the UI, instead of
  reporting "signature invalid" we report "unknown problems".
  The message "unknown problems" is already contained in our string bundles.

That way, we at least do not mislead the user.
Because the detailed information dialog also shows the error code (1039) for
that situation, we will be able to classify bug reports.

Comment 18

[Kai Engert (:KaiE:)]

Attachment: Suggested temporary fix

Comment 19

[Kai Engert (:KaiE:)]

Attachment: Icon for "unknown signature status"

This icon will be shown in the message header area, a corresponding smaller
icon on the status bar.

Comment 20

[Kai Engert (:KaiE:)]

Question: Should we take this temporary fix for beta?

If yes, please review the patch.

Comment 21

[Jean-Francois Ducarroz]

Are we already in UI freeze mode? I would personnaly suggest to add the needed
string event if it might not be translated in other supported language, but that
just my opinion.

Comment 22

[Jean-Francois Ducarroz]

Comment on attachment 79813 [details] [diff] [review]
Suggested temporary fix

looks good. R=ducarroz

Comment 23

[Kai Engert (:KaiE:)]

bienvenu or mscott, can you please sr ?

Comment 24

[Stephane Saux]

A more complete fix for RTM will allow the user to cause the attachments to be
downloaded to allow the signature to be verified.

For beta, the fix shows the signature with a question mark. The reason given
when the message security dialog is opened is that the signature couldn't be
verified for "unknown reaons" (this is the only existing string we have that's
better that what we have now).  I propose that even though it's late in the game
we add a couple of strings to give the real reason.

The real reason is that in imap mode, the message is downloaded, but not the
attachments (when they're large enough). Note that in POP mode the same email,
because it would be fully downloaded would verify.

Comment 25

[Kai Engert (:KaiE:)]

Attachment: Please ignore this

- uses ugly slow workaround for unexpected NSS behaviour
- fixes the display
- avoids crash
- fixes a memory leak, which will help for bug 125561

Comment 26

[Kai Engert (:KaiE:)]

PLEASE IGNORE the previous comment. Wrong bug.

Comment 27

[David :Bienvenu]

Comment on attachment 79813 [details] [diff] [review]
Suggested temporary fix

sr=bienvenu

Comment 28

[scottputterman]

can you check this into the trunk and get this tested?

Comment 29

[scottputterman]

adding adt1.0.0+.  Please checkin to the branch as soon as possible and add the
fixed1.0.0 keyword.

Comment 30

[Kai Engert (:KaiE:)]

The workaround (suggested for the branch) has been checked in to the trunk.

Bug stays open because the real problem is not yet fixed.

Please verify that tomorrows trunk build shows a question-mark-signature-icon
and does no longer say "invalid signature", but shows a general message about
problems with the signature.

Comment 31

[Charles Rosendahl]

Per Kai, in the 4/23 Trunk build.  Changing to Resolved Fixed for verf purposes.

Comment 32

[Charles Rosendahl]

Verified on the trunk - build 2002-04-23-10-trunk.  Will reopen after beta 
release.  Marked fixed in keywords after landed in the branch.

Comment 33

[Kai Engert (:KaiE:)]

Thanks for verifying - my intention was to leave this bug open since the real
problem has not been addressed yet.

But I agree this is complicated to track. I will file a separate bug for the not
yet resolved issues.

Comment 34

[Kai Engert (:KaiE:)]

I filed bug 139561 for the remaining parts.

Comment 35

[Scott Collins]

Comment on attachment 79813 [details] [diff] [review]
Suggested temporary fix

a=scc for checkin to the mozilla 1.0 branch

Comment 36

[Kai Engert (:KaiE:)]

Checked in to branch.