HI Arek

Sorry for late response. Please see response inline

From: Kusztal, ArkadiuszX <arkadiuszx.kusz...@intel.com> 
Sent: 13 December 2018 01:56
To: Verma, Shally <shally.ve...@cavium.com>
Cc: dev@dpdk.org; Trahe, Fiona <fiona.tr...@intel.com>; Doherty, Declan 
<declan.dohe...@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
              
              

Reply via email to