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

[Richard Purdie <richard.purdie@linuxfoundation.org>]

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