[v4,02/12] cryptodev: separate key exchange operation enum
Checks
Commit Message
- Separated key exchange enum from asym op type.
Key exchange and asymmetric crypto operations like signatures,
encryption/decryption should not share same operation enum as
its use cases are unrelated and mutually exclusive.
Therefore op_type was separate into:
1) operation type
2) key exchange operation type
Signed-off-by: Arek Kusztal <arkadiuszx.kusztal@intel.com>
---
app/test/test_cryptodev_asym.c | 52 +++++++++++++++-------------
drivers/crypto/openssl/rte_openssl_pmd.c | 10 +++---
drivers/crypto/openssl/rte_openssl_pmd_ops.c | 18 +++++-----
lib/cryptodev/rte_crypto_asym.h | 45 +++++++++++++++---------
lib/cryptodev/rte_cryptodev.c | 14 +++++---
lib/cryptodev/rte_cryptodev.h | 4 ++-
lib/cryptodev/version.map | 1 +
7 files changed, 84 insertions(+), 60 deletions(-)
Comments
> -----Original Message-----
> From: Kusztal, ArkadiuszX <arkadiuszx.kusztal@intel.com>
> Sent: Tuesday, May 31, 2022 5:04 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 v4 02/12] cryptodev: separate key exchange operation
> enum
>
> - Separated key exchange enum from asym op type.
> Key exchange and asymmetric crypto operations like signatures,
> encryption/decryption should not share same operation enum as
> its use cases are unrelated and mutually exclusive.
> Therefore op_type was separate into:
> 1) operation type
> 2) key exchange operation type
>
> Signed-off-by: Arek Kusztal <arkadiuszx.kusztal@intel.com>
> ---
Acked-by: Fan Zhang <roy.fan.zhang@intel.com>
> /**
> diff --git a/lib/cryptodev/rte_cryptodev.h b/lib/cryptodev/rte_cryptodev.h
> index 2c2c2edeb7..7d683fd728 100644
> --- a/lib/cryptodev/rte_cryptodev.h
> +++ b/lib/cryptodev/rte_cryptodev.h
> @@ -168,7 +168,9 @@ struct rte_cryptodev_asymmetric_xform_capability {
> /**< Transform type: RSA/MODEXP/DH/DSA/MODINV */
>
> uint32_t op_types;
> - /**< bitmask for supported rte_crypto_asym_op_type */
> + /**< bitmask for supported rte_crypto_asym_op_type or
> + * rte_crypto_asym_ke_type
> + */
How is this supposed to work?
Ke_type and op_type are 2 separate enums which can have same value.
How will the user identify which one to use?
Shouldn't we split this too?
>
> __extension__
> union {
> diff --git a/lib/cryptodev/version.map b/lib/cryptodev/version.map
> index f0abfaa47d..dbf1f62199 100644
> --- a/lib/cryptodev/version.map
> +++ b/lib/cryptodev/version.map
> @@ -108,6 +108,7 @@ EXPERIMENTAL {
>
> #added in 22.07
> rte_cryptodev_session_event_mdata_set;
> + rte_crypto_asym_ke_strings;
> };
>
> INTERNAL {
> --
> 2.13.6
> -----Original Message-----
> From: Akhil Goyal <gakhil@marvell.com>
> Sent: Tuesday, May 31, 2022 1:47 PM
> To: Kusztal, ArkadiuszX <arkadiuszx.kusztal@intel.com>; dev@dpdk.org
> Cc: Zhang, Roy Fan <roy.fan.zhang@intel.com>
> Subject: RE: [EXT] [PATCH v4 02/12] cryptodev: separate key exchange
> operation enum
>
> > /**
> > diff --git a/lib/cryptodev/rte_cryptodev.h
> > b/lib/cryptodev/rte_cryptodev.h index 2c2c2edeb7..7d683fd728 100644
> > --- a/lib/cryptodev/rte_cryptodev.h
> > +++ b/lib/cryptodev/rte_cryptodev.h
> > @@ -168,7 +168,9 @@ struct rte_cryptodev_asymmetric_xform_capability {
> > /**< Transform type: RSA/MODEXP/DH/DSA/MODINV */
> >
> > uint32_t op_types;
> > - /**< bitmask for supported rte_crypto_asym_op_type */
> > + /**< bitmask for supported rte_crypto_asym_op_type or
> > + * rte_crypto_asym_ke_type
> > + */
>
> How is this supposed to work?
> Ke_type and op_type are 2 separate enums which can have same value.
> How will the user identify which one to use?
[Arek] - by algorithm - xform_type.
> Shouldn't we split this too?
[Arek] - for me both options are ok. For some SM2 may be bit challenging here, but in such situations we should have different op types for KE and OP. This would spare this API from having one op with majority of fields unused.
Though it may be split too, not big problem.
>
> >
> > __extension__
> > union {
> > diff --git a/lib/cryptodev/version.map b/lib/cryptodev/version.map
> > index f0abfaa47d..dbf1f62199 100644
> > --- a/lib/cryptodev/version.map
> > +++ b/lib/cryptodev/version.map
> > @@ -108,6 +108,7 @@ EXPERIMENTAL {
> >
> > #added in 22.07
> > rte_cryptodev_session_event_mdata_set;
> > + rte_crypto_asym_ke_strings;
> > };
> >
> > INTERNAL {
> > --
> > 2.13.6
> -----Original Message-----
> From: Kusztal, ArkadiuszX
> Sent: Tuesday, May 31, 2022 3:50 PM
> To: Akhil Goyal <gakhil@marvell.com>; dev@dpdk.org
> Cc: Zhang, Roy Fan <roy.fan.zhang@intel.com>
> Subject: RE: [EXT] [PATCH v4 02/12] cryptodev: separate key exchange
> operation enum
>
>
>
> > -----Original Message-----
> > From: Akhil Goyal <gakhil@marvell.com>
> > Sent: Tuesday, May 31, 2022 1:47 PM
> > To: Kusztal, ArkadiuszX <arkadiuszx.kusztal@intel.com>; dev@dpdk.org
> > Cc: Zhang, Roy Fan <roy.fan.zhang@intel.com>
> > Subject: RE: [EXT] [PATCH v4 02/12] cryptodev: separate key exchange
> > operation enum
> >
> > > /**
> > > diff --git a/lib/cryptodev/rte_cryptodev.h
> > > b/lib/cryptodev/rte_cryptodev.h index 2c2c2edeb7..7d683fd728 100644
> > > --- a/lib/cryptodev/rte_cryptodev.h
> > > +++ b/lib/cryptodev/rte_cryptodev.h
> > > @@ -168,7 +168,9 @@ struct rte_cryptodev_asymmetric_xform_capability {
> > > /**< Transform type: RSA/MODEXP/DH/DSA/MODINV */
> > >
> > > uint32_t op_types;
> > > - /**< bitmask for supported rte_crypto_asym_op_type */
> > > + /**< bitmask for supported rte_crypto_asym_op_type or
> > > + * rte_crypto_asym_ke_type
> > > + */
> >
> > How is this supposed to work?
> > Ke_type and op_type are 2 separate enums which can have same value.
> > How will the user identify which one to use?
> [Arek] - by algorithm - xform_type.
> > Shouldn't we split this too?
> [Arek] - for me both options are ok. For some SM2 may be bit challenging here,
[Arek] - wanted to say here that I prefer not to split it.
> but in such situations we should have different op types for KE and OP. This
> would spare this API from having one op with majority of fields unused.
> Though it may be split too, not big problem.
>
> >
> > >
> > > __extension__
> > > union {
> > > diff --git a/lib/cryptodev/version.map b/lib/cryptodev/version.map
> > > index f0abfaa47d..dbf1f62199 100644
> > > --- a/lib/cryptodev/version.map
> > > +++ b/lib/cryptodev/version.map
> > > @@ -108,6 +108,7 @@ EXPERIMENTAL {
> > >
> > > #added in 22.07
> > > rte_cryptodev_session_event_mdata_set;
> > > + rte_crypto_asym_ke_strings;
> > > };
> > >
> > > INTERNAL {
> > > --
> > > 2.13.6
> > > /**
> > > diff --git a/lib/cryptodev/rte_cryptodev.h
> > > b/lib/cryptodev/rte_cryptodev.h index 2c2c2edeb7..7d683fd728 100644
> > > --- a/lib/cryptodev/rte_cryptodev.h
> > > +++ b/lib/cryptodev/rte_cryptodev.h
> > > @@ -168,7 +168,9 @@ struct rte_cryptodev_asymmetric_xform_capability {
> > > /**< Transform type: RSA/MODEXP/DH/DSA/MODINV */
> > >
> > > uint32_t op_types;
> > > - /**< bitmask for supported rte_crypto_asym_op_type */
> > > + /**< bitmask for supported rte_crypto_asym_op_type or
> > > + * rte_crypto_asym_ke_type
> > > + */
> >
> > How is this supposed to work?
> > Ke_type and op_type are 2 separate enums which can have same value.
> > How will the user identify which one to use?
> [Arek] - by algorithm - xform_type.
Ok got it. But please update the comments to clarify this confusion that this
Based on xform type
> > Shouldn't we split this too?
> [Arek] - for me both options are ok. For some SM2 may be bit challenging here,
> but in such situations we should have different op types for KE and OP. This
> would spare this API from having one op with majority of fields unused.
> Though it may be split too, not big problem.
@@ -976,27 +976,30 @@ static inline void print_asym_capa(
for (i = 0; i < RTE_CRYPTO_ASYM_OP_LIST_END; i++) {
/* check supported operations */
- if (rte_cryptodev_asym_xform_capability_check_optype(capa, i))
- printf(" %s",
- rte_crypto_asym_op_strings[i]);
+ if (rte_cryptodev_asym_xform_capability_check_optype(capa, i)) {
+ if (capa->xform_type == RTE_CRYPTO_ASYM_XFORM_DH)
+ printf(" %s", rte_crypto_asym_ke_strings[i]);
+ else
+ printf(" %s", rte_crypto_asym_op_strings[i]);
}
- switch (capa->xform_type) {
- case RTE_CRYPTO_ASYM_XFORM_RSA:
- case RTE_CRYPTO_ASYM_XFORM_MODINV:
- case RTE_CRYPTO_ASYM_XFORM_MODEX:
- case RTE_CRYPTO_ASYM_XFORM_DH:
- case RTE_CRYPTO_ASYM_XFORM_DSA:
- printf(" modlen: min %d max %d increment %d",
- capa->modlen.min,
- capa->modlen.max,
- capa->modlen.increment);
+ }
+ switch (capa->xform_type) {
+ case RTE_CRYPTO_ASYM_XFORM_RSA:
+ case RTE_CRYPTO_ASYM_XFORM_MODINV:
+ case RTE_CRYPTO_ASYM_XFORM_MODEX:
+ case RTE_CRYPTO_ASYM_XFORM_DH:
+ case RTE_CRYPTO_ASYM_XFORM_DSA:
+ printf(" modlen: min %d max %d increment %d",
+ capa->modlen.min,
+ capa->modlen.max,
+ capa->modlen.increment);
+ break;
+ case RTE_CRYPTO_ASYM_XFORM_ECDSA:
+ case RTE_CRYPTO_ASYM_XFORM_ECPM:
+ default:
break;
- case RTE_CRYPTO_ASYM_XFORM_ECDSA:
- case RTE_CRYPTO_ASYM_XFORM_ECPM:
- default:
- break;
- }
- printf("\n");
+ }
+ printf("\n");
}
static int
@@ -1064,7 +1067,7 @@ test_dh_gen_shared_sec(struct rte_crypto_asym_xform *xfrm)
asym_op = op->asym;
/* Setup a xform and op to generate private key only */
- xform.dh.type = RTE_CRYPTO_ASYM_OP_SHARED_SECRET_COMPUTE;
+ xform.dh.ke_type = RTE_CRYPTO_ASYM_KE_SHARED_SECRET_COMPUTE;
xform.next = NULL;
asym_op->dh.priv_key.data = dh_test_params.priv_key.data;
asym_op->dh.priv_key.length = dh_test_params.priv_key.length;
@@ -1146,7 +1149,7 @@ test_dh_gen_priv_key(struct rte_crypto_asym_xform *xfrm)
asym_op = op->asym;
/* Setup a xform and op to generate private key only */
- xform.dh.type = RTE_CRYPTO_ASYM_OP_PRIVATE_KEY_GENERATE;
+ xform.dh.ke_type = RTE_CRYPTO_ASYM_KE_PRIV_KEY_GENERATE;
xform.next = NULL;
asym_op->dh.priv_key.data = output;
asym_op->dh.priv_key.length = sizeof(output);
@@ -1229,7 +1232,7 @@ test_dh_gen_pub_key(struct rte_crypto_asym_xform *xfrm)
* using test private key
*
*/
- xform.dh.type = RTE_CRYPTO_ASYM_OP_PUBLIC_KEY_GENERATE;
+ xform.dh.ke_type = RTE_CRYPTO_ASYM_KE_PUB_KEY_GENERATE;
xform.next = NULL;
asym_op->dh.pub_key.data = output;
@@ -1319,9 +1322,10 @@ test_dh_gen_kp(struct rte_crypto_asym_xform *xfrm)
/* Setup a xform chain to generate
* private key first followed by
* public key
- */xform.dh.type = RTE_CRYPTO_ASYM_OP_PRIVATE_KEY_GENERATE;
+ */
+ xform.dh.ke_type = RTE_CRYPTO_ASYM_KE_PRIV_KEY_GENERATE;
pub_key_xform.xform_type = RTE_CRYPTO_ASYM_XFORM_DH;
- pub_key_xform.dh.type = RTE_CRYPTO_ASYM_OP_PUBLIC_KEY_GENERATE;
+ pub_key_xform.dh.ke_type = RTE_CRYPTO_ASYM_KE_PUB_KEY_GENERATE;
xform.next = &pub_key_xform;
asym_op->dh.pub_key.data = out_pub_key;
@@ -1697,7 +1697,7 @@ process_openssl_dh_op(struct rte_crypto_op *cop,
int ret = 0;
if (sess->u.dh.key_op &
- (1 << RTE_CRYPTO_ASYM_OP_SHARED_SECRET_COMPUTE)) {
+ (1 << RTE_CRYPTO_ASYM_KE_SHARED_SECRET_COMPUTE)) {
/* compute shared secret using peer public key
* and current private key
* shared secret = peer_key ^ priv_key mod p
@@ -1754,9 +1754,9 @@ process_openssl_dh_op(struct rte_crypto_op *cop,
* then first set DH with user provided private key
*/
if ((sess->u.dh.key_op &
- (1 << RTE_CRYPTO_ASYM_OP_PUBLIC_KEY_GENERATE)) &&
+ (1 << RTE_CRYPTO_ASYM_KE_PUB_KEY_GENERATE)) &&
!(sess->u.dh.key_op &
- (1 << RTE_CRYPTO_ASYM_OP_PRIVATE_KEY_GENERATE))) {
+ (1 << RTE_CRYPTO_ASYM_KE_PRIV_KEY_GENERATE))) {
/* generate public key using user-provided private key
* pub_key = g ^ priv_key mod p
*/
@@ -1790,7 +1790,7 @@ process_openssl_dh_op(struct rte_crypto_op *cop,
return 0;
}
- if (sess->u.dh.key_op & (1 << RTE_CRYPTO_ASYM_OP_PUBLIC_KEY_GENERATE)) {
+ if (sess->u.dh.key_op & (1 << RTE_CRYPTO_ASYM_KE_PUB_KEY_GENERATE)) {
const BIGNUM *pub_key = NULL;
OPENSSL_LOG(DEBUG, "%s:%d update public key\n",
@@ -1805,7 +1805,7 @@ process_openssl_dh_op(struct rte_crypto_op *cop,
}
if (sess->u.dh.key_op &
- (1 << RTE_CRYPTO_ASYM_OP_PRIVATE_KEY_GENERATE)) {
+ (1 << RTE_CRYPTO_ASYM_KE_PRIV_KEY_GENERATE)) {
const BIGNUM *priv_key = NULL;
OPENSSL_LOG(DEBUG, "%s:%d updated priv key\n",
@@ -533,10 +533,10 @@ static const struct rte_cryptodev_capabilities openssl_pmd_capabilities[] = {
.xform_capa = {
.xform_type = RTE_CRYPTO_ASYM_XFORM_DH,
.op_types =
- ((1<<RTE_CRYPTO_ASYM_OP_PRIVATE_KEY_GENERATE) |
- (1 << RTE_CRYPTO_ASYM_OP_PUBLIC_KEY_GENERATE |
+ ((1<<RTE_CRYPTO_ASYM_KE_PRIV_KEY_GENERATE) |
+ (1 << RTE_CRYPTO_ASYM_KE_PUB_KEY_GENERATE |
(1 <<
- RTE_CRYPTO_ASYM_OP_SHARED_SECRET_COMPUTE))),
+ RTE_CRYPTO_ASYM_KE_SHARED_SECRET_COMPUTE))),
{
.modlen = {
/* value 0 symbolizes no limit on min length */
@@ -1006,16 +1006,16 @@ static int openssl_set_asym_session_parameters(
* DH Priv key generate, or both
* public and private key generate
*/
- asym_session->u.dh.key_op = (1 << xform->dh.type);
+ asym_session->u.dh.key_op = (1 << xform->dh.ke_type);
- if (xform->dh.type ==
- RTE_CRYPTO_ASYM_OP_PRIVATE_KEY_GENERATE) {
+ if (xform->dh.ke_type ==
+ RTE_CRYPTO_ASYM_KE_PRIV_KEY_GENERATE) {
/* check if next is pubkey */
if ((xform->next != NULL) &&
(xform->next->xform_type ==
RTE_CRYPTO_ASYM_XFORM_DH) &&
- (xform->next->dh.type ==
- RTE_CRYPTO_ASYM_OP_PUBLIC_KEY_GENERATE)
+ (xform->next->dh.ke_type ==
+ RTE_CRYPTO_ASYM_KE_PUB_KEY_GENERATE)
) {
/*
* setup op as pub/priv key
@@ -1023,7 +1023,7 @@ static int openssl_set_asym_session_parameters(
*/
asym_session->u.dh.key_op |=
(1 <<
- RTE_CRYPTO_ASYM_OP_PUBLIC_KEY_GENERATE);
+ RTE_CRYPTO_ASYM_KE_PUB_KEY_GENERATE);
}
}
asym_session->u.dh.dh_key = dh;
@@ -33,6 +33,10 @@ struct rte_cryptodev_asym_session;
extern const char *
rte_crypto_asym_xform_strings[];
+/** asym key exchange operation type name strings */
+extern const char *
+rte_crypto_asym_ke_strings[];
+
/** asym operations type name strings */
extern const char *
rte_crypto_asym_op_strings[];
@@ -113,16 +117,22 @@ enum rte_crypto_asym_op_type {
/**< Signature Generation operation */
RTE_CRYPTO_ASYM_OP_VERIFY,
/**< Signature Verification operation */
- RTE_CRYPTO_ASYM_OP_PRIVATE_KEY_GENERATE,
- /**< DH Private Key generation operation */
- RTE_CRYPTO_ASYM_OP_PUBLIC_KEY_GENERATE,
- /**< DH Public Key generation operation */
- RTE_CRYPTO_ASYM_OP_SHARED_SECRET_COMPUTE,
- /**< DH Shared Secret compute operation */
RTE_CRYPTO_ASYM_OP_LIST_END
};
/**
+ * Asymmetric crypto key exchange operation type
+ */
+enum rte_crypto_asym_ke_type {
+ RTE_CRYPTO_ASYM_KE_PRIV_KEY_GENERATE,
+ /**< Private Key generation operation */
+ RTE_CRYPTO_ASYM_KE_PUB_KEY_GENERATE,
+ /**< Public Key generation operation */
+ RTE_CRYPTO_ASYM_KE_SHARED_SECRET_COMPUTE
+ /**< Shared Secret compute operation */
+};
+
+/**
* Padding types for RSA signature.
*/
enum rte_crypto_rsa_padding_type {
@@ -260,7 +270,7 @@ struct rte_crypto_modinv_xform {
*
*/
struct rte_crypto_dh_xform {
- enum rte_crypto_asym_op_type type;
+ enum rte_crypto_asym_ke_type ke_type;
/**< Setup xform for key generate or shared secret compute */
rte_crypto_uint p;
/**< Prime modulus data */
@@ -397,26 +407,27 @@ struct rte_crypto_rsa_op_param {
struct rte_crypto_dh_op_param {
rte_crypto_uint pub_key;
/**<
- * Output generated public key when xform type is
- * DH PUB_KEY_GENERATION.
- * Input peer public key when xform type is DH
- * SHARED_SECRET_COMPUTATION
+ * Output - generated public key, when dh xform ke_type is
+ * RTE_CRYPTO_ASYM_KE_PUB_KEY_GENERATE.
*
+ * Input - peer's public key, when dh xform ke_type is
+ * RTE_CRYPTO_ASYM_KE_SHARED_SECRET_COMPUTE.
*/
rte_crypto_uint priv_key;
/**<
- * Output generated private key if xform type is
- * DH PRIVATE_KEY_GENERATION
- * Input when xform type is DH SHARED_SECRET_COMPUTATION.
+ * Output - generated private key, when dh xform ke_type is
+ * RTE_CRYPTO_ASYM_KE_PRIV_KEY_GENERATE.
*
+ * Input - private key, when dh xform ke_type is one of:
+ * RTE_CRYPTO_ASYM_KE_PUB_KEY_GENERATE,
+ * RTE_CRYPTO_ASYM_KE_SHARED_SECRET_COMPUTE.
*/
rte_crypto_uint shared_secret;
/**<
- * Output with calculated shared secret
- * when dh xform set up with op type = SHARED_SECRET_COMPUTATION.
- *
+ * Output - calculated shared secret when dh xform ke_type is
+ * RTE_CRYPTO_ASYM_KE_SHARED_SECRET_COMPUTE.
*/
};
@@ -177,10 +177,16 @@ const char *rte_crypto_asym_op_strings[] = {
[RTE_CRYPTO_ASYM_OP_ENCRYPT] = "encrypt",
[RTE_CRYPTO_ASYM_OP_DECRYPT] = "decrypt",
[RTE_CRYPTO_ASYM_OP_SIGN] = "sign",
- [RTE_CRYPTO_ASYM_OP_VERIFY] = "verify",
- [RTE_CRYPTO_ASYM_OP_PRIVATE_KEY_GENERATE] = "priv_key_generate",
- [RTE_CRYPTO_ASYM_OP_PUBLIC_KEY_GENERATE] = "pub_key_generate",
- [RTE_CRYPTO_ASYM_OP_SHARED_SECRET_COMPUTE] = "sharedsecret_compute",
+ [RTE_CRYPTO_ASYM_OP_VERIFY] = "verify"
+};
+
+/**
+ * Asymmetric crypto key exchange operation strings identifiers.
+ */
+const char *rte_crypto_asym_ke_strings[] = {
+ [RTE_CRYPTO_ASYM_KE_PRIV_KEY_GENERATE] = "priv_key_generate",
+ [RTE_CRYPTO_ASYM_KE_PUB_KEY_GENERATE] = "pub_key_generate",
+ [RTE_CRYPTO_ASYM_KE_SHARED_SECRET_COMPUTE] = "sharedsecret_compute"
};
/**
@@ -168,7 +168,9 @@ struct rte_cryptodev_asymmetric_xform_capability {
/**< Transform type: RSA/MODEXP/DH/DSA/MODINV */
uint32_t op_types;
- /**< bitmask for supported rte_crypto_asym_op_type */
+ /**< bitmask for supported rte_crypto_asym_op_type or
+ * rte_crypto_asym_ke_type
+ */
__extension__
union {
@@ -108,6 +108,7 @@ EXPERIMENTAL {
#added in 22.07
rte_cryptodev_session_event_mdata_set;
+ rte_crypto_asym_ke_strings;
};
INTERNAL {