[PATCH] crypto/ipsec_mb: fix usage of untrusted value

[PATCH] crypto/ipsec_mb: fix usage of untrusted value

[Piotr Bronowski]

This patch removes coverity defect CID 375828:
Untrusted value as argument (TAINTED_SCALAR)

Coverity issue: CID 375828
Fixes: 918fd2f1466b ("crypto/ipsec_mb: move aesni_mb PMD")

Signed-off-by: Piotr Bronowski <piotrx.bronowski@intel.com>
---
 drivers/crypto/ipsec_mb/pmd_aesni_gcm.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/crypto/ipsec_mb/pmd_aesni_gcm.c b/drivers/crypto/ipsec_mb/pmd_aesni_gcm.c
index e5ad629fe5..f735b77680 100644
--- a/drivers/crypto/ipsec_mb/pmd_aesni_gcm.c
+++ b/drivers/crypto/ipsec_mb/pmd_aesni_gcm.c
@@ -206,6 +206,9 @@ post_process_gcm_crypto_op(struct ipsec_mb_qp *qp,
 			op->status = RTE_CRYPTO_OP_STATUS_AUTH_FAILED;
 	} else {
 		if (session->req_digest_length != session->gen_digest_length) {
+			session->req_digest_length =
+				RTE_MIN(session->req_digest_length,
+						DIGEST_LENGTH_MAX);
 			if (session->op ==
 				IPSEC_MB_OP_AEAD_AUTHENTICATED_ENCRYPT)
 				memcpy(op->sym->aead.digest.data,
-- 
2.30.2

--------------------------------------------------------------
Intel Research and Development Ireland Limited
Registered in Ireland
Registered Office: Collinstown Industrial Park, Leixlip, County Kildare
Registered Number: 308263


This e-mail and any attachments may contain confidential material for the sole
use of the intended recipient(s). Any review or distribution by others is
strictly prohibited. If you are not the intended recipient, please contact the
sender and delete all copies.

[PATCH v2] crypto/ipsec_mb: fix usage of untrusted value

[Piotr Bronowski]

This patch removes coverity defect CID 375828:
Untrusted value as argument (TAINTED_SCALAR)

Coverity issue: CID 375828
Fixes: 918fd2f1466b ("crypto/ipsec_mb: move aesni_mb PMD")

Signed-off-by: Piotr Bronowski <piotrx.bronowski@intel.com>

Cc: stable@dpdk.org

---
v2: use a different logic to check digest length
---
 drivers/crypto/ipsec_mb/pmd_aesni_gcm.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/crypto/ipsec_mb/pmd_aesni_gcm.c b/drivers/crypto/ipsec_mb/pmd_aesni_gcm.c
index e5ad629fe5..7cd20fc1cf 100644
--- a/drivers/crypto/ipsec_mb/pmd_aesni_gcm.c
+++ b/drivers/crypto/ipsec_mb/pmd_aesni_gcm.c
@@ -96,7 +96,9 @@ aesni_gcm_session_configure(IMB_MGR *mb_mgr, void *session,
 		sess->iv.length = auth_xform->auth.iv.length;
 		key_length = auth_xform->auth.key.length;
 		key = auth_xform->auth.key.data;
-		sess->req_digest_length = auth_xform->auth.digest_length;
+		sess->req_digest_length =
+		    RTE_MIN(auth_xform->auth.digest_length,
+				DIGEST_LENGTH_MAX);
 		break;
 	case IPSEC_MB_OP_AEAD_AUTHENTICATED_ENCRYPT:
 	case IPSEC_MB_OP_AEAD_AUTHENTICATED_DECRYPT:
@@ -116,7 +118,9 @@ aesni_gcm_session_configure(IMB_MGR *mb_mgr, void *session,
 		key_length = aead_xform->aead.key.length;
 		key = aead_xform->aead.key.data;
 		sess->aad_length = aead_xform->aead.aad_length;
-		sess->req_digest_length = aead_xform->aead.digest_length;
+		sess->req_digest_length =
+			RTE_MIN(aead_xform->aead.digest_length,
+				DIGEST_LENGTH_MAX);
 		break;
 	default:
 		IPSEC_MB_LOG(
@@ -146,7 +150,7 @@ aesni_gcm_session_configure(IMB_MGR *mb_mgr, void *session,
 	}
 
 	/* Digest check */
-	if (sess->req_digest_length > 16) {
+	if (sess->req_digest_length > DIGEST_LENGTH_MAX) {
 		IPSEC_MB_LOG(ERR, "Invalid digest length");
 		ret = -EINVAL;
 		goto error_exit;
@@ -157,7 +161,7 @@ aesni_gcm_session_configure(IMB_MGR *mb_mgr, void *session,
 	 * the requested number of bytes.
 	 */
 	if (sess->req_digest_length < 4)
-		sess->gen_digest_length = 16;
+		sess->gen_digest_length = DIGEST_LENGTH_MAX;
 	else
 		sess->gen_digest_length = sess->req_digest_length;
 
-- 
2.30.2

--------------------------------------------------------------
Intel Research and Development Ireland Limited
Registered in Ireland
Registered Office: Collinstown Industrial Park, Leixlip, County Kildare
Registered Number: 308263


This e-mail and any attachments may contain confidential material for the sole
use of the intended recipient(s). Any review or distribution by others is
strictly prohibited. If you are not the intended recipient, please contact the
sender and delete all copies.

RE: [PATCH v2] crypto/ipsec_mb: fix usage of untrusted value

[Zhang, Roy Fan]

> -----Original Message-----
> From: Bronowski, PiotrX <piotrx.bronowski@intel.com>
> Sent: Monday, March 7, 2022 3:33 PM
> To: dev@dpdk.org
> Cc: Zhang, Roy Fan <roy.fan.zhang@intel.com>; thomas@monjalon.net;
> gakhil@marvell.com; Yigit, Ferruh <ferruh.yigit@intel.com>; Doherty, Declan
> <declan.doherty@intel.com>; Bronowski, PiotrX
> <piotrx.bronowski@intel.com>; stable@dpdk.org
> Subject: [PATCH v2] crypto/ipsec_mb: fix usage of untrusted value
> 
> This patch removes coverity defect CID 375828:
> Untrusted value as argument (TAINTED_SCALAR)
> 
> Coverity issue: CID 375828
> Fixes: 918fd2f1466b ("crypto/ipsec_mb: move aesni_mb PMD")
> 
> Signed-off-by: Piotr Bronowski <piotrx.bronowski@intel.com>
> 
> Cc: stable@dpdk.org
> 
> ---
> v2: use a different logic to check digest length
> ---
Acked-by: Fan Zhang <roy.fan.zhang@intel.com>

RE: [PATCH v2] crypto/ipsec_mb: fix usage of untrusted value

[Ji, Kai]

> > -----Original Message-----
> > From: Bronowski, PiotrX <piotrx.bronowski@intel.com>
> > Sent: Monday, March 7, 2022 3:33 PM
> > To: dev@dpdk.org
> > Cc: Zhang, Roy Fan <roy.fan.zhang@intel.com>; thomas@monjalon.net;
> > gakhil@marvell.com; Yigit, Ferruh <ferruh.yigit@intel.com>; Doherty,
> > Declan <declan.doherty@intel.com>; Bronowski, PiotrX
> > <piotrx.bronowski@intel.com>; stable@dpdk.org
> > Subject: [PATCH v2] crypto/ipsec_mb: fix usage of untrusted value
> >
> > This patch removes coverity defect CID 375828:
> > Untrusted value as argument (TAINTED_SCALAR)
> >
> > Coverity issue: CID 375828
> > Fixes: 918fd2f1466b ("crypto/ipsec_mb: move aesni_mb PMD")
> >
> > Signed-off-by: Piotr Bronowski <piotrx.bronowski@intel.com>
> >
> > Cc: stable@dpdk.org
> >
> > ---
> > v2: use a different logic to check digest length
> > ---
> Acked-by: Fan Zhang <roy.fan.zhang@intel.com>
Acked-by: Kai Ji <kai.ji@intel.com>

RE: [PATCH v2] crypto/ipsec_mb: fix usage of untrusted value

[Zhang, Roy Fan]

Hi Piotr,

> -----Original Message-----
> From: Zhang, Roy Fan <roy.fan.zhang@intel.com>
> Sent: Monday, March 7, 2022 4:27 PM
> To: Bronowski, PiotrX <piotrx.bronowski@intel.com>; dev@dpdk.org
> Cc: thomas@monjalon.net; gakhil@marvell.com; Yigit, Ferruh
> <ferruh.yigit@intel.com>; Doherty, Declan <declan.doherty@intel.com>;
> stable@dpdk.org
> Subject: RE: [PATCH v2] crypto/ipsec_mb: fix usage of untrusted value
> 
> > -----Original Message-----
> > From: Bronowski, PiotrX <piotrx.bronowski@intel.com>
> > Sent: Monday, March 7, 2022 3:33 PM
> > To: dev@dpdk.org
> > Cc: Zhang, Roy Fan <roy.fan.zhang@intel.com>; thomas@monjalon.net;
> > gakhil@marvell.com; Yigit, Ferruh <ferruh.yigit@intel.com>; Doherty,
> Declan
> > <declan.doherty@intel.com>; Bronowski, PiotrX
> > <piotrx.bronowski@intel.com>; stable@dpdk.org
> > Subject: [PATCH v2] crypto/ipsec_mb: fix usage of untrusted value
> >
> > This patch removes coverity defect CID 375828:
> > Untrusted value as argument (TAINTED_SCALAR)
> >
> > Coverity issue: CID 375828
> > Fixes: 918fd2f1466b ("crypto/ipsec_mb: move aesni_mb PMD")
> >
> > Signed-off-by: Piotr Bronowski <piotrx.bronowski@intel.com>
> >
> > Cc: stable@dpdk.org
> >
> > ---
> > v2: use a different logic to check digest length
> > ---
> Acked-by: Fan Zhang <roy.fan.zhang@intel.com>

Sorry I missed a point in your change and thanks for Ciara pointing this out.
You are changing the gen_digest_size to 64 which is wrong.
Please send v3.
Also instead of ack - Nack this patch.

RE: [PATCH v2] crypto/ipsec_mb: fix usage of untrusted value

[Power, Ciara]

Hi Piotr,

>-----Original Message-----
>From: Zhang, Roy Fan <roy.fan.zhang@intel.com>
>Sent: Wednesday 9 March 2022 14:35
>To: Zhang, Roy Fan <roy.fan.zhang@intel.com>; Bronowski, PiotrX
><piotrx.bronowski@intel.com>; dev@dpdk.org
>Cc: thomas@monjalon.net; gakhil@marvell.com; Yigit, Ferruh
><ferruh.yigit@intel.com>; Doherty, Declan <declan.doherty@intel.com>;
>stable@dpdk.org; Power, Ciara <ciara.power@intel.com>
>Subject: RE: [PATCH v2] crypto/ipsec_mb: fix usage of untrusted value
>
>Hi Piotr,
>
>> -----Original Message-----
>> From: Zhang, Roy Fan <roy.fan.zhang@intel.com>
>> Sent: Monday, March 7, 2022 4:27 PM
>> To: Bronowski, PiotrX <piotrx.bronowski@intel.com>; dev@dpdk.org
>> Cc: thomas@monjalon.net; gakhil@marvell.com; Yigit, Ferruh
>> <ferruh.yigit@intel.com>; Doherty, Declan <declan.doherty@intel.com>;
>> stable@dpdk.org
>> Subject: RE: [PATCH v2] crypto/ipsec_mb: fix usage of untrusted value
>>
>> > -----Original Message-----
>> > From: Bronowski, PiotrX <piotrx.bronowski@intel.com>
>> > Sent: Monday, March 7, 2022 3:33 PM
>> > To: dev@dpdk.org
>> > Cc: Zhang, Roy Fan <roy.fan.zhang@intel.com>; thomas@monjalon.net;
>> > gakhil@marvell.com; Yigit, Ferruh <ferruh.yigit@intel.com>; Doherty,
>> Declan
>> > <declan.doherty@intel.com>; Bronowski, PiotrX
>> > <piotrx.bronowski@intel.com>; stable@dpdk.org
>> > Subject: [PATCH v2] crypto/ipsec_mb: fix usage of untrusted value
>> >
>> > This patch removes coverity defect CID 375828:
>> > Untrusted value as argument (TAINTED_SCALAR)
>> >
>> > Coverity issue: CID 375828
>> > Fixes: 918fd2f1466b ("crypto/ipsec_mb: move aesni_mb PMD")
>> >
>> > Signed-off-by: Piotr Bronowski <piotrx.bronowski@intel.com>
>> >
>> > Cc: stable@dpdk.org
>> >
>> > ---
>> > v2: use a different logic to check digest length
>> > ---
>> Acked-by: Fan Zhang <roy.fan.zhang@intel.com>
>
>Sorry I missed a point in your change and thanks for Ciara pointing this out.
>You are changing the gen_digest_size to 64 which is wrong.
>Please send v3.
>Also instead of ack - Nack this patch.

[CP]

In the v3 I think Fixes line should also be updated to either:

Fixes: 746825e5c0ea ("crypto/ipsec_mb: move aesni_gcm PMD")
Or
Fixes: ceb863938708 ("crypto/aesni_gcm: support all truncated digest sizes")
Cc: pablo.de.lara.guarch@intel.com

(The second one seems to be where the code was introduced before being moved into the consolidated ipsec_mb PMD in 21.11)

Thanks,
Ciara

[PATCH v3] crypto/ipsec_mb: fix coverity issue

[Piotr Bronowski]

This patch removes coverity defect CID 375828:
Untrusted value as argument (TAINTED_SCALAR)

Coverity issue: CID 375828

Fixes: 918fd2f1466b ("crypto/ipsec_mb: move aesni_mb PMD")

Signed-off-by: Piotr Bronowski <piotrx.bronowski@intel.com>

Cc: stable@dpdk.org

---
v3: use a different logic to check digest length
---
 drivers/crypto/ipsec_mb/pmd_aesni_gcm.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/crypto/ipsec_mb/pmd_aesni_gcm.c b/drivers/crypto/ipsec_mb/pmd_aesni_gcm.c
index e5ad629fe5..2c033c6f28 100644
--- a/drivers/crypto/ipsec_mb/pmd_aesni_gcm.c
+++ b/drivers/crypto/ipsec_mb/pmd_aesni_gcm.c
@@ -96,7 +96,9 @@ aesni_gcm_session_configure(IMB_MGR *mb_mgr, void *session,
 		sess->iv.length = auth_xform->auth.iv.length;
 		key_length = auth_xform->auth.key.length;
 		key = auth_xform->auth.key.data;
-		sess->req_digest_length = auth_xform->auth.digest_length;
+		sess->req_digest_length =
+		    RTE_MIN(auth_xform->auth.digest_length,
+				DIGEST_LENGTH_MAX);
 		break;
 	case IPSEC_MB_OP_AEAD_AUTHENTICATED_ENCRYPT:
 	case IPSEC_MB_OP_AEAD_AUTHENTICATED_DECRYPT:
@@ -116,7 +118,9 @@ aesni_gcm_session_configure(IMB_MGR *mb_mgr, void *session,
 		key_length = aead_xform->aead.key.length;
 		key = aead_xform->aead.key.data;
 		sess->aad_length = aead_xform->aead.aad_length;
-		sess->req_digest_length = aead_xform->aead.digest_length;
+		sess->req_digest_length =
+			RTE_MIN(aead_xform->aead.digest_length,
+				DIGEST_LENGTH_MAX);
 		break;
 	default:
 		IPSEC_MB_LOG(
-- 
2.30.2

--------------------------------------------------------------
Intel Research and Development Ireland Limited
Registered in Ireland
Registered Office: Collinstown Industrial Park, Leixlip, County Kildare
Registered Number: 308263


This e-mail and any attachments may contain confidential material for the sole
use of the intended recipient(s). Any review or distribution by others is
strictly prohibited. If you are not the intended recipient, please contact the
sender and delete all copies.

[PATCH v4] crypto/ipsec_mb: fix coverity issue

[Piotr Bronowski]

This patch removes coverity defect CID 375828:
Untrusted value as argument (TAINTED_SCALAR)

Coverity issue: CID 375828

Fixes: ceb863938708 ("crypto/aesni_gcm: support all truncated digest sizes")

Signed-off-by: Piotr Bronowski <piotrx.bronowski@intel.com>

Cc: stable@dpdk.org

---
v4: commit message corrected
---
 drivers/crypto/ipsec_mb/pmd_aesni_gcm.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/crypto/ipsec_mb/pmd_aesni_gcm.c b/drivers/crypto/ipsec_mb/pmd_aesni_gcm.c
index e5ad629fe5..2c033c6f28 100644
--- a/drivers/crypto/ipsec_mb/pmd_aesni_gcm.c
+++ b/drivers/crypto/ipsec_mb/pmd_aesni_gcm.c
@@ -96,7 +96,9 @@ aesni_gcm_session_configure(IMB_MGR *mb_mgr, void *session,
 		sess->iv.length = auth_xform->auth.iv.length;
 		key_length = auth_xform->auth.key.length;
 		key = auth_xform->auth.key.data;
-		sess->req_digest_length = auth_xform->auth.digest_length;
+		sess->req_digest_length =
+		    RTE_MIN(auth_xform->auth.digest_length,
+				DIGEST_LENGTH_MAX);
 		break;
 	case IPSEC_MB_OP_AEAD_AUTHENTICATED_ENCRYPT:
 	case IPSEC_MB_OP_AEAD_AUTHENTICATED_DECRYPT:
@@ -116,7 +118,9 @@ aesni_gcm_session_configure(IMB_MGR *mb_mgr, void *session,
 		key_length = aead_xform->aead.key.length;
 		key = aead_xform->aead.key.data;
 		sess->aad_length = aead_xform->aead.aad_length;
-		sess->req_digest_length = aead_xform->aead.digest_length;
+		sess->req_digest_length =
+			RTE_MIN(aead_xform->aead.digest_length,
+				DIGEST_LENGTH_MAX);
 		break;
 	default:
 		IPSEC_MB_LOG(
-- 
2.30.2

--------------------------------------------------------------
Intel Research and Development Ireland Limited
Registered in Ireland
Registered Office: Collinstown Industrial Park, Leixlip, County Kildare
Registered Number: 308263


This e-mail and any attachments may contain confidential material for the sole
use of the intended recipient(s). Any review or distribution by others is
strictly prohibited. If you are not the intended recipient, please contact the
sender and delete all copies.

RE: [PATCH v4] crypto/ipsec_mb: fix coverity issue

[Power, Ciara]

>-----Original Message-----
>From: Piotr Bronowski <piotrx.bronowski@intel.com>
>Sent: Wednesday 9 March 2022 18:02
>To: dev@dpdk.org
>Cc: Zhang, Roy Fan <roy.fan.zhang@intel.com>; thomas@monjalon.net;
>gakhil@marvell.com; Yigit, Ferruh <ferruh.yigit@intel.com>; Doherty, Declan
><declan.doherty@intel.com>; Bronowski, PiotrX
><piotrx.bronowski@intel.com>; stable@dpdk.org
>Subject: [PATCH v4] crypto/ipsec_mb: fix coverity issue
>
>This patch removes coverity defect CID 375828:
>Untrusted value as argument (TAINTED_SCALAR)
>
>Coverity issue: CID 375828
>
>Fixes: ceb863938708 ("crypto/aesni_gcm: support all truncated digest sizes")
>
>Signed-off-by: Piotr Bronowski <piotrx.bronowski@intel.com>
>
>Cc: stable@dpdk.org
>
>---
>v4: commit message corrected
>---
> drivers/crypto/ipsec_mb/pmd_aesni_gcm.c | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
<snip>

Acked-by: Ciara Power <ciara.power@intel.com>

Re: [PATCH v4] crypto/ipsec_mb: fix coverity issue

[Thomas Monjalon]

> >This patch removes coverity defect CID 375828:
> >Untrusted value as argument (TAINTED_SCALAR)

It lacks an explanation of the cause.

> >Coverity issue: CID 375828

You should not write CID above.
> >
> >Fixes: ceb863938708 ("crypto/aesni_gcm: support all truncated digest sizes")
> >
> >Signed-off-by: Piotr Bronowski <piotrx.bronowski@intel.com>
> >
> >Cc: stable@dpdk.org

This Cc should be just below the "Fixes".

> Acked-by: Ciara Power <ciara.power@intel.com>

Fixed formatting, writing a better title, and applied.