git: d43ac3b76ef7 - main - bridge: Make 802.1ad (Q-in-Q) configurable

From: Lexi Winter <ivy_at_FreeBSD.org>
Date: Tue, 05 Aug 2025 19:26:01 UTC
The branch main has been updated by ivy:

URL: https://cgit.FreeBSD.org/src/commit/?id=d43ac3b76ef7897c72dcc6153cb1c33363b7b024

commit d43ac3b76ef7897c72dcc6153cb1c33363b7b024
Author:     Lexi Winter <ivy@FreeBSD.org>
AuthorDate: 2025-08-05 17:43:31 +0000
Commit:     Lexi Winter <ivy@FreeBSD.org>
CommitDate: 2025-08-05 18:34:47 +0000

    bridge: Make 802.1ad (Q-in-Q) configurable
    
    Allowing tag stacking by default can permit VLAN-hopping attacks in
    certain configurations.  To mitigate this, disallow sending Q-in-Q
    frames by default unless the new "qinq" option is enabled on the
    interface.  The bridge flag "defqinq" can be used to restore the
    previous behaviour of allowing Q-in-Q on all interfaces.
    
    The bridge.4 changes from the differential are omitted here and
    will be landed via D51185.
    
    Reviewed by:            kevans, pauamma_gundo.com (manpages)
    Differential Revision:  https://reviews.freebsd.org/D51227
---
 sbin/ifconfig/ifbridge.c        | 18 ++++++++++++++++++
 sbin/ifconfig/ifconfig.8        | 20 +++++++++++++++++++-
 sys/net/if_bridge.c             | 24 ++++++++++++++++++++++++
 sys/net/if_bridgevar.h          |  8 ++++----
 tests/sys/net/if_bridge_test.sh | 36 +++++++++++++++++++++++-------------
 5 files changed, 88 insertions(+), 18 deletions(-)

diff --git a/sbin/ifconfig/ifbridge.c b/sbin/ifconfig/ifbridge.c
index 335ed9bf513f..3566acdcf54c 100644
--- a/sbin/ifconfig/ifbridge.c
+++ b/sbin/ifconfig/ifbridge.c
@@ -883,6 +883,18 @@ unsetbridge_defuntagged(if_ctx *ctx, const char *val __unused, int dummy __unuse
 		err(1, "BRDGSDEFPVID");
 }
 
+static void
+setbridge_qinq(if_ctx *ctx, const char *val, int dummy __unused)
+{
+	do_bridgeflag(ctx, val, IFBIF_QINQ, 1);
+}
+
+static void
+unsetbridge_qinq(if_ctx *ctx, const char *val, int dummy __unused)
+{
+	do_bridgeflag(ctx, val, IFBIF_QINQ, 0);
+}
+
 static struct cmd bridge_cmds[] = {
 	DEF_CMD_ARG("addm",		setbridge_add),
 	DEF_CMD_ARG("deletem",		setbridge_delete),
@@ -933,6 +945,12 @@ static struct cmd bridge_cmds[] = {
 					unsetbridge_flags),
 	DEF_CMD_ARG("defuntagged",	setbridge_defuntagged),
 	DEF_CMD("-defuntagged", 0,	unsetbridge_defuntagged),
+	DEF_CMD("defqinq", (int32_t)IFBRF_DEFQINQ,
+					setbridge_flags),
+	DEF_CMD("-defqinq", (int32_t)IFBRF_DEFQINQ,
+					unsetbridge_flags),
+	DEF_CMD_ARG("qinq",		setbridge_qinq),
+	DEF_CMD_ARG("-qinq",		unsetbridge_qinq),
 };
 
 static struct afswtch af_bridge = {
diff --git a/sbin/ifconfig/ifconfig.8 b/sbin/ifconfig/ifconfig.8
index 06ec62197fba..69a81f72421f 100644
--- a/sbin/ifconfig/ifconfig.8
+++ b/sbin/ifconfig/ifconfig.8
@@ -28,7 +28,7 @@
 .\" OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
 .\" SUCH DAMAGE.
 .\"
-.Dd July 30, 2025
+.Dd August 5, 2025
 .Dt IFCONFIG 8
 .Os
 .Sh NAME
@@ -2747,6 +2747,24 @@ Remove the provided list of VLAN IDs from the interface's VLAN access
 list.
 The list should be formatted as described for
 .Cm tagged .
+.It Cm qinq Ar interface
+Allow this interface to send 802.1ad
+.Dq Q-in-Q
+frames.
+.It Cm -qinq Ar interface
+Do not allow this interface to send 802.1ad
+.Dq Q-in-Q
+frames.
+This is the default behavior.
+.It Cm defqinq
+Enable the
+.Cm qinq
+option by default on newly added members.
+.It Cm -defqinq
+Do not enable the
+.Cm qinq
+option by default on newly added members.
+This is the default behavior.
 .El
 .Ss Link Aggregation and Link Failover Parameters
 The following parameters are specific to lagg interfaces:
diff --git a/sys/net/if_bridge.c b/sys/net/if_bridge.c
index 46e54339a171..26ea4400e67d 100644
--- a/sys/net/if_bridge.c
+++ b/sys/net/if_bridge.c
@@ -1496,6 +1496,8 @@ bridge_ioctl_add(struct bridge_softc *sc, void *arg)
 	bif->bif_savedcaps = ifs->if_capenable;
 	if (sc->sc_flags & IFBRF_VLANFILTER)
 		bif->bif_pvid = sc->sc_defpvid;
+	if (sc->sc_flags & IFBRF_DEFQINQ)
+		bif->bif_flags |= IFBIF_QINQ;
 
 	/*
 	 * Assign the interface's MAC address to the bridge if it's the first
@@ -2828,6 +2830,16 @@ bridge_input(struct ifnet *ifp, struct mbuf *m)
 
 	NET_EPOCH_ASSERT();
 
+	/* We need the Ethernet header later, so make sure we have it now. */
+	if (m->m_len < ETHER_HDR_LEN) {
+		m = m_pullup(m, ETHER_HDR_LEN);
+		if (m == NULL) {
+			if_inc_counter(sc->sc_ifp, IFCOUNTER_IERRORS, 1);
+			m_freem(m);
+			return (NULL);
+		}
+	}
+
 	eh = mtod(m, struct ether_header *);
 	vlan = VLANTAGOF(m);
 
@@ -3250,6 +3262,18 @@ bridge_vfilter_in(const struct bridge_iflist *sbif, struct mbuf *m)
 	if ((sbif->bif_sc->sc_flags & IFBRF_VLANFILTER) == 0)
 		return (true);
 
+	/* If Q-in-Q is disabled, check for stacked tags. */
+	if ((sbif->bif_flags & IFBIF_QINQ) == 0) {
+		struct ether_header *eh;
+		uint16_t proto;
+
+		eh = mtod(m, struct ether_header *);
+		proto = ntohs(eh->ether_type);
+
+		if (proto == ETHERTYPE_VLAN || proto == ETHERTYPE_QINQ)
+			return (false);
+	}
+
 	if (vlan == DOT1Q_VID_NULL) {
 		/*
 		 * The frame doesn't have a tag.  If the interface does not
diff --git a/sys/net/if_bridgevar.h b/sys/net/if_bridgevar.h
index 6718c5ebcc34..15194fecff7c 100644
--- a/sys/net/if_bridgevar.h
+++ b/sys/net/if_bridgevar.h
@@ -136,8 +136,9 @@
 typedef uint32_t ifbr_flags_t;
 
 #define	IFBRF_VLANFILTER	(1U<<0)	/* VLAN filtering enabled */
+#define	IFBRF_DEFQINQ		(1U<<1)	/* 802.1ad Q-in-Q allowed by default */
 
-#define	IFBRFBITS	"\020\01VLANFILTER"
+#define	IFBRFBITS	"\020\01VLANFILTER\02DEFQINQ"
 
 /*
  * Generic bridge control request.
@@ -172,12 +173,11 @@ struct ifbreq {
 #define	IFBIF_BSTP_ADMEDGE	0x0200	/* member stp admin edge enabled */
 #define	IFBIF_BSTP_ADMCOST	0x0400	/* member stp admin path cost */
 #define	IFBIF_PRIVATE		0x0800	/* if is a private segment */
-/* was	IFBIF_VLANFILTER	0x1000 */
-#define	IFBIF_QINQ		0x2000	/* if allows 802.1ad Q-in-Q */
+#define	IFBIF_QINQ		0x1000	/* if allows 802.1ad Q-in-Q */
 
 #define	IFBIFBITS	"\020\001LEARNING\002DISCOVER\003STP\004SPAN" \
 			"\005STICKY\014PRIVATE\006EDGE\007AUTOEDGE\010PTP" \
-			"\011AUTOPTP"
+			"\011AUTOPTP\015QINQ"
 #define	IFBIFMASK	~(IFBIF_BSTP_EDGE|IFBIF_BSTP_AUTOEDGE|IFBIF_BSTP_PTP| \
 			IFBIF_BSTP_AUTOPTP|IFBIF_BSTP_ADMEDGE| \
 			IFBIF_BSTP_ADMCOST)	/* not saved */
diff --git a/tests/sys/net/if_bridge_test.sh b/tests/sys/net/if_bridge_test.sh
index 534144f46632..8a6c730014ce 100755
--- a/tests/sys/net/if_bridge_test.sh
+++ b/tests/sys/net/if_bridge_test.sh
@@ -1268,21 +1268,25 @@ vlan_qinq_body()
 	# Create a QinQ trunk between the two jails.  The outer (provider) tag
 	# is 5, and the inner tag is 10.
 
-	jexec one ifconfig ${epone}b up
-	jexec one ifconfig ${epone}b.5 create vlanproto 802.1ad up
-	jexec one ifconfig ${epone}b.5.10 create inet 192.0.2.1/24 up
+	atf_check -s exit:0 jexec one ifconfig ${epone}b up
+	atf_check -s exit:0 jexec one \
+	    ifconfig ${epone}b.5 create vlanproto 802.1ad up
+	atf_check -s exit:0 jexec one \
+	    ifconfig ${epone}b.5.10 create inet 192.0.2.1/24 up
 
-	jexec two ifconfig ${eptwo}b up
-	jexec two ifconfig ${eptwo}b.5 create vlanproto 802.1ad up
-	jexec two ifconfig ${eptwo}b.5.10 create inet 192.0.2.2/24 up
+	atf_check -s exit:0 jexec two ifconfig ${eptwo}b up
+	atf_check -s exit:0 jexec two ifconfig \
+	    ${eptwo}b.5 create vlanproto 802.1ad up
+	atf_check -s exit:0 jexec two ifconfig \
+	    ${eptwo}b.5.10 create inet 192.0.2.2/24 up
 
 	bridge=$(vnet_mkbridge)
 
-	ifconfig ${bridge} up
-	ifconfig ${epone}a up
-	ifconfig ${eptwo}a up
-	ifconfig ${bridge} addm ${epone}a vlanfilter ${epone}a
-	ifconfig ${bridge} addm ${eptwo}a vlanfilter ${eptwo}a
+	atf_check -s exit:0 ifconfig ${bridge} vlanfilter defqinq up
+	atf_check -s exit:0 ifconfig ${epone}a up
+	atf_check -s exit:0 ifconfig ${eptwo}a up
+	atf_check -s exit:0 ifconfig ${bridge} addm ${epone}a
+	atf_check -s exit:0 ifconfig ${bridge} addm ${eptwo}a
 
 	# Right now there are no VLANs on the access list, so everything
 	# should be blocked.
@@ -1290,10 +1294,16 @@ vlan_qinq_body()
 	atf_check -s exit:2 -o ignore jexec two ping -c 3 -t 1 192.0.2.1
 
 	# Add the provider tag to the access list; now traffic should be passed.
-	ifconfig ${bridge} +tagged ${epone}a 5
-	ifconfig ${bridge} +tagged ${eptwo}a 5
+	atf_check -s exit:0 ifconfig ${bridge} +tagged ${epone}a 5
+	atf_check -s exit:0 ifconfig ${bridge} +tagged ${eptwo}a 5
 	atf_check -s exit:0 -o ignore jexec one ping -c 3 -t 1 192.0.2.2
 	atf_check -s exit:0 -o ignore jexec two ping -c 3 -t 1 192.0.2.1
+
+	# Remove the qinq flag from one of the interfaces; traffic should
+	# be blocked again.
+	atf_check -s exit:0 ifconfig ${bridge} -qinq ${epone}a
+	atf_check -s exit:2 -o ignore jexec one ping -c 3 -t 1 192.0.2.2
+	atf_check -s exit:2 -o ignore jexec two ping -c 3 -t 1 192.0.2.1
 }
 
 vlan_qinq_cleanup()