Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hash-pin GitHub Actions to increase workflow resiliency #109110

Closed
pnacht opened this issue Sep 7, 2023 · 4 comments
Closed

Hash-pin GitHub Actions to increase workflow resiliency #109110

pnacht opened this issue Sep 7, 2023 · 4 comments
Labels
type-feature A feature request or enhancement

Comments

@pnacht
Copy link

pnacht commented Sep 7, 2023

Feature or enhancement

Proposal:

When developing with CI workflows, it's common to version-pin dependencies (i.e. actions/checkout@v3). Such major-version pins can be useful in that you get immediate access to the latest version of each Action. However, this also makes the project vulnerable should a broken or malicious release be published. However, version tags are mutable, so minor version tags aren't the best solution, since a malicious attacker could overwrite a version tag to point to a malicious or vulnerable commit instead.

Hash-pinning workflow dependencies ensures the dependency is immutable and its behavior is guaranteed.

These hashes (and comments indicating the respective version) can be kept up-to-date by dependabot, which cpython already uses. However, the current configuration (ignoring patch and minor version bumps, set in #92186) would need to be modified. My understanding of the motivation for #92186 was the flood of PRs generated by dependabot. However, this concern can be mostly eliminated by relying on dependabot's new grouped updates; dependabot can now be configured to send a single PR per month updating all Actions at once (see this example).

I'll send a PR pinning the Actions and changing dependabot's configuration along with this issue.


Hey, I'm Pedro. I work with Google and the OpenSSF to help critical projects improve their supply-chain security.

Has this already been discussed elsewhere?

This is a minor feature, which does not need previous discussion elsewhere

Links to previous discussion of this feature:

I previously suggested this change to pypa/setuptools, where I had an excellent exchange with @jaraco. I recommend taking a look at our conversation there.

The conclusion there was that the change wasn't feasible for that project. My understanding is that the main "hurdle" was the project's use of jaraco/skeleton as its template. Unfortunately, keeping the hashes updated in the skeleton and its "child" repositories would lead to frequent conflicts when merging skeleton updates into the repositories. The maintainers therefore reasonably concluded (and I couldn't really disagree) that the benefits of hash-pinning weren't worth the significant overhead they would cause in that situation.

However, I believe the situation for CPython is much more straightforward. The "overhead" here would be merging at most one dependabot PR per month.

Linked PRs

@pnacht pnacht added the type-feature A feature request or enhancement label Sep 7, 2023
@hugovk
Copy link
Member

hugovk commented Sep 8, 2023

Has this already been discussed elsewhere?

This is a minor feature, which does not need previous discussion elsewhere

Thanks for the suggestion, we discussed this recently in #103635.

The summary is that we can trust GitHub's own actions/*, seeing as we trust them for the actions infra etc (#103635 (comment)). The three references at pypa/setuptools#4025 (comment) talk of pinning third-party actions. One explicitly says apache/*, github/* and actions/* can be used without restrictions (e.g. no pinning).

Looking closer at potential threat scenarios (#103635 (comment)), we have two (now three) workflows with write permissions, with actions from actions/* and readthedocs/*, both of which are trusted. And workflows with secrets.* only use GitHub's own actions/*.


Thanks for the link to pypa/setuptools#4025, I see there are other issues beyond integrating with jaraco/skeleton.

As mentioned at pypa/setuptools#4025 (comment), the planned github/roadmap#592 sounds useful.

Also noted in pypa/setuptools#4025, pinning increases the toil for without adding a lot of value. For example, we'll be pinned to a given x.y.z patch version for up to a month; when not pinning, we use the latest x.* version available. If a given version is compromised, it could be timed to do so just before the pin is bumped for maximum exposure (pypa/setuptools#4025 (comment)). The unpinned ones would mean that a compromised action would have a shorter window of use until a fix is released.

So far we've only had three dependabot PRs this year. When pinning x.y.z patch versions, I expect it's very likely there will be an update each and every month. (Although once a month isn't too spammy, and grouped updates sounds like it'll help.)

Also, there's been a series of incidents at GitHub this week, including one that needed an upgrade to actions/checkout@v4, which would have been harder to do with pinned hashes.

@pnacht
Copy link
Author

pnacht commented Sep 8, 2023

The summary is that we can trust GitHub's own actions/*, seeing as we trust them for the actions infra etc (#103635 (comment)).

That's a reasonable position, but I'd say trusting GitHub's infrastructure isn't necessarily the same as trusting its public repositories. As far as I can tell, the actions org is just an org like any other. I wouldn't be surprised if they have some added protections on the backend (they might not!), but these are still public repositories that accept external pull requests. PRs that might introduce (accidentally or maliciously) vulnerabilities in the Actions.

The three references at pypa/setuptools#4025 (comment) talk of pinning third-party actions. One explicitly says apache/*, github/* and actions/* can be used without restrictions (e.g. no pinning).

The other references speak only of "third-party Actions", which I understand to mean "all Actions not maintained by my organization". But yes, the Apache policy allows using GitHub-managed Actions without pinning.

There is no right answer. It's a matter of how much you trust GitHub's public repositories.

Looking closer at potential threat scenarios (#103635 (comment)), we have two (now three) workflows with write permissions, with actions from actions/* and readthedocs/*, both of which are trusted. And workflows with secrets.* only use GitHub's own actions/*.

Ah, I wasn't aware that readthedocs was considered a trusted party (don't know what the relation is between them and the PSF). That was one that had caught my eye.

Also noted in pypa/setuptools#4025, pinning increases the toil for without adding a lot of value. For example, we'll be pinned to a given x.y.z patch version for up to a month; when not pinning, we use the latest x.* version available. If a given version is compromised, it could be timed to do so just before the pin is bumped for maximum exposure (pypa/setuptools#4025 (comment)). The unpinned ones would mean that a compromised action would have a shorter window of use until a fix is released.

CPython seems to have dependabot security alerts set up (see this PR bumping /Doc/requirements.txt). Should an Action be compromised, you should receive an "out-of-season" PR bumping that dependency as soon as a patched version is released. So you'd be exposed until that PR is merged, not necessarily for an entire month. I didn't bring this up when discussing jaraco/skeleton because there's no way to ensure all its "children" also have security alerts active.

Meanwhile, if the attack isn't "properly timed", you likely won't be impacted at all. And some attack vectors are "randomly timed" (i.e. merging a PR with a subtle vulnerability – accidental or malicious – that only impact users after an eventual release), so I believe this isn't entirely implausible.

Also, there's been a series of incidents at GitHub this week, including one that needed an upgrade to actions/checkout@v4, which would have been harder to do with pinned hashes.

Yeah, GitHub had an unpleasant week, for sure. Though I hadn't heard of this need to bump to v4 before. It's not mentioned in the explanations given in https://www.githubstatus.com/history, and one comment in your linked issue says they had the same problem with v4.

@AA-Turner
Copy link
Member

Bumping to v4 resolved the immediate problems, from memory, though it may have been correlation-causation.

I tend to agree with Hugo here though, and we've taken the recommendations to move to read-only jobs, etc already.

A

@pnacht
Copy link
Author

pnacht commented Sep 11, 2023

Understood. I'll close the issue and PR now.

Thank you for your time!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type-feature A feature request or enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants