cryptodev: add support for 25519 and 448 curves
Checks
Commit Message
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(-)
Comments
> 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?
> -----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?
> > > 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
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
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.
> -----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!
> -----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".
> -----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.
@@ -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
+ */
};
/**