Re: [PATCH net v2 0/4] Fix isolation of broadcast traffic and unmatched unicast traffic with MACsec offload

[Benjamin Poirier <bpoirier@nvidia.com>]

On 2024-04-19 14:30 -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 or
> unicast messages with an unmatched destination address 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
>     * 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
>       - MACsec security channel was able to be looked up from skb offload
>         context (mlx5 only) -> forward to MACsec port
>     * 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
>       - The offload behavior violates this principle in MACsec
> 
> I believe this behavior is a security bug since applications utilizing
> MACsec could be exploited using this behavior, and the correct way to
> resolve this is by having the hardware correctly indicate whether MACsec
> offload occurred for the packet or not. In the patches in this series, I
> leave a warning for when the problematic path occurs because I cannot
> figure out a secure way to fix the security issue that applies to the core
> MACsec offload handling in the Rx path without breaking MACsec offload for
> other vendors.
> 
> Shown at the bottom is an example use case where plaintext traffic sent to
> a physical port of a NIC configured for MACsec offload is unable to be
> handled correctly by the software stack when the NIC provides awareness to
> the kernel about whether the received packet is MACsec traffic or not. In
> this specific example, plaintext ARP requests are being responded with
> MACsec encrypted ARP replies (which leads to routing information being
> unable to be built for the requester).
> 
>     Side 1
> 
>       ip link del macsec0
>       ip address flush mlx5_1
>       ip address add 1.1.1.1/24 dev mlx5_1
>       ip link set dev mlx5_1 up
>       ip link add link mlx5_1 macsec0 type macsec sci 1 encrypt on
>       ip link set dev macsec0 address 00:11:22:33:44:66
>       ip macsec offload macsec0 mac
>       ip macsec add macsec0 tx sa 0 pn 1 on key 00 dffafc8d7b9a43d5b9a3dfbbf6a30c16
>       ip macsec add macsec0 rx sci 2 on
>       ip macsec add macsec0 rx sci 2 sa 0 pn 1 on key 00 ead3664f508eb06c40ac7104cdae4ce5
>       ip address flush macsec0
>       ip address add 2.2.2.1/24 dev macsec0
>       ip link set dev macsec0 up
> 
>       # macsec0 enters promiscuous mode.
>       # This enables all traffic received on macsec_vlan to be processed by
>       # the macsec offload rx datapath. This however means that traffic
>       # meant to be received by mlx5_1 will be incorrectly steered to
>       # macsec0 as well.
> 
>       ip link add link macsec0 name macsec_vlan type vlan id 1
>       ip link set dev macsec_vlan address 00:11:22:33:44:88
>       ip address flush macsec_vlan
>       ip address add 3.3.3.1/24 dev macsec_vlan
>       ip link set dev macsec_vlan up
> 
>     Side 2
> 
>       ip link del macsec0
>       ip address flush mlx5_1
>       ip address add 1.1.1.2/24 dev mlx5_1
>       ip link set dev mlx5_1 up
>       ip link add link mlx5_1 macsec0 type macsec sci 2 encrypt on
>       ip link set dev macsec0 address 00:11:22:33:44:77
>       ip macsec offload macsec0 mac
>       ip macsec add macsec0 tx sa 0 pn 1 on key 00 ead3664f508eb06c40ac7104cdae4ce5
>       ip macsec add macsec0 rx sci 1 on
>       ip macsec add macsec0 rx sci 1 sa 0 pn 1 on key 00 dffafc8d7b9a43d5b9a3dfbbf6a30c16
>       ip address flush macsec0
>       ip address add 2.2.2.2/24 dev macsec0
>       ip link set dev macsec0 up
> 
>       # macsec0 enters promiscuous mode.
>       # This enables all traffic received on macsec_vlan to be processed by
>       # the macsec offload rx datapath. This however means that traffic
>       # meant to be received by mlx5_1 will be incorrectly steered to
>       # macsec0 as well.
> 
>       ip link add link macsec0 name macsec_vlan type vlan id 1
>       ip link set dev macsec_vlan address 00:11:22:33:44:99
>       ip address flush macsec_vlan
>       ip address add 3.3.3.2/24 dev macsec_vlan
>       ip link set dev macsec_vlan up
> 
>     Side 1
> 
>       ping -I mlx5_1 1.1.1.2
>       PING 1.1.1.2 (1.1.1.2) from 1.1.1.1 mlx5_1: 56(84) bytes of data.
>       From 1.1.1.1 icmp_seq=1 Destination Host Unreachable
>       ping: sendmsg: No route to host
>       From 1.1.1.1 icmp_seq=2 Destination Host Unreachable
>       From 1.1.1.1 icmp_seq=3 Destination Host Unreachable
> 
> Changes:
> 
>   v1->v2:
>     * Fixed series subject to detail the issue being fixed
>     * Removed strange characters from cover letter
>     * Added comment in example that illustrates the impact involving
>       promiscuous mode
>     * Added patch for generalizing packet type detection
>     * Added Fixes: tags and targeting net
>     * Removed pointless warning in the heuristic Rx path for macsec offload
>     * Applied small refactor in Rx path offload to minimize scope of rx_sc
>       local variable
> 
> 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/20240419011740.333714-1-rrameshbabu@nvidia.com/
> 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>

Tested-by: Benjamin Poirier <bpoirier@nvidia.com>