DPDK patches and discussions
 help / color / mirror / Atom feed
From: "Kusztal, ArkadiuszX" <arkadiuszx.kusztal@intel.com>
To: Gowrishankar Muthukrishnan <gmuthukrishn@marvell.com>,
	"dev@dpdk.org" <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
Date: Sun, 17 Sep 2023 15:59:49 +0000	[thread overview]
Message-ID: <PH0PR11MB5013F9C702FA862227BE33789FF4A@PH0PR11MB5013.namprd11.prod.outlook.com> (raw)
In-Reply-To: <CO1PR18MB47142E32FE672CE5BF15A161CB17A@CO1PR18MB4714.namprd18.prod.outlook.com>



> -----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


  reply	other threads:[~2023-09-17 15:59 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-08-11 17:39 Arkadiusz Kusztal
2023-08-14  7:49 ` [EXT] " Gowrishankar Muthukrishnan
2023-09-17 15:59   ` Kusztal, ArkadiuszX [this message]
2023-09-21  4:26     ` Gowrishankar Muthukrishnan
2023-10-04  9:34 ` Power, Ciara
2024-01-19  9:18   ` Akhil Goyal

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=PH0PR11MB5013F9C702FA862227BE33789FF4A@PH0PR11MB5013.namprd11.prod.outlook.com \
    --to=arkadiuszx.kusztal@intel.com \
    --cc=ciara.power@intel.com \
    --cc=dev@dpdk.org \
    --cc=gakhil@marvell.com \
    --cc=gmuthukrishn@marvell.com \
    --cc=kai.ji@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).