netdev - Re: [PATCH] selftest/bpf: fix IPV6FR handling in flow dissector

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: <292adb9d-899a-fcb0-a37f-cd21e848fede@iogearbox.net>
Date:   Thu, 12 Nov 2020 00:06:23 +0100
From:   Daniel Borkmann <daniel@...earbox.net>
To:     Santucci Pierpaolo <santucci@...genesys.com>,
        Jakub Sitnicki <jakub@...udflare.com>
Cc:     Andrii Nakryiko <andrii.nakryiko@...il.com>,
        Shuah Khan <shuah@...nel.org>,
        Alexei Starovoitov <ast@...nel.org>, Martin Lau <kafai@...com>,
        Song Liu <songliubraving@...com>, Yonghong Song <yhs@...com>,
        Andrii Nakryiko <andrii@...nel.org>,
        john fastabend <john.fastabend@...il.com>,
        KP Singh <kpsingh@...omium.org>,
        Networking <netdev@...r.kernel.org>, bpf <bpf@...r.kernel.org>,
        sdf@...gle.com
Subject: Re: [PATCH] selftest/bpf: fix IPV6FR handling in flow dissector

On 11/11/20 3:12 PM, Santucci Pierpaolo wrote:
> Hi Jakub,
> 
> thanks for your reply.

(Santucci, please do not top-post but always reply inline which makes it
  easier for discussions to follow.)

> Let me explain the problem with an example.
> 
> Please consider the PCAP file:
> https://github.com/named-data/ndn-tools/blob/master/tests/dissect-wireshark/ipv6-udp-fragmented.pcap
> Let's assume that the dissector is invoked without the flag:
> BPF_FLOW_DISSECTOR_F_STOP_AT_FLOW_LABEL.
>   
> Without the proposed patch, the flow keys for the second fragment (packet
> timestamp 0.256997) will contain the value 0x6868 for the source and
> destination port fields: this is obviously wrong.
> The same happens for the third fragment (packet timestamp 0.256998) and for
> the fourth fragment (packet timestamp 0.257001).
> 
> So it seems that the correct thing to do is to stop the dissector after the
> IPV6 fragmentation header for all fragments from the second on.
> 
[...]
>>
>> I'm not initimately familiar with this test, but looking at the change
>> I'd consider that Destinations Options and encapsulation headers can
>> follow the Fragment Header.
>>
>> With enough of Dst Opts or levels of encapsulation, transport header
>> could be pushed to the 2nd fragment. So I'm not sure if the assertion
>> from the IPv4 dissector that 2nd fragment and following doesn't contain
>> any parseable header holds.

Hm, staring at rfc8200, it says that the first fragment packet must include
the upper-layer header (e.g. tcp, udp). The patch here should probably add a
comment wrt to the rfc.

Thanks,
Daniel