netdev - Re: [PATCH v3 23/30] ipv6: defrag: drop non-last frags smaller than min mtu

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>] [day] [month] [year] [list]

Date:   Fri, 11 Jan 2019 06:09:13 -0800
From:   Eric Dumazet <edumazet@...gle.com>
To:     twinters@....unh.edu
Cc:     mkubecek@...e.cz, Eric Dumazet <eric.dumazet@...il.com>,
        Florian Westphal <fw@...len.de>,
        Tom Herbert <tom@...bertland.com>,
        netdev <netdev@...r.kernel.org>,
        David Miller <davem@...emloft.net>,
        Peter Oskolkov <posk@...gle.com>
Subject: Re: [PATCH v3 23/30] ipv6: defrag: drop non-last frags smaller than
 min mtu

On Fri, Jan 11, 2019 at 5:26 AM Timothy Winters <twinters@....unh.edu> wrote:
>
> Hi Eric,
>
> So I understand correctly the attack that you are trying to prevent is many small fragments from different IPs?

No... Please  look for CVE-2018-5391

This has been one of the most discussed topic in 2018

https://access.redhat.com/articles/3553061

https://tools.cisco.com/security/center/content/CiscoSecurityAdvisory/cisco-sa-20180824-linux-ip-fragment

>
> The 6MAN working group has had some discussion about this topic, if you want read some IPv6 networking prospectives.
>
> https://mailarchive.ietf.org/arch/browse/ipv6/?gbt=1&index=zsYUwQTt0mAdXdk9MN4dP-uiiag
>
> The issue you will see is devices that don't send a first fragment of a size 1280 will never be able to communicate with newer kernel.  Another issue is if a the packets are traversing over tunneled interfaces (VPNs) they will be lower than 1280 due to the headers used on tunnels.
>
> What about lowering the value of accepted fragments?  to something like 1280/2?
>
> ~Tim
>
> On Fri, Jan 11, 2019 at 8:07 AM Eric Dumazet <edumazet@...gle.com> wrote:
>>
>> On Fri, Jan 11, 2019 at 4:52 AM Michal Kubecek <mkubecek@...e.cz> wrote:
>> >
>> > On Fri, Jan 11, 2019 at 04:27:24AM -0800, Eric Dumazet wrote:
>> > > On Fri, Jan 11, 2019 at 4:21 AM Michal Kubecek <mkubecek@...e.cz> wrote:
>> > > >
>> > > > On Fri, Jan 11, 2019 at 02:57:39AM -0800, Eric Dumazet wrote:
>> > > > > On 01/10/2019 02:22 PM, Florian Westphal wrote:
>> > > > > > Tom Herbert <tom@...bertland.com> wrote:
>> > > > > >> I couldn't find any mention of the advisory in the commit logs or
>> > > > > >> netdev discussion, and apparently there's no protocol requirement that
>> > > > > >> intermediate fragements need to be at least minimal MTU. Maybe this
>> > > > > >> patch should be reverted?
>> > > > > >
>> > > > > > Currently ipv6 reasm doesn't use rbtree infrastructure, so it would
>> > > > > > have to be converted first.
>> > > > >
>> > > > > <quote>
>> > > > > Section 4.5 of RFC 8200 allows for sending any fragment for
>> > > > > fragments as long they add up to the original packet.
>> > > > > </quote>
>> > > > >
>> > > > > I do not believe we need an rbtree to implement this idea.
>> > > >
>> > > > IMHO Florian meant that allowing arbitrarily small fragments would harm
>> > > > resistance against FragmentSmack type attacks so that we might need
>> > > > rbtree based queues to be reasonably safe.
>> > > >
>> > >
>> > > I fail to see why.
>> > >
>> > > Adding a fragment to the tail of the list is O(1), whatever of its size.
>> > >
>> > > If we ever receive a 'too small for IPv6' fragment not at the tail,
>> > > we would immediately discard the whole datagram in O(1) as well.
>> > >
>> > > rbtree was needed because we assumed that we needed to allow IPv4 to
>> > > receive arbitrary fragments.
>> >
>> > My understanding of the original e-mail was that our assumption that
>> > only last IPv6 fragment is allowed to be shorter than 1280 bytes (which
>> > commit 0ed4229b08c1 ("ipv6: defrag: drop non-last frags smaller than min
>> > mtu") is based on) was wrong and we should probably revert that commit.
>> > How do you understand it?
>> >
>>
>> Reverting the commit is a no go, hackers will simply kill the hosts.
>>
>> I was saying that we can refine the check, and allow whatever
>> fragments to come in order.
>> (Even if this makes no sense, since fragments could come out of order)
>>
>> Of course, the rb-tree implementation might be okay (even for stable
>> trees), since we can copy/paste the IPv4 one.