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

Improvement of OpenSSF Scorecard Score #15474

Open
1 of 5 tasks
harshitasao opened this issue Aug 16, 2024 · 11 comments
Open
1 of 5 tasks

Improvement of OpenSSF Scorecard Score #15474

harshitasao opened this issue Aug 16, 2024 · 11 comments
Labels
kind/feature Well-understood/specified features, ready for coding.

Comments

@harshitasao
Copy link

harshitasao commented Aug 16, 2024

Describe the feature

Is this a bug report or feature request?

  • Feature Request

What should the feature do:
Hi, I'm Harshita. I’m working with CNCF and the Google Open Source Security Team for the GSoC 2024 term. We are collaborating to enhance security practices across various CNCF projects. The goal is to improve security for all CNCF projects by both using OpenSSF Scorecards and implementing its security improvements.

After the opening of the scorecard action addition PR, I'm here to increase the final score by going over each check. I've listed all of the checks where work needs to be done, in order of its criticality. I plan to submit each PR for each fix. Please let me know what you think and for which ones a PR is welcome that I will submit it ASAP.

Current Score: 7.3

Scorecard report: https://scorecard.dev/viewer/?uri=github.com/knative/serving

Here's a few checks we can work on to improve the project's security posture [ in order of its criticality ]:

  • Token-Permissions: Score = 0

    • The issue here is that many workflows doesn’t have a top-level read-only permissions block like present in the other workflows. Scorecard is quite severe in this check: a single workflow without top-level permissions gets a 0/10 for the check.
  • Vulnerabilities: Score = 7

    • After running the osv scanner locally, a significant amount of vulnerabilities in Go dependencies were found that need to be fixed.
    • May need to add an osv-scanner.toml to mark some of these as not impacting/ignored.
    • Open vulnerabilities are easily exploited by attackers and should be fixed as soon as possible.
  • Signed-Releases: Score = 8

    • SLSA provenance needs to be added, which can be done using this, increasing the overall score to 10.
  • Fuzzing: Score = 0

    • Integrating the project with OSS-Fuzz by following the instructions here. The most difficult one on the list, maintainers help, is highly appreciated. For example, helping in identifying the components where fuzz testing will be added.
  • Pinned-Dependencies: Score = 3

    • Github actions and go commands are not pinned by hash, which is resulting in a low score. But pinning go commands would introduce the risk of running outdated versions.
    • Pinning dependencies to a specific hash rather than allowing mutable versions or ranges of versions improves supply chain security.
    • PR: fix: fixed the pinned dependencies issue #15475

/cc @joycebrum @diogoteles08 @pnacht @nate-double-u

@harshitasao harshitasao added the kind/feature Well-understood/specified features, ready for coding. label Aug 16, 2024
@harshitasao
Copy link
Author

cc @davidhadas

@davidhadas
Copy link
Contributor

Welcome @harshitasao

As for Fuzzing - see https://knative.dev/blog/events/fuzzing-audit-2023/
See also https://knative.dev/blog/events/security-audit-2023/

cc @evankanderson

@ReToCode
Copy link
Member

Hm, regarding #15475 I'm not sure I fully agree with:

Github actions and go commands are not pinned by hash, which is resulting in a low score. But pinning go commands would introduce the risk of running outdated versions.
Pinning dependencies to a specific hash rather than allowing mutable versions or ranges of versions improves supply chain security.

Currently, we get every patch of lets say v4 of an action. Sure we have do work to get to v5, but with having fixed hashes without a mechanism to patch those automatically, we would miss 4.1, 4.2 (just an example) and so on.

Opinions @dprotaso @skonto ?

@pnacht
Copy link

pnacht commented Sep 17, 2024

Hey @ReToCode. I see Knative Serving already uses Dependabot to bump Actions.

Dependabot also works with hash-pinned Actions, so it will simply send you PRs whenever an Action releases a new version. It'll even update the "version comment" next to each hash.

If you are concerned about receiving more such PRs (updating from minor versions you get "for free" by using @v4), you can modify your dependabot.yml config file to use "grouped updates", where you'll receive a single PR updating all Actions at once (example PR):

  - package-ecosystem: "github-actions"
    directory: "/"
    schedule:
      interval: "monthly"
    groups:
      github-actions:
        patterns:
          - "*"

@ReToCode
Copy link
Member

@pnacht I see. I still don't really follow the initial argumentation. Hashes are not more secure nor more up to date than versions (assuming dependabot is used to bump both). Furthermore, preventing supply chain attacks would require us to review the actual changes between versions of actions, otherwise how can we make sure a new version/hash is ok?

@cardil WDYT about the grouped updates? I think that could help to prevent even more automated, noisy PRs to all repos.

@pnacht
Copy link

pnacht commented Sep 18, 2024

Hey @ReToCode. Indeed, the security of hash-pinning + Dependabot PRs comes from the "lag" between the new version being released and the PR being sent. This is actually why I suggest using monthly updates, not weekly: this will mean that a new version will exist in the wild for ~2 weeks (on average) before you receive the PR itself. During this period, the Actions' maintainers and other users might discover problems with the new version and release other fixes, such that Knative Service won't ever be exposed to the problematic release.

I also suggest that you enable Dependabot Security Updates if you haven't already. With this enabled, Dependabot will send "out of season" PRs to immediately update dependencies with known vulnerabilities.

Therefore, hash-pinning + monthly updates you get some "bake time" before being exposed to new (and untested) versions of dependencies, and "out of season" security updates help you quickly get updated to new versions in case one is eventually found to be dangerous.

@skonto
Copy link
Contributor

skonto commented Oct 3, 2024

@cardil @dsimansk wdyth?

@dsimansk
Copy link
Contributor

dsimansk commented Oct 3, 2024

@pnacht I see. I still don't really follow the initial argumentation. Hashes are not more secure nor more up to date than versions (assuming dependabot is used to bump both). Furthermore, preventing supply chain attacks would require us to review the actual changes between versions of actions, otherwise how can we make sure a new version/hash is ok?

+1 to this argument by @ReToCode. There's double edged sword for me, with pinned hash action we can stay longer on the vulnerable version until Dependabot PR is merged, with floating version on the other hand we are exposed to 0-day vulnerabilities. I don't have any statistics handy, but my gut feeling tells me the plausibility of older version being discovered as vulnerable is still a bit higher. In addition, it's relaying on maintainers (human factor and time constraints) to promptly merge update PRs.

We don't use very exotic GH actions, it's very commonly used across the whole platform. I'd rather rely on GitHub promptly disably action runs whenever Critical CVE is in action/checkout rather than Dependabot + update PR.

@cardil WDYT about the grouped updates? I think that could help to prevent even more automated, noisy PRs to all repos.

+1 to grouped updates. At from the maitenance burden & noise of automated PRs it makes sense to me.

@skonto
Copy link
Contributor

skonto commented Oct 7, 2024

@harshitasao gentle ping.

@nate-double-u
Copy link

Hi @skonto, just a note, @harshitasao was a Google Summer of Code mentee. Since the term is over, she may not have as much availability as she did when she opened this issue.

@evankanderson
Copy link
Member

@pnacht I see. I still don't really follow the initial argumentation. Hashes are not more secure nor more up to date than versions (assuming dependabot is used to bump both). Furthermore, preventing supply chain attacks would require us to review the actual changes between versions of actions, otherwise how can we make sure a new version/hash is ok?

Beyond the configuration of Dependabot (batched, monthly updates for non-security fixes plus immediate PRs for security-related fixes), pinning actions to specific hashes prevents a few other attacks. I'll buy the argument that actions/checkout is probably not a particularly high-risk dependency given that it's provided and managed by GitHub, who could do a number of other attacks. However, we have non-GitHub actions (e.g. ko-build and chainguard-dev) where despite best intentions, it's possible that a maintainer could get compromised without immediate detection.

Such an attack would look like:

  1. Re-point the tag (e.g. v4) to a vulnerable commit. This could either be an earlier revision in the tree, or a non-reviewed commit to a development branch.
  2. Wait for (or trigger) a run of the vulnerable action.
  3. Revert the tag to the original action.

I think things are slightly mitigated because many of the actions these impact are of the pull_request type, so they don't have high permission levels on things like secrets or release artifacts, but needing to do this analysis of "when is this pattern safe" increases the odds that it will get overlooked somewhere.

I have a tool (Minder, submitted as a sandbox OpenSSF project) that could help standardize a lot of this configuration that I'd love to show off to the productivity WG on next meeting. In any case, I think the combination of Dependabot configuration + actions pinning is probably a good idea. We should try to minimize the extra review overhead costs, though.

@cardil WDYT about the grouped updates? I think that could help to prevent even more automated, noisy PRs to all repos.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Well-understood/specified features, ready for coding.
Projects
None yet
Development

No branches or pull requests

8 participants