test/crypto: skip plain text compare for null cipher OOP

PATCH Superseded Anoob Joseph 4 participants 7 messages Nov 8, 2021 – Nov 11, 2021

Share & tools

Anoob Joseph

author

2021-11-08 15:20 UTC #0 lore raw

PATCH 1/1 test/crypto: skip plain text compare for null cipher OOP

NULL cipher is used for validating auth only cases. With out of place processing, validating plain text should not be done as the PMD is only expected to update auth data.

Signed-off-by Anoob Joseph <[email protected]>

---
 app/test/test_cryptodev.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

app/test/test_cryptodev.c +17 -0

index e54a1a9..964f44f 100644
@@ -7490,6 +7490,22 @@ test_mixed_auth_cipher(const struct mixed_cipher_auth_test_data *tdata,
 				tdata->digest_enc.len);
 	}
 
+	/*
+	 * NULL cipher is used for auth only cases where only authentication
+	 * is done. With verify operation, MAC would be validated by the PMD.
+	 * With generate operation, verify MAC generated by the PMD.
+	 */
+	if (op_mode == OUT_OF_PLACE &&
+	    tdata->cipher_algo == RTE_CRYPTO_CIPHER_NULL) {
+		if (!verify)
+			TEST_ASSERT_BUFFERS_ARE_EQUAL(
+					ut_params->digest,
+					tdata->digest_enc.data,
+					tdata->digest_enc.len,
+					"Generated auth tag not as expected");
+		goto op_status_check;
+	}
+
 	/* Validate obuf */
 	if (verify) {
 		TEST_ASSERT_BUFFERS_ARE_EQUAL_BIT(

@@ -7511,6 +7527,7 @@ test_mixed_auth_cipher(const struct mixed_cipher_auth_test_data *tdata,
 				"Generated auth tag not as expected");
 	}
 
+op_status_check:
 	TEST_ASSERT_EQUAL(ut_params->op->status, RTE_CRYPTO_OP_STATUS_SUCCESS,
 			"crypto op processing failed");

Signature · 1 line

2.7.4

De Lara Guarch, Pablo

2021-11-10 11:23 UTC #1 lore raw

Hi Anoob,

Quoted 29 lines

-----Original Message-----
From: dev <[email protected]> On Behalf Of Anoob Joseph
Sent: Monday, November 8, 2021 3:20 PM
To: Akhil Goyal <[email protected]>; Doherty, Declan <[email protected]>; Zhang, Roy Fan <[email protected]>
Cc: Anoob Joseph <[email protected]>; Jerin Jacob <[email protected]>; Archana Muniganti <[email protected]>; Tejasree Kondoj <[email protected]>; Hemant Agrawal <[email protected]>; Nicolau, Radu <[email protected]>; Power, Ciara <[email protected]>; Gagandeep Singh <[email protected]>; [email protected]
Subject: [dpdk-dev] [PATCH] test/crypto: skip plain text compare for null cipher OOP
NULL cipher is used for validating auth only cases. With out of place processing, validating plain text should not be done as the PMD is only expected to update auth data.
Signed-off-by Anoob Joseph <[email protected]>
---
 app/test/test_cryptodev.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)
app/test/test_cryptodev.c index renamed from app/test/test_cryptodev.c +0 -0
e54a1a9..964f44f 100644
app/test/test_cryptodev.c +0 -0
@@ -7490,6 +7490,22 @@ test_mixed_auth_cipher(const struct
 mixed_cipher_auth_test_data *tdata,

app/test/test_cryptodev.c test_mixed_auth_cipher(const struct · line 7500

Quoted 10 lines

app/test/test_cryptodev.c +7 -0

 				tdata->digest_enc.len);
 	}
 
+	/*
+	 * NULL cipher is used for auth only cases where only authentication
+	 * is done. With verify operation, MAC would be validated by the PMD.
+	 * With generate operation, verify MAC generated by the PMD.
+	 */
+	if (op_mode == OUT_OF_PLACE &&
+	    tdata->cipher_algo == RTE_CRYPTO_CIPHER_NULL) {

Why only checking for OUT_OF_PLACE? As far as cipher algorithm is NULL, only digest should be checked.
Also, looking at the code, there is this same check, but with hardcoded tag length. Could you rearrange the code to have less lines and generic?

Something like:

<pseudo-code>
if (!verify)
check_digest

if (cipher_algo != NULL)
check_ciphertext/plaintext

check op status
</pseudo-code>

Also, I think this change is applicable in test_mixed_auth_cipher_sgl.

Thanks,
Pablo

Anoob Joseph

author

2021-11-10 11:48 UTC #2 lore raw

Hi Pablo,

Thanks for the review. Please see inline.

Thanks,
Anoob

Quoted 55 lines

-----Original Message-----
From: De Lara Guarch, Pablo <[email protected]>
Sent: Wednesday, November 10, 2021 4:54 PM
To: Anoob Joseph <[email protected]>; Akhil Goyal <[email protected]>; Doherty, Declan <[email protected]>; Zhang, Roy Fan <[email protected]>
Cc: Jerin Jacob Kollanukkaran <[email protected]>; Archana Muniganti <[email protected]>; Tejasree Kondoj <[email protected]>; Hemant Agrawal <[email protected]>; Nicolau, Radu <[email protected]>; Power, Ciara <[email protected]>; Gagandeep Singh <[email protected]>; [email protected]
Subject: [EXT] RE: [dpdk-dev] [PATCH] test/crypto: skip plain text compare for null cipher OOP
External Email
----------------------------------------------------------------------
Hi Anoob,
Quoted 28 lines
-----Original Message-----
From: dev <[email protected]> On Behalf Of Anoob Joseph
Sent: Monday, November 8, 2021 3:20 PM
To: Akhil Goyal <[email protected]>; Doherty, Declan <[email protected]>; Zhang, Roy Fan <[email protected]>
Cc: Anoob Joseph <[email protected]>; Jerin Jacob <[email protected]>; Archana Muniganti <[email protected]>; Tejasree Kondoj <[email protected]>; Hemant Agrawal <[email protected]>; Nicolau, Radu <[email protected]>; Power, Ciara <[email protected]>; Gagandeep Singh <[email protected]>; [email protected]
Subject: [dpdk-dev] [PATCH] test/crypto: skip plain text compare for null cipher OOP
NULL cipher is used for validating auth only cases. With out of place processing, validating plain text should not be done as the PMD is only expected to update auth data.
Signed-off-by Anoob Joseph <[email protected]>
---
 app/test/test_cryptodev.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)
app/test/test_cryptodev.c +0 -0
index e54a1a9..964f44f 100644
app/test/test_cryptodev.c test_mixed_auth_cipher(const struct · line 7496
Quoted 7 lines
app/test/test_cryptodev.c +3 -0
@@ -7490,6 +7490,22 @@ test_mixed_auth_cipher(const struct
 mixed_cipher_auth_test_data *tdata,
 				tdata->digest_enc.len);
 	}
 
+	/*
+	 * NULL cipher is used for auth only cases where only authentication
+	 * is done. With verify operation, MAC would be validated by the
PMD.

Quoted 7 lines

app/test/test_cryptodev.c test_mixed_auth_cipher(const struct mixed_cipher_auth_test_data *tdata, · line 7499
Quoted 4 lines
app/test/test_cryptodev.c +4 -0
+	 * With generate operation, verify MAC generated by the PMD.
+	 */
+	if (op_mode == OUT_OF_PLACE &&
+	    tdata->cipher_algo == RTE_CRYPTO_CIPHER_NULL) {
Why only checking for OUT_OF_PLACE? As far as cipher algorithm is NULL, only digest should be checked.

[Anoob] Agreed. I will make this change in v2.

Quoted 3 lines

Also, looking at the code, there is this same check, but with hardcoded tag length.
Could you rearrange the code to have less lines and generic?

[Anoob] Yes. This looks better and the code would be self-explanatory as well. Will make this change in v2.

Quoted 7 lines

Something like:
<pseudo-code>
if (!verify)
check_digest

Quoted 7 lines

if (cipher_algo != NULL)
check_ciphertext/plaintext
check op status
</pseudo-code>
Also, I think this change is applicable in test_mixed_auth_cipher_sgl.

[Anoob] Yes. I'll make that change as well.

Quoted 3 lines

Thanks,
Pablo

Anoob Joseph

author

2021-11-10 13:05 UTC #3 lore raw

PATCH v2 1/1 test/crypto: skip plain text compare for null cipher

In reply to Anoob Joseph

NULL cipher is used for validating auth only cases. With NULL cipher, validating plain text should not be done as the PMD is only expected to update auth data.

Fixes e847fc512817 ("test/crypto: add encrypted digest case for AES-CTR-CMAC")

Cc [email protected]

Signed-off-by Anoob Joseph <[email protected]>

---
Changes in v2:
- Made the skip applicable for in place also (comment from Pablo)
- Re-organized code as suggested by Pablo
- Made same change in test_mixed_auth_cipher_sgl (comment from Pablo)

 app/test/test_cryptodev.c | 63 ++++++++++++++++++++++++++---------------------
 1 file changed, 35 insertions(+), 28 deletions(-)

app/test/test_cryptodev.c +35 ~~-28~~

index 8137b8a..c223588 100644
@@ -7343,27 +7343,30 @@ test_mixed_auth_cipher(const struct mixed_cipher_auth_test_data *tdata,
 				tdata->digest_enc.len);
 	}
 
-	/* Validate obuf */
-	if (verify) {
-		TEST_ASSERT_BUFFERS_ARE_EQUAL_BIT(
-				plaintext,
-				tdata->plaintext.data,
-				tdata->plaintext.len_bits >> 3,
-				"Plaintext data not as expected");
-	} else {
-		TEST_ASSERT_BUFFERS_ARE_EQUAL_BIT(
-				ciphertext,
-				tdata->ciphertext.data,
-				tdata->validDataLen.len_bits,
-				"Ciphertext data not as expected");
-
+	if (!verify) {
 		TEST_ASSERT_BUFFERS_ARE_EQUAL(
 				ut_params->digest,
 				tdata->digest_enc.data,
-				DIGEST_BYTE_LENGTH_SNOW3G_UIA2,
+				tdata->digest_enc.len,
 				"Generated auth tag not as expected");
 	}
 
+	if (tdata->cipher_algo != RTE_CRYPTO_CIPHER_NULL) {
+		if (verify) {
+			TEST_ASSERT_BUFFERS_ARE_EQUAL_BIT(
+					plaintext,
+					tdata->plaintext.data,
+					tdata->plaintext.len_bits >> 3,
+					"Plaintext data not as expected");
+		} else {
+			TEST_ASSERT_BUFFERS_ARE_EQUAL_BIT(
+					ciphertext,
+					tdata->ciphertext.data,
+					tdata->validDataLen.len_bits,
+					"Ciphertext data not as expected");
+		}
+	}
+
 	TEST_ASSERT_EQUAL(ut_params->op->status, RTE_CRYPTO_OP_STATUS_SUCCESS,
 			"crypto op processing failed");

@@ -7560,19 +7563,7 @@ test_mixed_auth_cipher_sgl(const struct mixed_cipher_auth_test_data *tdata,
 				tdata->digest_enc.data, tdata->digest_enc.len);
 	}
 
-	/* Validate obuf */
-	if (verify) {
-		TEST_ASSERT_BUFFERS_ARE_EQUAL_BIT(
-				plaintext,
-				tdata->plaintext.data,
-				tdata->plaintext.len_bits >> 3,
-				"Plaintext data not as expected");
-	} else {
-		TEST_ASSERT_BUFFERS_ARE_EQUAL_BIT(
-				ciphertext,
-				tdata->ciphertext.data,
-				tdata->validDataLen.len_bits,
-				"Ciphertext data not as expected");
+	if (!verify) {
 		TEST_ASSERT_BUFFERS_ARE_EQUAL(
 				digest,
 				tdata->digest_enc.data,

@@ -7580,6 +7571,22 @@ test_mixed_auth_cipher_sgl(const struct mixed_cipher_auth_test_data *tdata,
 				"Generated auth tag not as expected");
 	}
 
+	if (tdata->cipher_algo != RTE_CRYPTO_CIPHER_NULL) {
+		if (verify) {
+			TEST_ASSERT_BUFFERS_ARE_EQUAL_BIT(
+					plaintext,
+					tdata->plaintext.data,
+					tdata->plaintext.len_bits >> 3,
+					"Plaintext data not as expected");
+		} else {
+			TEST_ASSERT_BUFFERS_ARE_EQUAL_BIT(
+					ciphertext,
+					tdata->ciphertext.data,
+					tdata->validDataLen.len_bits,
+					"Ciphertext data not as expected");
+		}
+	}
+
 	TEST_ASSERT_EQUAL(ut_params->op->status, RTE_CRYPTO_OP_STATUS_SUCCESS,
 			"crypto op processing failed");

Signature · 1 line

2.7.4

Power, Ciara

Acked-by on v2 1/1

2021-11-10 17:01 UTC #4 lore raw

RE: [PATCH v2] test/crypto: skip plain text compare for null cipher

Hi Anoob,

Quoted 22 lines

-----Original Message-----
From: Anoob Joseph <[email protected]>
Sent: Wednesday 10 November 2021 13:04
To: Akhil Goyal <[email protected]>; Doherty, Declan <[email protected]>; Zhang, Roy Fan <[email protected]>; De Lara Guarch, Pablo <[email protected]>
Cc: Anoob Joseph <[email protected]>; Jerin Jacob <[email protected]>; Archana Muniganti <[email protected]>; Tejasree Kondoj <[email protected]>; Hemant Agrawal <[email protected]>; Nicolau, Radu <[email protected]>; Power, Ciara <[email protected]>; Gagandeep Singh <[email protected]>; Ji, Kai <[email protected]>; [email protected]; [email protected]
Subject: [PATCH v2] test/crypto: skip plain text compare for null cipher
NULL cipher is used for validating auth only cases. With NULL cipher, validating plain text should not be done as the PMD is only expected to update auth data.
Fixes e847fc512817 ("test/crypto: add encrypted digest case for AES-CTR-
CMAC")
Cc: [email protected]

Quoted 8 lines

Signed-off-by Anoob Joseph <[email protected]>
---
Changes in v2:
- Made the skip applicable for in place also (comment from Pablo)
- Re-organized code as suggested by Pablo
- Made same change in test_mixed_auth_cipher_sgl (comment from Pablo)
app/test/test_cryptodev.c | 63 ++++++++++++++++++++++++++--------------

Acked-by Ciara Power <[email protected]>

De Lara Guarch, Pablo

Acked-by

2021-11-10 17:16 UTC #5 lore raw

RE: [PATCH v2] test/crypto: skip plain text compare for null cipher

Hi Anoob,

Quoted 49 lines

-----Original Message-----
From: Power, Ciara <[email protected]>
Sent: Wednesday, November 10, 2021 5:01 PM
To: Anoob Joseph <[email protected]>; Akhil Goyal <[email protected]>; Doherty, Declan <[email protected]>; Zhang, Roy Fan <[email protected]>; De Lara Guarch, Pablo <[email protected]>
Cc: Jerin Jacob <[email protected]>; Archana Muniganti <[email protected]>; Tejasree Kondoj <[email protected]>; Hemant Agrawal <[email protected]>; Nicolau, Radu <[email protected]>; Gagandeep Singh <[email protected]>; Ji, Kai <[email protected]>; [email protected]; [email protected]; De Lara Guarch, Pablo <[email protected]>
Subject: RE: [PATCH v2] test/crypto: skip plain text compare for null cipher
Hi Anoob,
Quoted 32 lines
-----Original Message-----
From: Anoob Joseph <[email protected]>
Sent: Wednesday 10 November 2021 13:04
To: Akhil Goyal <[email protected]>; Doherty, Declan <[email protected]>; Zhang, Roy Fan <[email protected]>; De Lara Guarch, Pablo <[email protected]>
Cc: Anoob Joseph <[email protected]>; Jerin Jacob <[email protected]>; Archana Muniganti <[email protected]>; Tejasree Kondoj <[email protected]>; Hemant Agrawal <[email protected]>; Nicolau, Radu <[email protected]>; Power, Ciara <[email protected]>; Gagandeep Singh <[email protected]>; Ji, Kai <[email protected]>; [email protected]; [email protected]
Subject: [PATCH v2] test/crypto: skip plain text compare for null cipher
NULL cipher is used for validating auth only cases. With NULL cipher, validating plain text should not be done as the PMD is only expected to update auth data.
Fixes e847fc512817 ("test/crypto: add encrypted digest case for
AES-CTR-
CMAC")
Cc: [email protected]
Signed-off-by Anoob Joseph <[email protected]>
---
Changes in v2:
- Made the skip applicable for in place also (comment from Pablo)
- Re-organized code as suggested by Pablo
- Made same change in test_mixed_auth_cipher_sgl (comment from Pablo)

Quoted 4 lines

Quoted 2 lines
app/test/test_cryptodev.c | 63
++++++++++++++++++++++++++--------------
Acked-by Ciara Power <[email protected]>

Wonder if this patch needs to CC stable, as it's fixing a commit from 2019?

Apart from this:

Acked-by Pablo de Lara <[email protected]>

Akhil Goyal

Applied

2021-11-11 11:04 UTC #6 lore raw

RE: [PATCH v2] test/crypto: skip plain text compare for null cipher

Quoted 7 lines

Quoted 2 lines
Acked-by Ciara Power <[email protected]>
Wonder if this patch needs to CC stable, as it's fixing a commit from 2019?
Apart from this:
Acked-by Pablo de Lara <[email protected]>

Applied to dpdk-next-crypto

Cc [email protected]