[Openvpn-devel,ovpn,net,3/4] ovpn: reject UDP remotes incompatible with socket family
Commit Message
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.
Fixes: 1d36a36f6d53 ("ovpn: implement peer add/get/dump/delete via netlink")
Signed-off-by: Ralf Lici <ralf@mandelbit.com>
---
drivers/net/ovpn/netlink.c | 130 ++++++++++++++++++++++++++-----------
1 file changed, 92 insertions(+), 38 deletions(-)
Comments
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).
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!
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)?
On Wed, 03 Jun 2026 19:53:58 +0200, Sabrina Dubroca <sd@queasysnail.net> wrote:
> Sorry Ralf, I wanted to reply to this earlier.
No worries, thanks for taking another look.
>
> 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.
>
I agree that counters would improve observability, and I am not opposed
to adding them later if we find they are useful in practice. For this
series, though, I am not sure they are worth adding UAPI for (netlink
operations are currently all peer-related).
These drops are not expected to be normal packet-loss events: they mean
userspace handed ovpn a socket, then left it in a state where that peer
cannot use it anymore. In that case the most useful immediate signal is
the reason why the peer became unusable, rather than a counter of how
many packets were already dropped because of it.
Antonio and I discussed this more in depth and our plan for the respin
is to delete the peer with TRANSPORT_ERROR via deferred work for
socket/remote address family mismatches, instead of keeping a peer
around that can no longer transmit. We would also add a _ratelimited
warning on those TX-side failures (_once appears to be once per call
site, so one bad peer/socket would silence the warning for all later
cases).
For sport == 0 I'd keep dropping and warning for now and think about a
more systematic approach in net-next. For example, we could reject this
at socket setup time and override the UDP socket protocol callbacks to
intercept and prevent disconnect (though that needs some caution because
IPV6_ADDRFORM can apparently switch the sk_prot back to udp_prot).
Does that make sense to you?
> > 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.
Sure, I will add it.
>
> 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)?
No, luckily current openvpn userspace does not change these socket
properties after handing the socket to ovpn. The setup time checks
should therefore be enough for well-behaved userspace. The TX-side
checks are still needed because the socket remains mutable after attach.
>
> --
> Sabrina
>
@@ -271,15 +271,17 @@ static int ovpn_nl_peer_precheck(struct ovpn_priv *ovpn,
* @peer: the peer to modify
* @info: generic netlink info from the user request
* @attrs: the attributes from the user request
+ * @remote: remote address, if provided
*
* Return: a negative error code in case of failure, 0 on success or 1 on
* success and the VPN IPs have been modified (requires rehashing in MP
* mode)
*/
static int ovpn_nl_peer_modify(struct ovpn_peer *peer, struct genl_info *info,
- struct nlattr **attrs)
+ struct nlattr **attrs,
+ const struct sockaddr_storage *remote)
{
- struct sockaddr_storage ss = {};
+ struct sockaddr_storage empty_remote = {};
void *local_ip = NULL;
u32 interv, timeout;
bool rehash = false;
@@ -287,15 +289,15 @@ static int ovpn_nl_peer_modify(struct ovpn_peer *peer, struct genl_info *info,
spin_lock_bh(&peer->lock);
- if (ovpn_nl_attr_sockaddr_remote(attrs, &ss)) {
+ if (remote) {
/* we carry the local IP in a generic container.
* ovpn_peer_reset_sockaddr() will properly interpret it
- * based on ss.ss_family
+ * based on remote->ss_family
*/
local_ip = ovpn_nl_attr_local_ip(attrs);
/* set peer sockaddr */
- ret = ovpn_peer_reset_sockaddr(peer, &ss, local_ip);
+ ret = ovpn_peer_reset_sockaddr(peer, remote, local_ip);
if (ret < 0) {
NL_SET_ERR_MSG_FMT_MOD(info->extack,
"cannot set peer sockaddr: %d",
@@ -333,7 +335,7 @@ static int ovpn_nl_peer_modify(struct ovpn_peer *peer, struct genl_info *info,
netdev_dbg(peer->ovpn->dev,
"modify peer id=%u tx_id=%u endpoint=%pIScp VPN-IPv4=%pI4 VPN-IPv6=%pI6c\n",
- peer->id, peer->tx_id, &ss,
+ peer->id, peer->tx_id, remote ?: &empty_remote,
&peer->vpn_addrs.ipv4.s_addr, &peer->vpn_addrs.ipv6);
spin_unlock_bh(&peer->lock);
@@ -344,10 +346,41 @@ static int ovpn_nl_peer_modify(struct ovpn_peer *peer, struct genl_info *info,
return ret;
}
+/**
+ * ovpn_nl_udp_remote_compatible - check if a UDP socket can use a remote
+ * @sk: UDP socket to validate
+ * @remote: remote endpoint to validate against the socket
+ * @extack: netlink extended ACK for reporting validation errors
+ *
+ * Return: 0 if the remote endpoint is compatible with the socket or a negative
+ * error code otherwise.
+ */
+static int ovpn_nl_udp_remote_compatible(const struct sock *sk,
+ const struct sockaddr_storage *remote,
+ struct netlink_ext_ack *extack)
+{
+ if (sk->sk_family == AF_INET && remote->ss_family == AF_INET6) {
+ NL_SET_ERR_MSG_FMT_MOD(extack,
+ "UDP socket is IPv4 but remote is IPv6");
+ return -EAFNOSUPPORT;
+ }
+
+ if (sk->sk_family == AF_INET6 && ipv6_only_sock(sk) &&
+ remote->ss_family == AF_INET) {
+ NL_SET_ERR_MSG_FMT_MOD(extack,
+ "UDP socket is IPv6-only but remote is IPv4");
+ return -EAFNOSUPPORT;
+ }
+
+ return 0;
+}
+
int ovpn_nl_peer_new_doit(struct sk_buff *skb, struct genl_info *info)
{
- struct nlattr *attrs[OVPN_A_PEER_MAX + 1];
+ const struct sockaddr_storage *remote = NULL;
struct ovpn_priv *ovpn = info->user_ptr[0];
+ struct nlattr *attrs[OVPN_A_PEER_MAX + 1];
+ struct sockaddr_storage ss = {};
struct ovpn_socket *ovpn_sock;
struct socket *sock = NULL;
struct ovpn_peer *peer;
@@ -411,26 +444,34 @@ int ovpn_nl_peer_new_doit(struct sk_buff *skb, struct genl_info *info)
goto peer_release;
}
- /* Only when using UDP as transport protocol the remote endpoint
- * can be configured so that ovpn knows where to send packets to.
- */
- if (sk_is_udp(sock->sk) &&
- !attrs[OVPN_A_PEER_REMOTE_IPV4] &&
- !attrs[OVPN_A_PEER_REMOTE_IPV6]) {
- NL_SET_ERR_MSG_FMT_MOD(info->extack,
- "missing remote IP address for UDP socket");
- sockfd_put(sock);
- ret = -EINVAL;
- goto peer_release;
- }
+ if (ovpn_nl_attr_sockaddr_remote(attrs, &ss))
+ remote = &ss;
- /* In case of TCP, the socket is connected to the peer and ovpn
- * will just send bytes over it, without the need to specify a
- * destination.
- */
- if (sk_is_tcp(sock->sk) &&
- (attrs[OVPN_A_PEER_REMOTE_IPV4] ||
- attrs[OVPN_A_PEER_REMOTE_IPV6])) {
+ if (sk_is_udp(sock->sk)) {
+ /* Only when using UDP as transport protocol the remote
+ * endpoint can be configured so that ovpn knows where to send
+ * packets to.
+ */
+ if (!remote) {
+ NL_SET_ERR_MSG_FMT_MOD(info->extack,
+ "missing remote IP address for UDP socket");
+ sockfd_put(sock);
+ ret = -EINVAL;
+ goto peer_release;
+ }
+
+ /* can the socket be used with this remote? */
+ ret = ovpn_nl_udp_remote_compatible(sock->sk, remote,
+ info->extack);
+ if (ret < 0) {
+ sockfd_put(sock);
+ goto peer_release;
+ }
+ } else if (remote) {
+ /* In case of TCP, the socket is connected to the peer and ovpn
+ * will just send bytes over it, without the need to specify a
+ * destination.
+ */
NL_SET_ERR_MSG_FMT_MOD(info->extack,
"unexpected remote IP address with TCP socket");
sockfd_put(sock);
@@ -456,7 +497,7 @@ int ovpn_nl_peer_new_doit(struct sk_buff *skb, struct genl_info *info)
rcu_assign_pointer(peer->sock, ovpn_sock);
- ret = ovpn_nl_peer_modify(peer, info, attrs);
+ ret = ovpn_nl_peer_modify(peer, info, attrs, remote);
if (ret < 0)
goto sock_release;
@@ -483,8 +524,10 @@ int ovpn_nl_peer_new_doit(struct sk_buff *skb, struct genl_info *info)
int ovpn_nl_peer_set_doit(struct sk_buff *skb, struct genl_info *info)
{
- struct nlattr *attrs[OVPN_A_PEER_MAX + 1];
+ const struct sockaddr_storage *remote = NULL;
struct ovpn_priv *ovpn = info->user_ptr[0];
+ struct nlattr *attrs[OVPN_A_PEER_MAX + 1];
+ struct sockaddr_storage ss = {};
struct ovpn_socket *sock;
struct ovpn_peer *peer;
u32 peer_id;
@@ -516,22 +559,33 @@ int ovpn_nl_peer_set_doit(struct sk_buff *skb, struct genl_info *info)
return -ENOENT;
}
- /* when using a TCP socket the remote IP is not expected */
+ if (ovpn_nl_attr_sockaddr_remote(attrs, &ss))
+ remote = &ss;
+
rcu_read_lock();
sock = rcu_dereference(peer->sock);
- if (sock && sock->sk->sk_protocol == IPPROTO_TCP &&
- (attrs[OVPN_A_PEER_REMOTE_IPV4] ||
- attrs[OVPN_A_PEER_REMOTE_IPV6])) {
- rcu_read_unlock();
- NL_SET_ERR_MSG_FMT_MOD(info->extack,
- "unexpected remote IP address with TCP socket");
- ovpn_peer_put(peer);
- return -EINVAL;
+ if (sock && remote) {
+ if (sk_is_udp(sock->sk)) {
+ ret = ovpn_nl_udp_remote_compatible(sock->sk, remote,
+ info->extack);
+ if (ret < 0) {
+ rcu_read_unlock();
+ ovpn_peer_put(peer);
+ return ret;
+ }
+ } else if (sk_is_tcp(sock->sk)) {
+ /* when using a TCP socket remote IP is not expected */
+ NL_SET_ERR_MSG_FMT_MOD(info->extack,
+ "unexpected remote IP address with TCP socket");
+ rcu_read_unlock();
+ ovpn_peer_put(peer);
+ return -EINVAL;
+ }
}
rcu_read_unlock();
spin_lock_bh(&ovpn->lock);
- ret = ovpn_nl_peer_modify(peer, info, attrs);
+ ret = ovpn_nl_peer_modify(peer, info, attrs, remote);
if (ret < 0) {
spin_unlock_bh(&ovpn->lock);
ovpn_peer_put(peer);