[PATCH v2 3/8] lib: crypto: enable x509_check_for_self_signed()

[Heinrich Schuchardt]

On 16.06.20 07:26, AKASHI Takahiro wrote:
> When the file, x509_public_key.c, was imported from linux code in
>     commit b4adf627d5b7 ("lib: crypto: add x509 parser"),
> x509_check_for_self_signed() was commented out for simplicity.
>
> Now it need be enabled in order to make pkcs7_verify_one(), which will be
> imported in a later patch, functional.
>
> Signed-off-by: AKASHI Takahiro <takahiro.akashi at linaro.org>
> ---
>  lib/crypto/x509_cert_parser.c |  2 --
>  lib/crypto/x509_public_key.c  | 33 +++++++++++++++++++++++++--------
>  2 files changed, 25 insertions(+), 10 deletions(-)
>
> diff --git a/lib/crypto/x509_cert_parser.c b/lib/crypto/x509_cert_parser.c
> index 5f984b9dfdae..eb24349460c2 100644
> --- a/lib/crypto/x509_cert_parser.c
> +++ b/lib/crypto/x509_cert_parser.c
> @@ -142,12 +142,10 @@ struct x509_certificate *x509_cert_parse(const void *data, size_t datalen)
>  	}
>  	cert->id = kid;
>
> -#ifndef __UBOOT__
>  	/* Detect self-signed certificates */
>  	ret = x509_check_for_self_signed(cert);
>  	if (ret < 0)
>  		goto error_decode;
> -#endif
>
>  	kfree(ctx);
>  	return cert;
> diff --git a/lib/crypto/x509_public_key.c b/lib/crypto/x509_public_key.c
> index 571af9a0adf9..91810a864049 100644
> --- a/lib/crypto/x509_public_key.c
> +++ b/lib/crypto/x509_public_key.c
> @@ -8,6 +8,7 @@
>  #define pr_fmt(fmt) "X.509: "fmt
>  #ifdef __UBOOT__
>  #include <common.h>
> +#include <image.h>
>  #include <dm/devres.h>
>  #include <linux/compat.h>
>  #include <linux/err.h>
> @@ -18,6 +19,7 @@
>  #include <linux/kernel.h>
>  #ifdef __UBOOT__
>  #include <crypto/x509_parser.h>
> +#include <u-boot/rsa-checksum.h>
>  #else
>  #include <linux/slab.h>
>  #include <keys/asymmetric-subtype.h>
> @@ -35,7 +37,9 @@
>  int x509_get_sig_params(struct x509_certificate *cert)
>  {
>  	struct public_key_signature *sig = cert->sig;
> -#ifndef __UBOOT__
> +#ifdef __UBOOT__
> +	struct image_region region;
> +#else
>  	struct crypto_shash *tfm;
>  	struct shash_desc *desc;
>  	size_t desc_size;
> @@ -63,12 +67,25 @@ int x509_get_sig_params(struct x509_certificate *cert)
>  	sig->s_size = cert->raw_sig_size;
>
>  #ifdef __UBOOT__
> -	/*
> -	 * Note:
> -	 * This part (filling sig->digest) should be implemented if
> -	 * x509_check_for_self_signed() is enabled x509_cert_parse().
> -	 * Currently, this check won't affect UEFI secure boot.
> -	 */
> +	if (!sig->hash_algo)
> +		return -ENOPKG;
> +	if (!strcmp(sig->hash_algo, "sha256"))
> +		sig->digest_size = SHA256_SUM_LEN;
> +	else if (!strcmp(sig->hash_algo, "sha1"))
> +		sig->digest_size = SHA1_SUM_LEN;
> +	else
> +		return -ENOPKG;

It would be preferable to call hash_lookup_algo() instead of hard coding
hash sizes in multiple places.

Best regards

Heinrich

> +
> +	sig->digest = calloc(1, sig->digest_size);
> +	if (!sig->digest)
> +		return -ENOMEM;
> +
> +	region.data = cert->tbs;
> +	region.size = cert->tbs_size;
> +	hash_calculate(sig->hash_algo, &region, 1, sig->digest);
> +
> +	/* TODO: is_hash_blacklisted()? */
> +
>  	ret = 0;
>  #else
>  	/* Allocate the hashing algorithm we're going to need and find out how
> @@ -118,7 +135,6 @@ error:
>  	return ret;
>  }
>
> -#ifndef __UBOOT__
>  /*
>   * Check for self-signedness in an X.509 cert and if found, check the signature
>   * immediately if we can.
> @@ -175,6 +191,7 @@ not_self_signed:
>  	return 0;
>  }
>
> +#ifndef __UBOOT__
>  /*
>   * Attempt to parse a data blob for a key as an X509 certificate.
>   */
>