Re: [PATCH v2 1/2] integrity: Refactor asymmetric_verify for reusability

On 4/14/26 10:00 PM, Mimi Zohar wrote:
> On Wed, 2026-04-08 at 13:41 -0400, Stefan Berger wrote:
>> Refactor asymmetric_verify for reusability. Have it call
>> asymmetric_verify_common with the signature verification key and the
>> public_key structure as parameters. sigv3 support for ML-DSA will need to
>> check the public key type first to decide how to do the signature
>> verification and therefore will have these parameters available for
>> calling asymmetric_verify_common.
>>
>> Signed-off-by: Stefan Berger <[email protected]>
> 
> Thanks, Stefan.
> 
>> ---
>>   security/integrity/digsig_asymmetric.c | 42 +++++++++++++++++---------
>>   1 file changed, 28 insertions(+), 14 deletions(-)
>>
>> diff --git a/security/integrity/digsig_asymmetric.c b/security/integrity/digsig_asymmetric.c
>> index 6e68ec3becbd..e29ed73f15cd 100644
>> --- a/security/integrity/digsig_asymmetric.c
>> +++ b/security/integrity/digsig_asymmetric.c
>> @@ -79,18 +79,15 @@ static struct key *request_asymmetric_key(struct key *keyring, uint32_t keyid)
>>   	return key;
>>   }
>>   
>> -int asymmetric_verify(struct key *keyring, const char *sig,
>> -		      int siglen, const char *data, int datalen)
>> +static int asymmetric_verify_common(const struct key *key,
>> +				    const struct public_key *pk,
>> +				    const char *sig, int siglen,
>> +				    const char *data, int datalen)
>>   {
>> -	struct public_key_signature pks;
>>   	struct signature_v2_hdr *hdr = (struct signature_v2_hdr *)sig;
>> -	const struct public_key *pk;
>> -	struct key *key;
>> +	struct public_key_signature pks;
>>   	int ret;
>>   
>> -	if (siglen <= sizeof(*hdr))
>> -		return -EBADMSG;
>> -
>>   	siglen -= sizeof(*hdr);
> 
> Normally kernel-doc is unnecessary for static functions.  Here, however, since
> only the caller verifies the signature length, there should be a kernel-doc
> function definition.  It should indicate that all callers must verify the
> signature length (siglen) and that the public key (pk) is not NULL, before
 > calling asymmetric_verify_common().

Will add.

> 
>>   
>>   	if (siglen != be16_to_cpu(hdr->sig_size))
>> @@ -99,15 +96,10 @@ int asymmetric_verify(struct key *keyring, const char *sig,
>>   	if (hdr->hash_algo >= HASH_ALGO__LAST)
>>   		return -ENOPKG;
>>   
>> -	key = request_asymmetric_key(keyring, be32_to_cpu(hdr->keyid));
>> -	if (IS_ERR(key))
>> -		return PTR_ERR(key);
>> -
>>   	memset(&pks, 0, sizeof(pks));
>>   
>>   	pks.hash_algo = hash_algo_name[hdr->hash_algo];
>>   
>> -	pk = asymmetric_key_public_key(key);
>>   	pks.pkey_algo = pk->pkey_algo;
>>   	if (!strcmp(pk->pkey_algo, "rsa")) {
>>   		pks.encoding = "pkcs1";
>> @@ -127,11 +119,33 @@ int asymmetric_verify(struct key *keyring, const char *sig,
>>   	pks.s_size = siglen;
>>   	ret = verify_signature(key, &pks);
>>   out:
>> -	key_put(key);
> 
> The kernel-doc function definition should also indicate that the caller must
> free the key.

Ok, I will add it. However, symmetric_verify_common cannot free the key 
since it is passed as const(!) struct key *key...

> 
>>   	pr_debug("%s() = %d\n", __func__, ret);
>>   	return ret;
>>   }
>>   
>> +int asymmetric_verify(struct key *keyring, const char *sig,
>> +		      int siglen, const char *data, int datalen)
>> +{
>> +	struct signature_v2_hdr *hdr = (struct signature_v2_hdr *)sig;
>> +	const struct public_key *pk;
>> +	struct key *key;
>> +	int ret;
>> +
>> +	if (siglen <= sizeof(*hdr))
>> +		return -EBADMSG;
>> +
>> +	key = request_asymmetric_key(keyring, be32_to_cpu(hdr->keyid));
>> +	if (IS_ERR(key))
>> +		return PTR_ERR(key);
>> +	pk = asymmetric_key_public_key(key);
> 
> Please add a test here making sure pk is not null.

As a separate patch for backporting?

Return -ENOKEY in case we hit a NULL pointer?

> 
> thanks,
> 
> Mimi
> 
>> +
>> +	ret = asymmetric_verify_common(key, pk, sig, siglen, data, datalen);
>> +
>> +	key_put(key);
>> +
>> +	return ret;
>> +}
>> +
>>   /*
>>    * calc_file_id_hash - calculate the hash of the ima_file_id struct data
>>    * @type: xattr type [enum evm_ima_xattr_type]
>