artifacthub / hub

Find, install and publish Cloud Native packages
https://artifacthub.io
Apache License 2.0
1.66k stars 227 forks source link

Provide mechanism to suppress/ignore individual vulnerabilities with commentary #3567

Closed chadlwilson closed 8 months ago

chadlwilson commented 9 months ago

Is your feature request related to a problem? Please describe.

Currently, security vulnerability reports are somewhat all or nothing - they can be completely disabled via whitelisting an image or all discovered vulnerabilities are reported.

The problem is that application vulnerability scanning is notoriously rife with false positives which probably decreases the utility of the reports through the FUD effect.

In any case, since maintainers can whitelist entire images, I propose that it'd be useful for the purposes of ArtifactHub to have a mechanism by which image authors can (optionally) express suppressions/ignores vuln-by-vuln.

This would allow the risk ratings (derived from CVSS scores on "components") to effectively be augmented with something that reflects the "environmental" factors that are supposed to be taken into account alongside base CVSS scores from such tools as Trivy.

Example of a noisy report https://artifacthub.io/packages/helm/gocd/gocd?modal=security-report

Describe the solution you'd like

I propose something like

Nice to haves

Describe alternatives you've considered

Whitelisting the entire image.

Additional context

I understand that this proposal would be somewhat coupled to the use of Trivy as the underlying scanner, which is not ideal and may make this a bad idea.

tegioz commented 9 months ago

Hi @chadlwilson 👋

Thanks for raising this issue!

The problem with allowing ignoring individual vulnerabilities is that it could be misused accidentally or abused intentionally. Sometimes it can be hard to track down the different scenarios you described related to false positives, and it's possible to mistakenly jump to the conclusion that certain vulnerability isn't affecting some image. Similarly, this mechanism would make it possible to suppress vulnerabilities that aren't actually false positives, just to make a package look better security wise. On top of this, please note that vulnerabilities severity can change over time, or they may start affecting a component that previously weren't, which may not be properly tracked -or reflected in AH-.

When an image is whitelisted, we make it clear in the UI that the publisher didn't want their images to be scanned. Users can use that information if they wish to decide whether they want to use that package or not. However, not providing a security report is not the same as providing a report that has been altered or masked in some way. Somehow we have a responsibility when producing security reports, and IMHO we should be as transparent as possible and not hide any information from final users or make it harder for them to find it.

However, augmenting the security report with some extra information provided by the publisher about the vulnerabilities detected sounds interesting and helpful. The number of vulnerabilities would still be the same, but publishers would have a mechanism to let users know their thoughts about how those vulnerabilities affect their packages. We could highlight this somehow in the report summary, as well as per target and individual vulnerabilities.

Regarding how could we achieve this, we need to keep in mind that new security vulnerabilities will appear and existing ones may change after a package version has been published. So I don't think this information should be part of the images or the chart metadata, as existing images/charts versions ideally shouldn't change. I think it'd probably be best to allow this information to be managed per package/version from the Artifact Hub control panel.

chadlwilson commented 9 months ago

While I understand where you’re coming from and have heard many of these arguments before, philosophically I don’t agree that the current state is better - because the maturity of all of this tooling (trivy included), the databases behind them, and indeed the main MITRE/NVD CVE process is low and leads to massive amounts of false positives by design.

The amount of noise such tooling is spraying into the ecosystem is doing as much harm as good in my view. Users of a helm chart cannot be reasonably expected to know which of the ‘n’ vulnerabilities are relevant or exploitable and are looking for guidance from maintainers. The current signal-to-noise ratio in the industry from such scans is horrific. In some contexts it’s enough to make containers difficult to recommend in many cases, just due to the noise factor from non-distroless images. When we promote base CVSS scores like this without any hints about the critical “Environmental” score or any hints/opinions, we are forcing consumers of software that have no idea of how is is implemented to make assessments far beyond a reasonable level.

Given ArtifactHub allows blanket whitelisting already, I don’t really understand the abuse argument. As long as it is specifically noted that “‘n’ vulnerabilities were suppressed by maintainers” or perhaps with two different scores displayed (as I suggested above) with prominent billing, and possibly with a “raw” score, I don’t see how that is any worse than blanket whitelisting and in my view a lot better and more transparent. Trivy doesn’t understand all languages and/or statically compiled software and the current setup is blanket punishing anything that is more easily inspectable. I don’t think this is making us safer at reasonable cost.

At the end of the day, if we don’t trust maintainers to manage suppressions, why would we trust them to use the software they produce? If I wanted to do this, right now I could already go in and strip the pom.xml files from the JAR files inside images I maintain, and rename the jars and render the Trivy scanner useless and unable to find vulnerabilities. I could switch to CentOS Stream base images which Trivy is unable to find vulnerabilities in. We don’t do that because we’re not maniacs 😄 I’m not saying that we should blindly trust maintainers, and independent SCA scans are useful - but I don’t think we should be overstating such independent scans’ value in building trust in the community and slapping Fs over charts with a few unexploitable CVEs in them which no-one has ever proved are exploitable in the given software context. I fear that amounts to FUD-spreading.

Maybe that’s at the root of my concern here. I view these A-F scores as being Artifact Hub editorializing based on radically incomplete information. It has no way of making such an assessment and is over simplified.

Yes, sometimes a component can become vulnerable to something it was previously not, however there are other options to mitigate this, such as an opinionated Artifact Hub requiring all suppressions for new chart versions to be time-bound, requiring periodic update (say max 1 year from current) and otherwise filtering them from what is passed to trivy.

As for where the suppressions are held, if it were in the metadata since that is released alongside every version of the chart, a suppression wouldn’t apply to past versions of the chart, so isn’t that a reasonable trade off? Managing from the control panel doesn’t sound very automatable and scalable to me. As a maintainer I’d want those to sit with the code used to produce the images. In many cases projects (such as the one I maintain) are running these scans ourselves upstream and having a way to manage those suppresssions/commentary centrally and feed into the produced artifact would be desirable - even at the downside of not being able to change it after the fact. In that sense it’s an ‘opinion at a point in time’ and may be missing suppresssions/commentary for newer vulnerabilities - but can always cut a new chart release.

tegioz commented 9 months ago

To make this more constructive let's focus on some specific points instead of the philosophical part 🙂

A-F rating. This is something we can consider removing, as the raw data is enough. In the early days of AH that rating was even visible in the packages cards on searches, but we removed it precisely for that reason, it was an over simplification that could be misleading. The intention was to make things easier for users, but it's hard to summarize in a single letter something so complex.

Like I mentioned in my previous comment, I like the idea of being able to augment the security report with maintainers notes about the vulnerabilities, and we could try to highlight this information in several places to make sure users see them. That would be helpful for users when making their decisions.

Regarding where those notes should live, I don't think that including that information when the chart is released would work. At that point, publishers may not be even be aware that those vulnerabilities exist. Not all publishers will scan in advance, and even if they do, those vulnerabilities may need to be analyzed and documented, sometimes manually. Not being able to change it after the fact sounds unjustifiably limiting, specially for something like security vulnerabilities. And any functionality added to the control panel is first made available in the HTTP API, so automation shouldn't be a problem if that was actually needed.

chadlwilson commented 9 months ago

The philosophy is important insofar as we agree on what outcome ArtifactHub might be trying to achieve, and what we believe the role of such scans are in promoting truly improved security outcomes. If we don’t have the same prior assumptions on the goals, we’re not likely to agree much on proposed Improvements (which is OK, I’m not an AH maintainer just a Helm Chart and container Image maintainer for GoCD, and user of other charts). I looked but could not find alongside https://artifacthub.io/docs/topics/security_report/ (or anywhere else, but please point me somewhere if it’s discussed in the open) a description of the intent of this report, its design or commentary/advice to users or maintainers about its limitations.

On the specific points, I don’t think notes are enough without a broader rethink of the design. While you may have analytics to confirm or refute this, I fear many folks won’t look much further than either the rating, or the massive fear banner “WARNING: This package has high severity fixable vulnerabilities older than 2 years old that haven't been addressed yet.”, or the counts in the critical/high categories (based largely on CVSS base scores, usually NVD-originated, and unadjusted for environmental context).

I think either

I also don’t think “publishers not being aware” at publishing time is a relevant concern? That’s the case currently, right - if they don’t look at the security report of a previous chart version, and also don’t scan images or dependencies in advance - they will then blindly publish, hopefully see a big F rating and maybe then look at rebuilding some images, upgrading some dependencies or switching to different Images (and in the future maybe adding notes or suppressing). In either scenario they have to publish a new version of the chart (assuming not relying on mutable image tags….) and/or images?

tegioz commented 9 months ago

The intent of the security report is purely informational, both for users and publishers. We know of some cases where this report has helped maintainers make some improvements in their software, which is directly beneficial for its final users.

We appreciate your input but at the moment we don't have plans to do a broader rethink of the design. We think it's clear and intuitive, and allows navigating the information available easily. Displaying these summary numbers is something fairly common (sample from Docker Hub), and we think most users know how to interpret them properly.

tegioz commented 8 months ago

Will close this one for now, please feel free to reopen if needed.