Closed Bug 484247 Opened 13 years ago Closed 11 years ago

Apply securemail patch to bmo 4.0 branch and enable extension

Categories

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

All
Other
enhancement
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: justdave, Assigned: dkl)

References

Details

(Whiteboard: [bmo4.0-resolved])

Attachments

(1 file, 1 obsolete file)

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).
Assignee: server-ops → justdave
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.
(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.
Assignee: justdave → mrz
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
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.
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)
Tracking this as a separate "upgrade bugzilla" issue.
Component: Server Operations → Server Operations: Projects
Depends on: bmo-upgrade-34
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
Gerv: bmo is not using that patch.
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
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.
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.
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → WONTFIX
It may or may not negate it. It will provide better debugging facilities, though.
No longer depends on: bmo-upgrade-34
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?
Assignee: mrz → justdave
Status: RESOLVED → REOPENED
Component: Server Operations: Projects → Bugzilla: Other b.m.o Issues
QA Contact: mrz → other-bmo-issues
Resolution: WONTFIX → ---
Summary: GPG Bugmail needs debugging → Add GPG Bugmail patch to bmo
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
(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?
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/
Attached patch s/Bugzila/Bugzilla/ - v1 (obsolete) — Splinter Review
Fix misspelling in header.
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).
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?
(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.
Is that "someone" justdave?  If it is, justdave - can you estimate how long it'd take to do that?
Duplicate of this bug: 567336
Attached patch Patch 2 v1Splinter Review
<sigh> OK, reed, if you insist.

This is the hooks patch from bug 567336.

Gerv
Assignee: justdave → gerv
Attachment #412669 - Attachment is obsolete: true
Status: REOPENED → ASSIGNED
Attachment #446695 - Flags: review?
Severity: normal → enhancement
Summary: Add GPG Bugmail patch to bmo → Apply securemail patch to bmo 3.6 branch and enable extension
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...
Attachment #446695 - Flags: review? → review?(justdave)
Depends on: bmo-upgrade-3.6
(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.
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
Can someone please make this bug public?

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

Gerv
Group: infra
Component: Bugzilla: Other b.m.o Issues → Infrastructure
Product: mozilla.org → Mozilla Localizations
Version: other → unspecified
Component: Infrastructure → Bugzilla: Other b.m.o Issues
Product: Mozilla Localizations → mozilla.org
Version: unspecified → other
(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. :)
BTW, with the whitespace thing, you could have Mailer.pm delete the header if it exists, which would mean it would never get shown.
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
(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.
(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.
Blocks: bmo-upgrade
Assignee: gerv → dkl
Summary: Apply securemail patch to bmo 3.6 branch and enable extension → Apply securemail patch to bmo 4.0 branch and enable extension
Whiteboard: [bmo4.0-resolved]
Status: ASSIGNED → RESOLVED
Closed: 13 years ago11 years ago
Resolution: --- → FIXED
Component: Bugzilla: Other b.m.o Issues → General
Product: mozilla.org → bugzilla.mozilla.org
Comment on attachment 446695 [details] [diff] [review]
Patch 2 v1

canceling unneeded review on fixed bug
Attachment #446695 - Flags: review?(justdave)
You need to log in before you can comment on or make changes to this bug.