Re: [PATCH] u-boot: ensure keys are generated before assembling U-Boot FIT image

[Rogerio Guerra Borin <rogerio.borin@toradex.com>]

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