[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <87jzkt6xg0.fsf@nvidia.com>
Date: Fri, 19 Apr 2024 10:56:20 -0700
From: Rahul Rameshbabu <rrameshbabu@...dia.com>
To: Sabrina Dubroca <sd@...asysnail.net>
Cc: netdev@...r.kernel.org, stable@...r.kernel.org, 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>, Yossi Kuperman
<yossiku@...dia.com>, Benjamin Poirier <bpoirier@...dia.com>, Cosmin Ratiu
<cratiu@...dia.com>
Subject: 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@...asysnail.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_violation_in_core_stack_offload_rx_handling.pdf
>> 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@...asysnail.net>
>> Cc: stable@...r.kernel.org
>> Signed-off-by: Rahul Rameshbabu <rrameshbabu@...dia.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
Powered by blists - more mailing lists