Re: [PATCH net-next 0/3] Resolve security issue in MACsec offload Rx datapath

On Fri, 19 Apr, 2024 17:04:07 +0200 Sabrina Dubroca sd@queasysnail.net 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_violatio... Link: https://lore.kernel.org/netdev/87r0l25y1c.fsf@nvidia.com/ Link: https://lore.kernel.org/netdev/20231116182900.46052-1-rrameshbabu@nvidia.com... Cc: Sabrina Dubroca sd@queasysnail.net Cc: stable@vger.kernel.org Signed-off-by: Rahul Rameshbabu rrameshbabu@nvidia.com

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