[OpenWrt-Devel] [PATCH] [RFC] kernel: disable EAP local hack when using group_fwd_mask

[Etienne Champetier]

Hi Petr,

Le jeu. 1 août 2019 à 01:51, Petr Štetiar <ynezz at true.cz> a écrit :
>
> Etienne Champetier <champetier.etienne at gmail.com> [2019-07-26 19:23:02]:
>
> Hi,
>
> I've noticed your request for feedback on IRC.
Thanks for taking some time

>
> > and 640-bridge-only-accept-EAP-locally.patch hack is there to prevent
> > bridges from forwarding these EAP frames
>
> it would be nice to know if this patch is still needed so we could possibly
> remove[1] it from 4.19 kernel.

I revived this old email thread last week to try to get more
information from Felix
(http://lists.infradead.org/pipermail/openwrt-devel/2019-July/018127.html)
If the fix was to workaround a client bug we might want to keep it,
else maybe we could rework it and add a warn_once if this hack prevent
a EAP frame from being forwarded

>
> > -+    if (skb->protocol == htons(ETH_P_PAE))
> > ++    if (skb->protocol == htons(ETH_P_PAE) && !(br->group_fwd_mask & (1u << 3)))
> >  +            return br_pass_frame_up(skb);
>
> This usage of magic numbers is usually a warn sign to me, so I went ahead and
> read the surrounding code and it seems to me, that you probably wanted
> something like this instead:
>
>   u16 fwd_mask = p->br->group_fwd_mask_required;
>   fwd_mask |= p->br->group_fwd_mask;
>   const unsigned char *dest = eth_hdr(skb)->h_dest;
>
>   if (skb->protocol == htons(ETH_P_PAE) && !(fwd_mask & (1u << dest[5])))
>           return br_pass_frame_up(skb);

Actually no, let me try to explain it again
By default a bridge forward everything except 01-80-C2-00-00-XX, which
includes 01-80-C2-00-00-03, which is the multicast mac for EAP frames.
For wifi, EAP are not using multicast but unicast (it's what I
understood reading various email thread, I haven't double checked
that).

What I think an admin wants when he do
echo 8 > /sys/class/net/brX/bridge/group_fwd_mask
is not to allow 01-80-C2-00-00-03 forwarding, but allow all EAP forwarding.
Your patch would allow multicast EAP forwarding but not unicast EAP
forwarding, except if dest mac ends with 03,
which would be very surprising.

Another approach would be to add a setting like
'disable-eap-local-hack' to enable/disable this hack, what do you
prefer ?

Etienne


>
> 1. https://patchwork.ozlabs.org/patch/884518/
>
> -- ynezz

_______________________________________________
openwrt-devel mailing list
openwrt-devel at lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel