1239815 - Associate LDAP account with MozReview via Bugzilla LDAP search

Status: RESOLVED INVALID

(MozReview Graveyard :: General, defect)

Description

[Gregory Szorc [:gps]]

MozReview's user database is currently backed by Bugzilla. In addition, we can associate an LDAP account with a MozReview user. This association is used to query scm_level_* group membership so we can see which permissions/repos people can push to. This is used to control authorization for Autoland and Try push, for example.

We currently associate the LDAP account as a side effect of an SSH-based push. During an SSH-based push, the client is able to authenticate with SSH (proves LDAP account ownership). In addition, the client passes its Bugzilla/MozReview API key. Since both are authenticated, we associate them together.

For various reasons, we want to move away from SSH-based submission to MozReview. We want to move towards submitting via HTTP. Unfortunately, when we use HTTP we don't have any LDAP foo as part of the request, so we don't associate an LDAP account.

We /could/ pass LDAP credentials to MozReview, try to bind on the server, and if successful associate the LDAP account. However, giving MozReview keys to kingdom is bad for security. I'm guessing this is a non-starter (despite Mozilla doing this in numerous places elsewhere - see all our internal sites using HTTP Basic auth). I certainly don't want MozReview handling LDAP credentials - it makes us a bigger target.

We /could/ pass a proxy for LDAP credentials - an API key if you will. But I don't believe such a thing exists. So again, a non-starter.

What I propose is something that doesn't involve the client passing LDAP foo to the server at all. Instead, when the server encounters valid Bugzilla credentials, it searches LDAP for a DN having the bugmail attribute (or whatever attribute it is) of that value. If an entry is found, we assume that LDAP account is associated with that Bugzilla account and record the association in MozReview.

There is some level of implicit trust here: we trust that the LDAP account holder specified a Bugzilla account they own. There is nothing preventing them from putting someone else's Bugzilla account. This would result in an LDAP account holder granting someone else's Bugzilla account their privileges. I don't think there is a major security concern here because it falls under the class of self-inflicted issues - just liking giving someone else your credentials.

kang: please weigh in with your security opinion on this matter.

Comment 1

[Justin Dow [:jabba]]

I believe this reasoning is valid, and this logic, or similar, is actually used by bugzilla admins as well to associate LDAP accounts with Bugzilla accounts (for purposes of assigning proper employee-only group memberships).

The attribute you'd want to look for in LDAP is bugzillaEmail and I believe a standard bind-user permission level can read this attribute, or at least search on it, to get a DN returned, or a mail attribute.

I'll leave the needinfo for :kang to review the security aspect of doing this.

Comment 2

[Guillaume Destuynder [:kang] [this account is no longer active]]

The description from comment 0 is AFAIK akin to what Bugzilla does to set the "moco confidential group flag" already.
I've discussed (proposed, I guess) this option with :gps orally. I believe it's an OK intermediate until we come up with a better IAM (hence the whiteboard tag as I keep track of these), on the rationale that this is what we already trust for bugzilla.
The threat mentioned by comment 0 is also the only threat I currently have recorded for this as well, which I believe is an acceptable risk, should you choose to accept it.

We could also ask for Phonebook to be modified to clearly state that any email you add must be under your control (I would highly recommend that, specially since Bugzilla does it already, again AFAIK).
I would also recommend having a first manual look at the output of what access is being granted to which email when first implementing the solution, just to ensure no one has any email address set that looks incorrect. Ideally, phone should verify all emails with the user.

Note that SSO (as in SAML) is an option that solves the 2 first proposal, but is currently unavailable for community LDAP accounts, hence why it's not in the list of possibilities at this time.

Finally, reviewboard RRA: https://docs.google.com/spreadsheets/d/1csA2Nt6BE0akwprFbrLmUl-nW6JpaG6vNDO4qAvWZTw/edit#gid=0 (mentions HIGH impacts thus we must be specially careful when implementing this)

Comment 3

[:glob ✱]

> The description from comment 0 is AFAIK akin to what Bugzilla does to set the "moco confidential group flag" already.

for bmo we look in the bugzillaEmail, emailAlias, and zimbraAlias fields to find a match for a given bmo address, skipping entities with an employeeType of 'DISABLED'.  we also have to canonise addresses: eg.  collapse [e-mail] to [e-mail].

that all said, i'm all for stricter requirements for mozreview as long as the error message when no association is found clearly states how to remedy the issue.

> I would also recommend having a first manual look at the output of what
> access is being granted to which email when first implementing the solution,
> just to ensure no one has any email address set that looks incorrect.

yup - there are currently 17 people with a mismatch between bmo and phonebook, even after our shenanigans :|

> Ideally, phone should verify all emails with the user.

+infinity

Comment 4

[:glob ✱]

(In reply to Byron Jones ‹:glob› from comment #3)
> for bmo we look in the bugzillaEmail, emailAlias, and zimbraAlias fields to

oops - plus the 'mail' field .. it's pretty common for someone to use their ldap email address on bmo and not populate the bugzillaEmail field.

Comment 5

[Gregory Szorc [:gps]]

Is there a way for non-staff to set bugzillaEmail?

If not, we need to figure out a way to provide that otherwise non-staff won't be able to associate an LDAP account and won't be able to use the autoland features. We would probably configure a command on the SSH server to perform this association as a stop-gap until a more robust IAM is deployed. Basically what we are doing today as a side-effect of pushing the review via SSH. Annoying, but doable.

Comment 6

[Gregory Szorc [:gps]]

This impacts the ability for Git users to use Autoland, since Git users always push via HTTP and therefore don't have an LDAP account associated.

In addition to this bug, we may want to provide a custom SSH command for associating LDAP users, as I doubt every user will have their Bugzilla address registered in LDAP :/

Comment 7

[Gregory Szorc [:gps]]

We now have a manual mechanism to associate LDAP accounts with MozReview. People pushing to MozReview over HTTP (all Git users) should associate their LDAP accounts.

See https://mozilla-version-control-tools.readthedocs.org/en/latest/mozreview/install.html#manually-associating-your-ldap-account-with-mozreview for instructions.

Comment 8

[BMO Automation]

MozReview is now obsolete. Please use Phabricator instead. Closing this bug.