netdev - Re: [PATCHv4 net 2/2] IPv6: reply ICMP error if the first fragment don't include 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]

Date:   Mon, 26 Oct 2020 14:49:25 +0000
From:   "Georg Kohmann (geokohma)" <geokohma@...co.com>
To:     Hangbin Liu <liuhangbin@...il.com>
CC:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        Alexey Kuznetsov <kuznet@....inr.ac.ru>,
        Hideaki YOSHIFUJI <yoshfuji@...ux-ipv6.org>,
        "Jakub Kicinski" <kuba@...nel.org>,
        Willem de Bruijn <willemb@...gle.com>,
        "David S . Miller" <davem@...emloft.net>,
        Eric Dumazet <eric.dumazet@...il.com>
Subject: Re: [PATCHv4 net 2/2] IPv6: reply ICMP error if the first fragment
 don't include all headers

On 26.10.2020 13:55, Hangbin Liu wrote:
> On Mon, Oct 26, 2020 at 08:09:21AM +0000, Georg Kohmann (geokohma) wrote:
>>> +	nexthdr = hdr->nexthdr;
>>> +	offset = ipv6_skip_exthdr(skb, skb_transport_offset(skb), &nexthdr, &frag_off);
>>> +	if (offset < 0)
>>> +		goto fail_hdr;
>>> +
>>> +	/* Check some common protocols' header */
>>> +	if (nexthdr == IPPROTO_TCP)
>>> +		offset += sizeof(struct tcphdr);
>>> +	else if (nexthdr == IPPROTO_UDP)
>>> +		offset += sizeof(struct udphdr);
>>> +	else if (nexthdr == IPPROTO_ICMPV6)
>>> +		offset += sizeof(struct icmp6hdr);
>>> +	else
>>> +		offset += 1;
>> Maybe also check the special case IPPROTO_NONE?
> IPPROTO_NONE defines the same with NEXTHDR_NONE. So ipv6_skip_exthdr() will
> return -1, and we will goto fail_hdr and send ICMP parameter error message.
>
> The question is if it's OK to reply a ICMP error for fragment + IPPROTO_NONE
> packet? For pure IPPROTO_NONE message, we should drop silently, but what about
> fragment message?
According to RFC8200 section 4.7: "If the Payload Length field of the IPv6

header indicates the presence of octets past the end of a header whose

Next Header field contains 59, those octets must be ignored and passed

on unchanged if the packet is forwarded." I have not found any RFC

describing different behaviour for fragmented packets.

>
>>> +
>>> +	if (frag_off == htons(IP6_MF) && offset > skb->len) {
>>> +		__IP6_INC_STATS(net, __in6_dev_get_safely(skb->dev), IPSTATS_MIB_INHDRERRORS);
>>> +		icmpv6_param_prob(skb, ICMPV6_HDR_INCOMP, 0);
>>> +		return -1;
>>> +	}
>>> +
>>>  	iif = skb->dev ? skb->dev->ifindex : 0;
>>>  	fq = fq_find(net, fhdr->identification, hdr, iif);
>>>  	if (fq) {
>> Are you planning to also add this fix for the fragmentation handling in the netfilter?
>>
> I have no plan to fix this on netfilter as netfilter is a module.
> It may have different behavior during defragment.
>
> Thanks
> Hangbin

I might have a look at the netfilter myself then.


Thanks

Georg