[PATCH 6/8] lib: crypto: export and enhance pkcs7_verify_one()

[Heinrich Schuchardt]

On 6/9/20 7:13 AM, AKASHI Takahiro wrote:
> The function, pkcs7_verify_one(), will be utilized to rework signature
> verification logic aiming to support intermediate certificates in
> "chain of trust."
>
> To do that, its function interface is expanded, adding an extra argument
> which is expected to return the last certificate in trusted chain.
> Then, this last one must further be verified with signature database, db
> and/or dbx.
>
> Signed-off-by: AKASHI Takahiro <takahiro.akashi at linaro.org>
> ---
>  include/crypto/pkcs7.h    |  9 ++++++++-
>  lib/crypto/pkcs7_verify.c | 36 +++++++++++++++++++++++++++++++-----
>  2 files changed, 39 insertions(+), 6 deletions(-)
>
> diff --git a/include/crypto/pkcs7.h b/include/crypto/pkcs7.h
> index 8f5c8a7ee3b9..ca35df29f6fb 100644
> --- a/include/crypto/pkcs7.h
> +++ b/include/crypto/pkcs7.h
> @@ -27,7 +27,14 @@ extern int pkcs7_get_content_data(const struct pkcs7_message *pkcs7,
>  				  const void **_data, size_t *_datalen,
>  				  size_t *_headerlen);
>
> -#ifndef __UBOOT__
> +#ifdef __UBOOT__
> +struct pkcs7_signed_info;
> +struct x509_certificate;
> +
> +int pkcs7_verify_one(struct pkcs7_message *pkcs7,
> +		     struct pkcs7_signed_info *sinfo,
> +		     struct x509_certificate **signer);
> +#else
>  /*
>   * pkcs7_trust.c
>   */
> diff --git a/lib/crypto/pkcs7_verify.c b/lib/crypto/pkcs7_verify.c
> index 8b5cd7a443ae..220ce9f77cf0 100644
> --- a/lib/crypto/pkcs7_verify.c
> +++ b/lib/crypto/pkcs7_verify.c
> @@ -295,8 +295,14 @@ static int pkcs7_find_key(struct pkcs7_message *pkcs7,
>  /*
>   * Verify the internal certificate chain as best we can.
Hello Takahiro,

please, add a function description. See
https://www.kernel.org/doc/html/latest/doc-guide/kernel-doc.html#function-documentation

>   */
> +#ifdef __UBOOT__
> +static int pkcs7_verify_sig_chain(struct pkcs7_message *pkcs7,
> +				  struct pkcs7_signed_info *sinfo,
> +				  struct x509_certificate **signer)
> +#else
>  static int pkcs7_verify_sig_chain(struct pkcs7_message *pkcs7,
>  				  struct pkcs7_signed_info *sinfo)

I would prefer if you could change this to a single signature with the
argument 'signer' which is required below anyway.

Please, remove all #ifdef __UBOOT__.

Functions that are not always used can be marked as __maybe_unused if
this is required to avoid compiler warnings.

> +#endif
>  {
>  	struct public_key_signature *sig;
>  	struct x509_certificate *x509 = sinfo->signer, *p;
> @@ -305,6 +311,8 @@ static int pkcs7_verify_sig_chain(struct pkcs7_message *pkcs7,
>
>  	kenter("");
>
> +	*signer = NULL;
> +
>  	for (p = pkcs7->certs; p; p = p->next)
>  		p->seen = false;
>
> @@ -322,6 +330,9 @@ static int pkcs7_verify_sig_chain(struct pkcs7_message *pkcs7,
>  			for (p = sinfo->signer; p != x509; p = p->signer)
>  				p->blacklisted = true;
>  			pr_debug("- blacklisted\n");
> +#ifdef __UBOOT__
> +			*signer = x509;
> +#endif
>  			return 0;
>  		}
>
> @@ -347,6 +358,9 @@ static int pkcs7_verify_sig_chain(struct pkcs7_message *pkcs7,
>  				goto unsupported_crypto_in_x509;
>  			x509->signer = x509;
>  			pr_debug("- self-signed\n");
> +#ifdef __UBOOT__
> +			*signer = x509;
> +#endif
>  			return 0;
>  		}
a>
> @@ -377,6 +391,9 @@ static int pkcs7_verify_sig_chain(struct pkcs7_message *pkcs7,
>
>  		/* We didn't find the root of this chain */
>  		pr_debug("- top\n");
> +#ifdef __UBOOT__
> +		*signer = x509;
> +#endif
>  		return 0;
>
>  	found_issuer_check_skid:
> @@ -394,6 +411,9 @@ static int pkcs7_verify_sig_chain(struct pkcs7_message *pkcs7,
>  		if (p->seen) {
>  			pr_warn("Sig %u: X.509 chain contains loop\n",
>  				sinfo->index);
> +#ifdef __UBOOT__
> +			*signer = p;
> +#endif
>  			return 0;
>  		}
>  		ret = public_key_verify_signature(p->pub, x509->sig);
> @@ -402,6 +422,9 @@ static int pkcs7_verify_sig_chain(struct pkcs7_message *pkcs7,
>  		x509->signer = p;
>  		if (x509 == p) {
>  			pr_debug("- self-signed\n");
> +#ifdef __UBOOT__
> +			*signer = p;
> +#endif
>  			return 0;
>  		}
>  		x509 = p;
> @@ -423,11 +446,14 @@ unsupported_crypto_in_x509:
>  /*
>   * Verify one signed information block from a PKCS#7 message.

A proper function description is missing here.

>   */
> -#ifndef __UBOOT__
> -static
> -#endif
> +#ifdef __UBOOT__
>  int pkcs7_verify_one(struct pkcs7_message *pkcs7,
> -		     struct pkcs7_signed_info *sinfo)
> +		     struct pkcs7_signed_info *sinfo,
> +		     struct x509_certificate **signer)
> +#else
> +static int pkcs7_verify_one(struct pkcs7_message *pkcs7,
> +			    struct pkcs7_signed_info *sinfo)

Please, use a single function signature here too.

Best regards

Heinrich

> +#endif
>  {
>  	int ret;
>
> @@ -471,7 +497,7 @@ int pkcs7_verify_one(struct pkcs7_message *pkcs7,
>  	pr_devel("Verified signature %u\n", sinfo->index);
>
>  	/* Verify the internal certificate chain */
> -	return pkcs7_verify_sig_chain(pkcs7, sinfo);
> +	return pkcs7_verify_sig_chain(pkcs7, sinfo, signer);
>  }
>
>  #ifndef __UBOOT__
>