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

[AKASHI Takahiro <takahiro.akashi@linaro.org>]

Heinrich,

On Tue, Jul 07, 2020 at 12:32:02PM +0200, Heinrich Schuchardt wrote:
> On 16.06.20 07:26, 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."
> 
> Is this also copied from Linux's crypto/asymmetric_keys/pkcs7_verify.c?
> If so, please, mention it in the commit message.

Please read my comments carefully.

I clearly mentioned that this function was imported from linux
in the previous commit, patch 4/8.
Moreover, in this commit message, I explicitly mentioned

> > To do that, its function interface is expanded, adding an extra argument
> > which is expected to return the last certificate in trusted chain.

What more do you expect?

-Takahiro Akashi

> If everything copied from crypto/asymmetric_keys/pkcs7_verify.c were in
> the same commit, the comparison would be easier.
> 
> Best regards
> 
> Heinrich
> 
> 
> >
> > 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@linaro.org>
> > ---
> >  include/crypto/pkcs7.h    |  9 +++++-
> >  lib/crypto/pkcs7_verify.c | 61 ++++++++++++++++++++++++++++++++++-----
> >  2 files changed, 62 insertions(+), 8 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 9b9030ea4440..dda96ccf57a2 100644
> > --- a/lib/crypto/pkcs7_verify.c
> > +++ b/lib/crypto/pkcs7_verify.c
> > @@ -302,10 +302,27 @@ static int pkcs7_find_key(struct pkcs7_message *pkcs7,
> >  }
> >
> >  /*
> > - * Verify the internal certificate chain as best we can.
> > + * pkcs7_verify_sig_chain - Verify the internal certificate chain as best
> > + *                          as we can.
> > + * @pkcs7:	PKCS7 Signed Data
> > + * @sinfo:	PKCS7 Signed Info
> > + * @signer:	Singer's certificate
> > + *
> > + * Build up and verify the internal certificate chain against a signature
> > + * in @sinfo, using certificates contained in @pkcs7 as best as we can.
> > + * If the chain reaches the end, the last certificate will be returned
> > + * in @signer.
> > + *
> > + * Return:	0 - on success, non-zero error code - otherwise
> >   */
> > +#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)
> > +#endif
> >  {
> >  	struct public_key_signature *sig;
> >  	struct x509_certificate *x509 = sinfo->signer, *p;
> > @@ -314,6 +331,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;
> >
> > @@ -331,6 +350,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;
> >  		}
> >
> > @@ -356,6 +378,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;
> >  		}
> >
> > @@ -386,6 +411,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:
> > @@ -403,6 +431,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);
> > @@ -411,6 +442,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;
> > @@ -430,13 +464,26 @@ unsupported_crypto_in_x509:
> >  }
> >
> >  /*
> > - * Verify one signed information block from a PKCS#7 message.
> > + * pkcs7_verify_one - Verify one signed information block from a PKCS#7
> > + *                    message.
> > + * @pkcs7:	PKCS7 Signed Data
> > + * @sinfo:	PKCS7 Signed Info
> > + * @signer:	Signer's certificate
> > + *
> > + * Verify one signature in @sinfo and follow the certificate chain.
> > + * If the chain reaches the end, the last certificate will be returned
> > + * in @signer.
> > + *
> > + * Return:	0 - on success, non-zero error code - otherwise
> >   */
> > -#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)
> > +#endif
> >  {
> >  	int ret;
> >
> > @@ -480,7 +527,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__
> >
>