nlohmann / json

JSON for Modern C++
https://json.nlohmann.me
MIT License
41.46k stars 6.59k forks source link

Feat: hash pin github workflow dependencies #4058

Open joycebrum opened 1 year ago

joycebrum commented 1 year ago

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:

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:

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:

A [typosquatting attack][typosquatting] 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 commented 7 months ago

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 commented 7 months ago

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 commented 7 months ago

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.