From patchwork Mon Feb 17 01:42:41 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: AKASHI Takahiro X-Patchwork-Id: 236417 List-Id: U-Boot discussion From: takahiro.akashi at linaro.org (AKASHI Takahiro) Date: Mon, 17 Feb 2020 10:42:41 +0900 Subject: [PATCH v6 0/7] rsa: extend rsa_verify() for UEFI secure boot In-Reply-To: <20200214122937.GA27143@bill-the-cat> References: <20200127102740.26831-1-takahiro.akashi@linaro.org> <20200214122937.GA27143@bill-the-cat> Message-ID: <20200217014240.GC22953@linaro.org> 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 # 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 #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' *