| Message ID | 20220520055445.40063-30-arkadiuszx.kusztal@intel.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | akhil goyal |
| Headers | show |
| Series | cryptodev: rsa, dh, ecdh changes | expand |
| Context | Check | Description |
|---|---|---|
| ci/checkpatch | success | coding style OK |
> - 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
> -----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
> > > 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 --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. + */ }; /**
- 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(-)