> -----Original Message-----
> From: Gowrishankar Muthukrishnan <gmuthukri...@marvell.com>
> Sent: Monday, August 14, 2023 9:49 AM
> To: Kusztal, ArkadiuszX <arkadiuszx.kusz...@intel.com>; dev@dpdk.org
> Cc: Akhil Goyal <gak...@marvell.com>; Ji, Kai <kai...@intel.com>; Power, Ciara
> <ciara.po...@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.kusz...@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.gmuthukri...@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.gmuthukri...@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
