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 |
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() {
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 ?
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 --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() {