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 */
const char *pkey_algo;
const char *hash_algo;
const char *encoding;Hi David,
On Mon, Jan 5, 2026 at 3:22 PM David Howells <dhowells@redhat.com> 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);
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
Ignat Korchagin <ignat@cloudflare.com> wrote: > > + 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. The problem is that we don't know the right size until we've tried parsing it. > Alternatively, should we just unconditionally use this approach > "overallocating" some times? In some ways, what I'd rather do is push the hash calculation down into the crypto/ layer for all public key algos. Also, we probably don't actually need to copy the authattrs, just retain a pointer into the source buffer and the length since we don't intend to keep the digest around beyond the verification procedure. So I might be able to get away with just a flag saying I don't need to free it. However, there's an intermediate hash if there are authattrs, so I will need to store that somewhere - though that could be allocated on demand. David
David Howells <dhowells@redhat.com> wrote: > Also, we probably don't actually need to copy the authattrs, just retain a > pointer into the source buffer and the length since we don't intend to keep > the digest around beyond the verification procedure. So I might be able to > get away with just a flag saying I don't need to free it. Actually, we probably do need to copy it. The problem is that we have to modify the tag on the authenticatedAttributes (PKCS#7)/signedAttrs (CMS) blob before we digest it, e.g. in pkcs7_digest(): memcpy(sig->digest, sinfo->authattrs, sinfo->authattrs_len); ((u8 *)sig->digest)[0] = ASN1_CONS_BIT | ASN1_SET; as specified in RFC9882 and other places: 3.2. Signature Generation and Verification ... When signed attributes are included, ML-DSA (pure mode) signatures are computed over the complete DER encoding of the SignedAttrs value contained in the SignerInfo's signedAttrs field. As described in Section 5.4 of [RFC5652], this encoding includes the tag and length octets, but an EXPLICIT SET OF tag is used rather than the IMPLICIT [0] tag that appears in the final message. ... We might be able to get away with modifying it in place - but I don't know that that's true for all users. David
On Wed, Jan 7, 2026 at 1:53 PM David Howells <dhowells@redhat.com> wrote: > > Ignat Korchagin <ignat@cloudflare.com> wrote: > > > > + 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. > > The problem is that we don't know the right size until we've tried parsing it. > > > Alternatively, should we just unconditionally use this approach > > "overallocating" some times? > > In some ways, what I'd rather do is push the hash calculation down into the > crypto/ layer for all public key algos. Probably better indeed > Also, we probably don't actually need to copy the authattrs, just retain a > pointer into the source buffer and the length since we don't intend to keep > the digest around beyond the verification procedure. So I might be able to > get away with just a flag saying I don't need to free it. > > However, there's an intermediate hash if there are authattrs, so I will need > to store that somewhere - though that could be allocated on demand. > > David > Ignat
© 2016 - 2026 Red Hat, Inc.