Skip to contentSkip to conversation

New Bridge security features

PATCH In review Stephen Hemminger 2 participants 6 messages Nov 13, 2012 – Nov 15, 2012

Subsystems (3)
Networking Network drivers Ethernet bridge
lore
Share & tools
lore
author
#1 lore raw

PATCH 1/4 bridge: bridge port parameters over netlink

Expose bridge port parameter over netlink. By switching to a nested message, this can be used for other bridge parameters.

This changes IFLA_PROTINFO attribute from one byte to a full nested set of attributes. This is safe for application interface because the old message used IFLA_PROTINFO and new one uses
IFLA_PROTINFO | NLA_F_NESTED.

The code adapts to old format requests, and therefore stays compatible with user mode RSTP daemon. Since the type field for nested and unnested attributes are different, and the old code in libnetlink doesn't do the mask, it is also safe to use with old versions of bridge monitor command.

Note: although mode is only a boolean, treating it as a full byte since in the future someone will probably want to add more values (like macvlan has).

Signed-off-by Stephen Hemminger <[email protected]>

---
v2 - adapt to new bridge generic setlink infrastructure
fix cost/priority width in policy

 include/uapi/linux/if_link.h |   10 ++
 net/bridge/br_netlink.c      |  145 ++++++++++++++++++++++++++++++++-----------
 2 files changed, 119 insertions(+), 36 deletions(-)
include/uapi/linux/if_link.h +15 -0
@@ -205,6 +205,21 @@ enum {
 
 #define IFLA_INET6_MAX	(__IFLA_INET6_MAX - 1)
 
+enum {
+	BRIDGE_MODE_UNSPEC,
+	BRIDGE_MODE_HAIRPIN,
+};
+
+enum {
+	IFLA_BRPORT_UNSPEC,
+	IFLA_BRPORT_STATE,	/* Spanning tree state     */
+	IFLA_BRPORT_PRIORITY,	/* "             priority  */
+	IFLA_BRPORT_COST,	/* "             cost      */
+	IFLA_BRPORT_MODE,	/* mode (hairpin)          */
+	__IFLA_BRPORT_MAX
+};
+#define IFLA_BRPORT_MAX (__IFLA_BRPORT_MAX - 1)
+
 struct ifla_cacheinfo {
 	__u32	max_reasm_len;
 	__u32	tstamp;		/* ipv6InterfaceTable updated timestamp */
net/bridge/br_netlink.c +137 -38

212 lines not on this page · raw message

author
#2 lore raw

PATCH 2/4 bridge: add template for bridge port flags

In reply to Stephen Hemminger

Provide macro to build sysfs data structures and functions for accessing flag bits. If flag bits change do netlink notification.

Signed-off-by Stephen Hemminger <[email protected]>
net/bridge/br_sysfs_if.c +23 -15
@@ -34,6 +34,28 @@ const struct brport_attribute brport_att
 	.store	= _store,					\
 };
 
+#define BRPORT_ATTR_FLAG(_name, _mask)				\
+static ssize_t show_##_name(struct net_bridge_port *p, char *buf) \
+{								\
+	return sprintf(buf, "%d\n", !!(p->flags & _mask));	\
+}								\
+static int store_##_name(struct net_bridge_port *p, unsigned long v) \
+{								\
+	unsigned long flags = p->flags;				\
+	if (v)							\
+		flags |= _mask;					\
+	else							\
+		flags &= ~_mask;				\
+	if (flags != p->flags) {				\
+		p->flags = flags;				\
+		br_ifinfo_notify(RTM_NEWLINK, p);		\
+	}							\
+	return 0;						\
+}								\
+static BRPORT_ATTR(_name, S_IRUGO | S_IWUSR,			\
+		   show_##_name, store_##_name)
+
+
 static ssize_t show_path_cost(struct net_bridge_port *p, char *buf)
 {
 	return sprintf(buf, "%d\n", p->path_cost);

@@ -133,21 +155,7 @@ static int store_flush(struct net_bridge
 }
 static BRPORT_ATTR(flush, S_IWUSR, NULL, store_flush);
 
-static ssize_t show_hairpin_mode(struct net_bridge_port *p, char *buf)
-{
-	int hairpin_mode = (p->flags & BR_HAIRPIN_MODE) ? 1 : 0;
-	return sprintf(buf, "%d\n", hairpin_mode);
-}
-static int store_hairpin_mode(struct net_bridge_port *p, unsigned long v)
-{
-	if (v)
-		p->flags |= BR_HAIRPIN_MODE;
-	else
-		p->flags &= ~BR_HAIRPIN_MODE;
-	return 0;
-}
-static BRPORT_ATTR(hairpin_mode, S_IRUGO | S_IWUSR,
-		   show_hairpin_mode, store_hairpin_mode);
+BRPORT_ATTR_FLAG(hairpin_mode, BR_HAIRPIN_MODE);
 
 #ifdef CONFIG_BRIDGE_IGMP_SNOOPING
 static ssize_t show_multicast_router(struct net_bridge_port *p, char *buf)
author
#3 lore raw

PATCH 3/4 bridge: implement BPDU blocking

In reply to Stephen Hemminger

This is Linux bridge implementation of STP protection (Cisco BPDU guard/Juniper BPDU block). BPDU block disables the bridge port if a STP BPDU packet is received.

Why would you want to do this?
If running Spanning Tree on bridge, hostile devices on the network may send BPDU and cause network failure. Enabling bpdu block will detect and stop this.

How to recover the port?
The port will be restarted if link is brought down, or removed and reattached. For example:

 # ip li set dev eth0 down; ip li set dev eth0 up
Signed-off-by Stephen Hemminger <[email protected]>

---
v2 - refresh for earlier changes related to generic bridge setlink

 include/uapi/linux/if_link.h |    1 +
 net/bridge/br_netlink.c      |    7 ++++++-
 net/bridge/br_private.h      |    1 +
 net/bridge/br_stp_bpdu.c     |    7 +++++++
 net/bridge/br_sysfs_if.c     |    2 ++
 5 files changed, 17 insertions(+), 1 deletion(-)
net/bridge/br_private.h +1 -0
@@ -135,6 +135,7 @@ struct net_bridge_port
 
 	unsigned long 			flags;
 #define BR_HAIRPIN_MODE		0x00000001
+#define BR_BPDU_GUARD           0x00000002
 
 #ifdef CONFIG_BRIDGE_IGMP_SNOOPING
 	u32				multicast_startup_queries_sent;
net/bridge/br_stp_bpdu.c +7 -0
@@ -170,6 +170,13 @@ void br_stp_rcv(const struct stp_proto *
 	if (!ether_addr_equal(dest, br->group_addr))
 		goto out;
 
+	if (p->flags & BR_BPDU_GUARD) {
+		br_notice(br, "BPDU received on blocked port %u(%s)\n",
+			  (unsigned int) p->port_no, p->dev->name);
+		br_stp_disable_port(p);
+		goto out;
+	}
+
 	buf = skb_pull(skb, 3);
 
 	if (buf[0] == BPDU_TYPE_CONFIG) {
net/bridge/br_sysfs_if.c +2 -0
@@ -156,6 +156,7 @@ static int store_flush(struct net_bridge
 static BRPORT_ATTR(flush, S_IWUSR, NULL, store_flush);
 
 BRPORT_ATTR_FLAG(hairpin_mode, BR_HAIRPIN_MODE);
+BRPORT_ATTR_FLAG(bpdu_guard, BR_BPDU_GUARD);
 
 #ifdef CONFIG_BRIDGE_IGMP_SNOOPING
 static ssize_t show_multicast_router(struct net_bridge_port *p, char *buf)

@@ -189,6 +190,7 @@ static const struct brport_attribute *br
 	&brport_attr_hold_timer,
 	&brport_attr_flush,
 	&brport_attr_hairpin_mode,
+	&brport_attr_bpdu_guard,
 #ifdef CONFIG_BRIDGE_IGMP_SNOOPING
 	&brport_attr_multicast_router,
 #endif
include/uapi/linux/if_link.h +1 -0
@@ -216,6 +216,7 @@ enum {
 	IFLA_BRPORT_PRIORITY,	/* "             priority  */
 	IFLA_BRPORT_COST,	/* "             cost      */
 	IFLA_BRPORT_MODE,	/* mode (hairpin)          */
+	IFLA_BRPORT_GUARD,	/* bpdu guard              */
 	__IFLA_BRPORT_MAX
 };
 #define IFLA_BRPORT_MAX (__IFLA_BRPORT_MAX - 1)
net/bridge/br_netlink.c +5 -1
@@ -26,6 +26,7 @@ static inline size_t br_port_info_size(v
 		+ nla_total_size(2)	/* IFLA_BRPORT_PRIORITY */
 		+ nla_total_size(4)	/* IFLA_BRPORT_COST */
 		+ nla_total_size(1)	/* IFLA_BRPORT_MODE */
+		+ nla_total_size(1)	/* IFLA_BRPORT_GUARD */
 		+ 0;
 }
 

@@ -49,7 +50,8 @@ static int br_port_fill_attrs(struct sk_
 	if (nla_put_u8(skb, IFLA_BRPORT_STATE, p->state) ||
 	    nla_put_u16(skb, IFLA_BRPORT_PRIORITY, p->priority) ||
 	    nla_put_u32(skb, IFLA_BRPORT_COST, p->path_cost) ||
-	    nla_put_u8(skb, IFLA_BRPORT_MODE, mode))
+	    nla_put_u8(skb, IFLA_BRPORT_MODE, mode) ||
+	    nla_put_u8(skb, IFLA_BRPORT_GUARD, !!(p->flags & BR_BPDU_GUARD)))
 		return -EMSGSIZE;
 
 	return 0;

@@ -162,6 +164,7 @@ static const struct nla_policy ifla_brpo
 	[IFLA_BRPORT_COST]	= { .type = NLA_U32 },
 	[IFLA_BRPORT_PRIORITY]	= { .type = NLA_U16 },
 	[IFLA_BRPORT_MODE]	= { .type = NLA_U8 },
+	[IFLA_BRPORT_GUARD]	= { .type = NLA_U8 },
 };
 
 /* Change the state of the port and notify spanning tree */

@@ -203,6 +206,7 @@ static int br_setport(struct net_bridge_
 	int err;
 
 	br_set_port_flag(p, tb, IFLA_BRPORT_MODE, BR_HAIRPIN_MODE);
+	br_set_port_flag(p, tb, IFLA_BRPORT_GUARD, BR_BPDU_GUARD);
 
 	if (tb[IFLA_BRPORT_COST]) {
 		err = br_stp_set_path_cost(p, nla_get_u32(tb[IFLA_BRPORT_COST]));
author
#4 lore raw

PATCH 4/4 bridge: add root port blocking

In reply to Stephen Hemminger

This is Linux bridge implementation of root port guard. If BPDU is received from a leaf (edge) port, it should not be elected as root port.

Why would you want to do this?
If using STP on a bridge and the downstream bridges are not fully trusted; this prevents a hostile guest for rerouting traffic.

Why not just use netfilter?
Netfilter does not track of follow spanning tree decisions. It would be difficult and error prone to try and mirror STP resolution in netfilter module.

Signed-off-by Stephen Hemminger <[email protected]>
---
 include/uapi/linux/if_link.h |    1 +
 net/bridge/br_netlink.c      |    6 +++++-
 net/bridge/br_private.h      |    1 +
 net/bridge/br_stp.c          |   22 +++++++++++++++++++++-
 net/bridge/br_sysfs_if.c     |    2 ++
 5 files changed, 30 insertions(+), 2 deletions(-)
net/bridge/br_private.h +1 -0
@@ -136,6 +136,7 @@ struct net_bridge_port
 	unsigned long 			flags;
 #define BR_HAIRPIN_MODE		0x00000001
 #define BR_BPDU_GUARD           0x00000002
+#define BR_ROOT_BLOCK		0x00000004
 
 #ifdef CONFIG_BRIDGE_IGMP_SNOOPING
 	u32				multicast_startup_queries_sent;
net/bridge/br_stp.c +21 -1
@@ -100,6 +100,21 @@ static int br_should_become_root_port(co
 	return 0;
 }
 
+static void br_root_port_block(const struct net_bridge *br,
+			       struct net_bridge_port *p)
+{
+
+	br_notice(br, "port %u(%s) tried to become root port (blocked)",
+		  (unsigned int) p->port_no, p->dev->name);
+
+	p->state = BR_STATE_LISTENING;
+	br_log_state(p);
+	br_ifinfo_notify(RTM_NEWLINK, p);
+
+	if (br->forward_delay > 0)
+		mod_timer(&p->forward_delay_timer, jiffies + br->forward_delay);
+}
+
 /* called under bridge lock */
 static void br_root_selection(struct net_bridge *br)
 {

@@ -107,7 +122,12 @@ static void br_root_selection(struct net
 	u16 root_port = 0;
 
 	list_for_each_entry(p, &br->port_list, list) {
-		if (br_should_become_root_port(p, root_port))
+		if (!br_should_become_root_port(p, root_port))
+			continue;
+
+		if (p->flags & BR_ROOT_BLOCK)
+			br_root_port_block(br, p);
+		else
 			root_port = p->port_no;
 	}
net/bridge/br_sysfs_if.c +2 -0
@@ -157,6 +157,7 @@ static BRPORT_ATTR(flush, S_IWUSR, NULL,
 
 BRPORT_ATTR_FLAG(hairpin_mode, BR_HAIRPIN_MODE);
 BRPORT_ATTR_FLAG(bpdu_guard, BR_BPDU_GUARD);
+BRPORT_ATTR_FLAG(root_block, BR_ROOT_BLOCK);
 
 #ifdef CONFIG_BRIDGE_IGMP_SNOOPING
 static ssize_t show_multicast_router(struct net_bridge_port *p, char *buf)

@@ -191,6 +192,7 @@ static const struct brport_attribute *br
 	&brport_attr_flush,
 	&brport_attr_hairpin_mode,
 	&brport_attr_bpdu_guard,
+	&brport_attr_root_block,
 #ifdef CONFIG_BRIDGE_IGMP_SNOOPING
 	&brport_attr_multicast_router,
 #endif
include/uapi/linux/if_link.h +1 -0
@@ -217,6 +217,7 @@ enum {
 	IFLA_BRPORT_COST,	/* "             cost      */
 	IFLA_BRPORT_MODE,	/* mode (hairpin)          */
 	IFLA_BRPORT_GUARD,	/* bpdu guard              */
+	IFLA_BRPORT_PROTECT,	/* root port protection    */
 	__IFLA_BRPORT_MAX
 };
 #define IFLA_BRPORT_MAX (__IFLA_BRPORT_MAX - 1)
net/bridge/br_netlink.c +4 -1
@@ -27,6 +27,7 @@ static inline size_t br_port_info_size(v
 		+ nla_total_size(4)	/* IFLA_BRPORT_COST */
 		+ nla_total_size(1)	/* IFLA_BRPORT_MODE */
 		+ nla_total_size(1)	/* IFLA_BRPORT_GUARD */
+		+ nla_total_size(1)	/* IFLA_BRPORT_PROTECT */
 		+ 0;
 }
 

@@ -51,7 +52,8 @@ static int br_port_fill_attrs(struct sk_
 	    nla_put_u16(skb, IFLA_BRPORT_PRIORITY, p->priority) ||
 	    nla_put_u32(skb, IFLA_BRPORT_COST, p->path_cost) ||
 	    nla_put_u8(skb, IFLA_BRPORT_MODE, mode) ||
-	    nla_put_u8(skb, IFLA_BRPORT_GUARD, !!(p->flags & BR_BPDU_GUARD)))
+	    nla_put_u8(skb, IFLA_BRPORT_GUARD, !!(p->flags & BR_BPDU_GUARD)) ||
+	    nla_put_u8(skb, IFLA_BRPORT_PROTECT, !!(p->flags & BR_ROOT_BLOCK)))
 		return -EMSGSIZE;
 
 	return 0;

@@ -165,6 +167,7 @@ static const struct nla_policy ifla_brpo
 	[IFLA_BRPORT_PRIORITY]	= { .type = NLA_U16 },
 	[IFLA_BRPORT_MODE]	= { .type = NLA_U8 },
 	[IFLA_BRPORT_GUARD]	= { .type = NLA_U8 },
+	[IFLA_BRPORT_PROTECT]	= { .type = NLA_U8 },
 };
 
 /* Change the state of the port and notify spanning tree */