Page MenuHomePhabricator

Evaluate the feasibility of phasing out the Phabricator Security extension
Closed, ResolvedPublic

Assigned To
Authored By
mmodell
Jan 28 2016, 5:20 PM
Referenced Files
F3290022: escalate.png
Jan 29 2016, 6:48 AM
F3288975: lock2.png
Jan 28 2016, 6:08 PM
F3288973: lock1.png
Jan 28 2016, 6:08 PM
F3288977: lock3.png
Jan 28 2016, 6:08 PM
F3288906: Screenshot from 2016-01-28 11:05:59.png
Jan 28 2016, 5:20 PM
F3288898: report-security-issue.png
Jan 28 2016, 5:20 PM

Description

I'd like to deploy phabricator upgrade in the near future since there are a lot of really good features introduced since we last updated. The problem is, the upgrade will disable the security extension because the infrastructure it depends on has been removed from upstream phabricator. They didn't kill our feature without consideration, however, on the contrary, @epriestley did a lot of work to support our workflows without the need for any extensions.

Please take a look

You can try out the updated code which is currently deployed on https://phab-01.wmflabs.org.

  1. Log in (create an account if you haven't used phab-01 before)
  2. Click the icon on the toolbar in the top right of the page.
  3. The menu shows a new option labeled Report Security Issue which will take you to a new streamlined form to submit a task.

Tasks created through this form will be automatically assigned the appropriate policy. The result should be essentially the same as choosing "software security bug" on our current task submission form.

You'll get a better feel for it if you log in to phab-01, however, pictures are good too:

|
report-security-issue.png (184×284 px, 16 KB)
|
Screenshot from 2016-01-28 11:05:59.png (576×687 px, 35 KB)
|

A lot of discussion has happened in the upstream task, you can read all the details if you'd like more context: https://secure.phabricator.com/T8434

I just don't want to blindside anyone by breaking critical workflows or introducing major changes without much warning.

The only thing that I can think of which might be disrupted is converting a task from public to restricted. Currently that is supported by editing the value of the security field on a task. Without the security extension that won't be an option for most users, only someone with enough privileges to edit task policies will be able to restrict access to a task. How much impact will that have on people's workflows? I don't know if it's common to change a task from public to private. Once a task is submitted as public then it's potentially indexed on public search engines so restricting it after the fact does not prevent it from being seen. To me that limits the value of restricting tasks after they are already made public.

I really don't want to blindside anyone by breaking critical workflows or introducing major changes without much warning, which is why I am submitting this task. I'd like to solicit @csteipp's opinion, along with anyone else who is regularly involved in handling security reports.

IMPORTANT: tldr; Your feedback will be greatly appreciated. If there isn't any objection then I can proceed with deploying these changes next Wednesday during the scheduled deployment window.

Related Objects

StatusSubtypeAssignedTask
ResolvedNone
ResolvedNone
ResolvedNone
Resolvedepriestley
ResolvedNone
Resolvedmmodell
ResolvedNone
Resolvedmmodell
ResolvedAklapper
Resolvedmmodell
ResolvedAklapper
DeclinedNone
Resolvedmmodell
ResolvedNone
Declinedmmodell
Resolvedmmodell
Resolvedmmodell
ResolvedAklapper
Resolvedmmodell
ResolvedNone
DeclinedVarnent
Resolvedmmodell
Resolvedmmodell
ResolvedTTO
Resolvedmmodell
Resolvedmmodell
ResolvedAklapper
ResolvedAklapper
ResolvedAklapper
DeclinedNone
ResolvedNone
ResolvedAklapper
ResolvedAklapper
Resolvedmmodell
Resolvedcsteipp

Event Timeline

mmodell assigned this task to csteipp.
mmodell raised the priority of this task from to Medium.
mmodell updated the task description. (Show Details)
mmodell added projects: acl*security, Phabricator.
mmodell added subscribers: mmodell, Krenair, chasemp and 2 others.

The only thing that I can think of which might be disrupted is converting a task from public to restricted. Currently that is supported by editing the value of the security field on a task. Without the security extension that won't be an option for most users, only someone with enough privileges to edit task policies will be able to restrict access to a task. How much impact will that have on people's workflows? I don't know if it's common to change a task from public to private. Once a task is submitted as public then it's potentially indexed on public search engines so restricting it after the fact does not prevent it from being seen. To me that limits the value of restricting tasks after they are already made public.

This is a huge issue, and honestly this is a blocker in the short term.

Since it sounds like this is the direction your taking with Phabricator, we'll make it work. But it's going to take a fair amount of prep on my end to get ready for this. This is a very common workflow (I don't have the time to run a conduit query to count out how many issues have gone through this, but I can think of at least 3 recently), so changing this needs planning and communication.

Is there any way we can overlap the processes for a period of time, so users get used to pointing users to one form for creating normal issues, and another form for security issues? Will the security form be linked to from the normal form?

I think we'll need to give the right to escalate tasks into security bugs to a number of community members so we have multiple users able to perform this around the clock. Preferably, those users can't see tasks that are already security bugs. Is that possible?

Your feedback will be greatly appreciated. If there isn't any objection then I can proceed with deploying these changes next Wednesday during the scheduled deployment window.

^ This cannot happen. It's going to take a lot longer to get ready for this.

I think we'll need to give the right to escalate tasks into security bugs to a number of community members so we have multiple users able to perform this around the clock. Preferably, those users can't see tasks that are already security bugs. Is that possible?

Here's a possible approach:

Here's what it looks like in the UI;

lock1.png (1×1 px, 150 KB)

lock2.png (1×1 px, 142 KB)

lock3.png (1×1 px, 193 KB)

Notes:

  • The controller just changes the title rather than adjusting policies or custom fields, so that needs to be tweaked.
  • Some of the other UI stuff can probably be better tailored for your use case, it's mostly just placeholder text.

(You may also need to fiddle with setIcon() vs setFontIcon() vs setIconFont() if you try to run that slightly behind HEAD -- everything is setIcon() at HEAD, but some UI elements had inconsistent APIs until ~48 hours ago.)

Is there any way we can overlap the processes for a period of time, so users get used to pointing users to one form for creating normal issues, and another form for security issues?

Not really, upstream already removed the api we use after a short (~2 week) deprecation warning.

Will the security form be linked to from the normal form?

Yes, that's easy to do, each form can have arbitrary markup in the header above the form.

I think we'll need to give the right to escalate tasks into security bugs to a number of community members so we have multiple users able to perform this around the clock. Preferably, those users can't see tasks that are already security bugs. Is that possible?

I think we can give people the ability to edit policies without letting them see all security issues, however, anyone who can edit policies has a lot of power in their hands to restrict things that probably shouldn't be restricted (or vise-versa, however, they can only edit policy on objects they can edit)


That said, I may be able to implement some sort of escalate procedure that uses herald, to avoid the whole situation. I'm not entirely sure how that would work though. The way it works now is less than ideal. The dropdown is fairly cumbersome and the code is convoluted.

Here's a possible approach:
...

Holy crap that was fast. @epriestley never ceases to amaze me.

I'm one of the very fastest Phabricator implementors in the whole world!

@epriestley: indeed, but you sell yourself short. I'd say you are one of the most prolific and generally helpful developers I've ever known. (in general, not just in phabricator)

UI changes would require someone (who?) to update at least the following documentation pages on mw.org related to managing Security reports (I have not checked other wiki sites):

Here's my attempt at writing the markup for the security escalation dialog box:

escalate.png (700×604 px, 47 KB)

Discussion is in the diff: D113: Add a button to maniphest tasks to escalate security issues which need to be restricted.

@csteipp: Does the current version seem like a suitable solution? If so, please accept D113