From mboxrd@z Thu Jan 1 00:00:00 1970 From: AKASHI Takahiro Date: Wed, 20 Nov 2019 14:53:28 +0900 Subject: [U-Boot] [PATCH v3 4/6] lib: rsa: generate additional parameters for public key In-Reply-To: References: <20191113004730.30139-1-takahiro.akashi@linaro.org> <20191113004730.30139-5-takahiro.akashi@linaro.org> Message-ID: <20191120055326.GA22427@linaro.org> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Simon, On Tue, Nov 19, 2019 at 06:59:59PM -0800, Simon Glass wrote: > Hi Takahiro, > > On Tue, 12 Nov 2019 at 16:47, AKASHI Takahiro > wrote: > > > > In the current implementation of FIT_SIGNATURE, five parameters for > > a RSA public key are required while only two of them are essential. > > (See rsa-mod-exp.h and uImage.FIT/signature.txt) > > This is a result of considering relatively limited computer power > > and resources on embedded systems, while such a assumption may not > > be quite practical for other use cases. > > > > In this patch, added is a function, rsa_gen_key_prop(), which will > > generate additional parameters for other uses, in particular > > UEFI secure boot, on the fly. > > > > Note: the current code uses some "big number" routines from BearSSL > > for the calculation. > > > > Signed-off-by: AKASHI Takahiro > > --- > > include/u-boot/rsa-mod-exp.h | 21 + > > lib/rsa/Kconfig | 1 + > > lib/rsa/Makefile | 1 + > > lib/rsa/rsa-keyprop.c | 717 +++++++++++++++++++++++++++++++++++ > > 4 files changed, 740 insertions(+) > > create mode 100644 lib/rsa/rsa-keyprop.c > > > > diff --git a/include/u-boot/rsa-mod-exp.h b/include/u-boot/rsa-mod-exp.h > > index 8a428c4b6a1a..374169b8304e 100644 > > --- a/include/u-boot/rsa-mod-exp.h > > +++ b/include/u-boot/rsa-mod-exp.h > > @@ -26,6 +26,27 @@ struct key_prop { > > uint32_t exp_len; /* Exponent length in number of uint8_t */ > > }; > > > > +/** > > + * rsa_gen_key_prop() - Generate key properties of RSA public key > > + * @key: Specifies key data in DER format > > + * @keylen: Length of @key > > + * > > + * This function takes a blob of encoded RSA public key data in DER > > + * format, parse it and generate all the relevant properties > > + * in key_prop structure. > > + * > > + * Return: Pointer to struct key_prop on success, NULL on error > > + */ > > +struct key_prop *rsa_gen_key_prop(const void *key, uint32_t keylen); > > + > > +/** > > + * rsa_free_key_prop() - Free key properties > > + * @prop: Pointer to struct key_prop > > + * > > + * This function frees all the memories allocated by rsa_gen_key_prop(). > > + */ > > +void rsa_free_key_prop(struct key_prop *prop); > > + > > /** > > * rsa_mod_exp_sw() - Perform RSA Modular Exponentiation in sw > > * > > diff --git a/lib/rsa/Kconfig b/lib/rsa/Kconfig > > index 71e4c06bf883..d1d6f6cf64a3 100644 > > --- a/lib/rsa/Kconfig > > +++ b/lib/rsa/Kconfig > > @@ -33,6 +33,7 @@ config RSA_VERIFY > > config RSA_VERIFY_WITH_PKEY > > bool "Execute RSA verification without key parameters from FDT" > > depends on RSA > > + imply 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 > > diff --git a/lib/rsa/Makefile b/lib/rsa/Makefile > > index c07305188e0c..14ed3cb4012b 100644 > > --- a/lib/rsa/Makefile > > +++ b/lib/rsa/Makefile > > @@ -6,4 +6,5 @@ > > # Wolfgang Denk, DENX Software Engineering, wd at denx.de. > > > > obj-$(CONFIG_$(SPL_)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-keyprop.c b/lib/rsa/rsa-keyprop.c > > new file mode 100644 > > index 000000000000..9458337cc608 > > --- /dev/null > > +++ b/lib/rsa/rsa-keyprop.c > > @@ -0,0 +1,717 @@ > > +// SPDX-License-Identifier: GPL-2.0+ and MIT > > +/* > > + * RSA library - generate parameters for a public key > > + * > > + * Copyright (c) 2019 Linaro Limited > > + * Author: AKASHI Takahiro > > + * > > + * Big number routines in this file come from BearSSL: > > Can we put these in their own file if we expect to update them? Really > should be called by the original filename and go in lib/bearsll. Honestly, I'd rather disagree with you because * in the original code, each file has only a single function, then we will have to introduce bunch of small source files. * it is quite unlikely for other components to re-use some of those functions in the future. So splitting the file would practically make little use. > [..] > > > +/** > > + * rsa_gen_key_prop() - Generate key properties of RSA public key > > + * @key: Specifies key data in DER format > > + * @keylen: Length of @key > > + * > > + * This function takes a blob of encoded RSA public key data in DER > > + * format, parse it and generate all the relevant properties > > + * in key_prop structure. > > + * > > + * Return: Pointer to struct key_prop on success, NULL on error > > + */ > > +struct key_prop *rsa_gen_key_prop(const void *key, uint32_t keylen) > > +{ > > + struct key_prop *prop; > > + struct rsa_key rsa_key; > > +#define BR_MAX_RSA_SIZE 4096 > > const int? Will change it to const int max_rsa_size = 4096; > > + uint32_t *n, *rr, *rrtmp; > > + int rlen, i, ret; > > + > > + prop = calloc(sizeof(*prop), 1); > > + if (!prop) > > + return NULL; > > + n = calloc(sizeof(uint32_t), 1 + (BR_MAX_RSA_SIZE >> 5)); > > + rr = calloc(sizeof(uint32_t), 1 + (BR_MAX_RSA_SIZE >> 5)); > > + rrtmp = calloc(sizeof(uint32_t), 1 + (BR_MAX_RSA_SIZE >> 5)); > > + if (!n || !rr || !rrtmp) > > + return NULL; > > This can cause memory leak and will likely generate a coverity warning. Okay, will fix it. > > + > > + ret = rsa_parse_pub_key(&rsa_key, key, keylen); > > + if (ret) > > + goto err; > > + > > + /* modulus */ > > + /* removing leading 0's */ > > + for (i = 0; i < rsa_key.n_sz && !rsa_key.n[i]; i++) > > + ; > > + prop->num_bits = (rsa_key.n_sz - i) * 8; > > + prop->modulus = malloc(rsa_key.n_sz - i); > > + if (!prop->modulus) > > + goto err; > > + memcpy((void *)prop->modulus, &rsa_key.n[i], rsa_key.n_sz - i); > > + > > + /* exponent */ > > + prop->public_exponent = calloc(1, sizeof(uint64_t)); > > + if (!prop->public_exponent) > > + goto err; > > + 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; > > + > > + /* n0 inverse */ > > + br_i32_decode(n, &rsa_key.n[i], rsa_key.n_sz - i); > > + prop->n0inv = br_i32_ninv32(n[1]); > > + > > + /* R^2 mod n; R = 2^(num_bits) */ > > + rlen = prop->num_bits * 2; /* #bits of R^2 = (2^num_bits)^2 */ > > + rr[0] = 0; > > + *(uint8_t *)&rr[0] = (1 << (rlen % 8)); > > + for (i = 1; i < (((rlen + 31) >> 5) + 1); i++) > > + rr[i] = 0; > > + br_i32_decode(rrtmp, rr, ((rlen + 7) >> 3) + 1); > > + br_i32_reduce(rr, rrtmp, n); > > + > > + rlen = (prop->num_bits + 7) >> 3; /* #bytes of R^2 mod n */ > > + prop->rr = malloc(rlen); > > + if (!prop->rr) > > + goto err; > > + br_i32_encode((void *)prop->rr, rlen, rr); > > + > > + return prop; > > + > > +err: > > + free(n); > > + free(rr); > > + free(rrtmp); > > + rsa_free_key_prop(prop); > > + return NULL; > > This should return -ENOMEM if we are out of memory. The caller cannot > divine what went wrong. Agree. Thank you for your review, -Takahiro Akashi > > +} > > -- > > 2.21.0 > > > > Regards, > Simon