[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-ID: <20241210140654.108998-1-jonas.gorski@bisdn.de>
Date: Tue, 10 Dec 2024 15:06:53 +0100
From: Jonas Gorski <jonas.gorski@...dn.de>
To: Roopa Prabhu <roopa@...dia.com>,
Nikolay Aleksandrov <razor@...ckwall.org>,
"David S. Miller" <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>,
Paolo Abeni <pabeni@...hat.com>,
Simon Horman <horms@...nel.org>,
Ido Schimmel <idosch@...dia.com>,
Hans Schultz <schultz.hans@...il.com>,
"Hans J. Schultz" <netdev@...io-technology.com>,
Vladimir Oltean <vladimir.oltean@....com>
Cc: bridge@...ts.linux.dev,
netdev@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: [PATCH RFC] net: bridge: handle ports in locked mode for ll learning
When support for locked ports was added with commit a21d9a670d81 ("net:
bridge: Add support for bridge port in locked mode"), learning is
inhibited when the port is locked in br_handle_frame_finish().
It was later extended in commit a35ec8e38cdd ("bridge: Add MAC
Authentication Bypass (MAB) support") where optionally learning is done
with locked entries.
Unfortunately both missed that learning may also happen on frames to
link local addresses (01:80:c2:00:00:0X) in br_handle_frame(), which
will call __br_handle_local_finish(), which may update the fdb unless
(ll) learning is disabled as well.
This can be easily observed by e.g. EAPOL frames to 01:80:c2:00:00:03 on
a port causing the source mac to be learned, which is then forwarded
normally, essentially bypassing any authentication.
Fix this by moving the BR_PORT_LOCKED handling into its own function,
and call it from both places.
Fixes: a21d9a670d81 ("net: bridge: Add support for bridge port in locked mode")
Fixes: a35ec8e38cdd ("bridge: Add MAC Authentication Bypass (MAB) support")
Signed-off-by: Jonas Gorski <jonas.gorski@...dn.de>
---
Sent as RFC since I'm not 100% sure this is the right way to fix.
net/bridge/br_input.c | 78 ++++++++++++++++++++++++-------------------
1 file changed, 44 insertions(+), 34 deletions(-)
diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c
index ceaa5a89b947..f269a9f1b871 100644
--- a/net/bridge/br_input.c
+++ b/net/bridge/br_input.c
@@ -72,6 +72,46 @@ static int br_pass_frame_up(struct sk_buff *skb, bool promisc)
br_netif_receive_skb);
}
+static int br_fdb_locked_learn(struct net_bridge_port *p, struct sk_buff *skb,
+ u16 vid, bool mark)
+{
+ struct net_bridge *br = p->br;
+
+ if (p->flags & BR_PORT_LOCKED) {
+ struct net_bridge_fdb_entry *fdb_src =
+ br_fdb_find_rcu(br, eth_hdr(skb)->h_source, vid);
+ if (!fdb_src) {
+ /* FDB miss. Create locked FDB entry if MAB is enabled
+ * and drop the packet.
+ */
+ if (p->flags & BR_PORT_MAB)
+ br_fdb_update(br, p, eth_hdr(skb)->h_source,
+ vid, BIT(BR_FDB_LOCKED));
+ return NET_RX_DROP;
+ } else if (READ_ONCE(fdb_src->dst) != p ||
+ test_bit(BR_FDB_LOCAL, &fdb_src->flags)) {
+ /* FDB mismatch. Drop the packet without roaming. */
+ return NET_RX_DROP;
+ } else if (test_bit(BR_FDB_LOCKED, &fdb_src->flags)) {
+ /* FDB match, but entry is locked. Refresh it and drop
+ * the packet.
+ */
+ br_fdb_update(br, p, eth_hdr(skb)->h_source, vid,
+ BIT(BR_FDB_LOCKED));
+ return NET_RX_DROP;
+ }
+ }
+
+ if (mark)
+ nbp_switchdev_frame_mark(p, skb);
+
+ /* insert into forwarding database after filtering to avoid spoofing */
+ if (p->flags & BR_LEARNING)
+ br_fdb_update(br, p, eth_hdr(skb)->h_source, vid, 0);
+
+ return NET_RX_SUCCESS;
+}
+
/* note: already called with rcu_read_lock */
int br_handle_frame_finish(struct net *net, struct sock *sk, struct sk_buff *skb)
{
@@ -108,37 +148,8 @@ int br_handle_frame_finish(struct net *net, struct sock *sk, struct sk_buff *skb
&state, &vlan))
goto out;
- if (p->flags & BR_PORT_LOCKED) {
- struct net_bridge_fdb_entry *fdb_src =
- br_fdb_find_rcu(br, eth_hdr(skb)->h_source, vid);
-
- if (!fdb_src) {
- /* FDB miss. Create locked FDB entry if MAB is enabled
- * and drop the packet.
- */
- if (p->flags & BR_PORT_MAB)
- br_fdb_update(br, p, eth_hdr(skb)->h_source,
- vid, BIT(BR_FDB_LOCKED));
- goto drop;
- } else if (READ_ONCE(fdb_src->dst) != p ||
- test_bit(BR_FDB_LOCAL, &fdb_src->flags)) {
- /* FDB mismatch. Drop the packet without roaming. */
- goto drop;
- } else if (test_bit(BR_FDB_LOCKED, &fdb_src->flags)) {
- /* FDB match, but entry is locked. Refresh it and drop
- * the packet.
- */
- br_fdb_update(br, p, eth_hdr(skb)->h_source, vid,
- BIT(BR_FDB_LOCKED));
- goto drop;
- }
- }
-
- nbp_switchdev_frame_mark(p, skb);
-
- /* insert into forwarding database after filtering to avoid spoofing */
- if (p->flags & BR_LEARNING)
- br_fdb_update(br, p, eth_hdr(skb)->h_source, vid, 0);
+ if (br_fdb_locked_learn(p, skb, vid, true) == NET_RX_DROP)
+ goto drop;
promisc = !!(br->dev->flags & IFF_PROMISC);
local_rcv = promisc;
@@ -234,11 +245,10 @@ static void __br_handle_local_finish(struct sk_buff *skb)
u16 vid = 0;
/* check if vlan is allowed, to avoid spoofing */
- if ((p->flags & BR_LEARNING) &&
- nbp_state_should_learn(p) &&
+ if (nbp_state_should_learn(p) &&
!br_opt_get(p->br, BROPT_NO_LL_LEARN) &&
br_should_learn(p, skb, &vid))
- br_fdb_update(p->br, p, eth_hdr(skb)->h_source, vid, 0);
+ br_fdb_locked_learn(p, skb, vid, false);
}
/* note: already called with rcu_read_lock */
--
2.47.1
--
BISDN GmbH
Körnerstraße 7-10
10785 Berlin
Germany
Phone:
+49-30-6108-1-6100
Managing Directors:
Dr.-Ing. Hagen Woesner, Andreas
Köpsel
Commercial register:
Amtsgericht Berlin-Charlottenburg HRB 141569
B
VAT ID No: DE283257294
Powered by blists - more mailing lists