On Fri, 19 Apr, 2024 17:04:07 +0200 Sabrina Dubroca <sd@xxxxxxxxxxxxxxx> wrote: > This should go to net, not net-next. It fixes a serious bug. Also > please change the title to: > fix isolation of broadcast traffic with MACsec offload > > "resolve security issue" is too vague. Ack. It also fixes an issue where macsec should not reply to arbitrary unicast traffic even in promiscuous mode. ARP unicast without a matching destination address should not be replied to by the macsec device even if its in promiscuous mode (the software implementation of macsec behaves correctly in this regard). > > 2024-04-18, 18:17:14 -0700, Rahul Rameshbabu wrote: >> Some device drivers support devices that enable them to annotate whether a >> Rx skb refers to a packet that was processed by the MACsec offloading >> functionality of the device. Logic in the Rx handling for MACsec offload >> does not utilize this information to preemptively avoid forwarding to the >> macsec netdev currently. Because of this, things like multicast messages >> such as ARP requests are forwarded to the macsec netdev whether the message >> received was MACsec encrypted or not. The goal of this patch series is to >> improve the Rx handling for MACsec offload for devices capable of >> annotating skbs received that were decrypted by the NIC offload for MACsec. >> >> Here is a summary of the issue that occurs with the existing logic today. >> >> * The current design of the MACsec offload handling path tries to use >> "best guess" mechanisms for determining whether a packet associated >> with the currently handled skb in the datapath was processed via HW >> offload > > nit: there's a strange character after "offload" and at the end of a > few other lines in this list Will clean up. They got carried over from the presentation I copied this list from. > >> * The best guess mechanism uses the following heuristic logic (in order of >> precedence) >> - Check if header destination MAC address matches MACsec netdev MAC >> address -> forward to MACsec port >> - Check if packet is multicast traffic -> forward to MACsec port > here ^ > >> - MACsec security channel was able to be looked up from skb offload >> context (mlx5 only) -> forward to MACsec port > here ^ > >> * Problem: plaintext traffic can potentially solicit a MACsec encrypted >> response from the offload device >> - Core aspect of MACsec is that it identifies unauthorized LAN connections >> and excludes them from communication >> + This behavior can be seen when not enabling offload for MACsec > here ^ > >> - The offload behavior violates this principle in MACsec >> > Thanks for taking the time to explicitly point them out. >> >> Link: https://github.com/Binary-Eater/macsec-rx-offload/blob/trunk/MACsec_violation_in_core_stack_offload_rx_handling.pdf >> Link: https://lore.kernel.org/netdev/87r0l25y1c.fsf@xxxxxxxxxx/ >> Link: https://lore.kernel.org/netdev/20231116182900.46052-1-rrameshbabu@xxxxxxxxxx/ >> Cc: Sabrina Dubroca <sd@xxxxxxxxxxxxxxx> >> Cc: stable@xxxxxxxxxxxxxxx >> Signed-off-by: Rahul Rameshbabu <rrameshbabu@xxxxxxxxxx> > > I would put some Fixes tags on this series. Since we can't do anything > about non-md_dst devices, I would say that the main patch fixes > 860ead89b851 ("net/macsec: Add MACsec skb_metadata_dst Rx Data path > support"), and the driver patch fixes b7c9400cbc48 ("net/mlx5e: > Implement MACsec Rx data path using MACsec skb_metadata_dst"). Jakub, > Rahul, does that sound ok to both of you? I am aligned with this. -- Thanks, Rahul Rameshbabu
