diff mbox series

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

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

Commit Message

Adrian Freihofer March 10, 2025, 9:05 a.m. UTC
Incorporate the lessons learned from a regression introduced with commit
  OE-Core rev: 259bfa86f384206f0d0a96a5b84887186c5f689e
               u-boot: kernel-fitimage: Fix dependency loop if
               UBOOT_SIGN_ENABLE and UBOOT_ENV enabled
and fixed with commit
  OE-Core rev: 0106e5efab99c8016836a2ab71e2327ce58a9a9d
               u-boot: kernel-fitimage: Restore FIT_SIGN_INDIVIDUAL="1"
               behavior
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 image 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 | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

Comments

Quentin Schulz March 13, 2025, 8:54 a.m. UTC | #1
Hi Adrian,

On 3/10/25 10:05 AM, Adrian Freihofer via lists.yoctoproject.org wrote:
> Incorporate the lessons learned from a regression introduced with commit
>    OE-Core rev: 259bfa86f384206f0d0a96a5b84887186c5f689e
>                 u-boot: kernel-fitimage: Fix dependency loop if
>                 UBOOT_SIGN_ENABLE and UBOOT_ENV enabled
> and fixed with commit
>    OE-Core rev: 0106e5efab99c8016836a2ab71e2327ce58a9a9d
>                 u-boot: kernel-fitimage: Restore FIT_SIGN_INDIVIDUAL="1"
>                 behavior
> 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 image 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 | 19 +++++++++++++++++--
>   1 file changed, 17 insertions(+), 2 deletions(-)
> 
> diff --git a/documentation/ref-manual/variables.rst b/documentation/ref-manual/variables.rst
> index 861b04eaab1..08cf15664b3 100644
> --- a/documentation/ref-manual/variables.rst
> +++ b/documentation/ref-manual/variables.rst
> @@ -3174,9 +3174,24 @@ system and gives an overview of their function and contents.
>         class will sign the kernel, dtb and ramdisk images individually in addition
>         to signing the FIT image itself. This could be useful if you are
>         intending to verify signatures in another context than booting via
> -      U-Boot.
> +      U-Boot. This variable is set to "0" by default.
>   
> -      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.

s/configurations/configuration nodes/

> +      However, the configuration signatures include the hashes of the referenced

s/configuration/configuration nodes/

> +      image nodes. This means that the entire FIT image is appropriately signed.

Not if you have image nodes not listed in any configuration node.

> +      Further information can be found in the U-Boot documentation:
> +      `U-Boot fit signature <https://docs.u-boot.org/en/latest/usage/fit/signature.html>`__
> +      and more specifically at:
> +      `U-Boot signed configurations <https://docs.u-boot.org/en/latest/usage/fit/signature.html#signed-configurations>`__.
> +
> +      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.

By "signed twice" I assume you mean "image nodes are signed, and 
configuration nodes are signed" not necessarily that this is done in two 
steps?

> +      As this leads to additional complexity without providing any obvious
> +      advantage, this feature will likely be removed in a future version.
> +

I'm not sure this is true? You could very well have a U-Boot script that 
loads a specific image outside of any configuration node?

> +      Signing only the image nodes is intentionally not implemented by OE-core,
> +      as it is vulnerable to mix-and-match attacks.
>   

Can you use " instead of “ everywhere in the patch?

Cheers,
Quentin
diff mbox series

Patch

diff --git a/documentation/ref-manual/variables.rst b/documentation/ref-manual/variables.rst
index 861b04eaab1..08cf15664b3 100644
--- a/documentation/ref-manual/variables.rst
+++ b/documentation/ref-manual/variables.rst
@@ -3174,9 +3174,24 @@  system and gives an overview of their function and contents.
       class will sign the kernel, dtb and ramdisk images individually in addition
       to signing the FIT image itself. This could be useful if you are
       intending to verify signatures in another context than booting via
-      U-Boot.
+      U-Boot. This variable is set to "0" by default.
 
-      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.
+      Further information can be found in the U-Boot documentation:
+      `U-Boot fit signature <https://docs.u-boot.org/en/latest/usage/fit/signature.html>`__
+      and more specifically at:
+      `U-Boot signed configurations <https://docs.u-boot.org/en/latest/usage/fit/signature.html#signed-configurations>`__.
+
+      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.