| Message ID | 20221025100024.1287157-2-idosch@nvidia.com (mailing list archive) |
|---|---|
| State | RFC |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | bridge: Add MAC Authentication Bypass (MAB) support with offload | expand |
On 25/10/2022 13:00, Ido Schimmel wrote: > From: "Hans J. Schultz" <netdev@kapio-technology.com> > > Hosts that support 802.1X authentication are able to authenticate > themselves by exchanging EAPOL frames with an authenticator (Ethernet > bridge, in this case) and an authentication server. Access to the > network is only granted by the authenticator to successfully > authenticated hosts. > > The above is implemented in the bridge using the "locked" bridge port > option. When enabled, link-local frames (e.g., EAPOL) can be locally > received by the bridge, but all other frames are dropped unless the host > is authenticated. That is, unless the user space control plane installed > an FDB entry according to which the source address of the frame is > located behind the locked ingress port. The entry can be dynamic, in > which case learning needs to be enabled so that the entry will be > refreshed by incoming traffic. > > There are deployments in which not all the devices connected to the > authenticator (the bridge) support 802.1X. Such devices can include > printers and cameras. One option to support such deployments is to > unlock the bridge ports connecting these devices, but a slightly more > secure option is to use MAB. When MAB is enabled, the MAC address of the > connected device is used as the user name and password for the > authentication. > > For MAB to work, the user space control plane needs to be notified about > MAC addresses that are trying to gain access so that they will be > compared against an allow list. This can be implemented via the regular > learning process with the following differences: > > 1. Learned FDB entries are installed with a new "locked" flag indicating > that the entry cannot be used to authenticate the device. The flag > cannot be set by user space, but user space can clear the flag by > replacing the entry, thereby authenticating the device. > > 2. FDB entries cannot roam to locked ports to prevent unauthenticated > devices from disrupting traffic destined to already authenticated > devices. > > Enable this behavior using a new bridge port option called "mab". It can > only be enabled on a bridge port that is both locked and has learning > enabled. A new option is added because there are pure 802.1X deployments > that are not interested in notifications about "locked" FDB entries. > > Signed-off-by: Hans J. Schultz <netdev@kapio-technology.com> > Signed-off-by: Ido Schimmel <idosch@nvidia.com> > --- > > Notes: > Changes made by me: > > * Reword commit message. > * Reword comment regarding 'NTF_EXT_LOCKED'. > * Use extack in br_fdb_add(). > * Forbid MAB when learning is disabled. > > include/linux/if_bridge.h | 1 + > include/uapi/linux/if_link.h | 1 + > include/uapi/linux/neighbour.h | 8 +++++++- > net/bridge/br_fdb.c | 24 ++++++++++++++++++++++++ > net/bridge/br_input.c | 15 +++++++++++++-- > net/bridge/br_netlink.c | 13 ++++++++++++- > net/bridge/br_private.h | 3 ++- > net/core/rtnetlink.c | 5 +++++ > 8 files changed, 65 insertions(+), 5 deletions(-) > Thanks for finalizing this, the patch looks good to me. Acked-by: Nikolay Aleksandrov <razor@blackwall.org> Thanks, Nik
Hi Ido, Thanks for the commit message. It is very good. On Tue, Oct 25, 2022 at 01:00:09PM +0300, Ido Schimmel wrote: > From: "Hans J. Schultz" <netdev@kapio-technology.com> > > Hosts that support 802.1X authentication are able to authenticate > themselves by exchanging EAPOL frames with an authenticator (Ethernet > bridge, in this case) and an authentication server. Access to the > network is only granted by the authenticator to successfully > authenticated hosts. > > The above is implemented in the bridge using the "locked" bridge port > option. When enabled, link-local frames (e.g., EAPOL) can be locally > received by the bridge, but all other frames are dropped unless the host > is authenticated. That is, unless the user space control plane installed > an FDB entry according to which the source address of the frame is > located behind the locked ingress port. The entry can be dynamic, in > which case learning needs to be enabled so that the entry will be > refreshed by incoming traffic. > > There are deployments in which not all the devices connected to the > authenticator (the bridge) support 802.1X. Such devices can include > printers and cameras. One option to support such deployments is to > unlock the bridge ports connecting these devices, but a slightly more > secure option is to use MAB. When MAB is enabled, the MAC address of the > connected device is used as the user name and password for the > authentication. > > For MAB to work, the user space control plane needs to be notified about > MAC addresses that are trying to gain access so that they will be > compared against an allow list. This can be implemented via the regular > learning process with the following differences: > > 1. Learned FDB entries are installed with a new "locked" flag indicating > that the entry cannot be used to authenticate the device. The flag > cannot be set by user space, but user space can clear the flag by > replacing the entry, thereby authenticating the device. > > 2. FDB entries cannot roam to locked ports to prevent unauthenticated > devices from disrupting traffic destined to already authenticated > devices. The behavior described in (2) has nothing to do with locked FDB entries or MAB (what is described in this paragraph), it applies to all of them, no? The code was already there: if (p->flags & BR_PORT_LOCKED) if (!fdb_src || READ_ONCE(fdb_src->dst) != p || test_bit(BR_FDB_LOCAL, &fdb_src->flags)) goto drop; I think you mean to say: the above already holds true, but the relevant implication here is that locked FDB entries will not be created if the MAC address is present in the FDB on any other port? I think some part of this comment should also go to the convoluted BR_PORT_LOCKED block from br_handle_frame_finish()? I was going to ask if we should bother to add code to prohibit packets from being forwarded to an FDB entry that was learned as LOCKED, since that FDB entry is more of a "ghost" and not something fully committed? But with the "never roam to locked port" policy, I don't think there is any practical risk that the extra code would mitigate. Assume that a "snooper" wants to get the traffic destined for a MAC DA X, so it creates a LOCKED FDB entry. It has to time itself just right, 5 minutes after the station it wants to intercept has gone silent (or before the station said anything). Anyone thinking it's talking to X now talks to the snooper. But this attack vector is bounded in time. As long as X says anything, the LOCKED FDB entry moves towards X, and the LOCKED flag gets cleared. > > Enable this behavior using a new bridge port option called "mab". It can > only be enabled on a bridge port that is both locked and has learning > enabled. A new option is added because there are pure 802.1X deployments > that are not interested in notifications about "locked" FDB entries. > > Signed-off-by: Hans J. Schultz <netdev@kapio-technology.com> > Signed-off-by: Ido Schimmel <idosch@nvidia.com> > --- > > Notes: > Changes made by me: > > * Reword commit message. > * Reword comment regarding 'NTF_EXT_LOCKED'. > * Use extack in br_fdb_add(). > * Forbid MAB when learning is disabled. Forbidding MAB when learning is disabled makes sense to me, since it means accepting that MAB is a form of learning (as the implementation also shows; all other callers of br_fdb_update() are guarded by a port learning check). I believe this will also make life easier with offloading drivers. Thanks. > include/linux/if_bridge.h | 1 + > include/uapi/linux/if_link.h | 1 + > include/uapi/linux/neighbour.h | 8 +++++++- > net/bridge/br_fdb.c | 24 ++++++++++++++++++++++++ > net/bridge/br_input.c | 15 +++++++++++++-- > net/bridge/br_netlink.c | 13 ++++++++++++- > net/bridge/br_private.h | 3 ++- > net/core/rtnetlink.c | 5 +++++ > 8 files changed, 65 insertions(+), 5 deletions(-) > > diff --git a/include/linux/if_bridge.h b/include/linux/if_bridge.h > index d62ef428e3aa..1668ac4d7adc 100644 > --- a/include/linux/if_bridge.h > +++ b/include/linux/if_bridge.h > @@ -59,6 +59,7 @@ struct br_ip_list { > #define BR_MRP_LOST_IN_CONT BIT(19) > #define BR_TX_FWD_OFFLOAD BIT(20) > #define BR_PORT_LOCKED BIT(21) > +#define BR_PORT_MAB BIT(22) Question about unsetting BR_PORT_MAB using IFLA_BRPORT_MAB: should this operation flush BR_FDB_LOCKED entries on the port? > diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c > index 68b3e850bcb9..068fced7693c 100644 > --- a/net/bridge/br_input.c > +++ b/net/bridge/br_input.c > @@ -109,9 +109,20 @@ int br_handle_frame_finish(struct net *net, struct sock *sk, struct sk_buff *skb > struct net_bridge_fdb_entry *fdb_src = > br_fdb_find_rcu(br, eth_hdr(skb)->h_source, vid); > > - if (!fdb_src || READ_ONCE(fdb_src->dst) != p || > - test_bit(BR_FDB_LOCAL, &fdb_src->flags)) > + if (!fdb_src) { > + unsigned long flags = 0; > + > + if (p->flags & BR_PORT_MAB) { > + __set_bit(BR_FDB_LOCKED, &flags); > + br_fdb_update(br, p, eth_hdr(skb)->h_source, > + vid, flags); > + } > goto drop; > + } else if (READ_ONCE(fdb_src->dst) != p || > + test_bit(BR_FDB_LOCAL, &fdb_src->flags) || > + test_bit(BR_FDB_LOCKED, &fdb_src->flags)) { Minor nitpick: shouldn't br_fdb_update() also be called when the packet matched on a BR_FDB_LOCKED entry on port p, so as to refresh it, if the station is persistent? Currently I believe the FDB entry will expire within 5 minutes of the first packet, regardless of subsequent traffic. > + goto drop; > + } > } > > nbp_switchdev_frame_mark(p, skb);
On 2022-10-28 00:58, Vladimir Oltean wrote: > I was going to ask if we should bother to add code to prohibit packets > from being forwarded to an FDB entry that was learned as LOCKED, since > that FDB entry is more of a "ghost" and not something fully committed? I think that it is a security flaw if there is any forwarding to BR_FDB_LOCKED entries. I can imagine a host behind a locked port with no credentials, that gets a BR_FDB_LOCKED entry and has a friend on another non-locked port who can now communicate uni-directional to the host with the BR_FDB_LOCKED entry. It should not be too hard to create a scheme using UDP packets or other for that.
On Thu, Oct 27, 2022 at 10:58:32PM +0000, Vladimir Oltean wrote: > Hi Ido, > > Thanks for the commit message. It is very good. > > On Tue, Oct 25, 2022 at 01:00:09PM +0300, Ido Schimmel wrote: > > From: "Hans J. Schultz" <netdev@kapio-technology.com> > > > > Hosts that support 802.1X authentication are able to authenticate > > themselves by exchanging EAPOL frames with an authenticator (Ethernet > > bridge, in this case) and an authentication server. Access to the > > network is only granted by the authenticator to successfully > > authenticated hosts. > > > > The above is implemented in the bridge using the "locked" bridge port > > option. When enabled, link-local frames (e.g., EAPOL) can be locally > > received by the bridge, but all other frames are dropped unless the host > > is authenticated. That is, unless the user space control plane installed > > an FDB entry according to which the source address of the frame is > > located behind the locked ingress port. The entry can be dynamic, in > > which case learning needs to be enabled so that the entry will be > > refreshed by incoming traffic. > > > > There are deployments in which not all the devices connected to the > > authenticator (the bridge) support 802.1X. Such devices can include > > printers and cameras. One option to support such deployments is to > > unlock the bridge ports connecting these devices, but a slightly more > > secure option is to use MAB. When MAB is enabled, the MAC address of the > > connected device is used as the user name and password for the > > authentication. > > > > For MAB to work, the user space control plane needs to be notified about > > MAC addresses that are trying to gain access so that they will be > > compared against an allow list. This can be implemented via the regular > > learning process with the following differences: > > > > 1. Learned FDB entries are installed with a new "locked" flag indicating > > that the entry cannot be used to authenticate the device. The flag > > cannot be set by user space, but user space can clear the flag by > > replacing the entry, thereby authenticating the device. > > > > 2. FDB entries cannot roam to locked ports to prevent unauthenticated > > devices from disrupting traffic destined to already authenticated > > devices. > > The behavior described in (2) has nothing to do with locked FDB entries > or MAB (what is described in this paragraph), it applies to all of them, > no? The code was already there: > > if (p->flags & BR_PORT_LOCKED) > if (!fdb_src || READ_ONCE(fdb_src->dst) != p || > test_bit(BR_FDB_LOCAL, &fdb_src->flags)) > goto drop; > > I think you mean to say: the above already holds true, but the relevant > implication here is that locked FDB entries will not be created if the > MAC address is present in the FDB on any other port? Yes, will reword to make this clearer. > > I think some part of this comment should also go to the convoluted > BR_PORT_LOCKED block from br_handle_frame_finish()? Sure, will add a comment. > > I was going to ask if we should bother to add code to prohibit packets > from being forwarded to an FDB entry that was learned as LOCKED, since > that FDB entry is more of a "ghost" and not something fully committed? > > But with the "never roam to locked port" policy, I don't think there is > any practical risk that the extra code would mitigate. Assume that a > "snooper" wants to get the traffic destined for a MAC DA X, so it creates > a LOCKED FDB entry. It has to time itself just right, 5 minutes after > the station it wants to intercept has gone silent (or before the station > said anything). Anyone thinking it's talking to X now talks to the snooper. > But this attack vector is bounded in time. As long as X says anything, > the LOCKED FDB entry moves towards X, and the LOCKED flag gets cleared. I think it is best if we keep the semantic of the "locked" flag as the "entry cannot be used to authenticate the device" and let the MAB user space daemon worry about the rest. For example, if a MAC address that is not in the allow list appears behind a locked port, user space can decide to shutdown the port or install a flower filter that drops packets destined to this MAC DA. > > > > > Enable this behavior using a new bridge port option called "mab". It can > > only be enabled on a bridge port that is both locked and has learning > > enabled. A new option is added because there are pure 802.1X deployments > > that are not interested in notifications about "locked" FDB entries. > > > > Signed-off-by: Hans J. Schultz <netdev@kapio-technology.com> > > Signed-off-by: Ido Schimmel <idosch@nvidia.com> > > --- > > > > Notes: > > Changes made by me: > > > > * Reword commit message. > > * Reword comment regarding 'NTF_EXT_LOCKED'. > > * Use extack in br_fdb_add(). > > * Forbid MAB when learning is disabled. > > Forbidding MAB when learning is disabled makes sense to me, since it > means accepting that MAB is a form of learning (as the implementation > also shows; all other callers of br_fdb_update() are guarded by a > port learning check). I believe this will also make life easier with > offloading drivers. Thanks. > > > include/linux/if_bridge.h | 1 + > > include/uapi/linux/if_link.h | 1 + > > include/uapi/linux/neighbour.h | 8 +++++++- > > net/bridge/br_fdb.c | 24 ++++++++++++++++++++++++ > > net/bridge/br_input.c | 15 +++++++++++++-- > > net/bridge/br_netlink.c | 13 ++++++++++++- > > net/bridge/br_private.h | 3 ++- > > net/core/rtnetlink.c | 5 +++++ > > 8 files changed, 65 insertions(+), 5 deletions(-) > > > > diff --git a/include/linux/if_bridge.h b/include/linux/if_bridge.h > > index d62ef428e3aa..1668ac4d7adc 100644 > > --- a/include/linux/if_bridge.h > > +++ b/include/linux/if_bridge.h > > @@ -59,6 +59,7 @@ struct br_ip_list { > > #define BR_MRP_LOST_IN_CONT BIT(19) > > #define BR_TX_FWD_OFFLOAD BIT(20) > > #define BR_PORT_LOCKED BIT(21) > > +#define BR_PORT_MAB BIT(22) > > Question about unsetting BR_PORT_MAB using IFLA_BRPORT_MAB: should this > operation flush BR_FDB_LOCKED entries on the port? Good point. Will try to do that using br_fdb_flush() and add a test case. > > > diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c > > index 68b3e850bcb9..068fced7693c 100644 > > --- a/net/bridge/br_input.c > > +++ b/net/bridge/br_input.c > > @@ -109,9 +109,20 @@ int br_handle_frame_finish(struct net *net, struct sock *sk, struct sk_buff *skb > > struct net_bridge_fdb_entry *fdb_src = > > br_fdb_find_rcu(br, eth_hdr(skb)->h_source, vid); > > > > - if (!fdb_src || READ_ONCE(fdb_src->dst) != p || > > - test_bit(BR_FDB_LOCAL, &fdb_src->flags)) > > + if (!fdb_src) { > > + unsigned long flags = 0; > > + > > + if (p->flags & BR_PORT_MAB) { > > + __set_bit(BR_FDB_LOCKED, &flags); > > + br_fdb_update(br, p, eth_hdr(skb)->h_source, > > + vid, flags); > > + } > > goto drop; > > + } else if (READ_ONCE(fdb_src->dst) != p || > > + test_bit(BR_FDB_LOCAL, &fdb_src->flags) || > > + test_bit(BR_FDB_LOCKED, &fdb_src->flags)) { > > Minor nitpick: shouldn't br_fdb_update() also be called when the packet > matched on a BR_FDB_LOCKED entry on port p, so as to refresh it, if the > station is persistent? Currently I believe the FDB entry will expire > within 5 minutes of the first packet, regardless of subsequent traffic. Makes sense. Will add. Thanks! > > > + goto drop; > > + } > > } > > > > nbp_switchdev_frame_mark(p, skb);
On Fri, Oct 28, 2022 at 09:45:52AM +0200, netdev@kapio-technology.com wrote: > On 2022-10-28 00:58, Vladimir Oltean wrote: > > > I was going to ask if we should bother to add code to prohibit packets > > from being forwarded to an FDB entry that was learned as LOCKED, since > > that FDB entry is more of a "ghost" and not something fully committed? > > I think that it is a security flaw if there is any forwarding to > BR_FDB_LOCKED > entries. I can imagine a host behind a locked port with no credentials, > that gets a BR_FDB_LOCKED entry and has a friend on another non-locked port > who can now communicate uni-directional to the host with the BR_FDB_LOCKED > entry. It should not be too hard to create a scheme using UDP packets or > other for that. User space knows that the MAC is not authorized (otherwise it would have cleared the "locked" flag) and can choose to mitigate this corner case (or not) by shutting down the port, installing flower filters or doing something else entirely. I think it is best to defer such policy decisions to user space instead of overloading the "locked" flag with more meaning which will likely result in more checks in the fast path for a corner case of a use case that is quite obscure to begin with.
On 2022-10-25 12:00, Ido Schimmel wrote: > diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c > index 5aeb3646e74c..bbc82c70b091 100644 > --- a/net/bridge/br_netlink.c > +++ b/net/bridge/br_netlink.c > @@ -188,6 +188,7 @@ static inline size_t br_port_info_size(void) > + nla_total_size(1) /* IFLA_BRPORT_NEIGH_SUPPRESS */ > + nla_total_size(1) /* IFLA_BRPORT_ISOLATED */ > + nla_total_size(1) /* IFLA_BRPORT_LOCKED */ > + + nla_total_size(1) /* IFLA_BRPORT_MAB */ > + nla_total_size(sizeof(struct ifla_bridge_id)) /* > IFLA_BRPORT_ROOT_ID */ > + nla_total_size(sizeof(struct ifla_bridge_id)) /* > IFLA_BRPORT_BRIDGE_ID */ > + nla_total_size(sizeof(u16)) /* IFLA_BRPORT_DESIGNATED_PORT */ > @@ -274,7 +275,8 @@ static int br_port_fill_attrs(struct sk_buff *skb, > nla_put_u8(skb, IFLA_BRPORT_MRP_IN_OPEN, > !!(p->flags & BR_MRP_LOST_IN_CONT)) || > nla_put_u8(skb, IFLA_BRPORT_ISOLATED, !!(p->flags & BR_ISOLATED)) > || > - nla_put_u8(skb, IFLA_BRPORT_LOCKED, !!(p->flags & > BR_PORT_LOCKED))) > + nla_put_u8(skb, IFLA_BRPORT_LOCKED, !!(p->flags & > BR_PORT_LOCKED)) || > + nla_put_u8(skb, IFLA_BRPORT_MAB, !!(p->flags & BR_PORT_MAB))) > return -EMSGSIZE; > > timerval = br_timer_value(&p->message_age_timer); > @@ -876,6 +878,7 @@ static const struct nla_policy > br_port_policy[IFLA_BRPORT_MAX + 1] = { > [IFLA_BRPORT_NEIGH_SUPPRESS] = { .type = NLA_U8 }, > [IFLA_BRPORT_ISOLATED] = { .type = NLA_U8 }, > [IFLA_BRPORT_LOCKED] = { .type = NLA_U8 }, > + [IFLA_BRPORT_MAB] = { .type = NLA_U8 }, > [IFLA_BRPORT_BACKUP_PORT] = { .type = NLA_U32 }, > [IFLA_BRPORT_MCAST_EHT_HOSTS_LIMIT] = { .type = NLA_U32 }, > }; > @@ -943,6 +946,14 @@ static int br_setport(struct net_bridge_port *p, > struct nlattr *tb[], > br_set_port_flag(p, tb, IFLA_BRPORT_NEIGH_SUPPRESS, > BR_NEIGH_SUPPRESS); > br_set_port_flag(p, tb, IFLA_BRPORT_ISOLATED, BR_ISOLATED); > br_set_port_flag(p, tb, IFLA_BRPORT_LOCKED, BR_PORT_LOCKED); > + br_set_port_flag(p, tb, IFLA_BRPORT_MAB, BR_PORT_MAB); > + > + if ((p->flags & BR_PORT_MAB) && > + (!(p->flags & BR_PORT_LOCKED) || !(p->flags & BR_LEARNING))) { > + NL_SET_ERR_MSG(extack, "MAB can only be enabled on a locked port > with learning enabled"); It's a bit odd to get this message when turning off learning on a port with MAB on, e.g.... # bridge link set dev a2 learning off Error: MAB can only be enabled on a locked port with learning enabled.
On Sun, Oct 30, 2022 at 11:09:31PM +0100, netdev@kapio-technology.com wrote: > On 2022-10-25 12:00, Ido Schimmel wrote: > > @@ -943,6 +946,14 @@ static int br_setport(struct net_bridge_port *p, > > struct nlattr *tb[], > > br_set_port_flag(p, tb, IFLA_BRPORT_NEIGH_SUPPRESS, > > BR_NEIGH_SUPPRESS); > > br_set_port_flag(p, tb, IFLA_BRPORT_ISOLATED, BR_ISOLATED); > > br_set_port_flag(p, tb, IFLA_BRPORT_LOCKED, BR_PORT_LOCKED); > > + br_set_port_flag(p, tb, IFLA_BRPORT_MAB, BR_PORT_MAB); > > + > > + if ((p->flags & BR_PORT_MAB) && > > + (!(p->flags & BR_PORT_LOCKED) || !(p->flags & BR_LEARNING))) { > > + NL_SET_ERR_MSG(extack, "MAB can only be enabled on a locked port > > with learning enabled"); > > It's a bit odd to get this message when turning off learning on a port with > MAB on, e.g.... > > # bridge link set dev a2 learning off > Error: MAB can only be enabled on a locked port with learning enabled. It's better if you suggest something else. How about: "Bridge port must be locked and have learning enabled when MAB is enabled" ?
On 2022-10-31 15:43, Ido Schimmel wrote: > On Sun, Oct 30, 2022 at 11:09:31PM +0100, netdev@kapio-technology.com > wrote: >> On 2022-10-25 12:00, Ido Schimmel wrote: >> > @@ -943,6 +946,14 @@ static int br_setport(struct net_bridge_port *p, >> > struct nlattr *tb[], >> > br_set_port_flag(p, tb, IFLA_BRPORT_NEIGH_SUPPRESS, >> > BR_NEIGH_SUPPRESS); >> > br_set_port_flag(p, tb, IFLA_BRPORT_ISOLATED, BR_ISOLATED); >> > br_set_port_flag(p, tb, IFLA_BRPORT_LOCKED, BR_PORT_LOCKED); >> > + br_set_port_flag(p, tb, IFLA_BRPORT_MAB, BR_PORT_MAB); >> > + >> > + if ((p->flags & BR_PORT_MAB) && >> > + (!(p->flags & BR_PORT_LOCKED) || !(p->flags & BR_LEARNING))) { >> > + NL_SET_ERR_MSG(extack, "MAB can only be enabled on a locked port >> > with learning enabled"); >> >> It's a bit odd to get this message when turning off learning on a port >> with >> MAB on, e.g.... >> >> # bridge link set dev a2 learning off >> Error: MAB can only be enabled on a locked port with learning enabled. > > It's better if you suggest something else. How about: > > "Bridge port must be locked and have learning enabled when MAB is > enabled" > > ? Yes, I think that is better in case it should not be split into more than one message. At least it is not bound to a specific action.
diff --git a/include/linux/if_bridge.h b/include/linux/if_bridge.h index d62ef428e3aa..1668ac4d7adc 100644 --- a/include/linux/if_bridge.h +++ b/include/linux/if_bridge.h @@ -59,6 +59,7 @@ struct br_ip_list { #define BR_MRP_LOST_IN_CONT BIT(19) #define BR_TX_FWD_OFFLOAD BIT(20) #define BR_PORT_LOCKED BIT(21) +#define BR_PORT_MAB BIT(22) #define BR_DEFAULT_AGEING_TIME (300 * HZ) diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h index 5e7a1041df3a..d92b3f79eba3 100644 --- a/include/uapi/linux/if_link.h +++ b/include/uapi/linux/if_link.h @@ -561,6 +561,7 @@ enum { IFLA_BRPORT_MCAST_EHT_HOSTS_LIMIT, IFLA_BRPORT_MCAST_EHT_HOSTS_CNT, IFLA_BRPORT_LOCKED, + IFLA_BRPORT_MAB, __IFLA_BRPORT_MAX }; #define IFLA_BRPORT_MAX (__IFLA_BRPORT_MAX - 1) diff --git a/include/uapi/linux/neighbour.h b/include/uapi/linux/neighbour.h index a998bf761635..5e67a7eaf4a7 100644 --- a/include/uapi/linux/neighbour.h +++ b/include/uapi/linux/neighbour.h @@ -52,7 +52,8 @@ enum { #define NTF_STICKY (1 << 6) #define NTF_ROUTER (1 << 7) /* Extended flags under NDA_FLAGS_EXT: */ -#define NTF_EXT_MANAGED (1 << 0) +#define NTF_EXT_MANAGED (1 << 0) +#define NTF_EXT_LOCKED (1 << 1) /* * Neighbor Cache Entry States. @@ -86,6 +87,11 @@ enum { * NTF_EXT_MANAGED flagged neigbor entries are managed by the kernel on behalf * of a user space control plane, and automatically refreshed so that (if * possible) they remain in NUD_REACHABLE state. + * + * NTF_EXT_LOCKED flagged bridge FDB entries are entries generated by the + * bridge in response to a host trying to communicate via a locked bridge port + * with MAB enabled. Their purpose is to notify user space that a host requires + * authentication. */ struct nda_cacheinfo { diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c index e7f4fccb6adb..3b83af4458b8 100644 --- a/net/bridge/br_fdb.c +++ b/net/bridge/br_fdb.c @@ -105,6 +105,7 @@ static int fdb_fill_info(struct sk_buff *skb, const struct net_bridge *br, struct nda_cacheinfo ci; struct nlmsghdr *nlh; struct ndmsg *ndm; + u32 ext_flags = 0; nlh = nlmsg_put(skb, portid, seq, type, sizeof(*ndm), flags); if (nlh == NULL) @@ -125,11 +126,16 @@ static int fdb_fill_info(struct sk_buff *skb, const struct net_bridge *br, ndm->ndm_flags |= NTF_EXT_LEARNED; if (test_bit(BR_FDB_STICKY, &fdb->flags)) ndm->ndm_flags |= NTF_STICKY; + if (test_bit(BR_FDB_LOCKED, &fdb->flags)) + ext_flags |= NTF_EXT_LOCKED; if (nla_put(skb, NDA_LLADDR, ETH_ALEN, &fdb->key.addr)) goto nla_put_failure; if (nla_put_u32(skb, NDA_MASTER, br->dev->ifindex)) goto nla_put_failure; + if (nla_put_u32(skb, NDA_FLAGS_EXT, ext_flags)) + goto nla_put_failure; + ci.ndm_used = jiffies_to_clock_t(now - fdb->used); ci.ndm_confirmed = 0; ci.ndm_updated = jiffies_to_clock_t(now - fdb->updated); @@ -171,6 +177,7 @@ static inline size_t fdb_nlmsg_size(void) return NLMSG_ALIGN(sizeof(struct ndmsg)) + nla_total_size(ETH_ALEN) /* NDA_LLADDR */ + nla_total_size(sizeof(u32)) /* NDA_MASTER */ + + nla_total_size(sizeof(u32)) /* NDA_FLAGS_EXT */ + nla_total_size(sizeof(u16)) /* NDA_VLAN */ + nla_total_size(sizeof(struct nda_cacheinfo)) + nla_total_size(0) /* NDA_FDB_EXT_ATTRS */ @@ -879,6 +886,11 @@ void br_fdb_update(struct net_bridge *br, struct net_bridge_port *source, &fdb->flags))) clear_bit(BR_FDB_ADDED_BY_EXT_LEARN, &fdb->flags); + /* Clear locked flag when roaming to an + * unlocked port. + */ + if (unlikely(test_bit(BR_FDB_LOCKED, &fdb->flags))) + clear_bit(BR_FDB_LOCKED, &fdb->flags); } if (unlikely(test_bit(BR_FDB_ADDED_BY_USER, &flags))) @@ -1082,6 +1094,9 @@ static int fdb_add_entry(struct net_bridge *br, struct net_bridge_port *source, modified = true; } + if (test_and_clear_bit(BR_FDB_LOCKED, &fdb->flags)) + modified = true; + if (fdb_handle_notify(fdb, notify)) modified = true; @@ -1150,6 +1165,7 @@ int br_fdb_add(struct ndmsg *ndm, struct nlattr *tb[], struct net_bridge_port *p = NULL; struct net_bridge_vlan *v; struct net_bridge *br = NULL; + u32 ext_flags = 0; int err = 0; trace_br_fdb_add(ndm, dev, addr, vid, nlh_flags); @@ -1178,6 +1194,14 @@ int br_fdb_add(struct ndmsg *ndm, struct nlattr *tb[], vg = nbp_vlan_group(p); } + if (tb[NDA_FLAGS_EXT]) + ext_flags = nla_get_u32(tb[NDA_FLAGS_EXT]); + + if (ext_flags & NTF_EXT_LOCKED) { + NL_SET_ERR_MSG_MOD(extack, "Cannot add FDB entry with \"locked\" flag set"); + return -EINVAL; + } + if (tb[NDA_FDB_EXT_ATTRS]) { attr = tb[NDA_FDB_EXT_ATTRS]; err = nla_parse_nested(nfea_tb, NFEA_MAX, attr, diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c index 68b3e850bcb9..068fced7693c 100644 --- a/net/bridge/br_input.c +++ b/net/bridge/br_input.c @@ -109,9 +109,20 @@ int br_handle_frame_finish(struct net *net, struct sock *sk, struct sk_buff *skb struct net_bridge_fdb_entry *fdb_src = br_fdb_find_rcu(br, eth_hdr(skb)->h_source, vid); - if (!fdb_src || READ_ONCE(fdb_src->dst) != p || - test_bit(BR_FDB_LOCAL, &fdb_src->flags)) + if (!fdb_src) { + unsigned long flags = 0; + + if (p->flags & BR_PORT_MAB) { + __set_bit(BR_FDB_LOCKED, &flags); + br_fdb_update(br, p, eth_hdr(skb)->h_source, + vid, flags); + } goto drop; + } else if (READ_ONCE(fdb_src->dst) != p || + test_bit(BR_FDB_LOCAL, &fdb_src->flags) || + test_bit(BR_FDB_LOCKED, &fdb_src->flags)) { + goto drop; + } } nbp_switchdev_frame_mark(p, skb); diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c index 5aeb3646e74c..bbc82c70b091 100644 --- a/net/bridge/br_netlink.c +++ b/net/bridge/br_netlink.c @@ -188,6 +188,7 @@ static inline size_t br_port_info_size(void) + nla_total_size(1) /* IFLA_BRPORT_NEIGH_SUPPRESS */ + nla_total_size(1) /* IFLA_BRPORT_ISOLATED */ + nla_total_size(1) /* IFLA_BRPORT_LOCKED */ + + nla_total_size(1) /* IFLA_BRPORT_MAB */ + nla_total_size(sizeof(struct ifla_bridge_id)) /* IFLA_BRPORT_ROOT_ID */ + nla_total_size(sizeof(struct ifla_bridge_id)) /* IFLA_BRPORT_BRIDGE_ID */ + nla_total_size(sizeof(u16)) /* IFLA_BRPORT_DESIGNATED_PORT */ @@ -274,7 +275,8 @@ static int br_port_fill_attrs(struct sk_buff *skb, nla_put_u8(skb, IFLA_BRPORT_MRP_IN_OPEN, !!(p->flags & BR_MRP_LOST_IN_CONT)) || nla_put_u8(skb, IFLA_BRPORT_ISOLATED, !!(p->flags & BR_ISOLATED)) || - nla_put_u8(skb, IFLA_BRPORT_LOCKED, !!(p->flags & BR_PORT_LOCKED))) + nla_put_u8(skb, IFLA_BRPORT_LOCKED, !!(p->flags & BR_PORT_LOCKED)) || + nla_put_u8(skb, IFLA_BRPORT_MAB, !!(p->flags & BR_PORT_MAB))) return -EMSGSIZE; timerval = br_timer_value(&p->message_age_timer); @@ -876,6 +878,7 @@ static const struct nla_policy br_port_policy[IFLA_BRPORT_MAX + 1] = { [IFLA_BRPORT_NEIGH_SUPPRESS] = { .type = NLA_U8 }, [IFLA_BRPORT_ISOLATED] = { .type = NLA_U8 }, [IFLA_BRPORT_LOCKED] = { .type = NLA_U8 }, + [IFLA_BRPORT_MAB] = { .type = NLA_U8 }, [IFLA_BRPORT_BACKUP_PORT] = { .type = NLA_U32 }, [IFLA_BRPORT_MCAST_EHT_HOSTS_LIMIT] = { .type = NLA_U32 }, }; @@ -943,6 +946,14 @@ static int br_setport(struct net_bridge_port *p, struct nlattr *tb[], br_set_port_flag(p, tb, IFLA_BRPORT_NEIGH_SUPPRESS, BR_NEIGH_SUPPRESS); br_set_port_flag(p, tb, IFLA_BRPORT_ISOLATED, BR_ISOLATED); br_set_port_flag(p, tb, IFLA_BRPORT_LOCKED, BR_PORT_LOCKED); + br_set_port_flag(p, tb, IFLA_BRPORT_MAB, BR_PORT_MAB); + + if ((p->flags & BR_PORT_MAB) && + (!(p->flags & BR_PORT_LOCKED) || !(p->flags & BR_LEARNING))) { + NL_SET_ERR_MSG(extack, "MAB can only be enabled on a locked port with learning enabled"); + p->flags = old_flags; + return -EINVAL; + } changed_mask = old_flags ^ p->flags; diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h index 06e5f6faa431..4ce8b8e5ae0b 100644 --- a/net/bridge/br_private.h +++ b/net/bridge/br_private.h @@ -251,7 +251,8 @@ enum { BR_FDB_ADDED_BY_EXT_LEARN, BR_FDB_OFFLOADED, BR_FDB_NOTIFY, - BR_FDB_NOTIFY_INACTIVE + BR_FDB_NOTIFY_INACTIVE, + BR_FDB_LOCKED, }; struct net_bridge_fdb_key { diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c index 74864dc46a7e..d6e4d2854edb 100644 --- a/net/core/rtnetlink.c +++ b/net/core/rtnetlink.c @@ -4045,6 +4045,11 @@ int ndo_dflt_fdb_add(struct ndmsg *ndm, return err; } + if (tb[NDA_FLAGS_EXT]) { + netdev_info(dev, "invalid flags given to default FDB implementation\n"); + return err; + } + if (vid) { netdev_info(dev, "vlans aren't supported yet for dev_uc|mc_add()\n"); return err;