1092578 - Decide if an email needs to be encrypted at the time it is generated, not at the time it is sent

Status: RESOLVED FIXED

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

Description

[Bob Clary [:bc] (inactive)]

The last encrypted email I have received was on 10/27/2014 07:44 AM with subject:

[Bug 1089665] New: (Secure bug 1089665 in Core :: DOM)

The next bug I can confirm that should have been received as encrypted was on 10/28/2014 01:45 AM:

[Bug 1090105] New: The POODLE attack (SSLv3 supported)

It did not have the lock indicator and the contents were not encrypted.

Marking this as a security sensitive and confidential bug. I can attach the bugmails if that would help.

Comment 1

[Bob Clary [:bc] (inactive)]

I should have mentioned I use s/mime and not pgp.

Comment 2

[Daniel Veditz [:dveditz]]

I confirm roughly the same, receiving a secure mail as late as 2:52 PM (PDT) on the 27th, and then getting unencrypted mail on the 28th. Also using an s/mime cert.

The secure mail to the security mail alias uses a PGP cert and continues to be encrypted.

Comment 4

[Daniel Veditz [:dveditz]]

Gavin: do you have an S/MIME key or a PGP key?

Comment 5

[:Gavin Sharp [email: [e-mail]]]

S/MIME

Comment 6

[:glob ✱]

odd; i can't replicate this issue locally.

the headers on attachment 8515535 [details] indicate that we aren't even attempting to encrypt the message (no X-Bugzilla-Secure-Email header).  if i grab that message and feed it into my dev environment it spews out a s/mime encrypted email.


the investigation continue..

Comment 7

[:glob ✱]

(In reply to Bob Clary [:bc:] from comment #0)
> I can attach the bugmails if that would help.

yes, more examples would help.  i don't need the message body, just the headers.

Comment 8

[:glob ✱]

possibly related - there's a lot of emails stuck in the jobqueue, all with:
  utf8 "\x82" does not map to Unicode at /usr/lib64/perl5/Encode.pm line 174

will fix that in bug 1092949.

Comment 9

[Bob Clary [:bc] (inactive)]

Attachment: last good headers - Mon Oct 27 07:49:07 2014 PST

Comment 10

[Bob Clary [:bc] (inactive)]

Attachment: first bad headers - Tue Oct 28 12:19:16 2014

Comment 11

[:glob ✱]

thanks bc :)

here's what i've been able to determine:

- there isn't an obvious bug in the code; unable to reproduce locally or on stage
- no errors visible on the jobqueue server (in syslog)
- the code deployed to jobqueue2 looks good (and securemail isn't disabled there)
- both jobqueue1 and jobqueue2 have generated unencrypted email
- the mailer_before_send hook is being executed, because the @@body-header@@ placeholder is being removed by the bmo extension
- timestamps on the email show it didn't fail at any stage
- even if an error did occur during encryption, the default is to encrypt, not send in the clear
- bug 1092949 seems related, as it impacts s/mime encrypted emails only, however that triggers an error

still trying to figure this one out..

Comment 12

[Daniel Veditz [:dveditz]]

This morning I had a big chunk of older encrypted mail delivered (oldest was 2014-10-28 1:37pm) from an odd server [e-mail] (I noticed because it evaded my bug filter which includes [e-mail]). That would seem to cover the time range of when I stopped getting encrypted mail, but I haven't done the work yet to see if the encrypted ones match up 1:1 with the ones I got unencrypted.

Comment 13

[:glob ✱]

(In reply to Daniel Veditz [:dveditz] from comment #12)
> This morning I had a big chunk of older encrypted mail delivered (oldest was
> 2014-10-28 1:37pm) from an odd server
> [e-mail]

that's bug 1093147 .. for reasons currently unknown an old version of our email daemon was started on that server :(


i have finally figured out what happened..

the crux of the issue is: securemail determines if an email should be encrypted at the time that email is processed for delivery, and not at the time the email is created.  because of bug 1092949, any encrypted email for s/mime recipients was failing, resulting in a long gap between those two events.

the flow of events are:
- bug filed in core-security
- email metadata pushed into delivery queue
- email daemon tries to deliver, cannot due to bug 1092949, email remains in the queue
- the bug is made public
- email daemon attempts again to deliver, securemail doesn't encrypt because the bug is now public


dropping priority because the unencrypted emails should be for bugs that are now public.  i'll see how hard it is to change securemail so it tags emails for encryption at the time they are created.

Comment 14

[Gervase Markham [:gerv]]

(In reply to Byron Jones ‹:glob› from comment #13)
> dropping priority because the unencrypted emails should be for bugs that are
> now public.  i'll see how hard it is to change securemail so it tags emails
> for encryption at the time they are created.

A brief look suggests that email sending isn't really centralized until MessageToMTA. You could do it in _generate_bugmail in Bugzilla/BugMail.pm, but it would need a new hook.

This is a problem to a degree because sometimes bugs are made public but some comments remain hidden. So if bugmail for those comments in particular is stuck, it'll get sent in the clear. So there is a leak, although if mail is working properly, it seems small to me.

Gerv

Comment 15

[:glob ✱]

(In reply to Gervase Markham [:gerv] from comment #14)
> A brief look suggests that email sending isn't really centralized until
> MessageToMTA. You could do it in _generate_bugmail in Bugzilla/BugMail.pm,
> but it would need a new hook.

_generate_bugmail is too late, as it's executed just prior to sending the message.


i've already got a fix for this, just doing testing.

we need two new hooks, one in bugmail_enqueue to tag the email as requiring encryption, and another in bugmail_generate.  the way i'm doing it means it should work both with and without the new hooks.

Comment 16

[:glob ✱]

Attachment: 1092578_1.patch

this adds two new hooks at enqueue and bugmail-generation time.

it also addresses two encoding issues i encountered during testing:
- product/components containing non-7-bit characters were not encoded correctly in the subject
- inserting the original subject into the message was performing unnecessary encoding

Comment 17

[Gervase Markham [:gerv]]

Comment on attachment 8521463 [details] [diff] [review]
1092578_1.patch

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

r=gerv.

Gerv

::: Bugzilla/BugMail.pm
@@ +548,5 @@
>          $email->content_type_set('multipart/alternative');
>      }
>      $email->parts_set(\@parts);
> +
> +    # BMO: allow modifiation of the email given the enqueued variables

modifi_c_ation.

Comment 18

[:glob ✱]

To ssh://[e-mail]/webtools/bmo/bugzilla.git
   412b826..1aa5e76  master -> master