RE: [RFC PATCH] cryptodev: add diffie hellman verify, change ec enum

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

Hi,

Some explanations below.

> -----Original Message-----
> From: Kusztal, ArkadiuszX <arkadiuszx.kusztal@intel.com>
> Sent: Thursday, April 7, 2022 3:43 PM
> To: dev@dpdk.org
> Cc: gakhil@marvell.com; Zhang, Roy Fan <roy.fan.zhang@intel.com>; Kusztal,
> ArkadiuszX <arkadiuszx.kusztal@intel.com>
> Subject: [RFC PATCH] cryptodev: add diffie hellman verify, change ec enum
> 
> This commit:
> 1) adds Diffie-Hellman verify operation.
> 2) splits asym_op_type with dh op_type
> 3) removes next pointer from asym_xform
> 4) changes enumeration of elliptic curves
> 
> Signed-off-by: Arek Kusztal <arkadiuszx.kusztal@intel.com>
> ---
>  lib/cryptodev/rte_crypto_asym.h | 36 +++++++++++++++++-------------------
>  1 file changed, 17 insertions(+), 19 deletions(-)
> 
> diff --git a/lib/cryptodev/rte_crypto_asym.h b/lib/cryptodev/rte_crypto_asym.h
> index cd24d4b07b..6fbc7b7708 100644
> --- a/lib/cryptodev/rte_crypto_asym.h
> +++ b/lib/cryptodev/rte_crypto_asym.h
> @@ -43,11 +43,11 @@ rte_crypto_asym_op_strings[];
>   */
>  enum rte_crypto_ec_group {
>  	RTE_CRYPTO_EC_GROUP_UNKNOWN  = 0,
> -	RTE_CRYPTO_EC_GROUP_SECP192R1 = 19,
> -	RTE_CRYPTO_EC_GROUP_SECP224R1 = 21,
> -	RTE_CRYPTO_EC_GROUP_SECP256R1 = 23,
> -	RTE_CRYPTO_EC_GROUP_SECP384R1 = 24,
> -	RTE_CRYPTO_EC_GROUP_SECP521R1 = 25,
> +	RTE_CRYPTO_EC_GROUP_SECP192R1 = 1,
> +	RTE_CRYPTO_EC_GROUP_SECP224R1 = 2,
> +	RTE_CRYPTO_EC_GROUP_SECP256R1 = 3,
> +	RTE_CRYPTO_EC_GROUP_SECP384R1 = 4,
> +	RTE_CRYPTO_EC_GROUP_SECP521R1 = 5,
>  };

[Arek] - this one we could change for the following reasons:
- this is TLS specific registry, this values does not need to correspond with other protocols like Ikev2
- we cannot set deprecated values < 19
- TLS registry to some extent is incorrectly named to. It contains mod exp groups as well, and we do not even support that. But if we would, it probably be not "crypto_ec_group"
 

> 
>  /**
> @@ -109,13 +109,19 @@ enum rte_crypto_asym_op_type {
>  	/**< Signature Generation operation */
>  	RTE_CRYPTO_ASYM_OP_VERIFY,
>  	/**< Signature Verification operation */
> -	RTE_CRYPTO_ASYM_OP_PRIVATE_KEY_GENERATE,
> +	RTE_CRYPTO_ASYM_OP_LIST_END
> +};
> +
> +enum rte_crypto_dh_op_type {
> +	RTE_CRYPTO_DH_OP_INVALID,
> +	RTE_CRYPTO_DH_OP_PRIVATE_KEY_GENERATE,
>  	/**< DH Private Key generation operation */
> -	RTE_CRYPTO_ASYM_OP_PUBLIC_KEY_GENERATE,
> +	RTE_CRYPTO_DH_OP_PUBLIC_KEY_GENERATE,
>  	/**< DH Public Key generation operation */
> -	RTE_CRYPTO_ASYM_OP_SHARED_SECRET_COMPUTE,
> +	RTE_CRYPTO_DH_OP_SHARED_SECRET_COMPUTE,
>  	/**< DH Shared Secret compute operation */
> -	RTE_CRYPTO_ASYM_OP_LIST_END
> +	RTE_CRYPTO_DH_OP_PUBLIC_KEY_VERIFY,
> +	/**< DH verify correctness of public key */
>  };
[Arek] - DH Verify need to be added anyway, but rfc here is because I have split asym_op_type into dh_op_type and asym_op_type. This leaves asym_op_type like that:
{
	ENCRYPT,
	DECRYPT,
	AUTHENTICATE = SIGN,
	VERIFY = VERIFY_SIGNATURE,
}
I know it is too late to create one generic op_type for symmetric and asymmetric but still it is better to group crypto and key exchange operations.
> 
>  /**
> @@ -256,7 +262,7 @@ struct rte_crypto_modinv_xform {
>   *
>   */
>  struct rte_crypto_dh_xform {
> -	enum rte_crypto_asym_op_type type;
> +	enum rte_crypto_dh_op_type type;
>  	/**< Setup xform for key generate or shared secret compute */
>  	rte_crypto_uint p;
>  	/**< Prime modulus data */
> @@ -278,13 +284,7 @@ struct rte_crypto_dsa_xform {
>  	rte_crypto_uint g;
>  	/**< Generator of the subgroup */
>  	rte_crypto_uint x;
> -	/**< x: Private key of the signer in octet-string network
> -	 * byte order format.
> -	 * Used when app has pre-defined private key.
> -	 * Valid only when xform chain is DSA ONLY.
> -	 * if xform chain is DH private key generate + DSA, then DSA sign
> -	 * compute will use internally generated key.
> -	 */
> +	/**< x: Private key */
[Arek] - unless someone can show how it works we should drop it.
>  };
> 
>  /**
> @@ -504,8 +504,6 @@ struct rte_crypto_ecpm_op_param {
>   * Structure describing asym xforms.
>   */
>  struct rte_crypto_asym_xform {
> -	struct rte_crypto_asym_xform *next;
> -	/**< Pointer to next xform to set up xform chain.*/
[Arek] - same as above, unless there is any application for it we should drop it.
>  	enum rte_crypto_asym_xform_type xform_type;
>  	/**< Asymmetric crypto transform */
> 
> --
> 2.13.6