From patchwork Fri May 22 14:13:25 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Heiko Stuebner X-Patchwork-Id: 246284 List-Id: U-Boot discussion From: heiko at sntech.de (Heiko Stuebner) Date: Fri, 22 May 2020 16:13:25 +0200 Subject: [PATCH v4 1/8] lib: rsa: distinguish between tpl and spl for CONFIG_RSA_VERIFY Message-ID: <20200522141332.3523031-1-heiko@sntech.de> From: Heiko Stuebner 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 Reviewed-by: Philipp Tomsich --- 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); From patchwork Fri May 22 14:13:26 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Heiko Stuebner X-Patchwork-Id: 246285 List-Id: U-Boot discussion From: heiko at sntech.de (Heiko Stuebner) Date: Fri, 22 May 2020 16:13:26 +0200 Subject: [PATCH v4 2/8] lib: rsa: take spl/non-spl into account when building rsa_verify_with_pkey() In-Reply-To: <20200522141332.3523031-1-heiko@sntech.de> References: <20200522141332.3523031-1-heiko@sntech.de> Message-ID: <20200522141332.3523031-2-heiko@sntech.de> From: Heiko Stuebner 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 Reviewed-by: Philipp Tomsich --- 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. From patchwork Fri May 22 14:13:27 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Heiko Stuebner X-Patchwork-Id: 246280 List-Id: U-Boot discussion From: heiko at sntech.de (Heiko Stuebner) Date: Fri, 22 May 2020 16:13:27 +0200 Subject: [PATCH v4 3/8] lib: rsa: bring exp_len in line when generating a key_prop In-Reply-To: <20200522141332.3523031-1-heiko@sntech.de> References: <20200522141332.3523031-1-heiko@sntech.de> Message-ID: <20200522141332.3523031-3-heiko@sntech.de> From: Heiko Stuebner 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 Reviewed-by: Philipp Tomsich --- 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); From patchwork Fri May 22 14:13:28 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Heiko Stuebner X-Patchwork-Id: 246278 List-Id: U-Boot discussion From: heiko at sntech.de (Heiko Stuebner) Date: Fri, 22 May 2020 16:13:28 +0200 Subject: [PATCH v4 4/8] lib: rsa: fix allocated size for rr and rrtmp in rsa_gen_key_prop() In-Reply-To: <20200522141332.3523031-1-heiko@sntech.de> References: <20200522141332.3523031-1-heiko@sntech.de> Message-ID: <20200522141332.3523031-4-heiko@sntech.de> From: Heiko Stuebner 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 Reviewed-by: Simon Glass --- 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; From patchwork Fri May 22 14:13:29 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Heiko Stuebner X-Patchwork-Id: 246283 List-Id: U-Boot discussion From: heiko at sntech.de (Heiko Stuebner) Date: Fri, 22 May 2020 16:13:29 +0200 Subject: [PATCH v4 5/8] lib: rsa: free local arrays after use in rsa_gen_key_prop() In-Reply-To: <20200522141332.3523031-1-heiko@sntech.de> References: <20200522141332.3523031-1-heiko@sntech.de> Message-ID: <20200522141332.3523031-5-heiko@sntech.de> From: Heiko Stuebner 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 Reviewed-by: Philipp Tomsich --- 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; } From patchwork Fri May 22 14:13:30 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Heiko Stuebner X-Patchwork-Id: 246281 List-Id: U-Boot discussion From: heiko at sntech.de (Heiko Stuebner) Date: Fri, 22 May 2020 16:13:30 +0200 Subject: [PATCH v4 6/8] lib: rsa: add documentation to padding_pss_verify to document limitations In-Reply-To: <20200522141332.3523031-1-heiko@sntech.de> References: <20200522141332.3523031-1-heiko@sntech.de> Message-ID: <20200522141332.3523031-6-heiko@sntech.de> From: Heiko Stuebner 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 Reviewed-by: Philipp Tomsich --- 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) From patchwork Fri May 22 14:13:31 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Heiko Stuebner X-Patchwork-Id: 246279 List-Id: U-Boot discussion From: heiko at sntech.de (Heiko Stuebner) Date: Fri, 22 May 2020 16:13:31 +0200 Subject: [PATCH v4 7/8] spl: fit: select SPL_HASH_SUPPORT for SPL_FIT_SIGNATURE In-Reply-To: <20200522141332.3523031-1-heiko@sntech.de> References: <20200522141332.3523031-1-heiko@sntech.de> Message-ID: <20200522141332.3523031-7-heiko@sntech.de> From: Heiko Stuebner 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 Signed-off-by: Heiko Stuebner Reviewed-by: Philipp Tomsich --- 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 From patchwork Fri May 22 14:13:32 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Heiko Stuebner X-Patchwork-Id: 246282 List-Id: U-Boot discussion From: heiko at sntech.de (Heiko Stuebner) Date: Fri, 22 May 2020 16:13:32 +0200 Subject: [PATCH v4 8/8] spl: fit: select SPL_CRYPTO_SUPPORT for SPL_FIT_SIGNATURE In-Reply-To: <20200522141332.3523031-1-heiko@sntech.de> References: <20200522141332.3523031-1-heiko@sntech.de> Message-ID: <20200522141332.3523031-8-heiko@sntech.de> From: Heiko Stuebner 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 Reviewed-by: Philipp Tomsich --- 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