From: Antonio Quartulli <[email protected]> The previous patches bound resource use within a single ovpn interface, but a userns owner can still spin many MP-mode devices, each costing ~128 KiB just for its peer hashtables.
Cap the number of ovpn netdevs per netns with a compile-time constant (OVPN_MAX_DEVS, 256). The counter lives in a per-netns struct registered via register_pernet_subsys() - the framework zero-allocates and frees the storage, so no init/exit callbacks are needed. ovpn_net_init() increments and rejects with -ENOSPC past the cap; ovpn_net_uninit() decrements, keeping the counter balanced across both ndo_init failure and normal teardown. Also set dev->netns_immutable in ovpn_setup() to forbid moving an ovpn device between netns. Without it, ndo_uninit would decrement a different netns' counter than ndo_init incremented, eventually wrapping the atomic_t and defeating the cap - a userns owner with multiple child netns could trigger this on purpose. A sysctl was rejected: net.* sysctls in a non-init netns are writable by anyone with CAP_NET_ADMIN in that netns, i.e. the exact actor this cap constrains. A compile-time constant cannot be raised by an attacker; 256 is generous for any realistic deployment. Signed-off-by: Antonio Quartulli <[email protected]> --- drivers/net/ovpn/main.c | 71 ++++++++++++++++++++++++++++++++++++----- 1 file changed, 63 insertions(+), 8 deletions(-) diff --git a/drivers/net/ovpn/main.c b/drivers/net/ovpn/main.c index 425f4367f0ab..540ba89d81b7 100644 --- a/drivers/net/ovpn/main.c +++ b/drivers/net/ovpn/main.c @@ -14,6 +14,8 @@ #include <linux/inetdevice.h> #include <net/gro_cells.h> #include <net/ip.h> +#include <net/net_namespace.h> +#include <net/netns/generic.h> #include <net/rtnetlink.h> #include <uapi/linux/if_arp.h> @@ -26,6 +28,29 @@ #include "tcp.h" #include "udp.h" +/* Hard cap on number of ovpn netdevs per network namespace. Pairs with the + * per-MP-instance OVPN_MAX_PEERS cap to bound total kernel memory an + * unprivileged userns owner can pin via repeated `ip link add ... type ovpn` + * calls (each MP-mode device costs ~128 KiB just for its peer hashtables). + * + * This is a compile-time constant on purpose: a sysctl in net.* would be + * writable by anyone with CAP_NET_ADMIN in the namespace - i.e. exactly the + * unprivileged userns owner this cap is meant to constrain - which would + * defeat the defence. 256 is generous for any realistic deployment. + */ +#define OVPN_MAX_DEVS 256 + +struct ovpn_net { + atomic_t n_devs; +}; + +static unsigned int ovpn_net_id __read_mostly; + +static struct pernet_operations ovpn_pernet_ops = { + .id = &ovpn_net_id, + .size = sizeof(struct ovpn_net), +}; + static void ovpn_priv_free(struct net_device *net) { struct ovpn_priv *ovpn = netdev_priv(net); @@ -74,27 +99,40 @@ static int ovpn_mp_alloc(struct ovpn_priv *ovpn) static int ovpn_net_init(struct net_device *dev) { struct ovpn_priv *ovpn = netdev_priv(dev); - int err = gro_cells_init(&ovpn->gro_cells, dev); + struct ovpn_net *on = net_generic(dev_net(dev), ovpn_net_id); + int err; + + if (atomic_fetch_inc(&on->n_devs) >= OVPN_MAX_DEVS) { + atomic_dec(&on->n_devs); + return -ENOSPC; + } + err = gro_cells_init(&ovpn->gro_cells, dev); if (err < 0) - return err; + goto err_dec; err = ovpn_mp_alloc(ovpn); - if (err < 0) { - gro_cells_destroy(&ovpn->gro_cells); - return err; - } + if (err < 0) + goto err_gro; return 0; + +err_gro: + gro_cells_destroy(&ovpn->gro_cells); +err_dec: + atomic_dec(&on->n_devs); + return err; } static void ovpn_net_uninit(struct net_device *dev) { struct ovpn_priv *ovpn = netdev_priv(dev); + struct ovpn_net *on = net_generic(dev_net(dev), ovpn_net_id); disable_delayed_work_sync(&ovpn->keepalive_work); ovpn_peers_free(ovpn, NULL, OVPN_DEL_PEER_REASON_TEARDOWN); gro_cells_destroy(&ovpn->gro_cells); + atomic_dec(&on->n_devs); } static const struct net_device_ops ovpn_netdev_ops = { @@ -173,6 +211,14 @@ static void ovpn_setup(struct net_device *dev) dev->needed_headroom = ALIGN(OVPN_HEAD_ROOM, 4); dev->needed_tailroom = OVPN_MAX_PADDING; + /* forbid moving the device between network namespaces: ndo_init and + * ndo_uninit are called in the originating and current netns + * respectively, so a migrated device would dec a different netns' + * n_devs counter than the one it incremented, eventually wrapping it + * and defeating the per-netns cap. + */ + dev->netns_immutable = true; + SET_NETDEV_DEVTYPE(dev, &ovpn_type); } @@ -233,13 +279,19 @@ static struct rtnl_link_ops ovpn_link_ops = { static int __init ovpn_init(void) { - int err = rtnl_link_register(&ovpn_link_ops); + int err = register_pernet_subsys(&ovpn_pernet_ops); if (err) { - pr_err("ovpn: can't register rtnl link ops: %d\n", err); + pr_err("ovpn: can't register pernet ops: %d\n", err); return err; } + err = rtnl_link_register(&ovpn_link_ops); + if (err) { + pr_err("ovpn: can't register rtnl link ops: %d\n", err); + goto unreg_pernet; + } + err = ovpn_nl_register(); if (err) { pr_err("ovpn: can't register netlink family: %d\n", err); @@ -252,6 +304,8 @@ static int __init ovpn_init(void) unreg_rtnl: rtnl_link_unregister(&ovpn_link_ops); +unreg_pernet: + unregister_pernet_subsys(&ovpn_pernet_ops); return err; } @@ -259,6 +313,7 @@ static __exit void ovpn_cleanup(void) { ovpn_nl_unregister(); rtnl_link_unregister(&ovpn_link_ops); + unregister_pernet_subsys(&ovpn_pernet_ops); rcu_barrier(); } -- 2.53.0 _______________________________________________ Openvpn-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/openvpn-devel
