diff mbox series

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

Message ID 20200421002333.111461-6-heiko@sntech.de
State Superseded
Headers show
Series rockchip: make it possible to sign the u-boot.itb | expand

Commit Message

Heiko Stuebner April 21, 2020, 12:23 a.m. UTC 
From: Heiko Stuebner <heiko.stuebner at 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 at theobroma-systems.com>
Reviewed-by: Philipp Tomsich <philipp.tomsich at theobroma-systems.com>
---
 Kconfig  |  8 ++++++++
 Makefile | 11 ++++++++++-
 2 files changed, 18 insertions(+), 1 deletion(-)

Comments

Kever Yang April 28, 2020, 1:48 p.m. UTC | #1 
On 2020/4/21 ??8:23, Heiko Stuebner wrote:
> From: Heiko Stuebner <heiko.stuebner at 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 at theobroma-systems.com>
> Reviewed-by: Philipp Tomsich <philipp.tomsich at theobroma-systems.com>

Reviewed-by: Kever Yang <kever.yang at 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
Kever Yang April 30, 2020, 9:03 a.m. UTC | #2 
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 at 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 at theobroma-systems.com>
> Reviewed-by: Philipp Tomsich <philipp.tomsich at 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
Heiko Stuebner April 30, 2020, 12:18 p.m. UTC | #3 
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 at 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 at theobroma-systems.com>
> > Reviewed-by: Philipp Tomsich <philipp.tomsich at 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
diff mbox series

Patch

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