[dpdk-dev] [RFC] cryptodev/asymm: propose changes to modexp and modinv API

[dpdk-dev] [RFC] cryptodev/asymm: propose changes to modexp and modinv API

[Kusztal, ArkadiuszX]

Hi Shally,

I'm implementing a crypto asymmetric PMD and have some concerns about the API which I
will work through over the next few months. Starting with modexp and modinv I have
the following questions / suggestions:

              rte_crypto_asym.h:233
                             rte_crypto_param modulus;
                             /**< modulus
                             * Prime modulus of the modexp transform operation in octet-string
                             * network byte order format.
                             */
                             [AK] - Why prime? RSA for example use semi-prime or "RSA multi-prime".
                             It should be just any positive integer.
                             [AK] - If session API layer should check if it is non-zero and set flag accordingly.

              rte_crypto_asym.h:253
                             rte_crypto_param modulus;
                             /**<
                             * Pointer to the prime modulus data for modular
                             * inverse operation in octet-string network byte
                             * order format.
                             */
                             [AK] - Same situation as for mod exp. Just any number.
                             For example when using with RSA Carmichael and Euler Totient function will even
                             have composite factors.

              rte_crypto_asym.h:323
                             struct rte_crypto_mod_op_param {
                             [AK] - There should be a result field. It size should be equal to the size
                             of modulus. Same apply to mod mult inverse. It should be driver responsability to check if result
                             will not overflow
                             [AK] - Any particular reason modulus and exponent is in session? Not saying
                             it is wrong but is it for DH, RSA use cases only?

                             rte_crypto.h:39
                             enum rte_crypto_op_status {
                             [AK] - There will be many more status options in asymmetric,
                             could we probably create new one for asymmetric crypto? Even if asymmetric and symmetric
                             overlap?
                             For mod exp, mod inv potentially it will be:
                            DIVIDING_BY_ZERO_ERROR, INVERSE_NOT_EXISTS_ERROR...

              rte_crypto_asym.h:33
                             size_t length;
                             /**< length of data in bytes */
                             [AK] - Is it guaranteed to be length of actual data, not allocated memory (i mean no leading 0ed bytes), so the most significant bit will be in data[0]?
                             [AK] - could it be uint16/32_t instead as size_t can have different sizes in different implementations, uint16_t should be enough
                             for all algorithms big integer sizes

              rte_crypto_asym.h:74, 250, 257, 351
                             /**< Modular Inverse
                             [AK] - Modular Multiplicative Inverse


Thanks,
Arek

Re: [dpdk-dev] [RFC] cryptodev/asymm: propose changes to modexp and modinv API

[Verma, Shally]

HI Arek

Sorry for late response. Please see response inline

From: Kusztal, ArkadiuszX <arkadiuszx.kusztal@intel.com> 
Sent: 13 December 2018 01:56
To: Verma, Shally <Shally.Verma@cavium.com>
Cc: dev@dpdk.org; Trahe, Fiona <fiona.trahe@intel.com>; Doherty, Declan <declan.doherty@intel.com>
Subject: [RFC] cryptodev/asymm: propose changes to modexp and modinv API

External Email
Hi Shally,

I'm implementing a crypto asymmetric PMD and have some concerns about the API which I 
will work through over the next few months. Starting with modexp and modinv I have
the following questions / suggestions:

              rte_crypto_asym.h:233
                             rte_crypto_param modulus;
                             /**< modulus
                             * Prime modulus of the modexp transform operation in octet-string
                             * network byte order format.
                             */
                             [AK] - Why prime? RSA for example use semi-prime or "RSA multi-prime".
                             It should be just any positive integer.
[Shally] Hmm.. yes you're right . by the purpose of it , it is a semi-prime input.. so prime shouldn't be mentioned here.
                             [AK] - If session API layer should check if it is non-zero and set flag accordingly.
[Shally] Sorry I didn't get this.. which flag you mean here? if modulus value 0 is passed, it should be considered as INVALID_PARAM.
                             
              rte_crypto_asym.h:253
                             rte_crypto_param modulus;
                             /**<
                             * Pointer to the prime modulus data for modular
                             * inverse operation in octet-string network byte
                             * order format.
                             */
                             [AK] - Same situation as for mod exp. Just any number.
[Shally] Yea. It should be reworded as modulus data instead of *prime* modulus data

                             For example when using with RSA Carmichael and Euler Totient function will even
                             have composite factors. 
              
              rte_crypto_asym.h:323
                             struct rte_crypto_mod_op_param {
                             [AK] - There should be a result field. It size should be equal to the size
                             of modulus. Same apply to mod mult inverse. It should be driver responsability to check if result
                             will not overflow
[Shally] so these are in-place operation. Output will be written back to base param. It also imply length of allocated array should be >= modulus length which is passed in session param.

                             [AK] - Any particular reason modulus and exponent is in session? Not saying
                             it is wrong but is it for DH, RSA use cases only?
[Shally] no that's not the intent. For RSA and DH respective xforms have been defined. It is kept in session envisioning modulus and exponent wont change frequently across operation but only base value. 
So once context is loaded with modulus and exponent , app can call modexp on different base values.

                             rte_crypto.h:39
                             enum rte_crypto_op_status {
                             [AK] - There will be many more status options in asymmetric,
                             could we probably create new one for asymmetric crypto? Even if asymmetric and symmetric
                             overlap?
                             For mod exp, mod inv potentially it will be:
                            DIVIDING_BY_ZERO_ERROR, INVERSE_NOT_EXISTS_ERROR...                             
[Shally] So far RTE_CRYPTO_OP_STATUS_INVALID_PARAM has been sufficient for such cases. Do you have any use-cases where you need specific error code to indicate asym specific error codes?

              rte_crypto_asym.h:33
                             size_t length;
                             /**< length of data in bytes */
                             [AK] - Is it guaranteed to be length of actual data, not allocated memory (i mean no leading 0ed bytes), so the most significant bit will be in data[0]?
[Shally] it should be length of actual data not length of allocated memory to an array. 
However it might create bit confusion on modular exponentiation op param as that expect length passed should tell actual data length in base array but array itself should be allocated upto modulus length.

                             [AK] - could it be uint16/32_t instead as size_t can have different sizes in different implementations, uint16_t should be enough
                             for all algorithms big integer sizes 
[Shally] no hard choices here though. But size_t would never be less than uint16_t so it guarantee to be large enough for any machines
              
              rte_crypto_asym.h:74, 250, 257, 351
                             /**< Modular Inverse
                             [AK] - Modular Multiplicative Inverse
    [Shally] Ack.    

Thanks,
Arek
              
              

Re: [dpdk-dev] [RFC] cryptodev/asymm: propose changes to modexp and modinv API

[Kusztal, ArkadiuszX]

Hi Shally,

Thanks for your answers :).

My answers in [AK-v2]

> -----Original Message-----
> From: Verma, Shally [mailto:Shally.Verma@cavium.com]
> Sent: Monday, December 17, 2018 6:45 AM
> To: Kusztal, ArkadiuszX <arkadiuszx.kusztal@intel.com>
> Cc: dev@dpdk.org; Trahe, Fiona <fiona.trahe@intel.com>; Doherty, Declan
> <declan.doherty@intel.com>; Kanaka Durga Kotamarthy
> <kkotamarthy@marvell.com>; Sunila Sahu <ssahu@marvell.com>;
> Kotamarthy, Kanaka <Kanaka.Kotamarthy@cavium.com>; Sahu, Sunila
> <Sunila.Sahu@cavium.com>
> Subject: RE: [RFC] cryptodev/asymm: propose changes to modexp and
> modinv API
> 
> HI Arek
> 
> Sorry for late response. Please see response inline
> 
> From: Kusztal, ArkadiuszX <arkadiuszx.kusztal@intel.com>
> Sent: 13 December 2018 01:56
> To: Verma, Shally <Shally.Verma@cavium.com>
> Cc: dev@dpdk.org; Trahe, Fiona <fiona.trahe@intel.com>; Doherty, Declan
> <declan.doherty@intel.com>
> Subject: [RFC] cryptodev/asymm: propose changes to modexp and modinv
> API
> 
> External Email
> Hi Shally,
> 
> I'm implementing a crypto asymmetric PMD and have some concerns about
> the API which I will work through over the next few months. Starting with
> modexp and modinv I have the following questions / suggestions:
> 
>               rte_crypto_asym.h:233
>                              rte_crypto_param modulus;
>                              /**< modulus
>                              * Prime modulus of the modexp transform operation in
> octet-string
>                              * network byte order format.
>                              */
>                              [AK] - Why prime? RSA for example use semi-prime or "RSA
> multi-prime".
>                              It should be just any positive integer.
> [Shally] Hmm.. yes you're right . by the purpose of it , it is a semi-prime
> input.. so prime shouldn't be mentioned here.
[AK-v2] I think it could be any nonzero number even composite, for DH, DSA it would be prime etc.
>                              [AK] - If session API layer should check if it is non-zero and
> set flag accordingly.
> [Shally] Sorry I didn't get this.. which flag you mean here? if modulus value 0
> is passed, it should be considered as INVALID_PARAM.
[AK-v2] - INVALID_PARAM is perfectly fine for me :).
> 
>               rte_crypto_asym.h:253
>                              rte_crypto_param modulus;
>                              /**<
>                              * Pointer to the prime modulus data for modular
>                              * inverse operation in octet-string network byte
>                              * order format.
>                              */
>                              [AK] - Same situation as for mod exp. Just any number.
> [Shally] Yea. It should be reworded as modulus data instead of *prime*
> modulus data
> 
>                              For example when using with RSA Carmichael and Euler
> Totient function will even
>                              have composite factors.
> 
>               rte_crypto_asym.h:323
>                              struct rte_crypto_mod_op_param {
>                              [AK] - There should be a result field. It size should be equal to
> the size
>                              of modulus. Same apply to mod mult inverse. It should be
> driver responsability to check if result
>                              will not overflow [Shally] so these are in-place operation.
> Output will be written back to base param. It also imply length of allocated
> array should be >= modulus length which is passed in session param.
[AK-v2] I would abandon in-place/oop approach at all in asymmetric. For symmetric reason for in-place is that very often structure of packet is almost intact (macs, ip addresses, ttl etc are changed but structure remains the same, it may differ for IPSec ESP mode etc). For asymmetric it is mainly used for handshakes (for example in TLS RSA use case client will send 48byte of pre master secret which server will use to generate master secret after decryption). I generally don't think asymmetric crypto can be used as symmetric especially that only RSA would be (to some extent) capable of it.
> 
>                              [AK] - Any particular reason modulus and exponent is in
> session? Not saying
>                              it is wrong but is it for DH, RSA use cases only?
> [Shally] no that's not the intent. For RSA and DH respective xforms have been
> defined. It is kept in session envisioning modulus and exponent wont change
> frequently across operation but only base value.
> So once context is loaded with modulus and exponent , app can call modexp
> on different base values.
> 
>                              rte_crypto.h:39
>                              enum rte_crypto_op_status {
>                              [AK] - There will be many more status options in asymmetric,
>                              could we probably create new one for asymmetric crypto?
> Even if asymmetric and symmetric
>                              overlap?
>                              For mod exp, mod inv potentially it will be:
>                             DIVIDING_BY_ZERO_ERROR, INVERSE_NOT_EXISTS_ERROR...
> [Shally] So far RTE_CRYPTO_OP_STATUS_INVALID_PARAM has been
> sufficient for such cases. Do you have any use-cases where you need specific
> error code to indicate asym specific error codes?
There would be many examples, one of which:
[AK-v2] Ok, still to discussion i think though.
> 
>               rte_crypto_asym.h:33
>                              size_t length;
>                              /**< length of data in bytes */
>                              [AK] - Is it guaranteed to be length of actual data, not
> allocated memory (i mean no leading 0ed bytes), so the most significant bit
> will be in data[0]?
> [Shally] it should be length of actual data not length of allocated memory to
> an array.
> However it might create bit confusion on modular exponentiation op param
> as that expect length passed should tell actual data length in base array but
> array itself should be allocated upto modulus length.
[AK-v2] - so it is maybe good idea to have allocated data in bytes and actual len in bits here.
> 
>                              [AK] - could it be uint16/32_t instead as size_t can have
> different sizes in different implementations, uint16_t should be enough
>                              for all algorithms big integer sizes [Shally] no hard choices
> here though. But size_t would never be less than uint16_t so it guarantee to
> be large enough for any machines
> 
>               rte_crypto_asym.h:74, 250, 257, 351
>                              /**< Modular Inverse
>                              [AK] - Modular Multiplicative Inverse
>     [Shally] Ack.
> 
> Thanks,
> Arek
> 
> 

Re: [dpdk-dev] [RFC] cryptodev/asymm: propose changes to modexp and modinv API

[Trahe, Fiona]

Hi Shally, Arek,

> -----Original Message-----
> From: Kusztal, ArkadiuszX
> Sent: Monday, December 17, 2018 7:25 AM
> To: Verma, Shally <Shally.Verma@cavium.com>
> Cc: dev@dpdk.org; Trahe, Fiona <fiona.trahe@intel.com>; Doherty, Declan
> <declan.doherty@intel.com>; Kanaka Durga Kotamarthy <kkotamarthy@marvell.com>; Sunila Sahu
> <ssahu@marvell.com>; Kotamarthy, Kanaka <Kanaka.Kotamarthy@cavium.com>; Sahu, Sunila
> <Sunila.Sahu@cavium.com>; Cel, TomaszX <tomaszx.cel@intel.com>; Jozwiak, TomaszX
> <tomaszx.jozwiak@intel.com>
> Subject: RE: [RFC] cryptodev/asymm: propose changes to modexp and modinv API
> 
> >               rte_crypto_asym.h:233
> >                              rte_crypto_param modulus;
> >                              /**< modulus
> >                              * Prime modulus of the modexp transform operation in
> > octet-string
> >                              * network byte order format.
> >                              */
> >                              [AK] - Why prime? RSA for example use semi-prime or "RSA
> > multi-prime".
> >                              It should be just any positive integer.
> > [Shally] Hmm.. yes you're right . by the purpose of it , it is a semi-prime
> > input.. so prime shouldn't be mentioned here.
> [AK-v2] I think it could be any nonzero number even composite, for DH, DSA it would be prime etc.
> >                              [AK] - If session API layer should check if it is non-zero and
> > set flag accordingly.
> > [Shally] Sorry I didn't get this.. which flag you mean here? if modulus value 0
> > is passed, it should be considered as INVALID_PARAM.
> [AK-v2] - INVALID_PARAM is perfectly fine for me :).
[Fiona] About where the error checking should be done. 
In symmetric crypto the API layer doesn't do param checking it's up to the PMD.
But it seems, for asymm at least, like a good idea that the API layer checks for valid session
params, that way all PMDs benefit, rather than having multiple implementations do the same checks.
@Arek, I think that's what you're suggesting?
For op params I'm not so sure, the API layer probably shouldn't open up the op and should just pass to the 
PMD, to enable performance optimisation. The PMD generally does very little checking on the data-path
 - but I think for asymm at least divide-by-zero cases should be checked by the PMDs. 

Re: [dpdk-dev] [RFC] cryptodev/asymm: propose changes to modexp and modinv API

[Verma, Shally]

HI Arek, Fiona

>-----Original Message-----
>From: Kusztal, ArkadiuszX <arkadiuszx.kusztal@intel.com>
>Sent: 17 December 2018 19:55
>To: Verma, Shally <Shally.Verma@cavium.com>
>Cc: dev@dpdk.org; Trahe, Fiona <fiona.trahe@intel.com>; Doherty, Declan <declan.doherty@intel.com>; Kanaka Durga Kotamarthy
><kkotamarthy@marvell.com>; Sunila Sahu <ssahu@marvell.com>; Kotamarthy, Kanaka <Kanaka.Kotamarthy@cavium.com>; Sahu,
>Sunila <Sunila.Sahu@cavium.com>; Cel, TomaszX <tomaszx.cel@intel.com>; Jozwiak, TomaszX <tomaszx.jozwiak@intel.com>
>Subject: RE: [RFC] cryptodev/asymm: propose changes to modexp and modinv API
>
>External Email
>
>Hi Shally,
>
>Thanks for your answers :).
>
>My answers in [AK-v2]
>
>> -----Original Message-----
>> From: Verma, Shally [mailto:Shally.Verma@cavium.com]
>> Sent: Monday, December 17, 2018 6:45 AM
>> To: Kusztal, ArkadiuszX <arkadiuszx.kusztal@intel.com>
>> Cc: dev@dpdk.org; Trahe, Fiona <fiona.trahe@intel.com>; Doherty, Declan
>> <declan.doherty@intel.com>; Kanaka Durga Kotamarthy
>> <kkotamarthy@marvell.com>; Sunila Sahu <ssahu@marvell.com>;
>> Kotamarthy, Kanaka <Kanaka.Kotamarthy@cavium.com>; Sahu, Sunila
>> <Sunila.Sahu@cavium.com>
>> Subject: RE: [RFC] cryptodev/asymm: propose changes to modexp and
>> modinv API
>>
>> HI Arek
>>
>> Sorry for late response. Please see response inline
>>
>> From: Kusztal, ArkadiuszX <arkadiuszx.kusztal@intel.com>
>> Sent: 13 December 2018 01:56
>> To: Verma, Shally <Shally.Verma@cavium.com>
>> Cc: dev@dpdk.org; Trahe, Fiona <fiona.trahe@intel.com>; Doherty, Declan
>> <declan.doherty@intel.com>
>> Subject: [RFC] cryptodev/asymm: propose changes to modexp and modinv
>> API
>>
>> External Email
>> Hi Shally,
>>
>> I'm implementing a crypto asymmetric PMD and have some concerns about
>> the API which I will work through over the next few months. Starting with
>> modexp and modinv I have the following questions / suggestions:
>>
>>               rte_crypto_asym.h:233
>>                              rte_crypto_param modulus;
>>                              /**< modulus
>>                              * Prime modulus of the modexp transform operation in
>> octet-string
>>                              * network byte order format.
>>                              */
>>                              [AK] - Why prime? RSA for example use semi-prime or "RSA
>> multi-prime".
>>                              It should be just any positive integer.
>> [Shally] Hmm.. yes you're right . by the purpose of it , it is a semi-prime
>> input.. so prime shouldn't be mentioned here.
>[AK-v2] I think it could be any nonzero number even composite, for DH, DSA it would be prime etc.
>>                              [AK] - If session API layer should check if it is non-zero and
>> set flag accordingly.
>> [Shally] Sorry I didn't get this.. which flag you mean here? if modulus value 0
>> is passed, it should be considered as INVALID_PARAM.
>[AK-v2] - INVALID_PARAM is perfectly fine for me :).

[Shally] Just to club Fiona response here to which I agree. If you intend to check modulus value to 0 in session_init, then cryptodev lib asym_session_configure API  can check for Invalid Param.
For any invalid op during enqueue/dequeue it should be in PMD. 

>>
>>               rte_crypto_asym.h:253
>>                              rte_crypto_param modulus;
>>                              /**<
>>                              * Pointer to the prime modulus data for modular
>>                              * inverse operation in octet-string network byte
>>                              * order format.
>>                              */
>>                              [AK] - Same situation as for mod exp. Just any number.
>> [Shally] Yea. It should be reworded as modulus data instead of *prime*
>> modulus data
>>
>>                              For example when using with RSA Carmichael and Euler
>> Totient function will even
>>                              have composite factors.
>>
>>               rte_crypto_asym.h:323
>>                              struct rte_crypto_mod_op_param {
>>                              [AK] - There should be a result field. It size should be equal to
>> the size
>>                              of modulus. Same apply to mod mult inverse. It should be
>> driver responsability to check if result
>>                              will not overflow [Shally] so these are in-place operation.
>> Output will be written back to base param. It also imply length of allocated
>> array should be >= modulus length which is passed in session param.
>[AK-v2] I would abandon in-place/oop approach at all in asymmetric. For symmetric reason for in-place is that very often structure of
>packet is almost intact (macs, ip addresses, ttl etc are changed but structure remains the same, it may differ for IPSec ESP mode etc).
>For asymmetric it is mainly used for handshakes (for example in TLS RSA use case client will send 48byte of pre master secret which
>server will use to generate master secret after decryption). I generally don't think asymmetric crypto can be used as symmetric
>especially that only RSA would be (to some extent) capable of it.

[Shally] So you suggest all asym ops should be out of place? Am okay with add that. However would like to ask if anyone has preference to keep in-place option in Asym.
If so, then we would need to add Feature flag indicating in-place processing capability.

>>
>>                              [AK] - Any particular reason modulus and exponent is in
>> session? Not saying
>>                              it is wrong but is it for DH, RSA use cases only?
>> [Shally] no that's not the intent. For RSA and DH respective xforms have been
>> defined. It is kept in session envisioning modulus and exponent wont change
>> frequently across operation but only base value.
>> So once context is loaded with modulus and exponent , app can call modexp
>> on different base values.
>>
>>                              rte_crypto.h:39
>>                              enum rte_crypto_op_status {
>>                              [AK] - There will be many more status options in asymmetric,
>>                              could we probably create new one for asymmetric crypto?
>> Even if asymmetric and symmetric
>>                              overlap?
>>                              For mod exp, mod inv potentially it will be:
>>                             DIVIDING_BY_ZERO_ERROR, INVERSE_NOT_EXISTS_ERROR...
>> [Shally] So far RTE_CRYPTO_OP_STATUS_INVALID_PARAM has been
>> sufficient for such cases. Do you have any use-cases where you need specific
>> error code to indicate asym specific error codes?
>There would be many examples, one of which:
>[AK-v2] Ok, still to discussion i think though.
>>
>>               rte_crypto_asym.h:33
>>                              size_t length;
>>                              /**< length of data in bytes */
>>                              [AK] - Is it guaranteed to be length of actual data, not
>> allocated memory (i mean no leading 0ed bytes), so the most significant bit
>> will be in data[0]?
>> [Shally] it should be length of actual data not length of allocated memory to
>> an array.
>> However it might create bit confusion on modular exponentiation op param
>> as that expect length passed should tell actual data length in base array but
>> array itself should be allocated upto modulus length.
>[AK-v2] - so it is maybe good idea to have allocated data in bytes and actual len in bits here.

[Shally] No that will make it complex and breaks compatibility too. I would propose to keep it in bytes which states length of actual data present in array.
Any confusion around it will be resolved if we add out of place or proper documentation if in-place is retained.

I would suggest you to send a patch with things that we agree or you propose. We can discuss on that further.

Thanks
Shally
>>
>>                              [AK] - could it be uint16/32_t instead as size_t can have
>> different sizes in different implementations, uint16_t should be enough
>>                              for all algorithms big integer sizes [Shally] no hard choices
>> here though. But size_t would never be less than uint16_t so it guarantee to
>> be large enough for any machines
>>
>>               rte_crypto_asym.h:74, 250, 257, 351
>>                              /**< Modular Inverse
>>                              [AK] - Modular Multiplicative Inverse
>>     [Shally] Ack.
>>
>> Thanks,
>> Arek
>>
>>

Re: [dpdk-dev] [RFC] cryptodev/asymm: propose changes to modexp and modinv API

[Trahe, Fiona]

Hi Shally, Arek,

> -----Original Message-----
> From: Verma, Shally [mailto:Shally.Verma@cavium.com]
> Sent: Tuesday, December 18, 2018 6:54 AM
> To: Kusztal, ArkadiuszX <arkadiuszx.kusztal@intel.com>
> Cc: dev@dpdk.org; Trahe, Fiona <fiona.trahe@intel.com>; Doherty, Declan
> <declan.doherty@intel.com>; Kanaka Durga Kotamarthy <kkotamarthy@marvell.com>; Sunila Sahu
> <ssahu@marvell.com>; Kotamarthy, Kanaka <Kanaka.Kotamarthy@cavium.com>; Sahu, Sunila
> <Sunila.Sahu@cavium.com>; Cel, TomaszX <tomaszx.cel@intel.com>; Jozwiak, TomaszX
> <tomaszx.jozwiak@intel.com>
> Subject: RE: [RFC] cryptodev/asymm: propose changes to modexp and modinv API
> 
> HI Arek, Fiona
> 
> >-----Original Message-----
> >From: Kusztal, ArkadiuszX <arkadiuszx.kusztal@intel.com>
> >Sent: 17 December 2018 19:55
> >To: Verma, Shally <Shally.Verma@cavium.com>
> >Cc: dev@dpdk.org; Trahe, Fiona <fiona.trahe@intel.com>; Doherty, Declan
> <declan.doherty@intel.com>; Kanaka Durga Kotamarthy
> ><kkotamarthy@marvell.com>; Sunila Sahu <ssahu@marvell.com>; Kotamarthy, Kanaka
> <Kanaka.Kotamarthy@cavium.com>; Sahu,
> >Sunila <Sunila.Sahu@cavium.com>; Cel, TomaszX <tomaszx.cel@intel.com>; Jozwiak, TomaszX
> <tomaszx.jozwiak@intel.com>
> >Subject: RE: [RFC] cryptodev/asymm: propose changes to modexp and modinv API
> >
> >External Email
> >
> >Hi Shally,
> >
> >Thanks for your answers :).
> >
> >My answers in [AK-v2]
> >
> >> -----Original Message-----
> >> From: Verma, Shally [mailto:Shally.Verma@cavium.com]
> >> Sent: Monday, December 17, 2018 6:45 AM
> >> To: Kusztal, ArkadiuszX <arkadiuszx.kusztal@intel.com>
> >> Cc: dev@dpdk.org; Trahe, Fiona <fiona.trahe@intel.com>; Doherty, Declan
> >> <declan.doherty@intel.com>; Kanaka Durga Kotamarthy
> >> <kkotamarthy@marvell.com>; Sunila Sahu <ssahu@marvell.com>;
> >> Kotamarthy, Kanaka <Kanaka.Kotamarthy@cavium.com>; Sahu, Sunila
> >> <Sunila.Sahu@cavium.com>
> >> Subject: RE: [RFC] cryptodev/asymm: propose changes to modexp and
> >> modinv API
> >>
> >> HI Arek
> >>
> >> Sorry for late response. Please see response inline
> >>
> >> From: Kusztal, ArkadiuszX <arkadiuszx.kusztal@intel.com>
> >> Sent: 13 December 2018 01:56
> >> To: Verma, Shally <Shally.Verma@cavium.com>
> >> Cc: dev@dpdk.org; Trahe, Fiona <fiona.trahe@intel.com>; Doherty, Declan
> >> <declan.doherty@intel.com>
> >> Subject: [RFC] cryptodev/asymm: propose changes to modexp and modinv
> >> API
> >>
> >> External Email
> >> Hi Shally,
> >>
> >> I'm implementing a crypto asymmetric PMD and have some concerns about
> >> the API which I will work through over the next few months. Starting with
> >> modexp and modinv I have the following questions / suggestions:
> >>
> >>               rte_crypto_asym.h:233
> >>                              rte_crypto_param modulus;
> >>                              /**< modulus
> >>                              * Prime modulus of the modexp transform operation in
> >> octet-string
> >>                              * network byte order format.
> >>                              */
> >>                              [AK] - Why prime? RSA for example use semi-prime or "RSA
> >> multi-prime".
> >>                              It should be just any positive integer.
> >> [Shally] Hmm.. yes you're right . by the purpose of it , it is a semi-prime
> >> input.. so prime shouldn't be mentioned here.
> >[AK-v2] I think it could be any nonzero number even composite, for DH, DSA it would be prime etc.
> >>                              [AK] - If session API layer should check if it is non-zero and
> >> set flag accordingly.
> >> [Shally] Sorry I didn't get this.. which flag you mean here? if modulus value 0
> >> is passed, it should be considered as INVALID_PARAM.
> >[AK-v2] - INVALID_PARAM is perfectly fine for me :).
> 
> [Shally] Just to club Fiona response here to which I agree. If you intend to check modulus value to 0 in
> session_init, then cryptodev lib asym_session_configure API  can check for Invalid Param.
> For any invalid op during enqueue/dequeue it should be in PMD.
> 
> >>
> >>               rte_crypto_asym.h:253
> >>                              rte_crypto_param modulus;
> >>                              /**<
> >>                              * Pointer to the prime modulus data for modular
> >>                              * inverse operation in octet-string network byte
> >>                              * order format.
> >>                              */
> >>                              [AK] - Same situation as for mod exp. Just any number.
> >> [Shally] Yea. It should be reworded as modulus data instead of *prime*
> >> modulus data
> >>
> >>                              For example when using with RSA Carmichael and Euler
> >> Totient function will even
> >>                              have composite factors.
> >>
> >>               rte_crypto_asym.h:323
> >>                              struct rte_crypto_mod_op_param {
> >>                              [AK] - There should be a result field. It size should be equal to
> >> the size
> >>                              of modulus. Same apply to mod mult inverse. It should be
> >> driver responsability to check if result
> >>                              will not overflow [Shally] so these are in-place operation.
> >> Output will be written back to base param. It also imply length of allocated
> >> array should be >= modulus length which is passed in session param.
> >[AK-v2] I would abandon in-place/oop approach at all in asymmetric. For symmetric reason for in-
> place is that very often structure of
> >packet is almost intact (macs, ip addresses, ttl etc are changed but structure remains the same, it
> may differ for IPSec ESP mode etc).
> >For asymmetric it is mainly used for handshakes (for example in TLS RSA use case client will send
> 48byte of pre master secret which
> >server will use to generate master secret after decryption). I generally don't think asymmetric crypto
> can be used as symmetric
> >especially that only RSA would be (to some extent) capable of it.
> 
> [Shally] So you suggest all asym ops should be out of place? Am okay with add that. However would
> like to ask if anyone has preference to keep in-place option in Asym.
> If so, then we would need to add Feature flag indicating in-place processing capability.
[Fiona] I'm ok with dropping all in-place for asymm. As there are multiple inputs and output for various
algos it would be quite difficult to craft set of capabilities to reflect which field(s) was used for in-place result.
I don't see a need for in-place, would use out-of-place for all.

> >>                              [AK] - Any particular reason modulus and exponent is in
> >> session? Not saying
> >>                              it is wrong but is it for DH, RSA use cases only?
> >> [Shally] no that's not the intent. For RSA and DH respective xforms have been
> >> defined. It is kept in session envisioning modulus and exponent wont change
> >> frequently across operation but only base value.
> >> So once context is loaded with modulus and exponent , app can call modexp
> >> on different base values.
> >>
> >>                              rte_crypto.h:39
> >>                              enum rte_crypto_op_status {
> >>                              [AK] - There will be many more status options in asymmetric,
> >>                              could we probably create new one for asymmetric crypto?
> >> Even if asymmetric and symmetric
> >>                              overlap?
> >>                              For mod exp, mod inv potentially it will be:
> >>                             DIVIDING_BY_ZERO_ERROR, INVERSE_NOT_EXISTS_ERROR...
> >> [Shally] So far RTE_CRYPTO_OP_STATUS_INVALID_PARAM has been
> >> sufficient for such cases. Do you have any use-cases where you need specific
> >> error code to indicate asym specific error codes?
> >There would be many examples, one of which:
> >[AK-v2] Ok, still to discussion i think though.
> >>
> >>               rte_crypto_asym.h:33
> >>                              size_t length;
> >>                              /**< length of data in bytes */
> >>                              [AK] - Is it guaranteed to be length of actual data, not
> >> allocated memory (i mean no leading 0ed bytes), so the most significant bit
> >> will be in data[0]?
> >> [Shally] it should be length of actual data not length of allocated memory to
> >> an array.
> >> However it might create bit confusion on modular exponentiation op param
> >> as that expect length passed should tell actual data length in base array but
> >> array itself should be allocated upto modulus length.
> >[AK-v2] - so it is maybe good idea to have allocated data in bytes and actual len in bits here.
> 
> [Shally] No that will make it complex and breaks compatibility too. I would propose to keep it in bytes
> which states length of actual data present in array.
> Any confusion around it will be resolved if we add out of place or proper documentation if in-place is
> retained.
> 
> I would suggest you to send a patch with things that we agree or you propose. We can discuss on that
> further.
[Fiona] yes, agreed, Arek will send a patch.

> Thanks
> Shally
> >>
> >>                              [AK] - could it be uint16/32_t instead as size_t can have
> >> different sizes in different implementations, uint16_t should be enough
> >>                              for all algorithms big integer sizes [Shally] no hard choices
> >> here though. But size_t would never be less than uint16_t so it guarantee to
> >> be large enough for any machines
> >>
> >>               rte_crypto_asym.h:74, 250, 257, 351
> >>                              /**< Modular Inverse
> >>                              [AK] - Modular Multiplicative Inverse
> >>     [Shally] Ack.
> >>
> >> Thanks,
> >> Arek
> >>
> >>

Re: [dpdk-dev] [RFC] cryptodev/asymm: propose changes to modexp and modinv API

[Verma, Shally]

>-----Original Message-----
>From: Trahe, Fiona <fiona.trahe@intel.com>
>Sent: 18 December 2018 21:23
>To: Verma, Shally <Shally.Verma@cavium.com>; Kusztal, ArkadiuszX <arkadiuszx.kusztal@intel.com>
>Cc: dev@dpdk.org; Doherty, Declan <declan.doherty@intel.com>; Kanaka Durga Kotamarthy <kkotamarthy@marvell.com>; Sunila
>Sahu <ssahu@marvell.com>; Kotamarthy, Kanaka <Kanaka.Kotamarthy@cavium.com>; Sahu, Sunila <Sunila.Sahu@cavium.com>; Cel,
>TomaszX <tomaszx.cel@intel.com>; Jozwiak, TomaszX <tomaszx.jozwiak@intel.com>; Trahe, Fiona <fiona.trahe@intel.com>
>Subject: RE: [RFC] cryptodev/asymm: propose changes to modexp and modinv API
>
>External Email
>
>Hi Shally, Arek,
>
...

>> >>
>> >>               rte_crypto_asym.h:323
>> >>                              struct rte_crypto_mod_op_param {
>> >>                              [AK] - There should be a result field. It size should be equal to
>> >> the size
>> >>                              of modulus. Same apply to mod mult inverse. It should be
>> >> driver responsability to check if result
>> >>                              will not overflow [Shally] so these are in-place operation.
>> >> Output will be written back to base param. It also imply length of allocated
>> >> array should be >= modulus length which is passed in session param.
>> >[AK-v2] I would abandon in-place/oop approach at all in asymmetric. For symmetric reason for in-
>> place is that very often structure of
>> >packet is almost intact (macs, ip addresses, ttl etc are changed but structure remains the same, it
>> may differ for IPSec ESP mode etc).
>> >For asymmetric it is mainly used for handshakes (for example in TLS RSA use case client will send
>> 48byte of pre master secret which
>> >server will use to generate master secret after decryption). I generally don't think asymmetric crypto
>> can be used as symmetric
>> >especially that only RSA would be (to some extent) capable of it.
>>
>> [Shally] So you suggest all asym ops should be out of place? Am okay with add that. However would
>> like to ask if anyone has preference to keep in-place option in Asym.
>> If so, then we would need to add Feature flag indicating in-place processing capability.
>[Fiona] I'm ok with dropping all in-place for asymm. As there are multiple inputs and output for various
>algos it would be quite difficult to craft set of capabilities to reflect which field(s) was used for in-place result.
>I don't see a need for in-place, would use out-of-place for all.
>
[Shally] Am okay with that too. So, Arek will you send patches with these changes?

Thanks
Shally

>> >>                              [AK] - Any particular reason modulus and exponent is in
>> >> session? Not saying
>> >>                              it is wrong but is it for DH, RSA use cases only?
>> >> [Shally] no that's not the intent. For RSA and DH respective xforms have been
>> >> defined. It is kept in session envisioning modulus and exponent wont change
>> >> frequently across operation but only base value.
>> >> So once context is loaded with modulus and exponent , app can call modexp
>> >> on different base values.
>> >>
>> >>                              rte_crypto.h:39
>> >>                              enum rte_crypto_op_status {
>> >>                              [AK] - There will be many more status options in asymmetric,
>> >>                              could we probably create new one for asymmetric crypto?
>> >> Even if asymmetric and symmetric
>> >>                              overlap?
>> >>                              For mod exp, mod inv potentially it will be:
>> >>                             DIVIDING_BY_ZERO_ERROR, INVERSE_NOT_EXISTS_ERROR...
>> >> [Shally] So far RTE_CRYPTO_OP_STATUS_INVALID_PARAM has been
>> >> sufficient for such cases. Do you have any use-cases where you need specific
>> >> error code to indicate asym specific error codes?
>> >There would be many examples, one of which:
>> >[AK-v2] Ok, still to discussion i think though.
>> >>
>> >>               rte_crypto_asym.h:33
>> >>                              size_t length;
>> >>                              /**< length of data in bytes */
>> >>                              [AK] - Is it guaranteed to be length of actual data, not
>> >> allocated memory (i mean no leading 0ed bytes), so the most significant bit
>> >> will be in data[0]?
>> >> [Shally] it should be length of actual data not length of allocated memory to
>> >> an array.
>> >> However it might create bit confusion on modular exponentiation op param
>> >> as that expect length passed should tell actual data length in base array but
>> >> array itself should be allocated upto modulus length.
>> >[AK-v2] - so it is maybe good idea to have allocated data in bytes and actual len in bits here.
>>
>> [Shally] No that will make it complex and breaks compatibility too. I would propose to keep it in bytes
>> which states length of actual data present in array.
>> Any confusion around it will be resolved if we add out of place or proper documentation if in-place is
>> retained.
>>
>> I would suggest you to send a patch with things that we agree or you propose. We can discuss on that
>> further.
>[Fiona] yes, agreed, Arek will send a patch.
>
>> Thanks
>> Shally
>> >>
>> >>                              [AK] - could it be uint16/32_t instead as size_t can have
>> >> different sizes in different implementations, uint16_t should be enough
>> >>                              for all algorithms big integer sizes [Shally] no hard choices
>> >> here though. But size_t would never be less than uint16_t so it guarantee to
>> >> be large enough for any machines
>> >>
>> >>               rte_crypto_asym.h:74, 250, 257, 351
>> >>                              /**< Modular Inverse
>> >>                              [AK] - Modular Multiplicative Inverse
>> >>     [Shally] Ack.
>> >>
>> >> Thanks,
>> >> Arek
>> >>
>> >>

Re: [dpdk-dev] [RFC] cryptodev/asymm: propose changes to modexp and modinv API

[Kusztal, ArkadiuszX]

Hi Shally, Fiona,

> -----Original Message-----
> From: Verma, Shally [mailto:Shally.Verma@cavium.com]
> Sent: Thursday, December 20, 2018 11:53 AM
> To: Trahe, Fiona <fiona.trahe@intel.com>; Kusztal, ArkadiuszX
> <arkadiuszx.kusztal@intel.com>
> Cc: dev@dpdk.org; Doherty, Declan <declan.doherty@intel.com>; Kanaka
> Durga Kotamarthy <kkotamarthy@marvell.com>; Sunila Sahu
> <ssahu@marvell.com>; Kotamarthy, Kanaka
> <Kanaka.Kotamarthy@cavium.com>; Sahu, Sunila
> <Sunila.Sahu@cavium.com>; Cel, TomaszX <tomaszx.cel@intel.com>;
> Jozwiak, TomaszX <tomaszx.jozwiak@intel.com>
> Subject: RE: [RFC] cryptodev/asymm: propose changes to modexp and
> modinv API
> 
> 
> 
> >-----Original Message-----
> >From: Trahe, Fiona <fiona.trahe@intel.com>
> >Sent: 18 December 2018 21:23
> >To: Verma, Shally <Shally.Verma@cavium.com>; Kusztal, ArkadiuszX
> ><arkadiuszx.kusztal@intel.com>
> >Cc: dev@dpdk.org; Doherty, Declan <declan.doherty@intel.com>; Kanaka
> >Durga Kotamarthy <kkotamarthy@marvell.com>; Sunila Sahu
> ><ssahu@marvell.com>; Kotamarthy, Kanaka
> <Kanaka.Kotamarthy@cavium.com>;
> >Sahu, Sunila <Sunila.Sahu@cavium.com>; Cel, TomaszX
> ><tomaszx.cel@intel.com>; Jozwiak, TomaszX
> <tomaszx.jozwiak@intel.com>;
> >Trahe, Fiona <fiona.trahe@intel.com>
> >Subject: RE: [RFC] cryptodev/asymm: propose changes to modexp and
> >modinv API
> >
> >External Email
> >
> >Hi Shally, Arek,
> >
> ...
> 
> >> >>
> >> >>               rte_crypto_asym.h:323
> >> >>                              struct rte_crypto_mod_op_param {
> >> >>                              [AK] - There should be a result
> >> >> field. It size should be equal to the size
> >> >>                              of modulus. Same apply to mod mult
> >> >> inverse. It should be driver responsability to check if result
> >> >>                              will not overflow [Shally] so these are in-place
> operation.
> >> >> Output will be written back to base param. It also imply length of
> >> >> allocated array should be >= modulus length which is passed in session
> param.
> >> >[AK-v2] I would abandon in-place/oop approach at all in asymmetric.
> >> >For symmetric reason for in-
> >> place is that very often structure of
> >> >packet is almost intact (macs, ip addresses, ttl etc are changed but
> >> >structure remains the same, it
> >> may differ for IPSec ESP mode etc).
> >> >For asymmetric it is mainly used for handshakes (for example in TLS
> >> >RSA use case client will send
> >> 48byte of pre master secret which
> >> >server will use to generate master secret after decryption). I
> >> >generally don't think asymmetric crypto
> >> can be used as symmetric
> >> >especially that only RSA would be (to some extent) capable of it.
> >>
> >> [Shally] So you suggest all asym ops should be out of place? Am okay
> >> with add that. However would like to ask if anyone has preference to keep
> in-place option in Asym.
> >> If so, then we would need to add Feature flag indicating in-place
> processing capability.
> >[Fiona] I'm ok with dropping all in-place for asymm. As there are
> >multiple inputs and output for various algos it would be quite difficult to
> craft set of capabilities to reflect which field(s) was used for in-place result.
> >I don't see a need for in-place, would use out-of-place for all.
> >
> [Shally] Am okay with that too. So, Arek will you send patches with these
> changes?

Yes, I will send patches.

Thanks,
Arek

> 
> Thanks
> Shally
> 
> >> >>                              [AK] - Any particular reason modulus
> >> >> and exponent is in session? Not saying
> >> >>                              it is wrong but is it for DH, RSA use cases only?
> >> >> [Shally] no that's not the intent. For RSA and DH respective
> >> >> xforms have been defined. It is kept in session envisioning
> >> >> modulus and exponent wont change frequently across operation but
> only base value.
> >> >> So once context is loaded with modulus and exponent , app can call
> >> >> modexp on different base values.
> >> >>
> >> >>                              rte_crypto.h:39
> >> >>                              enum rte_crypto_op_status {
> >> >>                              [AK] - There will be many more status options in
> asymmetric,
> >> >>                              could we probably create new one for asymmetric
> crypto?
> >> >> Even if asymmetric and symmetric
> >> >>                              overlap?
> >> >>                              For mod exp, mod inv potentially it will be:
> >> >>                             DIVIDING_BY_ZERO_ERROR,
> INVERSE_NOT_EXISTS_ERROR...
> >> >> [Shally] So far RTE_CRYPTO_OP_STATUS_INVALID_PARAM has been
> >> >> sufficient for such cases. Do you have any use-cases where you
> >> >> need specific error code to indicate asym specific error codes?
> >> >There would be many examples, one of which:
> >> >[AK-v2] Ok, still to discussion i think though.
> >> >>
> >> >>               rte_crypto_asym.h:33
> >> >>                              size_t length;
> >> >>                              /**< length of data in bytes */
> >> >>                              [AK] - Is it guaranteed to be length
> >> >> of actual data, not allocated memory (i mean no leading 0ed
> >> >> bytes), so the most significant bit will be in data[0]?
> >> >> [Shally] it should be length of actual data not length of
> >> >> allocated memory to an array.
> >> >> However it might create bit confusion on modular exponentiation op
> >> >> param as that expect length passed should tell actual data length
> >> >> in base array but array itself should be allocated upto modulus length.
> >> >[AK-v2] - so it is maybe good idea to have allocated data in bytes and
> actual len in bits here.
> >>
> >> [Shally] No that will make it complex and breaks compatibility too. I
> >> would propose to keep it in bytes which states length of actual data
> present in array.
> >> Any confusion around it will be resolved if we add out of place or
> >> proper documentation if in-place is retained.
> >>
> >> I would suggest you to send a patch with things that we agree or you
> >> propose. We can discuss on that further.
> >[Fiona] yes, agreed, Arek will send a patch.
> >
> >> Thanks
> >> Shally
> >> >>
> >> >>                              [AK] - could it be uint16/32_t
> >> >> instead as size_t can have different sizes in different implementations,
> uint16_t should be enough
> >> >>                              for all algorithms big integer sizes
> >> >> [Shally] no hard choices here though. But size_t would never be
> >> >> less than uint16_t so it guarantee to be large enough for any
> >> >> machines
> >> >>
> >> >>               rte_crypto_asym.h:74, 250, 257, 351
> >> >>                              /**< Modular Inverse
> >> >>                              [AK] - Modular Multiplicative Inverse
> >> >>     [Shally] Ack.
> >> >>
> >> >> Thanks,
> >> >> Arek
> >> >>
> >> >>