| Message ID | 20200717071630.7363-2-takahiro.akashi@linaro.org |
|---|---|
| State | Superseded, archived |
| Delegated to: | Heinrich Schuchardt |
| Headers | show |
| Series | efi_loader: secure boot: support intermediate certificates in signature | expand |
On 7/17/20 9:16 AM, 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 <takahiro.akashi@linaro.org> > --- > include/crypto/public_key.h | 2 +- > lib/crypto/public_key.c | 70 ++++++++++++++++++++++++++++++++++++- > 2 files changed, 70 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..71a0e0356beb 100644 > --- a/lib/crypto/public_key.c > +++ b/lib/crypto/public_key.c > @@ -25,7 +25,10 @@ > #include <keys/asymmetric-subtype.h> > #endif > #include <crypto/public_key.h> > -#ifndef __UBOOT__ > +#ifdef __UBOOT__ > +#include <image.h> > +#include <u-boot/rsa.h> > +#else > #include <crypto/akcipher.h> > #endif Shouldn't we describe in the header of the file that the code is taken (at least partially) from Linux' crypto/asymmetric_keys/public_key.c? Please, also state the Linux version number. > > @@ -80,6 +83,71 @@ 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 a string > + * argument like "<checksum>,<crypto>" > + * TODO: support other hash algorithms > + */ > + if (strcmp(sig->pkey_algo, "rsa") || (sig->s_size * 8) != 2048) { > + pr_warn("Encryption is not RSA2048: %s%d\n", > + sig->pkey_algo, sig->s_size * 8); > + return -ENOPKG; > + } > + if (!strcmp(sig->hash_algo, "sha1")) { > + 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); > + return -ENOPKG; > + } > + info.crypto = image_get_crypto_algo(info.name); > + if (unlikely(IS_ERR(info.checksum) || IS_ERR(info.crypto))) > + return -ENOPKG; scripts/checkpatch.pl complains: WARNING: nested (un)?likely() calls, IS_ERR already uses unlikely() internally #104: FILE: lib/crypto/public_key.c:134: + if (unlikely(IS_ERR(info.checksum) || IS_ERR(info.crypto))) I cannot find this unlikely in the Linux v5.8-rc4 crypto/asymmetric_keys/public_key.c file. From where did you copy this code? Best regards Heinrich > + > + 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)) > + ret = -EKEYREJECTED; > + else > + ret = 0; > + > + pr_devel("<==%s() = %d\n", __func__, ret); > + return ret; > +} > #else > /* > * Destroy a public key algorithm key. >
Heinrich, On Sun, Jul 19, 2020 at 10:20:58AM +0200, Heinrich Schuchardt wrote: > On 7/17/20 9:16 AM, 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 <takahiro.akashi@linaro.org> > > --- > > include/crypto/public_key.h | 2 +- > > lib/crypto/public_key.c | 70 ++++++++++++++++++++++++++++++++++++- > > 2 files changed, 70 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..71a0e0356beb 100644 > > --- a/lib/crypto/public_key.c > > +++ b/lib/crypto/public_key.c > > @@ -25,7 +25,10 @@ > > #include <keys/asymmetric-subtype.h> > > #endif > > #include <crypto/public_key.h> > > -#ifndef __UBOOT__ > > +#ifdef __UBOOT__ > > +#include <image.h> > > +#include <u-boot/rsa.h> > > +#else > > #include <crypto/akcipher.h> > > #endif > > Shouldn't we describe in the header of the file that the code is taken > (at least partially) from Linux' crypto/asymmetric_keys/public_key.c? > > Please, also state the Linux version number. Please take a look at the commit c4e961ecec99 ("lib: crypto: add public key utility") as this file itself was imported in v2020-01. While I prefer to add such an information in a commit message, we should have an explicit rule for inclusion from outer sources for consistency over the tree if you want to see such an information in a file. > > > > @@ -80,6 +83,71 @@ 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 a string > > + * argument like "<checksum>,<crypto>" > > + * TODO: support other hash algorithms > > + */ > > + if (strcmp(sig->pkey_algo, "rsa") || (sig->s_size * 8) != 2048) { > > + pr_warn("Encryption is not RSA2048: %s%d\n", > > + sig->pkey_algo, sig->s_size * 8); > > + return -ENOPKG; > > + } > > + if (!strcmp(sig->hash_algo, "sha1")) { > > + 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); > > + return -ENOPKG; > > + } > > + info.crypto = image_get_crypto_algo(info.name); > > + if (unlikely(IS_ERR(info.checksum) || IS_ERR(info.crypto))) > > + return -ENOPKG; > > scripts/checkpatch.pl complains: > > WARNING: nested (un)?likely() calls, IS_ERR already uses unlikely() > internally > #104: FILE: lib/crypto/public_key.c:134: > + if (unlikely(IS_ERR(info.checksum) || IS_ERR(info.crypto))) Probably I missed it out when checking. > I cannot find this unlikely in the Linux v5.8-rc4 > crypto/asymmetric_keys/public_key.c file. From where did you copy this code? As I stated in the commit message, the whole code of this function was written from the scratch rather than by re-using linux code. -Takahiro Akashi > Best regards > > Heinrich > > > + > > + 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)) > > + ret = -EKEYREJECTED; > > + else > > + ret = 0; > > + > > + pr_devel("<==%s() = %d\n", __func__, ret); > > + return ret; > > +} > > #else > > /* > > * Destroy a public key algorithm key. > > >
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..71a0e0356beb 100644 --- a/lib/crypto/public_key.c +++ b/lib/crypto/public_key.c @@ -25,7 +25,10 @@ #include <keys/asymmetric-subtype.h> #endif #include <crypto/public_key.h> -#ifndef __UBOOT__ +#ifdef __UBOOT__ +#include <image.h> +#include <u-boot/rsa.h> +#else #include <crypto/akcipher.h> #endif @@ -80,6 +83,71 @@ 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 a string + * argument like "<checksum>,<crypto>" + * TODO: support other hash algorithms + */ + if (strcmp(sig->pkey_algo, "rsa") || (sig->s_size * 8) != 2048) { + pr_warn("Encryption is not RSA2048: %s%d\n", + sig->pkey_algo, sig->s_size * 8); + return -ENOPKG; + } + if (!strcmp(sig->hash_algo, "sha1")) { + 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); + return -ENOPKG; + } + info.crypto = image_get_crypto_algo(info.name); + if (unlikely(IS_ERR(info.checksum) || IS_ERR(info.crypto))) + return -ENOPKG; + + 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)) + ret = -EKEYREJECTED; + else + ret = 0; + + pr_devel("<==%s() = %d\n", __func__, ret); + return ret; +} #else /* * Destroy a public key algorithm key.
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 <takahiro.akashi@linaro.org> --- include/crypto/public_key.h | 2 +- lib/crypto/public_key.c | 70 ++++++++++++++++++++++++++++++++++++- 2 files changed, 70 insertions(+), 2 deletions(-)