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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CALx6S37ENf9AVc9oqoJ+Uh-6DAG6ctNdi0H-Y-8KHQnwZt0AFg@mail.gmail.com>
Date: Wed, 14 Aug 2024 13:37:14 -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 Wed, Aug 14, 2024 at 1:28 PM Tom Herbert <tom@...bertland.com> wrote:
>
> 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.

Also, there's another wrinkle with doing this in eBPF. UDP
encapsulations are identified by port number, not a protocol number,
so we can't hardcode port numbers into eBPF like we can other protocol
constants. We'd probably need to hook into setup_udp_tunnel_sock
somehow.

Tom

>
> >
> > 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