rust-osdev / x86_64

Library to program x86_64 hardware.
https://docs.rs/x86_64
Apache License 2.0
797 stars 132 forks source link

Hash pin workflow dependencies #418

Closed joycebrum closed 1 year ago

joycebrum commented 1 year ago

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. In the x86_64 case we would only need changes on the GitHub workflow files.

This means:

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 one PR for each type of change I'd like to suggest (to make if easier to evaluate and review).

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:

Freax13 commented 1 year ago

I don't quite understand how this improves the project's security. What is this defending against? You mentioned typo-squatting, but surely pinning dependencies to a given hash wouldn't help if we were to mistype a dependency because we would then also likely just use the hash for said typod dependency.

Is this to prevent our dependencies from publishing malicious updates? If that's the case, I understand why we would want to pin dependencies to a specific version, though I have to admit that I never audited the original versions in the first place and very likely won't be doing so in the future, so I don't see how we would remain secure when dependabot sends us PRs to update our dependencies.

josephlr commented 1 year ago

I agree with @Freax13 here in that I don't understand the threat this prevents against.

I think the only attack this prevents is:

So it seems like we really only need to do this pining on our release workflow if we need to do it at all. The pinning only introduces additional complexity for the build workflow, without adding additional security.

For the release workflow, I think the better approach is to just not use pip at all, and just use tomllib from the python standard library. I don't think it's worthwhile to pin sources from Github's own repos (like those in the actions organization). If Github itself if compromised, we will likely have larger problems.

joycebrum commented 1 year ago

Is this to prevent our dependencies from publishing malicious updates?

Yes, it also prevents from publishing malicious updates just like a full version pinning would.

Hash pinning versions, by the other hand, is the only way to use an action as an immutable release, regardless they being third-party or github owned libraries.

What hash pinning prevents are cases regarding third-party actions on GitHub for example. It prevents a bad actor of adding a backdoor to the action's repository or even deleting the version you are refering to (v1.0.1 for example) and create a malicious version with the same tag (v1.0.1). The next time the action would run, it would use the malicious version.

I don't see how we would remain secure when dependabot sends us PRs to update our dependencies.

Dependabot or renovatebot role on supply-chain security is ensuring that you will have vulnerability fixes as soon as possible, avoiding the project to be exposed to known vulnerabilities for long times.

So it seems like we really only need to do this pining on our release workflow if we need to do it at all. The pinning only introduces additional complexity for the build workflow, without adding additional security.

Actually you are right. Adding hash pinning on build.yml, which only has read only permission, is just a preciousness. Build.yml is, by now, safe enough. Even if it got compromised no damage could be done to the project and no credentials could be compromised.

The benefit of also hash-pinning build.yml would be keeping consistency on the dependency reference method and also ensuring that, even if the permissions given to the workflow change, it would still using immutable and trustful versions at least.

About the complexity it might bring I've suggested the dependency update tools to try mitigate this complexity.

Freax13 commented 1 year ago

Is this to prevent our dependencies from publishing malicious updates?

Yes, it also prevents from publishing malicious updates just like a full version pinning would.

Hash pinning versions, by the other hand, is the only way to use an action as an immutable release, regardless they being third-party or github owned libraries.

What hash pinning prevents are cases regarding third-party actions on GitHub for example. It prevents a bad actor of adding a backdoor to the action's repository or even deleting the version you are refering to (v1.0.1 for example) and create a malicious version with the same tag (v1.0.1). The next time the action would run, it would use the malicious version.

I don't see how we would remain secure when dependabot sends us PRs to update our dependencies.

Dependabot or renovatebot role on supply-chain security is ensuring that you will have vulnerability fixes as soon as possible, avoiding the project to be exposed to known vulnerabilities for long times.

So you're arguing that we should pin GitHub actions to prevent attackers from pushing malicious updates, but also that we should use dependabot to get security updates (i.e. update to pinned hash) as soon as possible? My problem with this is that we have no way to differentiate an attacker from pushing an malicious update from the maintainer pushing security updates. Once an attacker pushes a malicious update, dependabot will send us an pr for the updated version, and we'll merge that pr thinking that we're applying a security update. What am I missing here?

joycebrum commented 1 year ago

My problem with this is that we have no way to differentiate an attacker from pushing an malicious update from the maintainer pushing security updates

I'd say that the attacker usually won't publish new versions, it will try to reach the users using the existing versions. I'd say that because publishing a new version involves release notes and signing release and so on.

Affecting an existing release would be basically making the existing tag reference their malicious fork (what unfortunately it is possible on github) instead of the official one. This could only require a compromised workflow (in the action repo) with contents write permission for example.

What you really could not be sure is if a official release is being published with a unknown security vulnerability (that the maintainers don't know about). But I'd say this case we don't much to do on.

Freax13 commented 1 year ago

so then why would we pin the actions to a specific hash?

joycebrum commented 1 year ago

so then why would we pin the actions to a specific hash?

Basically because when the attacker does what I've mentioned above you won't be affected at all because you are refering the official release.

Freax13 commented 1 year ago

Eh, I hate it, this is stupid, GitHub really needs to fix their defaults. You've convinced me that this is something we should do.

Freax13 commented 1 year ago

Thanks for bringing this up @joycebrum