[RFC] bridging: don't forward EAPOL frames

[RFC] bridging: don't forward EAPOL frames

[Johannes Berg]

This patch makes the bridging code drop EAPOL frames as recommended by
802.1X-2004 in C.3.3.

Is this really the right place to put it?
---
 include/linux/if_ether.h |    1 +
 include/net/ieee80211.h  |    6 ------
 net/bridge/br_input.c    |    3 +++
 3 files changed, 4 insertions(+), 6 deletions(-)

--- everything.orig/include/linux/if_ether.h	2007-11-22 11:47:14.178686360 +0100
+++ everything/include/linux/if_ether.h	2007-11-22 11:48:21.438679036 +0100
@@ -74,6 +74,7 @@
 #define ETH_P_ATMFATE	0x8884		/* Frame-based ATM Transport
 					 * over Ethernet
 					 */
+#define ETH_P_PAE	0x888E		/* Port Access Entity (IEEE 802.1X) */
 #define ETH_P_AOE	0x88A2		/* ATA over Ethernet		*/
 #define ETH_P_TIPC	0x88CA		/* TIPC 			*/
 
--- everything.orig/include/net/ieee80211.h	2007-11-22 11:46:29.908682888 +0100
+++ everything/include/net/ieee80211.h	2007-11-22 11:48:51.908679037 +0100
@@ -183,12 +183,6 @@ const char *escape_essid(const char *ess
 #endif
 #include <net/iw_handler.h>	/* new driver API */
 
-#ifndef ETH_P_PAE
-#define ETH_P_PAE 0x888E	/* Port Access Entity (IEEE 802.1X) */
-#endif				/* ETH_P_PAE */
-
-#define ETH_P_PREAUTH 0x88C7	/* IEEE 802.11i pre-authentication */
-
 #ifndef ETH_P_80211_RAW
 #define ETH_P_80211_RAW (ETH_P_ECONET + 1)
 #endif
--- everything.orig/net/bridge/br_input.c	2007-11-22 11:54:44.798683106 +0100
+++ everything/net/bridge/br_input.c	2007-11-22 11:57:23.248680285 +0100
@@ -145,6 +145,9 @@ struct sk_buff *br_handle_frame(struct n
 		}
 	}
 
+	if (unlikely(skb->protocol = htons(ETH_P_PAE)))
+		goto drop;
+
 	switch (p->state) {
 	case BR_STATE_FORWARDING:
 

Re: [RFC] bridging: don't forward EAPOL frames

[Stephen Hemminger]

On Thu, 22 Nov 2007 14:23:28 +0100
Johannes Berg <johannes@sipsolutions.net> wrote:

> This patch makes the bridging code drop EAPOL frames as recommended by
> 802.1X-2004 in C.3.3.
> 
> Is this really the right place to put it?
> ---
>  include/linux/if_ether.h |    1 +
>  include/net/ieee80211.h  |    6 ------
>  net/bridge/br_input.c    |    3 +++
>  3 files changed, 4 insertions(+), 6 deletions(-)
> 
> --- everything.orig/include/linux/if_ether.h	2007-11-22 11:47:14.178686360 +0100
> +++ everything/include/linux/if_ether.h	2007-11-22 11:48:21.438679036 +0100
> @@ -74,6 +74,7 @@
>  #define ETH_P_ATMFATE	0x8884		/* Frame-based ATM Transport
>  					 * over Ethernet
>  					 */
> +#define ETH_P_PAE	0x888E		/* Port Access Entity (IEEE 802.1X) */
>  #define ETH_P_AOE	0x88A2		/* ATA over Ethernet		*/
>  #define ETH_P_TIPC	0x88CA		/* TIPC 			*/
>  
> --- everything.orig/include/net/ieee80211.h	2007-11-22 11:46:29.908682888 +0100
> +++ everything/include/net/ieee80211.h	2007-11-22 11:48:51.908679037 +0100
> @@ -183,12 +183,6 @@ const char *escape_essid(const char *ess
>  #endif
>  #include <net/iw_handler.h>	/* new driver API */
>  
> -#ifndef ETH_P_PAE
> -#define ETH_P_PAE 0x888E	/* Port Access Entity (IEEE 802.1X) */
> -#endif				/* ETH_P_PAE */
> -
> -#define ETH_P_PREAUTH 0x88C7	/* IEEE 802.11i pre-authentication */
> -
>  #ifndef ETH_P_80211_RAW
>  #define ETH_P_80211_RAW (ETH_P_ECONET + 1)
>  #endif
> --- everything.orig/net/bridge/br_input.c	2007-11-22 11:54:44.798683106 +0100
> +++ everything/net/bridge/br_input.c	2007-11-22 11:57:23.248680285 +0100
> @@ -145,6 +145,9 @@ struct sk_buff *br_handle_frame(struct n
>  		}
>  	}
>  
> +	if (unlikely(skb->protocol = htons(ETH_P_PAE)))
> +		goto drop;
> +
>  	switch (p->state) {
>  	case BR_STATE_FORWARDING:

Not needed because the bridge is already handling it:

1) If running STP (ie true bridge), then all link local multicast is only received by
the bridge and never forwarded.

2) If not running sTP (ie invisible bridge), then it will be forwarded.

Despite what the standards say, many users are using bridging code for invisible
firewalls etc, and in those cases they want STP and EAPOL frames to be forwarded.


-- 
Stephen Hemminger <shemminger@linux-foundation.org>

Re: [RFC] bridging: don't forward EAPOL frames

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 786 bytes --]


> > +	if (unlikely(skb->protocol = htons(ETH_P_PAE)))
> > +		goto drop;
> > +
> >  	switch (p->state) {
> >  	case BR_STATE_FORWARDING:
> 
> Not needed because the bridge is already handling it:
> 
> 1) If running STP (ie true bridge), then all link local multicast is only received by
> the bridge and never forwarded.

Well, typical access point setups bridge the wireless AP interface with
wired, EAPOL frames can be unicast (and 802.11 specifies to do so) and
we want to avoid having them unicast to another host.

Also, 802.1X in C.3.3 recommends not bridging the *ethertype* rather
than depending on the link-local multicast address because otherwise
eapol frames can be unicast into the network behind the (authorized)
port which is undesirable.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: [Bridge] Re: [RFC] bridging: don't forward EAPOL frames

[Andy Gospodarek]

On Nov 27, 2007 8:24 AM, Johannes Berg <johannes@sipsolutions.net> wrote:
>
> > > +   if (unlikely(skb->protocol = htons(ETH_P_PAE)))
> > > +           goto drop;
> > > +
> > >     switch (p->state) {
> > >     case BR_STATE_FORWARDING:
> >
> > Not needed because the bridge is already handling it:
> >
> > 1) If running STP (ie true bridge), then all link local multicast is only received by
> > the bridge and never forwarded.
>
> Well, typical access point setups bridge the wireless AP interface with
> wired, EAPOL frames can be unicast (and 802.11 specifies to do so) and
> we want to avoid having them unicast to another host.
>
> Also, 802.1X in C.3.3 recommends not bridging the *ethertype* rather
> than depending on the link-local multicast address because otherwise
> eapol frames can be unicast into the network behind the (authorized)
> port which is undesirable.
>
> johannes
>
>

I agree with Stephen, that based on the way it's likely people use
linux bridges it seems like this is something that could be configured
rather than simply dropping those frames without any chance to forward
them.  There are probably quite a few people out there who will not
expect this change, so it should be easy to change during runtime.

Don't forget that a simple ebtables rule could also drop EAPOL if needed.

Re: [Bridge] Re: [RFC] bridging: don't forward EAPOL frames

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 1691 bytes --]


> > > Not needed because the bridge is already handling it:
> > >
> > > 1) If running STP (ie true bridge), then all link local multicast is only received by
> > > the bridge and never forwarded.
> >
> > Well, typical access point setups bridge the wireless AP interface with
> > wired, EAPOL frames can be unicast (and 802.11 specifies to do so) and
> > we want to avoid having them unicast to another host.
> >
> > Also, 802.1X in C.3.3 recommends not bridging the *ethertype* rather
> > than depending on the link-local multicast address because otherwise
> > eapol frames can be unicast into the network behind the (authorized)
> > port which is undesirable.

> I agree with Stephen, that based on the way it's likely people use
> linux bridges it seems like this is something that could be configured
> rather than simply dropping those frames without any chance to forward
> them.

Well Stephen is wrong in one thing that eapol need not be link local
multicast for 802.11, it's unicast there so the dropping of link local
packets doesn't help.

> There are probably quite a few people out there who will not
> expect this change, so it should be easy to change during runtime.

I'm not aware of any use for EAPOL frames traversing the network. I'm
also not aware of any proper 802.1X implementation for linux bridges but
I didn't do too much research yet. I don't see why people would rely on
EAPOL frames being bridged when the protocol is by definition local to a
link.

> Don't forget that a simple ebtables rule could also drop EAPOL if needed.

Indeed, but I'd prefer the bridge to do the right thing in absence of
configuration.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]