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

Date:   Thu, 10 Jan 2019 11:30:16 -0800
From:   Tom Herbert <tom@...bertland.com>
To:     Linux Kernel Network Developers <netdev@...r.kernel.org>
Cc:     "David S. Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Florian Westphal <fw@...len.de>,
        Peter Oskolkov <posk@...gle.com>,
        Timothy Winters <twinters@....unh.edu>
Subject: Re: [PATCH v3 23/30] ipv6: defrag: drop non-last frags smaller than
 min mtu

Hi,

This patch popped on 6man IETF list as possibly causing
interoperability problems. Here's the description from Timothy
Winters:

"We have encountered a potential Interoperability issue at the UNH-IOL
while running some testing.  The issue is around fragments. The Linux
Kernel updated based on the following CVE:
https://nvd.nist.gov/vuln/detail/CVE-2018-5391.

The fix was to reject IPv6 fragments less than 1280 that aren't last
fragment.  Section 4.5 of RFC 8200 allows for sending any fragment for
fragments as long they add up to the original packet.  This means that
an implementation that generates a non-last fragments with a size then
1280, will be dropped by the updated kernel."

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?

Tom

On Thu, Sep 13, 2018 at 8:00 AM Stephen Hemminger
<stephen@...workplumber.org> wrote:
>
> From: Florian Westphal <fw@...len.de>
>
> don't bother with pathological cases, they only waste cycles.
> IPv6 requires a minimum MTU of 1280 so we should never see fragments
> smaller than this (except last frag).
>
> v3: don't use awkward "-offset + len"
> v2: drop IPv4 part, which added same check w. IPV4_MIN_MTU (68).
>     There were concerns that there could be even smaller frags
>     generated by intermediate nodes, e.g. on radio networks.
>
> Cc: Peter Oskolkov <posk@...gle.com>
> Cc: Eric Dumazet <edumazet@...gle.com>
> Signed-off-by: Florian Westphal <fw@...len.de>
> Signed-off-by: David S. Miller <davem@...emloft.net>
> (cherry picked from commit 0ed4229b08c13c84a3c301a08defdc9e7f4467e6)
> ---
>  net/ipv6/netfilter/nf_conntrack_reasm.c | 4 ++++
>  net/ipv6/reassembly.c                   | 4 ++++
>  2 files changed, 8 insertions(+)
>
> diff --git a/net/ipv6/netfilter/nf_conntrack_reasm.c b/net/ipv6/netfilter/nf_conntrack_reasm.c
> index a1dc0d6a5949..1d2f07cde01a 100644
> --- a/net/ipv6/netfilter/nf_conntrack_reasm.c
> +++ b/net/ipv6/netfilter/nf_conntrack_reasm.c
> @@ -565,6 +565,10 @@ int nf_ct_frag6_gather(struct net *net, struct sk_buff *skb, u32 user)
>         hdr = ipv6_hdr(skb);
>         fhdr = (struct frag_hdr *)skb_transport_header(skb);
>
> +       if (skb->len - skb_network_offset(skb) < IPV6_MIN_MTU &&
> +           fhdr->frag_off & htons(IP6_MF))
> +               return -EINVAL;
> +
>         skb_orphan(skb);
>         fq = fq_find(net, fhdr->identification, user, hdr,
>                      skb->dev ? skb->dev->ifindex : 0);
> diff --git a/net/ipv6/reassembly.c b/net/ipv6/reassembly.c
> index e1c5fa5e3873..afaad60dc2ac 100644
> --- a/net/ipv6/reassembly.c
> +++ b/net/ipv6/reassembly.c
> @@ -522,6 +522,10 @@ static int ipv6_frag_rcv(struct sk_buff *skb)
>                 return 1;
>         }
>
> +       if (skb->len - skb_network_offset(skb) < IPV6_MIN_MTU &&
> +           fhdr->frag_off & htons(IP6_MF))
> +               goto fail_hdr;
> +
>         iif = skb->dev ? skb->dev->ifindex : 0;
>         fq = fq_find(net, fhdr->identification, hdr, iif);
>         if (fq) {
> --
> 2.18.0
>