| Message ID | 20241014202746.1818435-1-sergio.prado@e-labworks.com |
|---|---|
| State | Rejected |
| Delegated to: | Ryan Eatmon |
| Headers | show |
| Series | [master/scarthgap] conf: machine: k3: use weak assignment for FIT image variables | expand |
On 10/14/2024 3:27 PM, Sergio Prado wrote: > Use weak assignment to make it possible to override the default > value via classes parsed after the machine configuration file. > > This is the case when using the tdxref-signed class from > meta-toradex-security to generated signed images for AM6X. > > Additionally, explicitly set FIT_SIGN_NUMBITS to 4096 to align with the > default rsa4096 algorithm, preventing potential mismatches between the > signing algorithm and key length. > > Signed-off-by: Sergio Prado <sergio.prado@e-labworks.com> > Signed-off-by: Rogerio Borin <rogerio.borin@toradex.com> > --- > meta-ti-bsp/conf/machine/include/k3.inc | 9 +++++---- > 1 file changed, 5 insertions(+), 4 deletions(-) > > diff --git a/meta-ti-bsp/conf/machine/include/k3.inc b/meta-ti-bsp/conf/machine/include/k3.inc > index a296f64fa141..07f0bcb48bfc 100644 > --- a/meta-ti-bsp/conf/machine/include/k3.inc > +++ b/meta-ti-bsp/conf/machine/include/k3.inc > @@ -26,10 +26,11 @@ UBOOT_SUFFIX = "img" > > UBOOT_SIGN_ENABLE = "1" > UBOOT_MKIMAGE_DTCOPTS = "-I dts -O dtb" > -UBOOT_SIGN_KEYNAME ?= "custMpk" > -UBOOT_SIGN_KEYDIR ?= "${TI_SECURE_DEV_PKG}/keys" > -FIT_HASH_ALG ?= "sha512" > -FIT_SIGN_ALG ?= "rsa4096" > +UBOOT_SIGN_KEYNAME ??= "custMpk" > +UBOOT_SIGN_KEYDIR ??= "${TI_SECURE_DEV_PKG}/keys" > +FIT_HASH_ALG ??= "sha512" > +FIT_SIGN_ALG ??= "rsa4096" > +FIT_SIGN_NUMBITS ??= "4096" These changes appear to break the build flow. Since the the only real change is the addition of FIT_SIGN_NUMBITS, I'm going to assume that is the culprit. Signature written to '<snip>/build/arago-tmp-default-glibc/work/am62axx_evm-oe-linux/u-boot-ti-staging/2024.04+git/build/fitImage-linux', node '/configurations/conf-ti_k3-v3link-imx219-0-2.dtbo/signature-1' Public key written to 'u-boot.dtb', node '/signature/key-custMpk' Signature check bad (error 1) Verifying Hash Integrity for node 'conf-ti_k3-am62a7-sk.dtb'... sha256,rsa2048:custMpk- error! Verification failed for '(null)' hash node in 'conf-ti_k3-am62a7-sk.dtb' config node Failed to verify required signature 'key-custMpk' > EXTRA_IMAGEDEPENDS += "virtual/bootloader" > > > > > -=-=-=-=-=-=-=-=-=-=-=- > Links: You receive all messages sent to this group. > View/Reply Online (#18006): https://lists.yoctoproject.org/g/meta-ti/message/18006 > Mute This Topic: https://lists.yoctoproject.org/mt/109009846/6551054 > Group Owner: meta-ti+owner@lists.yoctoproject.org > Unsubscribe: https://lists.yoctoproject.org/g/meta-ti/unsub [reatmon@ti.com] > -=-=-=-=-=-=-=-=-=-=-=- >
On 10/16/2024 2:51 PM, Ryan Eatmon via lists.yoctoproject.org wrote: > > > On 10/14/2024 3:27 PM, Sergio Prado wrote: >> Use weak assignment to make it possible to override the default >> value via classes parsed after the machine configuration file. >> >> This is the case when using the tdxref-signed class from >> meta-toradex-security to generated signed images for AM6X. >> >> Additionally, explicitly set FIT_SIGN_NUMBITS to 4096 to align with the >> default rsa4096 algorithm, preventing potential mismatches between the >> signing algorithm and key length. >> >> Signed-off-by: Sergio Prado <sergio.prado@e-labworks.com> >> Signed-off-by: Rogerio Borin <rogerio.borin@toradex.com> >> --- >> meta-ti-bsp/conf/machine/include/k3.inc | 9 +++++---- >> 1 file changed, 5 insertions(+), 4 deletions(-) >> >> diff --git a/meta-ti-bsp/conf/machine/include/k3.inc >> b/meta-ti-bsp/conf/machine/include/k3.inc >> index a296f64fa141..07f0bcb48bfc 100644 >> --- a/meta-ti-bsp/conf/machine/include/k3.inc >> +++ b/meta-ti-bsp/conf/machine/include/k3.inc >> @@ -26,10 +26,11 @@ UBOOT_SUFFIX = "img" >> UBOOT_SIGN_ENABLE = "1" >> UBOOT_MKIMAGE_DTCOPTS = "-I dts -O dtb" >> -UBOOT_SIGN_KEYNAME ?= "custMpk" >> -UBOOT_SIGN_KEYDIR ?= "${TI_SECURE_DEV_PKG}/keys" >> -FIT_HASH_ALG ?= "sha512" >> -FIT_SIGN_ALG ?= "rsa4096" >> +UBOOT_SIGN_KEYNAME ??= "custMpk" >> +UBOOT_SIGN_KEYDIR ??= "${TI_SECURE_DEV_PKG}/keys" >> +FIT_HASH_ALG ??= "sha512" >> +FIT_SIGN_ALG ??= "rsa4096" >> +FIT_SIGN_NUMBITS ??= "4096" > > These changes appear to break the build flow. Since the the only real > change is the addition of FIT_SIGN_NUMBITS, I'm going to assume that is > the culprit. > > Signature written to > '<snip>/build/arago-tmp-default-glibc/work/am62axx_evm-oe-linux/u-boot-ti-staging/2024.04+git/build/fitImage-linux', node '/configurations/conf-ti_k3-v3link-imx219-0-2.dtbo/signature-1' > Public key written to 'u-boot.dtb', node '/signature/key-custMpk' > Signature check bad (error 1) > Verifying Hash Integrity for node 'conf-ti_k3-am62a7-sk.dtb'... > sha256,rsa2048:custMpk- > error! > Verification failed for '(null)' hash node in 'conf-ti_k3-am62a7-sk.dtb' > config node > Failed to verify required signature 'key-custMpk' I think the issue is that the weak assignment happens at the very end of parsing if there no other value for the variable. But the various classes (kernel-fitimage.bbclass, uboot-sign.bbclass, etc...) set the variables using the ?= so that means the values we want for our stuff never gets set correctly. I can see how this works for you in your case, but doing this in meta-ti-bsp breaks everyone not using meta-tordex-security. I'm not really sure the best solution for this. Should the tordex layer be in final control of these variables and not allow overrides? Should it not be using the ?= assignment and instead use the = assignment to force the issue? Denys, Any ideas? > > >> EXTRA_IMAGEDEPENDS += "virtual/bootloader" >> >> >> >> >> > > > > -=-=-=-=-=-=-=-=-=-=-=- > Links: You receive all messages sent to this group. > View/Reply Online (#18007): https://lists.yoctoproject.org/g/meta-ti/message/18007 > Mute This Topic: https://lists.yoctoproject.org/mt/109009846/6551054 > Group Owner: meta-ti+owner@lists.yoctoproject.org > Unsubscribe: https://lists.yoctoproject.org/g/meta-ti/unsub [reatmon@ti.com] > -=-=-=-=-=-=-=-=-=-=-=- >
Hi Ryan > I think the issue is that the weak assignment happens at the very end of > parsing if there no other value for the variable. But the various > classes (kernel-fitimage.bbclass, uboot-sign.bbclass, etc...) set the > variables using the ?= so that means the values we want for our stuff > never gets set correctly. > > I can see how this works for you in your case, but doing this in > meta-ti-bsp breaks everyone not using meta-tordex-security. Yeah, you're right. I have done more tests here. Indeed it works with meta-toradex-security, but it fails without it. The reason is that, because of the weak assignment, the classes inherited by the kernel will take precedence and will set the variables with a different value from the machine, and later on, when the check is done in the U-Boot recipe, the configuration from the machine is used, which differs from the configuration used to sign the FIT image. Possibly the best solution for our use case is to force a default in our class with an override (e.g. FIT_SIGN_ALG:k3="rsa2048"), and instruct the users to change it with the forcevariable override (e.g. FIT_SIGN_ALG:forcevariable="rsa4096"). @Rogerio, do you see any other solution? Best regards, Sergio Prado
On Wed, Oct 16, 2024 at 04:35:17PM -0500, Ryan Eatmon via lists.yoctoproject.org wrote: > > > On 10/16/2024 2:51 PM, Ryan Eatmon via lists.yoctoproject.org wrote: > > > > > >On 10/14/2024 3:27 PM, Sergio Prado wrote: > >>Use weak assignment to make it possible to override the default > >>value via classes parsed after the machine configuration file. > >> > >>This is the case when using the tdxref-signed class from > >>meta-toradex-security to generated signed images for AM6X. > >> > >>Additionally, explicitly set FIT_SIGN_NUMBITS to 4096 to align with the > >>default rsa4096 algorithm, preventing potential mismatches between the > >>signing algorithm and key length. > >> > >>Signed-off-by: Sergio Prado <sergio.prado@e-labworks.com> > >>Signed-off-by: Rogerio Borin <rogerio.borin@toradex.com> > >>--- > >> meta-ti-bsp/conf/machine/include/k3.inc | 9 +++++---- > >> 1 file changed, 5 insertions(+), 4 deletions(-) > >> > >>diff --git a/meta-ti-bsp/conf/machine/include/k3.inc > >>b/meta-ti-bsp/conf/machine/include/k3.inc > >>index a296f64fa141..07f0bcb48bfc 100644 > >>--- a/meta-ti-bsp/conf/machine/include/k3.inc > >>+++ b/meta-ti-bsp/conf/machine/include/k3.inc > >>@@ -26,10 +26,11 @@ UBOOT_SUFFIX = "img" > >> UBOOT_SIGN_ENABLE = "1" > >> UBOOT_MKIMAGE_DTCOPTS = "-I dts -O dtb" > >>-UBOOT_SIGN_KEYNAME ?= "custMpk" > >>-UBOOT_SIGN_KEYDIR ?= "${TI_SECURE_DEV_PKG}/keys" > >>-FIT_HASH_ALG ?= "sha512" > >>-FIT_SIGN_ALG ?= "rsa4096" > >>+UBOOT_SIGN_KEYNAME ??= "custMpk" > >>+UBOOT_SIGN_KEYDIR ??= "${TI_SECURE_DEV_PKG}/keys" > >>+FIT_HASH_ALG ??= "sha512" > >>+FIT_SIGN_ALG ??= "rsa4096" > >>+FIT_SIGN_NUMBITS ??= "4096" > > > >These changes appear to break the build flow. Since the the only > >real change is the addition of FIT_SIGN_NUMBITS, I'm going to > >assume that is the culprit. > > > >Signature written to '<snip>/build/arago-tmp-default-glibc/work/am62axx_evm-oe-linux/u-boot-ti-staging/2024.04+git/build/fitImage-linux', > >node > >'/configurations/conf-ti_k3-v3link-imx219-0-2.dtbo/signature-1' > >Public key written to 'u-boot.dtb', node '/signature/key-custMpk' > >Signature check bad (error 1) > >Verifying Hash Integrity for node 'conf-ti_k3-am62a7-sk.dtb'... > >sha256,rsa2048:custMpk- > > error! > >Verification failed for '(null)' hash node in > >'conf-ti_k3-am62a7-sk.dtb' config node > >Failed to verify required signature 'key-custMpk' > > I think the issue is that the weak assignment happens at the very > end of parsing if there no other value for the variable. But the > various classes (kernel-fitimage.bbclass, uboot-sign.bbclass, > etc...) set the variables using the ?= so that means the values we > want for our stuff never gets set correctly. > > I can see how this works for you in your case, but doing this in > meta-ti-bsp breaks everyone not using meta-tordex-security. > > I'm not really sure the best solution for this. Should the tordex > layer be in final control of these variables and not allow > overrides? Should it not be using the ?= assignment and instead use > the = assignment to force the issue? > > Denys, > Any ideas? Yeah, I had my suspicions about the patch, so not surprised by your confirmation. I'd suggest using machine or SoC overrides in toradex layer to change the defaults set in meta-ti k3.inc
diff --git a/meta-ti-bsp/conf/machine/include/k3.inc b/meta-ti-bsp/conf/machine/include/k3.inc index a296f64fa141..07f0bcb48bfc 100644 --- a/meta-ti-bsp/conf/machine/include/k3.inc +++ b/meta-ti-bsp/conf/machine/include/k3.inc @@ -26,10 +26,11 @@ UBOOT_SUFFIX = "img" UBOOT_SIGN_ENABLE = "1" UBOOT_MKIMAGE_DTCOPTS = "-I dts -O dtb" -UBOOT_SIGN_KEYNAME ?= "custMpk" -UBOOT_SIGN_KEYDIR ?= "${TI_SECURE_DEV_PKG}/keys" -FIT_HASH_ALG ?= "sha512" -FIT_SIGN_ALG ?= "rsa4096" +UBOOT_SIGN_KEYNAME ??= "custMpk" +UBOOT_SIGN_KEYDIR ??= "${TI_SECURE_DEV_PKG}/keys" +FIT_HASH_ALG ??= "sha512" +FIT_SIGN_ALG ??= "rsa4096" +FIT_SIGN_NUMBITS ??= "4096" EXTRA_IMAGEDEPENDS += "virtual/bootloader"