Re: [PATCH v2 06/19] crypto: rsassa-pkcs1 - Migrate to sig_alg backend

[Klara Modin <klarasmodin@gmail.com>]

On 2024-10-21 21:02, Lukas Wunner wrote:
> On Mon, Oct 21, 2024 at 06:08:03PM +0200, Klara Modin wrote:
>> On 2024-09-10 16:30, Lukas Wunner wrote:
>>> A sig_alg backend has just been introduced with the intent of moving all
>>> asymmetric sign/verify algorithms to it one by one.
>>>
>>> Migrate the sign/verify operations from rsa-pkcs1pad.c to a separate
>>> rsassa-pkcs1.c which uses the new backend.
> [...]
>> This commit (1e562deacecca1f1bec7d23da526904a1e87525e in next-20241021)
>> seems to break connecting to wpa2-enterprise with iwd.
> 
> Thanks for the report and sorry for the breakage.
> 
> There is one pending fix for an issue I inadvertently introduced
> with my sig_alg rework:
> 
> https://lore.kernel.org/r/ff7a28cddfc28e7a3fb8292c680510f35ec54391.1728898147.git.lukas@wunner.de/
> 
> However it fixes a different commit than the one you found through
> bisection, so I suspect it won't fix the problem, though it would
> still be good if you could test it.
> 
> There is a *second* issue I discovered last week.  I cooked up
> a fix this morning, but haven't written a commit message yet.
> The patch is included below and it could indeed solve the
> problem because it fixes an issue introduced by the commit you
> identified as culprit.  So if you could test the patch below as well
> I'd be grateful.
> 
> I'll now look at the config and dmesg output you've provided.
> Just wanted to get this e-mail out the door quickly to point you
> to potential fixes.
> 
> Thanks!
> 
> Lukas
> 
> -- >8 --
> 
> diff --git a/crypto/asymmetric_keys/public_key.c b/crypto/asymmetric_keys/public_key.c
> index c98c158..af19f9c 100644
> --- a/crypto/asymmetric_keys/public_key.c
> +++ b/crypto/asymmetric_keys/public_key.c
> @@ -165,14 +165,22 @@ static int software_key_query(const struct kernel_pkey_params *params,
>   {
>   	struct crypto_akcipher *tfm;
>   	struct public_key *pkey = params->key->payload.data[asym_crypto];
> +	const char *hash_algo = params->hash_algo;
>   	char alg_name[CRYPTO_MAX_ALG_NAME];
>   	struct crypto_sig *sig;
>   	u8 *key, *ptr;
>   	int ret, len;
>   	bool issig;
>   
> +	/*
> +	 * Specifying hash_algo has historically been optional for pkcs1,
> +	 * so use an arbitrary algorithm for backward compatibility.
> +	 */
> +	if (strcmp(params->encoding, "pkcs1") == 0 && !hash_algo)
> +		hash_algo = "sha256";
> +
>   	ret = software_key_determine_akcipher(pkey, params->encoding,
> -					      params->hash_algo, alg_name,
> +					      hash_algo, alg_name,
>   					      &issig, kernel_pkey_sign);
>   	if (ret < 0)
>   		return ret;
> 

I don't think I have hit the first issue you mention but I'll apply the 
fix and see if it changes anything. I'll probably be able to test these 
two sometime tomorrow.

Thanks,
Klara Modin