[PATCH v3] common/cnxk: validate cipher key length

[Tejasree Kondoj]

Validate DES/3DES and AES key lengths before copying
into SA cipher_key[] to avoid out-of-bounds write
into adjacent IV/salt fields.

Fixes: 24d10645bdfb ("common/cnxk: support CN20K IPsec session")
Cc: stable at dpdk.org

Signed-off-by: Tejasree Kondoj <ktejasree at marvell.com>
---
 drivers/common/cnxk/cnxk_security.c | 127 ++++++++++++++++------------
 drivers/common/cnxk/roc_cpt.h       |   1 +
 2 files changed, 76 insertions(+), 52 deletions(-)

diff --git a/drivers/common/cnxk/cnxk_security.c b/drivers/common/cnxk/cnxk_security.c
index 14d29e605a..04aca12131 100644
--- a/drivers/common/cnxk/cnxk_security.c
+++ b/drivers/common/cnxk/cnxk_security.c
@@ -170,6 +170,35 @@ ot_ipsec_sa_common_param_fill(union roc_ot_ipsec_sa_word2 *w2, uint8_t *cipher_k
 	w2->s.spi = ipsec_xfrm->spi;
 
 	if (key != NULL && length != 0) {
+		/* Validate key length and set AES key len before copy to avoid overflow */
+		if (w2->s.enc_type == ROC_IE_SA_ENC_AES_CBC ||
+		    w2->s.enc_type == ROC_IE_SA_ENC_AES_CTR ||
+		    w2->s.enc_type == ROC_IE_SA_ENC_AES_GCM ||
+		    w2->s.enc_type == ROC_IE_SA_ENC_AES_CCM ||
+		    w2->s.auth_type == ROC_IE_SA_AUTH_AES_GMAC) {
+			switch (length) {
+			case ROC_CPT_AES128_KEY_LEN:
+				w2->s.aes_key_len = ROC_IE_SA_AES_KEY_LEN_128;
+				break;
+			case ROC_CPT_AES192_KEY_LEN:
+				w2->s.aes_key_len = ROC_IE_SA_AES_KEY_LEN_192;
+				break;
+			case ROC_CPT_AES256_KEY_LEN:
+				w2->s.aes_key_len = ROC_IE_SA_AES_KEY_LEN_256;
+				break;
+			default:
+				plt_err("Invalid AES key length");
+				return -EINVAL;
+			}
+		}
+		if (w2->s.enc_type == ROC_IE_SA_ENC_DES_CBC && length != ROC_CPT_DES_KEY_LEN) {
+			plt_err("Invalid DES key length");
+			return -EINVAL;
+		}
+		if (w2->s.enc_type == ROC_IE_SA_ENC_3DES_CBC && length != ROC_CPT_DES3_KEY_LEN) {
+			plt_err("Invalid 3DES key length");
+			return -EINVAL;
+		}
 		/* Copy encryption key */
 		memcpy(cipher_key, key, length);
 		tmp_key = (uint64_t *)cipher_key;
@@ -177,30 +206,7 @@ ot_ipsec_sa_common_param_fill(union roc_ot_ipsec_sa_word2 *w2, uint8_t *cipher_k
 			tmp_key[i] = rte_be_to_cpu_64(tmp_key[i]);
 	}
 
-	/* Set AES key length */
-	if (w2->s.enc_type == ROC_IE_SA_ENC_AES_CBC ||
-	    w2->s.enc_type == ROC_IE_SA_ENC_AES_CTR ||
-	    w2->s.enc_type == ROC_IE_SA_ENC_AES_GCM ||
-	    w2->s.enc_type == ROC_IE_SA_ENC_AES_CCM ||
-	    w2->s.auth_type == ROC_IE_SA_AUTH_AES_GMAC) {
-		switch (length) {
-		case ROC_CPT_AES128_KEY_LEN:
-			w2->s.aes_key_len = ROC_IE_SA_AES_KEY_LEN_128;
-			break;
-		case ROC_CPT_AES192_KEY_LEN:
-			w2->s.aes_key_len = ROC_IE_SA_AES_KEY_LEN_192;
-			break;
-		case ROC_CPT_AES256_KEY_LEN:
-			w2->s.aes_key_len = ROC_IE_SA_AES_KEY_LEN_256;
-			break;
-		default:
-			plt_err("Invalid AES key length");
-			return -EINVAL;
-		}
-	}
-
-	if (ipsec_xfrm->life.packets_soft_limit != 0 ||
-	    ipsec_xfrm->life.packets_hard_limit != 0) {
+	if (ipsec_xfrm->life.packets_soft_limit != 0 || ipsec_xfrm->life.packets_hard_limit != 0) {
 		if (ipsec_xfrm->life.bytes_soft_limit != 0 ||
 		    ipsec_xfrm->life.bytes_hard_limit != 0) {
 			plt_err("Expiry tracking with both packets & bytes is not supported");
@@ -844,9 +850,11 @@ on_ipsec_sa_ctl_set(struct rte_security_ipsec_xform *ipsec,
 				break;
 			case RTE_CRYPTO_CIPHER_DES_CBC:
 				ctl->enc_type = ROC_IE_SA_ENC_DES_CBC;
+				aes_key_len = cipher_xform->cipher.key.length;
 				break;
 			case RTE_CRYPTO_CIPHER_3DES_CBC:
 				ctl->enc_type = ROC_IE_SA_ENC_3DES_CBC;
+				aes_key_len = cipher_xform->cipher.key.length;
 				break;
 			case RTE_CRYPTO_CIPHER_AES_CBC:
 				ctl->enc_type = ROC_IE_SA_ENC_AES_CBC;
@@ -897,20 +905,18 @@ on_ipsec_sa_ctl_set(struct rte_security_ipsec_xform *ipsec,
 		}
 	}
 
-	/* Set AES key length */
-	if (ctl->enc_type == ROC_IE_SA_ENC_AES_CBC ||
-	    ctl->enc_type == ROC_IE_SA_ENC_AES_CTR ||
-	    ctl->enc_type == ROC_IE_SA_ENC_AES_GCM ||
-	    ctl->enc_type == ROC_IE_SA_ENC_AES_CCM ||
+	/* Validate and set AES key length before copy */
+	if (ctl->enc_type == ROC_IE_SA_ENC_AES_CBC || ctl->enc_type == ROC_IE_SA_ENC_AES_CTR ||
+	    ctl->enc_type == ROC_IE_SA_ENC_AES_GCM || ctl->enc_type == ROC_IE_SA_ENC_AES_CCM ||
 	    ctl->auth_type == ROC_IE_SA_AUTH_AES_GMAC) {
 		switch (aes_key_len) {
-		case 16:
+		case ROC_CPT_AES128_KEY_LEN:
 			ctl->aes_key_len = ROC_IE_SA_AES_KEY_LEN_128;
 			break;
-		case 24:
+		case ROC_CPT_AES192_KEY_LEN:
 			ctl->aes_key_len = ROC_IE_SA_AES_KEY_LEN_192;
 			break;
-		case 32:
+		case ROC_CPT_AES256_KEY_LEN:
 			ctl->aes_key_len = ROC_IE_SA_AES_KEY_LEN_256;
 			break;
 		default:
@@ -918,6 +924,14 @@ on_ipsec_sa_ctl_set(struct rte_security_ipsec_xform *ipsec,
 			return -EINVAL;
 		}
 	}
+	if (ctl->enc_type == ROC_IE_SA_ENC_DES_CBC && aes_key_len != ROC_CPT_DES_KEY_LEN) {
+		plt_err("Invalid DES key length");
+		return -EINVAL;
+	}
+	if (ctl->enc_type == ROC_IE_SA_ENC_3DES_CBC && aes_key_len != ROC_CPT_DES3_KEY_LEN) {
+		plt_err("Invalid 3DES key length");
+		return -EINVAL;
+	}
 
 	if (ipsec->options.esn)
 		ctl->esn_en = 1;
@@ -1364,6 +1378,35 @@ ow_ipsec_sa_common_param_fill(union roc_ow_ipsec_sa_word2 *w2, uint8_t *cipher_k
 	w2->s.spi = ipsec_xfrm->spi;
 
 	if (key != NULL && length != 0) {
+		/* Validate key length and set AES key len before copy to avoid overflow */
+		if (w2->s.enc_type == ROC_IE_SA_ENC_AES_CBC ||
+		    w2->s.enc_type == ROC_IE_SA_ENC_AES_CTR ||
+		    w2->s.enc_type == ROC_IE_SA_ENC_AES_GCM ||
+		    w2->s.enc_type == ROC_IE_SA_ENC_AES_CCM ||
+		    w2->s.auth_type == ROC_IE_SA_AUTH_AES_GMAC) {
+			switch (length) {
+			case ROC_CPT_AES128_KEY_LEN:
+				w2->s.aes_key_len = ROC_IE_SA_AES_KEY_LEN_128;
+				break;
+			case ROC_CPT_AES192_KEY_LEN:
+				w2->s.aes_key_len = ROC_IE_SA_AES_KEY_LEN_192;
+				break;
+			case ROC_CPT_AES256_KEY_LEN:
+				w2->s.aes_key_len = ROC_IE_SA_AES_KEY_LEN_256;
+				break;
+			default:
+				plt_err("Invalid AES key length");
+				return -EINVAL;
+			}
+		}
+		if (w2->s.enc_type == ROC_IE_SA_ENC_DES_CBC && length != ROC_CPT_DES_KEY_LEN) {
+			plt_err("Invalid DES key length");
+			return -EINVAL;
+		}
+		if (w2->s.enc_type == ROC_IE_SA_ENC_3DES_CBC && length != ROC_CPT_DES3_KEY_LEN) {
+			plt_err("Invalid 3DES key length");
+			return -EINVAL;
+		}
 		/* Copy encryption key */
 		memcpy(cipher_key, key, length);
 		tmp_key = (uint64_t *)cipher_key;
@@ -1371,26 +1414,6 @@ ow_ipsec_sa_common_param_fill(union roc_ow_ipsec_sa_word2 *w2, uint8_t *cipher_k
 			tmp_key[i] = rte_be_to_cpu_64(tmp_key[i]);
 	}
 
-	/* Set AES key length */
-	if (w2->s.enc_type == ROC_IE_SA_ENC_AES_CBC || w2->s.enc_type == ROC_IE_SA_ENC_AES_CCM ||
-	    w2->s.enc_type == ROC_IE_SA_ENC_AES_CTR || w2->s.enc_type == ROC_IE_SA_ENC_AES_GCM ||
-	    w2->s.enc_type == ROC_IE_SA_ENC_AES_CCM || w2->s.auth_type == ROC_IE_SA_AUTH_AES_GMAC) {
-		switch (length) {
-		case ROC_CPT_AES128_KEY_LEN:
-			w2->s.aes_key_len = ROC_IE_SA_AES_KEY_LEN_128;
-			break;
-		case ROC_CPT_AES192_KEY_LEN:
-			w2->s.aes_key_len = ROC_IE_SA_AES_KEY_LEN_192;
-			break;
-		case ROC_CPT_AES256_KEY_LEN:
-			w2->s.aes_key_len = ROC_IE_SA_AES_KEY_LEN_256;
-			break;
-		default:
-			plt_err("Invalid AES key length");
-			return -EINVAL;
-		}
-	}
-
 	if (ipsec_xfrm->life.packets_soft_limit != 0 || ipsec_xfrm->life.packets_hard_limit != 0) {
 		if (ipsec_xfrm->life.bytes_soft_limit != 0 ||
 		    ipsec_xfrm->life.bytes_hard_limit != 0) {
diff --git a/drivers/common/cnxk/roc_cpt.h b/drivers/common/cnxk/roc_cpt.h
index 4715359f49..533d194bd4 100644
--- a/drivers/common/cnxk/roc_cpt.h
+++ b/drivers/common/cnxk/roc_cpt.h
@@ -79,6 +79,7 @@
 #define ROC_CPT_SHA2_HMAC_LEN	16
 #define ROC_CPT_DES_IV_LEN	8
 
+#define ROC_CPT_DES_KEY_LEN	    8
 #define ROC_CPT_DES3_KEY_LEN	    24
 #define ROC_CPT_AES128_KEY_LEN	    16
 #define ROC_CPT_AES192_KEY_LEN	    24
-- 
2.25.1