Re: [PATCH v2 2/2] integrity: Add support for sigv3 verification using ML-DSA keys

Stefan Berger
2026-04-15T20:32:53Z

On 4/14/26 10:01 PM, Mimi Zohar wrote:
> On Wed, 2026-04-08 at 13:41 -0400, Stefan Berger wrote:
>> Add support for sigv3 signature verification using ML-DSA in pure mode.
>> When a sigv3 signature is verified, first check whether the key to use
>> for verification is an ML-DSA key and therefore uses a hashless signature
>> verification scheme. The hashless signature verification method uses the
>> ima_file_id structure directly for signature verification rather than
>> its digest.
>>
>> Suggested-by: Eric Biggers <ebiggers@kernel.org>
>> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
>>
>
> Thanks, Stefan.
>> ---
>> v2: Set hash_algo in public_key_signature to "none"
>> ---
>> security/integrity/digsig_asymmetric.c | 84 ++++++++++++++++++++++++--
>> 1 file changed, 79 insertions(+), 5 deletions(-)
>>
>> diff --git a/security/integrity/digsig_asymmetric.c b/security/integrity/digsig_asymmetric.c
>> index e29ed73f15cd..c80cb2b117a6 100644
>> --- a/security/integrity/digsig_asymmetric.c
>> +++ b/security/integrity/digsig_asymmetric.c
>> @@ -190,17 +190,91 @@ static int calc_file_id_hash(enum evm_ima_xattr_type type,
>> return rc;
>> }
>>
>> +/*
>
> kernel-doc starts with "/**".

I followed the pattern of documentation of a static function that you
just moved:

/*
* calc_file_id_hash - calculate the hash of the ima_file_id struct data
* @type: xattr type [enum evm_ima_xattr_type]

>
>> + * asymmetric_verify_v3_hashless - Use hashless signature verification on sigv3
>> + * @key: The key to use for signature verification
>> + * @pk: The associated public key
>> + * @encoding: The encoding the key type uses
>> + * @sig: The signature
>> + * @siglen: The length of the xattr signature
>> + * @algo: The hash algorithm
>> + * @digest: The file digest
>> + *
>> + * Create an ima_file_id structure and use it for signature verification
>> + * directly. This can be used for ML-DSA in pure mode for example.
>
> Like the comments on 1/2, please add a comment here indicating that all callers
> must verify the signature length (siglen) and the public key (pk) is not NULL,
> before calling asymmetric_verify_v3_hashless(). Also indicate that the caller
> must free the key.
>
>> + */
>> +static int asymmetric_verify_v3_hashless(struct key *key,
>> + const struct public_key *pk,
>> + const char *encoding,
>> + const char *sig, int siglen,
>> + u8 algo,
>> + const u8 *digest)
>> +{
>> + struct signature_v2_hdr *hdr = (struct signature_v2_hdr *)sig;
>> + struct ima_file_id file_id = {
>> + .hash_type = hdr->type,
>> + .hash_algorithm = algo,
>> + };
>> + size_t digest_size = hash_digest_size[algo];
>
> Defer initializing the digest_size and .m_size, below, until after checking the
> hash algorithm is valid.

This function is called by asymmetric_verify. asymmetric_verify calls
calc_file_id_hash, which doesn't check algo for valid range, either. I
suppose it's an untrusted value at this point (IMA never checked it's
value for valid range?) an we should check it in asymmetric_verify then
to cover both cases? Or you want to check it individually?

>
>> + struct public_key_signature pks = {
>> + .m = (u8 *)&file_id,
>> + .m_size = sizeof(file_id) - (HASH_MAX_DIGESTSIZE - digest_size),
>> + .s = hdr->sig,
>> + .s_size = siglen - sizeof(*hdr),
>> + .pkey_algo = pk->pkey_algo,
>> + .hash_algo = "none",
>> + .encoding = encoding,
>> + };
>> + int ret;
>> +
>> + if (hdr->type != IMA_VERITY_DIGSIG &&
>> + hdr->type != EVM_IMA_XATTR_DIGSIG &&
>> + hdr->type != EVM_XATTR_PORTABLE_DIGSIG)
>> + return -EINVAL;
>> +
>> + if (pks.s_size != be16_to_cpu(hdr->sig_size))
>> + return -EBADMSG;
>> +
>> + memcpy(file_id.hash, digest, digest_size);
>
> First check the hash algorithm is valid, before using digest_size.
>
>> +
>> + ret = verify_signature(key, &pks);
>> + pr_debug("%s() = %d\n", __func__, ret);
>> + return ret;
>> +}
>> +
>> int asymmetric_verify_v3(struct key *keyring, const char *sig, int siglen,
>> const char *data, int datalen, u8 algo)
>> {
>> struct signature_v2_hdr *hdr = (struct signature_v2_hdr *)sig;
>> struct ima_max_digest_data hash;
>> + const struct public_key *pk;
>> + struct key *key;
>> int rc;
>>
>> - rc = calc_file_id_hash(hdr->type, algo, data, &hash);
>> - if (rc)
>> - return -EINVAL;
>> + if (siglen <= sizeof(*hdr))
>> + return -EBADMSG;
>> +
>> + key = request_asymmetric_key(keyring, be32_to_cpu(hdr->keyid));
>> + if (IS_ERR(key))
>> + return PTR_ERR(key);
>>
>> - return asymmetric_verify(keyring, sig, siglen, hash.digest,
>> - hash.hdr.length);
>> + pk = asymmetric_key_public_key(key);
>
> Please add a test to check that 'pk' isn't null.
>
>> + if (!strncmp(pk->pkey_algo, "mldsa", 5)) {
>> + rc = asymmetric_verify_v3_hashless(key, pk, "raw",
>> + sig, siglen, algo, data);
>> + } else {
>> + rc = calc_file_id_hash(hdr->type, algo, data, &hash);
>> + if (rc) {
>> + rc = -EINVAL;
>> + goto err_exit;
>> + }
>> +
>> + rc = asymmetric_verify_common(key, pk, sig, siglen, hash.digest,
>> + hash.hdr.length);
>> + }
>> +
>> +err_exit:
>
> Normally a label named 'err*' would be preceded by a return. Here, the label
> "err_exit" is always called, not only when there is an error. Please rename the
> label to something more appropriate - out, cleanup, etc.

Ok, will call it 'out'.

>
>> + key_put(key);
>> +
>> + return rc;
>> }
>
> thanks,
>
> Mimi
>