[PATCH RFC] net: bridge: handle ports in locked mode for ll learning
From: Jonas Gorski <hidden>
Date: 2024-12-10 14:06:59
Also in:
bridge, lkml
Subsystem:
ethernet bridge, networking [general], the rest · Maintainers:
Nikolay Aleksandrov, Ido Schimmel, "David S. Miller", Eric Dumazet, Jakub Kicinski, Paolo Abeni, Linus Torvalds
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 <redacted>
---
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