[PATCH] cryptodev: add support for 25519 and 448 curves

[PATCH] cryptodev: add support for 25519 and 448 curves

[Arek Kusztal]

This commit adds support for following elliptic curves:
1) Curve25519
2) Curve448

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

diff --git a/lib/cryptodev/rte_crypto_asym.h b/lib/cryptodev/rte_crypto_asym.h
index cd24d4b07b..775b2f6277 100644
--- a/lib/cryptodev/rte_crypto_asym.h
+++ b/lib/cryptodev/rte_crypto_asym.h
@@ -48,6 +48,8 @@ enum rte_crypto_ec_group {
 	RTE_CRYPTO_EC_GROUP_SECP256R1 = 23,
 	RTE_CRYPTO_EC_GROUP_SECP384R1 = 24,
 	RTE_CRYPTO_EC_GROUP_SECP521R1 = 25,
+	RTE_CRYPTO_EC_GROUP_CURVE25519 = 29,
+	RTE_CRYPTO_EC_GROUP_CURVE448 = 30,
 };
 
 /**
@@ -180,9 +182,17 @@ typedef rte_crypto_param rte_crypto_uint;
  */
 struct rte_crypto_ec_point {
 	rte_crypto_param x;
-	/**< X coordinate */
+	/**<
+	 * X coordinate
+	 * For curve25519 and curve448 - little-endian integer
+	 * otherwise, big-endian integer
+	 */
 	rte_crypto_param y;
-	/**< Y coordinate */
+	/**<
+	 * Y coordinate
+	 * For curve25519 and curve448 - little-endian integer
+	 * otherwise, big-endian integer
+	 */
 };
 
 /**
-- 
2.13.6

RE: [EXT] [PATCH] cryptodev: add support for 25519 and 448 curves

[Akhil Goyal]

> This commit adds support for following elliptic curves:
> 1) Curve25519
> 2) Curve448
> 
> Signed-off-by: Arek Kusztal <arkadiuszx.kusztal@intel.com>
> ---
>  lib/cryptodev/rte_crypto_asym.h | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/cryptodev/rte_crypto_asym.h b/lib/cryptodev/rte_crypto_asym.h
> index cd24d4b07b..775b2f6277 100644
> --- a/lib/cryptodev/rte_crypto_asym.h
> +++ b/lib/cryptodev/rte_crypto_asym.h
> @@ -48,6 +48,8 @@ enum rte_crypto_ec_group {
>  	RTE_CRYPTO_EC_GROUP_SECP256R1 = 23,
>  	RTE_CRYPTO_EC_GROUP_SECP384R1 = 24,
>  	RTE_CRYPTO_EC_GROUP_SECP521R1 = 25,
> +	RTE_CRYPTO_EC_GROUP_CURVE25519 = 29,
> +	RTE_CRYPTO_EC_GROUP_CURVE448 = 30,
>  };
> 
>  /**
> @@ -180,9 +182,17 @@ typedef rte_crypto_param rte_crypto_uint;
>   */
>  struct rte_crypto_ec_point {
>  	rte_crypto_param x;
> -	/**< X coordinate */
> +	/**<
> +	 * X coordinate
> +	 * For curve25519 and curve448 - little-endian integer
> +	 * otherwise, big-endian integer
> +	 */
>  	rte_crypto_param y;
> -	/**< Y coordinate */
> +	/**<
> +	 * Y coordinate
> +	 * For curve25519 and curve448 - little-endian integer
> +	 * otherwise, big-endian integer
> +	 */
Can you give reference of the document which specify this endianness?

And if it is implicit as per the protocol, do we need to add explicit comments here?

RE: [EXT] [PATCH] cryptodev: add support for 25519 and 448 curves

[Kusztal, ArkadiuszX]

> -----Original Message-----
> From: Akhil Goyal <gakhil@marvell.com>
> Sent: Monday, May 16, 2022 8:58 PM
> To: Kusztal, ArkadiuszX <arkadiuszx.kusztal@intel.com>; dev@dpdk.org
> Cc: Zhang, Roy Fan <roy.fan.zhang@intel.com>
> Subject: RE: [EXT] [PATCH] cryptodev: add support for 25519 and 448 curves
> 
> > This commit adds support for following elliptic curves:
> > 1) Curve25519
> > 2) Curve448
> >
> > Signed-off-by: Arek Kusztal <arkadiuszx.kusztal@intel.com>
> > ---
> >  lib/cryptodev/rte_crypto_asym.h | 14 ++++++++++++--
> >  1 file changed, 12 insertions(+), 2 deletions(-)
> >
> > diff --git a/lib/cryptodev/rte_crypto_asym.h
> > b/lib/cryptodev/rte_crypto_asym.h index cd24d4b07b..775b2f6277 100644
> > --- a/lib/cryptodev/rte_crypto_asym.h
> > +++ b/lib/cryptodev/rte_crypto_asym.h
> > @@ -48,6 +48,8 @@ enum rte_crypto_ec_group {
> >  	RTE_CRYPTO_EC_GROUP_SECP256R1 = 23,
> >  	RTE_CRYPTO_EC_GROUP_SECP384R1 = 24,
> >  	RTE_CRYPTO_EC_GROUP_SECP521R1 = 25,
> > +	RTE_CRYPTO_EC_GROUP_CURVE25519 = 29,
> > +	RTE_CRYPTO_EC_GROUP_CURVE448 = 30,
> >  };
> >
> >  /**
> > @@ -180,9 +182,17 @@ typedef rte_crypto_param rte_crypto_uint;
> >   */
> >  struct rte_crypto_ec_point {
> >  	rte_crypto_param x;
> > -	/**< X coordinate */
> > +	/**<
> > +	 * X coordinate
> > +	 * For curve25519 and curve448 - little-endian integer
> > +	 * otherwise, big-endian integer
> > +	 */
> >  	rte_crypto_param y;
> > -	/**< Y coordinate */
> > +	/**<
> > +	 * Y coordinate
> > +	 * For curve25519 and curve448 - little-endian integer
> > +	 * otherwise, big-endian integer
> > +	 */
> Can you give reference of the document which specify this endianness?
[Arek] - sure, I may give rfc reference here, but if it will go into crypodev in this form I am not yet sure.
These curves could be used with DH, but cannot be used with ECDSA. Even with DH it may be that we will go with separate {dh_op, ecdh_op, x25519_op, x448_op} but this would make TLS group reference pointless, and we would not add Montgomery/Edwards curves at all as an enum.
> 
> And if it is implicit as per the protocol, do we need to add explicit comments
> here?

RE: [EXT] [PATCH] cryptodev: add support for 25519 and 448 curves

[Akhil Goyal]

> > > This commit adds support for following elliptic curves:
> > > 1) Curve25519
> > > 2) Curve448
> > >
> > > Signed-off-by: Arek Kusztal <arkadiuszx.kusztal@intel.com>
> > > ---
Acked-by: Akhil Goyal <gakhil@marvell.com>

Applied to dpdk-next-crypto

RE: [EXT] [PATCH] cryptodev: add support for 25519 and 448 curves

[Kusztal, ArkadiuszX]

Hi Akhil,

Sorry I have missed that, and I think we should revert this patch.
It would make sense to have TLS derived numbers for these curves if DH and ECDH would be in the same op.
But since we decided to split it we are going to go with separate structs for x448 and x25519 as per:
https://patchwork.dpdk.org/project/dpdk/patch/20220531040439.15862-7-arkadiuszx.kusztal@intel.com/


> -----Original Message-----
> From: Akhil Goyal <gakhil@marvell.com>
> Sent: Thursday, May 26, 2022 6:38 PM
> To: Kusztal, ArkadiuszX <arkadiuszx.kusztal@intel.com>; dev@dpdk.org
> Cc: Zhang, Roy Fan <roy.fan.zhang@intel.com>
> Subject: RE: [EXT] [PATCH] cryptodev: add support for 25519 and 448 curves
> 
> > > > This commit adds support for following elliptic curves:
> > > > 1) Curve25519
> > > > 2) Curve448
> > > >
> > > > Signed-off-by: Arek Kusztal <arkadiuszx.kusztal@intel.com>
> > > > ---
> Acked-by: Akhil Goyal <gakhil@marvell.com>
> 
> Applied to dpdk-next-crypto

RE: [EXT] [PATCH] cryptodev: add support for 25519 and 448 curves

[Akhil Goyal]

Hi Arek,
> Hi Akhil,
> 
> Sorry I have missed that, and I think we should revert this patch.
> It would make sense to have TLS derived numbers for these curves if DH and
> ECDH would be in the same op.
> But since we decided to split it we are going to go with separate structs for x448
> and x25519 as per:
> https://patchwork.dpdk.org/project/dpdk/patch/20220531040439.15862-7-arkadiuszx.kusztal@intel.com/

I have asked you previously also to update patchworks with superseded
When you are sending new versions or if the patch is not valid.
I will remove this patch from the tree, since the patch is not pulled in main.
But please take care in future.

RE: [EXT] [PATCH] cryptodev: add support for 25519 and 448 curves

[Kusztal, ArkadiuszX]

> -----Original Message-----
> From: Akhil Goyal <gakhil@marvell.com>
> Sent: Tuesday, May 31, 2022 4:40 PM
> To: Kusztal, ArkadiuszX <arkadiuszx.kusztal@intel.com>; dev@dpdk.org
> Cc: Zhang, Roy Fan <roy.fan.zhang@intel.com>
> Subject: RE: [EXT] [PATCH] cryptodev: add support for 25519 and 448 curves
> 
> Hi Arek,
> > Hi Akhil,
> >
> > Sorry I have missed that, and I think we should revert this patch.
> > It would make sense to have TLS derived numbers for these curves if DH
> > and ECDH would be in the same op.
> > But since we decided to split it we are going to go with separate
> > structs for x448 and x25519 as per:
> > https://patchwork.dpdk.org/project/dpdk/patch/20220531040439.15862-7-a
> > rkadiuszx.kusztal@intel.com/
> 
> I have asked you previously also to update patchworks with superseded When
> you are sending new versions or if the patch is not valid.
> I will remove this patch from the tree, since the patch is not pulled in main.
> But please take care in future.
> 
Thank you! 

RE: [EXT] [PATCH] cryptodev: add support for 25519 and 448 curves

[Akhil Goyal]

> -----Original Message-----
> From: Kusztal, ArkadiuszX <arkadiuszx.kusztal@intel.com>
> Sent: Tuesday, May 31, 2022 8:03 PM
> To: Akhil Goyal <gakhil@marvell.com>; dev@dpdk.org
> Cc: Zhang, Roy Fan <roy.fan.zhang@intel.com>
> Subject: RE: [EXT] [PATCH] cryptodev: add support for 25519 and 448 curves
> 
> Hi Akhil,
> 
> Sorry I have missed that, and I think we should revert this patch.
> It would make sense to have TLS derived numbers for these curves if DH and
> ECDH would be in the same op.
> But since we decided to split it we are going to go with separate structs for x448
> and x25519 as per:

Even after removing this patch, the patches are not getting applied.
Applying: cryptodev: move RSA padding into separate struct
error: patch failed: drivers/crypto/qat/qat_asym.c:345
error: drivers/crypto/qat/qat_asym.c: patch does not apply
Patch failed at 0010 cryptodev: move RSA padding into separate struct
hint: Use 'git am --show-current-patch' to see the failed patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

RE: [EXT] [PATCH] cryptodev: add support for 25519 and 448 curves

[Kusztal, ArkadiuszX]

> -----Original Message-----
> From: Akhil Goyal <gakhil@marvell.com>
> Sent: Tuesday, May 31, 2022 5:26 PM
> To: Kusztal, ArkadiuszX <arkadiuszx.kusztal@intel.com>; dev@dpdk.org
> Cc: Zhang, Roy Fan <roy.fan.zhang@intel.com>
> Subject: RE: [EXT] [PATCH] cryptodev: add support for 25519 and 448 curves
> 
> 
> 
> > -----Original Message-----
> > From: Kusztal, ArkadiuszX <arkadiuszx.kusztal@intel.com>
> > Sent: Tuesday, May 31, 2022 8:03 PM
> > To: Akhil Goyal <gakhil@marvell.com>; dev@dpdk.org
> > Cc: Zhang, Roy Fan <roy.fan.zhang@intel.com>
> > Subject: RE: [EXT] [PATCH] cryptodev: add support for 25519 and 448
> > curves
> >
> > Hi Akhil,
> >
> > Sorry I have missed that, and I think we should revert this patch.
> > It would make sense to have TLS derived numbers for these curves if DH
> > and ECDH would be in the same op.
> > But since we decided to split it we are going to go with separate
> > structs for x448 and x25519 as per:
> 
> Even after removing this patch, the patches are not getting applied.
> Applying: cryptodev: move RSA padding into separate struct
> error: patch failed: drivers/crypto/qat/qat_asym.c:345
> error: drivers/crypto/qat/qat_asym.c: patch does not apply Patch failed at 0010
> cryptodev: move RSA padding into separate struct
> hint: Use 'git am --show-current-patch' to see the failed patch When you have
> resolved this problem, run "git am --continue".
> If you prefer to skip this patch, run "git am --skip" instead.
> To restore the original branch and stop patching, run "git am --abort".

Yes, it needs to be rebased against:
crypto/qat: refactor asym algorithm macros and logs
I will do it in v5.