Page MenuHomePhabricator

Accommodate the "Security" workflow
Closed, ResolvedPublic

Description

See some discussion in D13156.

Having built enough of Spaces to click around them a bit, giving spaces very hard boundaries (where you can either see the space and stuff inside it or you can't) feels like the right way forward, and softening boundaries to support the "Security" use case doesn't feel great (it adds a lot of complexity to configuring and understanding spaces).

However, the security use case is a mandatory use case for WMF and their interest in Spaces.

An alternative to softening the boundaries of Spaces is pushing Nuance up and giving it enough capabilities to supporting a dedicated security queue. This is preferable in a lot of ways. In particular, it would let you keep the reporter in the loop without sharing everything with them.

It's undesirable in that it may be a lot of work.

I'm going to look in to how practical this approach might be, so we can evaluate it against softening the boundaries of Spaces or making them act after automatic capabilities.

Related Objects

Event Timeline

epriestley claimed this task.
epriestley raised the priority of this task from to Normal.
epriestley updated the task description. (Show Details)

I read the Nuance description, but it is still unclear to me what it does.

Just for clarity the "Security" use case means per-task exceptions to the policy of a Space, allowing the author of the task and whoever is added to subscribers to access/comment/edit the task regardless of their permissions for the current space (right?).

Yeah, the "Security" use case is letting users see objects in a Space they normally don't have access to because they have some special relationship to those objects (for example, giving users access to security tasks if they reported them).

I'll walk through Nuance in greater detail once I have more clarity about whether it's viable, but briefly it would be an alternate workflow for security bugs:

  • Users are directed to https://your.install.com/form/security/ to report security issues.
  • This is a custom form with security-specific fields.
  • When submitted, this form creates a "Nuance Item", which is a bit like a lightweight task.
    • This task submits into a "Security" queue in Nuance.
    • The reporting user can see individual items they reported.
    • The queue as a whole is only visible to the security team.
  • Security team members triaging the queue have options to take simple actions on the report: comment, close it, or create a task from it.
    • If they choose to create a task, the task prefills with the correct Space, links to the Nuance Item, prefills description/title/etc.
    • The original reporter can not see the task or any work done on the task, only the separate discussion on their original report. This allows security response to be separated from communication with the reporter.

My current thinking is that approaching the problem in this way might be better for all users. However, Nuance has a lot of parts that don't exist yet, so it's not clear if it's actually an alternative that's realistically available. I want to assess:

  1. Is using Nuance viable from upstream technical and product perspectives?
  2. Is using Nuance viable on the June timeline we've committed to?
  3. Would using Nuance really be a better solution to WMF's problem?

I hope to have an answer for (1) and (2) today or tomorrow. If things still look OK, I'll present something more concrete so WMF can answer (3).

  • The original reporter can not see the task or any work done on the task, only the separate discussion on their original report. This allows security response to be separated from communication with the reporter.

That does not sound desirable to me in our context (unless the Nuance entry was basically just a special task, where we could have to option to continue as normal while including the reporter), what do you think @csteipp?

qgil added a comment.Jun 5 2015, 12:30 PM

In general I also like the concept of "hard spaces", simpler to understand
and to protect.

There is this "misuse" case that hard spaces would about: Security team
member leaves the team for some reason while staying as a regular
contributor.... but keeps access to old tasks that he authored. It is
simpler if you are either in our out.

In terms of timing, for what is worth we have a local solution for Security
currently, so that is not the most urgent part. Migrating teams that are
still using Trello etc because of lack of private spaces is the first
priority.

Getting into details, could an existing Maniphest task be converted or
cloned as a Nuance object? Some bugs might be filed without knowing that
they disclose a security vulnerability. Manual cloning means extra work and
risk for mistakes. A relatively minor detail at this point, though.

unless the Nuance entry was basically just a special task, where we could have to option to continue as normal while including the reporter

It will have some task-like properties: you can assign it, loop in other users (who have permission to see it) as subscribers, add comments, change its state, and maybe do a few other things (like move it between queues or add projects). You probably won't be able to change its priority, won't be able to add dependencies, etc. You probably can't put it on a workboard or give it points. Overall, this is still a bit up in the air. These will generally be sort of lightweight task-like items; they may be powerful enough for you to work in them in most cases, but they also might fall short of what you need.

There will be an easy way to "upgrade" a Nuance Item into a task and maintain a tight relationship between them, but they'll remain separate objects, not be two views of the same object.

Security team member leaves the team for some reason while staying as a regular contributor.... but keeps access to old tasks that he authored. It is simpler if you are either in our out.

Yeah, it's a lot easier to just remove someone from a space and know that they definitely can't see things inside the space than to have to additionally check that they aren't CC'd, assigned, auditing, reviewing, etc., anything in the space.

Migrating teams that are still using Trello etc because of lack of private spaces is the first priority.

Cool, that's good to hear. This "private spaces" use case is on much firmer ground right now than the "security" use case.

Getting into details, could an existing Maniphest task be converted or cloned as a Nuance object? Some bugs might be filed without knowing that they disclose a security vulnerability. Manual cloning means extra work and risk for mistakes. A relatively minor detail at this point, though.

Hmm, I haven't thought about this in any detail yet. I can imagine a few ways we could deal with this, but none jump out at me as definitely the best approach. I don't currently plan to make it easy to "reduce" a Task into a Nuance Item, but we could look at doing that.

A very soft workaround would be something like: move the thing into the Security space and then send the user a message saying "Thanks for the report, but please use the security form to report security issues in the future, so they're only visible to the right people. I've locked down your report and we'll follow up on it internally. If you'd like to be kept up to date, just send us a message through the security form and we'll connect you to the issue and keep you in the loop."

In T8434#118721, @qgil wrote:

In general I also like the concept of "hard spaces", simpler to understand
and to protect.
There is this "misuse" case that hard spaces would about: Security team
member leaves the team for some reason while staying as a regular
contributor.... but keeps access to old tasks that he authored. It is
simpler if you are either in our out.

So... you propose that we simply kill the ability to add specific users to specific private tasks, is that right? Just completely chuck out the policy system? The Wikimedia security project does not have a developer for every piece of software that might get security tasks reported to our Phabricator instance (many such pieces of software are not even run by wikimedia, but are instead extensions to partly-wikimedia-developed software), so this cannot happen.

Hmm, alright.

How would you like the flow where you give permission to new users to work? In T4411, @chasemp expressed some concerns about using CC/Subscribers for this (particularly, that users can add other users to CC). Do you share those concerns, or is adding users to CC to give them rights satisfactory for you?

Assuming adding users to CC is satisfactory and we implement T4411, what are the remaining issues with the current (pre-Spaces) policy model? The only thing I'm aware of offhand is that the default "Create Task" form doesn't have the right defaults.

If we had T4411 to let you share a task with users by CC'ing them, and some alternate way to present the "Create Task" form so that the applicable settings (notably, view policy) were impossible to get wrong, would that be a good solution to the problem? Or would there be remaining issues?

From https://phabricator.wikimedia.org/T76401, maybe that's actually ideal/desirable, and @chasemp's concerns aren't related to this use case (or perhaps are allayed by "hard spaces")?

Specifically, that task lists the expected behavior as "CC'd users can CC other users".

  • The original reporter can not see the task or any work done on the task, only the separate discussion on their original report. This allows security response to be separated from communication with the reporter.

That does not sound desirable to me in our context

It can be desirable. Being able to exclude reporters/ commenters on certain tasks from certain updates is very useful for handling procurement requests (such an RT feature was used at least by Wikimedia's Operations team).

Krenair added a comment.EditedJun 5 2015, 1:57 PM

How would you like the flow where you give permission to new users to work? In T4411, @chasemp expressed some concerns about using CC/Subscribers for this (particularly, that users can add other users to CC). Do you share those concerns, or is adding users to CC to give them rights satisfactory for you?

Fine with me personally, that was the system in BZ. I do wonder what Chris thinks about this now though.

Assuming adding users to CC is satisfactory and we implement T4411, what are the remaining issues with the current (pre-Spaces) policy model? The only thing I'm aware of offhand is that the default "Create Task" form doesn't have the right defaults.

Users who don't understand it. And - off the top of my head - I'd quite like our custom 'security' option to simply be the name of a pre-defined policy 'template' (we don't want unprivileged users to be able to set arbitrary policies, that can only result in a mess), but I haven't really thought through all of the implications of this (and it's possible that I'm forgetting some of the more obscure behaviour of it).

If we had T4411 to let you share a task with users by CC'ing them, and some alternate way to present the "Create Task" form so that the applicable settings (notably, view policy) were impossible to get wrong, would that be a good solution to the problem? Or would there be remaining issues?

Honestly now that our installation has had some existing hidden tasks modified to include subscribers in their view policy, I'm less unhappy about the CC thing, although I still feel that it is a bit silly that by default users can't CC each other on Phabricator directly to grant access (their email clients will certainly still allow it...)

  • The original reporter can not see the task or any work done on the task, only the separate discussion on their original report. This allows security response to be separated from communication with the reporter.

That does not sound desirable to me in our context

It can be desirable. Being able to exclude reporters/ commenters on certain tasks from certain updates is very useful for handling procurement requests (such an RT feature was used at least by Wikimedia's Operations team).

Yes, in some cases such as procurement, I agree. But this task is about the security workflow.

Users who don't understand it. And - off the top of my head - I'd quite like our custom 'security' option simply be the name of a pre-defined policy (we don't want unprivileged users to be able to set arbitrary policies, that can only result in a mess), but I haven't really thought through all of the implications of this.

Hypothetically, suppose there was a /maniphest/create/security/ form which only had a subset of the fields (title, description, whatever else you want) and forced all the other ones to be set to values you chose and didn't even show them to the user.

This form is for reporting **security issues**. For bug reports and feature
requests, use the _report bug_ form.

        Title: [___________________________]

           CC: [___________________________]

  Description: [___________________________]
               [___________________________]
               [___________________________]

                       [ Cancel ] [ Submit ]

Values not shown in the form would be locked to pre-sets. For example, "View Policy" would be locked to "Members of Security Team" (or whatever) and not editable by the reporter. Do you think that would be clear enough?

You'd still have to route users to this form in the first place, which might be a concern, but I think this would be hard for users to get wrong.

Or is the confusion you're concerned about after the report phase (like, how to deal with a "security" task)? What are the sources of confusion you've seen users run into, other than CC not adding view permissions?

Honestly now that our installation has had some existing hidden tasks modified to include subscribers in their view policy, I'm less unhappy about the CC thing, although I still feel that it is a bit silly that by default users can't CC each other on Phabricator directly to grant access (their email clients will certainly still allow it...)

I personally agree, and suspect we'll end up implementing it, but have hesitated to move forward too quickly because of @chasemp's concerns. I think discussion there outlines some reasonable steps to mitigate them, though, and if we space boundaries remain absolute that also helps make it easier to understand policies.

epriestley moved this task from Backlog to Unprototype (v1) on the Spaces board.Jun 5 2015, 2:24 PM
qgil added a comment.Jun 5 2015, 3:30 PM

So... you propose that we simply kill the ability to add specific users to specific private tasks, is that right?

No, I'm just agreeing with Evan that Spaces might not be the best option to handle the Security use case. Wikimedia Phabricator has its own Security extension working, and we will not remove it until there is an equivalent feature implemented in Phabricator upstream in a way or another.

We spent so much time on this it's almost nostalgic now :)

There is a lot going on with this task so I am going to kind of describe where we ended up as it's only loosely represented here.

We do have some magic to our use of policy and ‘Security’ tasks.

The primary case is for security bugs of Mediawiki software. We created a custom field drop down (called 'Security') that allows users to say "hey this is a security bug!" and with this magic happens. Mukunda has come up with custom policy options we can apply as ‘foolproof'. We associate the right group ('security'), we set the right policy (more to come here), and we generally make sure it's hidden well and for the correct people. For this special case, where we want unprivileged users to be able to report things securely and even better correctly, we do a few special policy things. We automatically add the author to the view policy. They are generally unprivileged policy wise but almost universally need to interact as initial reports are lacking or some such thing. The second policy WMFism is based around a custom reflective policy rule that Mukunda created that basically says anyone CC'd is allowed to view or edit (for which we use the same reflective rule in the normal policy locations). This is a version of the infamous T4411 but I swear I have $reasons why it's being done in a sane way.

For us the CC as viewer rule looks like this:

Any user who can view a task that has been set as a security bug for Mediawiki and Co can also loop in another user pretty seamlessly here. This mimics closely the user experience from Bugzilla where there was no actual policy abstraction in any strict sense. Security bug reports are also generally varied and covered in minutia involving a different group of tangential people each time with a consistent core. Because of this, the CC interface as policy editor does have a certain charm. Everyone understands it and for the most part it “just works”.

The reason I think this is good or at least acceptable.

  • This is explicit behavior so auditing the policy itself exposes the need to also look at the CC list.
  • If someone is abusing this CC as policy editing ability we can easily remove that rule from the policy set to mitigate and address

Reasons I have previously thought of CC as implicit policy override as bad:

  • It pollutes the idea of policy editing rights granting
  • If we said only users who already have policy edit can affect the policy in this way it would be functionally more appropriate but the UI for that and the user experience I can’t imagine being more straight forward than a “please allow CC here” explicit policy allowance. Then again UI is not my strong suit.
  • Alert on CC is generally the first “notify” thing that users ignore as it can have a lot of churn making ongoing audit or protection, especially for more long lived out of the spotlight sensitive issues, more difficult and error prone.
  • It is potentially difficult to diplomatically handle people who…frankly don’t know what the hell they are doing. :) But lowers the barrier for them to trip over their own feet.
  • Policy and implicit is generally bad (this where I admit to liking the assignee override but it could be stockholm syndrome)
  • Policy and implicit is generally bad and scary.
  • More ramblings https://secure.phabricator.com/T4411#78297

The second use case for us is a catch-all in the sense that we collapsed a few historical and new cases into one. We call this ‘other confidential issues’ and it is another option in the same drop down. ‘Other Confidential Issues' is for things such as “some guy is harassing me on phab” or “I need this thing which is not public knowledge in some way”. This is aimed at a much lager group of people we call WMF-NDA. The magic here sets WMF-NDA for policy, adds the project, and allows the author of the issue to view. As with the primary case it is common to have to ask sometimes why a user filed something as sensitive.

For both of these use cases we have an additional special behavior that is somewhat problematic. We do prevent the setting of ‘public’ or ‘users’ for any task with either sensitive information indicating drop down setting. We do this with a custom herald rule that fires as a global rule and says “hey this is marked confidential and you set public here so I am going to revert to sane defaults.”. In this way we prevent accidental disclosure in a somewhat sane way. This has been an issue as herald is inherently gameable, and not really suited for it. If you have a herald rule that says “always cc me on stuff” and you cannot see security issues you’ll get CC’d before our cleanup herald rule re-secures an almost accidentally disclosed task. So it works but only sort of. We have written the extension to be smart enough to say “this is a default I added but users also added some adhoc stuff and I shouldn’t nuke it”, and in this way we allow maximum flexibility.

The tertiary use case we have is an attempt to handle the “but I want a private project in a way that doesn’t exist”. We setup a template with a few attributes: policy, cc list, and project. We then grant the edit policy rule to a few people on the team that has this need for confidentiality and ask them to please only use their powers in their case and for good. This allows a few designated secure task loggers for a specific team, and allows them to manage their own default settings for those tasks. If this use case grows to the point where we outgrow the approach we can add their case to the Security drop down, or …figure something else out.

The quaternary case involves the general adhoc use of policy which we more or less frown upon, but does have a few special cases. We are attempting to port over our procurement logic to this medium, but per our legal department the general requirements for vendor communications denote a fewest people possible clause. We don’t own their communications per say, and we incur some real liability for sharing them in ways that are not necessary. So we have to keep what data we have in this category to #operations only as the legitimate audience for these matters. I’m not trying very hard to justify the legal stuff only because I just accepted it as coming from actual lawyers. We have had a few cases of restricted pastes that are even too sensitive for WMF-NDA in crisis situations, and I’m sure there is some generic policy use case I’m missing. Suffice to say we do use policy outside of our magic realm but it’s limited to a few people and cases.

So I don't know that our handling of Security things is right for nuance, I think if we could pass in the custom field via auxilliary with web parameters, allowing us to create a link in the link to a real-deal security task form, it would put us in pretty friendly territory already. I have gotten much fewer confused looks in the last 3 months since we hammered out a few of these edge cases.

I had always thought of Nuance as closer to the OTRS use case for autoreporting crashes and bugs, which would be perfect for the simple interface and possible task escalation.

Things from my position right now that would be great now-ish:

  • A native implementation of reflective policy that is an option (allow all cc as view or edit, allow author as view/edit…)
  • The ability to pass in auxiliary values via web param. If we had this we could create a link right now that opened a security reportable security task directly as our ‘magic’ is really hooked into the native custom field logic.
  • A more robust and awesome templating framework

Dreamy:

  • A way to designate different policy editors per Space?
  • Spaces that are, like, totally secure against hackers, man

reference:

http://www.mediawiki.org/wiki/Phabricator/Security

Thanks for writing that up!

It generally sounds like this use case is handled adequately (if not ideally) right now, and that Spaces won't conflict with the existing setup, so I'm going to not worry about this too much for now and focus on the primary "private" Spaces use cases instead. Once that feels more solid we should have a better sense of what tools it offers to improve the "security" case.

A native implementation of reflective policy that is an option (allow all cc as view or edit, allow author as view/edit…)

I'd ideally like to do this via T4411, but we could also meet halfway via T5681: that would let you write a "subscribers to this object" rule instead of needing to write a "subscribers to [explicitly, type in this object with your keyboard]" rule, which might simplify things a little bit, at least. (I assume the "type in this object" part is sort-of-automated via secret magic right now, at least some of the time, so maybe this is only very slightly nice-to-have.)

The ability to pass in auxiliary values via web param. If we had this we could create a link right now that opened a security reportable security task directly as our ‘magic’ is really hooked into the native custom field logic.

We can definitely do this.

A more robust and awesome templating framework

Yeah, I think there's probably room to improve this for these use cases while still getting a generally useful tool out of it.

A way to designate different policy editors per Space?

I hesitate to do too much magic here, but maybe a checkbox like this on the Space could work?

[√] Gooey, Sticky Space: Only users who have permission to edit this Space may remove objects from it.

Since (at least, with the current approach) spaces are always strictly stronger than any policy settings on an object, that would mean that a new hire in Operations couldn't do any damage by accidentally disclosing any secret plots with a careless edit: even if they set "visible to" to "public", the space policy is still stronger, and they can't remove the task from the Operations space.

Spaces that are, like, totally secure against hackers, man

like ya

20after4 added a comment.EditedJun 6 2015, 12:16 AM

I'd ideally like to do this via T4411, but we could also meet halfway via T5681: that would let you write a "subscribers to this object" rule instead of needing to write a "subscribers to [explicitly, type in this object with your keyboard]" rule, which might simplify things a little bit, at least. (I assume the "type in this object" part is sort-of-automated via secret magic right now, at least some of the time, so maybe this is only very slightly nice-to-have.)

Making it "subscribers to this object" instead of hard-coding the Tnnn, that would be a real nice thing to have. I had to do some nasty things to make that work automatically. Things I'm not at all proud of ;)

epriestley renamed this task from Evaluate Nuance for "Security" workflow to Accommodate the "Security" workflow.Jun 12 2015, 7:12 PM

The rest of Spaces is on more solid ground, now, so I'd like to keep working on a path forward here.

Giving Spaces hard boundaries continues to feel good to me from technical and product perspectives, and has not encountered or created any complications or conflicts yet. I'm not totally certain it will survive its first meeting with the real world, but am currently resistant to weakening it. Particularly, it makes the behavior of Spaces much easier to understand and predict.

I've also done a bit of work on T5681, to allow object-specific policies (for example, a "Task Author" rule which is only available for Maniphest). This isn't a complete solution, but feels pretty good to me as a tool we can use as part of the solution.

From earlier discussion, it sounds like Nuance almost certainly isn't the right way forward here, at least for the main use case (maybe it will eventually play a part in some other use cases).

From @chasemp's description above, I suspect we won't be able to provide all the tools you need to exactly replicate the workflow with standard Phabricator components, because it does so many specialized things. However, I think we can chip away at it and at least reduce the amount of custom/specialized code you need.


Some possible approaches:

Do Nothing: Leave everything as-is. It sounds like the current solution works and solves all the problems, it just isn't very elegant.

Reduce PolicyRule Hackiness: After T5681, it will be possible to implement an object "Security" policy which could mean "task author + subscribers + mebmers of Security project" in a reflective way. It would look like this:

This could simplify the implementation of WMFSubscribersPolicyRule and reduce the amount of stuff happening in SecurityPolicyListener, but would primarily be an infrastructure change. This kind of policy is also a little harder to add custom exceptions to; I'm not sure how common it is to do that (it seems like CC'ing them instead makes more sense in most cases?). If custom exceptions are important, the WMFSubscribersPolicyRule could at least be replaced with (or implemented in a manner similar to) the upstream rule, and SecurityPolicyListener could be simplified slightly.

Support Templating Custom Fields: We can fill custom fields from GET, letting you prefill settings for the "Security" dropdown. This would let you link directly to a "create a security issue" form.

Herald "Select" Custom Fields: T5016 got unblocked a while ago and is straightforward to pursue. This would let you implement "add the Security Project to security issues" as a Herald rule and simplify SecurityPolicyEventListener.

More Advanced Templating: We could create a workflow which let administrators formally create templates. A simple versio of this would be something like an admin clicking "New 'Create Task' Template", then choosing a template task ID and a name for it, like "Create Security Issue".

When users browsed to this form (via some new pathway), the fields would fill in with the template values even if the user did not have permission to edit them. This would let users select the "Security" policy by using that form,
without needing "Can Edit Policies" permission.

This would potentially let you get rid of the custom field and event listener (leaving only a small PolicyRule extension). However, either you or Maniphest would have to get users to make a modal choice before reaching the form ("file normal task" vs "file security issue").

If we pursued this, we could potentially pursue refinements to make this workflow cleaner or make the modal choice more obvious (e.g., let templates have intro text to point users at other 'create task' forms, let templates hide fields, let "Create Task" be a dropdown with several templates in it, etc).


I don't currently have a great path forward for these parts of the behavior:

Preventing Policy Changes to "All Users": I don't want to pursue field-level policies in the upstream at this time. I think they're too complex and have too few use cases, and I can't come up with any ways to achieve this goal in a different way. The "Sticky Space" option above would be OK in the upstream, but it looks like Spaces probably aren't a part of the solution here.

Adding Projects based on Policy: With enough changes above, you might be able to remove the custom field, but then you couldn't use a builtin Herald rule to add the "Security" project. I hestiate to let users write "Visibility policy is..." conditions in Herald: this feels very complex and like it has very few use cases. You could potentially use a simple custom Herald action.

With the templating approach, this could be a default value (but users could potentially remove it).

A "Task was created from template..." Herald condition would be reasonable with the templating approach.


I'm not sure which of these approaches are attractive. If preventing the policy changes to "All Users" is important, you probably need to retain the event listener in some form, and maybe the advanced templating stuff isn't really an improvement then.

I'd guess that supporting 'select' custom fields in Herald and allowing templating of custom fields are desirable no matter what. I think they're also quite simple, so I'll plan to pursue them.

I'm less sure about making templating more advanced. It seems like that mostly rests on how important preventing accidental "All Users" policy changes is. If that's a hard requirement, I think you need to keep the listener (or similar custom code in some other form).


How does all that sound?

Oh, and on these cases:

The tertiary use case we have is an attempt to handle the “but I want a private project in a way that doesn’t exist”.
The quaternary case involves the general adhoc use of policy which we more or less frown upon, but does have a few special cases. We are attempting to port over our procurement logic to this medium, but per our legal department the general requirements for vendor communications denote a fewest people possible clause.

I think these are resolved properly by Spaces, so maybe the number of users who need "Edit Policies" permissions can be reduced, which might make the risk of them setting something to "Visible to: All users" more palatable.

I also realize that you currently need "Can Edit Policies" permission to edit Spaces. That might need another look given how locked down "Can Edit Policies" permission appears to be in your use case.

@epriestley:

Reduce PolicyRule Hackiness + Support Templating Custom Fields would probably serve our needs well and it would also be useful, I'd imagine, for other installs to implement their own custom access controls.

If we could implement our custom rule in a less hacky way, and present it in the UI like your mock:

That seems pretty great to me. If we had that plus a way to link to a form with that policy pre-selected then I think it would probably satisfy our needs well. I'd like to hear some feedback from others on this but that's my two cents.

The form templating stuff sounds really cool.

Regarding editing policy on a task: I'm not sure how to do it but I think what might be desirable is that the task author has some sort of limited ability to comment on a 'security' task but not really edit it in any significant way after submission.

Krenair added a comment.EditedJun 17 2015, 6:27 PM

Regarding editing policy on a task: I'm not sure how to do it but I think what might be desirable is that the task author has some sort of limited ability to comment on a 'security' task but not really edit it in any significant way after submission.

... Why?

revi added a subscriber: revi.Jun 29 2015, 2:54 PM
anda added a subscriber: anda.Jun 30 2015, 12:48 PM

Status here:

  • Object policies (like "Task Author", "Subscribers", etc) seem to be working well and we've used them to solve other product problems in Passphrase and Conpherence successfully.
  • Herald rules now support custom <select> fields, so you can write a Herald rule based on the value of a Security: dropdown. The API for actions has also changed, see T8957 for discussion. (All internal fields and actions now use the same API that extensions have access to, so you can also write your own fields now.)
  • My next step here is improving templating. I'm still thinking about the best scope for the change. I am currently leaning toward substantially abstracting edit actions, as I think this has the potential to give us an attack on a large number of related problems. I'll file and consolidate this shortly.
epriestley moved this task from Backlog to The Queue on the Prioritized board.Aug 11 2015, 2:21 PM

@epriestley: Sounds good to me. Agreed that object policies are working well, I've already replaced the hacked-up policy rule with your Subscribers rule and it's working as expected.

Awesome, glad to hear it!

(T9132 roughly describes the approach I'm planning for the templating/edit actions. I have a moderately-clear picture of how it's going to work, but I want to clear some stuff out of the queue before I pursue it.)

I ultimately expanded the scope here into a lot of adjacent work in T9132, but a rough version of "better templating" is now available in HEAD and deployed and configured on this server. There's documentation here:

https://secure.phabricator.com/book/phabricator/article/forms/

This is still a bit rough in places and probably has some bugs, but I think the big picture stuff is working reasonably well and mostly usable.

In particular, the Use Case: Security Issues section of that document describes what I believe may be a viable pathway forward here, at least in rough terms, for some of the use cases. The executive summary is that you can now create a custom Maniphest form configuration for "New Security Issue", which only has fields you want users editing (e.g., title, description) and locks the values of other fields to specifiable defaults (e.g., "Visible To: Security + Task Author + Subscribers") and hides them completely.

Because this choice is modal (a user chooses to report a security issue or a normal issue at the beginning, and uses different forms to do each task), the task is never created with any other policy settings, so personal Herald rules can't interact with it (the owners of those rules are never able to see it). Each form can also have tailored instructions to help users complete workflows successfully.

Here are some sticking points from earlier, and how you might address them with the new toolset:

  • Adding Projects: You can prefill projects in these forms, and optionally lock/hide the "Projects" field so it can not be adjusted. You can also prefill custom fields now, and could use a Herald rule to add projects based on the value of the custom field. I'm also not strongly resistant to "Task was created with form: ..." as a Herald rule if it would be helpful.
  • Support Templating Custom Fields: Now supported with HTTP GET, template, and form customization. Moreover, you can set defaults and hide the field to force the field to have a specific value (though this may be moot anyway, at least eventually).

I think this may also work for "kind of private projects": create a form with all the policy stuff locked up, and users have to use that form to create tasks in their "kind of private project". You can restrict access to that form to users who have permission to interact with that project. This will let users create kind-of-private tasks without having any direct UI access to policy controls.


None of this fully prevents "accidental" disclosure by explicitly changing the view policy of an object you otherwise have permission to edit. Broadly, this concern feels very hypothetical to me. A user who seeks to intentionally disclose this information has myriad tools to do so (screenshot, print out a copy, etc) regardless of technical limitations.

It is difficult for me to imagine a user accidentally changing "Visible to:" to "All users" without anticipating the effects of that action. I don't think we've ever seen any reports of this happening in practice from any installs.

You can still make this kind of "accidental" disclosure hugely difficult by customizing edit forms (and we've done this on this install: users who are not members of Community have very streamlined create/edit experiences, now, on this install) but we do not offer technical mechanisms to completely prevent it if a user is sufficiently determined (e.g., looks up how to make the change using the API).

Anyway, perhaps "Create semi-private stuff with custom create forms + very restricted access to actual policy controls + ask an administrator for help if you need to move a task between policy zones" is reasonable overall.

On these:

I'm not sure how to do it but I think what might be desirable is that the task author has some sort of limited ability to comment on a 'security' task but not really edit it in any significant way after submission
Why?

...I think you can now more or less pick whatever you want. On this install, today, you can submit bugs and tasks and have little ability to edit them unless you are a member of Community.

I ultimately expanded the scope here into a lot of adjacent work in T9132, but a rough version of "better templating" is now available in HEAD and deployed and configured on this server. There's documentation here:
https://secure.phabricator.com/book/phabricator/article/forms/
This is still a bit rough in places and probably has some bugs, but I think the big picture stuff is working reasonably well and mostly usable.

This all sounds really good, I'll pull from HEAD and play with the code to get a better feel for it.

@epriestley: We haven't pushed this to production but my initial impression from configuring forms on https://phab-01.wmflabs.org is that this should work pretty well. The only thing I haven't figured out is a way to easily transition a task between private & security sensitive and public. It seems that will require someone with adequate permissions to manually edit the task policies, which isn't the end of the world. We could possibly continue to use herald rules to make the transition easier, though I am more keen on the idea of totally killing off both the herald rules and the custom event listeners.

At any rate I believe this should be workable and I hope to deploy the latest upstream code in the next couple of weeks. I'll report back if there are any issues or if any ideas come up that might be useful to you. I know ideas are worth about $0.00 so I'll spare you unless something brilliant comes up.

Awesome, glad to hear things sound promising. Thanks for the update!

I'll think about the declassify-a-task workflow a bit -- I don't immediately have a way to do that cleanly without petitioning a higher-level admin.

I think one of the issues was people making a public task, and then deciding they should change it to security, but not having the permissions to do so?

I think one of the issues was people making a public task, and then deciding they should change it to security, but not having the permissions to do so?

@Krenair I really don't know of any solution to that situation. Also, once something is published in the open it can't be taken back. At the rate that our phabricator instance is being hit by search robots, anything that gets published is likely to be searchable within minutes.

epriestley moved this task from The Queue to Paused on the Prioritized board.Feb 7 2016, 4:15 AM

I believe we generally have an acceptable way forward here, at least in broad strokes, now so I'm going to close this out since I think it has served its purpose, but feel free to file a followup if things stall or you run into trouble.

Here's a quick summary of the ultimate approach for any non-WMF readers:

New Form: A new modal form allows users to support security issues explicitly, and provides relevant controls and language (this is configuration via EditEngine):

Protect Action: A new "Protect" action allows public issues to be escalated to the security team (this is a custom flow which locks the task down). This is implemented as an extension:

epriestley closed this task as Resolved.Feb 7 2016, 10:42 PM
urzds added a subscriber: urzds.Jul 12 2017, 11:12 AM