diff mbox series

[20/40] cryptodev: add elliptic curve diffie hellman

Message ID 20220520055445.40063-21-arkadiuszx.kusztal@intel.com (mailing list archive)
State Changes Requested, archived
Delegated to: akhil goyal
Headers
Series cryptodev: rsa, dh, ecdh changes |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Arkadiusz Kusztal May 20, 2022, 5:54 a.m. UTC 
  - added elliptic curve Diffie-Hellman parameters.
Point multiplication allows the user to process every phase of
ECDH, but for phase 1, user should not really care about the generator.
The user does not even need to know what the generator looks like,
therefore setting ec xform would make this work.

Signed-off-by: Arek Kusztal <arkadiuszx.kusztal@intel.com>
---
 lib/cryptodev/rte_crypto_asym.h | 34 +++++++++++++++++++++-------------
 1 file changed, 21 insertions(+), 13 deletions(-)

Comments

Akhil Goyal May 24, 2022, 12:08 p.m. UTC | #1 
> - added elliptic curve Diffie-Hellman parameters.
> Point multiplication allows the user to process every phase of
> ECDH, but for phase 1, user should not really care about the generator.
> The user does not even need to know what the generator looks like,
> therefore setting ec xform would make this work.
> 
> Signed-off-by: Arek Kusztal <arkadiuszx.kusztal@intel.com>
> ---
>  lib/cryptodev/rte_crypto_asym.h | 34 +++++++++++++++++++++-------------
>  1 file changed, 21 insertions(+), 13 deletions(-)
> 
> diff --git a/lib/cryptodev/rte_crypto_asym.h b/lib/cryptodev/rte_crypto_asym.h
> index 52cb1c5c80..09edf2ac3d 100644
> --- a/lib/cryptodev/rte_crypto_asym.h
> +++ b/lib/cryptodev/rte_crypto_asym.h
> @@ -82,8 +82,10 @@ enum rte_crypto_asym_xform_type {
>  	/**< Modular Exponentiation */
>  	RTE_CRYPTO_ASYM_XFORM_ECDSA,
>  	/**< Elliptic Curve Digital Signature Algorithm */
> -	RTE_CRYPTO_ASYM_XFORM_ECPM
> +	RTE_CRYPTO_ASYM_XFORM_ECPM,
>  	/**< Elliptic Curve Point Multiplication */
> +	RTE_CRYPTO_ASYM_XFORM_ECDH
> +	/**< Elliptic Curve Diffie Hellman */
>  };
Please add relevant information in "doc/guides/prog_guide/cryptodev_lib.rst"

> 
>  /**
> @@ -383,22 +385,28 @@ struct rte_crypto_dh_op_param {
>  	/**<
>  	 * Output generated private key when op_type is
>  	 * DH PRIVATE_KEY_GENERATION

Update with actual enum name for this as well

> -	 * Input when op_type is DH SHARED_SECRET_COMPUTATION.
> -	 *
> +	 * Input for RTE_CRYPTO_ASYM_KE_SHARED_SECRET_COMPUTE
>  	 */
> -	rte_crypto_uint pub_key;
> +	union {
> +		rte_crypto_uint pub_key;
> +		struct rte_crypto_ec_point pub_point;
> +	};
>  	/**<
> -	 * 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 public part
> +	 * For DH it is big-endian unsigned integer.
> +	 * For ECDH it is a point on the curve.
> +	 * Output for RTE_CRYPTO_ASYM_KE_PUBLIC_KEY_GENERATE
> +	 * Input for RTE_CRYPTO_ASYM_KE_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 op type is 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_KE_SHARED_SECRET_COMPUTE
>  	 */

Is it output for others as well?
Please be consistent in the comments for all params.


>  };
> 
> --
> 2.13.6
Arkadiusz Kusztal May 24, 2022, 2:52 p.m. UTC | #2 
> -----Original Message-----
> From: Akhil Goyal <gakhil@marvell.com>
> Sent: Tuesday, May 24, 2022 2:08 PM
> To: Kusztal, ArkadiuszX <arkadiuszx.kusztal@intel.com>; dev@dpdk.org
> Cc: Anoob Joseph <anoobj@marvell.com>; Zhang, Roy Fan
> <roy.fan.zhang@intel.com>
> Subject: RE: [EXT] [PATCH 20/40] cryptodev: add elliptic curve diffie hellman
> 
> 
> > - added elliptic curve Diffie-Hellman parameters.
> > Point multiplication allows the user to process every phase of ECDH,
> > but for phase 1, user should not really care about the generator.
> > The user does not even need to know what the generator looks like,
> > therefore setting ec xform would make this work.
> >
> > Signed-off-by: Arek Kusztal <arkadiuszx.kusztal@intel.com>
> > ---
> >  lib/cryptodev/rte_crypto_asym.h | 34
> > +++++++++++++++++++++-------------
> >  1 file changed, 21 insertions(+), 13 deletions(-)
> >
> > diff --git a/lib/cryptodev/rte_crypto_asym.h
> > b/lib/cryptodev/rte_crypto_asym.h index 52cb1c5c80..09edf2ac3d 100644
> > --- a/lib/cryptodev/rte_crypto_asym.h
> > +++ b/lib/cryptodev/rte_crypto_asym.h
> > @@ -82,8 +82,10 @@ enum rte_crypto_asym_xform_type {
> >  	/**< Modular Exponentiation */
> >  	RTE_CRYPTO_ASYM_XFORM_ECDSA,
> >  	/**< Elliptic Curve Digital Signature Algorithm */
> > -	RTE_CRYPTO_ASYM_XFORM_ECPM
> > +	RTE_CRYPTO_ASYM_XFORM_ECPM,
> >  	/**< Elliptic Curve Point Multiplication */
> > +	RTE_CRYPTO_ASYM_XFORM_ECDH
> > +	/**< Elliptic Curve Diffie Hellman */
> >  };
> Please add relevant information in "doc/guides/prog_guide/cryptodev_lib.rst"
> 
> >
> >  /**
> > @@ -383,22 +385,28 @@ struct rte_crypto_dh_op_param {
> >  	/**<
> >  	 * Output generated private key when op_type is
> >  	 * DH PRIVATE_KEY_GENERATION
> 
> Update with actual enum name for this as well
> 
> > -	 * Input when op_type is DH SHARED_SECRET_COMPUTATION.
> > -	 *
> > +	 * Input for RTE_CRYPTO_ASYM_KE_SHARED_SECRET_COMPUTE
> >  	 */
> > -	rte_crypto_uint pub_key;
> > +	union {
> > +		rte_crypto_uint pub_key;
> > +		struct rte_crypto_ec_point pub_point;
> > +	};
> >  	/**<
> > -	 * 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 public part
> > +	 * For DH it is big-endian unsigned integer.
> > +	 * For ECDH it is a point on the curve.
> > +	 * Output for RTE_CRYPTO_ASYM_KE_PUBLIC_KEY_GENERATE
> > +	 * Input for RTE_CRYPTO_ASYM_KE_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 op type is 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_KE_SHARED_SECRET_COMPUTE
> >  	 */
> 
> Is it output for others as well?
[Arek] - No, it is output for phase2 only.
> Please be consistent in the comments for all params.
> 
> 
> >  };
> >
> > --
> > 2.13.6
Arkadiusz Kusztal May 24, 2022, 2:55 p.m. UTC | #3 
> -----Original Message-----
> From: Kusztal, ArkadiuszX
> Sent: Tuesday, May 24, 2022 4:52 PM
> To: Akhil Goyal <gakhil@marvell.com>; dev@dpdk.org
> Cc: Anoob Joseph <anoobj@marvell.com>; Zhang, Roy Fan
> <roy.fan.zhang@intel.com>
> Subject: RE: [EXT] [PATCH 20/40] cryptodev: add elliptic curve diffie hellman
> 
> 
> 
> > -----Original Message-----
> > From: Akhil Goyal <gakhil@marvell.com>
> > Sent: Tuesday, May 24, 2022 2:08 PM
> > To: Kusztal, ArkadiuszX <arkadiuszx.kusztal@intel.com>; dev@dpdk.org
> > Cc: Anoob Joseph <anoobj@marvell.com>; Zhang, Roy Fan
> > <roy.fan.zhang@intel.com>
> > Subject: RE: [EXT] [PATCH 20/40] cryptodev: add elliptic curve diffie
> > hellman
> >
> >
> > > - added elliptic curve Diffie-Hellman parameters.
> > > Point multiplication allows the user to process every phase of ECDH,
> > > but for phase 1, user should not really care about the generator.
> > > The user does not even need to know what the generator looks like,
> > > therefore setting ec xform would make this work.
> > >
> > > Signed-off-by: Arek Kusztal <arkadiuszx.kusztal@intel.com>
> > > ---
> > >  lib/cryptodev/rte_crypto_asym.h | 34
> > > +++++++++++++++++++++-------------
> > >  1 file changed, 21 insertions(+), 13 deletions(-)
> > >
> > > diff --git a/lib/cryptodev/rte_crypto_asym.h
> > > b/lib/cryptodev/rte_crypto_asym.h index 52cb1c5c80..09edf2ac3d
> > > 100644
> > > --- a/lib/cryptodev/rte_crypto_asym.h
> > > +++ b/lib/cryptodev/rte_crypto_asym.h
> > > @@ -82,8 +82,10 @@ enum rte_crypto_asym_xform_type {
> > >  	/**< Modular Exponentiation */
> > >  	RTE_CRYPTO_ASYM_XFORM_ECDSA,
> > >  	/**< Elliptic Curve Digital Signature Algorithm */
> > > -	RTE_CRYPTO_ASYM_XFORM_ECPM
> > > +	RTE_CRYPTO_ASYM_XFORM_ECPM,
> > >  	/**< Elliptic Curve Point Multiplication */
> > > +	RTE_CRYPTO_ASYM_XFORM_ECDH
> > > +	/**< Elliptic Curve Diffie Hellman */
> > >  };
> > Please add relevant information in "doc/guides/prog_guide/cryptodev_lib.rst"
> >
> > >
> > >  /**
> > > @@ -383,22 +385,28 @@ struct rte_crypto_dh_op_param {
> > >  	/**<
> > >  	 * Output generated private key when op_type is
> > >  	 * DH PRIVATE_KEY_GENERATION
> >
> > Update with actual enum name for this as well
> >
> > > -	 * Input when op_type is DH SHARED_SECRET_COMPUTATION.
> > > -	 *
> > > +	 * Input for RTE_CRYPTO_ASYM_KE_SHARED_SECRET_COMPUTE
> > >  	 */
> > > -	rte_crypto_uint pub_key;
> > > +	union {
> > > +		rte_crypto_uint pub_key;
> > > +		struct rte_crypto_ec_point pub_point;
> > > +	};
> > >  	/**<
> > > -	 * 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 public part
> > > +	 * For DH it is big-endian unsigned integer.
> > > +	 * For ECDH it is a point on the curve.
> > > +	 * Output for RTE_CRYPTO_ASYM_KE_PUBLIC_KEY_GENERATE
> > > +	 * Input for RTE_CRYPTO_ASYM_KE_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 op type is 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_KE_SHARED_SECRET_COMPUTE
> > >  	 */
> >
> > Is it output for others as well?
> [Arek] - No, it is output for phase2 only.
[Arek] - additionally there is still an option to split it into DH_PARAM, ECDH_PARAM, x25519. For secp and Edwards/Montgomery it should be ok with current approach but for SM2 we may need new struct.
> > Please be consistent in the comments for all params.
> >
> >
> > >  };
> > >
> > > --
> > > 2.13.6
Akhil Goyal May 25, 2022, 6:02 a.m. UTC | #4 
> > > >  /**
> > > > @@ -383,22 +385,28 @@ struct rte_crypto_dh_op_param {
> > > >  	/**<
> > > >  	 * Output generated private key when op_type is
> > > >  	 * DH PRIVATE_KEY_GENERATION
> > >
> > > Update with actual enum name for this as well
> > >
> > > > -	 * Input when op_type is DH SHARED_SECRET_COMPUTATION.
> > > > -	 *
> > > > +	 * Input for RTE_CRYPTO_ASYM_KE_SHARED_SECRET_COMPUTE
> > > >  	 */
> > > > -	rte_crypto_uint pub_key;
> > > > +	union {
> > > > +		rte_crypto_uint pub_key;
> > > > +		struct rte_crypto_ec_point pub_point;
> > > > +	};
> > > >  	/**<
> > > > -	 * 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 public part
> > > > +	 * For DH it is big-endian unsigned integer.
> > > > +	 * For ECDH it is a point on the curve.
> > > > +	 * Output for RTE_CRYPTO_ASYM_KE_PUBLIC_KEY_GENERATE
> > > > +	 * Input for RTE_CRYPTO_ASYM_KE_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 op type is 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_KE_SHARED_SECRET_COMPUTE
> > > >  	 */
> > >
> > > Is it output for others as well?
> > [Arek] - No, it is output for phase2 only.
> [Arek] - additionally there is still an option to split it into DH_PARAM,
> ECDH_PARAM, x25519. For secp and Edwards/Montgomery it should be ok with
> current approach but for SM2 we may need new struct.

I believe we need better documentation for asymmetric cryptography.
Arkadiusz Kusztal May 25, 2022, 6:33 a.m. UTC | #5 
> -----Original Message-----
> From: Akhil Goyal <gakhil@marvell.com>
> Sent: Wednesday, May 25, 2022 8:03 AM
> To: Kusztal, ArkadiuszX <arkadiuszx.kusztal@intel.com>; dev@dpdk.org
> Cc: Anoob Joseph <anoobj@marvell.com>; Zhang, Roy Fan
> <roy.fan.zhang@intel.com>
> Subject: RE: [EXT] [PATCH 20/40] cryptodev: add elliptic curve diffie hellman
> 
> > > > >  /**
> > > > > @@ -383,22 +385,28 @@ struct rte_crypto_dh_op_param {
> > > > >  	/**<
> > > > >  	 * Output generated private key when op_type is
> > > > >  	 * DH PRIVATE_KEY_GENERATION
> > > >
> > > > Update with actual enum name for this as well
> > > >
> > > > > -	 * Input when op_type is DH SHARED_SECRET_COMPUTATION.
> > > > > -	 *
> > > > > +	 * Input for
> RTE_CRYPTO_ASYM_KE_SHARED_SECRET_COMPUTE
> > > > >  	 */
> > > > > -	rte_crypto_uint pub_key;
> > > > > +	union {
> > > > > +		rte_crypto_uint pub_key;
> > > > > +		struct rte_crypto_ec_point pub_point;
> > > > > +	};
> > > > >  	/**<
> > > > > -	 * 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 public part
> > > > > +	 * For DH it is big-endian unsigned integer.
> > > > > +	 * For ECDH it is a point on the curve.
> > > > > +	 * Output for RTE_CRYPTO_ASYM_KE_PUBLIC_KEY_GENERATE
> > > > > +	 * Input for
> RTE_CRYPTO_ASYM_KE_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 op type is 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_KE_SHARED_SECRET_COMPUTE
> > > > >  	 */
> > > >
> > > > Is it output for others as well?
> > > [Arek] - No, it is output for phase2 only.
> > [Arek] - additionally there is still an option to split it into
> > DH_PARAM, ECDH_PARAM, x25519. For secp and Edwards/Montgomery it
> > should be ok with current approach but for SM2 we may need new struct.
> 
> I believe we need better documentation for asymmetric cryptography.
And we definitely need better tests, I have removed part of dh code from openssl pmd at random some time ago and all DH tests still passed!
I have added key exchange tests some time ago, this could be good start, I have v2 for ECDH too.
https://patchwork.dpdk.org/project/dpdk/list/?series=22401
Please take a look.

>
diff mbox series

Patch

diff --git a/lib/cryptodev/rte_crypto_asym.h b/lib/cryptodev/rte_crypto_asym.h
index 52cb1c5c80..09edf2ac3d 100644
--- a/lib/cryptodev/rte_crypto_asym.h
+++ b/lib/cryptodev/rte_crypto_asym.h
@@ -82,8 +82,10 @@  enum rte_crypto_asym_xform_type {
 	/**< Modular Exponentiation */
 	RTE_CRYPTO_ASYM_XFORM_ECDSA,
 	/**< Elliptic Curve Digital Signature Algorithm */
-	RTE_CRYPTO_ASYM_XFORM_ECPM
+	RTE_CRYPTO_ASYM_XFORM_ECPM,
 	/**< Elliptic Curve Point Multiplication */
+	RTE_CRYPTO_ASYM_XFORM_ECDH
+	/**< Elliptic Curve Diffie Hellman */
 };
 
 /**
@@ -383,22 +385,28 @@  struct rte_crypto_dh_op_param {
 	/**<
 	 * Output generated private key when op_type is
 	 * DH PRIVATE_KEY_GENERATION
-	 * Input when op_type is DH SHARED_SECRET_COMPUTATION.
-	 *
+	 * Input for RTE_CRYPTO_ASYM_KE_SHARED_SECRET_COMPUTE
 	 */
-	rte_crypto_uint pub_key;
+	union {
+		rte_crypto_uint pub_key;
+		struct rte_crypto_ec_point pub_point;
+	};
 	/**<
-	 * 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 public part
+	 * For DH it is big-endian unsigned integer.
+	 * For ECDH it is a point on the curve.
+	 * Output for RTE_CRYPTO_ASYM_KE_PUBLIC_KEY_GENERATE
+	 * Input for RTE_CRYPTO_ASYM_KE_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 op type is 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_KE_SHARED_SECRET_COMPUTE
 	 */
 };