netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Sabrina Dubroca <sd@queasysnail.net>
To: Rahul Rameshbabu <rrameshbabu@nvidia.com>
Cc: netdev@vger.kernel.org, stable@vger.kernel.org,
	Jakub Kicinski <kuba@kernel.org>,
	Eric Dumazet <edumazet@google.com>,
	"David S. Miller" <davem@davemloft.net>,
	Paolo Abeni <pabeni@redhat.com>, Gal Pressman <gal@nvidia.com>,
	Tariq Toukan <tariqt@nvidia.com>,
	Yossi Kuperman <yossiku@nvidia.com>,
	Benjamin Poirier <bpoirier@nvidia.com>,
	Cosmin Ratiu <cratiu@nvidia.com>
Subject: Re: [PATCH net-next 0/3] Resolve security issue in MACsec offload Rx datapath
Date: Fri, 19 Apr 2024 17:04:07 +0200	[thread overview]
Message-ID: <ZiKH52u_sjpm2mhf@hog> (raw)
In-Reply-To: <20240419011740.333714-1-rrameshbabu@nvidia.com>

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.

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

>     * 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
> 

> 
> 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@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?

-- 
Sabrina


  parent reply	other threads:[~2024-04-19 15:04 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-19  1:17 [PATCH net-next 0/3] Resolve security issue in MACsec offload Rx datapath Rahul Rameshbabu
2024-04-19  1:17 ` [PATCH net-next 1/3] macsec: Enable devices to advertise whether they update sk_buff md_dst during offloads Rahul Rameshbabu
2024-04-19  1:17 ` [PATCH net-next 2/3] macsec: Detect if Rx skb is macsec-related for offloading devices that update md_dst Rahul Rameshbabu
2024-04-19 15:05   ` Sabrina Dubroca
2024-04-19 18:01     ` Rahul Rameshbabu
2024-04-22  9:23       ` Sabrina Dubroca
2024-04-23  5:55         ` Rahul Rameshbabu
2024-04-24 10:18           ` Sabrina Dubroca
2024-04-19  1:17 ` [PATCH net-next 3/3] net/mlx5e: Advertise mlx5 ethernet driver updates sk_buff md_dst for MACsec Rahul Rameshbabu
2024-04-19 15:04 ` Sabrina Dubroca [this message]
2024-04-19 17:56   ` [PATCH net-next 0/3] Resolve security issue in MACsec offload Rx datapath Rahul Rameshbabu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZiKH52u_sjpm2mhf@hog \
    --to=sd@queasysnail.net \
    --cc=bpoirier@nvidia.com \
    --cc=cratiu@nvidia.com \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=gal@nvidia.com \
    --cc=kuba@kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=rrameshbabu@nvidia.com \
    --cc=stable@vger.kernel.org \
    --cc=tariqt@nvidia.com \
    --cc=yossiku@nvidia.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).