2022-09-21 01:47:53

by Michael Weiß

[permalink] [raw]
Subject: [PATCH v2 net 0/2] net: openvswitch: metering and conntrack in userns

Currently using openvswitch in a non-initial user namespace, e.g., an
unprivileged container, is possible but without metering and conntrack
support. This is due to the restriction of the corresponding Netlink
interfaces to the global CAP_NET_ADMIN.

This simple patches switch from GENL_ADMIN_PERM to GENL_UNS_ADMIN_PERM
in several cases to allow this also for the unprivileged container
use case.

We tested this for unprivileged containers created by the container
manager of GyroidOS (gyroidos.github.io). However, for other container
managers such as LXC or systemd which provide unprivileged containers
this should be apply equally.

Changes in v2:
- changed GFP_KERNEL to GFP_KERNEL_ACCOUNT in dp_meter_create()
as suggested by Paolo
- Rebased on net branch of networking tree

Michael Weiß (2):
net: openvswitch: allow metering in non-initial user namespace
net: openvswitch: allow conntrack in non-initial user namespace

net/openvswitch/conntrack.c | 10 ++++++----
net/openvswitch/meter.c | 14 +++++++-------
2 files changed, 13 insertions(+), 11 deletions(-)

--
2.30.2


2022-09-21 01:52:09

by Michael Weiß

[permalink] [raw]
Subject: [PATCH v2 net 2/2] net: openvswitch: allow conntrack in non-initial user namespace

Similar to the previous commit, the Netlink interface of the OVS
conntrack module was restricted to global CAP_NET_ADMIN by using
GENL_ADMIN_PERM. This is changed to GENL_UNS_ADMIN_PERM to support
unprivileged containers in non-initial user namespace.

Signed-off-by: Michael Weiß <[email protected]>
---
net/openvswitch/conntrack.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/net/openvswitch/conntrack.c b/net/openvswitch/conntrack.c
index 4e70df91d0f2..9142ba322991 100644
--- a/net/openvswitch/conntrack.c
+++ b/net/openvswitch/conntrack.c
@@ -2252,14 +2252,16 @@ static int ovs_ct_limit_cmd_get(struct sk_buff *skb, struct genl_info *info)
static const struct genl_small_ops ct_limit_genl_ops[] = {
{ .cmd = OVS_CT_LIMIT_CMD_SET,
.validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
- .flags = GENL_ADMIN_PERM, /* Requires CAP_NET_ADMIN
- * privilege. */
+ .flags = GENL_UNS_ADMIN_PERM, /* Requires CAP_NET_ADMIN
+ * privilege.
+ */
.doit = ovs_ct_limit_cmd_set,
},
{ .cmd = OVS_CT_LIMIT_CMD_DEL,
.validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
- .flags = GENL_ADMIN_PERM, /* Requires CAP_NET_ADMIN
- * privilege. */
+ .flags = GENL_UNS_ADMIN_PERM, /* Requires CAP_NET_ADMIN
+ * privilege.
+ */
.doit = ovs_ct_limit_cmd_del,
},
{ .cmd = OVS_CT_LIMIT_CMD_GET,
--
2.30.2

2022-09-22 15:06:26

by Jakub Kicinski

[permalink] [raw]
Subject: Re: [PATCH v2 net 2/2] net: openvswitch: allow conntrack in non-initial user namespace

On Wed, 21 Sep 2022 03:19:46 +0200 Michael Weiß wrote:
> Similar to the previous commit, the Netlink interface of the OVS
> conntrack module was restricted to global CAP_NET_ADMIN by using
> GENL_ADMIN_PERM. This is changed to GENL_UNS_ADMIN_PERM to support
> unprivileged containers in non-initial user namespace.

Should we bump

ct_limit = kmalloc(sizeof(*ct_limit), GFP_KERNEL);

to also being accounted?

Otherwise LGTM, please repost with [PATCH net-next v3] in the subject.
net is for fixes only, and we're quite late in the -rc process.

Please try to CC the original authors as well, for Joe the address
will be Joe Stringer <[email protected]>.

> diff --git a/net/openvswitch/conntrack.c b/net/openvswitch/conntrack.c
> index 4e70df91d0f2..9142ba322991 100644
> --- a/net/openvswitch/conntrack.c
> +++ b/net/openvswitch/conntrack.c
> @@ -2252,14 +2252,16 @@ static int ovs_ct_limit_cmd_get(struct sk_buff *skb, struct genl_info *info)
> static const struct genl_small_ops ct_limit_genl_ops[] = {
> { .cmd = OVS_CT_LIMIT_CMD_SET,
> .validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
> - .flags = GENL_ADMIN_PERM, /* Requires CAP_NET_ADMIN
> - * privilege. */
> + .flags = GENL_UNS_ADMIN_PERM, /* Requires CAP_NET_ADMIN
> + * privilege.
> + */
> .doit = ovs_ct_limit_cmd_set,
> },
> { .cmd = OVS_CT_LIMIT_CMD_DEL,
> .validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
> - .flags = GENL_ADMIN_PERM, /* Requires CAP_NET_ADMIN
> - * privilege. */
> + .flags = GENL_UNS_ADMIN_PERM, /* Requires CAP_NET_ADMIN
> + * privilege.
> + */
> .doit = ovs_ct_limit_cmd_del,
> },
> { .cmd = OVS_CT_LIMIT_CMD_GET,