All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 0/5] rockchip: make it possible to sign the u-boot.itb
@ 2020-05-26 10:44 Heiko Stuebner
  2020-05-26 10:44 ` [PATCH v3 1/5] imx: mkimage_fit_atf: Fix FIT image if BL31.bin missing Heiko Stuebner
                   ` (5 more replies)
  0 siblings, 6 replies; 14+ messages in thread
From: Heiko Stuebner @ 2020-05-26 10:44 UTC (permalink / raw)
  To: u-boot

From: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>

This series makes it possible to sign a generated u-boot.itb automatically
even if the its-source got created by a generator script.

To let the SPL know about the key, the -K option for mkimage points
to the dts/dt-spl.dtb which can then get included into the spl binary.

Tested on Rockchip PX30 with a TPL -> SPL -> U-Boot.itb bootchain.

I've split out the the rsa/crypto fixes into a separate series
starting at [0].

[0] https://patchwork.ozlabs.org/project/uboot/patch/20200522141937.3523692-1-heiko at sntech.de/


changes in v3:
- add patch to fix imx make_fit_atf.sh error handling
- split out rsa fixes into separate series
changes in v2.1:
- depend on $(CONFIG_SPL_FIT_SIGNATURE)$(U_BOOT_ITS)
  instead of only $(CONFIG_SPL_FIT_GENERATOR)
changes in v2:
- add received reviews
- fix commit message typo
- add doc snippet explaining CONFIG_SPL_FIT_GENERATOR_KEY_HINT

Heiko Stuebner (5):
  imx: mkimage_fit_atf: Fix FIT image if BL31.bin missing
  mkimage: fit_image: handle multiple errors when writing signatures
  spl: fit: enable signing a generated u-boot.itb
  spl: fit: add Kconfig option to specify key-hint for fit_generator
  rockchip: make_fit_atf: add signature handling

 Kconfig                                | 16 ++++++++
 Makefile                               | 11 +++++-
 arch/arm/mach-imx/mkimage_fit_atf.sh   |  4 +-
 arch/arm/mach-rockchip/make_fit_atf.py | 51 +++++++++++++++++++++++++-
 doc/uImage.FIT/howto.txt               | 13 +++++++
 tools/image-host.c                     |  2 +-
 6 files changed, 92 insertions(+), 5 deletions(-)

-- 
2.25.1

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v3 1/5] imx: mkimage_fit_atf: Fix FIT image if BL31.bin missing
  2020-05-26 10:44 [PATCH v3 0/5] rockchip: make it possible to sign the u-boot.itb Heiko Stuebner
@ 2020-05-26 10:44 ` Heiko Stuebner
  2020-05-26 12:53   ` Peng Fan
  2020-05-26 10:44 ` [PATCH v3 2/5] mkimage: fit_image: handle multiple errors when writing signatures Heiko Stuebner
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 14+ messages in thread
From: Heiko Stuebner @ 2020-05-26 10:44 UTC (permalink / raw)
  To: u-boot

From: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>

Right now if its bl31.bin is missing, the imx make_fit_atf.sh does
return "sucessful" without generating an .its source file, which
makes autobuilders unhappy.

So this change is similar to Tom Rini's
commit 4c78028737c3 ("mksunxi_fit_atf.sh: Allow for this to complete when bl31.bin is missing")
in that it changes the behaviour to a warning and still lets the script
generate a usable u-boot.its and thus also lets the u-boot.itb get build
successfully

Cc: Stefano Babic <sbabic@denx.de>
Cc: Fabio Estevam <festevam@gmail.com>
Cc: uboot-imx at nxp.com
Signed-off-by: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
---
changes in v3:
- new patch

 arch/arm/mach-imx/mkimage_fit_atf.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-imx/mkimage_fit_atf.sh b/arch/arm/mach-imx/mkimage_fit_atf.sh
index dd1ca5ad3f..2224d85281 100755
--- a/arch/arm/mach-imx/mkimage_fit_atf.sh
+++ b/arch/arm/mach-imx/mkimage_fit_atf.sh
@@ -12,8 +12,8 @@
 [ -z "$BL33_LOAD_ADDR" ] && BL33_LOAD_ADDR="0x40200000"
 
 if [ ! -f $BL31 ]; then
-	echo "ERROR: BL31 file $BL31 NOT found" >&2
-	exit 0
+	echo "WARNING: BL31 file $BL31 NOT found, resulting binary is not-functional" >&2
+	BL31=/dev/null
 else
 	echo "$BL31 size: " >&2
 	ls -lct $BL31 | awk '{print $5}' >&2
-- 
2.25.1

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v3 2/5] mkimage: fit_image: handle multiple errors when writing signatures
  2020-05-26 10:44 [PATCH v3 0/5] rockchip: make it possible to sign the u-boot.itb Heiko Stuebner
  2020-05-26 10:44 ` [PATCH v3 1/5] imx: mkimage_fit_atf: Fix FIT image if BL31.bin missing Heiko Stuebner
@ 2020-05-26 10:44 ` Heiko Stuebner
  2020-05-31 14:07   ` Simon Glass
  2020-05-26 10:44 ` [PATCH v3 3/5] spl: fit: enable signing a generated u-boot.itb Heiko Stuebner
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 14+ messages in thread
From: Heiko Stuebner @ 2020-05-26 10:44 UTC (permalink / raw)
  To: u-boot

From: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>

fit_image_write_sig() contains mostly functions from libfdt that
return FDT_ERR_foo errors but also a call to fit_set_timestamp()
which returns a regular error.

When handling the size increase via multiple iterations, check
for both -FDT_ERR_NOSPACE but also for -ENOSPC.

There is no real conflict, as FDT_ERR_NOSPACE = 3 = ESRCH
(No such process) and ENOSPC = 28 which is above any FDT_ERR_*.

Signed-off-by: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
Reviewed-by: Kever Yang <kever.yang@rock-chips.com>
---
 tools/image-host.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/image-host.c b/tools/image-host.c
index 9a83b7f675..baf9590f3b 100644
--- a/tools/image-host.c
+++ b/tools/image-host.c
@@ -241,7 +241,7 @@ static int fit_image_process_sig(const char *keydir, void *keydest,
 	ret = fit_image_write_sig(fit, noffset, value, value_len, comment,
 			NULL, 0, cmdname);
 	if (ret) {
-		if (ret == -FDT_ERR_NOSPACE)
+		if (ret == -FDT_ERR_NOSPACE || ret == -ENOSPC)
 			return -ENOSPC;
 		printf("Can't write signature for '%s' signature node in '%s' conf node: %s\n",
 		       node_name, image_name, fdt_strerror(ret));
-- 
2.25.1

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v3 3/5] spl: fit: enable signing a generated u-boot.itb
  2020-05-26 10:44 [PATCH v3 0/5] rockchip: make it possible to sign the u-boot.itb Heiko Stuebner
  2020-05-26 10:44 ` [PATCH v3 1/5] imx: mkimage_fit_atf: Fix FIT image if BL31.bin missing Heiko Stuebner
  2020-05-26 10:44 ` [PATCH v3 2/5] mkimage: fit_image: handle multiple errors when writing signatures Heiko Stuebner
@ 2020-05-26 10:44 ` Heiko Stuebner
  2020-05-31 14:07   ` Simon Glass
  2020-05-26 10:44 ` [PATCH v3 4/5] spl: fit: add Kconfig option to specify key-hint for fit_generator Heiko Stuebner
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 14+ messages in thread
From: Heiko Stuebner @ 2020-05-26 10:44 UTC (permalink / raw)
  To: u-boot

From: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>

With SPL_FIT_SIGNATURE enabled we will likely want a generated
u-boot.itb to be signed and the key stores so that the spl can
reach it.

So add a SPL_FIT_SIGNATURE_KEY_DIR option and suitable hooks
into the Makefile to have mkimage sign the .itb and store the
used key into the spl dtb file.

The added dependencies should make sure that the u-boot.itb
gets generated before the spl-binary gets build, so that there
is the necessary space for the key to get included.

Signed-off-by: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
Reviewed-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
---
changes in v2.1:
- depend on $(CONFIG_SPL_FIT_SIGNATURE)$(U_BOOT_ITS)
  instead of only $(CONFIG_SPL_FIT_GENERATOR)

 Kconfig  |  8 ++++++++
 Makefile | 11 ++++++++++-
 2 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/Kconfig b/Kconfig
index 0c184f7f06..bab7c4f3ee 100644
--- a/Kconfig
+++ b/Kconfig
@@ -465,6 +465,14 @@ config SPL_FIT_SIGNATURE
 	select SPL_RSA_VERIFY
 	select SPL_IMAGE_SIGN_INFO
 
+config SPL_FIT_SIGNATURE_KEY_DIR
+	string "key directory for signing U-Boot FIT image"
+	depends on SPL_FIT_SIGNATURE
+	default "keys"
+	help
+	  The directory to give to mkimage to retrieve keys from when
+	  generating a signed U-Boot FIT image.
+
 config SPL_LOAD_FIT
 	bool "Enable SPL loading U-Boot as a FIT (basic fitImage features)"
 	select SPL_FIT
diff --git a/Makefile b/Makefile
index 463fa72e3f..b8f7536940 100644
--- a/Makefile
+++ b/Makefile
@@ -1407,6 +1407,14 @@ MKIMAGEFLAGS_u-boot.itb =
 else
 MKIMAGEFLAGS_u-boot.itb = -E
 endif
+ifdef CONFIG_SPL_FIT_SIGNATURE
+ifdef CONFIG_SPL_OF_CONTROL
+MKIMAGEFLAGS_u-boot.itb += -K dts/dt-spl.dtb -r
+ifneq ($(CONFIG_SPL_FIT_SIGNATURE_KEY_DIR),"")
+MKIMAGEFLAGS_u-boot.itb += -k $(CONFIG_SPL_FIT_SIGNATURE_KEY_DIR)
+endif
+endif
+endif
 
 u-boot.itb: u-boot-nodtb.bin \
 		$(if $(CONFIG_OF_SEPARATE)$(CONFIG_OF_EMBED)$(CONFIG_OF_HOSTFILE),dts/dt.dtb) \
@@ -1929,7 +1937,8 @@ spl/u-boot-spl.bin: spl/u-boot-spl
 
 spl/u-boot-spl: tools prepare \
 		$(if $(CONFIG_OF_SEPARATE)$(CONFIG_OF_EMBED)$(CONFIG_SPL_OF_PLATDATA),dts/dt.dtb) \
-		$(if $(CONFIG_OF_SEPARATE)$(CONFIG_OF_EMBED)$(CONFIG_TPL_OF_PLATDATA),dts/dt.dtb)
+		$(if $(CONFIG_OF_SEPARATE)$(CONFIG_OF_EMBED)$(CONFIG_TPL_OF_PLATDATA),dts/dt.dtb) \
+		$(if $(CONFIG_SPL_FIT_SIGNATURE)$(U_BOOT_ITS),u-boot.itb FORCE)
 	$(Q)$(MAKE) obj=spl -f $(srctree)/scripts/Makefile.spl all
 
 spl/sunxi-spl.bin: spl/u-boot-spl
-- 
2.25.1

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v3 4/5] spl: fit: add Kconfig option to specify key-hint for fit_generator
  2020-05-26 10:44 [PATCH v3 0/5] rockchip: make it possible to sign the u-boot.itb Heiko Stuebner
                   ` (2 preceding siblings ...)
  2020-05-26 10:44 ` [PATCH v3 3/5] spl: fit: enable signing a generated u-boot.itb Heiko Stuebner
@ 2020-05-26 10:44 ` Heiko Stuebner
  2020-05-26 10:44 ` [PATCH v3 5/5] rockchip: make_fit_atf: add signature handling Heiko Stuebner
  2020-05-31  8:28 ` [PATCH v3 0/5] rockchip: make it possible to sign the u-boot.itb Kever Yang
  5 siblings, 0 replies; 14+ messages in thread
From: Heiko Stuebner @ 2020-05-26 10:44 UTC (permalink / raw)
  To: u-boot

From: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>

The u-boot.itb can be generated either from a static .its that can
simply include the needed signature nodes with key-hints or from a
fit-generator script referenced in CONFIG_SPL_FIT_GENERATOR.

In the script-case it will need to know what key to include for the
key-hint and specified algorithm, so add an option for that key-name.

Signed-off-by: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
Reviewed-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
Reviewed-by: Kever Yang <kever.yang@rock-chips.com>
---
changes in v2:
- add doc snippet explaining the option

 Kconfig                  |  8 ++++++++
 doc/uImage.FIT/howto.txt | 13 +++++++++++++
 2 files changed, 21 insertions(+)

diff --git a/Kconfig b/Kconfig
index bab7c4f3ee..6a9bf8d865 100644
--- a/Kconfig
+++ b/Kconfig
@@ -548,6 +548,14 @@ config SPL_FIT_GENERATOR
 	  passed a list of supported device tree file stub names to
 	  include in the generated image.
 
+config SPL_FIT_GENERATOR_KEY_HINT
+	string "key hint for signing U-Boot FIT image"
+	depends on SPL_FIT_SIGNATURE
+	default "dev"
+	help
+	  The key hint to store in both the generated .its file as well as
+	  u-boot-key.dtb generated separately and embedded into the SPL.
+
 endif # SPL
 
 endif # FIT
diff --git a/doc/uImage.FIT/howto.txt b/doc/uImage.FIT/howto.txt
index 8592719685..f409b3770e 100644
--- a/doc/uImage.FIT/howto.txt
+++ b/doc/uImage.FIT/howto.txt
@@ -66,6 +66,19 @@ can point to a script which generates this image source file during
 the build process. It gets passed a list of device tree files (taken from the
 CONFIG_OF_LIST symbol).
 
+Signing u-boot.itb with SPL_FIT_GENERATOR
+-----------------------------------------
+
+u-boot.itb can be signed to verify the integrity of its components.
+When CONFIG_SPL_FIT_SIGNATURE is enabled the CONFIG_SPL_FIT_SIGNATURE_KEY_DIR
+option can be used to specifiy the key directory - either a relative or
+absolute path.
+
+See signature.txt for general signature handling, but when
+CONFIG_SPL_FIT_GENERATOR is used the option CONFIG_SPL_FIT_GENERATOR_KEY_HINT
+can be used to specify the key-hint that should be included into the
+created u-boot.its by the generator.
+
 Example 1 -- old-style (non-FDT) kernel booting
 -----------------------------------------------
 
-- 
2.25.1

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v3 5/5] rockchip: make_fit_atf: add signature handling
  2020-05-26 10:44 [PATCH v3 0/5] rockchip: make it possible to sign the u-boot.itb Heiko Stuebner
                   ` (3 preceding siblings ...)
  2020-05-26 10:44 ` [PATCH v3 4/5] spl: fit: add Kconfig option to specify key-hint for fit_generator Heiko Stuebner
@ 2020-05-26 10:44 ` Heiko Stuebner
  2020-05-31 14:07   ` Simon Glass
  2020-05-31  8:28 ` [PATCH v3 0/5] rockchip: make it possible to sign the u-boot.itb Kever Yang
  5 siblings, 1 reply; 14+ messages in thread
From: Heiko Stuebner @ 2020-05-26 10:44 UTC (permalink / raw)
  To: u-boot

From: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>

If the newly added fit-generator key-options are found, append needed
signature nodes to all generated image blocks, so that they can get
signed when mkimage later compiles the .itb from the generated .its.

Signed-off-by: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
Reviewed-by: Kever Yang <kever.yang@rock-chips.com>
---
 arch/arm/mach-rockchip/make_fit_atf.py | 51 +++++++++++++++++++++++++-
 1 file changed, 50 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-rockchip/make_fit_atf.py b/arch/arm/mach-rockchip/make_fit_atf.py
index d15c32b303..5b353f9d0a 100755
--- a/arch/arm/mach-rockchip/make_fit_atf.py
+++ b/arch/arm/mach-rockchip/make_fit_atf.py
@@ -14,6 +14,8 @@ import sys
 import getopt
 import logging
 import struct
+import Crypto
+from Crypto.PublicKey import RSA
 
 DT_HEADER = """
 /*
@@ -37,7 +39,9 @@ DT_UBOOT = """
 			arch = "arm64";
 			compression = "none";
 			load = <0x%08x>;
-		};
+"""
+
+DT_UBOOT_NODE_END = """		};
 
 """
 
@@ -47,6 +51,46 @@ DT_IMAGES_NODE_END = """	};
 
 DT_END = "};"
 
+def append_signature(file):
+    if not os.path.exists("u-boot.cfg"):
+        return
+
+    config = {}
+    with open("u-boot.cfg") as fd:
+        for line in fd:
+            line = line.strip()
+            values = line[8:].split(' ', 1)
+            if len(values) > 1:
+                key, value = values
+                value = value.strip('"')
+            else:
+                key = values[0]
+                value = '1'
+            if not key.startswith('CONFIG_'):
+                continue
+            config[key] = value
+
+    try:
+        keyhint = config["CONFIG_SPL_FIT_GENERATOR_KEY_HINT"]
+    except KeyError:
+        return
+
+    try:
+        keyfile = os.path.join(config["CONFIG_SPL_FIT_SIGNATURE_KEY_DIR"], keyhint)
+    except KeyError:
+        keyfile = keyhint
+
+    if not os.path.exists('%s.key' % keyfile):
+        return
+
+    f = open('%s.key' % keyfile,'r')
+    key = RSA.importKey(f.read())
+
+    file.write('\t\t\tsignature {\n')
+    file.write('\t\t\t\talgo = "sha256,rsa%s";\n' % key.n.bit_length())
+    file.write('\t\t\t\tkey-name-hint = "%s";\n' % keyhint)
+    file.write('\t\t\t};\n')
+
 def append_bl31_node(file, atf_index, phy_addr, elf_entry):
     # Append BL31 DT node to input FIT dts file.
     data = 'bl31_0x%08x.bin' % phy_addr
@@ -60,6 +104,7 @@ def append_bl31_node(file, atf_index, phy_addr, elf_entry):
     file.write('\t\t\tload = <0x%08x>;\n' % phy_addr)
     if atf_index == 1:
         file.write('\t\t\tentry = <0x%08x>;\n' % elf_entry)
+    append_signature(file);
     file.write('\t\t};\n')
     file.write('\n')
 
@@ -75,6 +120,7 @@ def append_tee_node(file, atf_index, phy_addr, elf_entry):
     file.write('\t\t\tcompression = "none";\n')
     file.write('\t\t\tload = <0x%08x>;\n' % phy_addr)
     file.write('\t\t\tentry = <0x%08x>;\n' % elf_entry)
+    append_signature(file);
     file.write('\t\t};\n')
     file.write('\n')
 
@@ -88,6 +134,7 @@ def append_fdt_node(file, dtbs):
         file.write('\t\t\tdata = /incbin/("%s");\n' % dtb)
         file.write('\t\t\ttype = "flat_dt";\n')
         file.write('\t\t\tcompression = "none";\n')
+        append_signature(file);
         file.write('\t\t};\n')
         file.write('\n')
         cnt = cnt + 1
@@ -129,6 +176,8 @@ def generate_atf_fit_dts_uboot(fit_file, uboot_file_name):
         raise ValueError("Invalid u-boot ELF image '%s'" % uboot_file_name)
     index, entry, p_paddr, data = segments[0]
     fit_file.write(DT_UBOOT % p_paddr)
+    append_signature(fit_file)
+    fit_file.write(DT_UBOOT_NODE_END)
 
 def generate_atf_fit_dts_bl31(fit_file, bl31_file_name, tee_file_name, dtbs_file_name):
     segments = unpack_elf(bl31_file_name)
-- 
2.25.1

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v3 1/5] imx: mkimage_fit_atf: Fix FIT image if BL31.bin missing
  2020-05-26 10:44 ` [PATCH v3 1/5] imx: mkimage_fit_atf: Fix FIT image if BL31.bin missing Heiko Stuebner
@ 2020-05-26 12:53   ` Peng Fan
  0 siblings, 0 replies; 14+ messages in thread
From: Peng Fan @ 2020-05-26 12:53 UTC (permalink / raw)
  To: u-boot

> Subject: [PATCH v3 1/5] imx: mkimage_fit_atf: Fix FIT image if BL31.bin
> missing
> 
> From: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
> 
> Right now if its bl31.bin is missing, the imx make_fit_atf.sh does return
> "sucessful" without generating an .its source file, which makes autobuilders
> unhappy.
> 
> So this change is similar to Tom Rini's
> commit 4c78028737c3 ("mksunxi_fit_atf.sh: Allow for this to complete when
> bl31.bin is missing") in that it changes the behaviour to a warning and still lets
> the script generate a usable u-boot.its and thus also lets the u-boot.itb get
> build successfully
> 
> Cc: Stefano Babic <sbabic@denx.de>
> Cc: Fabio Estevam <festevam@gmail.com>
> Cc: uboot-imx at nxp.com
> Signed-off-by: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
> ---
> changes in v3:
> - new patch
> 
>  arch/arm/mach-imx/mkimage_fit_atf.sh | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/mach-imx/mkimage_fit_atf.sh
> b/arch/arm/mach-imx/mkimage_fit_atf.sh
> index dd1ca5ad3f..2224d85281 100755
> --- a/arch/arm/mach-imx/mkimage_fit_atf.sh
> +++ b/arch/arm/mach-imx/mkimage_fit_atf.sh
> @@ -12,8 +12,8 @@
>  [ -z "$BL33_LOAD_ADDR" ] && BL33_LOAD_ADDR="0x40200000"
> 
>  if [ ! -f $BL31 ]; then
> -	echo "ERROR: BL31 file $BL31 NOT found" >&2
> -	exit 0
> +	echo "WARNING: BL31 file $BL31 NOT found, resulting binary is
> not-functional" >&2
> +	BL31=/dev/null
>  else
>  	echo "$BL31 size: " >&2
>  	ls -lct $BL31 | awk '{print $5}' >&2

Reviewed-by: Peng Fan <peng.fan@nxp.com>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v3 0/5] rockchip: make it possible to sign the u-boot.itb
  2020-05-26 10:44 [PATCH v3 0/5] rockchip: make it possible to sign the u-boot.itb Heiko Stuebner
                   ` (4 preceding siblings ...)
  2020-05-26 10:44 ` [PATCH v3 5/5] rockchip: make_fit_atf: add signature handling Heiko Stuebner
@ 2020-05-31  8:28 ` Kever Yang
  2020-06-19 10:47   ` Heiko Stuebner
  5 siblings, 1 reply; 14+ messages in thread
From: Kever Yang @ 2020-05-31  8:28 UTC (permalink / raw)
  To: u-boot

Hi Heiko,

 ??? This patch set make rk3288 series board build fail. Could you help 
to check again?


Thanks,

- Kever

On 2020/5/26 ??6:44, Heiko Stuebner wrote:
> From: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
>
> This series makes it possible to sign a generated u-boot.itb automatically
> even if the its-source got created by a generator script.
>
> To let the SPL know about the key, the -K option for mkimage points
> to the dts/dt-spl.dtb which can then get included into the spl binary.
>
> Tested on Rockchip PX30 with a TPL -> SPL -> U-Boot.itb bootchain.
>
> I've split out the the rsa/crypto fixes into a separate series
> starting at [0].
>
> [0] https://patchwork.ozlabs.org/project/uboot/patch/20200522141937.3523692-1-heiko at sntech.de/
>
>
> changes in v3:
> - add patch to fix imx make_fit_atf.sh error handling
> - split out rsa fixes into separate series
> changes in v2.1:
> - depend on $(CONFIG_SPL_FIT_SIGNATURE)$(U_BOOT_ITS)
>    instead of only $(CONFIG_SPL_FIT_GENERATOR)
> changes in v2:
> - add received reviews
> - fix commit message typo
> - add doc snippet explaining CONFIG_SPL_FIT_GENERATOR_KEY_HINT
>
> Heiko Stuebner (5):
>    imx: mkimage_fit_atf: Fix FIT image if BL31.bin missing
>    mkimage: fit_image: handle multiple errors when writing signatures
>    spl: fit: enable signing a generated u-boot.itb
>    spl: fit: add Kconfig option to specify key-hint for fit_generator
>    rockchip: make_fit_atf: add signature handling
>
>   Kconfig                                | 16 ++++++++
>   Makefile                               | 11 +++++-
>   arch/arm/mach-imx/mkimage_fit_atf.sh   |  4 +-
>   arch/arm/mach-rockchip/make_fit_atf.py | 51 +++++++++++++++++++++++++-
>   doc/uImage.FIT/howto.txt               | 13 +++++++
>   tools/image-host.c                     |  2 +-
>   6 files changed, 92 insertions(+), 5 deletions(-)
>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v3 2/5] mkimage: fit_image: handle multiple errors when writing signatures
  2020-05-26 10:44 ` [PATCH v3 2/5] mkimage: fit_image: handle multiple errors when writing signatures Heiko Stuebner
@ 2020-05-31 14:07   ` Simon Glass
  2020-06-19 10:47     ` Heiko Stuebner
  0 siblings, 1 reply; 14+ messages in thread
From: Simon Glass @ 2020-05-31 14:07 UTC (permalink / raw)
  To: u-boot

Hi Heiko,

On Tue, 26 May 2020 at 04:44, Heiko Stuebner <heiko@sntech.de> wrote:
>
> From: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
>
> fit_image_write_sig() contains mostly functions from libfdt that
> return FDT_ERR_foo errors but also a call to fit_set_timestamp()
> which returns a regular error.
>
> When handling the size increase via multiple iterations, check
> for both -FDT_ERR_NOSPACE but also for -ENOSPC.
>
> There is no real conflict, as FDT_ERR_NOSPACE = 3 = ESRCH
> (No such process) and ENOSPC = 28 which is above any FDT_ERR_*.
>
> Signed-off-by: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
> Reviewed-by: Simon Glass <sjg@chromium.org>
> Reviewed-by: Kever Yang <kever.yang@rock-chips.com>
> ---
>  tools/image-host.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Changing my mind on this - I wonder if we can change
fit_image_write_sig() to always return an errno code, never an FDT
code? Could be a follow-on patch.

>
> diff --git a/tools/image-host.c b/tools/image-host.c
> index 9a83b7f675..baf9590f3b 100644
> --- a/tools/image-host.c
> +++ b/tools/image-host.c
> @@ -241,7 +241,7 @@ static int fit_image_process_sig(const char *keydir, void *keydest,
>         ret = fit_image_write_sig(fit, noffset, value, value_len, comment,
>                         NULL, 0, cmdname);
>         if (ret) {
> -               if (ret == -FDT_ERR_NOSPACE)
> +               if (ret == -FDT_ERR_NOSPACE || ret == -ENOSPC)
>                         return -ENOSPC;
>                 printf("Can't write signature for '%s' signature node in '%s' conf node: %s\n",
>                        node_name, image_name, fdt_strerror(ret));
> --
> 2.25.1
>

Regards,
Simon

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v3 3/5] spl: fit: enable signing a generated u-boot.itb
  2020-05-26 10:44 ` [PATCH v3 3/5] spl: fit: enable signing a generated u-boot.itb Heiko Stuebner
@ 2020-05-31 14:07   ` Simon Glass
  0 siblings, 0 replies; 14+ messages in thread
From: Simon Glass @ 2020-05-31 14:07 UTC (permalink / raw)
  To: u-boot

On Tue, 26 May 2020 at 04:44, Heiko Stuebner <heiko@sntech.de> wrote:
>
> From: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
>
> With SPL_FIT_SIGNATURE enabled we will likely want a generated
> u-boot.itb to be signed and the key stores so that the spl can
> reach it.
>
> So add a SPL_FIT_SIGNATURE_KEY_DIR option and suitable hooks
> into the Makefile to have mkimage sign the .itb and store the
> used key into the spl dtb file.
>
> The added dependencies should make sure that the u-boot.itb
> gets generated before the spl-binary gets build, so that there
> is the necessary space for the key to get included.
>
> Signed-off-by: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
> Reviewed-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
> ---
> changes in v2.1:
> - depend on $(CONFIG_SPL_FIT_SIGNATURE)$(U_BOOT_ITS)
>   instead of only $(CONFIG_SPL_FIT_GENERATOR)
>
>  Kconfig  |  8 ++++++++
>  Makefile | 11 ++++++++++-
>  2 files changed, 18 insertions(+), 1 deletion(-)
>

Reviewed-by: Simon Glass <sjg@chromium.org>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v3 5/5] rockchip: make_fit_atf: add signature handling
  2020-05-26 10:44 ` [PATCH v3 5/5] rockchip: make_fit_atf: add signature handling Heiko Stuebner
@ 2020-05-31 14:07   ` Simon Glass
  0 siblings, 0 replies; 14+ messages in thread
From: Simon Glass @ 2020-05-31 14:07 UTC (permalink / raw)
  To: u-boot

On Tue, 26 May 2020 at 04:44, Heiko Stuebner <heiko@sntech.de> wrote:
>
> From: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
>
> If the newly added fit-generator key-options are found, append needed
> signature nodes to all generated image blocks, so that they can get
> signed when mkimage later compiles the .itb from the generated .its.
>
> Signed-off-by: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
> Reviewed-by: Kever Yang <kever.yang@rock-chips.com>
> ---
>  arch/arm/mach-rockchip/make_fit_atf.py | 51 +++++++++++++++++++++++++-
>  1 file changed, 50 insertions(+), 1 deletion(-)
>

Maybe I am repeating myself, but this really should move to binman I
think. I'll send a few patches that might help.


- Simon

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v3 2/5] mkimage: fit_image: handle multiple errors when writing signatures
  2020-05-31 14:07   ` Simon Glass
@ 2020-06-19 10:47     ` Heiko Stuebner
  0 siblings, 0 replies; 14+ messages in thread
From: Heiko Stuebner @ 2020-06-19 10:47 UTC (permalink / raw)
  To: u-boot

Am Sonntag, 31. Mai 2020, 16:07:56 CEST schrieb Simon Glass:
> Hi Heiko,
> 
> On Tue, 26 May 2020 at 04:44, Heiko Stuebner <heiko@sntech.de> wrote:
> >
> > From: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
> >
> > fit_image_write_sig() contains mostly functions from libfdt that
> > return FDT_ERR_foo errors but also a call to fit_set_timestamp()
> > which returns a regular error.
> >
> > When handling the size increase via multiple iterations, check
> > for both -FDT_ERR_NOSPACE but also for -ENOSPC.
> >
> > There is no real conflict, as FDT_ERR_NOSPACE = 3 = ESRCH
> > (No such process) and ENOSPC = 28 which is above any FDT_ERR_*.
> >
> > Signed-off-by: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
> > Reviewed-by: Simon Glass <sjg@chromium.org>
> > Reviewed-by: Kever Yang <kever.yang@rock-chips.com>
> > ---
> >  tools/image-host.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> Changing my mind on this - I wonder if we can change
> fit_image_write_sig() to always return an errno code, never an FDT
> code? Could be a follow-on patch.

I'll take that suggestion and do a followup with this suggestion,
simply because this may open up additional discussion.

Heiko


> > diff --git a/tools/image-host.c b/tools/image-host.c
> > index 9a83b7f675..baf9590f3b 100644
> > --- a/tools/image-host.c
> > +++ b/tools/image-host.c
> > @@ -241,7 +241,7 @@ static int fit_image_process_sig(const char *keydir, void *keydest,
> >         ret = fit_image_write_sig(fit, noffset, value, value_len, comment,
> >                         NULL, 0, cmdname);
> >         if (ret) {
> > -               if (ret == -FDT_ERR_NOSPACE)
> > +               if (ret == -FDT_ERR_NOSPACE || ret == -ENOSPC)
> >                         return -ENOSPC;
> >                 printf("Can't write signature for '%s' signature node in '%s' conf node: %s\n",
> >                        node_name, image_name, fdt_strerror(ret));
> > --
> > 2.25.1
> >
> 
> Regards,
> Simon
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v3 0/5] rockchip: make it possible to sign the u-boot.itb
  2020-05-31  8:28 ` [PATCH v3 0/5] rockchip: make it possible to sign the u-boot.itb Kever Yang
@ 2020-06-19 10:47   ` Heiko Stuebner
  2020-06-26  1:12     ` Simon Glass
  0 siblings, 1 reply; 14+ messages in thread
From: Heiko Stuebner @ 2020-06-19 10:47 UTC (permalink / raw)
  To: u-boot

Hi Kever,

Am Sonntag, 31. Mai 2020, 10:28:45 CEST schrieb Kever Yang:
>      This patch set make rk3288 series board build fail. Could you help 
> to check again?

the additional patch in v4 adding the ifdef CONFIG_SPL_FIT clamp,
should hopefully fix this

Heiko

> On 2020/5/26 ??6:44, Heiko Stuebner wrote:
> > From: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
> >
> > This series makes it possible to sign a generated u-boot.itb automatically
> > even if the its-source got created by a generator script.
> >
> > To let the SPL know about the key, the -K option for mkimage points
> > to the dts/dt-spl.dtb which can then get included into the spl binary.
> >
> > Tested on Rockchip PX30 with a TPL -> SPL -> U-Boot.itb bootchain.
> >
> > I've split out the the rsa/crypto fixes into a separate series
> > starting at [0].
> >
> > [0] https://patchwork.ozlabs.org/project/uboot/patch/20200522141937.3523692-1-heiko at sntech.de/
> >
> >
> > changes in v3:
> > - add patch to fix imx make_fit_atf.sh error handling
> > - split out rsa fixes into separate series
> > changes in v2.1:
> > - depend on $(CONFIG_SPL_FIT_SIGNATURE)$(U_BOOT_ITS)
> >    instead of only $(CONFIG_SPL_FIT_GENERATOR)
> > changes in v2:
> > - add received reviews
> > - fix commit message typo
> > - add doc snippet explaining CONFIG_SPL_FIT_GENERATOR_KEY_HINT
> >
> > Heiko Stuebner (5):
> >    imx: mkimage_fit_atf: Fix FIT image if BL31.bin missing
> >    mkimage: fit_image: handle multiple errors when writing signatures
> >    spl: fit: enable signing a generated u-boot.itb
> >    spl: fit: add Kconfig option to specify key-hint for fit_generator
> >    rockchip: make_fit_atf: add signature handling
> >
> >   Kconfig                                | 16 ++++++++
> >   Makefile                               | 11 +++++-
> >   arch/arm/mach-imx/mkimage_fit_atf.sh   |  4 +-
> >   arch/arm/mach-rockchip/make_fit_atf.py | 51 +++++++++++++++++++++++++-
> >   doc/uImage.FIT/howto.txt               | 13 +++++++
> >   tools/image-host.c                     |  2 +-
> >   6 files changed, 92 insertions(+), 5 deletions(-)
> >
> 
> 
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v3 0/5] rockchip: make it possible to sign the u-boot.itb
  2020-06-19 10:47   ` Heiko Stuebner
@ 2020-06-26  1:12     ` Simon Glass
  0 siblings, 0 replies; 14+ messages in thread
From: Simon Glass @ 2020-06-26  1:12 UTC (permalink / raw)
  To: u-boot

Hi,

On Fri, 19 Jun 2020 at 04:47, Heiko Stuebner <heiko@sntech.de> wrote:
>
> Hi Kever,
>
> Am Sonntag, 31. Mai 2020, 10:28:45 CEST schrieb Kever Yang:
> >      This patch set make rk3288 series board build fail. Could you help
> > to check again?
>
> the additional patch in v4 adding the ifdef CONFIG_SPL_FIT clamp,
> should hopefully fix this
>
> Heiko
>
> > On 2020/5/26 ??6:44, Heiko Stuebner wrote:
> > > From: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
> > >
> > > This series makes it possible to sign a generated u-boot.itb automatically
> > > even if the its-source got created by a generator script.
> > >
> > > To let the SPL know about the key, the -K option for mkimage points
> > > to the dts/dt-spl.dtb which can then get included into the spl binary.
> > >
> > > Tested on Rockchip PX30 with a TPL -> SPL -> U-Boot.itb bootchain.
> > >
> > > I've split out the the rsa/crypto fixes into a separate series
> > > starting at [0].
> > >
> > > [0] https://patchwork.ozlabs.org/project/uboot/patch/20200522141937.3523692-1-heiko at sntech.de/
> > >
> > >
> > > changes in v3:
> > > - add patch to fix imx make_fit_atf.sh error handling
> > > - split out rsa fixes into separate series
> > > changes in v2.1:
> > > - depend on $(CONFIG_SPL_FIT_SIGNATURE)$(U_BOOT_ITS)
> > >    instead of only $(CONFIG_SPL_FIT_GENERATOR)
> > > changes in v2:
> > > - add received reviews
> > > - fix commit message typo
> > > - add doc snippet explaining CONFIG_SPL_FIT_GENERATOR_KEY_HINT
> > >
> > > Heiko Stuebner (5):
> > >    imx: mkimage_fit_atf: Fix FIT image if BL31.bin missing
> > >    mkimage: fit_image: handle multiple errors when writing signatures
> > >    spl: fit: enable signing a generated u-boot.itb
> > >    spl: fit: add Kconfig option to specify key-hint for fit_generator
> > >    rockchip: make_fit_atf: add signature handling
> > >
> > >   Kconfig                                | 16 ++++++++
> > >   Makefile                               | 11 +++++-
> > >   arch/arm/mach-imx/mkimage_fit_atf.sh   |  4 +-
> > >   arch/arm/mach-rockchip/make_fit_atf.py | 51 +++++++++++++++++++++++++-
> > >   doc/uImage.FIT/howto.txt               | 13 +++++++
> > >   tools/image-host.c                     |  2 +-
> > >   6 files changed, 92 insertions(+), 5 deletions(-)

Please no more adding to this script. It should be removed and use
binman to make images.

Regards,
Simon

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2020-06-26  1:12 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-26 10:44 [PATCH v3 0/5] rockchip: make it possible to sign the u-boot.itb Heiko Stuebner
2020-05-26 10:44 ` [PATCH v3 1/5] imx: mkimage_fit_atf: Fix FIT image if BL31.bin missing Heiko Stuebner
2020-05-26 12:53   ` Peng Fan
2020-05-26 10:44 ` [PATCH v3 2/5] mkimage: fit_image: handle multiple errors when writing signatures Heiko Stuebner
2020-05-31 14:07   ` Simon Glass
2020-06-19 10:47     ` Heiko Stuebner
2020-05-26 10:44 ` [PATCH v3 3/5] spl: fit: enable signing a generated u-boot.itb Heiko Stuebner
2020-05-31 14:07   ` Simon Glass
2020-05-26 10:44 ` [PATCH v3 4/5] spl: fit: add Kconfig option to specify key-hint for fit_generator Heiko Stuebner
2020-05-26 10:44 ` [PATCH v3 5/5] rockchip: make_fit_atf: add signature handling Heiko Stuebner
2020-05-31 14:07   ` Simon Glass
2020-05-31  8:28 ` [PATCH v3 0/5] rockchip: make it possible to sign the u-boot.itb Kever Yang
2020-06-19 10:47   ` Heiko Stuebner
2020-06-26  1:12     ` Simon Glass

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.