diff mbox series

Fix cve-check false negative

Message ID 20230328102349.57990-1-geoffrey.giry@smile.fr
State New
Headers show
Series Fix cve-check false negative | expand

Commit Message

Geoffrey GIRY March 28, 2023, 10:23 a.m. UTC
Fixes [YOCTO #14127]

NVD DB store version and update in the same value, separated by '_'.
The proposed patch check if the version from NVD DB contains a "_",
ie 9.2.0_p1 is convert to 9.2.0p1 before version comparison.

Reviewed-by: Yoann CONGAL <yoann.congal@smile.fr>
Signed-off-by: Geoffrey GIRY <geoffrey.giry@smile.fr>
---
 meta/classes/cve-check.bbclass            |  5 ++-
 meta/lib/oe/cve_check.py                  | 39 +++++++++++++++++++++++
 meta/lib/oeqa/selftest/cases/cve_check.py | 19 +++++++++++
 3 files changed, 62 insertions(+), 1 deletion(-)

Comments

Marta Rybczynska March 29, 2023, 4:45 a.m. UTC | #1
On Tue, Mar 28, 2023 at 12:24 PM Geoffrey GIRY <geoffrey.giry@smile.fr>
wrote:

> Fixes [YOCTO #14127]
>
> NVD DB store version and update in the same value, separated by '_'.
> The proposed patch check if the version from NVD DB contains a "_",
> ie 9.2.0_p1 is convert to 9.2.0p1 before version comparison.
>
>
Thank you for the patch. Which layers (and world builds) have you verified
it with?
I'm asking because versioning is always a complicated problems with
frequent exceptions to all "rules".

Kind regards,
Marta
Geoffrey GIRY March 29, 2023, 1:30 p.m. UTC | #2
Hello Marta,

We only tested core-image-minimal and some recipes that use the update
and release candidate formats (pX and -rcX)

Geoffrey GIRY
SMILE ECS - R&D Engineer

Le mer. 29 mars 2023 à 06:45, Marta Rybczynska <rybczynska@gmail.com> a écrit :
>
> On Tue, Mar 28, 2023 at 12:24 PM Geoffrey GIRY <geoffrey.giry@smile.fr> wrote:
>>
>> Fixes [YOCTO #14127]
>>
>> NVD DB store version and update in the same value, separated by '_'.
>> The proposed patch check if the version from NVD DB contains a "_",
>> ie 9.2.0_p1 is convert to 9.2.0p1 before version comparison.
>>
>
> Thank you for the patch. Which layers (and world builds) have you verified it with?
> I'm asking because versioning is always a complicated problems with frequent exceptions to all "rules".
>
> Kind regards,
> Marta
Marta Rybczynska March 31, 2023, 7:48 a.m. UTC | #3
Hello Geoffrey,
Would it be possible to run it over the world build of oe-core and possibly
meta-oe ?

My build farm will be available only next week and I would like to know if
there are unexpected changes.

Kind regards,
Marta

On Wed, Mar 29, 2023 at 3:31 PM Geoffrey GIRY <geoffrey.giry@smile.fr>
wrote:

> Hello Marta,
>
> We only tested core-image-minimal and some recipes that use the update
> and release candidate formats (pX and -rcX)
>
> Geoffrey GIRY
> SMILE ECS - R&D Engineer
>
> Le mer. 29 mars 2023 à 06:45, Marta Rybczynska <rybczynska@gmail.com> a
> écrit :
> >
> > On Tue, Mar 28, 2023 at 12:24 PM Geoffrey GIRY <geoffrey.giry@smile.fr>
> wrote:
> >>
> >> Fixes [YOCTO #14127]
> >>
> >> NVD DB store version and update in the same value, separated by '_'.
> >> The proposed patch check if the version from NVD DB contains a "_",
> >> ie 9.2.0_p1 is convert to 9.2.0p1 before version comparison.
> >>
> >
> > Thank you for the patch. Which layers (and world builds) have you
> verified it with?
> > I'm asking because versioning is always a complicated problems with
> frequent exceptions to all "rules".
> >
> > Kind regards,
> > Marta
>
Alexandre Belloni March 31, 2023, 8:59 a.m. UTC | #4
Hello Marta,

On 31/03/2023 09:48:27+0200, Marta Rybczynska wrote:
> Hello Geoffrey,
> Would it be possible to run it over the world build of oe-core and possibly
> meta-oe ?
> 

It has already run successfully and is already merged.

> My build farm will be available only next week and I would like to know if
> there are unexpected changes.
> 
> Kind regards,
> Marta
> 
> On Wed, Mar 29, 2023 at 3:31 PM Geoffrey GIRY <geoffrey.giry@smile.fr>
> wrote:
> 
> > Hello Marta,
> >
> > We only tested core-image-minimal and some recipes that use the update
> > and release candidate formats (pX and -rcX)
> >
> > Geoffrey GIRY
> > SMILE ECS - R&D Engineer
> >
> > Le mer. 29 mars 2023 à 06:45, Marta Rybczynska <rybczynska@gmail.com> a
> > écrit :
> > >
> > > On Tue, Mar 28, 2023 at 12:24 PM Geoffrey GIRY <geoffrey.giry@smile.fr>
> > wrote:
> > >>
> > >> Fixes [YOCTO #14127]
> > >>
> > >> NVD DB store version and update in the same value, separated by '_'.
> > >> The proposed patch check if the version from NVD DB contains a "_",
> > >> ie 9.2.0_p1 is convert to 9.2.0p1 before version comparison.
> > >>
> > >
> > > Thank you for the patch. Which layers (and world builds) have you
> > verified it with?
> > > I'm asking because versioning is always a complicated problems with
> > frequent exceptions to all "rules".
> > >
> > > Kind regards,
> > > Marta
> >

> 
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#179402): https://lists.openembedded.org/g/openembedded-core/message/179402
> Mute This Topic: https://lists.openembedded.org/mt/97902020/3617179
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [alexandre.belloni@bootlin.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>
Richard Purdie March 31, 2023, 9:15 a.m. UTC | #5
On Fri, 2023-03-31 at 10:59 +0200, Alexandre Belloni via
lists.openembedded.org wrote:
> Hello Marta,
> 
> On 31/03/2023 09:48:27+0200, Marta Rybczynska wrote:
> > Hello Geoffrey,
> > Would it be possible to run it over the world build of oe-core and possibly
> > meta-oe ?
> > 
> 
> It has already run successfully and is already merged.

It has merged but I think Marta's question is a valid one. The
autobuilder doesn't test this.

I'd note that our patchmetrics do for OE-Core:

https://autobuilder.yocto.io/pub/non-release/patchmetrics/cve-status-master.txt

and those don't look worse as a result. That doesn't cover meta-oe
though.

Cheers,

Richard
Geoffrey GIRY March 31, 2023, 4:03 p.m. UTC | #6
Hello,

Marta Rybczynska wrote:
> Would it be possible to run it over the world build of oe-core and possibly meta-oe ?

I tried the following:

The command `bibake -c cve_check world` reports the same CVE with and
without the patch applied.
I did test for oe-core alone, and found the same results as the autobuilder.
I also did test with meta-oe and found the same result (many more CVE,
but the same appears with and without the patch).

Sincerely,
Geoffrey GIRY
SMILE ECS - R&D Engineer
diff mbox series

Patch

diff --git a/meta/classes/cve-check.bbclass b/meta/classes/cve-check.bbclass
index 41fdf8363f..5e2da56046 100644
--- a/meta/classes/cve-check.bbclass
+++ b/meta/classes/cve-check.bbclass
@@ -260,7 +260,7 @@  def check_cves(d, patched_cves):
     """
     Connect to the NVD database and find unpatched cves.
     """
-    from oe.cve_check import Version
+    from oe.cve_check import Version, convert_cve_version
 
     pn = d.getVar("PN")
     real_pv = d.getVar("PV")
@@ -324,6 +324,9 @@  def check_cves(d, patched_cves):
                 if cve in cve_ignore:
                     ignored = True
 
+                version_start = convert_cve_version(version_start)
+                version_end = convert_cve_version(version_end)
+
                 if (operator_start == '=' and pv == version_start) or version_start == '-':
                     vulnerable = True
                 else:
diff --git a/meta/lib/oe/cve_check.py b/meta/lib/oe/cve_check.py
index 4f1d80f050..dbaa0b373a 100644
--- a/meta/lib/oe/cve_check.py
+++ b/meta/lib/oe/cve_check.py
@@ -179,3 +179,42 @@  def update_symlinks(target_path, link_path):
         if os.path.exists(os.path.realpath(link_path)):
             os.remove(link_path)
         os.symlink(os.path.basename(target_path), link_path)
+
+
+def convert_cve_version(version):
+    """
+    This function converts from CVE format to Yocto version format.
+    eg 8.3_p1 -> 8.3p1, 6.2_rc1 -> 6.2-rc1
+
+    Unless it is redefined using CVE_VERSION in the recipe,
+    cve_check uses the version in the name of the recipe (${PV})
+    to check vulnerabilities against a CVE in the database downloaded from NVD.
+
+    When the version has an update, i.e.
+    "p1" in OpenSSH 8.3p1,
+    "-rc1" in linux kernel 6.2-rc1,
+    the database stores the version as version_update (8.3_p1, 6.2_rc1).
+    Therefore, we must transform this version before comparing to the
+    recipe version.
+
+    In this case, the parameter of the function is 8.3_p1.
+    If the version uses the Release Candidate format, "rc",
+    this function replaces the '_' by '-'.
+    If the version uses the Update format, "p",
+    this function removes the '_' completely.
+    """
+    import re
+
+    matches = re.match('^([0-9.]+)_((p|rc)[0-9]+)$', version)
+
+    if not matches:
+        return version
+
+    version = matches.group(1)
+    update = matches.group(2)
+
+    if matches.group(3) == "rc":
+        return version + '-' + update
+
+    return version + update
+
diff --git a/meta/lib/oeqa/selftest/cases/cve_check.py b/meta/lib/oeqa/selftest/cases/cve_check.py
index ac47af1990..9534c9775c 100644
--- a/meta/lib/oeqa/selftest/cases/cve_check.py
+++ b/meta/lib/oeqa/selftest/cases/cve_check.py
@@ -54,6 +54,25 @@  class CVECheck(OESelftestTestCase):
         self.assertTrue( result ,msg="Failed to compare version with suffix '1.0_patch2' < '1.0_patch3'")
 
 
+    def test_convert_cve_version(self):
+        from oe.cve_check import convert_cve_version
+
+        # Default format
+        self.assertEqual(convert_cve_version("8.3"), "8.3")
+        self.assertEqual(convert_cve_version(""), "")
+
+        # OpenSSL format version
+        self.assertEqual(convert_cve_version("1.1.1t"), "1.1.1t")
+
+        # OpenSSH format
+        self.assertEqual(convert_cve_version("8.3_p1"), "8.3p1")
+        self.assertEqual(convert_cve_version("8.3_p22"), "8.3p22")
+
+        # Linux kernel format
+        self.assertEqual(convert_cve_version("6.2_rc8"), "6.2-rc8")
+        self.assertEqual(convert_cve_version("6.2_rc31"), "6.2-rc31")
+
+
     def test_recipe_report_json(self):
         config = """
 INHERIT += "cve-check"