ovpn: validate peer VPN addresses

PATCH Superseded Ralf Lici 2 participants 10 messages Jul 29 – Aug 14

Subsystems (4)

Networking Network drivers Kselftest Openvpn data channel offload

lore

Share & tools

lore

Ralf Lici

author reviewer

2026-07-29 15:42 UTC #0 lore raw

cover ovpn: validate peer VPN addresses

Hi,

In MP mode, ovpn uses peer VPN addresses as lookup keys when selecting the peer for an outgoing packet. The peer configuration path should therefore maintain a few basic invariants around those addresses.

This series ensures that cleared addresses are removed from the hash tables, prevents the same address from being assigned to multiple peers, requires every MP peer to have at least one configured VPN address, and rejects addresses that cannot identify a unicast peer.

The final patch extends the existing selftests to exercise these rules through both peer creation and update, including transitions between single-stack and dual-stack configurations.

Regards,

Ralf Lici
Mandelbit Srl

---
Ralf Lici (5):
ovpn: always unhash old VPN addresses before rehashing
ovpn: reject duplicate peer VPN addresses
ovpn: reject multipeer peers without VPN addresses
ovpn: reject invalid peer VPN addresses
selftests: ovpn: validate peer VPN addresses

 drivers/net/ovpn/netlink.c                  | 100 +++++++++++++++++---
 drivers/net/ovpn/peer.c                     |  58 ++++++++++--
 drivers/net/ovpn/peer.h                     |   4 +
 tools/testing/selftests/net/ovpn/common.sh  |  13 +++
 tools/testing/selftests/net/ovpn/ovpn-cli.c |  54 +++++++----
 tools/testing/selftests/net/ovpn/test.sh    |  75 ++++++++++++++-
 6 files changed, 264 insertions(+), 40 deletions(-)

Signature · 8 lines

2.55.0



_______________________________________________
Openvpn-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Ralf Lici

author reviewer

2026-07-29 15:42 UTC #1 lore raw

PATCH 1/5 ovpn: always unhash old VPN addresses before rehashing

ovpn_peer_hash_vpn_ip updates the per-peer VPN address hash entries after userspace changes a peer VPN address. The current code removes an old hash entry only when the new address for that family is not the unspecified address.

When an address is cleared to 0.0.0.0 or ::, its hash node therefore remains linked in the bucket selected by the old address. The address comparison performed during lookup prevents the old address from matching, but the table retains a stale entry until the peer is removed or another address is configured for that family.

Always remove both old VPN address hash entries before conditionally adding the currently configured addresses back. This ensures that a cleared address leaves its hash node unhashed.

Fixes 1d36a36f6d53 ("ovpn: implement peer add/get/dump/delete via netlink")

Signed-off-by Ralf Lici <[email protected]>

---
 drivers/net/ovpn/peer.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

drivers/net/ovpn/peer.c +4 -6

index a21d02ac715e..6b1f176433d9 100644
@@ -906,10 +906,11 @@ void ovpn_peer_hash_vpn_ip(struct ovpn_peer *peer)
 	if (peer->ovpn->mode != OVPN_MODE_MP)
 		return;
 
-	if (peer->vpn_addrs.ipv4.s_addr != htonl(INADDR_ANY)) {
-		/* remove potential old hashing */
-		hlist_nulls_del_init_rcu(&peer->hash_entry_addr4);
+	/* remove potential old hashing */
+	hlist_nulls_del_init_rcu(&peer->hash_entry_addr4);
+	hlist_nulls_del_init_rcu(&peer->hash_entry_addr6);
 
+	if (peer->vpn_addrs.ipv4.s_addr != htonl(INADDR_ANY)) {
 		nhead = ovpn_get_hash_head(peer->ovpn->peers->by_vpn_addr4,
 					   &peer->vpn_addrs.ipv4,
 					   sizeof(peer->vpn_addrs.ipv4));

@@ -917,9 +918,6 @@ void ovpn_peer_hash_vpn_ip(struct ovpn_peer *peer)
 	}
 
 	if (!ipv6_addr_any(&peer->vpn_addrs.ipv6)) {
-		/* remove potential old hashing */
-		hlist_nulls_del_init_rcu(&peer->hash_entry_addr6);
-
 		nhead = ovpn_get_hash_head(peer->ovpn->peers->by_vpn_addr6,
 					   &peer->vpn_addrs.ipv6,
 					   sizeof(peer->vpn_addrs.ipv6));

Signature · 8 lines

2.55.0



_______________________________________________
Openvpn-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Ralf Lici

author reviewer

2026-07-29 15:42 UTC #2 lore raw

PATCH 2/5 ovpn: reject duplicate peer VPN addresses

In reply to Ralf Lici

In MP mode, ovpn uses the peer VPN addresses as lookup keys for selecting the peer that should receive an outgoing tunnel packet. However, the netlink peer configuration path does not currently reject duplicate VPN addresses.

If two peers are configured with the same VPN address, both can be inserted in the VPN address hash table and lookups return whichever peer is found first. This makes peer selection ambiguous and dependent on hash insertion order.

Reject peer creation or update when the resulting VPN address is already assigned to another peer. Ignore unspecified addresses because those are not inserted in the VPN address hash tables.

Fixes 1d36a36f6d53 ("ovpn: implement peer add/get/dump/delete via netlink")

Signed-off-by Ralf Lici <[email protected]>

---
 drivers/net/ovpn/netlink.c | 37 ++++++++++++++++++++++-------
 drivers/net/ovpn/peer.c    | 48 +++++++++++++++++++++++++++++++++++++-
 drivers/net/ovpn/peer.h    |  4 ++++
 3 files changed, 80 insertions(+), 9 deletions(-)

drivers/net/ovpn/netlink.c +29 -8

index 4c66c1ec497e..79775af26fda 100644
@@ -474,8 +474,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];
 	struct ovpn_priv *ovpn = info->user_ptr[0];
+	struct nlattr *attrs[OVPN_A_PEER_MAX + 1];
+	struct in6_addr vpn_addr6;
+	struct in_addr vpn_addr4;
 	struct ovpn_socket *sock;
 	struct ovpn_peer *peer;
 	u32 peer_id;

@@ -522,22 +524,41 @@ int ovpn_nl_peer_set_doit(struct sk_buff *skb, struct genl_info *info)
 	rcu_read_unlock();
 
 	spin_lock_bh(&ovpn->lock);
-	ret = ovpn_nl_peer_modify(peer, info, attrs);
-	if (ret < 0) {
-		spin_unlock_bh(&ovpn->lock);
-		ovpn_peer_put(peer);
-		return ret;
+
+	vpn_addr4 = peer->vpn_addrs.ipv4;
+	vpn_addr6 = peer->vpn_addrs.ipv6;
+
+	if (attrs[OVPN_A_PEER_VPN_IPV4])
+		vpn_addr4.s_addr = nla_get_in_addr(attrs[OVPN_A_PEER_VPN_IPV4]);
+
+	if (attrs[OVPN_A_PEER_VPN_IPV6])
+		vpn_addr6 = nla_get_in6_addr(attrs[OVPN_A_PEER_VPN_IPV6]);
+
+	/* reject peer with conflicting VPN address */
+	if ((attrs[OVPN_A_PEER_VPN_IPV4] || attrs[OVPN_A_PEER_VPN_IPV6]) &&
+	    ovpn_peer_vpn_addr_conflict(ovpn, peer, &vpn_addr4, &vpn_addr6)) {
+		NL_SET_ERR_MSG_FMT_MOD(info->extack,
+				       "VPN IP is already assigned to another peer");
+		ret = -EADDRINUSE;
+		goto unlock;
 	}
 
+	ret = ovpn_nl_peer_modify(peer, info, attrs);
+	if (ret < 0)
+		goto unlock;
+
 	/* ret == 1 means that VPN IPv4/6 has been modified and rehashing
 	 * is required
 	 */
-	if (ret > 0)
+	if (ret > 0) {
 		ovpn_peer_hash_vpn_ip(peer);
+		ret = 0;
+	}
+unlock:
 	spin_unlock_bh(&ovpn->lock);
 	ovpn_peer_put(peer);
 
-	return 0;
+	return ret;
 }
 
 static int ovpn_nl_send_peer(struct sk_buff *skb, const struct genl_info *info,

drivers/net/ovpn/peer.c +47 -1

66 lines not on this page · raw message

drivers/net/ovpn/peer.h +4 -0

index 86c8cffada6d..6f7df9ae8fe4 100644
@@ -149,6 +149,10 @@ struct ovpn_peer *ovpn_peer_get_by_transp_addr(struct ovpn_priv *ovpn,
 struct ovpn_peer *ovpn_peer_get_by_id(struct ovpn_priv *ovpn, u32 peer_id);
 struct ovpn_peer *ovpn_peer_get_by_dst(struct ovpn_priv *ovpn,
 				       struct sk_buff *skb);
+bool ovpn_peer_vpn_addr_conflict(struct ovpn_priv *ovpn,
+				 const struct ovpn_peer *peer,
+				 const struct in_addr *addr4,
+				 const struct in6_addr *addr6);
 void ovpn_peer_hash_vpn_ip(struct ovpn_peer *peer);
 bool ovpn_peer_check_by_src(struct ovpn_priv *ovpn, struct sk_buff *skb,
 			    struct ovpn_peer *peer);

Signature · 8 lines

2.55.0



_______________________________________________
Openvpn-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Sabrina Dubroca

reviewer

2026-08-14 09:52 UTC #3 lore raw

Re: [Openvpn-devel] [PATCH ovpn net 2/5] ovpn: reject duplicate peer VPN addresses

Quoted 5 lines · Ralf Lici

In MP mode, ovpn uses the peer VPN addresses as lookup keys for selecting the peer that should receive an outgoing tunnel packet. However, the netlink peer configuration path does not currently reject duplicate VPN addresses.

Quoted 8 lines

If two peers are configured with the same VPN address, both can be inserted in the VPN address hash table and lookups return whichever peer is found first. This makes peer selection ambiguous and dependent on hash insertion order.
Reject peer creation or update when the resulting VPN address is already assigned to another peer. Ignore unspecified addresses because those are not inserted in the VPN address hash tables.

Maybe mention that it's a user-visible change of behavior, but it doesn't matter since that setup has never worked.

Quoted 17 lines

drivers/net/ovpn/netlink.c +8 -5

@@ -522,22 +524,41 @@ int ovpn_nl_peer_set_doit(struct sk_buff *skb, struct genl_info *info)
 	rcu_read_unlock();
 
 	spin_lock_bh(&ovpn->lock);
-	ret = ovpn_nl_peer_modify(peer, info, attrs);
-	if (ret < 0) {
-		spin_unlock_bh(&ovpn->lock);
-		ovpn_peer_put(peer);
-		return ret;
+
+	vpn_addr4 = peer->vpn_addrs.ipv4;
+	vpn_addr6 = peer->vpn_addrs.ipv6;
+
+	if (attrs[OVPN_A_PEER_VPN_IPV4])
+		vpn_addr4.s_addr = nla_get_in_addr(attrs[OVPN_A_PEER_VPN_IPV4]);
+
+	if (attrs[OVPN_A_PEER_VPN_IPV6])

drivers/net/ovpn/netlink.c int ovpn_nl_peer_set_doit(struct sk_buff *skb, struct genl_info *info) · line 544

Quoted 10 lines

drivers/net/ovpn/netlink.c +9 -0

+		vpn_addr6 = nla_get_in6_addr(attrs[OVPN_A_PEER_VPN_IPV6]);
+
+	/* reject peer with conflicting VPN address */
+	if ((attrs[OVPN_A_PEER_VPN_IPV4] || attrs[OVPN_A_PEER_VPN_IPV6]) &&
+	    ovpn_peer_vpn_addr_conflict(ovpn, peer, &vpn_addr4, &vpn_addr6)) {
+		NL_SET_ERR_MSG_FMT_MOD(info->extack,
+				       "VPN IP is already assigned to another peer");
+		ret = -EADDRINUSE;
+		goto unlock;
 	}

nit: maybe splitting conflict() into conflict{4,6}() would be a bit cleaner? passing addresses of 2 families feels kind of strange.

	if (attrs[OVPN_A_PEER_VPN_IPV4]) {
		struct in_addr vpn_addr4 = {
			.s_addr = nla_get_in_addr(attrs[OVPN_A_PEER_VPN_IPV4]),
		};
		if (ovpn_peer_vpn_addr_conflict4(ovpn, peer, &vpn_addr4))
			goto addr_conflict;
	}

And then conflict* looks slightly nicer too:

bool ovpn_peer_vpn_addr_conflict4(struct ovpn_priv *ovpn,
const struct ovpn_peer *peer, const struct in_addr *addr)

{
	struct ovpn_peer *tmp = NULL;

	lockdep_assert_held(&ovpn->lock);

	/* we don't hash INADDR_ANY, no conflict in that case */
	if (addr->s_addr != htonl(INADDR_ANY))
		tmp = ovpn_peer_get_by_vpn_addr4(ovpn, addr->s_addr);

	return tmp && tmp != peer;
}

Signature · 7 lines

Sabrina


_______________________________________________
Openvpn-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Ralf Lici

author reviewer

2026-08-14 15:32 UTC #4 lore raw

Re: [Openvpn-devel] [PATCH ovpn net 2/5] ovpn: reject duplicate peer VPN addresses

Quoted 11 lines · Sabrina Dubroca

Quoted 10 lines · Ralf Lici
In MP mode, ovpn uses the peer VPN addresses as lookup keys for selecting the peer that should receive an outgoing tunnel packet. However, the netlink peer configuration path does not currently reject duplicate VPN addresses.
If two peers are configured with the same VPN address, both can be inserted in the VPN address hash table and lookups return whichever peer is found first. This makes peer selection ambiguous and dependent on hash insertion order.

Quoted 8 lines

Quoted 3 lines
Reject peer creation or update when the resulting VPN address is already assigned to another peer. Ignore unspecified addresses because those are not inserted in the VPN address hash tables.
Maybe mention that it's a user-visible change of behavior, but it doesn't matter since that setup has never worked.

Will do.

Quoted 45 lines

Quoted 17 lines

drivers/net/ovpn/netlink.c +8 -5

@@ -522,22 +524,41 @@ int ovpn_nl_peer_set_doit(struct sk_buff *skb, struct genl_info *info)
 	rcu_read_unlock();
 
 	spin_lock_bh(&ovpn->lock);
-	ret = ovpn_nl_peer_modify(peer, info, attrs);
-	if (ret < 0) {
-		spin_unlock_bh(&ovpn->lock);
-		ovpn_peer_put(peer);
-		return ret;
+
+	vpn_addr4 = peer->vpn_addrs.ipv4;
+	vpn_addr6 = peer->vpn_addrs.ipv6;
+
+	if (attrs[OVPN_A_PEER_VPN_IPV4])
+		vpn_addr4.s_addr = nla_get_in_addr(attrs[OVPN_A_PEER_VPN_IPV4]);
+
+	if (attrs[OVPN_A_PEER_VPN_IPV6])

drivers/net/ovpn/netlink.c int ovpn_nl_peer_set_doit(struct sk_buff *skb, struct genl_info *info) · line 544

Quoted 10 lines

drivers/net/ovpn/netlink.c +9 -0

+		vpn_addr6 = nla_get_in6_addr(attrs[OVPN_A_PEER_VPN_IPV6]);
+
+	/* reject peer with conflicting VPN address */
+	if ((attrs[OVPN_A_PEER_VPN_IPV4] || attrs[OVPN_A_PEER_VPN_IPV6]) &&
+	    ovpn_peer_vpn_addr_conflict(ovpn, peer, &vpn_addr4, &vpn_addr6)) {
+		NL_SET_ERR_MSG_FMT_MOD(info->extack,
+				       "VPN IP is already assigned to another peer");
+		ret = -EADDRINUSE;
+		goto unlock;
 	}

nit: maybe splitting conflict() into conflict{4,6}() would be a bit cleaner? passing addresses of 2 families feels kind of strange.

	if (attrs[OVPN_A_PEER_VPN_IPV4]) {
		struct in_addr vpn_addr4 = {
			.s_addr = nla_get_in_addr(attrs[OVPN_A_PEER_VPN_IPV4]),
		};
		if (ovpn_peer_vpn_addr_conflict4(ovpn, peer, &vpn_addr4))
			goto addr_conflict;
	}

And then conflict* looks slightly nicer too:

bool ovpn_peer_vpn_addr_conflict4(struct ovpn_priv *ovpn,
const struct ovpn_peer *peer, const struct in_addr *addr)

Quoted 12 lines

{
	struct ovpn_peer *tmp = NULL;

	lockdep_assert_held(&ovpn->lock);

	/* we don't hash INADDR_ANY, no conflict in that case */
	if (addr->s_addr != htonl(INADDR_ANY))
		tmp = ovpn_peer_get_by_vpn_addr4(ovpn, addr->s_addr);

	return tmp && tmp != peer;
}

Yes, it's a nice refactoring. I'll apply it to v2.

Signature · 8 lines

Ralf Lici
Mandelbit Srl


_______________________________________________
Openvpn-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Ralf Lici

author reviewer

2026-07-29 15:42 UTC #5 lore raw

PATCH 3/5 ovpn: reject multipeer peers without VPN addresses

In reply to Ralf Lici

In MP mode, ovpn uses the peer VPN addresses to select the peer for outgoing tunnel packets. Peer creation currently requires a VPN IPv4 or IPv6 attribute, but it only checks for the presence of the attribute and not for a usable address value.

This allows userspace to create an MP peer with only unspecified VPN addresses, or to update an existing peer so that both VPN address families become unspecified. Such a peer cannot be selected through the VPN address hash tables.

Reject MP peer creation or update when the resulting peer would not have at least one VPN address configured.

Fixes 1d36a36f6d53 ("ovpn: implement peer add/get/dump/delete via netlink")

Signed-off-by Ralf Lici <[email protected]>

---
 drivers/net/ovpn/netlink.c | 47 +++++++++++++++++++++++++++-----------
 1 file changed, 34 insertions(+), 13 deletions(-)

drivers/net/ovpn/netlink.c +34 ~~-13~~

index 79775af26fda..43e6c7a29a6f 100644
@@ -346,8 +346,10 @@ static int ovpn_nl_peer_modify(struct ovpn_peer *peer, struct genl_info *info,
 
 int ovpn_nl_peer_new_doit(struct sk_buff *skb, struct genl_info *info)
 {
-	struct nlattr *attrs[OVPN_A_PEER_MAX + 1];
+	struct in_addr vpn_addr4 = { .s_addr = INADDR_ANY };
+	struct in6_addr vpn_addr6 = IN6ADDR_ANY_INIT;
 	struct ovpn_priv *ovpn = info->user_ptr[0];
+	struct nlattr *attrs[OVPN_A_PEER_MAX + 1];
 	struct ovpn_socket *ovpn_sock;
 	struct socket *sock = NULL;
 	struct ovpn_peer *peer;

@@ -371,11 +373,19 @@ int ovpn_nl_peer_new_doit(struct sk_buff *skb, struct genl_info *info)
 		return -EINVAL;
 
 	/* in MP mode VPN IPs are required for selecting the right peer */
-	if (ovpn->mode == OVPN_MODE_MP && !attrs[OVPN_A_PEER_VPN_IPV4] &&
-	    !attrs[OVPN_A_PEER_VPN_IPV6]) {
-		NL_SET_ERR_MSG_FMT_MOD(info->extack,
-				       "VPN IP must be provided in MP mode");
-		return -EINVAL;
+	if (ovpn->mode == OVPN_MODE_MP) {
+		if (attrs[OVPN_A_PEER_VPN_IPV4])
+			vpn_addr4.s_addr =
+				nla_get_in_addr(attrs[OVPN_A_PEER_VPN_IPV4]);
+		if (attrs[OVPN_A_PEER_VPN_IPV6])
+			vpn_addr6 =
+				nla_get_in6_addr(attrs[OVPN_A_PEER_VPN_IPV6]);
+
+		if (!vpn_addr4.s_addr && ipv6_addr_any(&vpn_addr6)) {
+			NL_SET_ERR_MSG_FMT_MOD(info->extack,
+					       "VPN IP must be provided in MP mode");
+			return -EINVAL;
+		}
 	}
 
 	peer_id = nla_get_u32(attrs[OVPN_A_PEER_ID]);

@@ -534,13 +544,24 @@ int ovpn_nl_peer_set_doit(struct sk_buff *skb, struct genl_info *info)
 	if (attrs[OVPN_A_PEER_VPN_IPV6])
 		vpn_addr6 = nla_get_in6_addr(attrs[OVPN_A_PEER_VPN_IPV6]);
 
-	/* reject peer with conflicting VPN address */
-	if ((attrs[OVPN_A_PEER_VPN_IPV4] || attrs[OVPN_A_PEER_VPN_IPV6]) &&
-	    ovpn_peer_vpn_addr_conflict(ovpn, peer, &vpn_addr4, &vpn_addr6)) {
-		NL_SET_ERR_MSG_FMT_MOD(info->extack,
-				       "VPN IP is already assigned to another peer");
-		ret = -EADDRINUSE;
-		goto unlock;
+	/* in MP mode VPN IPs are required for selecting the right peer */
+	if (ovpn->mode == OVPN_MODE_MP &&
+	    (attrs[OVPN_A_PEER_VPN_IPV4] || attrs[OVPN_A_PEER_VPN_IPV6])) {
+		if (!vpn_addr4.s_addr && ipv6_addr_any(&vpn_addr6)) {
+			NL_SET_ERR_MSG_FMT_MOD(info->extack,
+					       "MP peer must have at least one valid VPN IP");
+			ret = -EINVAL;
+			goto unlock;
+		}
+
+		/* reject peer with conflicting VPN address */
+		if (ovpn_peer_vpn_addr_conflict(ovpn, peer, &vpn_addr4,
+						&vpn_addr6)) {
+			NL_SET_ERR_MSG_FMT_MOD(info->extack,
+					       "VPN IP is already assigned to another peer");
+			ret = -EADDRINUSE;
+			goto unlock;
+		}
 	}
 
 	ret = ovpn_nl_peer_modify(peer, info, attrs);

Signature · 8 lines

2.55.0



_______________________________________________
Openvpn-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Sabrina Dubroca

reviewer

2026-08-14 10:00 UTC #6 lore raw

Re: [Openvpn-devel] [PATCH ovpn net 3/5] ovpn: reject multipeer peers without VPN addresses

Quoted 5 lines · Ralf Lici

In MP mode, ovpn uses the peer VPN addresses to select the peer for outgoing tunnel packets. Peer creation currently requires a VPN IPv4 or IPv6 attribute, but it only checks for the presence of the attribute and not for a usable address value.

Quoted 7 lines

This allows userspace to create an MP peer with only unspecified VPN addresses, or to update an existing peer so that both VPN address families become unspecified. Such a peer cannot be selected through the VPN address hash tables.
Reject MP peer creation or update when the resulting peer would not have at least one VPN address configured.

Same comment about "this has never worked".

Quoted 8 lines

drivers/net/ovpn/netlink.c +0 -4

@@ -371,11 +373,19 @@ int ovpn_nl_peer_new_doit(struct sk_buff *skb, struct genl_info *info)
 		return -EINVAL;
 
 	/* in MP mode VPN IPs are required for selecting the right peer */
-	if (ovpn->mode == OVPN_MODE_MP && !attrs[OVPN_A_PEER_VPN_IPV4] &&
-	    !attrs[OVPN_A_PEER_VPN_IPV6]) {
-		NL_SET_ERR_MSG_FMT_MOD(info->extack,
-				       "VPN IP must be provided in MP mode");

drivers/net/ovpn/netlink.c int ovpn_nl_peer_new_doit(struct sk_buff *skb, struct genl_info *info) · line 384

Quoted 10 lines

drivers/net/ovpn/netlink.c +9 -1

-		return -EINVAL;
+	if (ovpn->mode == OVPN_MODE_MP) {
+		if (attrs[OVPN_A_PEER_VPN_IPV4])
+			vpn_addr4.s_addr =
+				nla_get_in_addr(attrs[OVPN_A_PEER_VPN_IPV4]);
+		if (attrs[OVPN_A_PEER_VPN_IPV6])
+			vpn_addr6 =
+				nla_get_in6_addr(attrs[OVPN_A_PEER_VPN_IPV6]);
+
+		if (!vpn_addr4.s_addr && ipv6_addr_any(&vpn_addr6)) {

minor nit, only if you end up resending this patch: the inconsistency between !s_addr and != htonl(INADDR_ANY) isn't great. (but I don't think it's confusing anyone)

Signature · 7 lines

Sabrina


_______________________________________________
Openvpn-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Ralf Lici

author reviewer

2026-08-14 15:35 UTC #7 lore raw

Re: [Openvpn-devel] [PATCH ovpn net 3/5] ovpn: reject multipeer peers without VPN addresses

Quoted 11 lines · Sabrina Dubroca

Quoted 10 lines · Ralf Lici
In MP mode, ovpn uses the peer VPN addresses to select the peer for outgoing tunnel packets. Peer creation currently requires a VPN IPv4 or IPv6 attribute, but it only checks for the presence of the attribute and not for a usable address value.
This allows userspace to create an MP peer with only unspecified VPN addresses, or to update an existing peer so that both VPN address families become unspecified. Such a peer cannot be selected through the VPN address hash tables.

Quoted 5 lines

Quoted 2 lines
Reject MP peer creation or update when the resulting peer would not have at least one VPN address configured.
Same comment about "this has never worked".

ACK.

Quoted 14 lines

drivers/net/ovpn/netlink.c +5 -5

@@ -371,11 +373,19 @@ int ovpn_nl_peer_new_doit(struct sk_buff *skb, struct genl_info *info)
 		return -EINVAL;
 
 	/* in MP mode VPN IPs are required for selecting the right peer */
-	if (ovpn->mode == OVPN_MODE_MP && !attrs[OVPN_A_PEER_VPN_IPV4] &&
-	    !attrs[OVPN_A_PEER_VPN_IPV6]) {
-		NL_SET_ERR_MSG_FMT_MOD(info->extack,
-				       "VPN IP must be provided in MP mode");
-		return -EINVAL;
+	if (ovpn->mode == OVPN_MODE_MP) {
+		if (attrs[OVPN_A_PEER_VPN_IPV4])
+			vpn_addr4.s_addr =
+				nla_get_in_addr(attrs[OVPN_A_PEER_VPN_IPV4]);
+		if (attrs[OVPN_A_PEER_VPN_IPV6])

Quoted 9 lines

drivers/net/ovpn/netlink.c int ovpn_nl_peer_new_doit(struct sk_buff *skb, struct genl_info *info) · line 384
Quoted 4 lines
drivers/net/ovpn/netlink.c +4 -0
+			vpn_addr6 =
+				nla_get_in6_addr(attrs[OVPN_A_PEER_VPN_IPV6]);
+
+		if (!vpn_addr4.s_addr && ipv6_addr_any(&vpn_addr6)) {
minor nit, only if you end up resending this patch: the inconsistency between !s_addr and != htonl(INADDR_ANY) isn't great. (but I don't think it's confusing anyone)

Ah right, I missed this one. I'll resend the series anyway so I'll make sure to use != htonl(INADDR_ANY) here as well.

Signature · 8 lines

Ralf Lici
Mandelbit Srl


_______________________________________________
Openvpn-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Ralf Lici

author reviewer

2026-07-29 15:42 UTC #8 lore raw

PATCH 4/5 ovpn: reject invalid peer VPN addresses

In reply to Ralf Lici

In MP mode, ovpn uses peer VPN addresses as lookup keys for selecting the peer that should receive outgoing tunnel packets. The netlink configuration path currently accepts address values that cannot sensibly identify a VPN peer, such as multicast, broadcast or loopback addresses.

Reject invalid peer VPN addresses when creating or updating an MP peer. Keep accepting the unspecified address as the internal unset value, provided that at least one VPN address family remains configured.

Fixes 1d36a36f6d53 ("ovpn: implement peer add/get/dump/delete via netlink")

Signed-off-by Ralf Lici <[email protected]>

---
 drivers/net/ovpn/netlink.c | 50 ++++++++++++++++++++++++++++++--------
 1 file changed, 40 insertions(+), 10 deletions(-)

drivers/net/ovpn/netlink.c +40 ~~-10~~

index 43e6c7a29a6f..333e9237e594 100644
@@ -179,6 +179,39 @@ static sa_family_t ovpn_nl_family_get(struct nlattr *addr4,
 	return AF_UNSPEC;
 }
 
+static int ovpn_nl_peer_check_vpn_addrs(const struct in_addr *addr4,
+					const struct in6_addr *addr6,
+					struct genl_info *info)
+{
+	int addr6_type;
+
+	if (addr4->s_addr == htonl(INADDR_ANY) && ipv6_addr_any(addr6)) {
+		NL_SET_ERR_MSG_MOD(info->extack,
+				   "at least one VPN IP must be configured");
+		return -EINVAL;
+	}
+
+	if (ipv4_is_multicast(addr4->s_addr) || ipv4_is_lbcast(addr4->s_addr) ||
+	    ipv4_is_loopback(addr4->s_addr)) {
+		NL_SET_ERR_MSG_MOD(info->extack,
+				   "VPN IPv4 address must be valid unicast or any");
+		return -EADDRNOTAVAIL;
+	}
+
+	if (!ipv6_addr_any(addr6)) {
+		addr6_type = ipv6_addr_type(addr6);
+
+		if (!(addr6_type & IPV6_ADDR_UNICAST) ||
+		    (addr6_type & (IPV6_ADDR_LOOPBACK | IPV6_ADDR_COMPATv4))) {
+			NL_SET_ERR_MSG_MOD(info->extack,
+					   "VPN IPv6 address must be valid unicast or any");
+			return -EADDRNOTAVAIL;
+		}
+	}
+
+	return 0;
+}
+
 static int ovpn_nl_peer_precheck(struct ovpn_priv *ovpn,
 				 struct genl_info *info,
 				 struct nlattr **attrs)

@@ -381,11 +414,10 @@ int ovpn_nl_peer_new_doit(struct sk_buff *skb, struct genl_info *info)
 			vpn_addr6 =
 				nla_get_in6_addr(attrs[OVPN_A_PEER_VPN_IPV6]);
 
-		if (!vpn_addr4.s_addr && ipv6_addr_any(&vpn_addr6)) {
-			NL_SET_ERR_MSG_FMT_MOD(info->extack,
-					       "VPN IP must be provided in MP mode");
-			return -EINVAL;
-		}
+		ret = ovpn_nl_peer_check_vpn_addrs(&vpn_addr4, &vpn_addr6,
+						   info);
+		if (ret < 0)
+			return ret;
 	}
 
 	peer_id = nla_get_u32(attrs[OVPN_A_PEER_ID]);

@@ -547,12 +579,10 @@ int ovpn_nl_peer_set_doit(struct sk_buff *skb, struct genl_info *info)
 	/* in MP mode VPN IPs are required for selecting the right peer */
 	if (ovpn->mode == OVPN_MODE_MP &&
 	    (attrs[OVPN_A_PEER_VPN_IPV4] || attrs[OVPN_A_PEER_VPN_IPV6])) {
-		if (!vpn_addr4.s_addr && ipv6_addr_any(&vpn_addr6)) {
-			NL_SET_ERR_MSG_FMT_MOD(info->extack,
-					       "MP peer must have at least one valid VPN IP");
-			ret = -EINVAL;
+		ret = ovpn_nl_peer_check_vpn_addrs(&vpn_addr4, &vpn_addr6,
+						   info);
+		if (ret < 0)
 			goto unlock;
-		}
 
 		/* reject peer with conflicting VPN address */
 		if (ovpn_peer_vpn_addr_conflict(ovpn, peer, &vpn_addr4,

Signature · 8 lines

2.55.0



_______________________________________________
Openvpn-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Ralf Lici

author reviewer

2026-07-29 15:42 UTC #9 lore raw

PATCH 5/5 selftests: ovpn: validate peer VPN addresses

In reply to Ralf Lici

Exercise peer VPN address validation through both peer creation and update. Check missing, unspecified, duplicate, multicast, broadcast, loopback, IPv4-compatible and IPv4-mapped addresses.

Temporarily configure a peer with both address families to verify that either family can be cleared while the other remains configured, then restore the original addresses before running the existing traffic tests.

Extend ovpn-cli peer updates with an optional VPN address and preserve peer creation errors so the negative tests can observe rejected netlink requests.

Signed-off-by Ralf Lici <[email protected]>

---
 tools/testing/selftests/net/ovpn/common.sh  | 13 ++++
 tools/testing/selftests/net/ovpn/ovpn-cli.c | 54 ++++++++++-----
 tools/testing/selftests/net/ovpn/test.sh    | 75 ++++++++++++++++++++-
 3 files changed, 123 insertions(+), 19 deletions(-)

tools/testing/selftests/net/ovpn/common.sh +13 -0

index 2d844eb3aa6e..5e9c81e885e6 100644
@@ -136,6 +136,19 @@ ovpn_create_ns() {
 	ip netns add "ovpn_peer${1}"
 }
 
+ovpn_peer_vpn_addr() {
+	local peer="$1"
+	local file
+
+	if [ "${OVPN_PROTO}" == "UDP" ]; then
+		file="${OVPN_UDP_PEERS_FILE}"
+	else
+		file="${OVPN_TCP_PEERS_FILE}"
+	fi
+
+	awk -v peer="${peer}" '$1 == peer {print $NF; exit}' "${file}"
+}
+
 ovpn_setup_ns() {
 	local peer="ovpn_peer${1}"
 	local server_ns="ovpn_peer0"

tools/testing/selftests/net/ovpn/ovpn-cli.c +37 ~~-17~~

96 lines not on this page · raw message

tools/testing/selftests/net/ovpn/test.sh +73 -2

94 lines not on this page · raw message

Signature · 8 lines

2.55.0



_______________________________________________
Openvpn-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel