netdev - RE: [PATCH net-next v5 5/5] macsec: Don't rely solely on the dst MAC address to identify destination MACsec device

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>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CO1PR18MB4666A6E343DBB5A1CCD005D4A1999@CO1PR18MB4666.namprd18.prod.outlook.com>
Date:   Fri, 14 Apr 2023 06:32:28 +0000
From:   Subbaraya Sundeep Bhatta <sbhatta@...vell.com>
To:     "ehakim@...dia.com" <ehakim@...dia.com>,
        "davem@...emloft.net" <davem@...emloft.net>,
        "kuba@...nel.org" <kuba@...nel.org>,
        "pabeni@...hat.com" <pabeni@...hat.com>,
        "edumazet@...gle.com" <edumazet@...gle.com>,
        "sd@...asysnail.net" <sd@...asysnail.net>
CC:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "leon@...nel.org" <leon@...nel.org>,
        Sunil Kovvuri Goutham <sgoutham@...vell.com>,
        Naveen Mamindlapalli <naveenm@...vell.com>
Subject: RE: [PATCH net-next v5 5/5] macsec: Don't rely solely on the dst MAC
 address to identify destination MACsec device

Hi,

>-----Original Message-----
>From: Emeel Hakim <ehakim@...dia.com> <ehakim@...dia.com>
>Sent: Thursday, April 13, 2023 4:26 PM
>To: davem@...emloft.net; kuba@...nel.org; pabeni@...hat.com;
>edumazet@...gle.com; sd@...asysnail.net
>Cc: netdev@...r.kernel.org; leon@...nel.org; Emeel Hakim
><ehakim@...dia.com>
>Subject: [PATCH net-next v5 5/5] macsec: Don't rely solely on the dst MAC
>address to identify destination MACsec device
>
>Offloading device drivers will mark offloaded MACsec SKBs with the
>corresponding SCI in the skb_metadata_dst so the macsec rx handler will know to
>which interface to divert those skbs, in case of a marked skb and a mismatch on
>the dst MAC address, divert the skb to the macsec net_device where the macsec
>rx_handler will be called to consider cases where relying solely on the dst MAC
>address is insufficient.
>
>One such instance is when using MACsec with a VLAN as an inner header, where
>the packet structure is ETHERNET | SECTAG | VLAN.
>In such a scenario, the dst MAC address in the ethernet header will correspond to
>the VLAN MAC address, resulting in a mismatch.
>

I did below commands:
ifconfig eth2 up
ip link add link eth2 macsec0 type macsec sci cacbcd4142430002
ifconfig macsec0 hw ether ca:cb:cd:41:42:43
ip macsec offload macsec0 mac
ifconfig macsec0 up
ip macsec add macsec0 tx sa 0 on pn 5 key 02 22222222222222222222222222222222
ip macsec add macsec0 rx sci cacbcd2122230001
ip macsec add macsec0 rx sci cacbcd2122230001 sa 0 pn 5 on key 01 11111111111111111111111111111111
ip link add link macsec0 vlan0 type vlan id 2

ifconfig vlan0 hw ether ca:cb:cd:21:22:23
ifconfig vlan0 up
[ 7106.072451] device macsec0 entered promiscuous mode
[ 7106.077330] device eth2 entered promiscuous mode

macsec0 entered promisc mode when upper_dev mac address is not equal to its mac.
I think we should check if macsec device is in promisc mode instead of omitting mac address compare.
Also all drivers/hardware do not support md_dst->type == METADATA_MACSEC hence if macsec is
offloaded and in promisc mode then go for another round.
Correct me if I am wrong.

Thanks,
Sundeep

>Signed-off-by: Emeel Hakim <ehakim@...dia.com>
>---
> drivers/net/macsec.c | 13 +++++++++++--
> 1 file changed, 11 insertions(+), 2 deletions(-)
>
>diff --git a/drivers/net/macsec.c b/drivers/net/macsec.c index
>25616247d7a5..c19a45dc6977 100644
>--- a/drivers/net/macsec.c
>+++ b/drivers/net/macsec.c
>@@ -1021,8 +1021,11 @@ static enum rx_handler_result
>handle_not_macsec(struct sk_buff *skb)
> 		 * the SecTAG, so we have to deduce which port to deliver to.
> 		 */
> 		if (macsec_is_offloaded(macsec) && netif_running(ndev)) {
>-			if (md_dst && md_dst->type == METADATA_MACSEC &&
>-			    (!find_rx_sc(&macsec->secy, md_dst-
>>u.macsec_info.sci)))
>+			struct macsec_rx_sc *rx_sc = (md_dst && md_dst->type
>== METADATA_MACSEC) ?
>+						     find_rx_sc(&macsec->secy,
>+								md_dst-
>>u.macsec_info.sci) : NULL;
>+
>+			if (md_dst && md_dst->type == METADATA_MACSEC &&
>!rx_sc)
> 				continue;
>
> 			if (ether_addr_equal_64bits(hdr->h_dest,
>@@ -1047,7 +1050,13 @@ static enum rx_handler_result
>handle_not_macsec(struct sk_buff *skb)
> 					nskb->pkt_type = PACKET_MULTICAST;
>
> 				__netif_rx(nskb);
>+			} else if (rx_sc) {
>+				skb->dev = ndev;
>+				skb->pkt_type = PACKET_HOST;
>+				ret = RX_HANDLER_ANOTHER;
>+				goto out;
> 			}
>+
> 			continue;
> 		}
>
>--
>2.21.3
>