Hi David,
On Mon, Jan 5, 2026 at 3:22 PM David Howells <[email protected]> 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 <[email protected]>
> cc: Lukas Wunner <[email protected]>
> cc: Ignat Korchagin <[email protected]>
> cc: Stephan Mueller <[email protected]>
> cc: Eric Biggers <[email protected]>
> cc: Herbert Xu <[email protected]>
> cc: [email protected]
> cc: [email protected]
> ---
> 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);
Can we refactor this so we allocate the right size from the start.
Alternatively, should we just unconditionally use this approach
"overallocating" some times?
> + 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 */
It seems this controls if we hash authenticated attributes, not the
data itself. Maybe reflect this in the name? Something like
do_authattrs_hash or authattrs_algo_passthrough?
> const char *pkey_algo;
> const char *hash_algo;
> const char *encoding;
>
Ignat
