1070119 - OpSec review for transplant tool

Status: RESOLVED FIXED

(mozilla.org :: Security Assurance: Review Request, task)

Description

[Chris AtLee [:catlee]]

Transplant tool is going to be an important part of both autoland and uplift process for sheriffs.

Current code is at https://github.com/laggyluke/transplant, and there's a test instance running at http://review.infinity.com.ua:5000/.

We'll be working on integrating that into relengapi shortly.

Comment 1

[Chris AtLee [:catlee]]

The most critical piece here is that this service will need to push code to L3 repositories on hg.

Comment 2

[Chris AtLee [:catlee]]

We've been through this (or something like this) before:
https://bugzilla.mozilla.org/show_bug.cgi?id=910029

Comment 3

[Curtis Koenig [:curtisk-use curtis.koenig+bzATgmail.com]]]

assigned to joes for assignment to an opsec team member

Comment 4

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

Let's go through a 30min RRA together - Chris, I tried to add you in the calendar but I think you're out of office for the week. Do you want me to go through this with anyone else or to wait until you get back?

Comment 5

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

RRA planned wed 2014-10-01

Comment 6

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

RRA is at https://docs.google.com/a/mozilla.com/spreadsheets/d/1Vs_n24l2_QKhVyYWYsj_m5qRdMaXsEka4KyV4xqTLgI/edit#gid=1112083357

Chris as you want to follow up with a sec review of the design I'm leaving this open, but I'll need a ping from you when you're ready (as discussed on vidyo).
if it takes over a month we might close up and reopen the bug. I'm setting you as needinfo but feel free to clear it now or later, however you like to manage this on your end.

Also thanks for attending the RRA!

Comment 7

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

Added diagram PR https://github.com/laggyluke/transplant/pull/4

Comment 8

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

Result of RRA: https://docs.google.com/a/mozilla.com/spreadsheets/d/1Vs_n24l2_QKhVyYWYsj_m5qRdMaXsEka4KyV4xqTLgI/edit

Relevant items:

PR/Image confidentiality HIGH:
Some RESTRICTED data such as SSH private keys used for commits, and login assertions.

PR/Image access control MAXIMUM:
Attacker could merge commits that are shipped into Firefox and other Mozilla products with malicious code.

PR/Image availability may become MEDIUM in the future if this tool is relied upon for releases, chemspill, etc.

For ops (RelOps in this case)
Recommended system assurance level HIGH.
System handles RESTRICTED data.

Other recommendations:
- Access control via LDAP group membership for users
- Notice of successful transplants to sheriffs recommended (mail notifiction for example)
- Heavy logging and alerting support recommended, on user login, on transplant (with list of commits hashes), on failures at least.
- Ensuring thorough internal code review recommended due to the MAXIMUM risk impact noted in PR/Image access control. If you need help with the review of the code of the webapp itself/running webapp, you can try to needinfo or ping :yboily

Finally, please reopen as necessary with NEEDINFO! :)

Comment 9

[Ed Morley [:emorley]]

(In reply to Guillaume Destuynder [:kang] from comment #8)
> - Notice of successful transplants to sheriffs recommended (mail notifiction
> for example)

Whenever a commit is made, firebot already comments in #developers with the commit message and author.

Other than that, we don't currently notify any of:
1) the sheriffs
2) the pusher
3) the person listed as commit author
4) owners/peers of the code being changed

So I'm not sure why we need email notifications for the transplant tool specifically? Either we need mail notifications in general (to warn users of breach of access) or we don't need them at all.

Now I guess you could say "but the transplant tool means someone just needs to compromise an LDAP password/the LDAP authentication system, rather than an SSH key" - however this is already the case - someone can use https://login.mozilla.com/ to add another SSH key after compromising an LDAP password.

Also, given that the majority of uplifts performed at the moment are done via checkin-needed - and that the sheriffs take an r+ at face value (ie: not even checking whether that person is a peer of the component in question) - either a breach of Bugzilla password (on the part of a patch author/reviewer), or even a cleverly named bugzilla email that is similar to an actual reviewer, could result in malicious code being checked into the tree by a sheriff inadvertently with the current system. As such, the transplant tool would actually likely be at lower risk, since it could check users are peers of the code in question.

Even if email notifications are wanted for only the transplant tool (and not normal commits), I would suggest that we should be notifying someone other than the sheriffs - eg the person whose LDAP credentials were used, or the owner/peer of the code in question, or the person listed as patch author, or a comment in the bug concerned (this is already part of the design).

That said, the above is in reference to email notifications specifically - having an audit log somewhere (even just so sheriffs can keep track of what changes are in the queued/have been handled) would be useful regardless.

Comment 10

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

Note that members of the releng teams can enable MFA on login.mozilla.com as well - it's still in "pilot" mode but does enforce 2FA on login.mozilla.com itself once registered.

The email notification is a suggestion, from my understanding of your comment, you have the notification implemented via IRC bot instead even thus its not directly in the transplant tool. Also - notifying the peers/owners sounds good to me. Again, it doesn't have to be email - you likely know best which kind is most likely to be seen/read and not just ignored as you have a deeper understanding of the tool.

RRA's are 30min and expose the main risks, generally triggering these discussions. Given the MAXIMUM and HIGH risks that were found during the RRA I would highly recommend ensuring these are mitigated in some way (not necessarily the ones listed as recommendation, if there is are solutions that are already in place and we were unaware of, or if there are more convenient or safer solutions of course)