Open abelsromero opened 2 years ago
Hello! Thanks for reporting and proposing this idea!
Would you be willing to start working on it and proposing a PR? (I can, as a maintainer, but new contribution and help is always a good thing :) ).
Running the scan on each make build
is a really good idea under the following conditions:
make build
on a contributor machine should never require to create an account anywhere (Haven't look at the security scan tool you're mentioning, but some former tools in this area required an account to access vuln. databases)The amount of cases of reported vulnerabilities is always noisy on such project: it feels like we should use it as an indicator rather than a "blocker". The reasoning (might be wrong, or not a consensus: don't hesitate to tell me if it is the case) is the following:
However starting by scanning and then acting on the current one is a good idea.
* Running a `make build` on a contributor machine should never require to create an account anywhere (Haven't look at the security scan tool you're mentioning, but some former tools in this area required an account to access vuln. databases)
It doesn't require anything. As far as I've seen it scans the files for specific files in a CVE database. That means that if I rename a known jar to something else it wont' match it. Also, the action is just a wrapper of https://github.com/aquasecurity/trivy which can be run as a plain CLI locally.
* It should not break the build if vulnerabilities are detected (at least not for the first iterations).
Then, it discards using it as part of the main build, but a failure will definetly happen. So if we want to reduce noise the only option is just ran as a weekly to scan latest version on dockerhub.
* Fixing the vulnerability might break a use case
The thing then is "can we trust the test?". For instance, 2 things to do are upgrading java to at least 11 and epubcheck-ruby:4.2.4.0
but do the tests cover componentes that use those?
At it again, to go baby steps and avoid breaking things I like the idea of having a weekly scheduled job running on the latest release from dockerhub. If it helps to jump-start security fixing we can move to main build when we feel confortable. So, I migrated my test pr to the official trivy github action and ended with a very simple config (runs here).
name: "CVE Scan"
on:
workflow_dispatch: { }
schedule:
- cron: '0 9 * * 1'
jobs:
scan-images:
name: Scan latest public image
runs-on: ubuntu-latest
strategy:
matrix:
image: [ docker-asciidoctor ]
tag: [ latest ]
steps:
- name: Pull
run: docker pull asciidoctor/${{ matrix.image }}:${{ matrix.tag }}
- name: Run Trivy vulnerability scanner
uses: aquasecurity/trivy-action@master
with:
image-ref: 'docker.io/asciidoctor/${{ matrix.image }}:${{ matrix.tag }}'
severity: 'CRITICAL,HIGH'
format: 'table'
# we can set to 0 to avoid breaking the pipeline
exit-code: '1'
wdyt? Seems there are already a few things to fix. One can argue we don't expose anything and CVEs don't affect us, but currectly publishing an image without CVEs is an important thing in corporate environments. Even if we don't fix some, we should document them.
Hello @abelsromero , many thanks for this huge work!
I'm reopening the issue to keep track of the next steps: the security scan runs daily as expected and is now documented, all thanks to the heavy lifting work you did.
However the scan tells me daily that "there are CVEs" which means: security issues detected.
Please note that th work you did on #240 showed a great impact: no more CVEs detected on Alpine.
We have to iterate on the remaining list and decide if there are false positive (e.g. not exploitable or not highly enough), or if they are fixable (and fix them).
As for today, there are:
Side note: I've been shown https://github.com/anchore/grype, an alternative to trivy or azure scan.
I don't say we should reconsider the choice, but it has a feature that is really useful: you can remove the CVEs from the scan that does not have a fix (yet?) available.
The rationale is that these CVEs alerts must provide an actionable, or there are false positive indeed. If there are no actionable (e.g. not CRITICAL and no fix), then better to ignore them.
Do you know if there is such a feature in trivy?
The rationale is that these CVEs alerts must provide an actionable, or there are false positive indeed. If there are no actionable (e.g. not CRITICAL and no fix), then better to ignore them.
The problem still remains that current pipeline just scans a publicly released version. It's been a good starting point and provided feedback but maybe not the best approach. I don't agree with the "false positive" or "no actioanable" semantics, it's more won't fix. For example, we can "ignore" a CVE to avoid the error notification during nightly, but that doesn't make it a false positive, it's just removing a nuisance. Could be a relevant issue ("actionable") that must be fixed in main branch. Once released, we should "add" the CVE back for scanning to avoid a regression. But this models is complicated to handle because requires keeping track of what gets released and does not offer info of main branch, unless with manual work. What I have been testing in other projects is having several pipelines:
1 & 2 are complementary and necessary, 3 is lower priority to me. I could start migrating to Grype so at least we can ignore some CVEs and reduce noise but I would definetly not ignore one unless some research is done, at least it would be nice to have an issue with some explanation or a README.
Do you know if there is such a feature in trivy?
The feature is there, but not exposed in the github-action. Grype scan action seems to offers that already adding a config file to the repo https://github.com/anchore/scan-action#additional-configuration. I personally have no preference, I know some projects use any or both, so long it's not a commercial project any is fine.
May be precipitate, but I just run grype locally and my first impression in not very positive. As you can see in the output is not classified by OS, Java, Ruby, etc. which means we need to dig deeper (the full json report helps though) but the real point is it's reporting what I consider false values, many of the CVEs related to pdf-reader relate to "Foxit PDF Reader". The extra GHSA are nice though, but those definetly we cannot tackle.
Wdyt? I still think I can work with trivy gh action to add the CVE filtering.
~ >>> grype asciidoctor/docker-asciidoctor:latest
✔ Vulnerability DB [updated]
✔ Loaded image
✔ Parsed image
✔ Cataloged packages [308 packages]
✔ Scanned image [69 vulnerabilities]
NAME INSTALLED FIXED-IN VULNERABILITY SEVERITY
Pillow 8.4.0 9.0.0 GHSA-xrcv-f9gm-v42c Critical
Pillow 8.4.0 9.0.0 GHSA-pw3c-h7wp-cvhx Critical
Pillow 8.4.0 9.0.0 GHSA-8vj2-vxx3-667w Critical
cgi 0.2.1 0.3.1 GHSA-4vf4-qmvg-mh7h High
commons-compress 1.20 CVE-2021-35516 High
commons-compress 1.20 1.21 GHSA-7hfm-57qf-j43q High
commons-compress 1.20 CVE-2021-35515 High
commons-compress 1.20 CVE-2021-36090 High
commons-compress 1.20 1.21 GHSA-xqfj-vm6h-2x34 High
commons-compress 1.20 CVE-2021-35517 High
commons-compress 1.20 1.21 GHSA-crv7-7245-f45f High
commons-compress 1.20 1.21 GHSA-mc84-pj99-q6hh High
date 3.1.3 CVE-2014-5169 Low
delegate 0.2.0 CVE-2005-0036 Medium
delegate 0.2.0 CVE-1999-1338 Medium
delegate 0.2.0 CVE-2005-0861 High
find 0.1.0 CVE-2020-24550 Medium
graphviz 2.49.3-r0 CVE-2014-9157 High
guava 24.1.1-android GHSA-5mg8-w23w-74h3 Low
guava 24.1.1-android CVE-2020-8908 Low
guava 24.1.1-android 24.1.1 GHSA-mvr2-9pj6-7w5j Medium
guava 24.1.1-android CVE-2018-10237 Medium
i18n 1.8.11 CVE-2020-7791 High
json 1.8.6 CVE-2020-7712 High
json 1.8.6 CVE-2020-10663 High
json 2.5.1 CVE-2020-7712 High
json 1.8.6 2.3.0 GHSA-jphg-qwrw-7w9g High
logger 1.4.3 CVE-2017-14727 High
matrix 0.3.1 CVE-2017-14198 High
matrix 0.3.1 CVE-2017-14197 Medium
nokogiri 1.11.7 CVE-2021-41098 High
nokogiri 1.11.7 1.12.5 GHSA-2rr5-8q37-2w7h High
observer 0.1.1 CVE-2008-4318 High
pdf-reader 2.9.1 CVE-2021-34845 High
pdf-reader 2.9.1 CVE-2021-34841 High
pdf-reader 2.9.1 CVE-2021-45980 High
pdf-reader 2.9.1 CVE-2021-34853 High
pdf-reader 2.9.1 CVE-2021-38563 Critical
pdf-reader 2.9.1 CVE-2021-38566 High
pdf-reader 2.9.1 CVE-2021-34847 High
pdf-reader 2.9.1 CVE-2021-34851 High
pdf-reader 2.9.1 CVE-2021-45979 High
pdf-reader 2.9.1 CVE-2021-34833 High
pdf-reader 2.9.1 CVE-2021-34836 High
pdf-reader 2.9.1 CVE-2021-34835 High
pdf-reader 2.9.1 CVE-2021-34848 High
pdf-reader 2.9.1 CVE-2021-45978 High
pdf-reader 2.9.1 CVE-2021-34842 High
pdf-reader 2.9.1 CVE-2021-38565 High
pdf-reader 2.9.1 CVE-2021-34839 High
pdf-reader 2.9.1 CVE-2021-34840 High
pdf-reader 2.9.1 CVE-2021-34849 High
pdf-reader 2.9.1 CVE-2021-34831 High
pdf-reader 2.9.1 CVE-2021-34844 High
pdf-reader 2.9.1 CVE-2021-38564 Critical
pdf-reader 2.9.1 CVE-2013-0732 High
pdf-reader 2.9.1 CVE-2021-34834 High
pdf-reader 2.9.1 CVE-2021-34843 High
pdf-reader 2.9.1 CVE-2021-34850 High
pdf-reader 2.9.1 CVE-2021-34852 High
pdf-reader 2.9.1 CVE-2021-34832 High
pdf-reader 2.9.1 CVE-2021-34837 High
pdf-reader 2.9.1 CVE-2021-38567 High
pdf-reader 2.9.1 CVE-2021-34846 High
pdf-reader 2.9.1 CVE-2021-34838 High
readline 0.0.2 CVE-2014-2524 Low
reportlab 3.6.6 CVE-2020-28463 Medium
set 1.0.1 CVE-2021-23497 Critical
As discussed in Zulip, we could add azure/container-scan to obtain a report on CVEs included in the image.
Ideally, we want it to run on every build after
make build
which is simple when running in the same job since we are scaning the locally generatedasciidoctor:latest
image. However the issue is that if we find a HIGH CVE (which happens now*) the build will break and maybe that's not desirable. If we don't want that, we can:build
.* The issue are jars pulled by
epubcheck-ruby:4.2.4.0
and indirect ruby dependencies. Upgrading to epubcheck-ruby latest v4.2.6.0 fixes the jar issues but I don't know the impact of that. For Ruby we may need to go deeper...