diff mbox series

[29/40] cryptodev: add salt length and optional label

Message ID 20220520055445.40063-30-arkadiuszx.kusztal@intel.com (mailing list archive)
State Changes Requested, archived
Delegated to: akhil goyal
Headers
Series cryptodev: rsa, dh, ecdh changes |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Arkadiusz Kusztal May 20, 2022, 5:54 a.m. UTC 
  - added salt length and optional label.
Common parameters to PSS and OAEP padding for RSA.
- Fixed hash API in RSA padding.
Now it is specified how hash should be used with
particular RSA padding modes.

Signed-off-by: Arek Kusztal <arkadiuszx.kusztal@intel.com>
---
 lib/cryptodev/rte_crypto_asym.h | 44 +++++++++++++++++++++++++++++++++++------
 1 file changed, 38 insertions(+), 6 deletions(-)

Comments

Akhil Goyal May 24, 2022, 12:30 p.m. UTC | #1 
> - added salt length and optional label.
> Common parameters to PSS and OAEP padding for RSA.

Please add description about how it is expected to be used.

> - Fixed hash API in RSA padding.
> Now it is specified how hash should be used with
> particular RSA padding modes.

I believe this should be a separate patch. Right?
Patch title does not justify this

> 
> Signed-off-by: Arek Kusztal <arkadiuszx.kusztal@intel.com>
> ---
>  lib/cryptodev/rte_crypto_asym.h | 44
> +++++++++++++++++++++++++++++++++++------
>  1 file changed, 38 insertions(+), 6 deletions(-)
> 
> diff --git a/lib/cryptodev/rte_crypto_asym.h b/lib/cryptodev/rte_crypto_asym.h
> index 97c3fbee38..c864b8a115 100644
> --- a/lib/cryptodev/rte_crypto_asym.h
> +++ b/lib/cryptodev/rte_crypto_asym.h
> @@ -205,12 +205,29 @@ struct rte_crypto_rsa_priv_key_qt {
>   */
>  struct rte_crypto_rsa_padding {
>  	enum rte_crypto_rsa_padding_type type;
> -	/**< RSA padding scheme to be used for transform */
> -	enum rte_crypto_auth_algorithm md;

Any specific reason to change the field name?
I think this matches with the next field mgf1md

> -	/**< 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
> +	/**< Type of RSA padding */
> +	enum rte_crypto_auth_algorithm hash;
> +	/**<
> +	 * RSA padding hash function
> +	 *
> +	 * When a specific padding type is selected, the following rule apply:
> +	 * - RTE_CRYPTO_RSA_PADDING_NONE:
> +	 * This field is ignored by the PMD
> +	 *
> +	 * - RTE_CRYPTO_RSA_PADDING_PKCS1_5:
> +	 * When signing operation this field is used to determine value
> +	 * of the DigestInfo structure, therefore specifying which algorithm
> +	 * was used to create the message digest.
> +	 * When doing encryption/decryption this field is ignored for this
> +	 * padding type.
> +	 *
> +	 * - RTE_CRYPTO_RSA_PADDING_OAEP
> +	 * This field shall be set with the hash algorithm used
> +	 * in the padding scheme
> +	 *
> +	 * - RTE_CRYPTO_RSA_PADDING_PSS
> +	 * This field shall be set with the hash algorithm used
> +	 * in the padding scheme (and to create the input message digest)
>  	 */
>  	enum rte_crypto_auth_algorithm mgf1md;
>  	/**<
> @@ -220,6 +237,21 @@ struct rte_crypto_rsa_padding {
>  	 * for mask generation. Valid hash algorithms are:
>  	 * MD5, SHA1, SHA224, SHA256, SHA384, SHA512
>  	 */
> +	uint16_t saltlen;
> +	/**<
> +	 * RSA PSS padding salt length
> +	 *
> +	 * Used only when RTE_CRYPTO_RSA_PADDING_PSS padding is
> selected,

Used only when RTE_CRYPTO_RSA_PADDING_PSS is selected,

> +	 * otherwise ignored.
> +	 */
> +	rte_crypto_param label;
> +	/**<
> +	 * RSA OAEP padding optional label
> +	 *
> +	 * Used only when RTE_CRYPTO_RSA_PADDING_OAEP padding is
> selected,

Drop the word padding.

BTW, can this be a union for label and saltlen?
Also can we name them as pss_saltlen and oaep_label?

> +	 * otherwise ignored. If label.data == NULL, a default
> +	 * label (empty string) is used.
> +	 */
>  };
> 
>  /**
> --
> 2.13.6
Arkadiusz Kusztal May 24, 2022, 3:14 p.m. UTC | #2 
> -----Original Message-----
> From: Akhil Goyal <gakhil@marvell.com>
> Sent: Tuesday, May 24, 2022 2:30 PM
> To: Kusztal, ArkadiuszX <arkadiuszx.kusztal@intel.com>; dev@dpdk.org
> Cc: Anoob Joseph <anoobj@marvell.com>; Zhang, Roy Fan
> <roy.fan.zhang@intel.com>
> Subject: RE: [EXT] [PATCH 29/40] cryptodev: add salt length and optional label
> 
> > - added salt length and optional label.
> > Common parameters to PSS and OAEP padding for RSA.
> 
> Please add description about how it is expected to be used.
> 
> > - Fixed hash API in RSA padding.
> > Now it is specified how hash should be used with particular RSA
> > padding modes.
> 
> I believe this should be a separate patch. Right?
[Arek] +1
> Patch title does not justify this
> 
> >
> > Signed-off-by: Arek Kusztal <arkadiuszx.kusztal@intel.com>
> > ---
> >  lib/cryptodev/rte_crypto_asym.h | 44
> > +++++++++++++++++++++++++++++++++++------
> >  1 file changed, 38 insertions(+), 6 deletions(-)
> >
> > diff --git a/lib/cryptodev/rte_crypto_asym.h
> > b/lib/cryptodev/rte_crypto_asym.h index 97c3fbee38..c864b8a115 100644
> > --- a/lib/cryptodev/rte_crypto_asym.h
> > +++ b/lib/cryptodev/rte_crypto_asym.h
> > @@ -205,12 +205,29 @@ struct rte_crypto_rsa_priv_key_qt {
> >   */
> >  struct rte_crypto_rsa_padding {
> >  	enum rte_crypto_rsa_padding_type type;
> > -	/**< RSA padding scheme to be used for transform */
> > -	enum rte_crypto_auth_algorithm md;
> 
> Any specific reason to change the field name?
> I think this matches with the next field mgf1md
[Arek] - now it aligns with RSA RFC. Both current names comes from the OpenSSL EVP_MD naming, in my rfc initially mgf1md was changed too into:
+enum rte_crypto_mgf {
+	RTE_CRYPTO_MGF_DEFAULT,
+	/**< Default mask generation function */
+	RTE_CRYPTO_MGF_MGF1_SHA1,
+	/**< MGF1 function with SHA1 hash algorithm */
But we do not need to be that conformant with the standard I think, so I have left it out.
As for names it may be 'md' as well, every name is ok if is not excessively long.

> 
> > -	/**< 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
> > +	/**< Type of RSA padding */
> > +	enum rte_crypto_auth_algorithm hash;
> > +	/**<
> > +	 * RSA padding hash function
> > +	 *
> > +	 * When a specific padding type is selected, the following rule apply:
> > +	 * - RTE_CRYPTO_RSA_PADDING_NONE:
> > +	 * This field is ignored by the PMD
> > +	 *
> > +	 * - RTE_CRYPTO_RSA_PADDING_PKCS1_5:
> > +	 * When signing operation this field is used to determine value
> > +	 * of the DigestInfo structure, therefore specifying which algorithm
> > +	 * was used to create the message digest.
> > +	 * When doing encryption/decryption this field is ignored for this
> > +	 * padding type.
> > +	 *
> > +	 * - RTE_CRYPTO_RSA_PADDING_OAEP
> > +	 * This field shall be set with the hash algorithm used
> > +	 * in the padding scheme
> > +	 *
> > +	 * - RTE_CRYPTO_RSA_PADDING_PSS
> > +	 * This field shall be set with the hash algorithm used
> > +	 * in the padding scheme (and to create the input message digest)
> >  	 */
> >  	enum rte_crypto_auth_algorithm mgf1md;
> >  	/**<
> > @@ -220,6 +237,21 @@ struct rte_crypto_rsa_padding {
> >  	 * for mask generation. Valid hash algorithms are:
> >  	 * MD5, SHA1, SHA224, SHA256, SHA384, SHA512
> >  	 */
> > +	uint16_t saltlen;
> > +	/**<
> > +	 * RSA PSS padding salt length
> > +	 *
> > +	 * Used only when RTE_CRYPTO_RSA_PADDING_PSS padding is
> > selected,
> 
> Used only when RTE_CRYPTO_RSA_PADDING_PSS is selected,
> 
> > +	 * otherwise ignored.
> > +	 */
> > +	rte_crypto_param label;
> > +	/**<
> > +	 * RSA OAEP padding optional label
> > +	 *
> > +	 * Used only when RTE_CRYPTO_RSA_PADDING_OAEP padding is
> > selected,
> 
> Drop the word padding.
> 
> BTW, can this be a union for label and saltlen?
Yes, will do.
> Also can we name them as pss_saltlen and oaep_label?
Yes, though I am not entirely convinced. These names are unique anyway.
> 
> > +	 * otherwise ignored. If label.data == NULL, a default
> > +	 * label (empty string) is used.
> > +	 */
> >  };
> >
> >  /**
> > --
> > 2.13.6
Akhil Goyal May 25, 2022, 5:57 a.m. UTC | #3 
> > > diff --git a/lib/cryptodev/rte_crypto_asym.h
> > > b/lib/cryptodev/rte_crypto_asym.h index 97c3fbee38..c864b8a115 100644
> > > --- a/lib/cryptodev/rte_crypto_asym.h
> > > +++ b/lib/cryptodev/rte_crypto_asym.h
> > > @@ -205,12 +205,29 @@ struct rte_crypto_rsa_priv_key_qt {
> > >   */
> > >  struct rte_crypto_rsa_padding {
> > >  	enum rte_crypto_rsa_padding_type type;
> > > -	/**< RSA padding scheme to be used for transform */
> > > -	enum rte_crypto_auth_algorithm md;
> >
> > Any specific reason to change the field name?
> > I think this matches with the next field mgf1md
> [Arek] - now it aligns with RSA RFC. Both current names comes from the
> OpenSSL EVP_MD naming, in my rfc initially mgf1md was changed too into:
> +enum rte_crypto_mgf {
> +	RTE_CRYPTO_MGF_DEFAULT,
> +	/**< Default mask generation function */
> +	RTE_CRYPTO_MGF_MGF1_SHA1,
> +	/**< MGF1 function with SHA1 hash algorithm */
> But we do not need to be that conformant with the standard I think, so I have
> left it out.
> As for names it may be 'md' as well, every name is ok if is not excessively long.
> 
No strong opinion, you can keep any of them.

> >
> > > -	/**< 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
> > > +	/**< Type of RSA padding */
> > > +	enum rte_crypto_auth_algorithm hash;
> > > +	/**<
> > > +	 * RSA padding hash function
> > > +	 *
> > > +	 * When a specific padding type is selected, the following rule apply:
> > > +	 * - RTE_CRYPTO_RSA_PADDING_NONE:
> > > +	 * This field is ignored by the PMD
> > > +	 *
> > > +	 * - RTE_CRYPTO_RSA_PADDING_PKCS1_5:
> > > +	 * When signing operation this field is used to determine value
> > > +	 * of the DigestInfo structure, therefore specifying which algorithm
> > > +	 * was used to create the message digest.
> > > +	 * When doing encryption/decryption this field is ignored for this
> > > +	 * padding type.
> > > +	 *
> > > +	 * - RTE_CRYPTO_RSA_PADDING_OAEP
> > > +	 * This field shall be set with the hash algorithm used
> > > +	 * in the padding scheme
> > > +	 *
> > > +	 * - RTE_CRYPTO_RSA_PADDING_PSS
> > > +	 * This field shall be set with the hash algorithm used
> > > +	 * in the padding scheme (and to create the input message digest)
> > >  	 */
> > >  	enum rte_crypto_auth_algorithm mgf1md;
> > >  	/**<
> > > @@ -220,6 +237,21 @@ struct rte_crypto_rsa_padding {
> > >  	 * for mask generation. Valid hash algorithms are:
> > >  	 * MD5, SHA1, SHA224, SHA256, SHA384, SHA512
> > >  	 */
> > > +	uint16_t saltlen;
> > > +	/**<
> > > +	 * RSA PSS padding salt length
> > > +	 *
> > > +	 * Used only when RTE_CRYPTO_RSA_PADDING_PSS padding is
> > > selected,
> >
> > Used only when RTE_CRYPTO_RSA_PADDING_PSS is selected,
> >
> > > +	 * otherwise ignored.
> > > +	 */
> > > +	rte_crypto_param label;
> > > +	/**<
> > > +	 * RSA OAEP padding optional label
> > > +	 *
> > > +	 * Used only when RTE_CRYPTO_RSA_PADDING_OAEP padding is
> > > selected,
> >
> > Drop the word padding.
> >
> > BTW, can this be a union for label and saltlen?
> Yes, will do.
> > Also can we name them as pss_saltlen and oaep_label?
> Yes, though I am not entirely convinced. These names are unique anyway.
I believe it will improve readability.

> >
> > > +	 * otherwise ignored. If label.data == NULL, a default
> > > +	 * label (empty string) is used.
> > > +	 */
> > >  };
> > >
> > >  /**
> > > --
> > > 2.13.6
diff mbox series

Patch

diff --git a/lib/cryptodev/rte_crypto_asym.h b/lib/cryptodev/rte_crypto_asym.h
index 97c3fbee38..c864b8a115 100644
--- a/lib/cryptodev/rte_crypto_asym.h
+++ b/lib/cryptodev/rte_crypto_asym.h
@@ -205,12 +205,29 @@  struct rte_crypto_rsa_priv_key_qt {
  */
 struct rte_crypto_rsa_padding {
 	enum rte_crypto_rsa_padding_type type;
-	/**< 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
+	/**< Type of RSA padding */
+	enum rte_crypto_auth_algorithm hash;
+	/**<
+	 * RSA padding hash function
+	 *
+	 * When a specific padding type is selected, the following rule apply:
+	 * - RTE_CRYPTO_RSA_PADDING_NONE:
+	 * This field is ignored by the PMD
+	 *
+	 * - RTE_CRYPTO_RSA_PADDING_PKCS1_5:
+	 * When signing operation this field is used to determine value
+	 * of the DigestInfo structure, therefore specifying which algorithm
+	 * was used to create the message digest.
+	 * When doing encryption/decryption this field is ignored for this
+	 * padding type.
+	 *
+	 * - RTE_CRYPTO_RSA_PADDING_OAEP
+	 * This field shall be set with the hash algorithm used
+	 * in the padding scheme
+	 *
+	 * - RTE_CRYPTO_RSA_PADDING_PSS
+	 * This field shall be set with the hash algorithm used
+	 * in the padding scheme (and to create the input message digest)
 	 */
 	enum rte_crypto_auth_algorithm mgf1md;
 	/**<
@@ -220,6 +237,21 @@  struct rte_crypto_rsa_padding {
 	 * for mask generation. Valid hash algorithms are:
 	 * MD5, SHA1, SHA224, SHA256, SHA384, SHA512
 	 */
+	uint16_t saltlen;
+	/**<
+	 * RSA PSS padding salt length
+	 *
+	 * Used only when RTE_CRYPTO_RSA_PADDING_PSS padding is selected,
+	 * otherwise ignored.
+	 */
+	rte_crypto_param label;
+	/**<
+	 * RSA OAEP padding optional label
+	 *
+	 * Used only when RTE_CRYPTO_RSA_PADDING_OAEP padding is selected,
+	 * otherwise ignored. If label.data == NULL, a default
+	 * label (empty string) is used.
+	 */
 };
 
 /**