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

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

[Heiko Stuebner]

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

Simon asked for fit_image_write_sig() to always return an errno code,
never an FDT code and suggested that this could be a follow-on patch.
So I've kept code that way and will provide a follow up series
to convert the return code handling.


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


changes in v4:
- add patch to fix the always defined U_BOOT_ITS in Makefile
- adapt Rockchip make_fit_atf to both python2+3 caused by the
  different crypto-implementations
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 (6):
  imx: mkimage_fit_atf: Fix FIT image if BL31.bin missing
  mkimage: fit_image: handle multiple errors when writing signatures
  spl: fit: dont set U_BOOT_ITS var if not build SPL_FIT support
  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                               | 13 +++++-
 arch/arm/mach-imx/mkimage_fit_atf.sh   |  4 +-
 arch/arm/mach-rockchip/make_fit_atf.py | 57 +++++++++++++++++++++++++-
 doc/uImage.FIT/howto.txt               | 13 ++++++
 tools/image-host.c                     |  2 +-
 6 files changed, 100 insertions(+), 5 deletions(-)

-- 
2.26.2

[PATCH v4 1/6] imx: mkimage_fit_atf: Fix FIT image if BL31.bin missing

[Heiko Stuebner]

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

Signed-off-by: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
Reviewed-by: Peng Fan <peng.fan@nxp.com>
---
 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 fe12b7bb4b..2f77c6d70d 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.26.2

[PATCH v4 2/6] 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 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.26.2

[PATCH v4 3/6] spl: fit: dont set U_BOOT_ITS var if not build SPL_FIT support

[Heiko Stuebner]

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

Both CONFIG_SPL_FIT_SOURCE and CONFIG_SPL_FIT_GENERATOR
depend on CONFIG_SPL_FIT, so U_BOOT_ITS should only be
defined if one of them is set.

When undefined, the
    ifneq ($(CONFIG_SPL_FIT_SOURCE),"")
seems to evaluate to true all the time though, setting
U_BOOT_ITS to u-boot.its, even if no FIT support gets build.

This may prove cumbersome if later parts want to check against
"do we need an U_BOOT_ITS", so to fix that just wrap the whole
block in a "ifdef CONFIG_SPL_FIT" which needs to be defined
if any SPL_FIT_* options get used.

Signed-off-by: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
---
 Makefile | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Makefile b/Makefile
index 149f839948..1cc7e37c37 100644
--- a/Makefile
+++ b/Makefile
@@ -1320,6 +1320,7 @@ endif
 
 # Boards with more complex image requirements can provide an .its source file
 # or a generator script
+ifdef CONFIG_SPL_FIT
 ifneq ($(CONFIG_SPL_FIT_SOURCE),"")
 U_BOOT_ITS := u-boot.its
 $(U_BOOT_ITS): $(subst ",,$(CONFIG_SPL_FIT_SOURCE))
@@ -1338,6 +1339,7 @@ $(U_BOOT_ITS): $(U_BOOT_ITS_DEPS) FORCE
 	$(patsubst %,arch/$(ARCH)/dts/%.dtb,$(subst ",,$(CONFIG_OF_LIST))) > $@
 endif
 endif
+endif
 
 ifdef CONFIG_SPL_LOAD_FIT
 MKIMAGEFLAGS_u-boot.img = -f auto -A $(ARCH) -T firmware -C none -O u-boot \
-- 
2.26.2

[PATCH v4 4/6] 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>
Reviewed-by: Simon Glass <sjg@chromium.org>
---
 Kconfig  |  8 ++++++++
 Makefile | 11 ++++++++++-
 2 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/Kconfig b/Kconfig
index b34fbf5827..05a13d1836 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 1cc7e37c37..d19f556a04 100644
--- a/Makefile
+++ b/Makefile
@@ -1409,6 +1409,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) \
@@ -1924,7 +1932,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.26.2

[PATCH v4 5/6] 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>
---
 Kconfig                  |  8 ++++++++
 doc/uImage.FIT/howto.txt | 13 +++++++++++++
 2 files changed, 21 insertions(+)

diff --git a/Kconfig b/Kconfig
index 05a13d1836..de144ae430 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.26.2

[PATCH v4 6/6] 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 | 57 +++++++++++++++++++++++++-
 1 file changed, 56 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..de7dc19d11 100755
--- a/arch/arm/mach-rockchip/make_fit_atf.py
+++ b/arch/arm/mach-rockchip/make_fit_atf.py
@@ -14,6 +14,14 @@ import sys
 import getopt
 import logging
 import struct
+try:
+	# in python3 Cryptodome succeeds Crypto
+	import Cryptodome
+	from Cryptodome.PublicKey import RSA
+except:
+	import Crypto
+	from Crypto.PublicKey import RSA
+
 
 DT_HEADER = """
 /*
@@ -37,7 +45,9 @@ DT_UBOOT = """
 			arch = "arm64";
 			compression = "none";
 			load = <0x%08x>;
-		};
+"""
+
+DT_UBOOT_NODE_END = """		};
 
 """
 
@@ -47,6 +57,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 +110,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 +126,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 +140,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 +182,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.26.2

[PATCH v4 6/6] rockchip: make_fit_atf: add signature handling

[Simon Glass]

Hi Heiko,

On Fri, 19 Jun 2020 at 04:45, 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 | 57 +++++++++++++++++++++++++-
>  1 file changed, 56 insertions(+), 1 deletion(-)

We have to stop the growth of these ad-hoc scripts or U-Boot with
become a tangled mess of vendor-specific scripts.

I sent a series to convert more of rockchip to use binman. Can you
please take a look at that?

It deletes the fit_spl_optee.sh script. We need to delete make_fit_atf as well.

Regards,
Simon

Re: [PATCH v4 6/6] rockchip: make_fit_atf: add signature handling【请注意，邮件由sjg@google.com代发】

[Kever Yang]

Hi Simon,

On 2020/6/26 ??9:12, Simon Glass wrote:
> Hi Heiko,
>
> On Fri, 19 Jun 2020 at 04:45, 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 | 57 +++++++++++++++++++++++++-
>>   1 file changed, 56 insertions(+), 1 deletion(-)
> We have to stop the growth of these ad-hoc scripts or U-Boot with
> become a tangled mess of vendor-specific scripts.
>
> I sent a series to convert more of rockchip to use binman. Can you
> please take a look at that?
>
> It deletes the fit_spl_optee.sh script. We need to delete make_fit_atf as well.

The fit_spl_optee.sh is much more simple because it use optee.bin 
directly for u-boot.itb, while make_fit_atf

needs to parse bl31.elf and get the start address for different blob. 
Any suggestion on how this

could be implement in binman?


Thanks,

- Kever

>
> Regards,
> Simon
>
>

Re: [PATCH v4 6/6] rockchip: make_fit_atf: add signature handling【请注意，邮件由sjg@google.com代发】

[Simon Glass]

Hi Kever,

On Tue, 7 Jul 2020 at 06:01, Kever Yang <kever.yang@rock-chips.com> wrote:
>
> Hi Simon,
>
> On 2020/6/26 ??9:12, Simon Glass wrote:
> > Hi Heiko,
> >
> > On Fri, 19 Jun 2020 at 04:45, 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 | 57 +++++++++++++++++++++++++-
> >>   1 file changed, 56 insertions(+), 1 deletion(-)
> > We have to stop the growth of these ad-hoc scripts or U-Boot with
> > become a tangled mess of vendor-specific scripts.
> >
> > I sent a series to convert more of rockchip to use binman. Can you
> > please take a look at that?
> >
> > It deletes the fit_spl_optee.sh script. We need to delete make_fit_atf as well.
>
> The fit_spl_optee.sh is much more simple because it use optee.bin
> directly for u-boot.itb, while make_fit_atf
>
> needs to parse bl31.elf and get the start address for different blob.
> Any suggestion on how this
>
> could be implement in binman?

See for example elf.GetSymbolAddress() which reads an ELF symbol
value. This is used in the u-boot-spl-bss-pad entry. It should be
pretty simple.

You can create new entry types (in the etypes/) dir to do all sorts of
things fairly easily.

Regards,
Simon

[PATCH v4 6/6] rockchip: make_fit_atf: add signature handling

[Tom Rini]

On Fri, Jun 19, 2020 at 12:45:50PM +0200, 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>

First, I want to echo what Simon said.  We need to move towards having
less ad-hoc scripts for these kind of final modifiers.

> ---
>  arch/arm/mach-rockchip/make_fit_atf.py | 57 +++++++++++++++++++++++++-
>  1 file changed, 56 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..de7dc19d11 100755
> --- a/arch/arm/mach-rockchip/make_fit_atf.py
> +++ b/arch/arm/mach-rockchip/make_fit_atf.py
> @@ -14,6 +14,14 @@ import sys
>  import getopt
>  import logging
>  import struct
> +try:
> +	# in python3 Cryptodome succeeds Crypto
> +	import Cryptodome
> +	from Cryptodome.PublicKey import RSA
> +except:
> +	import Crypto
> +	from Crypto.PublicKey import RSA

Is it that older python3 would support "Cryto" not "Cryptodome"  or
python2?  If the latter, we should just drop it.  We do however need to
document, and should try and be user friendly about catching the
failure, that we now need the pycrypto module installed.  Thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200630/a6d2003a/attachment.sig>

[PATCH v4 6/6] rockchip: make_fit_atf: add signature handling

[Heiko Stübner]

Hi Tom,

Am Dienstag, 30. Juni 2020, 14:36:40 CEST schrieb Tom Rini:
> On Fri, Jun 19, 2020 at 12:45:50PM +0200, 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>
> 
> First, I want to echo what Simon said.  We need to move towards having
> less ad-hoc scripts for these kind of final modifiers.

looking at Simon's binman series is on my todo list, so yes I do agree
with you :-) .

> > ---
> >  arch/arm/mach-rockchip/make_fit_atf.py | 57 +++++++++++++++++++++++++-
> >  1 file changed, 56 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..de7dc19d11 100755
> > --- a/arch/arm/mach-rockchip/make_fit_atf.py
> > +++ b/arch/arm/mach-rockchip/make_fit_atf.py
> > @@ -14,6 +14,14 @@ import sys
> >  import getopt
> >  import logging
> >  import struct
> > +try:
> > +	# in python3 Cryptodome succeeds Crypto
> > +	import Cryptodome
> > +	from Cryptodome.PublicKey import RSA
> > +except:
> > +	import Crypto
> > +	from Crypto.PublicKey import RSA
> 
> Is it that older python3 would support "Cryto" not "Cryptodome"  or
> python2?  If the latter, we should just drop it.  We do however need to
> document, and should try and be user friendly about catching the
> failure, that we now need the pycrypto module installed.  Thanks!

python3 only seems to have Cryptodome, while python2 only seems to
have Crypto. And with for example Debian's default python is still being
python2 for a bit longer, I wanted to support both, especially as it it
works like a drop-in replacement.

Heiko

[PATCH v4 6/6] rockchip: make_fit_atf: add signature handling

[Tom Rini]

On Tue, Jun 30, 2020 at 02:46:51PM +0200, Heiko St?bner wrote:
> Hi Tom,
> 
> Am Dienstag, 30. Juni 2020, 14:36:40 CEST schrieb Tom Rini:
> > On Fri, Jun 19, 2020 at 12:45:50PM +0200, 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>
> > 
> > First, I want to echo what Simon said.  We need to move towards having
> > less ad-hoc scripts for these kind of final modifiers.
> 
> looking at Simon's binman series is on my todo list, so yes I do agree
> with you :-) .

OK, thanks.

> > > ---
> > >  arch/arm/mach-rockchip/make_fit_atf.py | 57 +++++++++++++++++++++++++-
> > >  1 file changed, 56 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..de7dc19d11 100755
> > > --- a/arch/arm/mach-rockchip/make_fit_atf.py
> > > +++ b/arch/arm/mach-rockchip/make_fit_atf.py
> > > @@ -14,6 +14,14 @@ import sys
> > >  import getopt
> > >  import logging
> > >  import struct
> > > +try:
> > > +	# in python3 Cryptodome succeeds Crypto
> > > +	import Cryptodome
> > > +	from Cryptodome.PublicKey import RSA
> > > +except:
> > > +	import Crypto
> > > +	from Crypto.PublicKey import RSA
> > 
> > Is it that older python3 would support "Cryto" not "Cryptodome"  or
> > python2?  If the latter, we should just drop it.  We do however need to
> > document, and should try and be user friendly about catching the
> > failure, that we now need the pycrypto module installed.  Thanks!
> 
> python3 only seems to have Cryptodome, while python2 only seems to
> have Crypto. And with for example Debian's default python is still being
> python2 for a bit longer, I wanted to support both, especially as it it
> works like a drop-in replacement.

I don't _think_ we worry about running on distros where we can't say
/usr/bin/env python3 at this point so I'd rather not add more python2
compatibility code.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200630/8a44357d/attachment.sig>