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

["Valek, Andrej" <andrej.valek@siemens.com>]

On Fri, 2023-06-23 at 10:02 +0000, Ross Burton wrote:
> On 22 Jun 2023, at 13:00, Andrej Valek via lists.openembedded.org
> <andrej.valek=siemens.com@lists.openembedded.org> wrote:
> > - Replace CVE_CHECK_IGNORE with CVE_STATUS to be more flexible.
> > The CVE_STATUS should contain an information about status wich
> > is decoded in 3 items:
> > - generic status: "Ignored", "Patched" or "Unpatched"
> > - more detailed status enum
> > - description: free text describing reason for status
> 
> I think this needs to be clearer about what the intended use of the keywords
> are.
> 
> Is the canonical data the CVE_STATUS[CVE-1234-5678] attribute, and the mapping
> from the status there via CVE_CHECK_STATUSMAP simply for backwards
> compatibility with the existing file format? Is this deprecating the status
> fields in those files or is it just a high-level summary? Either way, that
> should be made clear.
> 
Yes, it's for backport compatibility, and extending the existing "Ignored",
"Patched" statuses with reasons.

> > +# Possible options for CVE statuses
> > +
> > +# used by this class internally when fix is detected (NVD DB version check
> > or CVE patch file)
> > +CVE_CHECK_STATUSMAP[patched] = "Patched"
> > +# use when this class does not detect backported patch (e.g. vendor kernel
> > repo with cherry-picked CVE patch)
> > +CVE_CHECK_STATUSMAP[backported-patch] = "Patched"
> > +# use when NVD DB does not mention patched versions of stable/LTS branches
> > which have upstream CVE backports
> > +CVE_CHECK_STATUSMAP[cpe-stable-backport] = "Patched"
> > +# use when NVD DB does not mention correct version or does not mention any
> > verion at all
> > +CVE_CHECK_STATUSMAP[fixed-version] = "Patched"
> 
> It bothers me that some of these status flags are working around the fact that
> the CPE is incorrect, when that CPE data can be fixed.  Instead of setting
> fixed-version, we can just mail NIST and fix the CPE.
> 
Yes, but while you're sending it, the current status has to be covered. And you
don't know, if the CPE will be fixed or not.
> > +# used internally by this class if CVE vulnerability is detected which is
> > not marked as fixed or ignored
> > +CVE_CHECK_STATUSMAP[unpatched] = "Unpatched"
> > +# use when CVE is confirmed by upstream but fix is still not available
> > +CVE_CHECK_STATUSMAP[vulnerable-investigating] = "Unpatched"
> > +
> > +# used for migration from old concept, do not use for new vulnerabilities
> > +CVE_CHECK_STATUSMAP[ignored] = "Ignored"
> > +# use when NVD DB wrongly indicates vulnerability which is actually for a
> > different component
> > +CVE_CHECK_STATUSMAP[cpe-incorrect] = "Ignored"
> > +# use when upstream does not accept the report as a vulnerability (e.g.
> > works as designed)
> > +CVE_CHECK_STATUSMAP[disputed] = "Ignored"
> > +# use when vulnerability depends on build or runtime configuration which is
> > not used
> > +CVE_CHECK_STATUSMAP[not-applicable-config] = "Ignored"
> > +# use when vulnerability affects other platform (e.g. Windows or Debian)
> > +CVE_CHECK_STATUSMAP[not-applicable-platform] = "Ignored"
> 
> > +# use when upstream acknowledged the vulnerability but does not plan to fix
> > it
> > +CVE_CHECK_STATUSMAP[upstream-wontfix] = "Ignored"
> 
> Is this any different to ‘disputed’?
> 
Of course. In the "upstream-wontfix" status, we know, that it won't be fixed.
But for "disputed" you don't know, if it's a bug or not.

> Do we expect to add a lot more statuses to this table, or for users to add
> their own values? It feels like maybe this should be a dict in
> lib/oe/cve_check.py instead of exposed in the data store.
> 
Exactly, know I moved it separated file, where users could extend their own
statuses. The current version is just a "basement" of supported one.

> > +    # Process CVE_STATUS_GROUPS to set multiple statuses and optional
> > detail or description at once
> > +    for cve_status_group in (d.getVar("CVE_STATUS_GROUPS") or "").split():
> > +        cve_group = d.getVar(cve_status_group)
> > +        if cve_group is not None:
> > +            for cve in cve_group.split():
> > +                d.setVarFlag("CVE_STATUS", cve,
> > d.getVarFlag(cve_status_group, "status"))
> > +        else:
> > +            bb.warn("CVE_STATUS_GROUPS contains undefined variable %s" %
> > cve_status_group)
> > +}
> 
> CVE_STATUS_GROUPS isn’t documented in the class or the commit message.
> 
Added a description directly into class.
> 

Regards,
Andrej