| Message ID | 20230622120009.28939-2-andrej.valek@siemens.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v8,1/3] cve-check: add option to add additional patched CVEs | expand |
On 22 Jun 2023, at 13:00, Andrej Valek via lists.openembedded.org <andrej.valek=siemens.com@lists.openembedded.org> wrote: > - Replace CVE_CHECK_IGNORE with CVE_STATUS to be more flexible. > The CVE_STATUS should contain an information about status wich > is decoded in 3 items: > - generic status: "Ignored", "Patched" or "Unpatched" > - more detailed status enum > - description: free text describing reason for status I think this needs to be clearer about what the intended use of the keywords are. Is the canonical data the CVE_STATUS[CVE-1234-5678] attribute, and the mapping from the status there via CVE_CHECK_STATUSMAP simply for backwards compatibility with the existing file format? Is this deprecating the status fields in those files or is it just a high-level summary? Either way, that should be made clear. > +# Possible options for CVE statuses > + > +# used by this class internally when fix is detected (NVD DB version check or CVE patch file) > +CVE_CHECK_STATUSMAP[patched] = "Patched" > +# use when this class does not detect backported patch (e.g. vendor kernel repo with cherry-picked CVE patch) > +CVE_CHECK_STATUSMAP[backported-patch] = "Patched" > +# use when NVD DB does not mention patched versions of stable/LTS branches which have upstream CVE backports > +CVE_CHECK_STATUSMAP[cpe-stable-backport] = "Patched" > +# use when NVD DB does not mention correct version or does not mention any verion at all > +CVE_CHECK_STATUSMAP[fixed-version] = "Patched" It bothers me that some of these status flags are working around the fact that the CPE is incorrect, when that CPE data can be fixed. Instead of setting fixed-version, we can just mail NIST and fix the CPE. > +# used internally by this class if CVE vulnerability is detected which is not marked as fixed or ignored > +CVE_CHECK_STATUSMAP[unpatched] = "Unpatched" > +# use when CVE is confirmed by upstream but fix is still not available > +CVE_CHECK_STATUSMAP[vulnerable-investigating] = "Unpatched" > + > +# used for migration from old concept, do not use for new vulnerabilities > +CVE_CHECK_STATUSMAP[ignored] = "Ignored" > +# use when NVD DB wrongly indicates vulnerability which is actually for a different component > +CVE_CHECK_STATUSMAP[cpe-incorrect] = "Ignored" > +# use when upstream does not accept the report as a vulnerability (e.g. works as designed) > +CVE_CHECK_STATUSMAP[disputed] = "Ignored" > +# use when vulnerability depends on build or runtime configuration which is not used > +CVE_CHECK_STATUSMAP[not-applicable-config] = "Ignored" > +# use when vulnerability affects other platform (e.g. Windows or Debian) > +CVE_CHECK_STATUSMAP[not-applicable-platform] = "Ignored" > +# use when upstream acknowledged the vulnerability but does not plan to fix it > +CVE_CHECK_STATUSMAP[upstream-wontfix] = "Ignored" Is this any different to ‘disputed’? Do we expect to add a lot more statuses to this table, or for users to add their own values? It feels like maybe this should be a dict in lib/oe/cve_check.py instead of exposed in the data store. > + # Process CVE_STATUS_GROUPS to set multiple statuses and optional detail or description at once > + for cve_status_group in (d.getVar("CVE_STATUS_GROUPS") or "").split(): > + cve_group = d.getVar(cve_status_group) > + if cve_group is not None: > + for cve in cve_group.split(): > + d.setVarFlag("CVE_STATUS", cve, d.getVarFlag(cve_status_group, "status")) > + else: > + bb.warn("CVE_STATUS_GROUPS contains undefined variable %s" % cve_status_group) > +} CVE_STATUS_GROUPS isn’t documented in the class or the commit message.
On Fri, 2023-06-23 at 10:02 +0000, Ross Burton wrote: > On 22 Jun 2023, at 13:00, Andrej Valek via lists.openembedded.org > <andrej.valek=siemens.com@lists.openembedded.org> wrote: > > - Replace CVE_CHECK_IGNORE with CVE_STATUS to be more flexible. > > The CVE_STATUS should contain an information about status wich > > is decoded in 3 items: > > - generic status: "Ignored", "Patched" or "Unpatched" > > - more detailed status enum > > - description: free text describing reason for status > > I think this needs to be clearer about what the intended use of the keywords > are. > > Is the canonical data the CVE_STATUS[CVE-1234-5678] attribute, and the mapping > from the status there via CVE_CHECK_STATUSMAP simply for backwards > compatibility with the existing file format? Is this deprecating the status > fields in those files or is it just a high-level summary? Either way, that > should be made clear. > Yes, it's for backport compatibility, and extending the existing "Ignored", "Patched" statuses with reasons. > > +# Possible options for CVE statuses > > + > > +# used by this class internally when fix is detected (NVD DB version check > > or CVE patch file) > > +CVE_CHECK_STATUSMAP[patched] = "Patched" > > +# use when this class does not detect backported patch (e.g. vendor kernel > > repo with cherry-picked CVE patch) > > +CVE_CHECK_STATUSMAP[backported-patch] = "Patched" > > +# use when NVD DB does not mention patched versions of stable/LTS branches > > which have upstream CVE backports > > +CVE_CHECK_STATUSMAP[cpe-stable-backport] = "Patched" > > +# use when NVD DB does not mention correct version or does not mention any > > verion at all > > +CVE_CHECK_STATUSMAP[fixed-version] = "Patched" > > It bothers me that some of these status flags are working around the fact that > the CPE is incorrect, when that CPE data can be fixed. Instead of setting > fixed-version, we can just mail NIST and fix the CPE. > Yes, but while you're sending it, the current status has to be covered. And you don't know, if the CPE will be fixed or not. > > +# used internally by this class if CVE vulnerability is detected which is > > not marked as fixed or ignored > > +CVE_CHECK_STATUSMAP[unpatched] = "Unpatched" > > +# use when CVE is confirmed by upstream but fix is still not available > > +CVE_CHECK_STATUSMAP[vulnerable-investigating] = "Unpatched" > > + > > +# used for migration from old concept, do not use for new vulnerabilities > > +CVE_CHECK_STATUSMAP[ignored] = "Ignored" > > +# use when NVD DB wrongly indicates vulnerability which is actually for a > > different component > > +CVE_CHECK_STATUSMAP[cpe-incorrect] = "Ignored" > > +# use when upstream does not accept the report as a vulnerability (e.g. > > works as designed) > > +CVE_CHECK_STATUSMAP[disputed] = "Ignored" > > +# use when vulnerability depends on build or runtime configuration which is > > not used > > +CVE_CHECK_STATUSMAP[not-applicable-config] = "Ignored" > > +# use when vulnerability affects other platform (e.g. Windows or Debian) > > +CVE_CHECK_STATUSMAP[not-applicable-platform] = "Ignored" > > > +# use when upstream acknowledged the vulnerability but does not plan to fix > > it > > +CVE_CHECK_STATUSMAP[upstream-wontfix] = "Ignored" > > Is this any different to ‘disputed’? > Of course. In the "upstream-wontfix" status, we know, that it won't be fixed. But for "disputed" you don't know, if it's a bug or not. > Do we expect to add a lot more statuses to this table, or for users to add > their own values? It feels like maybe this should be a dict in > lib/oe/cve_check.py instead of exposed in the data store. > Exactly, know I moved it separated file, where users could extend their own statuses. The current version is just a "basement" of supported one. > > + # Process CVE_STATUS_GROUPS to set multiple statuses and optional > > detail or description at once > > + for cve_status_group in (d.getVar("CVE_STATUS_GROUPS") or "").split(): > > + cve_group = d.getVar(cve_status_group) > > + if cve_group is not None: > > + for cve in cve_group.split(): > > + d.setVarFlag("CVE_STATUS", cve, > > d.getVarFlag(cve_status_group, "status")) > > + else: > > + bb.warn("CVE_STATUS_GROUPS contains undefined variable %s" % > > cve_status_group) > > +} > > CVE_STATUS_GROUPS isn’t documented in the class or the commit message. > Added a description directly into class. > Regards, Andrej
diff --git a/meta/classes/cve-check.bbclass b/meta/classes/cve-check.bbclass index bd9e7e7445..4eb6dff7de 100644 --- a/meta/classes/cve-check.bbclass +++ b/meta/classes/cve-check.bbclass @@ -70,14 +70,48 @@ CVE_CHECK_COVERAGE ??= "1" # Skip CVE Check for packages (PN) CVE_CHECK_SKIP_RECIPE ?= "" -# Ingore the check for a given list of CVEs. If a CVE is found, -# then it is considered patched. The value is a string containing -# space separated CVE values: +# Replace NVD DB check status for a given CVE. Each of CVE has to be mentioned +# separately with optional detail and description for this status. # -# CVE_CHECK_IGNORE = 'CVE-2014-2524 CVE-2018-1234' +# CVE_STATUS[CVE-1234-0001] = "not-applicable-platform: Issue only applies on Windows" +# CVE_STATUS[CVE-1234-0002] = "fixed-version: Fixed externally" # +# CVE_CHECK_STATUSMAP[not-applicable-platform] = "Ignored" +# CVE_CHECK_STATUSMAP[fixed-version] = "Patched" +# +# CVE_CHECK_IGNORE is deprecated and CVE_STATUS has to be used instead. +# Keep CVE_CHECK_IGNORE until other layers migrate to new variables CVE_CHECK_IGNORE ?= "" +# Possible options for CVE statuses + +# used by this class internally when fix is detected (NVD DB version check or CVE patch file) +CVE_CHECK_STATUSMAP[patched] = "Patched" +# use when this class does not detect backported patch (e.g. vendor kernel repo with cherry-picked CVE patch) +CVE_CHECK_STATUSMAP[backported-patch] = "Patched" +# use when NVD DB does not mention patched versions of stable/LTS branches which have upstream CVE backports +CVE_CHECK_STATUSMAP[cpe-stable-backport] = "Patched" +# use when NVD DB does not mention correct version or does not mention any verion at all +CVE_CHECK_STATUSMAP[fixed-version] = "Patched" + +# used internally by this class if CVE vulnerability is detected which is not marked as fixed or ignored +CVE_CHECK_STATUSMAP[unpatched] = "Unpatched" +# use when CVE is confirmed by upstream but fix is still not available +CVE_CHECK_STATUSMAP[vulnerable-investigating] = "Unpatched" + +# used for migration from old concept, do not use for new vulnerabilities +CVE_CHECK_STATUSMAP[ignored] = "Ignored" +# use when NVD DB wrongly indicates vulnerability which is actually for a different component +CVE_CHECK_STATUSMAP[cpe-incorrect] = "Ignored" +# use when upstream does not accept the report as a vulnerability (e.g. works as designed) +CVE_CHECK_STATUSMAP[disputed] = "Ignored" +# use when vulnerability depends on build or runtime configuration which is not used +CVE_CHECK_STATUSMAP[not-applicable-config] = "Ignored" +# use when vulnerability affects other platform (e.g. Windows or Debian) +CVE_CHECK_STATUSMAP[not-applicable-platform] = "Ignored" +# use when upstream acknowledged the vulnerability but does not plan to fix it +CVE_CHECK_STATUSMAP[upstream-wontfix] = "Ignored" + # Layers to be excluded CVE_CHECK_LAYER_EXCLUDELIST ??= "" @@ -88,6 +122,24 @@ CVE_CHECK_LAYER_INCLUDELIST ??= "" # set to "alphabetical" for version using single alphabetical character as increment release CVE_VERSION_SUFFIX ??= "" +python () { + # Fallback all CVEs from CVE_CHECK_IGNORE to CVE_STATUS + cve_check_ignore = d.getVar("CVE_CHECK_IGNORE") + if cve_check_ignore: + bb.warn("CVE_CHECK_IGNORE is deprecated in favor of CVE_STATUS") + for cve in (d.getVar("CVE_CHECK_IGNORE") or "").split(): + d.setVarFlag("CVE_STATUS", cve, "ignored") + + # Process CVE_STATUS_GROUPS to set multiple statuses and optional detail or description at once + for cve_status_group in (d.getVar("CVE_STATUS_GROUPS") or "").split(): + cve_group = d.getVar(cve_status_group) + if cve_group is not None: + for cve in cve_group.split(): + d.setVarFlag("CVE_STATUS", cve, d.getVarFlag(cve_status_group, "status")) + else: + bb.warn("CVE_STATUS_GROUPS contains undefined variable %s" % cve_status_group) +} + def generate_json_report(d, out_path, link_path): if os.path.exists(d.getVar("CVE_CHECK_SUMMARY_INDEX_PATH")): import json @@ -260,7 +312,7 @@ def check_cves(d, patched_cves): """ Connect to the NVD database and find unpatched cves. """ - from oe.cve_check import Version, convert_cve_version + from oe.cve_check import Version, convert_cve_version, decode_cve_status pn = d.getVar("PN") real_pv = d.getVar("PV") @@ -282,7 +334,12 @@ def check_cves(d, patched_cves): bb.note("Recipe has been skipped by cve-check") return ([], [], [], []) - cve_ignore = d.getVar("CVE_CHECK_IGNORE").split() + # Convert CVE_STATUS into ignored CVEs and check validity + cve_ignore = [] + for cve in (d.getVarFlags("CVE_STATUS") or {}): + decoded_status, _, _ = decode_cve_status(d, cve) + if decoded_status == "Ignored": + cve_ignore.append(cve) import sqlite3 db_file = d.expand("file:${CVE_CHECK_DB_FILE}?mode=ro") @@ -413,6 +470,8 @@ def cve_write_data_text(d, patched, unpatched, ignored, cve_data): CVE manifest if enabled. """ + from oe.cve_check import decode_cve_status + cve_file = d.getVar("CVE_CHECK_LOG") fdir_name = d.getVar("FILE_DIRNAME") layer = fdir_name.split("/")[-3] @@ -441,20 +500,27 @@ def cve_write_data_text(d, patched, unpatched, ignored, cve_data): is_patched = cve in patched is_ignored = cve in ignored + status = "Unpatched" if (is_patched or is_ignored) and not report_all: continue + if is_ignored: + status = "Ignored" + elif is_patched: + status = "Patched" + else: + # default value of status is Unpatched + unpatched_cves.append(cve) write_string += "LAYER: %s\n" % layer write_string += "PACKAGE NAME: %s\n" % d.getVar("PN") write_string += "PACKAGE VERSION: %s%s\n" % (d.getVar("EXTENDPE"), d.getVar("PV")) write_string += "CVE: %s\n" % cve - if is_ignored: - write_string += "CVE STATUS: Ignored\n" - elif is_patched: - write_string += "CVE STATUS: Patched\n" - else: - unpatched_cves.append(cve) - write_string += "CVE STATUS: Unpatched\n" + write_string += "CVE STATUS: %s\n" % status + _, detail, description = decode_cve_status(d, cve) + if detail: + write_string += "CVE DETAIL: %s\n" % detail + if description: + write_string += "CVE DESCRIPTION: %s\n" % description write_string += "CVE SUMMARY: %s\n" % cve_data[cve]["summary"] write_string += "CVSS v2 BASE SCORE: %s\n" % cve_data[cve]["scorev2"] write_string += "CVSS v3 BASE SCORE: %s\n" % cve_data[cve]["scorev3"] @@ -516,6 +582,8 @@ def cve_write_data_json(d, patched, unpatched, ignored, cve_data, cve_status): Prepare CVE data for the JSON format, then write it. """ + from oe.cve_check import decode_cve_status + output = {"version":"1", "package": []} nvd_link = "https://nvd.nist.gov/vuln/detail/" @@ -576,6 +644,11 @@ def cve_write_data_json(d, patched, unpatched, ignored, cve_data, cve_status): "status" : status, "link": issue_link } + _, detail, description = decode_cve_status(d, cve) + if detail: + cve_item["detail"] = detail + if description: + cve_item["description"] = description cve_list.append(cve_item) package_data["issue"] = cve_list diff --git a/meta/lib/oe/cve_check.py b/meta/lib/oe/cve_check.py index dbaa0b373a..5bf3caac47 100644 --- a/meta/lib/oe/cve_check.py +++ b/meta/lib/oe/cve_check.py @@ -130,6 +130,13 @@ def get_patched_cves(d): if not fname_match and not text_match: bb.debug(2, "Patch %s doesn't solve CVEs" % patch_file) + # Search for additional patched CVEs + for cve in (d.getVarFlags("CVE_STATUS") or {}): + decoded_status, _, _ = decode_cve_status(d, cve) + if decoded_status == "Patched": + bb.debug(2, "CVE %s is additionally patched" % cve) + patched_cves.add(cve) + return patched_cves @@ -218,3 +225,21 @@ def convert_cve_version(version): return version + update +def decode_cve_status(d, cve): + """ + Convert CVE_STATUS into status, detail and description. + """ + status = d.getVarFlag("CVE_STATUS", cve) + if status is None: + return ("", "", "") + + status_split = status.split(':', 1) + detail = status_split[0] + description = status_split[1].strip() if (len(status_split) > 1) else "" + + status_mapping = d.getVarFlag("CVE_CHECK_STATUSMAP", detail) + if status_mapping is None: + bb.warn('Invalid detail %s for CVE_STATUS[%s] = "%s", fallback to Unpatched' % (detail, cve, status)) + status_mapping = "Unpatched" + + return (status_mapping, detail, description)