[PATCH] manuals: further documentation for cve-check

[PATCH] manuals: further documentation for cve-check

[Michael Opdenacker]

This adds details about the actual implementation
of vulnerability checks, about how to fix or ignore
vulnerabilities in recipes, and documents the
CVE_CHECK_PN_WHITELIST and CVE_CHECK_WHITELIST variables.

Signed-off-by: Michael Opdenacker <michael.opdenacker@bootlin.com>
---
 documentation/dev-manual/common-tasks.rst | 67 +++++++++++++++++++++++
 documentation/ref-manual/variables.rst    | 13 ++++-
 2 files changed, 79 insertions(+), 1 deletion(-)

diff --git a/documentation/dev-manual/common-tasks.rst b/documentation/dev-manual/common-tasks.rst
index 7fa0df4d39..a7eb4642de 100644
--- a/documentation/dev-manual/common-tasks.rst
+++ b/documentation/dev-manual/common-tasks.rst
@@ -11131,6 +11131,73 @@ Enabling vulnerabily tracking in recipes
 The :term:`CVE_PRODUCT` variable defines the name used to match the recipe name
 against the name in the upstream `NIST CVE database <https://nvd.nist.gov/>`__.
 
+Editing recipes to fix vulnerabilities
+--------------------------------------
+
+To fix a given known vulnerability, you need to add a patch file to your recipe. Here's
+an example from the :oe_layerindex:`ffmpeg recipe</layerindex/recipe/47350>`::
+
+   SRC_URI = "https://www.ffmpeg.org/releases/${BP}.tar.xz \
+              file://0001-libavutil-include-assembly-with-full-path-from-sourc.patch \
+              file://fix-CVE-2020-20446.patch \
+              file://fix-CVE-2020-20453.patch \
+              file://fix-CVE-2020-22015.patch \
+              file://fix-CVE-2020-22021.patch \
+              file://fix-CVE-2020-22033-CVE-2020-22019.patch \
+              file://fix-CVE-2021-33815.patch \
+
+``cve-check.bbclass`` defines two ways of supplying a patch for a given CVE. The first
+way is to use a patch file name that matches the below pattern::
+
+   cve_file_name_match = re.compile(".*([Cc][Vv][Ee]\-\d{4}\-\d+)")
+
+As shown in the example above, multiple CVE IDs can appear in a patch file name,
+but the ``cve-check.bbclass`` code will only consider the last CVE ID in the
+file name as patched.
+
+The second way to recognize a patched CVE ID is when a line matching the
+below pattern is found in any patch file provided by the recipe::
+
+  cve_match = re.compile("CVE:( CVE\-\d{4}\-\d+)+")
+
+This allows a single patch file to address multiple CVE IDs at the same time.
+
+Of course, another way to fix vulnerabilities is to upgrade to a version
+of the package which is not impacted, typically a more recent one.
+The NIST database knows which versions are vulnerable and which ones
+are not.
+
+Last but not least, you can choose to ignore vulnerabilities through
+the :term:`CVE_CHECK_PN_WHITELIST` and :term:`CVE_CHECK_WHITELIST`
+variables.
+
+Implementation details
+----------------------
+
+Here's what ``cve-check.bbclass`` does to find unpatched CVE IDs.
+
+First the code goes through each patch file provided by a recipe. If a valid CVE ID
+is found in the name of the file, the corresponding CVE is considered as patched.
+Don't forget that if multiple CVE IDs are found in the file name, only the last
+one is considered. Then, the code looks for ``CVE: CVE-ID`` lines in the patch
+file. The found CVE IDs are also considered as patched.
+
+Then, the code looks-up all the CVE IDs in the NIST database for all the
+products defined in :term:`CVE_PRODUCT`. Then, for each found CVE:
+
+ - If the package name (:term:`PN`) is part of
+   :term:`CVE_CHECK_PN_WHITELIST`, it is considered as patched.
+
+ - If the CVE ID is part of :term:`CVE_CHECK_WHITELIST`, it is
+   considered as patched too.
+
+ - If the CVE ID is part of the patched CVEs for the recipe, it is
+   already considered as patched.
+
+ - Otherwise, the code checks whether the recipe version (:term:`PV`)
+   is within the range of versions impacted by the CVE. If so, the CVE
+   is considered as unpatched.
+
 The CVE database is stored in :term:`DL_DIR` and can be inspected using
 ``sqlite3`` command as follows::
 
diff --git a/documentation/ref-manual/variables.rst b/documentation/ref-manual/variables.rst
index 1150940133..cc7be01fc5 100644
--- a/documentation/ref-manual/variables.rst
+++ b/documentation/ref-manual/variables.rst
@@ -1471,11 +1471,22 @@ system and gives an overview of their function and contents.
          variable only in certain contexts (e.g. when building for kernel
          and kernel module recipes).
 
+   :term:`CVE_CHECK_PN_WHITELIST`
+      The list of package names (:term:`PN`) for which
+      CVE vulnerabilities are ignored.
+
+   :term:`CVE_CHECK_WHITELIST`
+      The list of vulnerability CVE IDs which are ignored. Here is
+      an example from the :oe_layerindex:`Python3 recipe</layerindex/recipe/23823>`::
+
+         # This is windows only issue.
+         CVE_CHECK_WHITELIST += "CVE-2020-15523"
+
    :term:`CVE_PRODUCT`
       In a recipe, defines the name used to match the recipe name
       against the name in the upstream `NIST CVE database <https://nvd.nist.gov/>`__.
 
-      The default is ${:term:`BPN`}. If it does not match the name in NIST CVE
+      The default is ${:term:`BPN`}. If it does not match the name in the NIST CVE
       database or matches with multiple entries in the database, the default
       value needs to be changed.
 
-- 
2.25.1

Re: [docs] [PATCH] manuals: further documentation for cve-check

[Quentin Schulz]

Hi Michael,

On Fri, Aug 06, 2021 at 12:37:20PM +0200, Michael Opdenacker wrote:
> This adds details about the actual implementation
> of vulnerability checks, about how to fix or ignore
> vulnerabilities in recipes, and documents the
> CVE_CHECK_PN_WHITELIST and CVE_CHECK_WHITELIST variables.
> 
> Signed-off-by: Michael Opdenacker <michael.opdenacker@bootlin.com>
> ---
>  documentation/dev-manual/common-tasks.rst | 67 +++++++++++++++++++++++
>  documentation/ref-manual/variables.rst    | 13 ++++-
>  2 files changed, 79 insertions(+), 1 deletion(-)
> 
> diff --git a/documentation/dev-manual/common-tasks.rst b/documentation/dev-manual/common-tasks.rst
> index 7fa0df4d39..a7eb4642de 100644
> --- a/documentation/dev-manual/common-tasks.rst
> +++ b/documentation/dev-manual/common-tasks.rst
> @@ -11131,6 +11131,73 @@ Enabling vulnerabily tracking in recipes
>  The :term:`CVE_PRODUCT` variable defines the name used to match the recipe name
>  against the name in the upstream `NIST CVE database <https://urldefense.proofpoint.com/v2/url?u=https-3A__nvd.nist.gov_&d=DwIDAg&c=_sEr5x9kUWhuk4_nFwjJtA&r=LYjLexDn7rXIzVmkNPvw5ymA1XTSqHGq8yBP6m6qZZ4njZguQhZhkI_-172IIy1t&m=9ML--ZrDALJnWP-cdKUdSr7aZojrZSP_ZftKiDw8b90&s=X75uMn2Jgp5D1ZeIRtUVm_E0hUPd6JAN5YALxFwNawI&e= >`__.
>  
> +Editing recipes to fix vulnerabilities
> +--------------------------------------
> +
> +To fix a given known vulnerability, you need to add a patch file to your recipe. Here's
> +an example from the :oe_layerindex:`ffmpeg recipe</layerindex/recipe/47350>`::
> +
> +   SRC_URI = "https://urldefense.proofpoint.com/v2/url?u=https-3A__www.ffmpeg.org_releases_-24-257BBP-257D.tar.xz&d=DwIDAg&c=_sEr5x9kUWhuk4_nFwjJtA&r=LYjLexDn7rXIzVmkNPvw5ymA1XTSqHGq8yBP6m6qZZ4njZguQhZhkI_-172IIy1t&m=9ML--ZrDALJnWP-cdKUdSr7aZojrZSP_ZftKiDw8b90&s=jlaowuJQXhq1Lp2jXZTs3bYLiFspslcuyaTiPLrhe-o&e=  \
> +              file://0001-libavutil-include-assembly-with-full-path-from-sourc.patch \
> +              file://fix-CVE-2020-20446.patch \
> +              file://fix-CVE-2020-20453.patch \
> +              file://fix-CVE-2020-22015.patch \
> +              file://fix-CVE-2020-22021.patch \
> +              file://fix-CVE-2020-22033-CVE-2020-22019.patch \
> +              file://fix-CVE-2021-33815.patch \
> +
> +``cve-check.bbclass`` defines two ways of supplying a patch for a given CVE. The first

I think we could afford saying a few words about this class in the
Reference Manual? In which case:
s/``cve-check.bbclass``/:ref:`cve-check.bbclass <ref-classes-cve-check>`/ ?

> +way is to use a patch file name that matches the below pattern::
> +

Not sure about that one but s/file name/filename/ ? If chosen, fix the
other occurences at the same time :)

> +   cve_file_name_match = re.compile(".*([Cc][Vv][Ee]\-\d{4}\-\d+)")
> +
> +As shown in the example above, multiple CVE IDs can appear in a patch file name,
> +but the ``cve-check.bbclass`` code will only consider the last CVE ID in the
> +file name as patched.
> +
> +The second way to recognize a patched CVE ID is when a line matching the
> +below pattern is found in any patch file provided by the recipe::
> +
> +  cve_match = re.compile("CVE:( CVE\-\d{4}\-\d+)+")
> +
> +This allows a single patch file to address multiple CVE IDs at the same time.
> +
> +Of course, another way to fix vulnerabilities is to upgrade to a version
> +of the package which is not impacted, typically a more recent one.
> +The NIST database knows which versions are vulnerable and which ones
> +are not.
> +
> +Last but not least, you can choose to ignore vulnerabilities through
> +the :term:`CVE_CHECK_PN_WHITELIST` and :term:`CVE_CHECK_WHITELIST`
> +variables.
> +
> +Implementation details
> +----------------------
> +
> +Here's what ``cve-check.bbclass`` does to find unpatched CVE IDs.
> +

Would need the :ref: link here too if chosen.

> +First the code goes through each patch file provided by a recipe. If a valid CVE ID
> +is found in the name of the file, the corresponding CVE is considered as patched.
> +Don't forget that if multiple CVE IDs are found in the file name, only the last
> +one is considered. Then, the code looks for ``CVE: CVE-ID`` lines in the patch
> +file. The found CVE IDs are also considered as patched.
> +
> +Then, the code looks-up all the CVE IDs in the NIST database for all the

s/looks-up all the CVE IDs/looks all the CVE IDs up/ ?

> +products defined in :term:`CVE_PRODUCT`. Then, for each found CVE:
> +
> + - If the package name (:term:`PN`) is part of
> +   :term:`CVE_CHECK_PN_WHITELIST`, it is considered as patched.
> +
> + - If the CVE ID is part of :term:`CVE_CHECK_WHITELIST`, it is
> +   considered as patched too.
> +
> + - If the CVE ID is part of the patched CVEs for the recipe, it is
> +   already considered as patched.
> +
> + - Otherwise, the code checks whether the recipe version (:term:`PV`)
> +   is within the range of versions impacted by the CVE. If so, the CVE
> +   is considered as unpatched.
> +
>  The CVE database is stored in :term:`DL_DIR` and can be inspected using
>  ``sqlite3`` command as follows::
>  
> diff --git a/documentation/ref-manual/variables.rst b/documentation/ref-manual/variables.rst
> index 1150940133..cc7be01fc5 100644
> --- a/documentation/ref-manual/variables.rst
> +++ b/documentation/ref-manual/variables.rst
> @@ -1471,11 +1471,22 @@ system and gives an overview of their function and contents.
>           variable only in certain contexts (e.g. when building for kernel
>           and kernel module recipes).
>  
> +   :term:`CVE_CHECK_PN_WHITELIST`
> +      The list of package names (:term:`PN`) for which
> +      CVE vulnerabilities are ignored.
> +

Since CVE stands for "Common Vulnerabilities and Exposures", isn't
adding vulnerabilities redundant?

> +   :term:`CVE_CHECK_WHITELIST`
> +      The list of vulnerability CVE IDs which are ignored. Here is

Ditto.

Thanks for the patch :)
Quentin

Re: [docs] [PATCH] manuals: further documentation for cve-check

[Michael Opdenacker]

Hi Quentin

Thank you once again for the quick (and thorough!) review!

On 8/6/21 1:41 PM, Quentin Schulz wrote:
>
> I think we could afford saying a few words about this class in the
> Reference Manual? In which case:
> s/``cve-check.bbclass``/:ref:`cve-check.bbclass <ref-classes-cve-check>`/ ?

Very good idea, and it wouldn't cost too much. I'll do it (good catch by
the way).

>
>> +way is to use a patch file name that matches the below pattern::
>> +
> Not sure about that one but s/file name/filename/ ? If chosen, fix the
> other occurences at the same time :)

Good idea!

$ git grep "filename" |wc -l
56
git grep "file name" |wc -l
22

So "filename" wins. That's an even stronger win for the plural variant.
I'll update.

>
>> +   cve_file_name_match = re.compile(".*([Cc][Vv][Ee]\-\d{4}\-\d+)")
>> +
>> +As shown in the example above, multiple CVE IDs can appear in a patch file name,
>> +but the ``cve-check.bbclass`` code will only consider the last CVE ID in the
>> +file name as patched.
>> +
>> +The second way to recognize a patched CVE ID is when a line matching the
>> +below pattern is found in any patch file provided by the recipe::
>> +
>> +  cve_match = re.compile("CVE:( CVE\-\d{4}\-\d+)+")
>> +
>> +This allows a single patch file to address multiple CVE IDs at the same time.
>> +
>> +Of course, another way to fix vulnerabilities is to upgrade to a version
>> +of the package which is not impacted, typically a more recent one.
>> +The NIST database knows which versions are vulnerable and which ones
>> +are not.
>> +
>> +Last but not least, you can choose to ignore vulnerabilities through
>> +the :term:`CVE_CHECK_PN_WHITELIST` and :term:`CVE_CHECK_WHITELIST`
>> +variables.
>> +
>> +Implementation details
>> +----------------------
>> +
>> +Here's what ``cve-check.bbclass`` does to find unpatched CVE IDs.
>> +
> Would need the :ref: link here too if chosen.


Sure.

>
>> +First the code goes through each patch file provided by a recipe. If a valid CVE ID
>> +is found in the name of the file, the corresponding CVE is considered as patched.
>> +Don't forget that if multiple CVE IDs are found in the file name, only the last
>> +one is considered. Then, the code looks for ``CVE: CVE-ID`` lines in the patch
>> +file. The found CVE IDs are also considered as patched.
>> +
>> +Then, the code looks-up all the CVE IDs in the NIST database for all the
> s/looks-up all the CVE IDs/looks all the CVE IDs up/ ?


Pfooh, it seems that both are valid buth "all the CVE IDs" is perhaps a
bit too long for "shifting" to be the preferred solution. See
https://en.wikipedia.org/wiki/English_phrasal_verbs#Shifting (nice
reading by the way, at least the first sections that I read).

Actually, the hyphen is bad, so I'll write:

"looks up all the CVE IDs"

>
>> +      The list of package names (:term:`PN`) for which
>> +      CVE vulnerabilities are ignored.
>> +
> Since CVE stands for "Common Vulnerabilities and Exposures", isn't
> adding vulnerabilities redundant?


Oh yes, agreed!

>
>> +   :term:`CVE_CHECK_WHITELIST`
>> +      The list of vulnerability CVE IDs which are ignored. Here is
> Ditto.
>
> Thanks for the patch :)
Thanks again for the review.
Cheers,
Michael

-- 
Michael Opdenacker, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [docs] [PATCH] manuals: further documentation for cve-check

[Michael Opdenacker]

Hello Quentin,

Preemptive nitpicking ;-)

On 8/6/21 4:05 PM, Michael Opdenacker wrote:
>
>> Since CVE stands for "Common Vulnerabilities and Exposures", isn't
>> adding vulnerabilities redundant?
>
> Oh yes, agreed!


Worse, CVE is already a plural acronym. So, we should use "CVE" instead
of "CVEs", otherwise we're making a multi-plural! I'm sure you would
have ended up noticing this.

Fortunately, not all readers are like you, but those are the best ones :-P
Cheers,
Michael.

-- 
Michael Opdenacker, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [docs] [PATCH] manuals: further documentation for cve-check

[Peter Kjellerstedt]

> -----Original Message-----
> From: docs@lists.yoctoproject.org <docs@lists.yoctoproject.org> On Behalf
> Of Michael Opdenacker
> Sent: den 6 augusti 2021 16:59
> To: Quentin Schulz <quentin.schulz@theobroma-systems.com>
> Cc: docs@lists.yoctoproject.org
> Subject: Re: [docs] [PATCH] manuals: further documentation for cve-check
> 
> Hello Quentin,
> 
> Preemptive nitpicking ;-)
> 
> On 8/6/21 4:05 PM, Michael Opdenacker wrote:
> >
> >> Since CVE stands for "Common Vulnerabilities and Exposures", isn't
> >> adding vulnerabilities redundant?

Not much different from using, e.g., "a CD disc". ;) 

> > Oh yes, agreed!
> 
> Worse, CVE is already a plural acronym. So, we should use "CVE" instead
> of "CVEs", otherwise we're making a multi-plural! I'm sure you would
> have ended up noticing this.

That doesn't make sense. I would argue that "a CVE" is singular and refers 
to one entry in the CVE database, regardless of what the acronym actually 
stands for. Thus "bugs and CVEs" make sense, while "bugs and CVE" does not.

> Fortunately, not all readers are like you, but those are the best ones :-P
> Cheers,
> Michael.

//Peter

Re: [docs] [PATCH] manuals: further documentation for cve-check

[Michael Opdenacker]

Hi Peter,

Thank you for your review and feedback.

On 8/7/21 5:33 AM, Peter Kjellerstedt wrote:
>
>> Worse, CVE is already a plural acronym. So, we should use "CVE" instead
>> of "CVEs", otherwise we're making a multi-plural! I'm sure you would
>> have ended up noticing this.
> That doesn't make sense. I would argue that "a CVE" is singular and refers 
> to one entry in the CVE database, regardless of what the acronym actually 
> stands for. Thus "bugs and CVEs" make sense, while "bugs and CVE" does not.


You have a point, and this is consistent with the CVE warning messages
issued by the cve-check class.
I fixed this in "master-next", actually before Richard merged it into
master.

Thanks again,
Cheers,
Michael.

-- 
Michael Opdenacker, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [docs] [PATCH] manuals: further documentation for cve-check

[Michael Opdenacker]

Greetings,

On 8/6/21 12:37 PM, Michael Opdenacker wrote:
> +Implementation details
> +----------------------
> +
> +Here's what ``cve-check.bbclass`` does to find unpatched CVE IDs.
> +
> +First the code goes through each patch file provided by a recipe. If a valid CVE ID
> +is found in the name of the file, the corresponding CVE is considered as patched.
> +Don't forget that if multiple CVE IDs are found in the file name, only the last
> +one is considered. Then, the code looks for ``CVE: CVE-ID`` lines in the patch
> +file. The found CVE IDs are also considered as patched.
> +
> +Then, the code looks-up all the CVE IDs in the NIST database for all the
> +products defined in :term:`CVE_PRODUCT`. Then, for each found CVE:
> +
> + - If the package name (:term:`PN`) is part of
> +   :term:`CVE_CHECK_PN_WHITELIST`, it is considered as patched.
> +
> + - If the CVE ID is part of :term:`CVE_CHECK_WHITELIST`, it is
> +   considered as patched too.
> +
> + - If the CVE ID is part of the patched CVEs for the recipe, it is
> +   already considered as patched.
> +
> + - Otherwise, the code checks whether the recipe version (:term:`PV`)
> +   is within the range of versions impacted by the CVE. If so, the CVE
> +   is considered as unpatched.


I guess this description would save time to people trying to understand
the cve-check.bbclass code. I bet most code readers won't think about
checking the documentation first.

Would it make sense to add a link to such documentation as a comment in
the code, or directly add the above text as comments?

Thanks in advance,
Michael.

-- 
Michael Opdenacker, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com