On Mon, Oct 21, 2019 at 06:17:03PM -0600, Simon Glass wrote: > Hi Takahiro, > > On Tue, 8 Oct 2019 at 23:27, AKASHI Takahiro <takahiro.aka...@linaro.org> > 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 <takahiro.aka...@linaro.org> > > --- > > include/u-boot/rsa-mod-exp.h | 3 + > > lib/rsa/Kconfig | 7 + > > lib/rsa/Makefile | 1 + > > lib/rsa/rsa-keyprop.c | 585 +++++++++++++++++++++++++++++++++++ > > 4 files changed, 596 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..ca189292d869 100644 > > --- a/include/u-boot/rsa-mod-exp.h > > +++ b/include/u-boot/rsa-mod-exp.h > > @@ -26,6 +26,9 @@ struct key_prop { > > uint32_t exp_len; /* Exponent length in number of uint8_t */ > > }; > > > > +struct key_prop *rsa_gen_key_prop(const void *key, uint32_t keylen); > > +void rsa_free_key_prop(struct key_prop *prop); > > Please add full function comments.
Sure. > > + > > /** > > * rsa_mod_exp_sw() - Perform RSA Modular Exponentiation in sw > > * > > diff --git a/lib/rsa/Kconfig b/lib/rsa/Kconfig > > index 62b7ab9c5e5c..d1743d7a4c47 100644 > > --- a/lib/rsa/Kconfig > > +++ b/lib/rsa/Kconfig > > @@ -30,6 +30,13 @@ config RSA_VERIFY > > help > > Add RSA signature verification support. > > > > +config RSA_VERIFY_WITH_PKEY > > + bool "Execute RSA verification without key parameters from FDT" > > + depends on RSA > > + help > > + This options enables RSA signature verification without > > + using public key parameters which is embedded control FDT. > > How about something like... > > The standard RSA-signature verification code uses .... > > This does not suit the use case where ... > > This option enables ... I will try to improve ... > > + > > config RSA_SOFTWARE_EXP > > bool "Enable driver for RSA Modular Exponentiation in software" > > depends on DM > > 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, w...@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..d7d222e9bed9 > > --- /dev/null > > +++ b/lib/rsa/rsa-keyprop.c > > @@ -0,0 +1,585 @@ > > +// 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: > > + * Copyright (c) 2016 Thomas Pornin <por...@bolet.org> > > + */ > > + > > +#include <common.h> > > +#include <image.h> > > +#include <malloc.h> > > +#include <asm/byteorder.h> > > +#include <crypto/internal/rsa.h> > > +#include <u-boot/rsa-mod-exp.h> > > + > > +static inline unsigned > > Please drop the inlines unless needed. Would prefer to leave this to > the compiler. Okay. > Also please keep the function name on the same line as the 'static'. Okay. > > +br_dec16be(const void *src) > > +{ > > + return be16_to_cpup(src); > > +} > > + > > +static inline uint32_t > > +br_dec32be(const void *src) > > +{ > > + return be32_to_cpup(src); > > +} > > + > > +static inline void > > +br_enc32be(void *dst, uint32_t x) > > +{ > > + __be32 tmp; > > + > > + tmp = cpu_to_be32(x); > > + memcpy(dst, &tmp, sizeof(tmp)); > > +} > > + > > +/* stripped version of src/inner.h */ > > ?? I mean that only a small subset of BearSSL's src/inner.h was imported here. > > + > > +static inline uint32_t > > +NOT(uint32_t ctl) > > +{ > > + return ctl ^ 1; > > +} > > + > > All of these functions need full comments please. All the functions imported from BearSSL? Please note that all the functions, except rsa_gen_key_prop() rsa_free_key_prop(), are just imported without changes and all are declared as static. > > +static inline uint32_t > > +MUX(uint32_t ctl, uint32_t x, uint32_t y) > > +{ > > + return y ^ (-ctl & (x ^ y)); > > +} > > + > > +static inline uint32_t > > +EQ(uint32_t x, uint32_t y) > > Should use lower case. Please run patman. I would like to honer the original code(BearSSL)'s notation including upper/lower cases for minimizing differences. > > +{ > > + uint32_t q; > > + > > + q = x ^ y; > > + return NOT((q | -q) >> 31); > > +} > > + > > [...] > > Regards, > Simon _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot