netdev - Re: [PATCH] bridge: Forward EAPOL Kconfig option BRIDGE_PAE

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 28 Jun 2011 20:58:12 +0200
From:	David Lamparter <equinox@...c24.net>
To:	Nick Carter <ncarter100@...il.com>
Cc:	David Lamparter <equinox@...c24.net>,
	Stephen Hemminger <shemminger@...ux-foundation.org>,
	netdev@...r.kernel.org, davem@...emloft.net
Subject: Re: [PATCH] bridge: Forward EAPOL Kconfig option BRIDGE_PAE_FORWARD

On Tue, Jun 28, 2011 at 07:34:57PM +0100, Nick Carter wrote:
> > Yes, the default is a dumb hub (IMHO correctly so). And a dumb hub will
> > forward 802.1X packets (IMHO also correctly so).
> >
> > Why should we specifically add a knob for EAPOL? Next we're adding one
> > for STP itself, then one for LLDP, then one for Cisco's deprecated
> > crap (CDP, DTP, ...) etc.
> >
> > If you want a dumb hub that drops EAPOL, use ebtables.
> >
> > -David
> >
> >
> If we are not going to have an EAPOL knob, but we are going to act as
> a repeater when STP is off then we still need these diffs to forward
> the PAE group address.
> (In fact we cant just act as a repeater because of the recent ethernet
> bonding regression)
> 
> diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c
> index 90e985b..267f581 100644
> --- a/net/bridge/br_input.c
> +++ b/net/bridge/br_input.c
> @@ -163,7 +163,8 @@ struct sk_buff *br_handle_frame(struct sk_buff *skb)
>                         goto drop;
> 
>                 /* If STP is turned off, then forward */
> -               if (p->br->stp_enabled == BR_NO_STP && dest[5] == 0)
> +               if (p->br->stp_enabled == BR_NO_STP &&
> +                       (dest[5] == 0 || skb->protocol == htons(ETH_P_PAE)))
>                         goto forward;
> Nick

That code actually looks quite wrong to me, we should be forwarding all of
the 01:80:C2:00:00:0x groups in non-STP mode, especially :0E and :0D.
(LLDP and GVRP/MVRP)

Pause frames are the one exception that makes the rule, but as the
comment a few lines above states, "Pause frames shouldn't be passed up by
driver anyway".

Btw, what might make sense is a general knob for forwarding those
link-local groups, split off from the STP switch so the STP switch
controls only the :00 (STP) group. That way you can decide separately
whether you want to be LLDP/GVRP/802.1X/... transparent and whether you
want to run STP.
Not sure if it's needed, it can always be done with ebtables...


-David

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html