Sorry Ralf, I wanted to reply to this earlier. 2026-05-28, 22:44:57 +0200, Ralf Lici wrote: > 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.
Thanks for confirming. > 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. I think adding stats counters could help (probably per-socket, and maybe a separate one for each case in patch 4/4). A _once/_ratelimited message also sounds reasonable. > 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. Yup, that makes sense. > 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. A small note to explain that this patch doesn't fully solve the issue would be useful. This patch is a bit large, but I guess it would solve the issue entirely for "well-behaved" userspace (not doing weird setsockopts after "gifting" the socket to the kernel module). Is the current openvpn client hitting the code in next patch once this one is applied (ie finishing config, and then doing V6ONLY/ADDRFORM afterwards)? -- Sabrina _______________________________________________ Openvpn-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/openvpn-devel
