-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
fix(vex): CSAF filtering should consider relationships #5923
fix(vex): CSAF filtering should consider relationships #5923
Conversation
Signed-off-by: juan131 <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am a little concerned about this implementation. Using the VEX added to the test as an example, the Kubernetes library in Argo may not be affected by CVE-2023-2727, but the same library in Kyverno may be affected. This implementation just filters out vulnerabilities, like CVE-2023-2727, with Kubernetes, leading to false suppression. I'm not familiar with CSAF VEX. Please correct me if I'm missing something.
Strictly speaking, a tree of products would need to be built on the Trivy side as well and compared with the CSAF VEX product tree.
However, that implementation will take some time, so the current implementation is fine as a work-around until then!
Hi @knqyf263 Please note the CSAF document added to the test was created to filter out vulnerabilities affecting a container in particular (Bitnami ArgoCD 2.9.3-2 Amd64 Debian12). Therefore, it must be interpreted within that context. That's the reason for the explanation provided: "threats": [
{
"category": "impact",
"date": "2024-01-04T17:17:25+01:00",
"details": "The asset uses the component as a dependency in the code, but the vulnerability only affects Kubernetes clusters https://github.com/kubernetes/kubernetes/issues/118640"
}
] That CSAF document shouldn't be used to filter vulnerabilities from Kyverno or any other solution. Does it make sense? |
I think VEX documents should be interpreted in a context-agnostic way. Both |
I can think of scenarios where VEX documents could be provided within some specific context (e.g. to express that a vulnerability affecting a container is not exploitable when it's part of a specific Helm chart as a consequence of the way that chart configures the K8s manifests that make use of the container). Regardless of this topic, the relationships described in the document do link the products unambiguously (in the example saying "kubernetes-v1.24.2' is a component of "argo-cd-2.9.3-2-amd64-debian-12": "relationships": [
{
"product_reference": "kubernetes-v1.24.2",
"category": "default_component_of",
"relates_to_product_reference": "argo-cd-2.9.3-2-amd64-debian-12",
"full_product_name": {
"product_id": "argo-cd-2.9.3-2-amd64-debian-12-kubernetes",
"name": "Argo CD uses kubernetes golang library"
}
}
]
} And.. These relationships have its own "product_id" in the CSAF document ("argo-cd-2.9.3-2-amd64-debian-12-kubernetes" in this case). When a vulnerability is tied to a "product_id" identifying a relationship we ensure we're not filtering it for other product that may also have the same component. {
"cve": "CVE-2023-2727",
"flags": [
{
"date": "2024-01-04T17:17:25+01:00",
"label": "vulnerable_code_cannot_be_controlled_by_adversary",
"product_ids": [
"argo-cd-2.9.3-2-amd64-debian-12-kubernetes"
]
}
],
(..._ |
In this case, the relationship should be described in the document. It should not depend on the context (like the VEX document is included in the Helm chart), IMO. Just to clarify, my thought is applied when relationships are defined.
If the relationship is defined as above, we should not filter out the vulnerability on k8s used by other products than Argo CD, even if the VEX document is passed.
Yes, it should work as you described. Is the current implementation working like so? This PR seems to just extract Lines 100 to 102 in 2f24ac3
|
You're right @knqyf263 ! I was confused since I was under the impression the validity of the provided CSAF doc was questioned. That said, as you mentioned the current implementation doesn't take into account whether the tree of products on the Trivy side. Do you think we could at least include it even it's not a perfect solution (give the VEX functionality is considered experimental)? I guess we could some some warning when a vulnerability on a package is filtered out as a consequence of being a "default component of/external component of/optional component of/installed on/installed with" of another package. Sth like this:
|
@knqyf263 what do you think about my last comment? |
Yes, I think we can go with the current approach, although it is not perfect. That's what I meant earlier.
It sounds like a good idea. |
if p.Match(pkgURL) { | ||
return true | ||
// getProductPurls returns a slice of PackageURLs associated to a given product | ||
func (v *CSAF) getProductPurls(product csaf.ProductID) []*purl.PackageURL { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
question: In what cases can a single product id have multiple PURLs?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To be honest, I can't think of any situation like that but the official library is returning a slice:
Co-authored-by: Teppei Fukuda <[email protected]>
Signed-off-by: juan131 <[email protected]>
Signed-off-by: juan131 <[email protected]>
@knqyf263 sorry for the long delay on addressing your comments, I've been involved in other projects these last weeks and I didn't find the time for this. |
@juan131 No problem. Thanks for taking your time. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
Description
We're currently not considering relationships within CSAF documents. However, as it's described in the CSAF reference below, relationships could be used to indicate a product is a "default component of/external component of/optional component of/installed on/installed with' another one.
With this in mind, when we detect a vulnerability affecting certain package, we need to inspect relationships because that package could be included in the CSAF document using the above mentioned relationships.
Related PRs
Checklist