2026-07-29, 17:37:40 +0200, Ralf Lici wrote:
> 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.
Maybe mention that it's a user-visible change of behavior, but it
doesn't matter since that setup has never worked.
> @@ -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;
> }
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;
}
--
Sabrina
_______________________________________________
Openvpn-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
