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

x/vuln: add support for silencing vulnerability findings with govulncheck #61211

Open
julieqiu opened this issue Jul 6, 2023 · 9 comments
Open
Assignees
Labels
NeedsFix The path to resolution is known, but the work has not been done. vulncheck or vulndb Issues for the x/vuln or x/vulndb repo

Comments

@julieqiu
Copy link
Member

julieqiu commented Jul 6, 2023

This is a tracking issue to add support for silencing vulnerability findings with govulncheck.

#59507 is related.

@gopherbot gopherbot added the vulncheck or vulndb Issues for the x/vuln or x/vulndb repo label Jul 6, 2023
@gopherbot gopherbot modified the milestones: Unreleased, vuln/unplanned Jul 6, 2023
@joedian joedian added the NeedsFix The path to resolution is known, but the work has not been done. label Jul 7, 2023
rootulp added a commit to rootulp/celestia-app that referenced this issue Jun 4, 2024
govulncheck does not support ignoring a particular vulnerability. Since
we're on ibc-go v6.2.x which has a vulnerability, CI will report a red X
on all future PRs because govulncheck fails.

This PR removes govulncheck. We can re-enable it when govulncheck adds
support for ignoring a particular vulnerability. See:
1. golang/go#59507
1. golang/go#61211
@rikatz
Copy link

rikatz commented Jun 28, 2024

@julieqiu is there someone working on this feature? Otherwise, I can try working on something to support this exclude flag :) Not sure I am the best person to, but happy to help!

@zpavlinovic zpavlinovic self-assigned this Jun 28, 2024
@zpavlinovic
Copy link
Contributor

Although we don't currently have a plan of adding support for it, we thought if we add it then we'll use OpenVex format. This is also a recently added format for the output of govulncheck. Would OpenVex format work for what you are trying to suppress?

@rikatz
Copy link

rikatz commented Jun 28, 2024

not sure, tbh I don't know almost anything on OpenVex.

My use case is simpler, this false positive started to appear and I need to ignore it on my pipelines while this is not fixed on DBs :)

So is more like a "skip CVE" (a la go test -skip)

@tianon
Copy link
Contributor

tianon commented Jun 28, 2024

OpenVEX is workable here, but kind of annoying for the same reasons the current output format struggles with: defining an appropriate purl is hard (I suppose worst case we could use a bogus placeholder, like govulncheck itself is doing)

If we were to provide an input VEX, what would the VEX output look like? Would it just copy entries from the input verbatim for the output where relevant?

@zpavlinovic
Copy link
Contributor

So is more like a "skip CVE" (a la go test -skip)

Then OpenVex is fine here.

OpenVEX is workable here, but kind of annoying for the same reasons the current output format struggles with: defining an appropriate purl is hard (I suppose worst case we could use a bogus placeholder, like govulncheck itself is doing)

We are in the process of figuring out the product fields for OpenVex. That would need to be done first.

If we were to provide an input VEX, what would the VEX output look like? Would it just copy entries from the input verbatim for the output where relevant?

From the top of my head, we would take the <OSV, product> pairs that are classified as not_affected and skip analyzing for those. Everything else would be the same I guess. I would have to think more.

@PushkarJ
Copy link

@zpavlinovic Suggestion on product fields:

  • It might make sense to default it to module name in go.mod file e.g. https://github.com/kubernetes/kubernetes/blob/master/go.mod#L7
  • It would also be super-useful to allow users to define a comma-listed list of products. For example, in case of kubernetes, all the code in core kubernetes is in k/k but we would like to add all the core k8s release images under product potentially. This is mainly because scanners that will consume openvex output will be run on release images by end users

@PushkarJ
Copy link

Btw, for anyone trying this out for their golang projects, shared a few experiments with k/k here: kubernetes/sig-security#116 (comment)

@zpavlinovic
Copy link
Contributor

@zpavlinovic Suggestion on product fields:

This seems to be in stark contrast to what has been recently proposed.

IMO, what is proposed there makes more sense.

  • It would also be super-useful to allow users to define a comma-listed list of products. For example, in case of kubernetes, all the code in core kubernetes is in k/k but we would like to add all the core k8s release images under product potentially. This is mainly because scanners that will consume openvex output will be run on release images by end users

I think it is best for users to post-process the govulncheck output themselves. Customizing the output can turn into a rabbit hole with flags and different user needs.

@puerco
Copy link

puerco commented Jul 9, 2024

Hey all I am one of the OpenVEX maintainers. Thanks for getting this discussion going!

I added some comments about the product/subcomponent values over in #68152 (comment)

The TLDR: As @PushkarJ suggests the product, when govulncheck is looking at the source, the product should be the package URL (purl) of the main module. And the dependency with the vulnerability would go in the subcomponent.

When it comes to analyzing a binary, just use a file: URI but specify the file hashes. Our tooling will prefer them when matching with data SBOMs, etc that content-addresses artifacts with hashes. This gets rid of using purls for binaries as @tianon correctly mentioned above. We prefer not using a purl in a case like this.

See the examples on the comment in #68152

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsFix The path to resolution is known, but the work has not been done. vulncheck or vulndb Issues for the x/vuln or x/vulndb repo
Projects
None yet
Development

No branches or pull requests

8 participants