Re: [PATCH v3 3/5] crypto: pkcs7: allow pkcs7_digest() to be called from pkcs7_trust

[Eric Biggers <ebiggers@kernel.org>]

On Thu, Mar 05, 2026 at 03:11:29PM -0500, James Bottomley wrote:
> On Thu, 2026-03-05 at 10:50 -0800, Eric Biggers wrote:
> > On Thu, Mar 05, 2026 at 09:53:56AM -0500, James Bottomley wrote:
> > > On Wed, 2026-03-04 at 23:58 -0800, Eric Biggers wrote:
> > > > On Thu, Feb 26, 2026 at 10:50:10PM -0500, James Bottomley wrote:
> > > > > On Thu, 2026-02-26 at 12:31 -0800, Eric Biggers wrote:
> > > > > > On Wed, Feb 25, 2026 at 04:19:05PM -0500, James Bottomley
> > > > > > wrote:
> > > > > > > +	/*
> > > > > > > +	 * if we're being called immediately after parse,
> > > > > > > the
> > > > > > > +	 * signature won't have a calculated digest yet,
> > > > > > > so
> > > > > > > calculate
> > > > > > > +	 * one.  This function returns immediately if a
> > > > > > > digest
> > > > > > > has
> > > > > > > +	 * already been calculated
> > > > > > > +	 */
> > > > > > > +	pkcs7_digest(pkcs7, sinfo);
> > > > > > 
> > > > > > pkcs7_digest() can fail, returning an error code and leaving
> > > > > > sig-
> > > > > > > m
> > > > > > == NULL && sig->m_size == 0.  Here, the error is just being
> > > > > > ignored.
> > > > > 
> > > > > That's right.  Basically I wasn't sure what to return on error
> > > > > (although -ENOKEY looks about right since it will cause retries
> > > > > on
> > > > > a
> > > > > different sig chain).
> > > > > 
> > > > > > Doesn't that then cause the signature verification to proceed
> > > > > > against an empty message, rather than anything related to the
> > > > > > data provided?
> > > > > 
> > > > > Not if sig->m is NULL, no, because the verifier will try to
> > > > > reget the digest in that case (and error out if it fails).
> > > > 
> > > > Can you point to where that happens?  It still looks like it just
> > > > proceeds with an empty message.
> > > 
> > > It's the obvious one:
> > > 
> > > verify_pkcs7_message_sig->pkcs7_verify->pkcs7_verify_one-
> > > >pkcs7_digest
> > > 
> > > The latter will allocate and calculate the digest if sig->m is
> > > null.
> > > 
> > > Regards,
> > > 
> > > James
> > > 
> > 
> > But looking at hornet_check_program() from
> > https://lore.kernel.org/linux-security-module/20251211021257.1208712-9-bboscaccy@linux.microsoft.com/
> > ,
> > it calls:
> > 
> >     pkcs7_parse_message()
> >     validate_pkcs7_trust()
> >     pkcs7_get_authattr()
> > 
> > The actual signature check happens in validate_pkcs7_trust(), which
> > appears to have the issue where it can proceed with an empty message,
> > as I mentioned.
> 
> The whole design of validate_pkccs7_trust() is to validate the
> signature only so we can trust the attributes.  It doesn't actually
> verify the digest against the data, that's the job of the
> verify_pkcs7_sig.. class of functions.  The original thought behind
> this was that we might not have the original data by the time we came
> to extract the OID.  However, it turns out we do, so the split of the
> trust functions is not really necessary..

But surely you *do* want to verify the data as well, as that is the
contents of the BPF program?  I think the API you want is something like
verify_pkcs7_signature() extended to return your signed attribute as
extra information on success.

Of course, again it would be a lot simpler if you could find a way to
format all the information you want to sign into one message, instead of
relying on the PKCS#7 message-within-a-message thing that is hard to
understand and implement correctly.

- Eric