Re: [PATCH v16 4/7] pkcs7: Allow the signing algo to do whatever digestion it wants itself

[Jarkko Sakkinen <jarkko@kernel.org>]

On Mon, Feb 02, 2026 at 05:02:09PM +0000, David Howells wrote:
> Allow the data to be verified in a PKCS#7 or CMS message to be passed
> directly to an asymmetric cipher algorithm (e.g. ML-DSA) if it wants to do
> whatever passes for hashing/digestion itself.  The normal digestion of the
> data is then skipped as that would be ignored unless another signed info in
> the message has some other algorithm that needs it.
> 
> The 'data to be verified' may be the content of the PKCS#7 message or it
> will be the authenticatedAttributes (signedAttrs if CMS), modified, if
> those are present.
> 
> This is done by:
> 
>  (1) Make ->m and ->m_size point to the data to be verified rather than
>      making public_key_verify_signature() access the data directly.  This
>      is so that keyctl(KEYCTL_PKEY_VERIFY) will still work.
> 
>  (2) Add a flag, ->algo_takes_data, to indicate that the verification
>      algorithm wants to access the data to be verified directly rather than
>      having it digested first.
> 
>  (3) If the PKCS#7 message has authenticatedAttributes (or CMS
>      signedAttrs), then the digest contained therein will be validated as
>      now, and the modified attrs blob will either be digested or assigned
>      to ->m as appropriate.
> 
>  (4) If present, always copy and modify the authenticatedAttributes (or
>      signedAttrs) then digest that in one go rather than calling the shash
>      update twice (once for the tag and once for the rest).
> 
>  (5) For ML-DSA, point ->m to the TBSCertificate instead of digesting it
>      and using the digest.
> 
> Note that whilst ML-DSA does allow for an "external mu", CMS doesn't yet
> have that standardised.
> 
> 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    | 52 ++++++++++++++++--------
>  crypto/asymmetric_keys/signature.c       |  3 +-
>  crypto/asymmetric_keys/x509_public_key.c | 10 +++++
>  include/crypto/public_key.h              |  2 +
>  5 files changed, 51 insertions(+), 20 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 aa085ec6fb1c..06abb9838f95 100644
> --- a/crypto/asymmetric_keys/pkcs7_verify.c
> +++ b/crypto/asymmetric_keys/pkcs7_verify.c
> @@ -30,6 +30,16 @@ static int pkcs7_digest(struct pkcs7_message *pkcs7,
>  
>  	kenter(",%u,%s", sinfo->index, sinfo->sig->hash_algo);
>  
> +	if (!sinfo->authattrs && sig->algo_takes_data) {
> +		/* There's no intermediate digest and the signature algo
> +		 * doesn't want the data prehashing.
> +		 */
> +		sig->m = (void *)pkcs7->data;
> +		sig->m_size = pkcs7->data_len;
> +		sig->m_free = false;
> +		return 0;
> +	}
> +
>  	/* The digest was calculated already. */
>  	if (sig->m)
>  		return 0;
> @@ -48,9 +58,10 @@ static int pkcs7_digest(struct pkcs7_message *pkcs7,
>  	sig->m_size = crypto_shash_digestsize(tfm);
>  
>  	ret = -ENOMEM;
> -	sig->m = kmalloc(sig->m_size, GFP_KERNEL);
> +	sig->m = kmalloc(umax(sinfo->authattrs_len, sig->m_size), GFP_KERNEL);
>  	if (!sig->m)
>  		goto error_no_desc;
> +	sig->m_free = true;
>  
>  	desc = kzalloc(desc_size, GFP_KERNEL);
>  	if (!desc)
> @@ -69,8 +80,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;
> @@ -96,21 +105,25 @@ 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->m, 0, sig->m_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->m);
> -		if (ret < 0)
> -			goto error;
> +		memcpy(sig->m, sinfo->authattrs, sinfo->authattrs_len);
> +		sig->m[0] = ASN1_CONS_BIT | ASN1_SET;
> +
> +		if (sig->algo_takes_data) {
> +			sig->m_size = sinfo->authattrs_len;
> +			ret = 0;
> +		} else {
> +			ret = crypto_shash_digest(desc, sig->m,
> +						  sinfo->authattrs_len,
> +						  sig->m);
> +			if (ret < 0)
> +				goto error;
> +		}
>  		pr_devel("AADigest = [%*ph]\n", 8, sig->m);
>  	}
>  
> @@ -137,6 +150,11 @@ int pkcs7_get_digest(struct pkcs7_message *pkcs7, const u8 **buf, u32 *len,
>  	ret = pkcs7_digest(pkcs7, sinfo);
>  	if (ret)
>  		return ret;
> +	if (!sinfo->sig->m_free) {
> +		pr_notice_once("%s: No digest available\n", __func__);
> +		return -EINVAL; /* TODO: MLDSA doesn't necessarily calculate an
> +				 * intermediate digest. */

Is this logic going to change in the foreseeable future?

> +	}
>  
>  	*buf = sinfo->sig->m;
>  	*len = sinfo->sig->m_size;
> diff --git a/crypto/asymmetric_keys/signature.c b/crypto/asymmetric_keys/signature.c
> index f4ec126121b3..a5ac7a53b670 100644
> --- a/crypto/asymmetric_keys/signature.c
> +++ b/crypto/asymmetric_keys/signature.c
> @@ -28,7 +28,8 @@ void public_key_signature_free(struct public_key_signature *sig)
>  		for (i = 0; i < ARRAY_SIZE(sig->auth_ids); i++)
>  			kfree(sig->auth_ids[i]);
>  		kfree(sig->s);
> -		kfree(sig->m);
> +		if (sig->m_free)
> +			kfree(sig->m);
>  		kfree(sig);
>  	}
>  }
> diff --git a/crypto/asymmetric_keys/x509_public_key.c b/crypto/asymmetric_keys/x509_public_key.c
> index 3854f7ae4ed0..27b4fea37845 100644
> --- a/crypto/asymmetric_keys/x509_public_key.c
> +++ b/crypto/asymmetric_keys/x509_public_key.c
> @@ -50,6 +50,14 @@ int x509_get_sig_params(struct x509_certificate *cert)
>  
>  	sig->s_size = cert->raw_sig_size;
>  
> +	if (sig->algo_takes_data) {
> +		/* The signature algorithm does whatever passes for hashing. */
> +		sig->m = (u8 *)cert->tbs;
> +		sig->m_size = cert->tbs_size;
> +		sig->m_free = false;
> +		goto out;
> +	}
> +
>  	/* Allocate the hashing algorithm we're going to need and find out how
>  	 * big the hash operational data will be.
>  	 */
> @@ -69,6 +77,7 @@ int x509_get_sig_params(struct x509_certificate *cert)
>  	sig->m = kmalloc(sig->m_size, GFP_KERNEL);
>  	if (!sig->m)
>  		goto error;
> +	sig->m_free = true;
>  
>  	desc = kzalloc(desc_size, GFP_KERNEL);
>  	if (!desc)
> @@ -84,6 +93,7 @@ int x509_get_sig_params(struct x509_certificate *cert)
>  	kfree(desc);
>  error:
>  	crypto_free_shash(tfm);
> +out:
>  	pr_devel("<==%s() = %d\n", __func__, ret);
>  	return ret;
>  }
> diff --git a/include/crypto/public_key.h b/include/crypto/public_key.h
> index bd38ba4d217d..4c5199b20338 100644
> --- a/include/crypto/public_key.h
> +++ b/include/crypto/public_key.h
> @@ -46,6 +46,8 @@ struct public_key_signature {
>  	u8 *m;			/* Message data to pass to verifier */
>  	u32 s_size;		/* Number of bytes in signature */
>  	u32 m_size;		/* Number of bytes in ->m */
> +	bool m_free;		/* T if ->m needs freeing */
> +	bool algo_takes_data;	/* T if public key algo operates on data, not a hash */
>  	const char *pkey_algo;
>  	const char *hash_algo;
>  	const char *encoding;
> 

BR, Jarkko