Value of having Trivy scan workflow

[ivanvc]

What would you like to be added?

I'm spinning off the conversation of keeping the Trivy scan out of #19358 to improve the discussion.

Release tests GitHub workflow

Currently, the release tests' GitHub workflow is split into two steps:

1. Ensure that the release scripts work and also verifies the built Docker images.

   etcd/.github/workflows/release.yaml

   Lines 15 to 31 in e25d605

   	- name: release
   	run: |
   	set -euo pipefail
   	git config --global user.email "[email protected]"
   	git config --global user.name "Github Action"
   	gpg --batch --gen-key <<EOF
   	%no-protection
   	Key-Type: 1
   	Key-Length: 2048
   	Subkey-Type: 1
   	Subkey-Length: 2048
   	Name-Real: Github Action
   	Name-Email: [email protected]
   	Expire-Date: 0
   	EOF
   	DRY_RUN=true ./scripts/release.sh --no-upload --no-docker-push --no-gh-release --in-place 3.6.99

2. Run a Trivy image scan on the built images.

   etcd/.github/workflows/release.yaml

   Lines 62 to 68 in e25d605

   	- name: trivy-scan
   	uses: aquasecurity/trivy-action@18f2510ee396bbf400402947b394f2dd8c87dbb0 # v0.29.0
   	with:
   	image-ref: 'gcr.io/etcd-development/etcd:v3.6.99-${{ matrix.platforms }}'
   	severity: 'CRITICAL,HIGH'
   	format: 'sarif'
   	output: 'trivy-results-${{ matrix.platforms }}.sarif'

Trivy Workflow

If we analyze what the Trivy image scan is doing by reading our Dockerfile:

etcd/Dockerfile

Lines 1 to 6 in 9de211d

	ARG ARCH=amd64
	FROM --platform=linux/${ARCH} gcr.io/distroless/static-debian12@sha256:3f2b64ef97bd285e36132c684e6b2ae8f2723293d09aae046196cca64251acac
	ADD etcd /usr/local/bin/
	ADD etcdctl /usr/local/bin/
	ADD etcdutl /usr/local/bin/

It will scan the base image we're using and the etcd static binaries, as there's nothing else we're installing in the Docker image.

• Our image uses gcr.io/distroless/static-debian12, already stripped down from libraries, external dependencies, etc. We also keep this image up to date with Dependabot, so having this as a CI job is highly likely never to find a vulnerability. It's still possible that it will find a vulnerability as soon as the CVE is published. However, the current configuration has the severity set to CRITICAL,HIGH. Because when new vulnerabilities are published their severity is UNKNOWN, the workflow is very likely never to fail.
• The second part of the scan checks the compiled static binaries. As I mentioned in github workflows: remove release tests #19358, this check duplicates govulncheck. As it checks for vulnerabilities in Golang's stdlib, and following the previous point, it won't fail because initially the CVEs have an UNKNOWN severity. Contrary to govulncheck, it fails without analyzing the severity (and govuln only fails if we're directly affected, and it also checks all of the dependencies, not just Golang's).

Example of a false negative

Using the CVE CVE-2023-32082, which belongs to us, if we do a Trivy scan on an affected version, i.e., v3.5.0, the report doesn't return it:

$ trivy image -q quay.io/coreos/etcd:v3.5.0 | grep CVE-2023-32082
[nothing]

Note: Of course, the report is not empty, as we're using an old Debian version as the base, which has several reported vulnerabilities with defined severities.

Proposed enhancements

In my humble opinion, I don't see a lot of value in this check. However, I can think of two possible alternatives:

1. If we decide to keep it, I think a better alternative would be to run a Trivy scan on our base image (gcr.io/distroless/static-debian12). And decouple it from the release script tests (maybe create a new Prow job, or consolidate it with govulncheck)
2. We're not checking the latest released image for vulnerabilities, as this is configured as presubmit. So, we may never find out if our latest released images (i.e., v3.5.18 and v3.4.35 [as of today]) are using vulnerable dependencies. That's why, in github workflows: remove release tests #19358, I propose to move this to a periodic job.

If we choose to keep it in any possible implementations, we must decide where it would live: as a GitHub workflow (we can upload the result serif output to GitHub as we're doing right now); or as a Prow job with a regular text output.

References

• Original issue that implemented the Trivy scan: Build etcd and perform security scan on container images weekly  #14991
• Migration from GitHub workflows to a Prow job: github workflows: remove release tests #19358

Why is this needed?

This is meant to open the discussion for a possible cleanup/enhancement.

[ivanvc]

/cc @ahrtr @serathius @ArkaSaha30 @jmhbnz

[jmhbnz]

Thanks for raising @ivanvc, my vote would be to either remove it as since it was introduced I can't recall this check ever adding value. If we don't have consensus to remove I agree we decouple it from release process and focus it solely on the vulnerability status of the base image.

[ahrtr]

Please proceed based on your best judgement. @ivanvc @jmhbnz @ArkaSaha30

My thoughts:

• It makes sense to run the CVE scans periodically, but suggest to also run it on each PR, reasons:
  • It won't take much time or resources;
  • Usually, periodic workflows don't get much attention, at least not immediately.
• govulncheck is great, and we should definitely keep it since it's already proven useful. However, it would be better to have an image scanning tool as well (e.g., Trivy or an alternative). The reason is simple: it scans the final deliverables (container images) and uses a black-box scanning approach, which is typically used by end users.
  • One issue I noticed: some CVEs were reported by users but not detected by Trivy. So, instead of removing it, I should think about how to improve its efficiency. Probably running it periodically can improve the situation? Or change to another tool?

[ArkaSaha30]

Thank you @ivanvc, for raising the issue. It makes sense for me to remove the trivy scan in its current form as a presubmit job and maybe have it as a periodic job to scan the latest images across 3.4, 3.5, 3.6 (which means it will also scan the static-debian12, static-debian11 base images used)

I agree with @ahrtr 's suggestions, having a periodic trivy scan(or any other CVE scanner) either as a workflow or a prowjob can help us to expedite bumps and releases in case of a CVE. As for govulncheck, k/k is already running it as a non-blocking presubmit and so are we for main, release-3.6(thanks @ivanvc ).
We can backport it for release-3.4 and release-3.5 as per the opened issue. If anyone is interested, they are welcome to take it up, otherwise I will revive the work in a couple of weeks.

[ivanvc]

We can backport it for release-3.4 and release-3.5 as per the opened issue. If anyone is interested, they are welcome to take it up, otherwise I will revive the work in a couple of weeks.

I was planning on doing it later. But definitely we can sync later to see how we can implement it. I have some ideas already.