[dpdk-dev] [PATCH 0/3] Rework API for RSA algorithm in asymmetric crypto

[dpdk-dev] [PATCH 0/3] Rework API for RSA algorithm in asymmetric crypto

[Arek Kusztal]

Major changes:
- Cipher field was introduced
- Padding struct was created
- PKCS1-v1_5 Block type 0 was removed
- Fixed comments about prime numbers, etc.

Arek Kusztal (3):
  cryptodev: rework api of rsa algorithm
  crypto/openssl: rework openssl rsa implementation
  test: rework rsa test implementation

 app/test/test_cryptodev_asym.c           |  11 ++-
 drivers/crypto/openssl/rte_openssl_pmd.c |  14 ++-
 lib/librte_cryptodev/rte_crypto_asym.h   | 149 +++++++++++++++++++++++--------
 3 files changed, 127 insertions(+), 47 deletions(-)

-- 
2.1.0

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

[Arek Kusztal]

This patch reworks API of RSA algorithm.
Major changes:
- Cipher field was introduced
- Padding struct was created
- PKCS1-v1_5 Block type 0 was removed
- Fixed comments about prime numbers

Signed-off-by: Arek Kusztal <arkadiuszx.kusztal@intel.com>
---
 lib/librte_cryptodev/rte_crypto_asym.h | 149 +++++++++++++++++++++++++--------
 1 file changed, 114 insertions(+), 35 deletions(-)

diff --git a/lib/librte_cryptodev/rte_crypto_asym.h b/lib/librte_cryptodev/rte_crypto_asym.h
index 8672f21..bb94fa5 100644
--- a/lib/librte_cryptodev/rte_crypto_asym.h
+++ b/lib/librte_cryptodev/rte_crypto_asym.h
@@ -111,23 +111,33 @@ 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
-	 */
-	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
+	/**< RSA no padding scheme.
+	 * In this case user is responsible for providing and verification
+	 * of padding.
+	 * In case RTE_CRYPTO_ASYM_OP_VERIFY op type is used if no
+	 * problems in public key 'encryption' detected driver SHALL return
+	 * RTE_CRYPTO_OP_STATUS_SUCCESS. But it is USER RESPONSABILITY to
+	 * remove padding and verify signature.
+	 */
+	RTE_CRYPTO_RSA_PADDING_PKCS1,
+	/**< RSA PKCS#1 PKCS1-v1_5 padding scheme. For signatures block type 01,
+	 * for encryption block type 02 are used.
+	 *
+	 * In case RTE_CRYPTO_ASYM_OP_VERIFY op type is used crypto op status
+	 * is set to RTE_CRYPTO_OP_STATUS_SUCCESS when signature is properly
+	 * verified, RTE_CRYPTO_OP_STATUS_ERROR when it failed.
 	 */
 	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.
+	 *
+	 * Crypto op status is set to RTE_CRYPTO_OP_STATUS_SUCCESS
+	 * when signature is properly verified, RTE_CRYPTO_OP_STATUS_ERROR
+	 * when it failed.
+	 */
 	RTE_CRYPTO_RSA_PADDING_TYPE_LIST_END
 };
 
@@ -199,8 +209,8 @@ struct rte_crypto_rsa_priv_key_qt {
  */
 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,11 +407,23 @@ 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.
 	 */
 
+	rte_crypto_param cipher;
+	/**<
+	 * Pointer to data
+	 * - to be decrypted for RSA private decrypt.
+	 *
+	 * When RTE_CRYPTO_ASYM_OP_ENCRYPT op_type used size in bytes
+	 * of this field need to be equal to the size of corresponding
+	 * RSA key. Returned data is in Big-Endian format which means
+	 * that Least-Significant byte will be placed at top byte of an array
+	 * (at message.data[message.length - 1]), cipher.length SHALL
+	 * therefore remain unchanged.
+	 */
+
 	rte_crypto_param sign;
 	/**<
 	 * Pointer to RSA signature data. If operation is RSA
@@ -410,25 +432,82 @@ struct rte_crypto_rsa_op_param {
 	 *
 	 * Length of the signature data will be equal to the
 	 * RSA prime modulus length.
-	 */
-
-	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
-	 */
-
-	enum rte_crypto_auth_algorithm mgf1md;
+	 *
+	 * Returned data is in Big-Endian format which means
+	 * that Least-Significant byte will be placed at top byte of an array
+	 * (at message.data[message.length - 1]), sign.length SHALL
+	 * therefore remain unchanged.
+	 */
+
+	struct {
+		enum rte_crypto_rsa_padding_type type;
+		/**<
+		 * 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 SHALL be
+		 * available in in specific driver guide or capabilities.
+		 */
+		enum rte_crypto_auth_algorithm hash;
+		/**<
+		 * -	For PKCS1-v1_5 signature (Block type 01) this field
+		 * represents hash function that will be used to create
+		 * message hash. This hash will be then concatenated with
+		 * hash algorithm identifier into DER encoded sequence.
+		 * If RTE_CRYPTO_HASH_INVALID is set, driver default will be set.
+		 * If RTE_CRYPTO_HASH_NONE is set, message will be signed as it is.
+		 *
+		 * -	For OAEP this field represents hash function that will
+		 * be used to produce hash of the optional label.
+		 * If RTE_CRYPTO_HASH_INVALID or RTE_CRYPTO_HASH_NONE is set
+		 * driver will use default value. For OAEP usually it is SHA-1.
+		 *
+		 * -	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 RTE_CRYPTO_HASH_INVALID or RTE_CRYPTO_HASH_NONE is set
+		 * driver will use default value.
+		 *
+		 * -	If driver supports only one function RTE_CRYPTO_HASH_NONE
+		 * according to aformentioned restrictions should be used or
+		 * specific function should be set, otherwise on dequeue the driver
+		 * SHALL set crypto_op_status to RTE_CRYPTO_OP_STATUS_INVALID_ARGS.
+		 */
+		union {
+			struct {
+				enum rte_crypto_auth_algorithm mgf;
+				/**<
+				 * Mask genereation function hash algorithm.
+				 * If this field is not supported by the driver,
+				 * it should be set to RTE_CRYPTO_HASH_NONE.
+				 */
+				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;
+				/**<
+				 * Mask genereation function hash algorithm.
+				 * If this field is not supported by the driver,
+				 * it should be set to RTE_CRYPTO_HASH_NONE.
+				 */
+				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;
+		};
+	} padding;
 	/**<
-	 * 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
+	 * SHALL be put in specific driver guide
 	 */
 };
 
-- 
2.1.0

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

[Shally Verma]

Hi Arek,


> -----Original Message-----
> From: Arek Kusztal <arkadiuszx.kusztal@intel.com>
> Sent: Friday, May 31, 2019 7:22 PM
> To: dev@dpdk.org
> Cc: akhil.goyal@nxp.com; fiona.trahe@intel.com;
> shally.verma@caviumnetworks.com; Arek Kusztal
> <arkadiuszx.kusztal@intel.com>
> Subject: [EXT] [PATCH 1/3] cryptodev: rework api of rsa algorithm
> 
> External Email
> 
> ----------------------------------------------------------------------
> This patch reworks API of RSA algorithm.
> Major changes:
> - Cipher field was introduced
> - Padding struct was created
> - PKCS1-v1_5 Block type 0 was removed
> - Fixed comments about prime numbers
> 
> Signed-off-by: Arek Kusztal <arkadiuszx.kusztal@intel.com>
> ---
>  lib/librte_cryptodev/rte_crypto_asym.h | 149
> +++++++++++++++++++++++++--------
>  1 file changed, 114 insertions(+), 35 deletions(-)
> 
> diff --git a/lib/librte_cryptodev/rte_crypto_asym.h
> b/lib/librte_cryptodev/rte_crypto_asym.h
> index 8672f21..bb94fa5 100644
> --- a/lib/librte_cryptodev/rte_crypto_asym.h
> +++ b/lib/librte_cryptodev/rte_crypto_asym.h
> @@ -111,23 +111,33 @@ 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
> -	 */
> -	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
> +	/**< RSA no padding scheme.
> +	 * In this case user is responsible for providing and verification
> +	 * of padding.
> +	 * In case RTE_CRYPTO_ASYM_OP_VERIFY op type is used if no
> +	 * problems in public key 'encryption' detected driver SHALL return
> +	 * RTE_CRYPTO_OP_STATUS_SUCCESS.

[Shally] This is not clear to me. OP_VERIFY is public key decryption, then why it is mentioned as 'encryption' above?
Secondly,  Any public/private key encryption with NO_PADDING scheme, would result in encryption data without any padding string. 
And, if same is passed to PMD for decryption with PADDING_NONE, then PMD would assume encryption block is without any padding string. 
So, which case are we trying to clarify here? Or, do you intend to mention case when app can pass data with padding for decryption with NO_PADDING scheme?

> But it is USER
> RESPONSABILITY to
> +	 * remove padding and verify signature.
> +	 */
> +	RTE_CRYPTO_RSA_PADDING_PKCS1,
> +	/**< RSA PKCS#1 PKCS1-v1_5 padding scheme. For signatures block
> type 01,
> +	 * for encryption block type 02 are used.

[Shally] Though I understand PKCS1_V1.5 dictates the use of specific BT for pub/private key encryption/decryption,
So, seems we are making that as an implicit decision by removing individual control of BT by app. However, only point is then it states only about BT1 and BT2 not BT0.
What if , an app want to use BT0 for padding, then how do we support that? what is rationale for removing BT0 here?

> +	 *
> +	 * In case RTE_CRYPTO_ASYM_OP_VERIFY op type is used crypto op
> status
> +	 * is set to RTE_CRYPTO_OP_STATUS_SUCCESS when signature is
> properly
> +	 * verified, RTE_CRYPTO_OP_STATUS_ERROR when it failed.

[Shally] This description should go under OP_TYPE_VERIFICATION than here. Here it should limit to description about padding scheme

>  	 */
>  	RTE_CRYPTO_RSA_PADDING_OAEP,
> -	/**< RSA PKCS#1 OAEP padding scheme */
> +	/**< RSA PKCS#1 OAEP padding scheme, can be used only for
> encryption/
> +	 * decryption.

[Shally] Better to add version V2 here: RSA PKCS#1 V2 OAEP padding scheme

> +	 */

>  	RTE_CRYPTO_RSA_PADDING_PSS,
> -	/**< RSA PKCS#1 PSS padding scheme */
> +	/**< RSA PKCS#1 PSS padding scheme, can be used only for
> signatures.

[Shally] It is enough to mention till this here. following about op status should better go to an op_type description than here

> +	 *
> +	 * Crypto op status is set to RTE_CRYPTO_OP_STATUS_SUCCESS
> +	 * when signature is properly verified,
> RTE_CRYPTO_OP_STATUS_ERROR
> +	 * when it failed.
> +	 */
>  	RTE_CRYPTO_RSA_PADDING_TYPE_LIST_END
>  };
> 
> @@ -199,8 +209,8 @@ struct rte_crypto_rsa_priv_key_qt {
>   */
>  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,11 +407,23 @@ 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.
>  	 */
> 
> +	rte_crypto_param cipher;
> +	/**<
> +	 * Pointer to data
> +	 * - to be decrypted for RSA private decrypt.

[Shally] Since spec use terminology RSA Decryption for private key decryption, so , it is enough to mention : "pointer to data to be decrypted." And same is applicable to other as well.

> +	 *
> +	 * When RTE_CRYPTO_ASYM_OP_ENCRYPT op_type used size in
> bytes
> +	 * of this field need to be equal to the size of corresponding
> +	 * RSA key. 

[Shally] Since it is meant as an input for decryption, reference to op_type ENCRYPT look redundant here. It can simply stated as, " length of cipher data should be equal to RSA modulus length."

>Returned data is in Big-Endian format which means
> +	 * that Least-Significant byte will be placed at top byte of an array
> +	 * (at message.data[message.length - 1]), cipher.length SHALL
> +	 * therefore remain unchanged.

[Shally] Do we really need to mention it?

> +	 */
> +
>  	rte_crypto_param sign;
>  	/**<
>  	 * Pointer to RSA signature data. If operation is RSA @@ -410,25
> +432,82 @@ struct rte_crypto_rsa_op_param {
>  	 *
>  	 * Length of the signature data will be equal to the
>  	 * RSA prime modulus length.
> -	 */
> -
> -	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
> -	 */
> -
> -	enum rte_crypto_auth_algorithm mgf1md;
> +	 *
> +	 * Returned data is in Big-Endian format which means
> +	 * that Least-Significant byte will be placed at top byte of an array
> +	 * (at message.data[message.length - 1]), sign.length SHALL
> +	 * therefore remain unchanged.
> +	 */

[Shally] Again, this looks redundant info here.

> +
> +	struct {
> +		enum rte_crypto_rsa_padding_type type;
> +		/**<
> +		 * 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 SHALL be
> +		 * available in in specific driver guide or capabilities.
> +		 */
[Shally] you mentioned it as part of capability, but this patch does not have any capa struct changes to return this. So do you mean capability or PMD release note here?

> +		enum rte_crypto_auth_algorithm hash;
> +		/**<
> +		 * -	For PKCS1-v1_5 signature (Block type 01) this field
> +		 * represents hash function that will be used to create
> +		 * message hash. This hash will be then concatenated with
> +		 * hash algorithm identifier into DER encoded sequence.

[Shally] This whole description is already in RFC. It looks like an overload to add all such details here. If really intended it is better to give rfc PKCS1 V2 reference here.

> +		 * If RTE_CRYPTO_HASH_INVALID is set, driver default will be
> set.
> +		 * If RTE_CRYPTO_HASH_NONE is set, message will be signed
> as it is.

[Shally] who will set it? PMD or application? Its bit unclear here where and when it will  be set to INVALID / NONE?

> +		 *
> +		 * -	For OAEP this field represents hash function that will
> +		 * be used to produce hash of the optional label.
> +		 * If RTE_CRYPTO_HASH_INVALID or
> RTE_CRYPTO_HASH_NONE is set
> +		 * driver will use default value. For OAEP usually it is SHA-1.
> +		 *
> +		 * -	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 RTE_CRYPTO_HASH_INVALID or
> RTE_CRYPTO_HASH_NONE is set
> +		 * driver will use default value.
> +		 *
> +		 * -	If driver supports only one function
> RTE_CRYPTO_HASH_NONE
> +		 * according to aformentioned restrictions should be used or
> +		 * specific function should be set, otherwise on dequeue the
> driver
> +		 * SHALL set crypto_op_status to
> RTE_CRYPTO_OP_STATUS_INVALID_ARGS.

[Shally] Similar  feedback here.

> +		 */
> +		union {
> +			struct {
> +				enum rte_crypto_auth_algorithm mgf;
> +				/**<
> +				 * Mask genereation function hash algorithm.
> +				 * If this field is not supported by the driver,
> +				 * it should be set to
> RTE_CRYPTO_HASH_NONE.
> +				 */
> +				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;
> +				/**<
> +				 * Mask genereation function hash algorithm.
> +				 * If this field is not supported by the driver,
> +				 * it should be set to
> RTE_CRYPTO_HASH_NONE.
> +				 */
> +				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;
> +		};
> +	} padding;

[Shally] having additional struct padding within op_param looks unnecessary. Why do we need to rearrange it like this? It can simply be:

struct rte_crypto_rsa_op_param {
	enum rte_crypto_asym_op_type op_type;
	/**< Type of RSA operation for transform */;

	rte_crypto_param cipher;
	/**<
	 * Pointer to data to be decrypted with length of data equal to RSA modulus length
                */

	rte_crypto_param message;
	/**<
	 * Pointer to data
	 * - to be encrypted.
	 * - to be signed.
	 * - to be verified.
	 */

	rte_crypto_param sign;
	/**<
	 * Pointer to RSA signature data. If operation is RSA
	 * 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.
	 */

	enum rte_crypto_rsa_padding_type pad;
	/**< RSA padding scheme to be used for transform */

> +		union {
> +			struct {
> +				enum rte_crypto_auth_algorithm mgf;
> +				/**<
> +				 * Mask genereation function hash algorithm.
> +				 * If this field is not supported by the driver,
> +				 * it should be set to
> RTE_CRYPTO_HASH_NONE.
> +				 */
> +				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;
> +				/**<
> +				 * Mask genereation function hash algorithm.
> +				 * If this field is not supported by the driver,
> +				 * it should be set to
> RTE_CRYPTO_HASH_NONE.
> +				 */
> +				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
> +	 * SHALL be put in specific driver guide

[Shally] Again, probably this reference is unnecessary here.

>  	 */
>  };
> 
> --
> 2.1.0

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

[Kusztal, ArkadiuszX]

Hi Shally,

Thanks a lot for your detailed feedback. Sorry for delayed answer.
My comments with [AK]

> -----Original Message-----
> From: Shally Verma [mailto:shallyv@marvell.com]
> Sent: Wednesday, June 5, 2019 2:12 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: [EXT] [PATCH 1/3] cryptodev: rework api of rsa algorithm
> 
> Hi Arek,
> 
> 
> > -----Original Message-----
> > From: Arek Kusztal <arkadiuszx.kusztal@intel.com>
> > Sent: Friday, May 31, 2019 7:22 PM
> > To: dev@dpdk.org
> > Cc: akhil.goyal@nxp.com; fiona.trahe@intel.com;
> > shally.verma@caviumnetworks.com; Arek Kusztal
> > <arkadiuszx.kusztal@intel.com>
> > Subject: [EXT] [PATCH 1/3] cryptodev: rework api of rsa algorithm
> >
> > External Email
> >
> > ----------------------------------------------------------------------
> > This patch reworks API of RSA algorithm.
> > Major changes:
> > - Cipher field was introduced
> > - Padding struct was created
> > - PKCS1-v1_5 Block type 0 was removed
> > - Fixed comments about prime numbers
> >
> > Signed-off-by: Arek Kusztal <arkadiuszx.kusztal@intel.com>
> > ---
> >  lib/librte_cryptodev/rte_crypto_asym.h | 149
> > +++++++++++++++++++++++++--------
> >  1 file changed, 114 insertions(+), 35 deletions(-)
> >
> > diff --git a/lib/librte_cryptodev/rte_crypto_asym.h
> > b/lib/librte_cryptodev/rte_crypto_asym.h
> > index 8672f21..bb94fa5 100644
> > --- a/lib/librte_cryptodev/rte_crypto_asym.h
> > +++ b/lib/librte_cryptodev/rte_crypto_asym.h
> > @@ -111,23 +111,33 @@ 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
> > -	 */
> > -	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
> > +	/**< RSA no padding scheme.
> > +	 * In this case user is responsible for providing and verification
> > +	 * of padding.
> > +	 * In case RTE_CRYPTO_ASYM_OP_VERIFY op type is used if no
> > +	 * problems in public key 'encryption' detected driver SHALL return
> > +	 * RTE_CRYPTO_OP_STATUS_SUCCESS.
> 
> [Shally] This is not clear to me. OP_VERIFY is public key decryption, then why
> it is mentioned as 'encryption' above?

[AK] - yes, you right 'decryption', public key but technically decryption.

> Secondly,  Any public/private key encryption with NO_PADDING scheme,
> would result in encryption data without any padding string.
> And, if same is passed to PMD for decryption with PADDING_NONE, then
> PMD would assume encryption block is without any padding string.
> So, which case are we trying to clarify here? Or, do you intend to mention
> case when app can pass data with padding for decryption with NO_PADDING
> scheme?

[AK] Yes. It may be common situation that HW drivers wont natively be doing any padding,
Especially PSS and OAEP which expects very strong TRNG. NO_PADDING would mean that
driver should not be responsible for padding, and not care what is in there as long
as parameters are correct.

> 
> > But it is USER
> > RESPONSABILITY to
> > +	 * remove padding and verify signature.

[AK] - Same as above.

> > +	 */
> > +	RTE_CRYPTO_RSA_PADDING_PKCS1,
> > +	/**< RSA PKCS#1 PKCS1-v1_5 padding scheme. For signatures block
> > type 01,
> > +	 * for encryption block type 02 are used.
> 
> [Shally] Though I understand PKCS1_V1.5 dictates the use of specific BT for
> pub/private key encryption/decryption, So, seems we are making that as an
> implicit decision by removing individual control of BT by app. However, only
> point is then it states only about BT1 and BT2 not BT0.
> What if , an app want to use BT0 for padding, then how do we support that?
> what is rationale for removing BT0 here?

[AK] About BT 00 it is nothing else than zero padding, plaintext RSA, more or less the same thing as NO_PADDING. It has been silently abandoned
long time ago. Only RFC 2313 mentions it. No 2437,3447 and 8017.

> 
> > +	 *
> > +	 * In case RTE_CRYPTO_ASYM_OP_VERIFY op type is used crypto op
> > status
> > +	 * is set to RTE_CRYPTO_OP_STATUS_SUCCESS when signature is
> > properly
> > +	 * verified, RTE_CRYPTO_OP_STATUS_ERROR when it failed.
> 
> [Shally] This description should go under OP_TYPE_VERIFICATION than here.
> Here it should limit to description about padding scheme
[AK] Could you send your proposal please?

> 
> >  	 */
> >  	RTE_CRYPTO_RSA_PADDING_OAEP,
> > -	/**< RSA PKCS#1 OAEP padding scheme */
> > +	/**< RSA PKCS#1 OAEP padding scheme, can be used only for
> > encryption/
> > +	 * decryption.
> 
> [Shally] Better to add version V2 here: RSA PKCS#1 V2 OAEP padding scheme

[AK] Usually V2 is not added to it. Example from openssl " RSA_padding_add_PKCS1_OAEP",

> 
> > +	 */
> 
> >  	RTE_CRYPTO_RSA_PADDING_PSS,
> > -	/**< RSA PKCS#1 PSS padding scheme */
> > +	/**< RSA PKCS#1 PSS padding scheme, can be used only for
> > signatures.
> 
> [Shally] It is enough to mention till this here. following about op status
> should better go to an op_type description than here
[AK] Same as above, could you send your proposal please?

> 
> > +	 *
> > +	 * Crypto op status is set to RTE_CRYPTO_OP_STATUS_SUCCESS
> > +	 * when signature is properly verified,
> > RTE_CRYPTO_OP_STATUS_ERROR
> > +	 * when it failed.
> > +	 */
> >  	RTE_CRYPTO_RSA_PADDING_TYPE_LIST_END
> >  };
> >
> > @@ -199,8 +209,8 @@ struct rte_crypto_rsa_priv_key_qt {
> >   */
> >  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,11 +407,23 @@ 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.
> >  	 */
> >
> > +	rte_crypto_param cipher;
> > +	/**<
> > +	 * Pointer to data
> > +	 * - to be decrypted for RSA private decrypt.
> 
> [Shally] Since spec use terminology RSA Decryption for private key
> decryption, so , it is enough to mention : "pointer to data to be decrypted."
> And same is applicable to other as well.

[AK] Ok.
> 
> > +	 *
> > +	 * When RTE_CRYPTO_ASYM_OP_ENCRYPT op_type used size in
> > bytes
> > +	 * of this field need to be equal to the size of corresponding
> > +	 * RSA key.
> 
> [Shally] Since it is meant as an input for decryption, reference to op_type
> ENCRYPT look redundant here. It can simply stated as, " length of cipher data
> should be equal to RSA modulus length."
> 
> >Returned data is in Big-Endian format which means
> > +	 * that Least-Significant byte will be placed at top byte of an array
> > +	 * (at message.data[message.length - 1]), cipher.length SHALL
> > +	 * therefore remain unchanged.
> 
> [Shally] Do we really need to mention it?

[AK] I have not pronounced myself very clear here, and I forgot to add it to the message data.
For example we have 256 bytes allocated for RSA-2048 let say decryption. But we decrypted only 255 bytes (one leading zero).
So should we trim this zero or not? If we trim, size is 255 and data[0] is some non zero byte. If we don't, size if 256 and data[0]=0.
One leading zero is a good example of PKCS1 padding.
 
 
> 
> > +	 */
> > +
> >  	rte_crypto_param sign;
> >  	/**<
> >  	 * Pointer to RSA signature data. If operation is RSA @@ -410,25
> > +432,82 @@ struct rte_crypto_rsa_op_param {
> >  	 *
> >  	 * Length of the signature data will be equal to the
> >  	 * RSA prime modulus length.
> > -	 */
> > -
> > -	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
> > -	 */
> > -
> > -	enum rte_crypto_auth_algorithm mgf1md;
> > +	 *
> > +	 * Returned data is in Big-Endian format which means
> > +	 * that Least-Significant byte will be placed at top byte of an array
> > +	 * (at message.data[message.length - 1]), sign.length SHALL
> > +	 * therefore remain unchanged.
> > +	 */
> 
> [Shally] Again, this looks redundant info here.
> 
> > +
> > +	struct {
> > +		enum rte_crypto_rsa_padding_type type;
> > +		/**<
> > +		 * 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 SHALL be
> > +		 * available in in specific driver guide or capabilities.
> > +		 */
> [Shally] you mentioned it as part of capability, but this patch does not have
> any capa struct changes to return this. So do you mean capability or PMD
> release note here?

[AK] - this would be part of v2. Thanks.
> 
> > +		enum rte_crypto_auth_algorithm hash;
> > +		/**<
> > +		 * -	For PKCS1-v1_5 signature (Block type 01) this field
> > +		 * represents hash function that will be used to create
> > +		 * message hash. This hash will be then concatenated with
> > +		 * hash algorithm identifier into DER encoded sequence.
> 
> [Shally] This whole description is already in RFC. It looks like an overload to
> add all such details here. If really intended it is better to give rfc PKCS1 V2
> reference here.

[AK] Ok.
> 
> > +		 * If RTE_CRYPTO_HASH_INVALID is set, driver default will be
> > set.
> > +		 * If RTE_CRYPTO_HASH_NONE is set, message will be signed
> > as it is.
[AK] - provided message fits into key size.
> 
> [Shally] who will set it? PMD or application? Its bit unclear here where and
> when it will  be set to INVALID / NONE?

[AK] PMD according to its default. Still only a proposal.
> 
> > +		 *
> > +		 * -	For OAEP this field represents hash function that will
> > +		 * be used to produce hash of the optional label.
> > +		 * If RTE_CRYPTO_HASH_INVALID or
> > RTE_CRYPTO_HASH_NONE is set
> > +		 * driver will use default value. For OAEP usually it is SHA-1.
> > +		 *
> > +		 * -	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 RTE_CRYPTO_HASH_INVALID or
> > RTE_CRYPTO_HASH_NONE is set
> > +		 * driver will use default value.
> > +		 *
> > +		 * -	If driver supports only one function
> > RTE_CRYPTO_HASH_NONE
> > +		 * according to aformentioned restrictions should be used or
> > +		 * specific function should be set, otherwise on dequeue the
> > driver
> > +		 * SHALL set crypto_op_status to
> > RTE_CRYPTO_OP_STATUS_INVALID_ARGS.
> 
> [Shally] Similar  feedback here.
> 
> > +		 */
> > +		union {
> > +			struct {
> > +				enum rte_crypto_auth_algorithm mgf;
> > +				/**<
> > +				 * Mask genereation function hash algorithm.
> > +				 * If this field is not supported by the driver,
> > +				 * it should be set to
> > RTE_CRYPTO_HASH_NONE.
> > +				 */
> > +				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;
> > +				/**<
> > +				 * Mask genereation function hash algorithm.
> > +				 * If this field is not supported by the driver,
> > +				 * it should be set to
> > RTE_CRYPTO_HASH_NONE.
> > +				 */
> > +				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;
> > +		};
> > +	} padding;
> 
> [Shally] having additional struct padding within op_param looks unnecessary.
> Why do we need to rearrange it like this? It can simply be:

[AK] - its proposal, to discuss I think.

> 
> struct rte_crypto_rsa_op_param {
> 	enum rte_crypto_asym_op_type op_type;
> 	/**< Type of RSA operation for transform */;
> 
> 	rte_crypto_param cipher;
> 	/**<
> 	 * Pointer to data to be decrypted with length of data equal to RSA
> modulus length
>                 */
> 
> 	rte_crypto_param message;
> 	/**<
> 	 * Pointer to data
> 	 * - to be encrypted.
> 	 * - to be signed.
> 	 * - to be verified.
> 	 */
> 
> 	rte_crypto_param sign;
> 	/**<
> 	 * Pointer to RSA signature data. If operation is RSA
> 	 * 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.
> 	 */
> 
> 	enum rte_crypto_rsa_padding_type pad;
> 	/**< RSA padding scheme to be used for transform */
> 
> > +		union {
> > +			struct {
> > +				enum rte_crypto_auth_algorithm mgf;
> > +				/**<
> > +				 * Mask genereation function hash algorithm.
> > +				 * If this field is not supported by the driver,
> > +				 * it should be set to
> > RTE_CRYPTO_HASH_NONE.
> > +				 */
> > +				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;
> > +				/**<
> > +				 * Mask genereation function hash algorithm.
> > +				 * If this field is not supported by the driver,
> > +				 * it should be set to
> > RTE_CRYPTO_HASH_NONE.
> > +				 */
> > +				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
> > +	 * SHALL be put in specific driver guide
> 
> [Shally] Again, probably this reference is unnecessary here.
> 
> >  	 */
> >  };
> >
> > --
> > 2.1.0

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

[Trahe, Fiona]

Hi Arek, Shally,

Comments below.
@Arek, I'd suggest sending a v2 after this, updated with whatever issues can be closed and a listing of the issues still open.
As getting hard to follow the thread. 
@Shally, can you reply please - just with whatever items below you're ok with, so Arek can do the v2.
And we can continue discussion on the v2 on anything still open.

> -----Original Message-----
> From: Kusztal, ArkadiuszX
> Sent: Sunday, June 16, 2019 10:40 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: [EXT] [PATCH 1/3] cryptodev: rework api of rsa algorithm
> 
> Hi Shally,
> 
> Thanks a lot for your detailed feedback. Sorry for delayed answer.
> My comments with [AK]
> 
> > -----Original Message-----
> > From: Shally Verma [mailto:shallyv@marvell.com]
> > Sent: Wednesday, June 5, 2019 2:12 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: [EXT] [PATCH 1/3] cryptodev: rework api of rsa algorithm
> >
> > Hi Arek,
> >
> >
> > > -----Original Message-----
> > > From: Arek Kusztal <arkadiuszx.kusztal@intel.com>
> > > Sent: Friday, May 31, 2019 7:22 PM
> > > To: dev@dpdk.org
> > > Cc: akhil.goyal@nxp.com; fiona.trahe@intel.com;
> > > shally.verma@caviumnetworks.com; Arek Kusztal
> > > <arkadiuszx.kusztal@intel.com>
> > > Subject: [EXT] [PATCH 1/3] cryptodev: rework api of rsa algorithm
> > >
> > > External Email
> > >
> > > ----------------------------------------------------------------------
> > > This patch reworks API of RSA algorithm.
> > > Major changes:
> > > - Cipher field was introduced
> > > - Padding struct was created
> > > - PKCS1-v1_5 Block type 0 was removed
> > > - Fixed comments about prime numbers
> > >
> > > Signed-off-by: Arek Kusztal <arkadiuszx.kusztal@intel.com>
> > > ---
> > >  lib/librte_cryptodev/rte_crypto_asym.h | 149
> > > +++++++++++++++++++++++++--------
> > >  1 file changed, 114 insertions(+), 35 deletions(-)
> > >
> > > diff --git a/lib/librte_cryptodev/rte_crypto_asym.h
> > > b/lib/librte_cryptodev/rte_crypto_asym.h
> > > index 8672f21..bb94fa5 100644
> > > --- a/lib/librte_cryptodev/rte_crypto_asym.h
> > > +++ b/lib/librte_cryptodev/rte_crypto_asym.h
> > > @@ -111,23 +111,33 @@ 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
> > > -	 */
> > > -	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
> > > +	/**< RSA no padding scheme.
> > > +	 * In this case user is responsible for providing and verification
> > > +	 * of padding.
[Fiona] Grammar: provision not providing

> > > +	 * In case RTE_CRYPTO_ASYM_OP_VERIFY op type is used if no
> > > +	 * problems in public key 'encryption' detected driver SHALL return
> > > +	 * RTE_CRYPTO_OP_STATUS_SUCCESS.
> >
> > [Shally] This is not clear to me. OP_VERIFY is public key decryption, then why
> > it is mentioned as 'encryption' above?
> 
> [AK] - yes, you right 'decryption', public key but technically decryption.
> > Secondly,  Any public/private key encryption with NO_PADDING scheme,
> > would result in encryption data without any padding string.
> > And, if same is passed to PMD for decryption with PADDING_NONE, then
> > PMD would assume encryption block is without any padding string.
> > So, which case are we trying to clarify here? Or, do you intend to mention
> > case when app can pass data with padding for decryption with NO_PADDING
> > scheme?
> 
> [AK] Yes. It may be common situation that HW drivers wont natively be doing any padding,
> Especially PSS and OAEP which expects very strong TRNG. NO_PADDING would mean that
> driver should not be responsible for padding, and not care what is in there as long
> as parameters are correct.
[Fiona] just to add to that - PADDING_NONE typically really means PADDING_DONE_BY_APPLICATION
But using the NONE terminology is consistent with openssl lib, so I think we should stick with that. 

> 
> >
> > > But it is USER
> > > RESPONSABILITY to
> > > +	 * remove padding and verify signature.
> 
> [AK] - Same as above.
> 
> > > +	 */
> > > +	RTE_CRYPTO_RSA_PADDING_PKCS1,
> > > +	/**< RSA PKCS#1 PKCS1-v1_5 padding scheme. For signatures block
> > > type 01,
> > > +	 * for encryption block type 02 are used.
> >
> > [Shally] Though I understand PKCS1_V1.5 dictates the use of specific BT for
> > pub/private key encryption/decryption, So, seems we are making that as an
> > implicit decision by removing individual control of BT by app. However, only
> > point is then it states only about BT1 and BT2 not BT0.
> > What if , an app want to use BT0 for padding, then how do we support that?
> > what is rationale for removing BT0 here?
> 
> [AK] About BT 00 it is nothing else than zero padding, plaintext RSA, more or less the same thing as
> NO_PADDING. It has been silently abandoned
> long time ago. Only RFC 2313 mentions it. No 2437,3447 and 8017.
[Fiona] Actually as PKCS#1 v1.5 is less secure for encryption than OAEP, arguably it should be
removed too from this enum to discourage its use. However as it's still commonly used we 
need to keep it. We should make sure that capability allows a PMD to opt out of supporting it
if it chooses to only support PSS and OAEP.  
 
> 
> >
> > > +	 *
> > > +	 * In case RTE_CRYPTO_ASYM_OP_VERIFY op type is used crypto op
> > > status
> > > +	 * is set to RTE_CRYPTO_OP_STATUS_SUCCESS when signature is
> > > properly
> > > +	 * verified, RTE_CRYPTO_OP_STATUS_ERROR when it failed.
> >
> > [Shally] This description should go under OP_TYPE_VERIFICATION than here.
> > Here it should limit to description about padding scheme
> [AK] Could you send your proposal please?
[Fiona] I agree with shally that it's not needed here. But think it can be left out
altogether - it's stating the obvious. The PADDING_NONE case is different as it's
not possible for the PMD to do full signature verification in that case. So it makes
sense to call that out there.

> 
> >
> > >  	 */
> > >  	RTE_CRYPTO_RSA_PADDING_OAEP,
> > > -	/**< RSA PKCS#1 OAEP padding scheme */
> > > +	/**< RSA PKCS#1 OAEP padding scheme, can be used only for
> > > encryption/
> > > +	 * decryption.
> >
> > [Shally] Better to add version V2 here: RSA PKCS#1 V2 OAEP padding scheme
> 
> [AK] Usually V2 is not added to it. Example from openssl " RSA_padding_add_PKCS1_OAEP",
[Fiona] Agree with Arek, better to leave out v2. 
 
> >
> > > +	 */
> >
> > >  	RTE_CRYPTO_RSA_PADDING_PSS,
> > > -	/**< RSA PKCS#1 PSS padding scheme */
> > > +	/**< RSA PKCS#1 PSS padding scheme, can be used only for
> > > signatures.
> >
> > [Shally] It is enough to mention till this here. following about op status
> > should better go to an op_type description than here
> [AK] Same as above, could you send your proposal please?
[Fiona] As above, I'd just leave out following sentence.
 

> > > +	 *
> > > +	 * Crypto op status is set to RTE_CRYPTO_OP_STATUS_SUCCESS
> > > +	 * when signature is properly verified,
> > > RTE_CRYPTO_OP_STATUS_ERROR
> > > +	 * when it failed.
> > > +	 */
> > >  	RTE_CRYPTO_RSA_PADDING_TYPE_LIST_END
> > >  };
> > >
> > > @@ -199,8 +209,8 @@ struct rte_crypto_rsa_priv_key_qt {
> > >   */
> > >  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,11 +407,23 @@ 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.
> > >  	 */
> > >
> > > +	rte_crypto_param cipher;
> > > +	/**<
> > > +	 * Pointer to data
> > > +	 * - to be decrypted for RSA private decrypt.
> >
> > [Shally] Since spec use terminology RSA Decryption for private key
> > decryption, so , it is enough to mention : "pointer to data to be decrypted."
> > And same is applicable to other as well.
> 
> [AK] Ok.
> >
> > > +	 *
> > > +	 * When RTE_CRYPTO_ASYM_OP_ENCRYPT op_type used size in
> > > bytes
> > > +	 * of this field need to be equal to the size of corresponding
> > > +	 * RSA key.
> >
> > [Shally] Since it is meant as an input for decryption, reference to op_type
> > ENCRYPT look redundant here. It can simply stated as, " length of cipher data
> > should be equal to RSA modulus length."
> >
> > >Returned data is in Big-Endian format which means
> > > +	 * that Least-Significant byte will be placed at top byte of an array
> > > +	 * (at message.data[message.length - 1]), cipher.length SHALL
> > > +	 * therefore remain unchanged.
> >
> > [Shally] Do we really need to mention it?
> 
> [AK] I have not pronounced myself very clear here, and I forgot to add it to the message data.
> For example we have 256 bytes allocated for RSA-2048 let say decryption. But we decrypted only 255
> bytes (one leading zero).
> So should we trim this zero or not? If we trim, size is 255 and data[0] is some non zero byte. If we don't,
> size if 256 and data[0]=0.
> One leading zero is a good example of PKCS1 padding.
> 
> 
> >
> > > +	 */
> > > +
> > >  	rte_crypto_param sign;
> > >  	/**<
> > >  	 * Pointer to RSA signature data. If operation is RSA @@ -410,25
> > > +432,82 @@ struct rte_crypto_rsa_op_param {
> > >  	 *
> > >  	 * Length of the signature data will be equal to the
> > >  	 * RSA prime modulus length.
> > > -	 */
> > > -
> > > -	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
> > > -	 */
> > > -
> > > -	enum rte_crypto_auth_algorithm mgf1md;
> > > +	 *
> > > +	 * Returned data is in Big-Endian format which means
> > > +	 * that Least-Significant byte will be placed at top byte of an array
> > > +	 * (at message.data[message.length - 1]), sign.length SHALL
> > > +	 * therefore remain unchanged.
> > > +	 */
> >
> > [Shally] Again, this looks redundant info here.
> >
> > > +
> > > +	struct {
> > > +		enum rte_crypto_rsa_padding_type type;
> > > +		/**<
> > > +		 * 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 SHALL be
> > > +		 * available in in specific driver guide or capabilities.
> > > +		 */
> > [Shally] you mentioned it as part of capability, but this patch does not have
> > any capa struct changes to return this. So do you mean capability or PMD
> > release note here?
> 
> [AK] - this would be part of v2. Thanks.
> >
> > > +		enum rte_crypto_auth_algorithm hash;
> > > +		/**<
> > > +		 * -	For PKCS1-v1_5 signature (Block type 01) this field
> > > +		 * represents hash function that will be used to create
> > > +		 * message hash. This hash will be then concatenated with
> > > +		 * hash algorithm identifier into DER encoded sequence.
> >
> > [Shally] This whole description is already in RFC. It looks like an overload to
> > add all such details here. If really intended it is better to give rfc PKCS1 V2
> > reference here.
> 
> [AK] Ok.
> >
> > > +		 * If RTE_CRYPTO_HASH_INVALID is set, driver default will be
> > > set.
> > > +		 * If RTE_CRYPTO_HASH_NONE is set, message will be signed
> > > as it is.
> [AK] - provided message fits into key size.
> >
> > [Shally] who will set it? PMD or application? Its bit unclear here where and
> > when it will  be set to INVALID / NONE?
> 
> [AK] PMD according to its default. Still only a proposal.
> >
> > > +		 *
> > > +		 * -	For OAEP this field represents hash function that will
> > > +		 * be used to produce hash of the optional label.
> > > +		 * If RTE_CRYPTO_HASH_INVALID or
> > > RTE_CRYPTO_HASH_NONE is set
> > > +		 * driver will use default value. For OAEP usually it is SHA-1.
> > > +		 *
> > > +		 * -	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 RTE_CRYPTO_HASH_INVALID or
> > > RTE_CRYPTO_HASH_NONE is set
> > > +		 * driver will use default value.
> > > +		 *
> > > +		 * -	If driver supports only one function
> > > RTE_CRYPTO_HASH_NONE
> > > +		 * according to aformentioned restrictions should be used or
> > > +		 * specific function should be set, otherwise on dequeue the
> > > driver
> > > +		 * SHALL set crypto_op_status to
> > > RTE_CRYPTO_OP_STATUS_INVALID_ARGS.
> >
> > [Shally] Similar  feedback here.
> >
> > > +		 */
> > > +		union {
> > > +			struct {
> > > +				enum rte_crypto_auth_algorithm mgf;
> > > +				/**<
> > > +				 * Mask genereation function hash algorithm.
> > > +				 * If this field is not supported by the driver,
> > > +				 * it should be set to
> > > RTE_CRYPTO_HASH_NONE.
> > > +				 */
> > > +				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;
> > > +				/**<
> > > +				 * Mask genereation function hash algorithm.
> > > +				 * If this field is not supported by the driver,
> > > +				 * it should be set to
> > > RTE_CRYPTO_HASH_NONE.
> > > +				 */
> > > +				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;
> > > +		};
> > > +	} padding;
> >
> > [Shally] having additional struct padding within op_param looks unnecessary.
> > Why do we need to rearrange it like this? It can simply be:
> 
> [AK] - its proposal, to discuss I think.
> 
> >
> > struct rte_crypto_rsa_op_param {
> > 	enum rte_crypto_asym_op_type op_type;
> > 	/**< Type of RSA operation for transform */;
> >
> > 	rte_crypto_param cipher;
> > 	/**<
> > 	 * Pointer to data to be decrypted with length of data equal to RSA
> > modulus length
> >                 */
> >
> > 	rte_crypto_param message;
> > 	/**<
> > 	 * Pointer to data
> > 	 * - to be encrypted.
> > 	 * - to be signed.
> > 	 * - to be verified.
> > 	 */
> >
> > 	rte_crypto_param sign;
> > 	/**<
> > 	 * Pointer to RSA signature data. If operation is RSA
> > 	 * 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.
> > 	 */
> >
> > 	enum rte_crypto_rsa_padding_type pad;
> > 	/**< RSA padding scheme to be used for transform */
> >
> > > +		union {
> > > +			struct {
> > > +				enum rte_crypto_auth_algorithm mgf;
> > > +				/**<
> > > +				 * Mask genereation function hash algorithm.
> > > +				 * If this field is not supported by the driver,
> > > +				 * it should be set to
> > > RTE_CRYPTO_HASH_NONE.
> > > +				 */
> > > +				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;
> > > +				/**<
> > > +				 * Mask genereation function hash algorithm.
> > > +				 * If this field is not supported by the driver,
> > > +				 * it should be set to
> > > RTE_CRYPTO_HASH_NONE.
> > > +				 */
> > > +				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
> > > +	 * SHALL be put in specific driver guide
> >
> > [Shally] Again, probably this reference is unnecessary here.
> >
> > >  	 */
> > >  };
> > >
> > > --
> > > 2.1.0

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

[Shally Verma]

> -----Original Message-----
> From: Trahe, Fiona <fiona.trahe@intel.com>
> Sent: Friday, June 28, 2019 10:25 PM
> To: Kusztal, ArkadiuszX <arkadiuszx.kusztal@intel.com>; Shally Verma
> <shallyv@marvell.com>; dev@dpdk.org
> Cc: akhil.goyal@nxp.com; shally.verma@caviumnetworks.com; Trahe, Fiona
> <fiona.trahe@intel.com>
> Subject: RE: [EXT] [PATCH 1/3] cryptodev: rework api of rsa algorithm
> 
> Hi Arek, Shally,
> 
> Comments below.
> @Arek, I'd suggest sending a v2 after this, updated with whatever issues can
> be closed and a listing of the issues still open.
> As getting hard to follow the thread.
> @Shally, can you reply please - just with whatever items below you're ok
> with, so Arek can do the v2.
> And we can continue discussion on the v2 on anything still open.
> 
...

> > > > --- a/lib/librte_cryptodev/rte_crypto_asym.h
> > > > +++ b/lib/librte_cryptodev/rte_crypto_asym.h
> > > > @@ -111,23 +111,33 @@ 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
> > > > -	 */
> > > > -	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
> > > > +	/**< RSA no padding scheme.
> > > > +	 * In this case user is responsible for providing and verification
> > > > +	 * of padding.
> [Fiona] Grammar: provision not providing
> 
> > > > +	 * In case RTE_CRYPTO_ASYM_OP_VERIFY op type is used if no
> > > > +	 * problems in public key 'encryption' detected driver SHALL return
> > > > +	 * RTE_CRYPTO_OP_STATUS_SUCCESS.
> > >
> > > [Shally] This is not clear to me. OP_VERIFY is public key
> > > decryption, then why it is mentioned as 'encryption' above?
> >
> > [AK] - yes, you right 'decryption', public key but technically decryption.
> > > Secondly,  Any public/private key encryption with NO_PADDING scheme,
> > > would result in encryption data without any padding string.
> > > And, if same is passed to PMD for decryption with PADDING_NONE, then
> > > PMD would assume encryption block is without any padding string.
> > > So, which case are we trying to clarify here? Or, do you intend to
> > > mention case when app can pass data with padding for decryption with
> > > NO_PADDING scheme?
> >
> > [AK] Yes. It may be common situation that HW drivers wont natively be
> > doing any padding, Especially PSS and OAEP which expects very strong
> > TRNG. NO_PADDING would mean that driver should not be responsible for
> > padding, and not care what is in there as long as parameters are correct.
> [Fiona] just to add to that - PADDING_NONE typically really means
> PADDING_DONE_BY_APPLICATION But using the NONE terminology is
> consistent with openssl lib, so I think we should stick with that.
> 
[Shally] I look forward to v2 to comment further on this.

...

> >
> > [AK] About BT 00 it is nothing else than zero padding, plaintext RSA,
> > more or less the same thing as NO_PADDING. It has been silently
> > abandoned long time ago. Only RFC 2313 mentions it. No 2437,3447 and
> 8017.
> [Fiona] Actually as PKCS#1 v1.5 is less secure for encryption than OAEP,
> arguably it should be removed too from this enum to discourage its use.
> However as it's still commonly used we need to keep it. We should make
> sure that capability allows a PMD to opt out of supporting it if it chooses to
> only support PSS and OAEP.
> 
[Shally] We have been practically using it , so I can say it is still very much in use.
we can update capability to check for supported padding type if PMD does not support all. 
Regarding BT0 did we conclude to remove BT0 as practically not in use and then we're using PKCS _V1.5 only.
If yes, am fine with this.

> >
....

> > > [Shally] Better to add version V2 here: RSA PKCS#1 V2 OAEP padding
> > > scheme
> >
> > [AK] Usually V2 is not added to it. Example from openssl "
> > RSA_padding_add_PKCS1_OAEP",
> [Fiona] Agree with Arek, better to leave out v2.
> 
[Shally] Okay. Agree to both Arek, Fiona here. 

...

> > >
> > > >Returned data is in Big-Endian format which means
> > > > +	 * that Least-Significant byte will be placed at top byte of an array
> > > > +	 * (at message.data[message.length - 1]), cipher.length SHALL
> > > > +	 * therefore remain unchanged.
> > >
> > > [Shally] Do we really need to mention it?
> >
> > [AK] I have not pronounced myself very clear here, and I forgot to add it to
> the message data.
> > For example we have 256 bytes allocated for RSA-2048 let say
> > decryption. But we decrypted only 255 bytes (one leading zero).
> > So should we trim this zero or not? If we trim, size is 255 and
> > data[0] is some non zero byte. If we don't, size if 256 and data[0]=0.
> > One leading zero is a good example of PKCS1 padding.
> >
[Shally] Output of RSA public/private key encryption operation is length of modulus.
So it should return 256 length output here as is and let app use that.


> > >
> > > > +
> > > > +	struct {
> > > > +		enum rte_crypto_rsa_padding_type type;
> > > > +		/**<
> > > > +		 * 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 SHALL be
> > > > +		 * available in in specific driver guide or capabilities.
> > > > +		 */
> > > [Shally] you mentioned it as part of capability, but this patch does
> > > not have any capa struct changes to return this. So do you mean
> > > capability or PMD release note here?
> >
> > [AK] - this would be part of v2. Thanks.
[Shally] Okay.

....

> > > > +		 * If RTE_CRYPTO_HASH_INVALID is set, driver default will be
> > > > set.
> > > > +		 * If RTE_CRYPTO_HASH_NONE is set, message will be signed
> > > > as it is.
> > [AK] - provided message fits into key size.
> > >
> > > [Shally] who will set it? PMD or application? Its bit unclear here
> > > where and when it will  be set to INVALID / NONE?
> >
> > [AK] PMD according to its default. Still only a proposal.
[Shally] Okay. I look forward further discussion on this in v2.

...
> > > [Shally] having additional struct padding within op_param looks
> unnecessary.
> > > Why do we need to rearrange it like this? It can simply be:
> >
> > [AK] - its proposal, to discuss I think.
[Shally] I believe not needed though but look forward to v2 for further discussion.

...

Thanks
Shally
> > > > 2.1.0

[dpdk-dev] [PATCH 2/3] crypto/openssl: rework openssl rsa implementation

[Arek Kusztal]

This commit reworks implementation of RSA algorithm
in OPENSSL PMD to be conformant to API changes.

Signed-off-by: Arek Kusztal <arkadiuszx.kusztal@intel.com>
---
 drivers/crypto/openssl/rte_openssl_pmd.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/crypto/openssl/rte_openssl_pmd.c b/drivers/crypto/openssl/rte_openssl_pmd.c
index 73ce383..b0a8ea2 100644
--- a/drivers/crypto/openssl/rte_openssl_pmd.c
+++ b/drivers/crypto/openssl/rte_openssl_pmd.c
@@ -1842,15 +1842,13 @@ process_openssl_rsa_op(struct rte_crypto_op *cop,
 	int ret = 0;
 	struct rte_crypto_asym_op *op = cop->asym;
 	RSA *rsa = sess->u.r.rsa;
-	uint32_t pad = (op->rsa.pad);
+	uint32_t pad = (op->rsa.padding.type);
 	uint8_t *tmp;
 
 	cop->status = RTE_CRYPTO_OP_STATUS_SUCCESS;
 
 	switch (pad) {
-	case RTE_CRYPTO_RSA_PKCS1_V1_5_BT0:
-	case RTE_CRYPTO_RSA_PKCS1_V1_5_BT1:
-	case RTE_CRYPTO_RSA_PKCS1_V1_5_BT2:
+	case RTE_CRYPTO_RSA_PADDING_PKCS1:
 		pad = RSA_PKCS1_PADDING;
 		break;
 	case RTE_CRYPTO_RSA_PADDING_NONE:
@@ -1867,19 +1865,19 @@ process_openssl_rsa_op(struct rte_crypto_op *cop,
 	case RTE_CRYPTO_ASYM_OP_ENCRYPT:
 		ret = RSA_public_encrypt(op->rsa.message.length,
 				op->rsa.message.data,
-				op->rsa.message.data,
+				op->rsa.cipher.data,
 				rsa,
 				pad);
 
 		if (ret > 0)
-			op->rsa.message.length = ret;
+			op->rsa.cipher.length = ret;
 		OPENSSL_LOG(DEBUG,
 				"length of encrypted text %d\n", ret);
 		break;
 
 	case RTE_CRYPTO_ASYM_OP_DECRYPT:
-		ret = RSA_private_decrypt(op->rsa.message.length,
-				op->rsa.message.data,
+		ret = RSA_private_decrypt(op->rsa.cipher.length,
+				op->rsa.cipher.data,
 				op->rsa.message.data,
 				rsa,
 				pad);
-- 
2.1.0

Re: [dpdk-dev] [EXT] [PATCH 2/3] crypto/openssl: rework openssl rsa implementation

[Shally Verma]

> -----Original Message-----
> From: Arek Kusztal <arkadiuszx.kusztal@intel.com>
> Sent: Friday, May 31, 2019 7:23 PM
> To: dev@dpdk.org
> Cc: akhil.goyal@nxp.com; fiona.trahe@intel.com;
> shally.verma@caviumnetworks.com; Arek Kusztal
> <arkadiuszx.kusztal@intel.com>
> Subject: [EXT] [PATCH 2/3] crypto/openssl: rework openssl rsa
> implementation
> 
> External Email
> 
> ----------------------------------------------------------------------
> This commit reworks implementation of RSA algorithm in OPENSSL PMD to
> be conformant to API changes.
> 
> Signed-off-by: Arek Kusztal <arkadiuszx.kusztal@intel.com>
> ---
>  drivers/crypto/openssl/rte_openssl_pmd.c | 14 ++++++--------
>  1 file changed, 6 insertions(+), 8 deletions(-)
> 
Since I have some doubts on v1, so will wait for clarifications on that before commenting on v2 and v3.

Thanks
Shally

..
> 2.1.0

[dpdk-dev] [PATCH 3/3] test: rework rsa test implementation

[Arek Kusztal]

This commit reworks rsa test implementation to be conformant
to the RSA API.

Signed-off-by: Arek Kusztal <arkadiuszx.kusztal@intel.com>
---
 app/test/test_cryptodev_asym.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/app/test/test_cryptodev_asym.c b/app/test/test_cryptodev_asym.c
index fc92d3d..ea786ba 100644
--- a/app/test/test_cryptodev_asym.c
+++ b/app/test/test_cryptodev_asym.c
@@ -402,7 +402,7 @@ test_rsa_sign_verify(void)
 	asym_op->rsa.message.data = input_buf;
 	asym_op->rsa.message.length = rsaplaintext.len;
 	asym_op->rsa.sign.data = output_buf;
-	asym_op->rsa.pad = RTE_CRYPTO_RSA_PKCS1_V1_5_BT1;
+	asym_op->rsa.padding.type = RTE_CRYPTO_RSA_PADDING_PKCS1;
 
 	debug_hexdump(stdout, "message", asym_op->rsa.message.data,
 			asym_op->rsa.message.length);
@@ -437,7 +437,7 @@ test_rsa_sign_verify(void)
 
 	/* Verify sign */
 	asym_op->rsa.op_type = RTE_CRYPTO_ASYM_OP_VERIFY;
-	asym_op->rsa.pad = RTE_CRYPTO_RSA_PKCS1_V1_5_BT2;
+	asym_op->rsa.padding.type = RTE_CRYPTO_RSA_PADDING_PKCS1;
 
 	/* Process crypto operation */
 	if (rte_cryptodev_enqueue_burst(dev_id, 0, &op, 1) != 1) {
@@ -495,6 +495,7 @@ test_rsa_enc_dec(void)
 	struct rte_cryptodev_asym_session *sess = NULL;
 	int status = TEST_SUCCESS;
 	uint8_t input_buf[TEST_DATA_SIZE] = {0};
+	uint8_t cipher_buf[TEST_DATA_SIZE] = {0};
 
 	/* test case supports op with exponent key only,
 	 * Check in PMD feature flag for RSA exponent key type support.
@@ -547,7 +548,9 @@ test_rsa_enc_dec(void)
 			rsaplaintext.len);
 	asym_op->rsa.message.data = input_buf;
 	asym_op->rsa.message.length = rsaplaintext.len;
-	asym_op->rsa.pad = RTE_CRYPTO_RSA_PKCS1_V1_5_BT2;
+	asym_op->rsa.cipher.data = cipher_buf;
+	asym_op->rsa.cipher.length = 0;
+	asym_op->rsa.padding.type = RTE_CRYPTO_RSA_PADDING_PKCS1;
 
 	debug_hexdump(stdout, "message", asym_op->rsa.message.data,
 			asym_op->rsa.message.length);
@@ -581,7 +584,7 @@ test_rsa_enc_dec(void)
 	/* Use the resulted output as decryption Input vector*/
 	asym_op = result_op->asym;
 	asym_op->rsa.op_type = RTE_CRYPTO_ASYM_OP_DECRYPT;
-	asym_op->rsa.pad = RTE_CRYPTO_RSA_PKCS1_V1_5_BT1;
+	asym_op->rsa.padding.type = RTE_CRYPTO_RSA_PADDING_PKCS1;
 
 	/* Process crypto operation */
 	if (rte_cryptodev_enqueue_burst(dev_id, 0, &op, 1) != 1) {
-- 
2.1.0