Re: [OE-core][PATCH v5 1/2] cve-check: add option to add additional patched CVEs

Richard Purdie
2023-06-15T12:47:11Z

On Mon, 2023-06-12 at 13:57 +0200, Andrej Valek via
lists.openembedded.org wrote:
> - Replace CVE_CHECK_IGNORE with CVE_STATUS + [CVE_STATUS_DETAIL] +
> [CVE_STATUS_DESCRIPTION] to be more flexible. CVE_STATUS should
> contain flag for each CVE with accepted values "Ignored", "Patched"
> or "Unpatched". It allows to add a status for each CVEs.
> - Optional CVE_STATUS_DEATAIL flag variable may contain a detailed
> status. Possible options for each status:
> - Patched
> - fixed-version, backported-patch, cpe-stable-backport or other
> - Unpatched
> - vulnerable-investigating or other
> - Ignored
> - cpe-incorrect, not-applicable-platform, upstream-wontfix
> not-applicable-config, not-affected or other
> - Optional CVE_STATUS_DESCRIPTION flag variable may contain a reason
> why the CVE status was used. Both optionals will be added in csv/json
> report like a new "detail" an "description" entries
> - 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 "Patched" or "Unpatched"
> CVE_STATUS[CVE-1234-0002] = "Ignored"
> CVE_STATUS_DETAIL[CVE-1234-0002] = "not-applicable-platform"
> CVE_STATUS_DESCRIPTION[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] = "Ignored"
> CVE_STATUS_DETAIL[detail] = "not-applicable-platform"
> CVE_STATUS_WIN[description] = "Issue only applies on Windows"
>
> CVE_STATUS_PATCHED = "CVE-1234-0003 CVE-1234-0004"
> CVE_STATUS_PATCHED[status] = "Patched"
> CVE_STATUS_DETAIL[detail] = "fixed-version"
> CVE_STATUS_PATCHED[description] = "Fixed externally"
>
> Signed-off-by: Andrej Valek <andrej.valek@siemens.com>
> Signed-off-by: Peter Marko <peter.marko@siemens.com>
> ---
> meta/classes/cve-check.bbclass | 89 +++++++++++++++++++++++++++++-----
> meta/lib/oe/cve_check.py | 6 +++
> 2 files changed, 83 insertions(+), 12 deletions(-)

I'm afraid I really don't like this :(. Why?:

* we now have three different pieces of information, "status", "detail"
and "description" when we should only need two

* this needs a group mapping mechanism which is confusing above

* the information is spread over multiple differently named variables

* two pieces of the status information are connected in a hardcoded way

As a counter proposal, consider:

CVE_STATUS[CVE-1234-0001] = "not-applicable-platform: Issue only applies on Windows"
CVE_STATUS[CVE-1234-0002] = "not-applicable-platform: Issue only applies on Windows"
CVE_STATUS[CVE-1234-0003] = "fixed-version: Fixed externally"
CVE_STATUS[CVE-1234-0004] = "fixed-version: Fixed externally"

CVE_CHECK_STATUSMAP[not-applicable-platform] = "Ignored"
CVE_CHECK_STATUSMAP[fixed-version] = "Patched"

which conveys the same information with a slight bit of copy/paste but
not at a level I'd lose sleep over. To me it is a lot more readable.

Thoughts?

Cheers,

Richard