All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] ref-manual: variable FIT_SIGN_INDIVIDUAL mix-and-match attacks
@ 2025-02-24 23:11 Adrian Freihofer
  2025-03-03  9:53 ` [docs] " Antonin Godard
  0 siblings, 1 reply; 2+ messages in thread
From: Adrian Freihofer @ 2025-02-24 23:11 UTC (permalink / raw)
  To: docs
  Cc: marex, rogerio.borin, L.Anderweit, quaresma.jose, quentin.schulz,
	richard.purdie, seanga2, Adrian Freihofer

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
+         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"
-- 
2.47.1



^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [docs] [PATCH] ref-manual: variable FIT_SIGN_INDIVIDUAL mix-and-match attacks
  2025-02-24 23:11 [PATCH] ref-manual: variable FIT_SIGN_INDIVIDUAL mix-and-match attacks Adrian Freihofer
@ 2025-03-03  9:53 ` Antonin Godard
  0 siblings, 0 replies; 2+ messages in thread
From: Antonin Godard @ 2025-03-03  9:53 UTC (permalink / raw)
  To: adrian.freihofer, docs
  Cc: marex, rogerio.borin, L.Anderweit, quaresma.jose, quentin.schulz,
	richard.purdie, seanga2, Adrian Freihofer

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

-- 
Antonin Godard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com


^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2025-03-03  9:53 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-02-24 23:11 [PATCH] ref-manual: variable FIT_SIGN_INDIVIDUAL mix-and-match attacks Adrian Freihofer
2025-03-03  9:53 ` [docs] " Antonin Godard

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.