Re: [RFC PATCH v2 1/4] crypto: ecc - add privkey generation support

["Stephan Müller" <smueller@chronox.de>]

Am Mittwoch, 17. Mai 2017, 17:26:50 CEST schrieb Tudor Ambarus:

Hi Tudor,

> Add support for generating ecc private keys.
> 
> Generation of ecc private keys is helpful in a user-space to kernel
> ecdh offload because the keys are not revealed to user-space. Private
> key generation is also helpful to implement forward secrecy.
> 
> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
> ---
>  crypto/ecc.c | 20 ++++++++++++++++++++
>  crypto/ecc.h | 14 ++++++++++++++
>  2 files changed, 34 insertions(+)
> 
> diff --git a/crypto/ecc.c b/crypto/ecc.c
> index 414c78a..a591907 100644
> --- a/crypto/ecc.c
> +++ b/crypto/ecc.c
> @@ -927,6 +927,26 @@ int ecc_is_key_valid(unsigned int curve_id, unsigned
> int ndigits, return 0;
>  }
> 
> +int ecc_gen_privkey(unsigned int curve_id, unsigned int ndigits, u64
> *privkey) +{
> +	const struct ecc_curve *curve = ecc_get_curve(curve_id);

Shouldn't there be a check that a curve is selected? I.e. a check for an error 
should be added?

> +	u64 priv[ndigits];

Shouldn't there be a size check of ndigits?

> +	unsigned int nbytes = ndigits << ECC_DIGITS_TO_BYTES_SHIFT;
> +
> +	get_random_bytes(priv, nbytes);

Can you please use crypto_get_default_rng / crypto_rng_get_bytes / 
crypto_put_default_rng?
> +
> +	if (vli_is_zero(priv, ndigits))
> +		return -EINVAL;
> +
> +	/* Make sure the private key is in the range [1, n-1]. */
> +	if (vli_cmp(curve->n, priv, ndigits) != 1)
> +		return -EINVAL;
> +
> +	ecc_swap_digits(priv, privkey, ndigits);

Is a byteswap faster than a copy operation by looping through priv/privkey and 
simply assinging the value?
> +
> +	return 0;
> +}
> +
>  int ecdh_make_pub_key(unsigned int curve_id, unsigned int ndigits,
>  		      const u8 *private_key, unsigned int private_key_len,
>  		      u8 *public_key, unsigned int public_key_len)
> diff --git a/crypto/ecc.h b/crypto/ecc.h
> index 663d598..b94b7ce 100644
> --- a/crypto/ecc.h
> +++ b/crypto/ecc.h
> @@ -44,6 +44,20 @@ int ecc_is_key_valid(unsigned int curve_id, unsigned int
> ndigits, const u8 *private_key, unsigned int private_key_len);
> 
>  /**
> + * ecc_gen_privkey() -  Generates an ECC private key.
> + * The private key is a random integer in the range 0 < random < n, where n
> is a + * prime that is the order of the cyclic subgroup generated by the
> distinguished + * point G.
> + * @curve_id:		id representing the curve to use
> + * @ndigits:		curve number of digits
> + * @private_key:	buffer for storing the generated private key
> + *
> + * Returns 0 if the private key was generated successfully, a negative
> value + * if an error occurred.
> + */
> +int ecc_gen_privkey(unsigned int curve_id, unsigned int ndigits, u64
> *privkey); +
> +/**
>   * ecdh_make_pub_key() - Compute an ECC public key
>   *
>   * @curve_id:		id representing the curve to use


Ciao
Stephan