139561 - Implement S/Mime "on demand verification" for IMAP messages

Status: VERIFIED FIXED

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

Description

[Kai Engert [:KaiE:]]

(See bug 125607, where a first step and temporary workaround were added.)

Messages with attachments coming from IMAP servers may have their parts
downloaded on demand only. This is even the default for some types of attachments.

Currently, Mozilla will display a question mark icon to visualize the fact that
the signature status is yet unknown.

What we need to do is:
- add UI strings that better explain what is happening
- explain that new behaviour in the help
- allow the user to trigger verification of the message, which will cause
downloading of all parts, and finally updating the signature icon to the
verification result. Maybe we want even a progress indication while verifying
large attachments over slow connections. Maybe even allowing the user to cancel.

Comment 1

[Charles Rosendahl]

*** Bug 139460 has been marked as a duplicate of this bug. ***

Comment 2

[Charles Rosendahl]

*** Bug 142115 has been marked as a duplicate of this bug. ***

Comment 3

[Stephane Saux]

*** Bug 144553 has been marked as a duplicate of this bug. ***

Comment 4

[Scott MacGregor]

I had a bug complaining about an integer showing up in the failed signature
dialog that was duped against this one. This bug seems to be about fixing the
problem for IMAP MPOD where we can't validate the signature. My bug was about
the contents of the signature dialog. 

Anway, I'll put the comments I put in that bug here:

"Hmm that sounds like a really strange feature. I think showing a random integer
string in the middle of some dialog text is going to generate more support calls
than not showing the integer. That really seems couter intuitive. Aren't they
going to wonder why this integer is there? I'm sure the UE teams don't like that
much.

The text already clearly states what's going on so can't they just describe the
text to the support person? I'll take that up in the bug you marked this a dup of."

Comment 5

[Kai Engert [:KaiE:]]

Scott: At the time I produced the temporary solution in bug 125607, I was not
allowed to add new UI strings. The only option I had was to introduce that new
icon and adding at least an error code, so that we can identify the problem when
people report it.

The first step in fixing this bug will be landing of more informative UI strings.

Comment 6

[Charles Rosendahl]

*** Bug 145203 has been marked as a duplicate of this bug. ***

Comment 7

[Kai Engert [:KaiE:]]

*** Bug 130164 has been marked as a duplicate of this bug. ***

Comment 8

[Kai Engert [:KaiE:]]

Attachment: Suggested simple fix

This patch is a minimalistic solution to the problem.
It is even a little bit better than what Communicator did, IMHO.

Communicator also used the "question mark signature status" icon.
The only way to trigger verification, to my knowledge, is to click the icon
using the mouse. I can't find menu items in Communicator or UI in the security
advisor to trigger it.

In Communicator, when you click the message, the message contents will go away
from the view. You will not be able to read the message until everything is
downloaded completely.


This patch does the following:

You can trigger verification using multiple ways:
- click the icon in the header area.
- click the status bar icon
- select "message security info" from the view menu

I think we should inform the user about what is happening.
Therefore the patch opens a prompt and asks for confirmation to download.

If the user clicks cancel, no downloading is triggered, and the question mark
icon will remain to be shown.

If the user clicks ok, downloading is started.
The message will shortly go out of view, and become visible again as it gets
downloaded.
While it gets downloaded, the status bar progress bar grows.
While it gets downloaded, there is no message security icon shown.
As soon as downloading is ready, the UI will get updated to show the correct
icon.


Caveat: If the user tries to open the "message security info" while the message
is still downloading, he will see the message, that the message does not
contain a signature.
I wonder whether I should enhance that to show a message like "message
downloading still in progress", or maybe we simply disable that menu command
until the message has been downloaded. What do you think?


Notes on the implementation: I had to add the "reload with all parts" mechanism
outside the S/Mime extension.

1.)
nsMsgDBView::ReloadMessageWithAllParts() is a clone of 
nsMsgDBView::ReloadMessage() and nsMsgDBView::LoadMessageByMsgKey(nsMsgKey
aMsgKey)

If you prefer it to not clone the code, but instead parameterize those
functions to do a full fetch or not, let me know.

2.)
I'm not sure whether 
  uri.Append("?fetchCompleteMessage=true");
is correct. Should I make the code detect, whether there is already a "?"
contained or not?

Comment 9

[Kai Engert [:KaiE:]]

> I wonder whether I should enhance that to show a message like "message
> downloading still in progress", or maybe we simply disable that menu command
> until the message has been downloaded. What do you think?

I tried to implement the latter but failed. But approaches would require that I
receive a notification when the message is fully downloaded. The current code
even already seems to be prepared for such an event, look at onSMIMEEndHeaders.
But unfortunately I never receive that notification.

I suggest, for now we accept the Caveat (as described in the previous comment).

Comment 10

[Kai Engert [:KaiE:]]

Attachment: Slightly updated fix

Has one empty function removed from the patch.

Comment 11

[David :Bienvenu]

I think this looks OK. But I'd prefer that you not clone that whole method, so
parameterizing it would be good. I think the ?= part is fine, since it's a
message uri, which shouldn't have any ?= part originally.

Comment 12

[Kai Engert [:KaiE:]]

Attachment: Updated patch, avoid cloning, parameterize existing functions

Comment 13

[Stephane Saux]

*** Bug 146317 has been marked as a duplicate of this bug. ***

Comment 14

[Charles Rosendahl]

*** Bug 147350 has been marked as a duplicate of this bug. ***

Comment 15

[Javier Delgadillo]

Comment on attachment 84564 [details] [diff] [review]
Updated patch, avoid cloning, parameterize existing functions

r=javi

Comment 16

[David :Bienvenu]

I think the "Sub" method names are not very readable. "Helper" is a bit better.
Or, I don't think you really even need to have different names for the helper
routines since they take different arguments than the callers, but Helper would
be sufficient.

Comment 17

[Kai Engert [:KaiE:]]

Attachment: New patch, "Sub" replaced with "Helper"

Comment 18

[Sean Cotter]

> +ImapOnDemand=The displayed message includes a digital signature, but the
signature status has not yet been verified. In order to verify the signature,
the complete message must be downloaded from the server.

I suggest modifying this as follows:

The displayed message has been digitally signed, but not all its attachments
have been downloaded yet. Therefore, the signature cannot be validated. Do you
want to download the complete message and validate the signature?

Another alternative for the last sentence would be "Click OK to download the
complete message and validate the signature," if a question inviting a Yes
answer seems inappropriate for an OK button.

Comment 19

[Kai Engert [:KaiE:]]

Attachment: Updated patch with new wording

I personally like the last alternative most.

Attaching a new patch that uses the wording:

"The displayed message has been digitally signed, but not all its attachments
have been downloaded yet. Therefore, the signature cannot be validated. Click
OK to download the complete message and validate the signature."

Comment 20

[Kai Engert [:KaiE:]]

Comment on attachment 85379 [details] [diff] [review]
Updated patch with new wording

has r=javi and r=cotter on wording

Comment 21

[David :Bienvenu]

Comment on attachment 85379 [details] [diff] [review]
Updated patch with new wording

sr=bienvenu, thx

Comment 22

[Kai Engert [:KaiE:]]

Checked in to trunk.

Comment 23

[Scott MacGregor]

*** Bug 147610 has been marked as a duplicate of this bug. ***

Comment 24

[Michael Baffoni]

The patch appears to be in the latest nightly, but it doesn't seem to work for
me (Win2k, Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.0.0+)
Gecko/20020529).  When I look at a message with an attachment (not viewed
in-line), I get the pen with the question mark icon, I click on the icon and get
the wording per comment #18, however the icon does not change after clicking on
the "OK".  If I click on the questionmark icon again, it gives me the same
dialog.  It's not downloading the attachment properly?  The attachment can be
saved locally (e.g. on the desktop) OK.

Comment 25

[Vladimír Solnický]

The problem seems to be solved for signed messages (I cannot see problems
mentioned in comment #24; icon changes after downloading the one attachment). 

BUT! The problem is NOT solved for long encrypted messages -- small broken key
icon is shown and it says (after cilking the icon): ``This message was encrypted
before it was sent to you, but it cannot be decrypted. There are uknown problems
with this encrypted message''. The whole message has the size about 438 KB (as
mozilla mesanger says). The same message in same IMAP folder can be decrypted
using Netscape 4.7x. Encrypted messages of small size OR the same message moved
to a local folder is successfully decrypted by mozilla. I think this is the same
problem with IMAP delayed download as was with signature verification. 

Comment 26

[Kai Engert [:KaiE:]]

I have a hard time reproducing what is being said in the previous two comments.

When I send a large encrypted message to myself, everything is fine. The message
gets even downloaded immediately.

Charles, can you please verify this bug on the trunk?

I would like to ask more people to verify whether the checkin has corrected the
problem for you, please comment, thanks.

Comment 27

[Vladimír Solnický]

I forget to mention, that the message I have a problem to decrypt is sent from
outlook and used the following S/MIME type:

                Type            : Application
                Subtype         : PKCS7-MIME
                Encoding        : BASE64
                Parameters      : SMIME-TYPE = signed-data
                                  NAME = smime.p7m
                Approx. Size    : 336,016 bytes

When I tried to send to myself a message encrypted by mozilla, it worked fine
and the only difference I can see was in MIME subtype:

                Type            : Application
                Subtype         : X-PKCS7-MIME
                Encoding        : BASE64
                Parameters      : NAME = smime.p7m
                Description     : "S/MIME Encrypted Message"
                Disposition     : ATTACHMENT; FILENAME = smime.p7m
                Approx. Size    : 2,909,850 bytes

Is it possible that the developers forgot that both officiall registered and
revious non-offician X- types can be used for encrypted messages?

Comment 28

[Kai Engert [:KaiE:]]

Looking at the message headers alone, I don't see what is happening. In any
case, as this seems to be an issue with Outlook/Mozilla interoperability, I
would like to move that problem to a separate bug.

Comment 29

[Vladimír Solnický]

It really seems to be an interoperability bug. I tried to send a large encrpted
message from NsC 4.7x and it is successfully decrypted by mozilla even in an
IMAP folder. May I be entered into the Cc: of the new bug you plan to create?
Thanks.

Comment 30

[Charles Rosendahl]

Which version of Outlook are you using?  I have tried with both Outlook 2000 and
Outlook Express 5.5 without any problems.  In fact, I cannot find any problems
between any of the clients (opaque/clear, signed, encrypted, signed/encrypted).

The mime types in both of these clients is tagged as x-pkcs-...

Comment 31

[Charles Rosendahl]

OK - I copied over the files to my local folders (aka emulated POP) and found
the following - This is a separate bug unrelated to the on-Demand verification -
has to do with MIME TYPES x-pkcs7-... vs pkcs7-...

Changing the protocol from x-pkcs7-signature to pkcs7-signature yields
functionality as if the message were never signed as below:

Content-Type: multipart/signed;
	protocol="application/x-pkcs7-signature";
	micalg=SHA1;
	boundary="----=_NextPart_000_002D_01C207E1.3DC14EB0"

Changing the following line to pkcs7-signature yields a broken signature icon.

Content-Type: application/x-pkcs7-signature; name="smime.p7s"

To reproduce the broken signature icon:
send a simple signed message to yourself (subject "test", body "test")over imap.
copy the message to your local folders.
edit the mime type in the local folder mail (I created a local folder called
inbox and copied the message there) file by removing the "x-" prefix.
restart the browser and view the message.

To reproduce the non-signed behavior:
send a simple signed message to yourself (subject "test", body "test" and a
small attachment)over imap.
copy the message to your local folders.
edit the mime type in the local folder mail (I created a local folder called
inbox1 and copied the message there) file by removing the "x-" prefix.
restart the browser and view the message.

New bug created - bug 148232.  This bug is fixed.

Comment 32

[Judson Valeski]

please checkin to the 1.0.1 branch ASAP. once there, remove the mozilla1.0.1+
keyword and add the fixed1.0.1 keyword.

Comment 33

[scottputterman]

adding adt1.0.1+.  

Comment 34

[Kai Engert [:KaiE:]]

Checked in to branch.

Comment 35

[Charles Rosendahl]

Verified - 20020606 Branch builds - Win2k, OSX, and Linux7.3

Comment 36

[Charles Rosendahl]

*** Bug 152943 has been marked as a duplicate of this bug. ***

Comment 37

[Charles Rosendahl]

*** Bug 148656 has been marked as a duplicate of this bug. ***