Re: [PATCH net-next 3/5] net/rxrpc: Reimplement DES-PCBC using DES library

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Weak DES keys will now be rejected when loading a server key, whereas previously they were accepted.
--

commit c24ad4e831d157713b86ce93d80475105c2362d5
Author: Eric Biggers <ebiggers@kernel.org>

net/rxrpc: Reimplement DES-PCBC using DES library

This commit reimplements the obsolete PCBC mode on top of the DES library
specifically for rxrpc, replacing its use of the crypto API "pcbc(des)"
template. This allows removing PCBC support from the generic crypto API.

> diff --git a/net/rxrpc/rxkad.c b/net/rxrpc/rxkad.c
> index 3c9e7f636b42..4e04625f40eb 100644
> --- a/net/rxrpc/rxkad.c
> +++ b/net/rxrpc/rxkad.c
[ ... ]
> @@ -52,40 +54,41 @@ static void rxkad_prime_packet_security(struct rxrpc_connection *conn,
>   */
>  static int rxkad_preparse_server_key(struct key_preparsed_payload *prep)
>  {
[ ... ]
> -	if (crypto_skcipher_setkey(ci, prep->data, 8) < 0)
> -		BUG();
> +	err = des_expand_key(des_key, prep->data, 8);
> +	if (err) {
> +		kfree_sensitive(des_key);
> +		_leave(" = %d", err);
> +		return err;
> +	}

Does this introduce an unintended behavior change regarding weak DES keys?

The old code used crypto_skcipher_setkey(), which in the generic DES
implementation suppresses the -ENOKEY error and returns 0 (success) unless
CRYPTO_TFM_REQ_FORBID_WEAK_KEYS is explicitly set. The RxRPC code did not
set this flag, meaning weak DES keys were silently accepted.

By calling des_expand_key() directly, which unconditionally returns -ENOKEY
for weak keys, rxkad_preparse_server_key() will now fail and reject these
keys instead of succeeding. 

While weak keys shouldn't be used, could this undocumented change break
backward compatibility for existing userspace configurations or test
environments relying on fixed weak keys?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260428024400.123337-1-ebiggers@kernel.org?part=3