Open joycebrum opened 1 year ago
Scorecards add new checks which based on findings and trends. If the team is on top of the security updates and would find scorecard as redundant then they have a choice to make.
Convincing everyone is hard.
On a somewhat related note I wonder why scorecard
keeps promoting questionable services: https://github.com/systemd/systemd/pull/25205#issuecomment-1321103480? Generally I don't care about what scorecard
promotes but projects showing badges happen to promote that sort of stuff transitively as well. I wonder if links to that can be removed from https://api.securityscorecards.dev/projects/github.com/systemd/systemd and the security tab? Or should I just revert scorecard
to fix that?
Looking at the action a bit closer it appears it still can't handle PRs so issues like https://github.com/systemd/systemd/pull/23693#discussion_r904911716 can be caught only after PRs are merged (which isn't as useful as it can be). It's probably worth mentioning that in that particular case scorecard
could probably have helped to catch the unpinned action and the missing permissions but it would have missed the typo (because typos and typosquatting in general isn't covered at all) and wouldn't have helped to audit the external actions in any way. Other than that https://github.com/ossf/scorecard-action/issues/176 has never been addressed.
I have to admit I have no idea how the action in its current form ended up in the systemd repository. (I was on vacation and probably missed PRs where it was introduced). I'll take a closer look.
Anyway I really hope that links to bogus bolt-on "security" stuff can be removed from https://api.securityscorecards.dev/projects/github.com/systemd/systemd and the security tab.
It appears those links are all over the place partly because due to I assume some scorecard
bug the "Token-Permissions" check is somehow special and scorecard
insists on clicking those links even when it doesn't make sense:
{
"name": "Token-Permissions",
"score": 9,
"reason": "non read-only tokens detected in GitHub workflows",
"details": [
"Info: topLevel 'contents' permission set to 'read': .github/workflows/build_test.yml:16: update your workflow using https://app.stepsecurity.io/secureworkflow/systemd/systemd/build_test.yml/main?enable=permissions",
"Info: topLevel permissions set to 'read-all': .github/workflows/cflite_pr.yml:12: update your workflow using https://app.stepsecurity.io/secureworkflow/systemd/systemd/cflite_pr.yml/main?enable=permissions",
"Info: topLevel 'contents' permission set to 'read': .github/workflows/cifuzz.yml:9: update your workflow using https://app.stepsecurity.io/secureworkflow/systemd/systemd/cifuzz.yml/main?enable=permissions",
"Info: topLevel 'contents' permission set to 'read': .github/workflows/codeql.yml:24: update your workflow using https://app.stepsecurity.io/secureworkflow/systemd/systemd/codeql.yml/main?enable=permissions",
"Info: jobLevel 'actions' permission set to 'read': .github/workflows/codeql.yml:34: update your workflow using https://app.stepsecurity.io/secureworkflow/systemd/systemd/codeql.yml/main?enable=permissions",
"Info: topLevel 'contents' permission set to 'read': .github/workflows/coverity.yml:13: update your workflow using https://app.stepsecurity.io/secureworkflow/systemd/systemd/coverity.yml/main?enable=permissions",
"Info: topLevel 'contents' permission set to 'read': .github/workflows/development_freeze.yml:11: update your workflow using https://app.stepsecurity.io/secureworkflow/systemd/systemd/development_freeze.yml/main?enable=permissions",
"Info: topLevel 'contents' permission set to 'read': .github/workflows/differential-shellcheck.yml:11: update your workflow using https://app.stepsecurity.io/secureworkflow/systemd/systemd/differential-shellcheck.yml/main?enable=permissions",
"Warn: jobLevel 'security-events' permission set to 'write': .github/workflows/differential-shellcheck.yml:19: update your workflow using https://app.stepsecurity.io/secureworkflow/systemd/systemd/differential-shellcheck.yml/main?enable=permissions",
"Info: topLevel 'contents' permission set to 'read': .github/workflows/issue_labeler.yml:9: update your workflow using https://app.stepsecurity.io/secureworkflow/systemd/systemd/issue_labeler.yml/main?enable=permissions",
"Info: topLevel 'contents' permission set to 'read': .github/workflows/labeler.yml:11: update your workflow using https://app.stepsecurity.io/secureworkflow/systemd/systemd/labeler.yml/main?enable=permissions",
"Info: topLevel 'contents' permission set to 'read': .github/workflows/linter.yml:14: update your workflow using https://app.stepsecurity.io/secureworkflow/systemd/systemd/linter.yml/main?enable=permissions",
"Info: topLevel 'contents' permission set to 'read': .github/workflows/mkosi.yml:45: update your workflow using https://app.stepsecurity.io/secureworkflow/systemd/systemd/mkosi.yml/main?enable=permissions",
"Info: topLevel permissions set to 'read-all': .github/workflows/scorecards.yml:20: update your workflow using https://app.stepsecurity.io/secureworkflow/systemd/systemd/scorecards.yml/main?enable=permissions",
"Info: topLevel 'contents' permission set to 'read': .github/workflows/unit_tests.yml:13: update your workflow using https://app.stepsecurity.io/secureworkflow/systemd/systemd/unit_tests.yml/main?enable=permissions"
],
"documentation": {
"short": "Determines if the project's workflows follow the principle of least privilege.",
"url": "https://github.com/ossf/scorecard/blob/c40859202d739b31fd060ac5b30d17326cd74275/docs/checks.md#token-permissions"
}
},
I'm not sure why what should actually be debug messages pop up as info messages and why exactly top-level read permissions should be remediated by updating all the workflows. The "warn" shouldn't be there either (and as far as I understand that's what https://github.com/ossf/scorecard/issues/2338 was supposed to address).
The remaining links come from the "Pinned-Dependencies" check and flag CIFuzz and ClusterFuzzLite. They shouldn't be flagged either.
As far as I can tell if the underlying scorecard bugs were fixed those links wouldn't even pop up anywhere.
Hi @evverx thanks for reaching out, I really appreciate you sharing your concerns on the tool once you are now using it, it is a valuable feedback!
About the link you mentioned, you mean the ones in the suggestions bellow?
Just to clarify we are not suggesting (in this suggestions) or recommending the Harden Runner, the link mentioned is only about the online tool provided by the step security which you can use to automatically pin all dependencies (Pin-Dependencies Check) and also set the content permission to read only by default (Token Permission Check).
That's why the flags marked should be the first (enable=permissions) and third ones (enable=pin), probably all of them have been flagged and since the Harden Runner is a Step Security tool, it was also added to the https://github.com/systemd/systemd/pull/25205#issuecomment-1321103480 PR, but it is not related to any scorecard check.
About the Scorecard running on PRs, it currently runs on PRs but we have recently created a issue to really make the run on PR useful to maintainers https://github.com/ossf/scorecard-action/issues/1019.
About the token permissions, it seems that you are right, it doesn't make sense this json output "links" in the Token-Permission check, since there is nothing there to fix. I'll open an issue to look it closely and see if I am able to try to fix it myself to help scorecard team.
About the Pinned-Dependencies, good to see that scorecard team is already aware that it is not really a security issue not to pin the action that has read only permissions (https://github.com/ossf/scorecard/issues/2018) but this would not interfere with this links mentioned in the Token-Permissions.
At least the Infos in the Pinned Dependencies details seems right
Info: GitHub-owned GitHubActions are pinned Info: no insecure (not pinned by hash) dependency downloads
found in Dockerfiles
Info: no insecure (not pinned by hash) dependency downloads found in shell scripts
Just to clarify we are not suggesting (in this suggestions) or recommending the Harden Runner
I agree that scorecard
doesn't recommend it directly but the links provided by scorecard
lead to a page where it can be easily enabled and included into those "security" PRs (and well-meaning contributors are likely to bring it because why wouldn't they? On paper it looks like it actually makes sense plus it's somewhat indirectly endorsed by OpenSSF through scorecard
). Of course that kind of "sandbox" is totally bogus but nobody is going to look at how it works under the hood because they would probably just trust scorecard
.
FWIW those semi-automated PRs can actually be considered harmful in the case of CFLite: https://github.com/ossf/scorecard/issues/1907#issuecomment-1325287154
About the Scorecard running on PRs, it currently runs on PRs but we have recently created a issue to really make the run on PR useful to maintainers https://github.com/ossf/scorecard-action/issues/1019.
Good to know. Thanks!
About the token permissions, it seems that you are right, it doesn't make sense this json output "links" in the Token-Permission check, since there is nothing there to fix. I'll open an issue to look it closely and see if I am able to try to fix it myself to help scorecard team.
Thanks!
good to see that scorecard team is already aware that it is not really a security issue not to pin the action that has read only permissions (https://github.com/ossf/scorecard/issues/2018)
I think the scorecard team has been aware of that issue (and a bunch of other issues) for a long time. The problem is that they have never been addressed for various reasons.
At least the Infos in the Pinned Dependencies details seems right
Looks like it.
Just to clarify we are not suggesting (in this suggestions) or recommending the Harden Runner
I agree that
scorecard
doesn't recommend it directly but the links provided byscorecard
lead to a page where it can be easily enabled and included into those "security" PRs (and well-meaning contributors are likely to bring it because why wouldn't they? On paper it looks like it actually makes sense plus it's somewhat indirectly endorsed by OpenSSF throughscorecard
). Of course that kind of "sandbox" is totally bogus but nobody is going to look at how it works under the hood because they would probably just trustscorecard
.
@evverx, please feel free to create an issue or discussion at https://github.com/step-security/harden-runner if you think there are areas of improvement. I also replied to your comment on https://github.com/systemd/systemd/pull/25205#issuecomment-1321103480. The whole point of having it be open source is that developers look at how it works under the hood and help improve it.
The whole point of having it be open source is that developers look at how it works under the hood and help improve it.
From https://github.com/systemd/systemd/pull/25205#issuecomment-1321103480
Can you please clarify what you mean by “shady” in technical terms? W.r.t switch licenses on the fly, for secure-workflows it was set to AGPL in Feb 2022 and we don’t intend to change it.
It was set to AGPL after I pointed out that it was relisenced on the fly as soon as it was integrated into the scorecard documentation masquerading as an open-source project with the Apache license. I doubt it would be AGPL now if that move hadn't been noticed.
Regarding the harden runner I appreciate the details but I'm not here to audit or help to improve it. I think if anyone is interested they can take a look at the code and decide for themselves how hard it is to escape from that "sandbox" (or whether they need to escape from it at all).
The whole point of having it be open source is that developers look at how it works under the hood and help improve it.
From systemd/systemd#25205 (comment)
Can you please clarify what you mean by “shady” in technical terms? W.r.t switch licenses on the fly, for secure-workflows it was set to AGPL in Feb 2022 and we don’t intend to change it.
It was set to AGPL after I pointed out that it was relisenced on the fly as soon as it was integrated into the scorecard documentation masquerading as an open-source project with the Apache license. I doubt it would be AGPL now if that move hadn't been noticed.
I already explained earlier that I was new to open-source licenses. I looked at a few projects and decided to use the license that dependabot uses. After getting feedback, I apologized, and promptly updated the license. As I mentioned before, we haven’t changed the project license since then and don’t intend to do so in the future.
Regarding the harden runner I appreciate the details but I'm not here to audit or help to improve it. I think if anyone is interested they can take a look at the code and decide for themselves how hard it is to escape from that "sandbox" (or whether they need to escape from it at all).
That is fair. Request you to please not refer to it as bogus without a thorough audit or review. We spend a significant amount of effort building & maintaining these open-source solutions and there are many open-source contributors who provide constructive feedback. Reading a comment that the project is bogus without proper reasoning or discussion is disheartening.
Request you to please not refer to it as bogus without a thorough audit or review
Is this some sort of cease and desist? Should I also remove the comments where I said that those semi-automated PRs fix imaginary "security" issues and in the case of CFLite they are actually malicious?
Updating a case of unclear value proposition of the tool. Scorecard is not able to identify Cargo dependencies in Pinned-Dependencies check, it is disagreeable if some dependencies should be pinned or not in a library case, and Clippy tool is not detected in SAST check. This is seen as an "incorrect" analysis by maintainers and leads to a lack of trust of wheather the security recommendations are worth or not. Additionally, the securityscorecards.dev/viewer
does not provide further information on why an unpinned dependency is dangerous, or why a SAST tool is important. The page does provide links to the documentation, but without entering the documentation and studying about each check to validate if the recommendation is valid or not, it is hard to glance at the page and understand the value of the recommendations.
In some discussions, maintainers questioned what are the real benefit of having scorecard running as an action with arguments such as:
PHP
Curl
How can we improve or show the value of scorecard as an action?