Re: [dpdk-dev] [PATCH 1/4] lib/crypto: add support for ECDSA

["Kusztal, ArkadiuszX" <arkadiuszx.kusztal@intel.com>]

Hi Anoob,

Few suggestions inline.

>  ;
>  [Asymmetric]
> -RSA =
> -DSA =
> -Modular Exponentiation =
> -Modular Inversion =
> -Diffie-hellman =
> \ No newline at end of file
> +RSA                     =
> +DSA                     =
> +Modular Exponentiation  =
> +Modular Inversion       =
> +Diffie-hellman          =
> +ECDSA                   =
> diff --git a/lib/librte_cryptodev/rte_crypto_asym.h
> b/lib/librte_cryptodev/rte_crypto_asym.h
> index 0d34ce8..dd5e6e3 100644
> --- a/lib/librte_cryptodev/rte_crypto_asym.h
> +++ b/lib/librte_cryptodev/rte_crypto_asym.h
> @@ -81,6 +81,10 @@ enum rte_crypto_asym_xform_type {
>  	/**< Modular Exponentiation
>  	 * Perform Modular Exponentiation b^e mod n
>  	 */
> +	RTE_CRYPTO_ASYM_XFORM_ECDSA,
> +	/**< Elliptic Curve Digital Signature Algorithm
> +	 * Perform Signature Generation and Verification.
> +	 */
>  	RTE_CRYPTO_ASYM_XFORM_TYPE_LIST_END
>  	/**< End of list */
>  };
> @@ -319,6 +323,46 @@ struct rte_crypto_dsa_xform {  };
> 
>  /**
> + * TLS named curves
> + * https://www.iana.org/assignments/tls-parameters/
> + * tls-parameters.xhtml#tls-parameters-8
> + * secp192r1 = 19,
> + * secp224r1 = 21,
> + * secp256r1 = 23,
> + * secp384r1 = 24,
> + * secp521r1 = 25,
> + */
> +enum rte_crypto_ec_group {
> +	RTE_CRYPTO_EC_GROUP_UNKNOWN  = 0,
> +	RTE_CRYPTO_EC_GROUP_NISTP192 = 19,
> +	RTE_CRYPTO_EC_GROUP_NISTP224 = 21,
> +	RTE_CRYPTO_EC_GROUP_NISTP256 = 23,
> +	RTE_CRYPTO_EC_GROUP_NISTP384 = 24,
> +	RTE_CRYPTO_EC_GROUP_NISTP521 = 25,
> +};

[Arek] Since in comment we use SECG naming maybe enum should follow to avoid confusion?
> +
> +/**
> + * Structure for elliptic curve point
> + */
> +struct rte_crypto_ec_point {
> +	rte_crypto_param x;
> +	/**< X coordinate */
> +	rte_crypto_param y;
> +	/**< Y coordinate */
> +};
> +
> +/**
> + * Asymmetric elliptic curve transform data
> + *
> + * Structure describing all EC based xform params
> + *
> + */
> +struct rte_crypto_ec_xform {
> +	enum rte_crypto_ec_group curve_id;
> +	/**< Pre-defined ec groups */
> +};
> +
> +/**
>   * Operations params for modular operations:
>   * exponentiation and multiplicative inverse
>   *
> @@ -372,6 +416,11 @@ struct rte_crypto_asym_xform {
> 
>  		struct rte_crypto_dsa_xform dsa;
>  		/**< DSA xform parameters */
> +
> +		struct rte_crypto_ec_xform ec;
> +		/**< EC xform parameters, used by elliptic curve based
> +		 * operations.
> +		 */
>  	};
>  };
> 
> @@ -516,6 +565,39 @@ struct rte_crypto_dsa_op_param {  };
> 
>  /**
> + * ECDSA operation params
> + */
> +struct rte_crypto_ecdsa_op_param {
> +	enum rte_crypto_asym_op_type op_type;
> +	/**< Signature generation or verification */
> +
> +	rte_crypto_param pkey;
> +	/**< Private key of the signer for signature generation */
[Arek] - for DSA we have private key in xform, why this inconsistency?
> +
> +	struct rte_crypto_ec_point q;
> +	/**< Public key of the signer for verification */
> +
> +	rte_crypto_param message;
> +	/**< Input message to be signed or verified */
[Arek] - This I expect should be message digest instead of message itself?
> +
> +	rte_crypto_param k;
> +	/**< The ECDSA per-message secret number, which is an integer
> +	 * in the interval (1, n-1)
> +	 */
[Arek] - If pmd can generate 'k' internally we could do something like:
'if k.data == NULL => PMD will generate 'k' internally, k.data remains untouched.'
Another option is to provide user with some callback function to generate CSRN which could be useful for RSA PSS, OAEP as well (we already discussed that internally in Intel, I will elaborate on this bit more in different thread).

> +
> +	rte_crypto_param r;
> +	/**< r component of elliptic curve signature
> +	 *     output : for signature generation
> +	 *     input  : for signature verification
> +	 */
> +	rte_crypto_param s;
> +	/**< s component of elliptic curve signature
> +	 *     output : for signature generation
> +	 *     input  : for signature verification
> +	 */
[Arek] - Do we want to add any constraints like 'this field should be big enough to hold...'
> +};
> +
> +/**
>   * Asymmetric Cryptographic Operation.
>   *
>   * Structure describing asymmetric crypto operation params.
> @@ -537,6 +619,7 @@ struct rte_crypto_asym_op {
>  		struct rte_crypto_mod_op_param modinv;
>  		struct rte_crypto_dh_op_param dh;
>  		struct rte_crypto_dsa_op_param dsa;
> +		struct rte_crypto_ecdsa_op_param ecdsa;
>  	};
>  };
> 
> diff --git a/lib/librte_cryptodev/rte_cryptodev.c
> b/lib/librte_cryptodev/rte_cryptodev.c
> index 89aa2ed..0d6babb 100644
> --- a/lib/librte_cryptodev/rte_cryptodev.c
> +++ b/lib/librte_cryptodev/rte_cryptodev.c
> @@ -173,6 +173,7 @@ const char *rte_crypto_asym_xform_strings[] = {
>  	[RTE_CRYPTO_ASYM_XFORM_MODINV]	= "modinv",
>  	[RTE_CRYPTO_ASYM_XFORM_DH]	= "dh",
>  	[RTE_CRYPTO_ASYM_XFORM_DSA]	= "dsa",
> +	[RTE_CRYPTO_ASYM_XFORM_ECDSA]	= "ecdsa",
>  };
> 
>  /**
> --
> 2.7.4

Regards,
Arek