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]

Message-ID: <ZICbhz1PqGU3I408@hog>
Date: Wed, 7 Jun 2023 17:00:23 +0200
From: Sabrina Dubroca <sd@...asysnail.net>
To: Maciej Żenczykowski <maze@...gle.com>
Cc: Simon Horman <simon.horman@...igine.com>,
	Linux Network Development Mailing List <netdev@...r.kernel.org>,
	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

2023-06-06, 06:38:04 +0900, Maciej Żenczykowski wrote:
> 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...

Reviewed-by: Sabrina Dubroca <sd@...asysnail.net>

Thanks Maciej.

-- 
Sabrina