DPDK patches and discussions
 help / color / mirror / Atom feed
* [PATCH 0/5] cryptodev: fix inconsistency in RSA op usage
@ 2021-11-29  9:51 Ramkumar Balu
  2021-11-29  9:51 ` [PATCH 1/5] cryptodev: fix RSA op cipher field description Ramkumar Balu
                   ` (5 more replies)
  0 siblings, 6 replies; 9+ messages in thread
From: Ramkumar Balu @ 2021-11-29  9:51 UTC (permalink / raw)
  To: Akhil Goyal, Anoob Joseph, Declan Doherty, Fan Zhang,
	Ankur Dwivedi, Tejasree Kondoj
  Cc: stable, dev, Ramkumar

From: Ramkumar <rbalu@marvell.com>

The RSA verify operation is performed in two stages: 1. decrypt using
public key (output: plaintext message) 2. Compare resultant plaintext
message with the expected plaintext message to verify. (return
succ/fail in status field) Some applications need the decrypted
plaintext (stage 1 result) also to be retunred. For reference, OpenSSL
also provides similar API (RSA_public_decrypt).

lib cryptodev API failed to specify a field in 'struct
rte_crypto_rsa_op_param' to return the plaintext result after public
key decryption. It created inconsistency among crypto PMDs in returning
plaintext during RSA verify.

Inconsistency in RSA verify,
crypto/octeontx - uses 'sign' field to return plaintext
crypto/cnxk - uses 'sign' field to return plaintext
crypto/openssl - does not return plaintext
crypto/qat - uses 'cipher' field to return plaintext
test/cryptodev_asym - expects PMDs to use 'cipher' field

Thus, this patch series fixes all usages to only use 'cipher' field for
above described scenario.  The 'sign' and 'message' fields are not
chosen as they are used for different purpose under same operation.

rte_crypto_rsa_op_param struct fields to use for
RTE_CRYPTO_ASYM_OP_VERIFY:
1. input: rsa.sign - signature to be decrypted or verified
2. input: rsa.message - expected plaintext, used to compare
3. output: rsa.cipher - resultant plaintext from decryption


Ramkumar (5):
  cryptodev: fix RSA op cipher field description
  crypto/openssl: fix output of RSA verify op
  crypto/octeontx: fix output field for RSA verify
  crypto/octeontx2: fix output field for RSA verify
  crypto/cnxk: fix output field for RSA verify

 drivers/crypto/cnxk/cnxk_ae.h                 | 15 +++++++++------
 drivers/crypto/octeontx/otx_cryptodev_ops.c   | 10 ++++++----
 drivers/crypto/octeontx2/otx2_cryptodev_ops.c | 16 +++++++++-------
 drivers/crypto/openssl/rte_openssl_pmd.c      | 16 +++++++++++-----
 lib/cryptodev/rte_crypto_asym.h               |  7 ++++---
 5 files changed, 39 insertions(+), 25 deletions(-)

-- 
2.17.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/5] cryptodev: fix RSA op cipher field description
  2021-11-29  9:51 [PATCH 0/5] cryptodev: fix inconsistency in RSA op usage Ramkumar Balu
@ 2021-11-29  9:51 ` Ramkumar Balu
  2021-11-29  9:51 ` [PATCH 2/5] crypto/openssl: fix output of RSA verify op Ramkumar Balu
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 9+ messages in thread
From: Ramkumar Balu @ 2021-11-29  9:51 UTC (permalink / raw)
  To: Akhil Goyal, Anoob Joseph, Declan Doherty, Fan Zhang,
	Ankur Dwivedi, Tejasree Kondoj
  Cc: stable, dev, Ramkumar

From: Ramkumar <rbalu@marvell.com>

The description for 'struct rte_crypto_rsa_op_param' failed to specify
a field for returning the plaintext from RSA public key decryption.

This patch fixes the rte_crypto_rsa_op_param description to specify
'cipher' field to be used for returning plaintext during RSA op_type
== RTE_CRYPTO_ASYM_OP_VERIFY.

Fixes: 501ed9c6611f ("cryptodev: add cipher field to RSA op")
Cc: stable@dpdk.org

Signed-off-by: Ramkumar <rbalu@marvell.com>
---
 lib/cryptodev/rte_crypto_asym.h | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/lib/cryptodev/rte_crypto_asym.h b/lib/cryptodev/rte_crypto_asym.h
index 9c866f553f..b99cf6843c 100644
--- a/lib/cryptodev/rte_crypto_asym.h
+++ b/lib/cryptodev/rte_crypto_asym.h
@@ -461,11 +461,12 @@ struct rte_crypto_rsa_op_param {
 	 * - to be decrypted for RSA private decrypt.
 	 *
 	 * Pointer to output data
-	 * - for RSA public encrypt.
+	 * - for RSA public encrypt/decrypt.
 	 * In this case the underlying array should have been allocated
-	 * with enough memory to hold ciphertext output (i.e. must be
+	 * with enough memory to hold ciphertext/plaintext 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.
+	 * be 0 and will be overwritten by the PMD with the encrypted/decrypted
+	 * length.
 	 *
 	 * All data is in Octet-string network byte order format.
 	 */
-- 
2.17.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 2/5] crypto/openssl: fix output of RSA verify op
  2021-11-29  9:51 [PATCH 0/5] cryptodev: fix inconsistency in RSA op usage Ramkumar Balu
  2021-11-29  9:51 ` [PATCH 1/5] cryptodev: fix RSA op cipher field description Ramkumar Balu
@ 2021-11-29  9:51 ` Ramkumar Balu
  2021-12-28  9:10   ` Kusztal, ArkadiuszX
  2021-11-29  9:51 ` [PATCH 3/5] crypto/octeontx: fix output field for RSA verify Ramkumar Balu
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 9+ messages in thread
From: Ramkumar Balu @ 2021-11-29  9:51 UTC (permalink / raw)
  To: Akhil Goyal, Anoob Joseph, Declan Doherty, Fan Zhang,
	Ankur Dwivedi, Tejasree Kondoj
  Cc: stable, dev, Ramkumar

From: Ramkumar <rbalu@marvell.com>

During RSA verify, the OpenSSL PMD fails to return the plaintext after
public key decryption.
This patch fixes the OpenSSL PMD to return the decrypted plaintext in
cipher.data / cipher.length fields

Fixes: 3e9d6bd447fb ("crypto/openssl: add RSA and mod asym operations")
Fixes: fe1606e0138c ("crypto/openssl: fix RSA verify operation")
Cc: stable@dpdk.org

Signed-off-by: Ramkumar <rbalu@marvell.com>
---
 drivers/crypto/openssl/rte_openssl_pmd.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/drivers/crypto/openssl/rte_openssl_pmd.c b/drivers/crypto/openssl/rte_openssl_pmd.c
index 5794ed8159..3ab2c3b5c1 100644
--- a/drivers/crypto/openssl/rte_openssl_pmd.c
+++ b/drivers/crypto/openssl/rte_openssl_pmd.c
@@ -1953,12 +1953,16 @@ process_openssl_rsa_op(struct rte_crypto_op *cop,
 		break;
 
 	case RTE_CRYPTO_ASYM_OP_VERIFY:
-		tmp = rte_malloc(NULL, op->rsa.sign.length, 0);
+		tmp = op->rsa.cipher.data;
 		if (tmp == NULL) {
-			OPENSSL_LOG(ERR, "Memory allocation failed");
-			cop->status = RTE_CRYPTO_OP_STATUS_ERROR;
-			break;
+			tmp = rte_malloc(NULL, op->rsa.sign.length, 0);
+			if (tmp == NULL) {
+				OPENSSL_LOG(ERR, "Memory allocation failed");
+				cop->status = RTE_CRYPTO_OP_STATUS_ERROR;
+				break;
+			}
 		}
+
 		ret = RSA_public_decrypt(op->rsa.sign.length,
 				op->rsa.sign.data,
 				tmp,
@@ -1974,7 +1978,9 @@ process_openssl_rsa_op(struct rte_crypto_op *cop,
 			OPENSSL_LOG(ERR, "RSA sign Verification failed");
 			cop->status = RTE_CRYPTO_OP_STATUS_ERROR;
 		}
-		rte_free(tmp);
+		op->rsa.cipher.length = ret;
+		if (tmp != op->rsa.cipher.data)
+			rte_free(tmp);
 		break;
 
 	default:
-- 
2.17.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 3/5] crypto/octeontx: fix output field for RSA verify
  2021-11-29  9:51 [PATCH 0/5] cryptodev: fix inconsistency in RSA op usage Ramkumar Balu
  2021-11-29  9:51 ` [PATCH 1/5] cryptodev: fix RSA op cipher field description Ramkumar Balu
  2021-11-29  9:51 ` [PATCH 2/5] crypto/openssl: fix output of RSA verify op Ramkumar Balu
@ 2021-11-29  9:51 ` Ramkumar Balu
  2021-11-29  9:51 ` [PATCH 4/5] crypto/octeontx2: " Ramkumar Balu
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 9+ messages in thread
From: Ramkumar Balu @ 2021-11-29  9:51 UTC (permalink / raw)
  To: Akhil Goyal, Anoob Joseph, Declan Doherty, Fan Zhang,
	Ankur Dwivedi, Tejasree Kondoj
  Cc: stable, dev, Ramkumar

From: Ramkumar <rbalu@marvell.com>

During RSA sign verification, the OCTEONTX PMD returns the decrypted
plaintext in 'sign' field of rte_crypto_rsa_op_param. The 'sign' field
is actually used to pass input to the operation. This PMD overwrites
the 'sign' field buffer. This is non-compliance to lib cryptodev.

This patch fixes the PMD to use 'cipher' field to return the decrypted
plaintext during RSA verify operation.

Fixes: e9a356e2fc71 ("crypto/octeontx: add asymmetric enqueue/dequeue ops")
Cc: stable@dpdk.org

Signed-off-by: Ramkumar <rbalu@marvell.com>
---
 drivers/crypto/octeontx/otx_cryptodev_ops.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/crypto/octeontx/otx_cryptodev_ops.c b/drivers/crypto/octeontx/otx_cryptodev_ops.c
index 9e8fd495cf..07ce079d87 100644
--- a/drivers/crypto/octeontx/otx_cryptodev_ops.c
+++ b/drivers/crypto/octeontx/otx_cryptodev_ops.c
@@ -788,18 +788,20 @@ otx_cpt_asym_rsa_op(struct rte_crypto_op *cop, struct cpt_request_info *req,
 		break;
 	case RTE_CRYPTO_ASYM_OP_VERIFY:
 		if (rsa->pad == RTE_CRYPTO_RSA_PADDING_NONE)
-			rsa->sign.length = rsa_ctx->n.length;
+			rsa->cipher.length = rsa_ctx->n.length;
 		else {
 			/* Get length of decrypted output */
-			rsa->sign.length = rte_cpu_to_be_16
+			rsa->cipher.length = rte_cpu_to_be_16
 					(*((uint16_t *)req->rptr));
 
 			/* Offset data pointer by length fields */
 			req->rptr += 2;
 		}
-		memcpy(rsa->sign.data, req->rptr, rsa->sign.length);
 
-		if (memcmp(rsa->sign.data, rsa->message.data,
+		if (rsa->cipher.data != NULL)
+			memcpy(rsa->cipher.data, req->rptr, rsa->cipher.length);
+
+		if (memcmp(req->rptr, rsa->message.data,
 			   rsa->message.length)) {
 			CPT_LOG_DP_ERR("RSA verification failed");
 			cop->status = RTE_CRYPTO_OP_STATUS_ERROR;
-- 
2.17.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 4/5] crypto/octeontx2: fix output field for RSA verify
  2021-11-29  9:51 [PATCH 0/5] cryptodev: fix inconsistency in RSA op usage Ramkumar Balu
                   ` (2 preceding siblings ...)
  2021-11-29  9:51 ` [PATCH 3/5] crypto/octeontx: fix output field for RSA verify Ramkumar Balu
@ 2021-11-29  9:51 ` Ramkumar Balu
  2021-11-29  9:51 ` [PATCH 5/5] crypto/cnxk: " Ramkumar Balu
  2021-12-28  8:58 ` [PATCH 0/5] cryptodev: fix inconsistency in RSA op usage Kusztal, ArkadiuszX
  5 siblings, 0 replies; 9+ messages in thread
From: Ramkumar Balu @ 2021-11-29  9:51 UTC (permalink / raw)
  To: Akhil Goyal, Anoob Joseph, Declan Doherty, Fan Zhang,
	Ankur Dwivedi, Tejasree Kondoj
  Cc: stable, dev, Ramkumar

From: Ramkumar <rbalu@marvell.com>

During RSA sign verification, the OCTEONTX2 PMD returns the decrypted
plaintext in 'sign' field of rte_crypto_rsa_op_param. The 'sign'
field is actually used to pass input to the operation. This PMD
overwrites the 'sign' field buffer. This is non-compliance to lib
cryptodev.

This patch fixes the PMD to use 'cipher' field to return the decrypted
plaintext during RSA verify operation.

Fixes: 04227377c81b ("crypto/octeontx2: support asymmetric in enqueue/dequeue")
Cc: stable@dpdk.org

Signed-off-by: Ramkumar <rbalu@marvell.com>
---
 drivers/crypto/octeontx2/otx2_cryptodev_ops.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/drivers/crypto/octeontx2/otx2_cryptodev_ops.c b/drivers/crypto/octeontx2/otx2_cryptodev_ops.c
index 339b82f33e..fb38e309aa 100644
--- a/drivers/crypto/octeontx2/otx2_cryptodev_ops.c
+++ b/drivers/crypto/octeontx2/otx2_cryptodev_ops.c
@@ -876,20 +876,22 @@ otx2_cpt_asym_rsa_op(struct rte_crypto_op *cop, struct cpt_request_info *req,
 		break;
 	case RTE_CRYPTO_ASYM_OP_VERIFY:
 		if (rsa->pad == RTE_CRYPTO_RSA_PADDING_NONE) {
-			rsa->sign.length = rsa_ctx->n.length;
-			memcpy(rsa->sign.data, req->rptr, rsa->sign.length);
+			rsa->cipher.length = rsa_ctx->n.length;
 		} else {
 			/* Get length of signed output */
-			rsa->sign.length = rte_cpu_to_be_16
+			rsa->cipher.length = rte_cpu_to_be_16
 					  (*((uint16_t *)req->rptr));
 			/*
 			 * Offset output data pointer by length field
-			 * (2 bytes) and copy signed data.
+			 * (2 bytes).
 			 */
-			memcpy(rsa->sign.data, req->rptr + 2,
-			       rsa->sign.length);
+			req->rptr += 2;
 		}
-		if (memcmp(rsa->sign.data, rsa->message.data,
+
+		if (rsa->cipher.data != NULL)
+			memcpy(rsa->cipher.data, req->rptr, rsa->cipher.length);
+
+		if (memcmp(req->rptr, rsa->message.data,
 			   rsa->message.length)) {
 			CPT_LOG_DP_ERR("RSA verification failed");
 			cop->status = RTE_CRYPTO_OP_STATUS_ERROR;
-- 
2.17.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 5/5] crypto/cnxk: fix output field for RSA verify
  2021-11-29  9:51 [PATCH 0/5] cryptodev: fix inconsistency in RSA op usage Ramkumar Balu
                   ` (3 preceding siblings ...)
  2021-11-29  9:51 ` [PATCH 4/5] crypto/octeontx2: " Ramkumar Balu
@ 2021-11-29  9:51 ` Ramkumar Balu
  2021-12-28  8:58 ` [PATCH 0/5] cryptodev: fix inconsistency in RSA op usage Kusztal, ArkadiuszX
  5 siblings, 0 replies; 9+ messages in thread
From: Ramkumar Balu @ 2021-11-29  9:51 UTC (permalink / raw)
  To: Akhil Goyal, Anoob Joseph, Declan Doherty, Fan Zhang,
	Ankur Dwivedi, Tejasree Kondoj
  Cc: stable, dev, Ramkumar

From: Ramkumar <rbalu@marvell.com>

During RSA sign verification, this PMD returns the decrypted plaintext
in 'sign' field of rte_crypto_rsa_op_param. The 'sign' field is
actually used to pass input to the operation. This PMD overwrites the
'sign' field buffer. This is non-compliance to lib cryptodev.

This patch fixes the PMD to use 'cipher' field to return the decrypted
plaintext during RSA verify operation.

Fixes: 6661bedf1605 ("crypto/cnxk: add asymmetric datapath")
Cc: stable@dpdk.org

Signed-off-by: Ramkumar <rbalu@marvell.com>
---
 drivers/crypto/cnxk/cnxk_ae.h | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/crypto/cnxk/cnxk_ae.h b/drivers/crypto/cnxk/cnxk_ae.h
index 6222171fe6..f4c6c92880 100644
--- a/drivers/crypto/cnxk/cnxk_ae.h
+++ b/drivers/crypto/cnxk/cnxk_ae.h
@@ -696,19 +696,22 @@ cnxk_ae_dequeue_rsa_op(struct rte_crypto_op *cop, uint8_t *rptr,
 		break;
 	case RTE_CRYPTO_ASYM_OP_VERIFY:
 		if (rsa->pad == RTE_CRYPTO_RSA_PADDING_NONE) {
-			rsa->sign.length = rsa_ctx->n.length;
-			memcpy(rsa->sign.data, rptr, rsa->sign.length);
+			rsa->cipher.length = rsa_ctx->n.length;
 		} else {
 			/* Get length of signed output */
-			rsa->sign.length =
+			rsa->cipher.length =
 				rte_cpu_to_be_16(*((uint16_t *)rptr));
 			/*
 			 * Offset output data pointer by length field
-			 * (2 bytes) and copy signed data.
+			 * (2 bytes).
 			 */
-			memcpy(rsa->sign.data, rptr + 2, rsa->sign.length);
+			rptr += 2;
 		}
-		if (memcmp(rsa->sign.data, rsa->message.data,
+
+		if (rsa->cipher.data != NULL)
+			memcpy(rsa->cipher.data, rptr, rsa->cipher.length);
+
+		if (memcmp(rptr, rsa->message.data,
 			   rsa->message.length)) {
 			cop->status = RTE_CRYPTO_OP_STATUS_ERROR;
 		}
-- 
2.17.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* RE: [PATCH 0/5] cryptodev: fix inconsistency in RSA op usage
  2021-11-29  9:51 [PATCH 0/5] cryptodev: fix inconsistency in RSA op usage Ramkumar Balu
                   ` (4 preceding siblings ...)
  2021-11-29  9:51 ` [PATCH 5/5] crypto/cnxk: " Ramkumar Balu
@ 2021-12-28  8:58 ` Kusztal, ArkadiuszX
  5 siblings, 0 replies; 9+ messages in thread
From: Kusztal, ArkadiuszX @ 2021-12-28  8:58 UTC (permalink / raw)
  To: Ramkumar Balu, Akhil Goyal, Anoob Joseph, Doherty, Declan, Zhang,
	Roy Fan, Ankur Dwivedi, Tejasree Kondoj
  Cc: stable, dev

Hi Ramkumar,

> -----Original Message-----
> From: Ramkumar Balu <rbalu@marvell.com>
> Sent: Monday, November 29, 2021 10:52 AM
> To: Akhil Goyal <gakhil@marvell.com>; Anoob Joseph <anoobj@marvell.com>;
> Doherty, Declan <declan.doherty@intel.com>; Zhang, Roy Fan
> <roy.fan.zhang@intel.com>; Ankur Dwivedi <adwivedi@marvell.com>; Tejasree
> Kondoj <ktejasree@marvell.com>
> Cc: stable@dpdk.org; dev@dpdk.org; Ramkumar <rbalu@marvell.com>
> Subject: [PATCH 0/5] cryptodev: fix inconsistency in RSA op usage
> 
> From: Ramkumar <rbalu@marvell.com>
> 
> The RSA verify operation is performed in two stages: 1. decrypt using public key
> (output: plaintext message) 2. Compare resultant plaintext message with the
> expected plaintext message to verify. (return succ/fail in status field) Some
> applications need the decrypted plaintext (stage 1 result) also to be retunred.
[Arek] - It should only be the case when NO_PADDING selected (that's why QAT returns decrypted data, and only because of that), but I would propose to change it that NO_PADDING signature is not possible -> change it to NO_PADDING Private/Public operations (we cannot verify it in PMD anyway as we do not know padding type).
> For reference, OpenSSL also provides similar API (RSA_public_decrypt).
[Arek] - this function is not only deprecated but incorrect. It should not be used for signatures when padding selected. Normally functions that handle padding will only return verification status only, not data.
> 
> lib cryptodev API failed to specify a field in 'struct rte_crypto_rsa_op_param' to
> return the plaintext result after public key decryption. It created inconsistency
> among crypto PMDs in returning plaintext during RSA verify.
> 
> Inconsistency in RSA verify,
> crypto/octeontx - uses 'sign' field to return plaintext crypto/cnxk - uses 'sign'
> field to return plaintext crypto/openssl - does not return plaintext crypto/qat -
> uses 'cipher' field to return plaintext test/cryptodev_asym - expects PMDs to use
> 'cipher' field
> 
> Thus, this patch series fixes all usages to only use 'cipher' field for above
> described scenario.  The 'sign' and 'message' fields are not chosen as they are
> used for different purpose under same operation.
> 
> rte_crypto_rsa_op_param struct fields to use for
> RTE_CRYPTO_ASYM_OP_VERIFY:
> 1. input: rsa.sign - signature to be decrypted or verified 2. input: rsa.message -
> expected plaintext, used to compare 3. output: rsa.cipher - resultant plaintext
> from decryption
> 
> 
> Ramkumar (5):
>   cryptodev: fix RSA op cipher field description
>   crypto/openssl: fix output of RSA verify op
>   crypto/octeontx: fix output field for RSA verify
>   crypto/octeontx2: fix output field for RSA verify
>   crypto/cnxk: fix output field for RSA verify
> 
>  drivers/crypto/cnxk/cnxk_ae.h                 | 15 +++++++++------
>  drivers/crypto/octeontx/otx_cryptodev_ops.c   | 10 ++++++----
>  drivers/crypto/octeontx2/otx2_cryptodev_ops.c | 16 +++++++++-------
>  drivers/crypto/openssl/rte_openssl_pmd.c      | 16 +++++++++++-----
>  lib/cryptodev/rte_crypto_asym.h               |  7 ++++---
>  5 files changed, 39 insertions(+), 25 deletions(-)
> 
> --
> 2.17.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* RE: [PATCH 2/5] crypto/openssl: fix output of RSA verify op
  2021-11-29  9:51 ` [PATCH 2/5] crypto/openssl: fix output of RSA verify op Ramkumar Balu
@ 2021-12-28  9:10   ` Kusztal, ArkadiuszX
  2022-01-13 10:34     ` Ramkumar Balu
  0 siblings, 1 reply; 9+ messages in thread
From: Kusztal, ArkadiuszX @ 2021-12-28  9:10 UTC (permalink / raw)
  To: Ramkumar Balu, Akhil Goyal, Anoob Joseph, Doherty, Declan, Zhang,
	Roy Fan, Ankur Dwivedi, Tejasree Kondoj
  Cc: stable, dev



> -----Original Message-----
> From: Ramkumar Balu <rbalu@marvell.com>
> Sent: Monday, November 29, 2021 10:52 AM
> To: Akhil Goyal <gakhil@marvell.com>; Anoob Joseph <anoobj@marvell.com>;
> Doherty, Declan <declan.doherty@intel.com>; Zhang, Roy Fan
> <roy.fan.zhang@intel.com>; Ankur Dwivedi <adwivedi@marvell.com>; Tejasree
> Kondoj <ktejasree@marvell.com>
> Cc: stable@dpdk.org; dev@dpdk.org; Ramkumar <rbalu@marvell.com>
> Subject: [PATCH 2/5] crypto/openssl: fix output of RSA verify op
> 
> From: Ramkumar <rbalu@marvell.com>
> 
> During RSA verify, the OpenSSL PMD fails to return the plaintext after public key
> decryption.
> This patch fixes the OpenSSL PMD to return the decrypted plaintext in
> cipher.data / cipher.length fields
> 
> Fixes: 3e9d6bd447fb ("crypto/openssl: add RSA and mod asym operations")
> Fixes: fe1606e0138c ("crypto/openssl: fix RSA verify operation")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Ramkumar <rbalu@marvell.com>
> ---
>  drivers/crypto/openssl/rte_openssl_pmd.c | 16 +++++++++++-----
>  1 file changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/crypto/openssl/rte_openssl_pmd.c
> b/drivers/crypto/openssl/rte_openssl_pmd.c
> index 5794ed8159..3ab2c3b5c1 100644
> --- a/drivers/crypto/openssl/rte_openssl_pmd.c
> +++ b/drivers/crypto/openssl/rte_openssl_pmd.c
> @@ -1953,12 +1953,16 @@ process_openssl_rsa_op(struct rte_crypto_op
> *cop,
>  		break;
> 
>  	case RTE_CRYPTO_ASYM_OP_VERIFY:
> -		tmp = rte_malloc(NULL, op->rsa.sign.length, 0);
> +		tmp = op->rsa.cipher.data;
>  		if (tmp == NULL) {
> -			OPENSSL_LOG(ERR, "Memory allocation failed");
> -			cop->status = RTE_CRYPTO_OP_STATUS_ERROR;
> -			break;
> +			tmp = rte_malloc(NULL, op->rsa.sign.length, 0);
> +			if (tmp == NULL) {
> +				OPENSSL_LOG(ERR, "Memory allocation
> failed");
> +				cop->status =
> RTE_CRYPTO_OP_STATUS_ERROR;
> +				break;
> +			}
>  		}
> +
>  		ret = RSA_public_decrypt(op->rsa.sign.length,
>  				op->rsa.sign.data,
>  				tmp,
[Arek] - this function is deprecated and more importantly it properly handle only NO_PADDING situation (no der encoding, like pre TLS 1.2). OpenSSL code needs major refactor in this area soon (mostly in asymmetric crypto).
> @@ -1974,7 +1978,9 @@ process_openssl_rsa_op(struct rte_crypto_op *cop,
>  			OPENSSL_LOG(ERR, "RSA sign Verification failed");
>  			cop->status = RTE_CRYPTO_OP_STATUS_ERROR;
>  		}
> -		rte_free(tmp);
> +		op->rsa.cipher.length = ret;
> +		if (tmp != op->rsa.cipher.data)
> +			rte_free(tmp);
>  		break;
> 
>  	default:
> --
> 2.17.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* RE: [PATCH 2/5] crypto/openssl: fix output of RSA verify op
  2021-12-28  9:10   ` Kusztal, ArkadiuszX
@ 2022-01-13 10:34     ` Ramkumar Balu
  0 siblings, 0 replies; 9+ messages in thread
From: Ramkumar Balu @ 2022-01-13 10:34 UTC (permalink / raw)
  To: Kusztal, ArkadiuszX, Akhil Goyal, Anoob Joseph, Doherty, Declan,
	Zhang, Roy Fan, Ankur Dwivedi, Tejasree Kondoj
  Cc: stable, dev

Thank you for the comments. I agree that OpenSSL PMD needs a major refactoring in asym crypto. 
I have asked Akhil to reject this patch series.

-----Original Message-----
From: Kusztal, ArkadiuszX <arkadiuszx.kusztal@intel.com> 
Sent: Tuesday, December 28, 2021 2:41 PM
To: Ramkumar Balu <rbalu@marvell.com>; Akhil Goyal <gakhil@marvell.com>; Anoob Joseph <anoobj@marvell.com>; Doherty, Declan <declan.doherty@intel.com>; Zhang, Roy Fan <roy.fan.zhang@intel.com>; Ankur Dwivedi <adwivedi@marvell.com>; Tejasree Kondoj <ktejasree@marvell.com>
Cc: stable@dpdk.org; dev@dpdk.org
Subject: [EXT] RE: [PATCH 2/5] crypto/openssl: fix output of RSA verify op

----------------------------------------------------------------------


> -----Original Message-----
> From: Ramkumar Balu <rbalu@marvell.com>
> Sent: Monday, November 29, 2021 10:52 AM
> To: Akhil Goyal <gakhil@marvell.com>; Anoob Joseph 
> <anoobj@marvell.com>; Doherty, Declan <declan.doherty@intel.com>; 
> Zhang, Roy Fan <roy.fan.zhang@intel.com>; Ankur Dwivedi 
> <adwivedi@marvell.com>; Tejasree Kondoj <ktejasree@marvell.com>
> Cc: stable@dpdk.org; dev@dpdk.org; Ramkumar <rbalu@marvell.com>
> Subject: [PATCH 2/5] crypto/openssl: fix output of RSA verify op
> 
> From: Ramkumar <rbalu@marvell.com>
> 
> During RSA verify, the OpenSSL PMD fails to return the plaintext after 
> public key decryption.
> This patch fixes the OpenSSL PMD to return the decrypted plaintext in 
> cipher.data / cipher.length fields
> 
> Fixes: 3e9d6bd447fb ("crypto/openssl: add RSA and mod asym 
> operations")
> Fixes: fe1606e0138c ("crypto/openssl: fix RSA verify operation")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Ramkumar <rbalu@marvell.com>
> ---
>  drivers/crypto/openssl/rte_openssl_pmd.c | 16 +++++++++++-----
>  1 file changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/crypto/openssl/rte_openssl_pmd.c
> b/drivers/crypto/openssl/rte_openssl_pmd.c
> index 5794ed8159..3ab2c3b5c1 100644
> --- a/drivers/crypto/openssl/rte_openssl_pmd.c
> +++ b/drivers/crypto/openssl/rte_openssl_pmd.c
> @@ -1953,12 +1953,16 @@ process_openssl_rsa_op(struct rte_crypto_op 
> *cop,
>  		break;
> 
>  	case RTE_CRYPTO_ASYM_OP_VERIFY:
> -		tmp = rte_malloc(NULL, op->rsa.sign.length, 0);
> +		tmp = op->rsa.cipher.data;
>  		if (tmp == NULL) {
> -			OPENSSL_LOG(ERR, "Memory allocation failed");
> -			cop->status = RTE_CRYPTO_OP_STATUS_ERROR;
> -			break;
> +			tmp = rte_malloc(NULL, op->rsa.sign.length, 0);
> +			if (tmp == NULL) {
> +				OPENSSL_LOG(ERR, "Memory allocation
> failed");
> +				cop->status =
> RTE_CRYPTO_OP_STATUS_ERROR;
> +				break;
> +			}
>  		}
> +
>  		ret = RSA_public_decrypt(op->rsa.sign.length,
>  				op->rsa.sign.data,
>  				tmp,
[Arek] - this function is deprecated and more importantly it properly handle only NO_PADDING situation (no der encoding, like pre TLS 1.2). OpenSSL code needs major refactor in this area soon (mostly in asymmetric crypto).
> @@ -1974,7 +1978,9 @@ process_openssl_rsa_op(struct rte_crypto_op *cop,
>  			OPENSSL_LOG(ERR, "RSA sign Verification failed");
>  			cop->status = RTE_CRYPTO_OP_STATUS_ERROR;
>  		}
> -		rte_free(tmp);
> +		op->rsa.cipher.length = ret;
> +		if (tmp != op->rsa.cipher.data)
> +			rte_free(tmp);
>  		break;
> 
>  	default:
> --
> 2.17.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2022-01-13 10:34 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-29  9:51 [PATCH 0/5] cryptodev: fix inconsistency in RSA op usage Ramkumar Balu
2021-11-29  9:51 ` [PATCH 1/5] cryptodev: fix RSA op cipher field description Ramkumar Balu
2021-11-29  9:51 ` [PATCH 2/5] crypto/openssl: fix output of RSA verify op Ramkumar Balu
2021-12-28  9:10   ` Kusztal, ArkadiuszX
2022-01-13 10:34     ` Ramkumar Balu
2021-11-29  9:51 ` [PATCH 3/5] crypto/octeontx: fix output field for RSA verify Ramkumar Balu
2021-11-29  9:51 ` [PATCH 4/5] crypto/octeontx2: " Ramkumar Balu
2021-11-29  9:51 ` [PATCH 5/5] crypto/cnxk: " Ramkumar Balu
2021-12-28  8:58 ` [PATCH 0/5] cryptodev: fix inconsistency in RSA op usage Kusztal, ArkadiuszX

DPDK patches and discussions

This inbox may be cloned and mirrored by anyone:

	git clone --mirror http://inbox.dpdk.org/dev/0 dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dev dev/ http://inbox.dpdk.org/dev \
		dev@dpdk.org
	public-inbox-index dev

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git