[v5,2/7] cryptodev: add cipher field to RSA op

Message ID 20190718160943.10724-3-arkadiuszx.kusztal@intel.com (mailing list archive)
State Accepted, archived
Delegated to: akhil goyal
Headers
Series Rework API for RSA algorithm in asymmetric crypto |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation fail Compilation issues

Commit Message

Arkadiusz Kusztal July 18, 2019, 4:09 p.m. UTC
  Asymmetric nature of RSA algorithm suggest to use
additional field for output. In place operations
still can be done by setting cipher and message pointers
with the same memory address.

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

Comments

Shally Verma July 19, 2019, 4:42 a.m. UTC | #1
> -----Original Message-----
> From: Arek Kusztal <arkadiuszx.kusztal@intel.com>
> Sent: Thursday, July 18, 2019 9:40 PM
> To: dev@dpdk.org
> Cc: akhil.goyal@nxp.com; fiona.trahe@intel.com; Shally Verma
> <shallyv@marvell.com>; damianx.nowak@intel.com; Arek Kusztal
> <arkadiuszx.kusztal@intel.com>
> Subject: [EXT] [PATCH v5 2/7] cryptodev: add cipher field to RSA op
> 
> External Email
> 
> ----------------------------------------------------------------------
> Asymmetric nature of RSA algorithm suggest to use additional field for
> output. In place operations still can be done by setting cipher and message
> pointers with the same memory address.
> 
> Signed-off-by: Arek Kusztal <arkadiuszx.kusztal@intel.com>
> ---
>  lib/librte_cryptodev/rte_crypto_asym.h | 43
> ++++++++++++++++++++++++++++------
>  1 file changed, 36 insertions(+), 7 deletions(-)
> 
> diff --git a/lib/librte_cryptodev/rte_crypto_asym.h
> b/lib/librte_cryptodev/rte_crypto_asym.h
> index 02ec304..1d4ec80 100644
> --- a/lib/librte_cryptodev/rte_crypto_asym.h
> +++ b/lib/librte_cryptodev/rte_crypto_asym.h
> @@ -395,21 +395,50 @@ struct rte_crypto_rsa_op_param {
> 
>  	rte_crypto_param message;
>  	/**<
> -	 * Pointer to data
> +	 * Pointer to input 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.
> +	 *
> +	 * Pointer to output data
> +	 * - for RSA private decrypt.
> +	 * In this case the underlying array should have been
> +	 * allocated with enough memory to hold plaintext output
> +	 * (i.e. must be at least RSA key size). The message.length
> +	 * field should be 0 and will be overwritten by the PMD
> +	 * with the decrypted length.
> +	 *
> +	 * All data is in Octet-string network byte order format.
> +	 */
As per Fiona feedback in another email, for PMD it does not matter what output buffer length is set to. All matters if it should be allocated large enough as per description in spec.
Given that, there is no need to mention specifically, that length should be set to 0. App can leave it to anything as PMD don't care. It does not and should not check for any valid params here.
Ditto is my feedback on cipher.length description below. There is no need to mention, it should be set to 0 specifically

If we agree, this change can be taken as part of next patch set. Current one can still go on.

Thanks
Shally



> +
> +	rte_crypto_param cipher;
> +	/**<
> +	 * Pointer to input data
> +	 * - to be decrypted for RSA private decrypt.
> +	 *
> +	 * Pointer to output data
> +	 * - for RSA public encrypt.
> +	 * In this case the underlying array should have been allocated
> +	 * with enough memory to hold ciphertext output (i.e. must be
> +	 * at least RSA key size). The cipher.length field should
> +	 * be 0 and will be overwritten by the PMD with the encrypted
> length.
> +	 *
> +	 * All data is in Octet-string network byte order format.
>  	 */
> 
>  	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.
> +	 * Pointer to input data
> +	 * - to be verified for RSA public decrypt.
> +	 *
> +	 * Pointer to output data
> +	 * - for RSA private encrypt.
> +	 * In this case the underlying array should have been allocated
> +	 * with enough memory to hold signature output (i.e. must be
> +	 * at least RSA key size). The sign.length field should
> +	 * be 0 and will be overwritten by the PMD with the signature length.
>  	 *
> -	 * Length of the signature data will be equal to the
> -	 * RSA modulus length.
> +	 * All data is in Octet-string network byte order format.
>  	 */
> 
>  	enum rte_crypto_rsa_padding_type pad;
> --
> 2.1.0
  
Arkadiusz Kusztal July 19, 2019, 5:10 a.m. UTC | #2
> > ----------------------------------------------------------------------
> > Asymmetric nature of RSA algorithm suggest to use additional field for
> > output. In place operations still can be done by setting cipher and
> > message pointers with the same memory address.
> >
> > Signed-off-by: Arek Kusztal <arkadiuszx.kusztal@intel.com>
> > ---
> >  lib/librte_cryptodev/rte_crypto_asym.h | 43
> > ++++++++++++++++++++++++++++------
> >  1 file changed, 36 insertions(+), 7 deletions(-)
> >
> > diff --git a/lib/librte_cryptodev/rte_crypto_asym.h
> > b/lib/librte_cryptodev/rte_crypto_asym.h
> > index 02ec304..1d4ec80 100644
> > --- a/lib/librte_cryptodev/rte_crypto_asym.h
> > +++ b/lib/librte_cryptodev/rte_crypto_asym.h
> > @@ -395,21 +395,50 @@ struct rte_crypto_rsa_op_param {
> >
> >  	rte_crypto_param message;
> >  	/**<
> > -	 * Pointer to data
> > +	 * Pointer to input 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.
> > +	 *
> > +	 * Pointer to output data
> > +	 * - for RSA private decrypt.
> > +	 * In this case the underlying array should have been
> > +	 * allocated with enough memory to hold plaintext output
> > +	 * (i.e. must be at least RSA key size). The message.length
> > +	 * field should be 0 and will be overwritten by the PMD
> > +	 * with the decrypted length.
> > +	 *
> > +	 * All data is in Octet-string network byte order format.
> > +	 */
> As per Fiona feedback in another email, for PMD it does not matter what
> output buffer length is set to. All matters if it should be allocated large
> enough as per description in spec.
> Given that, there is no need to mention specifically, that length should be set
> to 0. App can leave it to anything as PMD don't care. It does not and should
> not check for any valid params here.
> Ditto is my feedback on cipher.length description below. There is no need to
> mention, it should be set to 0 specifically
> 
> If we agree, this change can be taken as part of next patch set. Current one
> can still go on.

I agree with Shally that it could be anything to work, but on the other hand I agree with Pablo and Fiona comment on future extensions and ABI breakage. Especially on so early level of API development. When we change this field in future that it can be random (which is possible) it will not break anything, but it would not work in the opposite direction.

> 
> Thanks
> Shally
> 
> 
> 
> > +
> > +	rte_crypto_param cipher;
> > +	/**<
> > +	 * Pointer to input data
> > +	 * - to be decrypted for RSA private decrypt.
> > +	 *
> > +	 * Pointer to output data
> > +	 * - for RSA public encrypt.
> > +	 * In this case the underlying array should have been allocated
> > +	 * with enough memory to hold ciphertext output (i.e. must be
> > +	 * at least RSA key size). The cipher.length field should
> > +	 * be 0 and will be overwritten by the PMD with the encrypted
> > length.
> > +	 *
> > +	 * All data is in Octet-string network byte order format.
> >  	 */
> >
> >  	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.
> > +	 * Pointer to input data
> > +	 * - to be verified for RSA public decrypt.
> > +	 *
> > +	 * Pointer to output data
> > +	 * - for RSA private encrypt.
> > +	 * In this case the underlying array should have been allocated
> > +	 * with enough memory to hold signature output (i.e. must be
> > +	 * at least RSA key size). The sign.length field should
> > +	 * be 0 and will be overwritten by the PMD with the signature length.
> >  	 *
> > -	 * Length of the signature data will be equal to the
> > -	 * RSA modulus length.
> > +	 * All data is in Octet-string network byte order format.
> >  	 */
> >
> >  	enum rte_crypto_rsa_padding_type pad;
> > --
> > 2.1.0
  

Patch

diff --git a/lib/librte_cryptodev/rte_crypto_asym.h b/lib/librte_cryptodev/rte_crypto_asym.h
index 02ec304..1d4ec80 100644
--- a/lib/librte_cryptodev/rte_crypto_asym.h
+++ b/lib/librte_cryptodev/rte_crypto_asym.h
@@ -395,21 +395,50 @@  struct rte_crypto_rsa_op_param {
 
 	rte_crypto_param message;
 	/**<
-	 * Pointer to data
+	 * Pointer to input 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.
+	 *
+	 * Pointer to output data
+	 * - for RSA private decrypt.
+	 * In this case the underlying array should have been
+	 * allocated with enough memory to hold plaintext output
+	 * (i.e. must be at least RSA key size). The message.length
+	 * field should be 0 and will be overwritten by the PMD
+	 * with the decrypted length.
+	 *
+	 * All data is in Octet-string network byte order format.
+	 */
+
+	rte_crypto_param cipher;
+	/**<
+	 * Pointer to input data
+	 * - to be decrypted for RSA private decrypt.
+	 *
+	 * Pointer to output data
+	 * - for RSA public encrypt.
+	 * In this case the underlying array should have been allocated
+	 * with enough memory to hold ciphertext output (i.e. must be
+	 * at least RSA key size). The cipher.length field should
+	 * be 0 and will be overwritten by the PMD with the encrypted length.
+	 *
+	 * All data is in Octet-string network byte order format.
 	 */
 
 	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.
+	 * Pointer to input data
+	 * - to be verified for RSA public decrypt.
+	 *
+	 * Pointer to output data
+	 * - for RSA private encrypt.
+	 * In this case the underlying array should have been allocated
+	 * with enough memory to hold signature output (i.e. must be
+	 * at least RSA key size). The sign.length field should
+	 * be 0 and will be overwritten by the PMD with the signature length.
 	 *
-	 * Length of the signature data will be equal to the
-	 * RSA modulus length.
+	 * All data is in Octet-string network byte order format.
 	 */
 
 	enum rte_crypto_rsa_padding_type pad;