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

[PATCH v4 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 v4:
- amound -> amount
- found another entry to handle
changes in v2:
- fix typo "distinguis(h)"

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

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
diff --git a/lib/rsa/rsa-verify.c b/lib/rsa/rsa-verify.c
index 1d55b997e3..a19867742f 100644
--- a/lib/rsa/rsa-verify.c
+++ b/lib/rsa/rsa-verify.c
@@ -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.25.1

[PATCH v4 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 v3.1:
- drop changeid
changes in v3:
- new patch with another build issue

 lib/rsa/Makefile     | 2 +-
 lib/rsa/rsa-verify.c | 4 ++--
 2 files changed, 3 insertions(+), 3 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 a19867742f..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.
-- 
2.25.1

[PATCH v4 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.25.1

[PATCH v4 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 double the number of elements in rr and rrtmp so that it matches
the needed number and should increase nicely if max_rsa_size gets
increased in the future.

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

 lib/rsa/rsa-keyprop.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/rsa/rsa-keyprop.c b/lib/rsa/rsa-keyprop.c
index 4b54db44c4..e28fbb7472 100644
--- a/lib/rsa/rsa-keyprop.c
+++ b/lib/rsa/rsa-keyprop.c
@@ -659,8 +659,8 @@ int rsa_gen_key_prop(const void *key, uint32_t keylen, struct key_prop **prop)
 
 	*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));
+	rr = calloc(sizeof(uint32_t), 1 + ((max_rsa_size * 2) >> 5));
+	rrtmp = calloc(sizeof(uint32_t), 1 + ((max_rsa_size * 2) >> 5));
 	if (!(*prop) || !n || !rr || !rrtmp) {
 		ret = -ENOMEM;
 		goto err;
-- 
2.25.1

[PATCH v4 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 e28fbb7472..ac33b35ff9 100644
--- a/lib/rsa/rsa-keyprop.c
+++ b/lib/rsa/rsa-keyprop.c
@@ -655,7 +655,7 @@ 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;
+	int rlen, i, ret = 0;
 
 	*prop = calloc(sizeof(**prop), 1);
 	n = calloc(sizeof(uint32_t), 1 + (max_rsa_size >> 5));
@@ -663,12 +663,12 @@ int rsa_gen_key_prop(const void *key, uint32_t keylen, struct key_prop **prop)
 	rrtmp = calloc(sizeof(uint32_t), 1 + ((max_rsa_size * 2) >> 5));
 	if (!(*prop) || !n || !rr || !rrtmp) {
 		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 */
@@ -678,7 +678,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);
 
@@ -686,7 +686,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,
@@ -710,16 +710,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.25.1

[PATCH v4 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>
---
change 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.25.1

[PATCH v4 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 0e7ccc0b07..482f39c66f 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.25.1

[PATCH v4 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 482f39c66f..0c184f7f06 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.25.1

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

[Philipp Tomsich]

> On 22.05.2020, at 16:13, 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>

Reviewed-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>

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

[Philipp Tomsich]

> On 22.05.2020, at 16:13, 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>

Reviewed-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>

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

[Philipp Tomsich]

> On 22.05.2020, at 16:13, 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>

Reviewed-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>

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

[Philipp Tomsich]

> On 22.05.2020, at 16:13, 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>

Reviewed-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>

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

[Simon Glass]

On Fri, 22 May 2020 at 08:13, 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 double the number of elements in rr and rrtmp so that it matches
> the needed number and should increase nicely if max_rsa_size gets
> increased in the future.
>
> Signed-off-by: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
> ---
> changes in v4:
> - new patch
>
>  lib/rsa/rsa-keyprop.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

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