Re: [PATCH 5/5] crypto: ecdsa - Support P1363 signature decoding

[Jonathan Cameron <Jonathan.Cameron@Huawei.com>]

On Mon, 29 Jul 2024 15:51:00 +0200
Lukas Wunner <lukas@wunner.de> wrote:

> Alternatively to the X9.62 encoding of ecdsa signatures, which uses
> ASN.1 and is already supported by the kernel, there's another common
> encoding called P1363.  It stores r and s as the concatenation of two
> big endian, unsigned integers.  The name originates from IEEE P1363.
> 
> Add a P1363 template in support of the forthcoming SPDM library
> (Security Protocol and Data Model) for PCI device authentication.
> 
> P1363 is prescribed by SPDM 1.2.1 margin no 44:
> 
>    "For ECDSA signatures, excluding SM2, in SPDM, the signature shall be
>     the concatenation of r and s.  The size of r shall be the size of
>     the selected curve.  Likewise, the size of s shall be the size of
>     the selected curve.  See BaseAsymAlgo in NEGOTIATE_ALGORITHMS for
>     the size of r and s.  The byte order for r and s shall be in big
>     endian order.  When placing ECDSA signatures into an SPDM signature
>     field, r shall come first followed by s."
> 
> Link: https://www.dmtf.org/sites/default/files/standards/documents/DSP0274_1.2.1.pdf
> Signed-off-by: Lukas Wunner <lukas@wunner.de>
One trivial follow on from previous patch. Up to you though as style
comment only.  FWIW as this all gives me a headache ;)

Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>


> diff --git a/crypto/ecdsa-p1363.c b/crypto/ecdsa-p1363.c
> new file mode 100644
> index 000000000000..c0610d88aa9e
> --- /dev/null
> +++ b/crypto/ecdsa-p1363.c
> @@ -0,0 +1,155 @@

> +static int ecdsa_p1363_create(struct crypto_template *tmpl, struct rtattr **tb)
> +{
> +	struct crypto_akcipher_spawn *spawn;
> +	struct akcipher_instance *inst;
> +	struct akcipher_alg *ecdsa_alg;
> +	u32 mask;
> +	int err;
> +
> +	err = crypto_check_attr_type(tb, CRYPTO_ALG_TYPE_AKCIPHER, &mask);
> +	if (err)
> +		return err;
> +
> +	inst = kzalloc(sizeof(*inst) + sizeof(*spawn), GFP_KERNEL);
> +	if (!inst)
> +		return -ENOMEM;
> +
> +	spawn = akcipher_instance_ctx(inst);
> +
> +	err = crypto_grab_akcipher(spawn, akcipher_crypto_instance(inst),
> +				   crypto_attr_alg_name(tb[1]), 0, mask);
> +	if (err)
> +		goto err_free_inst;
> +
> +	ecdsa_alg = crypto_spawn_akcipher_alg(spawn);
> +
> +	err = -EINVAL;
> +	if (strncmp(ecdsa_alg->base.cra_name, "ecdsa", 5) != 0)
> +		goto err_free_inst;
> +
> +	err = crypto_inst_setname(akcipher_crypto_instance(inst), tmpl->name,
> +				  &ecdsa_alg->base);
> +	if (err)
> +		goto err_free_inst;
> +
> +	inst->alg.base.cra_priority = ecdsa_alg->base.cra_priority;
> +	inst->alg.base.cra_ctxsize = sizeof(struct ecdsa_p1363_ctx);
> +
> +	inst->alg.init = ecdsa_p1363_init_tfm;
> +	inst->alg.exit = ecdsa_p1363_exit_tfm;
> +
> +	inst->alg.verify = ecdsa_p1363_verify;
> +	inst->alg.max_size = ecdsa_p1363_max_size;
> +	inst->alg.set_pub_key = ecdsa_p1363_set_pub_key;
> +
> +	inst->free = ecdsa_p1363_free;
> +
> +	err = akcipher_register_instance(tmpl, inst);
> +	if (err) {
> +err_free_inst:
Same comment as in previous patch. I'd use a separate error path after
a return 0 to improve readability.

> +		ecdsa_p1363_free(inst);
> +	}
> +	return err;
> +}