subject:"\[OE\-core\]\[PATCH\] kernel\-fitimage\: images should not be signed with the same keys as the configurations"

Re: [OE-core][PATCH] kernel-fitimage: images should not be signed with the same keys as the configurations

2021-08-09 Thread Thomas Perrot

Hello Alexandre,

On Sun, 2021-08-08 at 21:19 +0200, Alexandre Belloni wrote:
> Hello,
> 
> On 06/08/2021 18:10:38+0200, Thomas Perrot wrote:
> > Otherwise the "required" property, from UBOOT_DTB_BINARY, will be set
> > to "conf"
> > and no error will be raised in case of error.
> > 
> > Signed-off-by: Thomas Perrot 
> > ---
> >  meta/classes/kernel-fitimage.bbclass | 40 --
> > --
> >  1 file changed, 35 insertions(+), 5 deletions(-)
> > 
> > diff --git a/meta/classes/kernel-fitimage.bbclass
> > b/meta/classes/kernel-fitimage.bbclass
> > index a9d1002200c9..72f692e40e63 100644
> > --- a/meta/classes/kernel-fitimage.bbclass
> > +++ b/meta/classes/kernel-fitimage.bbclass
> > @@ -60,6 +60,14 @@ FIT_DESC ?= "Kernel fitImage for
> > ${DISTRO_NAME}/${PV}/${MACHINE}"
> >  # Sign individual images as well
> >  FIT_SIGN_INDIVIDUAL ?= "0"
> >  
> > +# Keys used to sign individually images nodes.
> > +# The keys to sign images nodes must be different from those used to
> > sign
> > +# configurations nodes, otherwise the "required" property, from
> > +# UBOOT_DTB_BINARY, will be set to "conf", because "conf" prevails
> > on "image".
> > +# Then images signature checking will not be mandatory and no error
> > will be
> > +# raised.
> > +# UBOOT_SIGN_IMG_KEYNAME = "dev2" # keys name in keydir (eg.
> > "dev2.crt", "dev2.key")
> > +
> >  #
> >  # Emit the fitImage ITS header
> >  #
> > @@ -121,7 +129,7 @@ fitimage_emit_section_kernel() {
> >  
> > kernel_csum="${FIT_HASH_ALG}"
> > kernel_sign_algo="${FIT_SIGN_ALG}"
> > -   kernel_sign_keyname="${UBOOT_SIGN_KEYNAME}"
> > +   kernel_sign_keyname="${UBOOT_SIGN_IMG_KEYNAME}"
> >  
> > ENTRYPOINT="${UBOOT_ENTRYPOINT}"
> > if [ -n "${UBOOT_ENTRYSYMBOL}" ]; then
> > @@ -167,7 +175,7 @@ fitimage_emit_section_dtb() {
> >  
> > dtb_csum="${FIT_HASH_ALG}"
> > dtb_sign_algo="${FIT_SIGN_ALG}"
> > -   dtb_sign_keyname="${UBOOT_SIGN_KEYNAME}"
> > +   dtb_sign_keyname="${UBOOT_SIGN_IMG_KEYNAME}"
> >  
> > dtb_loadline=""
> > dtb_ext=${DTB##*.}
> > @@ -214,7 +222,7 @@ fitimage_emit_section_boot_script() {
> >  
> >  bootscr_csum="${FIT_HASH_ALG}"
> > bootscr_sign_algo="${FIT_SIGN_ALG}"
> > -   bootscr_sign_keyname="${UBOOT_SIGN_KEYNAME}"
> > +   bootscr_sign_keyname="${UBOOT_SIGN_IMG_KEYNAME}"
> >  
> >  cat << EOF >> ${1}
> >  bootscr-${2} {
> > @@ -278,7 +286,7 @@ fitimage_emit_section_ramdisk() {
> >  
> > ramdisk_csum="${FIT_HASH_ALG}"
> > ramdisk_sign_algo="${FIT_SIGN_ALG}"
> > -   ramdisk_sign_keyname="${UBOOT_SIGN_KEYNAME}"
> > +   ramdisk_sign_keyname="${UBOOT_SIGN_IMG_KEYNAME}"
> > ramdisk_loadline=""
> > ramdisk_entryline=""
> >  
> > @@ -475,6 +483,10 @@ fitimage_assemble() {
> > bootscr_id=""
> > rm -f ${1} arch/${ARCH}/boot/${2}
> >  
> > +   if [ "${UBOOT_SIGN_KEYNAME}" = "${UBOOT_SIGN_IMG_KEYNAME}" ];
> > then
> > +   bbfatal "Keys used to sign images and configuration
> > nodes must be different."
> 
> This breaks oe-selftest, as seen in:
> https://autobuilder.yoctoproject.org/typhoon/#/builders/79/builds/2383/steps/14/logs/stdio
> 

Thank you for the feedback.

The tests also need to be updated, so I will submit a v2, including the
required changes on the test side.

Best regards,
Thomas Perrot

> 
> 
> 
> 

-- 
Thomas Perrot, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com



signature.asc
Description: This is a digitally signed message part

-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#154605): 
https://lists.openembedded.org/g/openembedded-core/message/154605
Mute This Topic: https://lists.openembedded.org/mt/84711813/21656
Group Owner: openembedded-core+ow...@lists.openembedded.org
Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub 
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [OE-core][PATCH] kernel-fitimage: images should not be signed with the same keys as the configurations

2021-08-08 Thread Alexandre Belloni

Hello,

On 06/08/2021 18:10:38+0200, Thomas Perrot wrote:
> Otherwise the "required" property, from UBOOT_DTB_BINARY, will be set to 
> "conf"
> and no error will be raised in case of error.
> 
> Signed-off-by: Thomas Perrot 
> ---
>  meta/classes/kernel-fitimage.bbclass | 40 
>  1 file changed, 35 insertions(+), 5 deletions(-)
> 
> diff --git a/meta/classes/kernel-fitimage.bbclass 
> b/meta/classes/kernel-fitimage.bbclass
> index a9d1002200c9..72f692e40e63 100644
> --- a/meta/classes/kernel-fitimage.bbclass
> +++ b/meta/classes/kernel-fitimage.bbclass
> @@ -60,6 +60,14 @@ FIT_DESC ?= "Kernel fitImage for 
> ${DISTRO_NAME}/${PV}/${MACHINE}"
>  # Sign individual images as well
>  FIT_SIGN_INDIVIDUAL ?= "0"
>  
> +# Keys used to sign individually images nodes.
> +# The keys to sign images nodes must be different from those used to sign
> +# configurations nodes, otherwise the "required" property, from
> +# UBOOT_DTB_BINARY, will be set to "conf", because "conf" prevails on 
> "image".
> +# Then images signature checking will not be mandatory and no error will be
> +# raised.
> +# UBOOT_SIGN_IMG_KEYNAME = "dev2" # keys name in keydir (eg. "dev2.crt", 
> "dev2.key")
> +
>  #
>  # Emit the fitImage ITS header
>  #
> @@ -121,7 +129,7 @@ fitimage_emit_section_kernel() {
>  
>   kernel_csum="${FIT_HASH_ALG}"
>   kernel_sign_algo="${FIT_SIGN_ALG}"
> - kernel_sign_keyname="${UBOOT_SIGN_KEYNAME}"
> + kernel_sign_keyname="${UBOOT_SIGN_IMG_KEYNAME}"
>  
>   ENTRYPOINT="${UBOOT_ENTRYPOINT}"
>   if [ -n "${UBOOT_ENTRYSYMBOL}" ]; then
> @@ -167,7 +175,7 @@ fitimage_emit_section_dtb() {
>  
>   dtb_csum="${FIT_HASH_ALG}"
>   dtb_sign_algo="${FIT_SIGN_ALG}"
> - dtb_sign_keyname="${UBOOT_SIGN_KEYNAME}"
> + dtb_sign_keyname="${UBOOT_SIGN_IMG_KEYNAME}"
>  
>   dtb_loadline=""
>   dtb_ext=${DTB##*.}
> @@ -214,7 +222,7 @@ fitimage_emit_section_boot_script() {
>  
>  bootscr_csum="${FIT_HASH_ALG}"
>   bootscr_sign_algo="${FIT_SIGN_ALG}"
> - bootscr_sign_keyname="${UBOOT_SIGN_KEYNAME}"
> + bootscr_sign_keyname="${UBOOT_SIGN_IMG_KEYNAME}"
>  
>  cat << EOF >> ${1}
>  bootscr-${2} {
> @@ -278,7 +286,7 @@ fitimage_emit_section_ramdisk() {
>  
>   ramdisk_csum="${FIT_HASH_ALG}"
>   ramdisk_sign_algo="${FIT_SIGN_ALG}"
> - ramdisk_sign_keyname="${UBOOT_SIGN_KEYNAME}"
> + ramdisk_sign_keyname="${UBOOT_SIGN_IMG_KEYNAME}"
>   ramdisk_loadline=""
>   ramdisk_entryline=""
>  
> @@ -475,6 +483,10 @@ fitimage_assemble() {
>   bootscr_id=""
>   rm -f ${1} arch/${ARCH}/boot/${2}
>  
> + if [ "${UBOOT_SIGN_KEYNAME}" = "${UBOOT_SIGN_IMG_KEYNAME}" ]; then
> + bbfatal "Keys used to sign images and configuration nodes must 
> be different."

This breaks oe-selftest, as seen in:
https://autobuilder.yoctoproject.org/typhoon/#/builders/79/builds/2383/steps/14/logs/stdio


-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#154597): 
https://lists.openembedded.org/g/openembedded-core/message/154597
Mute This Topic: https://lists.openembedded.org/mt/84711813/21656
Group Owner: openembedded-core+ow...@lists.openembedded.org
Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub 
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[OE-core][PATCH] kernel-fitimage: images should not be signed with the same keys as the configurations

2021-08-06 Thread Thomas Perrot

Otherwise the "required" property, from UBOOT_DTB_BINARY, will be set to "conf"
and no error will be raised in case of error.

Signed-off-by: Thomas Perrot 
---
 meta/classes/kernel-fitimage.bbclass | 40 
 1 file changed, 35 insertions(+), 5 deletions(-)

diff --git a/meta/classes/kernel-fitimage.bbclass 
b/meta/classes/kernel-fitimage.bbclass
index a9d1002200c9..72f692e40e63 100644
--- a/meta/classes/kernel-fitimage.bbclass
+++ b/meta/classes/kernel-fitimage.bbclass
@@ -60,6 +60,14 @@ FIT_DESC ?= "Kernel fitImage for 
${DISTRO_NAME}/${PV}/${MACHINE}"
 # Sign individual images as well
 FIT_SIGN_INDIVIDUAL ?= "0"
 
+# Keys used to sign individually images nodes.
+# The keys to sign images nodes must be different from those used to sign
+# configurations nodes, otherwise the "required" property, from
+# UBOOT_DTB_BINARY, will be set to "conf", because "conf" prevails on "image".
+# Then images signature checking will not be mandatory and no error will be
+# raised.
+# UBOOT_SIGN_IMG_KEYNAME = "dev2" # keys name in keydir (eg. "dev2.crt", 
"dev2.key")
+
 #
 # Emit the fitImage ITS header
 #
@@ -121,7 +129,7 @@ fitimage_emit_section_kernel() {
 
kernel_csum="${FIT_HASH_ALG}"
kernel_sign_algo="${FIT_SIGN_ALG}"
-   kernel_sign_keyname="${UBOOT_SIGN_KEYNAME}"
+   kernel_sign_keyname="${UBOOT_SIGN_IMG_KEYNAME}"
 
ENTRYPOINT="${UBOOT_ENTRYPOINT}"
if [ -n "${UBOOT_ENTRYSYMBOL}" ]; then
@@ -167,7 +175,7 @@ fitimage_emit_section_dtb() {
 
dtb_csum="${FIT_HASH_ALG}"
dtb_sign_algo="${FIT_SIGN_ALG}"
-   dtb_sign_keyname="${UBOOT_SIGN_KEYNAME}"
+   dtb_sign_keyname="${UBOOT_SIGN_IMG_KEYNAME}"
 
dtb_loadline=""
dtb_ext=${DTB##*.}
@@ -214,7 +222,7 @@ fitimage_emit_section_boot_script() {
 
 bootscr_csum="${FIT_HASH_ALG}"
bootscr_sign_algo="${FIT_SIGN_ALG}"
-   bootscr_sign_keyname="${UBOOT_SIGN_KEYNAME}"
+   bootscr_sign_keyname="${UBOOT_SIGN_IMG_KEYNAME}"
 
 cat << EOF >> ${1}
 bootscr-${2} {
@@ -278,7 +286,7 @@ fitimage_emit_section_ramdisk() {
 
ramdisk_csum="${FIT_HASH_ALG}"
ramdisk_sign_algo="${FIT_SIGN_ALG}"
-   ramdisk_sign_keyname="${UBOOT_SIGN_KEYNAME}"
+   ramdisk_sign_keyname="${UBOOT_SIGN_IMG_KEYNAME}"
ramdisk_loadline=""
ramdisk_entryline=""
 
@@ -475,6 +483,10 @@ fitimage_assemble() {
bootscr_id=""
rm -f ${1} arch/${ARCH}/boot/${2}
 
+   if [ "${UBOOT_SIGN_KEYNAME}" = "${UBOOT_SIGN_IMG_KEYNAME}" ]; then
+   bbfatal "Keys used to sign images and configuration nodes must 
be different."
+   fi
+
fitimage_emit_fit_header ${1}
 
#
@@ -674,7 +686,7 @@ do_kernel_generate_rsa_keys() {
 
if [ "${UBOOT_SIGN_ENABLE}" = "1" ] && [ "${FIT_GENERATE_KEYS}" = "1" 
]; then
 
-   # Generate keys only if they don't already exist
+   # Generate keys to sign configuration nodes, only if they don't 
already exist
if [ ! -f "${UBOOT_SIGN_KEYDIR}/${UBOOT_SIGN_KEYNAME}".key ] || 
\
[ ! -f "${UBOOT_SIGN_KEYDIR}/${UBOOT_SIGN_KEYNAME}".crt 
]; then
 
@@ -691,6 +703,24 @@ do_kernel_generate_rsa_keys() {
-key 
"${UBOOT_SIGN_KEYDIR}/${UBOOT_SIGN_KEYNAME}".key \
-out 
"${UBOOT_SIGN_KEYDIR}/${UBOOT_SIGN_KEYNAME}".crt
fi
+
+   # Generate keys to sign image nodes, only if they don't already 
exist
+   if [ ! -f "${UBOOT_SIGN_KEYDIR}/${UBOOT_SIGN_IMG_KEYNAME}".key 
] || \
+   [ ! -f 
"${UBOOT_SIGN_KEYDIR}/${UBOOT_SIGN_IMG_KEYNAME}".crt ]; then
+
+   # make directory if it does not already exist
+   mkdir -p "${UBOOT_SIGN_KEYDIR}"
+
+   echo "Generating RSA private key for signing fitImage"
+   openssl genrsa ${FIT_KEY_GENRSA_ARGS} -out \
+   
"${UBOOT_SIGN_KEYDIR}/${UBOOT_SIGN_IMG_KEYNAME}".key \
+   "${FIT_SIGN_NUMBITS}"
+
+   echo "Generating certificate for signing fitImage"
+   openssl req ${FIT_KEY_REQ_ARGS} "${FIT_KEY_SIGN_PKCS}" \
+   -key 
"${UBOOT_SIGN_KEYDIR}/${UBOOT_SIGN_IMG_KEYNAME}".key \
+   -out 
"${UBOOT_SIGN_KEYDIR}/${UBOOT_SIGN_IMG_KEYNAME}".crt
+   fi
fi
 }
 
-- 
2.31.1


-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#154568): 
https://lists.openembedded.org/g/openembedded-core/message/154568
Mute This Topic: https://lists.openembedded.org/mt/84711813/21656
Group Owner: openembedded-core+ow...@lists.openembedded.org
Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub 
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [OE-core][PATCH] kernel-fitimage: images should not be signed with the same keys as the configurations

Re: [OE-core][PATCH] kernel-fitimage: images should not be signed with the same keys as the configurations

[OE-core][PATCH] kernel-fitimage: images should not be signed with the same keys as the configurations

3 matches

Site Navigation

Mail list logo

Footer information