Re: [dpdk-dev] [PATCH v2 1/3] cryptodev: rework api of rsa algorithm

["Kusztal, ArkadiuszX" <arkadiuszx.kusztal@intel.com>]

Hi Akhil, Shally, Fiona

I thought to split it in smaller patches, which from some could be applied in this release.
And I can send these patches today.

- 4 important enough to put in this release are:

1. Remove RSA prime modulus nonsense.
2. Add cipher field.
3. Specify what data should be provided for signing (in current only working case - openssl with pkcs1 user would need to append ASN.1 DER encoded algorithmIdentifier and concat it with message digest. You can easily verify it with functions RSA_sign, RSA_verify from openssl).
4. Remove BT0 padding.
5. Padding NONE behavior
6. Fix grammar and bad increment  in simulate pkcs_1 function in test for block type 1.

- Others and opens are:
1. Creating padding struct
2. Padding parameters (seedlen, optional label etc)
3. Leading zeroes questions.
4. Random number requirements.
5. Capabilities.

> -----Original Message-----
> From: Akhil Goyal [mailto:akhil.goyal@nxp.com]
> Sent: Tuesday, July 16, 2019 3:51 PM
> To: Kusztal, ArkadiuszX <arkadiuszx.kusztal@intel.com>; Shally Verma
> <shallyv@marvell.com>; dev@dpdk.org
> Cc: Trahe, Fiona <fiona.trahe@intel.com>
> Subject: RE: [PATCH v2 1/3] cryptodev: rework api of rsa algorithm
> 
> Hi Shally,
> 
> Do you have further comments on this series?
> 
> If yes, we may need to defer this to next release. As cryptodev change should
> not be there beyond RC2. In fact all of them should be closed by RC1.
> 
> Regards,
> Akhil
> 
> > -----Original Message-----
> > From: Kusztal, ArkadiuszX <arkadiuszx.kusztal@intel.com>
> > Sent: Tuesday, July 9, 2019 3:33 PM
> > To: Shally Verma <shallyv@marvell.com>; dev@dpdk.org
> > Cc: Akhil Goyal <akhil.goyal@nxp.com>; Trahe, Fiona
> > <fiona.trahe@intel.com>
> > Subject: RE: [PATCH v2 1/3] cryptodev: rework api of rsa algorithm
> >
> > To clarify bit more
> > With [AK2]
> >
> > > -----Original Message-----
> > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Kusztal,
> > > ArkadiuszX
> > > Sent: Monday, July 8, 2019 7:44 PM
> > > To: Shally Verma <shallyv@marvell.com>; dev@dpdk.org
> > > Cc: akhil.goyal@nxp.com; Trahe, Fiona <fiona.trahe@intel.com>;
> > > shally.verma@caviumnetworks.com
> > > Subject: Re: [dpdk-dev] [PATCH v2 1/3] cryptodev: rework api of rsa
> > > algorithm
> > >
> > > Hi Shally,
> > >
> > > With [AK]
> > >
> > > > -----Original Message-----
> > > > From: Shally Verma [mailto:shallyv@marvell.com]
> > > > Sent: Saturday, July 6, 2019 3:14 PM
> > > > To: Kusztal, ArkadiuszX <arkadiuszx.kusztal@intel.com>;
> > > > dev@dpdk.org
> > > > Cc: akhil.goyal@nxp.com; Trahe, Fiona <fiona.trahe@intel.com>;
> > > > shally.verma@caviumnetworks.com
> > > > Subject: RE: [PATCH v2 1/3] cryptodev: rework api of rsa algorithm
> > > >
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: Kusztal, ArkadiuszX <arkadiuszx.kusztal@intel.com>
> > > > > Sent: Thursday, July 4, 2019 6:10 PM
> > > > > To: dev@dpdk.org
> > > > > Cc: akhil.goyal@nxp.com; Trahe, Fiona <fiona.trahe@intel.com>;
> > > > > shally.verma@caviumnetworks.com; Shally Verma
> > > <shallyv@marvell.com>
> > > > > Subject: [EXT] RE: [PATCH v2 1/3] cryptodev: rework api of rsa
> > > > > algorithm
> > > > >
> > > > > External Email
> > > > >
> > > > ..
> > > >
> > > > > > diff --git a/lib/librte_cryptodev/rte_crypto_asym.h
> > > > > > b/lib/librte_cryptodev/rte_crypto_asym.h
> > > > > > index 8672f21..486399c 100644
> > > > > > --- a/lib/librte_cryptodev/rte_crypto_asym.h
> > > > > > +++ b/lib/librte_cryptodev/rte_crypto_asym.h
> > > > > > @@ -111,23 +111,21 @@ enum rte_crypto_asym_op_type {
> > > > > >   */
> > > > > >  enum rte_crypto_rsa_padding_type {
> > > > > >  	RTE_CRYPTO_RSA_PADDING_NONE = 0,
> > > > > > -	/**< RSA no padding scheme */
> > > > > > -	RTE_CRYPTO_RSA_PKCS1_V1_5_BT0,
> > > > > > -	/**< RSA PKCS#1 V1.5 Block Type 0 padding scheme
> > > > > > -	 * as described in rfc2313
> > > > > > +	/**< RSA no padding scheme.
> > > > > > +	 * In this case user is responsible for provision and
> verification
> > > > > > +	 * of padding.
> > > > > >  	 */
> > > > > > -	RTE_CRYPTO_RSA_PKCS1_V1_5_BT1,
> > > > > > -	/**< RSA PKCS#1 V1.5 Block Type 01 padding scheme
> > > > > > -	 * as described in rfc2313
> > > > > > -	 */
> > > > > > -	RTE_CRYPTO_RSA_PKCS1_V1_5_BT2,
> > > > > > -	/**< RSA PKCS#1 V1.5 Block Type 02 padding scheme
> > > > > > -	 * as described in rfc2313
> > > > > > +	RTE_CRYPTO_RSA_PADDING_PKCS1,
> > > > [Shally] My preference would still be to rename as PKCS1_V1.5 to
> > > > align more to standard
> > > [AK] - Agree.
> > >
> > > >
> > > > > > +	/**< RSA PKCS#1 PKCS1-v1_5 padding scheme. For signatures
> > block
> > > > > > type 01,
> > > > > > +	 * for encryption block type 02 are used.
> > > > > >  	 */
> > > > > >  	RTE_CRYPTO_RSA_PADDING_OAEP,
> > > > > > -	/**< RSA PKCS#1 OAEP padding scheme */
> > > > > > +	/**< RSA PKCS#1 OAEP padding scheme, can be used only for
> > > > > > encryption/
> > > > > > +	 * decryption.
> > > > > > +	 */
> > > > > >  	RTE_CRYPTO_RSA_PADDING_PSS,
> > > > > > -	/**< RSA PKCS#1 PSS padding scheme */
> > > > > > +	/**< RSA PKCS#1 PSS padding scheme, can be used only for
> > > > > > signatures.
> > > > > > +	 */
> > > > > >  	RTE_CRYPTO_RSA_PADDING_TYPE_LIST_END
> > > > > >  };
> > > > > >
> > > > ...
> > > >
> > > > > >  struct rte_crypto_rsa_xform {
> > > > > >  	rte_crypto_param n;
> > > > > > -	/**< n - Prime modulus
> > > > > > -	 * Prime modulus data of RSA operation in Octet-string
> network
> > > > > > +	/**< n - Modulus
> > > > > > +	 * Modulus data of RSA operation in Octet-string network
> > > > > >  	 * byte order format.
> > > > > >  	 */
> > > > > >
> > > > > > @@ -397,9 +395,36 @@ struct rte_crypto_rsa_op_param {
> > > > > >  	/**<
> > > > > >  	 * Pointer to data
> > > > > >  	 * - to be encrypted for RSA public encrypt.
> > > > > > -	 * - to be decrypted for RSA private decrypt.
> > > > > >  	 * - to be signed for RSA sign generation.
> > > > > >  	 * - to be authenticated for RSA sign verification.
> > > > > > +	 *
> > > > > > +	 * Octet-string network byte order format.
> > > > > > +	 *
> > > > > > +	 * This field is an input to RTE_CRYPTO_ASYM_OP_ENCRYPT
> > > > > > +	 * operation, and output to
> RTE_CRYPTO_ASYM_OP_DECRYPT
> > > > > > operation.
> > > > > > +	 *
> > > > > > +	 * When RTE_CRYPTO_ASYM_OP_DECRYPT op_type used
> > length in
> > > > > > bytes
> > > > > > +	 * of this field needs to be greater or equal to the length of
> > > > > > +	 * corresponding RSA key in bytes.
> > > > [Shally] better rephrased " When an op_type ASYM_OP_DECRYPT used,
> > > > length of output buffer should be greater than  or equal to RSA
> > > > key modulus length
> > > [AK] - RSA key size is a RSA modulus size, but can be changed.
> > > >
> > > > > > +	 *
> > > > > > +	 * When padding field is set to
> > RTE_CRYPTO_RSA_PADDING_NONE
> > > > > > +	 * returned data size will be equal to the size of RSA key
> > > > > > +	 * in bytes. All leading zeroes will be preserved.
> > > > > > +	 */
> > > > [Shally] Is it in context of OP_TYPE_DECRYPT? Even if it is
> > > > PADDING_NONE, whether it is encrypted/decrypted, o/p length can
> > > > vary between 0 .. RSA modulus length - 1 as perf rfc8017
> > > [AK]
> > > example. 20 bytes was encrypted with 2048bit key PKCS_1.5 1. Padding
> > > PKCS_1.5 set - Upon decryption we return 20 bytes of recovered message.
> > > 2. Padding NONE set (padding done by user) - we return 236 bytes of
> > > padding (one leading zero) | 20 bytes of message = 256 bytes.
> > > (like in example test case I have added) 3. Padding NONE set
> > > (textbook rsa) - we return 236 bytes of zeroes | 20 bytes of message =
> 256 bytes.
> > > >
> > > > > > +
> > > > > > +	rte_crypto_param cipher;
> > > > > > +	/**<
> > > > > > +	 * Pointer to data
> > > > > > +	 * - to be decrypted for RSA private decrypt.
> > > > > > +	 *
> > > > > > +	 * Octet-string network byte order format.
> > > > > > +	 *
> > > > > > +	 * This field is an input to RTE_CRYPTO_ASYM_OP_DECRYPT
> > > > > > +	 * operation, and output to
> RTE_CRYPTO_ASYM_OP_ENCRYPT
> > > > > > operation.
> > > > > > +	 *
> > > > > > +	 * When RTE_CRYPTO_ASYM_OP_ENCRYPT op_type used
> > length in
> > > > > > bytes
> > > > > > +	 * of this field needs to be greater or equal to the length of
> > > > > > +	 * corresponding RSA key in bytes.
> > > > > >  	 */
> > > > [Shally] recommend rephrasing as above
> > > >
> > > > > >
> > > > > >  	rte_crypto_param sign;
> > > > > > @@ -408,27 +433,88 @@ struct rte_crypto_rsa_op_param {
> > > > > >  	 * sign @ref RTE_CRYPTO_ASYM_OP_SIGN, buffer will be
> > > > > >  	 * over-written with generated signature.
> > > > > >  	 *
> > > > > > -	 * Length of the signature data will be equal to the
> > > > > > -	 * RSA prime modulus length.
> > > > > > +	 * Octet-string network byte order format.
> > > > > > +	 *
> > > > > > +	 * When RTE_CRYPTO_ASYM_OP_SIGN op_type used length
> in
> > bytes
> > > > > > +	 * of this field needs to be greater or equal to the length of
> > > > > > +	 * corresponding RSA key in bytes.
> > > > [Shally] field ---> buffer
> > > [AK] - Agreed
> > > >
> > > > > >  	 */
> > > > > >
> > > > > > -	enum rte_crypto_rsa_padding_type pad;
> > > > > > -	/**< RSA padding scheme to be used for transform */
> > > > > > -
> > > > > > -	enum rte_crypto_auth_algorithm md;
> > > > > > -	/**< Hash algorithm to be used for data hash if padding
> > > > > > -	 * scheme is either OAEP or PSS. Valid hash algorithms
> > > > > > -	 * are:
> > > > > > -	 * MD5, SHA1, SHA224, SHA256, SHA384, SHA512
> > > > > > +	rte_crypto_param message_to_verify;
> > > > > > +	/**<
> > > > > > +	 * Pointer to the message 'm' that was signed with
> > > > > > +	 * RSASP1 in RFC8017.
> > > > >> It is the result of operation RSAVP1
> > > > > > +	 * defined in RFC8017, where field `sign` is the input
> > > > > > +	 * parameter `s`.
> > > > > > +	 *
> > > > [Shally] This is confusing. Are you trying to say "this is output
> > > > to VERIFY_OP
> > > ?
> > > > where output should be same as message buffer provided above?
> > > > If yes, then why not just use that message buffer as an output of
> > > > VERIFY_OP than adding a new one?
> > > [AK] - it is output of signature verify (in openssl Public_Decrypt)
> > > for PADDING_NONE case. It will be used to check if signature is correct.
> > > In `message` then there could be original message.
> > > But yes, we can use message as an output buffer, I just though it
> > > would be more transparent.
> > > Since user need to verify it by himself it is not that important to
> > > keep original message in `message` field, both ways will do.
> > > Waiting for opinions.
> > > >
> > > > > > +	 * Used only when padding type is set to
> > > > > > RTE_CRYPTO_RSA_PADDING_NONE
> > > > [Shally] I think regardless of padding, we can provide it as
> > > > output to sign operation
> > > [AK] - `Sign` is the place to put signature into.
> > > >
> > > > > > +	 * and `op_type` is set to RTE_CRYPTO_ASYM_OP_VERIFY.
> > > > > > +	 *
> > > > > > +	 * Returned data size will be equal to the size of RSA key
> > > > > > +	 * in bytes. All leading zeroes will be preserved.
> > > > > > +	 *
> > > > [Shally] Again, I think it should instead be mentioned as return
> > > > size can be 0 ... modulus_len - 1
> > > >
> > > > > > +	 * When RTE_CRYPTO_ASYM_OP_VERIFY op_type used
> length
> > in
> > > > > > bytes
> > > > > > +	 * of this field needs to be greater or equal to the length of
> > > > > > +	 * corresponding RSA key in bytes.
> > > > > >  	 */
> > > > [Shally] There're multiple statements starting with when op_type =
> > > > VERIFY, can we club them and make description shorter?
> > > >
> > > > > >
> > > > > > -	enum rte_crypto_auth_algorithm mgf1md;
> > > > > > +	enum rte_crypto_rsa_padding_type padding;
> > > > > > +	/**<
> > > > > > +	 * In case RTE_CRYPTO_RSA_PADDING_PKCS1 is selected,
> > > > > > +	 * driver will distinguish between block type basing
> > > > > > +	 * on rte_crypto_asym_op_type of the operation.
> > > > > > +	 *
> > > > > > +	 * Which padding type is supported by the driver can be
> > > > > > +	 * found in in specific driver guide.
> > > > [Shally] better rephrase " PMD expose padding type support in its
> > > capability.
> > > > Refer to <struct / API definition>
> > > >
> > > > > > +	 */
> > > > > > +	enum rte_crypto_auth_algorithm padding_hash;
> > > > > > +	/**<
> > > > > > +	 * -	For PKCS1-v1_5 signature (Block type 01) this field
> > > > > > +	 * represents hash function that will be used to create
> > > > > > +	 * message hash.
> > > > [Shally] Currently PMD input pre-computed hash atleast for PKCSV_1.5
> ..
> > > > does your hw support this offload for RSA?
> > > > So far , in our testing we observe openssl does it already before
> > > > calling private/public_key_enc so we should not be mentioning it
> > > > in spec unless any hw provide that combined offload hash + rsa
> > >
> > > [AK] - Openssl for PKCS1_5 signature use RSA_private_encrypt which
> > > does not handle algorithmIdentifier.
> > >
> >
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.
> > op
> >
> enssl.org%2Fdocs%2Fman1.0.2%2Fman3%2FRSA_private_encrypt.html&amp
> ;da
> >
> ta=02%7C01%7Cakhil.goyal%40nxp.com%7Cdb7fd9c8893144df755808d7045
> 49
> >
> 746%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C6369826336688
> 53
> >
> 558&amp;sdata=FGOMjneOQsB7DRg8fKYHWhn5TKH3eO8QKA1bGbbt25w%3
> D&
> > amp;reserved=0
> > > And it is not said what data should be provided for signing. To be
> > > properly signed input buffer should be DER encoded concatenation of
> > > algorithmIdentifier and message digest.
> > > There would be much work to be done by user then: adding OID,
> > > computing Hash of message, ASN.1 call. Waiting for comments on that.
> >
> > [AK2] - we do not specify what data should user provide to be signed.
> >
> > >
> > > >
> > > > > > +	 *
> > > > > > +	 * -	For OAEP this field represents hash function that will
> > > > > > +	 * be used to produce hash of the optional label.
> > > > > > +	 *
> > > > > > +	 * -	For PSS this field represents hash function that will be
> > used
> > > > > > +	 * to produce hash (mHash) of message M and of M'
> (padding1
> > > > > > +|
> > > > > > mHash | salt)
> > > > > > +	 *
> > > > > > +	 * If not set driver will use default value.
> > > > > > +	 */
> > > > > > +	union {
> > > > > > +		struct {
> > > > > > +			enum rte_crypto_auth_algorithm mgf;
> > > > > > +			/**<
> > > > > > +			 * Mask genereation function hash algorithm.
> > > > > > +			 *
> > > > > > +			 * If not set driver will use default value.
> > > > > > +			 */
> > > > > > +			rte_crypto_param label;
> > > > > > +			/**<
> > > > > > +			 * Optional label, if driver does not support
> > > > > > +			 * this option, optional label is just an empty
> > string.
> > > > > > +			 */
> > > > > > +		} OAEP;
> > > > > > +		struct {
> > > > > > +			enum rte_crypto_auth_algorithm mgf;
> > > > [Shally] Though  it is mentioned in current spec, but I have
> > > > similar doubt here do we need to provide this offload in spec? I
> > > > will use terms from rfc8017 for further discussion.
> > > >   if we have any PMD whose HW provide full RSAES-OAEP offload i.e.
> > > > doing EME-OAEP followed by RSAEP, then it make sense to have it in
> > > > spec. But if we don't have any PMD example which support that full
> > > > offload, then we can redefine spec only to support RSAEP and omit
> > > > md and
> > > mgf from spec.
> > > >
> > > [AK] - I thought to add PSS/OAEP to Openssl. Though OAEP and PSS for
> > > sure will be implemented having custom MGF is not that necessary.
> > > Waiting for opinions.
> >
> > >
> > > >
> > > >
> > > > > > +			/**<
> > > > > > +			 * Mask genereation function hash algorithm.
> > > > > > +			 *
> > > > > > +			 * If not set driver will use default value.
> > > > > > +			 */
> > > > > > +			int seed_len;
> > > > > > +			/**<
> > > > > > +			 * Intended seed length. Nagative number
> has
> > > > > special
> > > > > > +			 * value as follows:
> > > > > > +			 * -1 : seed len = length of output ot used
> hash
> > > > > > function
> > > > > > +			 * -2 : seed len is maximized
> > > > > > +			 */
> > > > > > +		} PSS;
> > > > > > +	};
> > > > > >  	/**<
> > > > > > -	 * Hash algorithm to be used for mask generation if
> > > > > > -	 * padding scheme is either OAEP or PSS. If padding
> > > > > > -	 * scheme is unspecified data hash algorithm is used
> > > > > > -	 * for mask generation. Valid hash algorithms are:
> > > > > > -	 * MD5, SHA1, SHA224, SHA256, SHA384, SHA512
> > > > > > +	 * Padding type of RSA crypto operation.
> > > > > > +	 * What are random number generator requirements and
> > prequisites
> > > > > > +	 * can be found specific driver guide.
> > > > [Shally] I would suggest it to rephrase again " app should refer
> > > > to PMD guide to check for RNG requirement and other pre-requisites
> > > > used in hash generation.
> > > > However , this feedback is relevant if we are retaining full OAEP offload.
> > > >
> > > > > >  	 */
> > > > > >  };
> > > > > >
> > > > > > --
> > > > > > 2.1.0