[PATCH net-next 0/2] bridge: Add MAC Authentication Bypass (MAB) support

[PATCH net-next 0/2] bridge: Add MAC Authentication Bypass (MAB) support

[Ido Schimmel]

Patch #1 adds MAB support in the bridge driver. See the commit message
for motivation, design choices and implementation details.

Patch #2 adds corresponding test cases.

Follow-up patchsets will add offload support in mlxsw and mv88e6xxx.

Hans J. Schultz (2):
  bridge: Add MAC Authentication Bypass (MAB) support
  selftests: forwarding: Add MAC Authentication Bypass (MAB) test cases

 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                         |  21 ++-
 net/bridge/br_netlink.c                       |  21 ++-
 net/bridge/br_private.h                       |   3 +-
 net/core/rtnetlink.c                          |   5 +
 .../net/forwarding/bridge_locked_port.sh      | 155 +++++++++++++++++-
 tools/testing/selftests/net/forwarding/lib.sh |   8 +
 10 files changed, 241 insertions(+), 6 deletions(-)

-- 
2.37.3

[PATCH net-next 1/2] bridge: Add MAC Authentication Bypass (MAB) support

[Ido Schimmel]

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 sole difference that 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.

Locked FDB entries implement the following semantics with regards to
roaming, aging and forwarding:

1. Roaming: Locked FDB entries can roam to unlocked (authorized) ports,
   in which case the "locked" flag is cleared. FDB entries cannot roam
   to locked ports regardless of MAB being enabled or not. Therefore,
   locked FDB entries are only created if an FDB entry with the given {MAC,
   VID} does not already exist. This behavior prevents unauthenticated
   devices from disrupting traffic destined to already authenticated
   devices.

2. Aging: Locked FDB entries age and refresh by incoming traffic like
   regular entries.

3. Forwarding: Locked FDB entries forward traffic like regular entries.
   If user space detects an unauthorized MAC behind a locked port and
   wishes to prevent traffic with this MAC DA from reaching the host, it
   can do so using tc or a different mechanism.

Enable the above 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. Locked FDB entries are flushed from the port once MAB
is disabled. 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:
    v1:
    * Extend commit message.
    * Adjust extack message.
    * Flush locked FDB entries when MAB is disabled.
    * Refresh locked FDB entries.
    * Add comments in br_handle_frame_finish().
    
    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          | 21 +++++++++++++++++++--
 net/bridge/br_netlink.c        | 21 ++++++++++++++++++++-
 net/bridge/br_private.h        |  3 ++-
 net/core/rtnetlink.c           |  5 +++++
 8 files changed, 79 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)
 
 #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..d04d2205ad4e 100644
--- a/net/bridge/br_input.c
+++ b/net/bridge/br_input.c
@@ -109,9 +109,26 @@ 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) {
+			/* 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);
diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
index 5aeb3646e74c..722fcfb857fc 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,22 @@ 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, "Bridge port must be locked and have learning enabled when MAB is enabled");
+		p->flags = old_flags;
+		return -EINVAL;
+	} else if (!(p->flags & BR_PORT_MAB) && (old_flags & BR_PORT_MAB)) {
+		struct net_bridge_fdb_flush_desc desc = {
+			.flags = BIT(BR_FDB_LOCKED),
+			.flags_mask = BIT(BR_FDB_LOCKED),
+			.port_ifindex = p->dev->ifindex,
+		};
+
+		br_fdb_flush(p->br, &desc);
+	}
 
 	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 d2f27548fc0b..b64fffeb3844 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -4051,6 +4051,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;
-- 
2.37.3

Re: [PATCH net-next 1/2] bridge: Add MAC Authentication Bypass (MAB) support

[Nikolay Aleksandrov]

On 01/11/2022 21:39, 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 sole difference that 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.
> 
> Locked FDB entries implement the following semantics with regards to
> roaming, aging and forwarding:
> 
> 1. Roaming: Locked FDB entries can roam to unlocked (authorized) ports,
>    in which case the "locked" flag is cleared. FDB entries cannot roam
>    to locked ports regardless of MAB being enabled or not. Therefore,
>    locked FDB entries are only created if an FDB entry with the given {MAC,
>    VID} does not already exist. This behavior prevents unauthenticated
>    devices from disrupting traffic destined to already authenticated
>    devices.
> 
> 2. Aging: Locked FDB entries age and refresh by incoming traffic like
>    regular entries.
> 
> 3. Forwarding: Locked FDB entries forward traffic like regular entries.
>    If user space detects an unauthorized MAC behind a locked port and
>    wishes to prevent traffic with this MAC DA from reaching the host, it
>    can do so using tc or a different mechanism.
> 
> Enable the above 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. Locked FDB entries are flushed from the port once MAB
> is disabled. 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:
>     v1:
>     * Extend commit message.
>     * Adjust extack message.
>     * Flush locked FDB entries when MAB is disabled.
>     * Refresh locked FDB entries.
>     * Add comments in br_handle_frame_finish().
>     
>     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          | 21 +++++++++++++++++++--
>  net/bridge/br_netlink.c        | 21 ++++++++++++++++++++-
>  net/bridge/br_private.h        |  3 ++-
>  net/core/rtnetlink.c           |  5 +++++
>  8 files changed, 79 insertions(+), 5 deletions(-)
> 

Acked-by: Nikolay Aleksandrov <razor@blackwall.org>

Re: [PATCH net-next 1/2] bridge: Add MAC Authentication Bypass (MAB) support

[Vladimir Oltean]

On Tue, Nov 01, 2022 at 09:39:21PM +0200, 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 sole difference that 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.
> 
> Locked FDB entries implement the following semantics with regards to
> roaming, aging and forwarding:
> 
> 1. Roaming: Locked FDB entries can roam to unlocked (authorized) ports,
>    in which case the "locked" flag is cleared. FDB entries cannot roam
>    to locked ports regardless of MAB being enabled or not. Therefore,
>    locked FDB entries are only created if an FDB entry with the given {MAC,
>    VID} does not already exist. This behavior prevents unauthenticated
>    devices from disrupting traffic destined to already authenticated
>    devices.
> 
> 2. Aging: Locked FDB entries age and refresh by incoming traffic like
>    regular entries.
> 
> 3. Forwarding: Locked FDB entries forward traffic like regular entries.
>    If user space detects an unauthorized MAC behind a locked port and
>    wishes to prevent traffic with this MAC DA from reaching the host, it
>    can do so using tc or a different mechanism.

In other words, a user space MAB daemon has a lot of extra work to do.
I'm willing to bet it's going to cut 90% of those corners ;) anyway...

> 
> Enable the above 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. Locked FDB entries are flushed from the port once MAB
> is disabled. 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>
> ---

Reviewed-by: Vladimir Oltean <vladimir.oltean@nxp.com>

Re: [PATCH net-next 1/2] bridge: Add MAC Authentication Bypass (MAB) support

[netdev]

On 2022-11-04 00:18, Vladimir Oltean wrote:
> On Tue, Nov 01, 2022 at 09:39:21PM +0200, 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 sole difference that 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.
>> 
>> Locked FDB entries implement the following semantics with regards to
>> roaming, aging and forwarding:
>> 
>> 1. Roaming: Locked FDB entries can roam to unlocked (authorized) 
>> ports,
>>    in which case the "locked" flag is cleared. FDB entries cannot roam
>>    to locked ports regardless of MAB being enabled or not. Therefore,
>>    locked FDB entries are only created if an FDB entry with the given 
>> {MAC,
>>    VID} does not already exist. This behavior prevents unauthenticated
>>    devices from disrupting traffic destined to already authenticated
>>    devices.
>> 
>> 2. Aging: Locked FDB entries age and refresh by incoming traffic like
>>    regular entries.
>> 
>> 3. Forwarding: Locked FDB entries forward traffic like regular 
>> entries.
>>    If user space detects an unauthorized MAC behind a locked port and
>>    wishes to prevent traffic with this MAC DA from reaching the host, 
>> it
>>    can do so using tc or a different mechanism.
> 
> In other words, a user space MAB daemon has a lot of extra work to do.
> I'm willing to bet it's going to cut 90% of those corners ;) anyway...
> 

I would like to know your (Vladimir) take on the approach of the
implementation for the mv88e6xxx that I have made and which will also be
the basis for how the WesterMo hostapd fork will be afaik...

Is it in general a good idea to use TC filters for specific MACs instead
of having the driver installing blocking entries, which I know the 
Marvell
XCat switchcore will also have (switchcore installed blockig entries)?


>> 
>> Enable the above 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. Locked FDB entries are flushed from the port once 
>> MAB
>> is disabled. 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>
>> ---
> 
> Reviewed-by: Vladimir Oltean <vladimir.oltean@nxp.com>

Re: [PATCH net-next 1/2] bridge: Add MAC Authentication Bypass (MAB) support

[Vladimir Oltean]

On Fri, Nov 04, 2022 at 12:23:07PM +0100, netdev@kapio-technology.com wrote:
> On 2022-11-04 00:18, Vladimir Oltean wrote:
> > > 3. Forwarding: Locked FDB entries forward traffic like regular entries.
> > >    If user space detects an unauthorized MAC behind a locked port and
> > >    wishes to prevent traffic with this MAC DA from reaching the host, it
> > >    can do so using tc or a different mechanism.
> > 
> > In other words, a user space MAB daemon has a lot of extra work to do.
> > I'm willing to bet it's going to cut 90% of those corners ;) anyway...
> 
> I would like to know your (Vladimir) take on the approach of the
> implementation for the mv88e6xxx that I have made and which will also be
> the basis for how the WesterMo hostapd fork will be afaik...
> 
> Is it in general a good idea to use TC filters for specific MACs instead
> of having the driver installing blocking entries, which I know the Marvell
> XCat switchcore will also have (switchcore installed blockig entries)?

Well, the mv88e6xxx driver does not offload tc filters in general, so
let's keep that in mind.

Achieving the behavior of not forwarding traffic to a BR_FDB_LOCKED
entry can be done in a variety of ways using tc. Simplest would be to
put an "action drop" filter on the egress chain of the port where the
BR_FDB_LOCKED entry is located. Although that's probably least amenable
to offloading. I think "action drop" is more popular as an offload
action on ingress chains, which means you'd either have to (a) put an
"action drop" on the ingress chain of every other bridge port, or
(b) create a shared tc block and put all bridge ports in that. The
problem with (b) is that it doesn't play all that well with bridge ports
belonging to different hardware blocks.

All in all, I think the yet-to-be-introduced 'blackhole' FDB flag makes
the most sense for this behavior. Its scope is the entire bridge
forwarding domain by definition (no need to attach it as filter to the
egress or ingress block of one/multiple bridge ports), and it's also
easily offloadable.

I think it could make a lot of sense for the MAB daemon to do one of 2
things: replace the BR_FDB_LOCKED entry with a static/dynamic FDB entry
if it's going to authorize it, or with a blackhole entry on br0 if it's
going to deny it. So you wouldn't have to manually add the blackhole
entry from the mv88e6xxx driver; user space would do it.

[PATCH net-next 2/2] selftests: forwarding: Add MAC Authentication Bypass (MAB) test cases

[Ido Schimmel]

From: "Hans J. Schultz" <netdev@kapio-technology.com>

Add four test cases to verify MAB functionality:

* Verify that a locked FDB entry can be generated by the bridge,
  preventing a host from communicating via the bridge. Test that user
  space can clear the "locked" flag by replacing the entry, thereby
  authenticating the host and allowing it to communicate via the bridge.

* Test that an entry cannot roam to a locked port, but that it can roam
  to an unlocked port.

* Test that MAB can only be enabled on a port that is both locked and
  has learning enabled.

* Test that locked FDB entries are flushed from a port when MAB is
  disabled.

Signed-off-by: Hans J. Schultz <netdev@kapio-technology.com>
Signed-off-by: Ido Schimmel <idosch@nvidia.com>
---

Notes:
    v1:
    * Adjust commit message.
    * Add FDB flushing test case.
    
    Changes made by me:
    * Reword commit message.
    * Remove blackhole tests as they are not relevant for this series.
    * Add configuration test case.
    * Add a few additional test cases.

 .../net/forwarding/bridge_locked_port.sh      | 155 +++++++++++++++++-
 tools/testing/selftests/net/forwarding/lib.sh |   8 +
 2 files changed, 162 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/net/forwarding/bridge_locked_port.sh b/tools/testing/selftests/net/forwarding/bridge_locked_port.sh
index 5b02b6b60ce7..dc92d32464f6 100755
--- a/tools/testing/selftests/net/forwarding/bridge_locked_port.sh
+++ b/tools/testing/selftests/net/forwarding/bridge_locked_port.sh
@@ -1,7 +1,16 @@
 #!/bin/bash
 # SPDX-License-Identifier: GPL-2.0
 
-ALL_TESTS="locked_port_ipv4 locked_port_ipv6 locked_port_vlan"
+ALL_TESTS="
+	locked_port_ipv4
+	locked_port_ipv6
+	locked_port_vlan
+	locked_port_mab
+	locked_port_mab_roam
+	locked_port_mab_config
+	locked_port_mab_flush
+"
+
 NUM_NETIFS=4
 CHECK_TC="no"
 source lib.sh
@@ -166,6 +175,150 @@ locked_port_ipv6()
 	log_test "Locked port ipv6"
 }
 
+locked_port_mab()
+{
+	RET=0
+	check_port_mab_support || return 0
+
+	ping_do $h1 192.0.2.2
+	check_err $? "Ping did not work before locking port"
+
+	bridge link set dev $swp1 learning on locked on
+
+	ping_do $h1 192.0.2.2
+	check_fail $? "Ping worked on a locked port without an FDB entry"
+
+	bridge fdb get `mac_get $h1` br br0 vlan 1 &> /dev/null
+	check_fail $? "FDB entry created before enabling MAB"
+
+	bridge link set dev $swp1 learning on locked on mab on
+
+	ping_do $h1 192.0.2.2
+	check_fail $? "Ping worked on MAB enabled port without an FDB entry"
+
+	bridge fdb get `mac_get $h1` br br0 vlan 1 | grep "dev $swp1" | grep -q "locked"
+	check_err $? "Locked FDB entry not created"
+
+	bridge fdb replace `mac_get $h1` dev $swp1 master static
+
+	ping_do $h1 192.0.2.2
+	check_err $? "Ping did not work after replacing FDB entry"
+
+	bridge fdb get `mac_get $h1` br br0 vlan 1 | grep "dev $swp1" | grep -q "locked"
+	check_fail $? "FDB entry marked as locked after replacement"
+
+	bridge fdb del `mac_get $h1` dev $swp1 master
+	bridge link set dev $swp1 learning off locked off mab off
+
+	log_test "Locked port MAB"
+}
+
+# Check that entries cannot roam to a locked port, but that entries can roam
+# to an unlocked port.
+locked_port_mab_roam()
+{
+	local mac=a0:b0:c0:c0:b0:a0
+
+	RET=0
+	check_port_mab_support || return 0
+
+	bridge link set dev $swp1 learning on locked on mab on
+
+	$MZ $h1 -q -c 5 -d 100msec -t udp -a $mac -b rand
+	bridge fdb get $mac br br0 vlan 1 | grep "dev $swp1" | grep -q "locked"
+	check_err $? "No locked entry on first injection"
+
+	$MZ $h2 -q -c 5 -d 100msec -t udp -a $mac -b rand
+	bridge fdb get $mac br br0 vlan 1 | grep -q "dev $swp2"
+	check_err $? "Entry did not roam to an unlocked port"
+
+	bridge fdb get $mac br br0 vlan 1 | grep -q "locked"
+	check_fail $? "Entry roamed with locked flag on"
+
+	$MZ $h1 -q -c 5 -d 100msec -t udp -a $mac -b rand
+	bridge fdb get $mac br br0 vlan 1 | grep -q "dev $swp1"
+	check_fail $? "Entry roamed back to locked port"
+
+	bridge fdb del $mac vlan 1 dev $swp2 master
+	bridge link set dev $swp1 learning off locked off mab off
+
+	log_test "Locked port MAB roam"
+}
+
+# Check that MAB can only be enabled on a port that is both locked and has
+# learning enabled.
+locked_port_mab_config()
+{
+	RET=0
+	check_port_mab_support || return 0
+
+	bridge link set dev $swp1 learning on locked off mab on &> /dev/null
+	check_fail $? "MAB enabled while port is unlocked"
+
+	bridge link set dev $swp1 learning off locked on mab on &> /dev/null
+	check_fail $? "MAB enabled while port has learning disabled"
+
+	bridge link set dev $swp1 learning on locked on mab on
+	check_err $? "Failed to enable MAB when port is locked and has learning enabled"
+
+	bridge link set dev $swp1 learning off locked off mab off
+
+	log_test "Locked port MAB configuration"
+}
+
+# Check that locked FDB entries are flushed from a port when MAB is disabled.
+locked_port_mab_flush()
+{
+	local locked_mac1=00:01:02:03:04:05
+	local unlocked_mac1=00:01:02:03:04:06
+	local locked_mac2=00:01:02:03:04:07
+	local unlocked_mac2=00:01:02:03:04:08
+
+	RET=0
+	check_port_mab_support || return 0
+
+	bridge link set dev $swp1 learning on locked on mab on
+	bridge link set dev $swp2 learning on locked on mab on
+
+	# Create regular and locked FDB entries on each port.
+	bridge fdb add $unlocked_mac1 dev $swp1 vlan 1 master static
+	bridge fdb add $unlocked_mac2 dev $swp2 vlan 1 master static
+
+	$MZ $h1 -q -c 5 -d 100msec -t udp -a $locked_mac1 -b rand
+	bridge fdb get $locked_mac1 br br0 vlan 1 | grep "dev $swp1" | \
+		grep -q "locked"
+	check_err $? "Failed to create locked FDB entry on first port"
+
+	$MZ $h2 -q -c 5 -d 100msec -t udp -a $locked_mac2 -b rand
+	bridge fdb get $locked_mac2 br br0 vlan 1 | grep "dev $swp2" | \
+		grep -q "locked"
+	check_err $? "Failed to create locked FDB entry on second port"
+
+	# Disable MAB on the first port and check that only the first locked
+	# FDB entry was flushed.
+	bridge link set dev $swp1 mab off
+
+	bridge fdb get $unlocked_mac1 br br0 vlan 1 &> /dev/null
+	check_err $? "Regular FDB entry on first port was flushed after disabling MAB"
+
+	bridge fdb get $unlocked_mac2 br br0 vlan 1 &> /dev/null
+	check_err $? "Regular FDB entry on second port was flushed after disabling MAB"
+
+	bridge fdb get $locked_mac1 br br0 vlan 1 &> /dev/null
+	check_fail $? "Locked FDB entry on first port was not flushed after disabling MAB"
+
+	bridge fdb get $locked_mac2 br br0 vlan 1 &> /dev/null
+	check_err $? "Locked FDB entry on second port was flushed after disabling MAB"
+
+	bridge fdb del $unlocked_mac2 dev $swp2 vlan 1 master static
+	bridge fdb del $unlocked_mac1 dev $swp1 vlan 1 master static
+
+	bridge link set dev $swp2 learning on locked off mab off
+	bridge link set dev $swp1 learning off locked off mab off
+
+	log_test "Locked port MAB FDB flush"
+}
+
 trap cleanup EXIT
 
 setup_prepare
diff --git a/tools/testing/selftests/net/forwarding/lib.sh b/tools/testing/selftests/net/forwarding/lib.sh
index 3ffb9d6c0950..1c4f866de7d7 100755
--- a/tools/testing/selftests/net/forwarding/lib.sh
+++ b/tools/testing/selftests/net/forwarding/lib.sh
@@ -137,6 +137,14 @@ check_locked_port_support()
 	fi
 }
 
+check_port_mab_support()
+{
+	if ! bridge -d link show | grep -q "mab"; then
+		echo "SKIP: iproute2 too old; MacAuth feature not supported."
+		return $ksft_skip
+	fi
+}
+
 if [[ "$(id -u)" -ne 0 ]]; then
 	echo "SKIP: need root privileges"
 	exit $ksft_skip
-- 
2.37.3

Re: [PATCH net-next 2/2] selftests: forwarding: Add MAC Authentication Bypass (MAB) test cases

[Nikolay Aleksandrov]

On 01/11/2022 21:39, Ido Schimmel wrote:
> From: "Hans J. Schultz" <netdev@kapio-technology.com>
> 
> Add four test cases to verify MAB functionality:
> 
> * Verify that a locked FDB entry can be generated by the bridge,
>   preventing a host from communicating via the bridge. Test that user
>   space can clear the "locked" flag by replacing the entry, thereby
>   authenticating the host and allowing it to communicate via the bridge.
> 
> * Test that an entry cannot roam to a locked port, but that it can roam
>   to an unlocked port.
> 
> * Test that MAB can only be enabled on a port that is both locked and
>   has learning enabled.
> 
> * Test that locked FDB entries are flushed from a port when MAB is
>   disabled.
> 
> Signed-off-by: Hans J. Schultz <netdev@kapio-technology.com>
> Signed-off-by: Ido Schimmel <idosch@nvidia.com>
> ---
> 
> Notes:
>     v1:
>     * Adjust commit message.
>     * Add FDB flushing test case.
>     
>     Changes made by me:
>     * Reword commit message.
>     * Remove blackhole tests as they are not relevant for this series.
>     * Add configuration test case.
>     * Add a few additional test cases.
> 
>  .../net/forwarding/bridge_locked_port.sh      | 155 +++++++++++++++++-
>  tools/testing/selftests/net/forwarding/lib.sh |   8 +
>  2 files changed, 162 insertions(+), 1 deletion(-)
> 

Acked-by: Nikolay Aleksandrov <razor@blackwall.org>

Re: [PATCH net-next 2/2] selftests: forwarding: Add MAC Authentication Bypass (MAB) test cases

[Vladimir Oltean]

On Tue, Nov 01, 2022 at 09:39:22PM +0200, Ido Schimmel wrote:
> From: "Hans J. Schultz" <netdev@kapio-technology.com>
> 
> Add four test cases to verify MAB functionality:
> 
> * Verify that a locked FDB entry can be generated by the bridge,
>   preventing a host from communicating via the bridge. Test that user
>   space can clear the "locked" flag by replacing the entry, thereby
>   authenticating the host and allowing it to communicate via the bridge.
> 
> * Test that an entry cannot roam to a locked port, but that it can roam
>   to an unlocked port.
> 
> * Test that MAB can only be enabled on a port that is both locked and
>   has learning enabled.
> 
> * Test that locked FDB entries are flushed from a port when MAB is
>   disabled.
> 
> Signed-off-by: Hans J. Schultz <netdev@kapio-technology.com>
> Signed-off-by: Ido Schimmel <idosch@nvidia.com>
> ---

Reviewed-by: Vladimir Oltean <vladimir.oltean@nxp.com>

Re: [PATCH net-next 0/2] bridge: Add MAC Authentication Bypass (MAB) support

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net-next.git (master)
by Jakub Kicinski <kuba@kernel.org>:

On Tue,  1 Nov 2022 21:39:20 +0200 you wrote:
> Patch #1 adds MAB support in the bridge driver. See the commit message
> for motivation, design choices and implementation details.
> 
> Patch #2 adds corresponding test cases.
> 
> Follow-up patchsets will add offload support in mlxsw and mv88e6xxx.
> 
> [...]

Here is the summary with links:
  - [net-next,1/2] bridge: Add MAC Authentication Bypass (MAB) support
    https://git.kernel.org/netdev/net-next/c/a35ec8e38cdd
  - [net-next,2/2] selftests: forwarding: Add MAC Authentication Bypass (MAB) test cases
    https://git.kernel.org/netdev/net-next/c/4a331d346996

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html