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

Feat: hash pin github workflow dependencies #4058

Open
2 tasks done
joycebrum opened this issue Jun 16, 2023 · 3 comments · May be fixed by #4214
Open
2 tasks done

Feat: hash pin github workflow dependencies #4058

joycebrum opened this issue Jun 16, 2023 · 3 comments · May be fixed by #4214

Comments

@joycebrum
Copy link
Contributor

joycebrum commented Jun 16, 2023

Description

Hi again, I'd like to suggest another security practice recommended by the OpenSSF Scorecard which is to hash pin dependencies to prevent dependency-confusion, typosquatting and tag renaming attacks. Besides this is currently the only way to make your CI run as an immutable release.

The change would only be applied to GitHub workflows, dockerfiles and shell scripts dependencies.

This means:

  • Hash pinning GitHub Workflow actions.
  • Using --require-hashes on pip installs.

I can submit one PR for each type of change above to be easier to review if you prefer. Just let me know if that's the case.

Also it might be important to notice that the dependabot, that seems to be already enabled, is able to update both the hash and the comment version related to it.

Let me know if you are open to evaluate those changes and I'll submit the PR(s) ASAP.

Any questions or concerns just let me know.
Thanks!

Additional Context

A tag renaming attack is a type of attack whereby an attacker:

  • Hijack an action.
  • Upload a malicious version.
  • Replace existing tags with malicious versions.

Both Dependency Confusion and Typosquatting attacks are more applicable to package managers (such as pip, npm, choco, etc)

A dependency-confusion attack occurs when an attacker:

  • Find the name of a package that the victims wants to install
  • Create an identically named package and publish it under the public or default registry.
  • Assign the package with a higher version number to trick the package manager tool to download it from the public repo.

A typosquatting attack is a type of attack whereby an attacker:

  • Create a malicious package
  • Publish it with a similar name of a known package (example: numpi instead of numpy)

Reproduction steps

None

Expected vs. actual results

Actions

Current

actions/checkout@v3

Expected

actions/checkout@c85c95e3d7251135ab7dc9ce3241c5835cc595a9 # v3.5.3

Pip installs

Current

run: python -m pip install reuse

Expected

run: python -m pip install --require-hashes -r ci-deps.txt

Minimal code example

None

Error messages

None - not a bug

Compiler and operating system

None

Library version

None

Validation

@joycebrum
Copy link
Contributor Author

Hi @nlohmann have you considered this issue? I can submit a PR if you'd like (it may be easier to understand the suggestion seeing it in practice). Just let me know. Otherwise I can close it as not planned.

@nlohmann
Copy link
Owner

Sorry for not getting back on this earlier. I understand the issue, but given that this is a hobby project that is current struggling against deprecating services and little time, this had no priority to me. My issue right now is that I always have to choose between doing things myself (adding complexity) or using external services (adding dependencies). Fixing this issue would be nice, but it feels it falls to the complexity part.

@joycebrum
Copy link
Contributor Author

Hi, no problems, sorry to hear that. I totally understand your point, and hash pining in fact would require an "extra work" which would be reviewing dependabot PRs for keeping the dependencies up to date on reliable versions. Although we can try to mitigate the work related to it by configuring dependabot to run monthly and by configuring it to "group" dependency updates, it is yet an "extra" task that you wouldn't have to perform as it is now.

Since this feature is really simple to implement, I'll be submiting the PR for you to be able to evaluate better the impact it would have on the maintenance complexity, but feel free to reject it if you consider this increase not worth it.

It is worth mentioning too that only the pull-request-labeler and codeql actions have write permissions (and codeql permission is not one of the most dangerous ones either), which means that, as it is, all the other workflows are "safe enough" by now and no harm could be done with them even though they got compromised.

@joycebrum joycebrum linked a pull request Nov 24, 2023 that will close this issue
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants