aquasecurity / trivy

Find vulnerabilities, misconfigurations, secrets, SBOM in containers, Kubernetes, code repositories, clouds and more
https://aquasecurity.github.io/trivy
Apache License 2.0
23.59k stars 2.32k forks source link

Warn after OS detection about if unfixed CVE's are included #488

Open BretFisher opened 4 years ago

BretFisher commented 4 years ago

TL;DR: Issue a warning on any scan of OS that lacks unfixed CVE detection to include a message in the results about its lack of that info. For example, the default behavior leads a user to believe that Alpine has zero CVE's in it, while a Debian base image has many.

Feature idea:

Add a new INFO line to output after Alpine is detected:

trivy openjdk:15-alpine
2020-05-06T22:13:40.801-0400    INFO        Need to update DB
2020-05-06T22:13:40.802-0400    INFO        Downloading DB...
2020-05-06T22:13:44.550-0400    INFO        Detecting Alpine vulnerabilities...
2020-05-06T22:13:44.550-0400    INFO        Alpine results do not include unfixed CVE's

openjdk:15-alpine (alpine 3.11.6)
=================================
Total: 0 (UNKNOWN: 0, LOW: 0, MEDIUM: 0, HIGH: 0, CRITICAL: 0)

Backstory:

Trivy is great, and I often suggest it due to all the reasons mentioned in the README comparison, but largely due to its great Alpine support. I do, however, think it's still misleading in the results, which leads to people always thinking "alpine-based images have no CVE's, while debian-based of the same app have 80-100+". This is an incorrect assumption and isn't a true apples-to-apples comparison, It's just a difference in scanning and reporting settings/limits.

Someone just starting with trivy for image scanning may not understand the significant differences when they scan an alpine-based image vs. other images (debian-based, etc). They also likely don't know about the --ignore-unfixed significance.

Alpine/Oracle/etc scans currently act like --ignore-unfixed is always enabled, due to limits in the scanning behavior (as I understand it). Since this setting is disabled by default for all scans, if one were to do a scan like so:

trivy openjdk:15-alpine The results would show

Total: 0 (UNKNOWN: 0, LOW: 0, MEDIUM: 0, HIGH: 0, CRITICAL: 0)

This isn't really true, it's just not able to show unfixed CVE's like it can for other images.

Then if you were to trivy openjdk:15-slim you would get back:

Total: 86 (UNKNOWN: 0, LOW: 19, MEDIUM: 58, HIGH: 8, CRITICAL: 1)

Even with my understanding of CVE scanning with Alpine, it took me a while of comparing scan results and researching Alpine and Debian base images, as well as the --ignore-unfixed option, to realize that if I were to compare the two apples-to-apples, I should really be running:

trivy --ignore-unfixed openjdk:15-slim

Total: 0 (UNKNOWN: 0, LOW: 0, MEDIUM: 0, HIGH: 0, CRITICAL: 0)

I'm curious about the opinions on this, as I continue to see container users who are trying scanning tools but don't understand the subtle differences between one scan and another, and use the results to inform their decisions without realizing they aren't seeing the full picture.

yashvardhan-kukreja commented 4 years ago

Hi @BretFisher. So, would you think it to be a nice option for, say, alpine-based images, trivy more explicitly conveyed the user that the result, due to unfixed CVEs, is causing the scan to return results same as the one with "--ignore-unfixed" ?

yashvardhan-kukreja commented 4 years ago

@knqyf263 , can I work on this issue?

knqyf263 commented 4 years ago

@BretFisher Thank you for the suggestion. Yeah, fair enough. At the same time, I'm thinking if --ignore-unfixed should be the default. I mean we'll add --show-unfixed or something like that. https://github.com/aquasecurity/trivy/issues/323

@yashvardhan-kukreja Thank you for the offer! But, let me discuss the options with my team.

@simar7 @lizrice What do you think? It breaks the backward compatibility as trivy doesn't show unfixed vulnerabilities by default. But, it wouldn't be a problem if many users use --ignore-unfixed. For such users, it should display --ignore-unfixed is enabled by default now.

BretFisher commented 4 years ago

Thanks for considering my thoughts. Regardless of the functionality changes, I'm mostly asking for improved documentation in the CLI scans, so that unaware people (on either side of show-unfixed) will see info that they maybe want to investigate further for a better understanding. If a scan is unable to show all potential CVE's due to various limitations or is hiding a subset of them (whether by optional setting or default), the output should indicate that.

On Sun, May 10, 2020 at 4:49 AM Teppei Fukuda notifications@github.com wrote:

@BretFisher https://github.com/BretFisher Thank you for the suggestion. Yeah, fair enough. At the same time, I'm thinking if --ignore-unfixed should be the default. I mean we'll add --show-unfixed or something like that.

323 https://github.com/aquasecurity/trivy/issues/323

@yashvardhan-kukreja https://github.com/yashvardhan-kukreja Thank you for the offer! But, let me discuss the options with my team.

@simar7 https://github.com/simar7 @lizrice https://github.com/lizrice What do you think? It breaks the backward compatibility as trivy doesn't show unfixed vulnerabilities by default. But, it wouldn't be a problem if many users use --ignore-unfixed. For such users, it should display --ignore-unfixed is enabled by default now.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/aquasecurity/trivy/issues/488#issuecomment-626294509, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAGBNX5V2SP7VOCQYEP4FEDRQZTC7ANCNFSM4M26TPPQ .

lizrice commented 4 years ago

I like @BretFisher's idea of adding in an INFO message for the OSs where unfixed vulnerabilities aren't included. In fact I'd take it further - we could have an INFO message in all cases indicating whether unfixed vulns are included or not. In the cases where unfixed vuln information isn't available, we should say so.

That said, it wouldn't be obvious to anyone comparing results in, say, JSON, and that's a reason to think about (also) changing the default behaviour to exclude unfixed vulns, and adding support for --show-unfixed.

Currently I'd lean towards improving the INFO messages but leaving the --ignore-unfixed behavior as it is - what am I missing?

github-actions[bot] commented 3 years ago

This issue is stale because it has been labeled with inactivity.