| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Mon, 17 Jul 2023 10:49:30 +0530
From: Kalesh Anakkur Purayil <kalesh-anakkur.purayil@...adcom.com>
To: Subbaraya Sundeep Bhatta <sbhatta@...vell.com>
Cc: "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, "kuba@...nel.org" <kuba@...nel.org>,
"davem@...emloft.net" <davem@...emloft.net>, "edumazet@...gle.com" <edumazet@...gle.com>,
"pabeni@...hat.com" <pabeni@...hat.com>, Sunil Kovvuri Goutham <sgoutham@...vell.com>,
Geethasowjanya Akula <gakula@...vell.com>, Hariprasad Kelam <hkelam@...vell.com>,
Naveen Mamindlapalli <naveenm@...vell.com>
Subject: Re: [EXT] Re: [net PATCH] octeontx2-pf: mcs: Generate hash key using ecb(aes)
On Mon, Jul 17, 2023 at 10:20 AM Subbaraya Sundeep Bhatta <
sbhatta@...vell.com> wrote:
> Hi,
>
> >From: Kalesh Anakkur Purayil <kalesh-anakkur.purayil@...adcom.com>
> >Sent: Sunday, July 16, 2023 4:20 PM
> >To: Subbaraya Sundeep Bhatta <sbhatta@...vell.com>
> >Cc: netdev@...r.kernel.org; linux-kernel@...r.kernel.org; kuba@...nel.org
> ;
> >davem@...emloft.net; edumazet@...gle.com; pabeni@...hat.com; Sunil
> >Kovvuri Goutham <sgoutham@...vell.com>; Geethasowjanya Akula
> ><gakula@...vell.com>; Hariprasad Kelam <hkelam@...vell.com>; Naveen
> >Mamindlapalli <naveenm@...vell.com>
> >Subject: Re: [net PATCH] octeontx2-pf: mcs: Generate hash key using
> ecb(aes)
> >
> >
> >On Fri, Jul 14, 2023 at 9:53 PM Subbaraya Sundeep
> ><mailto:sbhatta@...vell.com> wrote:
> >Hardware generated encryption and ICV tags are found to
> >be wrong when tested with IEEE MACSEC test vectors.
> >This is because as per the HRM, the hash key (derived by
> >AES-ECB block encryption of an all 0s block with the SAK)
> >has to be programmed by the software in
> >MCSX_RS_MCS_CPM_TX_SLAVE_SA_PLCY_MEM_4X register.
> >Hence fix this by generating hash key in software and
> >configuring in hardware.
> >
> >Fixes: c54ffc73601c ("octeontx2-pf: mcs: Introduce MACSEC hardware
> >offloading")
> >Signed-off-by: Subbaraya Sundeep <mailto:sbhatta@...vell.com>
> >---
> > .../ethernet/marvell/octeontx2/nic/cn10k_macsec.c | 132
> +++++++++++++++---
> >---
> > 1 file changed, 95 insertions(+), 37 deletions(-)
> >
> >diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/cn10k_macsec.c
> >b/drivers/net/ethernet/marvell/octeontx2/nic/cn10k_macsec.c
> >index 6e2fb24..9f23118 100644
> >--- a/drivers/net/ethernet/marvell/octeontx2/nic/cn10k_macsec.c
> >+++ b/drivers/net/ethernet/marvell/octeontx2/nic/cn10k_macsec.c
> >@@ -4,6 +4,7 @@
> > * Copyright (C) 2022 Marvell.
> > */
> >
> >+#include <crypto/skcipher.h>
> > #include <linux/rtnetlink.h>
> > #include <linux/bitfield.h>
> > #include "otx2_common.h"
> >@@ -42,6 +43,51 @@
> > #define MCS_TCI_E 0x08 /* encryption */
> > #define MCS_TCI_C 0x04 /* changed text */
> >
> >+#define CN10K_MAX_HASH_LEN 16
> >+#define CN10K_MAX_SAK_LEN 32
> >+
> >+static int cn10k_ecb_aes_encrypt(struct otx2_nic *pfvf, u8 *sak,
> >+ u16 sak_len, u8 *hash)
> >+{
> >+ u8 data[CN10K_MAX_HASH_LEN] = { 0 };
> >[Kalesh]: There is no need in 0 here, just use {}
> >
> Input has to be all zeroes. AES-ECB block encryption of an all 0s block
> with the SAK key.
> Hence needed.
>
> >+ struct skcipher_request *req = NULL;
> >+ struct scatterlist sg_src, sg_dst;
> >+ struct crypto_skcipher *tfm;
> >+ DECLARE_CRYPTO_WAIT(wait);
> >+ int err;
> >+
> >+ tfm = crypto_alloc_skcipher("ecb(aes)", 0, 0);
> >+ if (IS_ERR(tfm)) {
> >+ dev_err(pfvf->dev, "failed to allocate transform for
> ecb-aes\n");
> >+ return PTR_ERR(tfm);
> >+ }
> >+
> >+ req = skcipher_request_alloc(tfm, GFP_KERNEL);
> >+ if (!req) {
> >+ dev_err(pfvf->dev, "failed to allocate request for
> skcipher\n");
> >+ err = -ENOMEM;
> >+ goto out;
> >+ }
> >+
> >+ err = crypto_skcipher_setkey(tfm, sak, sak_len);
> >[Kalesh]: No need for a return value check here?
> Missed it. Will add.
>
> >+
> >+ /* build sg list */
> >+ sg_init_one(&sg_src, data, CN10K_MAX_HASH_LEN);
> >+ sg_init_one(&sg_dst, hash, CN10K_MAX_HASH_LEN);
> >+
> >+ skcipher_request_set_callback(req, 0, crypto_req_done, &wait);
> >+ skcipher_request_set_crypt(req, &sg_src, &sg_dst,
> >+ CN10K_MAX_HASH_LEN, NULL);
> >+
> >+ err = crypto_skcipher_encrypt(req);
> >+ err = crypto_wait_req(err, &wait);
> >+
> >+out:
> >+ skcipher_request_free(req);
> >[Kalesh]: I think you should move the label here.
>
> No. After adding the new check, label must be above only.
[Kalesh] Sorry, I did not get that.
Why to invoke skcipher_request_free() when skcipher_request_alloc() fails?
Am I missing something here?
>
>
> >+ crypto_free_skcipher(tfm);
> >+ return err;
> >+}
> >+
> > static struct cn10k_mcs_txsc *cn10k_mcs_get_txsc(struct cn10k_mcs_cfg
> *cfg,
> > struct macsec_secy *secy)
> > {
> >@@ -330,19 +376,53 @@ static int cn10k_mcs_write_sc_cam(struct otx2_nic
> >*pfvf,
> > return ret;
> > }
> >
> >+static int cn10k_mcs_write_keys(struct otx2_nic *pfvf,
> >+ struct macsec_secy *secy,
> >+ struct mcs_sa_plcy_write_req *req,
> >+ u8 *sak, u8 *salt, ssci_t ssci)
> >+{
> >+ u8 hash_rev[CN10K_MAX_HASH_LEN] = { 0 };
> >[Kalesh]: There is no need in 0 here, just use {}
> >
> Okay
>
> Thanks,
> Sundeep
>
> >+ u8 sak_rev[CN10K_MAX_SAK_LEN] = { 0 };
> >+ u8 salt_rev[MACSEC_SALT_LEN] = { 0 };
> >+ u8 hash[CN10K_MAX_HASH_LEN] = { 0 };
> >+ u32 ssci_63_32;
> >+ int err, i;
> >+
> >+ err = cn10k_ecb_aes_encrypt(pfvf, sak, secy->key_len, hash);
> >+ if (err) {
> >+ dev_err(pfvf->dev, "Generating hash using ECB(AES)
> failed\n");
> >+ return err;
> >+ }
> >+
> >+ for (i = 0; i < secy->key_len; i++)
> >+ sak_rev[i] = sak[secy->key_len - 1 - i];
> >+
> >+ for (i = 0; i < CN10K_MAX_HASH_LEN; i++)
> >+ hash_rev[i] = hash[CN10K_MAX_HASH_LEN - 1 - i];
> >+
> >+ for (i = 0; i < MACSEC_SALT_LEN; i++)
> >+ salt_rev[i] = salt[MACSEC_SALT_LEN - 1 - i];
> >+
> >+ ssci_63_32 = (__force u32)cpu_to_be32((__force u32)ssci);
> >+
> >+ memcpy(&req->plcy[0][0], sak_rev, secy->key_len);
> >+ memcpy(&req->plcy[0][4], hash_rev, CN10K_MAX_HASH_LEN);
> >+ memcpy(&req->plcy[0][6], salt_rev, MACSEC_SALT_LEN);
> >+ req->plcy[0][7] |= (u64)ssci_63_32 << 32;
> >+
> >+ return 0;
> >+}
> >+
> > static int cn10k_mcs_write_rx_sa_plcy(struct otx2_nic *pfvf,
> > struct macsec_secy *secy,
> > struct cn10k_mcs_rxsc *rxsc,
> > u8 assoc_num, bool sa_in_use)
> > {
> >- unsigned char *src = rxsc->sa_key[assoc_num];
> > struct mcs_sa_plcy_write_req *plcy_req;
> >- u8 *salt_p = rxsc->salt[assoc_num];
> >+ u8 *sak = rxsc->sa_key[assoc_num];
> >+ u8 *salt = rxsc->salt[assoc_num];
> > struct mcs_rx_sc_sa_map *map_req;
> > struct mbox *mbox = &pfvf->mbox;
> >- u64 ssci_salt_95_64 = 0;
> >- u8 reg, key_len;
> >- u64 salt_63_0;
> > int ret;
> >
> > mutex_lock(&mbox->lock);
> >@@ -360,20 +440,10 @@ static int cn10k_mcs_write_rx_sa_plcy(struct
> otx2_nic
> >*pfvf,
> > goto fail;
> > }
> >
> >- for (reg = 0, key_len = 0; key_len < secy->key_len; key_len += 8)
> {
> >- memcpy((u8 *)&plcy_req->plcy[0][reg],
> >- (src + reg * 8), 8);
> >- reg++;
> >- }
> >-
> >- if (secy->xpn) {
> >- memcpy((u8 *)&salt_63_0, salt_p, 8);
> >- memcpy((u8 *)&ssci_salt_95_64, salt_p + 8, 4);
> >- ssci_salt_95_64 |= (__force u64)rxsc->ssci[assoc_num] <<
> 32;
> >-
> >- plcy_req->plcy[0][6] = salt_63_0;
> >- plcy_req->plcy[0][7] = ssci_salt_95_64;
> >- }
> >+ ret = cn10k_mcs_write_keys(pfvf, secy, plcy_req, sak,
> >+ salt, rxsc->ssci[assoc_num]);
> >+ if (ret)
> >+ goto fail;
> >
> > plcy_req->sa_index[0] = rxsc->hw_sa_id[assoc_num];
> > plcy_req->sa_cnt = 1;
> >@@ -586,13 +656,10 @@ static int cn10k_mcs_write_tx_sa_plcy(struct
> otx2_nic
> >*pfvf,
> > struct cn10k_mcs_txsc *txsc,
> > u8 assoc_num)
> > {
> >- unsigned char *src = txsc->sa_key[assoc_num];
> > struct mcs_sa_plcy_write_req *plcy_req;
> >- u8 *salt_p = txsc->salt[assoc_num];
> >+ u8 *sak = txsc->sa_key[assoc_num];
> >+ u8 *salt = txsc->salt[assoc_num];
> > struct mbox *mbox = &pfvf->mbox;
> >- u64 ssci_salt_95_64 = 0;
> >- u8 reg, key_len;
> >- u64 salt_63_0;
> > int ret;
> >
> > mutex_lock(&mbox->lock);
> >@@ -603,19 +670,10 @@ static int cn10k_mcs_write_tx_sa_plcy(struct
> otx2_nic
> >*pfvf,
> > goto fail;
> > }
> >
> >- for (reg = 0, key_len = 0; key_len < secy->key_len; key_len += 8)
> {
> >- memcpy((u8 *)&plcy_req->plcy[0][reg], (src + reg * 8), 8);
> >- reg++;
> >- }
> >-
> >- if (secy->xpn) {
> >- memcpy((u8 *)&salt_63_0, salt_p, 8);
> >- memcpy((u8 *)&ssci_salt_95_64, salt_p + 8, 4);
> >- ssci_salt_95_64 |= (__force u64)txsc->ssci[assoc_num] <<
> 32;
> >-
> >- plcy_req->plcy[0][6] = salt_63_0;
> >- plcy_req->plcy[0][7] = ssci_salt_95_64;
> >- }
> >+ ret = cn10k_mcs_write_keys(pfvf, secy, plcy_req, sak,
> >+ salt, txsc->ssci[assoc_num]);
> >+ if (ret)
> >+ goto fail;
> >
> > plcy_req->plcy[0][8] = assoc_num;
> > plcy_req->sa_index[0] = txsc->hw_sa_id[assoc_num];
> >--
> >2.7.4
> >
> >
> >
> >
> >--
> >Regards,
> >Kalesh A P
>
--
Regards,
Kalesh A P
Content of type "text/html" skipped
Download attachment "smime.p7s" of type "application/pkcs7-signature" (4239 bytes)
Powered by blists - more mailing lists