[v3,1/3] cve-check: add option to add additional patched CVEs

- Replace CVE_CHECK_IGNORE with CVE_STATUS + [CVE_STATUS_REASONING] to be
more flexible. CVE_STATUS should contain flag for each CVE with accepted
values "Ignored", "Not applicable" or "Patched". It allows to add
a status for each CVEs.
- Optional CVE_STATUS_REASONING flag variable may contain a reason
why the CVE status was used. It will be added in csv/json report like
a new "reason" entry.
- Settings the same status and reason for multiple CVEs is possible
via CVE_STATUS_GROUPS variable.
- All listed CVEs in CVE_CHECK_IGNORE are copied to CVE_STATUS with
value "Ignored" like a fallback.

Examples of usage:
CVE_STATUS[CVE-1234-0001] = "Ignored" # or "Not applicable" or "Patched"
CVE_STATUS[CVE-1234-0002] = "Not applicable"
CVE_STATUS_REASONING[CVE-1234-0002] = "Issue only applies on Windows"

CVE_STATUS_GROUPS = "CVE_STATUS_WIN CVE_STATUS_PATCHED"
CVE_STATUS_WIN = "CVE-1234-0001 CVE-1234-0002"
CVE_STATUS_WIN[status] = "Not applicable"
CVE_STATUS_WIN[reason] = "Issue only applies on Windows"

CVE_STATUS_PATCHED = "CVE-1234-0003 CVE-1234-0004"
CVE_STATUS_PATCHED[status] = "Patched"
CVE_STATUS_PATCHED[reason] = "Fixed externally"

Signed-off-by: Andrej Valek <andrej.valek@siemens.com>
Signed-off-by: Peter Marko <peter.marko@siemens.com>
---
 documentation/dev-manual/new-recipe.rst      |  4 +-
 documentation/dev-manual/vulnerabilities.rst | 11 ++---
 documentation/ref-manual/classes.rst         |  9 ++--
 documentation/ref-manual/variables.rst       | 33 ++++++++++++---
 meta/classes/cve-check.bbclass               | 44 +++++++++++++++++---
 meta/lib/oe/cve_check.py                     |  6 +++
 6 files changed, 87 insertions(+), 20 deletions(-)

Hi,

Looks really good but could you split the documentation to separate
patch and send to docs@lists.yoctoproject.org instead of oe-core?

Thanks!

-Mikko

On Fri, May 19, 2023 at 08:24:18AM +0200, Andrej Valek wrote:
> - Replace CVE_CHECK_IGNORE with CVE_STATUS + [CVE_STATUS_REASONING] to be
> more flexible. CVE_STATUS should contain flag for each CVE with accepted
> values "Ignored", "Not applicable" or "Patched". It allows to add
> a status for each CVEs.
> - Optional CVE_STATUS_REASONING flag variable may contain a reason
> why the CVE status was used. It will be added in csv/json report like
> a new "reason" entry.
> - Settings the same status and reason for multiple CVEs is possible
> via CVE_STATUS_GROUPS variable.
> - All listed CVEs in CVE_CHECK_IGNORE are copied to CVE_STATUS with
> value "Ignored" like a fallback.
> 
> Examples of usage:
> CVE_STATUS[CVE-1234-0001] = "Ignored" # or "Not applicable" or "Patched"
> CVE_STATUS[CVE-1234-0002] = "Not applicable"
> CVE_STATUS_REASONING[CVE-1234-0002] = "Issue only applies on Windows"
> 
> CVE_STATUS_GROUPS = "CVE_STATUS_WIN CVE_STATUS_PATCHED"
> CVE_STATUS_WIN = "CVE-1234-0001 CVE-1234-0002"
> CVE_STATUS_WIN[status] = "Not applicable"
> CVE_STATUS_WIN[reason] = "Issue only applies on Windows"
> 
> CVE_STATUS_PATCHED = "CVE-1234-0003 CVE-1234-0004"
> CVE_STATUS_PATCHED[status] = "Patched"
> CVE_STATUS_PATCHED[reason] = "Fixed externally"
> 
> Signed-off-by: Andrej Valek <andrej.valek@siemens.com>
> Signed-off-by: Peter Marko <peter.marko@siemens.com>
> ---
>  documentation/dev-manual/new-recipe.rst      |  4 +-
>  documentation/dev-manual/vulnerabilities.rst | 11 ++---
>  documentation/ref-manual/classes.rst         |  9 ++--
>  documentation/ref-manual/variables.rst       | 33 ++++++++++++---
>  meta/classes/cve-check.bbclass               | 44 +++++++++++++++++---
>  meta/lib/oe/cve_check.py                     |  6 +++
>  6 files changed, 87 insertions(+), 20 deletions(-)
> 
> diff --git a/documentation/dev-manual/new-recipe.rst b/documentation/dev-manual/new-recipe.rst
> index 4e74246a4e9..008f4b1ceb7 100644
> --- a/documentation/dev-manual/new-recipe.rst
> +++ b/documentation/dev-manual/new-recipe.rst
> @@ -1253,8 +1253,8 @@ In the following example, ``lz4`` is a makefile-based package::
>  
>     S = "${WORKDIR}/git"
>  
> -   # Fixed in r118, which is larger than the current version.
> -   CVE_CHECK_IGNORE += "CVE-2014-4715"
> +   CVE_STATUS[CVE-2014-4715] = "Patched"
> +   CVE_STATUS_REASONING[CVE-2014-4715] = "Fixed in r118, which is larger than the current version"
>  
>     EXTRA_OEMAKE = "PREFIX=${prefix} CC='${CC}' CFLAGS='${CFLAGS}' DESTDIR=${D} LIBDIR=${libdir} INCLUDEDIR=${includedir} BUILD_STATIC=no"
>  
> diff --git a/documentation/dev-manual/vulnerabilities.rst b/documentation/dev-manual/vulnerabilities.rst
> index 0ee3ec52c5c..ca1ea87ba7e 100644
> --- a/documentation/dev-manual/vulnerabilities.rst
> +++ b/documentation/dev-manual/vulnerabilities.rst
> @@ -158,7 +158,8 @@ CVE checker will then capture this information and change the CVE status to ``Pa
>  in the generated reports.
>  
>  If analysis shows that the CVE issue does not impact the recipe due to configuration, platform,
> -version or other reasons, the CVE can be marked as ``Ignored`` using the :term:`CVE_CHECK_IGNORE` variable.
> +version or other reasons, the CVE can be marked as ``Ignored`` or ``Not applicable`` using
> +the :term:`CVE_STATUS[]` variable flag.
>  As mentioned previously, if data in the CVE database is wrong, it is recommend to fix those
>  issues in the CVE database directly.
>  
> @@ -182,11 +183,11 @@ products defined in :term:`CVE_PRODUCT`. Then, for each found CVE:
>  -  If the package name (:term:`PN`) is part of
>     :term:`CVE_CHECK_SKIP_RECIPE`, it is considered as ``Patched``.
>  
> --  If the CVE ID is part of :term:`CVE_CHECK_IGNORE`, it is
> -   set as ``Ignored``.
> +-  If the CVE ID has status :term:`CVE_STATUS[<CVE ID>] = "Ignored"`, it is
> +   set as ``Ignored`` as same as for :term:`CVE_STATUS[<CVE ID>] = "Not applicable"`.
>  
> --  If the CVE ID is part of the patched CVE for the recipe, it is
> -   already considered as ``Patched``.
> +-  If the CVE ID is part of the patched CVE for the recipe or has status
> +   :term:`CVE_STATUS[<CVE ID>] = "Patched"`, it is considered as ``Patched``.
>  
>  -  Otherwise, the code checks whether the recipe version (:term:`PV`)
>     is within the range of versions impacted by the CVE. If so, the CVE
> diff --git a/documentation/ref-manual/classes.rst b/documentation/ref-manual/classes.rst
> index ab1628401e9..2811244b8f7 100644
> --- a/documentation/ref-manual/classes.rst
> +++ b/documentation/ref-manual/classes.rst
> @@ -517,10 +517,13 @@ The ``Patched`` state of a CVE issue is detected from patch files with the forma
>  ``CVE-ID.patch``, e.g. ``CVE-2019-20633.patch``, in the :term:`SRC_URI` and using
>  CVE metadata of format ``CVE: CVE-ID`` in the commit message of the patch file.
>  
> -If the recipe lists the ``CVE-ID`` in :term:`CVE_CHECK_IGNORE` variable, then the CVE state is reported
> -as ``Ignored``. Multiple CVEs can be listed separated by spaces. Example::
> +If the recipe adds the ``CVE-ID`` as flag of :term:`CVE_STATUS` variable with status
> +``Ignored`` or ``Not applicable``, then the CVE state is reported as ``Ignored``.
>  
> -   CVE_CHECK_IGNORE += "CVE-2020-29509 CVE-2020-29511"
> +   CVE_STATUS[CVE-2020-15523] = "Ignored"
> +
> +Possible CVE's statuses are ``Ignored``, ``Not applicable`` and ``Patched``.
> +Check :ref:`ref-variables-CVE_STATUS` for more details.
>  
>  If CVE check reports that a recipe contains false positives or false negatives, these may be
>  fixed in recipes by adjusting the CVE product name using :term:`CVE_PRODUCT` and :term:`CVE_VERSION` variables.
> diff --git a/documentation/ref-manual/variables.rst b/documentation/ref-manual/variables.rst
> index 6ee65e17884..cd5f1d65d27 100644
> --- a/documentation/ref-manual/variables.rst
> +++ b/documentation/ref-manual/variables.rst
> @@ -1653,11 +1653,7 @@ system and gives an overview of their function and contents.
>           and kernel module recipes).
>  
>     :term:`CVE_CHECK_IGNORE`
> -      The list of CVE IDs which are ignored. Here is
> -      an example from the :oe_layerindex:`Python3 recipe</layerindex/recipe/23823>`::
> -
> -         # This is windows only issue.
> -         CVE_CHECK_IGNORE += "CVE-2020-15523"
> +      Is deprecated and should be replaced by :term:`CVE_STATUS`
>  
>     :term:`CVE_CHECK_SHOW_WARNINGS`
>        Specifies whether or not the :ref:`ref-classes-cve-check`
> @@ -1698,6 +1694,33 @@ system and gives an overview of their function and contents.
>  
>           CVE_PRODUCT = "vendor:package"
>  
> +   :term:`CVE_STATUS`
> +      The CVE ID which is patched or should be ignored. Here is
> +      an example from the :oe_layerindex:`Python3 recipe</layerindex/recipe/23823>`::
> +
> +         CVE_STATUS[CVE-2020-15523] = "Ignored"
> +
> +      Possible CVE's statuses ``Ignored``, ``Not applicable`` or ``Patched``, while the ``reasoning``
> +      is optional.
> +
> +   :term:`CVE_STATUS_GROUPS`
> +      If there is a many CVEs with the same status and reason can by simplified by using this
> +      variable instead of many similar lines with ``CVE_STATUS`` and ``CVE_STATUS_REASONING``
> +
> +         CVE_STATUS_GROUPS = "CVE_STATUS_WIN CVE_STATUS_PATCHED"
> +         CVE_STATUS_WIN = "CVE-1234-0001 CVE-1234-0002"
> +         CVE_STATUS_WIN[status] = "Not applicable"
> +         CVE_STATUS_WIN[reason] = "Issue only applies on Windows"
> +
> +         CVE_STATUS_PATCHED = "CVE-1234-0003 CVE-1234-0004"
> +         CVE_STATUS_PATCHED[status] = "Patched"
> +         CVE_STATUS_PATCHED[reason] = "Fixed externally"
> +
> +   :term:`CVE_STATUS_REASONING`
> +      Optional explanation for :term:`CVE_STATUS`
> +
> +         CVE_STATUS_REASONING[CVE-2020-15523] = "Issue only applies on Windows"
> +
>     :term:`CVE_VERSION`
>        In a recipe, defines the version used to match the recipe version
>        against the version in the `NIST CVE database <https://nvd.nist.gov/>`__
> diff --git a/meta/classes/cve-check.bbclass b/meta/classes/cve-check.bbclass
> index bd9e7e7445c..44462de7445 100644
> --- a/meta/classes/cve-check.bbclass
> +++ b/meta/classes/cve-check.bbclass
> @@ -70,12 +70,15 @@ 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 reason for this status.
>  #
> -# CVE_CHECK_IGNORE = 'CVE-2014-2524 CVE-2018-1234'
> +# CVE_STATUS[CVE-1234-0001] = "Ignored" # or "Not applicable" or "Patched"
> +# CVE_STATUS[CVE-1234-0002] = "Not applicable"
> +# CVE_STATUS_REASONING[CVE-1234-0002] = "Issue only applies on Windows"
>  #
> +# 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
> @@ -88,6 +91,25 @@ 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 has been deprecated, use CVE_STATUS instead")
> +        set_cves_statuses(d, d.getVar("CVE_CHECK_IGNORE"), "Ignored")
> +
> +    # Process CVE_STATUS_GROUPS to set multiple statuses and optional reasons at once
> +    for cve_status_group in (d.getVar("CVE_STATUS_GROUPS") or "").split():
> +        set_cves_statuses(d, d.getVar(cve_status_group) or "",
> +                          d.getVarFlag(cve_status_group, "status"),
> +                          d.getVarFlag(cve_status_group, "reason"))
> +}
> +
> +def set_cves_statuses(d, cves, status, reason=""):
> +    for cve in cves.split():
> +        d.setVarFlag("CVE_STATUS", cve, status)
> +        d.setVarFlag("CVE_STATUS_REASONING", cve, reason)
> +
>  def generate_json_report(d, out_path, link_path):
>      if os.path.exists(d.getVar("CVE_CHECK_SUMMARY_INDEX_PATH")):
>          import json
> @@ -282,7 +304,13 @@ 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, status in (d.getVarFlags("CVE_STATUS") or {}).items():
> +        if status in ["Not applicable", "Ignored"]:
> +            cve_ignore.append(cve)
> +        elif status not in ["Patched"]:
> +            bb.error("Unsupported status %s in CVE_STATUS[%s]" % (status, cve))
>  
>      import sqlite3
>      db_file = d.expand("file:${CVE_CHECK_DB_FILE}?mode=ro")
> @@ -455,6 +483,9 @@ def cve_write_data_text(d, patched, unpatched, ignored, cve_data):
>          else:
>              unpatched_cves.append(cve)
>              write_string += "CVE STATUS: Unpatched\n"
> +        reasoning = d.getVarFlag("CVE_STATUS_REASONING", cve)
> +        if reasoning:
> +            write_string += "CVE REASON: %s\n" % reasoning
>          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"]
> @@ -576,6 +607,9 @@ def cve_write_data_json(d, patched, unpatched, ignored, cve_data, cve_status):
>              "status" : status,
>              "link": issue_link
>          }
> +        reasoning = d.getVarFlag("CVE_STATUS_REASONING", cve)
> +        if reasoning:
> +            cve_item["reason"] = reasoning
>          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 dbaa0b373a3..f47dd9920ef 100644
> --- a/meta/lib/oe/cve_check.py
> +++ b/meta/lib/oe/cve_check.py
> @@ -130,6 +130,12 @@ 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, status in (d.getVarFlags("CVE_STATUS") or {}).items():
> +        if status == "Patched":
> +            bb.debug(2, "CVE %s is additionally patched" % cve)
> +            patched_cves.add(cve)
> +
>      return patched_cves
>  
>  
> -- 
> 2.40.1
>

Hi Andrej

On 19.05.23 at 08:24, Andrej Valek via lists.openembedded.org wrote:
> - Replace CVE_CHECK_IGNORE with CVE_STATUS + [CVE_STATUS_REASONING] to be
> more flexible. CVE_STATUS should contain flag for each CVE with accepted
> values "Ignored", "Not applicable" or "Patched". It allows to add
> a status for each CVEs.
> - Optional CVE_STATUS_REASONING flag variable may contain a reason
> why the CVE status was used. It will be added in csv/json report like
> a new "reason" entry.
> - Settings the same status and reason for multiple CVEs is possible
> via CVE_STATUS_GROUPS variable.
> - All listed CVEs in CVE_CHECK_IGNORE are copied to CVE_STATUS with
> value "Ignored" like a fallback.
>
> Examples of usage:
> CVE_STATUS[CVE-1234-0001] = "Ignored" # or "Not applicable" or "Patched"
> CVE_STATUS[CVE-1234-0002] = "Not applicable"
> CVE_STATUS_REASONING[CVE-1234-0002] = "Issue only applies on Windows"
>
> CVE_STATUS_GROUPS = "CVE_STATUS_WIN CVE_STATUS_PATCHED"
> CVE_STATUS_WIN = "CVE-1234-0001 CVE-1234-0002"
> CVE_STATUS_WIN[status] = "Not applicable"
> CVE_STATUS_WIN[reason] = "Issue only applies on Windows"
>
> CVE_STATUS_PATCHED = "CVE-1234-0003 CVE-1234-0004"
> CVE_STATUS_PATCHED[status] = "Patched"
> CVE_STATUS_PATCHED[reason] = "Fixed externally"
>
> Signed-off-by: Andrej Valek <andrej.valek@siemens.com>
> Signed-off-by: Peter Marko <peter.marko@siemens.com>
> ---
>   documentation/dev-manual/new-recipe.rst      |  4 +-
>   documentation/dev-manual/vulnerabilities.rst | 11 ++---
>   documentation/ref-manual/classes.rst         |  9 ++--
>   documentation/ref-manual/variables.rst       | 33 ++++++++++++---
>   meta/classes/cve-check.bbclass               | 44 +++++++++++++++++---
>   meta/lib/oe/cve_check.py                     |  6 +++
>   6 files changed, 87 insertions(+), 20 deletions(-)

Many thanks for the patch and for the documentation changes too !
However, could you send the documentation changes separately, using the 
yocto-docs repository as a reference, and sending them to the 
docs@lists.yoctoproject.org mailing list?

You seem to have produced your patches against "poky", but that's a 
repository aggregating stuff from other repositories. Your code changes 
should be for the "openembedded-core" repository.

Another advantage is that we can merge the documentation changes only 
when the code changes are accepted.

Thanks in advance
Cheers
Michael.

Thank you for this work. I think we are going in a good direction. My
comments in the text.

In general, I would like that we come with the fixed list of possible
statuses and avoid adding new ones too frequently. Changing them will break
my parsing and status scripts each time.


On Fri, May 19, 2023 at 8:24 AM Andrej Valek via lists.openembedded.org
<andrej.valek=siemens.com@lists.openembedded.org> wrote:

> - Replace CVE_CHECK_IGNORE with CVE_STATUS + [CVE_STATUS_REASONING] to be
> more flexible. CVE_STATUS should contain flag for each CVE with accepted
> values "Ignored", "Not applicable" or "Patched". It allows to add
> a status for each CVEs.
>

I'm missing a status to cover the situation when the NVD (or any other
database) has an incorrect entry. We have quite many of those. This might
be a temporary situation, but not always.

SPDX (the 3.0 draft) has some other possible reasons
https://github.com/spdx/spdx-spec/blob/vulnerability-profile/chapters/profile-vulnerabilities.md
What looks like interesting ideas are:
* "Can't fix" / "Will not fix"
* "Not applicable" (SPDX language: Ineffective) when the code is not used
* "Invalid match" (this is our NVD mismatch case)
* "Mitigated" measures taken so that it cannot be exploited
* "Workarounded"

There is still one big missing part: related to configuration options. It
could be used with "Not applicable"/"Ineffective" code, but only in cases
where it is not possible to activate the code. If the user can switch
between vulnerable/not vulnerable versions by a packageconfig change or so,
this is not covered.

Addiional question: why CVE_STATUS_REASONING and not CVE_STATUS_REASON ?
(reason variable is used nearly everywhere)


> diff --git a/meta/classes/cve-check.bbclass
> b/meta/classes/cve-check.bbclass
> index bd9e7e7445c..44462de7445 100644
> --- a/meta/classes/cve-check.bbclass
> +++ b/meta/classes/cve-check.bbclass
> @@ -70,12 +70,15 @@ 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 reason for this status.
>  #
> -# CVE_CHECK_IGNORE = 'CVE-2014-2524 CVE-2018-1234'
> +# CVE_STATUS[CVE-1234-0001] = "Ignored" # or "Not applicable" or "Patched"
> +# CVE_STATUS[CVE-1234-0002] = "Not applicable"
> +# CVE_STATUS_REASONING[CVE-1234-0002] = "Issue only applies on Windows"
>  #
> +# 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
> @@ -88,6 +91,25 @@ 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 has been deprecated, use CVE_STATUS
> instead")
> +        set_cves_statuses(d, d.getVar("CVE_CHECK_IGNORE"), "Ignored")
> +
> +    # Process CVE_STATUS_GROUPS to set multiple statuses and optional
> reasons at once
> +    for cve_status_group in (d.getVar("CVE_STATUS_GROUPS") or "").split():
> +        set_cves_statuses(d, d.getVar(cve_status_group) or "",
> +                          d.getVarFlag(cve_status_group, "status"),
> +                          d.getVarFlag(cve_status_group, "reason"))
> +}
> +
> +def set_cves_statuses(d, cves, status, reason=""):
> +    for cve in cves.split():
> +        d.setVarFlag("CVE_STATUS", cve, status)
> +        d.setVarFlag("CVE_STATUS_REASONING", cve, reason)
> +
>  def generate_json_report(d, out_path, link_path):
>      if os.path.exists(d.getVar("CVE_CHECK_SUMMARY_INDEX_PATH")):
>          import json
> @@ -282,7 +304,13 @@ 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, status in (d.getVarFlags("CVE_STATUS") or {}).items():
> +        if status in ["Not applicable", "Ignored"]:
> +            cve_ignore.append(cve)
> +        elif status not in ["Patched"]:
> +            bb.error("Unsupported status %s in CVE_STATUS[%s]" % (status,
> cve))
>
I do not see this entry added into the "Patched" list.

IMO would be better to handle Patched separately, and so a complete "else"
for all other reasons. Allows to avoid hard-coding all possible options.


>
>      import sqlite3
>      db_file = d.expand("file:${CVE_CHECK_DB_FILE}?mode=ro")
> @@ -455,6 +483,9 @@ def cve_write_data_text(d, patched, unpatched,
> ignored, cve_data):
>          else:
>              unpatched_cves.append(cve)
>              write_string += "CVE STATUS: Unpatched\n"
> +        reasoning = d.getVarFlag("CVE_STATUS_REASONING", cve)
> +        if reasoning:
> +            write_string += "CVE REASON: %s\n" % reasoning
>

Do we want adding new features to the (deprecated) text output?

Kind regards,
Marta

>
>

Hello Marta,

On Fri, 2023-05-19 at 15:11 +0200, Marta Rybczynska wrote:
Thank you for this work. I think we are going in a good direction. My comments in the text.

In general, I would like that we come with the fixed list of possible statuses and avoid adding new ones too frequently. Changing them will break my parsing and status scripts each time.


On Fri, May 19, 2023 at 8:24 AM Andrej Valek via lists.openembedded.org<http://lists.openembedded.org> <andrej.valek=siemens.com@lists.openembedded.org<mailto:siemens.com@lists.openembedded.org>> wrote:
- Replace CVE_CHECK_IGNORE with CVE_STATUS + [CVE_STATUS_REASONING] to be
more flexible. CVE_STATUS should contain flag for each CVE with accepted
values "Ignored", "Not applicable" or "Patched". It allows to add
a status for each CVEs.


I'm missing a status to cover the situation when the NVD (or any other database) has an incorrect entry. We have quite many of those. This might be a temporary situation, but not always.

SPDX (the 3.0 draft) has some other possible reasons https://github.com/spdx/spdx-spec/blob/vulnerability-profile/chapters/profile-vulnerabilities.md
What looks like interesting ideas are:
* "Can't fix" / "Will not fix"
* "Not applicable" (SPDX language: Ineffective) when the code is not used
* "Invalid match" (this is our NVD mismatch case)
* "Mitigated" measures taken so that it cannot be exploited
* "Workarounded"

I would say, "Ignored", "Not applicable" or "Patched" are enough, because everything important is covered. Of course we can extend some keywords in the feature, but we shouldn't confuse users.

There is still one big missing part: related to configuration options. It could be used with "Not applicable"/"Ineffective" code, but only in cases where it is not possible to activate the code. If the user can switch between vulnerable/not vulnerable versions by a packageconfig change or so, this is not covered.

Addiional question: why CVE_STATUS_REASONING and not CVE_STATUS_REASON ? (reason variable is used nearly everywhere)

See explanation here: https://lists.openembedded.org/g/openembedded-core/message/181551 . Once we have a decision, I can change it.


diff --git a/meta/classes/cve-check.bbclass b/meta/classes/cve-check.bbclass
index bd9e7e7445c..44462de7445 100644
--- a/meta/classes/cve-check.bbclass
+++ b/meta/classes/cve-check.bbclass
@@ -70,12 +70,15 @@ 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 reason for this status.
 #
-# CVE_CHECK_IGNORE = 'CVE-2014-2524 CVE-2018-1234'
+# CVE_STATUS[CVE-1234-0001] = "Ignored" # or "Not applicable" or "Patched"
+# CVE_STATUS[CVE-1234-0002] = "Not applicable"
+# CVE_STATUS_REASONING[CVE-1234-0002] = "Issue only applies on Windows"
 #
+# 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
@@ -88,6 +91,25 @@ 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 has been deprecated, use CVE_STATUS instead")
+        set_cves_statuses(d, d.getVar("CVE_CHECK_IGNORE"), "Ignored")
+
+    # Process CVE_STATUS_GROUPS to set multiple statuses and optional reasons at once
+    for cve_status_group in (d.getVar("CVE_STATUS_GROUPS") or "").split():
+        set_cves_statuses(d, d.getVar(cve_status_group) or "",
+                          d.getVarFlag(cve_status_group, "status"),
+                          d.getVarFlag(cve_status_group, "reason"))
+}
+
+def set_cves_statuses(d, cves, status, reason=""):
+    for cve in cves.split():
+        d.setVarFlag("CVE_STATUS", cve, status)
+        d.setVarFlag("CVE_STATUS_REASONING", cve, reason)
+
 def generate_json_report(d, out_path, link_path):
     if os.path.exists(d.getVar("CVE_CHECK_SUMMARY_INDEX_PATH")):
         import json
@@ -282,7 +304,13 @@ 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, status in (d.getVarFlags("CVE_STATUS") or {}).items():
+        if status in ["Not applicable", "Ignored"]:
+            cve_ignore.append(cve)
+        elif status not in ["Patched"]:
+            bb.error("Unsupported status %s in CVE_STATUS[%s]" % (status, cve))

I do not see this entry added into the "Patched" list.

Of course this code is not covering the "Patched" ;). Check cve_check.py how the "Patched" is handled. Elif case is covering the typos.

IMO would be better to handle Patched separately, and so a complete "else" for all other reasons. Allows to avoid hard-coding all possible options.


     import sqlite3
     db_file = d.expand("file:${CVE_CHECK_DB_FILE}?mode=ro")
@@ -455,6 +483,9 @@ def cve_write_data_text(d, patched, unpatched, ignored, cve_data):
         else:
             unpatched_cves.append(cve)
             write_string += "CVE STATUS: Unpatched\n"
+        reasoning = d.getVarFlag("CVE_STATUS_REASONING", cve)
+        if reasoning:
+            write_string += "CVE REASON: %s\n" % reasoning


Do we want adding new features to the (deprecated) text output?

Is "reasoning/reason" deprecated? We're just adding new entry.

Kind regards,
Marta


Regards,
Andrej

Hi,

On Fri, May 19, 2023 at 03:11:57PM +0200, Marta Rybczynska wrote:
> I'm missing a status to cover the situation when the NVD (or any other
> database) has an incorrect entry. We have quite many of those. This might
> be a temporary situation, but not always.
> 
> SPDX (the 3.0 draft) has some other possible reasons
> https://github.com/spdx/spdx-spec/blob/vulnerability-profile/chapters/profile-vulnerabilities.md
> What looks like interesting ideas are:
> * "Can't fix" / "Will not fix"
> * "Not applicable" (SPDX language: Ineffective) when the code is not used
> * "Invalid match" (this is our NVD mismatch case)
> * "Mitigated" measures taken so that it cannot be exploited
> * "Workarounded"

To me the SPDX details don't seem very usable when actually maintaining
a linux distro for a long time. Anyone from major Linux distro
stable/security teams participating in the work?

So I'd rather compare to Debian security tracker CVE status data and ask
what our LTS and master branch maintainers and those in the community
who maintain yocto based SW stacks need. Do the maintainers want to read
SPDX output, for example? What common statuses do the maintainers want to
encode for each CVE?

Debian security tracker https://security-team.debian.org/security_tracker.html
shows states:

 * vulnerable: binary package with specified version in their distro
   version is vulnerable to the issue

 * fixed: binary package in their distro version has fixed the issue

 * undetermined: it is not yet clear if the issue affects Debian and
   their version of the packages

And "vulnerable" has sub states:

 * ignored: the issue does not impact Debian packages

 * postponed: no security patch updates will be provided, e.g. such a
   minor issue that update will happen for example via normal package
   version updates to next stable version

There are a lot of additional "standards" and sub states when looking at
CVE data in the tracker (info not public, no upstream fix available, not
supported configuration etc), but those major high level states are enough.
And then there are security relevant bugs without CVEs.

I've been happy with "Unpatched", "Patched" and "Ignored" states for
each CVE detected by cve-check.bbclass. There could be a few more sub
stated to "Ignored" and the "Patched" state should better reflect reality,
which this patch set helps. But I'm happy with that.

I'm not so happy with the SPDX states names and meanings.

Cheers,

-Mikko

Hello Richard,

Could you please take a look on the latest revision a make a decision there?
There are still bunch of unclear statements. So please make a final design and
we will try to implement it.

Thank you,
Andrej

On Mon, 2023-05-22 at 10:57 +0300, Mikko Rapeli wrote:
> Hi,
> 
> On Fri, May 19, 2023 at 03:11:57PM +0200, Marta Rybczynska wrote:
> > I'm missing a status to cover the situation when the NVD (or any other
> > database) has an incorrect entry. We have quite many of those. This might
> > be a temporary situation, but not always.
> > 
> > SPDX (the 3.0 draft) has some other possible reasons
> > https://github.com/spdx/spdx-spec/blob/vulnerability-profile/chapters/profile-vulnerabilities.md
> > What looks like interesting ideas are:
> > * "Can't fix" / "Will not fix"
> > * "Not applicable" (SPDX language: Ineffective) when the code is not used
> > * "Invalid match" (this is our NVD mismatch case)
> > * "Mitigated" measures taken so that it cannot be exploited
> > * "Workarounded"
> 
> To me the SPDX details don't seem very usable when actually maintaining
> a linux distro for a long time. Anyone from major Linux distro
> stable/security teams participating in the work?
> 
> So I'd rather compare to Debian security tracker CVE status data and ask
> what our LTS and master branch maintainers and those in the community
> who maintain yocto based SW stacks need. Do the maintainers want to read
> SPDX output, for example? What common statuses do the maintainers want to
> encode for each CVE?
> 
> Debian security tracker https://security-team.debian.org/security_tracker.html
> shows states:
> 
>  * vulnerable: binary package with specified version in their distro
>    version is vulnerable to the issue
> 
>  * fixed: binary package in their distro version has fixed the issue
> 
>  * undetermined: it is not yet clear if the issue affects Debian and
>    their version of the packages
> 
> And "vulnerable" has sub states:
> 
>  * ignored: the issue does not impact Debian packages
> 
>  * postponed: no security patch updates will be provided, e.g. such a
>    minor issue that update will happen for example via normal package
>    version updates to next stable version
> 
> There are a lot of additional "standards" and sub states when looking at
> CVE data in the tracker (info not public, no upstream fix available, not
> supported configuration etc), but those major high level states are enough.
> And then there are security relevant bugs without CVEs.
> 
> I've been happy with "Unpatched", "Patched" and "Ignored" states for
> each CVE detected by cve-check.bbclass. There could be a few more sub
> stated to "Ignored" and the "Patched" state should better reflect reality,
> which this patch set helps. But I'm happy with that.
> 
> I'm not so happy with the SPDX states names and meanings.
> 
> Cheers,
> 
> -Mikko

Hello again Richard,

Maybe this email was little bit unclear..., so I will try to recap it here.
There are 2 open points, where some final decision has to be made.

- Could we rename the CVE_STATUS_REASONING -> CVE_STATUS_REASON? The first idea
came from you.
- What is the final enum for CVE_STATUS? I would say "patched" and "ignored".
Afaik, the "not applicable" status came also from you. Should we keep it, or
remove it? Of course all others are just like an additions which could be
implemented later on request.

So please, take a look on it and made a final decision.

Thank you,
Andrej

On Tue, 2023-05-23 at 10:41 +0200, Valek Andrej wrote:
> Hello Richard,
> 
> Could you please take a look on the latest revision a make a decision there?
> There are still bunch of unclear statements. So please make a final design and
> we will try to implement it.
> 
> Thank you,
> Andrej
> 
> On Mon, 2023-05-22 at 10:57 +0300, Mikko Rapeli wrote:
> > Hi,
> > 
> > On Fri, May 19, 2023 at 03:11:57PM +0200, Marta Rybczynska wrote:
> > > I'm missing a status to cover the situation when the NVD (or any other
> > > database) has an incorrect entry. We have quite many of those. This might
> > > be a temporary situation, but not always.
> > > 
> > > SPDX (the 3.0 draft) has some other possible reasons
> > > https://github.com/spdx/spdx-spec/blob/vulnerability-profile/chapters/profile-vulnerabilities.md
> > > What looks like interesting ideas are:
> > > * "Can't fix" / "Will not fix"
> > > * "Not applicable" (SPDX language: Ineffective) when the code is not used
> > > * "Invalid match" (this is our NVD mismatch case)
> > > * "Mitigated" measures taken so that it cannot be exploited
> > > * "Workarounded"
> > 
> > To me the SPDX details don't seem very usable when actually maintaining
> > a linux distro for a long time. Anyone from major Linux distro
> > stable/security teams participating in the work?
> > 
> > So I'd rather compare to Debian security tracker CVE status data and ask
> > what our LTS and master branch maintainers and those in the community
> > who maintain yocto based SW stacks need. Do the maintainers want to read
> > SPDX output, for example? What common statuses do the maintainers want to
> > encode for each CVE?
> > 
> > Debian security tracker
> > https://security-team.debian.org/security_tracker.html
> > shows states:
> > 
> >  * vulnerable: binary package with specified version in their distro
> >    version is vulnerable to the issue
> > 
> >  * fixed: binary package in their distro version has fixed the issue
> > 
> >  * undetermined: it is not yet clear if the issue affects Debian and
> >    their version of the packages
> > 
> > And "vulnerable" has sub states:
> > 
> >  * ignored: the issue does not impact Debian packages
> > 
> >  * postponed: no security patch updates will be provided, e.g. such a
> >    minor issue that update will happen for example via normal package
> >    version updates to next stable version
> > 
> > There are a lot of additional "standards" and sub states when looking at
> > CVE data in the tracker (info not public, no upstream fix available, not
> > supported configuration etc), but those major high level states are enough.
> > And then there are security relevant bugs without CVEs.
> > 
> > I've been happy with "Unpatched", "Patched" and "Ignored" states for
> > each CVE detected by cve-check.bbclass. There could be a few more sub
> > stated to "Ignored" and the "Patched" state should better reflect reality,
> > which this patch set helps. But I'm happy with that.
> > 
> > I'm not so happy with the SPDX states names and meanings.
> > 
> > Cheers,
> > 
> > -Mikko
>

On Mon, 2023-05-29 at 07:32 +0000, Valek, Andrej wrote:
> Hello again Richard,
> 
> Maybe this email was little bit unclear..., so I will try to recap it here.
> There are 2 open points, where some final decision has to be made.
> 
> - Could we rename the CVE_STATUS_REASONING -> CVE_STATUS_REASON? The first idea
> came from you.
> - What is the final enum for CVE_STATUS? I would say "patched" and "ignored".
> Afaik, the "not applicable" status came also from you. Should we keep it, or
> remove it? Of course all others are just like an additions which could be
> implemented later on request.
> 
> So please, take a look on it and made a final decision.

Whilst it is true that I get to make a final decision on whether to
merge a patch or not (for better or worse), what we need here is a
community consensus about what we need to do. Usually that is done with
a proposal which gets tweaked until there aren't strong objections to
it and a majority are in agreement.

If do change, we need to get it right as we can't keep changing. We
also need to be mindful that the LTS releases are more closely coupled
in this area. That said, we need to get things right for master, then
work out how to work with the LTSes.

I think we're all in agreement we need to do something more than what
we currently have as it isn't scaling as we need.

FWIW I'm not sold on CVE_STATUS_REASON*, how about CVE_STATUS_DETAIL?

My original concern with "ignored" also remains. I'd really like to
encode more detail about why we think it can be ignored.

For example, this set of values would seem to match the common reasons
we see:

patched - we carry a patch meaning the CVE has been fixed

cpe-incorrect - the CPE entry versioning is incorrect and our version (and newer versions) are fixed. An update to the database is hopefully pending.

cpe-stable-backport - the CPE entry doesn't cover backports to stable series but patches are applied there (e.g. kernel LTS versions)

not-applicable-platform - the platform isn't applicable for us (e.g. windows or OS-X)

upstream-wontfix - the upstream maintainers don't believe the issue needs to be fixed

not-applicable-config - our configuration means the issue isn't enabled/present/applicable

other - CVE doesn't apply for some other reason

The reason for having these different categories is it means people can
group and act upon them differently. If you were building on windows,
you'd take a close look at the platform ones. If you were changing
kernels, you'd know cpe-stable-backport was potentially incorrect. If
you're changing PACKAGECONFIG, you'd take a closer look at not-
applicable-configuration.

I'm not 100% sure I have all the right categories but I'm trying to
articulate my own thoughts having written a number of the exclusions.
I'd be interested to understand if the above works for others. The
challenge is we all have different reasons and uses for the data.

Cheers,

Richard

Hi

I like the VEX proposal from Sanjay.

- It is a standard that can be supported by many tools and requested by
customers. One use case I see is where a vendor sells a product with an
SBOM. The customer can then match the open vulnerabilities to the
current state of the NIST database using a standard tool based on SBOM.
Aligning the categories to a standard would be helpful for this.
(Yocto's CVE check is great for Yocto, but cannot be used independently
of Yocto.)
- A minimum number of categories is defined. All details can be added
to the REASON variable.

Regards,
Adrian

On Fri, 2023-06-02 at 23:10 +0200, adrian.freihofer@gmail.com wrote:
> I like the VEX proposal from Sanjay.
> 
> - It is a standard that can be supported by many tools and requested by
> customers. One use case I see is where a vendor sells a product with an
> SBOM. The customer can then match the open vulnerabilities to the
> current state of the NIST database using a standard tool based on SBOM.
> Aligning the categories to a standard would be helpful for this.
> (Yocto's CVE check is great for Yocto, but cannot be used independently
> of Yocto.)
> - A minimum number of categories is defined. All details can be added
> to the REASON variable.

I think you could map some of the status items I proposed to VEX
statuses but I'm not convinced it makes sense to go directly to that.

Anything we don't have a status for is effectively "under
investigation", anything we don't list is fixed or not affected and if
we know something is affected, a fix would likely follow very quickly.
The data set doesn't really fit what we're able to do or the wrkflows
we can follow, even if it is what some product customers would want to
know. Part of the issue is we're not the actual product here.

Cheers,

Richard

Hi Richard,

Thank you for acknowledgement on my proposal.
Please consider my additional input for VEX standard.

There is total four main VEX standard status:
- Fixed
- Affected
- Not Affected
- Under Investigation

Out for 4 standard we can adopt Fixed and Not affected status for CVE fixing.
As these two statuses will never get changed for specific package and CVE.

Regarding the CVE status of community and VEX standard, we can map like following:

Existing Status 	| VEX adoption
-------------------------------------------
Patched	 	| Fixed	 	
Ignore	 	| Not Affected
Not required 	| Not Affected

Remaining two statuses Affected and Under investigation would be changed with time as following:
* Under Investigation:
- When any new CVE is reported against any package then by default it would go with "under investigation" status
- Until we make the final status like fixed/not affected/affected status after our final investigation on specific CVE.
* Affected:
- Regarding affected status it would be temporary status until we find the actual fix for the CVE.
- Once we have a fix the CVE then status would be as fixed/not affected which we can input to our recipe.

Thanks,
Sanjay

-----Original Message-----
From: openembedded-core@lists.openembedded.org <openembedded-core@lists.openembedded.org> On Behalf Of Richard Purdie
Sent: Saturday, June 3, 2023 2:57 AM
To: adrian.freihofer@gmail.com; Valek, Andrej <andrej.valek@siemens.com>
Cc: rybczynska@gmail.com; openembedded-core@lists.openembedded.org; mikko.rapeli@linaro.org; Marko, Peter <Peter.Marko@siemens.com>; Sanjaykumar kantibhai Chitroda -X (schitrod - E-INFO CHIPS INC at Cisco) <schitrod@cisco.com>
Subject: Re: [OE-core][PATCH v3 1/3] cve-check: add option to add additional patched CVEs

On Fri, 2023-06-02 at 23:10 +0200, adrian.freihofer@gmail.com wrote:
> I like the VEX proposal from Sanjay.
> 
> - It is a standard that can be supported by many tools and requested 
> by customers. One use case I see is where a vendor sells a product 
> with an SBOM. The customer can then match the open vulnerabilities to 
> the current state of the NIST database using a standard tool based on SBOM.
> Aligning the categories to a standard would be helpful for this.
> (Yocto's CVE check is great for Yocto, but cannot be used 
> independently of Yocto.)
> - A minimum number of categories is defined. All details can be added 
> to the REASON variable.

I think you could map some of the status items I proposed to VEX statuses but I'm not convinced it makes sense to go directly to that.

Anything we don't have a status for is effectively "under investigation", anything we don't list is fixed or not affected and if we know something is affected, a fix would likely follow very quickly.
The data set doesn't really fit what we're able to do or the wrkflows we can follow, even if it is what some product customers would want to know. Part of the issue is we're not the actual product here.

Cheers,

Richard

On Sun, 2023-06-04 at 09:59 +0000, Sanjaykumar kantibhai Chitroda -X
(schitrod - E-INFO CHIPS INC at Cisco) wrote:
> Hi Richard,
> 
> Thank you for acknowledgement on my proposal.
> Please consider my additional input for VEX standard.
> 
> There is total four main VEX standard status:
> - Fixed
> - Affected
> - Not Affected
> - Under Investigation
> 
> Out for 4 standard we can adopt Fixed and Not affected status for CVE fixing.
> As these two statuses will never get changed for specific package and CVE.
> 
> Regarding the CVE status of community and VEX standard, we can map like following:
> 
> Existing Status 	| VEX adoption
> -------------------------------------------
> Patched	 	| Fixed	 	
> Ignore	 	| Not Affected
> Not required 	| Not Affected
> 
> Remaining two statuses Affected and Under investigation would be changed with time as following:
> * Under Investigation:
> - When any new CVE is reported against any package then by default it would go with "under investigation" status
> - Until we make the final status like fixed/not affected/affected status after our final investigation on specific CVE.
> * Affected:
> - Regarding affected status it would be temporary status until we find the actual fix for the CVE.
> - Once we have a fix the CVE then status would be as fixed/not affected which we can input to our recipe.

Whilst I understand the desire to use VEX, I don't think we should
directly. It serves a very specific purpose and "loses" some
information by only having two states. Tying ourselves too closely to a
limiting standard like that can be problematic.

The v6 from Adrian can be mapped into this if that is what you need. I
think that is a good compromise as it doesn't lose the information
others may need.

Cheers,

Richard