Chain of Trust: Restrict git/hg branches

[JohanLorenzo]

Today, @rpappalax came to me with an interesting case: he has some big changes to make on the focus-android taskgraph and he's willing to use a github branch (living the focus-android repo). This branch needs some taskcluster scopes.
He asked @imbstack to expand the following role: https://tools.taskcluster.net/auth/roles/repo%3Agithub.com%2Fmozilla-mobile%2Ffocus-android%3Abranch%3A*. Without too much context, all the scopes needed to sign and publish a Focus build ended up in the role (I actually wonder if these scopes were on the master role).

Immediate fix

I updated to role to strip all the signing/publishing scopes out. As of now, no one can publish Focus from a random branch on the main repo.

What scriptworker can do

In hg.mozilla.org, this is safeguarded by cot_restricted_scopes. Sadly, scriptworker doesn't allow to restrict branches within an existing repo. This case has never popped up on hg.mozilla.org, but we now have a case on github.

To me, scriptworker can enforce that. scriptworker knows what's the commit under checks. We can let it know what's the branch it's on. scriptworker will then verify against:

• github.com by fetching and parsing https://github.com/:user/:repo/branch_commits/:commit (for instance: https://github.com/mozilla-mobile/focus-android/branch_commits/1d0f51cd43f45a25b34191a5276d3f5d98023899). I couldn't find an API endpoint that exposes this piece of data.
• hg.mozilla.org withhttps://hg.mozilla.org/releases/mozilla-release/json-pushes?full=1&changeset=tip

This way scriptworker knows the branch against which the graph was triggered, and it knows the branch isn't a lie. Therefore, scriptworker can restrict by git/hg branches.

@escapewindow @tomprince, how does this look to you?

[imbstack]

The new role is just a copy-paste of the old role that was restricted to the master branch. I was suspicious that these scopes were not appropriate and did ask a good bit about whether or not those scopes were appropriate for every branch. rpappa/colee decided that it was fine. We can restrict the role to the master branch again if these scopes are not appropriate for every branch.

It would be quite easy to just have the signing scopes available to master and all other scopes available on every branch if that's what is desired?

I don't think you'll need to change scriptworker to enable this.

[escapewindow]

I agree we need to add branch restriction support in cot verification; a misconfiguration or malicious user with enough scopes could bypass master branch rules by using a separate branch until we have these checks. I also agree that we can solve the immediate problem by fixing the github repo’s configuration, and add cot restrictions soon. I don’t love parsing the text for github, but we might go with what works and refine when there’s a better solution (that’s pretty much how cot verification went from v1 to v3 or 4 today).

[JohanLorenzo]

Thank you for the context @imbstack. I'm sorry my first comment didn't reflect the full story. I agree it's easy to restrict the scopes on all branches but master. However, like Aki said, a malicious user may be able to add them back and ship a new release. I'm in favor of having these scopes safeguarded by CoT.

I wish there was a better way than parsing Github's html. I'll keep an eye on the API to see if the feature gets added.

[djmitche]

If that malicious user also has permission to push to non-master branches of the repository, there's been a serious break-down in access control and it's likely the malicious actor also has control of scriptworker.

Also, in this case the desire to allow non-master branches was very strong, and surely if scriptworker had already been configured to disallow it, that configuration would have been changed along with the roles.

I appreciate the merit of a belt-and-suspenders approach, but in this case the suspenders seem to be made of off-brand "duck tape" and will probably be more than a little fragile. Is it worth the ongoing investment of time to debug that?

[JohanLorenzo]

Thank you for your input, @djmitche !

If that malicious user also has permission to push to non-master branches of the repository, there's been a serious break-down in access control and it's likely the malicious actor also has control of scriptworker.

I'm sorry, I may be missing something. What scenario would you see?

From what I see, if someone gets write access on a Github repo, he/she doesn't necessarily get access to the scriptworker instance. He/she would have to access the machines either by connecting onto it or exploiting a security hole. What do you think?

[djmitche]

The hypothetical scenario was that Mallory got access to the Github repo and got access to edit the roles for that repo. Having compromised two systems, it's not unlikely they've compromised a third. The most likely situations where this would occur are a rogue employee or compromise of an employee's credentials.

I don't have a dog in this race -- it's no problem to me if scriptworker grows this feature. I'm just thinking of balancing risk against the effectiveness of the proposed solution and against the difficulty of implementing it and maintaining it. If it didn't involve scraping HTML that balance would shift a little :)

[escapewindow]

Aiui, Hal’s github rules put an emphasis on locking down the master branch and don’t necessarily have rules for locking down other branches. A repo with a locked down master branch but weaker rules on other branches may allow for pushes to those other branches without review, allowing for a single actor to compromise the whole stack. This may not be our highest priority fix, but I think it’s important to address.

[JohanLorenzo]

The hypothetical scenario was that Mallory got access to the Github repo and got access to edit the roles for that repo

I see! I agree the branch restriction has low value, in this scenario. That said, I think there are 2 more scenarios it can protect:

a. Mallory develops the product itself. She has commit access on the Github repo but nothing else. She turns rogue. She asks someone who's not too knowledgeable about the scopes of her target project and then, she ships something from a "testing" branch.
b. The roles aren't correctly configured, Mallory has commit access and is willing to test some changes on a testing branch. Due to a bug in .taskcluster.yml, she ends up shipping an APK from her testing branch.

Aiui, Hal’s github rules put an emphasis on locking down the master branch and don’t necessarily have rules for locking down other branches

That's my understanding too :)

[escapewindow]

@JohanLorenzo Aiui, we added git branch support in CoT recently, for Fenix releases? Is this issue resolved?

[JohanLorenzo]

Sorry for the delay. I don't remember git branches to be added recently. Would you have link to it @escapewindow?

[escapewindow]

Hm, I think @Callek added some commits somewhere to allow for non-master-branch releases for Fenix, but I'm not finding them. I suppose that's different from restricting scopes to specific non-master branches. If that's the case, let's leave this open.