All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v6 00/16] image: add a stage pre-load
@ 2022-02-25 14:57 Philippe Reynes
  2022-02-25 14:57 ` [PATCH v6 01/16] arch: Kconfig: imply BINMAN for SANDBOX Philippe Reynes
                   ` (15 more replies)
  0 siblings, 16 replies; 30+ messages in thread
From: Philippe Reynes @ 2022-02-25 14:57 UTC (permalink / raw)
  To: sjg, rasmus.villemoes; +Cc: u-boot, Philippe Reynes

This serie adds a stage pre-load before launching an image.
This stage is used to read a header before the image and
this header contains the signature of the full image.
So u-boot may check the full image before using any
data of the image.

The support of this header is added to binman, and
a command verify checks the signature of a blob and
set the u-boot env variable "loadaddr_verified" to 
the beginning of the "real" image.

The support of this header is only added to binman,
but it may also be added to mkimage. 


Changelog:
v6:
- set values in big endian in the pre-load header
- binman: etypes: pre-load: read image from other entry
                  instead of directly from a file
- binman: etypes: pre-load: add test unit
- lib: Makefile: no longer add -I$(obj) for SPL
       It was to fix build when oid is built on spl but not
       on u-boot. It is not longer possible.
v5:
- replace config SANDBOX_BINMAN by an imply
v4:
- add a config SANDBOX_BIN
- enhance help for asn1 and oid
- change the format of the pre-load header
- add the support of pre-load header in binman
- add py test for pre-load header
- add a command verify
v3:
- move image-pre-load.c to /boot
- update mkimage to add public key in u-boot device tree
- add script gen_pre_load_header.sh
v2:
- move the code to image-pre-load
- add support of stage pre-load for spl
- add support of stage pre-load on spl_ram

Philippe Reynes (16):
  arch: Kconfig: imply BINMAN for SANDBOX
  lib: Kconfig: enhance help for ASN1
  lib: Kconfig: enhance the help of OID_REGISTRY
  lib: allow to build asn1 decoder and oid registry in SPL
  lib: crypto: allow to build crypyo in SPL
  lib: rsa: allow rsa verify with pkey in SPL
  boot: image: add a stage pre-load
  cmd: bootm: add a stage pre-load
  common: spl: fit_ram: allow to use image pre load
  mkimage: add public key for image pre-load stage
  Makefile: provide sah-key to binman
  tools: binman: add support for pre-load header
  configs: sandbox_defconfig: enable stage pre-load in bootm
  test: py: vboot: add test for global image signature
  cmd: verify: initial import
  configs: sandbox_defconfig: enable config CMD_VERIFY

 Makefile                                      |   1 +
 arch/Kconfig                                  |   1 +
 arch/sandbox/dts/sandbox.dtsi                 |   3 +
 arch/sandbox/dts/test.dts                     |   3 +
 boot/Kconfig                                  |  55 +++
 boot/Makefile                                 |   1 +
 boot/bootm.c                                  |  33 ++
 boot/image-pre-load.c                         | 416 ++++++++++++++++++
 cmd/Kconfig                                   |  17 +
 cmd/Makefile                                  |   1 +
 cmd/bootm.c                                   |   2 +-
 cmd/verify.c                                  |  53 +++
 common/spl/spl_ram.c                          |  21 +-
 configs/sandbox_defconfig                     |   4 +
 include/image.h                               |  30 ++
 lib/Kconfig                                   |  37 +-
 lib/Makefile                                  |   7 +-
 lib/crypto/Kconfig                            |  29 ++
 lib/crypto/Makefile                           |  19 +-
 lib/rsa/Kconfig                               |  19 +
 test/py/tests/test_fit.py                     |   3 +
 test/py/tests/test_vboot.py                   | 125 +++++-
 test/py/tests/vboot/sandbox-binman-pss.dts    |  25 ++
 test/py/tests/vboot/sandbox-binman.dts        |  24 +
 .../tests/vboot/sandbox-u-boot-global-pss.dts |  28 ++
 test/py/tests/vboot/sandbox-u-boot-global.dts |  27 ++
 test/py/tests/vboot/sandbox-u-boot.dts        |   3 +
 test/py/tests/vboot/simple-images.its         |  36 ++
 tools/binman/etype/pre_load.py                | 165 +++++++
 tools/binman/ftest.py                         |  45 ++
 tools/binman/test/225_dev.key                 |  28 ++
 tools/binman/test/225_pre_load.dts            |  22 +
 tools/binman/test/226_pre_load_pkcs.dts       |  23 +
 tools/binman/test/227_pre_load_pss.dts        |  23 +
 .../test/228_pre_load_invalid_padding.dts     |  23 +
 .../binman/test/229_pre_load_invalid_sha.dts  |  23 +
 .../binman/test/230_pre_load_invalid_algo.dts |  23 +
 .../binman/test/231_pre_load_invalid_key.dts  |  23 +
 tools/fit_image.c                             |   3 +
 tools/image-host.c                            | 114 +++++
 40 files changed, 1510 insertions(+), 28 deletions(-)
 create mode 100644 boot/image-pre-load.c
 create mode 100644 cmd/verify.c
 create mode 100644 test/py/tests/vboot/sandbox-binman-pss.dts
 create mode 100644 test/py/tests/vboot/sandbox-binman.dts
 create mode 100644 test/py/tests/vboot/sandbox-u-boot-global-pss.dts
 create mode 100644 test/py/tests/vboot/sandbox-u-boot-global.dts
 create mode 100644 test/py/tests/vboot/simple-images.its
 create mode 100644 tools/binman/etype/pre_load.py
 create mode 100644 tools/binman/test/225_dev.key
 create mode 100644 tools/binman/test/225_pre_load.dts
 create mode 100644 tools/binman/test/226_pre_load_pkcs.dts
 create mode 100644 tools/binman/test/227_pre_load_pss.dts
 create mode 100644 tools/binman/test/228_pre_load_invalid_padding.dts
 create mode 100644 tools/binman/test/229_pre_load_invalid_sha.dts
 create mode 100644 tools/binman/test/230_pre_load_invalid_algo.dts
 create mode 100644 tools/binman/test/231_pre_load_invalid_key.dts

-- 
2.17.1


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

* [PATCH v6 01/16] arch: Kconfig: imply BINMAN for SANDBOX
  2022-02-25 14:57 [PATCH v6 00/16] image: add a stage pre-load Philippe Reynes
@ 2022-02-25 14:57 ` Philippe Reynes
  2022-02-25 14:57 ` [PATCH v6 02/16] lib: Kconfig: enhance help for ASN1 Philippe Reynes
                   ` (14 subsequent siblings)
  15 siblings, 0 replies; 30+ messages in thread
From: Philippe Reynes @ 2022-02-25 14:57 UTC (permalink / raw)
  To: sjg, rasmus.villemoes; +Cc: u-boot, Philippe Reynes

To be able to use the tool binman on sandbox,
the config SANDBOX should imply BINMAN.

Reviewed-by: Simon Glass <sjg@chromium.org>
Signed-off-by: Philippe Reynes <philippe.reynes@softathome.com>
---
 arch/Kconfig                           | 1 +
 arch/sandbox/dts/sandbox.dtsi          | 3 +++
 arch/sandbox/dts/test.dts              | 3 +++
 test/py/tests/test_fit.py              | 3 +++
 test/py/tests/vboot/sandbox-u-boot.dts | 3 +++
 5 files changed, 13 insertions(+)

diff --git a/arch/Kconfig b/arch/Kconfig
index e6191446a3..35624377ca 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -203,6 +203,7 @@ config SANDBOX
 	imply KEYBOARD
 	imply PHYSMEM
 	imply GENERATE_ACPI_TABLE
+	imply BINMAN
 
 config SH
 	bool "SuperH architecture"
diff --git a/arch/sandbox/dts/sandbox.dtsi b/arch/sandbox/dts/sandbox.dtsi
index 66b813faad..826db26fc2 100644
--- a/arch/sandbox/dts/sandbox.dtsi
+++ b/arch/sandbox/dts/sandbox.dtsi
@@ -7,6 +7,9 @@
 #define USB_CLASS_HUB			9
 
 / {
+	binman {
+	};
+
 	chosen {
 		stdout-path = "/serial";
 	};
diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts
index 48ca3e1e47..c11ad8cb9f 100644
--- a/arch/sandbox/dts/test.dts
+++ b/arch/sandbox/dts/test.dts
@@ -61,6 +61,9 @@
 		osd0 = "/osd";
 	};
 
+	binman {
+	};
+
 	config {
 		testing-bool;
 		testing-int = <123>;
diff --git a/test/py/tests/test_fit.py b/test/py/tests/test_fit.py
index 6d5b43c3ba..5856960be2 100755
--- a/test/py/tests/test_fit.py
+++ b/test/py/tests/test_fit.py
@@ -89,6 +89,9 @@ base_fdt = '''
 	model = "Sandbox Verified Boot Test";
 	compatible = "sandbox";
 
+	binman {
+	};
+
 	reset@0 {
 		compatible = "sandbox,reset";
 		reg = <0>;
diff --git a/test/py/tests/vboot/sandbox-u-boot.dts b/test/py/tests/vboot/sandbox-u-boot.dts
index 63f8f401de..5809c62fc1 100644
--- a/test/py/tests/vboot/sandbox-u-boot.dts
+++ b/test/py/tests/vboot/sandbox-u-boot.dts
@@ -4,6 +4,9 @@
 	model = "Sandbox Verified Boot Test";
 	compatible = "sandbox";
 
+	binman {
+	};
+
 	reset@0 {
 		compatible = "sandbox,reset";
 	};
-- 
2.17.1


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

* [PATCH v6 02/16] lib: Kconfig: enhance help for ASN1
  2022-02-25 14:57 [PATCH v6 00/16] image: add a stage pre-load Philippe Reynes
  2022-02-25 14:57 ` [PATCH v6 01/16] arch: Kconfig: imply BINMAN for SANDBOX Philippe Reynes
@ 2022-02-25 14:57 ` Philippe Reynes
  2022-02-25 14:57 ` [PATCH v6 03/16] lib: Kconfig: enhance the help of OID_REGISTRY Philippe Reynes
                   ` (13 subsequent siblings)
  15 siblings, 0 replies; 30+ messages in thread
From: Philippe Reynes @ 2022-02-25 14:57 UTC (permalink / raw)
  To: sjg, rasmus.villemoes; +Cc: u-boot, Philippe Reynes

Enhance the help for configs ASN1_COMPILER
and ASN1_decoder.

Reviewed-by: Simon Glass <sjg@chromium.org>
Signed-off-by: Philippe Reynes <philippe.reynes@softathome.com>
---
 lib/Kconfig | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/lib/Kconfig b/lib/Kconfig
index 3c6fa99b1a..b0e5d60b3d 100644
--- a/lib/Kconfig
+++ b/lib/Kconfig
@@ -791,11 +791,23 @@ endmenu
 
 config ASN1_COMPILER
 	bool
+	help
+	  ASN.1 (Abstract Syntax Notation One) is a standard interface
+	  description language for defining data structures that can be
+	  serialized and deserialized in a cross-platform way. It is
+	  broadly used in telecommunications and computer networking,
+	  and especially in cryptography (https://en.wikipedia.org/wiki/ASN.1).
+	  This option enables the support of the asn1 compiler.
 
 config ASN1_DECODER
 	bool
 	help
-	  Enable asn1 decoder library.
+	  ASN.1 (Abstract Syntax Notation One) is a standard interface
+	  description language for defining data structures that can be
+	  serialized and deserialized in a cross-platform way. It is
+	  broadly used in telecommunications and computer networking,
+	  and especially in cryptography (https://en.wikipedia.org/wiki/ASN.1).
+	  This option enables the support of the asn1 decoder.
 
 config OID_REGISTRY
 	bool
-- 
2.17.1


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

* [PATCH v6 03/16] lib: Kconfig: enhance the help of OID_REGISTRY
  2022-02-25 14:57 [PATCH v6 00/16] image: add a stage pre-load Philippe Reynes
  2022-02-25 14:57 ` [PATCH v6 01/16] arch: Kconfig: imply BINMAN for SANDBOX Philippe Reynes
  2022-02-25 14:57 ` [PATCH v6 02/16] lib: Kconfig: enhance help for ASN1 Philippe Reynes
@ 2022-02-25 14:57 ` Philippe Reynes
  2022-02-25 14:57 ` [PATCH v6 04/16] lib: allow to build asn1 decoder and oid registry in SPL Philippe Reynes
                   ` (12 subsequent siblings)
  15 siblings, 0 replies; 30+ messages in thread
From: Philippe Reynes @ 2022-02-25 14:57 UTC (permalink / raw)
  To: sjg, rasmus.villemoes; +Cc: u-boot, Philippe Reynes

Enhance the help for the config OID_REGISTRY.

Reviewed-by: Simon Glass <sjg@chromium.org>
Signed-off-by: Philippe Reynes <philippe.reynes@softathome.com>
---
 lib/Kconfig | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/lib/Kconfig b/lib/Kconfig
index b0e5d60b3d..e749826f22 100644
--- a/lib/Kconfig
+++ b/lib/Kconfig
@@ -812,6 +812,10 @@ config ASN1_DECODER
 config OID_REGISTRY
 	bool
 	help
+	  In computing, object identifiers or OIDs are an identifier mechanism
+	  standardized by the International Telecommunication Union (ITU) and
+	  ISO/IEC for naming any object, concept, or "thing" with a globally
+	  unambiguous persistent name (https://en.wikipedia.org/wiki/Object_identifier).
 	  Enable fast lookup object identifier registry.
 
 config SMBIOS_PARSER
-- 
2.17.1


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

* [PATCH v6 04/16] lib: allow to build asn1 decoder and oid registry in SPL
  2022-02-25 14:57 [PATCH v6 00/16] image: add a stage pre-load Philippe Reynes
                   ` (2 preceding siblings ...)
  2022-02-25 14:57 ` [PATCH v6 03/16] lib: Kconfig: enhance the help of OID_REGISTRY Philippe Reynes
@ 2022-02-25 14:57 ` Philippe Reynes
  2022-03-03  3:37   ` Simon Glass
  2022-02-25 14:57 ` [PATCH v6 05/16] lib: crypto: allow to build crypyo " Philippe Reynes
                   ` (11 subsequent siblings)
  15 siblings, 1 reply; 30+ messages in thread
From: Philippe Reynes @ 2022-02-25 14:57 UTC (permalink / raw)
  To: sjg, rasmus.villemoes; +Cc: u-boot, Philippe Reynes

This commit adds the options:
- SPL_ASN1_DECODER
- SPL_OID_REGISTRY

Signed-off-by: Philippe Reynes <philippe.reynes@softathome.com>
---
 lib/Kconfig  | 19 +++++++++++++++++++
 lib/Makefile |  4 ++--
 2 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/lib/Kconfig b/lib/Kconfig
index e749826f22..effe735365 100644
--- a/lib/Kconfig
+++ b/lib/Kconfig
@@ -809,6 +809,16 @@ config ASN1_DECODER
 	  and especially in cryptography (https://en.wikipedia.org/wiki/ASN.1).
 	  This option enables the support of the asn1 decoder.
 
+config SPL_ASN1_DECODER
+	bool
+	help
+	  ASN.1 (Abstract Syntax Notation One) is a standard interface
+	  description language for defining data structures that can be
+	  serialized and deserialized in a cross-platform way. It is
+	  broadly used in telecommunications and computer networking,
+	  and especially in cryptography (https://en.wikipedia.org/wiki/ASN.1).
+	  This option enables the support of the asn1 decoder in the SPL.
+
 config OID_REGISTRY
 	bool
 	help
@@ -818,6 +828,15 @@ config OID_REGISTRY
 	  unambiguous persistent name (https://en.wikipedia.org/wiki/Object_identifier).
 	  Enable fast lookup object identifier registry.
 
+config SPL_OID_REGISTRY
+	bool
+	help
+	  In computing, object identifiers or OIDs are an identifier mechanism
+	  standardized by the International Telecommunication Union (ITU) and
+	  ISO/IEC for naming any object, concept, or "thing" with a globally
+	  unambiguous persistent name (https://en.wikipedia.org/wiki/Object_identifier).
+	  Enable fast lookup object identifier registry in the SPL.
+
 config SMBIOS_PARSER
 	bool "SMBIOS parser"
 	help
diff --git a/lib/Makefile b/lib/Makefile
index 11b03d1cbe..13e5d8f7a6 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -17,7 +17,6 @@ obj-$(CONFIG_OF_LIVE) += of_live.o
 obj-$(CONFIG_CMD_DHRYSTONE) += dhry/
 obj-$(CONFIG_ARCH_AT91) += at91/
 obj-$(CONFIG_OPTEE_LIB) += optee/
-obj-$(CONFIG_ASN1_DECODER) += asn1_decoder.o
 obj-y += crypto/
 
 obj-$(CONFIG_AES) += aes.o
@@ -74,6 +73,7 @@ obj-$(CONFIG_SHA1) += sha1.o
 obj-$(CONFIG_SHA256) += sha256.o
 obj-$(CONFIG_SHA512) += sha512.o
 obj-$(CONFIG_CRYPT_PW) += crypt/
+obj-$(CONFIG_$(SPL_)ASN1_DECODER) += asn1_decoder.o
 
 obj-$(CONFIG_$(SPL_)ZLIB) += zlib/
 obj-$(CONFIG_$(SPL_)ZSTD) += zstd/
@@ -135,9 +135,9 @@ obj-$(CONFIG_$(SPL_TPL_)STRTO) += strto.o
 else
 # Main U-Boot always uses the full printf support
 obj-y += vsprintf.o strto.o
-obj-$(CONFIG_OID_REGISTRY) += oid_registry.o
 obj-$(CONFIG_SSCANF) += sscanf.o
 endif
+obj-$(CONFIG_$(SPL_)OID_REGISTRY) += oid_registry.o
 
 obj-y += abuf.o
 obj-y += date.o
-- 
2.17.1


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

* [PATCH v6 05/16] lib: crypto: allow to build crypyo in SPL
  2022-02-25 14:57 [PATCH v6 00/16] image: add a stage pre-load Philippe Reynes
                   ` (3 preceding siblings ...)
  2022-02-25 14:57 ` [PATCH v6 04/16] lib: allow to build asn1 decoder and oid registry in SPL Philippe Reynes
@ 2022-02-25 14:57 ` Philippe Reynes
  2022-02-25 14:57 ` [PATCH v6 06/16] lib: rsa: allow rsa verify with pkey " Philippe Reynes
                   ` (10 subsequent siblings)
  15 siblings, 0 replies; 30+ messages in thread
From: Philippe Reynes @ 2022-02-25 14:57 UTC (permalink / raw)
  To: sjg, rasmus.villemoes; +Cc: u-boot, Philippe Reynes

This commit adds the options:
- SPL_ASYMMETRIC_KEY_TYPE
- SPL_ASYMMETRIC_PUBLIC_KEY_SUBTYPE
- SPL_RSA_PUBLIC_KEY_PARSER

Reviewed-by: Simon Glass <sjg@chromium.org>
Signed-off-by: Philippe Reynes <philippe.reynes@softathome.com>
---
 lib/Makefile        |  3 ++-
 lib/crypto/Kconfig  | 29 +++++++++++++++++++++++++++++
 lib/crypto/Makefile | 19 +++++++++++++------
 3 files changed, 44 insertions(+), 7 deletions(-)

diff --git a/lib/Makefile b/lib/Makefile
index 13e5d8f7a6..13fe5fb7a4 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -17,7 +17,6 @@ obj-$(CONFIG_OF_LIVE) += of_live.o
 obj-$(CONFIG_CMD_DHRYSTONE) += dhry/
 obj-$(CONFIG_ARCH_AT91) += at91/
 obj-$(CONFIG_OPTEE_LIB) += optee/
-obj-y += crypto/
 
 obj-$(CONFIG_AES) += aes.o
 obj-$(CONFIG_AES) += aes/
@@ -63,6 +62,8 @@ obj-$(CONFIG_TPM_V1) += tpm-v1.o
 obj-$(CONFIG_TPM_V2) += tpm-v2.o
 endif
 
+obj-y += crypto/
+
 obj-$(CONFIG_$(SPL_TPL_)GENERATE_ACPI_TABLE) += acpi/
 obj-$(CONFIG_$(SPL_)MD5) += md5.o
 obj-$(CONFIG_ECDSA) += ecdsa/
diff --git a/lib/crypto/Kconfig b/lib/crypto/Kconfig
index 6369bafac0..509bc28311 100644
--- a/lib/crypto/Kconfig
+++ b/lib/crypto/Kconfig
@@ -8,6 +8,15 @@ menuconfig ASYMMETRIC_KEY_TYPE
 
 if ASYMMETRIC_KEY_TYPE
 
+config SPL_ASYMMETRIC_KEY_TYPE
+	bool "Asymmetric (public-key cryptographic) key Support within SPL"
+	depends on SPL
+	help
+	  This option provides support for a key type that holds the data for
+	  the asymmetric keys used for public key cryptographic operations such
+	  as encryption, decryption, signature generation and signature
+	  verification in the SPL.
+
 config ASYMMETRIC_PUBLIC_KEY_SUBTYPE
 	bool "Asymmetric public-key crypto algorithm subtype"
 	help
@@ -16,6 +25,15 @@ config ASYMMETRIC_PUBLIC_KEY_SUBTYPE
 	  appropriate hash algorithms (such as SHA-1) must be available.
 	  ENOPKG will be reported if the requisite algorithm is unavailable.
 
+config SPL_ASYMMETRIC_PUBLIC_KEY_SUBTYPE
+	bool "Asymmetric public-key crypto algorithm subtype within SPL"
+	depends on ASYMMETRIC_PUBLIC_KEY_SUBTYPE
+	help
+	  This option provides support for asymmetric public key type handling in the SPL.
+	  If signature generation and/or verification are to be used,
+	  appropriate hash algorithms (such as SHA-1) must be available.
+	  ENOPKG will be reported if the requisite algorithm is unavailable.
+
 config RSA_PUBLIC_KEY_PARSER
 	bool "RSA public key parser"
 	depends on ASYMMETRIC_PUBLIC_KEY_SUBTYPE
@@ -27,6 +45,17 @@ config RSA_PUBLIC_KEY_PARSER
 	  public key data and provides the ability to instantiate a public
 	  key.
 
+config SPL_RSA_PUBLIC_KEY_PARSER
+	bool "RSA public key parser within SPL"
+	depends on ASYMMETRIC_PUBLIC_KEY_SUBTYPE
+	select SPL_ASN1_DECODER
+	select ASN1_COMPILER
+	select SPL_OID_REGISTRY
+	help
+	  This option provides support for parsing a blob containing RSA
+	  public key data and provides the ability to instantiate a public
+	  key in the SPL.
+
 config X509_CERTIFICATE_PARSER
 	bool "X.509 certificate parser"
 	depends on ASYMMETRIC_PUBLIC_KEY_SUBTYPE
diff --git a/lib/crypto/Makefile b/lib/crypto/Makefile
index f3a414525d..6792b1d4f0 100644
--- a/lib/crypto/Makefile
+++ b/lib/crypto/Makefile
@@ -3,27 +3,34 @@
 # Makefile for asymmetric cryptographic keys
 #
 
-obj-$(CONFIG_ASYMMETRIC_KEY_TYPE) += asymmetric_keys.o
+obj-$(CONFIG_$(SPL_)ASYMMETRIC_KEY_TYPE) += asymmetric_keys.o
 
 asymmetric_keys-y := asymmetric_type.o
 
-obj-$(CONFIG_ASYMMETRIC_PUBLIC_KEY_SUBTYPE) += public_key.o
+obj-$(CONFIG_$(SPL_)ASYMMETRIC_PUBLIC_KEY_SUBTYPE) += public_key.o
 
 #
 # RSA public key parser
 #
-obj-$(CONFIG_RSA_PUBLIC_KEY_PARSER) += rsa_public_key.o
+obj-$(CONFIG_$(SPL_)RSA_PUBLIC_KEY_PARSER) += rsa_public_key.o
 rsa_public_key-y := \
 	rsapubkey.asn1.o \
 	rsa_helper.o
 
 $(obj)/rsapubkey.asn1.o: $(obj)/rsapubkey.asn1.c $(obj)/rsapubkey.asn1.h
+ifdef CONFIG_SPL_BUILD
+CFLAGS_rsapubkey.asn1.o += -I$(obj)
+endif
+
 $(obj)/rsa_helper.o: $(obj)/rsapubkey.asn1.h
+ifdef CONFIG_SPL_BUILD
+CFLAGS_rsa_helper.o += -I$(obj)
+endif
 
 #
 # X.509 Certificate handling
 #
-obj-$(CONFIG_X509_CERTIFICATE_PARSER) += x509_key_parser.o
+obj-$(CONFIG_$(SPL_)X509_CERTIFICATE_PARSER) += x509_key_parser.o
 x509_key_parser-y := \
 	x509.asn1.o \
 	x509_akid.asn1.o \
@@ -40,11 +47,11 @@ $(obj)/x509_akid.asn1.o: $(obj)/x509_akid.asn1.c $(obj)/x509_akid.asn1.h
 #
 # PKCS#7 message handling
 #
-obj-$(CONFIG_PKCS7_MESSAGE_PARSER) += pkcs7_message.o
+obj-$(CONFIG_$(SPL_)PKCS7_MESSAGE_PARSER) += pkcs7_message.o
 pkcs7_message-y := \
 	pkcs7.asn1.o \
 	pkcs7_parser.o
-obj-$(CONFIG_PKCS7_VERIFY) += pkcs7_verify.o
+obj-$(CONFIG_$(SPL_)PKCS7_VERIFY) += pkcs7_verify.o
 
 $(obj)/pkcs7_parser.o: $(obj)/pkcs7.asn1.h
 $(obj)/pkcs7.asn1.o: $(obj)/pkcs7.asn1.c $(obj)/pkcs7.asn1.h
-- 
2.17.1


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

* [PATCH v6 06/16] lib: rsa: allow rsa verify with pkey in SPL
  2022-02-25 14:57 [PATCH v6 00/16] image: add a stage pre-load Philippe Reynes
                   ` (4 preceding siblings ...)
  2022-02-25 14:57 ` [PATCH v6 05/16] lib: crypto: allow to build crypyo " Philippe Reynes
@ 2022-02-25 14:57 ` Philippe Reynes
  2022-02-25 14:57 ` [PATCH v6 07/16] boot: image: add a stage pre-load Philippe Reynes
                   ` (9 subsequent siblings)
  15 siblings, 0 replies; 30+ messages in thread
From: Philippe Reynes @ 2022-02-25 14:57 UTC (permalink / raw)
  To: sjg, rasmus.villemoes; +Cc: u-boot, Philippe Reynes

This commit adds the option SPL_RSA_VERIFY_WITH_PKEY.

Reviewed-by: Simon Glass <sjg@chromium.org>
Signed-off-by: Philippe Reynes <philippe.reynes@softathome.com>
---
 lib/rsa/Kconfig | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/lib/rsa/Kconfig b/lib/rsa/Kconfig
index be9775bcce..b773f17c26 100644
--- a/lib/rsa/Kconfig
+++ b/lib/rsa/Kconfig
@@ -47,6 +47,25 @@ config RSA_VERIFY_WITH_PKEY
 	  directly specified in image_sign_info, where all the necessary
 	  key properties will be calculated on the fly in verification code.
 
+config SPL_RSA_VERIFY_WITH_PKEY
+	bool "Execute RSA verification without key parameters from FDT within SPL"
+	depends on SPL
+	select SPL_RSA_VERIFY
+	select SPL_ASYMMETRIC_KEY_TYPE
+	select SPL_ASYMMETRIC_PUBLIC_KEY_SUBTYPE
+	select SPL_RSA_PUBLIC_KEY_PARSER
+	help
+	  The standard RSA-signature verification code (FIT_SIGNATURE) uses
+	  pre-calculated key properties, that are stored in fdt blob, in
+	  decrypting a signature.
+	  This does not suit the use case where there is no way defined to
+	  provide such additional key properties in standardized form,
+	  particularly UEFI secure boot.
+	  This options enables RSA signature verification with a public key
+	  directly specified in image_sign_info, where all the necessary
+	  key properties will be calculated on the fly in verification code
+	  in the SPL.
+
 config RSA_SOFTWARE_EXP
 	bool "Enable driver for RSA Modular Exponentiation in software"
 	depends on DM
-- 
2.17.1


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

* [PATCH v6 07/16] boot: image: add a stage pre-load
  2022-02-25 14:57 [PATCH v6 00/16] image: add a stage pre-load Philippe Reynes
                   ` (5 preceding siblings ...)
  2022-02-25 14:57 ` [PATCH v6 06/16] lib: rsa: allow rsa verify with pkey " Philippe Reynes
@ 2022-02-25 14:57 ` Philippe Reynes
  2022-03-03  3:37   ` Simon Glass
  2022-02-25 14:57 ` [PATCH v6 08/16] cmd: bootm: " Philippe Reynes
                   ` (8 subsequent siblings)
  15 siblings, 1 reply; 30+ messages in thread
From: Philippe Reynes @ 2022-02-25 14:57 UTC (permalink / raw)
  To: sjg, rasmus.villemoes; +Cc: u-boot, Philippe Reynes

Add a stage pre-load that could
check or modify an image.

For the moment, only a header with a signature is
supported. This header has the following format:
- magic : 4 bytes
- version : 4 bytes
- header size : 4 bytes
- image size : 4 bytes
- offset image signature : 4 bytes
- flags : 4 bytes
- reserved0 : 4 bytes
- reserved1 : 4 bytes
- sha256 of the image signature : 32 bytes
- signature of the first 64 bytes : n bytes
- image signature : n bytes
- padding : up to header size

The stage uses a node /image/pre-load/sig to
get some informations:
- algo-name (mandatory) : name of the algo used to sign
- padding-name : name of padding used to sign
- signature-size : size of the signature (in the header)
- mandatory : set to yes if this sig is mandatory
- public-key (madatory) : value of the public key

Before running the image, the stage pre-load checks
the signature provided in the header.

This is an initial support, later we could add the
support of:
- ciphering
- uncompressing
- ...

Signed-off-by: Philippe Reynes <philippe.reynes@softathome.com>
---
 boot/Kconfig          |  55 ++++++
 boot/Makefile         |   1 +
 boot/image-pre-load.c | 416 ++++++++++++++++++++++++++++++++++++++++++
 include/image.h       |  14 ++
 4 files changed, 486 insertions(+)
 create mode 100644 boot/image-pre-load.c

diff --git a/boot/Kconfig b/boot/Kconfig
index b83a4e8400..cb5f48dcf9 100644
--- a/boot/Kconfig
+++ b/boot/Kconfig
@@ -993,6 +993,61 @@ config AUTOBOOT_MENU_SHOW
 
 endmenu
 
+menu "Image support"
+
+config IMAGE_PRE_LOAD
+	bool "Image pre-load support"
+	help
+	  Enable an image pre-load stage in the SPL.
+	  This pre-load stage allows to do some manipulation
+	  or check (for example signature check) on an image
+	  before launching it.
+
+config SPL_IMAGE_PRE_LOAD
+	bool "Image pre-load support within SPL"
+	depends on SPL && IMAGE_PRE_LOAD
+	help
+	  Enable an image pre-load stage in the SPL.
+	  This pre-load stage allows to do some manipulation
+	  or check (for example signature check) on an image
+	  before launching it.
+
+config IMAGE_PRE_LOAD_SIG
+	bool "Image pre-load signature support"
+	depends on IMAGE_PRE_LOAD
+	select FIT_SIGNATURE
+	select RSA
+	select RSA_VERIFY_WITH_PKEY
+	help
+	  Enable signature check support in the pre-load stage.
+	  For this feature a very simple header is added before
+	  the image with few fields:
+	  - a magic
+	  - the image size
+	  - the signature
+	  All other information (header size, type of signature,
+	  ...) are provided in the node /image/pre-load/sig of
+	  u-boot.
+
+config SPL_IMAGE_PRE_LOAD_SIG
+	bool "Image pre-load signature support witin SPL"
+	depends on SPL_IMAGE_PRE_LOAD && IMAGE_PRE_LOAD_SIG
+	select SPL_FIT_SIGNATURE
+	select SPL_RSA
+	select SPL_RSA_VERIFY_WITH_PKEY
+	help
+	  Enable signature check support in the pre-load stage in the SPL.
+	  For this feature a very simple header is added before
+	  the image with few fields:
+	  - a magic
+	  - the image size
+	  - the signature
+	  All other information (header size, type of signature,
+	  ...) are provided in the node /image/pre-load/sig of
+	  u-boot.
+
+endmenu
+
 config USE_BOOTARGS
 	bool "Enable boot arguments"
 	help
diff --git a/boot/Makefile b/boot/Makefile
index 2938c3f145..59752c65ca 100644
--- a/boot/Makefile
+++ b/boot/Makefile
@@ -26,6 +26,7 @@ obj-$(CONFIG_$(SPL_TPL_)OF_LIBFDT) += image-fdt.o
 obj-$(CONFIG_$(SPL_TPL_)FIT_SIGNATURE) += fdt_region.o
 obj-$(CONFIG_$(SPL_TPL_)FIT) += image-fit.o
 obj-$(CONFIG_$(SPL_)MULTI_DTB_FIT) += boot_fit.o common_fit.o
+obj-$(CONFIG_$(SPL_TPL_)IMAGE_PRE_LOAD) += image-pre-load.o
 obj-$(CONFIG_$(SPL_TPL_)IMAGE_SIGN_INFO) += image-sig.o
 obj-$(CONFIG_$(SPL_TPL_)FIT_SIGNATURE) += image-fit-sig.o
 obj-$(CONFIG_$(SPL_TPL_)FIT_CIPHER) += image-cipher.o
diff --git a/boot/image-pre-load.c b/boot/image-pre-load.c
new file mode 100644
index 0000000000..78d89069a9
--- /dev/null
+++ b/boot/image-pre-load.c
@@ -0,0 +1,416 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2021 Philippe Reynes <philippe.reynes@softathome.com>
+ */
+
+#include <common.h>
+#include <asm/global_data.h>
+DECLARE_GLOBAL_DATA_PTR;
+#include <image.h>
+#include <mapmem.h>
+
+#include <u-boot/sha256.h>
+
+#define IMAGE_PRE_LOAD_SIG_MAGIC		0x55425348
+#define IMAGE_PRE_LOAD_SIG_OFFSET_MAGIC		0
+#define IMAGE_PRE_LOAD_SIG_OFFSET_IMG_LEN	4
+#define IMAGE_PRE_LOAD_SIG_OFFSET_SIG		8
+
+#define IMAGE_PRE_LOAD_PATH			"/image/pre-load/sig"
+#define IMAGE_PRE_LOAD_PROP_ALGO_NAME		"algo-name"
+#define IMAGE_PRE_LOAD_PROP_PADDING_NAME	"padding-name"
+#define IMAGE_PRE_LOAD_PROP_SIG_SIZE		"signature-size"
+#define IMAGE_PRE_LOAD_PROP_PUBLIC_KEY		"public-key"
+#define IMAGE_PRE_LOAD_PROP_MANDATORY		"mandatory"
+
+#ifndef CONFIG_SYS_BOOTM_LEN
+/* use 8MByte as default max gunzip size */
+#define CONFIG_SYS_BOOTM_LEN	0x800000
+#endif
+
+/*
+ * Information in the device-tree about the signature in the header
+ */
+struct image_sig_info {
+	char *algo_name;	/* Name of the algo (eg: sha256,rsa2048) */
+	char *padding_name;	/* Name of the padding */
+	u8 *key;		/* Public signature key */
+	int key_len;		/* Length of the public key */
+	u32 sig_size;		/* size of the signature (in the header) */
+	int mandatory;		/* Set if the signature is mandatory */
+
+	struct image_sign_info sig_info; /* Signature info */
+};
+
+/*
+ * Header of the signature header
+ */
+struct sig_header_s {
+	u32 magic;
+	u32 version;
+	u32 header_size;
+	u32 image_size;
+	u32 offset_img_sig;
+	u32 flags;
+	u32 reserved0;
+	u32 reserved1;
+	u8 sha256_img_sig[SHA256_SUM_LEN];
+};
+
+#define SIG_HEADER_LEN			(sizeof(struct sig_header_s))
+
+/*
+ * Offset of the image
+ *
+ * This value is used to skip the header before really launching the image
+ */
+ulong image_load_offset;
+
+/*
+ * This function gathers information about the signature check
+ * that could be done before launching the image.
+ *
+ * return:
+ * < 0 => an error has occurred
+ *   0 => OK
+ *   1 => no setup
+ */
+static int image_pre_load_sig_setup(struct image_sig_info *info)
+{
+	const void *algo_name, *padding_name, *key, *mandatory;
+	const u32 *sig_size;
+	int key_len;
+	int node, ret = 0;
+
+	if (!info) {
+		log_err("ERROR: info is NULL for image pre-load sig check\n");
+		ret = -EINVAL;
+		goto out;
+	}
+
+	memset(info, 0, sizeof(*info));
+
+	node = fdt_path_offset(gd_fdt_blob(), IMAGE_PRE_LOAD_PATH);
+	if (node < 0) {
+		log_info("INFO: no info for image pre-load sig check\n");
+		ret = 1;
+		goto out;
+	}
+
+	algo_name = fdt_getprop(gd_fdt_blob(), node,
+				IMAGE_PRE_LOAD_PROP_ALGO_NAME, NULL);
+	if (!algo_name) {
+		printf("ERROR: no algo_name for image pre-load sig check\n");
+		ret = -EINVAL;
+		goto out;
+	}
+
+	padding_name = fdt_getprop(gd_fdt_blob(), node,
+				   IMAGE_PRE_LOAD_PROP_PADDING_NAME, NULL);
+	if (!padding_name) {
+		log_info("INFO: no padding_name provided, so using pkcs-1.5\n");
+		padding_name = "pkcs-1.5";
+	}
+
+	sig_size = fdt_getprop(gd_fdt_blob(), node,
+			       IMAGE_PRE_LOAD_PROP_SIG_SIZE, NULL);
+	if (!sig_size) {
+		log_err("ERROR: no signature-size for image pre-load sig check\n");
+		ret = -EINVAL;
+		goto out;
+	}
+
+	key = fdt_getprop(gd_fdt_blob(), node,
+			  IMAGE_PRE_LOAD_PROP_PUBLIC_KEY, &key_len);
+	if (!key) {
+		log_err("ERROR: no key for image pre-load sig check\n");
+		ret = -EINVAL;
+		goto out;
+	}
+
+	info->algo_name		= (char *)algo_name;
+	info->padding_name	= (char *)padding_name;
+	info->key		= (uint8_t *)key;
+	info->key_len		= key_len;
+	info->sig_size		= fdt32_to_cpu(*sig_size);
+
+	mandatory = fdt_getprop(gd_fdt_blob(), node,
+				IMAGE_PRE_LOAD_PROP_MANDATORY, NULL);
+	if (mandatory && !strcmp((char *)mandatory, "yes"))
+		info->mandatory = 1;
+
+	/* Compute signature information */
+	info->sig_info.name     = info->algo_name;
+	info->sig_info.padding  = image_get_padding_algo(info->padding_name);
+	info->sig_info.checksum = image_get_checksum_algo(info->sig_info.name);
+	info->sig_info.crypto   = image_get_crypto_algo(info->sig_info.name);
+	info->sig_info.key      = info->key;
+	info->sig_info.keylen   = info->key_len;
+
+ out:
+	return ret;
+}
+
+static int image_pre_load_sig_get_magic(ulong addr, u32 *magic)
+{
+	struct sig_header_s *sig_header;
+	int ret = 0;
+
+	sig_header = (struct sig_header_s *)map_sysmem(addr, SIG_HEADER_LEN);
+	if (!sig_header) {
+		log_err("ERROR: can't map first header\n");
+		ret = -EFAULT;
+		goto out;
+	}
+
+	*magic = fdt32_to_cpu(sig_header->magic);
+
+	unmap_sysmem(sig_header);
+
+ out:
+	return ret;
+}
+
+static int image_pre_load_sig_get_header_size(ulong addr, u32 *header_size)
+{
+	struct sig_header_s *sig_header;
+	int ret = 0;
+
+	sig_header = (struct sig_header_s *)map_sysmem(addr, SIG_HEADER_LEN);
+	if (!sig_header) {
+		log_err("ERROR: can't map first header\n");
+		ret = -EFAULT;
+		goto out;
+	}
+
+	*header_size = fdt32_to_cpu(sig_header->header_size);
+
+	unmap_sysmem(sig_header);
+
+ out:
+	return ret;
+}
+
+/*
+ * return:
+ * < 0 => no magic and magic mandatory (or error when reading magic)
+ *   0 => magic found
+ *   1 => magic NOT found
+ */
+static int image_pre_load_sig_check_magic(struct image_sig_info *info, ulong addr)
+{
+	u32 magic;
+	int ret = 1;
+
+	ret = image_pre_load_sig_get_magic(addr, &magic);
+	if (ret < 0)
+		goto out;
+
+	if (magic != IMAGE_PRE_LOAD_SIG_MAGIC) {
+		if (info->mandatory) {
+			log_err("ERROR: signature is mandatory\n");
+			ret = -EINVAL;
+			goto out;
+		}
+		ret = 1;
+		goto out;
+	}
+
+	ret = 0; /* magic found */
+
+ out:
+	return ret;
+}
+
+static int image_pre_load_sig_check_header_sig(struct image_sig_info *info, ulong addr)
+{
+	void *header;
+	struct image_region reg;
+	u32 sig_len;
+	u8 *sig;
+	int ret = 0;
+
+	/* Only map header of the header and its signature */
+	header = (void *)map_sysmem(addr, SIG_HEADER_LEN + info->sig_size);
+	if (!header) {
+		log_err("ERROR: can't map header\n");
+		ret = -EFAULT;
+		goto out;
+	}
+
+	reg.data = header;
+	reg.size = SIG_HEADER_LEN;
+
+	sig = (uint8_t *)header + SIG_HEADER_LEN;
+	sig_len = info->sig_size;
+
+	ret = info->sig_info.crypto->verify(&info->sig_info, &reg, 1, sig, sig_len);
+	if (ret) {
+		log_err("ERROR: header signature check has failed (err=%d)\n", ret);
+		ret = -EINVAL;
+		goto out_unmap;
+	}
+
+ out_unmap:
+	unmap_sysmem(header);
+
+ out:
+	return ret;
+}
+
+static int image_pre_load_sig_check_img_sig_sha256(struct image_sig_info *info, ulong addr)
+{
+	struct sig_header_s *sig_header;
+	u32 header_size, offset_img_sig;
+	void *header;
+	u8 sha256_img_sig[SHA256_SUM_LEN];
+	int ret = 0;
+
+	sig_header = (struct sig_header_s *)map_sysmem(addr, SIG_HEADER_LEN);
+	if (!sig_header) {
+		log_err("ERROR: can't map first header\n");
+		ret = -EFAULT;
+		goto out;
+	}
+
+	header_size = fdt32_to_cpu(sig_header->header_size);
+	offset_img_sig = fdt32_to_cpu(sig_header->offset_img_sig);
+
+	header = (void *)map_sysmem(addr, header_size);
+	if (!header) {
+		log_err("ERROR: can't map header\n");
+		ret = -EFAULT;
+		goto out_sig_header;
+	}
+
+	sha256_csum_wd(header + offset_img_sig, info->sig_size,
+		       sha256_img_sig, CHUNKSZ_SHA256);
+
+	ret = memcmp(sig_header->sha256_img_sig, sha256_img_sig, SHA256_SUM_LEN);
+	if (ret) {
+		log_err("ERROR: sha256 of image signature is invalid\n");
+		ret = -EFAULT;
+		goto out_header;
+	}
+
+ out_header:
+	unmap_sysmem(header);
+ out_sig_header:
+	unmap_sysmem(sig_header);
+ out:
+	return ret;
+}
+
+static int image_pre_load_sig_check_img_sig(struct image_sig_info *info, ulong addr)
+{
+	struct sig_header_s *sig_header;
+	u32 header_size, image_size, offset_img_sig;
+	void *image;
+	struct image_region reg;
+	u32 sig_len;
+	u8 *sig;
+	int ret = 0;
+
+	sig_header = (struct sig_header_s *)map_sysmem(addr, SIG_HEADER_LEN);
+	if (!sig_header) {
+		log_err("ERROR: can't map first header\n");
+		ret = -EFAULT;
+		goto out;
+	}
+
+	header_size = fdt32_to_cpu(sig_header->header_size);
+	image_size = fdt32_to_cpu(sig_header->image_size);
+	offset_img_sig = fdt32_to_cpu(sig_header->offset_img_sig);
+
+	unmap_sysmem(sig_header);
+
+	image = (void *)map_sysmem(addr, header_size + image_size);
+	if (!image) {
+		log_err("ERROR: can't map full image\n");
+		ret = -EFAULT;
+		goto out;
+	}
+
+	reg.data = image + header_size;
+	reg.size = image_size;
+
+	sig = (uint8_t *)image + offset_img_sig;
+	sig_len = info->sig_size;
+
+	ret = info->sig_info.crypto->verify(&info->sig_info, &reg, 1, sig, sig_len);
+	if (ret) {
+		log_err("ERROR: signature check has failed (err=%d)\n", ret);
+		ret = -EINVAL;
+		goto out_unmap_image;
+	}
+
+	log_info("INFO: signature check has succeed\n");
+
+ out_unmap_image:
+	unmap_sysmem(image);
+
+ out:
+	return ret;
+}
+
+int image_pre_load_sig(ulong addr)
+{
+	struct image_sig_info info;
+	int ret;
+
+	ret = image_pre_load_sig_setup(&info);
+	if (ret < 0)
+		goto out;
+	if (ret > 0) {
+		ret = 0;
+		goto out;
+	}
+
+	ret = image_pre_load_sig_check_magic(&info, addr);
+	if (ret < 0)
+		goto out;
+	if (ret > 0) {
+		ret = 0;
+		goto out;
+	}
+
+	/* Check the signature of the signature header */
+	ret = image_pre_load_sig_check_header_sig(&info, addr);
+	if (ret < 0)
+		goto out;
+
+	/* Check sha256 of the image signature */
+	ret = image_pre_load_sig_check_img_sig_sha256(&info, addr);
+	if (ret < 0)
+		goto out;
+
+	/* Check the image signature */
+	ret = image_pre_load_sig_check_img_sig(&info, addr);
+	if (!ret) {
+		u32 header_size;
+
+		ret = image_pre_load_sig_get_header_size(addr, &header_size);
+		if (ret) {
+			log_err("%s: can't get header size\n", __func__);
+			ret = -EINVAL;
+			goto out;
+		}
+
+		image_load_offset += header_size;
+	}
+
+ out:
+	return ret;
+}
+
+int image_pre_load(ulong addr)
+{
+	int ret = 0;
+
+	image_load_offset = 0;
+
+	if (CONFIG_IS_ENABLED(IMAGE_PRE_LOAD_SIG))
+		ret = image_pre_load_sig(addr);
+
+	return ret;
+}
diff --git a/include/image.h b/include/image.h
index 97e5f2eb24..fbcf70f5e4 100644
--- a/include/image.h
+++ b/include/image.h
@@ -48,6 +48,7 @@ struct fdt_region;
 extern ulong image_load_addr;		/* Default Load Address */
 extern ulong image_save_addr;		/* Default Save Address */
 extern ulong image_save_size;		/* Default Save Size */
+extern ulong image_load_offset;	/* Default Load Address Offset */
 
 /* An invalid size, meaning that the image size is not known */
 #define IMAGE_SIZE_INVAL	(-1UL)
@@ -1323,6 +1324,19 @@ struct crypto_algo *image_get_crypto_algo(const char *full_name);
  */
 struct padding_algo *image_get_padding_algo(const char *name);
 
+/**
+ * image_pre_load() - Manage pre load header
+ *
+ * Manage the pre-load header before launching the image.
+ * It checks the signature of the image. It also set the
+ * variable image_load_offset to skip this header before
+ * launching the image.
+ *
+ * @param addr		Address of the image
+ * @return: 0 on success, -ve on error
+ */
+int image_pre_load(ulong addr);
+
 /**
  * fit_image_verify_required_sigs() - Verify signatures marked as 'required'
  *
-- 
2.17.1


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

* [PATCH v6 08/16] cmd: bootm: add a stage pre-load
  2022-02-25 14:57 [PATCH v6 00/16] image: add a stage pre-load Philippe Reynes
                   ` (6 preceding siblings ...)
  2022-02-25 14:57 ` [PATCH v6 07/16] boot: image: add a stage pre-load Philippe Reynes
@ 2022-02-25 14:57 ` Philippe Reynes
  2022-02-25 14:57 ` [PATCH v6 09/16] common: spl: fit_ram: allow to use image pre load Philippe Reynes
                   ` (7 subsequent siblings)
  15 siblings, 0 replies; 30+ messages in thread
From: Philippe Reynes @ 2022-02-25 14:57 UTC (permalink / raw)
  To: sjg, rasmus.villemoes; +Cc: u-boot, Philippe Reynes

Add a stage pre-load to the command bootm.
Right now, this stage may be used to read a
header and check the signature of the full
image.

Reviewed-by: Simon Glass <sjg@chromium.org>
Signed-off-by: Philippe Reynes <philippe.reynes@softathome.com>
---
 boot/bootm.c    | 33 +++++++++++++++++++++++++++++++++
 cmd/Kconfig     | 10 ++++++++++
 cmd/bootm.c     |  2 +-
 include/image.h |  1 +
 4 files changed, 45 insertions(+), 1 deletion(-)

diff --git a/boot/bootm.c b/boot/bootm.c
index 00c00aef84..714406ab66 100644
--- a/boot/bootm.c
+++ b/boot/bootm.c
@@ -87,6 +87,33 @@ static int bootm_start(struct cmd_tbl *cmdtp, int flag, int argc,
 	return 0;
 }
 
+static ulong bootm_data_addr(int argc, char *const argv[])
+{
+	ulong addr;
+
+	if (argc > 0)
+		addr = simple_strtoul(argv[0], NULL, 16);
+	else
+		addr = image_load_addr;
+
+	return addr;
+}
+
+static int bootm_pre_load(struct cmd_tbl *cmdtp, int flag, int argc,
+			  char *const argv[])
+{
+	ulong data_addr = bootm_data_addr(argc, argv);
+	int ret = 0;
+
+	if (CONFIG_IS_ENABLED(CMD_BOOTM_PRE_LOAD))
+		ret = image_pre_load(data_addr);
+
+	if (ret)
+		ret = CMD_RET_FAILURE;
+
+	return ret;
+}
+
 static int bootm_find_os(struct cmd_tbl *cmdtp, int flag, int argc,
 			 char *const argv[])
 {
@@ -677,6 +704,9 @@ int do_bootm_states(struct cmd_tbl *cmdtp, int flag, int argc,
 	if (states & BOOTM_STATE_START)
 		ret = bootm_start(cmdtp, flag, argc, argv);
 
+	if (!ret && (states & BOOTM_STATE_PRE_LOAD))
+		ret = bootm_pre_load(cmdtp, flag, argc, argv);
+
 	if (!ret && (states & BOOTM_STATE_FINDOS))
 		ret = bootm_find_os(cmdtp, flag, argc, argv);
 
@@ -866,6 +896,9 @@ static const void *boot_get_kernel(struct cmd_tbl *cmdtp, int flag, int argc,
 					      &fit_uname_config,
 					      &fit_uname_kernel);
 
+	if (CONFIG_IS_ENABLED(CMD_BOOTM_PRE_LOAD))
+		img_addr += image_load_offset;
+
 	bootstage_mark(BOOTSTAGE_ID_CHECK_MAGIC);
 
 	/* check image type, for FIT images get FIT kernel node */
diff --git a/cmd/Kconfig b/cmd/Kconfig
index 5e25e45fd2..87aa3fb11a 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -194,6 +194,16 @@ config CMD_BOOTM
 	help
 	  Boot an application image from the memory.
 
+config CMD_BOOTM_PRE_LOAD
+       bool "enable pre-load on bootm"
+       depends on CMD_BOOTM
+       depends on IMAGE_PRE_LOAD
+       default n
+       help
+         Enable support of stage pre-load for the bootm command.
+	 This stage allow to check or modify the image provided
+	 to the bootm command.
+
 config BOOTM_EFI
 	bool "Support booting UEFI FIT images"
 	depends on CMD_BOOTEFI && CMD_BOOTM && FIT
diff --git a/cmd/bootm.c b/cmd/bootm.c
index e8b7066888..c5de339fba 100644
--- a/cmd/bootm.c
+++ b/cmd/bootm.c
@@ -126,7 +126,7 @@ int do_bootm(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
 	}
 
 	return do_bootm_states(cmdtp, flag, argc, argv, BOOTM_STATE_START |
-		BOOTM_STATE_FINDOS | BOOTM_STATE_FINDOTHER |
+		BOOTM_STATE_FINDOS | BOOTM_STATE_PRE_LOAD | BOOTM_STATE_FINDOTHER |
 		BOOTM_STATE_LOADOS |
 #ifdef CONFIG_SYS_BOOT_RAMDISK_HIGH
 		BOOTM_STATE_RAMDISK |
diff --git a/include/image.h b/include/image.h
index fbcf70f5e4..496b7af3f3 100644
--- a/include/image.h
+++ b/include/image.h
@@ -351,6 +351,7 @@ typedef struct bootm_headers {
 #define	BOOTM_STATE_OS_PREP	(0x00000100)
 #define	BOOTM_STATE_OS_FAKE_GO	(0x00000200)	/* 'Almost' run the OS */
 #define	BOOTM_STATE_OS_GO	(0x00000400)
+#define	BOOTM_STATE_PRE_LOAD	0x00000800
 	int		state;
 
 #if defined(CONFIG_LMB) && !defined(USE_HOSTCC)
-- 
2.17.1


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

* [PATCH v6 09/16] common: spl: fit_ram: allow to use image pre load
  2022-02-25 14:57 [PATCH v6 00/16] image: add a stage pre-load Philippe Reynes
                   ` (7 preceding siblings ...)
  2022-02-25 14:57 ` [PATCH v6 08/16] cmd: bootm: " Philippe Reynes
@ 2022-02-25 14:57 ` Philippe Reynes
  2022-02-25 14:57 ` [PATCH v6 10/16] mkimage: add public key for image pre-load stage Philippe Reynes
                   ` (6 subsequent siblings)
  15 siblings, 0 replies; 30+ messages in thread
From: Philippe Reynes @ 2022-02-25 14:57 UTC (permalink / raw)
  To: sjg, rasmus.villemoes; +Cc: u-boot, Philippe Reynes

Add the support of image pre load in spl or tpl
when loading an image from ram.

Reviewed-by: Simon Glass <sjg@chromium.org>
Signed-off-by: Philippe Reynes <philippe.reynes@softathome.com>
---
 common/spl/spl_ram.c | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/common/spl/spl_ram.c b/common/spl/spl_ram.c
index 3f7f7accc1..8296459257 100644
--- a/common/spl/spl_ram.c
+++ b/common/spl/spl_ram.c
@@ -24,9 +24,17 @@
 static ulong spl_ram_load_read(struct spl_load_info *load, ulong sector,
 			       ulong count, void *buf)
 {
+	ulong addr;
+
 	debug("%s: sector %lx, count %lx, buf %lx\n",
 	      __func__, sector, count, (ulong)buf);
-	memcpy(buf, (void *)(CONFIG_SPL_LOAD_FIT_ADDRESS + sector), count);
+
+	addr = (ulong)CONFIG_SPL_LOAD_FIT_ADDRESS + sector;
+	if (CONFIG_IS_ENABLED(IMAGE_PRE_LOAD))
+		addr += image_load_offset;
+
+	memcpy(buf, (void *)addr, count);
+
 	return count;
 }
 
@@ -37,6 +45,17 @@ static int spl_ram_load_image(struct spl_image_info *spl_image,
 
 	header = (struct image_header *)CONFIG_SPL_LOAD_FIT_ADDRESS;
 
+	if (CONFIG_IS_ENABLED(IMAGE_PRE_LOAD)) {
+		unsigned long addr = (unsigned long)header;
+		int ret = image_pre_load(addr);
+
+		if (ret)
+			return ret;
+
+		addr += image_load_offset;
+		header = (struct image_header *)addr;
+	}
+
 #if CONFIG_IS_ENABLED(DFU)
 	if (bootdev->boot_device == BOOT_DEVICE_DFU)
 		spl_dfu_cmd(0, "dfu_alt_info_ram", "ram", "0");
-- 
2.17.1


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

* [PATCH v6 10/16] mkimage: add public key for image pre-load stage
  2022-02-25 14:57 [PATCH v6 00/16] image: add a stage pre-load Philippe Reynes
                   ` (8 preceding siblings ...)
  2022-02-25 14:57 ` [PATCH v6 09/16] common: spl: fit_ram: allow to use image pre load Philippe Reynes
@ 2022-02-25 14:57 ` Philippe Reynes
  2022-03-03  3:37   ` Simon Glass
  2022-02-25 14:57 ` [PATCH v6 11/16] Makefile: provide sah-key to binman Philippe Reynes
                   ` (5 subsequent siblings)
  15 siblings, 1 reply; 30+ messages in thread
From: Philippe Reynes @ 2022-02-25 14:57 UTC (permalink / raw)
  To: sjg, rasmus.villemoes; +Cc: u-boot, Philippe Reynes

This commit enhances mkimage to update the node
/image/pre-load/sig with the public key.

Signed-off-by: Philippe Reynes <philippe.reynes@softathome.com>
---
 include/image.h    |  15 ++++++
 tools/fit_image.c  |   3 ++
 tools/image-host.c | 114 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 132 insertions(+)

diff --git a/include/image.h b/include/image.h
index 496b7af3f3..498eb7f2e3 100644
--- a/include/image.h
+++ b/include/image.h
@@ -1019,6 +1019,21 @@ int fit_image_hash_get_value(const void *fit, int noffset, uint8_t **value,
 
 int fit_set_timestamp(void *fit, int noffset, time_t timestamp);
 
+/**
+ * fit_pre_load_data() - add public key to fdt blob
+ *
+ * Adds public key to the node pre load.
+ *
+ * @keydir:	Directory containing keys
+ * @keydest:	FDT blob to write public key
+ * @fit:	Pointer to the FIT format image header
+ *
+ * returns:
+ *	0, on success
+ *	< 0, on failure
+ */
+int fit_pre_load_data(const char *keydir, void *keydest, void *fit);
+
 int fit_cipher_data(const char *keydir, void *keydest, void *fit,
 		    const char *comment, int require_keys,
 		    const char *engine_id, const char *cmdname);
diff --git a/tools/fit_image.c b/tools/fit_image.c
index 15f7c82d61..1884a2eb0b 100644
--- a/tools/fit_image.c
+++ b/tools/fit_image.c
@@ -59,6 +59,9 @@ static int fit_add_file_data(struct image_tool_params *params, size_t size_inc,
 		ret = fit_set_timestamp(ptr, 0, time);
 	}
 
+	if (!ret)
+		ret = fit_pre_load_data(params->keydir, dest_blob, ptr);
+
 	if (!ret) {
 		ret = fit_cipher_data(params->keydir, dest_blob, ptr,
 				      params->comment,
diff --git a/tools/image-host.c b/tools/image-host.c
index eaeb76545c..ab6f756cf1 100644
--- a/tools/image-host.c
+++ b/tools/image-host.c
@@ -14,6 +14,11 @@
 #include <image.h>
 #include <version.h>
 
+#include <openssl/pem.h>
+#include <openssl/evp.h>
+
+#define IMAGE_PRE_LOAD_PATH                             "/image/pre-load/sig"
+
 /**
  * fit_set_hash_value - set hash value in requested has node
  * @fit: pointer to the FIT format image header
@@ -1111,6 +1116,115 @@ static int fit_config_add_verification_data(const char *keydir,
 	return 0;
 }
 
+/*
+ * 0) open file (open)
+ * 1) read certificate (PEM_read_X509)
+ * 2) get public key (X509_get_pubkey)
+ * 3) provide der format (d2i_RSAPublicKey)
+ */
+static int read_pub_key(const char *keydir, const void *name,
+			unsigned char **pubkey, int *pubkey_len)
+{
+	char path[1024];
+	EVP_PKEY *key = NULL;
+	X509 *cert;
+	FILE *f;
+	int ret;
+
+	memset(path, 0, 1024);
+	snprintf(path, sizeof(path), "%s/%s.crt", keydir, (char *)name);
+
+	/* Open certificate file */
+	f = fopen(path, "r");
+	if (!f) {
+		fprintf(stderr, "Couldn't open RSA certificate: '%s': %s\n",
+			path, strerror(errno));
+		return -EACCES;
+	}
+
+	/* Read the certificate */
+	cert = NULL;
+	if (!PEM_read_X509(f, &cert, NULL, NULL)) {
+		printf("Couldn't read certificate");
+		ret = -EINVAL;
+		goto err_cert;
+	}
+
+	/* Get the public key from the certificate. */
+	key = X509_get_pubkey(cert);
+	if (!key) {
+		printf("Couldn't read public key\n");
+		ret = -EINVAL;
+		goto err_pubkey;
+	}
+
+	/* Get DER form */
+	ret = i2d_PublicKey(key, pubkey);
+	if (ret < 0) {
+		printf("Couldn't get DER form\n");
+		ret = -EINVAL;
+		goto err_pubkey;
+	}
+
+	*pubkey_len = ret;
+	ret = 0;
+
+err_pubkey:
+	X509_free(cert);
+err_cert:
+	fclose(f);
+	return ret;
+}
+
+int fit_pre_load_data(const char *keydir, void *keydest, void *fit)
+{
+	int pre_load_noffset;
+	const void *algo_name;
+	const void *key_name;
+	unsigned char *pubkey = NULL;
+	int ret, pubkey_len;
+
+	if (!keydir || !keydest || !fit)
+		return 0;
+
+	/* Search node pre-load sig */
+	pre_load_noffset = fdt_path_offset(keydest, IMAGE_PRE_LOAD_PATH);
+	if (pre_load_noffset < 0) {
+		ret = 0;
+		goto out;
+	}
+
+	algo_name = fdt_getprop(keydest, pre_load_noffset, "algo-name", NULL);
+	key_name  = fdt_getprop(keydest, pre_load_noffset, "key-name", NULL);
+
+	/* Check that all mandatory properties are present */
+	if (!algo_name || !key_name) {
+		if (!algo_name)
+			printf("The property algo-name is missing in the node %s\n",
+			       IMAGE_PRE_LOAD_PATH);
+		if (!key_name)
+			printf("The property key-name is missing in the node %s\n",
+			       IMAGE_PRE_LOAD_PATH);
+		ret = -ENODATA;
+		goto out;
+	}
+
+	/* Read public key */
+	ret = read_pub_key(keydir, key_name, &pubkey, &pubkey_len);
+	if (ret < 0)
+		goto out;
+
+	/* Add the public key to the device tree */
+	ret = fdt_setprop(keydest, pre_load_noffset, "public-key",
+			  pubkey, pubkey_len);
+	if (ret)
+		printf("Can't set public-key in node %s (ret = %d)\n",
+		       IMAGE_PRE_LOAD_PATH, ret);
+
+ out:
+	return ret;
+}
+
 int fit_cipher_data(const char *keydir, void *keydest, void *fit,
 		    const char *comment, int require_keys,
 		    const char *engine_id, const char *cmdname)
-- 
2.17.1


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

* [PATCH v6 11/16] Makefile: provide sah-key to binman
  2022-02-25 14:57 [PATCH v6 00/16] image: add a stage pre-load Philippe Reynes
                   ` (9 preceding siblings ...)
  2022-02-25 14:57 ` [PATCH v6 10/16] mkimage: add public key for image pre-load stage Philippe Reynes
@ 2022-02-25 14:57 ` Philippe Reynes
  2022-03-03  3:37   ` Simon Glass
  2022-02-25 14:57 ` [PATCH v6 12/16] tools: binman: add support for pre-load header Philippe Reynes
                   ` (4 subsequent siblings)
  15 siblings, 1 reply; 30+ messages in thread
From: Philippe Reynes @ 2022-02-25 14:57 UTC (permalink / raw)
  To: sjg, rasmus.villemoes; +Cc: u-boot, Philippe Reynes

Set the variable key-path with the shell variable
KEY_PATH that contain the keys path (used for signature).
This variable key-path is provided to binman.

Signed-off-by: Philippe Reynes <philippe.reynes@softathome.com>
---
 Makefile | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Makefile b/Makefile
index 697cc51d67..c6da6cdba0 100644
--- a/Makefile
+++ b/Makefile
@@ -1335,6 +1335,7 @@ cmd_binman = $(srctree)/tools/binman/binman $(if $(BINMAN_DEBUG),-D) \
 		-a tpl-bss-pad=$(if $(CONFIG_TPL_SEPARATE_BSS),,1) \
 		-a spl-dtb=$(CONFIG_SPL_OF_REAL) \
 		-a tpl-dtb=$(CONFIG_TPL_OF_REAL) \
+		-a key-path=${KEY_PATH} \
 		$(BINMAN_$(@F))
 
 OBJCOPYFLAGS_u-boot.ldr.hex := -I binary -O ihex
-- 
2.17.1


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

* [PATCH v6 12/16] tools: binman: add support for pre-load header
  2022-02-25 14:57 [PATCH v6 00/16] image: add a stage pre-load Philippe Reynes
                   ` (10 preceding siblings ...)
  2022-02-25 14:57 ` [PATCH v6 11/16] Makefile: provide sah-key to binman Philippe Reynes
@ 2022-02-25 14:57 ` Philippe Reynes
  2022-03-03  3:37   ` Simon Glass
  2022-02-25 14:57 ` [PATCH v6 13/16] configs: sandbox_defconfig: enable stage pre-load in bootm Philippe Reynes
                   ` (3 subsequent siblings)
  15 siblings, 1 reply; 30+ messages in thread
From: Philippe Reynes @ 2022-02-25 14:57 UTC (permalink / raw)
  To: sjg, rasmus.villemoes; +Cc: u-boot, Philippe Reynes

Adds the support of the pre-load header with the image signature
to binman.

Signed-off-by: Philippe Reynes <philippe.reynes@softathome.com>
---
 tools/binman/etype/pre_load.py                | 165 ++++++++++++++++++
 tools/binman/ftest.py                         |  45 +++++
 tools/binman/test/225_dev.key                 |  28 +++
 tools/binman/test/225_pre_load.dts            |  22 +++
 tools/binman/test/226_pre_load_pkcs.dts       |  23 +++
 tools/binman/test/227_pre_load_pss.dts        |  23 +++
 .../test/228_pre_load_invalid_padding.dts     |  23 +++
 .../binman/test/229_pre_load_invalid_sha.dts  |  23 +++
 .../binman/test/230_pre_load_invalid_algo.dts |  23 +++
 .../binman/test/231_pre_load_invalid_key.dts  |  23 +++
 10 files changed, 398 insertions(+)
 create mode 100644 tools/binman/etype/pre_load.py
 create mode 100644 tools/binman/test/225_dev.key
 create mode 100644 tools/binman/test/225_pre_load.dts
 create mode 100644 tools/binman/test/226_pre_load_pkcs.dts
 create mode 100644 tools/binman/test/227_pre_load_pss.dts
 create mode 100644 tools/binman/test/228_pre_load_invalid_padding.dts
 create mode 100644 tools/binman/test/229_pre_load_invalid_sha.dts
 create mode 100644 tools/binman/test/230_pre_load_invalid_algo.dts
 create mode 100644 tools/binman/test/231_pre_load_invalid_key.dts

diff --git a/tools/binman/etype/pre_load.py b/tools/binman/etype/pre_load.py
new file mode 100644
index 0000000000..2af2857404
--- /dev/null
+++ b/tools/binman/etype/pre_load.py
@@ -0,0 +1,165 @@
+# SPDX-License-Identifier: GPL-2.0+
+# Copyright (c) 2022 Softathome
+# Written by Philippe Reynes <philippe.reynes@softathome.com>
+#
+# Entry-type for the global header
+#
+
+import struct
+from dtoc import fdt_util
+from patman import tools
+
+from binman.entry import Entry
+from binman.etype.collection import Entry_collection
+from binman.entry import EntryArg
+
+from Cryptodome.Hash import SHA256, SHA384, SHA512
+from Cryptodome.PublicKey import RSA
+from Cryptodome.Signature import pkcs1_15
+from Cryptodome.Signature import pss
+
+PRE_LOAD_MAGIC = b'UBSH'
+
+RSAS = {
+    'rsa1024': 1024 / 8,
+    'rsa2048': 2048 / 8,
+    'rsa4096': 4096 / 8
+}
+
+SHAS = {
+    'sha256': SHA256,
+    'sha384': SHA384,
+    'sha512': SHA512
+}
+
+class Entry_pre_load(Entry_collection):
+    """Pre load image header
+
+    Properties / Entry arguments:
+        - key-path: Path of the directory that store key (provided by the environment variable KEY_PATH)
+        - content: List of phandles to entries to sign
+        - algo-name: Hash and signature algo to use for the signature
+        - padding-name: Name of the padding (pkcs-1.5 or pss)
+        - key-name: Filename of the private key to sign
+        - header-size: Total size of the header
+        - version: Version of the header
+
+    This entry create a pre-load header that contain a global
+    image signature.
+
+    For example, this creates an image with a pre-load header and a binary::
+
+        binman {
+	    image2 {
+		filename = "sandbox.bin";
+
+		pre-load {
+		    content = <&image>;
+		    algo-name = "sha256,rsa2048";
+		    padding-name = "pss";
+		    key-name = "private.pem";
+		    header-size = <4096>;
+		    version = <1>;
+		};
+
+		image: blob-ext {
+		    filename = "sandbox.itb";
+		};
+	    };
+        };
+    """
+
+    def __init__(self, section, etype, node):
+        super().__init__(section, etype, node)
+        self.algo_name = fdt_util.GetString(self._node, 'algo-name')
+        self.padding_name = fdt_util.GetString(self._node, 'padding-name')
+        self.key_name = fdt_util.GetString(self._node, 'key-name')
+        self.header_size = fdt_util.GetInt(self._node, 'header-size')
+        self.version = fdt_util.GetInt(self._node, 'version')
+
+    def _CreateHeader(self):
+        """Create a pre load header"""
+        hash_name, sign_name  = self.algo_name.split(',')
+        padding_name=self.padding_name
+        key_path, = self.GetEntryArgsOrProps([EntryArg('key-path', str)])
+        if key_path is None or key_path == "":
+            key_name = self.key_name
+        else:
+            key_name = key_path + "/" + self.key_name
+
+        # Check hash and signature name/type
+        if hash_name not in SHAS:
+            raise ValueError(hash_name + " is not supported")
+        if sign_name not in RSAS:
+            raise ValueError(sign_name + "is not supported")
+
+        # Read the key
+        with open(key_name, 'rb') as pem:
+            key = RSA.import_key(pem.read())
+
+        # Check if the key has the expected size
+        if key.size_in_bytes() != RSAS[sign_name]:
+            raise ValueError("The key " + self.key_name + " don't have the expected size")
+
+        # Compute the hash
+        hash_image = SHAS[hash_name].new()
+        hash_image.update(self.image)
+
+        # Compute the signature
+        if padding_name is None:
+            padding_name = "pkcs-1.5"
+        if padding_name == "pss":
+            salt_len = key.size_in_bytes() - hash_image.digest_size - 2
+            padding = pss
+            padding_args = {'salt_bytes': salt_len}
+        elif padding_name == "pkcs-1.5":
+            padding = pkcs1_15
+            padding_args = {}
+        else:
+            raise ValueError(padding_name + " is not supported")
+
+        sig = padding.new(key, **padding_args).sign(hash_image)
+
+        hash_sig = SHA256.new()
+        hash_sig.update(sig)
+
+        version = self.version
+        header_size = self.header_size
+        image_size = len(self.image)
+        ofs_img_sig = 64 + len(sig)
+        flags = 0
+        reserved0 = 0
+        reserved1 = 0
+
+        first_header = bytearray(64)
+        struct.pack_into('4s', first_header, 0, PRE_LOAD_MAGIC)
+        struct.pack_into('>I', first_header, 4, version)
+        struct.pack_into('>I', first_header, 8, header_size)
+        struct.pack_into('>I', first_header, 12, image_size)
+        struct.pack_into('>I', first_header, 16, ofs_img_sig)
+        struct.pack_into('>I', first_header, 20, flags)
+        struct.pack_into('>I', first_header, 24, reserved0)
+        struct.pack_into('>I', first_header, 28, reserved1)
+        struct.pack_into('32s', first_header, 32, hash_sig.digest())
+
+        hash_first_header = SHAS[hash_name].new()
+        hash_first_header.update(first_header)
+        sig_first_header = padding.new(key, **padding_args).sign(hash_first_header)
+
+        data = first_header + sig_first_header + sig
+        pad  = bytearray(self.header_size - len(data))
+
+        return data + pad
+
+    def ObtainContents(self):
+        """Obtain a placeholder for the header contents"""
+        # wait that the image is available
+        self.image = self.GetContents(False)
+        if self.image is None:
+            return False
+        self.SetContents(self._CreateHeader())
+        return True
+
+    def ProcessContents(self):
+        data = self._CreateHeader()
+        return self.ProcessContentsUpdate(data)
diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
index 8f00db6945..06b8546354 100644
--- a/tools/binman/ftest.py
+++ b/tools/binman/ftest.py
@@ -91,6 +91,9 @@ SCP_DATA              = b'scp'
 TEST_FDT1_DATA        = b'fdt1'
 TEST_FDT2_DATA        = b'test-fdt2'
 ENV_DATA              = b'var1=1\nvar2="2"'
+PRE_LOAD_MAGIC        = b'UBSH'
+PRE_LOAD_VERSION      = 0x11223344.to_bytes(4, 'big')
+PRE_LOAD_HDR_SIZE     = 0x00001000.to_bytes(4, 'big')
 
 # Subdirectory of the input dir to use to put test FDTs
 TEST_FDT_SUBDIR       = 'fdts'
@@ -5321,6 +5324,48 @@ fdt         fdtmap                Extract the devicetree blob from the fdtmap
         self.assertIn("Node '/binman/fit': Unknown operation 'unknown'",
                       str(exc.exception))
 
+    def testPreLoad(self):
+        """Test an image with a pre-load header"""
+        entry_args = {
+            'key-path': '.',
+        }
+        data, _, _, _ = self._DoReadFileDtb('225_pre_load.dts',
+                                            entry_args=entry_args)
+        self.assertEqual(PRE_LOAD_MAGIC, data[:len(PRE_LOAD_MAGIC)])
+        self.assertEqual(PRE_LOAD_VERSION, data[4:4+len(PRE_LOAD_VERSION)])
+        self.assertEqual(PRE_LOAD_HDR_SIZE, data[8:8+len(PRE_LOAD_HDR_SIZE)])
+        data = self._DoReadFile('225_pre_load.dts')
+        self.assertEqual(PRE_LOAD_MAGIC, data[:len(PRE_LOAD_MAGIC)])
+        self.assertEqual(PRE_LOAD_VERSION, data[4:4+len(PRE_LOAD_VERSION)])
+        self.assertEqual(PRE_LOAD_HDR_SIZE, data[8:8+len(PRE_LOAD_HDR_SIZE)])
+        data = self._DoReadFile('226_pre_load_pkcs.dts')
+        self.assertEqual(PRE_LOAD_MAGIC, data[:len(PRE_LOAD_MAGIC)])
+        self.assertEqual(PRE_LOAD_VERSION, data[4:4+len(PRE_LOAD_VERSION)])
+        self.assertEqual(PRE_LOAD_HDR_SIZE, data[8:8+len(PRE_LOAD_HDR_SIZE)])
+        data = self._DoReadFile('227_pre_load_pss.dts')
+        self.assertEqual(PRE_LOAD_MAGIC, data[:len(PRE_LOAD_MAGIC)])
+        self.assertEqual(PRE_LOAD_VERSION, data[4:4+len(PRE_LOAD_VERSION)])
+        self.assertEqual(PRE_LOAD_HDR_SIZE, data[8:8+len(PRE_LOAD_HDR_SIZE)])
+
+    def testPreLoadInvalidPadding(self):
+        """Test an image with a pre-load header with an invalid padding"""
+        with self.assertRaises(ValueError) as e:
+            data = self._DoReadFile('228_pre_load_invalid_padding.dts')
+
+    def testPreLoadInvalidSha(self):
+        """Test an image with a pre-load header with an invalid hash"""
+        with self.assertRaises(ValueError) as e:
+            data = self._DoReadFile('229_pre_load_invalid_sha.dts')
+
+    def testPreLoadInvalidAlgo(self):
+        """Test an image with a pre-load header with an invalid algo"""
+        with self.assertRaises(ValueError) as e:
+            data = self._DoReadFile('230_pre_load_invalid_algo.dts')
+
+    def testPreLoadInvalidKey(self):
+        """Test an image with a pre-load header with an invalid key"""
+        with self.assertRaises(ValueError) as e:
+            data = self._DoReadFile('231_pre_load_invalid_key.dts')
 
 if __name__ == "__main__":
     unittest.main()
diff --git a/tools/binman/test/225_dev.key b/tools/binman/test/225_dev.key
new file mode 100644
index 0000000000..b36bad2cfb
--- /dev/null
+++ b/tools/binman/test/225_dev.key
@@ -0,0 +1,28 @@
+-----BEGIN PRIVATE KEY-----
+MIIEvgIBADANBgkqhkiG9w0BAQEFAASCBKgwggSkAgEAAoIBAQDYngNWUvXYRXX/
+WEUI7k164fcpv1srXz+u+5Y3Yhouw3kPs+ffvYyHAPfjF7aUIAgezKk/4o7AvsxE
+Rdih3T+0deAd/q/yuqN4Adzt6ImnsO/EqdtYl3Yh+Vck9xWhLd3SAw1++GfSmNMT
+gxlcc/z6z+bIh2tJNtPtRSNNHMmvYYOkBmkfwcjbMXD+fe4vBwYjVrIize+l7Yuv
+1qN2nFlq56pFi8Lj5vOvFyNhZHRvwcpWdUdkx39beNUfwrGhgewOeWngTcY75n7S
+FY45TBR1G2PR90CQvyDinCi9Mm0u5s+1WASQWPblovfD6CPbHQu4GZm+FAs7yUvr
+hA7VCyNxAgMBAAECggEAUbq0uaJNfc8faTtNuMPo2d9eGRNI+8FRTt0/3R+Xj2NT
+TvhrGUD0P4++96Df012OkshXZ3I8uD6E5ZGQ3emTeqwq5kZM7oE64jGZwO3G2k1o
++cO4reFfwgvItHrBX3HlyrI6KljhG1Vr9mW1cOuWXK+KfMiTUylrpo86dYLSGeg3
+7ZlsOPArr4eof/A0iPryQZX6X5POf7k/e9qRFYsOkoRQO8pBL3J4rIKwBl3uBN3K
++FY40vCkd8JyTo2DNfHeIe1XYA9fG2ahjD2qMsw10TUsRRMd5yhonEcJ7VzGzy8m
+MnuMDAr7CwbbLkKi4UfZUl6YDkojqerwLOrxikBqkQKBgQD6sS6asDgwiq5MtstE
+4/PxMrVEsCdkrU+jjQN749qIt/41a6lbp0Pr6aUKKKGs0QbcnCtlpp7qmhvymBcW
+hlqxk2wokKMChv4WLXjZS3DGcOdMglc81y2F+252bToN8vwUfm6DPp9/GKtejA0a
+GP57GeHxoVO7vfDX1F/vZRogRQKBgQDdNCLWOlGWvnKjfgNZHgX+Ou6ZgTSAzy+/
+hRsZPlY5nwO5iD7YkIKvqBdOmfyjlUpHWk2uAcT9pfgzYygvyBRaoQhAYBGkHItt
+slaMxnLd+09wWufoCbgJvFn+wVQxBLcA5PXB98ws0Dq8ZYuo6AOuoRivsSO4lblK
+MW0guBJXPQKBgQDGjf0ukbH/aGfC5Oi8SJvWhuYhYC/jQo2YKUEAKCjXLnuOThZW
+PHXEbUrFcAcVfH0l0B9jJIQrpiHKlAF9Wq6MhQoeWuhxQQAQCrXzzRemZJgd9gIo
+cvlgbBNCgyJ/F9vmU3kuRDRJkv1wJhbee7tbPtXA7pkGUttl5pSRZI87zQKBgQC/
+0ZkwCox72xTQP9MpcYai6nnDta5Q0NnIC+Xu4wakmwcA2WweIlqhdnMXnyLcu/YY
+n+9iqHgpuMXd0eukW62C1cexA13o4TPrYU36b5BmfKprdPlLVzo3fxTPfNjEVSFY
+7jNLC9YLOlrkym3sf53Jzjr5B/RA+d0ewHOwfs6wxQKBgFSyfjx5wtdHK4fO+Z1+
+q3bxouZryM/4CiPCFuw4+aZmRHPmufuNCvfXdF+IH8dM0E9ObwKZAe/aMP/Y+Abx
+Wz9Vm4CP6g7k3DU3INEygyjmIQQDKQ9lFdDnsP9ESzrPbaGxZhc4x2lo7qmeW1BR
+/RuiAofleFkT4s+EhLrfE/v5
+-----END PRIVATE KEY-----
diff --git a/tools/binman/test/225_pre_load.dts b/tools/binman/test/225_pre_load.dts
new file mode 100644
index 0000000000..c1ffe1a2ff
--- /dev/null
+++ b/tools/binman/test/225_pre_load.dts
@@ -0,0 +1,22 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+/dts-v1/;
+
+/ {
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	binman {
+		pre-load {
+			content = <&image>;
+			 algo-name = "sha256,rsa2048";
+			 key-name = "tools/binman/test/225_dev.key";
+			 header-size = <4096>;
+			 version = <0x11223344>;
+		};
+
+		image: blob-ext {
+			filename = "refcode.bin";
+		};
+	};
+};
diff --git a/tools/binman/test/226_pre_load_pkcs.dts b/tools/binman/test/226_pre_load_pkcs.dts
new file mode 100644
index 0000000000..3db0a37f77
--- /dev/null
+++ b/tools/binman/test/226_pre_load_pkcs.dts
@@ -0,0 +1,23 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+/dts-v1/;
+
+/ {
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	binman {
+		pre-load {
+			content = <&image>;
+			 algo-name = "sha256,rsa2048";
+			 padding-name = "pkcs-1.5";
+			 key-name = "tools/binman/test/225_dev.key";
+			 header-size = <4096>;
+			 version = <0x11223344>;
+		};
+
+		image: blob-ext {
+			filename = "refcode.bin";
+		};
+	};
+};
diff --git a/tools/binman/test/227_pre_load_pss.dts b/tools/binman/test/227_pre_load_pss.dts
new file mode 100644
index 0000000000..b1b01d5ad5
--- /dev/null
+++ b/tools/binman/test/227_pre_load_pss.dts
@@ -0,0 +1,23 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+/dts-v1/;
+
+/ {
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	binman {
+		pre-load {
+			content = <&image>;
+			 algo-name = "sha256,rsa2048";
+			 padding-name = "pss";
+			 key-name = "tools/binman/test/225_dev.key";
+			 header-size = <4096>;
+			 version = <0x11223344>;
+		};
+
+		image: blob-ext {
+			filename = "refcode.bin";
+		};
+	};
+};
diff --git a/tools/binman/test/228_pre_load_invalid_padding.dts b/tools/binman/test/228_pre_load_invalid_padding.dts
new file mode 100644
index 0000000000..84fe289183
--- /dev/null
+++ b/tools/binman/test/228_pre_load_invalid_padding.dts
@@ -0,0 +1,23 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+/dts-v1/;
+
+/ {
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	binman {
+		pre-load {
+			content = <&image>;
+			 algo-name = "sha256,rsa2048";
+			 padding-name = "padding";
+			 key-name = "tools/binman/test/225_dev.key";
+			 header-size = <4096>;
+			 version = <1>;
+		};
+
+		image: blob-ext {
+			filename = "refcode.bin";
+		};
+	};
+};
diff --git a/tools/binman/test/229_pre_load_invalid_sha.dts b/tools/binman/test/229_pre_load_invalid_sha.dts
new file mode 100644
index 0000000000..a2b6725c89
--- /dev/null
+++ b/tools/binman/test/229_pre_load_invalid_sha.dts
@@ -0,0 +1,23 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+/dts-v1/;
+
+/ {
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	binman {
+		pre-load {
+			content = <&image>;
+			 algo-name = "sha2560,rsa2048";
+			 padding-name = "pkcs-1.5";
+			 key-name = "tools/binman/test/225_dev.key";
+			 header-size = <4096>;
+			 version = <1>;
+		};
+
+		image: blob-ext {
+			filename = "refcode.bin";
+		};
+	};
+};
diff --git a/tools/binman/test/230_pre_load_invalid_algo.dts b/tools/binman/test/230_pre_load_invalid_algo.dts
new file mode 100644
index 0000000000..34c8d34f15
--- /dev/null
+++ b/tools/binman/test/230_pre_load_invalid_algo.dts
@@ -0,0 +1,23 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+/dts-v1/;
+
+/ {
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	binman {
+		pre-load {
+			content = <&image>;
+			 algo-name = "sha256,rsa20480";
+			 padding-name = "pkcs-1.5";
+			 key-name = "tools/binman/test/225_dev.key";
+			 header-size = <4096>;
+			 version = <1>;
+		};
+
+		image: blob-ext {
+			filename = "refcode.bin";
+		};
+	};
+};
diff --git a/tools/binman/test/231_pre_load_invalid_key.dts b/tools/binman/test/231_pre_load_invalid_key.dts
new file mode 100644
index 0000000000..08d5a75ddf
--- /dev/null
+++ b/tools/binman/test/231_pre_load_invalid_key.dts
@@ -0,0 +1,23 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+/dts-v1/;
+
+/ {
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	binman {
+		pre-load {
+			content = <&image>;
+			 algo-name = "sha256,rsa4096";
+			 padding-name = "pkcs-1.5";
+			 key-name = "tools/binman/test/225_dev.key";
+			 header-size = <4096>;
+			 version = <1>;
+		};
+
+		image: blob-ext {
+			filename = "refcode.bin";
+		};
+	};
+};
-- 
2.17.1


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

* [PATCH v6 13/16] configs: sandbox_defconfig: enable stage pre-load in bootm
  2022-02-25 14:57 [PATCH v6 00/16] image: add a stage pre-load Philippe Reynes
                   ` (11 preceding siblings ...)
  2022-02-25 14:57 ` [PATCH v6 12/16] tools: binman: add support for pre-load header Philippe Reynes
@ 2022-02-25 14:57 ` Philippe Reynes
  2022-03-03  3:37   ` Simon Glass
  2022-02-25 14:57 ` [PATCH v6 14/16] test: py: vboot: add test for global image signature Philippe Reynes
                   ` (2 subsequent siblings)
  15 siblings, 1 reply; 30+ messages in thread
From: Philippe Reynes @ 2022-02-25 14:57 UTC (permalink / raw)
  To: sjg, rasmus.villemoes; +Cc: u-boot, Philippe Reynes

Enable the support of stage pre-load in bootm.
For the moment, this stage allow to verify the
signature of the full image with a header.

Signed-off-by: Philippe Reynes <philippe.reynes@softathome.com>
---
 configs/sandbox_defconfig | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
index 7ebeb89264..46bf18bc98 100644
--- a/configs/sandbox_defconfig
+++ b/configs/sandbox_defconfig
@@ -27,6 +27,8 @@ CONFIG_AUTOBOOT_SHA256_FALLBACK=y
 CONFIG_AUTOBOOT_NEVER_TIMEOUT=y
 CONFIG_AUTOBOOT_STOP_STR_ENABLE=y
 CONFIG_AUTOBOOT_STOP_STR_CRYPT="$5$rounds=640000$HrpE65IkB8CM5nCL$BKT3QdF98Bo8fJpTr9tjZLZQyzqPASBY20xuK5Rent9"
+CONFIG_IMAGE_PRE_LOAD=y
+CONFIG_IMAGE_PRE_LOAD_SIG=y
 CONFIG_CONSOLE_RECORD=y
 CONFIG_CONSOLE_RECORD_OUT_SIZE=0x1000
 CONFIG_PRE_CONSOLE_BUFFER=y
@@ -37,6 +39,7 @@ CONFIG_STACKPROTECTOR=y
 CONFIG_ANDROID_AB=y
 CONFIG_CMD_CPU=y
 CONFIG_CMD_LICENSE=y
+CONFIG_CMD_BOOTM_PRE_LOAD=y
 CONFIG_CMD_BOOTZ=y
 CONFIG_CMD_BOOTEFI_HELLO=y
 CONFIG_CMD_ABOOTIMG=y
-- 
2.17.1


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

* [PATCH v6 14/16] test: py: vboot: add test for global image signature
  2022-02-25 14:57 [PATCH v6 00/16] image: add a stage pre-load Philippe Reynes
                   ` (12 preceding siblings ...)
  2022-02-25 14:57 ` [PATCH v6 13/16] configs: sandbox_defconfig: enable stage pre-load in bootm Philippe Reynes
@ 2022-02-25 14:57 ` Philippe Reynes
  2022-03-03  3:37   ` Simon Glass
  2022-02-25 14:57 ` [PATCH v6 15/16] cmd: verify: initial import Philippe Reynes
  2022-02-25 14:57 ` [PATCH v6 16/16] configs: sandbox_defconfig: enable config CMD_VERIFY Philippe Reynes
  15 siblings, 1 reply; 30+ messages in thread
From: Philippe Reynes @ 2022-02-25 14:57 UTC (permalink / raw)
  To: sjg, rasmus.villemoes; +Cc: u-boot, Philippe Reynes

Adds test units for the pre-load header signature.

Signed-off-by: Philippe Reynes <philippe.reynes@softathome.com>
---
 test/py/tests/test_vboot.py                   | 125 +++++++++++++++---
 test/py/tests/vboot/sandbox-binman-pss.dts    |  25 ++++
 test/py/tests/vboot/sandbox-binman.dts        |  24 ++++
 .../tests/vboot/sandbox-u-boot-global-pss.dts |  28 ++++
 test/py/tests/vboot/sandbox-u-boot-global.dts |  27 ++++
 test/py/tests/vboot/simple-images.its         |  36 +++++
 6 files changed, 249 insertions(+), 16 deletions(-)
 create mode 100644 test/py/tests/vboot/sandbox-binman-pss.dts
 create mode 100644 test/py/tests/vboot/sandbox-binman.dts
 create mode 100644 test/py/tests/vboot/sandbox-u-boot-global-pss.dts
 create mode 100644 test/py/tests/vboot/sandbox-u-boot-global.dts
 create mode 100644 test/py/tests/vboot/simple-images.its

diff --git a/test/py/tests/test_vboot.py b/test/py/tests/test_vboot.py
index ac8ed9f114..a4a2bb2955 100644
--- a/test/py/tests/test_vboot.py
+++ b/test/py/tests/test_vboot.py
@@ -35,19 +35,21 @@ import vboot_evil
 # Only run the full suite on a few combinations, since it doesn't add any more
 # test coverage.
 TESTDATA = [
-    ['sha1-basic', 'sha1', '', None, False, True, False],
-    ['sha1-pad', 'sha1', '', '-E -p 0x10000', False, False, False],
-    ['sha1-pss', 'sha1', '-pss', None, False, False, False],
-    ['sha1-pss-pad', 'sha1', '-pss', '-E -p 0x10000', False, False, False],
-    ['sha256-basic', 'sha256', '', None, False, False, False],
-    ['sha256-pad', 'sha256', '', '-E -p 0x10000', False, False, False],
-    ['sha256-pss', 'sha256', '-pss', None, False, False, False],
-    ['sha256-pss-pad', 'sha256', '-pss', '-E -p 0x10000', False, False, False],
-    ['sha256-pss-required', 'sha256', '-pss', None, True, False, False],
-    ['sha256-pss-pad-required', 'sha256', '-pss', '-E -p 0x10000', True, True, False],
-    ['sha384-basic', 'sha384', '', None, False, False, False],
-    ['sha384-pad', 'sha384', '', '-E -p 0x10000', False, False, False],
-    ['algo-arg', 'algo-arg', '', '-o sha256,rsa2048', False, False, True],
+    ['sha1-basic', 'sha1', '', None, False, True, False, False],
+    ['sha1-pad', 'sha1', '', '-E -p 0x10000', False, False, False, False],
+    ['sha1-pss', 'sha1', '-pss', None, False, False, False, False],
+    ['sha1-pss-pad', 'sha1', '-pss', '-E -p 0x10000', False, False, False, False],
+    ['sha256-basic', 'sha256', '', None, False, False, False, False],
+    ['sha256-pad', 'sha256', '', '-E -p 0x10000', False, False, False, False],
+    ['sha256-pss', 'sha256', '-pss', None, False, False, False, False],
+    ['sha256-pss-pad', 'sha256', '-pss', '-E -p 0x10000', False, False, False, False],
+    ['sha256-pss-required', 'sha256', '-pss', None, True, False, False, False],
+    ['sha256-pss-pad-required', 'sha256', '-pss', '-E -p 0x10000', True, True, False, False],
+    ['sha384-basic', 'sha384', '', None, False, False, False, False],
+    ['sha384-pad', 'sha384', '', '-E -p 0x10000', False, False, False, False],
+    ['algo-arg', 'algo-arg', '', '-o sha256,rsa2048', False, False, True, False],
+    ['sha256-global-sign', 'sha256', '', '', False, False, False, True],
+    ['sha256-global-sign-pss', 'sha256', '-pss', '', False, False, False, True],
 ]
 
 @pytest.mark.boardspec('sandbox')
@@ -56,10 +58,10 @@ TESTDATA = [
 @pytest.mark.requiredtool('fdtget')
 @pytest.mark.requiredtool('fdtput')
 @pytest.mark.requiredtool('openssl')
-@pytest.mark.parametrize("name,sha_algo,padding,sign_options,required,full_test,algo_arg",
+@pytest.mark.parametrize("name,sha_algo,padding,sign_options,required,full_test,algo_arg,global_sign",
                          TESTDATA)
 def test_vboot(u_boot_console, name, sha_algo, padding, sign_options, required,
-               full_test, algo_arg):
+               full_test, algo_arg, global_sign):
     """Test verified boot signing with mkimage and verification with 'bootm'.
 
     This works using sandbox only as it needs to update the device tree used
@@ -81,6 +83,29 @@ def test_vboot(u_boot_console, name, sha_algo, padding, sign_options, required,
         util.run_and_log(cons, 'dtc %s %s%s -O dtb '
                          '-o %s%s' % (dtc_args, datadir, dts, tmpdir, dtb))
 
+    def dtc_options(dts, options):
+        """Run the device tree compiler to compile a .dts file
+
+        The output file will be the same as the input file but with a .dtb
+        extension.
+
+        Args:
+            dts: Device tree file to compile.
+            options: Options provided to the compiler.
+        """
+        dtb = dts.replace('.dts', '.dtb')
+        util.run_and_log(cons, 'dtc %s %s%s -O dtb '
+                         '-o %s%s %s' % (dtc_args, datadir, dts, tmpdir, dtb, options))
+
+    def run_binman(dtb):
+        """Run binman to build an image
+
+        Args:
+            dtb: Device tree file used as input file.
+        """
+        util.run_and_log(cons, [binman, 'build', '-d', "%s/%s" % (tmpdir,dtb),
+                                '-a', "key-path=%s" % tmpdir, '-O', tmpdir, '-I', tmpdir])
+
     def run_bootm(sha_algo, test_type, expect_string, boots, fit=None):
         """Run a 'bootm' command U-Boot.
 
@@ -139,6 +164,23 @@ def test_vboot(u_boot_console, name, sha_algo, padding, sign_options, required,
         cons.log.action('%s: Sign images' % sha_algo)
         util.run_and_log(cons, args)
 
+    def sign_fit_dtb(sha_algo, options, dtb):
+        """Sign the FIT
+
+        Signs the FIT and writes the signature into it. It also writes the
+        public key into the dtb.
+
+        Args:
+            sha_algo: Either 'sha1' or 'sha256', to select the algorithm to
+                    use.
+            options: Options to provide to mkimage.
+        """
+        args = [mkimage, '-F', '-k', tmpdir, '-K', dtb, '-r', fit]
+        if options:
+            args += options.split(' ')
+        cons.log.action('%s: Sign images' % sha_algo)
+        util.run_and_log(cons, args)
+
     def sign_fit_norequire(sha_algo, options):
         """Sign the FIT
 
@@ -176,6 +218,11 @@ def test_vboot(u_boot_console, name, sha_algo, padding, sign_options, required,
             handle.write(struct.pack(">I", size))
         return struct.unpack(">I", total_size)[0]
 
+    def corrupt_file(fit,offset,value):
+        with open(fit, 'r+b') as handle:
+            handle.seek(offset)
+            handle.write(struct.pack(">I", value))
+
     def create_rsa_pair(name):
         """Generate a new RSA key paid and certificate
 
@@ -374,6 +421,49 @@ def test_vboot(u_boot_console, name, sha_algo, padding, sign_options, required,
                          (dtb))
         run_bootm(sha_algo, 'multi required key', '', False)
 
+    def test_global_sign(sha_algo, padding, sign_options):
+        """Test global image signature with the given hash algorithm and padding.
+
+        Args:
+            sha_algo: Either 'sha1' or 'sha256', to select the algorithm to use
+            padding: Either '' or '-pss', to select the padding to use for the
+                    rsa signature algorithm.
+        """
+
+        dtb = '%ssandbox-u-boot-global%s.dtb' % (tmpdir, padding)
+        cons.config.dtb = dtb
+
+        # Compile our device tree files for kernel and U-Boot. These are
+        # regenerated here since mkimage will modify them (by adding a
+        # public key) below.
+        dtc('sandbox-kernel.dts')
+        dtc_options('sandbox-u-boot-global%s.dts' % padding, '-p 1024')
+
+        # Build the FIT with dev key (keys NOT required). This adds the
+        # signature into sandbox-u-boot.dtb, NOT marked 'required'.
+        make_fit('simple-images.its')
+        sign_fit_dtb(sha_algo, '', dtb)
+
+        # Build the dtb for binman that define the pre-load header
+        # with the global sigature.
+        dtc('sandbox-binman%s.dts' % padding)
+
+        # Run binman to create the final image with the not signed fit
+        # and the pre-load header that contains the global signature.
+        run_binman('sandbox-binman%s.dtb' % padding)
+
+        # Check that the signature is correctly verified by u-boot
+        run_bootm(sha_algo, 'global image signature', 'signature check has succeed', True, "%ssandbox.img" % tmpdir)
+
+        # Corrupt the image (just one byte after the pre-load header)
+        corrupt_file("%ssandbox.img" % tmpdir, 4096, 255);
+
+        # Check that the signature verification fails
+        run_bootm(sha_algo, 'global image signature', 'signature check has failed', False, "%ssandbox.img" % tmpdir)
+
+        # Check that the boot fails if the global signature is not provided
+        run_bootm(sha_algo, 'global image signature', 'signature is mandatory', False)
+
     cons = u_boot_console
     tmpdir = os.path.join(cons.config.result_dir, name) + '/'
     if not os.path.exists(tmpdir):
@@ -381,6 +471,7 @@ def test_vboot(u_boot_console, name, sha_algo, padding, sign_options, required,
     datadir = cons.config.source_dir + '/test/py/tests/vboot/'
     fit = '%stest.fit' % tmpdir
     mkimage = cons.config.build_dir + '/tools/mkimage'
+    binman = cons.config.source_dir + '/tools/binman/binman'
     fit_check_sign = cons.config.build_dir + '/tools/fit_check_sign'
     dtc_args = '-I dts -O dtb -i %s' % tmpdir
     dtb = '%ssandbox-u-boot.dtb' % tmpdir
@@ -403,7 +494,9 @@ def test_vboot(u_boot_console, name, sha_algo, padding, sign_options, required,
         # afterwards.
         old_dtb = cons.config.dtb
         cons.config.dtb = dtb
-        if required:
+        if global_sign:
+            test_global_sign(sha_algo, padding, sign_options)
+        elif required:
             test_required_key(sha_algo, padding, sign_options)
         else:
             test_with_algo(sha_algo, padding, sign_options)
diff --git a/test/py/tests/vboot/sandbox-binman-pss.dts b/test/py/tests/vboot/sandbox-binman-pss.dts
new file mode 100644
index 0000000000..56e3a42fa6
--- /dev/null
+++ b/test/py/tests/vboot/sandbox-binman-pss.dts
@@ -0,0 +1,25 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+/dts-v1/;
+
+/ {
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	binman {
+		filename = "sandbox.img";
+
+		pre-load {
+			 content = <&image>;
+			 algo-name = "sha256,rsa2048";
+			 padding-name = "pss";
+			 key-name = "dev.key";
+			 header-size = <4096>;
+			 version = <1>;
+		};
+
+		image: blob-ext {
+			 filename = "test.fit";
+		};
+	};
+};
diff --git a/test/py/tests/vboot/sandbox-binman.dts b/test/py/tests/vboot/sandbox-binman.dts
new file mode 100644
index 0000000000..b24aeba0fa
--- /dev/null
+++ b/test/py/tests/vboot/sandbox-binman.dts
@@ -0,0 +1,24 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+/dts-v1/;
+
+/ {
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	binman {
+		filename = "sandbox.img";
+
+		pre-load {
+			 content = <&image>;
+			 algo-name = "sha256,rsa2048";
+			 key-name = "dev.key";
+			 header-size = <4096>;
+			 version = <1>;
+		};
+
+		image: blob-ext {
+			 filename = "test.fit";
+		};
+	};
+};
diff --git a/test/py/tests/vboot/sandbox-u-boot-global-pss.dts b/test/py/tests/vboot/sandbox-u-boot-global-pss.dts
new file mode 100644
index 0000000000..c59a68221b
--- /dev/null
+++ b/test/py/tests/vboot/sandbox-u-boot-global-pss.dts
@@ -0,0 +1,28 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+/dts-v1/;
+
+/ {
+	model = "Sandbox Verified Boot Test";
+	compatible = "sandbox";
+
+	binman {
+	};
+
+	reset@0 {
+		compatible = "sandbox,reset";
+	};
+
+	image {
+		pre-load {
+			sig {
+				algo-name = "sha256,rsa2048";
+				padding-name = "pss";
+				signature-size = <256>;
+				mandatory = "yes";
+
+				key-name = "dev";
+			};
+		};
+	};
+};
diff --git a/test/py/tests/vboot/sandbox-u-boot-global.dts b/test/py/tests/vboot/sandbox-u-boot-global.dts
new file mode 100644
index 0000000000..1409f9e1a1
--- /dev/null
+++ b/test/py/tests/vboot/sandbox-u-boot-global.dts
@@ -0,0 +1,27 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+/dts-v1/;
+
+/ {
+	model = "Sandbox Verified Boot Test";
+	compatible = "sandbox";
+
+	binman {
+	};
+
+	reset@0 {
+		compatible = "sandbox,reset";
+	};
+
+	image {
+		pre-load {
+			sig {
+				algo-name = "sha256,rsa2048";
+				signature-size = <256>;
+				mandatory = "yes";
+
+				key-name = "dev";
+			};
+		};
+	};
+};
diff --git a/test/py/tests/vboot/simple-images.its b/test/py/tests/vboot/simple-images.its
new file mode 100644
index 0000000000..f62786456b
--- /dev/null
+++ b/test/py/tests/vboot/simple-images.its
@@ -0,0 +1,36 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+/dts-v1/;
+
+/ {
+	description = "Chrome OS kernel image with one or more FDT blobs";
+	#address-cells = <1>;
+
+	images {
+		kernel {
+			data = /incbin/("test-kernel.bin");
+			type = "kernel_noload";
+			arch = "sandbox";
+			os = "linux";
+			compression = "none";
+			load = <0x4>;
+			entry = <0x8>;
+			kernel-version = <1>;
+		};
+		fdt-1 {
+			description = "snow";
+			data = /incbin/("sandbox-kernel.dtb");
+			type = "flat_dt";
+			arch = "sandbox";
+			compression = "none";
+			fdt-version = <1>;
+		};
+	};
+	configurations {
+		default = "conf-1";
+		conf-1 {
+			kernel = "kernel";
+			fdt = "fdt-1";
+		};
+	};
+};
-- 
2.17.1


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

* [PATCH v6 15/16] cmd: verify: initial import
  2022-02-25 14:57 [PATCH v6 00/16] image: add a stage pre-load Philippe Reynes
                   ` (13 preceding siblings ...)
  2022-02-25 14:57 ` [PATCH v6 14/16] test: py: vboot: add test for global image signature Philippe Reynes
@ 2022-02-25 14:57 ` Philippe Reynes
  2022-03-03  3:37   ` Simon Glass
  2022-02-25 14:57 ` [PATCH v6 16/16] configs: sandbox_defconfig: enable config CMD_VERIFY Philippe Reynes
  15 siblings, 1 reply; 30+ messages in thread
From: Philippe Reynes @ 2022-02-25 14:57 UTC (permalink / raw)
  To: sjg, rasmus.villemoes; +Cc: u-boot, Philippe Reynes

Add the command verify that check the signature of
an image with the pre-load header. If the check
succeed, the u-boot env variable 'loadaddr_verified'
is set to the address of the image (without the header).

It allows to run such commands:
tftp script.img && verify $loadaddr && source $loadaddr_verified

Signed-off-by: Philippe Reynes <philippe.reynes@softathome.com>
---
 cmd/Kconfig  |  7 +++++++
 cmd/Makefile |  1 +
 cmd/verify.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 61 insertions(+)
 create mode 100644 cmd/verify.c

diff --git a/cmd/Kconfig b/cmd/Kconfig
index 87aa3fb11a..0460d5c3a0 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -428,6 +428,13 @@ config CMD_THOR_DOWNLOAD
 	  There is no documentation about this within the U-Boot source code
 	  but you should be able to find something on the interwebs.
 
+config CMD_VERIFY
+	bool "verify the global signature"
+        depends on CMD_BOOTM_PRE_LOAD
+	help
+	  Verify the signature provided in a pre-load header of
+	  a full image.
+
 config CMD_ZBOOT
 	bool "zboot - x86 boot command"
 	help
diff --git a/cmd/Makefile b/cmd/Makefile
index 166c652d98..80e054e806 100644
--- a/cmd/Makefile
+++ b/cmd/Makefile
@@ -177,6 +177,7 @@ obj-$(CONFIG_CMD_THOR_DOWNLOAD) += thordown.o
 obj-$(CONFIG_CMD_XIMG) += ximg.o
 obj-$(CONFIG_CMD_YAFFS2) += yaffs2.o
 obj-$(CONFIG_CMD_SPL) += spl.o
+obj-$(CONFIG_CMD_VERIFY) += verify.o
 obj-$(CONFIG_CMD_W1) += w1.o
 obj-$(CONFIG_CMD_ZIP) += zip.o
 obj-$(CONFIG_CMD_ZFS) += zfs.o
diff --git a/cmd/verify.c b/cmd/verify.c
new file mode 100644
index 0000000000..4d055e0790
--- /dev/null
+++ b/cmd/verify.c
@@ -0,0 +1,53 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2022 Philippe Reynes <philippe.reynes@softathome.com>
+ */
+
+#include <common.h>
+#include <env.h>
+#include <image.h>
+#include <mapmem.h>
+
+static ulong verify_get_addr(int argc, char *const argv[])
+{
+	ulong addr;
+
+	if (argc > 0)
+		addr = simple_strtoul(argv[0], NULL, 16);
+	else
+		addr = image_load_addr;
+
+	return addr;
+}
+
+static int do_verify(struct cmd_tbl *cmdtp, int flag, int argc,
+		     char *const argv[])
+{
+	ulong addr = verify_get_addr(argc, argv);
+	int ret = 0;
+
+	argc--; argv++;
+
+	addr = verify_get_addr(argc, argv);
+
+	if (CONFIG_IS_ENABLED(CMD_BOOTM_PRE_LOAD)) {
+		ret = image_pre_load(addr);
+
+		if (ret) {
+			ret = CMD_RET_FAILURE;
+			goto out;
+		}
+
+		env_set_hex("loadaddr_verified", addr + image_load_offset);
+	}
+
+ out:
+	return ret;
+}
+
+U_BOOT_CMD(verify, 2, 1, do_verify,
+	   "verify the global signature provided in the pre-load header,\n"
+	   "\tif the check succeed, the u-boot env variable loadaddr_verified\n"
+	   "\tis set to the address of the image (without the header)",
+	   "<image addr>"
+);
-- 
2.17.1


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

* [PATCH v6 16/16] configs: sandbox_defconfig: enable config CMD_VERIFY
  2022-02-25 14:57 [PATCH v6 00/16] image: add a stage pre-load Philippe Reynes
                   ` (14 preceding siblings ...)
  2022-02-25 14:57 ` [PATCH v6 15/16] cmd: verify: initial import Philippe Reynes
@ 2022-02-25 14:57 ` Philippe Reynes
  2022-03-03  3:37   ` Simon Glass
  15 siblings, 1 reply; 30+ messages in thread
From: Philippe Reynes @ 2022-02-25 14:57 UTC (permalink / raw)
  To: sjg, rasmus.villemoes; +Cc: u-boot, Philippe Reynes

Enable the command verify on sandbox.

Signed-off-by: Philippe Reynes <philippe.reynes@softathome.com>
---
 configs/sandbox_defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
index 46bf18bc98..a56aa92f94 100644
--- a/configs/sandbox_defconfig
+++ b/configs/sandbox_defconfig
@@ -44,6 +44,7 @@ CONFIG_CMD_BOOTZ=y
 CONFIG_CMD_BOOTEFI_HELLO=y
 CONFIG_CMD_ABOOTIMG=y
 # CONFIG_CMD_ELF is not set
+CONFIG_CMD_VERIFY=y
 CONFIG_CMD_ASKENV=y
 CONFIG_CMD_GREPENV=y
 CONFIG_CMD_ERASEENV=y
-- 
2.17.1


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

* Re: [PATCH v6 04/16] lib: allow to build asn1 decoder and oid registry in SPL
  2022-02-25 14:57 ` [PATCH v6 04/16] lib: allow to build asn1 decoder and oid registry in SPL Philippe Reynes
@ 2022-03-03  3:37   ` Simon Glass
  0 siblings, 0 replies; 30+ messages in thread
From: Simon Glass @ 2022-03-03  3:37 UTC (permalink / raw)
  To: Philippe Reynes; +Cc: rasmus.villemoes, u-boot

On Fri, 25 Feb 2022 at 07:58, Philippe Reynes
<philippe.reynes@softathome.com> wrote:
>
> This commit adds the options:
> - SPL_ASN1_DECODER
> - SPL_OID_REGISTRY
>
> Signed-off-by: Philippe Reynes <philippe.reynes@softathome.com>
> ---
>  lib/Kconfig  | 19 +++++++++++++++++++
>  lib/Makefile |  4 ++--
>  2 files changed, 21 insertions(+), 2 deletions(-)

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

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

* Re: [PATCH v6 12/16] tools: binman: add support for pre-load header
  2022-02-25 14:57 ` [PATCH v6 12/16] tools: binman: add support for pre-load header Philippe Reynes
@ 2022-03-03  3:37   ` Simon Glass
  0 siblings, 0 replies; 30+ messages in thread
From: Simon Glass @ 2022-03-03  3:37 UTC (permalink / raw)
  To: Philippe Reynes; +Cc: rasmus.villemoes, u-boot

Hi Philippe,

On Fri, 25 Feb 2022 at 07:58, Philippe Reynes
<philippe.reynes@softathome.com> wrote:
>
> Adds the support of the pre-load header with the image signature
> to binman.
>
> Signed-off-by: Philippe Reynes <philippe.reynes@softathome.com>
> ---
>  tools/binman/etype/pre_load.py                | 165 ++++++++++++++++++
>  tools/binman/ftest.py                         |  45 +++++
>  tools/binman/test/225_dev.key                 |  28 +++
>  tools/binman/test/225_pre_load.dts            |  22 +++
>  tools/binman/test/226_pre_load_pkcs.dts       |  23 +++
>  tools/binman/test/227_pre_load_pss.dts        |  23 +++
>  .../test/228_pre_load_invalid_padding.dts     |  23 +++
>  .../binman/test/229_pre_load_invalid_sha.dts  |  23 +++
>  .../binman/test/230_pre_load_invalid_algo.dts |  23 +++
>  .../binman/test/231_pre_load_invalid_key.dts  |  23 +++
>  10 files changed, 398 insertions(+)
>  create mode 100644 tools/binman/etype/pre_load.py
>  create mode 100644 tools/binman/test/225_dev.key
>  create mode 100644 tools/binman/test/225_pre_load.dts
>  create mode 100644 tools/binman/test/226_pre_load_pkcs.dts
>  create mode 100644 tools/binman/test/227_pre_load_pss.dts
>  create mode 100644 tools/binman/test/228_pre_load_invalid_padding.dts
>  create mode 100644 tools/binman/test/229_pre_load_invalid_sha.dts
>  create mode 100644 tools/binman/test/230_pre_load_invalid_algo.dts
>  create mode 100644 tools/binman/test/231_pre_load_invalid_key.dts

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

But lots of nits below sorry

>
> diff --git a/tools/binman/etype/pre_load.py b/tools/binman/etype/pre_load.py
> new file mode 100644
> index 0000000000..2af2857404
> --- /dev/null
> +++ b/tools/binman/etype/pre_load.py
> @@ -0,0 +1,165 @@
> +# SPDX-License-Identifier: GPL-2.0+
> +# Copyright (c) 2022 Softathome
> +# Written by Philippe Reynes <philippe.reynes@softathome.com>
> +#
> +# Entry-type for the global header
> +#
> +
> +import struct
> +from dtoc import fdt_util
> +from patman import tools
> +
> +from binman.entry import Entry
> +from binman.etype.collection import Entry_collection
> +from binman.entry import EntryArg
> +
> +from Cryptodome.Hash import SHA256, SHA384, SHA512
> +from Cryptodome.PublicKey import RSA
> +from Cryptodome.Signature import pkcs1_15
> +from Cryptodome.Signature import pss
> +
> +PRE_LOAD_MAGIC = b'UBSH'
> +
> +RSAS = {
> +    'rsa1024': 1024 / 8,
> +    'rsa2048': 2048 / 8,
> +    'rsa4096': 4096 / 8
> +}
> +
> +SHAS = {
> +    'sha256': SHA256,
> +    'sha384': SHA384,
> +    'sha512': SHA512
> +}
> +
> +class Entry_pre_load(Entry_collection):
> +    """Pre load image header

Run binman entry-docs >tools/binman/entries.rst to regen that file

> +
> +    Properties / Entry arguments:
> +        - key-path: Path of the directory that store key (provided by the environment variable KEY_PATH)
> +        - content: List of phandles to entries to sign
> +        - algo-name: Hash and signature algo to use for the signature
> +        - padding-name: Name of the padding (pkcs-1.5 or pss)
> +        - key-name: Filename of the private key to sign
> +        - header-size: Total size of the header
> +        - version: Version of the header
> +
> +    This entry create a pre-load header that contain a global

creates

contains

> +    image signature.
> +
> +    For example, this creates an image with a pre-load header and a binary::
> +
> +        binman {
> +           image2 {
> +               filename = "sandbox.bin";
> +
> +               pre-load {
> +                   content = <&image>;
> +                   algo-name = "sha256,rsa2048";
> +                   padding-name = "pss";
> +                   key-name = "private.pem";
> +                   header-size = <4096>;
> +                   version = <1>;
> +               };
> +
> +               image: blob-ext {
> +                   filename = "sandbox.itb";
> +               };
> +           };
> +        };
> +    """
> +
> +    def __init__(self, section, etype, node):
> +        super().__init__(section, etype, node)
> +        self.algo_name = fdt_util.GetString(self._node, 'algo-name')
> +        self.padding_name = fdt_util.GetString(self._node, 'padding-name')
> +        self.key_name = fdt_util.GetString(self._node, 'key-name')
> +        self.header_size = fdt_util.GetInt(self._node, 'header-size')
> +        self.version = fdt_util.GetInt(self._node, 'version')
> +
> +    def _CreateHeader(self):
> +        """Create a pre load header"""
> +        hash_name, sign_name  = self.algo_name.split(',')
> +        padding_name=self.padding_name
> +        key_path, = self.GetEntryArgsOrProps([EntryArg('key-path', str)])

Please read this into self.key_path in a ReadNode() method. We try to
do all the reading in one place. See other entry types for examples.

> +        if key_path is None or key_path == "":

Please use single quotes by default

> +            key_name = self.key_name
> +        else:
> +            key_name = key_path + "/" + self.key_name

key_name = os.path.join(key_path, self.key_name)

(then you don't need the 'if')

> +
> +        # Check hash and signature name/type
> +        if hash_name not in SHAS:
> +            raise ValueError(hash_name + " is not supported")

self.Raise(...

> +        if sign_name not in RSAS:
> +            raise ValueError(sign_name + "is not supported")

self.Raise(...

> +
> +        # Read the key
> +        with open(key_name, 'rb') as pem:
> +            key = RSA.import_key(pem.read())
> +
> +        # Check if the key has the expected size
> +        if key.size_in_bytes() != RSAS[sign_name]:
> +            raise ValueError("The key " + self.key_name + " don't have the expected size")

self.Raise(...

(the reason for that is we want to report the node that had the problem)

> +
> +        # Compute the hash
> +        hash_image = SHAS[hash_name].new()
> +        hash_image.update(self.image)
> +
> +        # Compute the signature
> +        if padding_name is None:
> +            padding_name = "pkcs-1.5"
> +        if padding_name == "pss":
> +            salt_len = key.size_in_bytes() - hash_image.digest_size - 2
> +            padding = pss
> +            padding_args = {'salt_bytes': salt_len}
> +        elif padding_name == "pkcs-1.5":
> +            padding = pkcs1_15
> +            padding_args = {}
> +        else:
> +            raise ValueError(padding_name + " is not supported")
> +
> +        sig = padding.new(key, **padding_args).sign(hash_image)
> +
> +        hash_sig = SHA256.new()
> +        hash_sig.update(sig)
> +
> +        version = self.version
> +        header_size = self.header_size
> +        image_size = len(self.image)
> +        ofs_img_sig = 64 + len(sig)
> +        flags = 0
> +        reserved0 = 0
> +        reserved1 = 0
> +
> +        first_header = bytearray(64)
> +        struct.pack_into('4s', first_header, 0, PRE_LOAD_MAGIC)
> +        struct.pack_into('>I', first_header, 4, version)
> +        struct.pack_into('>I', first_header, 8, header_size)
> +        struct.pack_into('>I', first_header, 12, image_size)
> +        struct.pack_into('>I', first_header, 16, ofs_img_sig)
> +        struct.pack_into('>I', first_header, 20, flags)
> +        struct.pack_into('>I', first_header, 24, reserved0)
> +        struct.pack_into('>I', first_header, 28, reserved1)
> +        struct.pack_into('32s', first_header, 32, hash_sig.digest())

Can you do

 first_header = struct.pack('>4sIIIII...', PRE_LOAD_MAGIC, version,
header_size, ...

> +
> +        hash_first_header = SHAS[hash_name].new()
> +        hash_first_header.update(first_header)
> +        sig_first_header = padding.new(key, **padding_args).sign(hash_first_header)
> +
> +        data = first_header + sig_first_header + sig
> +        pad  = bytearray(self.header_size - len(data))
> +
> +        return data + pad
> +
> +    def ObtainContents(self):
> +        """Obtain a placeholder for the header contents"""
> +        # wait that the image is available
> +        self.image = self.GetContents(False)
> +        if self.image is None:
> +            return False
> +        self.SetContents(self._CreateHeader())
> +        return True
> +
> +    def ProcessContents(self):
> +        data = self._CreateHeader()
> +        return self.ProcessContentsUpdate(data)
> diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
> index 8f00db6945..06b8546354 100644
> --- a/tools/binman/ftest.py
> +++ b/tools/binman/ftest.py
> @@ -91,6 +91,9 @@ SCP_DATA              = b'scp'
>  TEST_FDT1_DATA        = b'fdt1'
>  TEST_FDT2_DATA        = b'test-fdt2'
>  ENV_DATA              = b'var1=1\nvar2="2"'
> +PRE_LOAD_MAGIC        = b'UBSH'
> +PRE_LOAD_VERSION      = 0x11223344.to_bytes(4, 'big')
> +PRE_LOAD_HDR_SIZE     = 0x00001000.to_bytes(4, 'big')
>
>  # Subdirectory of the input dir to use to put test FDTs
>  TEST_FDT_SUBDIR       = 'fdts'
> @@ -5321,6 +5324,48 @@ fdt         fdtmap                Extract the devicetree blob from the fdtmap
>          self.assertIn("Node '/binman/fit': Unknown operation 'unknown'",
>                        str(exc.exception))
>
> +    def testPreLoad(self):
> +        """Test an image with a pre-load header"""
> +        entry_args = {
> +            'key-path': '.',
> +        }
> +        data, _, _, _ = self._DoReadFileDtb('225_pre_load.dts',
> +                                            entry_args=entry_args)
> +        self.assertEqual(PRE_LOAD_MAGIC, data[:len(PRE_LOAD_MAGIC)])
> +        self.assertEqual(PRE_LOAD_VERSION, data[4:4+len(PRE_LOAD_VERSION)])
> +        self.assertEqual(PRE_LOAD_HDR_SIZE, data[8:8+len(PRE_LOAD_HDR_SIZE)])
> +        data = self._DoReadFile('225_pre_load.dts')

Please put each .dts file in its own test function, just because that
is how it currently works.

> +        self.assertEqual(PRE_LOAD_MAGIC, data[:len(PRE_LOAD_MAGIC)])
> +        self.assertEqual(PRE_LOAD_VERSION, data[4:4+len(PRE_LOAD_VERSION)])
> +        self.assertEqual(PRE_LOAD_HDR_SIZE, data[8:8+len(PRE_LOAD_HDR_SIZE)])
> +        data = self._DoReadFile('226_pre_load_pkcs.dts')
> +        self.assertEqual(PRE_LOAD_MAGIC, data[:len(PRE_LOAD_MAGIC)])
> +        self.assertEqual(PRE_LOAD_VERSION, data[4:4+len(PRE_LOAD_VERSION)])
> +        self.assertEqual(PRE_LOAD_HDR_SIZE, data[8:8+len(PRE_LOAD_HDR_SIZE)])
> +        data = self._DoReadFile('227_pre_load_pss.dts')
> +        self.assertEqual(PRE_LOAD_MAGIC, data[:len(PRE_LOAD_MAGIC)])
> +        self.assertEqual(PRE_LOAD_VERSION, data[4:4+len(PRE_LOAD_VERSION)])
> +        self.assertEqual(PRE_LOAD_HDR_SIZE, data[8:8+len(PRE_LOAD_HDR_SIZE)])

Spaces around + please

Regards,
Simon

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

* Re: [PATCH v6 13/16] configs: sandbox_defconfig: enable stage pre-load in bootm
  2022-02-25 14:57 ` [PATCH v6 13/16] configs: sandbox_defconfig: enable stage pre-load in bootm Philippe Reynes
@ 2022-03-03  3:37   ` Simon Glass
  0 siblings, 0 replies; 30+ messages in thread
From: Simon Glass @ 2022-03-03  3:37 UTC (permalink / raw)
  To: Philippe Reynes; +Cc: rasmus.villemoes, u-boot

On Fri, 25 Feb 2022 at 07:58, Philippe Reynes
<philippe.reynes@softathome.com> wrote:
>
> Enable the support of stage pre-load in bootm.
> For the moment, this stage allow to verify the
> signature of the full image with a header.
>
> Signed-off-by: Philippe Reynes <philippe.reynes@softathome.com>
> ---
>  configs/sandbox_defconfig | 3 +++
>  1 file changed, 3 insertions(+)
>

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

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

* Re: [PATCH v6 14/16] test: py: vboot: add test for global image signature
  2022-02-25 14:57 ` [PATCH v6 14/16] test: py: vboot: add test for global image signature Philippe Reynes
@ 2022-03-03  3:37   ` Simon Glass
  0 siblings, 0 replies; 30+ messages in thread
From: Simon Glass @ 2022-03-03  3:37 UTC (permalink / raw)
  To: Philippe Reynes; +Cc: rasmus.villemoes, u-boot

Hi Philippe,

On Fri, 25 Feb 2022 at 07:58, Philippe Reynes
<philippe.reynes@softathome.com> wrote:
>
> Adds test units for the pre-load header signature.
>
> Signed-off-by: Philippe Reynes <philippe.reynes@softathome.com>
> ---
>  test/py/tests/test_vboot.py                   | 125 +++++++++++++++---
>  test/py/tests/vboot/sandbox-binman-pss.dts    |  25 ++++
>  test/py/tests/vboot/sandbox-binman.dts        |  24 ++++
>  .../tests/vboot/sandbox-u-boot-global-pss.dts |  28 ++++
>  test/py/tests/vboot/sandbox-u-boot-global.dts |  27 ++++
>  test/py/tests/vboot/simple-images.its         |  36 +++++
>  6 files changed, 249 insertions(+), 16 deletions(-)
>  create mode 100644 test/py/tests/vboot/sandbox-binman-pss.dts
>  create mode 100644 test/py/tests/vboot/sandbox-binman.dts
>  create mode 100644 test/py/tests/vboot/sandbox-u-boot-global-pss.dts
>  create mode 100644 test/py/tests/vboot/sandbox-u-boot-global.dts
>  create mode 100644 test/py/tests/vboot/simple-images.its
>
> diff --git a/test/py/tests/test_vboot.py b/test/py/tests/test_vboot.py
> index ac8ed9f114..a4a2bb2955 100644
> --- a/test/py/tests/test_vboot.py
> +++ b/test/py/tests/test_vboot.py
> @@ -35,19 +35,21 @@ import vboot_evil

please add to the comment at the top of the file since you have added a new way

>  # Only run the full suite on a few combinations, since it doesn't add any more
>  # test coverage.
>  TESTDATA = [
> -    ['sha1-basic', 'sha1', '', None, False, True, False],
> -    ['sha1-pad', 'sha1', '', '-E -p 0x10000', False, False, False],
> -    ['sha1-pss', 'sha1', '-pss', None, False, False, False],
> -    ['sha1-pss-pad', 'sha1', '-pss', '-E -p 0x10000', False, False, False],
> -    ['sha256-basic', 'sha256', '', None, False, False, False],
> -    ['sha256-pad', 'sha256', '', '-E -p 0x10000', False, False, False],
> -    ['sha256-pss', 'sha256', '-pss', None, False, False, False],
> -    ['sha256-pss-pad', 'sha256', '-pss', '-E -p 0x10000', False, False, False],
> -    ['sha256-pss-required', 'sha256', '-pss', None, True, False, False],
> -    ['sha256-pss-pad-required', 'sha256', '-pss', '-E -p 0x10000', True, True, False],
> -    ['sha384-basic', 'sha384', '', None, False, False, False],
> -    ['sha384-pad', 'sha384', '', '-E -p 0x10000', False, False, False],
> -    ['algo-arg', 'algo-arg', '', '-o sha256,rsa2048', False, False, True],
> +    ['sha1-basic', 'sha1', '', None, False, True, False, False],
> +    ['sha1-pad', 'sha1', '', '-E -p 0x10000', False, False, False, False],
> +    ['sha1-pss', 'sha1', '-pss', None, False, False, False, False],
> +    ['sha1-pss-pad', 'sha1', '-pss', '-E -p 0x10000', False, False, False, False],
> +    ['sha256-basic', 'sha256', '', None, False, False, False, False],
> +    ['sha256-pad', 'sha256', '', '-E -p 0x10000', False, False, False, False],
> +    ['sha256-pss', 'sha256', '-pss', None, False, False, False, False],
> +    ['sha256-pss-pad', 'sha256', '-pss', '-E -p 0x10000', False, False, False, False],
> +    ['sha256-pss-required', 'sha256', '-pss', None, True, False, False, False],
> +    ['sha256-pss-pad-required', 'sha256', '-pss', '-E -p 0x10000', True, True, False, False],
> +    ['sha384-basic', 'sha384', '', None, False, False, False, False],
> +    ['sha384-pad', 'sha384', '', '-E -p 0x10000', False, False, False, False],
> +    ['algo-arg', 'algo-arg', '', '-o sha256,rsa2048', False, False, True, False],
> +    ['sha256-global-sign', 'sha256', '', '', False, False, False, True],
> +    ['sha256-global-sign-pss', 'sha256', '-pss', '', False, False, False, True],
>  ]
>
>  @pytest.mark.boardspec('sandbox')
> @@ -56,10 +58,10 @@ TESTDATA = [
>  @pytest.mark.requiredtool('fdtget')
>  @pytest.mark.requiredtool('fdtput')
>  @pytest.mark.requiredtool('openssl')
> -@pytest.mark.parametrize("name,sha_algo,padding,sign_options,required,full_test,algo_arg",
> +@pytest.mark.parametrize("name,sha_algo,padding,sign_options,required,full_test,algo_arg,global_sign",
>                           TESTDATA)
>  def test_vboot(u_boot_console, name, sha_algo, padding, sign_options, required,
> -               full_test, algo_arg):
> +               full_test, algo_arg, global_sign):
>      """Test verified boot signing with mkimage and verification with 'bootm'.
>
>      This works using sandbox only as it needs to update the device tree used
> @@ -81,6 +83,29 @@ def test_vboot(u_boot_console, name, sha_algo, padding, sign_options, required,
>          util.run_and_log(cons, 'dtc %s %s%s -O dtb '
>                           '-o %s%s' % (dtc_args, datadir, dts, tmpdir, dtb))
>
> +    def dtc_options(dts, options):
> +        """Run the device tree compiler to compile a .dts file
> +
> +        The output file will be the same as the input file but with a .dtb
> +        extension.
> +
> +        Args:
> +            dts: Device tree file to compile.
> +            options: Options provided to the compiler.
> +        """
> +        dtb = dts.replace('.dts', '.dtb')
> +        util.run_and_log(cons, 'dtc %s %s%s -O dtb '
> +                         '-o %s%s %s' % (dtc_args, datadir, dts, tmpdir, dtb, options))
> +
> +    def run_binman(dtb):
> +        """Run binman to build an image
> +
> +        Args:
> +            dtb: Device tree file used as input file.
> +        """
> +        util.run_and_log(cons, [binman, 'build', '-d', "%s/%s" % (tmpdir,dtb),
> +                                '-a', "key-path=%s" % tmpdir, '-O', tmpdir, '-I', tmpdir])
> +
>      def run_bootm(sha_algo, test_type, expect_string, boots, fit=None):
>          """Run a 'bootm' command U-Boot.
>
> @@ -139,6 +164,23 @@ def test_vboot(u_boot_console, name, sha_algo, padding, sign_options, required,
>          cons.log.action('%s: Sign images' % sha_algo)
>          util.run_and_log(cons, args)
>
> +    def sign_fit_dtb(sha_algo, options, dtb):
> +        """Sign the FIT
> +
> +        Signs the FIT and writes the signature into it. It also writes the
> +        public key into the dtb.
> +
> +        Args:
> +            sha_algo: Either 'sha1' or 'sha256', to select the algorithm to
> +                    use.
> +            options: Options to provide to mkimage.
> +        """
> +        args = [mkimage, '-F', '-k', tmpdir, '-K', dtb, '-r', fit]
> +        if options:
> +            args += options.split(' ')
> +        cons.log.action('%s: Sign images' % sha_algo)
> +        util.run_and_log(cons, args)
> +
>      def sign_fit_norequire(sha_algo, options):
>          """Sign the FIT
>
> @@ -176,6 +218,11 @@ def test_vboot(u_boot_console, name, sha_algo, padding, sign_options, required,
>              handle.write(struct.pack(">I", size))
>          return struct.unpack(">I", total_size)[0]
>
> +    def corrupt_file(fit,offset,value):

spaces after ,

also please comment the function

> +        with open(fit, 'r+b') as handle:
> +            handle.seek(offset)
> +            handle.write(struct.pack(">I", value))
> +
>      def create_rsa_pair(name):
>          """Generate a new RSA key paid and certificate
>
> @@ -374,6 +421,49 @@ def test_vboot(u_boot_console, name, sha_algo, padding, sign_options, required,
>                           (dtb))
>          run_bootm(sha_algo, 'multi required key', '', False)
>
> +    def test_global_sign(sha_algo, padding, sign_options):
> +        """Test global image signature with the given hash algorithm and padding.
> +
> +        Args:
> +            sha_algo: Either 'sha1' or 'sha256', to select the algorithm to use
> +            padding: Either '' or '-pss', to select the padding to use for the
> +                    rsa signature algorithm.
> +        """
> +
> +        dtb = '%ssandbox-u-boot-global%s.dtb' % (tmpdir, padding)
> +        cons.config.dtb = dtb
> +
> +        # Compile our device tree files for kernel and U-Boot. These are
> +        # regenerated here since mkimage will modify them (by adding a
> +        # public key) below.
> +        dtc('sandbox-kernel.dts')
> +        dtc_options('sandbox-u-boot-global%s.dts' % padding, '-p 1024')
> +
> +        # Build the FIT with dev key (keys NOT required). This adds the
> +        # signature into sandbox-u-boot.dtb, NOT marked 'required'.
> +        make_fit('simple-images.its')
> +        sign_fit_dtb(sha_algo, '', dtb)
> +
> +        # Build the dtb for binman that define the pre-load header
> +        # with the global sigature.
> +        dtc('sandbox-binman%s.dts' % padding)
> +
> +        # Run binman to create the final image with the not signed fit
> +        # and the pre-load header that contains the global signature.
> +        run_binman('sandbox-binman%s.dtb' % padding)
> +
> +        # Check that the signature is correctly verified by u-boot
> +        run_bootm(sha_algo, 'global image signature', 'signature check has succeed', True, "%ssandbox.img" % tmpdir)

make sure to wrap to 80 chars

> +
> +        # Corrupt the image (just one byte after the pre-load header)
> +        corrupt_file("%ssandbox.img" % tmpdir, 4096, 255);
> +
> +        # Check that the signature verification fails
> +        run_bootm(sha_algo, 'global image signature', 'signature check has failed', False, "%ssandbox.img" % tmpdir)
> +
> +        # Check that the boot fails if the global signature is not provided
> +        run_bootm(sha_algo, 'global image signature', 'signature is mandatory', False)
> +
>      cons = u_boot_console
>      tmpdir = os.path.join(cons.config.result_dir, name) + '/'
>      if not os.path.exists(tmpdir):
> @@ -381,6 +471,7 @@ def test_vboot(u_boot_console, name, sha_algo, padding, sign_options, required,
>      datadir = cons.config.source_dir + '/test/py/tests/vboot/'
>      fit = '%stest.fit' % tmpdir
>      mkimage = cons.config.build_dir + '/tools/mkimage'
> +    binman = cons.config.source_dir + '/tools/binman/binman'
>      fit_check_sign = cons.config.build_dir + '/tools/fit_check_sign'
>      dtc_args = '-I dts -O dtb -i %s' % tmpdir
>      dtb = '%ssandbox-u-boot.dtb' % tmpdir
> @@ -403,7 +494,9 @@ def test_vboot(u_boot_console, name, sha_algo, padding, sign_options, required,
>          # afterwards.
>          old_dtb = cons.config.dtb
>          cons.config.dtb = dtb
> -        if required:
> +        if global_sign:
> +            test_global_sign(sha_algo, padding, sign_options)

do you not want to also test the normal signature?

> +        elif required:
>              test_required_key(sha_algo, padding, sign_options)
>          else:
>              test_with_algo(sha_algo, padding, sign_options)
> diff --git a/test/py/tests/vboot/sandbox-binman-pss.dts b/test/py/tests/vboot/sandbox-binman-pss.dts
> new file mode 100644
> index 0000000000..56e3a42fa6
> --- /dev/null

[..]

Regards,
Simon

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

* Re: [PATCH v6 15/16] cmd: verify: initial import
  2022-02-25 14:57 ` [PATCH v6 15/16] cmd: verify: initial import Philippe Reynes
@ 2022-03-03  3:37   ` Simon Glass
  2022-03-10 16:53     ` Philippe REYNES
  0 siblings, 1 reply; 30+ messages in thread
From: Simon Glass @ 2022-03-03  3:37 UTC (permalink / raw)
  To: Philippe Reynes; +Cc: rasmus.villemoes, u-boot

Hi Philippe,

On Fri, 25 Feb 2022 at 07:58, Philippe Reynes
<philippe.reynes@softathome.com> wrote:
>
> Add the command verify that check the signature of
> an image with the pre-load header. If the check
> succeed, the u-boot env variable 'loadaddr_verified'
> is set to the address of the image (without the header).
>
> It allows to run such commands:
> tftp script.img && verify $loadaddr && source $loadaddr_verified
>
> Signed-off-by: Philippe Reynes <philippe.reynes@softathome.com>
> ---
>  cmd/Kconfig  |  7 +++++++
>  cmd/Makefile |  1 +
>  cmd/verify.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 61 insertions(+)
>  create mode 100644 cmd/verify.c
>

Using the 'verify' command seems a bit vague. Could it be a
sub-command of bootm perhaps?

> diff --git a/cmd/Kconfig b/cmd/Kconfig
> index 87aa3fb11a..0460d5c3a0 100644
> --- a/cmd/Kconfig
> +++ b/cmd/Kconfig
> @@ -428,6 +428,13 @@ config CMD_THOR_DOWNLOAD
>           There is no documentation about this within the U-Boot source code
>           but you should be able to find something on the interwebs.
>
> +config CMD_VERIFY
> +       bool "verify the global signature"
> +        depends on CMD_BOOTM_PRE_LOAD
> +       help
> +         Verify the signature provided in a pre-load header of
> +         a full image.

Please point to docs here

> +
>  config CMD_ZBOOT
>         bool "zboot - x86 boot command"
>         help
> diff --git a/cmd/Makefile b/cmd/Makefile
> index 166c652d98..80e054e806 100644
> --- a/cmd/Makefile
> +++ b/cmd/Makefile
> @@ -177,6 +177,7 @@ obj-$(CONFIG_CMD_THOR_DOWNLOAD) += thordown.o
>  obj-$(CONFIG_CMD_XIMG) += ximg.o
>  obj-$(CONFIG_CMD_YAFFS2) += yaffs2.o
>  obj-$(CONFIG_CMD_SPL) += spl.o
> +obj-$(CONFIG_CMD_VERIFY) += verify.o
>  obj-$(CONFIG_CMD_W1) += w1.o
>  obj-$(CONFIG_CMD_ZIP) += zip.o
>  obj-$(CONFIG_CMD_ZFS) += zfs.o
> diff --git a/cmd/verify.c b/cmd/verify.c
> new file mode 100644
> index 0000000000..4d055e0790
> --- /dev/null
> +++ b/cmd/verify.c
> @@ -0,0 +1,53 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (C) 2022 Philippe Reynes <philippe.reynes@softathome.com>
> + */
> +
> +#include <common.h>
> +#include <env.h>
> +#include <image.h>
> +#include <mapmem.h>
> +
> +static ulong verify_get_addr(int argc, char *const argv[])
> +{
> +       ulong addr;
> +
> +       if (argc > 0)
> +               addr = simple_strtoul(argv[0], NULL, 16);

hextoul

> +       else
> +               addr = image_load_addr;
> +
> +       return addr;
> +}
> +
> +static int do_verify(struct cmd_tbl *cmdtp, int flag, int argc,
> +                    char *const argv[])
> +{
> +       ulong addr = verify_get_addr(argc, argv);
> +       int ret = 0;
> +
> +       argc--; argv++;
> +
> +       addr = verify_get_addr(argc, argv);
> +
> +       if (CONFIG_IS_ENABLED(CMD_BOOTM_PRE_LOAD)) {
> +               ret = image_pre_load(addr);
> +
> +               if (ret) {
> +                       ret = CMD_RET_FAILURE;
> +                       goto out;
> +               }
> +
> +               env_set_hex("loadaddr_verified", addr + image_load_offset);
> +       }
> +
> + out:
> +       return ret;
> +}
> +
> +U_BOOT_CMD(verify, 2, 1, do_verify,
> +          "verify the global signature provided in the pre-load header,\n"
> +          "\tif the check succeed, the u-boot env variable loadaddr_verified\n"
> +          "\tis set to the address of the image (without the header)",
> +          "<image addr>"
> +);
> --
> 2.17.1
>

Regards,
Simon

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

* Re: [PATCH v6 16/16] configs: sandbox_defconfig: enable config CMD_VERIFY
  2022-02-25 14:57 ` [PATCH v6 16/16] configs: sandbox_defconfig: enable config CMD_VERIFY Philippe Reynes
@ 2022-03-03  3:37   ` Simon Glass
  0 siblings, 0 replies; 30+ messages in thread
From: Simon Glass @ 2022-03-03  3:37 UTC (permalink / raw)
  To: Philippe Reynes; +Cc: rasmus.villemoes, u-boot

On Fri, 25 Feb 2022 at 07:58, Philippe Reynes
<philippe.reynes@softathome.com> wrote:
>
> Enable the command verify on sandbox.
>
> Signed-off-by: Philippe Reynes <philippe.reynes@softathome.com>
> ---
>  configs/sandbox_defconfig | 1 +
>  1 file changed, 1 insertion(+)

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

Please add documentation to doc/

Also, is it possible to do the check automatically in the bootm
command, if enabled?

>
> diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
> index 46bf18bc98..a56aa92f94 100644
> --- a/configs/sandbox_defconfig
> +++ b/configs/sandbox_defconfig
> @@ -44,6 +44,7 @@ CONFIG_CMD_BOOTZ=y
>  CONFIG_CMD_BOOTEFI_HELLO=y
>  CONFIG_CMD_ABOOTIMG=y
>  # CONFIG_CMD_ELF is not set
> +CONFIG_CMD_VERIFY=y
>  CONFIG_CMD_ASKENV=y
>  CONFIG_CMD_GREPENV=y
>  CONFIG_CMD_ERASEENV=y
> --
> 2.17.1
>

Regards,
Simon

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

* Re: [PATCH v6 07/16] boot: image: add a stage pre-load
  2022-02-25 14:57 ` [PATCH v6 07/16] boot: image: add a stage pre-load Philippe Reynes
@ 2022-03-03  3:37   ` Simon Glass
  2022-03-10 17:34     ` Philippe REYNES
  0 siblings, 1 reply; 30+ messages in thread
From: Simon Glass @ 2022-03-03  3:37 UTC (permalink / raw)
  To: Philippe Reynes; +Cc: rasmus.villemoes, u-boot

Hi Philippe,

On Fri, 25 Feb 2022 at 07:58, Philippe Reynes
<philippe.reynes@softathome.com> wrote:
>
> Add a stage pre-load that could
> check or modify an image.
>
> For the moment, only a header with a signature is
> supported. This header has the following format:
> - magic : 4 bytes
> - version : 4 bytes
> - header size : 4 bytes
> - image size : 4 bytes
> - offset image signature : 4 bytes
> - flags : 4 bytes
> - reserved0 : 4 bytes
> - reserved1 : 4 bytes
> - sha256 of the image signature : 32 bytes
> - signature of the first 64 bytes : n bytes

It is a bit hard to understand without docs, but what is the point of
taking the sha256 of the signature?

Also, why is the signature only of the first 64 bytes? Normally we
hash the whole image and then sign that.

> - image signature : n bytes
> - padding : up to header size
>
> The stage uses a node /image/pre-load/sig to
> get some informations:
> - algo-name (mandatory) : name of the algo used to sign
> - padding-name : name of padding used to sign
> - signature-size : size of the signature (in the header)
> - mandatory : set to yes if this sig is mandatory
> - public-key (madatory) : value of the public key

Does this mean you read the DT properties to find out the sig info? I
thought the point of this series was to have a signature check that
did not rely on the devicetree, i.e. another layer of security?

>
> Before running the image, the stage pre-load checks
> the signature provided in the header.
>
> This is an initial support, later we could add the
> support of:
> - ciphering
> - uncompressing
> - ...
>
> Signed-off-by: Philippe Reynes <philippe.reynes@softathome.com>
> ---
>  boot/Kconfig          |  55 ++++++
>  boot/Makefile         |   1 +
>  boot/image-pre-load.c | 416 ++++++++++++++++++++++++++++++++++++++++++
>  include/image.h       |  14 ++
>  4 files changed, 486 insertions(+)
>  create mode 100644 boot/image-pre-load.c

Regards,
Simon

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

* Re: [PATCH v6 10/16] mkimage: add public key for image pre-load stage
  2022-02-25 14:57 ` [PATCH v6 10/16] mkimage: add public key for image pre-load stage Philippe Reynes
@ 2022-03-03  3:37   ` Simon Glass
  0 siblings, 0 replies; 30+ messages in thread
From: Simon Glass @ 2022-03-03  3:37 UTC (permalink / raw)
  To: Philippe Reynes; +Cc: rasmus.villemoes, u-boot

On Fri, 25 Feb 2022 at 07:58, Philippe Reynes
<philippe.reynes@softathome.com> wrote:
>
> This commit enhances mkimage to update the node
> /image/pre-load/sig with the public key.
>
> Signed-off-by: Philippe Reynes <philippe.reynes@softathome.com>
> ---
>  include/image.h    |  15 ++++++
>  tools/fit_image.c  |   3 ++
>  tools/image-host.c | 114 +++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 132 insertions(+)

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

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

* Re: [PATCH v6 11/16] Makefile: provide sah-key to binman
  2022-02-25 14:57 ` [PATCH v6 11/16] Makefile: provide sah-key to binman Philippe Reynes
@ 2022-03-03  3:37   ` Simon Glass
  0 siblings, 0 replies; 30+ messages in thread
From: Simon Glass @ 2022-03-03  3:37 UTC (permalink / raw)
  To: Philippe Reynes; +Cc: rasmus.villemoes, u-boot

On Fri, 25 Feb 2022 at 07:58, Philippe Reynes
<philippe.reynes@softathome.com> wrote:
>
> Set the variable key-path with the shell variable
> KEY_PATH that contain the keys path (used for signature).
> This variable key-path is provided to binman.
>
> Signed-off-by: Philippe Reynes <philippe.reynes@softathome.com>
> ---
>  Makefile | 1 +
>  1 file changed, 1 insertion(+)

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

>
> diff --git a/Makefile b/Makefile
> index 697cc51d67..c6da6cdba0 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1335,6 +1335,7 @@ cmd_binman = $(srctree)/tools/binman/binman $(if $(BINMAN_DEBUG),-D) \
>                 -a tpl-bss-pad=$(if $(CONFIG_TPL_SEPARATE_BSS),,1) \
>                 -a spl-dtb=$(CONFIG_SPL_OF_REAL) \
>                 -a tpl-dtb=$(CONFIG_TPL_OF_REAL) \
> +               -a key-path=${KEY_PATH} \

I wonder if preload-key might be better, since 'key' is very generic.

>                 $(BINMAN_$(@F))
>
>  OBJCOPYFLAGS_u-boot.ldr.hex := -I binary -O ihex
> --
> 2.17.1
>

Regards,
Simon

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

* Re: [PATCH v6 15/16] cmd: verify: initial import
  2022-03-03  3:37   ` Simon Glass
@ 2022-03-10 16:53     ` Philippe REYNES
  2022-03-28  6:35       ` Simon Glass
  0 siblings, 1 reply; 30+ messages in thread
From: Philippe REYNES @ 2022-03-10 16:53 UTC (permalink / raw)
  To: Simon Glass; +Cc: rasmus.villemoes, u-boot

Hi Simon,


Le 03/03/2022 à 04:37, Simon Glass a écrit :
> Hi Philippe,
>
> On Fri, 25 Feb 2022 at 07:58, Philippe Reynes
> <philippe.reynes@softathome.com> wrote:
>> Add the command verify that check the signature of
>> an image with the pre-load header. If the check
>> succeed, the u-boot env variable 'loadaddr_verified'
>> is set to the address of the image (without the header).
>>
>> It allows to run such commands:
>> tftp script.img && verify $loadaddr && source $loadaddr_verified
>>
>> Signed-off-by: Philippe Reynes <philippe.reynes@softathome.com>
>> ---
>>   cmd/Kconfig  |  7 +++++++
>>   cmd/Makefile |  1 +
>>   cmd/verify.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>>   3 files changed, 61 insertions(+)
>>   create mode 100644 cmd/verify.c
>>
> Using the 'verify' command seems a bit vague. Could it be a
> sub-command of bootm perhaps?


The command verify may be used with any binary (script, video firmware, 
.....).
So a lot of binaries that are not launched by bootm.
I think that it is not "logic" to used a bootm subcommand.
But we could use another name if you want.
For example : pre_load_verify ?


>> diff --git a/cmd/Kconfig b/cmd/Kconfig
>> index 87aa3fb11a..0460d5c3a0 100644
>> --- a/cmd/Kconfig
>> +++ b/cmd/Kconfig
>> @@ -428,6 +428,13 @@ config CMD_THOR_DOWNLOAD
>>            There is no documentation about this within the U-Boot source code
>>            but you should be able to find something on the interwebs.
>>
>> +config CMD_VERIFY
>> +       bool "verify the global signature"
>> +        depends on CMD_BOOTM_PRE_LOAD
>> +       help
>> +         Verify the signature provided in a pre-load header of
>> +         a full image.
> Please point to docs here
>
>> +
>>   config CMD_ZBOOT
>>          bool "zboot - x86 boot command"
>>          help
>> diff --git a/cmd/Makefile b/cmd/Makefile
>> index 166c652d98..80e054e806 100644
>> --- a/cmd/Makefile
>> +++ b/cmd/Makefile
>> @@ -177,6 +177,7 @@ obj-$(CONFIG_CMD_THOR_DOWNLOAD) += thordown.o
>>   obj-$(CONFIG_CMD_XIMG) += ximg.o
>>   obj-$(CONFIG_CMD_YAFFS2) += yaffs2.o
>>   obj-$(CONFIG_CMD_SPL) += spl.o
>> +obj-$(CONFIG_CMD_VERIFY) += verify.o
>>   obj-$(CONFIG_CMD_W1) += w1.o
>>   obj-$(CONFIG_CMD_ZIP) += zip.o
>>   obj-$(CONFIG_CMD_ZFS) += zfs.o
>> diff --git a/cmd/verify.c b/cmd/verify.c
>> new file mode 100644
>> index 0000000000..4d055e0790
>> --- /dev/null
>> +++ b/cmd/verify.c
>> @@ -0,0 +1,53 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +/*
>> + * Copyright (C) 2022 Philippe Reynes <philippe.reynes@softathome.com>
>> + */
>> +
>> +#include <common.h>
>> +#include <env.h>
>> +#include <image.h>
>> +#include <mapmem.h>
>> +
>> +static ulong verify_get_addr(int argc, char *const argv[])
>> +{
>> +       ulong addr;
>> +
>> +       if (argc > 0)
>> +               addr = simple_strtoul(argv[0], NULL, 16);
> hextoul
>
>> +       else
>> +               addr = image_load_addr;
>> +
>> +       return addr;
>> +}
>> +
>> +static int do_verify(struct cmd_tbl *cmdtp, int flag, int argc,
>> +                    char *const argv[])
>> +{
>> +       ulong addr = verify_get_addr(argc, argv);
>> +       int ret = 0;
>> +
>> +       argc--; argv++;
>> +
>> +       addr = verify_get_addr(argc, argv);
>> +
>> +       if (CONFIG_IS_ENABLED(CMD_BOOTM_PRE_LOAD)) {
>> +               ret = image_pre_load(addr);
>> +
>> +               if (ret) {
>> +                       ret = CMD_RET_FAILURE;
>> +                       goto out;
>> +               }
>> +
>> +               env_set_hex("loadaddr_verified", addr + image_load_offset);
>> +       }
>> +
>> + out:
>> +       return ret;
>> +}
>> +
>> +U_BOOT_CMD(verify, 2, 1, do_verify,
>> +          "verify the global signature provided in the pre-load header,\n"
>> +          "\tif the check succeed, the u-boot env variable loadaddr_verified\n"
>> +          "\tis set to the address of the image (without the header)",
>> +          "<image addr>"
>> +);
>> --
>> 2.17.1
>>
> Regards,
> Simon

Regards,

Philippe



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

* Re: [PATCH v6 07/16] boot: image: add a stage pre-load
  2022-03-03  3:37   ` Simon Glass
@ 2022-03-10 17:34     ` Philippe REYNES
  0 siblings, 0 replies; 30+ messages in thread
From: Philippe REYNES @ 2022-03-10 17:34 UTC (permalink / raw)
  To: Simon Glass; +Cc: rasmus.villemoes, u-boot

Hi Simon,


Le 03/03/2022 à 04:37, Simon Glass a écrit :
> Hi Philippe,
>
> On Fri, 25 Feb 2022 at 07:58, Philippe Reynes
> <philippe.reynes@softathome.com> wrote:
>> Add a stage pre-load that could
>> check or modify an image.
>>
>> For the moment, only a header with a signature is
>> supported. This header has the following format:
>> - magic : 4 bytes
>> - version : 4 bytes
>> - header size : 4 bytes
>> - image size : 4 bytes
>> - offset image signature : 4 bytes
>> - flags : 4 bytes
>> - reserved0 : 4 bytes
>> - reserved1 : 4 bytes
>> - sha256 of the image signature : 32 bytes
>> - signature of the first 64 bytes : n bytes
> It is a bit hard to understand without docs, but what is the point of
> taking the sha256 of the signature?
>
> Also, why is the signature only of the first 64 bytes? Normally we
> hash the whole image and then sign that.


The size of the header is dynamic and specified in the header.
The golden rule is: no data is used before being checked.
To check the header, the size of the header is needed, but
as this size is in the header and the header is not checked yet,
it is not possible to read/use it.

So to check the header, we use a two steps scheme:
1) the first 64 bytes of the header contains informations
about the header, call it header_header. As this header_header
has a fixed size, it possible to check its signature.
2) the header_header contains a hash of the image signature,
and its offset in the header. So it is possible to check the hash
of the image signature.

After those two steps, the image signature contained in the header
is checked and can be used to verify the signature of the image.

So the golden rule is respected, no data are used before being checked.


>> - image signature : n bytes
>> - padding : up to header size
>>
>> The stage uses a node /image/pre-load/sig to
>> get some informations:
>> - algo-name (mandatory) : name of the algo used to sign
>> - padding-name : name of padding used to sign
>> - signature-size : size of the signature (in the header)
>> - mandatory : set to yes if this sig is mandatory
>> - public-key (madatory) : value of the public key
> Does this mean you read the DT properties to find out the sig info? I
> thought the point of this series was to have a signature check that
> did not rely on the devicetree, i.e. another layer of security?


Yes, some properties are in the device tree, like the name of the algo 
(sha256,rsa4096, ....),
the padding name, the signature size, the public key, and if a signature 
is mandatory before
launching a binary with bootm.

The first goal of this serie is to be able to verify the signature of an 
image without using data not yet verified.
If the signature is in the FIT image, the header of the fit, and some 
node of the fit must be read and used
before verifying the signature of the FIT.

Another goal appears with this serie, to be able to verify the signature 
of other binary than FIT (script, firmware, ...).


>> Before running the image, the stage pre-load checks
>> the signature provided in the header.
>>
>> This is an initial support, later we could add the
>> support of:
>> - ciphering
>> - uncompressing
>> - ...
>>
>> Signed-off-by: Philippe Reynes <philippe.reynes@softathome.com>
>> ---
>>   boot/Kconfig          |  55 ++++++
>>   boot/Makefile         |   1 +
>>   boot/image-pre-load.c | 416 ++++++++++++++++++++++++++++++++++++++++++
>>   include/image.h       |  14 ++
>>   4 files changed, 486 insertions(+)
>>   create mode 100644 boot/image-pre-load.c
> Regards,
> Simon

Regards,
Philippe



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

* Re: [PATCH v6 15/16] cmd: verify: initial import
  2022-03-10 16:53     ` Philippe REYNES
@ 2022-03-28  6:35       ` Simon Glass
  2022-03-28 21:01         ` Philippe REYNES
  0 siblings, 1 reply; 30+ messages in thread
From: Simon Glass @ 2022-03-28  6:35 UTC (permalink / raw)
  To: Philippe REYNES; +Cc: Rasmus Villemoes, U-Boot Mailing List

Hi Philippe,

On Thu, 10 Mar 2022 at 09:53, Philippe REYNES
<philippe.reynes@softathome.com> wrote:
>
> Hi Simon,
>
>
> Le 03/03/2022 à 04:37, Simon Glass a écrit :
> > Hi Philippe,
> >
> > On Fri, 25 Feb 2022 at 07:58, Philippe Reynes
> > <philippe.reynes@softathome.com> wrote:
> >> Add the command verify that check the signature of
> >> an image with the pre-load header. If the check
> >> succeed, the u-boot env variable 'loadaddr_verified'
> >> is set to the address of the image (without the header).
> >>
> >> It allows to run such commands:
> >> tftp script.img && verify $loadaddr && source $loadaddr_verified
> >>
> >> Signed-off-by: Philippe Reynes <philippe.reynes@softathome.com>
> >> ---
> >>   cmd/Kconfig  |  7 +++++++
> >>   cmd/Makefile |  1 +
> >>   cmd/verify.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> >>   3 files changed, 61 insertions(+)
> >>   create mode 100644 cmd/verify.c
> >>
> > Using the 'verify' command seems a bit vague. Could it be a
> > sub-command of bootm perhaps?
>
>
> The command verify may be used with any binary (script, video firmware,
> .....).
> So a lot of binaries that are not launched by bootm.
> I think that it is not "logic" to used a bootm subcommand.
> But we could use another name if you want.
> For example : pre_load_verify ?

I see. Well, I suppose this is a boot loader, so 'verify' would be
expected to mean verifying an image or something to boot, so this
seems reasonable to me. But I do like the idea of putting pre_load in
there somewhere if you can, since we do most other verification as
part of the 'bootm' command. Up to you.

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

Regards,
Simon

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

* Re: [PATCH v6 15/16] cmd: verify: initial import
  2022-03-28  6:35       ` Simon Glass
@ 2022-03-28 21:01         ` Philippe REYNES
  0 siblings, 0 replies; 30+ messages in thread
From: Philippe REYNES @ 2022-03-28 21:01 UTC (permalink / raw)
  To: Simon Glass; +Cc: Rasmus Villemoes, U-Boot Mailing List

Hi Simon,


Le 28/03/2022 à 08:35, Simon Glass a écrit :
> Hi Philippe,
>
> On Thu, 10 Mar 2022 at 09:53, Philippe REYNES
> <philippe.reynes@softathome.com> wrote:
>> Hi Simon,
>>
>>
>> Le 03/03/2022 à 04:37, Simon Glass a écrit :
>>> Hi Philippe,
>>>
>>> On Fri, 25 Feb 2022 at 07:58, Philippe Reynes
>>> <philippe.reynes@softathome.com> wrote:
>>>> Add the command verify that check the signature of
>>>> an image with the pre-load header. If the check
>>>> succeed, the u-boot env variable 'loadaddr_verified'
>>>> is set to the address of the image (without the header).
>>>>
>>>> It allows to run such commands:
>>>> tftp script.img && verify $loadaddr && source $loadaddr_verified
>>>>
>>>> Signed-off-by: Philippe Reynes <philippe.reynes@softathome.com>
>>>> ---
>>>>    cmd/Kconfig  |  7 +++++++
>>>>    cmd/Makefile |  1 +
>>>>    cmd/verify.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>>    3 files changed, 61 insertions(+)
>>>>    create mode 100644 cmd/verify.c
>>>>
>>> Using the 'verify' command seems a bit vague. Could it be a
>>> sub-command of bootm perhaps?
>>
>> The command verify may be used with any binary (script, video firmware,
>> .....).
>> So a lot of binaries that are not launched by bootm.
>> I think that it is not "logic" to used a bootm subcommand.
>> But we could use another name if you want.
>> For example : pre_load_verify ?
> I see. Well, I suppose this is a boot loader, so 'verify' would be
> expected to mean verifying an image or something to boot, so this
> seems reasonable to me. But I do like the idea of putting pre_load in
> there somewhere if you can, since we do most other verification as
> part of the 'bootm' command. Up to you.


I have sent a v8 where I remove the command pre_load_verify,
and add the subcommand preload to bootm.


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

Regards,

Philippe



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

end of thread, other threads:[~2022-03-28 21:01 UTC | newest]

Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-25 14:57 [PATCH v6 00/16] image: add a stage pre-load Philippe Reynes
2022-02-25 14:57 ` [PATCH v6 01/16] arch: Kconfig: imply BINMAN for SANDBOX Philippe Reynes
2022-02-25 14:57 ` [PATCH v6 02/16] lib: Kconfig: enhance help for ASN1 Philippe Reynes
2022-02-25 14:57 ` [PATCH v6 03/16] lib: Kconfig: enhance the help of OID_REGISTRY Philippe Reynes
2022-02-25 14:57 ` [PATCH v6 04/16] lib: allow to build asn1 decoder and oid registry in SPL Philippe Reynes
2022-03-03  3:37   ` Simon Glass
2022-02-25 14:57 ` [PATCH v6 05/16] lib: crypto: allow to build crypyo " Philippe Reynes
2022-02-25 14:57 ` [PATCH v6 06/16] lib: rsa: allow rsa verify with pkey " Philippe Reynes
2022-02-25 14:57 ` [PATCH v6 07/16] boot: image: add a stage pre-load Philippe Reynes
2022-03-03  3:37   ` Simon Glass
2022-03-10 17:34     ` Philippe REYNES
2022-02-25 14:57 ` [PATCH v6 08/16] cmd: bootm: " Philippe Reynes
2022-02-25 14:57 ` [PATCH v6 09/16] common: spl: fit_ram: allow to use image pre load Philippe Reynes
2022-02-25 14:57 ` [PATCH v6 10/16] mkimage: add public key for image pre-load stage Philippe Reynes
2022-03-03  3:37   ` Simon Glass
2022-02-25 14:57 ` [PATCH v6 11/16] Makefile: provide sah-key to binman Philippe Reynes
2022-03-03  3:37   ` Simon Glass
2022-02-25 14:57 ` [PATCH v6 12/16] tools: binman: add support for pre-load header Philippe Reynes
2022-03-03  3:37   ` Simon Glass
2022-02-25 14:57 ` [PATCH v6 13/16] configs: sandbox_defconfig: enable stage pre-load in bootm Philippe Reynes
2022-03-03  3:37   ` Simon Glass
2022-02-25 14:57 ` [PATCH v6 14/16] test: py: vboot: add test for global image signature Philippe Reynes
2022-03-03  3:37   ` Simon Glass
2022-02-25 14:57 ` [PATCH v6 15/16] cmd: verify: initial import Philippe Reynes
2022-03-03  3:37   ` Simon Glass
2022-03-10 16:53     ` Philippe REYNES
2022-03-28  6:35       ` Simon Glass
2022-03-28 21:01         ` Philippe REYNES
2022-02-25 14:57 ` [PATCH v6 16/16] configs: sandbox_defconfig: enable config CMD_VERIFY Philippe Reynes
2022-03-03  3:37   ` 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.