[PATCH nft 0/3] netns support

[PATCH nft 0/3] netns support

[Pablo Neira Ayuso]

Hi,

This patchset adds native netns support:

1) Add nft_ctx_set_netns() to libnftables to specify the network
   namespace.

2) Add new -w/--netns option to specify the network namespace from
   the command line.

Pablo Neira Ayuso (3):
  libnftables: add nft_ctx_set_netns()
  main: split parsing from libnftables initialization
  main: add -w/--netns option

 doc/nft.txt                    |   4 ++
 include/namespace.h            |  21 ++++++++
 include/nftables/libnftables.h |   3 ++
 src/Makefile.am                |   1 +
 src/libnftables.c              |  20 ++++++++
 src/libnftables.map            |   4 ++
 src/main.c                     |  53 ++++++++++++--------
 src/namespace.c                | 107 +++++++++++++++++++++++++++++++++++++++++
 8 files changed, 194 insertions(+), 19 deletions(-)
 create mode 100644 include/namespace.h
 create mode 100644 src/namespace.c

-- 
2.11.0

[PATCH nft 1/3] libnftables: add nft_ctx_set_netns()

[Pablo Neira Ayuso]

This new function allows you to specify the net namespace for the
nftables command invocations.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 include/namespace.h            |  21 ++++++++
 include/nftables/libnftables.h |   3 ++
 src/Makefile.am                |   1 +
 src/libnftables.c              |  20 ++++++++
 src/libnftables.map            |   4 ++
 src/namespace.c                | 107 +++++++++++++++++++++++++++++++++++++++++
 6 files changed, 156 insertions(+)
 create mode 100644 include/namespace.h
 create mode 100644 src/namespace.c

diff --git a/include/namespace.h b/include/namespace.h
new file mode 100644
index 000000000000..0aa7524c1525
--- /dev/null
+++ b/include/namespace.h
@@ -0,0 +1,21 @@
+#ifndef __NAMESPACE_H__
+#define __NAMESPACE_H__
+
+#include <sched.h>
+#include <sys/mount.h>
+#include <unistd.h>
+#include <sys/syscall.h>
+#include <errno.h>
+
+/* Use the same default path as iproute2. */
+#ifndef NETNS_RUN_DIR
+#define NETNS_RUN_DIR "/var/run/netns"
+#endif
+
+#ifndef NETNS_ETC_DIR
+#define NETNS_ETC_DIR "/etc/netns"
+#endif
+
+int netns_switch(const char *netns);
+
+#endif /* __NAMESPACE_H__ */
diff --git a/include/nftables/libnftables.h b/include/nftables/libnftables.h
index 765b20dd71ee..887628959ac6 100644
--- a/include/nftables/libnftables.h
+++ b/include/nftables/libnftables.h
@@ -34,10 +34,13 @@ enum nft_debug_level {
  * Possible flags to pass to nft_ctx_new()
  */
 #define NFT_CTX_DEFAULT		0
+#define NFT_CTX_NETNS		1
 
 struct nft_ctx *nft_ctx_new(uint32_t flags);
 void nft_ctx_free(struct nft_ctx *ctx);
 
+int nft_ctx_set_netns(struct nft_ctx *ctx, const char *netns);
+
 bool nft_ctx_get_dry_run(struct nft_ctx *ctx);
 void nft_ctx_set_dry_run(struct nft_ctx *ctx, bool dry);
 
diff --git a/src/Makefile.am b/src/Makefile.am
index 740c21f2cac8..ecd9bb2f1447 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -46,6 +46,7 @@ libnftables_la_SOURCES =			\
 		numgen.c			\
 		ct.c				\
 		xfrm.c				\
+		namespace.c			\
 		netlink.c			\
 		netlink_linearize.c		\
 		netlink_delinearize.c		\
diff --git a/src/libnftables.c b/src/libnftables.c
index cd2fcf2fd522..cd1763657cec 100644
--- a/src/libnftables.c
+++ b/src/libnftables.c
@@ -12,6 +12,7 @@
 #include <parser.h>
 #include <utils.h>
 #include <iface.h>
+#include <namespace.h>
 
 #include <errno.h>
 #include <stdlib.h>
@@ -166,6 +167,25 @@ struct nft_ctx *nft_ctx_new(uint32_t flags)
 	return ctx;
 }
 
+EXPORT_SYMBOL(nft_ctx_set_netns);
+int nft_ctx_set_netns(struct nft_ctx *ctx, const char *netns)
+{
+	int err;
+
+	if (ctx->flags != NFT_CTX_NETNS) {
+		errno = EOPNOTSUPP;
+		return -1;
+	}
+
+	err = netns_switch(netns);
+	if (err < 0)
+		return err;
+
+	nft_ctx_netlink_init(ctx);
+
+	return 0;
+}
+
 static ssize_t cookie_write(void *cptr, const char *buf, size_t buflen)
 {
 	struct cookie *cookie = cptr;
diff --git a/src/libnftables.map b/src/libnftables.map
index 955af3803ee0..bb8bc5de9e59 100644
--- a/src/libnftables.map
+++ b/src/libnftables.map
@@ -23,3 +23,7 @@ global:
 
 local: *;
 };
+
+LIBNFTABLES_2 {
+  nft_ctx_set_netns;
+} LIBNFTABLES_1;
diff --git a/src/namespace.c b/src/namespace.c
new file mode 100644
index 000000000000..3ca465d68808
--- /dev/null
+++ b/src/namespace.c
@@ -0,0 +1,107 @@
+/*
+ * namespace.c
+ *
+ *		This program is free software; you can redistribute it and/or
+ *		modify it under the terms of the GNU General Public License
+ *		as published by the Free Software Foundation; either version
+ *		2 of the License, or (at your option) any later version.
+ *
+ * This code has been extracted from iproute/lib/namespace.c
+ */
+
+#define _GNU_SOURCE
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/statvfs.h>
+#include <fcntl.h>
+#include <dirent.h>
+#include <limits.h>
+#include <sched.h>
+#include <namespace.h>
+
+static void bind_etc(const char *name)
+{
+	char etc_netns_path[sizeof(NETNS_ETC_DIR) + NAME_MAX];
+	char netns_name[PATH_MAX];
+	char etc_name[PATH_MAX];
+	struct dirent *entry;
+	DIR *dir;
+
+	if (strlen(name) >= NAME_MAX)
+		return;
+
+	snprintf(etc_netns_path, sizeof(etc_netns_path), "%s/%s", NETNS_ETC_DIR, name);
+	dir = opendir(etc_netns_path);
+	if (!dir)
+		return;
+
+	while ((entry = readdir(dir)) != NULL) {
+		if (strcmp(entry->d_name, ".") == 0)
+			continue;
+		if (strcmp(entry->d_name, "..") == 0)
+			continue;
+		snprintf(netns_name, sizeof(netns_name), "%s/%s", etc_netns_path, entry->d_name);
+		snprintf(etc_name, sizeof(etc_name), "/etc/%s", entry->d_name);
+		if (mount(netns_name, etc_name, "none", MS_BIND, NULL) < 0) {
+			fprintf(stderr, "Bind %s -> %s failed: %s\n",
+				netns_name, etc_name, strerror(errno));
+		}
+	}
+	closedir(dir);
+}
+
+int netns_switch(const char *name)
+{
+	char net_path[PATH_MAX];
+	int netns;
+	unsigned long mountflags = 0;
+	struct statvfs fsstat;
+
+	snprintf(net_path, sizeof(net_path), "%s/%s", NETNS_RUN_DIR, name);
+	netns = open(net_path, O_RDONLY | O_CLOEXEC);
+	if (netns < 0) {
+		fprintf(stderr, "Cannot open network namespace \"%s\": %s\n",
+			name, strerror(errno));
+		return -1;
+	}
+
+	if (setns(netns, CLONE_NEWNET) < 0) {
+		fprintf(stderr, "setting the network namespace \"%s\" failed: %s\n",
+			name, strerror(errno));
+		close(netns);
+		return -1;
+	}
+	close(netns);
+
+	if (unshare(CLONE_NEWNS) < 0) {
+		fprintf(stderr, "unshare failed: %s\n", strerror(errno));
+		return -1;
+	}
+	/* Don't let any mounts propagate back to the parent */
+	if (mount("", "/", "none", MS_SLAVE | MS_REC, NULL)) {
+		fprintf(stderr, "\"mount --make-rslave /\" failed: %s\n",
+			strerror(errno));
+		return -1;
+	}
+
+	/* Mount a version of /sys that describes the network namespace */
+
+	if (umount2("/sys", MNT_DETACH) < 0) {
+		/* If this fails, perhaps there wasn't a sysfs instance mounted. Good. */
+		if (statvfs("/sys", &fsstat) == 0) {
+			/* We couldn't umount the sysfs, we'll attempt to overlay it.
+			 * A read-only instance can't be shadowed with a read-write one. */
+			if (fsstat.f_flag & ST_RDONLY)
+				mountflags = MS_RDONLY;
+		}
+	}
+	if (mount(name, "/sys", "sysfs", mountflags, NULL) < 0) {
+		fprintf(stderr, "mount of /sys failed: %s\n",strerror(errno));
+		return -1;
+	}
+
+	/* Setup bind mounts for config files in /etc */
+	bind_etc(name);
+	return 0;
+}
-- 
2.11.0

[PATCH nft 2/3] main: split parsing from libnftables initialization

[Pablo Neira Ayuso]

Parse options first, then initialize the nft context object.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 src/main.c | 29 +++++++++++++----------------
 1 file changed, 13 insertions(+), 16 deletions(-)

diff --git a/src/main.c b/src/main.c
index 6ab1b89f4dd5..0f2b57cbacbb 100644
--- a/src/main.c
+++ b/src/main.c
@@ -247,18 +247,14 @@ static bool nft_options_check(int argc, char * const argv[])
 
 int main(int argc, char * const *argv)
 {
-	char *buf = NULL, *filename = NULL;
-	unsigned int output_flags = 0;
-	bool interactive = false;
-	unsigned int debug_mask;
-	unsigned int len;
+	char *buf = NULL, *filename = NULL, *includepath = NULL;
+	unsigned int output_flags = 0, debug_mask = 0, len;
+	bool interactive = false, dry_run = false;
 	int i, val, rc;
 
 	if (!nft_options_check(argc, argv))
 		exit(EXIT_FAILURE);
 
-	nft = nft_ctx_new(NFT_CTX_DEFAULT);
-
 	while (1) {
 		val = getopt_long(argc, argv, OPTSTRING, options, NULL);
 		if (val == -1)
@@ -273,7 +269,7 @@ int main(int argc, char * const *argv)
 			       PACKAGE_NAME, PACKAGE_VERSION, RELEASE_NAME);
 			exit(EXIT_SUCCESS);
 		case OPT_CHECK:
-			nft_ctx_set_dry_run(nft, true);
+			dry_run = true;
 			break;
 		case OPT_FILE:
 			filename = optarg;
@@ -282,12 +278,7 @@ int main(int argc, char * const *argv)
 			interactive = true;
 			break;
 		case OPT_INCLUDEPATH:
-			if (nft_ctx_add_include_path(nft, optarg)) {
-				fprintf(stderr,
-					"Failed to add include path '%s'\n",
-					optarg);
-				exit(EXIT_FAILURE);
-			}
+			includepath = optarg;
 			break;
 		case OPT_NUMERIC:
 			output_flags |= NFT_CTX_OUTPUT_NUMERIC_ALL;
@@ -302,7 +293,6 @@ int main(int argc, char * const *argv)
 			output_flags |= NFT_CTX_OUTPUT_SERVICE;
 			break;
 		case OPT_DEBUG:
-			debug_mask = nft_ctx_output_get_debug(nft);
 			for (;;) {
 				unsigned int i;
 				char *end;
@@ -328,7 +318,6 @@ int main(int argc, char * const *argv)
 					break;
 				optarg = end + 1;
 			}
-			nft_ctx_output_set_debug(nft, debug_mask);
 			break;
 		case OPT_HANDLE_OUTPUT:
 			output_flags |= NFT_CTX_OUTPUT_HANDLE;
@@ -364,6 +353,14 @@ int main(int argc, char * const *argv)
 		}
 	}
 
+	nft = nft_ctx_new(NFT_CTX_DEFAULT);
+	if (includepath && nft_ctx_add_include_path(nft, includepath) < 0) {
+		fprintf(stderr, "Failed to add include path '%s'\n", optarg);
+		exit(EXIT_FAILURE);
+	}
+
+	nft_ctx_set_dry_run(nft, dry_run);
+	nft_ctx_output_set_debug(nft, debug_mask);
 	nft_ctx_output_set_flags(nft, output_flags);
 
 	if (optind != argc) {
-- 
2.11.0

[PATCH nft 3/3] main: add -w/--netns option

[Pablo Neira Ayuso]

This option allows you to specify which net namespace you want to run this
command on.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 doc/nft.txt |  4 ++++
 src/main.c  | 28 +++++++++++++++++++++++-----
 2 files changed, 27 insertions(+), 5 deletions(-)

diff --git a/doc/nft.txt b/doc/nft.txt
index 45350253ccbf..a668c33b3c6d 100644
--- a/doc/nft.txt
+++ b/doc/nft.txt
@@ -32,6 +32,10 @@ For a full summary of options, run *nft --help*.
 *--version*::
 	Show version.
 
+*-w*::
+*--netns 'namespace'*::
+	Run command on the specified net namespace.
+
 *-n*::
 *--numeric*::
 	Print fully numerical output.
diff --git a/src/main.c b/src/main.c
index 0f2b57cbacbb..df88c028fcb3 100644
--- a/src/main.c
+++ b/src/main.c
@@ -28,6 +28,7 @@ enum opt_vals {
 	OPT_HELP		= 'h',
 	OPT_VERSION		= 'v',
 	OPT_CHECK		= 'c',
+	OPT_NETNS		= 'w',
 	OPT_FILE		= 'f',
 	OPT_INTERACTIVE		= 'i',
 	OPT_INCLUDEPATH		= 'I',
@@ -46,7 +47,7 @@ enum opt_vals {
 	OPT_TERSE		= 't',
 	OPT_INVALID		= '?',
 };
-#define OPTSTRING	"+hvd:cf:iI:jvnsNaeSupypTt"
+#define OPTSTRING	"+hvd:cf:iI:jvnsNaeSupypTtw:"
 
 static const struct option options[] = {
 	{
@@ -129,6 +130,11 @@ static const struct option options[] = {
 		.val		= OPT_TERSE,
 	},
 	{
+		.name		= "netns",
+		.val		= OPT_NETNS,
+		.has_arg	= 1,
+	},
+	{
 		.name		= NULL
 	}
 };
@@ -150,6 +156,7 @@ static void show_help(const char *name)
 "  -n, --numeric			Print fully numerical output.\n"
 "  -s, --stateless		Omit stateful information of ruleset.\n"
 "  -t, --terse			Omit contents of sets.\n"
+"  -w, --netns			Run command on network namespace\n"
 "  -u, --guid			Print UID/GID as defined in /etc/passwd and /etc/group.\n"
 "  -N				Translate IP addresses to names.\n"
 "  -S, --service			Translate ports to service names as described in /etc/services.\n"
@@ -231,9 +238,11 @@ static bool nft_options_check(int argc, char * const argv[])
 			} else if (argv[i][1] == 'd' ||
 				   argv[i][1] == 'I' ||
 				   argv[i][1] == 'f' ||
+				   argv[i][1] == 'w' ||
 				   !strcmp(argv[i], "--debug") ||
 				   !strcmp(argv[i], "--includepath") ||
-				   !strcmp(argv[i], "--file")) {
+				   !strcmp(argv[i], "--file") ||
+				   !strcmp(argv[i], "--netns")) {
 				skip = true;
 				continue;
 			}
@@ -247,10 +256,10 @@ static bool nft_options_check(int argc, char * const argv[])
 
 int main(int argc, char * const *argv)
 {
-	char *buf = NULL, *filename = NULL, *includepath = NULL;
+	char *buf = NULL, *filename = NULL, *includepath = NULL, *netns = NULL;
 	unsigned int output_flags = 0, debug_mask = 0, len;
 	bool interactive = false, dry_run = false;
-	int i, val, rc;
+	int i, val, rc, ctx = NFT_CTX_DEFAULT;
 
 	if (!nft_options_check(argc, argv))
 		exit(EXIT_FAILURE);
@@ -271,6 +280,9 @@ int main(int argc, char * const *argv)
 		case OPT_CHECK:
 			dry_run = true;
 			break;
+		case OPT_NETNS:
+			netns = optarg;
+			break;
 		case OPT_FILE:
 			filename = optarg;
 			break;
@@ -353,12 +365,18 @@ int main(int argc, char * const *argv)
 		}
 	}
 
-	nft = nft_ctx_new(NFT_CTX_DEFAULT);
+	if (netns)
+		ctx = NFT_CTX_NETNS;
+
+	nft = nft_ctx_new(ctx);
 	if (includepath && nft_ctx_add_include_path(nft, includepath) < 0) {
 		fprintf(stderr, "Failed to add include path '%s'\n", optarg);
 		exit(EXIT_FAILURE);
 	}
 
+	if (netns && nft_ctx_set_netns(nft, netns) < 0)
+		exit(EXIT_FAILURE);
+
 	nft_ctx_set_dry_run(nft, dry_run);
 	nft_ctx_output_set_debug(nft, debug_mask);
 	nft_ctx_output_set_flags(nft, output_flags);
-- 
2.11.0

Re: [PATCH nft 1/3] libnftables: add nft_ctx_set_netns()

[Phil Sutter]

Hi Pablo,

On Thu, Jan 09, 2020 at 06:21:13PM +0100, Pablo Neira Ayuso wrote:
[...]
> diff --git a/include/nftables/libnftables.h b/include/nftables/libnftables.h
> index 765b20dd71ee..887628959ac6 100644
> --- a/include/nftables/libnftables.h
> +++ b/include/nftables/libnftables.h
> @@ -34,10 +34,13 @@ enum nft_debug_level {
>   * Possible flags to pass to nft_ctx_new()
>   */
>  #define NFT_CTX_DEFAULT		0
> +#define NFT_CTX_NETNS		1

What is this needed for?

>  struct nft_ctx *nft_ctx_new(uint32_t flags);
>  void nft_ctx_free(struct nft_ctx *ctx);
>  
> +int nft_ctx_set_netns(struct nft_ctx *ctx, const char *netns);

Is there a way to select init ns again?

Thanks, Phil

Re: [PATCH nft 1/3] libnftables: add nft_ctx_set_netns()

[Pablo Neira Ayuso]

On Fri, Jan 10, 2020 at 01:53:11PM +0100, Phil Sutter wrote:
> Hi Pablo,
> 
> On Thu, Jan 09, 2020 at 06:21:13PM +0100, Pablo Neira Ayuso wrote:
> [...]
> > diff --git a/include/nftables/libnftables.h b/include/nftables/libnftables.h
> > index 765b20dd71ee..887628959ac6 100644
> > --- a/include/nftables/libnftables.h
> > +++ b/include/nftables/libnftables.h
> > @@ -34,10 +34,13 @@ enum nft_debug_level {
> >   * Possible flags to pass to nft_ctx_new()
> >   */
> >  #define NFT_CTX_DEFAULT		0
> > +#define NFT_CTX_NETNS		1
> 
> What is this needed for?

The socket is initialized from nft_ctx_init(), and such initialization
needs to happen after the netns switch.

> >  struct nft_ctx *nft_ctx_new(uint32_t flags);
> >  void nft_ctx_free(struct nft_ctx *ctx);
> >  
> > +int nft_ctx_set_netns(struct nft_ctx *ctx, const char *netns);
> 
> Is there a way to select init ns again?

AFAIK, setns() does not let you go back to init ns once set.

Re: [PATCH nft 1/3] libnftables: add nft_ctx_set_netns()

[Pablo Neira Ayuso]

On Sun, Jan 12, 2020 at 11:28:02AM +0100, Pablo Neira Ayuso wrote:
> On Fri, Jan 10, 2020 at 01:53:11PM +0100, Phil Sutter wrote:
> > Hi Pablo,
> > 
> > On Thu, Jan 09, 2020 at 06:21:13PM +0100, Pablo Neira Ayuso wrote:
> > [...]
> > > diff --git a/include/nftables/libnftables.h b/include/nftables/libnftables.h
> > > index 765b20dd71ee..887628959ac6 100644
> > > --- a/include/nftables/libnftables.h
> > > +++ b/include/nftables/libnftables.h
> > > @@ -34,10 +34,13 @@ enum nft_debug_level {
> > >   * Possible flags to pass to nft_ctx_new()
> > >   */
> > >  #define NFT_CTX_DEFAULT		0
> > > +#define NFT_CTX_NETNS		1
> > 
> > What is this needed for?
> 
> The socket is initialized from nft_ctx_init(), and such initialization
> needs to happen after the netns switch.

s/nft_ctx_init()/nft_ctx_new()

> > >  struct nft_ctx *nft_ctx_new(uint32_t flags);
> > >  void nft_ctx_free(struct nft_ctx *ctx);
> > >  
> > > +int nft_ctx_set_netns(struct nft_ctx *ctx, const char *netns);
> > 
> > Is there a way to select init ns again?
> 
> AFAIK, setns() does not let you go back to init ns once set.

Re: [PATCH nft 1/3] libnftables: add nft_ctx_set_netns()

[Phil Sutter]

Hi Pablo,

On Sun, Jan 12, 2020 at 11:40:27AM +0100, Pablo Neira Ayuso wrote:
> On Sun, Jan 12, 2020 at 11:28:02AM +0100, Pablo Neira Ayuso wrote:
> > On Fri, Jan 10, 2020 at 01:53:11PM +0100, Phil Sutter wrote:
> > > On Thu, Jan 09, 2020 at 06:21:13PM +0100, Pablo Neira Ayuso wrote:
> > > [...]
> > > > diff --git a/include/nftables/libnftables.h b/include/nftables/libnftables.h
> > > > index 765b20dd71ee..887628959ac6 100644
> > > > --- a/include/nftables/libnftables.h
> > > > +++ b/include/nftables/libnftables.h
> > > > @@ -34,10 +34,13 @@ enum nft_debug_level {
> > > >   * Possible flags to pass to nft_ctx_new()
> > > >   */
> > > >  #define NFT_CTX_DEFAULT		0
> > > > +#define NFT_CTX_NETNS		1
> > > 
> > > What is this needed for?
> > 
> > The socket is initialized from nft_ctx_init(), and such initialization
> > needs to happen after the netns switch.
> 
> s/nft_ctx_init()/nft_ctx_new()

Ah, I missed that socket init in nft_ctx_new() happens only if flags is
zero.

> > > >  struct nft_ctx *nft_ctx_new(uint32_t flags);
> > > >  void nft_ctx_free(struct nft_ctx *ctx);
> > > >  
> > > > +int nft_ctx_set_netns(struct nft_ctx *ctx, const char *netns);
> > > 
> > > Is there a way to select init ns again?
> > 
> > AFAIK, setns() does not let you go back to init ns once set.

I noticed something I find worse, namely that libnftables as a library
changes the application's netns. Anything it does after changing the
context's netns applies to that netns only, no matter if it's creating a
new nft context with NFT_CtX_DEFAULT flag or call iproute via system().

If we can't find a way to exit the netns again, one can safely assume
that we are trapping a user's application in a netns with this feature.

Maybe we should restrict per-netns operation to nft utility and perform
the netns switch there? Maybe we could provide a "switch_netns()"
routine in libnftables which is not bound to nft context so users may
use it in their application?

Cheers, Phil

Re: [PATCH nft 1/3] libnftables: add nft_ctx_set_netns()

[Pablo Neira Ayuso]

On Tue, Jan 14, 2020 at 11:25:16AM +0100, Phil Sutter wrote:
> Hi Pablo,
> 
> On Sun, Jan 12, 2020 at 11:40:27AM +0100, Pablo Neira Ayuso wrote:
> > On Sun, Jan 12, 2020 at 11:28:02AM +0100, Pablo Neira Ayuso wrote:
> > > On Fri, Jan 10, 2020 at 01:53:11PM +0100, Phil Sutter wrote:
> > > > On Thu, Jan 09, 2020 at 06:21:13PM +0100, Pablo Neira Ayuso wrote:
> > > > [...]
> > > > > diff --git a/include/nftables/libnftables.h b/include/nftables/libnftables.h
> > > > > index 765b20dd71ee..887628959ac6 100644
> > > > > --- a/include/nftables/libnftables.h
> > > > > +++ b/include/nftables/libnftables.h
> > > > > @@ -34,10 +34,13 @@ enum nft_debug_level {
> > > > >   * Possible flags to pass to nft_ctx_new()
> > > > >   */
> > > > >  #define NFT_CTX_DEFAULT		0
> > > > > +#define NFT_CTX_NETNS		1
> > > > 
> > > > What is this needed for?
> > > 
> > > The socket is initialized from nft_ctx_init(), and such initialization
> > > needs to happen after the netns switch.
> > 
> > s/nft_ctx_init()/nft_ctx_new()
> 
> Ah, I missed that socket init in nft_ctx_new() happens only if flags is
> zero.
> 
> > > > >  struct nft_ctx *nft_ctx_new(uint32_t flags);
> > > > >  void nft_ctx_free(struct nft_ctx *ctx);
> > > > >  
> > > > > +int nft_ctx_set_netns(struct nft_ctx *ctx, const char *netns);
> > > > 
> > > > Is there a way to select init ns again?
> > > 
> > > AFAIK, setns() does not let you go back to init ns once set.
> 
> I noticed something I find worse, namely that libnftables as a library
> changes the application's netns. Anything it does after changing the
> context's netns applies to that netns only, no matter if it's creating a
> new nft context with NFT_CtX_DEFAULT flag or call iproute via system().
> 
> If we can't find a way to exit the netns again, one can safely assume
> that we are trapping a user's application in a netns with this feature.

IIRC, you can fork(), then let the child enter the netns while parent
remain in the original netns.

> Maybe we should restrict per-netns operation to nft utility and perform
> the netns switch there? Maybe we could provide a "switch_netns()"
> routine in libnftables which is not bound to nft context so users may
> use it in their application?

That's another possibility, yes. In that case, there is no need for
NFT_CTX_NETNS, which is just there to skip the socket initialization.

Re: [PATCH nft 1/3] libnftables: add nft_ctx_set_netns()

[Phil Sutter]

Hi Pablo,

On Tue, Jan 14, 2020 at 11:38:35AM +0100, Pablo Neira Ayuso wrote:
> On Tue, Jan 14, 2020 at 11:25:16AM +0100, Phil Sutter wrote:
> > On Sun, Jan 12, 2020 at 11:40:27AM +0100, Pablo Neira Ayuso wrote:
> > > On Sun, Jan 12, 2020 at 11:28:02AM +0100, Pablo Neira Ayuso wrote:
> > > > On Fri, Jan 10, 2020 at 01:53:11PM +0100, Phil Sutter wrote:
> > > > > On Thu, Jan 09, 2020 at 06:21:13PM +0100, Pablo Neira Ayuso wrote:
[...]
> > > > > >  struct nft_ctx *nft_ctx_new(uint32_t flags);
> > > > > >  void nft_ctx_free(struct nft_ctx *ctx);
> > > > > >  
> > > > > > +int nft_ctx_set_netns(struct nft_ctx *ctx, const char *netns);
> > > > > 
> > > > > Is there a way to select init ns again?
> > > > 
> > > > AFAIK, setns() does not let you go back to init ns once set.

FWIW, I found interesting Python code[1] dealing with that. The logic is
to open /proc/$$/ns/net before switching netns and storing the fd for
later. To exit the netns again, it is passed to setns() and then closed.
Note that the code there is much simpler and doesn't deal with mounts or
non-existing entries in /var/run/netns/. Maybe libnftables doesn't need
to either and it is OK to just bail if given netns doesn't exist?

> > I noticed something I find worse, namely that libnftables as a library
> > changes the application's netns. Anything it does after changing the
> > context's netns applies to that netns only, no matter if it's creating a
> > new nft context with NFT_CtX_DEFAULT flag or call iproute via system().
> > 
> > If we can't find a way to exit the netns again, one can safely assume
> > that we are trapping a user's application in a netns with this feature.
> 
> IIRC, you can fork(), then let the child enter the netns while parent
> remain in the original netns.

Yes, that's also the only way to operate in multiple netns in parallel.

> > Maybe we should restrict per-netns operation to nft utility and perform
> > the netns switch there? Maybe we could provide a "switch_netns()"
> > routine in libnftables which is not bound to nft context so users may
> > use it in their application?
> 
> That's another possibility, yes. In that case, there is no need for
> NFT_CTX_NETNS, which is just there to skip the socket initialization.

I just think that a routine which affects things outside of nft scope
shouldn't be tied as closely with nft context.

Cheers, Phil

[1] https://github.com/larsks/python-netns/blob/master/netns.py