484247 - Apply securemail patch to bmo 4.0 branch and enable extension

Status: RESOLVED FIXED

(bugzilla.mozilla.org :: General, enhancement)

Description

[Dave Miller [:justdave]]

Bug 190945 introduced the ability to encrypt bugmail with the user's GPG key for security bugs.  It worked fine in the staging environment, unfortunately it broke when we deployed it to production, so it got turned off, and for lack of being a squeeky wheel, kinda got forgotten about amid other issues happening at the time.

The main difference between staging and production (and what's likely the cause) is that production is using the queueing system for sending mail, while staging isn't.  So staging needs to be set up to use queueing and then debug the heck out of it.  (alternately, backport the upstream queueing system, since it did get committed upstream and it's different than what we're currently using, and see if that helps any).

Comment 1

[Max Kanat-Alexander]

One of the primary reasons that I wanted to update to the upstream system is that it has much better error reporting and handling. So we could at least tell what is going on.

Comment 2

[Reed Loden [:reed]]

(In reply to comment #1)
> One of the primary reasons that I wanted to update to the upstream system is
> that it has much better error reporting and handling. So we could at least tell
> what is going on.

I agree with swapping to 3.4's system over trying to debug the current one.

Comment 3

[Gervase Markham [:gerv]]

So what's the story here? We are going to upgrade b.m.o. to 3.4 and see if that makes the patch work? Is there a set date for that?

Do I need to update the bug 190945 patch to the Bugzilla trunk?

Gerv

Comment 4

[Frédéric Buclin]

We first have to release 3.4rc1, then 3.4. b.m.o shouldn't upgrade before 3.4rc1 is out as it still has several annoying blockers.

Comment 5

[Dave Miller [:justdave]]

Yeah, porting the GPG plugin to the 3.4 branch is the plan at this point, since the breakage appears to be in the mail queue code and not GPG bugmail specifically, and the mail queue code was a local hack in 3.2, and actually exists upstream in 3.4 (and is implemented differently)

Comment 6

[matthew zeier [:mrz]]

Tracking this as a separate "upgrade bugzilla" issue.

Comment 7

[Gervase Markham [:gerv]]

Once I've got the Perl modules reinstalled I plan to port this patch forward and test it again. Let me know if there's a deadline for that.
 
Gerv

Comment 8

[Max Kanat-Alexander]

Gerv: bmo is not using that patch.

Comment 9

[Gervase Markham [:gerv]]

You mean my patch? What is it using? Did you do one? Does it support both GPG and SMIME? Are you going to be porting it forward as part of the 3.4 work?

Gerv

Comment 10

[Max Kanat-Alexander]

It's using an extension that Everything Solved wrote called SecureMail. I don't recall, but I think it only uses GPG. As far as whether or not I'll be porting it forward, that depends on Justin, right now.

Comment 11

[matthew zeier [:mrz]]

Instead of debugging this we're spending resources and time into moving to 3.4 which I've been told negates this.  Targeting October at the moment.

If I have that wrong, please re-open.

Comment 12

[Max Kanat-Alexander]

It may or may not negate it. It will provide better debugging facilities, though.

Comment 13

[Reed Loden [:reed]]

Going to reopen this, change the summary, and move it to Bugzilla: Other b.m.o Issues. Now that bmo is Bugzilla 3.4, we should try to do this again.

Gerv, can you provide an updated patch against bmo's 3.4 bzr branch?

Comment 14

[Gervase Markham [:gerv]]

Reed or Max: can you clarify whether my patch is the chosen approach? Max mentions a Bugzilla extension called SecureMail in comment #10... If you want me to update my patch, I'd be happy to.

Gerv

Comment 15

[Reed Loden [:reed]]

(In reply to comment #14)
> Reed or Max: can you clarify whether my patch is the chosen approach? Max
> mentions a Bugzilla extension called SecureMail in comment #10... If you want
> me to update my patch, I'd be happy to.

Hmm, that's a good point. I don't know which one was actually used. Dave?

Comment 16

[Dave Miller [:justdave]]

We have a securemail extension, it's in the bzr repo at the parent level.  It worked fine standalone, but broke when queuing was enabled.  It hasn't been tested against 3.4's built-in queuing yet (our 3.2 queuing was a local hack).

bzr co https://dm-bugstage01.mozilla.org/securemail/

Comment 17

[Reed Loden [:reed]]

Attachment: s/Bugzila/Bugzilla/ - v1

Fix misspelling in header.

Comment 18

[Max Kanat-Alexander]

SecureMail also lives at:

  bzr://bzr.everythingsolved.com/securemail/trunk

If you want to update it for 3.4 or 3.6, I'd be possibly interested in doing review on the changes, as I'd like to ship it as its own Bugzilla extension, if possible (though the hooks for it don't yet all exist upstream, and some of the ways that it does hooks are going to work better in 3.6 than the hooks it was using for 3.2).

Comment 19

[matthew zeier [:mrz]]

What's the current status of this?  At one point it was stalled until we updated Bugzilla because we'd get better debug output.  

If that's the case, what's the time line to get this working?

Comment 20

[Reed Loden [:reed]]

(In reply to comment #19)
> What's the current status of this?  At one point it was stalled until we
> updated Bugzilla because we'd get better debug output.  

Somebody needs to update the securemail patch to work with Bugzilla 3.4 and then thoroughly test it before pushing live.

Comment 21

[matthew zeier [:mrz]]

Is that "someone" justdave?  If it is, justdave - can you estimate how long it'd take to do that?

Comment 23

[Gervase Markham [:gerv]]

Attachment: Patch 2 v1

<sigh> OK, reed, if you insist.

This is the hooks patch from bug 567336.

Gerv

Comment 24

[Reed Loden [:reed]]

Comment on attachment 446695 [details] [diff] [review]
Patch 2 v1

>+[%+ Hook.process("extra_headers") || "X-TT-Whitespace-Handling: Is-A-Pain" %]

...

>+[%+ Hook.process("extra_headers") || "X-TT-Whitespace-Handling: Is-A-Pain" %]

There are better ways to fix the whitespace bug here...

Comment 25

[Reed Loden [:reed]]

(In reply to comment #23)
> <sigh> OK, reed, if you insist.

Why have a second bug when an original one already exists? I'm ok with opening this bug up publicly, if you want, though.

Comment 26

[Gervase Markham [:gerv]]

This bug was previously about an entirely different patch; I thought it would be good to have a new bug for a new approach. But never mind.

Anyone object to this bug being opened?

If you know how to fix the whitespace issue, please tell me! Because I don't. The problem is that it has to work both when the Hook produces output, and when it doesn't. And it seems like the output of the hook gets trim()med before being inserted, and so I can't just add newlines to the hook template.

Gerv

Comment 27

[Gervase Markham [:gerv]]

Can someone please make this bug public?

justdave: we're blocked on your review of the hooks, is that right?

Gerv

Comment 28

[Dave Miller [:justdave]]

(In reply to comment #27)
> justdave: we're blocked on your review of the hooks, is that right?

I don't know the hooks code well enough to feel confident reviewing this without spending a bunch of time studying other code first.  My once-over of the code looks pretty good to me though, except I agree there's gotta be a better way to fix the whitespace thing.  Reed implied in comment 24 that there was a better way, so he should share. :)

Comment 29

[Max Kanat-Alexander]

BTW, with the whitespace thing, you could have Mailer.pm delete the header if it exists, which would mean it would never get shown.

Comment 30

[Gervase Markham [:gerv]]

Reed: ping?

justdave: what do you mean by "I don't know the hooks code"; the patch puts hooks into _other_ code (and only a few of them, to boot).

Max: true, but that would be a hack too :-)

Still, let's get this sorted out. We've waited quite long enough.

Gerv

Comment 31

[Max Kanat-Alexander]

(In reply to comment #30)
> Max: true, but that would be a hack too :-)

  True, but it's the best hack I can think of.

Comment 32

[Reed Loden [:reed]]

(In reply to comment #30)
> Reed: ping?

I can't get to this for a while. My time is pretty limited due to school + moving for the next few weeks.

Comment 33

[Dave Miller [:justdave]]

Comment on attachment 446695 [details] [diff] [review]
Patch 2 v1

canceling unneeded review on fixed bug