On 5/28/26 19:15, Sabrina Dubroca wrote:
Hi Ralf,
2026-05-26, 14:45:40 +0200, Ralf Lici wrote:
ovpn accepts a userspace-provided socket and a peer remote endpoint
through netlink. For UDP peers, the remote endpoint family selects the
transmit path used later by ovpn_udp_output.
An IPv4 UDP socket cannot be used with an IPv6 remote endpoint. If
accepted, the transmit path may reach IPv6 routing and UDP tunnel
helpers with an IPv4 socket. Similarly, an IPv6-only UDP socket cannot
be used with an IPv4 remote endpoint. Otherwise ovpn may enter the IPv4
UDP transmit path, where IPv4 routing treats the unspecified source as a
normal source-selection request and may choose an IPv4 source address,
bypassing the check that udpv6_sendmsg would normally enforce.
Parse the remote endpoint once in the peer new/set paths and reject UDP
remotes when the provided socket cannot send to them. Pass the parsed
endpoint into the common peer modify helper so the validation and the
stored endpoint use the same normalized sockaddr.
Since this can be changed at any time by userspace, I'm not convinced
checking it at setup time is very useful.
AFAIU, the next patch fixes the actual issue. This one seems to mainly
help userspace avoid drops in case they incorrectly set up their
socket (as long as they do that in the "right order", ie all
setsockopt before passing the socket to ovpn).
Hi Sabrina,
Yes, your understanding is correct: the new/set peer netlink messages
give us a chance to detect a broken configuration at setup time, but we
cannot prevent userspace from changing the socket properties later on.
In this sense this patch is not the actual correctness fix.
The next patch enforces the real gate on TX, but the error is ultimately
swallowed by ovpn_udp_send_skb after freeing the skb. Antonio suggested
adding a netdev_warn_once on these new socket-based failure points
(including the sport == 0 case), and I think that would be useful too.
But, aside from that, I don't think there's a clean way to report an
error to userspace once such misconfigurations are hit from the TX path.
So the rationale for the current patch is to at least report the
specific socket/remote mismatch through extack when it is already
present in the netlink request, and avoid installing a peer that would
immediately drop traffic.
That said, I see that the commit message may be selling the patch as
solving the issue at its core. I can reword it to make clear that this
is only early validation/reporting, or drop the patch if you think the
value is too low.
Thanks for the input!
--
Ralf Lici
Mandelbit Srl
_______________________________________________
Openvpn-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
