1402494 - BMO Integration User is a full administrative user on Phabricator

Status: RESOLVED FIXED

(Conduit :: Phabricator, enhancement)

Description

[Jonathan Claudius [:claudijd] (use NEEDINFO)]

I cannot confirm this in prod/dev, so please keep that in mind when reviewing this, but I was reviewing the constants used on the BMO phabricator integration saw that https://github.com/mozilla-bteam/bmo/blob/master/extensions/PhabBugz/lib/Constants.pm#L22 defines the integration user as [e-mail].

That user was familiar to me because I was using in my testing and I just happened to remember that that user is a full administrator of my local phabricator instance.  If this is true in say prod/dev, that could mean that a compromise of that user's credentials could result in a full compromise of phabricator.

I know these two systems need to trust each other, because Phabricator relies on BMO to provide the authentication layer for users, which will then use Phabricator to land code.  The concern I have is that the [e-mail] specifically might be over-trusted as an admin and that we might be better served it operated with a lower permissions set.

It could also be very possible that since the [e-mail] needs this higher-level permission to be able to allow the BMO auth to work and if it were compromised as a lower priv user, the same net effect might be possible where an attacker would simply make the necessary request to phabricator to authenticate an attacker as anyone they would like.

At any rate, I want to raise this as a potential concern and let someone who knows the internals a little better to give this some thought.

Comment 1

[David Lawrence [:dkl]]

Thoughts?

1) remove full admin from phab-bot
2) create special bmo-phabricator group (project) and add the phab-bot user to it.
3) make all new revisions default to private to bmo-phabricator group
4) phab-bot from bmo would then be able to access the new revision in order to update its policies.
5) also allow projects to be edited and created by the bmo-phabricator group which will also allow the integration to continue to work.

caveat: even though phab-bot is not longer full admin, it could still be potentially compromised and an attacker could add themselves to a security group (project) in phab. And also they could see private revisions in the short time before BMO comes in and updates the policies. Still dangerous but maybe not as much.

dkl

Comment 2

[Jonathan Claudius [:claudijd] (use NEEDINFO)]

:dkl - that seems reasonable to me.  It at least binds your risk exposure to one account, where as with an admin account they could create a new account to do all their work there and potentially avoid incident respondors down the road.

Comment 3

[Piotr Zalewa [:zalun]]

The policy shouldn't be called "secure-revision" or we should rename the existing "secure-revision" group.

Comment 4

[David Lawrence [:dkl]]

Testing has been successful and this can move forward.

Comment 5

[Dylan Hardison [:dylan] (he/him)]

There is no patch attached to this bug but there is a commit referencing this bug in the bmo repo.

Comment 6

[David Lawrence [:dkl]]

Attachment: 430.diff

Comment 7

[David Lawrence [:dkl]]

Now live.