[PATCH v6 1/8] lib: rsa: distinguish between tpl and spl for CONFIG_RSA_VERIFY

[PATCH v6 1/8] lib: rsa: distinguish between tpl and spl for CONFIG_RSA_VERIFY

[Heiko Stuebner]

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

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

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

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

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

Signed-off-by: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
Reviewed-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
---
changes in v5:
- drop change that belongs in patch 2/8
changes in v4:
- amound -> amount
- found another entry to handle
changes in v2:
- fix typo "distinguis(h)"

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

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

[PATCH v6 2/8] lib: rsa: take spl/non-spl into account when building rsa_verify_with_pkey()

[Heiko Stuebner]

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

Right now in multiple places there are only checks for the full
CONFIG_RSA_VERIFY_WITH_PKEY option, not split into main,spl,tpl variants.

This breaks when the rsa functions get enabled for SPL, for example to
verify u-boot proper from spl.

So fix this by using the existing helpers to distinguis between
build-steps.

Signed-off-by: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
---
changes in v5:
- include the additional config-check that landed in patch 1/8
  in v4
changes in v3.1:
- drop changeid
changes in v3:
- new patch with another build issue

 lib/rsa/Makefile     | 2 +-
 lib/rsa/rsa-verify.c | 6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/lib/rsa/Makefile b/lib/rsa/Makefile
index c61ebfd79e..8b75d41f04 100644
--- a/lib/rsa/Makefile
+++ b/lib/rsa/Makefile
@@ -6,5 +6,5 @@
 # Wolfgang Denk, DENX Software Engineering, wd at denx.de.
 
 obj-$(CONFIG_$(SPL_TPL_)RSA_VERIFY) += rsa-verify.o rsa-checksum.o
-obj-$(CONFIG_RSA_VERIFY_WITH_PKEY) += rsa-keyprop.o
+obj-$(CONFIG_$(SPL_TPL_)RSA_VERIFY_WITH_PKEY) += rsa-keyprop.o
 obj-$(CONFIG_RSA_SOFTWARE_EXP) += rsa-mod-exp.o
diff --git a/lib/rsa/rsa-verify.c b/lib/rsa/rsa-verify.c
index 1d55b997e3..048f1ab789 100644
--- a/lib/rsa/rsa-verify.c
+++ b/lib/rsa/rsa-verify.c
@@ -285,7 +285,7 @@ out:
 }
 #endif
 
-#if CONFIG_IS_ENABLED(FIT_SIGNATURE) || IS_ENABLED(CONFIG_RSA_VERIFY_WITH_PKEY)
+#if CONFIG_IS_ENABLED(FIT_SIGNATURE) || CONFIG_IS_ENABLED(RSA_VERIFY_WITH_PKEY)
 /**
  * rsa_verify_key() - Verify a signature against some data using RSA Key
  *
@@ -359,7 +359,7 @@ static int rsa_verify_key(struct image_sign_info *info,
 }
 #endif
 
-#ifdef CONFIG_RSA_VERIFY_WITH_PKEY
+#if CONFIG_IS_ENABLED(RSA_VERIFY_WITH_PKEY)
 /**
  * rsa_verify_with_pkey() - Verify a signature against some data using
  * only modulus and exponent as RSA key properties.
@@ -492,7 +492,7 @@ int rsa_verify(struct image_sign_info *info,
 		return -EINVAL;
 	}
 
-	if (IS_ENABLED(CONFIG_RSA_VERIFY_WITH_PKEY) && !info->fdt_blob) {
+	if (CONFIG_IS_ENABLED(RSA_VERIFY_WITH_PKEY) && !info->fdt_blob) {
 		/* don't rely on fdt properties */
 		ret = rsa_verify_with_pkey(info, hash, sig, sig_len);
 
-- 
2.26.2

[PATCH v6 3/8] lib: rsa: bring exp_len in line when generating a key_prop

[Heiko Stuebner]

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

The exponent field of struct key_prop gets allocated an uint64_t,
and the contents are positioned from the back, so an exponent of
"0x01 0x00 0x01" becomes 0x0 0x0 0x0 0x0 0x0 0x1 0x0 0x1"

Right now rsa_gen_key_prop() allocates a uint64_t but sets exp_len
to the size returned from the parser, while on the other hand the
when getting the key from the devicetree exp_len always gets set to
sizeof(uint64_t).

So bring that in line with the established code.

Signed-off-by: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
---
changes in v4:
- new patch

 lib/rsa/rsa-keyprop.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/rsa/rsa-keyprop.c b/lib/rsa/rsa-keyprop.c
index 9464df0093..4b54db44c4 100644
--- a/lib/rsa/rsa-keyprop.c
+++ b/lib/rsa/rsa-keyprop.c
@@ -691,7 +691,7 @@ int rsa_gen_key_prop(const void *key, uint32_t keylen, struct key_prop **prop)
 	memcpy((void *)(*prop)->public_exponent + sizeof(uint64_t)
 						- rsa_key.e_sz,
 	       rsa_key.e, rsa_key.e_sz);
-	(*prop)->exp_len = rsa_key.e_sz;
+	(*prop)->exp_len = sizeof(uint64_t);
 
 	/* n0 inverse */
 	br_i32_decode(n, &rsa_key.n[i], rsa_key.n_sz - i);
-- 
2.26.2

[PATCH v6 4/8] lib: rsa: fix allocated size for rr and rrtmp in rsa_gen_key_prop()

[Heiko Stuebner]

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

When calculating rrtmp/rr rsa_gen_key_prop() tries to make
(((rlen + 31) >> 5) + 1) steps in the rr uint32_t array and
(((rlen + 7) >> 3) + 1) / 4 steps in uint32_t rrtmp[]
with rlen being num_bits * 2

On a 4096bit key this comes down to to 257 uint32_t elements
in rr and 256 elements in rrtmp but with the current allocation
rr and rrtmp only have 129 uint32_t elements.

On 2048bit keys this works by chance as the defined max_rsa_size=4096
allocates a suitable number of elements, but with an actual 4096bit key
this results in other memory parts getting overwritten.

So as suggested by Heinrich Schuchardt just use the actual bis-size
of the key as base for the size calculation, in turn making the code
compatible to any future keysizes.

Suggested-by: Heinrich Schuchardt <xypron.debian@gmx.de>
Signed-off-by: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
---
changes in v6:
- drop max_rsa_size and use the keysize as base
changes in v4:
- new patch

 lib/rsa/rsa-keyprop.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/lib/rsa/rsa-keyprop.c b/lib/rsa/rsa-keyprop.c
index 4b54db44c4..83b942615f 100644
--- a/lib/rsa/rsa-keyprop.c
+++ b/lib/rsa/rsa-keyprop.c
@@ -654,14 +654,10 @@ int rsa_gen_key_prop(const void *key, uint32_t keylen, struct key_prop **prop)
 {
 	struct rsa_key rsa_key;
 	uint32_t *n = NULL, *rr = NULL, *rrtmp = NULL;
-	const int max_rsa_size = 4096;
 	int rlen, i, ret;
 
 	*prop = calloc(sizeof(**prop), 1);
-	n = calloc(sizeof(uint32_t), 1 + (max_rsa_size >> 5));
-	rr = calloc(sizeof(uint32_t), 1 + (max_rsa_size >> 5));
-	rrtmp = calloc(sizeof(uint32_t), 1 + (max_rsa_size >> 5));
-	if (!(*prop) || !n || !rr || !rrtmp) {
+	if (!(*prop)) {
 		ret = -ENOMEM;
 		goto err;
 	}
@@ -682,6 +678,14 @@ int rsa_gen_key_prop(const void *key, uint32_t keylen, struct key_prop **prop)
 	}
 	memcpy((void *)(*prop)->modulus, &rsa_key.n[i], rsa_key.n_sz - i);
 
+	n = calloc(sizeof(uint32_t), 1 + ((*prop)->num_bits >> 5));
+	rr = calloc(sizeof(uint32_t), 1 + (((*prop)->num_bits * 2) >> 5));
+	rrtmp = calloc(sizeof(uint32_t), 1 + (((*prop)->num_bits * 2) >> 5));
+	if (!n || !rr || !rrtmp) {
+		ret = -ENOMEM;
+		goto out;
+	}
+
 	/* exponent */
 	(*prop)->public_exponent = calloc(1, sizeof(uint64_t));
 	if (!(*prop)->public_exponent) {
-- 
2.26.2

[PATCH v6 5/8] lib: rsa: free local arrays after use in rsa_gen_key_prop()

[Heiko Stuebner]

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

n, rr and rrtmp are used for internal calculations, but in the end
the results are copied into separately allocated elements of the
actual key_prop, so the n, rr and rrtmp elements are not used anymore
when returning from the function and should of course be freed.

Signed-off-by: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
---
changes in v4:
- new patch

 lib/rsa/rsa-keyprop.c | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/lib/rsa/rsa-keyprop.c b/lib/rsa/rsa-keyprop.c
index 83b942615f..195ce30181 100644
--- a/lib/rsa/rsa-keyprop.c
+++ b/lib/rsa/rsa-keyprop.c
@@ -654,17 +654,17 @@ int rsa_gen_key_prop(const void *key, uint32_t keylen, struct key_prop **prop)
 {
 	struct rsa_key rsa_key;
 	uint32_t *n = NULL, *rr = NULL, *rrtmp = NULL;
-	int rlen, i, ret;
+	int rlen, i, ret = 0;
 
 	*prop = calloc(sizeof(**prop), 1);
 	if (!(*prop)) {
 		ret = -ENOMEM;
-		goto err;
+		goto out;
 	}
 
 	ret = rsa_parse_pub_key(&rsa_key, key, keylen);
 	if (ret)
-		goto err;
+		goto out;
 
 	/* modulus */
 	/* removing leading 0's */
@@ -674,7 +674,7 @@ int rsa_gen_key_prop(const void *key, uint32_t keylen, struct key_prop **prop)
 	(*prop)->modulus = malloc(rsa_key.n_sz - i);
 	if (!(*prop)->modulus) {
 		ret = -ENOMEM;
-		goto err;
+		goto out;
 	}
 	memcpy((void *)(*prop)->modulus, &rsa_key.n[i], rsa_key.n_sz - i);
 
@@ -690,7 +690,7 @@ int rsa_gen_key_prop(const void *key, uint32_t keylen, struct key_prop **prop)
 	(*prop)->public_exponent = calloc(1, sizeof(uint64_t));
 	if (!(*prop)->public_exponent) {
 		ret = -ENOMEM;
-		goto err;
+		goto out;
 	}
 	memcpy((void *)(*prop)->public_exponent + sizeof(uint64_t)
 						- rsa_key.e_sz,
@@ -714,16 +714,15 @@ int rsa_gen_key_prop(const void *key, uint32_t keylen, struct key_prop **prop)
 	(*prop)->rr = malloc(rlen);
 	if (!(*prop)->rr) {
 		ret = -ENOMEM;
-		goto err;
+		goto out;
 	}
 	br_i32_encode((void *)(*prop)->rr, rlen, rr);
 
-	return 0;
-
-err:
+out:
 	free(n);
 	free(rr);
 	free(rrtmp);
-	rsa_free_key_prop(*prop);
+	if (ret < 0)
+		rsa_free_key_prop(*prop);
 	return ret;
 }
-- 
2.26.2

[PATCH v6 6/8] lib: rsa: add documentation to padding_pss_verify to document limitations

[Heiko Stuebner]

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

padding_pss_verify only works with the default pss salt setting of -2
(length to be automatically determined based on the PSS block structure)
not -1 (salt length set to the maximum permissible value), which makes
verifications of signatures with that saltlen fail.

Until this gets implemented at least document this behaviour.

Signed-off-by: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
---
changes in v4:
- new patch

 lib/rsa/rsa-verify.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/lib/rsa/rsa-verify.c b/lib/rsa/rsa-verify.c
index 048f1ab789..61d98e6e2d 100644
--- a/lib/rsa/rsa-verify.c
+++ b/lib/rsa/rsa-verify.c
@@ -194,6 +194,19 @@ out:
 	return ret;
 }
 
+/*
+ * padding_pss_verify() - verify the pss padding of a signature
+ *
+ * Only works with a rsa_pss_saltlen:-2 (default value) right now
+ * saltlen:-1 "set the salt length to the digest length" is currently
+ * not supported.
+ *
+ * @info:	Specifies key and FIT information
+ * @msg:	byte array of message, len equal to msg_len
+ * @msg_len:	Message length
+ * @hash:	Pointer to the expected hash
+ * @hash_len:	Length of the hash
+ */
 int padding_pss_verify(struct image_sign_info *info,
 		       uint8_t *msg, int msg_len,
 		       const uint8_t *hash, int hash_len)
-- 
2.26.2

[PATCH v6 7/8] spl: fit: select SPL_HASH_SUPPORT for SPL_FIT_SIGNATURE

[Heiko Stuebner]

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

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

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

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

diff --git a/Kconfig b/Kconfig
index 8f3fba085a..091c500ecc 100644
--- a/Kconfig
+++ b/Kconfig
@@ -459,6 +459,7 @@ config SPL_FIT_SIGNATURE
 	bool "Enable signature verification of FIT firmware within SPL"
 	depends on SPL_DM
 	select SPL_FIT
+	select SPL_HASH_SUPPORT
 	select SPL_RSA
 	select SPL_RSA_VERIFY
 	select SPL_IMAGE_SIGN_INFO
-- 
2.26.2

[PATCH v6 8/8] spl: fit: select SPL_CRYPTO_SUPPORT for SPL_FIT_SIGNATURE

[Heiko Stuebner]

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

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

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

So select SPL_CRYPTO_SUPPORT in SPL_FIT_SIGNATURE.

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

diff --git a/Kconfig b/Kconfig
index 091c500ecc..b34fbf5827 100644
--- a/Kconfig
+++ b/Kconfig
@@ -459,6 +459,7 @@ config SPL_FIT_SIGNATURE
 	bool "Enable signature verification of FIT firmware within SPL"
 	depends on SPL_DM
 	select SPL_FIT
+	select SPL_CRYPTO_SUPPORT
 	select SPL_HASH_SUPPORT
 	select SPL_RSA
 	select SPL_RSA_VERIFY
-- 
2.26.2

[PATCH v6 1/8] lib: rsa: distinguish between tpl and spl for CONFIG_RSA_VERIFY

[Simon Glass]

On Thu, 18 Jun 2020 at 08:23, Heiko Stuebner <heiko@sntech.de> wrote:
>
> From: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
>
> While the SPL may want to do signature checking this won't be
> the case for TPL in all cases, as TPL is mostly used when the
> amount of initial memory is not enough for a full SPL.
>
> So on a system where SPL uses DM but TPL does not we currently
> end up with a TPL compile error of:
>
>     lib/rsa/rsa-verify.c:48:25: error: dereferencing pointer to incomplete type ?struct checksum_algo?
>
> To prevent that change the $(SPL_) to $(SPL_TPL_) to distinguish
> between both. If someone really needs FIT signature checking in
> TPL as well, a new TPL_RSA_VERIFY config symbol needs to be added.
>
> Signed-off-by: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
> Reviewed-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
> ---
> changes in v5:
> - drop change that belongs in patch 2/8
> changes in v4:
> - amound -> amount
> - found another entry to handle
> changes in v2:
> - fix typo "distinguis(h)"
>
>  lib/rsa/Makefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>

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

[PATCH v6 2/8] lib: rsa: take spl/non-spl into account when building rsa_verify_with_pkey()

[Simon Glass]

On Thu, 18 Jun 2020 at 08:23, Heiko Stuebner <heiko@sntech.de> wrote:
>
> From: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
>
> Right now in multiple places there are only checks for the full
> CONFIG_RSA_VERIFY_WITH_PKEY option, not split into main,spl,tpl variants.
>
> This breaks when the rsa functions get enabled for SPL, for example to
> verify u-boot proper from spl.
>
> So fix this by using the existing helpers to distinguis between
> build-steps.
>
> Signed-off-by: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
> ---
> changes in v5:
> - include the additional config-check that landed in patch 1/8
>   in v4
> changes in v3.1:
> - drop changeid
> changes in v3:
> - new patch with another build issue
>
>  lib/rsa/Makefile     | 2 +-
>  lib/rsa/rsa-verify.c | 6 +++---
>  2 files changed, 4 insertions(+), 4 deletions(-)

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

[PATCH v6 3/8] lib: rsa: bring exp_len in line when generating a key_prop

[Simon Glass]

On Thu, 18 Jun 2020 at 08:23, Heiko Stuebner <heiko@sntech.de> wrote:
>
> From: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
>
> The exponent field of struct key_prop gets allocated an uint64_t,
> and the contents are positioned from the back, so an exponent of
> "0x01 0x00 0x01" becomes 0x0 0x0 0x0 0x0 0x0 0x1 0x0 0x1"
>
> Right now rsa_gen_key_prop() allocates a uint64_t but sets exp_len
> to the size returned from the parser, while on the other hand the
> when getting the key from the devicetree exp_len always gets set to
> sizeof(uint64_t).
>
> So bring that in line with the established code.
>
> Signed-off-by: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
> ---
> changes in v4:
> - new patch
>
>  lib/rsa/rsa-keyprop.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

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

[PATCH v6 4/8] lib: rsa: fix allocated size for rr and rrtmp in rsa_gen_key_prop()

[Simon Glass]

On Thu, 18 Jun 2020 at 08:23, Heiko Stuebner <heiko@sntech.de> wrote:
>
> From: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
>
> When calculating rrtmp/rr rsa_gen_key_prop() tries to make
> (((rlen + 31) >> 5) + 1) steps in the rr uint32_t array and
> (((rlen + 7) >> 3) + 1) / 4 steps in uint32_t rrtmp[]
> with rlen being num_bits * 2
>
> On a 4096bit key this comes down to to 257 uint32_t elements
> in rr and 256 elements in rrtmp but with the current allocation
> rr and rrtmp only have 129 uint32_t elements.
>
> On 2048bit keys this works by chance as the defined max_rsa_size=4096
> allocates a suitable number of elements, but with an actual 4096bit key
> this results in other memory parts getting overwritten.
>
> So as suggested by Heinrich Schuchardt just use the actual bis-size
> of the key as base for the size calculation, in turn making the code
> compatible to any future keysizes.
>
> Suggested-by: Heinrich Schuchardt <xypron.debian@gmx.de>
> Signed-off-by: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
> ---
> changes in v6:
> - drop max_rsa_size and use the keysize as base
> changes in v4:
> - new patch
>
>  lib/rsa/rsa-keyprop.c | 14 +++++++++-----
>  1 file changed, 9 insertions(+), 5 deletions(-)

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

[PATCH v6 5/8] lib: rsa: free local arrays after use in rsa_gen_key_prop()

[Simon Glass]

On Thu, 18 Jun 2020 at 08:23, Heiko Stuebner <heiko@sntech.de> wrote:
>
> From: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
>
> n, rr and rrtmp are used for internal calculations, but in the end
> the results are copied into separately allocated elements of the
> actual key_prop, so the n, rr and rrtmp elements are not used anymore
> when returning from the function and should of course be freed.
>
> Signed-off-by: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
> ---
> changes in v4:
> - new patch
>
>  lib/rsa/rsa-keyprop.c | 19 +++++++++----------
>  1 file changed, 9 insertions(+), 10 deletions(-)

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

[PATCH v6 6/8] lib: rsa: add documentation to padding_pss_verify to document limitations

[Simon Glass]

On Thu, 18 Jun 2020 at 08:23, Heiko Stuebner <heiko@sntech.de> wrote:
>
> From: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
>
> padding_pss_verify only works with the default pss salt setting of -2
> (length to be automatically determined based on the PSS block structure)
> not -1 (salt length set to the maximum permissible value), which makes
> verifications of signatures with that saltlen fail.
>
> Until this gets implemented at least document this behaviour.
>
> Signed-off-by: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
> ---
> changes in v4:
> - new patch
>
>  lib/rsa/rsa-verify.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)

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

[PATCH v6 7/8] spl: fit: select SPL_HASH_SUPPORT for SPL_FIT_SIGNATURE

[Simon Glass]

On Thu, 18 Jun 2020 at 08:23, Heiko Stuebner <heiko@sntech.de> wrote:
>
> From: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
>
> rsa-checsum needs support for hash functions or else will run into
> compile errors like:
> u-boot/lib/rsa/rsa-checksum.c:28: undefined reference to `hash_progressive_lookup_algo'
>
> So similar to the main FIT_SIGNATURE entry selects HASH,
> select SPL_HASH_SUPPORT for SPL_FIT_SIGNATURE.
>
> Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
> Signed-off-by: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
> Reviewed-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
> ---
>  Kconfig | 1 +
>  1 file changed, 1 insertion(+)
>

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

[PATCH v6 8/8] spl: fit: select SPL_CRYPTO_SUPPORT for SPL_FIT_SIGNATURE

[Simon Glass]

On Thu, 18 Jun 2020 at 08:23, Heiko Stuebner <heiko@sntech.de> wrote:
>
> From: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
>
> Verifying FIT images obviously needs the rsa parts of crypto
> support and while main uboot always compiles crypto support,
> it's optional for SPL and we should thus select the necessary
> option to not end up in compile errors like:
>
>     u-boot/lib/rsa/rsa-verify.c:328: undefined reference to `rsa_mod_exp'
>
> So select SPL_CRYPTO_SUPPORT in SPL_FIT_SIGNATURE.
>
> Signed-off-by: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
> Reviewed-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
> ---
>  Kconfig | 1 +
>  1 file changed, 1 insertion(+)

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

[PATCH v6 1/8] lib: rsa: distinguish between tpl and spl for CONFIG_RSA_VERIFY

[Heiko Stübner]

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

with it looking like everybody is happy with the fixes series now,
whom do I need to pester into picking it up? :-D


Thanks
Heiko


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

[PATCH v6 1/8] lib: rsa: distinguish between tpl and spl for CONFIG_RSA_VERIFY

[Heinrich Schuchardt]

On 6/30/20 2:49 PM, Heiko St?bner wrote:
> Am Donnerstag, 18. Juni 2020, 16:23:21 CEST schrieb Heiko Stuebner:
>> From: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
>>
>> While the SPL may want to do signature checking this won't be
>> the case for TPL in all cases, as TPL is mostly used when the
>> amount of initial memory is not enough for a full SPL.
>>
>> So on a system where SPL uses DM but TPL does not we currently
>> end up with a TPL compile error of:
>>
>>     lib/rsa/rsa-verify.c:48:25: error: dereferencing pointer to incomplete type ?struct checksum_algo?
>>
>> To prevent that change the $(SPL_) to $(SPL_TPL_) to distinguish
>> between both. If someone really needs FIT signature checking in
>> TPL as well, a new TPL_RSA_VERIFY config symbol needs to be added.
>>
>> Signed-off-by: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
>> Reviewed-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
>
> with it looking like everybody is happy with the fixes series now,
> whom do I need to pester into picking it up? :-D

No pestering needed :) The series is assigned to Tom:

https://patchwork.ozlabs.org/project/uboot/list/?series=184291

Best regards

Heinrich

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

[PATCH v6 4/8] lib: rsa: fix allocated size for rr and rrtmp in rsa_gen_key_prop()

[Heinrich Schuchardt]

On 18.06.20 16:23, Heiko Stuebner wrote:
> From: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
>
> When calculating rrtmp/rr rsa_gen_key_prop() tries to make
> (((rlen + 31) >> 5) + 1) steps in the rr uint32_t array and
> (((rlen + 7) >> 3) + 1) / 4 steps in uint32_t rrtmp[]
> with rlen being num_bits * 2
>
> On a 4096bit key this comes down to to 257 uint32_t elements
> in rr and 256 elements in rrtmp but with the current allocation
> rr and rrtmp only have 129 uint32_t elements.
>
> On 2048bit keys this works by chance as the defined max_rsa_size=4096
> allocates a suitable number of elements, but with an actual 4096bit key
> this results in other memory parts getting overwritten.
>
> So as suggested by Heinrich Schuchardt just use the actual bis-size
> of the key as base for the size calculation, in turn making the code
> compatible to any future keysizes.
>
> Suggested-by: Heinrich Schuchardt <xypron.debian@gmx.de>
> Signed-off-by: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
> ---
> changes in v6:
> - drop max_rsa_size and use the keysize as base
> changes in v4:
> - new patch
>
>  lib/rsa/rsa-keyprop.c | 14 +++++++++-----
>  1 file changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/lib/rsa/rsa-keyprop.c b/lib/rsa/rsa-keyprop.c
> index 4b54db44c4..83b942615f 100644
> --- a/lib/rsa/rsa-keyprop.c
> +++ b/lib/rsa/rsa-keyprop.c
> @@ -654,14 +654,10 @@ int rsa_gen_key_prop(const void *key, uint32_t keylen, struct key_prop **prop)
>  {
>  	struct rsa_key rsa_key;
>  	uint32_t *n = NULL, *rr = NULL, *rrtmp = NULL;
> -	const int max_rsa_size = 4096;
>  	int rlen, i, ret;
>
>  	*prop = calloc(sizeof(**prop), 1);
> -	n = calloc(sizeof(uint32_t), 1 + (max_rsa_size >> 5));
> -	rr = calloc(sizeof(uint32_t), 1 + (max_rsa_size >> 5));
> -	rrtmp = calloc(sizeof(uint32_t), 1 + (max_rsa_size >> 5));
> -	if (!(*prop) || !n || !rr || !rrtmp) {
> +	if (!(*prop)) {
>  		ret = -ENOMEM;
>  		goto err;
>  	}
> @@ -682,6 +678,14 @@ int rsa_gen_key_prop(const void *key, uint32_t keylen, struct key_prop **prop)
>  	}
>  	memcpy((void *)(*prop)->modulus, &rsa_key.n[i], rsa_key.n_sz - i);
>
> +	n = calloc(sizeof(uint32_t), 1 + ((*prop)->num_bits >> 5));
> +	rr = calloc(sizeof(uint32_t), 1 + (((*prop)->num_bits * 2) >> 5));
> +	rrtmp = calloc(sizeof(uint32_t), 1 + (((*prop)->num_bits * 2) >> 5));

At least for rrtmp you need

2 + (((*prop)->num_bits * 2) >> 5)

For num_bits = 2048 br_i32_decode() writes to indices 0..129.

@Takahiro
This is why the code crashes even with an RSA 2048 key on
qemu_arm_defconfig.

Best regards

Heinrich

> +	if (!n || !rr || !rrtmp) {
> +		ret = -ENOMEM;
> +		goto out;
> +	}
> +
>  	/* exponent */
>  	(*prop)->public_exponent = calloc(1, sizeof(uint64_t));
>  	if (!(*prop)->public_exponent) {
>

[PATCH v6 1/8] lib: rsa: distinguish between tpl and spl for CONFIG_RSA_VERIFY

[Tom Rini]

On Thu, Jun 18, 2020 at 04:23:21PM +0200, Heiko Stuebner wrote:

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

Applied to u-boot/master, thanks!

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

[PATCH v6 2/8] lib: rsa: take spl/non-spl into account when building rsa_verify_with_pkey()

[Tom Rini]

On Thu, Jun 18, 2020 at 04:23:22PM +0200, Heiko Stuebner wrote:

> From: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
> 
> Right now in multiple places there are only checks for the full
> CONFIG_RSA_VERIFY_WITH_PKEY option, not split into main,spl,tpl variants.
> 
> This breaks when the rsa functions get enabled for SPL, for example to
> verify u-boot proper from spl.
> 
> So fix this by using the existing helpers to distinguis between
> build-steps.
> 
> Signed-off-by: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
> Reviewed-by: Simon Glass <sjg@chromium.org>

Applied to u-boot/master, thanks!

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

[PATCH v6 3/8] lib: rsa: bring exp_len in line when generating a key_prop

[Tom Rini]

On Thu, Jun 18, 2020 at 04:23:23PM +0200, Heiko Stuebner wrote:

> From: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
> 
> The exponent field of struct key_prop gets allocated an uint64_t,
> and the contents are positioned from the back, so an exponent of
> "0x01 0x00 0x01" becomes 0x0 0x0 0x0 0x0 0x0 0x1 0x0 0x1"
> 
> Right now rsa_gen_key_prop() allocates a uint64_t but sets exp_len
> to the size returned from the parser, while on the other hand the
> when getting the key from the devicetree exp_len always gets set to
> sizeof(uint64_t).
> 
> So bring that in line with the established code.
> 
> Signed-off-by: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
> Reviewed-by: Simon Glass <sjg@chromium.org>

Applied to u-boot/master, thanks!

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

[PATCH v6 5/8] lib: rsa: free local arrays after use in rsa_gen_key_prop()

[Tom Rini]

On Thu, Jun 18, 2020 at 04:23:25PM +0200, Heiko Stuebner wrote:

> From: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
> 
> n, rr and rrtmp are used for internal calculations, but in the end
> the results are copied into separately allocated elements of the
> actual key_prop, so the n, rr and rrtmp elements are not used anymore
> when returning from the function and should of course be freed.
> 
> Signed-off-by: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
> Reviewed-by: Simon Glass <sjg@chromium.org>

Applied to u-boot/master, thanks!

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

[PATCH v6 6/8] lib: rsa: add documentation to padding_pss_verify to document limitations

[Tom Rini]

On Thu, Jun 18, 2020 at 04:23:26PM +0200, Heiko Stuebner wrote:

> From: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
> 
> padding_pss_verify only works with the default pss salt setting of -2
> (length to be automatically determined based on the PSS block structure)
> not -1 (salt length set to the maximum permissible value), which makes
> verifications of signatures with that saltlen fail.
> 
> Until this gets implemented at least document this behaviour.
> 
> Signed-off-by: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
> Reviewed-by: Simon Glass <sjg@chromium.org>

Applied to u-boot/master, thanks!

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

[PATCH v6 7/8] spl: fit: select SPL_HASH_SUPPORT for SPL_FIT_SIGNATURE

[Tom Rini]

On Thu, Jun 18, 2020 at 04:23:27PM +0200, Heiko Stuebner wrote:

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

Applied to u-boot/master, thanks!

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

[PATCH v6 8/8] spl: fit: select SPL_CRYPTO_SUPPORT for SPL_FIT_SIGNATURE

[Tom Rini]

On Thu, Jun 18, 2020 at 04:23:28PM +0200, Heiko Stuebner wrote:

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

Applied to u-boot/master, thanks!

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