[v2] cryptodev: add elliptic curve diffie hellman

Message ID 20220427095524.2547-1-arkadiuszx.kusztal@intel.com (mailing list archive)
State Superseded, archived
Delegated to: akhil goyal
Headers
Series [v2] cryptodev: add elliptic curve diffie hellman |

Checks

Context Check Description
ci/checkpatch warning coding style issues
ci/Intel-compilation warning apply issues
ci/iol-testing warning apply patch failure

Commit Message

Arkadiusz Kusztal April 27, 2022, 9:55 a.m. UTC
  This commit adds Elliptic Curve Diffie-Hellman option to Cryptodev.
This could be achieved with EC point multiplication but:
1) Phase 1 of DH is used with EC generator, multiplication expect
setting generator manually.
2) It will unify usage of DH.
3) Can be extended easily to support X25519 and X448.

Signed-off-by: Arek Kusztal <arkadiuszx.kusztal@intel.com>
---
v2:
- added ecdh comments to operation types

Depends-on: series-22684 ("cryptodev: move dh type from xform to dh op")

 lib/cryptodev/rte_crypto_asym.h | 46 +++++++++++++++++++++++------------------
 1 file changed, 26 insertions(+), 20 deletions(-)
  

Comments

Fan Zhang May 10, 2022, 10:06 a.m. UTC | #1
> -----Original Message-----
> From: Kusztal, ArkadiuszX <arkadiuszx.kusztal@intel.com>
> Sent: Wednesday, April 27, 2022 10:55 AM
> To: dev@dpdk.org
> Cc: gakhil@marvell.com; Zhang, Roy Fan <roy.fan.zhang@intel.com>; Kusztal,
> ArkadiuszX <arkadiuszx.kusztal@intel.com>
> Subject: [PATCH v2] cryptodev: add elliptic curve diffie hellman
> 
> This commit adds Elliptic Curve Diffie-Hellman option to Cryptodev.
> This could be achieved with EC point multiplication but:
> 1) Phase 1 of DH is used with EC generator, multiplication expect
> setting generator manually.
> 2) It will unify usage of DH.
> 3) Can be extended easily to support X25519 and X448.
> 
> Signed-off-by: Arek Kusztal <arkadiuszx.kusztal@intel.com>
> ---
> v2:
> - added ecdh comments to operation types
> 
> Depends-on: series-22684 ("cryptodev: move dh type from xform to dh op")
> 
>  lib/cryptodev/rte_crypto_asym.h | 46 +++++++++++++++++++++++---------
> ---------
Acked-by: Fan Zhang <roy.fan.zhang@intel.com>
  
Akhil Goyal May 16, 2022, 6:42 p.m. UTC | #2
> This commit adds Elliptic Curve Diffie-Hellman option to Cryptodev.
> This could be achieved with EC point multiplication but:
> 1) Phase 1 of DH is used with EC generator, multiplication expect
> setting generator manually.
> 2) It will unify usage of DH.
> 3) Can be extended easily to support X25519 and X448.

It is not clear how adding ECDH option will achieve above three statements.

> 
> Signed-off-by: Arek Kusztal <arkadiuszx.kusztal@intel.com>
> ---
> v2:
> - added ecdh comments to operation types
> 
> Depends-on: series-22684 ("cryptodev: move dh type from xform to dh op")
> 
>  lib/cryptodev/rte_crypto_asym.h | 46 +++++++++++++++++++++++----------------
> --
>  1 file changed, 26 insertions(+), 20 deletions(-)
> 
> diff --git a/lib/cryptodev/rte_crypto_asym.h b/lib/cryptodev/rte_crypto_asym.h
> index 4697a7bc59..64d97ae054 100644
> --- a/lib/cryptodev/rte_crypto_asym.h
> +++ b/lib/cryptodev/rte_crypto_asym.h
> @@ -91,6 +91,8 @@ enum rte_crypto_asym_xform_type {
>  	/**< Elliptic Curve Digital Signature Algorithm
>  	 * Perform Signature Generation and Verification.
>  	 */
> +	RTE_CRYPTO_ASYM_XFORM_ECDH,
> +	/**< Elliptic Curve Diffie Hellman */

Can we add this after ECPM?

And we also need to remove the LIST_ENDs from Asym.

>  	RTE_CRYPTO_ASYM_XFORM_ECPM,
>  	/**< Elliptic Curve Point Multiplication */
>  	RTE_CRYPTO_ASYM_XFORM_TYPE_LIST_END
> @@ -112,9 +114,9 @@ enum rte_crypto_asym_op_type {
>  	RTE_CRYPTO_ASYM_OP_PRIVATE_KEY_GENERATE,
>  	/**< DH Private Key generation operation */
>  	RTE_CRYPTO_ASYM_OP_PUBLIC_KEY_GENERATE,
> -	/**< DH Public Key generation operation */
> +	/**< DH/ECDH Public Key generation operation */
>  	RTE_CRYPTO_ASYM_OP_SHARED_SECRET_COMPUTE,
> -	/**< DH Shared Secret compute operation */
> +	/**< DH/ECDH Shared Secret compute operation */
>  	RTE_CRYPTO_ASYM_OP_LIST_END
>  };
> 
> @@ -385,34 +387,38 @@ struct rte_crypto_rsa_op_param {
>  };
> 
>  /**
> - * Diffie-Hellman Operations params.
> + * Diffie-Hellman/Elliptic Curve Diffie-Hellman operation.
>   * @note:
>   */
>  struct rte_crypto_dh_op_param {
>  	enum rte_crypto_asym_op_type op_type;
>  	/**< Diffie-Hellman operation type */
> -	rte_crypto_uint pub_key;
> +	rte_crypto_param priv_key;
>  	/**<
> -	 * Output generated public key when op_type is
> -	 * DH PUB_KEY_GENERATION.
> -	 * Input peer public key when op_type is DH
> -	 * SHARED_SECRET_COMPUTATION
> -	 *
> +	 * Diffie-Hellman private part
> +	 * For DH and ECDH it is big-endian integer.
> +	 * Input for both phases of Diffie-Hellman
>  	 */
> -
> -	rte_crypto_uint priv_key;
> +	union {
> +		rte_crypto_uint pub_key;
> +		struct rte_crypto_ec_point pub_point;
> +	};
>  	/**<
> -	 * Output generated private key if op_type is
> -	 * DH PRIVATE_KEY_GENERATION
> -	 * Input when op_type is DH SHARED_SECRET_COMPUTATION.
> -	 *
> +	 * Diffie-Hellman public part
> +	 * For DH it is big-endian unsigned integer.
> +	 * For ECDH it is a point on the curve.
> +	 * Output for RTE_CRYPTO_ASYM_OP_PUBLIC_KEY_GENERATE
> +	 * Input for RTE_CRYPTO_ASYM_OP_SHARED_SECRET_COMPUTE
>  	 */
> -
> -	rte_crypto_uint shared_secret;
> +	union {
> +		rte_crypto_uint shared_secret;
> +		struct rte_crypto_ec_point shared_point;
> +	};
>  	/**<
> -	 * Output with calculated shared secret
> -	 * when dh op_type = SHARED_SECRET_COMPUTATION.
> -	 *
> +	 * Diffie-Hellman shared secret
> +	 * For DH it is big-endian unsigned integer.
> +	 * For ECDH it is a point on the curve.
> +	 * Output for RTE_CRYPTO_ASYM_OP_SHARED_SECRET_COMPUTE
>  	 */
>  };

There are 2 questions for this above change.
- why is the order of parameters change?
- The patch description does not describe this change. Could you separate out this change
And provide proper explanation to this change?
  
Arkadiusz Kusztal May 17, 2022, 11:58 a.m. UTC | #3
> -----Original Message-----
> From: Akhil Goyal <gakhil@marvell.com>
> Sent: Monday, May 16, 2022 8:42 PM
> To: Kusztal, ArkadiuszX <arkadiuszx.kusztal@intel.com>; dev@dpdk.org
> Cc: Zhang, Roy Fan <roy.fan.zhang@intel.com>
> Subject: RE: [EXT] [PATCH v2] cryptodev: add elliptic curve diffie hellman
> 
> > This commit adds Elliptic Curve Diffie-Hellman option to Cryptodev.
> > This could be achieved with EC point multiplication but:
> > 1) Phase 1 of DH is used with EC generator, multiplication expect
> > setting generator manually.
> > 2) It will unify usage of DH.
> > 3) Can be extended easily to support X25519 and X448.
> 
> It is not clear how adding ECDH option will achieve above three statements.
[Arek] - Sure I will add an explanation, more or less like this:
1) - EC point multiplication allows user to process every phase of ECDH but for phase 1, user should really not care about generator. He does no even need to know how generator looks like, therefore setting ec would make this work for him.
2) - Instead of having {dh_op, ecdh_op, x448_op, x25519_op} would would have on DH combined with different types of xform (dh_xform for FFDH and EC for curves).
3) - Addition of field in the union should be enough, provided that curves are added to enum.
> 
> >
> > Signed-off-by: Arek Kusztal <arkadiuszx.kusztal@intel.com>
> > ---
> > v2:
> > - added ecdh comments to operation types
> >
> > Depends-on: series-22684 ("cryptodev: move dh type from xform to dh
> > op")
> >
> >  lib/cryptodev/rte_crypto_asym.h | 46
> > +++++++++++++++++++++++----------------
> > --
> >  1 file changed, 26 insertions(+), 20 deletions(-)
> >
> > diff --git a/lib/cryptodev/rte_crypto_asym.h
> > b/lib/cryptodev/rte_crypto_asym.h index 4697a7bc59..64d97ae054 100644
> > --- a/lib/cryptodev/rte_crypto_asym.h
> > +++ b/lib/cryptodev/rte_crypto_asym.h
> > @@ -91,6 +91,8 @@ enum rte_crypto_asym_xform_type {
> >  	/**< Elliptic Curve Digital Signature Algorithm
> >  	 * Perform Signature Generation and Verification.
> >  	 */
> > +	RTE_CRYPTO_ASYM_XFORM_ECDH,
> > +	/**< Elliptic Curve Diffie Hellman */
> 
> Can we add this after ECPM?
> 
> And we also need to remove the LIST_ENDs from Asym.
> 
> >  	RTE_CRYPTO_ASYM_XFORM_ECPM,
> >  	/**< Elliptic Curve Point Multiplication */
> >  	RTE_CRYPTO_ASYM_XFORM_TYPE_LIST_END
> > @@ -112,9 +114,9 @@ enum rte_crypto_asym_op_type {
> >  	RTE_CRYPTO_ASYM_OP_PRIVATE_KEY_GENERATE,
> >  	/**< DH Private Key generation operation */
> >  	RTE_CRYPTO_ASYM_OP_PUBLIC_KEY_GENERATE,
> > -	/**< DH Public Key generation operation */
> > +	/**< DH/ECDH Public Key generation operation */
> >  	RTE_CRYPTO_ASYM_OP_SHARED_SECRET_COMPUTE,
> > -	/**< DH Shared Secret compute operation */
> > +	/**< DH/ECDH Shared Secret compute operation */
> >  	RTE_CRYPTO_ASYM_OP_LIST_END
> >  };
> >
> > @@ -385,34 +387,38 @@ struct rte_crypto_rsa_op_param {  };
> >
> >  /**
> > - * Diffie-Hellman Operations params.
> > + * Diffie-Hellman/Elliptic Curve Diffie-Hellman operation.
> >   * @note:
> >   */
> >  struct rte_crypto_dh_op_param {
> >  	enum rte_crypto_asym_op_type op_type;
> >  	/**< Diffie-Hellman operation type */
> > -	rte_crypto_uint pub_key;
> > +	rte_crypto_param priv_key;
> >  	/**<
> > -	 * Output generated public key when op_type is
> > -	 * DH PUB_KEY_GENERATION.
> > -	 * Input peer public key when op_type is DH
> > -	 * SHARED_SECRET_COMPUTATION
> > -	 *
> > +	 * Diffie-Hellman private part
> > +	 * For DH and ECDH it is big-endian integer.
> > +	 * Input for both phases of Diffie-Hellman
> >  	 */
> > -
> > -	rte_crypto_uint priv_key;
> > +	union {
> > +		rte_crypto_uint pub_key;
> > +		struct rte_crypto_ec_point pub_point;
> > +	};
> >  	/**<
> > -	 * Output generated private key if op_type is
> > -	 * DH PRIVATE_KEY_GENERATION
> > -	 * Input when op_type is DH SHARED_SECRET_COMPUTATION.
> > -	 *
> > +	 * Diffie-Hellman public part
> > +	 * For DH it is big-endian unsigned integer.
> > +	 * For ECDH it is a point on the curve.
> > +	 * Output for RTE_CRYPTO_ASYM_OP_PUBLIC_KEY_GENERATE
> > +	 * Input for RTE_CRYPTO_ASYM_OP_SHARED_SECRET_COMPUTE
> >  	 */
> > -
> > -	rte_crypto_uint shared_secret;
> > +	union {
> > +		rte_crypto_uint shared_secret;
> > +		struct rte_crypto_ec_point shared_point;
> > +	};
> >  	/**<
> > -	 * Output with calculated shared secret
> > -	 * when dh op_type = SHARED_SECRET_COMPUTATION.
> > -	 *
> > +	 * Diffie-Hellman shared secret
> > +	 * For DH it is big-endian unsigned integer.
> > +	 * For ECDH it is a point on the curve.
> > +	 * Output for RTE_CRYPTO_ASYM_OP_SHARED_SECRET_COMPUTE
> >  	 */
> >  };
> 
> There are 2 questions for this above change.
> - why is the order of parameters change?
> - The patch description does not describe this change. Could you separate out
> this change And provide proper explanation to this change?
  
Arkadiusz Kusztal May 17, 2022, 12:02 p.m. UTC | #4
> -----Original Message-----
> From: Kusztal, ArkadiuszX
> Sent: Tuesday, May 17, 2022 1:58 PM
> To: 'Akhil Goyal' <gakhil@marvell.com>; dev@dpdk.org
> Cc: Zhang, Roy Fan <roy.fan.zhang@intel.com>
> Subject: RE: [EXT] [PATCH v2] cryptodev: add elliptic curve diffie hellman
> 
> 
> 
> > -----Original Message-----
> > From: Akhil Goyal <gakhil@marvell.com>
> > Sent: Monday, May 16, 2022 8:42 PM
> > To: Kusztal, ArkadiuszX <arkadiuszx.kusztal@intel.com>; dev@dpdk.org
> > Cc: Zhang, Roy Fan <roy.fan.zhang@intel.com>
> > Subject: RE: [EXT] [PATCH v2] cryptodev: add elliptic curve diffie
> > hellman
> >
> > > This commit adds Elliptic Curve Diffie-Hellman option to Cryptodev.
> > > This could be achieved with EC point multiplication but:
> > > 1) Phase 1 of DH is used with EC generator, multiplication expect
> > > setting generator manually.
> > > 2) It will unify usage of DH.
> > > 3) Can be extended easily to support X25519 and X448.
> >
> > It is not clear how adding ECDH option will achieve above three statements.
> [Arek] - Sure I will add an explanation, more or less like this:
> 1) - EC point multiplication allows user to process every phase of ECDH but for
> phase 1, user should really not care about generator. He does no even need to
> know how generator looks like, therefore setting ec would make this work for
> him.
> 2) - Instead of having {dh_op, ecdh_op, x448_op, x25519_op} would would have
> on DH combined with different types of xform (dh_xform for FFDH and EC for
> curves).
> 3) - Addition of field in the union should be enough, provided that curves are
> added to enum.
> >
> > >
> > > Signed-off-by: Arek Kusztal <arkadiuszx.kusztal@intel.com>
> > > ---
> > > v2:
> > > - added ecdh comments to operation types
> > >
> > > Depends-on: series-22684 ("cryptodev: move dh type from xform to dh
> > > op")
> > >
> > >  lib/cryptodev/rte_crypto_asym.h | 46
> > > +++++++++++++++++++++++----------------
> > > --
> > >  1 file changed, 26 insertions(+), 20 deletions(-)
> > >
> > > diff --git a/lib/cryptodev/rte_crypto_asym.h
> > > b/lib/cryptodev/rte_crypto_asym.h index 4697a7bc59..64d97ae054
> > > 100644
> > > --- a/lib/cryptodev/rte_crypto_asym.h
> > > +++ b/lib/cryptodev/rte_crypto_asym.h
> > > @@ -91,6 +91,8 @@ enum rte_crypto_asym_xform_type {
> > >  	/**< Elliptic Curve Digital Signature Algorithm
> > >  	 * Perform Signature Generation and Verification.
> > >  	 */
> > > +	RTE_CRYPTO_ASYM_XFORM_ECDH,
> > > +	/**< Elliptic Curve Diffie Hellman */
> >
> > Can we add this after ECPM?
> >
> > And we also need to remove the LIST_ENDs from Asym.
> >
> > >  	RTE_CRYPTO_ASYM_XFORM_ECPM,
> > >  	/**< Elliptic Curve Point Multiplication */
> > >  	RTE_CRYPTO_ASYM_XFORM_TYPE_LIST_END
> > > @@ -112,9 +114,9 @@ enum rte_crypto_asym_op_type {
> > >  	RTE_CRYPTO_ASYM_OP_PRIVATE_KEY_GENERATE,
> > >  	/**< DH Private Key generation operation */
> > >  	RTE_CRYPTO_ASYM_OP_PUBLIC_KEY_GENERATE,
> > > -	/**< DH Public Key generation operation */
> > > +	/**< DH/ECDH Public Key generation operation */
> > >  	RTE_CRYPTO_ASYM_OP_SHARED_SECRET_COMPUTE,
> > > -	/**< DH Shared Secret compute operation */
> > > +	/**< DH/ECDH Shared Secret compute operation */
> > >  	RTE_CRYPTO_ASYM_OP_LIST_END
> > >  };
> > >
> > > @@ -385,34 +387,38 @@ struct rte_crypto_rsa_op_param {  };
> > >
> > >  /**
> > > - * Diffie-Hellman Operations params.
> > > + * Diffie-Hellman/Elliptic Curve Diffie-Hellman operation.
> > >   * @note:
> > >   */
> > >  struct rte_crypto_dh_op_param {
> > >  	enum rte_crypto_asym_op_type op_type;
> > >  	/**< Diffie-Hellman operation type */
> > > -	rte_crypto_uint pub_key;
> > > +	rte_crypto_param priv_key;
> > >  	/**<
> > > -	 * Output generated public key when op_type is
> > > -	 * DH PUB_KEY_GENERATION.
> > > -	 * Input peer public key when op_type is DH
> > > -	 * SHARED_SECRET_COMPUTATION
> > > -	 *
> > > +	 * Diffie-Hellman private part
> > > +	 * For DH and ECDH it is big-endian integer.
> > > +	 * Input for both phases of Diffie-Hellman
> > >  	 */
> > > -
> > > -	rte_crypto_uint priv_key;
> > > +	union {
> > > +		rte_crypto_uint pub_key;
> > > +		struct rte_crypto_ec_point pub_point;
> > > +	};
> > >  	/**<
> > > -	 * Output generated private key if op_type is
> > > -	 * DH PRIVATE_KEY_GENERATION
> > > -	 * Input when op_type is DH SHARED_SECRET_COMPUTATION.
> > > -	 *
> > > +	 * Diffie-Hellman public part
> > > +	 * For DH it is big-endian unsigned integer.
> > > +	 * For ECDH it is a point on the curve.
> > > +	 * Output for RTE_CRYPTO_ASYM_OP_PUBLIC_KEY_GENERATE
> > > +	 * Input for RTE_CRYPTO_ASYM_OP_SHARED_SECRET_COMPUTE
> > >  	 */
> > > -
> > > -	rte_crypto_uint shared_secret;
> > > +	union {
> > > +		rte_crypto_uint shared_secret;
> > > +		struct rte_crypto_ec_point shared_point;
> > > +	};
> > >  	/**<
> > > -	 * Output with calculated shared secret
> > > -	 * when dh op_type = SHARED_SECRET_COMPUTATION.
> > > -	 *
> > > +	 * Diffie-Hellman shared secret
> > > +	 * For DH it is big-endian unsigned integer.
> > > +	 * For ECDH it is a point on the curve.
> > > +	 * Output for RTE_CRYPTO_ASYM_OP_SHARED_SECRET_COMPUTE
> > >  	 */
> > >  };
> >
> > There are 2 questions for this above change.
> > - why is the order of parameters change?
[Arek] - order now reflects, order of Diffie Hellman operations, but it is not big deal, it can be the way it was previously. So there was no additional patch only for that.

> > - The patch description does not describe this change. Could you
> > separate out this change And provide proper explanation to this change?
[Arek] - sure, I will.
  

Patch

diff --git a/lib/cryptodev/rte_crypto_asym.h b/lib/cryptodev/rte_crypto_asym.h
index 4697a7bc59..64d97ae054 100644
--- a/lib/cryptodev/rte_crypto_asym.h
+++ b/lib/cryptodev/rte_crypto_asym.h
@@ -91,6 +91,8 @@  enum rte_crypto_asym_xform_type {
 	/**< Elliptic Curve Digital Signature Algorithm
 	 * Perform Signature Generation and Verification.
 	 */
+	RTE_CRYPTO_ASYM_XFORM_ECDH,
+	/**< Elliptic Curve Diffie Hellman */
 	RTE_CRYPTO_ASYM_XFORM_ECPM,
 	/**< Elliptic Curve Point Multiplication */
 	RTE_CRYPTO_ASYM_XFORM_TYPE_LIST_END
@@ -112,9 +114,9 @@  enum rte_crypto_asym_op_type {
 	RTE_CRYPTO_ASYM_OP_PRIVATE_KEY_GENERATE,
 	/**< DH Private Key generation operation */
 	RTE_CRYPTO_ASYM_OP_PUBLIC_KEY_GENERATE,
-	/**< DH Public Key generation operation */
+	/**< DH/ECDH Public Key generation operation */
 	RTE_CRYPTO_ASYM_OP_SHARED_SECRET_COMPUTE,
-	/**< DH Shared Secret compute operation */
+	/**< DH/ECDH Shared Secret compute operation */
 	RTE_CRYPTO_ASYM_OP_LIST_END
 };
 
@@ -385,34 +387,38 @@  struct rte_crypto_rsa_op_param {
 };
 
 /**
- * Diffie-Hellman Operations params.
+ * Diffie-Hellman/Elliptic Curve Diffie-Hellman operation.
  * @note:
  */
 struct rte_crypto_dh_op_param {
 	enum rte_crypto_asym_op_type op_type;
 	/**< Diffie-Hellman operation type */
-	rte_crypto_uint pub_key;
+	rte_crypto_param priv_key;
 	/**<
-	 * Output generated public key when op_type is
-	 * DH PUB_KEY_GENERATION.
-	 * Input peer public key when op_type is DH
-	 * SHARED_SECRET_COMPUTATION
-	 *
+	 * Diffie-Hellman private part
+	 * For DH and ECDH it is big-endian integer.
+	 * Input for both phases of Diffie-Hellman
 	 */
-
-	rte_crypto_uint priv_key;
+	union {
+		rte_crypto_uint pub_key;
+		struct rte_crypto_ec_point pub_point;
+	};
 	/**<
-	 * Output generated private key if op_type is
-	 * DH PRIVATE_KEY_GENERATION
-	 * Input when op_type is DH SHARED_SECRET_COMPUTATION.
-	 *
+	 * Diffie-Hellman public part
+	 * For DH it is big-endian unsigned integer.
+	 * For ECDH it is a point on the curve.
+	 * Output for RTE_CRYPTO_ASYM_OP_PUBLIC_KEY_GENERATE
+	 * Input for RTE_CRYPTO_ASYM_OP_SHARED_SECRET_COMPUTE
 	 */
-
-	rte_crypto_uint shared_secret;
+	union {
+		rte_crypto_uint shared_secret;
+		struct rte_crypto_ec_point shared_point;
+	};
 	/**<
-	 * Output with calculated shared secret
-	 * when dh op_type = SHARED_SECRET_COMPUTATION.
-	 *
+	 * Diffie-Hellman shared secret
+	 * For DH it is big-endian unsigned integer.
+	 * For ECDH it is a point on the curve.
+	 * Output for RTE_CRYPTO_ASYM_OP_SHARED_SECRET_COMPUTE
 	 */
 };