[v2] cve-check.bbclass: support embedded SW components with different version number

Many recipes embed other SW components. The name and version of the
embedded SW component differs from the main recipe. To detect CVEs in the
embedded SW component, it needs to be added to CVE_PRODUCT list using
name of the SW product in CVE database or with "vendor:product" syntax.
Then the version of the embedded SW component can be set using
CVE_VERSION_product variable.

For example in meta-arm, trusted-firmware-a embeds mbed_tls SW component.
Thus trusted-firmware-a can add CVE_PRODUCT for it since CVE database
uses product name "mbed_tls":

CVE_PRODUCT += "mbed_tls"

and set the version of mbed_tls:

CVE_VERSION_mbed_tls = "2.28.4"

(Real patches for both are a bit more complex due to conditional build
enabling mbed_tls support and due to mbed_tls version being set in an
.inc file.)

Now trusted-firmware-a CVE check output shows:

NOTE: recipe trusted-firmware-a-2.9.0+gitd3e71ead6ea5bc3555ac90a446efec84ef6c6122-r0: task do_cve_check: Started
WARNING: trusted-firmware-a-2.9.0+gitd3e71ead6ea5bc3555ac90a446efec84ef6c6122-r0 do_cve_check: Found unpatched CVE (CVE-2021-36647 CVE-2021-43666 CVE-2021-45451 CVE-2023-43615), for more information check /home/builder/src/base/build/tmp/work/arm64-poky-linux/trusted-firmware-a/2.9.0+gitd3e71ead6ea5bc3555ac90a446efec84ef6c6122/temp/cve.log
NOTE: recipe trusted-firmware-a-2.9.0+gitd3e71ead6ea5bc3555ac90a446efec84ef6c6122-r0: task do_cve_check: Succeeded

Here CVE-2023-43615 is a newly added and fixed CVE in version 2.28.5 and the CVEs
from 2021 need to be checked but are likely fixed in 2.28.3 and newer 2.28.y releases.

Note that CVE-2023-43615 does not impact trusted-firmware-a since it doesn't use
TLS or null or RC4 ciphers, but I think it's a good idea to extend
CVE checker for this use case. I hope the "CVE_VERSION_vendor:product"
does not cause odd breakages.

Signed-off-by: Mikko Rapeli <mikko.rapeli@linaro.org>
---
 meta/classes/create-spdx-2.2.bbclass |  2 +-
 meta/classes/cve-check.bbclass       | 28 +++++++++++++++++++---------
 meta/lib/oe/cve_check.py             |  5 ++---
 3 files changed, 22 insertions(+), 13 deletions(-)

v1: https://lists.openembedded.org/g/openembedded-core/message/189260

v2: adapt SPDX too

On 10/20/23 12:49 AM, Mikko Rapeli wrote:
> Many recipes embed other SW components. The name and version of the
> embedded SW component differs from the main recipe. To detect CVEs in the
> embedded SW component, it needs to be added to CVE_PRODUCT list using
> name of the SW product in CVE database or with "vendor:product" syntax.
> Then the version of the embedded SW component can be set using
> CVE_VERSION_product variable.
> 
> For example in meta-arm, trusted-firmware-a embeds mbed_tls SW component.
> Thus trusted-firmware-a can add CVE_PRODUCT for it since CVE database
> uses product name "mbed_tls":
> 
> CVE_PRODUCT += "mbed_tls"
> 
> and set the version of mbed_tls:
> 
> CVE_VERSION_mbed_tls = "2.28.4"
> 
> (Real patches for both are a bit more complex due to conditional build
> enabling mbed_tls support and due to mbed_tls version being set in an
> .inc file.)
> 
> Now trusted-firmware-a CVE check output shows:
> 
> NOTE: recipe trusted-firmware-a-2.9.0+gitd3e71ead6ea5bc3555ac90a446efec84ef6c6122-r0: task do_cve_check: Started
> WARNING: trusted-firmware-a-2.9.0+gitd3e71ead6ea5bc3555ac90a446efec84ef6c6122-r0 do_cve_check: Found unpatched CVE (CVE-2021-36647 CVE-2021-43666 CVE-2021-45451 CVE-2023-43615), for more information check /home/builder/src/base/build/tmp/work/arm64-poky-linux/trusted-firmware-a/2.9.0+gitd3e71ead6ea5bc3555ac90a446efec84ef6c6122/temp/cve.log
> NOTE: recipe trusted-firmware-a-2.9.0+gitd3e71ead6ea5bc3555ac90a446efec84ef6c6122-r0: task do_cve_check: Succeeded
> 
> Here CVE-2023-43615 is a newly added and fixed CVE in version 2.28.5 and the CVEs
> from 2021 need to be checked but are likely fixed in 2.28.3 and newer 2.28.y releases.
> 
> Note that CVE-2023-43615 does not impact trusted-firmware-a since it doesn't use
> TLS or null or RC4 ciphers, but I think it's a good idea to extend
> CVE checker for this use case. I hope the "CVE_VERSION_vendor:product"
> does not cause odd breakages.
> 

This is a good improvement. There is one more kink to it, where the 
vendored subpackage might be there in source but we might have 
configured the recipe to use the system version of the package instead, 
so how do we cater to such situation ?

> Signed-off-by: Mikko Rapeli <mikko.rapeli@linaro.org>
> ---
>   meta/classes/create-spdx-2.2.bbclass |  2 +-
>   meta/classes/cve-check.bbclass       | 28 +++++++++++++++++++---------
>   meta/lib/oe/cve_check.py             |  5 ++---
>   3 files changed, 22 insertions(+), 13 deletions(-)
> 
> v1: https://lists.openembedded.org/g/openembedded-core/message/189260
> 
> v2: adapt SPDX too
> 
> diff --git a/meta/classes/create-spdx-2.2.bbclass b/meta/classes/create-spdx-2.2.bbclass
> index b0aef80db1..5b842e67ee 100644
> --- a/meta/classes/create-spdx-2.2.bbclass
> +++ b/meta/classes/create-spdx-2.2.bbclass
> @@ -590,7 +590,7 @@ python do_create_spdx() {
>       if patched_cves:
>           recipe.sourceInfo = "CVEs fixed: " + patched_cves
>   
> -    cpe_ids = oe.cve_check.get_cpe_ids(d.getVar("CVE_PRODUCT"), d.getVar("CVE_VERSION"))
> +    cpe_ids = oe.cve_check.get_cpe_ids(d, d.getVar("CVE_PRODUCT"), d.getVar("CVE_VERSION"))
>       if cpe_ids:
>           for cpe_id in cpe_ids:
>               cpe = oe.spdx.SPDXExternalReference()
> diff --git a/meta/classes/cve-check.bbclass b/meta/classes/cve-check.bbclass
> index b55f4299da..9c41d54188 100644
> --- a/meta/classes/cve-check.bbclass
> +++ b/meta/classes/cve-check.bbclass
> @@ -309,7 +309,16 @@ def check_cves(d, patched_cves):
>       # If this has been unset then we're not scanning for CVEs here (for example, image recipes)
>       if not products:
>           return ([], [], [], [])
> -    pv = d.getVar("CVE_VERSION").split("+git")[0]
> +
> +    # Version is PV, CVE_VERSION or CVE_VERSION_%s where %s is one of the entries in CVE_PRODUCT.
> +    # Enables checking embedded SW component CVEs provided that CVE_PRODUCT contains the embedded SW
> +    # component and that version of that component is set via CVE_VERSION_embedded_component variable.
> +    pv = {}
> +    for product in products:
> +        version = (d.getVar("CVE_VERSION_%s" % product) or "").split("+git")[0]
> +        if version == "":
> +            version = d.getVar("CVE_VERSION").split("+git")[0]
> +        pv[product] = version
>   
>       # If the recipe has been skipped/ignored we return empty lists
>       if pn in d.getVar("CVE_CHECK_SKIP_RECIPE").split():
> @@ -329,6 +338,7 @@ def check_cves(d, patched_cves):
>   
>       # For each of the known product names (e.g. curl has CPEs using curl and libcurl)...
>       for product in products:
> +        full_product = product
>           cves_in_product = False
>           if ":" in product:
>               vendor, product = product.split(":", 1)
> @@ -341,7 +351,7 @@ def check_cves(d, patched_cves):
>               cve = cverow[0]
>   
>               if cve in cve_ignore:
> -                bb.note("%s-%s ignores %s" % (product, pv, cve))
> +                bb.note("%s-%s ignores %s" % (product, pv[full_product], cve))
>                   cves_ignored.append(cve)
>                   continue
>               elif cve in patched_cves:
> @@ -366,27 +376,27 @@ def check_cves(d, patched_cves):
>                   version_start = convert_cve_version(version_start)
>                   version_end = convert_cve_version(version_end)
>   
> -                if (operator_start == '=' and pv == version_start) or version_start == '-':
> +                if (operator_start == '=' and pv[full_product] == version_start) or version_start == '-':
>                       vulnerable = True
>                   else:
>                       if operator_start:
>                           try:
> -                            vulnerable_start =  (operator_start == '>=' and Version(pv,suffix) >= Version(version_start,suffix))
> -                            vulnerable_start |= (operator_start == '>' and Version(pv,suffix) > Version(version_start,suffix))
> +                            vulnerable_start =  (operator_start == '>=' and Version(pv[full_product],suffix) >= Version(version_start,suffix))
> +                            vulnerable_start |= (operator_start == '>' and Version(pv[full_product],suffix) > Version(version_start,suffix))
>                           except:
>                               bb.warn("%s: Failed to compare %s %s %s for %s" %
> -                                    (product, pv, operator_start, version_start, cve))
> +                                    (product, pv[full_product], operator_start, version_start, cve))
>                               vulnerable_start = False
>                       else:
>                           vulnerable_start = False
>   
>                       if operator_end:
>                           try:
> -                            vulnerable_end  = (operator_end == '<=' and Version(pv,suffix) <= Version(version_end,suffix) )
> -                            vulnerable_end |= (operator_end == '<' and Version(pv,suffix) < Version(version_end,suffix) )
> +                            vulnerable_end  = (operator_end == '<=' and Version(pv[full_product],suffix) <= Version(version_end,suffix) )
> +                            vulnerable_end |= (operator_end == '<' and Version(pv[full_product],suffix) < Version(version_end,suffix) )
>                           except:
>                               bb.warn("%s: Failed to compare %s %s %s for %s" %
> -                                    (product, pv, operator_end, version_end, cve))
> +                                    (product, pv[full_product], operator_end, version_end, cve))
>                               vulnerable_end = False
>                       else:
>                           vulnerable_end = False
> diff --git a/meta/lib/oe/cve_check.py b/meta/lib/oe/cve_check.py
> index 3979d521d1..c3514f7a27 100644
> --- a/meta/lib/oe/cve_check.py
> +++ b/meta/lib/oe/cve_check.py
> @@ -140,15 +140,14 @@ def get_patched_cves(d):
>       return patched_cves
>   
>   
> -def get_cpe_ids(cve_product, version):
> +def get_cpe_ids(d, cve_product, cve_version):
>       """
>       Get list of CPE identifiers for the given product and version
>       """
>   
> -    version = version.split("+git")[0]
> -
>       cpe_ids = []
>       for product in cve_product.split():
> +        version = (d.getVar("CVE_VERSION_%s" % product) or cve_version).split("+git")[0]
>           # CVE_PRODUCT in recipes may include vendor information for CPE identifiers. If not,
>           # use wildcard for vendor.
>           if ":" in product:
> 
> 
> 
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#189501): https://lists.openembedded.org/g/openembedded-core/message/189501
> Mute This Topic: https://lists.openembedded.org/mt/102076964/1997914
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [raj.khem@gmail.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>

Hi,

On Fri, Oct 20, 2023 at 08:54:14AM -0700, Khem Raj wrote:
> On 10/20/23 12:49 AM, Mikko Rapeli wrote:
> > Many recipes embed other SW components. The name and version of the
> > embedded SW component differs from the main recipe. To detect CVEs in the
> > embedded SW component, it needs to be added to CVE_PRODUCT list using
> > name of the SW product in CVE database or with "vendor:product" syntax.
> > Then the version of the embedded SW component can be set using
> > CVE_VERSION_product variable.
> > 
> > For example in meta-arm, trusted-firmware-a embeds mbed_tls SW component.
> > Thus trusted-firmware-a can add CVE_PRODUCT for it since CVE database
> > uses product name "mbed_tls":
> > 
> > CVE_PRODUCT += "mbed_tls"
> > 
> > and set the version of mbed_tls:
> > 
> > CVE_VERSION_mbed_tls = "2.28.4"
> > 
> > (Real patches for both are a bit more complex due to conditional build
> > enabling mbed_tls support and due to mbed_tls version being set in an
> > .inc file.)
> > 
> > Now trusted-firmware-a CVE check output shows:
> > 
> > NOTE: recipe trusted-firmware-a-2.9.0+gitd3e71ead6ea5bc3555ac90a446efec84ef6c6122-r0: task do_cve_check: Started
> > WARNING: trusted-firmware-a-2.9.0+gitd3e71ead6ea5bc3555ac90a446efec84ef6c6122-r0 do_cve_check: Found unpatched CVE (CVE-2021-36647 CVE-2021-43666 CVE-2021-45451 CVE-2023-43615), for more information check /home/builder/src/base/build/tmp/work/arm64-poky-linux/trusted-firmware-a/2.9.0+gitd3e71ead6ea5bc3555ac90a446efec84ef6c6122/temp/cve.log
> > NOTE: recipe trusted-firmware-a-2.9.0+gitd3e71ead6ea5bc3555ac90a446efec84ef6c6122-r0: task do_cve_check: Succeeded
> > 
> > Here CVE-2023-43615 is a newly added and fixed CVE in version 2.28.5 and the CVEs
> > from 2021 need to be checked but are likely fixed in 2.28.3 and newer 2.28.y releases.
> > 
> > Note that CVE-2023-43615 does not impact trusted-firmware-a since it doesn't use
> > TLS or null or RC4 ciphers, but I think it's a good idea to extend
> > CVE checker for this use case. I hope the "CVE_VERSION_vendor:product"
> > does not cause odd breakages.
> > 
> 
> This is a good improvement. There is one more kink to it, where the vendored
> subpackage might be there in source but we might have configured the recipe
> to use the system version of the package instead, so how do we cater to such
> situation ?

Very good point. I think yocto project maintainer should recommend recipe maintainers
what to do in these cases. I think safest option is to remove/delete such code paths from
upstream sources in do_patch(). Debian does something similar also due to license compliance
with their pristine/dfsg source package format
https://www.debian.org/doc/manuals/developers-reference/best-pkging-practices.html#repackaged-upstream-source
when they can't distributed certain files inside the source package.

This issue goes way beyond CVE related data though. LICENSE, PN and PV are affected.
AFAIK, currently recipe maintainers decide on what goes into LICENSE for example,
based on how they have configured the SW component build and dependencies. These may
not be correct though and for complex SW components this really is an issue where
users who create real products need to check a lot of details. High level SW components
like javascript engines and browser are very eager to build their own versions SW components
which are already available as recipes in the build environment.

At least for CVE scanning, this patch enables telling the tooling "this recipe
embeds a custom version of tool/lib xyz" so recipe maintainers can fill in the data
if they know and care about the details. For license compliance, I hope LICENSE is up to date :)
though I know that the deeper on looks the more problems will pop up...

Cheers,

-Mikko

Hi Richard,

I see master-next has both v1 and v2 of this change. If there is a rebase,
maybe you can squash them into a single commit which is v2.

Cheers,

-Mikko

On Thu, 2023-10-26 at 10:33 +0300, Mikko Rapeli wrote:
> Hi Richard,
> 
> I see master-next has both v1 and v2 of this change. If there is a rebase,
> maybe you can squash them into a single commit which is v2.

I've been meaning to look at what was going on there.

Many of the patches in -next are in a holding pattern whilst
discussions come to conclusions or there was feedback needed which I've
not gotten to yet.

Whilst I understand the corner case being addressed here, I worry it
creates a whole new set of issues (such as the CVE exclusions for an
embedded component needing to be duplicated). I'm therefore not
convinced it is quite in the right form yet. I've not had enough time
to think about any suggestions to improve though.

Cheers,

Richard

On 20 Oct 2023, at 08:49, Mikko Rapeli via lists.openembedded.org <mikko.rapeli=linaro.org@lists.openembedded.org> wrote:
> For example in meta-arm, trusted-firmware-a embeds mbed_tls SW component.
> Thus trusted-firmware-a can add CVE_PRODUCT for it since CVE database
> uses product name "mbed_tls":
> 
> CVE_PRODUCT += "mbed_tls"

Right now, CVE_PRODUCT is the set of names that the project is known by.  Pathological case being curl:

CVE_PRODUCT = "haxx:curl haxx:libcurl curl:curl curl:libcurl libcurl:libcurl daniel_stenberg:curl”

So this a change of semantics.

> and set the version of mbed_tls:
> 
> CVE_VERSION_mbed_tls = “2.28.4”
...
> CVE checker for this use case. I hope the "CVE_VERSION_vendor:product"
> does not cause odd breakages.

It will, because bitbake won’t know that the colon is meant to be part of the vendor/product pair and it looks exactly like an override.

Instead of trying to shoehorn this change into the existing variables, maybe we need new ones.

If we solve this problem we don’t want to solve it for just eg TF-A embedding mbedtls, but also arbitrary Rust applications which statically link to many crates.  It would be great if the same tooling that generates the SRC_URI with the 100 cargo:// entries would also set the right variables so that CVE tooling knows what crates and versions are in the recipe.

Totally making stuff up with little thinking, but how about we leave CVE_PRODUCT and CVE_VERSION as the set of known names and version for the top-level product, but add a new variable using varflags for each of the integrated components?

CVE_PRODUCT = “trusted-firmware-a”
CVE_COMPONENTS[mbedtls] = “3.30”

We could even do something clever and allow CVE_COMPONENTS = “” to mean “all of the components”, but if CVE_COMPONENTS itself is set then that’s the set of components that are actually being used, to handle cases where eg mbedtls may or may not be enabled via a PACKAGECONFIG.

Cheers,
Ross