netdev - [PATCH net-next 2/3] macsec: Detect if Rx skb is macsec-related for offloading devices that update md

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: <20240419011740.333714-3-rrameshbabu@nvidia.com>
Date: Thu, 18 Apr 2024 18:17:16 -0700
From: Rahul Rameshbabu <rrameshbabu@...dia.com>
To: netdev@...r.kernel.org,
	stable@...r.kernel.org
Cc: Jakub Kicinski <kuba@...nel.org>,
	Eric Dumazet <edumazet@...gle.com>,
	"David S. Miller" <davem@...emloft.net>,
	Paolo Abeni <pabeni@...hat.com>,
	Gal Pressman <gal@...dia.com>,
	Tariq Toukan <tariqt@...dia.com>,
	Sabrina Dubroca <sd@...asysnail.net>,
	Yossi Kuperman <yossiku@...dia.com>,
	Benjamin Poirier <bpoirier@...dia.com>,
	Cosmin Ratiu <cratiu@...dia.com>,
	Rahul Rameshbabu <rrameshbabu@...dia.com>
Subject: [PATCH net-next 2/3] macsec: Detect if Rx skb is macsec-related for offloading devices that update md_dst

Can now correctly identify where the packets should be delivered by using
md_dst or its absence on devices that provide it.

This detection is not possible without device drivers that update md_dst. A
fallback pattern should be used for supporting such device drivers. This
fallback mode causes multicast messages to be cloned to both the non-macsec
and macsec ports, independent of whether the multicast message received was
encrypted over MACsec or not. Other non-macsec traffic may also fail to be
handled correctly for devices in promiscuous mode.

Link: https://lore.kernel.org/netdev/ZULRxX9eIbFiVi7v@hog/
Cc: Sabrina Dubroca <sd@...asysnail.net>
Cc: stable@...r.kernel.org
Signed-off-by: Rahul Rameshbabu <rrameshbabu@...dia.com>
Reviewed-by: Benjamin Poirier <bpoirier@...dia.com>
Reviewed-by: Cosmin Ratiu <cratiu@...dia.com>
---
 drivers/net/macsec.c | 57 +++++++++++++++++++++++++++++++++++++-------
 1 file changed, 48 insertions(+), 9 deletions(-)

diff --git a/drivers/net/macsec.c b/drivers/net/macsec.c
index 0206b84284ab..679302ef1cd9 100644
--- a/drivers/net/macsec.c
+++ b/drivers/net/macsec.c
@@ -991,6 +991,19 @@ static struct macsec_rx_sc *find_rx_sc_rtnl(struct macsec_secy *secy, sci_t sci)
 	return NULL;
 }
 
+static __u8 macsec_offload_pkt_type(const u8 *h_dest, const u8 *ndev_broadcast)
+
+{
+	if (is_multicast_ether_addr_64bits(h_dest)) {
+		if (ether_addr_equal_64bits(h_dest, ndev_broadcast))
+			return PACKET_BROADCAST;
+		else
+			return PACKET_MULTICAST;
+	}
+
+	return PACKET_HOST;
+}
+
 static enum rx_handler_result handle_not_macsec(struct sk_buff *skb)
 {
 	/* Deliver to the uncontrolled port by default */
@@ -999,10 +1012,12 @@ static enum rx_handler_result handle_not_macsec(struct sk_buff *skb)
 	struct metadata_dst *md_dst;
 	struct macsec_rxh_data *rxd;
 	struct macsec_dev *macsec;
+	bool is_macsec_md_dst;
 
 	rcu_read_lock();
 	rxd = macsec_data_rcu(skb->dev);
 	md_dst = skb_metadata_dst(skb);
+	is_macsec_md_dst = md_dst && md_dst->type == METADATA_MACSEC;
 
 	list_for_each_entry_rcu(macsec, &rxd->secys, secys) {
 		struct sk_buff *nskb;
@@ -1014,13 +1029,40 @@ static enum rx_handler_result handle_not_macsec(struct sk_buff *skb)
 		 */
 		if (macsec_is_offloaded(macsec) && netif_running(ndev)) {
 			struct macsec_rx_sc *rx_sc = NULL;
+			const struct macsec_ops *ops;
 
-			if (md_dst && md_dst->type == METADATA_MACSEC)
-				rx_sc = find_rx_sc(&macsec->secy, md_dst->u.macsec_info.sci);
+			ops = macsec_get_ops(macsec, NULL);
 
-			if (md_dst && md_dst->type == METADATA_MACSEC && !rx_sc)
+			if (ops->rx_uses_md_dst && !is_macsec_md_dst)
 				continue;
 
+			if (is_macsec_md_dst) {
+				/* All drivers that implement MACsec offload
+				 * support using skb metadata destinations must
+				 * indicate that they do so.
+				 */
+				DEBUG_NET_WARN_ON_ONCE(!ops->rx_uses_md_dst);
+				rx_sc = find_rx_sc(&macsec->secy, md_dst->u.macsec_info.sci);
+				if (!rx_sc)
+					continue;
+				/* device indicated macsec offload occurred */
+				skb->dev = ndev;
+				skb->pkt_type = macsec_offload_pkt_type(
+					hdr->h_dest, ndev->broadcast);
+				ret = RX_HANDLER_ANOTHER;
+				goto out;
+			}
+
+			/* This datapath is insecure because it is unable to
+			 * enforce isolation of broadcast/multicast traffic and
+			 * unicast traffic with promiscuous mode on the macsec
+			 * netdev. Since the core stack has no mechanism to
+			 * check that the hardware did indeed receive MACsec
+			 * traffic, it is possible that the response handling
+			 * done by the MACsec port was to a plaintext packet.
+			 * This violates the MACsec protocol standard.
+			 */
+			DEBUG_NET_WARN_ON_ONCE(true);
 			if (ether_addr_equal_64bits(hdr->h_dest,
 						    ndev->dev_addr)) {
 				/* exact match, divert skb to this port */
@@ -1036,14 +1078,11 @@ static enum rx_handler_result handle_not_macsec(struct sk_buff *skb)
 					break;
 
 				nskb->dev = ndev;
-				if (ether_addr_equal_64bits(hdr->h_dest,
-							    ndev->broadcast))
-					nskb->pkt_type = PACKET_BROADCAST;
-				else
-					nskb->pkt_type = PACKET_MULTICAST;
+				nskb->pkt_type = macsec_offload_pkt_type(
+					hdr->h_dest, ndev->broadcast);
 
 				__netif_rx(nskb);
-			} else if (rx_sc || ndev->flags & IFF_PROMISC) {
+			} else if (ndev->flags & IFF_PROMISC) {
 				skb->dev = ndev;
 				skb->pkt_type = PACKET_HOST;
 				ret = RX_HANDLER_ANOTHER;
-- 
2.42.0