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

fix: update alpine matchers to use SecDB entries as fixed information rather than vuln source #1318

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

spiffcs
Copy link
Contributor

@spiffcs spiffcs commented May 25, 2023

Summary

The alpine matcher needs to be updated to behave a little differently from the other distro specific matchers.

Secdb is a collection of records that denotes if a related CVE has been fixed, and the version the package was fixed in.

Basic Example:

# https://secdb.alpinelinux.org/v3.18/community.yaml
- pkg:
    name: advancecomp
    secfixes:
      2.1-r2:
      - CVE-2019-9210
      2.4-r0:
      - CVE-2022-35014
      - CVE-2022-35015
      - CVE-2022-35016
      - CVE-2022-35017
      - CVE-2022-35018
      - CVE-2022-35019
      - CVE-2022-35020

In the above record, if:

  • the package advancecomp is found,
  • and it is found as a part of scanning the v3.18 distro,
  • and it's version is found to be exactly 2.1-r2,

Then, given the package has matched the NVD record CVE-2019-9210, we can correctly remove that record from the final match results.

Secdb informs which CVE should be turned off (not reported) given an exact version and distro match.

In the wild example showing the matching distinction

I’ve identified an example from previous security updates where how we use this data could matter as different feeds update:
https://nvd.nist.gov/vuln/detail/CVE-2023-27561#VulnChangeHistorySection

The above link shows the change history for CVE-2023-27561. It indicates how/when the nvd record updated.

NVD Timeline

The initial analysis for this was done on 3/10/2023 9:09:57 AM:
Constraint: versions up to (including) 1.1.4

A second modified analysis was done on 04/05/2023:
Constraint: versions up to and including 1.1.5

SecDB Timeline

Alpine iterated and issued a secfix twice for this dependency. These fixes happened between nvd updates:
3-25-23
https://git.alpinelinux.org/aports/commit/?id=aed110c1af5fa541884541180f34cb4a4bd32b14

3-29-2023
As well as a security upgrade to v1.1.5:
https://git.alpinelinux.org/aports/commit/community/runc/APKBUILD?h=v3.18.0&id=87362059d03b6abc02a72b6bb5d03231e6e574c7

Full commit history for that file for the alpine:3.18 branch
https://git.alpinelinux.org/aports/log/community/runc/APKBUILD?h=v3.18.0

Matching Summary

Given the above timeline - our alpine namespace for 3.18 would have two constraints that we intuited from secdb. One issued on 3-25, the other on 3-29.

282831	CVE-2023-27561	runc	alpine:distro:alpine:3.18		< 1.1.4-r0	apk		[{"id":"CVE-2023-27561","namespace":"nvd:cpe"}]	["1.1.4-r0"]	fixed	
282832	CVE-2023-27561	runc	alpine:distro:alpine:3.18		< 1.1.4-r7	apk		[{"id":"CVE-2023-27561","namespace":"nvd:cpe"}]	["1.1.4-r7"]	fixed	

Before 3-25-23 grype would use the NVD match as no secdb data has been issued. The constraint would be versions up to (including) 1.1.4
When the initial SecDB record comes out on 3-25-23 the constraint < 1.1.4-r0 from the alpine namespace is correct. The “fix” correctly indicates 1.1.4-r0 which is logically > 1.1.4

The maintainers of secdb make no guarantee for future rc candidates here and defer to nvd for future matching.
On 3-29-2023 the constraint < 1.1.4-r7 is less correct given that it can be inclusive of rc-0 and report a false positive.

On 04-05-23, NVD eventually catches up and issues versions up to and including 1.1.5 -

In this case, grype should still be able to report that the sec fixes of r0 and r7 as being excluded from this new match, while potential instances of r1 - r6 have the vulnerability reported against them. This is helpful for users who are in the process of updating or evaluating where they stand against the current set of matches and SecFixes.

On 04-05-23 NVD becomes the superior constraint and the alpine namespace instead returns to its function as an exclusion feed of versions that may have been backdated from the new NVD constraint

Notes:

  • matcher.go has been updated to secDBVulnerabilities are now considered secDBVulnFixes
    • There is some incongruency here in that they are still of the Vulnerability type, but the logic has been changed to focus on fixed versions rather than building a constraint. The original constraint logic is incorrect given that the secdb source only promotes singularly fixed versions and makes no guarantee to future/prior versions.
  • vuln.Fix.Versions from secDB is now the arbiter for if a given package should include its NVD match or not
  • deduplicateMatches has been removed as we are no longer using secDB as a source for vulnerability information
  • matcher tests have been updated to reflect that secDB data should only be used to inform if an NVD match should be included or not

Removed/Altered Tests:

  • TestSecDBOnlyMatch <-- We no longer match against SecDB
    • Instead we test of the data is used correctly to turn off a corresponding NVD match
  • TestBothSecdbAndNvdMatches_DifferentPackageName <--- We now test that related records are correlated given a package name mismatch
  • TestNvdMatchesWithSecDBFix <--- We now test that NVD matches are removed given a correct fix from SecDB
  • TestDistroMatchBySourceIndirection <--- We no longer do an alpine distro match given the prupose of SecDB data

To Be Determined

- Should secdb fix information that was not applied be appended as a new field to vulnerability?
- Should we keep the name as the `apk-matcher` given the list was run by the secdb fix information and not technically matched to it

we only want to match on cpe without sec db fixes

Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>
@spiffcs spiffcs marked this pull request as draft May 25, 2023 20:33
@spiffcs
Copy link
Contributor Author

spiffcs commented May 25, 2023

@luhring ^ re #970 does this look correct - I still need to update the tests (since we're not matching on specific cases anymore)

Summary of changes:

  • we're no longer de duplicating matches since we're not doing directMatches against the apk data
  • We only want to find CPE matches that are against NVD

I don't think we have a mechanism yet to use the feed to flip OFF(remove) vulnerabilities - but am taking a look at this now - I just wanted to be sure this was the spot you were discussing =) - apologies for the stale issue here!

@luhring
Copy link
Contributor

luhring commented Oct 2, 2023

Super late response! 🤦 Yes I think this is looking good!

@spiffcs
Copy link
Contributor Author

spiffcs commented Oct 3, 2023

Awesome thanks! I'll go ahead and rebase this and get the tests updated so we can get it released =)

* main: (137 commits)
  chore(deps): bump actions/checkout from 4.1.0 to 4.1.1 (#1564)
  Add --ignore-states flag for ignoring findings with specific fix states (#1473)
  feat: update go-sarif library to use latest release (#1563)
  bump clio to get stderr reporting fix (#1561)
  chore(deps): bump github.com/gabriel-vasile/mimetype from 1.4.2 to 1.4.3 (#1558)
  chore(deps): bump github.com/charmbracelet/lipgloss from 0.9.0 to 0.9.1 (#1557)
  Add checksum signing (#1535)
  chore(deps): bump golang.org/x/net from 0.16.0 to 0.17.0 (#1554)
  feat: disable CPE-based matching for GHSA ecosystems by default (#1412)
  chore(deps): bump github.com/google/go-cmp from 0.5.9 to 0.6.0 (#1552)
  chore(deps): update Syft to v0.93.0 (#1550)
  chore(deps): bump gorm.io/gorm from 1.25.4 to 1.25.5 (#1547)
  chore(deps): bump github.com/charmbracelet/lipgloss from 0.8.0 to 0.9.0 (#1548)
  chore(deps): bump github.com/hashicorp/go-getter from 1.7.2 to 1.7.3 (#1549)
  chore(deps): bump ossf/scorecard-action from 2.2.0 to 2.3.0 (#1544)
  fix: empty descriptor name and version (#1542)
  chore: removes unnecessary conditional (#1539)
  chore(deps): bump github.com/gkampitakis/go-snaps from 0.4.10 to 0.4.11 (#1533)
  chore(deps): update Syft to v0.92.0 (#1527)
  chore(deps): update bootstrap tools to latest versions (#1524)
  ...
Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>
Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>
Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>
@spiffcs spiffcs marked this pull request as ready for review October 23, 2023 20:21
Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>
@spiffcs
Copy link
Contributor Author

spiffcs commented Oct 23, 2023

Unit/Integration tests should be fixed now to reflect the CPE only matching directive now - the final part of this PR is to update the quality gate data.

@spiffcs spiffcs changed the title fix: update alpine matchers to no longer search by package distro fix: update alpine matchers to use SecDB entries as fixed information rather than vuln source Oct 24, 2023
@wagoodman
Copy link
Contributor

wagoodman commented Oct 24, 2023

Something to note in the example forrunc I think there is a data inconsistency in the upstream secdb data. Here's the secdb fixes in grype db:

# from grypedb

282831	CVE-2023-27561	runc	alpine:distro:alpine:3.18		< 1.1.4-r0	apk		[{"id":"CVE-2023-27561","namespace":"nvd:cpe"}]	["1.1.4-r0"]	fixed	
282832	CVE-2023-27561	runc	alpine:distro:alpine:3.18		< 1.1.4-r7	apk		[{"id":"CVE-2023-27561","namespace":"nvd:cpe"}]	["1.1.4-r7"]	fixed	
Be

Which matches the secfix comment from the APKBUILD file at v3.18.0 (through -stable):

# secfixes:
#   1.1.4-r0:
#     - CVE-2023-25809
#     - CVE-2023-27561
#     - CVE-2023-28642
#   1.1.4-r7:
#     - CVE-2023-27561
#   1.1.2-r0:
#     - CVE-2022-29162
#   1.0.3-r0:
#     - CVE-2021-43784
#   1.0.0_rc95-r0:
#     - CVE-2021-30465
#   1.0.0_rc10-r0:
#     - CVE-2019-19921
#   1.0.0_rc9-r0:
#     - CVE-2019-16884
#   1.0.0_rc7-r0:
#     - CVE-2019-5736

Note the 1.1.4-r0 and 1.1.4-r7

However, the commit that put in the patch and bumped the version to 1.1.5 seems to have copy-pasted the wrong version:

diff --git a/community/runc/APKBUILD b/community/runc/APKBUILDindex 12b009c0a06..fc56095c90c 100644--- a/community/runc/APKBUILD+++ b/community/runc/APKBUILD@@ -4,19 +4,22 @@ pkgname=runc pkgdesc="CLI tool for spawning and running containers according to the OCI specification" url="https://www.opencontainers.org"-_commit=5fd4c4d144137e991c4acebb2146ab1483a97925-pkgver=1.1.4-pkgrel=7+_commit=f19387a6bec4944c770f7668ab51c4348d9c2f38+pkgver=1.1.5+pkgrel=0 arch="all" license="Apache-2.0" makedepends="bash go go-md2man libseccomp-dev libtool" subpackages="$pkgname-doc" source="https://github.com/opencontainers/runc/archive/v$pkgver/runc-$pkgver.tar.gz-	CVE-2023-27561.patch 	" options="!check"  # secfixes:+#   1.1.4-r0:+#     - CVE-2023-25809+#     - CVE-2023-27561+#     - CVE-2023-28642 #   1.1.4-r7: #     - CVE-2023-27561 #   1.1.2-r0:@@ -47,6 +50,5 @@ package() { }  sha512sums="-c8e79ad839964680d29ab56a4de255f91192741951673025da6889c544a232d4d392db2da8005d8e22999a37bfbc9c9fe7f6043b165bc4edc2f2a29261d8a3d6  runc-1.1.4.tar.gz-47aa4d15e4b0e0ea419566361f95b26af24e71ff4b77e440f23b9f6f6c62cdb56e4e290f8d6c2b9f2622b76f0d5201b975e146a723b2ef64d1585499d7680323  CVE-2023-27561.patch+f3cc9b93b0fe8a4341d410010fe584febb8e975ec9e0fd569d7dff33ab74c5821a2e0c40b7aeafd6b90991a50eae9c352342437f09cf6884dc850ceccdc68944  runc-1.1.5.tar.gz "
--


diff --git a/community/runc/APKBUILD b/community/runc/APKBUILD
index 12b009c0a06..fc56095c90c 100644
--- a/[community/runc/APKBUILD](https://git.alpinelinux.org/aports/tree/community/runc/APKBUILD?h=3.18-stable&id=2782d1ed755d99b0864271e13759ec666656b493)
+++ b/[community/runc/APKBUILD](https://git.alpinelinux.org/aports/tree/community/runc/APKBUILD?h=3.18-stable&id=87362059d03b6abc02a72b6bb5d03231e6e574c7)
@@ -4,19 +4,22 @@
 pkgname=runc
 pkgdesc="CLI tool for spawning and running containers according to the OCI specification"
 url="https://www.opencontainers.org"
-_commit=5fd4c4d144137e991c4acebb2146ab1483a97925
-pkgver=1.1.4
-pkgrel=7
+_commit=f19387a6bec4944c770f7668ab51c4348d9c2f38
+pkgver=1.1.5
+pkgrel=0
 arch="all"
 license="Apache-2.0"
 makedepends="bash go go-md2man libseccomp-dev libtool"
 subpackages="$pkgname-doc"
 source="https://github.com/opencontainers/runc/archive/v$pkgver/runc-$pkgver.tar.gz
-	CVE-2023-27561.patch
 	"
 options="!check"
 
 # secfixes:
+#   1.1.4-r0:
+#     - CVE-2023-25809
+#     - CVE-2023-27561
+#     - CVE-2023-28642
 #   1.1.4-r7:
 #     - CVE-2023-27561
 #   1.1.2-r0:
@@ -47,6 +50,5 @@ package() {
 }
 
 sha512sums="
-c8e79ad839964680d29ab56a4de255f91192741951673025da6889c544a232d4d392db2da8005d8e22999a37bfbc9c9fe7f6043b165bc4edc2f2a29261d8a3d6  runc-1.1.4.tar.gz
-47aa4d15e4b0e0ea419566361f95b26af24e71ff4b77e440f23b9f6f6c62cdb56e4e290f8d6c2b9f2622b76f0d5201b975e146a723b2ef64d1585499d7680323  CVE-2023-27561.patch
+f3cc9b93b0fe8a4341d410010fe584febb8e975ec9e0fd569d7dff33ab74c5821a2e0c40b7aeafd6b90991a50eae9c352342437f09cf6884dc850ceccdc68944  runc-1.1.5.tar.gz
 "

Note that the package version is bumped to 1.1.5 but the comment is for 1.1.4, probably a copy-paste problem. The 1.1.5 is consistent with the surrounding git references and the audit trail in the upstream PR opencontainers/runc#3785 .

Amending the conclusion:

In this case, grype should still be able to report that the sec fix r7* as being excluded from this new match, while potential instances of r0* - r6 have the vulnerability reported against them. This is helpful for users who are in the process of updating or evaluating where they stand against the current set of matches and SecFixes.

@wagoodman
Copy link
Contributor

Reasons for quality gate failure:
   - current F1 score is lower than the latest release F1 score: current=0.68 latest=0.68 image=docker.io/anchore/test_images@sha256:10008791acbc5866de04108746a02a0c4029ce3a4400a9b3dad45d7f2245f9da
   - current F1 score is lower than the latest release F1 score: current=0.51 latest=0.69 image=docker.io/anchore/test_images@sha256:ba42ded8613fc643d407a050faf5ab48cfb405ad3ef2015bf6feeb5dff44738d
   - current indeterminate matches % is greater than 10%: current=35.71% image=docker.io/anchore/test_images@sha256:ba42ded8613fc643d407a050faf5ab48cfb405ad3ef2015bf6feeb5dff44738d
   - current false negatives is greater than the latest release false negatives: current=20 latest=12 image=docker.io/anchore/test_images@sha256:ba42ded8613fc643d407a050faf5ab48cfb405ad3ef2015bf6feeb5dff44738d
   - current F1 score is lower than the latest release F1 score: current=0.71 latest=0.80 image=docker.io/anchore/test_images@sha256:5763c8a225f950961bf01ddec68e36f18e236130e182f2b9290a6e03b9777bfe
   - current indeterminate matches % is greater than 10%: current=10.64% image=docker.io/anchore/test_images@sha256:5763c8a225f950961bf01ddec68e36f18e236130e182f2b9290a6e03b9777bfe
   - current false negatives is greater than the latest release false negatives: current=13 latest=7 image=docker.io/anchore/test_images@sha256:5763c8a225f950961bf01ddec68e36f18e236130e182f2b9290a6e03b9777bfe
   - current F1 score is lower than the latest release F1 score: current=0.76 latest=0.79 image=docker.io/anchore/test_images@sha256:d1819e59e89e8ea90073460acb4ebb2ee18ccead9fa880dae91e8fc61b19ca1c
   - current indeterminate matches % is greater than 10%: current=19.05% image=docker.io/anchore/test_images@sha256:d1819e59e89e8ea90073460acb4ebb2ee18ccead9fa880dae91e8fc61b19ca1c
   - current false negatives is greater than the latest release false negatives: current=2 latest=1 image=docker.io/anchore/test_images@sha256:d1819e59e89e8ea90073460acb4ebb2ee18ccead9fa880dae91e8fc61b19ca1c
   - current F1 score is lower than the latest release F1 score: current=0.79 latest=0.97 image=docker.io/anchore/test_images@sha256:01c78cee3fe398bf1f77566177770b07f1d2af01753c2434cb0735bd43a078b6
   - current indeterminate matches % is greater than 10%: current=16.67% image=docker.io/anchore/test_images@sha256:01c78cee3fe398bf1f77566177770b07f1d2af01753c2434cb0735bd43a078b6
   - current false negatives is greater than the latest release false negatives: current=5 latest=1 image=docker.io/anchore/test_images@sha256:01c78cee3fe398bf1f77566177770b07f1d2af01753c2434cb0735bd43a078b6
   - current F1 score is lower than the latest release F1 score: current=0.11 latest=1.00 image=docker.io/anchore/test_images@sha256:55c9ba4e24e15c0467a071d93fead0990b8f04bb60b359b4056a997598aa56a1
   - current indeterminate matches % is greater than 10%: current=47.83% image=docker.io/anchore/test_images@sha256:55c9ba4e24e15c0467a071d93fead0990b8f04bb60b359b4056a997598aa56a1
   - current false negatives is greater than the latest release false negatives: current=5 latest=0 image=docker.io/anchore/test_images@sha256:55c9ba4e24e15c0467a071d93fead0990b8f04bb60b359b4056a997598aa56a1
   - current F1 score is lower than the latest release F1 score: current=0.13 latest=1.00 image=docker.io/anchore/test_images@sha256:6749b1509fc4dd3f2b4e8688325fc5d447751bc9ae3be10c0f1fb92ec062b798
   - current indeterminate matches % is greater than 10%: current=47.83% image=docker.io/anchore/test_images@sha256:6749b1509fc4dd3f2b4e8688325fc5d447751bc9ae3be10c0f1fb92ec062b798
   - current false negatives is greater than the latest release false negatives: current=2 latest=0 image=docker.io/anchore/test_images@sha256:6749b1509fc4dd3f2b4e8688325fc5d447751bc9ae3be10c0f1fb92ec062b798
   - current F1 score is lower than the latest release F1 score: current=0.76 latest=1.00 image=docker.io/anchore/test_images@sha256:fe242a3a63699425317fba0a749253bceb700fb3d63e7a0f6497f53a587e38c5
   - current indeterminate matches % is greater than 10%: current=40.00% image=docker.io/anchore/test_images@sha256:fe242a3a63699425317fba0a749253bceb700fb3d63e7a0f6497f53a587e38c5
   - current false negatives is greater than the latest release false negatives: current=1 latest=0 image=docker.io/anchore/test_images@sha256:fe242a3a63699425317fba0a749253bceb700fb3d63e7a0f6497f53a587e38c5
   - current indeterminate matches % is greater than 10%: current=100.00% image=docker.io/anchore/test_images@sha256:7790691e5efae8bfe9cf4a[4447](https://github.com/anchore/grype/actions/runs/6618795428/job/17977966722?pr=1318#step:4:4448)312318d8daaf05ffd5f265ae913edf660f4653
   - current F1 score is lower than the latest release F1 score: current=0.82 latest=0.83 image=docker.io/anchore/test_images@sha256:58637f273108e3e9eb4df4d73f7b6b1da303cbbf64f65e65fb7762482f2de63d
   - current indeterminate matches % is greater than 10%: current=12.50% image=docker.io/anchore/test_images@sha256:58637f273108e3e9eb4df4d73f7b6b1da303cbbf64f65e65fb7762482f2de63d
   - current F1 score is lower than the latest release F1 score: current=0.52 latest=0.69 image=docker.io/anchore/test_images@sha256:a287a0ff98ac343aa710f4f4258d7198e240e9d416d5c7274663564202f832fb
   - current false negatives is greater than the latest release false negatives: current=28 latest=17 image=docker.io/anchore/test_images@sha256:a287a0ff98ac343aa710f4f4258d7198e240e9d416d5c7274663564202f832fb
   - current F1 score is lower than the latest release F1 score: current=0.93 latest=0.95 image=docker.io/anchore/test_images@sha256:c27b02c6322180fd8a7a3097d2b430bfdf9ea52ecf136edf258458e82f2c6f21
   - current indeterminate matches % is greater than 10%: current=10.40% image=docker.io/anchore/test_images@sha256:c27b02c6322180fd8a7a3097d2b430bfdf9ea52ecf136edf258458e82f2c6f21
   - current false negatives is greater than the latest release false negatives: current=6 latest=0 image=docker.io/anchore/test_images@sha256:c27b02c6322180fd8a7a3097d2b430bfdf9ea52ecf136edf258458e82f2c6f21
   - current F1 score is lower than the latest release F1 score: current=0.74 latest=0.91 image=docker.io/anchore/test_images@sha256:0825acea611c7c5cc792bc7cc20de44d7413fd287dc5afc4aab9c1891d037b4f
   - current indeterminate matches % is greater than 10%: current=28.89% image=docker.io/anchore/test_images@sha256:0825acea611c7c5cc792bc7cc20de44d7413fd287dc5afc4aab9c1891d037b4f
   - current indeterminate matches % is greater than 10%: current=100.00% image=cgr.dev/chainguard/wolfi-base@sha256:be3834598c3c4b76ace6a866edcbbe1fa18086f9ee238b57769e4d230cd7d507

I think the next step is to ensure the labels reflect the truth ("is the artifact vulnerable or not" and not biased towards a matching strategy) and if there is a corrective action on the labels needed we should make them before considering merging this feature.

That being said, I'm still skeptical of relying on CPE-based matching as heavily as this proposal suggests and having that as the default matching strategy for alpine-based images. Even if this proposal is theoretically correct, practically correct is more important. That is if the theory states we must have accurate CPEs to match, but in practice accurate CPEs are difficult to obtain, then there is a problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Alpine secdb should only be used to remove matches, never to add matches
4 participants