Message-ID: <20221022113238.beo5zhufl2x645lf@skbuf>
Date:   Sat, 22 Oct 2022 14:32:38 +0300
From:   Vladimir Oltean <olteanv@...il.com>
To:     Oleksandr Mazur <oleksandr.mazur@...ision.eu>
Cc:     "netdev@...io-technology.com" <netdev@...io-technology.com>,
        "davem@...emloft.net" <davem@...emloft.net>,
        "kuba@...nel.org" <kuba@...nel.org>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        Florian Fainelli <f.fainelli@...il.com>,
        Andrew Lunn <andrew@...n.ch>,
        Vivien Didelot <vivien.didelot@...il.com>,
        Eric Dumazet <edumazet@...gle.com>,
        Paolo Abeni <pabeni@...hat.com>,
        Kurt Kanzenbach <kurt@...utronix.de>,
        Hauke Mehrtens <hauke@...ke-m.de>,
        Woojung Huh <woojung.huh@...rochip.com>,
        "UNGLinuxDriver@...rochip.com" <UNGLinuxDriver@...rochip.com>,
        Sean Wang <sean.wang@...iatek.com>,
        Landen Chao <Landen.Chao@...iatek.com>,
        DENG Qingfang <dqfext@...il.com>,
        Matthias Brugger <matthias.bgg@...il.com>,
        Claudiu Manoil <claudiu.manoil@....com>,
        Alexandre Belloni <alexandre.belloni@...tlin.com>,
        Jiri Pirko <jiri@...nulli.us>,
        Ivan Vecera <ivecera@...hat.com>,
        Roopa Prabhu <roopa@...dia.com>,
        Nikolay Aleksandrov <razor@...ckwall.org>,
        Shuah Khan <shuah@...nel.org>,
        Russell King <linux@...linux.org.uk>,
        Christian Marangi <ansuelsmth@...il.com>,
        Daniel Borkmann <daniel@...earbox.net>,
        Yuwei Wang <wangyuweihx@...il.com>,
        Petr Machata <petrm@...dia.com>,
        Ido Schimmel <idosch@...dia.com>,
        Florent Fourcot <florent.fourcot@...irst.fr>,
        Hans Schultz <schultz.hans@...il.com>,
        Joachim Wiberg <troglobit@...il.com>,
        Amit Cohen <amcohen@...dia.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        "linux-mediatek@...ts.infradead.org" 
        <linux-mediatek@...ts.infradead.org>,
        "bridge@...ts.linux-foundation.org" 
        <bridge@...ts.linux-foundation.org>,
        "linux-kselftest@...r.kernel.org" <linux-kselftest@...r.kernel.org>
Subject: Re: [PATCH v8 net-next 10/12] net: dsa: mv88e6xxx: mac-auth/MAB
 implementation

Hi Oleksandr,

On Sat, Oct 22, 2022 at 08:50:20AM +0000, Oleksandr Mazur wrote:
> >The essence of my proposal was to send a bug fix now which denies
> >BR_LEARNING to be set together with BR_PORT_LOCKED. The fact that
> >link-local traffic is learned by the software bridge is something
> >unintended as far as I understand.
> 
> >You tried to fix it here, and as far as I could search in my inbox, that
> >didn't go anywhere:
> >https://lore.kernel.org/netdev/47d8d747-54ef-df52-3b9c-acb9a77fa14a@blackwall.org/T/#u
> 
> >I thought only mv88e6xxx offloads BR_PORT_LOCKED, but now, after
> >searching, I also see prestera has support for it, so let me add
> >Oleksandr Mazur to the discussion as well. I wonder how they deal with
> >this? Has somebody come to rely on learning being enabled on a locked
> >port?
> 
> Hello,
> 
> >The fact that
> >link-local traffic is learned by the software bridge is something
> >unintended as far as I understand.
> 
> In prestera driver, if port is in blocked state only the PAE frames
> can be trapped, so i'm not sure where other traffic might come from
> that you are talking. Or maybe i didn't get the issue here right,
> sorry?

I hope the following script will exemplify what I mean.

#!/bin/bash

ip netns add ns0
ip -n ns0 link add br0 type bridge
ip -n ns0 link add veth0 type veth peer name veth1
ip -n ns0 link set veth1 master br0
ip -n ns0 link set veth1 type bridge_slave locked on learning on
ip -n ns0 link set veth0 up
ip -n ns0 link set veth1 up
ip -n ns0 link set br0 up
addr=$(ip -j -n ns0 link show dev veth0 | jq -r '.[]["address"]')
ip netns exec ns0 mausezahn veth0 -q -c 1 -p 64 -b 01:80:c2:00:00:0e -t ip
sleep 1
ip netns exec ns0 bridge fdb show dev veth1 master | grep ${addr}
ip netns del ns0

It will print:

6e:71:0a:8d:85:9e master br0

or in other words, the brport veth1 has learned the MAC SA of veth0 as a
dynamic FDB entry even with no user space daemon to handle the
authentication protocol.

In turn, having this MAC SA present in the bridge FDB means that
communication with this station is now allowed. As far as I can tell,
this is *not* intended. Only the authentication protocol should create
the FDB entry.

Compare this with the same script, but with "locked on learning off".
No FDB entry will be printed.

> 
> Also, basically, prestera driver does not rely on the learning flag if
> the port's flag BR_PORT_LOCKED is set. What this means, is that we
> discard any learning changes on the port if LOCKED is still set (done
> inside firmware, if i recall correctly). E.g. learning is always off,
> if port is in BR_PORT_LOCKED state, or in a block state but also has a
> static fdb entry (aka mac-auth entry).

So I take this as meaning that we could deny BR_LEARNING on ports with
BR_PORT_LOCKED set, and prestera wouldn't be adversely affected. Ok.

> The concept we follow is basically:
>   - some userspace daemon blocks the port;
>   - speaks with the <auth-center> (PAE traffic);
>   - the daemon itself populates the FDB with authenticated MACs (adding static FDB MACs);
>   - forces learning flag disable, disables the PORT_LOCKED flag. At
>   this point switch can basically receive only the traffic from
>   authorized addresses (fdb still has static entries; learning
>   disabled).

I don't understand the last step. Why is the BR_PORT_LOCKED flag disabled?
If disabled, the port will receive frames with any unknown MAC SA,
not just the authorized ones.

Powered by blists - more mailing lists