| Message ID | 20200127102740.26831-1-takahiro.akashi@linaro.org |
|---|---|
| Headers | show |
| Series | rsa: extend rsa_verify() for UEFI secure boot | expand |
On Mon, Jan 27, 2020 at 07:27:33PM +0900, AKASHI Takahiro wrote: > # This patch set is a prerequisite for UEFI secure boot. > > The current rsa_verify() requires five parameters for a RSA public key > for efficiency while RSA, in theory, requires only two. In addition, > those parameters are expected to come from FIT image. > > So this function won't fit very well when we want to use it for the purpose > of implementing UEFI secure boot, in particular, image authentication > as well as variable authentication, where the essential two parameters > are set to be retrieved from one of X509 certificates in signature > database. > > So, in this patch, additional three parameters will be calculated > on the fly when rsa_verify() is called without fdt which should contain > parameters above. > > This calculation heavily relies on "big-number (or multi-precision) > library." Therefore some routines from BearSSL[1] under MIT license are > imported in this implementation. See Patch#4. > # Please let me know if this is not appropriate. > > Prerequisite: > * public key parser in my "import x509/pkcs7 parser" patch[2] > > # Checkpatch will complain with lots of warnings/errors, but > # I intentionally don't fix them for maximum maintainability. > > [1] https://bearssl.org/ > [2] https://lists.denx.de/pipermail/u-boot/2019-November/390127.html At this point it needs to be rebased again. There's a ton of failures in https://gitlab.denx.de/u-boot/u-boot/pipelines/2198 which is after I did https://gitlab.denx.de/u-boot/u-boot/commit/7db0379f85995d8c7673db7b04eb36d96546c9c8 and I'll put a proper commit message on that later today and post it and CC relevant parties. It's otherwise looking good. I do want to confirm that on boards like minnowmax the slight growth in fit_image_check_sig is expected. It's only 6 bytes so it probably is and we get a larger reduction in rsa_verify all-around. Thanks!
Hi Tom, On Fri, Feb 14, 2020 at 07:29:37AM -0500, Tom Rini wrote: > On Mon, Jan 27, 2020 at 07:27:33PM +0900, AKASHI Takahiro wrote: > > > # This patch set is a prerequisite for UEFI secure boot. > > > > The current rsa_verify() requires five parameters for a RSA public key > > for efficiency while RSA, in theory, requires only two. In addition, > > those parameters are expected to come from FIT image. > > > > So this function won't fit very well when we want to use it for the purpose > > of implementing UEFI secure boot, in particular, image authentication > > as well as variable authentication, where the essential two parameters > > are set to be retrieved from one of X509 certificates in signature > > database. > > > > So, in this patch, additional three parameters will be calculated > > on the fly when rsa_verify() is called without fdt which should contain > > parameters above. > > > > This calculation heavily relies on "big-number (or multi-precision) > > library." Therefore some routines from BearSSL[1] under MIT license are > > imported in this implementation. See Patch#4. > > # Please let me know if this is not appropriate. > > > > Prerequisite: > > * public key parser in my "import x509/pkcs7 parser" patch[2] > > > > # Checkpatch will complain with lots of warnings/errors, but > > # I intentionally don't fix them for maximum maintainability. > > > > [1] https://bearssl.org/ > > [2] https://lists.denx.de/pipermail/u-boot/2019-November/390127.html > > At this point it needs to be rebased again. There's a ton of failures > in https://gitlab.denx.de/u-boot/u-boot/pipelines/2198 which is after I I think that you have wrongly merged my rsa extension patch here. Looking at your modified commit, https://gitlab.denx.de/u-boot/u-boot/commit/13fb61ce20dcd65cd4ccba1554eca6343c92ed6b there is one missing hunk from my original. Please revert the change in include/image.h and then apply a diff attached below. > did > https://gitlab.denx.de/u-boot/u-boot/commit/7db0379f85995d8c7673db7b04eb36d96546c9c8 > and I'll put a proper commit message on that later today and post it and > CC relevant parties. I believe that your commit above has nothing to do with my patch (and test failures). > It's otherwise looking good. I do want to confirm > that on boards like minnowmax the slight growth in fit_image_check_sig > is expected. It's only 6 bytes so it probably is and we get a larger > reduction in rsa_verify all-around. Growth due to my patch?? Thanks, -Takahiro Akashi > Thanks! > > -- > Tom ===8<=== diff --git a/include/image.h b/include/image.h index b316d167d8d7..eb7aa5622aa3 100644 --- a/include/image.h +++ b/include/image.h @@ -1114,6 +1114,7 @@ int fit_conf_get_prop_node(const void *fit, int noffset, int fit_check_ramdisk(const void *fit, int os_noffset, uint8_t arch, int verify); +#endif /* IMAGE_ENABLE_FIT */ int calculate_hash(const void *data, int data_len, const char *algo, uint8_t *value, int *value_len); @@ -1126,16 +1127,20 @@ int calculate_hash(const void *data, int data_len, const char *algo, # if defined(CONFIG_FIT_SIGNATURE) # define IMAGE_ENABLE_SIGN 1 # define IMAGE_ENABLE_VERIFY 1 +# define FIT_IMAGE_ENABLE_VERIFY 1 # include <openssl/evp.h> # else # define IMAGE_ENABLE_SIGN 0 # define IMAGE_ENABLE_VERIFY 0 +# define FIT_IMAGE_ENABLE_VERIFY 0 # endif #else # define IMAGE_ENABLE_SIGN 0 -# define IMAGE_ENABLE_VERIFY CONFIG_IS_ENABLED(FIT_SIGNATURE) +# define IMAGE_ENABLE_VERIFY CONFIG_IS_ENABLED(RSA_VERIFY) +# define FIT_IMAGE_ENABLE_VERIFY CONFIG_IS_ENABLED(FIT_SIGNATURE) #endif +#if IMAGE_ENABLE_FIT #ifdef USE_HOSTCC void *image_get_host_blob(void); void image_set_host_blob(void *host_blob); @@ -1149,6 +1154,7 @@ void image_set_host_blob(void *host_blob); #else #define IMAGE_ENABLE_BEST_MATCH 0 #endif +#endif /* IMAGE_ENABLE_FIT */ /* Information passed to the signing routines */ struct image_sign_info { @@ -1166,16 +1172,12 @@ struct image_sign_info { const char *engine_id; /* Engine to use for signing */ }; -#endif /* Allow struct image_region to always be defined for rsa.h */ - /* A part of an image, used for hashing */ struct image_region { const void *data; int size; }; -#if IMAGE_ENABLE_FIT - #if IMAGE_ENABLE_VERIFY # include <u-boot/rsa-checksum.h> #endif @@ -1276,6 +1278,8 @@ struct crypto_algo *image_get_crypto_algo(const char *full_name); */ struct padding_algo *image_get_padding_algo(const char *name); +#if IMAGE_ENABLE_FIT + /** * fit_image_verify_required_sigs() - Verify signatures marked as 'required' *
On Mon, Feb 17, 2020 at 10:42:41AM +0900, AKASHI Takahiro wrote: > Hi Tom, > > On Fri, Feb 14, 2020 at 07:29:37AM -0500, Tom Rini wrote: > > On Mon, Jan 27, 2020 at 07:27:33PM +0900, AKASHI Takahiro wrote: > > > > > # This patch set is a prerequisite for UEFI secure boot. > > > > > > The current rsa_verify() requires five parameters for a RSA public key > > > for efficiency while RSA, in theory, requires only two. In addition, > > > those parameters are expected to come from FIT image. > > > > > > So this function won't fit very well when we want to use it for the purpose > > > of implementing UEFI secure boot, in particular, image authentication > > > as well as variable authentication, where the essential two parameters > > > are set to be retrieved from one of X509 certificates in signature > > > database. > > > > > > So, in this patch, additional three parameters will be calculated > > > on the fly when rsa_verify() is called without fdt which should contain > > > parameters above. > > > > > > This calculation heavily relies on "big-number (or multi-precision) > > > library." Therefore some routines from BearSSL[1] under MIT license are > > > imported in this implementation. See Patch#4. > > > # Please let me know if this is not appropriate. > > > > > > Prerequisite: > > > * public key parser in my "import x509/pkcs7 parser" patch[2] > > > > > > # Checkpatch will complain with lots of warnings/errors, but > > > # I intentionally don't fix them for maximum maintainability. > > > > > > [1] https://bearssl.org/ > > > [2] https://lists.denx.de/pipermail/u-boot/2019-November/390127.html > > > > At this point it needs to be rebased again. There's a ton of failures > > in https://gitlab.denx.de/u-boot/u-boot/pipelines/2198 which is after I > > I think that you have wrongly merged my rsa extension patch here. > Looking at your modified commit, > https://gitlab.denx.de/u-boot/u-boot/commit/13fb61ce20dcd65cd4ccba1554eca6343c92ed6b > there is one missing hunk from my original. > Please revert the change in include/image.h and then apply a diff attached below. Please rebase and repost the series. > > did > > https://gitlab.denx.de/u-boot/u-boot/commit/7db0379f85995d8c7673db7b04eb36d96546c9c8 > > and I'll put a proper commit message on that later today and post it and > > CC relevant parties. > > I believe that your commit above has nothing to do with my patch > (and test failures). I'll re-confirm things then with the next post. > > It's otherwise looking good. I do want to confirm > > that on boards like minnowmax the slight growth in fit_image_check_sig > > is expected. It's only 6 bytes so it probably is and we get a larger > > reduction in rsa_verify all-around. > > Growth due to my patch?? Unless it's something else I mis-merged, yes. But given the area this series works on, it's not unexpected growth. Thanks!
# This patch set is a prerequisite for UEFI secure boot. The current rsa_verify() requires five parameters for a RSA public key for efficiency while RSA, in theory, requires only two. In addition, those parameters are expected to come from FIT image. So this function won't fit very well when we want to use it for the purpose of implementing UEFI secure boot, in particular, image authentication as well as variable authentication, where the essential two parameters are set to be retrieved from one of X509 certificates in signature database. So, in this patch, additional three parameters will be calculated on the fly when rsa_verify() is called without fdt which should contain parameters above. This calculation heavily relies on "big-number (or multi-precision) library." Therefore some routines from BearSSL[1] under MIT license are imported in this implementation. See Patch#4. # Please let me know if this is not appropriate. Prerequisite: * public key parser in my "import x509/pkcs7 parser" patch[2] # Checkpatch will complain with lots of warnings/errors, but # I intentionally don't fix them for maximum maintainability. [1] https://bearssl.org/ [2] https://lists.denx.de/pipermail/u-boot/2019-November/390127.html Changes in v6 (Jan 27, 2020) * rebased to v2020.01 * change CONFIG_UT_LIB_RSA dependencies after Tom's suggestion (patch#6) * add patch#7 to enable RSA library test on sandbox Changes in v5 (Dec 17, 2019) * modify RSA_VERIFY-related configuration to fix size growth problem on some platforms (T1042RDB_PI_NAND_SECURE_BOOT and else), adding IMAGE_SIGN_INFO config (patch #1, #2, #6) * simplify kconfig dependencies; - RSA_VERIFY_WITH_PKEY *selects* RSA_VERIFY instead of "depends on" (patch#2) - remove some implicit dependencies (patch#6) Changes in v4 (Nov 21, 2019) * rebased to v2020.01-rc3 * change a function prototype of rsa_gen_key_prop() to return an error code (patch#4,#5) * re-order include files in alphabetical order (patch#6) * add some comments per Simon's review comments Changes in v3 (Nov 13, 2019) * remove RSA_VERIFY_WITH_PKEY, which is to be added in patch#2 (patch#1) * modify unit test Kconfg due to removal of test/lib/Kconfig (patch#6) Changes in v2 (Oct 29, 2019) * fix build errors at Travis CI * not include linux/kconfig.h (patch#1) * add a separate patch for adding CONFIG_RSA_VERIFY_WITH_PKEY (patch#2) * take a prerequisite patch from my "secure boot patch" (patch#3) * add a dependency on RSA_PUBLIC_KEY_PARSER (patch#4) * remove "inline" directives (patch#4) * add function descriptions, which mostly come from BearSSL's src/inner.h (patch#4) * improve Kconfig help text after Simon's comment (patch#5) * add function description of rsa_verify_with_pkey() (patch#5) * modify rsa_verify() to use "if (CONFIG_IS_ENABLED(...) " style rather than "#ifdef CONFIG_..." (patch#5) * add function tests (patch#6) Changes in v1 (Oct 9, 2019) * fix a build error on pine64-lts_defconfig (reported by Heinrich) by defining FIT_IMAGE_ENABLE_VERIFY flag and adding SPL_RSA_VERIFY config (patch#1) * remove FIT-specific code from image-sig.c and put them to new image-fit-sig.c to allow us to disable CONFIG_FIT_SIGNATURE (patch#1) * compile rsa-keyprop.c only if necessary (i.e. if CONFIG_RSA_VERIFY_WITH_PKEY) (patch#2) * add SPDX license identifier in rsa-keyprop.c (patch#2) * include <common.h> instead of <stdio.h> (patch#2) * use U-Boot's byteorder helper functions instead of BearSSL's (patch#2) AKASHI Takahiro (7): lib: rsa: decouple rsa from FIT image verification rsa: add CONFIG_RSA_VERIFY_WITH_PKEY config include: image.h: add key info to image_sign_info lib: rsa: generate additional parameters for public key lib: rsa: add rsa_verify_with_pkey() test: add rsa_verify() unit test test: enable RSA library test on sandbox Kconfig | 4 + common/Kconfig | 7 + common/Makefile | 3 +- common/image-fit-sig.c | 417 +++++++++++++++++ common/image-fit.c | 6 +- common/image-sig.c | 396 ---------------- configs/sandbox64_defconfig | 1 + configs/sandbox_defconfig | 1 + configs/sandbox_flattree_defconfig | 1 + configs/sandbox_spl_defconfig | 1 + include/image.h | 20 +- include/u-boot/rsa-mod-exp.h | 23 + lib/rsa/Kconfig | 27 ++ lib/rsa/Makefile | 3 +- lib/rsa/rsa-keyprop.c | 725 +++++++++++++++++++++++++++++ lib/rsa/rsa-verify.c | 137 ++++-- test/Kconfig | 10 + test/lib/Makefile | 1 + test/lib/rsa.c | 206 ++++++++ tools/Makefile | 2 +- 20 files changed, 1556 insertions(+), 435 deletions(-) create mode 100644 common/image-fit-sig.c create mode 100644 lib/rsa/rsa-keyprop.c create mode 100644 test/lib/rsa.c