[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.

diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
index 853da564b321a4ebddd193a3aae3e621875f7746..b19be26d801280e16fbf5b1d1c395c6ad5017b36 100644 (file)
--- a/net/bridge/br_fdb.c
+++ b/net/bridge/br_fdb.c
@@ -251,8 +251,21 @@ void br_fdb_insert(struct net_bridge *br, struct net_bridge_port *source,
        write_lock_bh(&br->hash_lock);
        hlist_for_each(h, &br->hash[hash]) {
                fdb = hlist_entry(h, struct net_bridge_fdb_entry, hlist);
-               if (!fdb->is_local &&
-                   !memcmp(fdb->addr.addr, addr, ETH_ALEN)) {
+               if (!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)) {
                                /* move to end of age list */
                                list_del(&fdb->age_list);

diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
index 7d72f02a50a800ec062d1ecc4fb4b26ce4e75003..cbfa8653f21076c59e8a8842aad18ac08b9359e2 100644 (file)
--- a/net/bridge/br_if.c
+++ b/net/bridge/br_if.c
@@ -95,7 +95,7 @@ static struct net_bridge *new_nb(const char *name)
        br->bridge_id.prio[1] = 0x00;
        memset(br->bridge_id.addr, 0, ETH_ALEN);
 
-       br->stp_enabled = 1;
+       br->stp_enabled = 0;
        br->designated_root = br->bridge_id;
        br->root_path_cost = 0;
        br->root_port = 0;

diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c
index 1e8502adb49c9a332b23bf4648c1d3c370dd651c..483aa63e3092a279430793769dd9a533f429f15e 100644 (file)
--- a/net/bridge/br_input.c
+++ b/net/bridge/br_input.c
@@ -129,12 +129,15 @@ int br_handle_frame(struct sk_buff *skb)
        if (p->br->stp_enabled &&
            !memcmp(dest, bridge_ula, 5) &&
            !(dest[5] & 0xF0)) {
-               if (!dest[5]) 
-                       br_stp_handle_bpdu(skb);
-               goto err;
+               if (!dest[5]) {
+                       NF_HOOK(PF_BRIDGE, NF_BR_LOCAL_IN, skb, skb->dev, 
+                               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)) {
                        rcu_read_unlock();
                        return -1;

diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index ca814cfff2dba582e41d1f47ee77af28ea95f735..a355ec4453a146cc24168215a53e15f65265afc7 100644 (file)
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -206,7 +206,7 @@ extern void br_stp_set_path_cost(struct net_bridge_port *p,
                          int path_cost);
 
 /* 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 */
 extern void br_stp_timer_init(struct net_bridge *br);

diff --git a/net/bridge/br_stp_bpdu.c b/net/bridge/br_stp_bpdu.c
index b95fa3a880e26aa57b7c329167559f1036de1515..3d5c0c3eac58fd9870f589242ddbb268401f9553 100644 (file)
--- a/net/bridge/br_stp_bpdu.c
+++ b/net/bridge/br_stp_bpdu.c
@@ -16,6 +16,7 @@
 #include <linux/kernel.h>
 #include <linux/if_ether.h>
 #include <linux/if_bridge.h>
+#include <linux/netfilter_bridge.h>
 #include "br_private.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
        memcpy(skb->nh.raw, data, length);
        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)
@@ -130,67 +132,75 @@ void br_send_tcn_bpdu(struct net_bridge_port *p)
        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 */
-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;
-       struct net_bridge_port *p;
-       struct net_bridge *br;
 
-       buf = skb->mac.raw + 14;
-       p = skb->dev->br_port;
-       br = p->br;
+       /* need at least the 802 and STP headers */
+       if (!pskb_may_pull(skb, sizeof(header)+1) ||
+           memcmp(skb->data, header, sizeof(header)))
+               goto err;
+
+       buf = skb_pull(skb, sizeof(header));
 
        spin_lock_bh(&br->lock);
        if (p->state == BR_STATE_DISABLED 
            || !(br->dev->flags & IFF_UP)
-           || !br->stp_enabled 
-           || memcmp(buf, header, 6)) 
+           || !br->stp_enabled)
                goto out;
 
-       if (buf[6] == BPDU_TYPE_CONFIG) {
+       if (buf[0] == BPDU_TYPE_CONFIG) {
                struct br_config_bpdu bpdu;
 
-               bpdu.topology_change = (buf[7] & 0x01) ? 1 : 0;
-               bpdu.topology_change_ack = (buf[7] & 0x80) ? 1 : 0;
-               bpdu.root.prio[0] = buf[8];
-               bpdu.root.prio[1] = buf[9];
-               bpdu.root.addr[0] = buf[10];
-               bpdu.root.addr[1] = buf[11];
-               bpdu.root.addr[2] = buf[12];
-               bpdu.root.addr[3] = buf[13];
-               bpdu.root.addr[4] = buf[14];
-               bpdu.root.addr[5] = buf[15];
+               if (!pskb_may_pull(skb, 32))
+                   goto out;
+
+               buf = skb->data;
+               bpdu.topology_change = (buf[1] & 0x01) ? 1 : 0;
+               bpdu.topology_change_ack = (buf[1] & 0x80) ? 1 : 0;
+
+               bpdu.root.prio[0] = buf[2];
+               bpdu.root.prio[1] = buf[3];
+               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 =
-                       (buf[16] << 24) |
-                       (buf[17] << 16) |
-                       (buf[18] << 8) |
-                       buf[19];
-               bpdu.bridge_id.prio[0] = buf[20];
-               bpdu.bridge_id.prio[1] = buf[21];
-               bpdu.bridge_id.addr[0] = buf[22];
-               bpdu.bridge_id.addr[1] = buf[23];
-               bpdu.bridge_id.addr[2] = buf[24];
-               bpdu.bridge_id.addr[3] = buf[25];
-               bpdu.bridge_id.addr[4] = buf[26];
-               bpdu.bridge_id.addr[5] = buf[27];
-               bpdu.port_id = (buf[28] << 8) | buf[29];
-
-               bpdu.message_age = br_get_ticks(buf+30);
-               bpdu.max_age = br_get_ticks(buf+32);
-               bpdu.hello_time = br_get_ticks(buf+34);
-               bpdu.forward_delay = br_get_ticks(buf+36);
+                       (buf[10] << 24) |
+                       (buf[11] << 16) |
+                       (buf[12] << 8) |
+                       buf[13];
+               bpdu.bridge_id.prio[0] = buf[14];
+               bpdu.bridge_id.prio[1] = buf[15];
+               bpdu.bridge_id.addr[0] = buf[16];
+               bpdu.bridge_id.addr[1] = buf[17];
+               bpdu.bridge_id.addr[2] = buf[18];
+               bpdu.bridge_id.addr[3] = buf[19];
+               bpdu.bridge_id.addr[4] = buf[20];
+               bpdu.bridge_id.addr[5] = buf[21];
+               bpdu.port_id = (buf[22] << 8) | buf[23];
+
+               bpdu.message_age = br_get_ticks(buf+24);
+               bpdu.max_age = br_get_ticks(buf+26);
+               bpdu.hello_time = br_get_ticks(buf+28);
+               bpdu.forward_delay = br_get_ticks(buf+30);
 
                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);
-               goto out;
        }
  out:
        spin_unlock_bh(&br->lock);
+ err:
+       kfree(skb);
+       return 0;
 }