Commit 317cfe29 authored by Stephen Hemminger's avatar Stephen Hemminger

[BRIDGE]: Ethernet bridge fixes.

	1. STP protocol has no security, so malcontents can fuck with the
	  bridge's topology.  The fixes are to ship with STP turned off
	  to protect the ignorant, and run STP packets through ebtables
	  netfilter for the smart.  

	  Got this one via a russian hacker "Oleg K. Artemjev" <olli@rbauto.ru>
	  before he published the paper.
	  Bridge netfilter still needs work to give a nice face on this
	  but this patch gives the hooks to filter.

	2. STP input processing was lax in it's length checking so I bet
	  you could make up a bomb packet.

	  My inspection while doing #1.

	3. Forwarding table could be abused by sending forged packets with
	   bogus source address same as the local host.  This came via
	   Lennart from Jerry Kreuscher <jerrykr@mindspring.com> who ran into
	   it by mistake.
parent fed61014
...@@ -251,8 +251,21 @@ void br_fdb_insert(struct net_bridge *br, struct net_bridge_port *source, ...@@ -251,8 +251,21 @@ void br_fdb_insert(struct net_bridge *br, struct net_bridge_port *source,
write_lock_bh(&br->hash_lock); write_lock_bh(&br->hash_lock);
hlist_for_each(h, &br->hash[hash]) { hlist_for_each(h, &br->hash[hash]) {
fdb = hlist_entry(h, struct net_bridge_fdb_entry, hlist); fdb = hlist_entry(h, struct net_bridge_fdb_entry, hlist);
if (!fdb->is_local && if (!memcmp(fdb->addr.addr, addr, ETH_ALEN)) {
!memcmp(fdb->addr.addr, addr, ETH_ALEN)) { /* attempt to update an entry for a local interface */
if (unlikely(fdb->is_local)) {
if (is_local)
printk(KERN_INFO "%s: attempt to add"
" interface with same source address.\n",
source->dev->name);
else if (net_ratelimit())
printk(KERN_WARNING "%s: received packet with "
" own address as source address\n",
source->dev->name);
goto out;
}
if (likely(!fdb->is_static || is_local)) { if (likely(!fdb->is_static || is_local)) {
/* move to end of age list */ /* move to end of age list */
list_del(&fdb->age_list); list_del(&fdb->age_list);
......
...@@ -95,7 +95,7 @@ static struct net_bridge *new_nb(const char *name) ...@@ -95,7 +95,7 @@ static struct net_bridge *new_nb(const char *name)
br->bridge_id.prio[1] = 0x00; br->bridge_id.prio[1] = 0x00;
memset(br->bridge_id.addr, 0, ETH_ALEN); memset(br->bridge_id.addr, 0, ETH_ALEN);
br->stp_enabled = 1; br->stp_enabled = 0;
br->designated_root = br->bridge_id; br->designated_root = br->bridge_id;
br->root_path_cost = 0; br->root_path_cost = 0;
br->root_port = 0; br->root_port = 0;
......
...@@ -129,12 +129,15 @@ int br_handle_frame(struct sk_buff *skb) ...@@ -129,12 +129,15 @@ int br_handle_frame(struct sk_buff *skb)
if (p->br->stp_enabled && if (p->br->stp_enabled &&
!memcmp(dest, bridge_ula, 5) && !memcmp(dest, bridge_ula, 5) &&
!(dest[5] & 0xF0)) { !(dest[5] & 0xF0)) {
if (!dest[5]) if (!dest[5]) {
br_stp_handle_bpdu(skb); NF_HOOK(PF_BRIDGE, NF_BR_LOCAL_IN, skb, skb->dev,
goto err; NULL, br_stp_handle_bpdu);
rcu_read_unlock();
return 0;
}
} }
if (p->state == BR_STATE_FORWARDING) { else if (p->state == BR_STATE_FORWARDING) {
if (br_should_route_hook && br_should_route_hook(&skb)) { if (br_should_route_hook && br_should_route_hook(&skb)) {
rcu_read_unlock(); rcu_read_unlock();
return -1; return -1;
......
...@@ -206,7 +206,7 @@ extern void br_stp_set_path_cost(struct net_bridge_port *p, ...@@ -206,7 +206,7 @@ extern void br_stp_set_path_cost(struct net_bridge_port *p,
int path_cost); int path_cost);
/* br_stp_bpdu.c */ /* br_stp_bpdu.c */
extern void br_stp_handle_bpdu(struct sk_buff *skb); extern int br_stp_handle_bpdu(struct sk_buff *skb);
/* br_stp_timer.c */ /* br_stp_timer.c */
extern void br_stp_timer_init(struct net_bridge *br); extern void br_stp_timer_init(struct net_bridge *br);
......
...@@ -16,6 +16,7 @@ ...@@ -16,6 +16,7 @@
#include <linux/kernel.h> #include <linux/kernel.h>
#include <linux/if_ether.h> #include <linux/if_ether.h>
#include <linux/if_bridge.h> #include <linux/if_bridge.h>
#include <linux/netfilter_bridge.h>
#include "br_private.h" #include "br_private.h"
#include "br_private_stp.h" #include "br_private_stp.h"
...@@ -53,7 +54,8 @@ static void br_send_bpdu(struct net_bridge_port *p, unsigned char *data, int len ...@@ -53,7 +54,8 @@ static void br_send_bpdu(struct net_bridge_port *p, unsigned char *data, int len
memcpy(skb->nh.raw, data, length); memcpy(skb->nh.raw, data, length);
memset(skb->nh.raw + length, 0xa5, size - length - 2*ETH_ALEN - 2); memset(skb->nh.raw + length, 0xa5, size - length - 2*ETH_ALEN - 2);
dev_queue_xmit(skb); NF_HOOK(PF_BRIDGE, NF_BR_LOCAL_OUT, skb, NULL, skb->dev,
dev_queue_xmit);
} }
static __inline__ void br_set_ticks(unsigned char *dest, int jiff) static __inline__ void br_set_ticks(unsigned char *dest, int jiff)
...@@ -130,67 +132,75 @@ void br_send_tcn_bpdu(struct net_bridge_port *p) ...@@ -130,67 +132,75 @@ void br_send_tcn_bpdu(struct net_bridge_port *p)
br_send_bpdu(p, buf, 7); br_send_bpdu(p, buf, 7);
} }
static unsigned char header[6] = {0x42, 0x42, 0x03, 0x00, 0x00, 0x00}; static const unsigned char header[6] = {0x42, 0x42, 0x03, 0x00, 0x00, 0x00};
/* NO locks */ /* NO locks */
void br_stp_handle_bpdu(struct sk_buff *skb) int br_stp_handle_bpdu(struct sk_buff *skb)
{ {
struct net_bridge_port *p = skb->dev->br_port;
struct net_bridge *br = p->br;
unsigned char *buf; unsigned char *buf;
struct net_bridge_port *p;
struct net_bridge *br;
buf = skb->mac.raw + 14; /* need at least the 802 and STP headers */
p = skb->dev->br_port; if (!pskb_may_pull(skb, sizeof(header)+1) ||
br = p->br; memcmp(skb->data, header, sizeof(header)))
goto err;
buf = skb_pull(skb, sizeof(header));
spin_lock_bh(&br->lock); spin_lock_bh(&br->lock);
if (p->state == BR_STATE_DISABLED if (p->state == BR_STATE_DISABLED
|| !(br->dev->flags & IFF_UP) || !(br->dev->flags & IFF_UP)
|| !br->stp_enabled || !br->stp_enabled)
|| memcmp(buf, header, 6))
goto out; goto out;
if (buf[6] == BPDU_TYPE_CONFIG) { if (buf[0] == BPDU_TYPE_CONFIG) {
struct br_config_bpdu bpdu; struct br_config_bpdu bpdu;
bpdu.topology_change = (buf[7] & 0x01) ? 1 : 0; if (!pskb_may_pull(skb, 32))
bpdu.topology_change_ack = (buf[7] & 0x80) ? 1 : 0; goto out;
bpdu.root.prio[0] = buf[8];
bpdu.root.prio[1] = buf[9]; buf = skb->data;
bpdu.root.addr[0] = buf[10]; bpdu.topology_change = (buf[1] & 0x01) ? 1 : 0;
bpdu.root.addr[1] = buf[11]; bpdu.topology_change_ack = (buf[1] & 0x80) ? 1 : 0;
bpdu.root.addr[2] = buf[12];
bpdu.root.addr[3] = buf[13]; bpdu.root.prio[0] = buf[2];
bpdu.root.addr[4] = buf[14]; bpdu.root.prio[1] = buf[3];
bpdu.root.addr[5] = buf[15]; bpdu.root.addr[0] = buf[4];
bpdu.root.addr[1] = buf[5];
bpdu.root.addr[2] = buf[6];
bpdu.root.addr[3] = buf[7];
bpdu.root.addr[4] = buf[8];
bpdu.root.addr[5] = buf[9];
bpdu.root_path_cost = bpdu.root_path_cost =
(buf[16] << 24) | (buf[10] << 24) |
(buf[17] << 16) | (buf[11] << 16) |
(buf[18] << 8) | (buf[12] << 8) |
buf[19]; buf[13];
bpdu.bridge_id.prio[0] = buf[20]; bpdu.bridge_id.prio[0] = buf[14];
bpdu.bridge_id.prio[1] = buf[21]; bpdu.bridge_id.prio[1] = buf[15];
bpdu.bridge_id.addr[0] = buf[22]; bpdu.bridge_id.addr[0] = buf[16];
bpdu.bridge_id.addr[1] = buf[23]; bpdu.bridge_id.addr[1] = buf[17];
bpdu.bridge_id.addr[2] = buf[24]; bpdu.bridge_id.addr[2] = buf[18];
bpdu.bridge_id.addr[3] = buf[25]; bpdu.bridge_id.addr[3] = buf[19];
bpdu.bridge_id.addr[4] = buf[26]; bpdu.bridge_id.addr[4] = buf[20];
bpdu.bridge_id.addr[5] = buf[27]; bpdu.bridge_id.addr[5] = buf[21];
bpdu.port_id = (buf[28] << 8) | buf[29]; bpdu.port_id = (buf[22] << 8) | buf[23];
bpdu.message_age = br_get_ticks(buf+30); bpdu.message_age = br_get_ticks(buf+24);
bpdu.max_age = br_get_ticks(buf+32); bpdu.max_age = br_get_ticks(buf+26);
bpdu.hello_time = br_get_ticks(buf+34); bpdu.hello_time = br_get_ticks(buf+28);
bpdu.forward_delay = br_get_ticks(buf+36); bpdu.forward_delay = br_get_ticks(buf+30);
br_received_config_bpdu(p, &bpdu); br_received_config_bpdu(p, &bpdu);
goto out;
} }
if (buf[6] == BPDU_TYPE_TCN) { else if (buf[0] == BPDU_TYPE_TCN) {
br_received_tcn_bpdu(p); br_received_tcn_bpdu(p);
goto out;
} }
out: out:
spin_unlock_bh(&br->lock); spin_unlock_bh(&br->lock);
err:
kfree(skb);
return 0;
} }
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment