Re: [PATCH v2 05/19] crypto: rsa-pkcs1pad - Deduplicate set_{pub,priv}_key callbacks

["Jarkko Sakkinen" <jarkko@kernel.org>]

On Tue Sep 10, 2024 at 5:30 PM EEST, Lukas Wunner wrote:
> pkcs1pad_set_pub_key() and pkcs1pad_set_priv_key() are almost identical.
>
> The upcoming migration of sign/verify operations from rsa-pkcs1pad.c
> into a separate crypto_template will require another copy of the exact
> same functions.  When RSASSA-PSS and RSAES-OAEP are introduced, each
> will need yet another copy.
>
> Deduplicate the functions into a single one which lives in a common
> header file for reuse by RSASSA-PKCS1-v1_5, RSASSA-PSS and RSAES-OAEP.

Nit: I'd simply swap the order of the two last paragraphs. I.e. I get
the question and then I have energy to read the answer ;-) For longer
feature patch starting with motivation makes more sense but here I
think opposite order would serve better...

>
> Signed-off-by: Lukas Wunner <lukas@wunner.de>
> ---
>  crypto/rsa-pkcs1pad.c         | 30 ++----------------------------
>  include/crypto/internal/rsa.h | 28 ++++++++++++++++++++++++++++
>  2 files changed, 30 insertions(+), 28 deletions(-)
>
> diff --git a/crypto/rsa-pkcs1pad.c b/crypto/rsa-pkcs1pad.c
> index cd501195f34a..3c5fe8c93938 100644
> --- a/crypto/rsa-pkcs1pad.c
> +++ b/crypto/rsa-pkcs1pad.c
> @@ -131,42 +131,16 @@ static int pkcs1pad_set_pub_key(struct crypto_akcipher *tfm, const void *key,
>  		unsigned int keylen)
>  {
>  	struct pkcs1pad_ctx *ctx = akcipher_tfm_ctx(tfm);
> -	int err;
> -
> -	ctx->key_size = 0;
>  
> -	err = crypto_akcipher_set_pub_key(ctx->child, key, keylen);
> -	if (err)
> -		return err;
> -
> -	/* Find out new modulus size from rsa implementation */
> -	err = crypto_akcipher_maxsize(ctx->child);
> -	if (err > PAGE_SIZE)
> -		return -ENOTSUPP;
> -
> -	ctx->key_size = err;
> -	return 0;
> +	return rsa_set_key(ctx->child, &ctx->key_size, RSA_PUB, key, keylen);
>  }
>  
>  static int pkcs1pad_set_priv_key(struct crypto_akcipher *tfm, const void *key,
>  		unsigned int keylen)
>  {
>  	struct pkcs1pad_ctx *ctx = akcipher_tfm_ctx(tfm);
> -	int err;
> -
> -	ctx->key_size = 0;
>  
> -	err = crypto_akcipher_set_priv_key(ctx->child, key, keylen);
> -	if (err)
> -		return err;
> -
> -	/* Find out new modulus size from rsa implementation */
> -	err = crypto_akcipher_maxsize(ctx->child);
> -	if (err > PAGE_SIZE)
> -		return -ENOTSUPP;
> -
> -	ctx->key_size = err;
> -	return 0;
> +	return rsa_set_key(ctx->child, &ctx->key_size, RSA_PRIV, key, keylen);
>  }
>  
>  static unsigned int pkcs1pad_get_max_size(struct crypto_akcipher *tfm)
> diff --git a/include/crypto/internal/rsa.h b/include/crypto/internal/rsa.h
> index e870133f4b77..754f687134df 100644
> --- a/include/crypto/internal/rsa.h
> +++ b/include/crypto/internal/rsa.h
> @@ -8,6 +8,7 @@
>  #ifndef _RSA_HELPER_
>  #define _RSA_HELPER_
>  #include <linux/types.h>
> +#include <crypto/akcipher.h>
>  
>  /**
>   * rsa_key - RSA key structure
> @@ -53,5 +54,32 @@ int rsa_parse_pub_key(struct rsa_key *rsa_key, const void *key,
>  int rsa_parse_priv_key(struct rsa_key *rsa_key, const void *key,
>  		       unsigned int key_len);
>  
> +#define RSA_PUB (true)
> +#define RSA_PRIV (false)
> +

/**
 * rsa_set_key() - <summary>
 * <params>
 *
 * <description>
 */
> +static inline int rsa_set_key(struct crypto_akcipher *child,
> +			      unsigned int *key_size, bool is_pubkey,
> +			      const void *key, unsigned int keylen)
> +{
> +	int err;
> +
> +	*key_size = 0;
> +
> +	if (is_pubkey)
> +		err = crypto_akcipher_set_pub_key(child, key, keylen);
> +	else
> +		err = crypto_akcipher_set_priv_key(child, key, keylen);
> +	if (err)
> +		return err;
> +
> +	/* Find out new modulus size from rsa implementation */
> +	err = crypto_akcipher_maxsize(child);
> +	if (err > PAGE_SIZE)
> +		return -ENOTSUPP;
> +
> +	*key_size = err;
> +	return 0;
> +}
> +
>  extern struct crypto_template rsa_pkcs1pad_tmpl;
>  #endif

BR, Jarkko