<meta http-equiv="Content-Type" content="text/html; charset=GB18030"><font face="ثخجه" size="2"><div> Hi,</div><div><br></div><div> I found a missing input validation issue in the Marvell cnxk IPsec</div><div> Security Association (SA) creation paths. When DES_CBC or 3DES_CBC is</div><div> used, the cipher key length from the user-supplied xform is never</div><div> validated before being passed to memcpy(), which can cause an</div><div> out-of-bounds write into adjacent SA structure members.</div><div><br></div><div> The issue affects three independent code paths (ON/OT/OW families).</div><div> The root cause is the same in all three: the AES key length validation</div><div> block is only entered for AES-family algorithms, and no equivalent</div><div> check exists for DES/3DES.</div><div><br></div><div> Confirmed against current HEAD (8dc80afd, 2026-03-05).</div><div><br></div><div> == Root Cause ==</div><div><br></div><div> Consider on_fill_ipsec_common_sa() in</div><div> drivers/common/cnxk/cnxk_security.c, lines 940-991.</div><div><br></div><div> The function first calls on_ipsec_sa_ctl_set(), which for 3DES_CBC</div><div> only sets the enc_type field without examining key length (line</div><div> 848-850):</div><div><br></div><div> case RTE_CRYPTO_CIPHER_3DES_CBC:</div><div> ctl->enc_type = ROC_IE_SA_ENC_3DES_CBC;</div><div> break;</div><div><br></div><div> Compare this with AES_CBC, which captures the key length into</div><div> aes_key_len for later validation (line 851-854):</div><div><br></div><div> case RTE_CRYPTO_CIPHER_AES_CBC:</div><div> ctl->enc_type = ROC_IE_SA_ENC_AES_CBC;</div><div> aes_key_len = cipher_xform->cipher.key.length;</div><div> break;</div><div><br></div><div> Back in on_fill_ipsec_common_sa(), the cipher key is then copied with</div><div> the user-supplied length, unconditionally (lines 973-988):</div><div><br></div><div> if (cipher_xform) {</div><div> cipher_key = cipher_xform->cipher.key.data;</div><div> cipher_key_len = cipher_xform->cipher.key.length;</div><div> }</div><div> ...</div><div> if (cipher_key_len != 0)</div><div> memcpy(common_sa->cipher_key, cipher_key, cipher_key_len);</div><div><br></div><div> The only length validation that follows is restricted to AES types</div><div> (lines 900-920):</div><div><br></div><div> if (ctl->enc_type == ROC_IE_SA_ENC_AES_CBC ||</div><div> ctl->enc_type == ROC_IE_SA_ENC_AES_CTR ||</div><div> ctl->enc_type == ROC_IE_SA_ENC_AES_GCM ||</div><div> ctl->enc_type == ROC_IE_SA_ENC_AES_CCM ||</div><div> ctl->auth_type == ROC_IE_SA_AUTH_AES_GMAC) {</div><div> switch (aes_key_len) {</div><div> case 16: ... case 24: ... case 32: ... break;</div><div> default: return -EINVAL;</div><div> }</div><div> }</div><div><br></div><div> For DES/3DES, this block is never entered. No other validation exists</div><div> in this path. The function returns 0 (success) regardless of</div><div> cipher_key_len.</div><div><br></div><div> == Affected Structure Layout ==</div><div><br></div><div> The destination buffer cipher_key[32] is immediately followed by</div><div> active SA members in all three SA structure families:</div><div><br></div><div> ON family (roc_ie_on.h, struct roc_ie_on_common_sa):</div><div><br></div><div> uint8_t cipher_key[32]; /* w1-w4 */</div><div> union roc_ie_on_bit_perfect_iv iv; /* w5-w6, adjacent */</div><div><br></div><div> OT family (roc_ie_ot.h, struct roc_ot_ipsec_outb_sa):</div><div><br></div><div> uint8_t cipher_key[32]; /* Word4-7 */</div><div> union roc_ot_ipsec_outb_iv iv; /* Word8-9, adjacent */</div><div><br></div><div> OW family (roc_ie_ow.h, struct roc_ow_ipsec_outb_sa):</div><div><br></div><div> uint8_t cipher_key[32]; /* Word4-7 */</div><div> union roc_ow_ipsec_outb_iv iv; /* Word8-9, adjacent */</div><div><br></div><div> For inbound SA structures, cipher_key[32] is followed by the w8 union</div><div> containing the salt field, also adjacent.</div><div><br></div><div> == All Affected Sites ==</div><div><br></div><div> 1. on_fill_ipsec_common_sa() -- line 988</div><div> memcpy(common_sa->cipher_key, cipher_key, cipher_key_len);</div><div> Called from: cnxk_on_ipsec_outb_sa_create() (line 1015)</div><div> cnxk_on_ipsec_inb_sa_create() (line 1151)</div><div> Callers: cn9k_eth_sec_session_create()</div><div><br></div><div> 2. ot_ipsec_sa_common_param_fill() -- line 174</div><div> memcpy(cipher_key, key, length);</div><div> Called from: cnxk_ot_ipsec_inb_sa_fill() (line 324)</div><div> cnxk_ot_ipsec_outb_sa_fill() (line 437)</div><div> Callers: cn10k_eth_sec_session_create()</div><div><br></div><div> 3. ow_ipsec_sa_common_param_fill() -- line 1368</div><div> memcpy(cipher_key, key, length);</div><div> Called from: cnxk_ow_ipsec_inb_sa_fill() (line 1509)</div><div> cnxk_ow_ipsec_outb_sa_fill() (line 1619)</div><div> Callers: cn20k_eth_sec_session_create()</div><div><br></div><div> None of these callers (cn9k/cn10k/cn20k_eth_sec_session_create)</div><div> invoke cnxk_ipsec_xform_verify() before calling the SA fill</div><div> functions.</div><div><br></div><div> == Why Inline Paths Are Unprotected ==</div><div><br></div><div> The validation function cnxk_ipsec_xform_verify() already exists in</div><div> drivers/crypto/cnxk/cnxk_ipsec.h and correctly rejects invalid key</div><div> lengths:</div><div><br></div><div> /* 3DES: exactly 24 bytes */</div><div> if (crypto_xform->cipher.algo == RTE_CRYPTO_CIPHER_3DES_CBC &&</div><div> crypto_xform->cipher.key.length == 24)</div><div> return 0;</div><div><br></div><div> /* DES: exactly 8 bytes */</div><div> if (crypto_xform->cipher.algo == RTE_CRYPTO_CIPHER_DES_CBC &&</div><div> crypto_xform->cipher.key.length == 8)</div><div> return 0;</div><div><br></div><div> However, this function is defined as static inline in the crypto</div><div> driver header and is only called from the lookaside crypto session</div><div> creation paths (cn9k_ipsec.c, cn10k_ipsec.c, cn20k_ipsec.c under</div><div> drivers/crypto/cnxk/). The inline IPsec ethdev paths (cn9k/cn10k/</div><div> cn20k_ethdev_sec.c under drivers/net/cnxk/) never call it.</div><div><br></div><div> Additionally, the DPDK security framework function</div><div> rte_security_session_create() (lib/security/rte_security.c:70)</div><div> performs no key length validation against the capability table -- it</div><div> passes conf directly to the driver's session_create callback.</div><div><br></div><div> == Concrete Example ==</div><div><br></div><div> 3DES_CBC with a 40-byte key through the CN9K outbound path:</div><div><br></div><div> cipher_xform.cipher.algo = RTE_CRYPTO_CIPHER_3DES_CBC;</div><div> cipher_xform.cipher.key.length = 40;</div><div> rte_security_session_create(ctx, &conf, mp);</div><div><br></div><div> Call chain:</div><div> rte_security_session_create() -- no validation</div><div> -> cn9k_eth_sec_session_create() -- no xform_verify call</div><div> -> cnxk_on_ipsec_outb_sa_create()</div><div> -> on_fill_ipsec_common_sa()</div><div> -> on_ipsec_sa_ctl_set() -- sets enc_type only</div><div> -> memcpy(cipher_key, key, 40) -- overflows by 8 bytes</div><div><br></div><div> Result: the first 32 bytes fill cipher_key[32], the remaining 8 bytes</div><div> overwrite the adjacent iv member. The function returns 0 (success),</div><div> and the corrupted SA is accepted for use.</div><div><br></div><div> == Suggested Fix ==</div><div><br></div><div> Add DES/3DES key length validation alongside the existing AES checks.</div><div> The simplest approach is to add explicit checks in each</div><div> *_common_param_fill / on_fill_ipsec_common_sa function, before the</div><div> memcpy. For example:</div><div><br></div><div> if (w2->s.enc_type == ROC_IE_SA_ENC_DES_CBC && length != 8)</div><div> return -EINVAL;</div><div> if (w2->s.enc_type == ROC_IE_SA_ENC_3DES_CBC && length != 24)</div><div> return -EINVAL;</div><div><br></div><div> Alternatively (and more comprehensively), the existing</div><div> cnxk_ipsec_xform_verify() could be moved to the shared common layer</div><div> (drivers/common/cnxk/) and called from all session_create paths --</div><div> both crypto and inline. This would also close similar gaps for other</div><div> algorithm/key combinations in the inline path.</div><div><br></div><div> == Impact ==</div><div><br></div><div> - Corrupts IV/salt fields in the IPsec SA, leading to incorrect</div><div> cryptographic operations or hardware errors on the Octeon crypto</div><div> engine.</div><div> - The SA is accepted as successfully created, so the caller has no</div><div> indication that the SA state is corrupted.</div><div> - Requires the application to pass an invalid key length, which would</div><div> not occur in correct usage but represents a missing defensive check</div><div> at an API boundary.</div><div><br></div><div><br></div><div> Best regards,</div></font><div style="font-family: ثخجه; font-size: small;"> Pengpeng Hou</div><div style="font-family: ثخجه; font-size: small;"> <a href="mailto:pengpeng@iscas.ac.cn" rel="noopener" target="_blank" style="outline: none; cursor: pointer; color: rgb(30, 84, 148);">pengpeng@iscas.ac.cn</a></div>