Page MenuHomePhabricator

Enabling Herald
Closed, ResolvedPublic

Subscribers
Tokens
"Manufacturing Defect?" token, awarded by mmodell."Love" token, awarded by Qgil."Mountain of Wealth" token, awarded by jayvdb."Mountain of Wealth" token, awarded by Nemo_bis.
Assigned To
Authored By
Nemo_bis, Oct 10 2014

Description

https://phabricator.wikimedia.org/applications/ doesn't contain Herald

https://phabricator.wikimedia.org/herald/ says:

Access Denied: Application Restricted Application
Access Denied: Application Restricted Application
You do not have permission to view this object.
Users with the "Can View" capability:

Administrators can take this action.

The existence of Herald was used as reason to WONTFIX some regressions from bugzilla, so it must certainly be available for everyone.

Related Objects

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
Qgil added a comment.Oct 10 2014, 4:46 PM

Yes, the intention is to bring it back as soon as T493: Prevent private information being leaked via Herald notifications is resolved.

Qgil renamed this task from Herald is not accessible to Enabling Herald.Oct 10 2014, 6:52 PM
revi added a subscriber: revi.Oct 10 2014, 8:09 PM
Qgil triaged this task as Normal priority.Oct 10 2014, 8:10 PM

Yes, the intention is to bring it back as soon as T493: Prevent that private information is leaked via Herald notifications is resolved.

Thanks. I assume this is meant for day 1, otherwise a number of other requests for day 1 will need to be reassessed.

Thanks. I assume this is meant for day 1, otherwise a number of other requests for day 1 will need to be reassessed.

Which requests? Not sure if I understand the "this" in "this is meant for day 1". :(

Nemo_bis added a comment.EditedNov 13 2014, 3:08 PM

Which requests?

Phabricator tasks about phabricator.

Not sure if I understand the "this" in "this is meant for day 1". :(

This = fixing the present task, T630.

A number of feature requests and bug reports have been declined on the basis that Herald existed; if Herald is not available, they need to be re-assessed.

Qgil added a comment.Nov 13 2014, 3:12 PM

We are still planning to fix T493 before the Bugzilla migration.

He7d3r added a subscriber: He7d3r.Dec 15 2014, 3:45 PM
Gilles added a subscriber: Gilles.Jan 8 2015, 9:38 AM
Aklapper lowered the priority of this task from Normal to Low.Jan 20 2015, 11:07 PM

FWIW, the lack of Herald makes it impossible for me to use an email-based workflow. Until this bug is fixed, I can't even consider properly following the issue tracker notifications, and I'm instead forced to keep multiple tabs open and check updates by refreshing regularly.

I'd be interested in your specific situation and usecase then if the current situation makes following "impossible".

Qgil added a comment.Feb 4 2015, 2:43 PM

I didn't think that we would be without Herald still in February...

What about a next step enabling Herald for a #Herald-Users group? wmf-nda and individually approved guests?

I would like to help test Herald. I've taken a look at some of its code before. Was T493 resolved with a Phabricator release? The latest update (T86772) doesn't mention it and I don't have any contacts to ask about it.

jayvdb added a subscriber: jayvdb.

Was T493 resolved with a Phabricator release?

No, that problem would still appear... mmodell should be the person who knows best about this.

scfc added a subscriber: scfc.Feb 9 2015, 4:20 PM
Aklapper added a comment.EditedMar 19 2015, 2:33 PM

As this comes up every now and then, and as only admins can currently access Herald:
Which conditions and actions Herald actually offers for Maniphest tasks (and what not):


Update: In addition, a "Remove project" option is available since May 2015.

Qgil added a comment.Mar 19 2015, 2:38 PM
In T630#1014570, @Qgil wrote:

I didn't think that we would be without Herald still in February March...

What about a next step enabling Herald for a #Herald-Users group? wmf-nda and individually approved guests?

T493 blocks this task.
I don't consider WMF-NDA sufficient, even less "individually approved guests" as we're talking about leaking security related information.
Setting Herald's "Can Use Applications" policy from "Administrators" to "Administrators, Security " seems to be the only acceptable/feasible option given the current situation, if I understand the technical implications correctly.

Snaevar removed a subscriber: Snaevar.Mar 24 2015, 8:20 PM

I'm quite tired of seeing "Restricted Application added a subscriber: Aklapper. · View Herald Transcript" in tasks.

In T493#1045918, @mmodell suggested just turning on Herald for everyone. What's stopping us from doing that?

People using it to gain unauthorised access to Security/WMF-NDA tasks.

Sorry, gaining access how? Presumably accessing private tasks requires being member of a specific project, right? Is Herald capable of changing project membership?

@MZMcBride: Accessing private tasks isn't strictly limited to members of any project. It's more complicated than that. In the case of security bug reports, herald allows bypassing security because of the interaction of two features:

  1. We (the phabricator migration team) were told that it is absolutely imperative that we keep the behavior from bugzilla such that anyone CC'd on a task can access it regardless of other security restrictions. So with significant objections (due to #2) we nevertheless implemented this behavior.
  2. Herald is capable of adding someone to the CC list of a task, and this action happens before any mechanism has a chance to prevent it. We thoroughly explored avenues available such as security policies, other herald rules, and even a custom phabricator plugin. None of these can reliably prevent someone who has access to herald from gaining access to all private tasks by CC'ing themself.

Okay, thanks for the explanation!

I remember the hard requirement that CC'd users be able to access a task. It probably makes sense to re-evaluate that requirement given its costs.

Otherwise, I'm not sure of other potential paths forward here currently. Hrm.

Qgil added a comment.EditedApr 24 2015, 5:14 PM

I don't consider WMF-NDA sufficient, even less "individually approved guests" as we're talking about leaking security related information.

Aren't security tasks visible to WMF-NDA by default?

In T630#1234114, @Qgil wrote:

I don't consider WMF-NDA sufficient, even less "individually approved guests" as we're talking about leaking security related information.

Aren't security tasks visible to WMF-NDA by default?

No. Private security tasks are visible to their creator, members of the Security project, and anyone who's been added as a subscriber. Security is effectively a subgroup of WMF-NDA, not the other way around.

This is now unblocked, pending the deployment tonight and some extensive testing, I think we can enable herald.

Qgil awarded a token.Jun 10 2015, 6:44 AM

Yes this is really unblocked now. It's been tested in production.

If @csteipp is satisfied with my testing, and nobody has any objections, then I will go ahead an enable herald for all registered users.

If @csteipp is satisfied with my testing, and nobody has any objections, then I will go ahead an enable herald for all registered users.

@mmodell, what's the revert plan if some weeks/months from now, we realize Herald has opened up an info leak? How easy will it be to turn it off again? I don't want us to be in the situation where we're using Herald for our internal processes, so we can't turn it off entirely, but someone finds a way to actively abuse it. Can we easily disable all Herald rules except ones we trust, and get back to the state we're currently in?

What about making the Herald application itself configurable by the security group? That would allow us to change who can use the application and who can manage global rules...

I don't know what happens when a user has stored a bad personal rule and then looses access to Herald this way though. I would hope that the rule ceases to take effect? @epriestley?

I don't know what happens when a user has stored a bad personal rule and then looses access to Herald this way though. I would hope that the rule ceases to take effect? @epriestley?

I guess someone could log straight into the DB and disable it there if it's still causing issues, although this kind of sucks.

We currently deactivate "Personal" rules when the owner's account is disabled, but not if they lose access to Herald. There's no technical reason we couldn't check that they still have access to Herald, although it might imply a bit of a performance penalty if the access policy is complex (it's currently relatively cheap to check whether a viewer can see a lot of different objects, but relatively expensive to check whether a lot of different viewers can see one object).

You can bin/remove destroy H1 to destroy a rule, although there's no CLI way to disable them without destroying them.

It would probably be reasonable to let admins or global-rule-managers disable any rule from the web UI. One concern is an attacker compromising an account, disabling all the rules, and then doing malicious stuff which escapes detection for longer than it normally would have, but we can make rules send email.

We currently deactivate "Personal" rules when the owner's account is disabled, but not if they lose access to Herald. There's no technical reason we couldn't check that they still have access to Herald, although it might imply a bit of a performance penalty if the access policy is complex (it's currently relatively cheap to check whether a viewer can see a lot of different objects, but relatively expensive to check whether a lot of different viewers can see one object).

You can bin/remove destroy H1 to destroy a rule, although there's no CLI way to disable them without destroying them.

It would probably be reasonable to let admins or global-rule-managers disable any rule from the web UI. One concern is an attacker compromising an account, disabling all the rules, and then doing malicious stuff which escapes detection for longer than it normally would have, but we can make rules send email.

Being able to destroy rules from the commandline is probably good enough for when we know that a malicious user is exploiting a loophole.

I'm more concerned that if we enable Herald for all users, we have thousands of Herald rules created, and then someone finds some rule or combination of rules that allows them to subscribe to all security bugs (or something else malicious), we probably can't realistically search through all defined rules and disable any bad ones. We'll either need turn Herald off entirely, or have a way to disable (maybe temporarily?) all rules that aren't defined by a person we trust. If we can do that with a db query or script, that's fine. I just want to make sure we have that plan documented (and tested) somewhere before we turn this on.

@csteipp: herald doesn't do anything without logging it. We can pretty easily notice if someone gained access to something they shouldn't because it'll show up in the maniphest transaction log for the task in question. Then it's pretty straightforward to track down which rule is an issue and delete it.

Another option is we could limit herald usage to members of some project WMF-NDA for example. But I suggested that before and it was rejected as exclusionary.

Qgil added a comment.Jun 12 2015, 7:09 AM

If we have tests that show that Herald's behavior with security tasks is now correct AND everything is logged anyway (i.e. we can see how certain user is sneaking into a security task right after its creation thanks to a Herald rule), then I think we have enough guarantees to enable Herald.

The log of this case is very explicit and visible at the top of the task history:

Restricted Application added a subscriber: Aklapper. · View Herald Transcript

@Qgil: agreed, there is a nice audit trail, so it's unlikely that any bad behavior would go unnoticed for too long and we could fix it up if anything does happen. At worst it'll be a headache to deal with. On the plus side it could provide quite a lot of utility for people who currently aren't able to utilize the power of herald to automate their workflows.

mmodell closed this task as Resolved.Jun 12 2015, 2:05 PM
mmodell claimed this task.
mmodell awarded a token.
TTO added a subscriber: TTO.EditedJun 12 2015, 2:20 PM

Cooool! Now I can have a happy medium between joining and watching a project - a "default CC" type behaviour where I can unsubscribe myself from the task if I'm not interested: H36

(BTW, can everybody see everyone else's personal Herald rules?)

I cant see H36 , and it doesnt auto link for me in this maniphest preview panel; I am guessing then that you cant see my H37 , which is auto linked for me in the preview panel.

In T630#1360416, @TTO wrote:

(BTW, can everybody see everyone else's personal Herald rules?)

It seems that the default policy is "No One" else can see (author can always see), and you can't change this.

Thanks everybody, also for the patience!
I've updated documentation on mw.org accordingly.

Couple of notes:

  • For completness, I'll mention that Herald rules are blocking and run in-process. If you accumulate 100K of them, creating and editing objects will slow down. They can't be moved to the daemons in cases where actions include workflow effects like "block commit" or "prevent diff", nor without degrading the user experience substantially. This hasn't been a problem for other large installs, but it's something you may want to keep an eye on. The "Herald Transcripts" view reports how much runtime was spent evaluating Herald rules. For comparison, we have about 100 rules on secure.phabricator.com and evaluating them costs about 30ms per create/edit operation.
  • Personal rules aren't visible to other users directly, but they're effectively visible in transcripts because their actions are public. It has been a challenge to balance privacy of, say, "keep an eye on these clumsy interns" rules against making the actions Herald take clear. I suspect we'll continue to leave personal rules effectively public and just make the UI more clear about this. It's possible they may just become properly public at some point, I'm not sure there's much value in limiting other users' ability to enumerate your rules given that I don't expect we'll ever prevent them from seeing and understanding their effects.
Qgil added a comment.Jun 12 2015, 3:26 PM

This one has been a great example of perseverance and patience indeed. Thank you to everybody involved!

@csteipp: herald doesn't do anything without logging it. We can pretty easily notice if someone gained access to something they shouldn't because it'll show up in the maniphest transaction log for the task in question. Then it's pretty straightforward to track down which rule is an issue and delete it.

My concern isn't that we wouldn't spot an individual case, but that after we disable their one rule, what's the plan to stop the next user from doing the same thing? Are we ok with turning off Herald completely? Or do we rely on it too much that turning it off isn't a viable option? If we restrict it to a particular group, do existing rules by users not in those groups not get run, or are users not in those groups just not able to create new ones?