netdev - Re: [PATCH v2] xfrm: fix inbound ipv4/udp/esp packets to UDPv6 dualstack sockets

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: Tue, 6 Jun 2023 06:38:04 +0900
From: Maciej Żenczykowski <maze@...gle.com>
To: Simon Horman <simon.horman@...igine.com>
Cc: Linux Network Development Mailing List <netdev@...r.kernel.org>, Sabrina Dubroca <sd@...asysnail.net>, 
	Steffen Klassert <steffen.klassert@...unet.com>, Jakub Kicinski <kuba@...nel.org>, 
	Benedict Wong <benedictwong@...gle.com>, Yan Yan <evitayan@...gle.com>
Subject: Re: [PATCH v2] xfrm: fix inbound ipv4/udp/esp packets to UDPv6
 dualstack sockets

On Mon, Jun 5, 2023 at 9:59 PM Simon Horman <simon.horman@...igine.com> wrote:
> Hi Maciej,
>
> Does the opposite case also need to be handled in xfrm4_udp_encap_rcv()?

I believe the answer is no:
- ipv4 (AF_INET) sockets only ever receive (native) ipv4 traffic.
- ipv6 (AF_INET6) ipv6-only sockets only ever receive (native) ipv6 traffic.
- ipv6 (AF_INET6) dualstack (ie. not ipv6-only) sockets can receive
both (native) ipv4 and (native) ipv6 traffic.

Ipv6 dualstack sockets map the ipv4 address space into the IPv6
"IPv4-mapped" range of ::ffff:0.0.0.0/96,
ie. 1.2.3.4 -> ::ffff:1.2.3.4 aka ::ffff:0102:0304

Whether ipv6 sockets default to dualstack or not is controlled by a
sysctl (net.ipv6.bindv6only - not entirely well named, it actually
affects the socket() system call, and bind() only as a later
consequence of that, it thus does also affect whether connect() to
ipv4 mapped addresses works or not), but can also be toggled manually
via IPV6_V6ONLY socket option.

Basically a dualstack ipv6 socket is a more-or-less drop-in
replacement for ipv4 sockets (*entirely* so for TCP/UDP, and likely
SCTP, DCCP & UDPLITE, though I think there might be some edge cases
like ICMP sockets or RAW sockets that do need AF_INET - any such
exceptions should probably be considered kernel bugs / missing
features -> hence this patch).

---

I believe we don't need to test the sk for:
  !ipv6_only_sock(sk), ie. !sk->sk_ipv6only
before we do the dispatch to the v4 code path,
because if the socket is ipv6-only then there should [IMHO/AFAICT] be
no way for ipv4 packets to arrive here in the first place.

---

Note: I can guarantee the currently existing code is wrong,
both because we've experimentally discovered AF_INET6 dualstack
sockets don't work for v4,
and because the code obviously tries to read payload length from the
ipv6 header,
which of course doesn't exist for skb->protocol ETH_P_IP packets.

However, I'm still not entirely sure this patch is 100% bug free...
though it seems straightforward enough...

---

I'll hold off on re-spinning for the ' -> " unless there's other comments.