[bug] Blocker release v1.2.1: verify-checkout compares SHA digests from different repos

[ianlewis]

The verify-checkout action seems to compare $GITHUB_SHA with the locally checked out version. However, $GITHUB_SHA refers to the digest of the commit that triggered the workflow, which might be from a different repo than the one we are checking out.

verify-checkout seems to have been created on the premise that one could update the tag between the time that it was pushed and when it was checked out by the workflow.

I tested a different attack. Say, there's a new tag pushed by maintainers. The checkout Action seems to fetch and do git checkout --progress --force refs/tags/<tag> - as per the logs. So one idea is to force push a different tag before the checkout happens, to trick the builder into fetching different code for the same tag. I was not able to make it work. It seems that the tag is "locked", it the old tag is still accessible until the workflow finishes.

I'm not sure this is actually the case since git checkout will only ever checkout the local tag reference and won't pull a new tag reference (that could be changed).

/cc @laurentsimon

[ianlewis]

#993 finishes changes made to add the secure-checkout action. I still need to update the pre-submits that check that we don't use checkout.

[ianlewis]

Reopening this because secure-checkout is still broken.

[laurentsimon]

There are 2 "types" of checkout:

• project checkout: these are subject to the attacks with force pushing a tag with a different hash than reported in the Git±Hub event. The GITHUB_SHA should match for most events, except for PRs where the PR sha should match.
• builder checkout: we can harden by verifying the ref from detect-env

[ianlewis]

There are 2 "types" of checkout:

• project checkout: these are subject to the attacks with force pushing a tag with a different hash than reported in the Git±Hub event. The GITHUB_SHA should match for most events, except for PRs where the PR sha should match.

To be clear, this is a situation where the tag is being updated between the time the event was generated and the time the checkout actually happens? Right now secure-checkout checks out github.sha by default so it doesn't use the complicated logic in actions/checkout. Though, I think you're right that it's not checking out the right sha for pull requests.

I also noticed something else. Since you mentioned that actions/checkout pulls a tar file, the tar could actually be a checkout with files modified. In that case git log -format '%H' could show that the git tree is correct and report the correct sha but not that the checkout is dirty.

• builder checkout: we can harden by verifying the ref from detect-env

It seems like we'll want to build two different actions to support each type of checkout since each needs to support different things. If we build specific actions for each, we probably don't even need to support repo or ref inputs. the project checkout can just use github.repository and github.sha or github.event.pull_request.head.sha. For builder checkout we can always use the repo and ref returned from detect-env.

However, that would mean we can't use checkout-go in a generic way to check out both the builder and the current repository in the go builder.

[ianlewis]

#1075 should probably fix this.

[laurentsimon]

Let's keep this issue open to keep track of hardening enhancements to checkout:

• dirty tree
• tree verification

[ianlewis]

Let's keep this issue open to keep track of hardening enhancements to checkout:

• dirty tree
• tree verification

I think those can be covered as part of #626 no?

[laurentsimon]

Good call. I forgot about #626. I added a comment in #626. Closing this issue then.

[github-actions]

This issue was reopened by the todo-issue-reopener action in the "TODO Issue Reopener" GitHub Actions workflow because there are TODOs referencing this issue:

1. .github/actions/secure-builder-checkout/action.yaml:36: verify the hash is on the main branch