141730 - replying to or forwarding inline encrypted mail should be first deciphered

Status: VERIFIED FIXED

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

Description

[Juergen Ruf]

From Bugzilla Helper:
User-Agent: Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.0.0+) Gecko/20020501
BuildID:    2002050108

When you receive an encrypted mail and forward this mail to another recipient,
the forwarded mail contains the original encrypted mail as an attachment
(smime.p7m). The attachment cannot be read by the recipient because it is
encrypted with the certificate of the initial recipient.


Reproducible: Always
Steps to Reproduce:
1.receive encrypted mail
2. forward mail to a recipient



Actual Results:  Forwarded mail contains original (encrypted) mail as attachment
(smime.p7m) which cannot be deciphered by recipient of the forwarded mail.

Expected Results:  Mail should be decrypted before it is forwarded. The dsame
behaviour is expected when replying a mail.

Comment 1

[John Unruh]

S/MIME

Comment 2

[Stephane Saux]

The correct behavior is that we don't allow encrypted email to be forwarded or
replied-to in the clear. I.e., you must encrypt the reply/forward yourself in
which case the email should be decrypted first then re-encrypted with the new
recipients' keys.

The behavior you're describing is with forward as attachments, isn't it?

While we're implementing the correct behavior, I'm actually happy that we
forward it encrypted. Encrypted emails are supposed to be confidential. Doing
what you suggest can get people in serious trouble.

Forwarding encrypted and signed email is a thorny subject. For example how do
you maintain the original signature?

We may not have a 100% solution but we're working on this. See bug 137071

Comment 3

[Kai Engert [:KaiE:]]

I'm extending this to include the "reply" case. Depending on the mail composing
preference (inline or attachment) what we forward is inline plain text, and
possible includes the original encrypted message, which is still encrypted.

Comment 4

[Stephane Saux]

marking adt3. There are no security issues.

Comment 5

[Jean-Francois Ducarroz]

the patch I posted in bug 156958 solve the forward inline issue and maybe the
reply one (I haven't tested replies)

Comment 6

[Kai Engert [:KaiE:]]

JF, your patch in 156958 looks very good at first sight!

When forwarding inline an encrypted message, or when replying to an encrypted
message, there is no longer an attachment, but the text is readable within the
new composed message.

Next I tested forward as attachment.
I have a message from A sent to A in encrypted form.
Then I tried to forward as attachment to B.
Up to today, the attachment would be forwarded in its original encrypted form,
and would therefore not be readable by B. I wanted to test, whether your patch
also helps to decrypt the attachment before it gets sent to B.
I sent that mail to B, again using encryption.

But when I tried to read the message using B's profile, I'm crashing.
It is not the fault of your new patch, because I tried to back out your patch,
and I still crash when reading the message.

Note that I can read the message with A's profile, and I do not crash.

So, your patch seems to produce a message that we are unable to handle without
crashing. I'm in the process of debugging. Maybe it's even a crash within the
crypto code, not mime, but I will know more soon.

Comment 7

[Kai Engert [:KaiE:]]

Good news. The crash is profile dependent and not related to your patch. It
crashes within the crypto code. When I use a fresh profile and import my private
certificate into the fresh profile, I do not crash. I filed bug 157750.


Re my intended test to check the status of your patch:

Message 1 is an encrypted message from A to A.
Message 2 is an encrypted message, that forwards Message 1 to B as an attachment.

My test shows that attached Message 1 is not readable by recipient B.
So decrypting attachments before sending still needs to be implemented.

(FYI Doing this same test with Communicator, the attached message Message 1 is
readable by recipient B.)

Comment 8

[Jean-Francois Ducarroz]

The crash I see with the patch for bug 156958 occurs only on MacOS X, works fine
on MacOS 9 and Win2K. I have a hard time to figure out what append! the crash is
kind of random but allways occurs when freeing a pointer.

Comment 9

[Jean-Francois Ducarroz]

I found the reason of the crash and it's not caused by the patch. The patch has
just made the problem to be easier to reproduce. I'll open a new bug for the
crash problem...

Comment 10

[Jean-Francois Ducarroz]

I filled bug 158334 for the crash.

Comment 11

[Jean-Francois Ducarroz]

Attachment: Proposed fix, v1

This patch enable security code which has been disabled in Mozilla.

Comment 12

[Jean-Francois Ducarroz]

reassign to myself

Comment 13

[Jean-Francois Ducarroz]

accepting

Comment 14

[Kai Engert [:KaiE:]]

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

r=kaie

Comment 15

[Kai Engert [:KaiE:]]

JF: While your patch looks good to me, I wonder whether we should check it in as
is, or whether we should wait.

A complete fix for this bug would reqire that all cases are covered, with one
not yet working case being described in comment 7.

I think one necessary step to complete the work is to fix bug 158356.
Once that work is done, we can check whether more is needed.

Comment 16

[Jean-Francois Ducarroz]

If I correctly understand the remaining problem, we need to decrypt messages
forwarderd as attachment. Right?

This patch currently take care of forwarding inline and replies.

Comment 17

[Stephane Saux]

Right, we decript it, and we then re-encrypt the current message, including the
attachment, with the new recipients certs.

Note that usually, encrypted message are also signed. Depending on how the
message was generated, it may have an encryption outer layer and inside you may
have a multipart signed message. In other cases, the signature is not detached.
In the case of a detached signature, it would be nice, but not a requirement to
be able to include the signature of the original message:

New message (forward)
  body from forwarded
  signature from forwarded
signature from new message.

Comment 18

[Jean-Francois Ducarroz]

This is a problem which require modification in message send and probably mime,
if we want to do like 4.5, we need also to warm the user if he/she forward as
attachment an encrypted email but does not encrypt his/her message. 

I would rather open a new bug for that case and check in this fix which covers
forward inline and replies.

Comment 19

[Kai Engert [:KaiE:]]

> If I correctly understand the remaining problem, we need to decrypt messages
> forwarderd as attachment. Right?

Yes.


> Depending on how the
> message was generated, it may have an encryption outer layer and inside you may
> have a multipart signed message. In other cases, the signature is not detached.
> In the case of a detached signature, it would be nice, but not a requirement to
> be able to include the signature of the original message:

I think we don't need to do special things to ensure that.
Encryption & Signing are separate layers within the message.
If we only decrypt, we only remove the encryption layer, but the other layers
like the signature layer should remain unaffected.


> This is a problem which require modification in message send and probably mime,

JF, do you have hope that fixing bug 158356 could brings us a step closer?


> if we want to do like 4.5, we need also to warm the user if he/she forward as
> attachment an encrypted email but does not encrypt his/her message. 

I have never seen such a message in Communicator 4.x.
But of course, we should assist the user to not forward without encryption.
That was our intention with bug 137071 and the not yet resolved bug 158175.

I think there is no difference between the forward inline and forward as
attachment situation. As soon as we check in the first patch that decrypts
previous content before we send out the new message, we'd need this protection.


> I would rather open a new bug for that case and check in this fix which covers
> forward inline and replies.

Do we really need a separate bug?

If you prefer to check in the partial fix before we have the fix for the
attachment case, I'd say that's fine. After having checked it in we could mark
the patch as obsolete, and continue to leave this bug open, until we have
addressed all the cases.

Comment 20

[Jean-Francois Ducarroz]

Fixing bug 158356 won't probably help much as we don't go through mime when
forwarding a message as attachment (at least not in 6.x). That's the main reason
why we should treat this remaining problem as a separate issue.

Comment 21

[Kai Engert [:KaiE:]]

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

Comment 22

[(not reading, please use [e-mail] instead)]

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

sr=sspitzer

Comment 23

[David :Bienvenu]

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

sr=bienvenu

Comment 24

[Jean-Francois Ducarroz]

Fix checked in the trunk (Mozilla 1.2alpha)

Comment 25

[Christopher Aillon (sabbatical, not receiving bugmail)]

Re-opening.  This caused tinderbox bustage and was backed out. 
http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1028695560.4857.gz&fulltext=1

Comment 26

[Jean-Francois Ducarroz]

I checked in again the fix but this time with couple #ifdef ENABLE_SMIME around
the new code when needed.

Comment 27

[Kai Engert [:KaiE:]]

Thanks, Jean-Francois!
I filed bug 161488 for the forward as attachment case that you suggested to do
in a separate step.

Comment 28

[Charles Rosendahl]

20020808 Trunk Builds Linux 7.3, Win2k, and OSX

Verified that encrypted messages are deciphered first before being sent.

Comment 29

[Kai Engert [:KaiE:]]

I have filed 162720. I have a signed and encrypted message sent from Outlook
Express, that will not get decrypted when forwarded inline.