On Tue, 10 Sep 2024 16:30:12 +0200
Lukas Wunner <lukas@xxxxxxxxx> wrote:
> Commit 6cb8815f41a9 ("crypto: sig - Add interface for sign/verify")
> began a transition of asymmetric sign/verify operations from
> crypto_akcipher to a new crypto_sig frontend.
>
> Internally, the crypto_sig frontend still uses akcipher_alg as backend,
> however:
>
> "The link between sig and akcipher is meant to be temporary. The
> plan is to create a new low-level API for sig and then migrate
> the signature code over to that from akcipher."
> https://lore.kernel.org/r/ZrG6w9wsb-iiLZIF@xxxxxxxxxxxxxxxxxxx/
>
> "having a separate alg for sig is definitely where we want to
> be since there is very little that the two types actually share."
> https://lore.kernel.org/r/ZrHlpz4qnre0zWJO@xxxxxxxxxxxxxxxxxxx/
>
> Take the next step of that migration and augment the crypto_sig frontend
> with a sig_alg backend to which all algorithms can be moved.
>
> During the migration, there will briefly be signature algorithms that
> are still based on crypto_akcipher, whilst others are already based on
> crypto_sig. Allow for that by building a fork into crypto_sig_*() API
> calls (i.e. crypto_sig_maxsize() and friends) such that one of the two
> backends is selected based on the transform's cra_type.
>
> Signed-off-by: Lukas Wunner <lukas@xxxxxxxxx>
Hi Lukas,
A few trivial comments.
Jonathan
> diff --git a/crypto/sig.c b/crypto/sig.c
> index 7645bedf3a1f..4f36ceb7a90b 100644
> --- a/crypto/sig.c
> +++ b/crypto/sig.c
> @@ -68,6 +93,14 @@ EXPORT_SYMBOL_GPL(crypto_alloc_sig);
>
> int crypto_sig_maxsize(struct crypto_sig *tfm)
> {
> + if (crypto_sig_tfm(tfm)->__crt_alg->cra_type != &crypto_sig_type)
> + goto akcipher;
> +
> + struct sig_alg *alg = crypto_sig_alg(tfm);
> +
> + return alg->max_size(tfm);
> +
> +akcipher:
Neat trick for temporary retention of the code.
Hideous code in the meantime ;) Not that I have a better idea.
> struct crypto_akcipher **ctx = crypto_sig_ctx(tfm);
>
> return crypto_akcipher_maxsize(*ctx);
> diff --git a/crypto/testmgr.c b/crypto/testmgr.c
> index f02cb075bd68..bb21378aa510 100644
> --- a/crypto/testmgr.c
> +++ b/crypto/testmgr.c
...
> @@ -4317,6 +4324,114 @@ static int alg_test_akcipher(const struct alg_test_desc *desc,
> return err;
> }
>
> +static int test_sig_one(struct crypto_sig *tfm, const struct sig_testvec *vecs)
> +{
> + u8 *ptr, *key __free(kfree);
I would move definition of key down to where the constructor is.
Current pattern is fine until some extra code sneaks inbetween with
an error return.
> + int err, sig_size;
> +
> + key = kmalloc(vecs->key_len + 2 * sizeof(u32) + vecs->param_len,
> + GFP_KERNEL);
> + if (!key)
> + return -ENOMEM;
git a/include/crypto/internal/sig.h b/include/crypto/internal/sig.h
> index 97cb26ef8115..b16648c1a986 100644
> --- a/include/crypto/internal/sig.h
> +++ b/include/crypto/internal/sig.h
> +static inline struct crypto_sig *crypto_spawn_sig(struct crypto_sig_spawn
> + *spawn)
That's an odd wrap. I'd just go long where this happens and slightly past 80 chars.
> +{
> + return crypto_spawn_tfm2(&spawn->base);
> +}
