[OE-core][PATCH v3 1/5] cve-check: annotate CVEs during analysis

[OE-core][PATCH v3 1/5] cve-check: annotate CVEs during analysis

[Marta Rybczynska]

Add status information for each CVE under analysis.

Previously the information passed between different function of the
cve-check class included only tables of patched, unpatched, ignored
vulnerabilities and the general status of the recipe.

The VEX work requires more information, and we need to pass them
between different functions, so that it can be enriched as the
analysis progresses. Instead of multiple tables, use a single one
with annotations for each CVE encountered. For example, a patched
CVE will have:

{"abbrev-status": "Patched", "status": "version-not-in-range"}

abbrev-status contains the general status (Patched, Unpatched,
Ignored and Unknown that will be added in the VEX code)
status contains more detailed information that can come from
CVE_STATUS and the analysis.

Additional fields of the annotation include for example the name
of the patch file fixing a given CVE.

The side-effect of this change is that all entries from CVE_STATUS
are available in the result file. That includes entries from
the optional file cve-extra-exclusions.inc even if they might have
no link with the recipe (apply to a different package). This will
be fixed by moving all entries from that file to appropriate recipes.

From now on, CVE_STATUS should be added directly in the recipe file
or in include files added only to affected recipes.

Signed-off-by: Marta Rybczynska <marta.rybczynska@syslinbit.com>
Signed-off-by: Samantha Jalabert <samantha.jalabert@syslinbit.com>
---
 meta/classes/cve-check.bbclass | 208 +++++++++++++++++----------------
 meta/lib/oe/cve_check.py       |  12 +-
 2 files changed, 115 insertions(+), 105 deletions(-)

diff --git a/meta/classes/cve-check.bbclass b/meta/classes/cve-check.bbclass
index 93a2a1413d..504310514e 100644
--- a/meta/classes/cve-check.bbclass
+++ b/meta/classes/cve-check.bbclass
@@ -188,10 +188,10 @@ python do_cve_check () {
                 patched_cves = get_patched_cves(d)
             except FileNotFoundError:
                 bb.fatal("Failure in searching patches")
-            ignored, patched, unpatched, status = check_cves(d, patched_cves)
-            if patched or unpatched or (d.getVar("CVE_CHECK_COVERAGE") == "1" and status):
-                cve_data = get_cve_info(d, patched + unpatched + ignored)
-                cve_write_data(d, patched, unpatched, ignored, cve_data, status)
+            cve_data, status = check_cves(d, patched_cves)
+            if len(cve_data) or (d.getVar("CVE_CHECK_COVERAGE") == "1" and status):
+                get_cve_info(d, cve_data)
+                cve_write_data(d, cve_data, status)
         else:
             bb.note("No CVE database found, skipping CVE check")
 
@@ -294,7 +294,51 @@ ROOTFS_POSTPROCESS_COMMAND:prepend = "${@'cve_check_write_rootfs_manifest ' if d
 do_rootfs[recrdeptask] += "${@'do_cve_check' if d.getVar('CVE_CHECK_CREATE_MANIFEST') == '1' else ''}"
 do_populate_sdk[recrdeptask] += "${@'do_cve_check' if d.getVar('CVE_CHECK_CREATE_MANIFEST') == '1' else ''}"
 
-def check_cves(d, patched_cves):
+def cve_is_ignored(d, cve_data, cve):
+    if cve not in cve_data:
+        return False
+    if cve_data[cve]['abbrev-status'] == "Ignored":
+        return True
+    return False
+
+def cve_is_patched(d, cve_data, cve):
+    if cve not in cve_data:
+        return False
+    if cve_data[cve]['abbrev-status'] == "Patched":
+        return True
+    return False
+
+def cve_update(d, cve_data, cve, entry):
+    # If no entry, just add it
+    if cve not in cve_data:
+        cve_data[cve] = entry
+        return
+    # If we are updating, there might be change in the status
+    bb.debug("Trying CVE entry update for %s from %s to %s" % (cve, cve_data[cve]['abbrev-status'], entry['abbrev-status']))
+    if cve_data[cve]['abbrev-status'] == "Unknown":
+        cve_data[cve] = entry
+        return
+    if cve_data[cve]['abbrev-status'] == entry['abbrev-status']:
+        return
+    # Update like in {'abbrev-status': 'Patched', 'status': 'version-not-in-range'} to {'abbrev-status': 'Unpatched', 'status': 'version-in-range'}
+    if entry['abbrev-status'] == "Unpatched" and cve_data[cve]['abbrev-status'] == "Patched":
+        if entry['status'] == "version-in-range" and cve_data[cve]['status'] == "version-not-in-range":
+            # New result from the scan, vulnerable
+            cve_data[cve] = entry
+            bb.debug("CVE entry %s update from Patched to Unpatched from the scan result" % cve)
+            return
+    if entry['abbrev-status'] == "Patched" and cve_data[cve]['abbrev-status'] == "Unpatched":
+        if entry['status'] == "version-not-in-range" and cve_data[cve]['status'] == "version-in-range":
+            # Range does not match the scan, but we already have a vulnerable match, ignore
+            bb.debug("CVE entry %s update from Patched to Unpatched from the scan result - not applying" % cve)
+            return
+    # If we have an "Ignored", it has a priority
+    if cve_data[cve]['abbrev-status'] == "Ignored":
+        bb.debug("CVE %s not updating because Ignored" % cve)
+        return
+    bb.warn("Unhandled CVE entry update for %s from %s to %s" % (cve, cve_data[cve], entry))
+
+def check_cves(d, cve_data):
     """
     Connect to the NVD database and find unpatched cves.
     """
@@ -304,28 +348,19 @@ def check_cves(d, patched_cves):
     real_pv = d.getVar("PV")
     suffix = d.getVar("CVE_VERSION_SUFFIX")
 
-    cves_unpatched = []
-    cves_ignored = []
     cves_status = []
     cves_in_recipe = False
     # CVE_PRODUCT can contain more than one product (eg. curl/libcurl)
     products = d.getVar("CVE_PRODUCT").split()
     # If this has been unset then we're not scanning for CVEs here (for example, image recipes)
     if not products:
-        return ([], [], [], [])
+        return ([], [])
     pv = d.getVar("CVE_VERSION").split("+git")[0]
 
     # If the recipe has been skipped/ignored we return empty lists
     if pn in d.getVar("CVE_CHECK_SKIP_RECIPE").split():
         bb.note("Recipe has been skipped by cve-check")
-        return ([], [], [], [])
-
-    # 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)
+        return ([], [])
 
     import sqlite3
     db_file = d.expand("file:${CVE_CHECK_DB_FILE}?mode=ro")
@@ -344,11 +379,10 @@ def check_cves(d, patched_cves):
         for cverow in cve_cursor:
             cve = cverow[0]
 
-            if cve in cve_ignore:
+            if cve_is_ignored(d, cve_data, cve):
                 bb.note("%s-%s ignores %s" % (product, pv, cve))
-                cves_ignored.append(cve)
                 continue
-            elif cve in patched_cves:
+            elif cve_is_patched(d, cve_data, cve):
                 bb.note("%s has been patched" % (cve))
                 continue
             # Write status once only for each product
@@ -364,7 +398,7 @@ def check_cves(d, patched_cves):
             for row in product_cursor:
                 (_, _, _, version_start, operator_start, version_end, operator_end) = row
                 #bb.debug(2, "Evaluating row " + str(row))
-                if cve in cve_ignore:
+                if cve_is_ignored(d, cve_data, cve):
                     ignored = True
 
                 version_start = convert_cve_version(version_start)
@@ -403,16 +437,16 @@ def check_cves(d, patched_cves):
                 if vulnerable:
                     if ignored:
                         bb.note("%s is ignored in %s-%s" % (cve, pn, real_pv))
-                        cves_ignored.append(cve)
+                        cve_update(d, cve_data, cve, {"abbrev-status": "Ignored"})
                     else:
                         bb.note("%s-%s is vulnerable to %s" % (pn, real_pv, cve))
-                        cves_unpatched.append(cve)
+                        cve_update(d, cve_data, cve, {"abbrev-status": "Unpatched", "status": "version-in-range"})
                     break
             product_cursor.close()
 
             if not vulnerable:
                 bb.note("%s-%s is not vulnerable to %s" % (pn, real_pv, cve))
-                patched_cves.add(cve)
+                cve_update(d, cve_data, cve, {"abbrev-status": "Patched", "status": "version-not-in-range"})
         cve_cursor.close()
 
         if not cves_in_product:
@@ -420,48 +454,45 @@ def check_cves(d, patched_cves):
             cves_status.append([product, False])
 
     conn.close()
-    diff_ignore = list(set(cve_ignore) - set(cves_ignored))
-    if diff_ignore:
-        oe.qa.handle_error("cve_status_not_in_db", "Found CVE (%s) with CVE_STATUS set that are not found in database for this component" % " ".join(diff_ignore), d)
 
     if not cves_in_recipe:
         bb.note("No CVE records for products in recipe %s" % (pn))
 
-    return (list(cves_ignored), list(patched_cves), cves_unpatched, cves_status)
+    return (cve_data, cves_status)
 
-def get_cve_info(d, cves):
+def get_cve_info(d, cve_data):
     """
     Get CVE information from the database.
     """
 
     import sqlite3
 
-    cve_data = {}
     db_file = d.expand("file:${CVE_CHECK_DB_FILE}?mode=ro")
     conn = sqlite3.connect(db_file, uri=True)
 
-    for cve in cves:
+    for cve in cve_data:
         cursor = conn.execute("SELECT * FROM NVD WHERE ID IS ?", (cve,))
         for row in cursor:
-            cve_data[row[0]] = {}
-            cve_data[row[0]]["summary"] = row[1]
-            cve_data[row[0]]["scorev2"] = row[2]
-            cve_data[row[0]]["scorev3"] = row[3]
-            cve_data[row[0]]["modified"] = row[4]
-            cve_data[row[0]]["vector"] = row[5]
-            cve_data[row[0]]["vectorString"] = row[6]
+            # The CVE itdelf has been added already
+            if row[0] not in cve_data:
+                bb.note("CVE record %s not present" % row[0])
+                continue
+            #cve_data[row[0]] = {}
+            cve_data[row[0]]["NVD-summary"] = row[1]
+            cve_data[row[0]]["NVD-scorev2"] = row[2]
+            cve_data[row[0]]["NVD-scorev3"] = row[3]
+            cve_data[row[0]]["NVD-modified"] = row[4]
+            cve_data[row[0]]["NVD-vector"] = row[5]
+            cve_data[row[0]]["NVD-vectorString"] = row[6]
         cursor.close()
     conn.close()
-    return cve_data
 
-def cve_write_data_text(d, patched, unpatched, ignored, cve_data):
+def cve_write_data_text(d, cve_data):
     """
     Write CVE information in WORKDIR; and to CVE_CHECK_DIR, and
     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]
@@ -478,7 +509,7 @@ def cve_write_data_text(d, patched, unpatched, ignored, cve_data):
         return
 
     # Early exit, the text format does not report packages without CVEs
-    if not patched+unpatched+ignored:
+    if not len(cve_data):
         return
 
     nvd_link = "https://nvd.nist.gov/vuln/detail/"
@@ -487,36 +518,29 @@ def cve_write_data_text(d, patched, unpatched, ignored, cve_data):
     bb.utils.mkdirhier(os.path.dirname(cve_file))
 
     for cve in sorted(cve_data):
-        is_patched = cve in patched
-        is_ignored = cve in ignored
-
-        status = "Unpatched"
-        if (is_patched or is_ignored) and not report_all:
+        if not report_all and (cve_data[cve]["abbrev-status"] == "Patched" or cve_data[cve]["abbrev-status"] == "Ignored"):
             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
-        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"]
-        write_string += "VECTOR: %s\n" % cve_data[cve]["vector"]
-        write_string += "VECTORSTRING: %s\n" % cve_data[cve]["vectorString"]
+        write_string += "CVE STATUS: %s\n" % cve_data[cve]["abbrev-status"]
+
+        if 'status' in cve_data[cve]:
+            write_string += "CVE DETAIL: %s\n" % cve_data[cve]["status"]
+        if 'justification' in cve_data[cve]:
+            write_string += "CVE DESCRIPTION: %s\n" % cve_data[cve]["justification"]
+
+        if "NVD-summary" in cve_data[cve]:
+            write_string += "CVE SUMMARY: %s\n" % cve_data[cve]["NVD-summary"]
+            write_string += "CVSS v2 BASE SCORE: %s\n" % cve_data[cve]["NVD-scorev2"]
+            write_string += "CVSS v3 BASE SCORE: %s\n" % cve_data[cve]["NVD-scorev3"]
+            write_string += "VECTOR: %s\n" % cve_data[cve]["NVD-vector"]
+            write_string += "VECTORSTRING: %s\n" % cve_data[cve]["NVD-vectorString"]
+
         write_string += "MORE INFORMATION: %s%s\n\n" % (nvd_link, cve)
+        if cve_data[cve]["abbrev-status"] == "Unpatched":
+            unpatched_cves.append(cve)
 
     if unpatched_cves and d.getVar("CVE_CHECK_SHOW_WARNINGS") == "1":
         bb.warn("Found unpatched CVE (%s), for more information check %s" % (" ".join(unpatched_cves),cve_file))
@@ -568,13 +592,11 @@ def cve_check_write_json_output(d, output, direct_file, deploy_file, manifest_fi
         with open(index_path, "a+") as f:
             f.write("%s\n" % fragment_path)
 
-def cve_write_data_json(d, patched, unpatched, ignored, cve_data, cve_status):
+def cve_write_data_json(d, 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/"
 
@@ -592,8 +614,6 @@ def cve_write_data_json(d, patched, unpatched, ignored, cve_data, cve_status):
     if include_layers and layer not in include_layers:
         return
 
-    unpatched_cves = []
-
     product_data = []
     for s in cve_status:
         p = {"product": s[0], "cvesInRecord": "Yes"}
@@ -608,39 +628,31 @@ def cve_write_data_json(d, patched, unpatched, ignored, cve_data, cve_status):
         "version" : package_version,
         "products": product_data
     }
+
     cve_list = []
 
     for cve in sorted(cve_data):
-        is_patched = cve in patched
-        is_ignored = cve in ignored
-        status = "Unpatched"
-        if (is_patched or is_ignored) and not report_all:
+        if not report_all and (cve_data[cve]["abbrev-status"] == "Patched" or cve_data[cve]["abbrev-status"] == "Ignored"):
             continue
-        if is_ignored:
-            status = "Ignored"
-        elif is_patched:
-            status = "Patched"
-        else:
-            # default value of status is Unpatched
-            unpatched_cves.append(cve)
-
         issue_link = "%s%s" % (nvd_link, cve)
 
         cve_item = {
             "id" : cve,
-            "summary" : cve_data[cve]["summary"],
-            "scorev2" : cve_data[cve]["scorev2"],
-            "scorev3" : cve_data[cve]["scorev3"],
-            "vector" : cve_data[cve]["vector"],
-            "vectorString" : cve_data[cve]["vectorString"],
-            "status" : status,
-            "link": issue_link
+            "status" : cve_data[cve]["abbrev-status"],
+            "link": issue_link,
         }
-        _, detail, description = decode_cve_status(d, cve)
-        if detail:
-            cve_item["detail"] = detail
-        if description:
-            cve_item["description"] = description
+        if 'NVD-summary' in cve_data[cve]:
+            cve_item["summary"] = cve_data[cve]["NVD-summary"]
+            cve_item["scorev2"] = cve_data[cve]["NVD-scorev2"]
+            cve_item["scorev3"] = cve_data[cve]["NVD-scorev3"]
+            cve_item["vector"] = cve_data[cve]["NVD-vector"]
+            cve_item["vectorString"] = cve_data[cve]["NVD-vectorString"]
+        if 'status' in cve_data[cve]:
+            cve_item["detail"] = cve_data[cve]["status"]
+        if 'justification' in cve_data[cve]:
+            cve_item["description"] = cve_data[cve]["justification"]
+        if 'resource' in cve_data[cve]:
+            cve_item["patch-file"] = cve_data[cve]["resource"]
         cve_list.append(cve_item)
 
     package_data["issue"] = cve_list
@@ -652,12 +664,12 @@ def cve_write_data_json(d, patched, unpatched, ignored, cve_data, cve_status):
 
     cve_check_write_json_output(d, output, direct_file, deploy_file, manifest_file)
 
-def cve_write_data(d, patched, unpatched, ignored, cve_data, status):
+def cve_write_data(d, cve_data, status):
     """
     Write CVE data in each enabled format.
     """
 
     if d.getVar("CVE_CHECK_FORMAT_TEXT") == "1":
-        cve_write_data_text(d, patched, unpatched, ignored, cve_data)
+        cve_write_data_text(d, cve_data)
     if d.getVar("CVE_CHECK_FORMAT_JSON") == "1":
-        cve_write_data_json(d, patched, unpatched, ignored, cve_data, status)
+        cve_write_data_json(d, cve_data, status)
diff --git a/meta/lib/oe/cve_check.py b/meta/lib/oe/cve_check.py
index ed5c714cb8..6aba90183a 100644
--- a/meta/lib/oe/cve_check.py
+++ b/meta/lib/oe/cve_check.py
@@ -88,7 +88,7 @@ def get_patched_cves(d):
     # (cve_match regular expression)
     cve_file_name_match = re.compile(r".*(CVE-\d{4}-\d+)", re.IGNORECASE)
 
-    patched_cves = set()
+    patched_cves = {}
     patches = oe.patch.src_patches(d)
     bb.debug(2, "Scanning %d patches for CVEs" % len(patches))
     for url in patches:
@@ -98,7 +98,7 @@ def get_patched_cves(d):
         fname_match = cve_file_name_match.search(patch_file)
         if fname_match:
             cve = fname_match.group(1).upper()
-            patched_cves.add(cve)
+            patched_cves[cve] = {"abbrev-status": "Patched", "status": "fix-file-included", "resource": patch_file}
             bb.debug(2, "Found %s from patch file name %s" % (cve, patch_file))
 
         # Remote patches won't be present and compressed patches won't be
@@ -124,7 +124,7 @@ def get_patched_cves(d):
             cves = patch_text[match.start()+5:match.end()]
             for cve in cves.split():
                 bb.debug(2, "Patch %s solves %s" % (patch_file, cve))
-                patched_cves.add(cve)
+                patched_cves[cve] = {"abbrev-status": "Patched", "status": "fix-file-included", "resource": patch_file}
                 text_match = True
 
         if not fname_match and not text_match:
@@ -132,10 +132,8 @@ def get_patched_cves(d):
 
     # 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)
+        decoded_status, detail, description = decode_cve_status(d, cve)
+        patched_cves[cve] = {"abbrev-status": decoded_status, "status": detail, "justification": description}
 
     return patched_cves
 
-- 
2.43.0

[OE-core][PATCH v3 2/5] cve_check: Update selftest with new status detail

[Marta Rybczynska]

From: Samantha Jalabert <samantha.jalabert@syslinbit.com>

Signed-off-by: Samantha Jalabert <samantha.jalabert@syslinbit.com>
Signed-off-by: Marta Rybczynska <marta.rybczynska@syslinbit.com>
---
 meta/lib/oeqa/selftest/cases/cve_check.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/meta/lib/oeqa/selftest/cases/cve_check.py b/meta/lib/oeqa/selftest/cases/cve_check.py
index 60cecd1328..a40272c919 100644
--- a/meta/lib/oeqa/selftest/cases/cve_check.py
+++ b/meta/lib/oeqa/selftest/cases/cve_check.py
@@ -217,9 +217,10 @@ CVE_CHECK_REPORT_PATCHED = "1"
             # m4 CVE should not be in logrotate
             self.assertNotIn("CVE-2008-1687", found_cves)
             # logrotate has both Patched and Ignored CVEs
+            detail = "version-not-in-range"
             self.assertIn("CVE-2011-1098", found_cves)
             self.assertEqual(found_cves["CVE-2011-1098"]["status"], "Patched")
-            self.assertEqual(len(found_cves["CVE-2011-1098"]["detail"]), 0)
+            self.assertEqual(found_cves["CVE-2011-1098"]["detail"], detail)
             self.assertEqual(len(found_cves["CVE-2011-1098"]["description"]), 0)
             detail = "not-applicable-platform"
             description = "CVE is debian, gentoo or SUSE specific on the way logrotate was installed/used"
-- 
2.43.0

[OE-core][PATCH v3 3/5] vex.bbclass: add a new class

[Marta Rybczynska]

The "vex" class generates the minimum information that is necessary
for VEX generation by an external CVE checking tool. It is a drop-in
replacement of "cve-check". It uses the same variables from recipes
to make the migration and backporting easier.

The goal of this class is to allow generation of the CVE list of
an image or distribution on-demand, including the latest information
from vulnerability databases. Vulnerability data changes every day,
so a status generated at build becomes out-of-date very soon.

Research done for this work shows that the current VEX formats (CSAF
and OpenVEX) do not provide enough information to generate such
rolling information. Instead, we extract the needed data from recipe
annotations (package names, CPEs, versions, CVE patches applied...)
and store for later use in the format that is an extension of the
CVE-check JSON output format.

This output can be then used (separately or with SPDX of the same
build) by an external tool to generate the vulnerability annotation
and VEX statements in standard formats.

Signed-off-by: Marta Rybczynska <marta.rybczynska@syslinbit.com>
Signed-off-by: Samantha Jalabert <samantha.jalabert@syslinbit.com>
---
 meta/classes/vex.bbclass | 310 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 310 insertions(+)
 create mode 100644 meta/classes/vex.bbclass

diff --git a/meta/classes/vex.bbclass b/meta/classes/vex.bbclass
new file mode 100644
index 0000000000..bb16e2a529
--- /dev/null
+++ b/meta/classes/vex.bbclass
@@ -0,0 +1,310 @@
+#
+# Copyright OpenEmbedded Contributors
+#
+# SPDX-License-Identifier: MIT
+#
+
+# This class is used to generate metadata needed by external
+# tools to check for vulnerabilities, for example CVEs.
+#
+# In order to use this class just inherit the class in the
+# local.conf file and it will add the generate_vex task for
+# every recipe. If an image is build it will generate a report
+# in DEPLOY_DIR_IMAGE for all the packages used, it will also
+# generate a file for all recipes used in the build.
+#
+# Variables use CVE_CHECK prefix to keep compatibility with
+# the cve-check class
+#
+# Example:
+#   bitbake -c generate_vex openssl
+#   bitbake core-image-sato
+#   bitbake -k -c generate_vex universe
+#
+# The product name that the CVE database uses defaults to BPN, but may need to
+# be overriden per recipe (for example tiff.bb sets CVE_PRODUCT=libtiff).
+CVE_PRODUCT ??= "${BPN}"
+CVE_VERSION ??= "${PV}"
+
+CVE_CHECK_SUMMARY_DIR ?= "${LOG_DIR}/cve"
+
+CVE_CHECK_SUMMARY_FILE_NAME_JSON = "cve-summary.json"
+CVE_CHECK_SUMMARY_INDEX_PATH = "${CVE_CHECK_SUMMARY_DIR}/cve-summary-index.txt"
+
+CVE_CHECK_DIR ??= "${DEPLOY_DIR}/cve"
+CVE_CHECK_RECIPE_FILE_JSON ?= "${CVE_CHECK_DIR}/${PN}_cve.json"
+CVE_CHECK_MANIFEST_JSON ?= "${IMGDEPLOYDIR}/${IMAGE_NAME}.json"
+
+# Skip CVE Check for packages (PN)
+CVE_CHECK_SKIP_RECIPE ?= ""
+
+# 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_STATUS[CVE-1234-0001] = "not-applicable-platform: Issue only applies on Windows"
+# CVE_STATUS[CVE-1234-0002] = "fixed-version: Fixed externally"
+#
+# Settings the same status and reason for multiple CVEs is possible
+# via CVE_STATUS_GROUPS variable.
+#
+# CVE_STATUS_GROUPS = "CVE_STATUS_WIN CVE_STATUS_PATCHED"
+#
+# CVE_STATUS_WIN = "CVE-1234-0001 CVE-1234-0003"
+# CVE_STATUS_WIN[status] = "not-applicable-platform: Issue only applies on Windows"
+# CVE_STATUS_PATCHED = "CVE-1234-0002 CVE-1234-0004"
+# CVE_STATUS_PATCHED[status] = "fixed-version: Fixed externally"
+#
+# All possible CVE statuses could be found in cve-check-map.conf
+# 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 ?= ""
+
+# Layers to be excluded
+CVE_CHECK_LAYER_EXCLUDELIST ??= ""
+
+# Layers to be included
+CVE_CHECK_LAYER_INCLUDELIST ??= ""
+
+
+# set to "alphabetical" for version using single alphabetical character as increment release
+CVE_VERSION_SUFFIX ??= ""
+
+python () {
+    if bb.data.inherits_class("cve-check", d):
+        raise bb.parse.SkipRecipe("Skipping recipe: found incompatible combination of cve-check and vex enabled at the same time.")
+
+    # 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
+        from oe.cve_check import cve_check_merge_jsons, update_symlinks
+
+        bb.note("Generating JSON CVE summary")
+        index_file = d.getVar("CVE_CHECK_SUMMARY_INDEX_PATH")
+        summary = {"version":"1", "package": []}
+        with open(index_file) as f:
+            filename = f.readline()
+            while filename:
+                with open(filename.rstrip()) as j:
+                    data = json.load(j)
+                    cve_check_merge_jsons(summary, data)
+                filename = f.readline()
+
+        summary["package"].sort(key=lambda d: d['name'])
+
+        with open(out_path, "w") as f:
+            json.dump(summary, f, indent=2)
+
+        update_symlinks(out_path, link_path)
+
+python vex_save_summary_handler () {
+    import shutil
+    import datetime
+    from oe.cve_check import update_symlinks
+
+    cvelogpath = d.getVar("CVE_CHECK_SUMMARY_DIR")
+
+    bb.utils.mkdirhier(cvelogpath)
+    timestamp = datetime.datetime.now().strftime('%Y%m%d%H%M%S')
+
+    json_summary_link_name = os.path.join(cvelogpath, d.getVar("CVE_CHECK_SUMMARY_FILE_NAME_JSON"))
+    json_summary_name = os.path.join(cvelogpath, "cve-summary-%s.json" % (timestamp))
+    generate_json_report(d, json_summary_name, json_summary_link_name)
+    bb.plain("Complete CVE JSON report summary created at: %s" % json_summary_link_name)
+}
+
+addhandler vex_save_summary_handler
+vex_save_summary_handler[eventmask] = "bb.event.BuildCompleted"
+
+python do_generate_vex () {
+    """
+    Generate metadata needed for vulnerability checking for
+    the current recipe
+    """
+    from oe.cve_check import get_patched_cves
+
+    try:
+        patched_cves = get_patched_cves(d)
+        cves_status = []
+        products = d.getVar("CVE_PRODUCT").split()
+        for product in products:
+            if ":" in product:
+                _, product = product.split(":", 1)
+            cves_status.append([product, False])
+
+    except FileNotFoundError:
+        bb.fatal("Failure in searching patches")
+
+    cve_write_data_json(d, patched_cves, cves_status)
+}
+
+addtask generate_vex before do_build
+
+python vex_cleanup () {
+    """
+    Delete the file used to gather all the CVE information.
+    """
+    bb.utils.remove(e.data.getVar("CVE_CHECK_SUMMARY_INDEX_PATH"))
+}
+
+addhandler vex_cleanup
+vex_cleanup[eventmask] = "bb.event.BuildCompleted"
+
+python vex_write_rootfs_manifest () {
+    """
+    Create VEX/CVE manifest when building an image
+    """
+
+    import json
+    from oe.rootfs import image_list_installed_packages
+    from oe.cve_check import cve_check_merge_jsons, update_symlinks
+
+    deploy_file_json = d.getVar("CVE_CHECK_RECIPE_FILE_JSON")
+    if os.path.exists(deploy_file_json):
+        bb.utils.remove(deploy_file_json)
+
+    # Create a list of relevant recipies
+    recipies = set()
+    for pkg in list(image_list_installed_packages(d)):
+        pkg_info = os.path.join(d.getVar('PKGDATA_DIR'),
+                                'runtime-reverse', pkg)
+        pkg_data = oe.packagedata.read_pkgdatafile(pkg_info)
+        recipies.add(pkg_data["PN"])
+
+    bb.note("Writing rootfs VEX manifest")
+    deploy_dir = d.getVar("IMGDEPLOYDIR")
+    link_name = d.getVar("IMAGE_LINK_NAME")
+
+    json_data = {"version":"1", "package": []}
+    text_data = ""
+
+    save_pn = d.getVar("PN")
+
+    for pkg in recipies:
+        # To be able to use the CVE_CHECK_RECIPE_FILE_JSON variable we have to evaluate
+        # it with the different PN names set each time.
+        d.setVar("PN", pkg)
+
+        pkgfilepath = d.getVar("CVE_CHECK_RECIPE_FILE_JSON")
+        if os.path.exists(pkgfilepath):
+            with open(pkgfilepath) as j:
+                data = json.load(j)
+                cve_check_merge_jsons(json_data, data)
+
+    d.setVar("PN", save_pn)
+
+    link_path = os.path.join(deploy_dir, "%s.json" % link_name)
+    manifest_name = d.getVar("CVE_CHECK_MANIFEST_JSON")
+
+    with open(manifest_name, "w") as f:
+        json.dump(json_data, f, indent=2)
+
+    update_symlinks(manifest_name, link_path)
+    bb.plain("Image VEX JSON report stored in: %s" % manifest_name)
+}
+
+ROOTFS_POSTPROCESS_COMMAND:prepend = "vex_write_rootfs_manifest; "
+do_rootfs[recrdeptask] += "do_generate_vex "
+do_populate_sdk[recrdeptask] += "do_generate_vex "
+
+def cve_write_data_json(d, cve_data, cve_status):
+    """
+    Prepare CVE data for the JSON format, then write it.
+    Done for each recipe.
+    """
+
+    from oe.cve_check import get_cpe_ids
+    import json
+
+    output = {"version":"1", "package": []}
+    nvd_link = "https://nvd.nist.gov/vuln/detail/"
+
+    fdir_name  = d.getVar("FILE_DIRNAME")
+    layer = fdir_name.split("/")[-3]
+
+    include_layers = d.getVar("CVE_CHECK_LAYER_INCLUDELIST").split()
+    exclude_layers = d.getVar("CVE_CHECK_LAYER_EXCLUDELIST").split()
+
+    if exclude_layers and layer in exclude_layers:
+        return
+
+    if include_layers and layer not in include_layers:
+        return
+
+    product_data = []
+    for s in cve_status:
+        p = {"product": s[0], "cvesInRecord": "Yes"}
+        if s[1] == False:
+            p["cvesInRecord"] = "No"
+        product_data.append(p)
+    product_data = list({p['product']:p for p in product_data}.values())
+
+    package_version = "%s%s" % (d.getVar("EXTENDPE"), d.getVar("PV"))
+    cpes = get_cpe_ids(d.getVar("CVE_PRODUCT"), d.getVar("CVE_VERSION"))
+    package_data = {
+        "name" : d.getVar("PN"),
+        "layer" : layer,
+        "version" : package_version,
+        "products": product_data,
+        "cpes": cpes
+    }
+
+    cve_list = []
+
+    for cve in sorted(cve_data):
+        issue_link = "%s%s" % (nvd_link, cve)
+
+        cve_item = {
+            "id" : cve,
+            "status" : cve_data[cve]["abbrev-status"],
+            "link": issue_link,
+        }
+        if 'NVD-summary' in cve_data[cve]:
+            cve_item["summary"] = cve_data[cve]["NVD-summary"]
+            cve_item["scorev2"] = cve_data[cve]["NVD-scorev2"]
+            cve_item["scorev3"] = cve_data[cve]["NVD-scorev3"]
+            cve_item["vector"] = cve_data[cve]["NVD-vector"]
+            cve_item["vectorString"] = cve_data[cve]["NVD-vectorString"]
+        if 'status' in cve_data[cve]:
+            cve_item["detail"] = cve_data[cve]["status"]
+        if 'justification' in cve_data[cve]:
+            cve_item["description"] = cve_data[cve]["justification"]
+        if 'resource' in cve_data[cve]:
+            cve_item["patch-file"] = cve_data[cve]["resource"]
+        cve_list.append(cve_item)
+
+    package_data["issue"] = cve_list
+    output["package"].append(package_data)
+
+    deploy_file = d.getVar("CVE_CHECK_RECIPE_FILE_JSON")
+
+    write_string = json.dumps(output, indent=2)
+
+    cvelogpath = d.getVar("CVE_CHECK_SUMMARY_DIR")
+    index_path = d.getVar("CVE_CHECK_SUMMARY_INDEX_PATH")
+    bb.utils.mkdirhier(cvelogpath)
+    fragment_file = os.path.basename(deploy_file)
+    fragment_path = os.path.join(cvelogpath, fragment_file)
+    with open(fragment_path, "w") as f:
+        f.write(write_string)
+    with open(index_path, "a+") as f:
+        f.write("%s\n" % fragment_path)
-- 
2.43.0

Re: [OE-core][PATCH v3 3/5] vex.bbclass: add a new class

[Ross Burton]

On 24 Jul 2024, at 16:25, Marta Rybczynska via lists.openembedded.org <rybczynska=gmail.com@lists.openembedded.org> wrote:
> +# 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 ?= ""

In the new classes, I think we should just drop the deprecated variables entirely.

Ross

Re: [OE-core][PATCH v3 3/5] vex.bbclass: add a new class

[Ross Burton]

Also we should add a selftest for this class, to verify that the output is as expected.

Ross

> On 26 Jul 2024, at 13:09, Ross Burton via lists.openembedded.org <ross.burton=arm.com@lists.openembedded.org> wrote:
> 
> On 24 Jul 2024, at 16:25, Marta Rybczynska via lists.openembedded.org <rybczynska=gmail.com@lists.openembedded.org> wrote:
>> +# 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 ?= ""
> 
> In the new classes, I think we should just drop the deprecated variables entirely.
> 
> Ross
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#202530): https://lists.openembedded.org/g/openembedded-core/message/202530
> Mute This Topic: https://lists.openembedded.org/mt/107525294/6875888
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [ross.burton@arm.com]
> -=-=-=-=-=-=-=-=-=-=-=-
> 

Re: [OE-core][PATCH v3 3/5] vex.bbclass: add a new class

[Marta Rybczynska]

[-- Attachment #1: Type: text/plain, Size: 224 bytes --]

On Fri, Jul 26, 2024 at 2:12 PM Ross Burton <Ross.Burton@arm.com> wrote:

> Also we should add a selftest for this class, to verify that the output is
> as expected.
>
>
Yes, we'll add that.

Kind regards,
Marta

[-- Attachment #2: Type: text/html, Size: 589 bytes --]

Re: [OE-core][PATCH v3 3/5] vex.bbclass: add a new class

[Marta Rybczynska]

[-- Attachment #1: Type: text/plain, Size: 595 bytes --]

On Fri, Jul 26, 2024 at 2:09 PM Ross Burton <Ross.Burton@arm.com> wrote:

> On 24 Jul 2024, at 16:25, Marta Rybczynska via lists.openembedded.org
> <rybczynska=gmail.com@lists.openembedded.org> wrote:
> > +# 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 ?= ""
>
> In the new classes, I think we should just drop the deprecated variables
> entirely.


Yes, if we do not plan to backport. From what I know there was no decision
at this stage.

Regards,
Marta

[-- Attachment #2: Type: text/html, Size: 1131 bytes --]

[OE-core][PATCH v3 4/5] cve-check-map: add new statuses

[Marta Rybczynska]

Add 'fix-file-included', 'version-not-in-range' and 'version-in-range' generated
by the cve-check.

'fix-file-included' means that a fix file for the CVE has been located.

'version-not-in-range' means that the product version has been found outside of
the vulnerable range.

'version-in-range' means that the product version has been found inside of the
vulnerable range.

Signed-off-by: Marta Rybczynska <marta.rybczynska@syslinbit.com>
Signed-off-by: Samantha Jalabert <samantha.jalabert@syslinbit.com>
---
 meta/conf/cve-check-map.conf | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/meta/conf/cve-check-map.conf b/meta/conf/cve-check-map.conf
index 17b0f15571..ac956379d1 100644
--- a/meta/conf/cve-check-map.conf
+++ b/meta/conf/cve-check-map.conf
@@ -8,11 +8,17 @@ CVE_CHECK_STATUSMAP[backported-patch] = "Patched"
 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"
+# use when a fix file has been included (set automatically)
+CVE_CHECK_STATUSMAP[fix-file-included] = "Patched"
+# do not use directly: automatic scan reports version number NOT in the vulnerable range (set automatically)
+CVE_CHECK_STATUSMAP[version-not-in-range] = "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"
+# do not use directly: automatic scan reports version number IS in the vulnerable range (set automatically)
+CVE_CHECK_STATUSMAP[version-in-range] = "Unpatched"
 
 # used for migration from old concept, do not use for new vulnerabilities
 CVE_CHECK_STATUSMAP[ignored] = "Ignored"
@@ -26,3 +32,6 @@ CVE_CHECK_STATUSMAP[not-applicable-config] = "Ignored"
 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"
+
+# use when it is impossible to conclude if the vulnerability is present or not
+CVE_CHECK_STATUSMAP[unknown] = "Unknown"
-- 
2.43.0

[OE-core][PATCH v3 5/5] cve-extra-exclusions.inc: add deprecation notice

[Marta Rybczynska]

This file contains CVE_STATUS without machine-readable information on which
recipe it applies to. All entries should be verified and, if appropriate,
moved to their corresponding recipes.

Signed-off-by: Marta Rybczynska <marta.rybczynska@syslinbit.com>
---
 meta/conf/distro/include/cve-extra-exclusions.inc | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/meta/conf/distro/include/cve-extra-exclusions.inc b/meta/conf/distro/include/cve-extra-exclusions.inc
index fcef6a14fb..71c5bc31f8 100644
--- a/meta/conf/distro/include/cve-extra-exclusions.inc
+++ b/meta/conf/distro/include/cve-extra-exclusions.inc
@@ -1,3 +1,6 @@
+# THIS FILE IS DEPRECATED, DO NOT ADD NEW ENTRIES
+# All entries from this file should migrate to appropriate recipes.
+#
 # This file contains a list of CVE's where resolution has proven to be impractical
 # or there is no reasonable action the Yocto Project can take to resolve the issue.
 # It contains all the information we are aware of about an issue and analysis about
-- 
2.43.0

Re: [OE-core][PATCH v3 5/5] cve-extra-exclusions.inc: add deprecation notice

[Ross Burton]

On 24 Jul 2024, at 16:25, Marta Rybczynska via lists.openembedded.org <rybczynska=gmail.com@lists.openembedded.org> wrote:
> 
> This file contains CVE_STATUS without machine-readable information on which
> recipe it applies to. All entries should be verified and, if appropriate,
> moved to their corresponding recipes.

The point of this file was to be an opt-in for more exclusions where we didn’t feel 100% confident asserting the issues could be ignored.

How much of a problem is it if this file contains a a limited number of CVEs?  We can review what is in there and move/remove as needed to cut it down.

Ross

Re: [OE-core][PATCH v3 5/5] cve-extra-exclusions.inc: add deprecation notice

[Marta Rybczynska]

[-- Attachment #1: Type: text/plain, Size: 1001 bytes --]

On Fri, Jul 26, 2024 at 2:24 PM Ross Burton <Ross.Burton@arm.com> wrote:

> On 24 Jul 2024, at 16:25, Marta Rybczynska via lists.openembedded.org
> <rybczynska=gmail.com@lists.openembedded.org> wrote:
> >
> > This file contains CVE_STATUS without machine-readable information on
> which
> > recipe it applies to. All entries should be verified and, if appropriate,
> > moved to their corresponding recipes.
>
> The point of this file was to be an opt-in for more exclusions where we
> didn’t feel 100% confident asserting the issues could be ignored.
>
> How much of a problem is it if this file contains a a limited number of
> CVEs?  We can review what is in there and move/remove as needed to cut it
> down.
>

With the vex class (and with SPDX too, I think) they end up copied present
in every single package of the build. This brings enormous confusion.
Impossible to filter them out as there is no information about the affected
recipe/package.

Kind regards,
Marta

[-- Attachment #2: Type: text/html, Size: 1537 bytes --]

Re: [OE-core][PATCH v3 5/5] cve-extra-exclusions.inc: add deprecation notice

[Richard Purdie]

On Fri, 2024-07-26 at 14:28 +0200, Marta Rybczynska via
lists.openembedded.org wrote:
> 
> 
> On Fri, Jul 26, 2024 at 2:24 PM Ross Burton <Ross.Burton@arm.com>
> wrote:
> > On 24 Jul 2024, at 16:25, Marta Rybczynska via
> > lists.openembedded.org
> > <rybczynska=gmail.com@lists.openembedded.org> wrote:
> > > 
> > > This file contains CVE_STATUS without machine-readable
> > > information on which
> > > recipe it applies to. All entries should be verified and, if
> > > appropriate,
> > > moved to their corresponding recipes.
> > 
> > The point of this file was to be an opt-in for more exclusions
> > where we didn’t feel 100% confident asserting the issues could be
> > ignored.
> > 
> > How much of a problem is it if this file contains a a limited
> > number of CVEs?  We can review what is in there and move/remove as
> > needed to cut it down.
> 
> With the vex class (and with SPDX too, I think) they end up copied
> present in every single package of the build. This brings enormous
> confusion.
> Impossible to filter them out as there is no information about the
> affected recipe/package.

Difficult, yes, impossible, no.

Surely we know which recipes a given CVE apply to?

Cheers,

Richard

Re: [OE-core][PATCH v3 5/5] cve-extra-exclusions.inc: add deprecation notice

[Marta Rybczynska]

[-- Attachment #1: Type: text/plain, Size: 1894 bytes --]

On Thu, Aug 1, 2024 at 3:47 PM Richard Purdie <
richard.purdie@linuxfoundation.org> wrote:

> On Fri, 2024-07-26 at 14:28 +0200, Marta Rybczynska via
> lists.openembedded.org wrote:
> >
> >
> > On Fri, Jul 26, 2024 at 2:24 PM Ross Burton <Ross.Burton@arm.com>
> > wrote:
> > > On 24 Jul 2024, at 16:25, Marta Rybczynska via
> > > lists.openembedded.org
> > > <rybczynska=gmail.com@lists.openembedded.org> wrote:
> > > >
> > > > This file contains CVE_STATUS without machine-readable
> > > > information on which
> > > > recipe it applies to. All entries should be verified and, if
> > > > appropriate,
> > > > moved to their corresponding recipes.
> > >
> > > The point of this file was to be an opt-in for more exclusions
> > > where we didn’t feel 100% confident asserting the issues could be
> > > ignored.
> > >
> > > How much of a problem is it if this file contains a a limited
> > > number of CVEs?  We can review what is in there and move/remove as
> > > needed to cut it down.
> >
> > With the vex class (and with SPDX too, I think) they end up copied
> > present in every single package of the build. This brings enormous
> > confusion.
> > Impossible to filter them out as there is no information about the
> > affected recipe/package.
>
> Difficult, yes, impossible, no.
>
> Surely we know which recipes a given CVE apply to?
>
>
Without having the CVE data at the same time, no. Also, a CVE might apply
to multiple
recipes at the same time - and this could be dangerous. Imagine a situation
where
the CPE is incorrect and is pointing to the wrong package. It gets added to
.inc.
But then someone in their layer adds the package the CVE really applies
to...

If we keep the .inc file, I think the most correct way would be to add a
field marking
 which recipe it should be applied to.

What do you think?

Regards,
Marta

[-- Attachment #2: Type: text/html, Size: 2871 bytes --]

Re: [OE-core][PATCH v3 5/5] cve-extra-exclusions.inc: add deprecation notice

[Richard Purdie]

On Thu, 2024-08-01 at 15:58 +0200, Marta Rybczynska wrote:
> On Thu, Aug 1, 2024 at 3:47 PM Richard Purdie <richard.purdie@linuxfoundation.org> wrote:
> > On Fri, 2024-07-26 at 14:28 +0200, Marta Rybczynska via
> > lists.openembedded.org wrote:
> > > On Fri, Jul 26, 2024 at 2:24 PM Ross Burton <Ross.Burton@arm.com>
> > > wrote:
> > > > On 24 Jul 2024, at 16:25, Marta Rybczynska via
> > > > lists.openembedded.org
> > > > <rybczynska=gmail.com@lists.openembedded.org> wrote:
> > > > > 
> > > > > This file contains CVE_STATUS without machine-readable
> > > > > information on which
> > > > > recipe it applies to. All entries should be verified and, if
> > > > > appropriate,
> > > > > moved to their corresponding recipes.
> > > > 
> > > > The point of this file was to be an opt-in for more exclusions
> > > > where we didn’t feel 100% confident asserting the issues could be
> > > > ignored.
> > > > 
> > > > How much of a problem is it if this file contains a a limited
> > > > number of CVEs?  We can review what is in there and move/remove as
> > > > needed to cut it down.
> > > 
> > > With the vex class (and with SPDX too, I think) they end up copied
> > > present in every single package of the build. This brings enormous
> > > confusion.
> > > Impossible to filter them out as there is no information about the
> > > affected recipe/package.
> > 
> > Difficult, yes, impossible, no.
> > 
> > Surely we know which recipes a given CVE apply to?
> 
> Without having the CVE data at the same time, no.

I thought we had the CVE data?

>  Also, a CVE might apply to multiple
> recipes at the same time - and this could be dangerous. Imagine a situation where
> the CPE is incorrect and is pointing to the wrong package. It gets added to .inc.
> But then someone in their layer adds the package the CVE really applies to...

We can't cover every possible scenario and if the data is wrong, we
identify and fix it.

> If we keep the .inc file, I think the most correct way would be to add a field marking
>  which recipe it should be applied to. 
> 
> What do you think?

How do we mark up a CVE as applying to gcc-cross-*/gcc-cross-canadian-
*/gcc-runtime/libgcc and so on?

How do we convert "bash" to "nativesdk-bash" and "bash-native"?

There are ways we could do such markup but I don't think it is going to
work well. 

I guess the key question is what we're trying to achieve and where
we're expecting the data to be processed?

Could we write a common exclusion file out once as part of the CVE
fetch recipe and avoid putting it into the individual recipe output?

Cheers,

Richard

Re: [OE-core][PATCH v3 5/5] cve-extra-exclusions.inc: add deprecation notice

[Marta Rybczynska]

[-- Attachment #1: Type: text/plain, Size: 4768 bytes --]

On Thu, Aug 1, 2024 at 4:08 PM Richard Purdie <
richard.purdie@linuxfoundation.org> wrote:

> On Thu, 2024-08-01 at 15:58 +0200, Marta Rybczynska wrote:
> > On Thu, Aug 1, 2024 at 3:47 PM Richard Purdie <
> richard.purdie@linuxfoundation.org> wrote:
> > > On Fri, 2024-07-26 at 14:28 +0200, Marta Rybczynska via
> > > lists.openembedded.org wrote:
> > > > On Fri, Jul 26, 2024 at 2:24 PM Ross Burton <Ross.Burton@arm.com>
> > > > wrote:
> > > > > On 24 Jul 2024, at 16:25, Marta Rybczynska via
> > > > > lists.openembedded.org
> > > > > <rybczynska=gmail.com@lists.openembedded.org> wrote:
> > > > > >
> > > > > > This file contains CVE_STATUS without machine-readable
> > > > > > information on which
> > > > > > recipe it applies to. All entries should be verified and, if
> > > > > > appropriate,
> > > > > > moved to their corresponding recipes.
> > > > >
> > > > > The point of this file was to be an opt-in for more exclusions
> > > > > where we didn’t feel 100% confident asserting the issues could be
> > > > > ignored.
> > > > >
> > > > > How much of a problem is it if this file contains a a limited
> > > > > number of CVEs?  We can review what is in there and move/remove as
> > > > > needed to cut it down.
> > > >
> > > > With the vex class (and with SPDX too, I think) they end up copied
> > > > present in every single package of the build. This brings enormous
> > > > confusion.
> > > > Impossible to filter them out as there is no information about the
> > > > affected recipe/package.
> > >
> > > Difficult, yes, impossible, no.
> > >
> > > Surely we know which recipes a given CVE apply to?
> >
> > Without having the CVE data at the same time, no.
>
> I thought we had the CVE data?
>

When we do vex only, we have the CVE data only in the external tool. The
file that comes
out from the build contains additional entries. We can filter them out, but
only in the tool.

However, if we filter out in the tool (with a rule like: if this CVE does
not apply to the package,
remove it), we remove the feature of the VEX of being able to specify that
your package is
not affected by a widely known vulnerability. For example, you can say "I'm
not affected by
log4shell because I do not include log4j". This is a feature that has its
use, it avoids
responding to multiple procurement questions, where they all want the same
information.
You just say it once, in the VEX.


>
> >  Also, a CVE might apply to multiple
> > recipes at the same time - and this could be dangerous. Imagine a
> situation where
> > the CPE is incorrect and is pointing to the wrong package. It gets added
> to .inc.
> > But then someone in their layer adds the package the CVE really applies
> to...
>
> We can't cover every possible scenario and if the data is wrong, we
> identify and fix it.
>

I think we need to distinguish between two cases:
- the data is wrong and we fix it (and we're sure of the fix) - typically a
CPE fix
- the resolution depends on a configuration, typically
"not-applicable-config" - if someone adds a bbappend to the recipe, they
MUST review impact on all related vulnerabilities
- we make an assumption. All of "upstream-wontfix" are of this category.
The YP user MUST make their own assessment here.


>
> > If we keep the .inc file, I think the most correct way would be to add a
> field marking
> >  which recipe it should be applied to.
> >
> > What do you think?
>
> How do we mark up a CVE as applying to gcc-cross-*/gcc-cross-canadian-
> */gcc-runtime/libgcc and so on?
>

We do not have any gcc entries in cve-extra-exclusions.



>
> How do we convert "bash" to "nativesdk-bash" and "bash-native"?
>

We do not have anything related to bash either.

What is in there in cve-extra-exclusions are mostly very old CVEs, the
biggest part belonging to bdb.
Kernel ones can go to kernel includes etc.

With the directions the CVE programme is going, I do not *expect* a need
for new entries.


>
> There are ways we could do such markup but I don't think it is going to
> work well.
>
> I guess the key question is what we're trying to achieve and where
> we're expecting the data to be processed?
>
> Could we write a common exclusion file out once as part of the CVE
> fetch recipe and avoid putting it into the individual recipe output?
>

Yes, we can do that and additionally allow each vendor to have their own
file, with their own assessment. "upstream-wontfix" can be mapped to
"code-not-reached" or similar, when confirmed this is not explotable.

Autobuilder can have its own removing entries we don't want to see on CVE
list (if we really think it is a good idea).

Regards,
Marta

>
>
>

[-- Attachment #2: Type: text/html, Size: 7136 bytes --]

Re: [OE-core][PATCH v3 5/5] cve-extra-exclusions.inc: add deprecation notice

[Richard Purdie]

On Tue, 2024-08-06 at 15:16 +0200, Marta Rybczynska wrote:
> On Thu, Aug 1, 2024 at 4:08 PM Richard Purdie
> <richard.purdie@linuxfoundation.org> wrote:
> > On Thu, 2024-08-01 at 15:58 +0200, Marta Rybczynska wrote:
> > > > Difficult, yes, impossible, no.
> > > > 
> > > > Surely we know which recipes a given CVE apply to?
> > > 
> > > Without having the CVE data at the same time, no.
> > 
> > I thought we had the CVE data?
> 
> When we do vex only, we have the CVE data only in the external tool.
> The file that comes out from the build contains additional entries.
> We can filter them out, but only in the tool.

Since we're using CVE entries with CVE numbering, I think it is ok if
even in the vex path, we perform some evaluation to see if a given
CVE_STATUS applies to a given recipe, particularly if we're exporting
data specific to that recipe.

The code change in question doesn't just change VEX but the general CVE
output too.


> However, if we filter out in the tool (with a rule like: if this CVE
> does not apply to the package, remove it), we remove the feature of
> the VEX of being able to specify that your package is not affected by
> a widely known vulnerability. For example, you can say "I'm not
> affected by log4shell because I do not include log4j". This is a
> feature that has its use, it avoids responding to multiple
> procurement questions, where they all want the same information.
> You just say it once, in the VEX.

By that argument, you could export every CVE possible against every
recipe :/.

> 
> I think we need to distinguish between two cases:
> - the data is wrong and we fix it (and we're sure of the fix) -
> typically a CPE fix
> - the resolution depends on a configuration, typically "not-
> applicable-config" - if someone adds a bbappend to the recipe, they
> MUST review impact on all related vulnerabilities
> - we make an assumption. All of "upstream-wontfix" are of this
> category. The YP user MUST make their own assessment here.

The user needs to make an assessment, sure, but are we then going to
force the user to bbappend every recipe with their assessment?

> > > If we keep the .inc file, I think the most correct way would be
> > > to add a field marking
> > >  which recipe it should be applied to. 
> > > 
> > > What do you think?
> > 
> > How do we mark up a CVE as applying to gcc-cross-*/gcc-cross-
> > canadian-
> > */gcc-runtime/libgcc and so on?
> 
> We do not have any gcc entries in cve-extra-exclusions.

No, but we, or a downstream user might need to do this.

> > How do we convert "bash" to "nativesdk-bash" and "bash-native"?
> 
> We do not have anything related to bash either.

Today, no. Tomorrow, or in some other product with a different
configuration.

> What is in there in cve-extra-exclusions are mostly very old CVEs,
> the biggest part belonging to bdb.
> Kernel ones can go to kernel includes etc.
> 
> With the directions the CVE programme is going, I do not *expect* a
> need for new entries.

However, this mechanism is also the mechanism someone building a
product and performing their own assessments may want to use.

What your patches do is force all of that into the end tooling and out
of the metadata and make the metadata completely impractical to do it
with. That simply isn't acceptable.

> > There are ways we could do such markup but I don't think it is
> > going to
> > work well. 
> > 
> > I guess the key question is what we're trying to achieve and where
> > we're expecting the data to be processed?
> > 
> > Could we write a common exclusion file out once as part of the CVE
> > fetch recipe and avoid putting it into the individual recipe
> > output?
> > 
> 
> 
> Yes, we can do that and additionally allow each vendor to have their
> own file, with their own assessment. "upstream-wontfix" can be mapped
> to "code-not-reached" or similar, when confirmed this is not
> explotable.

You mean like an .inc file?

> Autobuilder can have its own removing entries we don't want to see on
> CVE list (if we really think it is a good idea).

This is exactly what the file you want to deprecate does. I agreed to
put it in the public metadata where people could see what we were doing
and why, so they could them make their own assessment of what we've
done.

I'm afraid I am adamant we cannot drop support for this.

(I am also find with reducing the entries in that file, I just think we
can't entirely remove support for the capability)

Cheers,

Richard

Patchtest results for [OE-core][PATCH v3 1/5] cve-check: annotate CVEs during analysis

[patchtest]

[-- Attachment #1: Type: text/plain, Size: 3073 bytes --]

Thank you for your submission. Patchtest identified one
or more issues with the patch. Please see the log below for
more information:

---
Testing patch /home/patchtest/share/mboxes/v3-1-5-cve-check-annotate-CVEs-during-analysis.patch

FAIL: test Signed-off-by presence: Mbox is missing Signed-off-by. Add it manually or with "git commit --amend -s" (test_mbox.TestMbox.test_signed_off_by_presence)

PASS: pretest pylint (test_python_pylint.PyLint.pretest_pylint)
PASS: test author valid (test_mbox.TestMbox.test_author_valid)
PASS: test commit message presence (test_mbox.TestMbox.test_commit_message_presence)
PASS: test max line length (test_metadata.TestMetadata.test_max_line_length)
PASS: test mbox format (test_mbox.TestMbox.test_mbox_format)
PASS: test non-AUH upgrade (test_mbox.TestMbox.test_non_auh_upgrade)
PASS: test pylint (test_python_pylint.PyLint.test_pylint)
PASS: test shortlog format (test_mbox.TestMbox.test_shortlog_format)
PASS: test shortlog length (test_mbox.TestMbox.test_shortlog_length)

SKIP: pretest src uri left files: No modified recipes, skipping pretest (test_metadata.TestMetadata.pretest_src_uri_left_files)
SKIP: test CVE check ignore: No modified recipes or older target branch, skipping test (test_metadata.TestMetadata.test_cve_check_ignore)
SKIP: test CVE tag format: No new CVE patches introduced (test_patch.TestPatch.test_cve_tag_format)
SKIP: test Signed-off-by presence: No new CVE patches introduced (test_patch.TestPatch.test_signed_off_by_presence)
SKIP: test Upstream-Status presence: No new CVE patches introduced (test_patch.TestPatch.test_upstream_status_presence_format)
SKIP: test bugzilla entry format: No bug ID found (test_mbox.TestMbox.test_bugzilla_entry_format)
SKIP: test lic files chksum modified not mentioned: No modified recipes, skipping test (test_metadata.TestMetadata.test_lic_files_chksum_modified_not_mentioned)
SKIP: test lic files chksum presence: No added recipes, skipping test (test_metadata.TestMetadata.test_lic_files_chksum_presence)
SKIP: test license presence: No added recipes, skipping test (test_metadata.TestMetadata.test_license_presence)
SKIP: test series merge on head: Merge test is disabled for now (test_mbox.TestMbox.test_series_merge_on_head)
SKIP: test src uri left files: No modified recipes, skipping pretest (test_metadata.TestMetadata.test_src_uri_left_files)
SKIP: test summary presence: No added recipes, skipping test (test_metadata.TestMetadata.test_summary_presence)
SKIP: test target mailing list: Series merged, no reason to check other mailing lists (test_mbox.TestMbox.test_target_mailing_list)

---

Please address the issues identified and
submit a new revision of the patch, or alternatively, reply to this
email with an explanation of why the patch should be accepted. If you
believe these results are due to an error in patchtest, please submit a
bug at https://bugzilla.yoctoproject.org/ (use the 'Patchtest' category
under 'Yocto Project Subprojects'). For more information on specific
failures, see: https://wiki.yoctoproject.org/wiki/Patchtest. Thank
you!

Re: [OE-core][PATCH v3 1/5] cve-check: annotate CVEs during analysis

[Richard Purdie]

Hi Marta,


With the v3 series applied we did just see this on the autobuilder
unfortunately so I'm not sure that problem is addressed:

https://autobuilder.yoctoproject.org/typhoon/#/builders/87/builds/7004/steps/14/logs/stdio


ERROR: m4-native-1.4.19-r0 do_cve_check: Error executing a python function in exec_func_python() autogenerated:
The stack trace of python calls that resulted in this exception/failure was:
File: 'exec_func_python() autogenerated', lineno: 2, function: <module>
     0001:
 *** 0002:do_cve_check(d)
     0003:
File: '/home/pokybuild/yocto-worker/oe-selftest-ubuntu/build/meta/classes/cve-check.bbclass', lineno: 191, function: do_cve_check
     0187:            try:
     0188:                patched_cves = get_patched_cves(d)
     0189:            except FileNotFoundError:
     0190:                bb.fatal("Failure in searching patches")
 *** 0191:            cve_data, status = check_cves(d, patched_cves)
     0192:            if len(cve_data) or (d.getVar("CVE_CHECK_COVERAGE") == "1" and status):
     0193:                get_cve_info(d, cve_data)
     0194:                cve_write_data(d, cve_data, status)
     0195:        else:
File: '/home/pokybuild/yocto-worker/oe-selftest-ubuntu/build/meta/classes/cve-check.bbclass', lineno: 379, function: check_cves
     0375:            vendor = "%"
     0376:
     0377:        # Find all relevant CVE IDs.
     0378:        cve_cursor = conn.execute("SELECT DISTINCT ID FROM PRODUCTS WHERE PRODUCT IS ? AND VENDOR LIKE ?", (product, vendor))
 *** 0379:        for cverow in cve_cursor:
     0380:            cve = cverow[0]
     0381:
     0382:            if cve_is_ignored(d, cve_data, cve):
     0383:                bb.note("%s-%s ignores %s" % (product, pv, cve))
Exception: sqlite3.DatabaseError: database disk image is malformed



Cheers,

Richard

Re: [OE-core][PATCH v3 1/5] cve-check: annotate CVEs during analysis

[Richard Purdie]

On Thu, 2024-07-25 at 16:48 +0200, mrybczynska@syslinbit.com wrote:
> On 25.07.2024 16:29, Richard Purdie wrote:
> > Hi Marta,
> > 
> > 
> > With the v3 series applied we did just see this on the autobuilder
> > unfortunately so I'm not sure that problem is addressed:
> > 
> > https://autobuilder.yoctoproject.org/typhoon/#/builders/87/builds/7004/steps/14/logs/stdio
> > 
> 
> Hello Richard,
> Thanks, this is unfortunate. Is it possible to have a copy of the 
> corrupted database somewhere?

I think it is transient as we never clean it up and not all tasks fail.
That seems to imply it is a race of some kind.

Cheers,

Richard

Re: [OE-core][PATCH v3 1/5] cve-check: annotate CVEs during analysis

[Marta Rybczynska]

[-- Attachment #1: Type: text/plain, Size: 1449 bytes --]

On Thu, Jul 25, 2024 at 5:27 PM Richard Purdie <
richard.purdie@linuxfoundation.org> wrote:

> On Thu, 2024-07-25 at 16:48 +0200, mrybczynska@syslinbit.com wrote:
> > On 25.07.2024 16:29, Richard Purdie wrote:
> > > Hi Marta,
> > >
> > >
> > > With the v3 series applied we did just see this on the autobuilder
> > > unfortunately so I'm not sure that problem is addressed:
> > >
> > >
> https://autobuilder.yoctoproject.org/typhoon/#/builders/87/builds/7004/steps/14/logs/stdio
> > >
> >
> > Hello Richard,
> > Thanks, this is unfortunate. Is it possible to have a copy of the
> > corrupted database somewhere?
>
> I think it is transient as we never clean it up and not all tasks fail.
> That seems to imply it is a race of some kind.
>
>
I have a few ideas of what it might be, but I do not have a reproducer
right now. With the
vex changes, the duration of the cve_check operation changed slightly. On
the other hand,
the database download is slower these days (I have had standalone runs that
lasted for 5+ hours).
Also, I noticed that there were cancellations of some of the build, so the
cancellation of the download
may be in play too.

A question: autobuilder configuration does share DL_DIR among multiple
builds?

My possibility list right now:
- the "download" job timeout too short
- download failure/timeout
- job cancellation during the download

What do you think?

Kind regards,
Marta

[-- Attachment #2: Type: text/html, Size: 2271 bytes --]

Re: [OE-core][PATCH v3 1/5] cve-check: annotate CVEs during analysis

[Richard Purdie]

On Fri, 2024-07-26 at 15:02 +0200, Marta Rybczynska wrote:
> On Thu, Jul 25, 2024 at 5:27 PM Richard Purdie <richard.purdie@linuxfoundation.org> wrote:
> > On Thu, 2024-07-25 at 16:48 +0200, mrybczynska@syslinbit.com wrote:
> > > On 25.07.2024 16:29, Richard Purdie wrote:
> > > > Hi Marta,
> > > > 
> > > > 
> > > > With the v3 series applied we did just see this on the autobuilder
> > > > unfortunately so I'm not sure that problem is addressed:
> > > > 
> > > > https://autobuilder.yoctoproject.org/typhoon/#/builders/87/builds/7004/steps/14/logs/stdio
> > > > 
> > > 
> > > Hello Richard,
> > > Thanks, this is unfortunate. Is it possible to have a copy of the
> > > corrupted database somewhere?
> > 
> > I think it is transient as we never clean it up and not all tasks fail.
> > That seems to imply it is a race of some kind.
> 
> I have a few ideas of what it might be, but I do not have a reproducer right now. With the
> vex changes, the duration of the cve_check operation changed slightly. On the other hand,
> the database download is slower these days (I have had standalone runs that lasted for 5+ hours).
> Also, I noticed that there were cancellations of some of the build, so the cancellation of the download
> may be in play too.
> 
> A question: autobuilder configuration does share DL_DIR among multiple builds?

DL_DIR is shared between all the workers over NFS.

> My possibility list right now:
> - the "download" job timeout too short
> - download failure/timeout
> - job cancellation during the download

While a download is in progress, the exclusive lock should be held. If
the database were damaged, I'd then expect all subsequent cve_check
tasks to fail the same way.

In the failures, 2 or 3 tasks fail, the rest all continue to work. So
ti doesn't really fit.

> What do you think?

I'm wondering if we should make the do_fetch of the database recipe
copy the database to somewhere in TMPDIR when it finishes, then have
the main cve_check class use the copy there. This would move NFS issues
out the equation?

That would be more in keeping with how other recipes work, just using
DL_DIR as an accelerator. 

Cheers,

Richard

Re: [OE-core][PATCH v3 1/5] cve-check: annotate CVEs during analysis

[Marta Rybczynska]

[-- Attachment #1: Type: text/plain, Size: 3803 bytes --]

On Thu, Aug 1, 2024 at 4:25 PM Richard Purdie <
richard.purdie@linuxfoundation.org> wrote:

> On Fri, 2024-07-26 at 15:02 +0200, Marta Rybczynska wrote:
> > On Thu, Jul 25, 2024 at 5:27 PM Richard Purdie <
> richard.purdie@linuxfoundation.org> wrote:
> > > On Thu, 2024-07-25 at 16:48 +0200, mrybczynska@syslinbit.com wrote:
> > > > On 25.07.2024 16:29, Richard Purdie wrote:
> > > > > Hi Marta,
> > > > >
> > > > >
> > > > > With the v3 series applied we did just see this on the autobuilder
> > > > > unfortunately so I'm not sure that problem is addressed:
> > > > >
> > > > >
> https://autobuilder.yoctoproject.org/typhoon/#/builders/87/builds/7004/steps/14/logs/stdio
> > > > >
> > > >
> > > > Hello Richard,
> > > > Thanks, this is unfortunate. Is it possible to have a copy of the
> > > > corrupted database somewhere?
> > >
> > > I think it is transient as we never clean it up and not all tasks fail.
> > > That seems to imply it is a race of some kind.
> >
> > I have a few ideas of what it might be, but I do not have a reproducer
> right now. With the
> > vex changes, the duration of the cve_check operation changed slightly.
> On the other hand,
> > the database download is slower these days (I have had standalone runs
> that lasted for 5+ hours).
> > Also, I noticed that there were cancellations of some of the build, so
> the cancellation of the download
> > may be in play too.
> >
> > A question: autobuilder configuration does share DL_DIR among multiple
> builds?
>
> DL_DIR is shared between all the workers over NFS.
>
> > My possibility list right now:
> > - the "download" job timeout too short
> > - download failure/timeout
> > - job cancellation during the download
>
> While a download is in progress, the exclusive lock should be held. If
> the database were damaged, I'd then expect all subsequent cve_check
> tasks to fail the same way.
>
> In the failures, 2 or 3 tasks fail, the rest all continue to work. So
> ti doesn't really fit.
>

I would suspect there's a fetch job running in addition somewhere and it
manages to do the download. From that point, subsequent checks will work.
But where does that corruption come from - no idea.

I've noticed that tests *could* cause a database update and that the
temporary
download path will be the same for all instances (CVE_DB_TEMP_FILE). This
could cause corruption if the lock doesn't work as we expect it to.

Now, between kirkstone and master there should be no corruption, as this is
not the same database - files have different names as changed in
048ff0ad927f4d37cc5547ebeba9e0c221687ea6.


>
> > What do you think?
>
> I'm wondering if we should make the do_fetch of the database recipe
> copy the database to somewhere in TMPDIR when it finishes, then have
> the main cve_check class use the copy there. This would move NFS issues
> out the equation?
>
> That would be more in keeping with how other recipes work, just using
> DL_DIR as an accelerator.
>
>
We could do tweaks to make sure tests do not download the database
(CVE_DB_UPDATE_INTERVAL = "-1"). We could even do a run or two with
that set for the whole build for all configurations, to make sure the
corruption
does not happen at runtime.

We also have a standalone script to download the database (no change in
the format from the master branch), so we can use it and then point builds
to the copy, while disabling updates.

The source is here:
https://gitlab.com/syslinbit/public/yocto-vex-check/-/blob/main/cve-update-nvd2-native.py?ref_type=heads

We can also change the location of the database and always keep it in TMPDIR
or such. This could mean a long wait for the download.

Which solution would you prefer to test?

Kind regards,
Marta

[-- Attachment #2: Type: text/html, Size: 5547 bytes --]

Re: [OE-core][PATCH v3 1/5] cve-check: annotate CVEs during analysis

[Richard Purdie]

On Fri, 2024-08-02 at 14:50 +0200, Marta Rybczynska wrote:
> On Thu, Aug 1, 2024 at 4:25 PM Richard Purdie
> <richard.purdie@linuxfoundation.org> wrote:
> > On Fri, 2024-07-26 at 15:02 +0200, Marta Rybczynska wrote:
> > > On Thu, Jul 25, 2024 at 5:27 PM Richard Purdie
> > > <richard.purdie@linuxfoundation.org> wrote:
> > > > On Thu, 2024-07-25 at 16:48 +0200, mrybczynska@syslinbit.com
> > > > wrote:
> > > > > On 25.07.2024 16:29, Richard Purdie wrote:
> > > > > > Hi Marta,
> > > > > > 
> > > > > > 
> > > > > > With the v3 series applied we did just see this on the
> > > > > > autobuilder
> > > > > > unfortunately so I'm not sure that problem is addressed:
> > > > > > 
> > > > > > https://autobuilder.yoctoproject.org/typhoon/#/builders/87/builds/7004/steps/14/logs/stdio
> > > > > > 
> > > > > 
> > > > > Hello Richard,
> > > > > Thanks, this is unfortunate. Is it possible to have a copy of
> > > > > the
> > > > > corrupted database somewhere?
> > > > 
> > > > I think it is transient as we never clean it up and not all
> > > > tasks fail.
> > > > That seems to imply it is a race of some kind.
> > > 
> > > I have a few ideas of what it might be, but I do not have a
> > > reproducer right now. With the
> > > vex changes, the duration of the cve_check operation changed
> > > slightly. On the other hand,
> > > the database download is slower these days (I have had standalone
> > > runs that lasted for 5+ hours).
> > > Also, I noticed that there were cancellations of some of the
> > > build, so the cancellation of the download
> > > may be in play too.
> > > 
> > > A question: autobuilder configuration does share DL_DIR among
> > > multiple builds?
> > 
> > DL_DIR is shared between all the workers over NFS.
> > 
> > > My possibility list right now:
> > > - the "download" job timeout too short
> > > - download failure/timeout
> > > - job cancellation during the download
> > 
> > While a download is in progress, the exclusive lock should be held.
> > If the database were damaged, I'd then expect all subsequent
> > cve_check tasks to fail the same way.
> > 
> > In the failures, 2 or 3 tasks fail, the rest all continue to work.
> > So ti doesn't really fit.
> > 
> 
> 
> I would suspect there's a fetch job running in addition somewhere and
> it manages to do the download. From that point, subsequent checks
> will work. But where does that corruption come from - no idea.

We only ever update the database through the recipe though, right? That
recipe does have the correct lockfile specified for do_fetch? That
should mean it always has an exclusive lock when updating.

> I've noticed that tests *could* cause a database update and that the
> temporary download path will be the same for all instances
> (CVE_DB_TEMP_FILE). This could cause corruption if the lock doesn't
> work as we expect it to.

I did some tests and the lock does work between different autobuilder
nodes. We've noticed that the issue only seems to happen on ubuntu
22.04 which makes me wonder if there is a bug somewhere there such as
in the host sqlite3?

> Now, between kirkstone and master there should be no corruption, as
> this is not the same database - files have different names as changed
> in 048ff0ad927f4d37cc5547ebeba9e0c221687ea6.

Steve has observed the same issue in kirkstone, only on ubuntu 22.04.

> 
> We could do tweaks to make sure tests do not download the database
> (CVE_DB_UPDATE_INTERVAL = "-1"). We could even do a run or two with
> that set for the whole build for all configurations, to make sure the
> corruption
> does not happen at runtime.
> 
> We also have a standalone script to download the database (no change
> in the format from the master branch), so we can use it and then
> point builds to the copy, while disabling updates.
> 
> The source is here:
> https://gitlab.com/syslinbit/public/yocto-vex-check/-/blob/main/cve-update-nvd2-native.py?ref_type=heads
> 
> We can also change the location of the database and always keep it in
> TMPDIR
> or such. This could mean a long wait for the download.
> 
> Which solution would you prefer to test?

I'm not convinced the issue is a parallel fetch, I think sqlite is
breaking somehow and it is host specific.

I think we should add a do_unpack to the recipe and work from a local
copy in TMPDIR, not one over NFS. It can copy from DL_DIR so we
shouldn't lose much speed. I'm willing to give that patch a go if that
helps and lets you focus on the other patches?

Cheers,

Richard

Re: [OE-core][PATCH v3 1/5] cve-check: annotate CVEs during analysis

[Richard Purdie]

On Wed, 2024-07-24 at 17:25 +0200, Marta Rybczynska via lists.openembedded.org wrote:
> Add status information for each CVE under analysis.
> 
> Previously the information passed between different function of the
> cve-check class included only tables of patched, unpatched, ignored
> vulnerabilities and the general status of the recipe.
> 
> The VEX work requires more information, and we need to pass them
> between different functions, so that it can be enriched as the
> analysis progresses. Instead of multiple tables, use a single one
> with annotations for each CVE encountered. For example, a patched
> CVE will have:
> 
> {"abbrev-status": "Patched", "status": "version-not-in-range"}
> 
> abbrev-status contains the general status (Patched, Unpatched,
> Ignored and Unknown that will be added in the VEX code)
> status contains more detailed information that can come from
> CVE_STATUS and the analysis.
> 
> Additional fields of the annotation include for example the name
> of the patch file fixing a given CVE.
> 
> The side-effect of this change is that all entries from CVE_STATUS
> are available in the result file. That includes entries from
> the optional file cve-extra-exclusions.inc even if they might have
> no link with the recipe (apply to a different package). This will
> be fixed by moving all entries from that file to appropriate recipes.
> 
> From now on, CVE_STATUS should be added directly in the recipe file
> or in include files added only to affected recipes.

Sorry about the delay in getting to this. Initially I thought things
were ok but now I understand what is happening here, I'm afraid I have
concerns.

A fundamental property of what we're offering that we can use a common
include file to inject CVE_STATUS entries for recipes. Whilst I can
understand some of the concerns about the existing .inc file, we are
never going to be in a position where all users agree on exactly what
we should do with all CVEs.

The alternative is requiring a bbappend per recipe every time some
distro/company wants to add an entry and this is clearly not a good
solution.

I'm afraid I'm therefore very much against mandating that CVE_STATUS
entries should be against individual recipes. We need to find a
different solution rather than requiring that. We can likely sort the
file in core but not in other people's layers.

Cheers,

Richard

RE: [OE-core][PATCH v3 1/5] cve-check: annotate CVEs during analysis

[Marko, Peter]

> -----Original Message-----
> From: openembedded-core@lists.openembedded.org <openembedded-
> core@lists.openembedded.org> On Behalf Of Richard Purdie via
> lists.openembedded.org
> Sent: Thursday, August 1, 2024 15:45
> To: rybczynska@gmail.com; openembedded-core@lists.openembedded.org
> Cc: Marta Rybczynska <marta.rybczynska@syslinbit.com>; Samantha Jalabert
> <samantha.jalabert@syslinbit.com>
> Subject: Re: [OE-core][PATCH v3 1/5] cve-check: annotate CVEs during
> analysis
> 
> On Wed, 2024-07-24 at 17:25 +0200, Marta Rybczynska via
> lists.openembedded.org wrote:
> > Add status information for each CVE under analysis.
> >
> > Previously the information passed between different function of the
> > cve-check class included only tables of patched, unpatched, ignored
> > vulnerabilities and the general status of the recipe.
> >
> > The VEX work requires more information, and we need to pass them
> > between different functions, so that it can be enriched as the
> > analysis progresses. Instead of multiple tables, use a single one with
> > annotations for each CVE encountered. For example, a patched CVE will
> > have:
> >
> > {"abbrev-status": "Patched", "status": "version-not-in-range"}
> >
> > abbrev-status contains the general status (Patched, Unpatched, Ignored
> > and Unknown that will be added in the VEX code) status contains more
> > detailed information that can come from CVE_STATUS and the analysis.
> >
> > Additional fields of the annotation include for example the name of
> > the patch file fixing a given CVE.
> >
> > The side-effect of this change is that all entries from CVE_STATUS are
> > available in the result file. That includes entries from the optional
> > file cve-extra-exclusions.inc even if they might have no link with the
> > recipe (apply to a different package). This will be fixed by moving
> > all entries from that file to appropriate recipes.
> >
> > From now on, CVE_STATUS should be added directly in the recipe file or
> > in include files added only to affected recipes.
> 
> Sorry about the delay in getting to this. Initially I thought things were ok but
> now I understand what is happening here, I'm afraid I have concerns.
> 
> A fundamental property of what we're offering that we can use a common
> include file to inject CVE_STATUS entries for recipes. Whilst I can understand
> some of the concerns about the existing .inc file, we are never going to be in a
> position where all users agree on exactly what we should do with all CVEs.
> 
> The alternative is requiring a bbappend per recipe every time some
> distro/company wants to add an entry and this is clearly not a good solution.

I wonder if can could add optional cpe product for which the ignored entry is targeted?
Something like converting first line of the general exclusion list to:
CVE_STATUS[CVE-2000-0006,strace] = ...
CVE_STATUS[CVE-2000-0006,linux_kernel] = ...

> 
> I'm afraid I'm therefore very much against mandating that CVE_STATUS entries
> should be against individual recipes. We need to find a different solution rather
> than requiring that. We can likely sort the file in core but not in other people's
> layers.
> 
> Cheers,
> 
> Richard