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

Date:   Fri, 11 Jan 2019 05:07:18 -0800
From:   Eric Dumazet <edumazet@...gle.com>
To:     mkubecek@...e.cz
Cc:     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>, twinters@....unh.edu
Subject: Re: [PATCH v3 23/30] ipv6: defrag: drop non-last frags smaller than
 min mtu

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.