diff mbox series

u-boot: ensure keys are generated before assembling U-Boot FIT image

Message ID 20250509213736.3950997-1-rogerio.borin@gmail.com
State Accepted, archived
Commit 036f20156b3c7d0a8b912e90aa29a9b986106d5a
Headers show
Series u-boot: ensure keys are generated before assembling U-Boot FIT image | expand

Commit Message

Rogerio Guerra Borin May 9, 2025, 9:37 p.m. UTC
From: Rogerio Guerra Borin <rogerio.borin@toradex.com>

Add the task dependency:

do_uboot_assemble_fitimage -> virtual/kernel:do_kernel_generate_rsa_keys

to ensure the kernel FIT image signing keys are available when creating
the U-Boot DTB. This is done only if the signing of the kernel FIT image
is enabled (UBOOT_SIGN_ENABLE="1").

The lack of the dependency causes build errors when executing a build
with no kernel FIT keys initially present in the keys directory. In such
cases one would see an output like this in the Bitbake logs:

Log data follows:
| DEBUG: Executing shell function do_uboot_assemble_fitimage
| Couldn't open RSA private key: '/workdir/build/keys/fit/dev.key': No such file or directory
| Failed to sign 'signature' signature node in 'conf-1' conf node
| FIT description: Kernel Image image with one or more FDT blobs
| ...

This issue was introduced by commit 259bfa86f384 where the dependency
between U-Boot and the kernel was removed (for good reasons). Before
that commit the dependency was set via DEPENDS so that, in terms of
tasks, one had:

u-boot:do_configure -> virtual/kernel:do_populate_sysroot

and the chain leading to the key generation was:

virtual/kernel:do_populate_sysroot -> virtual/kernel:do_install
virtual/kernel:do_install -> virtual/kernel:do_assemble_fitimage
virtual/kernel:do_assemble_fitimage -> virtual/kernel:do_kernel_generate_rsa_keys

With the removal of the first dependency, no more guarantees exist that
the keys would be present when assembling the U-Boot FIT image. That's
the situation we are solving with the present commit.

Fixes: 259bfa86f384 ("u-boot: kernel-fitimage: Fix dependency loop if UBOOT_SIGN_ENABLE and UBOOT_ENV enabled")
Signed-off-by: Rogerio Guerra Borin <rogerio.borin@toradex.com>
Cc: Marek Vasut <marex@denx.de>
Cc: Sean Anderson <sean.anderson@seco.com>
Cc: Adrian Freihofer <adrian.freihofer@siemens.com>
---
 meta/classes-recipe/uboot-sign.bbclass | 2 ++
 1 file changed, 2 insertions(+)

Comments

Freihofer, Adrian May 10, 2025, 4:59 p.m. UTC | #1
On Fri, 2025-05-09 at 18:37 -0300, Rogerio Guerra Borin wrote:
> From: Rogerio Guerra Borin <rogerio.borin@toradex.com>
> 
> Add the task dependency:
> 
> do_uboot_assemble_fitimage ->
> virtual/kernel:do_kernel_generate_rsa_keys
> 
> to ensure the kernel FIT image signing keys are available when
> creating
> the U-Boot DTB. This is done only if the signing of the kernel FIT
> image
> is enabled (UBOOT_SIGN_ENABLE="1").
> 
> The lack of the dependency causes build errors when executing a build
> with no kernel FIT keys initially present in the keys directory. In
> such
> cases one would see an output like this in the Bitbake logs:
> 
> Log data follows:
> > DEBUG: Executing shell function do_uboot_assemble_fitimage
> > Couldn't open RSA private key: '/workdir/build/keys/fit/dev.key':
> > No such file or directory
> > Failed to sign 'signature' signature node in 'conf-1' conf node
> > FIT description: Kernel Image image with one or more FDT blobs
> > ...
> 
> This issue was introduced by commit 259bfa86f384 where the dependency
> between U-Boot and the kernel was removed (for good reasons). Before
> that commit the dependency was set via DEPENDS so that, in terms of
> tasks, one had:
> 
> u-boot:do_configure -> virtual/kernel:do_populate_sysroot
> 
> and the chain leading to the key generation was:
> 
> virtual/kernel:do_populate_sysroot -> virtual/kernel:do_install
> virtual/kernel:do_install -> virtual/kernel:do_assemble_fitimage
> virtual/kernel:do_assemble_fitimage ->
> virtual/kernel:do_kernel_generate_rsa_keys
> 
> With the removal of the first dependency, no more guarantees exist
> that
> the keys would be present when assembling the U-Boot FIT image.
> That's
> the situation we are solving with the present commit.
> 
> Fixes: 259bfa86f384 ("u-boot: kernel-fitimage: Fix dependency loop if
> UBOOT_SIGN_ENABLE and UBOOT_ENV enabled")
> Signed-off-by: Rogerio Guerra Borin <rogerio.borin@toradex.com>
> Cc: Marek Vasut <marex@denx.de>
> Cc: Sean Anderson <sean.anderson@seco.com>
> Cc: Adrian Freihofer <adrian.freihofer@siemens.com>
> ---
>  meta/classes-recipe/uboot-sign.bbclass | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/meta/classes-recipe/uboot-sign.bbclass b/meta/classes-
> recipe/uboot-sign.bbclass
> index 76a81546e34..7744e0c5ab5 100644
> --- a/meta/classes-recipe/uboot-sign.bbclass
> +++ b/meta/classes-recipe/uboot-sign.bbclass
> @@ -113,6 +113,8 @@ python() {
>      sign = d.getVar('UBOOT_SIGN_ENABLE') == '1'
>      if d.getVar('UBOOT_FITIMAGE_ENABLE') == '1' or sign:
>          d.appendVar('DEPENDS', " u-boot-tools-native dtc-native")
> +    if sign:
> +        d.appendVarFlag('do_uboot_assemble_fitimage', 'depends', '
> virtual/kernel:do_kernel_generate_rsa_keys')

Short answer: Thank you for fixing this regression. I basically agree
with the fix.
However, could you please also check if FIT_GENERATE_KEYS is enabled?
Would a v2 like this be fine?

+ if sign and d.getVar('FIT_GENERATE_KEYS') == '1':
+ d.appendVarFlag('do_uboot_assemble_fitimage', 'depends', '



Longer answer: The way the keys are (and always have been) generated
presents two major issues from my point of view:
   1. It creates a dependency from U-Boot on the kernel, which should
      generally be avoided.
   2. More problematically, the current implementation leads to random
      key generation in many use cases:
       - When building from an existing TMPDIR, the existing key is reused
       - When building from an empty TMPDIR, a new key is generated, and
         signing works automatically with this fresh key
       - This also occurs when building with an SDK that uses the sstate-
         cache to create the TMPDIR. Since the key is not sstate-cached, each
         SDK will likely create its own key

While FIT_GENERATE_KEYS might be a convenient feature for quick
experiments, it becomes problematic when maintaining real products
where using random keys is typically not acceptable. Until now, my
assumption has been that FIT_GENERATE_KEYS is primarily used for
testing purposes rather than in actual product deployments. So offering
the possibility to switch it properly off without pulling in some
potentially problematic task dependencies is important.

We likely need a more sophisticated implementation for the future - one
that is reproducible, secure, and doesn't create a task dependency from
U-Boot to the kernel.
The key management should probably be moved to a separate (native?)
recipe that provides the key via (native-)sysroots to other recipes.
Yocto could provide a reasonably secure default implementation for such
a key provider recipe, which could be easily overridden in a downstream
layer.

I will consider this after the major refactoring of the FIT image is
merged. Basically, I'm thinking about a recipe that:
 * Takes an optionally encrypted key via SRC_URI (note that using the
   fetcher is important for handling key changes reliably)
 * Provides the decrypted key to other recipes via sysroots
 * Stores the encrypted key in sstate-cache
 * Uses a password provided by a non sstate-cached file for decrypting
   the symmetrically encrypted key
This would provide a semi-secure, reproducible implementation.

For enhanced security, I see two possible implementations:
 * Replacing the semi-secure signature created by the default
   implementation later on, independently from the build process (e.g.
   with a hardware token offering a PKCS#11 interface.
 * Passing a PKCS#11 interface to Bitbake. However, this appears
   extremely complicated and raises the question of how to pass the
   PKCS#11 secret to BitBake. It also raises the question if a PKCS#11
   interface is available on a build machine.

Regards,
Adrian


>  }
>  
>  concat_dtb() {
Marek Vasut May 11, 2025, 9 p.m. UTC | #2
On 5/9/25 11:37 PM, Rogerio Guerra Borin wrote:
> From: Rogerio Guerra Borin <rogerio.borin@toradex.com>
> 
> Add the task dependency:
> 
> do_uboot_assemble_fitimage -> virtual/kernel:do_kernel_generate_rsa_keys
> 
> to ensure the kernel FIT image signing keys are available when creating
> the U-Boot DTB. This is done only if the signing of the kernel FIT image
> is enabled (UBOOT_SIGN_ENABLE="1").
> 
> The lack of the dependency causes build errors when executing a build
> with no kernel FIT keys initially present in the keys directory. In such
> cases one would see an output like this in the Bitbake logs:
> 
> Log data follows:
> | DEBUG: Executing shell function do_uboot_assemble_fitimage
> | Couldn't open RSA private key: '/workdir/build/keys/fit/dev.key': No such file or directory
> | Failed to sign 'signature' signature node in 'conf-1' conf node
> | FIT description: Kernel Image image with one or more FDT blobs
> | ...
> 
> This issue was introduced by commit 259bfa86f384 where the dependency
> between U-Boot and the kernel was removed (for good reasons). Before
> that commit the dependency was set via DEPENDS so that, in terms of
> tasks, one had:
> 
> u-boot:do_configure -> virtual/kernel:do_populate_sysroot
> 
> and the chain leading to the key generation was:
> 
> virtual/kernel:do_populate_sysroot -> virtual/kernel:do_install
> virtual/kernel:do_install -> virtual/kernel:do_assemble_fitimage
> virtual/kernel:do_assemble_fitimage -> virtual/kernel:do_kernel_generate_rsa_keys
> 
> With the removal of the first dependency, no more guarantees exist that
> the keys would be present when assembling the U-Boot FIT image. That's
> the situation we are solving with the present commit.
> 
> Fixes: 259bfa86f384 ("u-boot: kernel-fitimage: Fix dependency loop if UBOOT_SIGN_ENABLE and UBOOT_ENV enabled")
> Signed-off-by: Rogerio Guerra Borin <rogerio.borin@toradex.com>
> Cc: Marek Vasut <marex@denx.de>
> Cc: Sean Anderson <sean.anderson@seco.com>
> Cc: Adrian Freihofer <adrian.freihofer@siemens.com>
> ---
>   meta/classes-recipe/uboot-sign.bbclass | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/meta/classes-recipe/uboot-sign.bbclass b/meta/classes-recipe/uboot-sign.bbclass
> index 76a81546e34..7744e0c5ab5 100644
> --- a/meta/classes-recipe/uboot-sign.bbclass
> +++ b/meta/classes-recipe/uboot-sign.bbclass
> @@ -113,6 +113,8 @@ python() {
>       sign = d.getVar('UBOOT_SIGN_ENABLE') == '1'
>       if d.getVar('UBOOT_FITIMAGE_ENABLE') == '1' or sign:
>           d.appendVar('DEPENDS', " u-boot-tools-native dtc-native")
> +    if sign:
> +        d.appendVarFlag('do_uboot_assemble_fitimage', 'depends', ' virtual/kernel:do_kernel_generate_rsa_keys')
This should also check for FIT_GENERATE_KEYS=1 before adding the 
dependency, right ?
Rogerio Guerra Borin May 12, 2025, 3:34 p.m. UTC | #3
On 5/10/25 13:59, Freihofer, Adrian wrote:
> This message originated from outside your organization
> 
> On Fri, 2025-05-09 at 18:37 -0300, Rogerio Guerra Borin wrote:
>> From: Rogerio Guerra Borin <rogerio.borin@toradex.com>
>>
>> Add the task dependency:
>>
>> do_uboot_assemble_fitimage ->
>> virtual/kernel:do_kernel_generate_rsa_keys
>>
>> to ensure the kernel FIT image signing keys are available when
>> creating
>> the U-Boot DTB. This is done only if the signing of the kernel FIT
>> image
>> is enabled (UBOOT_SIGN_ENABLE="1").
>>
>> The lack of the dependency causes build errors when executing a build
>> with no kernel FIT keys initially present in the keys directory. In
>> such
>> cases one would see an output like this in the Bitbake logs:
>>
>> Log data follows:
>>> DEBUG: Executing shell function do_uboot_assemble_fitimage
>>> Couldn't open RSA private key: '/workdir/build/keys/fit/dev.key':
>>> No such file or directory
>>> Failed to sign 'signature' signature node in 'conf-1' conf node
>>> FIT description: Kernel Image image with one or more FDT blobs
>>> ...
>>
>> This issue was introduced by commit 259bfa86f384 where the dependency
>> between U-Boot and the kernel was removed (for good reasons). Before
>> that commit the dependency was set via DEPENDS so that, in terms of
>> tasks, one had:
>>
>> u-boot:do_configure -> virtual/kernel:do_populate_sysroot
>>
>> and the chain leading to the key generation was:
>>
>> virtual/kernel:do_populate_sysroot -> virtual/kernel:do_install
>> virtual/kernel:do_install -> virtual/kernel:do_assemble_fitimage
>> virtual/kernel:do_assemble_fitimage ->
>> virtual/kernel:do_kernel_generate_rsa_keys
>>
>> With the removal of the first dependency, no more guarantees exist
>> that
>> the keys would be present when assembling the U-Boot FIT image.
>> That's
>> the situation we are solving with the present commit.
>>
>> Fixes: 259bfa86f384 ("u-boot: kernel-fitimage: Fix dependency loop if
>> UBOOT_SIGN_ENABLE and UBOOT_ENV enabled")
>> Signed-off-by: Rogerio Guerra Borin <rogerio.borin@toradex.com>
>> Cc: Marek Vasut <marex@denx.de>
>> Cc: Sean Anderson <sean.anderson@seco.com>
>> Cc: Adrian Freihofer <adrian.freihofer@siemens.com>
>> ---
>>   meta/classes-recipe/uboot-sign.bbclass | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/meta/classes-recipe/uboot-sign.bbclass b/meta/classes-
>> recipe/uboot-sign.bbclass
>> index 76a81546e34..7744e0c5ab5 100644
>> --- a/meta/classes-recipe/uboot-sign.bbclass
>> +++ b/meta/classes-recipe/uboot-sign.bbclass
>> @@ -113,6 +113,8 @@ python() {
>>       sign = d.getVar('UBOOT_SIGN_ENABLE') == '1'
>>       if d.getVar('UBOOT_FITIMAGE_ENABLE') == '1' or sign:
>>           d.appendVar('DEPENDS', " u-boot-tools-native dtc-native")
>> +    if sign:
>> +        d.appendVarFlag('do_uboot_assemble_fitimage', 'depends', '
>> virtual/kernel:do_kernel_generate_rsa_keys')
> 
> Short answer: Thank you for fixing this regression. I basically agree
> with the fix.
> However, could you please also check if FIT_GENERATE_KEYS is enabled?
> Would a v2 like this be fine?
> 
> + if sign and d.getVar('FIT_GENERATE_KEYS') == '1':
> + d.appendVarFlag('do_uboot_assemble_fitimage', 'depends', '
> 

Absolutely. I'll do it on v2.

> 
> Longer answer: The way the keys are (and always have been) generated
> presents two major issues from my point of view:
>     1. It creates a dependency from U-Boot on the kernel, which should
>        generally be avoided.
>     2. More problematically, the current implementation leads to random
>        key generation in many use cases:
>         - When building from an existing TMPDIR, the existing key is reused
>         - When building from an empty TMPDIR, a new key is generated, and
>           signing works automatically with this fresh key
>         - This also occurs when building with an SDK that uses the sstate-
>           cache to create the TMPDIR. Since the key is not sstate-cached, each
>           SDK will likely create its own key
> 
> While FIT_GENERATE_KEYS might be a convenient feature for quick
> experiments, it becomes problematic when maintaining real products
> where using random keys is typically not acceptable. Until now, my
> assumption has been that FIT_GENERATE_KEYS is primarily used for
> testing purposes rather than in actual product deployments. So offering
> the possibility to switch it properly off without pulling in some
> potentially problematic task dependencies is important.
> 
> We likely need a more sophisticated implementation for the future - one
> that is reproducible, secure, and doesn't create a task dependency from
> U-Boot to the kernel.
> The key management should probably be moved to a separate (native?)
> recipe that provides the key via (native-)sysroots to other recipes.
> Yocto could provide a reasonably secure default implementation for such
> a key provider recipe, which could be easily overridden in a downstream
> layer.
> 
> I will consider this after the major refactoring of the FIT image is
> merged. Basically, I'm thinking about a recipe that:
>   * Takes an optionally encrypted key via SRC_URI (note that using the
>     fetcher is important for handling key changes reliably)
>   * Provides the decrypted key to other recipes via sysroots
>   * Stores the encrypted key in sstate-cache
>   * Uses a password provided by a non sstate-cached file for decrypting
>     the symmetrically encrypted key
> This would provide a semi-secure, reproducible implementation.
> 
> For enhanced security, I see two possible implementations:
>   * Replacing the semi-secure signature created by the default
>     implementation later on, independently from the build process (e.g.
>     with a hardware token offering a PKCS#11 interface.
>   * Passing a PKCS#11 interface to Bitbake. However, this appears
>     extremely complicated and raises the question of how to pass the
>     PKCS#11 secret to BitBake. It also raises the question if a PKCS#11
>     interface is available on a build machine.
> 
> Regards,
> Adrian
> 
> 
>>   }
>>   
>>   concat_dtb() {
> 

Thanks for the detailed answer. Yeah, that looks like a promising path 
to follow for improving the signing infrastructure.

Cheers,
Rogerio
diff mbox series

Patch

diff --git a/meta/classes-recipe/uboot-sign.bbclass b/meta/classes-recipe/uboot-sign.bbclass
index 76a81546e34..7744e0c5ab5 100644
--- a/meta/classes-recipe/uboot-sign.bbclass
+++ b/meta/classes-recipe/uboot-sign.bbclass
@@ -113,6 +113,8 @@  python() {
     sign = d.getVar('UBOOT_SIGN_ENABLE') == '1'
     if d.getVar('UBOOT_FITIMAGE_ENABLE') == '1' or sign:
         d.appendVar('DEPENDS', " u-boot-tools-native dtc-native")
+    if sign:
+        d.appendVarFlag('do_uboot_assemble_fitimage', 'depends', ' virtual/kernel:do_kernel_generate_rsa_keys')
 }
 
 concat_dtb() {