From patchwork Thu Jun 18 14:23:21 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: =?utf-8?q?Heiko_St=C3=BCbner?= X-Patchwork-Id: 242602 List-Id: U-Boot discussion From: heiko at sntech.de (Heiko Stuebner) Date: Thu, 18 Jun 2020 16:23:21 +0200 Subject: [PATCH v6 1/8] lib: rsa: distinguish between tpl and spl for CONFIG_RSA_VERIFY Message-ID: <20200618142328.1753036-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 Reviewed-by: Simon Glass --- 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 From patchwork Thu Jun 18 14:23:22 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: =?utf-8?q?Heiko_St=C3=BCbner?= X-Patchwork-Id: 242604 List-Id: U-Boot discussion From: heiko at sntech.de (Heiko Stuebner) Date: Thu, 18 Jun 2020 16:23:22 +0200 Subject: [PATCH v6 2/8] lib: rsa: take spl/non-spl into account when building rsa_verify_with_pkey() In-Reply-To: <20200618142328.1753036-1-heiko@sntech.de> References: <20200618142328.1753036-1-heiko@sntech.de> Message-ID: <20200618142328.1753036-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: Simon Glass --- 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); From patchwork Thu Jun 18 14:23:23 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: =?utf-8?q?Heiko_St=C3=BCbner?= X-Patchwork-Id: 242601 List-Id: U-Boot discussion From: heiko at sntech.de (Heiko Stuebner) Date: Thu, 18 Jun 2020 16:23:23 +0200 Subject: [PATCH v6 3/8] lib: rsa: bring exp_len in line when generating a key_prop In-Reply-To: <20200618142328.1753036-1-heiko@sntech.de> References: <20200618142328.1753036-1-heiko@sntech.de> Message-ID: <20200618142328.1753036-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: Simon Glass --- 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 Thu Jun 18 14:23:24 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: =?utf-8?q?Heiko_St=C3=BCbner?= X-Patchwork-Id: 242603 List-Id: U-Boot discussion From: heiko at sntech.de (Heiko Stuebner) Date: Thu, 18 Jun 2020 16:23:24 +0200 Subject: [PATCH v6 4/8] lib: rsa: fix allocated size for rr and rrtmp in rsa_gen_key_prop() In-Reply-To: <20200618142328.1753036-1-heiko@sntech.de> References: <20200618142328.1753036-1-heiko@sntech.de> Message-ID: <20200618142328.1753036-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 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 Signed-off-by: Heiko Stuebner Reviewed-by: Simon Glass --- 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) { From patchwork Thu Jun 18 14:23:25 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: =?utf-8?q?Heiko_St=C3=BCbner?= X-Patchwork-Id: 242607 List-Id: U-Boot discussion From: heiko at sntech.de (Heiko Stuebner) Date: Thu, 18 Jun 2020 16:23:25 +0200 Subject: [PATCH v6 5/8] lib: rsa: free local arrays after use in rsa_gen_key_prop() In-Reply-To: <20200618142328.1753036-1-heiko@sntech.de> References: <20200618142328.1753036-1-heiko@sntech.de> Message-ID: <20200618142328.1753036-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: Simon Glass --- 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; } From patchwork Thu Jun 18 14:23:26 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: =?utf-8?q?Heiko_St=C3=BCbner?= X-Patchwork-Id: 242605 List-Id: U-Boot discussion From: heiko at sntech.de (Heiko Stuebner) Date: Thu, 18 Jun 2020 16:23:26 +0200 Subject: [PATCH v6 6/8] lib: rsa: add documentation to padding_pss_verify to document limitations In-Reply-To: <20200618142328.1753036-1-heiko@sntech.de> References: <20200618142328.1753036-1-heiko@sntech.de> Message-ID: <20200618142328.1753036-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: Simon Glass --- 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) From patchwork Thu Jun 18 14:23:27 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: =?utf-8?q?Heiko_St=C3=BCbner?= X-Patchwork-Id: 242606 List-Id: U-Boot discussion From: heiko at sntech.de (Heiko Stuebner) Date: Thu, 18 Jun 2020 16:23:27 +0200 Subject: [PATCH v6 7/8] spl: fit: select SPL_HASH_SUPPORT for SPL_FIT_SIGNATURE In-Reply-To: <20200618142328.1753036-1-heiko@sntech.de> References: <20200618142328.1753036-1-heiko@sntech.de> Message-ID: <20200618142328.1753036-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 Reviewed-by: Simon Glass --- 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 From patchwork Thu Jun 18 14:23:28 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: =?utf-8?q?Heiko_St=C3=BCbner?= X-Patchwork-Id: 242608 List-Id: U-Boot discussion From: heiko at sntech.de (Heiko Stuebner) Date: Thu, 18 Jun 2020 16:23:28 +0200 Subject: [PATCH v6 8/8] spl: fit: select SPL_CRYPTO_SUPPORT for SPL_FIT_SIGNATURE In-Reply-To: <20200618142328.1753036-1-heiko@sntech.de> References: <20200618142328.1753036-1-heiko@sntech.de> Message-ID: <20200618142328.1753036-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 Reviewed-by: Simon Glass --- 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