Re: [PATCH v13 02/12] pkcs7: Allow the signing algo to calculate the digest itself

[Jarkko Sakkinen <jarkko@kernel.org>]

On Tue, Jan 20, 2026 at 02:50:48PM +0000, David Howells wrote:
> The ML-DSA public key algorithm really wants to calculate the message
> digest itself, rather than having the digest precalculated and fed to it
> separately as RSA does[*].  The kernel's PKCS#7 parser, however, is
> designed around the latter approach.
> 
>   [*] ML-DSA does allow for an "external mu", but CMS doesn't yet have that
>   standardised.
> 
> Fix this by noting in the public_key_signature struct when the signing
> algorithm is going to want this and then, rather than doing the digest of
> the authenticatedAttributes ourselves and overwriting the sig->digest with
> that, replace sig->digest with a copy of the contents of the
> authenticatedAttributes section and adjust the digest length to match.
> 
> This will then be fed to the public key algorithm as normal which can do
> what it wants with the data.
> 
> Signed-off-by: David Howells <dhowells@redhat.com>
> cc: Lukas Wunner <lukas@wunner.de>
> cc: Ignat Korchagin <ignat@cloudflare.com>
> cc: Stephan Mueller <smueller@chronox.de>
> cc: Eric Biggers <ebiggers@kernel.org>
> cc: Herbert Xu <herbert@gondor.apana.org.au>
> cc: keyrings@vger.kernel.org
> cc: linux-crypto@vger.kernel.org
> ---
>  crypto/asymmetric_keys/pkcs7_parser.c |  4 +--
>  crypto/asymmetric_keys/pkcs7_verify.c | 48 ++++++++++++++++++---------
>  include/crypto/public_key.h           |  1 +
>  3 files changed, 36 insertions(+), 17 deletions(-)
> 
> diff --git a/crypto/asymmetric_keys/pkcs7_parser.c b/crypto/asymmetric_keys/pkcs7_parser.c
> index 423d13c47545..3cdbab3b9f50 100644
> --- a/crypto/asymmetric_keys/pkcs7_parser.c
> +++ b/crypto/asymmetric_keys/pkcs7_parser.c
> @@ -599,8 +599,8 @@ int pkcs7_sig_note_set_of_authattrs(void *context, size_t hdrlen,
>  	}
>  
>  	/* We need to switch the 'CONT 0' to a 'SET OF' when we digest */
> -	sinfo->authattrs = value - (hdrlen - 1);
> -	sinfo->authattrs_len = vlen + (hdrlen - 1);
> +	sinfo->authattrs = value - hdrlen;
> +	sinfo->authattrs_len = vlen + hdrlen;
>  	return 0;
>  }
>  
> diff --git a/crypto/asymmetric_keys/pkcs7_verify.c b/crypto/asymmetric_keys/pkcs7_verify.c
> index 6d6475e3a9bf..0f9f515b784d 100644
> --- a/crypto/asymmetric_keys/pkcs7_verify.c
> +++ b/crypto/asymmetric_keys/pkcs7_verify.c
> @@ -70,8 +70,6 @@ static int pkcs7_digest(struct pkcs7_message *pkcs7,
>  	 * digest we just calculated.
>  	 */
>  	if (sinfo->authattrs) {
> -		u8 tag;
> -
>  		if (!sinfo->msgdigest) {
>  			pr_warn("Sig %u: No messageDigest\n", sinfo->index);
>  			ret = -EKEYREJECTED;
> @@ -97,20 +95,40 @@ static int pkcs7_digest(struct pkcs7_message *pkcs7,
>  		 * as the contents of the digest instead.  Note that we need to
>  		 * convert the attributes from a CONT.0 into a SET before we
>  		 * hash it.
> +		 *
> +		 * However, for certain algorithms, such as ML-DSA, the digest
> +		 * is integrated into the signing algorithm.  In such a case,
> +		 * we copy the authattrs, modifying the tag type, and set that
> +		 * as the digest.
>  		 */
> -		memset(sig->digest, 0, sig->digest_size);
> -
> -		ret = crypto_shash_init(desc);
> -		if (ret < 0)
> -			goto error;
> -		tag = ASN1_CONS_BIT | ASN1_SET;
> -		ret = crypto_shash_update(desc, &tag, 1);
> -		if (ret < 0)
> -			goto error;
> -		ret = crypto_shash_finup(desc, sinfo->authattrs,
> -					 sinfo->authattrs_len, sig->digest);
> -		if (ret < 0)
> -			goto error;
> +		if (sig->algo_does_hash) {
> +			kfree(sig->digest);
> +
> +			ret = -ENOMEM;
> +			sig->digest = kmalloc(umax(sinfo->authattrs_len, sig->digest_size),
> +					      GFP_KERNEL);
> +			if (!sig->digest)
> +				goto error_no_desc;
> +
> +			sig->digest_size = sinfo->authattrs_len;
> +			memcpy(sig->digest, sinfo->authattrs, sinfo->authattrs_len);
> +			((u8 *)sig->digest)[0] = ASN1_CONS_BIT | ASN1_SET;
> +			ret = 0;
> +		} else {
> +			u8 tag = ASN1_CONS_BIT | ASN1_SET;
> +
> +			ret = crypto_shash_init(desc);
> +			if (ret < 0)
> +				goto error;
> +			ret = crypto_shash_update(desc, &tag, 1);
> +			if (ret < 0)
> +				goto error;
> +			ret = crypto_shash_finup(desc, sinfo->authattrs + 1,
> +						 sinfo->authattrs_len - 1,
> +						 sig->digest);
> +			if (ret < 0)
> +				goto error;
> +		}
>  		pr_devel("AADigest = [%*ph]\n", 8, sig->digest);
>  	}
>  
> diff --git a/include/crypto/public_key.h b/include/crypto/public_key.h
> index 81098e00c08f..e4ec8003a3a4 100644
> --- a/include/crypto/public_key.h
> +++ b/include/crypto/public_key.h
> @@ -46,6 +46,7 @@ struct public_key_signature {
>  	u8 *digest;
>  	u32 s_size;		/* Number of bytes in signature */
>  	u32 digest_size;	/* Number of bytes in digest */
> +	bool algo_does_hash;	/* Public key algo does its own hashing */

I'd use the wording you used already in commit message, which
factors more descriptive than what you have here. E.g., name
it "external_digest".

It would be easier to digest this when revisiting the code later...

>  	const char *pkey_algo;
>  	const char *hash_algo;
>  	const char *encoding;
> 

Allocation scheme is not the prettiest but I neither have
anything other to offer, so other than the rename request,
I think this is acceptable.

BR, Jarkko