Add CVE scan

[abelsromero]

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 generated asciidoctor: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:

1. Have another job that runs make and scans on PRs. I assume the docker buildx engine should produce equivalent images. In case the scan jobs fails we can still proceed with release steps in build.
2. Same as 1, but manual job to avoid noise. I personally don't like this, I'd rather have an annoying reminder of CVEs that not.

* 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...

Ruby (gemspec)
==============
Total: 2 (UNKNOWN: 0, LOW: 0, MEDIUM: 0, HIGH: 2, CRITICAL: 0)

+----------+------------------+----------+-------------------+---------------+---------------------------------------+
| LIBRARY  | VULNERABILITY ID | SEVERITY | INSTALLED VERSION | FIXED VERSION |                 TITLE                 |
+----------+------------------+----------+-------------------+---------------+---------------------------------------+
| json     | CVE-2020-10663   | HIGH     | 1.8.6             | 2.3.0         | rubygem-json: Unsafe object           |
|          |                  |          |                   |               | creation vulnerability in JSON        |
|          |                  |          |                   |               | -->avd.aquasec.com/nvd/cve-2020-10663 |
+----------+------------------+          +-------------------+---------------+---------------------------------------+
| nokogiri | CVE-2021-41098   |          | 1.11.7            | 1.12.5        | rubygem-nokogiri: XEE on JRuby        |
|          |                  |          |                   |               | -->avd.aquasec.com/nvd/cve-2021-41098 |
+----------+------------------+----------+-------------------+---------------+---------------------------------------+

[dduportal]

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:

• 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 should not break the build if vulnerabilities are detected (at least not for the first iterations).

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:

• A given vulnerability might not be exploitable (there are a lot coming from a glibc behavior that are not a reality in the Alpine musl for instance)
• There might not be a fix available: if it comes from an upstream dependency for instance
• Fixing the vulnerability might break a use case
• etc.
  => all this examples are tricky of course, there isn't really a general approach that would work on all cases.

However starting by scanning and then acting on the current one is a good idea.

[abelsromero]

* 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?

[abelsromero]

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.

[dduportal]

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:

• 6 "high" in Java jar detected
• 2 "high" in ruby

[dduportal]

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?

[abelsromero]

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. Scan nightly on supported released versions (identified by tag) to catch new issues. That why the list of ignored CVEs works as document of known issues in released versions, we can optionally comment on whether they apply or not. 👉 This could be our current pipeline, for now latest should be fine.
2. Scan nightly on main branch to ensure main has no new issues or ignore. One release we just need to move the CVEs to the stable scan pipeline.
3. Scan on PRs to identify possible new issues before merging & and test fixes as any other code would do.

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.

[abelsromero]

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