[PATCH 0/2] net: openvswitch: metering and conntrack in userns

[PATCH 0/2] net: openvswitch: metering and conntrack in userns

[Michael Weiß]

Currently using openvswitch in a non-inital 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 unprivilegd 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.

Applies against v6.0-rc4

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 |  8 ++++----
 net/openvswitch/meter.c     | 12 ++++++------
 2 files changed, 10 insertions(+), 10 deletions(-)

-- 
2.30.2

[PATCH 1/2] net: openvswitch: allow metering in non-initial user namespace

[Michael Weiß]

The Netlink interface for metering was restricted to global CAP_NET_ADMIN
by using GENL_ADMIN_PERM. To allow metring in a non-inital user namespace,
e.g., a container, this is changed to GENL_UNS_ADMIN_PERM.

Signed-off-by: Michael Weiß <michael.weiss@aisec.fraunhofer.de>
---
 net/openvswitch/meter.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/net/openvswitch/meter.c b/net/openvswitch/meter.c
index 04a060ac7fdf..e9ef050a0dd5 100644
--- a/net/openvswitch/meter.c
+++ b/net/openvswitch/meter.c
@@ -687,9 +687,9 @@ static const struct genl_small_ops dp_meter_genl_ops[] = {
 	},
 	{ .cmd = OVS_METER_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_meter_cmd_set,
 	},
 	{ .cmd = OVS_METER_CMD_GET,
@@ -699,9 +699,9 @@ static const struct genl_small_ops dp_meter_genl_ops[] = {
 	},
 	{ .cmd = OVS_METER_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_meter_cmd_del
 	},
 };
-- 
2.30.2

Re: [PATCH 1/2] net: openvswitch: allow metering in non-initial user namespace

[Paolo Abeni]

On Sun, 2022-09-11 at 19:38 +0200, Michael Weiß wrote:
> The Netlink interface for metering was restricted to global CAP_NET_ADMIN
> by using GENL_ADMIN_PERM. To allow metring in a non-inital user namespace,
> e.g., a container, this is changed to GENL_UNS_ADMIN_PERM.
> 
> Signed-off-by: Michael Weiß <michael.weiss@aisec.fraunhofer.de>
> ---
>  net/openvswitch/meter.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/net/openvswitch/meter.c b/net/openvswitch/meter.c
> index 04a060ac7fdf..e9ef050a0dd5 100644
> --- a/net/openvswitch/meter.c
> +++ b/net/openvswitch/meter.c
> @@ -687,9 +687,9 @@ static const struct genl_small_ops dp_meter_genl_ops[] = {
>  	},
>  	{ .cmd = OVS_METER_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_meter_cmd_set,
>  	},
>  	{ .cmd = OVS_METER_CMD_GET,
> @@ -699,9 +699,9 @@ static const struct genl_small_ops dp_meter_genl_ops[] = {
>  	},
>  	{ .cmd = OVS_METER_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_meter_cmd_del
>  	},
>  };

It looks like the user namespace can allocate quite a bit of memory
with multiple meters, I think it's better additionally change
GFP_KERNEL to GFP_KERNEL_ACCOUNT in dp_meter_create().

Also plese add an explicit target tree in the subj when posting the
next revision, thanks!

Paolo

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

[Michael Weiß]

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ß <michael.weiss@aisec.fraunhofer.de>
---
 net/openvswitch/conntrack.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/net/openvswitch/conntrack.c b/net/openvswitch/conntrack.c
index 4e70df91d0f2..23f5b6261364 100644
--- a/net/openvswitch/conntrack.c
+++ b/net/openvswitch/conntrack.c
@@ -2252,14 +2252,14 @@ 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