diff mbox series

ref-manual: variable FIT_SIGN_INDIVIDUAL mix-and-match attacks

Message ID 20250224231156.3335643-1-adrian.freihofer@siemens.com
State Superseded
Headers show
Series ref-manual: variable FIT_SIGN_INDIVIDUAL mix-and-match attacks | expand

Commit Message

Adrian Freihofer Feb. 24, 2025, 11:11 p.m. UTC
Add a warning to the documentation of the FIT_SIGN_INDIVIDUAL variable.

This is a conclusion of this discussion:
https://lists.openembedded.org/g/openembedded-core/topic/111218371

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

Comments

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

On Tue Feb 25, 2025 at 12:11 AM CET, Adrian Freihofer via lists.yoctoproject.org wrote:
> Add a warning to the documentation of the FIT_SIGN_INDIVIDUAL variable.
>
> This is a conclusion of this discussion:
> https://lists.openembedded.org/g/openembedded-core/topic/111218371
>
> Signed-off-by: Adrian Freihofer <adrian.freihofer@siemens.com>
> ---
>  documentation/ref-manual/variables.rst | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
>
> diff --git a/documentation/ref-manual/variables.rst b/documentation/ref-manual/variables.rst
> index b432488a012..de7f0a3b292 100644
> --- a/documentation/ref-manual/variables.rst
> +++ b/documentation/ref-manual/variables.rst
> @@ -3175,6 +3175,27 @@ system and gives an overview of their function and contents.
>  
>        This variable is set to "0" by default.
>  
> +      .. note::
> +
> +         Setting this variable to “0” is recommended for several reasons in

We usually use regular quote here ("") not “” and Sphinx does the magic.

I would also remove "in particular" but that's my taste.

> +         particular:
> +
> +         -  If :term:`UBOOT_SIGN_ENABLE` is set to “1”, all image artifacts
> +            contained in the FIT image are signed correctly. This is because
> +            the hashes of the image nodes are signed via the corresponding

"of the image nodes" I'd say even all kind of nodes right?

Proposal for rephrasing:

"""
This is because the kernel, device tree and ramdisk image nodes hashes are
contained in the configuration nodes which is also signed.
"""

> +            configuration nodes. Signing the individual image nodes is
> +            therefore redundant as long as the configuration nodes are properly

You imply that there are multiple configuration nodes then? Or only one that
contains the hashes? Unclear to me

> +            signed.
> +
> +         -  Allowing to removing the image nodes from the context of the FIT
> +            image comes with a risk of mix-and-match attacks. This means that
> +            an attacker could combine different signed images which together
> +            have a vulnerability and allow an attack on the device.

Not sure I see how this is related to FIT_SIGN_INDIVIDUAL... I don't see how
this is unsafe since the configuration node is signed and contains the hashes
of the different nodes.

> +
> +         -  Not sure if this feature will be maintained for the long term.
> +            It adds complexity for a not obvious benefit. This can be seen as a
> +            problem, especially in a security context.
> +
>     :term:`FIT_SIGN_NUMBITS`
>        Size of the private key used in the FIT image, in number of bits.
>        The default value for this variable is set to "2048"

Antonin
diff mbox series

Patch

diff --git a/documentation/ref-manual/variables.rst b/documentation/ref-manual/variables.rst
index b432488a012..de7f0a3b292 100644
--- a/documentation/ref-manual/variables.rst
+++ b/documentation/ref-manual/variables.rst
@@ -3175,6 +3175,27 @@  system and gives an overview of their function and contents.
 
       This variable is set to "0" by default.
 
+      .. note::
+
+         Setting this variable to “0” is recommended for several reasons in
+         particular:
+
+         -  If :term:`UBOOT_SIGN_ENABLE` is set to “1”, all image artifacts
+            contained in the FIT image are signed correctly. This is because
+            the hashes of the image nodes are signed via the corresponding
+            configuration nodes. Signing the individual image nodes is
+            therefore redundant as long as the configuration nodes are properly
+            signed.
+
+         -  Allowing to removing the image nodes from the context of the FIT
+            image comes with a risk of mix-and-match attacks. This means that
+            an attacker could combine different signed images which together
+            have a vulnerability and allow an attack on the device.
+
+         -  Not sure if this feature will be maintained for the long term.
+            It adds complexity for a not obvious benefit. This can be seen as a
+            problem, especially in a security context.
+
    :term:`FIT_SIGN_NUMBITS`
       Size of the private key used in the FIT image, in number of bits.
       The default value for this variable is set to "2048"