netdev - Re: [Bridge] [PATCH net] net: bridge: remove ipv6 zero address check in mcast queries

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:   Sun, 28 Oct 2018 18:09:06 +0200
From:   Nikolay Aleksandrov <nikolay@...ulusnetworks.com>
To:     Stephen Hemminger <stephen@...workplumber.org>
Cc:     netdev@...r.kernel.org, roopa@...ulusnetworks.com,
        bridge@...ts.linux-foundation.org, yinxu@...hat.com,
        liuhangbin@...il.com, davem@...emloft.net
Subject: Re: [Bridge] [PATCH net] net: bridge: remove ipv6 zero address check
 in mcast queries

On 28/10/2018 17:20, Stephen Hemminger wrote:
> On Sat, 27 Oct 2018 12:07:47 +0300
> Nikolay Aleksandrov <nikolay@...ulusnetworks.com> wrote:
> 
>> Recently a check was added which prevents marking of routers with zero
>> source address, but for IPv6 that cannot happen as the relevant RFCs
>> actually forbid such packets:
>> RFC 2710 (MLDv1):
>> "To be valid, the Query message MUST
>>  come from a link-local IPv6 Source Address, be at least 24 octets
>>  long, and have a correct MLD checksum."
>>
>> Same goes for RFC 3810.
>>
>> And also it can be seen as a requirement in ipv6_mc_check_mld_query()
>> which is used by the bridge to validate the message before processing
>> it. Thus any queries with :: source address won't be processed anyway.
>> So just remove the check for zero IPv6 source address from the query
>> processing function.
>>
>> Fixes: 5a2de63fd1a5 ("bridge: do not add port to router list when receives query with source 0.0.0.0")
>> Signed-off-by: Nikolay Aleksandrov <nikolay@...ulusnetworks.com>
> 
> What about a broken/malicious sender? Could an all zero source be used
> to poison the multicast table?
> 

No, this has nothing to do with the table. This is about marking routers
and we shouldn't consider queries with src 0.0.0.0 (the original fix)
but for IPv6 such query is invalid and in fact doesn't reach that code at all.
As I've written in the commit message, ipv6_mc_check_mld_query() already checks
for that and marks it as invalid thus it isn't processed and we can drop that
check from the bridge mcast code, if you test with such src you can see in
the bridge mcast statistics that the MLD errors are going up showing that these
packets are treated as errors.

Thanks,
 Nik