Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add trivyignore file to ignore false positive CVEs in security scan #1546

Merged
merged 1 commit into from
Sep 5, 2024

Conversation

dandelany
Copy link
Collaborator

Description

This change mitigates certain false positives that appear during the security scan of our aerie-postgres container by ignoring them with a .trivyignore file, used by the trivy action which runs our scans.

Verification

To determine that these CVE's were in fact false positives, I worked with @skovati and we did the following:

  • Checked the output from the aerie-postgres Publish workflow, security scan step, and found that the only remaining CVEs are the three in this PR, all associated with version 1.18.2 of gobinary's stdlib package
  • On this DockerHub page we confirmed that the only usage of this stdlib package is from gosu, a small utility used to setup the postgres container
  • We found this page on the gosu docs which explains how most Go CVEs are false positives for this tool, and how to confirm.
  • We weren't able to get the ./govulncheck-with-excludes.sh script provided by this document to work correctly. However we validated a different way, using the regular govulncheck tool which checks specifically which parts of the Go library are being used by a binary:
    • Copied the gosu binary from the postgres machine to local machine with docker cp
    • Ran the regular govulncheck tool against this binary with the command govulncheck -mode=binary gosu
    • This produced only a single applicable CVE: GO-2023-1840 - Unsafe behavior in setuid/setgid binaries in runtime
    • ...and ^this specific CVE is explicitly excluded by the govulncheck-with-excludes script above, which explains in comments why it is a false positive/mitigated risk

Therefore, we believe these other gobinary stdlib CVEs from the security scan are all false positives in our case, and should be excluded from our scans.

Future work

Better process for tracking these down in the future?

@dandelany dandelany requested a review from a team as a code owner September 4, 2024 21:57
@dandelany dandelany requested review from skovati and Mythicaeda and removed request for jmdelfa and srschaffJPL September 4, 2024 21:57
@dandelany dandelany added the publish Tells GH to publish docker images for this PR label Sep 4, 2024
Copy link
Contributor

@skovati skovati left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good ole software supply chain 🙃

.github/config/.trivyignore Outdated Show resolved Hide resolved
.github/workflows/publish.yml Outdated Show resolved Hide resolved
@skovati
Copy link
Contributor

skovati commented Sep 4, 2024

Looks good of course besides the .trivyignore path issue

@Mythicaeda
Copy link
Contributor

Mythicaeda commented Sep 5, 2024

I took another look at the workflow. It turns out the repo isn't getting checked out at all in the scan subjob, meaning the file is inaccessible. This can be solved by either publishing the trivyignore as an artifact during the containers job and downloading it during the scan job or by checking out the repo at the start of the scan job.

@dandelany
Copy link
Collaborator Author

It turns out the repo isn't getting checked out at all in the scan subjob, meaning the file is inaccessible

Got it - thank you! Fixed. I had assumed that since the job needs: containers which needs: init it would be accessible, but I guess not. Seems to be passing all checks now (finally).

@dandelany dandelany force-pushed the ops/trivyignore-false-positives branch from 7426137 to e1bc357 Compare September 5, 2024 17:50
.trivyignore Outdated Show resolved Hide resolved
@Mythicaeda
Copy link
Contributor

I had assumed that since the job needs: containers which needs: init it would be accessible, but I guess not.

Yeah, the quick summary on that is that each subjob is executed on a different runner (you can actually occasionally see a waiting for runner message if you watch the later jobs of, for example, the DB Comparison workflow), so the workspace is completely fresh going into each subjob.

@dandelany dandelany force-pushed the ops/trivyignore-false-positives branch from e1bc357 to ec160c3 Compare September 5, 2024 18:11
@dandelany dandelany merged commit 647c66d into develop Sep 5, 2024
22 checks passed
@dandelany dandelany deleted the ops/trivyignore-false-positives branch September 5, 2024 19:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
publish Tells GH to publish docker images for this PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants