[PATCH v2 0/7] rockchip: make it possible to sign the u-boot.itb

[PATCH v2 0/7] rockchip: make it possible to sign the u-boot.itb

[Heiko Stuebner]

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

This series fixes some issues I found with SPL_FIT_SIGNATURE enabled
and then 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.


If the later parts are in doubt, maybe the first patches fixing
obvious errors could land first separately.


changes in v2:
- add received reviews
- fix commit message typo
- add doc snippet explaining CONFIG_SPL_FIT_GENERATOR_KEY_HINT

Heiko Stuebner (7):
  spl: fit: select SPL_HASH_SUPPORT for SPL_FIT_SIGNATURE
  spl: fit: select SPL_CRYPTO_SUPPORT for SPL_FIT_SIGNATURE
  lib: rsa: distinguish between tpl and spl for CONFIG_RSA_VERIFY
  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                                | 18 +++++++++
 Makefile                               | 11 +++++-
 arch/arm/mach-rockchip/make_fit_atf.py | 51 +++++++++++++++++++++++++-
 doc/uImage.FIT/howto.txt               | 13 +++++++
 lib/rsa/Makefile                       |  2 +-
 tools/image-host.c                     |  2 +-
 6 files changed, 93 insertions(+), 4 deletions(-)

-- 
2.25.1

[PATCH v2 1/7] spl: fit: select SPL_HASH_SUPPORT for SPL_FIT_SIGNATURE

[Heiko Stuebner]

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

rsa-checsum needs support for hash functions or else will run into
compile errors like:
u-boot/lib/rsa/rsa-checksum.c:28: undefined reference to `hash_progressive_lookup_algo'

So similar to the main FIT_SIGNATURE entry selects HASH,
select SPL_HASH_SUPPORT for SPL_FIT_SIGNATURE.

Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
Signed-off-by: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
Reviewed-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
---
 Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Kconfig b/Kconfig
index 1b0b6999d8..2b84402baa 100644
--- a/Kconfig
+++ b/Kconfig
@@ -445,6 +445,7 @@ config SPL_FIT_SIGNATURE
 	bool "Enable signature verification of FIT firmware within SPL"
 	depends on SPL_DM
 	select SPL_FIT
+	select SPL_HASH_SUPPORT
 	select SPL_RSA
 	select SPL_RSA_VERIFY
 	select IMAGE_SIGN_INFO
-- 
2.25.1

[PATCH v2 2/7] spl: fit: select SPL_CRYPTO_SUPPORT for SPL_FIT_SIGNATURE

[Heiko Stuebner]

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

Verifying FIT images obviously needs the rsa parts of crypto
support and while main uboot always compiles crypto support,
it's optional for SPL and we should thus select the necessary
option to not end up in compile errors like:

    u-boot/lib/rsa/rsa-verify.c:328: undefined reference to `rsa_mod_exp'

So select SPL_CRYPTO_SUPPORT in SPL_FIT_SIGNATURE.

Signed-off-by: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
Reviewed-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
---
 Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Kconfig b/Kconfig
index 2b84402baa..4051746319 100644
--- a/Kconfig
+++ b/Kconfig
@@ -445,6 +445,7 @@ config SPL_FIT_SIGNATURE
 	bool "Enable signature verification of FIT firmware within SPL"
 	depends on SPL_DM
 	select SPL_FIT
+	select SPL_CRYPTO_SUPPORT
 	select SPL_HASH_SUPPORT
 	select SPL_RSA
 	select SPL_RSA_VERIFY
-- 
2.25.1

[PATCH v2 3/7] lib: rsa: distinguish between tpl and spl for CONFIG_RSA_VERIFY

[Heiko Stuebner]

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

While the SPL may want to do signature checking this won't be
the case for TPL in all cases, as TPL is mostly used when the
amound of initial memory is not enough for a full SPL.

So on a system where SPL uses DM but TPL does not we currently
end up with a TPL compile error of:

    lib/rsa/rsa-verify.c:48:25: error: dereferencing pointer to incomplete type ?struct checksum_algo?

To prevent that change the $(SPL_) to $(SPL_TPL_) to distinguish
between both. If someone really needs FIT signature checking in
TPL as well, a new TPL_RSA_VERIFY config symbol needs to be added.

Signed-off-by: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
Reviewed-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
---
changes in v2:
- fix typo "distinguis(h)"

 lib/rsa/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/rsa/Makefile b/lib/rsa/Makefile
index 14ed3cb401..c61ebfd79e 100644
--- a/lib/rsa/Makefile
+++ b/lib/rsa/Makefile
@@ -5,6 +5,6 @@
 # (C) Copyright 2000-2007
 # Wolfgang Denk, DENX Software Engineering, wd at denx.de.
 
-obj-$(CONFIG_$(SPL_)RSA_VERIFY) += rsa-verify.o rsa-checksum.o
+obj-$(CONFIG_$(SPL_TPL_)RSA_VERIFY) += rsa-verify.o rsa-checksum.o
 obj-$(CONFIG_RSA_VERIFY_WITH_PKEY) += rsa-keyprop.o
 obj-$(CONFIG_RSA_SOFTWARE_EXP) += rsa-mod-exp.o
-- 
2.25.1

[PATCH v2 4/7] mkimage: fit_image: handle multiple errors when writing signatures

[Heiko Stuebner]

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>
---
 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 4e57ddea96..355ceb4591 100644
--- a/tools/image-host.c
+++ b/tools/image-host.c
@@ -240,7 +240,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

[PATCH v2 5/7] spl: fit: enable signing a generated u-boot.itb

[Heiko Stuebner]

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>
---
 Kconfig  |  8 ++++++++
 Makefile | 11 ++++++++++-
 2 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/Kconfig b/Kconfig
index 4051746319..15a783a67d 100644
--- a/Kconfig
+++ b/Kconfig
@@ -451,6 +451,14 @@ config SPL_FIT_SIGNATURE
 	select SPL_RSA_VERIFY
 	select 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 26307fd4a6..8e7a7cb50e 100644
--- a/Makefile
+++ b/Makefile
@@ -1394,6 +1394,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) \
@@ -1913,7 +1921,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_GENERATOR),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

[PATCH v2 6/7] spl: fit: add Kconfig option to specify key-hint for fit_generator

[Heiko Stuebner]

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>
---
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 15a783a67d..703762d740 100644
--- a/Kconfig
+++ b/Kconfig
@@ -534,6 +534,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

[PATCH v2 7/7] rockchip: make_fit_atf: add signature handling

[Heiko Stuebner]

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>
---
 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

[PATCH v2 6/7] spl: fit: add Kconfig option to specify key-hint for fit_generator

[Simon Glass]

On Mon, 20 Apr 2020 at 18:23, Heiko Stuebner <heiko@sntech.de> wrote:
>
> 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>
> ---
> changes in v2:
> - add doc snippet explaining the option
>
>  Kconfig                  |  8 ++++++++
>  doc/uImage.FIT/howto.txt | 13 +++++++++++++
>  2 files changed, 21 insertions(+)
>

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

[PATCH v2 7/7] rockchip: make_fit_atf: add signature handling

[Simon Glass]

Hi Heiko,

On Mon, 20 Apr 2020 at 18:23, 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>
> ---
>  arch/arm/mach-rockchip/make_fit_atf.py | 51 +++++++++++++++++++++++++-
>  1 file changed, 50 insertions(+), 1 deletion(-)
>

Can this move to binman?

Regards,
Simon

[PATCH v2 1/7] spl: fit: select SPL_HASH_SUPPORT for SPL_FIT_SIGNATURE

[Kever Yang]

On 2020/4/21 ??8:23, Heiko Stuebner wrote:
> From: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
>
> rsa-checsum needs support for hash functions or else will run into
> compile errors like:
> u-boot/lib/rsa/rsa-checksum.c:28: undefined reference to `hash_progressive_lookup_algo'
>
> So similar to the main FIT_SIGNATURE entry selects HASH,
> select SPL_HASH_SUPPORT for SPL_FIT_SIGNATURE.
>
> Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
> Signed-off-by: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
> Reviewed-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>

Reviewed-by: Kever Yang <kever.yang@rock-chips.com>

Thanks,
- Kever
> ---
>   Kconfig | 1 +
>   1 file changed, 1 insertion(+)
>
> diff --git a/Kconfig b/Kconfig
> index 1b0b6999d8..2b84402baa 100644
> --- a/Kconfig
> +++ b/Kconfig
> @@ -445,6 +445,7 @@ config SPL_FIT_SIGNATURE
>   	bool "Enable signature verification of FIT firmware within SPL"
>   	depends on SPL_DM
>   	select SPL_FIT
> +	select SPL_HASH_SUPPORT
>   	select SPL_RSA
>   	select SPL_RSA_VERIFY
>   	select IMAGE_SIGN_INFO

[PATCH v2 2/7] spl: fit: select SPL_CRYPTO_SUPPORT for SPL_FIT_SIGNATURE

[Kever Yang]

On 2020/4/21 ??8:23, Heiko Stuebner wrote:
> From: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
>
> Verifying FIT images obviously needs the rsa parts of crypto
> support and while main uboot always compiles crypto support,
> it's optional for SPL and we should thus select the necessary
> option to not end up in compile errors like:
>
>      u-boot/lib/rsa/rsa-verify.c:328: undefined reference to `rsa_mod_exp'
>
> So select SPL_CRYPTO_SUPPORT in SPL_FIT_SIGNATURE.
>
> Signed-off-by: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
> Reviewed-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>

Reviewed-by: Kever Yang <kever.yang@rock-chips.com>

Thanks,
- Kever
> ---
>   Kconfig | 1 +
>   1 file changed, 1 insertion(+)
>
> diff --git a/Kconfig b/Kconfig
> index 2b84402baa..4051746319 100644
> --- a/Kconfig
> +++ b/Kconfig
> @@ -445,6 +445,7 @@ config SPL_FIT_SIGNATURE
>   	bool "Enable signature verification of FIT firmware within SPL"
>   	depends on SPL_DM
>   	select SPL_FIT
> +	select SPL_CRYPTO_SUPPORT
>   	select SPL_HASH_SUPPORT
>   	select SPL_RSA
>   	select SPL_RSA_VERIFY

[PATCH v2 3/7] lib: rsa: distinguish between tpl and spl for CONFIG_RSA_VERIFY

[Kever Yang]

On 2020/4/21 ??8:23, Heiko Stuebner wrote:
> From: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
>
> While the SPL may want to do signature checking this won't be
> the case for TPL in all cases, as TPL is mostly used when the
> amound of initial memory is not enough for a full SPL.
>
> So on a system where SPL uses DM but TPL does not we currently
> end up with a TPL compile error of:
>
>      lib/rsa/rsa-verify.c:48:25: error: dereferencing pointer to incomplete type ?struct checksum_algo?
>
> To prevent that change the $(SPL_) to $(SPL_TPL_) to distinguish
> between both. If someone really needs FIT signature checking in
> TPL as well, a new TPL_RSA_VERIFY config symbol needs to be added.
>
> Signed-off-by: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
> Reviewed-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>

Reviewed-by: Kever Yang <kever.yang@rock-chips.com>

Thanks,
- Kever
> ---
> changes in v2:
> - fix typo "distinguis(h)"
>
>   lib/rsa/Makefile | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/lib/rsa/Makefile b/lib/rsa/Makefile
> index 14ed3cb401..c61ebfd79e 100644
> --- a/lib/rsa/Makefile
> +++ b/lib/rsa/Makefile
> @@ -5,6 +5,6 @@
>   # (C) Copyright 2000-2007
>   # Wolfgang Denk, DENX Software Engineering, wd at denx.de.
>   
> -obj-$(CONFIG_$(SPL_)RSA_VERIFY) += rsa-verify.o rsa-checksum.o
> +obj-$(CONFIG_$(SPL_TPL_)RSA_VERIFY) += rsa-verify.o rsa-checksum.o
>   obj-$(CONFIG_RSA_VERIFY_WITH_PKEY) += rsa-keyprop.o
>   obj-$(CONFIG_RSA_SOFTWARE_EXP) += rsa-mod-exp.o

[PATCH v2 4/7] mkimage: fit_image: handle multiple errors when writing signatures

[Kever Yang]

On 2020/4/21 ??8:23, Heiko Stuebner 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: Kever Yang <kever.yang@rock-chips.com>

Thanks,
- Kever
> Reviewed-by: Simon Glass <sjg@chromium.org>
> ---
>   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 4e57ddea96..355ceb4591 100644
> --- a/tools/image-host.c
> +++ b/tools/image-host.c
> @@ -240,7 +240,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));

[PATCH v2 5/7] spl: fit: enable signing a generated u-boot.itb

[Kever Yang]

On 2020/4/21 ??8:23, Heiko Stuebner 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>

Reviewed-by: Kever Yang <kever.yang@rock-chips.com>

Thanks,
- Kever
> ---
>   Kconfig  |  8 ++++++++
>   Makefile | 11 ++++++++++-
>   2 files changed, 18 insertions(+), 1 deletion(-)
>
> diff --git a/Kconfig b/Kconfig
> index 4051746319..15a783a67d 100644
> --- a/Kconfig
> +++ b/Kconfig
> @@ -451,6 +451,14 @@ config SPL_FIT_SIGNATURE
>   	select SPL_RSA_VERIFY
>   	select 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 26307fd4a6..8e7a7cb50e 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1394,6 +1394,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) \
> @@ -1913,7 +1921,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_GENERATOR),u-boot.itb FORCE)
>   	$(Q)$(MAKE) obj=spl -f $(srctree)/scripts/Makefile.spl all
>   
>   spl/sunxi-spl.bin: spl/u-boot-spl

[PATCH v2 7/7] rockchip: make_fit_atf: add signature handling

[Kever Yang]

On 2020/4/21 ??8:23, Heiko Stuebner 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>

Thanks,
- Kever
> ---
>   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)

[PATCH v2 6/7] spl: fit: add Kconfig option to specify key-hint for fit_generator

[Kever Yang]

On 2020/4/21 ??8:23, Heiko Stuebner wrote:
> 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: Kever Yang <kever.yang@rock-chips.com>

Thanks,
- Kever
> ---
> 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 15a783a67d..703762d740 100644
> --- a/Kconfig
> +++ b/Kconfig
> @@ -534,6 +534,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
>   -----------------------------------------------
>   

[PATCH v2 5/7] spl: fit: enable signing a generated u-boot.itb

[Kever Yang]

Heiko,

This patch will cause build fail on sandbox_spl_defconfig:

dtc: option requires an argument -- 'p'


Thanks,

- Kever

On 2020/4/21 ??8:23, Heiko Stuebner 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>
> ---
>   Kconfig  |  8 ++++++++
>   Makefile | 11 ++++++++++-
>   2 files changed, 18 insertions(+), 1 deletion(-)
>
> diff --git a/Kconfig b/Kconfig
> index 4051746319..15a783a67d 100644
> --- a/Kconfig
> +++ b/Kconfig
> @@ -451,6 +451,14 @@ config SPL_FIT_SIGNATURE
>   	select SPL_RSA_VERIFY
>   	select 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 26307fd4a6..8e7a7cb50e 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1394,6 +1394,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) \
> @@ -1913,7 +1921,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_GENERATOR),u-boot.itb FORCE)
>   	$(Q)$(MAKE) obj=spl -f $(srctree)/scripts/Makefile.spl all
>   
>   spl/sunxi-spl.bin: spl/u-boot-spl

[PATCH v2 5/7] spl: fit: enable signing a generated u-boot.itb

[Heiko Stübner]

Hi Kever,

Am Donnerstag, 30. April 2020, 11:03:38 CEST schrieb Kever Yang:
> This patch will cause build fail on sandbox_spl_defconfig:
> 
> dtc: option requires an argument -- 'p'

sandbox_spl is confusing on first glance, it enables the spl_fit-options
but does not define any fit sources.

But I also found a general issue with my code below, and by fixing that
one sandbox_spl also gets happy again.

> On 2020/4/21 ??8:23, Heiko Stuebner 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>
> > ---
> >   Kconfig  |  8 ++++++++
> >   Makefile | 11 ++++++++++-
> >   2 files changed, 18 insertions(+), 1 deletion(-)
> >
> > diff --git a/Kconfig b/Kconfig
> > index 4051746319..15a783a67d 100644
> > --- a/Kconfig
> > +++ b/Kconfig
> > @@ -451,6 +451,14 @@ config SPL_FIT_SIGNATURE
> >   	select SPL_RSA_VERIFY
> >   	select 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 26307fd4a6..8e7a7cb50e 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -1394,6 +1394,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) \
> > @@ -1913,7 +1921,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_GENERATOR),u-boot.itb FORCE)

I now realized that this is the wrong check ... i.e. it only checks for
SPL_FIT_GENERATOR but that is a string so always defined if SPL_LOAD_FIT
is enabled ... also this doesn't take into account SPL_FIT_SOURCE, so the
way to go seems to be to check against $U_BOOT_ITS and
CONFIG_SPL_FIT_SIGNATZRE instead which gets defined if a suitable fit
source is available.


Background for this dependency is that the signature must be done before
the spl-binary gets build, because mkimage for the .itb needs to write the
key to the spl dtb.


I'll send an updated patch as a reply to this mail.


Heiko

[PATCH v2.1 5/7] spl: fit: enable signing a generated u-boot.itb

[Heiko Stuebner]

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 b868e6a965..d479762af6 100644
--- a/Kconfig
+++ b/Kconfig
@@ -451,6 +451,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 6bb9cf55f2..7866d298b6 100644
--- a/Makefile
+++ b/Makefile
@@ -1411,6 +1411,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) \
@@ -1930,7 +1938,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

[PATCH v2 7/7] rockchip: make_fit_atf: add signature handling

[Kever Yang]

On 2020/4/21 ??8:23, Heiko Stuebner 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>
> ---
>   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
>   

+Traceback (most recent call last):
1395 
<https://gitlab.denx.de/u-boot/custodians/u-boot-rockchip/-/jobs/86952#L1395>+ 
File "arch/arm/mach-rockchip/make_fit_atf.py", line 17, in <module>
1396 
<https://gitlab.denx.de/u-boot/custodians/u-boot-rockchip/-/jobs/86952#L1396>+ 
import Crypto
1397 
<https://gitlab.denx.de/u-boot/custodians/u-boot-rockchip/-/jobs/86952#L1397>+ModuleNotFoundError: 
No module named 'Crypto'


Please help to update .gitlab-ci.yml, or else it will report the error.


Thanks,

- Kever

>   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)

[PATCH v2 7/7] rockchip: make_fit_atf: add signature handling

[Heiko Stübner]

Hi Kever,

Am Freitag, 1. Mai 2020, 12:32:23 CEST schrieb Kever Yang:
> On 2020/4/21 ??8:23, Heiko Stuebner 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>
> > ---
> >   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
> >   
> 
> +Traceback (most recent call last):
> 1395 
> <https://gitlab.denx.de/u-boot/custodians/u-boot-rockchip/-/jobs/86952#L1395>+ 
> File "arch/arm/mach-rockchip/make_fit_atf.py", line 17, in <module>
> 1396 
> <https://gitlab.denx.de/u-boot/custodians/u-boot-rockchip/-/jobs/86952#L1396>+ 
> import Crypto
> 1397 
> <https://gitlab.denx.de/u-boot/custodians/u-boot-rockchip/-/jobs/86952#L1397>+ModuleNotFoundError: 
> No module named 'Crypto'
> 
> 
> Please help to update .gitlab-ci.yml, or else it will report the error.

The ci stuff probably needs to install pycrypto from pip (or python-crypto
when using a .deb), but I have no clue how this works or how to test any
changes to that locally.

But I guess something like below might do the trick?

Heiko


diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index beaf9b9042..863c3dea51 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -68,6 +68,7 @@ build all 64bit ARM platforms:
     - virtualenv -p /usr/bin/python3 /tmp/venv
     - . /tmp/venv/bin/activate
     - pip install pyelftools
+    - pip install pycrypto
     - ret=0;
       ./tools/buildman/buildman -o /tmp -P -E -W aarch64 || ret=$?;
       if [[ $ret -ne 0 ]]; then

[PATCH v2 7/7] rockchip: make_fit_atf: add signature handling

[Heiko Stübner]

Hi Kever,

Am Freitag, 1. Mai 2020, 12:32:23 CEST schrieb Kever Yang:
> 
> On 2020/4/21 ??8:23, Heiko Stuebner 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>
> > ---
> >   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
> >   
> 
> +Traceback (most recent call last):
> 1395 
> <https://gitlab.denx.de/u-boot/custodians/u-boot-rockchip/-/jobs/86952#L1395>+ 
> File "arch/arm/mach-rockchip/make_fit_atf.py", line 17, in <module>
> 1396 
> <https://gitlab.denx.de/u-boot/custodians/u-boot-rockchip/-/jobs/86952#L1396>+ 
> import Crypto
> 1397 
> <https://gitlab.denx.de/u-boot/custodians/u-boot-rockchip/-/jobs/86952#L1397>+ModuleNotFoundError: 
> No module named 'Crypto'
> 
> 
> Please help to update .gitlab-ci.yml, or else it will report the error.

I'm not sure, how ... i.e. the missing package is 
"pycrypto" (or "python-crypto" when installing from a distribution package)

So I guess it's about adding that dependency to both .travis.yml and
.gitlab-ci.yml, but is it enough to just do a


diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index beaf9b9042..863c3dea51 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -68,6 +68,7 @@ build all 64bit ARM platforms:
     - virtualenv -p /usr/bin/python3 /tmp/venv
     - . /tmp/venv/bin/activate
     - pip install pyelftools
+    - pip install pycrypto
     - ret=0;
       ./tools/buildman/buildman -o /tmp -P -E -W aarch64 || ret=$?;
       if [[ $ret -ne 0 ]]; then