> -----Original Message-----
> From: Gowrishankar Muthukrishnan <gmuthukri...@marvell.com>
> Sent: Tuesday, October 8, 2024 11:32 AM
> To: Kusztal, ArkadiuszX <arkadiuszx.kusz...@intel.com>; dev@dpdk.org; Akhil
> Goyal <gak...@marvell.com>; Fan Zhang <fanzhang....@gmail.com>
> Cc: Anoob Joseph <ano...@marvell.com>; Richardson, Bruce
> <bruce.richard...@intel.com>; Jerin Jacob <jer...@marvell.com>; Ji, Kai
> <kai...@intel.com>; jack.bond-pres...@foss.arm.com; Marchand, David
> <david.march...@redhat.com>; hemant.agra...@nxp.com; De Lara Guarch,
> Pablo <pablo.de.lara.gua...@intel.com>; Trahe, Fiona
> <fiona.tr...@intel.com>; Doherty, Declan <declan.dohe...@intel.com>;
> ma...@nvidia.com; ruifeng.w...@arm.com
> Subject: RE: [PATCH v6 1/6] cryptodev: add EDDSA asymmetric crypto algorithm
>
> > Acked-by: Arkadiusz Kusztal <arkadiuszx. kusztal@ intel. com
>
> Thanks Arkadiusz.
>
> > > Hi Gowrishankar,
> > >
> > > I like the idea of adding EdDSA, but I have several comments.
> > >
> <cut>
> > > > +/**
> > > > + * EdDSA operation params
> > > > + */
> > > > +struct rte_crypto_eddsa_op_param {
> > > > + enum rte_crypto_asym_op_type op_type;
> > > > + /**< Signature generation or verification */
> > > > +
> > > > + rte_crypto_param message;
> > > > + /**< Input message digest to be signed or verified */
> > > HashEdDSA will require a message digest; pure EdDSA will require the
> > > message itself. For HW it will be more complicated.
>
> Do you mean some hardware may not have HashEdDSA support ?
Not in full. For example: ECDSA in QAT and Octeon accepts a digest, not a
message. So it does not support the full process, but EdDSA is more complicated
than that because of the two hash rounds, similar to the SM2.
For now we have only OpenSSL PMD that supports it, and it accepts a message not
a digest, so this should be changed to "message to be signed".
> If so, I think it can be addressed as an operation capability in EdDSA xform
> itself
> as proposed in another patch:
> https://patches.dpdk.org/project/dpdk/patch/20241004181255.916-1-
> gmuthukri...@marvell.com/
I have not yet reviewed this patch, but it looks that possibly yes.
>
> > > > +
> > > > + rte_crypto_param context;
> > > > + /**< Context value for the sign op.
> > > > + * Must not be empty for Ed25519ctx instance.
> > > > + */
> > > > +
> > > > + enum rte_crypto_edward_instance instance;
> > > > + /**< Type of Edwards curve. */
> > > All instances are using the same curve, where they differ is the way
> > > of handling input message.
> > > And I think this should be a session variable -> new xform for the EdDSA.
>
> Based on prehash and context string, these instances are listed in RFC.
> A context string per operation helps ensure each signature is uniquely tied
> to its
> specific context, thereby preventing reuse of signatures across different
> contexts or operations.
> Prehashing adds additional security by ensuring new prehash is computed from
> the message.
> Therefor it is more appropriate to treat both of these as operational
> variables.
Different 'instance' are basically different algorithms.
About the 'context' I am not sure, as not any major protocol specifies its
usage (TLS and IKEv2 forbids PH though),
But from RFC8032, it looks like it was defined to be used per protocol basis,
or some subprotocol routine. But about this I am not sure.
Yet, EdDSA should not be delayed really; it is basically a network standard for
quite a time.
These changes may be discussed later.
>
> Thanks,
> Gowrishankar
> > > > +
> > > > + rte_crypto_uint sign;
> > > > + /**< Edward curve signature
> > > > + * output : for signature generation
> > > > + * input : for signature verification
> > > > + */
> > > > +};
> > > > +
> > > > /**
> > > > * Structure for EC point multiplication operation param
> > > > */
> > > > @@ -720,6 +766,7 @@ struct rte_crypto_asym_op {
> > > > struct rte_crypto_ecdsa_op_param ecdsa;
> > > > struct rte_crypto_ecpm_op_param ecpm;
> > > > struct rte_crypto_sm2_op_param sm2;
> > > > + struct rte_crypto_eddsa_op_param eddsa;
> > > > };
> > > > uint16_t flags;
> > > > /**<
> > > > --
> > > > 2.21.0
