diff mbox series

[21/40] cryptodev: add public key verify option

Message ID	20220520055445.40063-22-arkadiuszx.kusztal@intel.com (mailing list archive)
State	Changes Requested, archived
Delegated to:	akhil goyal
Headers	From: Arek Kusztal <arkadiuszx.kusztal@intel.com> To: dev@dpdk.org Cc: gakhil@marvell.com, anoobj@marvell.com, roy.fan.zhang@intel.com, Arek Kusztal <arkadiuszx.kusztal@intel.com> Subject: [PATCH 21/40] cryptodev: add public key verify option Date: Fri, 20 May 2022 06:54:26 +0100 Message-Id: <20220520055445.40063-22-arkadiuszx.kusztal@intel.com> In-Reply-To: <20220520055445.40063-1-arkadiuszx.kusztal@intel.com> References: <20220520055445.40063-1-arkadiuszx.kusztal@intel.com> Precedence: list Errors-To: dev-bounces@dpdk.org
Series	cryptodev: rsa, dh, ecdh changes \| [00/40] cryptodev: rsa, dh, ecdh changes [01/40] cryptodev: redefine ec group enum [02/40] cryptodev: remove list end enumerators [03/40] test/crypto: remove list end enumerators [04/40] cryptodev: reduce number of comments in asym xform [05/40] test/crypto: removed asym xform none [06/40] cryptodev: separate key exchange operation enum [07/40] crypto/openssl: separate key exchange operation enum [08/40] test/crypto: separate key exchange operation enum [09/40] cryptodev: remove unnecessary zero assignement [10/40] cryptodev: remove comment about using ephemeral key in dsa [11/40] cryptodev: remove asym crypto next xform [12/40] crypto/openssl: remove asym crypto next xform [13/40] test/crypto: remove asym crypto next xform [14/40] app/test-crypto-perf: remove asym crypto next xform [15/40] app/test-eventdev: remove asym crypto next xform [16/40] cryptodev: move dh type from xform to dh op [17/40] crypto/openssl: move dh type from xform to dh op [18/40] test/crypto: move dh type from xform to dh op [19/40] cryptodev: changed order of dh fields [20/40] cryptodev: add elliptic curve diffie hellman [21/40] cryptodev: add public key verify option [22/40] cryptodev: move RSA padding into separate struct [23/40] crypto/qat: move RSA padding into separate struct [24/40] crypto/openssl: move RSA padding into separate struct [25/40] crypto/octeontx: move RSA padding into separate struct [26/40] crypto/cnxk: move RSA padding into separate struct [27/40] common/cpt: move RSA padding into separate struct [28/40] test/crypto: move RSA padding into separate struct [29/40] cryptodev: add salt length and optional label [30/40] cryptodev: reduce rsa struct to only necessary fields [31/40] crypto/qat: reduce rsa struct to only necessary fields [32/40] crypto/openssl: reduce rsa struct to only necessary fields [33/40] crypto/octeontx: reduce rsa struct to only necessary fields [34/40] crypto/cnxk: reduce rsa struct to only necessary fields [35/40] common/cpt: reduce rsa struct to only necessary fields [36/40] test/crypto: reduce rsa struct to only necessary fields [37/40] cryptodev: add asym op flags [38/40] cryptodev: clarify usage of private key in dh [39/40] crypto/openssl: generate dh private key [40/40] test/crypto: added test for dh priv key generation

Checks

Context	Check	Description
ci/checkpatch	warning	coding style issues

Commit Message

Arkadiusz Kusztal May 20, 2022, 5:54 a.m. UTC

  - Added key exchange public key verify option.
For some elliptic curves public point in DH exchange
needs to be checked, if it lays on the curve.
Modular exponentiation needs certain checks as well, though
mathematically much easier.
This commit adds verify option to asym_op operations.

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

Comments

Akhil Goyal May 24, 2022, 12:12 p.m. UTC | #1

> - Added key exchange public key verify option.
> For some elliptic curves public point in DH exchange
> needs to be checked, if it lays on the curve.
> Modular exponentiation needs certain checks as well, though
> mathematically much easier.
> This commit adds verify option to asym_op operations.
> 
> Signed-off-by: Arek Kusztal <arkadiuszx.kusztal@intel.com>
> ---
>  lib/cryptodev/rte_crypto_asym.h | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/cryptodev/rte_crypto_asym.h b/lib/cryptodev/rte_crypto_asym.h
> index 09edf2ac3d..73ff9ff815 100644
> --- a/lib/cryptodev/rte_crypto_asym.h
> +++ b/lib/cryptodev/rte_crypto_asym.h
> @@ -110,8 +110,10 @@ enum rte_crypto_asym_ke_type {
>  	/**< Private Key generation operation */
>  	RTE_CRYPTO_ASYM_KE_PUBLIC_KEY_GENERATE,
>  	/**< Public Key generation operation */
> -	RTE_CRYPTO_ASYM_KE_SHARED_SECRET_COMPUTE
> +	RTE_CRYPTO_ASYM_KE_SHARED_SECRET_COMPUTE,
>  	/**< Shared Secret compute operation */
> +	RTE_CRYPTO_ASYM_OP_DH_KEY_VERIFY
> +	/**< Public Key Verification */

Shouldn't this be RTE_CRYPTO_ASYM_KE_PUBLIC_KEY_VERIFY ??

>  };
> 
>  /**
> @@ -397,6 +399,10 @@ struct rte_crypto_dh_op_param {
>  	 * 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
> +	 *
> +	 * VERIFY option can be used only for elliptic curve
> +	 * point validation, for FFDH (DH) it is user's reponsibility
> +	 * to check the public key accordingly.

What is expected from the user? Please be specific.
Add reference to the ke_type for which this comment id valid.

>  	 */
>  	union {
>  		rte_crypto_uint shared_secret;
> --
> 2.13.6

Arkadiusz Kusztal May 24, 2022, 2:59 p.m. UTC | #2

> -----Original Message-----
> From: Akhil Goyal <gakhil@marvell.com>
> Sent: Tuesday, May 24, 2022 2:12 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 21/40] cryptodev: add public key verify option
> 
> > - Added key exchange public key verify option.
> > For some elliptic curves public point in DH exchange needs to be
> > checked, if it lays on the curve.
> > Modular exponentiation needs certain checks as well, though
> > mathematically much easier.
> > This commit adds verify option to asym_op operations.
> >
> > Signed-off-by: Arek Kusztal <arkadiuszx.kusztal@intel.com>
> > ---
> >  lib/cryptodev/rte_crypto_asym.h | 8 +++++++-
> >  1 file changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/lib/cryptodev/rte_crypto_asym.h
> > b/lib/cryptodev/rte_crypto_asym.h index 09edf2ac3d..73ff9ff815 100644
> > --- a/lib/cryptodev/rte_crypto_asym.h
> > +++ b/lib/cryptodev/rte_crypto_asym.h
> > @@ -110,8 +110,10 @@ enum rte_crypto_asym_ke_type {
> >  	/**< Private Key generation operation */
> >  	RTE_CRYPTO_ASYM_KE_PUBLIC_KEY_GENERATE,
> >  	/**< Public Key generation operation */
> > -	RTE_CRYPTO_ASYM_KE_SHARED_SECRET_COMPUTE
> > +	RTE_CRYPTO_ASYM_KE_SHARED_SECRET_COMPUTE,
> >  	/**< Shared Secret compute operation */
> > +	RTE_CRYPTO_ASYM_OP_DH_KEY_VERIFY
> > +	/**< Public Key Verification */
> 
> Shouldn't this be RTE_CRYPTO_ASYM_KE_PUBLIC_KEY_VERIFY ??
[Arek] - ah, yes. Sorry for that.
> 
> >  };
> >
> >  /**
> > @@ -397,6 +399,10 @@ struct rte_crypto_dh_op_param {
> >  	 * 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
> > +	 *
> > +	 * VERIFY option can be used only for elliptic curve
> > +	 * point validation, for FFDH (DH) it is user's reponsibility
> > +	 * to check the public key accordingly.
> 
> What is expected from the user? Please be specific.
[Arek] - Well, this depends on protocol, usually it is size check for FFDH and zero check for x25519/448. Not sure how much information should be provided here.
> Add reference to the ke_type for which this comment id valid.

> 
> >  	 */
> >  	union {
> >  		rte_crypto_uint shared_secret;
> > --
> > 2.13.6

Akhil Goyal May 25, 2022, 6 a.m. UTC | #3

> > >
> > >  /**
> > > @@ -397,6 +399,10 @@ struct rte_crypto_dh_op_param {
> > >  	 * 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
> > > +	 *
> > > +	 * VERIFY option can be used only for elliptic curve
> > > +	 * point validation, for FFDH (DH) it is user's reponsibility
> > > +	 * to check the public key accordingly.
> >
> > What is expected from the user? Please be specific.
> [Arek] - Well, this depends on protocol, usually it is size check for FFDH and zero
> check for x25519/448. Not sure how much information should be provided here.
I can understand we cannot specify everything here.
How about adding in documentation a section to give details?

> > Add reference to the ke_type for which this comment id valid.
> 
> >
> > >  	 */
> > >  	union {
> > >  		rte_crypto_uint shared_secret;
> > > --
> > > 2.13.6

Arkadiusz Kusztal May 25, 2022, 6:30 a.m. UTC | #4

> -----Original Message-----
> From: Akhil Goyal <gakhil@marvell.com>
> Sent: Wednesday, May 25, 2022 8:00 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 21/40] cryptodev: add public key verify option
> 
> > > >
> > > >  /**
> > > > @@ -397,6 +399,10 @@ struct rte_crypto_dh_op_param {
> > > >  	 * 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
> > > > +	 *
> > > > +	 * VERIFY option can be used only for elliptic curve
> > > > +	 * point validation, for FFDH (DH) it is user's reponsibility
> > > > +	 * to check the public key accordingly.
> > >
> > > What is expected from the user? Please be specific.
> > [Arek] - Well, this depends on protocol, usually it is size check for
> > FFDH and zero check for x25519/448. Not sure how much information should
> be provided here.
> I can understand we cannot specify everything here.
> How about adding in documentation a section to give details?
[Arek] - actually I came with another idea.
I have added ECDH param, VERIFY is valid only for ecdh_op and verify is:
RTE_CRYPTO_ASYM_KE_EC_PUBLIC_KEY_VERIFY, so it is not included in dh_op_param.
I doubt there will be any hw acceleration for anything else than EC point verification + I doubt there will be 4 steps verification (that's why I have not included it in flags). I have spilt DH into DH and ECDH, as it probably will be easier when adding sm2 key exchange (instead of extending union fields)

+ Kai
> 
> > > Add reference to the ke_type for which this comment id valid.
> >
> > >
> > > >  	 */
> > > >  	union {
> > > >  		rte_crypto_uint shared_secret;
> > > > --
> > > > 2.13.6

diff mbox series

Patch

diff --git a/lib/cryptodev/rte_crypto_asym.h b/lib/cryptodev/rte_crypto_asym.h
index 09edf2ac3d..73ff9ff815 100644
--- a/lib/cryptodev/rte_crypto_asym.h
+++ b/lib/cryptodev/rte_crypto_asym.h
@@ -110,8 +110,10 @@  enum rte_crypto_asym_ke_type {
 	/**< Private Key generation operation */
 	RTE_CRYPTO_ASYM_KE_PUBLIC_KEY_GENERATE,
 	/**< Public Key generation operation */
-	RTE_CRYPTO_ASYM_KE_SHARED_SECRET_COMPUTE
+	RTE_CRYPTO_ASYM_KE_SHARED_SECRET_COMPUTE,
 	/**< Shared Secret compute operation */
+	RTE_CRYPTO_ASYM_OP_DH_KEY_VERIFY
+	/**< Public Key Verification */
 };
 
 /**
@@ -397,6 +399,10 @@  struct rte_crypto_dh_op_param {
 	 * 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
+	 *
+	 * VERIFY option can be used only for elliptic curve
+	 * point validation, for FFDH (DH) it is user's reponsibility
+	 * to check the public key accordingly.
 	 */
 	union {
 		rte_crypto_uint shared_secret;