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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [day] [month] [year] [list]
Message-ID: <CO1PR18MB46669D5C91EFEA477D1E7A1CA13BA@CO1PR18MB4666.namprd18.prod.outlook.com>
Date:   Mon, 17 Jul 2023 05:39:18 +0000
From:   Subbaraya Sundeep Bhatta <sbhatta@...vell.com>
To:     Kalesh Anakkur Purayil <kalesh-anakkur.purayil@...adcom.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)

Hi,

>From: Kalesh Anakkur Purayil <mailto:kalesh-anakkur.purayil@...adcom.com>
>Sent: Monday, July 17, 2023 10:50 AM
>To: Subbaraya Sundeep Bhatta <mailto:sbhatta@...vell.com>
>Cc: mailto:netdev@...r.kernel.org; mailto:linux-kernel@...r.kernel.org;
>mailto:kuba@...nel.org; mailto:davem@...emloft.net;
>mailto:edumazet@...gle.com; mailto:pabeni@...hat.com; Sunil Kovvuri
>Goutham <mailto:sgoutham@...vell.com>; Geethasowjanya Akula
><mailto:gakula@...vell.com>; Hariprasad Kelam
><mailto:hkelam@...vell.com>; Naveen Mamindlapalli
><mailto:naveenm@...vell.com>
>Subject: Re: [net PATCH] octeontx2-pf: mcs: Generate hash key using ecb(aes)
>
>
>
>On Mon, Jul 17, 2023 at 10:20 AM Subbaraya Sundeep Bhatta
><mailto:sbhatta@...vell.com> wrote:
>Hi,
>
>>From: Kalesh Anakkur Purayil <mailto:kalesh-anakkur.purayil@...adcom.com>
>>Sent: Sunday, July 16, 2023 4:20 PM
>>To: Subbaraya Sundeep Bhatta <mailto:sbhatta@...vell.com>
>>Cc: mailto:netdev@...r.kernel.org; mailto:linux-kernel@...r.kernel.org;
>mailto:kuba@...nel.org;
>>mailto:davem@...emloft.net; mailto:edumazet@...gle.com;
>mailto:pabeni@...hat.com; Sunil
>>Kovvuri Goutham <mailto:sgoutham@...vell.com>; Geethasowjanya Akula
>><mailto:gakula@...vell.com>; Hariprasad Kelam
><mailto:hkelam@...vell.com>; Naveen
>>Mamindlapalli <mailto: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: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: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?
>

I avoided another label since we can call skcipher_request_free even input req is NULL.
Anyway will add new label so that it is easier to understand and send v2. Thanks for review.

Sundeep

>>+       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

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ