Add --vuln-detection-level=lax

[knqyf263]

Description

Description

Trivy doesn't analyze files managed by an OS package manager. It's important to reduce false positives. Let's take an example of RPM. In case rh-maven35-log4j12-1.2.17-19.2 is installed in a RHEL7 container image, we will find a JAR file, /opt/rh/rh-maven35/root/usr/share/java/log4j-1.2.17.jar in the image.

bash# rpm -ql rh-maven35-log4j12-1.2.17-19.2.el7.noarch
/opt/rh/rh-maven35/root/usr/share/java/log4j-1.2.17.jar
/opt/rh/rh-maven35/root/usr/share/java/log4j-1.jar
/opt/rh/rh-maven35/root/usr/share/java/log4j12-1.2.17.jar
/opt/rh/rh-maven35/root/usr/share/licenses/rh-maven35-log4j12-1.2.17/LICENSE
/opt/rh/rh-maven35/root/usr/share/licenses/rh-maven35-log4j12-1.2.17/NOTICE
/opt/rh/rh-maven35/root/usr/share/maven-metadata/log4j12.xml
/opt/rh/rh-maven35/root/usr/share/maven-poms/log4j-1.2.17.pom
/opt/rh/rh-maven35/root/usr/share/maven-poms/log4j12-1.2.17.pom

Log4j 1.2.17 looks vulnerable to CVE-2022-23302, which affect <=1.2.17. However, Red Hat applied a patch to 1.27.17 and released 1.2.17-18 in RHSA-2022:0442. So, this package is actually not affected by the vulnerability.

If Trivy analyzes /opt/rh/rh-maven35/root/usr/share/java/log4j-1.2.17.jar in the same way as a normal JAR, it leads to false positives. Even if we are lucky and it doesn't lead to a false positive, the result will be duplicated. It is necessary to determine where the file came from and apply the advisory correctly (e.g., upstream advisory vs vendor advisory).

This also applies to Go and Rust binaries, for example. However, some vendors may be delayed to provide advisories. In such cases, users may want to force detection even at the risk of identifying the wrong version or using the wrong advisory, leading to false detection of vulnerabilities.

There is also a possibility that vendors may not provide SBOMs or vulnerabilities for dependent packages of the packages they provide. Let's see an example.

Example

cgr.dev/chainguard/argocd has a go binary at /usr/local/bin/argocd, managed by apk.

$ docker run --rm -it cgr.dev/chainguard/argocd cat /lib/apk/db/installed | head -n 50
P:argo-cd-2.11-compat
V:2.11.5-r0
A:aarch64
L:Apache-2.0
T:Compatibility package for locating binaries according to upstream helm charts
o:argo-cd-2.11
m:
U:
D:
p:argo-cd-compat=2.11.5-r0
c:eae744a459056d6402b77afb5e49918f5a85007a
i:[]
t:1721077920
S:35983862
I:130940688
k:0
C:Q1NDIsUeKC5OQ3I2q958hlhTPbN00=
F:usr
F:usr/local
F:usr/local/bin
R:argocd
a:0:0:0755
Z:Q1eRB1Z8uqRNv9tb8b4kXQGjsb4Io=
R:argocd-application-controller
a:0:0:0777
Z:Q1SYhr2bGNJukWKQ9shpz9EtbRsI0=
R:argocd-applicationset-controller
a:0:0:0777
Z:Q1SYhr2bGNJukWKQ9shpz9EtbRsI0=
R:argocd-cmp-server
a:0:0:0777
Z:Q1SYhr2bGNJukWKQ9shpz9EtbRsI0=
R:argocd-k8s-auth
a:0:0:0777
Z:Q1SYhr2bGNJukWKQ9shpz9EtbRsI0=
R:argocd-notifications
a:0:0:0777
Z:Q1SYhr2bGNJukWKQ9shpz9EtbRsI0=
R:argocd-repo-server
a:0:0:0777
Z:Q1SYhr2bGNJukWKQ9shpz9EtbRsI0=
R:argocd-server
a:0:0:0777
Z:Q1SYhr2bGNJukWKQ9shpz9EtbRsI0=
F:var
F:var/lib
F:var/lib/db
F:var/lib/db/sbom
R:argo-cd-2.11-compat-2.11.5-r0.spdx.json
Z:Q1ByViJ5qiURT0AZEPudi+/qiovDo=

So, Trivy skips analyzing /usr/local/bin/argocd, but this binary actually contains some vulnerable dependencies, like
CVE-2024-35255 in github.com/Azure/azure-sdk-for-go/sdk/azidentity.

$ go version -m ./usr/local/bin/argocd | grep azure-sdk-for-go
        dep     github.com/Azure/azure-sdk-for-go/sdk/azcore    v1.1.1  h1:tz19qLF65vuu2ibfTqGVJxG/zZAI27NEIIbvAOQwYbw=
        dep     github.com/Azure/azure-sdk-for-go/sdk/azidentity        v1.1.0  h1:QkAcEIAKbNL4KoFr4SathZPhDhF4mVwpBMFlYjyAqy8=
        dep     github.com/Azure/azure-sdk-for-go/sdk/internal  v1.0.0  h1:jp0dGvZ7ZK0mgqnTSClMxa5xuRL7NZgHameVYF6BurY=

Their SBOM doesn't provide any information on ArgoCD's dependencies.

$ cat var/lib/db/sbom/argo-cd-2.11-2.11.5-r0.spdx.json
{
  "SPDXID": "SPDXRef-DOCUMENT",
  "name": "apk-argo-cd-2.11-2.11.5-r0",
  "spdxVersion": "SPDX-2.3",
  "creationInfo": {
    "created": "2024-07-15T21:12:00Z",
    "creators": [
      "Tool: melange (v0.19.7-3-g8bbd76c)",
      "Organization: Chainguard, Inc"
    ],
    "licenseListVersion": "3.22"
  },
  "dataLicense": "CC0-1.0",
  "documentNamespace": "https://spdx.org/spdxdocs/chainguard/melange/e251bfdff0f86ebbbc333abca0a8c64022fd1502",
  "documentDescribes": [
    "SPDXRef-Package-argo-cd-2.11-2.11.5-r0"
  ],
  "packages": [
    {
      "SPDXID": "SPDXRef-Package-argo-cd-2.11-2.11.5-r0",
      "name": "argo-cd-2.11",
      "versionInfo": "2.11.5-r0",
      "filesAnalyzed": false,
      "licenseConcluded": "NOASSERTION",
      "licenseDeclared": "Apache-2.0",
      "downloadLocation": "NOASSERTION",
      "originator": "Organization: Wolfi",
      "supplier": "Organization: Wolfi",
      "copyrightText": "\n",
      "externalRefs": [
        {
          "referenceCategory": "PACKAGE_MANAGER",
          "referenceLocator": "pkg:apk/wolfi/[email protected]?arch=aarch64",
          "referenceType": "purl"
        },
        {
          "referenceCategory": "PACKAGE_MANAGER",
          "referenceLocator": "pkg:github/argoproj/[email protected]",
          "referenceType": "purl"
        },
        {
          "referenceCategory": "PACKAGE_MANAGER",
          "referenceLocator": "pkg:github/argoproj/argo-cd@c4b283ce0c092aeda00c78ae7b3b2d3b28e7feec",
          "referenceType": "purl"
        },
        {
          "referenceCategory": "PACKAGE_MANAGER",
          "referenceLocator": "pkg:github/wolfi-dev/os@eae744a459056d6402b77afb5e49918f5a85007a#argo-cd-2.11.yaml",
          "referenceType": "purl"
        }
      ]
    }
  ],
  "relationships": []
}

Therefore, Trivy misses CVE-2024-35255 in this case. Ideally, the SBOM should include the package's dependencies, like Bitnami images.

Proposal

Still, it might be good to have an option to force analysis since there must be such a container image (and a vendor) with missing information. It produces false positives, so I don't think we should enable it by default, though.

Similar to #4109, but it's not easy for users to understand the details. I think we may want to provide a more abstract flag, like --vuln-detection-level=normal (default), --vuln-detection-level=lax (inspired by Cookie SameSite) and --vuln-detection-level=strict (in the future). This flag can be used for trade-off between false positives vs false negatives in the future.

Target

None

Scanner

Vulnerability

[knqyf263]

@DmitriyLewen Please let me know your thoughts.

[DmitriyLewen]

This is old problem.
And new flag fixes 2 problems:

• delay in adding advisories from the vendor (as you said)
• users will be able to scan third-party packages

Some thoughts:

• i think we need to use vuln prefix (--vuln-detection-mode) to show that this flag only works for vulnerability scanner.
• We can also add options to detect for vulnerabilities both from vendor and from another database (e.g., Debian + GitHub). Users will delete duplicates themselves. But it can be useful when the user uses vendor and third party packages at the same time.

[knqyf263]

i think we need to use vuln prefix (--vuln-detection-mode) to show that this flag only works for vulnerability scanner.

Agree. Updated. Also, mode may be unclear. I changed it to --vuln-detection-level.

We can also add options to detect for vulnerabilities both from vendor and from another database (e.g., Debian + GitHub).

This is what I intended with --vuln-detection-level=lax, like JAR file in the above example will be scanned with a Red Hat advisory and a GitHub advisory. Do you mean something else?

[DmitriyLewen]

This is what I intended with --vuln-detection-level=lax, like JAR file in the above example will be scanned with a Red Hat advisory and a GitHub advisory. Do you mean something else?

I misunderstand you.
I thought we would scan this jar using only GitHub database.

I think we can use 3 modes:

• only vendor DB is used (Debian, Ubuntu, RedHat, etc.)
• only common DB is used (e.g. GitHub advosory database)
• use both DBs (e.g. RedHat + GitHub)

[knqyf263]

Let's take an example of log4j I shared above. Here is the summary.

1. OS Package Detection:
   • Vendor advisories (such as Red Hat's) should always be used for detecting vulnerabilities in OS packages. In the example provided, log4j-1.2.17-18 is found via RPMDB, and it should definitely be flagged as vulnerable based on the vendor's advisory.
2. File System Scanning:
   • When log4j JAR files are found during a file system scan, I see two possible approaches:
     • 2.1 Current Behavior:
       Currently, if the JAR file was installed by the OS package manager (e.g., dnf), it's ignored as it is scanned as an OS package. Otherwise, a binary scan is performed using upstream advisories like GHSA. I think this is a reasonable approach.
     • 2.2 Proposed Behavior with --vuln-detection-level=lax:
       I'm proposing that even if the JAR file was installed by the OS package manager, it would be forcibly scanned using upstream advisories when this option is used. This could be useful in certain scenarios, even if there's a risk of false positives.

The main point I'm discussing is scenario 2.2. It doesn't make sense to use vendor advisories as well as upstream advisories in this case since it's anyway scanned as an OS package with vendor advisories.

[DmitriyLewen]

Got it.
This seems good to me.

Trivy supports filtering options such as VEX, so users can easily and conveniently set up filtering for duplicates and false positives.

[knqyf263]

Some more ideas on this flag.
#7163 (comment)

But then, it also affects SBOM as well as vulnerabilities. We may want to change a flag name back to --detection-level. Also, --detection-threshold or --detection-precision may describe it better. Trivy tries to achieve higher precision, but some users may prefer higher recall.
https://developers.google.com/machine-learning/crash-course/classification/precision-and-recall

[knqyf263]

Here are some more ideas:

• --detection-priority
  • precision (default)
  • recall (high recall, meaning more false positives, less false negatives)
  • custom (allow users to define)
• --detection-precision
  • high (default)
  • low (more false positives, less false negatives)
• --scan-profile
  • conservative (default)
  • exhaustive (more false positives, less false negatives)

[knqyf263]

@nikpivkin To get all scanners aligned, I'd ask about misconfiguration scanning. Does it also have a conflict, like precision vs recall?

[nikpivkin]

I think not

[knqyf263]

After thinking about it for a few days (and talking with AI many times), I'd vote for --detection-priority precision/coverage.

• precision: Prioritizes reducing false positives. This option increases the reliability of reported vulnerabilities but may miss some potential issues.
• coverage: Aims to detect as many potential vulnerabilities as possible. This option reports more findings but may increase false positives.

While I also considered using terms like "precision" and "recall" from machine learning, I'd opt for more intuitive language to make the option accessible to broader users. The term "coverage" was chosen instead of "recall" to avoid potential confusion and to present a more positive framing.

I also avoided using "accuracy" as it has a specific meaning in machine learning contexts that includes false negatives in its evaluation, which could be misleading in this context. Although "precision" is a technical term, it conveys the meaning well enough in this case.

Currently, it can be a boolean flag as it is binary, but I chose a string flag with a view to adding "custom" or more values in the future.

In addition, I suggest --detection-priority instead of --vuln-detection-priority since such an option may also be needed for other scanners.

[DmitriyLewen]

I agree with your choice - it sounds logical and reasonable, so I have nothing to add.

[knqyf263]

Created #7270