x/vuln: OpenVEX report lacks affected product #68152
Comments
|
A tentative fix/improvement for this is done in golang/vuln#11. |
|
To summarize, the proposal here is to use (purls of) packages of detected vulnerabilities as product IDs. This would allow govulncheck openvex output to work with other tools, such as trivy and grype. This proposal is in contrast with what we were thinking initially and that is to have the artifact under analysis as the product. That approach, however, is difficult when dealing with, say, different package patterns of a module. The proposal here does not have these issues. It also seems openvex spec allows what is proposed here. |
|
Hey @maceonthompson 👋🏻 @zpavlinovic thanks for explanation above. FYI we were having an initial discussion about a possible implementation for this in golang/vuln#11 (comment). The code is still available in my fork https://github.com/macedogm/vuln/commits/openvex-improvement/. It's simple, but works. The biggest challenge that I saw is that govulncheck's OSV struct doesn't contain a way to track the exact affected version of the vulnerable package/path that was identified in the scanned code (see in https://github.com/golang/vuln/blob/d44b651a2e0f43966413e20b65cb619bdb621e7e/internal/vulncheck/emit.go#L21). Due to that, I added an Internal struct inside OSV to hold the needed info. Otherwise, the biggest benefit of the OpenVEX spec cannot be achieved, which is to identify the exact product (package and version) not affected, which can be removed by the downstream vulnerability scanners (Trivy, Grype etc.), although govulncheck knows about the exact version. The product, in the OpenVEX spec, can have subcomponents. My initial implementation doesn't add this. Where I work we are planning to use this as a way to remove hundreds of false positives flagged by vulnerability scanners. Initially we are still using my forked version. Happy to collaborate further if needed. |
|
Pushing things inside OSV struct is not the way to go IMO. There should be a different way of doing this, but that is an implementation detail. Your code is useful nonetheless. We'll use the issue here to decide whether to proceed with using vulnerable packages as product IDs. |
|
Hi @macedogm, Thank you for the work/time you've already spent on this issue! If the idea is to use the source of the vulnerability (i.e. the identifier of the offending dependency) in the product field, I think govulncheck’s output already contains all the information we need. I really like this proposal, but I think it goes against the VEX specification (see Section 2.5.1 and 2.5.2), which state that the Product ID should be the artifact that you’re scanning, and subcomponents are where vulnerable product IDs can be filled in. If we kept the initial idea but populated the subcomponent field instead, would this work/help your use case? |
|
Hey @maceonthompson. Thanks for the update.
You are totally right, in the implementation that I did I was listing directly the affected path/package and version. Example from my forked version: {
"vulnerability": {
"@id": "https://pkg.go.dev/vuln/GO-2021-0061",
"name": "GO-2021-0061",
"description": "Denial of service in gopkg.in/yaml.v2",
"aliases": [
"CVE-2021-4235",
"GHSA-r88r-gmrh-7j83"
]
},
"products": [
{
"@id": "pkg:golang/gopkg.in/[email protected]"
}
],
"status": "not_affected",
"justification": "vulnerable_code_not_present",
"impact_statement": "Govulncheck determined that the vulnerable code isn't called"
},
Yes, it would definitely work and be in full compliance with the spec 👍🏻 Thanks for working on this, it's much appreciated. |
|
Hello from the @openvex project 👋 Thanks for opening this issue, I have been meaning to contribute a patch to fix this but I'm glad this discussion is already underway! Let me add some context on the VEX product and subproduct. @maceonthompson's reading of the spec is correct here:
Let's imagine two scenarios, source and binary. For example, when running in source mode, to craft a statement for the source of {
"vulnerability": {
"@id": "https://pkg.go.dev/vuln/GO-2024-2947",
"name": "GO-2024-2947",
"description": "Leak of sensitive information to log files in github.com/hashicorp/go-retryablehttp",
"aliases": [
"GHSA-3633-5h82-39pq"
]
},
"products": [
{
"@id": "pkg:golang/github.com/openvex/vexctl@c7362b73a06e2f8e4285b115d98e84f0ebfb3db8",
"subcomponents": [
{
"@id": "pkg:golang/github.com/hashicorp/[email protected]"
}
]
}
],
"status": "affected",
}Now when it comes to binaries, you don't need to specify a package URL (a purl). If you have a purl for one, its fine but the product identifier field in OpenVEX takes a URI so, in the case of binaries, the best way to specify them is to use a URI with a This is the same output but with a linux binary built from the same build point as above. Note that the path really becomes irrelevant once we can content-address the binary with a hash: {
"vulnerability": {
"@id": "https://pkg.go.dev/vuln/GO-2024-2947",
"name": "GO-2024-2947",
"description": "Leak of sensitive information to log files in github.com/hashicorp/go-retryablehttp",
"aliases": [
"GHSA-3633-5h82-39pq"
]
},
"products": [
{
"@id": "file:///vexctl-linux-amd64",
"hashes": {
"sha-256": "f8d37ec5f6ee5fb1352b0162443a9c8fcd50dab5125fde288daca0533a9286b7",
"sha-512": "875079d0b61c6a03dcba46eef3a877d5be8789a068ad5f182a876ee08f754e1f2d7cbb5f22be73a77ff49d5565e13579c85e3f9f32884ec107e5e7624484f093"
},
"subcomponents": [
{
"@id": "pkg:golang/github.com/hashicorp/[email protected]"
}
]
}
],
"status": "affected",
}The important part is that the vex statement operates on the triad of a vulnerability in a dependency evaluated in the context of the product (source or an artifact including the dependency). I'm happy to contribute a patch to handle these :) |
|
Thanks for all the details. A few questions/remarks.
The above scenarios are less likely to happen when users need openvex format, but we should have the story for them nonetheless. |
For those cases, the
Is it possible to know the hash of the package (the one that'd go into
This is an excellent question. Yes, it sounds to me like a qualifier that should be appended to the package URL (eg I'd prefer adding the |
I think this is an easier case. A bigger issue is when someone analyzes something like
Since we are talking about the module being analyzed, then there won't be anything for it in the go.sum. I also think just having the unversioned purl should be fine here.
In general, I think we would likely need to get an agreement on the purl encoding of Go artifacts at the level of the language. |
|
I feel that the best step forward (for now/the immediate next release) is to implement the dependencies as subcomponents, which won't be affected by the edge cases detailed above. This matches the originally requested functionality and allows us to keep working on identifying the module being analyzed in the meantime. |
|
Agree @maceonthompson that one is critical to match components and some initial uses cases can unblocked when tools reading the VEX documents have the full context of the artifact being scanned. |
By looking at trivy code, it seems that not having a package identifier for the product will result in nothing being matched regardless of subproducts. I might be wrong as I am not familiar with the code base. |
|
Yes, the package/module is needed inside the product, with the affected/fixed dependency/path as the subcomponent (please excuse if I'm mixing terminologies by package per module per dependency path). For example, if govulncheck generates the following Vex file (just an example that I partially generated with my forked govulncheck then passed it to {
"@context": "https://openvex.dev/ns/v0.2.0",
"@id": "https://openvex.dev/docs/public/vex-9c71e5482d226764f5e1da8f8fd88201aa79d143f9a37e651f6c4306e584dbde",
"author": "Unknown Author",
"timestamp": "2024-07-10T17:19:23.734221546-03:00",
"version": 1,
"statements": [
{
"vulnerability": {
"name": "CVE-2023-32193"
},
"timestamp": "2024-07-10T17:19:23.7342222-03:00",
"products": [
{
"@id": "pkg:golang/github.com/rancher/webhook",
"subcomponents": [
{
"@id": "pkg:golang/github.com/rancher/[email protected]"
}
]
}
],
"status": "not_affected",
"impact_statement": "vulnerable_code_not_present"
}
]
}Where the Then passing it to Trivy will result in the "not_affected" CVE being correctly suppressed. |
|
Right, if there is no product nothing will get matched (same in grype). This is why we should populate the product when possible. External tooling may be able to fill in the missing product but it's not ideal. |
Yes, that's correct. If a product has subcomponents, Trivy applies VEX to the dependency graph (the same idea is described in the OSV blog). Incorrect product ID results in nothing filtered as it doesn't match the graph. If a product has no subcomponents, it works as a simple filter in Trivy, but in that case it may incorrectly filter vulnerabilities in another product. For example, my project is not affected by GO-2021-0064 in
That's exactly what we're doing now. |
|
Change https://go.dev/cl/598956 mentions this issue: |
|
A follow up question on subcomponent IDs: |
|
I think the import path and version should be enough. What else could you encode in the purl? The more specific the purl is the harder it is to match but perhaps I'm missing another use case that may make more sense. |
|
I agree that the matching becomes more difficult, which is why we're currently basing it just off the module path. We could, however, include the package path as well - which is technically part of the import path. But if the consensus is that the extra info isn't useful for this use case, I'm happy to leave it as is. |
|
FWIW, this is also what the golang/vuln#11 proposed fix does. There is also a consistency issue. If someone runs govulncheck in |
|
My first answer is to agree with:
@maceonthompson could you please provide some examples of how the subcomponent ID would be regarding what you mentioned?
Asking, so we have detailed info to make a call and how different the information would be for each one of the scan mode - 'module', 'package', or 'symbol'. |
@macedogm An example: Say there is a vulnerability in the module
This matches how pkgsite presents packages vs modules. I was more interested in if that would be useful - it seems that the consensus is that easier matching is more important, so I'm happy to just include the module path. |
|
@maceonthompson thanks for providing the examples.
It's useful, but I fully agree that |
Populates the "subcomponent" field of a outputted vex statement with the PURL to the vulnerable dependency. updates golang/go#68152 Change-Id: I9e7b9a6686744496b3409ee9d4d0f3d70917db45 Reviewed-on: https://go-review.googlesource.com/c/vuln/+/598956 LUCI-TryBot-Result: Go LUCI <[email protected]> Reviewed-by: Zvonimir Pavlinovic <[email protected]>
govulncheck version
Does this issue reproduce at the latest version of golang.org/x/vuln?
Yes.
Output of
go envin your module/workspace:The text was updated successfully, but these errors were encountered: