Re: [dpdk-dev] [PATCH 1/4] lib/crypto: add support for ECDSA

[Shally Verma <shallyv@marvell.com>]

Hi Anoob, Akhil



> -----Original Message-----
> From: Anoob Joseph <anoobj@marvell.com>
> Sent: Monday, January 13, 2020 10:07 PM
> To: Kusztal, ArkadiuszX <arkadiuszx.kusztal@intel.com>; Akhil Goyal
> <akhil.goyal@nxp.com>; Doherty, Declan <declan.doherty@intel.com>; De
> Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>
> Cc: Ayuj Verma <ayverma@marvell.com>; Trahe, Fiona
> <fiona.trahe@intel.com>; Jerin Jacob Kollanukkaran <jerinj@marvell.com>;
> Narayana Prasad Raju Athreya <pathreya@marvell.com>; Shally Verma
> <shallyv@marvell.com>; Ankur Dwivedi <adwivedi@marvell.com>; Sunila
> Sahu <ssahu@marvell.com>; dev@dpdk.org
> Subject: RE: [PATCH 1/4] lib/crypto: add support for ECDSA
> 
....
> 
> > >
> > > > > +
> > > > > +	rte_crypto_param k;
> > > > > +	/**< The ECDSA per-message secret number, which is an
> integer
> > > > > +	 * in the interval (1, n-1)
> > > > > +	 */
> > > > [Arek] - If pmd can generate 'k' internally we could do something like:
> > > > 'if k.data == NULL => PMD will generate 'k' internally, k.data
> > > > remains untouched.'
> > >
> > > [Anoob] So that will be exposed as a new capability, right? Do you
> > > suggest any changes here to accommodate that?
> > [Arek] Or maybe feature flag, it would apply to DSA as well.
> 
> [Anoob] I meant feature flag only! Capability is for net devs etc.
> 
> > >
> > > > Another option is to provide user with some callback function to
> > > > generate CSRN which could be useful for RSA PSS, OAEP as well (we
> > > > already discussed that internally in Intel, I will elaborate on
> > > > this bit more in
> > > different thread).
> > >
> > > [Anoob] Do you intend to keep the generated CSRN hidden in the PMD?
> > [Arek] Openssl PMD does that with DSA but iam not a fan of that. But
> > if some hw can do DSA/ECDSA by generating 'k' by itself it would have to be
> added anyway.
> > Other option is to add aforementioned callbacks (for HW that not generate
> 'k'
> > by itself)
> 
> [Anoob] I think we can support either case. Our h/w also can generate CSRN
> and so are open to ideas. Can you explain the proposed callback function?
> Also, we can defer that discussion to a separate thread, if we have an
> agreement on the approach here.
> 
[Shally] Just a thought here. It can also be supported by exposing an API to application, like pre-compute / pre-setup which can generate 'k' or 'PRF' for other algo usage purpose. If PMD support it, return will be success with relevant o/p.. if not, return will be failure. Or no offload at all. In any case, generation of prf or 'k' is heavy on data path... app can call these pre-setup APIs to save cycles on actual sign op.

Shally
> > >
> > > >
> > > > > +
> > > > > +	rte_crypto_param r;
> > > > > +	/**< r component of elliptic curve signature
> > > > > +	 *     output : for signature generation
> > > > > +	 *     input  : for signature verification
> > > > > +	 */
> > > > > +	rte_crypto_param s;
> > > > > +	/**< s component of elliptic curve signature
> > > > > +	 *     output : for signature generation
> > > > > +	 *     input  : for signature verification
> > > > > +	 */
> > > > [Arek] - Do we want to add any constraints like 'this field should
> > > > be big enough to hold...'
> > >
> > > [Anoob] For every case where rte_crypto_param is used for 'output',
> > > application should make sure the buffers are large enough. Do you
> > > think we could document it somewhere common instead of adding per
> > operation?
> > [Arek]
> > Both options look good to me.
> 
> [Anoob] How about updating the description of 'rte_crypto_param' to reflect
> the same? I can update it in v2, if you can confirm.
> 
> > >
> > > > > +};
> > > > > +
> > > > > +/**
> > > > >   * Asymmetric Cryptographic Operation.
> > > > >   *
> > > > >   * Structure describing asymmetric crypto operation params.
> > > > > @@ -537,6 +619,7 @@ struct rte_crypto_asym_op {
> > > > >  		struct rte_crypto_mod_op_param modinv;
> > > > >  		struct rte_crypto_dh_op_param dh;
> > > > >  		struct rte_crypto_dsa_op_param dsa;
> > > > > +		struct rte_crypto_ecdsa_op_param ecdsa;
> > > > >  	};
> > > > >  };
> > > > >
> > > > > diff --git a/lib/librte_cryptodev/rte_cryptodev.c
> > > > > b/lib/librte_cryptodev/rte_cryptodev.c
> > > > > index 89aa2ed..0d6babb 100644
> > > > > --- a/lib/librte_cryptodev/rte_cryptodev.c
> > > > > +++ b/lib/librte_cryptodev/rte_cryptodev.c
> > > > > @@ -173,6 +173,7 @@ const char *rte_crypto_asym_xform_strings[]
> = {
> > > > >  	[RTE_CRYPTO_ASYM_XFORM_MODINV]	= "modinv",
> > > > >  	[RTE_CRYPTO_ASYM_XFORM_DH]	= "dh",
> > > > >  	[RTE_CRYPTO_ASYM_XFORM_DSA]	= "dsa",
> > > > > +	[RTE_CRYPTO_ASYM_XFORM_ECDSA]	= "ecdsa",
> > > > >  };
> > > > >
> > > > >  /**
> > > > > --
> > > > > 2.7.4
> > > >
> > > > Regards,
> > > > Arek