netdev - Re: [PATCH net v4] ipv6/netfilter: Discard first fragment not including all headers

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]

Message-ID: <20201115111520.GA24052@salvia>
Date:   Sun, 15 Nov 2020 12:15:20 +0100
From:   Pablo Neira Ayuso <pablo@...filter.org>
To:     Georg Kohmann <geokohma@...co.com>
Cc:     netdev@...r.kernel.org, kadlec@...filter.org, fw@...len.de,
        davem@...emloft.net, kuznet@....inr.ac.ru, yoshfuji@...ux-ipv6.org,
        kuba@...nel.org, netfilter-devel@...r.kernel.org,
        coreteam@...filter.org
Subject: Re: [PATCH net v4] ipv6/netfilter: Discard first fragment not
 including all headers

On Wed, Nov 11, 2020 at 12:50:25PM +0100, Georg Kohmann wrote:
> Packets are processed even though the first fragment don't include all
> headers through the upper layer header. This breaks TAHI IPv6 Core
> Conformance Test v6LC.1.3.6.
> 
> Referring to RFC8200 SECTION 4.5: "If the first fragment does not include
> all headers through an Upper-Layer header, then that fragment should be
> discarded and an ICMP Parameter Problem, Code 3, message should be sent to
> the source of the fragment, with the Pointer field set to zero."
> 
> The fragment needs to be validated the same way it is done in
> commit 2efdaaaf883a ("IPv6: reply ICMP error if the first fragment don't
> include all headers") for ipv6. Wrap the validation into a common function,
> ipv6_frag_thdr_truncated() to check for truncation in the upper layer
> header. This validation does not fullfill all aspects of RFC 8200,
> section 4.5, but is at the moment sufficient to pass mentioned TAHI test.
> 
> In netfilter, utilize the fragment offset returned by find_prev_fhdr() to
> let ipv6_frag_thdr_truncated() start it's traverse from the fragment
> header.
> 
> Return 0 to drop the fragment in the netfilter. This is the same behaviour
> as used on other protocol errors in this function, e.g. when
> nf_ct_frag6_queue() returns -EPROTO. The Fragment will later be picked up
> by ipv6_frag_rcv() in reassembly.c. ipv6_frag_rcv() will then send an
> appropriate ICMP Parameter Problem message back to the source.
> 
> References commit 2efdaaaf883a ("IPv6: reply ICMP error if the first
> fragment don't include all headers")
> 
> Signed-off-by: Georg Kohmann <geokohma@...co.com>

Acked-by: Pablo Neira Ayuso <pablo@...filter.org>

In case you would like to follow up with another patch for the IPv6
reassembly in netfilter.o

net/ipv6/netfilter/nf_conntrack_reasm.c uses pr_debug() everywhere.

net/ipv4/netfilter/nf_defrag_ipv4.c however uses ip_defrag() which is
updating IPSTATS_MIB_*, so IPv4 and IPv6 code behave differently with
regards to the stats.

It would be probably good to get them aligned, by replacing the
existing pr_debug() in the net/ipv6/netfilter/nf_conntrack_reasm.c
code by IPSTATS_MIB_*.

Thanks.