✨ Pinned-Dependencies: no longer complain about CIFuzz

[evverx]

CIFuzz can't be pinned properly and it isn't clear how it should be
versioned to let Dependabot update it automatically. Projects using it
shouldn't be penalized.

Related:
#1305
google/oss-fuzz#6836

[laurentsimon]

why can CIFuzz action not be pinned? They don't version or release it?

cc @oliverchang @jonathanmetzman

[jonathanmetzman]

why can CIFuzz action not be pinned? They don't version or release it?

cc @oliverchang @jonathanmetzman

In order to build projects, CIFuzz uses parts of the OSS-Fuzz repo that can be updated.
E.g. let's say I make a change to my project's repo that breaks OSS-Fuzz and then I submit a fix to the OSS-Fuzz repo.
CIFuzz can't use the fix if it is pinned to master.
I also think the nature of CIFuzz/ClusterFuzzLite makes using master a better choice.
They are far more complex than most actions and are incrementally developed as a result.

[laurentsimon]

Understood. What's the risk of using the action on push? Can project X's fuzzing integration affect a project Y's fuzzing targets?

Does the action require write permissions to store fuzzing results?

[jonathanmetzman]

Understood. What's the risk of using the action on push?

Push to the project?
I think I misunderstand, because:

1. I don't see how that would help
2. It's worse as it doesn't let devs iteratively fix bugs they introduced in a PR.

Can project X's fuzzing integration affect a project Y's fuzzing targets?

No.

Does the action require write permissions to store fuzzing results?

In CIFuzz no(t for now). In ClusterFuzzLite: yes.

[laurentsimon]

Understood. What's the risk of using the action on push?

Push to the project? I think I misunderstand, because:

Sorry, I meant on push trigger in a workflow.
On pull requests, permissions are read-only by default so it's not a problem.

1. I don't see how that would help
2. It's worse as it doesn't let devs iteratively fix bugs they introduced in a PR.

Can project X's fuzzing integration affect a project Y's fuzzing targets?

No.

great. Is the reason the action cannot be pinned because the fuzz targets are defined on the OSS-Fuzz repo in https://github.com/google/oss-fuzz/tree/master/projects?

Does the action require write permissions to store fuzzing results?

In CIFuzz no(t for now). In ClusterFuzzLite: yes.

Great. I'm curious about ClusterFuzzLite: requires contents:write or something less dangerous like security-events:write?

[jonathanmetzman]

Understood. What's the risk of using the action on push?

Push to the project? I think I misunderstand, because:

Sorry, I meant on push trigger in a workflow. On pull requests, permissions are read-only by default so it's not a problem.

1. I don't see how that would help
2. It's worse as it doesn't let devs iteratively fix bugs they introduced in a PR.

Can project X's fuzzing integration affect a project Y's fuzzing targets?

No.

great. Is the reason the action cannot be pinned because the fuzz targets are defined on the OSS-Fuzz repo in https://github.com/google/oss-fuzz/tree/master/projects?

Exactly

Does the action require write permissions to store fuzzing results?

In CIFuzz no(t for now). In ClusterFuzzLite: yes.

Great. I'm curious about ClusterFuzzLite: requires contents:write or something less dangerous like security-events:write?

So actually, I may have lied. ClusterFuzzLite uses github actions artifacts and writes to them. I don't think it requires specific permissions to do this though.
In fact, I think it only needs read-all. See the docs: https://google.github.io/clusterfuzzlite/running-clusterfuzzlite/github-actions/

[laurentsimon]

Understood. What's the risk of using the action on push?

Push to the project? I think I misunderstand, because:

Sorry, I meant on push trigger in a workflow. On pull requests, permissions are read-only by default so it's not a problem.

1. I don't see how that would help
2. It's worse as it doesn't let devs iteratively fix bugs they introduced in a PR.

Can project X's fuzzing integration affect a project Y's fuzzing targets?

No.

great. Is the reason the action cannot be pinned because the fuzz targets are defined on the OSS-Fuzz repo in https://github.com/google/oss-fuzz/tree/master/projects?

Exactly

would a proper fix be to have the list of projects in a separate google/oss-fuzz-integrations. Would that create problems?

Does the action require write permissions to store fuzzing results?

In CIFuzz no(t for now). In ClusterFuzzLite: yes.

Great. I'm curious about ClusterFuzzLite: requires contents:write or something less dangerous like security-events:write?

So actually, I may have lied. ClusterFuzzLite uses github actions artifacts and writes to them. I don't think it requires specific permissions to do this though. In fact, I think it only needs read-all. See the docs: https://google.github.io/clusterfuzzlite/running-clusterfuzzlite/github-actions/

+1

[evverx]

Does the action require write permissions to store fuzzing results?

FWIW CIFuzz seems to work just fine with just

permissions:
  contents: read

and the "global" setting set to "restricted".

[jonathanmetzman]

Understood. What's the risk of using the action on push?

Push to the project? I think I misunderstand, because:

Sorry, I meant on push trigger in a workflow. On pull requests, permissions are read-only by default so it's not a problem.

1. I don't see how that would help
2. It's worse as it doesn't let devs iteratively fix bugs they introduced in a PR.

Can project X's fuzzing integration affect a project Y's fuzzing targets?

No.

great. Is the reason the action cannot be pinned because the fuzz targets are defined on the OSS-Fuzz repo in https://github.com/google/oss-fuzz/tree/master/projects?

Exactly

would a proper fix be to have the list of projects in a separate google/oss-fuzz-integrations. Would that create problems?

I think the better alternative would be to move CIFuzz to another repo so that could be pinned and the OSS-Fuzz repo could be grabbed from HEAD.
But I don't think this contributes to security in any way since we will still be grabbing some unpinned code from another repo and executing it. It also makes runs non-reproducible (which I'm still unconvinced is useful for github actions) since we still are pulling code from another repo.
The only way to really support pinning perfectly would be if the OSS-Fuzz projects kept their dockerfiles and build.sh files in their own repos. But we have no plans to do this and doing so would have some significant drawbacks for us.

Does the action require write permissions to store fuzzing results?

In CIFuzz no(t for now). In ClusterFuzzLite: yes.

Great. I'm curious about ClusterFuzzLite: requires contents:write or something less dangerous like security-events:write?

So actually, I may have lied. ClusterFuzzLite uses github actions artifacts and writes to them. I don't think it requires specific permissions to do this though. In fact, I think it only needs read-all. See the docs: https://google.github.io/clusterfuzzlite/running-clusterfuzzlite/github-actions/

+1

[oliverchang]

So actually, I may have lied. ClusterFuzzLite uses github actions artifacts and writes to them. I don't think it requires specific permissions to do this though. In fact, I think it only needs read-all. See the docs: https://google.github.io/clusterfuzzlite/running-clusterfuzzlite/github-actions/

Yep, artifacts isn't affected by token permissions, so we only need read.

@laurentsimon do you have any other concerns about excluding CIFuzz and CFLite from this check? (This PR would also need to get updated with CFLite as well).

[evverx]

This PR would also need to get updated with CFLite as well

Done

The only way to really support pinning perfectly would be if the OSS-Fuzz projects kept their dockerfiles and build.sh files in their own repos

The systemd project does that :-)

[jonathanmetzman]

This PR would also need to get updated with CFLite as well

Done

The only way to really support pinning perfectly would be if the OSS-Fuzz projects kept their dockerfiles and build.sh files in their own repos

The systemd project does that :-)

Right, but the API for OSS-Fuzz forces you to have wrappers for those in the OSS-Fuzz repo, and those in theory can be modified. For systemd youre welcome to be disciplinned and pin since you know you wont modify them, but other projects certainly will.

[naveensrinivasan]

Thanks

[github-actions]

Integration tests success for
[97dec88]
(https://github.com/ossf/scorecard/actions/runs/1497712896)

[laurentsimon]

So actually, I may have lied. ClusterFuzzLite uses github actions artifacts and writes to them. I don't think it requires specific permissions to do this though. In fact, I think it only needs read-all. See the docs: https://google.github.io/clusterfuzzlite/running-clusterfuzzlite/github-actions/

Yep, artifacts isn't affected by token permissions, so we only need read.

@laurentsimon do you have any other concerns about excluding CIFuzz and CFLite from this check? (This PR would also need to get updated with CFLite as well).

I have mostly 2 concerns:

1. scorecard should flag risks, e.g. when dependencies are not pinned. I understand CIFuzz currently cannot be pinned, but it's still considered a risk that it is not pinned. Users os scorecard should be notified that the dependency is not pinned.
2. Once we create exceptions, others will ask for exceptions as well

[naveensrinivasan]

I have mostly 2 concerns:

1. scorecard should flag risks, e.g. when dependencies are not pinned. I understand CIFuzz currently cannot be pinned, but it's still considered a risk that it is not pinned. Users os scorecard should be notified that the dependency is not pinned.
2. Once we create exceptions, others will ask for exceptions as well

Valid points 👍

[laurentsimon]

As part of our pubic meeting today ,we decided to change the score calculation to mitigate this problem. Instead of giving a score of 0 if a non-pinned action is found, we'll remove 1 point for each.

We considered using a percentage for score (score = X if X% of actions are pinned). However this was deemed problematic for the following reason: if a repo has 100 actions and 80% are pinned, score would be 8. If a repo has 10 actions and 80% are pinned, score would be 8 as well. However, the risk for the former repo is higher since it effectively has 20 unpinned actions, whereas the latter only has 2 unpinned actions.

Please feel free to comment on this change.

[evverx]

I think I agree to some extent but it still seems a bit weird to me that projects using CIFuzz get lower scores than projects with no GHActions whatsoever. I think I complained somewhere that a project with no tests, no GHActions and so on got almost the same score as systemd :-) Anyway, I think the PR can be closed.

[laurentsimon]

by same score do you mean same aggregated score?

[evverx]

Yes, I meant the aggregated score for the most part but some individual checks like Pinned-Dependencies and Token-Permissions rated projects with no GHActions higher as well. Instead of -1 or 0 they got 10. I don't know if anything has changed since then though.

More generally, I'm not sure what those scores are supposed to mean. If they are supposed to be used to "improve the security posture of open source projects" I don't think that complaining about CIFuzz is in line with that considering that the only way to address that is to remove CIFuzz :-)

[laurentsimon]

Yes, I meant the aggregated score for the most part but some individual checks like Pinned-Dependencies and Token-Permissions rated projects with no GHActions higher as well. Instead of -1 or 0 they got 10. I don't know if anything has changed since then though.

I see what you mean. Aggregated scores are something we added because some people wanted a single number. But they are not meaningful in themselves. We want to have more meaningful levels with tiered checks like branch protection https://github.com/ossf/scorecard/blob/main/docs/checks.md#branch-protection, and later have more structured output with levels/policies based on individual settings. We hope this will fix the problem.
-1 should not happen unless there is an error: do you remember when it occurred? Can you also share the case where you got a 0? 0 suggests the check is too binary and we need to improv the scoring. Please share which checks it was so we can start improving it.

More generally, I'm not sure what those scores are supposed to mean. If they are supposed to be used to "improve the security posture of open source projects" I don't think that complaining about CIFuzz is in line with that considering that the only way to address that is to remove CIFuzz :-)

Or improve CIFuzz :-)

[evverx]

Aggregated scores are something we added because some people wanted a single number. But they are not meaningful in themselves

I agree they aren't but I'm pretty sure they will be used to compare projects. I'm not sure it's mentioned anywhere that it doesn't make much sense (in its current form at least)

We want to have more meaningful levels with tiered checks like branch protection https://github.com/ossf/scorecard/blob/main/docs/checks.md#branch-protection, and later have more structured output with levels/policies based on individual settings

Given that some checks are intertwined I'm not sure even tiered checks can always help. For example, unpinned actions like CIFuzz can in theory pwn repositories if they have write access but if they don't I think in practice it doesn't matter whether they are pinned or not. In this particular case the results of both checks should probably be combined somehow.

-1 should not happen unless there is an error: do you remember when it occurred?

CI-Tests comes to mind but that should be fixed in #1335. As far as I can remember the other checks that bothered me have already been fixed one way or another.

Can you also share the case where you got a 0?

That would be CI-Tests as well. With #1335 applied the bcc projects gets 0 because there are no tests there according to the check but with #1293 (comment) it should increase to 8

[laurentsimon]

Aggregated scores are something we added because some people wanted a single number. But they are not meaningful in themselves

I agree they aren't but I'm pretty sure they will be used to compare projects. I'm not sure it's mentioned anywhere that it doesn't make much sense (in its current form at least)

We want to have more meaningful levels with tiered checks like branch protection https://github.com/ossf/scorecard/blob/main/docs/checks.md#branch-protection, and later have more structured output with levels/policies based on individual settings

Given that some checks are intertwined I'm not sure even tiered checks can always help. For example, unpinned actions like CIFuzz can in theory pwn repositories if they have write access but if they don't I think in practice it doesn't matter whether they are pinned or not. In this particular case the results of both checks should probably be combined somehow.

thanks for the feedback. I think this is in line with what we're trying to do with the Dangerous workflow check: it will absorb the pinned dependencies (for GitHub workflows) and the token permissions in the long-term. This way we can take more signals into account.
Similarly, most of the sub-checks in Pinned-Dependencies will be moved to their own check: for package managers, we plan to create a Package Manager check where we can look for declaration of default registry (dependency confusion), pinned package dependencies (curl | bash, npm install, etc).
Dockerfile pining will probably move into its own Dockerfile Security Something as well once we have more recommendations for them.

Does the above align with your thinking? Which other checks do you think should be re-arranged?

cc @asraa who works no the dangerous workflow check.

-1 should not happen unless there is an error: do you remember when it occurred?

CI-Tests comes to mind but that should be fixed in #1335. As far as I can remember the other checks that bothered me have already been fixed one way or another.

Can you also share the case where you got a 0?

That would be CI-Tests as well. With #1335 applied the bcc projects gets 0 because there are no tests there according to the check but with #1293 (comment) it should increase to 8

[evverx]

For example, unpinned actions like CIFuzz can in theory pwn repositories if they have write access but if they don't I think in practice it doesn't matter whether they are pinned or not.

On second thought, even though they can't use GITHUB_TOKEN to pwn repositories they still can steal other credentials that in turn can be used to pwn repositories indirectly in mysterious ways :-)

Does the above align with your thinking?

At first glance it does

Which other checks do you think should be re-arranged?

I'm not sure. I haven't though carefully about it yet

[evverx]

@jonathanmetzman looking at google/oss-fuzz#6957 I wonder if CIFuzz can be bumped when clang rolls. As far as I know the CodeQL action for example gets updated every time the "default CodeQL bundle" gets updated: systemd/systemd#21504. It's probably safe to assume that every clang roll is a new version in a way

[evverx]

On second thought, for that to work all those Docker images with different clang versions would have to be kept somewhere. it's probably not worth it

[jonathanmetzman]

More generally, I'm not sure what those scores are supposed to mean. If they are supposed to be used to "improve the security posture of open source projects" I don't think that complaining about CIFuzz is in line with that considering that the only way to address that is to remove CIFuzz :-)

Or improve CIFuzz :-)

This is due to an OSS-Fuzz bug that I can't fix at this point.

+10000 to what evgeny said here. Discouraging CIFuzz use does the opposite of what scorecards is trying to achieve.