diff mbox series

[v6] u-boot: kernel-fitimage: Fix dependency loop if UBOOT_SIGN_ENABLE and UBOOT_ENV enabled

Message ID 20250121212152.150384-1-marex@denx.de
State Accepted, archived
Commit 259bfa86f384206f0d0a96a5b84887186c5f689e
Headers show
Series [v6] u-boot: kernel-fitimage: Fix dependency loop if UBOOT_SIGN_ENABLE and UBOOT_ENV enabled | expand

Commit Message

Marek Vasut Jan. 21, 2025, 9:20 p.m. UTC
In case both UBOOT_SIGN_ENABLE and UBOOT_ENV are enabled and
kernel-fitimage.bbclass is in use to generate signed kernel
fitImage, there is a circular dependency between uboot-sign
and kernel-fitimage bbclasses . The loop looks like this:

kernel-fitimage.bbclass:
- do_populate_sysroot depends on do_assemble_fitimage
  - do_assemble_fitimage depends on virtual/bootloader:do_populate_sysroot
    - virtual/bootloader:do_populate_sysroot depends on virtual/bootloader:do_install
      => The virtual/bootloader:do_install installs and the
         virtual/bootloader:do_populate_sysroot places into
         sysroot an U-Boot environment script embedded into
         kernel fitImage during do_assemble_fitimage run .

uboot-sign.bbclass:
- DEPENDS on KERNEL_PN, which is really virtual/kernel. More accurately
  - do_deploy depends on do_uboot_assemble_fitimage
  - do_install depends on do_uboot_assemble_fitimage
  - do_uboot_assemble_fitimage depends on virtual/kernel:do_populate_sysroot
    => do_install depends on virtual/kernel:do_populate_sysroot

=> virtual/bootloader:do_install depends on virtual/kernel:do_populate_sysroot
   virtual/kernel:do_populate_sysroot depends on virtual/bootloader:do_install

Attempt to resolve the loop. Pull fitimage configuration options into separate
new configuration file image-fitimage.conf so these configuration options can
be shared by both uboot-sign.bbclass and kernel-fitimage.bbclass, and make use
of mkimage -f auto-conf / mkimage -f auto option to insert /signature node key-*
subnode into U-Boot control DT without depending on the layout of kernel fitImage
itself. This is perfectly valid to do, because the U-Boot /signature node key-*
subnodes 'required' property can contain either of two values, 'conf' or 'image'
to authenticate either selected configuration or all of images when booting the
fitImage.

For details of the U-Boot fitImage signing process, see:
https://docs.u-boot.org/en/latest/usage/fit/signature.html
For details of mkimage -f auto-conf and -f auto, see:
https://manpages.debian.org/experimental/u-boot-tools/mkimage.1.en.html#EXAMPLES

Fixes: 5e12dc911d0c ("u-boot: Rework signing to remove interdependencies")
Reviewed-by: Adrian Freihofer <adrian.freihofer@siemens.com>
Signed-off-by: Marek Vasut <marex@denx.de>
---
Cc: Adrian Freihofer <adrian.freihofer@siemens.com>
Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>
Cc: Richard Purdie <richard.purdie@linuxfoundation.org>
Cc: Sean Anderson <sean.anderson@seco.com>
---
V2: Take a different approach, split the kernel-fitimage.bbclass and
    use it to generate dummy fitImage on demand
V3: Use mkimage -f auto-conf and mkimage -f auto to break the loop,
    the fitImage .its source is not even needed because the 'required'
    property can only have two values, 'conf' or 'image' .
V4: Restore CC list
V5: - Add missing trailing backslash after unused.itb
    - Replace ${UBOOT_MKIMAGE_MODE} with $UBOOT_MKIMAGE_MODE
    - Move -d /dev/null to the end of mkimage invocation
V6: - Turn kernel-fitimage-config.bbclass into conf/image-fitimage.conf
      and update commit message to reflect that change
    - Add a bit of a comment at the start of conf/image-fitimage.conf
      explaining what those options are, the documentation for each
      option is further in the conf/image-fitimage.conf above each
      option already.
    - Add RB from Adrian
---
 meta/classes-recipe/kernel-fitimage.bbclass | 53 +--------------------
 meta/classes-recipe/uboot-sign.bbclass      | 26 +++++-----
 meta/conf/image-fitimage.conf               | 53 +++++++++++++++++++++
 3 files changed, 68 insertions(+), 64 deletions(-)
 create mode 100644 meta/conf/image-fitimage.conf
diff mbox series

Patch

diff --git a/meta/classes-recipe/kernel-fitimage.bbclass b/meta/classes-recipe/kernel-fitimage.bbclass
index 67c98adb232..fe076badfa5 100644
--- a/meta/classes-recipe/kernel-fitimage.bbclass
+++ b/meta/classes-recipe/kernel-fitimage.bbclass
@@ -5,6 +5,7 @@ 
 #
 
 inherit kernel-uboot kernel-artifact-names uboot-config
+require conf/image-fitimage.conf
 
 def get_fit_replacement_type(d):
     kerneltypes = d.getVar('KERNEL_IMAGETYPES') or ""
@@ -52,58 +53,6 @@  python __anonymous () {
         d.setVar('EXTERNAL_KERNEL_DEVICETREE', "${RECIPE_SYSROOT}/boot/devicetree")
 }
 
-
-# Description string
-FIT_DESC ?= "Kernel fitImage for ${DISTRO_NAME}/${PV}/${MACHINE}"
-
-# Kernel fitImage Hash Algo
-FIT_HASH_ALG ?= "sha256"
-
-# Kernel fitImage Signature Algo
-FIT_SIGN_ALG ?= "rsa2048"
-
-# Kernel / U-Boot fitImage Padding Algo
-FIT_PAD_ALG ?= "pkcs-1.5"
-
-# Generate keys for signing Kernel fitImage
-FIT_GENERATE_KEYS ?= "0"
-
-# Size of private keys in number of bits
-FIT_SIGN_NUMBITS ?= "2048"
-
-# args to openssl genrsa (Default is just the public exponent)
-FIT_KEY_GENRSA_ARGS ?= "-F4"
-
-# args to openssl req (Default is -batch for non interactive mode and
-# -new for new certificate)
-FIT_KEY_REQ_ARGS ?= "-batch -new"
-
-# Standard format for public key certificate
-FIT_KEY_SIGN_PKCS ?= "-x509"
-
-# Sign individual images as well
-FIT_SIGN_INDIVIDUAL ?= "0"
-
-FIT_CONF_PREFIX ?= "conf-"
-FIT_CONF_PREFIX[doc] = "Prefix to use for FIT configuration node name"
-
-FIT_SUPPORTED_INITRAMFS_FSTYPES ?= "cpio.lz4 cpio.lzo cpio.lzma cpio.xz cpio.zst cpio.gz ext2.gz cpio"
-
-# Allow user to select the default DTB for FIT image when multiple dtb's exists.
-FIT_CONF_DEFAULT_DTB ?= ""
-
-# length of address in number of <u32> cells
-# ex: 1 32bits address, 2 64bits address
-FIT_ADDRESS_CELLS ?= "1"
-
-# Keys used to sign individually image nodes.
-# The keys to sign image nodes must be different from those used to sign
-# configuration nodes, otherwise the "required" property, from
-# UBOOT_DTB_BINARY, will be set to "conf", because "conf" prevails on "image".
-# Then the images signature checking will not be mandatory and no error will be
-# raised in case of failure.
-# UBOOT_SIGN_IMG_KEYNAME = "dev2" # keys name in keydir (eg. "dev2.crt", "dev2.key")
-
 #
 # Emit the fitImage ITS header
 #
diff --git a/meta/classes-recipe/uboot-sign.bbclass b/meta/classes-recipe/uboot-sign.bbclass
index a17be745cec..96c47ab0165 100644
--- a/meta/classes-recipe/uboot-sign.bbclass
+++ b/meta/classes-recipe/uboot-sign.bbclass
@@ -26,6 +26,7 @@ 
 
 # We need some variables from u-boot-config
 inherit uboot-config
+require conf/image-fitimage.conf
 
 # Enable use of a U-Boot fitImage
 UBOOT_FITIMAGE_ENABLE ?= "0"
@@ -85,9 +86,6 @@  UBOOT_FIT_KEY_SIGN_PKCS ?= "-x509"
 # ex: 1 32bits address, 2 64bits address
 UBOOT_FIT_ADDRESS_CELLS ?= "1"
 
-# This is only necessary for determining the signing configuration
-KERNEL_PN = "${PREFERRED_PROVIDER_virtual/kernel}"
-
 UBOOT_FIT_UBOOT_LOADADDRESS ?= "${UBOOT_LOADADDRESS}"
 UBOOT_FIT_UBOOT_ENTRYPOINT ?= "${UBOOT_ENTRYPOINT}"
 
@@ -96,8 +94,6 @@  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.appendVar('DEPENDS', " " + d.getVar('KERNEL_PN'))
 }
 
 concat_dtb() {
@@ -106,16 +102,26 @@  concat_dtb() {
 
 	if [ -e "${UBOOT_DTB_BINARY}" ]; then
 		# Re-sign the kernel in order to add the keys to our dtb
+		UBOOT_MKIMAGE_MODE="auto-conf"
+		# Signing individual images is not recommended as that
+		# makes fitImage susceptible to mix-and-match attack.
+		if [ "${FIT_SIGN_INDIVIDUAL}" = "1" ] ; then
+			UBOOT_MKIMAGE_MODE="auto"
+		fi
 		${UBOOT_MKIMAGE_SIGN} \
 			${@'-D "${UBOOT_MKIMAGE_DTCOPTS}"' if len('${UBOOT_MKIMAGE_DTCOPTS}') else ''} \
-			-F -k "${UBOOT_SIGN_KEYDIR}" \
+			-f $UBOOT_MKIMAGE_MODE \
+			-k "${UBOOT_SIGN_KEYDIR}" \
+			-o "${FIT_HASH_ALG},${FIT_SIGN_ALG}" \
+			-g "${UBOOT_SIGN_IMG_KEYNAME}" \
 			-K "${UBOOT_DTB_BINARY}" \
-			-r ${B}/fitImage-linux \
+			-d /dev/null \
+			-r ${B}/unused.itb \
 			${UBOOT_MKIMAGE_SIGN_ARGS}
 		# Verify the kernel image and u-boot dtb
 		${UBOOT_FIT_CHECK_SIGN} \
 			-k "${UBOOT_DTB_BINARY}" \
-			-f ${B}/fitImage-linux
+			-f ${B}/unused.itb
 		cp ${UBOOT_DTB_BINARY} ${UBOOT_DTB_SIGNED}
 	fi
 
@@ -351,10 +357,6 @@  uboot_assemble_fitimage_helper() {
 }
 
 do_uboot_assemble_fitimage() {
-	if [ "${UBOOT_SIGN_ENABLE}" = "1" ] ; then
-		cp "${STAGING_DIR_HOST}/sysroot-only/fitImage" "${B}/fitImage-linux"
-	fi
-
 	if [ -n "${UBOOT_CONFIG}" ]; then
 		unset i
 		for config in ${UBOOT_MACHINE}; do
diff --git a/meta/conf/image-fitimage.conf b/meta/conf/image-fitimage.conf
new file mode 100644
index 00000000000..be9ae301346
--- /dev/null
+++ b/meta/conf/image-fitimage.conf
@@ -0,0 +1,53 @@ 
+# Possible options for fitImage generation, mainly
+# related to signing of the fitImage content.
+
+# Description string
+FIT_DESC ?= "Kernel fitImage for ${DISTRO_NAME}/${PV}/${MACHINE}"
+
+# Kernel fitImage Hash Algo
+FIT_HASH_ALG ?= "sha256"
+
+# Kernel fitImage Signature Algo
+FIT_SIGN_ALG ?= "rsa2048"
+
+# Kernel / U-Boot fitImage Padding Algo
+FIT_PAD_ALG ?= "pkcs-1.5"
+
+# Generate keys for signing Kernel fitImage
+FIT_GENERATE_KEYS ?= "0"
+
+# Size of private keys in number of bits
+FIT_SIGN_NUMBITS ?= "2048"
+
+# args to openssl genrsa (Default is just the public exponent)
+FIT_KEY_GENRSA_ARGS ?= "-F4"
+
+# args to openssl req (Default is -batch for non interactive mode and
+# -new for new certificate)
+FIT_KEY_REQ_ARGS ?= "-batch -new"
+
+# Standard format for public key certificate
+FIT_KEY_SIGN_PKCS ?= "-x509"
+
+# Sign individual images as well
+FIT_SIGN_INDIVIDUAL ?= "0"
+
+FIT_CONF_PREFIX ?= "conf-"
+FIT_CONF_PREFIX[doc] = "Prefix to use for FIT configuration node name"
+
+FIT_SUPPORTED_INITRAMFS_FSTYPES ?= "cpio.lz4 cpio.lzo cpio.lzma cpio.xz cpio.zst cpio.gz ext2.gz cpio"
+
+# Allow user to select the default DTB for FIT image when multiple dtb's exists.
+FIT_CONF_DEFAULT_DTB ?= ""
+
+# length of address in number of <u32> cells
+# ex: 1 32bits address, 2 64bits address
+FIT_ADDRESS_CELLS ?= "1"
+
+# Keys used to sign individually image nodes.
+# The keys to sign image nodes must be different from those used to sign
+# configuration nodes, otherwise the "required" property, from
+# UBOOT_DTB_BINARY, will be set to "conf", because "conf" prevails on "image".
+# Then the images signature checking will not be mandatory and no error will be
+# raised in case of failure.
+# UBOOT_SIGN_IMG_KEYNAME = "dev2" # keys name in keydir (eg. "dev2.crt", "dev2.key")