[RFC] cryptodev: refactor sm2, add plain message flag

[RFC] cryptodev: refactor sm2, add plain message flag

[Arkadiusz Kusztal]

SM2 asymmetric crypto operation was split into cipher and
signature operation. Now it corresponds to the other crypto
algorithms and facilitates addition of other SM2 components
like the SM2 key exchange.

Flag to distinguish between a plain message or a hash used
for signature was added to the DSA, ECDSA and SM2.

Signed-off-by: Arkadiusz Kusztal <arkadiuszx.kusztal@intel.com>
---
 lib/cryptodev/rte_crypto_asym.h | 116 +++++++++++++++++---------------
 1 file changed, 63 insertions(+), 53 deletions(-)

diff --git a/lib/cryptodev/rte_crypto_asym.h b/lib/cryptodev/rte_crypto_asym.h
index 8b5794fb7c..43bdb392c5 100644
--- a/lib/cryptodev/rte_crypto_asym.h
+++ b/lib/cryptodev/rte_crypto_asym.h
@@ -54,6 +54,7 @@ rte_crypto_asym_op_strings[];
  * and if the flag is not set, shared secret will be padded to the left with
  * zeros to the size of the underlying algorithm (default)
  */
+#define RTE_CRYPTO_ASYM_FLAG_PLAIN_INPUT		RTE_BIT32(2)
 
 /**
  * List of elliptic curves. This enum aligns with
@@ -379,16 +380,6 @@ struct rte_crypto_ec_xform {
 	/**< Pre-defined ec groups */
 };
 
-/**
- * Asymmetric SM2 transform data.
- *
- * Structure describing SM2 xform params.
- */
-struct rte_crypto_sm2_xform {
-	enum rte_crypto_auth_algorithm hash;
-	/**< Hash algorithm used in SM2 op. */
-};
-
 /**
  * Operations params for modular operations:
  * exponentiation and multiplicative inverse
@@ -540,7 +531,12 @@ struct rte_crypto_dsa_op_param {
 	enum rte_crypto_asym_op_type op_type;
 	/**< Signature Generation or Verification */
 	rte_crypto_param message;
-	/**< input message to be signed or verified */
+	/**<
+	 * Pointer to the input data
+	 * In case RTE_CRYPTO_ASYM_FLAG_PLAIN_INPUT flag is set in the op flags field,
+	 * it is a message to be signed by the PMD.
+	 * Otherwise, it is a message hash.
+	 */
 	rte_crypto_uint k;
 	/**< Per-message secret number, which is an integer
 	 * in the interval (1, q-1).
@@ -579,7 +575,12 @@ struct rte_crypto_ecdsa_op_param {
 	/**< Public key of the signer for verification */
 
 	rte_crypto_param message;
-	/**< Input message digest to be signed or verified */
+	/**<
+	 * Pointer to the input data
+	 * In case RTE_CRYPTO_ASYM_FLAG_PLAIN_INPUT flag is set in the op flags field,
+	 * it is a message to be signed by the PMD.
+	 * Otherwise, it is a message hash.
+	 */
 
 	rte_crypto_uint k;
 	/**< The ECDSA per-message secret number, which is an integer
@@ -652,52 +653,20 @@ struct rte_crypto_asym_xform {
 	};
 };
 
-/**
- * SM2 operation params.
- */
-struct rte_crypto_sm2_op_param {
+struct rte_crypto_sm2_signature {
 	enum rte_crypto_asym_op_type op_type;
 	/**< Signature generation or verification. */
-
-	rte_crypto_uint pkey;
-	/**< Private key for encryption or sign generation. */
-
-	struct rte_crypto_ec_point q;
-	/**< Public key for decryption or verification. */
-
 	rte_crypto_param message;
 	/**<
-	 * Pointer to input data
-	 * - to be encrypted for SM2 public encrypt.
-	 * - to be signed for SM2 sign generation.
-	 * - to be authenticated for SM2 sign verification.
-	 *
-	 * Pointer to output data
-	 * - for SM2 private decrypt.
-	 * In this case the underlying array should have been
-	 * allocated with enough memory to hold plaintext output
-	 * (at least encrypted text length). The message.length field
-	 * will be overwritten by the PMD with the decrypted length.
-	 */
-
-	rte_crypto_param cipher;
-	/**<
-	 * Pointer to input data
-	 * - to be decrypted for SM2 private decrypt.
-	 *
-	 * Pointer to output data
-	 * - for SM2 public encrypt.
-	 * In this case the underlying array should have been allocated
-	 * with enough memory to hold ciphertext output (at least X bytes
-	 * for prime field curve of N bytes and for message M bytes,
-	 * where X = (C1 || C2 || C3) and computed based on SM2 RFC as
-	 * C1 (1 + N + N), C2 = M, C3 = N. The cipher.length field will
-	 * be overwritten by the PMD with the encrypted length.
+	 * Pointer to the input data
+	 * In case RTE_CRYPTO_ASYM_FLAG_PLAIN_INPUT flag is set in the op flags field,
+	 * it is a message to be signed by the PMD.
+	 * Otherwise, it is a message hash.
 	 */
-
 	rte_crypto_uint id;
-	/**< The SM2 id used by signer and verifier. */
-
+	/**< The SM2 id used by signer and verifier.
+	 * In case RTE_CRYPTO_ASYM_FLAG_PLAIN_INPUT flag is set this field is unused.
+	 */
 	rte_crypto_uint k;
 	/**< The SM2 per-message secret number, which is an integer
 	 * in the interval (1, n-1).
@@ -719,6 +688,46 @@ struct rte_crypto_sm2_op_param {
 	 */
 };
 
+struct rte_crypto_sm2_cipher {
+	enum rte_crypto_asym_op_type op_type;
+	/**< Ecryption or decryption. */
+	rte_crypto_param message;
+	/**<
+	 * Pointer to input data
+	 * - to be encrypted for SM2 public encrypt.	 *
+	 * Pointer to output data
+	 * - for SM2 private decrypt.
+	 */
+	rte_crypto_param cipher;
+	/**<
+	 * Pointer to input data
+	 * - to be decrypted for SM2 private decrypt.
+	 *
+	 * Pointer to output data
+	 * - for SM2 public encrypt.
+	 */
+	rte_crypto_uint k;
+	/**< The SM2 per-message secret number, which is an integer
+	 * in the interval (1, n-1).
+	 * If the random number is generated by the PMD,
+	 * the 'rte_crypto_param.data' parameter should be set to NULL.
+	 */
+};
+
+/*
+ * Asymmetric SM2 transform data.
+ *
+ * Structure describing SM2 xform params.
+ */
+struct rte_crypto_sm2_xform {
+	enum rte_crypto_auth_algorithm hash;
+	/**< Hash algorithm used in SM2 op. */
+	rte_crypto_uint dA;
+	/**< Private key. */
+	struct rte_crypto_ec_point PA;
+	/**< Public key. */
+};
+
 /**
  * Asymmetric Cryptographic Operation.
  *
@@ -743,7 +752,8 @@ struct rte_crypto_asym_op {
 		struct rte_crypto_dsa_op_param dsa;
 		struct rte_crypto_ecdsa_op_param ecdsa;
 		struct rte_crypto_ecpm_op_param ecpm;
-		struct rte_crypto_sm2_op_param sm2;
+		struct rte_crypto_sm2_signature sm2_signature;
+		struct rte_crypto_sm2_cipher sm2_cipher;
 	};
 	uint16_t flags;
 	/**<
-- 
2.34.1

RE: [EXT] [RFC] cryptodev: refactor sm2, add plain message flag

[Gowrishankar Muthukrishnan]

Hi,
Please find my comments inline. At the same time, may I request you to 
review my patch series :
https://patches.dpdk.org/project/dpdk/list/?series=29149

> SM2 asymmetric crypto operation was split into cipher and signature
> operation. Now it corresponds to the other crypto algorithms and facilitates
> addition of other SM2 components like the SM2 key exchange.
> 
> Flag to distinguish between a plain message or a hash used for signature was
> added to the DSA, ECDSA and SM2.
> 
> Signed-off-by: Arkadiusz Kusztal <arkadiuszx.kusztal@intel.com>
> ---
>  lib/cryptodev/rte_crypto_asym.h | 116 +++++++++++++++++---------------
>  1 file changed, 63 insertions(+), 53 deletions(-)
> 
> diff --git a/lib/cryptodev/rte_crypto_asym.h
> b/lib/cryptodev/rte_crypto_asym.h index 8b5794fb7c..43bdb392c5 100644
> --- a/lib/cryptodev/rte_crypto_asym.h
> +++ b/lib/cryptodev/rte_crypto_asym.h
> @@ -54,6 +54,7 @@ rte_crypto_asym_op_strings[];
>   * and if the flag is not set, shared secret will be padded to the left with
>   * zeros to the size of the underlying algorithm (default)
>   */
> +#define RTE_CRYPTO_ASYM_FLAG_PLAIN_INPUT		RTE_BIT32(2)
> 
>  /**
>   * List of elliptic curves. This enum aligns with @@ -379,16 +380,6 @@ struct
> rte_crypto_ec_xform {
>  	/**< Pre-defined ec groups */
>  };
> 
> -/**
> - * Asymmetric SM2 transform data.
> - *
> - * Structure describing SM2 xform params.
> - */
> -struct rte_crypto_sm2_xform {
> -	enum rte_crypto_auth_algorithm hash;
> -	/**< Hash algorithm used in SM2 op. */
> -};
> -
>  /**
>   * Operations params for modular operations:
>   * exponentiation and multiplicative inverse @@ -540,7 +531,12 @@ struct
> rte_crypto_dsa_op_param {
>  	enum rte_crypto_asym_op_type op_type;
>  	/**< Signature Generation or Verification */
>  	rte_crypto_param message;
> -	/**< input message to be signed or verified */
> +	/**<
> +	 * Pointer to the input data
> +	 * In case RTE_CRYPTO_ASYM_FLAG_PLAIN_INPUT flag is set in the
> op flags field,
> +	 * it is a message to be signed by the PMD.
> +	 * Otherwise, it is a message hash.
> +	 */

If a PMD does not support plain message but only hash digest, then this new flag
cannot help, as it is set by application without checking PMD capabilities. 
Instead, I had proposed adding hash capability for any EC xform in general
(as other EC curves too can benefit out of it).
https://patches.dpdk.org/project/dpdk/patch/086351e84370ce65dcf947dba12a46f9c62ae79b.1691658879.git.gmuthukrishn@marvell.com/

To note, more than one hash algorithm needs to be supported as in ECDSA for eg. so I made it bitmask of hash algorithms supported by PMD.
For SM2, today we set only SM3.

With this, the application can check the xform capability and set op params as shown in :
https://patches.dpdk.org/project/dpdk/patch/f3be0a425170ee26a1396d34f52a8e07941f7ce5.1691658879.git.gmuthukrishn@marvell.com/

>  	rte_crypto_uint k;
>  	/**< Per-message secret number, which is an integer
>  	 * in the interval (1, q-1).
> @@ -579,7 +575,12 @@ struct rte_crypto_ecdsa_op_param {
>  	/**< Public key of the signer for verification */
> 
>  	rte_crypto_param message;
> -	/**< Input message digest to be signed or verified */
> +	/**<
> +	 * Pointer to the input data
> +	 * In case RTE_CRYPTO_ASYM_FLAG_PLAIN_INPUT flag is set in the
> op flags field,
> +	 * it is a message to be signed by the PMD.
> +	 * Otherwise, it is a message hash.
> +	 */
> 
Please find above my comments for this flag.

>  	rte_crypto_uint k;
>  	/**< The ECDSA per-message secret number, which is an integer
> @@ -652,52 +653,20 @@ struct rte_crypto_asym_xform {
>  	};
>  };
> 
> -/**
> - * SM2 operation params.
> - */
> -struct rte_crypto_sm2_op_param {
> +struct rte_crypto_sm2_signature {

Yeah, it will help picking params for the application easily.
Just a suggestion: could we retain _param suffix. Say rte_crypto_sm2_sign_param.

>  	enum rte_crypto_asym_op_type op_type;
>  	/**< Signature generation or verification. */

Now op_type can either be sign/verify here.
> -
> -	rte_crypto_uint pkey;
> -	/**< Private key for encryption or sign generation. */
> -
> -	struct rte_crypto_ec_point q;
> -	/**< Public key for decryption or verification. */
> -
>  	rte_crypto_param message;
>  	/**<
> -	 * Pointer to input data
> -	 * - to be encrypted for SM2 public encrypt.
> -	 * - to be signed for SM2 sign generation.
> -	 * - to be authenticated for SM2 sign verification.
> -	 *
> -	 * Pointer to output data
> -	 * - for SM2 private decrypt.
> -	 * In this case the underlying array should have been
> -	 * allocated with enough memory to hold plaintext output
> -	 * (at least encrypted text length). The message.length field
> -	 * will be overwritten by the PMD with the decrypted length.
> -	 */
> -
> -	rte_crypto_param cipher;
> -	/**<
> -	 * Pointer to input data
> -	 * - to be decrypted for SM2 private decrypt.
> -	 *
> -	 * Pointer to output data
> -	 * - for SM2 public encrypt.
> -	 * In this case the underlying array should have been allocated
> -	 * with enough memory to hold ciphertext output (at least X bytes
> -	 * for prime field curve of N bytes and for message M bytes,
> -	 * where X = (C1 || C2 || C3) and computed based on SM2 RFC as
> -	 * C1 (1 + N + N), C2 = M, C3 = N. The cipher.length field will
> -	 * be overwritten by the PMD with the encrypted length.
> +	 * Pointer to the input data
> +	 * In case RTE_CRYPTO_ASYM_FLAG_PLAIN_INPUT flag is set in the
> op flags field,
> +	 * it is a message to be signed by the PMD.
> +	 * Otherwise, it is a message hash.
>  	 */
Please find above my comments for this flag.

> -
>  	rte_crypto_uint id;
> -	/**< The SM2 id used by signer and verifier. */
> -
> +	/**< The SM2 id used by signer and verifier.
> +	 * In case RTE_CRYPTO_ASYM_FLAG_PLAIN_INPUT flag is set this
> field is unused.
> +	 */
>  	rte_crypto_uint k;
>  	/**< The SM2 per-message secret number, which is an integer
>  	 * in the interval (1, n-1).
> @@ -719,6 +688,46 @@ struct rte_crypto_sm2_op_param {
>  	 */
>  };
> 
> +struct rte_crypto_sm2_cipher {
> +	enum rte_crypto_asym_op_type op_type;
> +	/**< Ecryption or decryption. */
> +	rte_crypto_param message;
> +	/**<
> +	 * Pointer to input data
> +	 * - to be encrypted for SM2 public encrypt.	 *
> +	 * Pointer to output data
> +	 * - for SM2 private decrypt.
> +	 */
> +	rte_crypto_param cipher;
> +	/**<
> +	 * Pointer to input data
> +	 * - to be decrypted for SM2 private decrypt.
> +	 *
> +	 * Pointer to output data
> +	 * - for SM2 public encrypt.
> +	 */
> +	rte_crypto_uint k;
> +	/**< The SM2 per-message secret number, which is an integer
> +	 * in the interval (1, n-1).
> +	 * If the random number is generated by the PMD,
> +	 * the 'rte_crypto_param.data' parameter should be set to NULL.
> +	 */
> +};
> +
> +/*
> + * Asymmetric SM2 transform data.
> + *
> + * Structure describing SM2 xform params.
> + */
> +struct rte_crypto_sm2_xform {
> +	enum rte_crypto_auth_algorithm hash;
> +	/**< Hash algorithm used in SM2 op. */
> +	rte_crypto_uint dA;
> +	/**< Private key. */
> +	struct rte_crypto_ec_point PA;
> +	/**< Public key. */
> +};
> +

sm2_xform is no more required, but ec_xform can be reused as I had proposed in:
[v1,4/6] cryptodev: use generic EC xform params for SM2

So, to summarize, may I request below of this patch to go above my patch series
If you agree.

(1). Splitting sm2 op params into sign and cipher ops.
(2). Move private and public keys from op param into EC xform.
       For DSA , should we move public key into xform as a session param ?

Thanks,
Gowrishankar

>  /**
>   * Asymmetric Cryptographic Operation.
>   *
> @@ -743,7 +752,8 @@ struct rte_crypto_asym_op {
>  		struct rte_crypto_dsa_op_param dsa;
>  		struct rte_crypto_ecdsa_op_param ecdsa;
>  		struct rte_crypto_ecpm_op_param ecpm;
> -		struct rte_crypto_sm2_op_param sm2;
> +		struct rte_crypto_sm2_signature sm2_signature;
> +		struct rte_crypto_sm2_cipher sm2_cipher;
>  	};
>  	uint16_t flags;
>  	/**<
> --
> 2.34.1

RE: [EXT] [RFC] cryptodev: refactor sm2, add plain message flag

[Kusztal, ArkadiuszX]

> -----Original Message-----
> From: Gowrishankar Muthukrishnan <gmuthukrishn@marvell.com>
> Sent: Monday, August 14, 2023 9:49 AM
> To: Kusztal, ArkadiuszX <arkadiuszx.kusztal@intel.com>; dev@dpdk.org
> Cc: Akhil Goyal <gakhil@marvell.com>; Ji, Kai <kai.ji@intel.com>; Power, Ciara
> <ciara.power@intel.com>
> Subject: RE: [EXT] [RFC] cryptodev: refactor sm2, add plain message flag
> 
> Hi,
> Please find my comments inline. At the same time, may I request you to review
> my patch series :
> https://patches.dpdk.org/project/dpdk/list/?series=29149
> 
> > SM2 asymmetric crypto operation was split into cipher and signature
> > operation. Now it corresponds to the other crypto algorithms and
> > facilitates addition of other SM2 components like the SM2 key exchange.
> >
> > Flag to distinguish between a plain message or a hash used for
> > signature was added to the DSA, ECDSA and SM2.
> >
> > Signed-off-by: Arkadiusz Kusztal <arkadiuszx.kusztal@intel.com>
> > ---
> >  lib/cryptodev/rte_crypto_asym.h | 116
> > +++++++++++++++++---------------
> >  1 file changed, 63 insertions(+), 53 deletions(-)
> >
> > diff --git a/lib/cryptodev/rte_crypto_asym.h
> > b/lib/cryptodev/rte_crypto_asym.h index 8b5794fb7c..43bdb392c5 100644
> > --- a/lib/cryptodev/rte_crypto_asym.h
> > +++ b/lib/cryptodev/rte_crypto_asym.h
> > @@ -54,6 +54,7 @@ rte_crypto_asym_op_strings[];
> >   * and if the flag is not set, shared secret will be padded to the left with
> >   * zeros to the size of the underlying algorithm (default)
> >   */
> > +#define RTE_CRYPTO_ASYM_FLAG_PLAIN_INPUT		RTE_BIT32(2)
> >
> >  /**
> >   * List of elliptic curves. This enum aligns with @@ -379,16 +380,6
> > @@ struct rte_crypto_ec_xform {
> >  	/**< Pre-defined ec groups */
> >  };
> >
> > -/**
> > - * Asymmetric SM2 transform data.
> > - *
> > - * Structure describing SM2 xform params.
> > - */
> > -struct rte_crypto_sm2_xform {
> > -	enum rte_crypto_auth_algorithm hash;
> > -	/**< Hash algorithm used in SM2 op. */
> > -};
> > -
> >  /**
> >   * Operations params for modular operations:
> >   * exponentiation and multiplicative inverse @@ -540,7 +531,12 @@
> > struct rte_crypto_dsa_op_param {
> >  	enum rte_crypto_asym_op_type op_type;
> >  	/**< Signature Generation or Verification */
> >  	rte_crypto_param message;
> > -	/**< input message to be signed or verified */
> > +	/**<
> > +	 * Pointer to the input data
> > +	 * In case RTE_CRYPTO_ASYM_FLAG_PLAIN_INPUT flag is set in the
> > op flags field,
> > +	 * it is a message to be signed by the PMD.
> > +	 * Otherwise, it is a message hash.
> > +	 */
> 
> If a PMD does not support plain message but only hash digest, then this new flag
> cannot help, as it is set by application without checking PMD capabilities.
> Instead, I had proposed adding hash capability for any EC xform in general (as
> other EC curves too can benefit out of it).
> https://patches.dpdk.org/project/dpdk/patch/086351e84370ce65dcf947dba12a
> 46f9c62ae79b.1691658879.git.gmuthukrishn@marvell.com/
Actually hash should be moved outside of xform, we do not want to have a session per hash I think.
Session should be per key, eventually per private key only.
> 
> To note, more than one hash algorithm needs to be supported as in ECDSA for
> eg. so I made it bitmask of hash algorithms supported by PMD.
> For SM2, today we set only SM3.
> 
> With this, the application can check the xform capability and set op params as
> shown in :
> https://patches.dpdk.org/project/dpdk/patch/f3be0a425170ee26a1396d34f52a
> 8e07941f7ce5.1691658879.git.gmuthukrishn@marvell.com/
> 
> >  	rte_crypto_uint k;
> >  	/**< Per-message secret number, which is an integer
> >  	 * in the interval (1, q-1).
> > @@ -579,7 +575,12 @@ struct rte_crypto_ecdsa_op_param {
> >  	/**< Public key of the signer for verification */
> >
> >  	rte_crypto_param message;
> > -	/**< Input message digest to be signed or verified */
> > +	/**<
> > +	 * Pointer to the input data
> > +	 * In case RTE_CRYPTO_ASYM_FLAG_PLAIN_INPUT flag is set in the
> > op flags field,
> > +	 * it is a message to be signed by the PMD.
> > +	 * Otherwise, it is a message hash.
> > +	 */
> >
> Please find above my comments for this flag.
> 
> >  	rte_crypto_uint k;
> >  	/**< The ECDSA per-message secret number, which is an integer @@
> > -652,52 +653,20 @@ struct rte_crypto_asym_xform {
> >  	};
> >  };
> >
> > -/**
> > - * SM2 operation params.
> > - */
> > -struct rte_crypto_sm2_op_param {
> > +struct rte_crypto_sm2_signature {
> 
> Yeah, it will help picking params for the application easily.
> Just a suggestion: could we retain _param suffix. Say
> rte_crypto_sm2_sign_param.
> 
> >  	enum rte_crypto_asym_op_type op_type;
> >  	/**< Signature generation or verification. */
> 
> Now op_type can either be sign/verify here.
> > -
> > -	rte_crypto_uint pkey;
> > -	/**< Private key for encryption or sign generation. */
> > -
> > -	struct rte_crypto_ec_point q;
> > -	/**< Public key for decryption or verification. */
> > -
> >  	rte_crypto_param message;
> >  	/**<
> > -	 * Pointer to input data
> > -	 * - to be encrypted for SM2 public encrypt.
> > -	 * - to be signed for SM2 sign generation.
> > -	 * - to be authenticated for SM2 sign verification.
> > -	 *
> > -	 * Pointer to output data
> > -	 * - for SM2 private decrypt.
> > -	 * In this case the underlying array should have been
> > -	 * allocated with enough memory to hold plaintext output
> > -	 * (at least encrypted text length). The message.length field
> > -	 * will be overwritten by the PMD with the decrypted length.
> > -	 */
> > -
> > -	rte_crypto_param cipher;
> > -	/**<
> > -	 * Pointer to input data
> > -	 * - to be decrypted for SM2 private decrypt.
> > -	 *
> > -	 * Pointer to output data
> > -	 * - for SM2 public encrypt.
> > -	 * In this case the underlying array should have been allocated
> > -	 * with enough memory to hold ciphertext output (at least X bytes
> > -	 * for prime field curve of N bytes and for message M bytes,
> > -	 * where X = (C1 || C2 || C3) and computed based on SM2 RFC as
> > -	 * C1 (1 + N + N), C2 = M, C3 = N. The cipher.length field will
> > -	 * be overwritten by the PMD with the encrypted length.
> > +	 * Pointer to the input data
> > +	 * In case RTE_CRYPTO_ASYM_FLAG_PLAIN_INPUT flag is set in the
> > op flags field,
> > +	 * it is a message to be signed by the PMD.
> > +	 * Otherwise, it is a message hash.
> >  	 */
> Please find above my comments for this flag.
> 
> > -
> >  	rte_crypto_uint id;
> > -	/**< The SM2 id used by signer and verifier. */
> > -
> > +	/**< The SM2 id used by signer and verifier.
> > +	 * In case RTE_CRYPTO_ASYM_FLAG_PLAIN_INPUT flag is set this
> > field is unused.
> > +	 */
> >  	rte_crypto_uint k;
> >  	/**< The SM2 per-message secret number, which is an integer
> >  	 * in the interval (1, n-1).
> > @@ -719,6 +688,46 @@ struct rte_crypto_sm2_op_param {
> >  	 */
> >  };
> >
> > +struct rte_crypto_sm2_cipher {
> > +	enum rte_crypto_asym_op_type op_type;
> > +	/**< Ecryption or decryption. */
> > +	rte_crypto_param message;
> > +	/**<
> > +	 * Pointer to input data
> > +	 * - to be encrypted for SM2 public encrypt.	 *
> > +	 * Pointer to output data
> > +	 * - for SM2 private decrypt.
> > +	 */
> > +	rte_crypto_param cipher;
> > +	/**<
> > +	 * Pointer to input data
> > +	 * - to be decrypted for SM2 private decrypt.
> > +	 *
> > +	 * Pointer to output data
> > +	 * - for SM2 public encrypt.
> > +	 */
> > +	rte_crypto_uint k;
> > +	/**< The SM2 per-message secret number, which is an integer
> > +	 * in the interval (1, n-1).
> > +	 * If the random number is generated by the PMD,
> > +	 * the 'rte_crypto_param.data' parameter should be set to NULL.
> > +	 */
> > +};
> > +
> > +/*
> > + * Asymmetric SM2 transform data.
> > + *
> > + * Structure describing SM2 xform params.
> > + */
> > +struct rte_crypto_sm2_xform {
> > +	enum rte_crypto_auth_algorithm hash;
> > +	/**< Hash algorithm used in SM2 op. */
> > +	rte_crypto_uint dA;
> > +	/**< Private key. */
> > +	struct rte_crypto_ec_point PA;
> > +	/**< Public key. */
> > +};
> > +
> 
> sm2_xform is no more required, but ec_xform can be reused as I had proposed
> in:
> [v1,4/6] cryptodev: use generic EC xform params for SM2
> 
> So, to summarize, may I request below of this patch to go above my patch series
> If you agree.
> 
> (1). Splitting sm2 op params into sign and cipher ops.
> (2). Move private and public keys from op param into EC xform.
>        For DSA , should we move public key into xform as a session param ?
Regardless which way we would go, it should be consistent across the API. I would say that private key should always be in session, public key eventually. Except for the key exchange, of course.
> 
> Thanks,
> Gowrishankar
> 
> >  /**
> >   * Asymmetric Cryptographic Operation.
> >   *
> > @@ -743,7 +752,8 @@ struct rte_crypto_asym_op {
> >  		struct rte_crypto_dsa_op_param dsa;
> >  		struct rte_crypto_ecdsa_op_param ecdsa;
> >  		struct rte_crypto_ecpm_op_param ecpm;
> > -		struct rte_crypto_sm2_op_param sm2;
> > +		struct rte_crypto_sm2_signature sm2_signature;
> > +		struct rte_crypto_sm2_cipher sm2_cipher;
> >  	};
> >  	uint16_t flags;
> >  	/**<
> > --
> > 2.34.1

RE: [EXT] [RFC] cryptodev: refactor sm2, add plain message flag

[Gowrishankar Muthukrishnan]

Hi,
> Actually hash should be moved outside of xform, we do not want to have a
> session per hash I think.
> Session should be per key, eventually per private key only.
> >

If a hardware does not support hashing plain input before any SM2 op,
It has to be first hashed in a separate session (based on hash alg), then forward
the digest into current op. To support these two possible kind of inputs,
capability check as proposed can help the application. OpenSSL PMD for eg takes only
plain text input (and it does internal hashing).

Would you think hash algo can change in the mid of session ? If not, it could be
a session attribute still.

> > To note, more than one hash algorithm needs to be supported as in
> > ECDSA for eg. so I made it bitmask of hash algorithms supported by PMD.
> > For SM2, today we set only SM3.
> >
> > With this, the application can check the xform capability and set op
> > params as shown in :
> > https://urldefense.proofpoint.com/v2/url?u=https-3A__patches.dpdk.org_
> >
> project_dpdk_patch_f3be0a425170ee26a1396d34f52a&d=DwIFAg&c=nKjWec2
> b6R0
> > mOyPaz7xtfQ&r=EAtr-g7yUFhtOio8r2Rtm13Aqe4WVp_S_gHpcu6KFVo&m=PH-
> nP4_D0b
> > HFdQJbLclZO68l2-
> LQCCcvOX3vuHeUdkYYZe3kXzKWxsZ9bJa_SKww&s=88sPzV8cxNP2j
> > qfXzX1RjUhuU1U_jE8PB55ZFEG-SP4&e=
> > 8e07941f7ce5.1691658879.git.gmuthukrishn@marvell.com/
> >
> > >  	rte_crypto_uint k;
> > >  	/**< Per-message secret number, which is an integer
> > >  	 * in the interval (1, q-1).
> > > @@ -579,7 +575,12 @@ struct rte_crypto_ecdsa_op_param {
> > >  	/**< Public key of the signer for verification */
> > >
> > >  	rte_crypto_param message;
> > > -	/**< Input message digest to be signed or verified */
> > > +	/**<
> > > +	 * Pointer to the input data
> > > +	 * In case RTE_CRYPTO_ASYM_FLAG_PLAIN_INPUT flag is set in the
> > > op flags field,
> > > +	 * it is a message to be signed by the PMD.
> > > +	 * Otherwise, it is a message hash.
> > > +	 */
> > >
> > Please find above my comments for this flag.
> >
> > >  	rte_crypto_uint k;
> > >  	/**< The ECDSA per-message secret number, which is an integer @@
> > > -652,52 +653,20 @@ struct rte_crypto_asym_xform {
> > >  	};
> > >  };
> > >
> > > -/**
> > > - * SM2 operation params.
> > > - */
> > > -struct rte_crypto_sm2_op_param {
> > > +struct rte_crypto_sm2_signature {
> >
> > Yeah, it will help picking params for the application easily.
> > Just a suggestion: could we retain _param suffix. Say
> > rte_crypto_sm2_sign_param.
> >
> > >  	enum rte_crypto_asym_op_type op_type;
> > >  	/**< Signature generation or verification. */
> >
> > Now op_type can either be sign/verify here.
> > > -
> > > -	rte_crypto_uint pkey;
> > > -	/**< Private key for encryption or sign generation. */
> > > -
> > > -	struct rte_crypto_ec_point q;
> > > -	/**< Public key for decryption or verification. */
> > > -
> > >  	rte_crypto_param message;
> > >  	/**<
> > > -	 * Pointer to input data
> > > -	 * - to be encrypted for SM2 public encrypt.
> > > -	 * - to be signed for SM2 sign generation.
> > > -	 * - to be authenticated for SM2 sign verification.
> > > -	 *
> > > -	 * Pointer to output data
> > > -	 * - for SM2 private decrypt.
> > > -	 * In this case the underlying array should have been
> > > -	 * allocated with enough memory to hold plaintext output
> > > -	 * (at least encrypted text length). The message.length field
> > > -	 * will be overwritten by the PMD with the decrypted length.
> > > -	 */
> > > -
> > > -	rte_crypto_param cipher;
> > > -	/**<
> > > -	 * Pointer to input data
> > > -	 * - to be decrypted for SM2 private decrypt.
> > > -	 *
> > > -	 * Pointer to output data
> > > -	 * - for SM2 public encrypt.
> > > -	 * In this case the underlying array should have been allocated
> > > -	 * with enough memory to hold ciphertext output (at least X bytes
> > > -	 * for prime field curve of N bytes and for message M bytes,
> > > -	 * where X = (C1 || C2 || C3) and computed based on SM2 RFC as
> > > -	 * C1 (1 + N + N), C2 = M, C3 = N. The cipher.length field will
> > > -	 * be overwritten by the PMD with the encrypted length.
> > > +	 * Pointer to the input data
> > > +	 * In case RTE_CRYPTO_ASYM_FLAG_PLAIN_INPUT flag is set in the
> > > op flags field,
> > > +	 * it is a message to be signed by the PMD.
> > > +	 * Otherwise, it is a message hash.
> > >  	 */
> > Please find above my comments for this flag.
> >
> > > -
> > >  	rte_crypto_uint id;
> > > -	/**< The SM2 id used by signer and verifier. */
> > > -
> > > +	/**< The SM2 id used by signer and verifier.
> > > +	 * In case RTE_CRYPTO_ASYM_FLAG_PLAIN_INPUT flag is set this
> > > field is unused.
> > > +	 */
> > >  	rte_crypto_uint k;
> > >  	/**< The SM2 per-message secret number, which is an integer
> > >  	 * in the interval (1, n-1).
> > > @@ -719,6 +688,46 @@ struct rte_crypto_sm2_op_param {
> > >  	 */
> > >  };
> > >
> > > +struct rte_crypto_sm2_cipher {
> > > +	enum rte_crypto_asym_op_type op_type;
> > > +	/**< Ecryption or decryption. */
> > > +	rte_crypto_param message;
> > > +	/**<
> > > +	 * Pointer to input data
> > > +	 * - to be encrypted for SM2 public encrypt.	 *
> > > +	 * Pointer to output data
> > > +	 * - for SM2 private decrypt.
> > > +	 */
> > > +	rte_crypto_param cipher;
> > > +	/**<
> > > +	 * Pointer to input data
> > > +	 * - to be decrypted for SM2 private decrypt.
> > > +	 *
> > > +	 * Pointer to output data
> > > +	 * - for SM2 public encrypt.
> > > +	 */
> > > +	rte_crypto_uint k;
> > > +	/**< The SM2 per-message secret number, which is an integer
> > > +	 * in the interval (1, n-1).
> > > +	 * If the random number is generated by the PMD,
> > > +	 * the 'rte_crypto_param.data' parameter should be set to NULL.
> > > +	 */
> > > +};
> > > +
> > > +/*
> > > + * Asymmetric SM2 transform data.
> > > + *
> > > + * Structure describing SM2 xform params.
> > > + */
> > > +struct rte_crypto_sm2_xform {
> > > +	enum rte_crypto_auth_algorithm hash;
> > > +	/**< Hash algorithm used in SM2 op. */
> > > +	rte_crypto_uint dA;
> > > +	/**< Private key. */
> > > +	struct rte_crypto_ec_point PA;
> > > +	/**< Public key. */
> > > +};
> > > +
> >
> > sm2_xform is no more required, but ec_xform can be reused as I had
> > proposed
> > in:
> > [v1,4/6] cryptodev: use generic EC xform params for SM2
> >
> > So, to summarize, may I request below of this patch to go above my
> > patch series If you agree.
> >
> > (1). Splitting sm2 op params into sign and cipher ops.
> > (2). Move private and public keys from op param into EC xform.
> >        For DSA , should we move public key into xform as a session param ?
> Regardless which way we would go, it should be consistent across the API. I
> would say that private key should always be in session, public key eventually.
> Except for the key exchange, of course.

Ack.
Could you get a chance to review my patch series as well ?

Thanks,
Gowrishankar

RE: [RFC] cryptodev: refactor sm2, add plain message flag

[Power, Ciara]

Hi Akhil,

> -----Original Message-----
> From: Kusztal, ArkadiuszX <arkadiuszx.kusztal@intel.com>
> Sent: Friday, August 11, 2023 6:40 PM
> To: dev@dpdk.org
> Cc: gakhil@marvell.com; Ji, Kai <kai.ji@intel.com>; Power, Ciara
> <ciara.power@intel.com>; Kusztal, ArkadiuszX <arkadiuszx.kusztal@intel.com>
> Subject: [RFC] cryptodev: refactor sm2, add plain message flag
> 
> SM2 asymmetric crypto operation was split into cipher and signature operation.
> Now it corresponds to the other crypto algorithms and facilitates addition of other
> SM2 components like the SM2 key exchange.
> 
> Flag to distinguish between a plain message or a hash used for signature was
> added to the DSA, ECDSA and SM2.
> 
> Signed-off-by: Arkadiusz Kusztal <arkadiuszx.kusztal@intel.com>
> ---
>  lib/cryptodev/rte_crypto_asym.h | 116 +++++++++++++++++---------------
>  1 file changed, 63 insertions(+), 53 deletions(-)

Due to ongoing discussion and rework needed on this patch, it will be picked back up for release, as we did not make 23.11 RC1.

Thanks,
Ciara

RE: [RFC] cryptodev: refactor sm2, add plain message flag

[Akhil Goyal]

Hi Ciara/Arek,
> Hi Akhil,
> 
> > -----Original Message-----
> > From: Kusztal, ArkadiuszX <arkadiuszx.kusztal@intel.com>
> > Sent: Friday, August 11, 2023 6:40 PM
> > To: dev@dpdk.org
> > Cc: gakhil@marvell.com; Ji, Kai <kai.ji@intel.com>; Power, Ciara
> > <ciara.power@intel.com>; Kusztal, ArkadiuszX
> <arkadiuszx.kusztal@intel.com>
> > Subject: [RFC] cryptodev: refactor sm2, add plain message flag
> >
> > SM2 asymmetric crypto operation was split into cipher and signature
> operation.
> > Now it corresponds to the other crypto algorithms and facilitates addition of
> other
> > SM2 components like the SM2 key exchange.
> >
> > Flag to distinguish between a plain message or a hash used for signature was
> > added to the DSA, ECDSA and SM2.
> >
> > Signed-off-by: Arkadiusz Kusztal <arkadiuszx.kusztal@intel.com>
> > ---
> >  lib/cryptodev/rte_crypto_asym.h | 116 +++++++++++++++++---------------
> >  1 file changed, 63 insertions(+), 53 deletions(-)
> 
> Due to ongoing discussion and rework needed on this patch, it will be picked
> back up for release, as we did not make 23.11 RC1.
> 
Is it planned for 24.03? Any update?