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

[Gowrishankar Muthukrishnan <gmuthukrishn@marvell.com>]

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