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

[Klara Modin <klarasmodin@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 2929 bytes --]

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;
> 

Tested on top of yesterday's next-20241022.

With the first patch only there is no change, same behavior as previously.

With the second patch only I get an oops, similar as the one you 
mentioned in the first fix

With both patches everything seems to work as expected. Thanks!

(for both fixes)
Tested-by: Klara Modin <klarasmodin@gmail.com>

[-- Attachment #2: iwd_oops_decoded.gz --]
[-- Type: application/gzip, Size: 25234 bytes --]