netdev - Re: [PATCH 00/12] flow_dissector: Dissect UDP encapsulation protocols

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: <CALx6S37FjSB6h6zLAdV+YF-C5H0O0968Zooo=9cJCm8Z3x0XvQ@mail.gmail.com>
Date: Wed, 14 Aug 2024 13:28:18 -0700
From: Tom Herbert <tom@...bertland.com>
To: Willem de Bruijn <willemdebruijn.kernel@...il.com>
Cc: davem@...emloft.net, kuba@...nel.org, edumazet@...gle.com, 
	netdev@...r.kernel.org, felipe@...anda.io
Subject: Re: [PATCH 00/12] flow_dissector: Dissect UDP encapsulation protocols

On Thu, Aug 1, 2024 at 6:20 AM Willem de Bruijn
<willemdebruijn.kernel@...il.com> wrote:
>
> Tom Herbert wrote:
> > Add support in flow_dissector for dissecting into UDP
> > encapsulations like VXLAN. __skb_flow_dissect_udp is called for
> > IPPROTO_UDP. The flag FLOW_DISSECTOR_F_PARSE_UDP_ENCAPS enables parsing
> > of UDP encapsulations. If the flag is set when parsing a UDP packet then
> > a socket lookup is performed. The offset of the base network header,
> > either an IPv4 or IPv6 header, is tracked and passed to
> > __skb_flow_dissect_udp so that it can perform the socket lookup.
> > If a socket is found and it's for a UDP encapsulation (encap_type is
> > set in the UDP socket) then a switch is performed on the encap_type
> > value (cases are UDP_ENCAP_* values)
>
> The main concern with the flow dissector is that its execution depends
> on untrusted packets.
>
> For this reason we added the BPF dissector for new protocols. What is
> the reason to prefer adding more C code?
>
> And somewhat academic, but: would it be different if the BPF would
> ship with the kernel and autoload at boot, just like C modules?

Hi Willem,

I agree with that, and believe the ultimate goal is to replace flow
dissector C code with eBPF which I still intend to work on that, but
right now I'm hoping to get support as part of obsoleting protocol
specific checksum offload on receive. We can use flow dissector to
identify the checksum in a packet marked checksum-unnecessary by a
legacy device for doing conversion to checksum-complete. This handles
the case where the device reports a valid L4 checksum in a UDP
encapsulation and the outer UDP checksum is zero.

>
> A second concern is changing the defaults. I have not looked at this
> closely, but if dissection today stops at the outer UDP header for
> skb_get_hash, then we don't want to accidentally change this behavior.
> Or if not accidental, call it out explicitly.

No defaults are being changed. Flow dissector flag
FLOW_DISSECTOR_F_PARSE_UDP_ENCAPS needs to be set in the call to flow
dissector. In this patch set it's not being used, but as I mentioned
it will be used in subsequent patch sets for obsoleting
CHECKSUM_UNNECESSARY.

For other use cases, the flag can be optionally set. TC-flower for
instance could use this for VXLAN and Geneve parsing.

>
> >
> > Tested: Verified fou, gue, vxlan, and geneve are properly dissected for
> > IPv4 and IPv6 cases. This includes testing ETH_P_TEB case
>
> Manually?

Yes for the time being.

Tom