From mboxrd@z Thu Jan 1 00:00:00 1970 From: AKASHI Takahiro Date: Wed, 8 Jul 2020 15:21:19 +0900 Subject: [PATCH v2 2/8] lib: crypto: add public_key_verify_signature() In-Reply-To: <54d2330a-3767-80e6-5ca6-373eb5e0c07c@gmx.de> References: <20200616052655.4845-1-takahiro.akashi@linaro.org> <20200616052655.4845-3-takahiro.akashi@linaro.org> <54d2330a-3767-80e6-5ca6-373eb5e0c07c@gmx.de> Message-ID: <20200708062119.GA19234@laputa> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Heinrich, On Tue, Jul 07, 2020 at 12:04:29PM +0200, Heinrich Schuchardt wrote: > On 16.06.20 07:26, AKASHI Takahiro wrote: > > This function will be called from x509_check_for_self_signed() and > > pkcs7_verify_one(), which will be imported from linux in a later patch. > > > > While it does exist in linux code and has a similar functionality of > > rsa_verify(), it calls further linux-specific interfaces inside. > > That could lead to more files being imported from linux. > > > > So simply re-implement it here instead of re-using the code. > > > > Signed-off-by: AKASHI Takahiro > > --- > > include/crypto/public_key.h | 2 +- > > lib/crypto/public_key.c | 63 ++++++++++++++++++++++++++++++++++++- > > 2 files changed, 63 insertions(+), 2 deletions(-) > > > > diff --git a/include/crypto/public_key.h b/include/crypto/public_key.h > > index 436a1ee1ee64..3ba90fcc3483 100644 > > --- a/include/crypto/public_key.h > > +++ b/include/crypto/public_key.h > > @@ -82,9 +82,9 @@ extern int decrypt_blob(struct kernel_pkey_params *, const void *, void *); > > extern int create_signature(struct kernel_pkey_params *, const void *, void *); > > extern int verify_signature(const struct key *, > > const struct public_key_signature *); > > +#endif /* __UBOOT__ */ > > > > int public_key_verify_signature(const struct public_key *pkey, > > const struct public_key_signature *sig); > > -#endif /* !__UBOOT__ */ > > > > #endif /* _LINUX_PUBLIC_KEY_H */ > > diff --git a/lib/crypto/public_key.c b/lib/crypto/public_key.c > > index e12ebbb3d0c5..013ea71a180f 100644 > > --- a/lib/crypto/public_key.c > > +++ b/lib/crypto/public_key.c > > @@ -25,7 +25,10 @@ > > #include > > #endif > > #include > > -#ifndef __UBOOT__ > > +#ifdef __UBOOT__ > > +#include > > +#include > > +#else > > #include > > #endif > > > > @@ -80,6 +83,64 @@ void public_key_signature_free(struct public_key_signature *sig) > > } > > EXPORT_SYMBOL_GPL(public_key_signature_free); > > > > +/** > > + * public_key_verify_signature - Verify a signature using a public key. > > + * > > + * @pkey: Public key > > + * @sig: Signature > > + * > > + * Verify a signature, @sig, using a RSA public key, @pkey. > > + * > > + * Return: 0 - verified, non-zero error code - otherwise > > + */ > > +int public_key_verify_signature(const struct public_key *pkey, > > + const struct public_key_signature *sig) > > +{ > > + struct image_sign_info info; > > + struct image_region region; > > + int ret; > > + > > + pr_devel("==>%s()\n", __func__); > > + > > + if (!pkey || !sig) > > + return -EINVAL; > > + > > + if (pkey->key_is_private) > > + return -EINVAL; > > + > > + memset(&info, '\0', sizeof(info)); > > + info.padding = image_get_padding_algo("pkcs-1.5"); > > + /* > > + * Note: image_get_[checksum|crypto]_algo takes an string > > nits > > %/an string/a string/g Fixed. > > + * argument like "," > > + * TODO: support other hash algorithms > > + */ > > + if (!strcmp(sig->hash_algo, "sha1")) { > > In our coding sig->key_algo can have values "rsa" or "ecrdsa". See > lib/crypto/x509_cert_parser.c:471. Please, check the value. > > You can use sig->s_size * 8 to get the actual key size. I think that you are mentioning a check against encryption algo. Yeah, I will add it although it will anyhow fail in rsa_verify_with_pkey(). > You should check the return value of image_get_checksum_algo(). I think it's nothing but an overhead as the arguments here have "static" values, but will add it if you want. > if (!strcmp(sig->key_algo, "rsa")) { > char algo[16]; > snprintf(algo, sizeof(algo), "%s,rsa%d", sig->hash_algo, 8 * sig->s_size); > info.checksum = image_get_checksum_algo(algo); > } > if (!info.checksum) > return -ENOPKG; > } > > > + info.checksum = image_get_checksum_algo("sha1,rsa2048"); > > + info.name = "sha1,rsa2048"; > > + } else if (!strcmp(sig->hash_algo, "sha256")) { > > + info.checksum = image_get_checksum_algo("sha256,rsa2048"); > > + info.name = "sha256,rsa2048"; > > + } else { > > + pr_warn("unknown msg digest algo: %s\n", sig->hash_algo); > > Please, use log_warning(). No, I can't agree. I prefer to call pr_xxx() very much more than log_xxx() as this code is embedded in a file imported from linux. Moreover, log_xxx() is seldom used even across U-Boot. > > + return -ENOPKG; > > + } > > + info.crypto = image_get_crypto_algo(info.name); > > + > > + info.key = pkey->key; > > + info.keylen = pkey->keylen; > > + > > + region.data = sig->digest; > > + region.size = sig->digest_size; > > + > > + if (rsa_verify_with_pkey(&info, sig->digest, sig->s, sig->s_size)) > > Why warning above but no debug message here? Because * the reason of EKEYREJECTED failure is trivial in this function. * rsa_verify_With_pkey() outputs a bunch of verbose and useful messages instead. > > + ret = -EKEYREJECTED; > > + else > > + ret = 0; > > + > > + pr_devel("<==%s() = %d\n", __func__, ret); > > log_content() See my opinion & policy above. -Takahiro Akashi > Best regards > > Heinrich > > > > + return ret; > > +} > > #else > > /* > > * Destroy a public key algorithm key. > > >