test/crypto: fix RSA decrypt op validation

Message ID 20250510104452.2186-1-gmuthukrishn@marvell.com (mailing list archive)
State Superseded
Delegated to: akhil goyal
Headers
Series test/crypto: fix RSA decrypt op validation |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/loongarch-compilation success Compilation OK
ci/loongarch-unit-testing success Unit Testing PASS
ci/Intel-compilation success Compilation OK
ci/intel-Testing success Testing PASS
ci/iol-marvell-Functional success Functional Testing PASS
ci/github-robot: build success github build: passed
ci/iol-broadcom-Performance success Performance Testing PASS
ci/intel-Functional success Functional PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-abi-testing success Testing PASS
ci/iol-compile-amd64-testing success Testing PASS
ci/iol-compile-arm64-testing success Testing PASS
ci/aws-unit-testing success Unit Testing PASS
ci/iol-unit-arm64-testing fail Testing issues RETEST #1
ci/iol-sample-apps-testing success Testing PASS RETEST #1
ci/iol-unit-amd64-testing fail Testing issues RETEST #1

Commit Message

Gowrishankar Muthukrishnan May 10, 2025, 10:44 a.m. UTC
Following RSA encrypt op, same plaintext buffer is used as output
buffer for decrypt op, hence comparing plaintext buffer against
same buffer pointer in crypto op always succeed irrespective of
whether decrypt op succeeds or not. This patch fixes this issue
with a local buffer for crypto op.

Fixes: 5ae36995f10 ("test/crypto: move RSA enqueue/dequeue into functions")
Cc: stable@dpdk.org

Signed-off-by: Gowrishankar Muthukrishnan <gmuthukrishn@marvell.com>
---
 app/test/test_cryptodev_asym.c | 21 +++++++++++++++++----
 1 file changed, 17 insertions(+), 4 deletions(-)
  

Comments

Gowrishankar Muthukrishnan May 28, 2025, 8:38 a.m. UTC | #1
Recheck unit test failure for openssl 1.1.1 in some distros.

Recheck-request: iol-unit-amd64-testing
--
Gowrishankar

> 
> Following RSA encrypt op, same plaintext buffer is used as output buffer for
> decrypt op, hence comparing plaintext buffer against same buffer pointer in
> crypto op always succeed irrespective of whether decrypt op succeeds or not.
> This patch fixes this issue with a local buffer for crypto op.
> 
> Fixes: 5ae36995f10 ("test/crypto: move RSA enqueue/dequeue into
> functions")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Gowrishankar Muthukrishnan <gmuthukrishn@marvell.com>
  
Gowrishankar Muthukrishnan June 12, 2025, 10:33 a.m. UTC | #2
Hi Kai Ji,
This patch adds a check on cop status after DECRYPT in existing RSA test.There are
few CI failures for this patch and the failures are very specific to Openssl 1.1.1 variants
used in below platforms:
* Ubuntu 20.04 - Openssl 1.1.1f
* RHEL 8 - Openssl 1.1.1k

When I manually build these Openssl variants in the above platforms (using
Docker images) and execute tests using respective openssl shared libraries,
all tests PASS. If I use Openssl library that is shipped within the above 
distributions, the test in patch FAIL.

I would like to get your opinion first to proceed on getting these patches CI acked.
In my debug, I don't see any issue with the patch and the patch adds better
Validation indeed. May be that, it has uncovered a potential failure in above
Distros. Can you check on your end if these failures disappear in manual try.
What would you suggest to go forward, for openssl PMD ?

If required, I can start filing a Bugzilla as well.

Thanks,
Gowrishankar

> 
> Following RSA encrypt op, same plaintext buffer is used as output buffer for
> decrypt op, hence comparing plaintext buffer against same buffer pointer in
> crypto op always succeed irrespective of whether decrypt op succeeds or not.
> This patch fixes this issue with a local buffer for crypto op.
> 
> Fixes: 5ae36995f10 ("test/crypto: move RSA enqueue/dequeue into
> functions")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Gowrishankar Muthukrishnan <gmuthukrishn@marvell.com>
  
Ji, Kai June 16, 2025, 2:17 p.m. UTC | #3
Hi Gowrishankar,

Thanks for reach out, I don't have those environments setup on my end, but Ubuntu 20.04 and RHEL8  are old but still common LTS packages.
Are you able to dump the actual msg buffer ?

It is highly possible relates to dsitro-specific patches ? Paddings ? Or crypto polices in RHEL ?

Regards

Kai
  
Gowrishankar Muthukrishnan June 18, 2025, 4:04 p.m. UTC | #4
Hi Kai Ji,
> 
> Hi Gowrishankar,
> 
> Thanks for reach out, I don't have those environments setup on my end, but
> Ubuntu 20.04 and RHEL8  are old but still common LTS packages.
> Are you able to dump the actual msg buffer ?
> 

RSA_private_decrypt fails and return code is -1. Output buffer is untouched.

I dumped error code of it:
error:04067084:rsa routines:rsa_ossl_public_decrypt:data too large for modulus

When I used same openssl library that I compiled, no error is seen. Also, public
decrypt function is called, but it should be private decrypt to be called.

JFYI end of life for full support already over for the above distros. Not sure if the
Openssl 1.1.1 in them need some fix for above.

Thanks,
Gowrishankar

> It is highly possible relates to dsitro-specific patches ? Paddings ? Or crypto
> polices in RHEL ?
> 
> Regards
> 
> Kai
> 
> ________________________________
> 
> From: Gowrishankar Muthukrishnan <gmuthukrishn@marvell.com>
> Sent: Thursday, June 12, 2025 11:33
> To: Ji, Kai <kai.ji@intel.com>
> Cc: Anoob Joseph <anoobj@marvell.com>; dev@dpdk.org <dev@dpdk.org>; Akhil
> Goyal <gakhil@marvell.com>; Fan Zhang <fanzhang.oss@gmail.com>; Shally
> Verma <shallyv@marvell.com>; Kanaka Durga Kotamarthy
> <kkotamarthy@marvell.com>; Ayuj Verma <ayverma@marvell.com>; Kusztal,
> ArkadiuszX <arkadiuszx.kusztal@intel.com>
> Subject: RE: [PATCH] test/crypto: fix RSA decrypt op validation
> 
> Hi Kai Ji,
> This patch adds a check on cop status after DECRYPT in existing RSA test.There
> are few CI failures for this patch and the failures are very specific to Openssl 1.1.1
> variants used in below platforms:
> * Ubuntu 20.04 - Openssl 1.1.1f
> * RHEL 8 - Openssl 1.1.1k
> 
> When I manually build these Openssl variants in the above platforms (using
> Docker images) and execute tests using respective openssl shared libraries, all
> tests PASS. If I use Openssl library that is shipped within the above distributions,
> the test in patch FAIL.
> 
> I would like to get your opinion first to proceed on getting these patches CI acked.
> In my debug, I don't see any issue with the patch and the patch adds better
> Validation indeed. May be that, it has uncovered a potential failure in above
> Distros. Can you check on your end if these failures disappear in manual try.
> What would you suggest to go forward, for openssl PMD ?
> 
> If required, I can start filing a Bugzilla as well.
> 
> Thanks,
> Gowrishankar
> 
> >
> > Following RSA encrypt op, same plaintext buffer is used as output
> > buffer for decrypt op, hence comparing plaintext buffer against same
> > buffer pointer in crypto op always succeed irrespective of whether decrypt op
> succeeds or not.
> > This patch fixes this issue with a local buffer for crypto op.
> >
> > Fixes: 5ae36995f10 ("test/crypto: move RSA enqueue/dequeue into
> > functions")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Gowrishankar Muthukrishnan <gmuthukrishn@marvell.com>
>
  
Gowrishankar Muthukrishnan June 19, 2025, 9:43 a.m. UTC | #5
Hi Kai Ji,
Debugged it using openssl source code used in these distros and the failure found due to below
Implicit rejection added in PKCS padding.

https://github.com/openssl/openssl/pull/13817/commits/3b99dcb1a6df6853581d3035c6d9a23a831716bb

As part of the above fix, rsa_ossl_private_decrypt() would additionally need private exponent
rsa->d, assuming that d is already part of RSA session, to derive kdk used in implicit rejection.
We did not have it in CRT test vectors today. I'll send patch to fix these.

Thanks,
Gowrishankar
> 
> Hi Kai Ji,
> >
> > Hi Gowrishankar,
> >
> > Thanks for reach out, I don't have those environments setup on my end,
> > but Ubuntu 20.04 and RHEL8  are old but still common LTS packages.
> > Are you able to dump the actual msg buffer ?
> >
> 
> RSA_private_decrypt fails and return code is -1. Output buffer is untouched.
> 
> I dumped error code of it:
> error:04067084:rsa routines:rsa_ossl_public_decrypt:data too large for
> modulus
> 
> When I used same openssl library that I compiled, no error is seen. Also, public
> decrypt function is called, but it should be private decrypt to be called.
> 
> JFYI end of life for full support already over for the above distros. Not sure if
> the Openssl 1.1.1 in them need some fix for above.
> 
> Thanks,
> Gowrishankar
>
  

Patch

diff --git a/app/test/test_cryptodev_asym.c b/app/test/test_cryptodev_asym.c
index 226270556d..cd4fe7e14e 100644
--- a/app/test/test_cryptodev_asym.c
+++ b/app/test/test_cryptodev_asym.c
@@ -174,7 +174,10 @@  queue_ops_rsa_enc_dec(void *sess)
 	struct rte_crypto_op *op, *result_op;
 	struct rte_crypto_asym_op *asym_op;
 	uint8_t cipher_buf[TEST_DATA_SIZE] = {0};
-	int ret, status = TEST_SUCCESS;
+	uint8_t msg_buf[TEST_DATA_SIZE] = {0};
+	int ret, status;
+
+	memcpy(msg_buf, rsaplaintext.data, rsaplaintext.len);
 
 	/* Set up crypto op data structure */
 	op = rte_crypto_op_alloc(op_mpool, RTE_CRYPTO_OP_TYPE_ASYMMETRIC);
@@ -189,7 +192,7 @@  queue_ops_rsa_enc_dec(void *sess)
 	/* Compute encryption on the test vector */
 	asym_op->rsa.op_type = RTE_CRYPTO_ASYM_OP_ENCRYPT;
 
-	asym_op->rsa.message.data = rsaplaintext.data;
+	asym_op->rsa.message.data = msg_buf;
 	asym_op->rsa.cipher.data = cipher_buf;
 	asym_op->rsa.cipher.length = RTE_DIM(rsa_n);
 	asym_op->rsa.message.length = rsaplaintext.len;
@@ -224,6 +227,7 @@  queue_ops_rsa_enc_dec(void *sess)
 	asym_op = result_op->asym;
 	asym_op->rsa.message.length = RTE_DIM(rsa_n);
 	asym_op->rsa.op_type = RTE_CRYPTO_ASYM_OP_DECRYPT;
+	memset(asym_op->rsa.message.data, 0, asym_op->rsa.message.length);
 
 	/* Process crypto operation */
 	if (rte_cryptodev_enqueue_burst(dev_id, 0, &op, 1) != 1) {
@@ -240,11 +244,20 @@  queue_ops_rsa_enc_dec(void *sess)
 		status = TEST_FAILED;
 		goto error_exit;
 	}
-	status = TEST_SUCCESS;
+
+	if (result_op->status != RTE_CRYPTO_OP_STATUS_SUCCESS) {
+		RTE_LOG(ERR, USER1, "Expected crypto op to succeed\n");
+		status = TEST_FAILED;
+		goto error_exit;
+	}
+
 	ret = rsa_verify(&rsaplaintext, result_op);
-	if (ret)
+	if (ret) {
 		status = TEST_FAILED;
+		goto error_exit;
+	}
 
+	status = TEST_SUCCESS;
 error_exit:
 
 	rte_crypto_op_free(op);