diff mbox series

[v2] ref-manual: variable FIT_SIGN_INDIVIDUAL mix-and-match attacks

Message ID 20250225213737.3343894-1-adrian.freihofer@siemens.com
State Under Review
Headers show
Series [v2] ref-manual: variable FIT_SIGN_INDIVIDUAL mix-and-match attacks | expand

Commit Message

Adrian Freihofer Feb. 25, 2025, 9:37 p.m. UTC
Incorporate the lessons learned from a regression introduced with commit
29d32063ac0abb1017756f62f94aec22ce305b60 and fixed with commit
d63dba2f98edf89558647e336b19d805b00f4d98 into the documentation.

The use of the variable FIT_SIGN_INDIVIDUAL is explicitly discouraged.
It is also noted that this variable may be removed. It is important that
we try to simplify the implementation of the FIT screen as much as
possible. Adding appropriate notes to the documentation is a first step
towards this direction.

Signed-off-by: Adrian Freihofer <adrian.freihofer@siemens.com>
---
 documentation/ref-manual/variables.rst | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

Comments

Antonin Godard March 3, 2025, 11:16 a.m. UTC | #1
Hi Adrian,

On Tue Feb 25, 2025 at 10:37 PM CET, Adrian Freihofer via lists.yoctoproject.org wrote:
> Incorporate the lessons learned from a regression introduced with commit
> 29d32063ac0abb1017756f62f94aec22ce305b60 and fixed with commit
> d63dba2f98edf89558647e336b19d805b00f4d98 into the documentation.
>
> The use of the variable FIT_SIGN_INDIVIDUAL is explicitly discouraged.
> It is also noted that this variable may be removed. It is important that
> we try to simplify the implementation of the FIT screen as much as
> possible. Adding appropriate notes to the documentation is a first step
> towards this direction.
>
> Signed-off-by: Adrian Freihofer <adrian.freihofer@siemens.com>
> ---
>  documentation/ref-manual/variables.rst | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/documentation/ref-manual/variables.rst b/documentation/ref-manual/variables.rst
> index b432488a012..645bb1453d1 100644
> --- a/documentation/ref-manual/variables.rst
> +++ b/documentation/ref-manual/variables.rst
> @@ -3173,7 +3173,18 @@ system and gives an overview of their function and contents.
>        intending to verify signatures in another context than booting via
>        U-Boot.
>  
> -      This variable is set to "0" by default.
> +      If :term:`UBOOT_SIGN_ENABLE` is set to “1” and :term:`FIT_SIGN_INDIVIDUAL`

s/“”/""/, Sphinx will handle the quotes

> +      is left at its default value of “0”, only the configurations are signed.
> +      However, the configuration signatures include the hashes of the referenced
> +      image nodes. This means that the entire FIT image is appropriately signed.

s/image nodes/nodes/? a node can be anything (kernel, device tree...)

I feel that "the entire FIT image is appropriately signed" is a bit of a
shortcut. Instead I would suggest something like:

"""
This means that the integrity of the entire FIT image is ensured because each
hash is compared against a runtime-computed hash for each node.
"""

> +
> +      If :term:`UBOOT_SIGN_ENABLE` is set to “1” and :term:`FIT_SIGN_INDIVIDUAL`
> +      is set to “1”, then the FIT image is signed twice, which is redundant.
> +      As this leads to additional complexity without providing any obvious
> +      advantage, this feature will likely be removed in a future version.
> +
> +      Signing only the image nodes is intentionally not implemented by OE-core,

s/OE-core/:term:`OpenEmbedded-Core (OE-Core)`/

> +      as it is vulnerable to mix-and-match attacks.
>  
>     :term:`FIT_SIGN_NUMBITS`
>        Size of the private key used in the FIT image, in number of bits.

Thanks this is a lot clearer to me than the previous version.

Antonin
diff mbox series

Patch

diff --git a/documentation/ref-manual/variables.rst b/documentation/ref-manual/variables.rst
index b432488a012..645bb1453d1 100644
--- a/documentation/ref-manual/variables.rst
+++ b/documentation/ref-manual/variables.rst
@@ -3173,7 +3173,18 @@  system and gives an overview of their function and contents.
       intending to verify signatures in another context than booting via
       U-Boot.
 
-      This variable is set to "0" by default.
+      If :term:`UBOOT_SIGN_ENABLE` is set to “1” and :term:`FIT_SIGN_INDIVIDUAL`
+      is left at its default value of “0”, only the configurations are signed.
+      However, the configuration signatures include the hashes of the referenced
+      image nodes. This means that the entire FIT image is appropriately signed.
+
+      If :term:`UBOOT_SIGN_ENABLE` is set to “1” and :term:`FIT_SIGN_INDIVIDUAL`
+      is set to “1”, then the FIT image is signed twice, which is redundant.
+      As this leads to additional complexity without providing any obvious
+      advantage, this feature will likely be removed in a future version.
+
+      Signing only the image nodes is intentionally not implemented by OE-core,
+      as it is vulnerable to mix-and-match attacks.
 
    :term:`FIT_SIGN_NUMBITS`
       Size of the private key used in the FIT image, in number of bits.