Hash pin workflow dependencies
joycebrum opened this issue · 5 comments
Description
I would like to suggest another security practice recommended by the OpenSSF Scorecard which is to hash pin dependencies to prevent dependency-confusion and typosquatting attacks.
The change would only be applied to GitHub workflows, dockerfiles and shell scripts dependencies. The volatile case would only need changes on the GitHub workflow build.yml
This means:
- Hash pinning GitHub Workflow actions.
Along with hash-pinning dependencies, I also recommend adopting dependabot or renovatebot to help keep the dependencies up to date. Both tools can update hashes and associated semantic version comments.
Together with the issue I'll also suggest a PR with the changes since they are quite simple.
Any questions or concerns just let me know.
Thanks!
Additional Context
For more informations about dependency confusion / typosquatting attacks:
For more informations about the dependency-update tools:
Thanks for opening this issue and the corresponding PR #37. It's not clear to me how this change should improve security though. You mention dependency-confusion and typosquatting attacks, which don't seem related to this pinning of versions?
My understanding is that pinning dependencies only makes sense if you don't trust the publisher or if you fear breaking changes. Given that we're only using official actions from GitHub and the widely-used actions-rs/cargo action, I'm inclined to trust these publishers. So to me, it seems more important to automatically get the latest bug fixes as quick as possible (instead of delaying them until dependabot creates a PR and we have time to review it).
Please let me know if I'm misunderstanding something!
Hi @phil-opp, thanks for the return.
I'd say the main reason on hash pinning is more about stability than trustfulness. Although GitHub Actions are very trustful, they are yet open source projects and are exposed to the same risks and vulnerabilities as any other open source project.
Considering it, using hash pinning dependencies benefits would be:
- Increasing CI stability since it is currently the only way to use actions as immutable releases
- Preventing tag renaming attacks --- although the actions are from reliable sources, it is always better to be safe than sorry.
And considering that the actions will be updated by either dependabot or renovatebot, this wouldn't generate any overhead for you folks on maintaining the project.
Anyway, I do get your point on rather using version pinning. If you still rather not hash pinning, I'd say for at least accept a minor version pinning + dependabot, to not be blindly updated for any potentially dangerous release or vulnerability.
Let me know whether you are open to any of the changes and I'll be happy to submit a PR.
Thanks for your thoughts!
And considering that the actions will be updated by either dependabot or renovatebot, this wouldn't generate any overhead for you folks on maintaining the project.
Unfortunately, this is not true. We would need to manually review every single new version if we consider them untrusted. Without such reviews, there wouldn't be any security advantage.
As I understand it, there are two options:
- Consider all external actions as untrusted.
- Hash pin the dependencies to guard against malicious updates or tag renaming attacks. Use dependabot etc to automatically open PRs for new versions.
- Manually review every single update of these actions. Do this quickly to ensure that critical bug and security fixes are applied as fast as possible.
- Lots of additional work for maintainers.
- Consider the external actions as trusted.
- Always use the latest semver-compatible release to ensure that we have all the latest bug and security fixes. Don't pin any versions or hashes.
- No additional maintainer overhead.
We're currently using option 2. Switching to option 1 would require more maintainer overhead, which is going to be difficult since I'm maintaining this project alone. This also means that there is the additional risk that a critical bugfix of some action is not applied when I'm busy with other things.
Given that our current CI job runs with read-only permissions, the amount of damage that a malicious action could do should be limited, right? They could of course abuse the GitHub actions runner to mine bitcoins or do DDOS attacks on something, but they should not be able to corrupt this project.
If you still rather not hash pinning, I'd say for at least accept a minor version pinning + dependabot, to not be blindly updated for any potentially dangerous release or vulnerability.
I'm not sure how this would improve security. We would still update to new patch versions automatically, so we still need to trust the maintainers. Then I think we can trust them to follow the semver rules too.
Given that our current CI job runs with read-only permissions, the amount of damage that a malicious action could do should be limited, right?
yeah! Considering the workflow is read only, it is secure because no harm could be done even if the action automatically upgrades to malicious version.
In the future, if you need to grant any write permission for an action, you can hash pin the action specifically, this would be a third option: it would reduce the number of PRs and version upgrades you would need to review.
There are other benefits on hash pining (or even minor version pinning) related to stability, but it would still exist the extra work of reviewing dependabot PRs.
I think it is possible to configure renovatebot to colapse all PR changes in one single PR to be sent once a week or month, I can try to take a look at it if you think it is a possibility.
Otherwise, the volatile workflows are safe enough as it is, this would be just an extra precaution -- if future actions require write permissions for example. Thanks for giving it a though!
I'm closing this issue since we agreed that hash pinning the workflow which has just read only permissions would be an overkill.
I've considered whether dependabot would still be security relevant but since the risk is minimal even if a compromised release is made, dependabot won't benefit much either.
Thanks for the great discussion @phil-opp!