netdev - Re: [PATCH 1/1] bridge: remove BR_GROUPFWD_RESTRICTED for arbitrary forwarding of reserved addresses

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: Windows password security audit tool. GUI, reports in PDF.

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

Date:   Mon, 1 Oct 2018 09:24:25 -0700
From:   Florian Fainelli <f.fainelli@...il.com>
To:     Richard Weinberger <richard.weinberger@...il.com>,
        Stephen Hemminger <stephen@...workplumber.org>
Cc:     bernhard.thaler@...et.at, "David S. Miller" <davem@...emloft.net>,
        bridge@...ts.linux-foundation.org, netdev@...r.kernel.org,
        David Gstir <david@...ma-star.at>
Subject: Re: [PATCH 1/1] bridge: remove BR_GROUPFWD_RESTRICTED for arbitrary
 forwarding of reserved addresses

On 10/01/2018 07:28 AM, Richard Weinberger wrote:
> Stephen, Bernhard,
> 
> On Tue, Jan 6, 2015 at 7:18 AM Stephen Hemminger
> <stephen@...workplumber.org> wrote:
>>
>> On Tue,  6 Jan 2015 01:56:15 +0100
>> Bernhard Thaler <bernhard.thaler@...et.at> wrote:
>>
>>> BR_GROUPFWD_RESTRICTED bitmask restricts users from setting values to
>>> /sys/class/net/brX/bridge/group_fwd_mask that allow forwarding of
>>> some IEEE 802.1D Table 7-10 Reserved addresses:
>>>       (MAC Control) 802.3             01-80-C2-00-00-01
>>>       (Link Aggregation) 802.3        01-80-C2-00-00-02
>>>       802.1AB LLDP                    01-80-C2-00-00-0E
>>> BR_GROUPFWD_RESTRICTED may have been set as an extra protection against
>>> forwarding these control frames as forwarding 802.1X PAE (01-80-C2-00-00-03)
>>> in 802.1X setups satisfies most common use-cases.
>>> Other situations, such as placing a software based bridge as a "TAP" between two
>>> devices may require to forward e.g. LLDP frames while debugging network problems
>>> or actively changing/filtering traffic with ebtables.
>>>
>>> This patch allows to set e.g.:
>>>       echo 65535 > /sys/class/net/brX/bridge/group_fwd_mask
>>> which sets no restrictions on the forwardable reserved addresses.
>>>
>>> - the default value 0 will still comply with 802.1D and not forward any
>>>   reserved addresses
>>> - values such as 8 for forwarding 802.1X related frames will behave the
>>>   same way as with BR_GROUPFWD_RESTRICTED currently in place, so backward
>>>   compatibility to current scripts using group_fwd_masks shoudl be possible
>>>
>>> Administrators and network engineers however will be able to arbitrarily
>>> forward any reserved addresses without BR_GROUPFWD_RESTRICTED. This will
>>> be non-standard compliant behavior, but forwarding of any reserved address
>>> right from the beginning is. Users should be aware of this anyway and
>>> know what/why they are doing when setting values such as 65535, 32768, 16384,
>>> 4, 2 for group_fwd_mask
>>>
>>> This patch was tested on a bridge with two interfaces created with bridge-utils.
>>>
>>> Signed-off-by: Bernhard Thaler <bernhard.thaler@...et.at>
>>
>> I am ok with forwarding LLDP because some people need it.
>> But allowing forwarding STP or PAUSE frames is bad.
>>
>> We don't let people do things that break networks. Other examples
>> already exist like set all 0 ethernet addresses, or the restrictions
>> on allowing net 127 in IP addresses.
> 
> Sorry for unearthing this old thread, while preparing our in-house
> patch I found it.
> This patch does exactly what we need and already do in our network. We
> have Linux bridges that
> forward anything. With one restriction, the bridge has always just two slaves.
> In other works, it acts like a cable. Therefore I see no problem in
> forwarding STP or PAUSE frames.

If all you are doing is forwarding anything, one thing I experimented
with before is the following:

# tc qdisc add dev eth1 handle ffff: ingress
# tc qdisc add dev eth3 handle ffff: ingress
# tc filter add dev eth3 parent ffff: u32 \
> match u32 0 0 \
> action mirred egress redirect dev eth1
# tc filter add dev eth1 parent ffff: u32 \
> match u32 0 0 \
> action mirred egress redirect dev eth3
# ifconfig eth3 promisc
# ifconfig eth1 promisc

and this works just fine actually, bypassing the bridge layer entirely.

> 
> Since Linux allows these days also to use 127/8 IP addresses, would
> you accept a patch which allows
> forwarding STP and PAUSE if the number of slaves is restricted to 2? :-)
> 


-- 
Florian