737401 - Secure email should send the "real" subject instead of the sanitized subject in the encrypted message body

Status: RESOLVED FIXED

(bugzilla.mozilla.org :: Extensions, defect)

Description

[Benjamin Smedberg]

Using S/MIME secure mail: the outer email subject is correct:

To: [e-mail]
Subject: [Bug 737307] (Secure bug updated)

But the encrypted subject inside the body should be the "real" subject and not the sanitized version:

$ openssl smime -decrypt -in securemail.eml -inkey ***
Enter pass phrase for ***:
Subject: [Bug 737307] (Secure bug updated)
Content-Type: text/plain; charset="UTF-8"
MIME-Version: 1.0

Comment 1

[:glob ✱]

i think you're actually getting a corrupted body somehow, because the content-type and mime-version headers shouldn't be present in the body.

looking at the code, the unencrypted subject is only injected into the body if pgp is being used, not s/mime.

Comment 2

[:glob ✱]

(In reply to Byron Jones ‹:glob› from comment #1)
> i think you're actually getting a corrupted body somehow, because the
> content-type and mime-version headers shouldn't be present in the body.

ah, that's just what Crypt::SMIME does.

we should be able to fix this by doing the subject fixup after the s/mime encryption (but before pgp).

Comment 3

[:glob ✱]

Attachment: patch v1

moves the subject sanitisation to after encryption has occurred.

Comment 4

[:glob ✱]

Comment on attachment 607554 [details] [diff] [review]
patch v1

oh, i forgot to fix:

> looking at the code, the unencrypted subject is only injected into the body
> if pgp is being used, not s/mime.

will fix after sleep.

Comment 5

[:glob ✱]

from bug 737725 - we have to use CRLF in the strings we prepend to the body.

Comment 6

[:glob ✱]

Attachment: patch v2

this revision always prepends the subject to the body (with CRLF line endings).

Comment 7

[Gervase Markham [:gerv]]

One question: has the call to encode() been removed on purpose?

Also, I'd prefer upper-case values for $key_type ("SMIME" and "PGP")... :-)

Gerv

Comment 8

[:glob ✱]

(In reply to Gervase Markham [:gerv] from comment #7)
> One question: has the call to encode() been removed on purpose?

yes, because i update the body in the $email object, and then pass $email->body to pgp, which is the encoded version.

> Also, I'd prefer upper-case values for $key_type ("SMIME" and "PGP")... :-)

no worries; i'll fix that on the next revision or on commit :)

Comment 9

[:glob ✱]

(In reply to Al Billings [:abillings] from Bug 737883 comment #6)
> There is discussion in another bug that if
> you're using s/mime, which includes an encrypted subject in the body, of
> getting that set up to be pulled into the subject of the message when
> decrypted somehow.

correct, however this subject isn't used to replace the non-encrypted subject in all email clients (the rfc says the client _may_ not _should_ do this).  thunderbird doesn't behave this way :(

it also doesn't help people who are using pgp encryption, or those who don't, or can't, provide a key (such as gmail and some accounts which are distribution lists).

Comment 10

[David Lawrence [:dkl]]

Comment on attachment 607862 [details] [diff] [review]
patch v2

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

Looks good and works as expected. r=dkl

Comment 11

[David Lawrence [:dkl]]

Error in the log:

[Thu Mar 22 00:22:25 2012] [error] [client ::1] Use of uninitialized value $key_type in string eq at ./extensions/SecureMail/Extension.pm line 285.  at ./extensions/SecureMail/Extension.pm line 258 \tBugzilla::Extension::SecureMail::_make_secure(...) called at ./extensions/SecureMail/Extension.pm line 232 \tBugzilla::Extension::SecureMail::mailer_before_send(...) called at Bugzilla/Hook.pm line 33 \tBugzilla::Hook::process(...) called at Bugzilla/Mailer.pm line 174 \tBugzilla::Mailer::MessageToMTA(...) called at Bugzilla/BugMail.pm line 602 \tBugzilla::BugMail::sendMail(...) called at Bugzilla/BugMail.pm line 442 \tBugzilla::BugMail::Send(...) called at /home/dkl/devel/htdocs/stable/attachment.cgi line 734 \tmain::update(...) called at /home/dkl/devel/htdocs/stable/attachment.cgi line 132 , referer: http://fedora/stable/attachment.cgi?id=142263&action=edit

Comment 12

[:glob ✱]

uninitialized warning fixed on commit.

Committing to: bzr+ssh://[e-mail]/bmo/4.0/
modified extensions/SecureMail/Extension.pm
Committed revision 8102.

Committing to: bzr+ssh://[e-mail]/bmo/4.2/
modified extensions/SecureMail/Extension.pm
Committed revision 8089.

Committing to: bzr+ssh://[e-mail]/bugzilla/extensions/securemail/4.0/
modified Extension.pm
Committed revision 17.