[PATCH 0/8] Allow pasta to take a command to spawn instead of shell

[PATCH 0/8] Allow pasta to take a command to spawn instead of shell

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 1124 bytes --]

When not attaching to an existing network namespace, pasta always
spawns an interactive shell in a new namespace to attach to.  Most
commands which can issue a shell in a modified environment can also
issue other commands as well (e.g. env, strace).  We want to allow
pasta to do the same.

Because of the way the non-option argument to pasta is currently
overloaded, allowing this requires some other changes to the way we
parse the command line.

David Gibson (8):
  conf: Make the argument to --pcap option mandatory
  conf: Use "-D none" and "-S none" instead of missing empty option
    arguments
  Correct manpage for --userns
  Remove --nsrun-dir option
  Move ENOENT error message into conf_ns_opt()
  More deterministic detection of whether argument is a PID, PATH or
    NAME
  Use explicit --netns option rather than multiplexing with PID
  Allow pasta to take a command to execute

 conf.c  | 269 +++++++++++++++++++++++++++++---------------------------
 passt.1 |  54 ++++++------
 pasta.c |  33 ++++---
 pasta.h |   2 +-
 pcap.c  |  28 ------
 5 files changed, 191 insertions(+), 195 deletions(-)

-- 
2.37.2

[PATCH 1/8] conf: Make the argument to --pcap option mandatory

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 4386 bytes --]

The --pcap or -p option can be used with or without an argument.  If given,
the argument gives the name of the file to save a packet trace to.  If
omitted, we generate a default name in /tmp.

Generating the default name isn't particularly useful though, since making
a suitable name can easily be done by the caller.  Remove this feature.

Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au>
---
 conf.c  | 18 +++---------------
 passt.1 | 10 ----------
 pcap.c  | 28 ----------------------------
 3 files changed, 3 insertions(+), 53 deletions(-)

diff --git a/conf.c b/conf.c
index d936157..4eb9e3d 100644
--- a/conf.c
+++ b/conf.c
@@ -737,14 +737,7 @@ static void usage(const char *name)
 		     UNIX_SOCK_PATH, 1);
 	}
 
-	info(   "  -p, --pcap [FILE]	Log tap-facing traffic to pcap file");
-	info(   "    if FILE is not given, log to:");
-
-	if (strstr(name, "pasta"))
-		info("      /tmp/pasta_ISO8601-TIMESTAMP_PID.pcap");
-	else
-		info("      /tmp/passt_ISO8601-TIMESTAMP_PID.pcap");
-
+	info(   "  -p, --pcap FILE	Log tap-facing traffic to pcap file");
 	info(   "  -P, --pid FILE	Write own PID to the given file");
 	info(   "  -m, --mtu MTU	Assign MTU via DHCP/NDP");
 	info(   "    a zero value disables assignment");
@@ -1021,7 +1014,7 @@ void conf(struct ctx *c, int argc, char **argv)
 		{"help",	no_argument,		NULL,		'h' },
 		{"socket",	required_argument,	NULL,		's' },
 		{"ns-ifname",	required_argument,	NULL,		'I' },
-		{"pcap",	optional_argument,	NULL,		'p' },
+		{"pcap",	required_argument,	NULL,		'p' },
 		{"pid",		required_argument,	NULL,		'P' },
 		{"mtu",		required_argument,	NULL,		'm' },
 		{"address",	required_argument,	NULL,		'a' },
@@ -1084,7 +1077,7 @@ void conf(struct ctx *c, int argc, char **argv)
 
 		name = getopt_long(argc, argv, optstring, options, NULL);
 
-		if ((name == 'p' || name == 'D' || name == 'S') && !optarg &&
+		if ((name == 'D' || name == 'S') && !optarg &&
 		    optind < argc && *argv[optind] && *argv[optind] != '-') {
 			if (c->mode == MODE_PASTA) {
 				if (conf_ns_opt(c, nsdir, userns, argv[optind]))
@@ -1289,11 +1282,6 @@ void conf(struct ctx *c, int argc, char **argv)
 				usage(argv[0]);
 			}
 
-			if (!optarg) {
-				*c->pcap = 1;
-				break;
-			}
-
 			ret = snprintf(c->pcap, sizeof(c->pcap), "%s", optarg);
 			if (ret <= 0 || ret >= (int)sizeof(c->pcap)) {
 				err("Invalid pcap path: %s", optarg);
diff --git a/passt.1 b/passt.1
index 378778c..9bed946 100644
--- a/passt.1
+++ b/passt.1
@@ -111,16 +111,6 @@ Display a help message and exit.
 Capture tap-facing (that is, guest-side or namespace-side) network packets to
 \fIfile\fR in \fBpcap\fR format.
 
-If \fIfile\fR is not given, capture packets to
-
-	\fB/tmp/passt_\fIISO8601-timestamp\fR_\fIPID\fB.pcap\fR
-
-in \fBpasst\fR mode and to
-
-	\fB/tmp/pasta_\fIISO8601-timestamp\fR_\fIPID\fB.pcap\fR
-
-in \fBpasta\fR mode, where \fIPID\fR is the ID of the running process.
-
 .TP
 .BR \-P ", " \-\-pid " " \fIfile
 Write own PID to \fIfile\fR once initialisation is done, before forking to
diff --git a/pcap.c b/pcap.c
index 64beb34..7a48baf 100644
--- a/pcap.c
+++ b/pcap.c
@@ -31,11 +31,6 @@
 #include "util.h"
 #include "passt.h"
 
-#define PCAP_PREFIX		"/tmp/passt_"
-#define PCAP_PREFIX_PASTA	"/tmp/pasta_"
-#define PCAP_ISO8601_FORMAT	"%FT%H:%M:%SZ"
-#define PCAP_ISO8601_STR	"YYYY-MM-ddTHH:mm:ssZ"
-
 #define PCAP_VERSION_MINOR 4
 
 static int pcap_fd = -1;
@@ -171,7 +166,6 @@ fail:
 void pcap_init(struct ctx *c)
 {
 	int flags = O_WRONLY | O_CREAT | O_TRUNC;
-	struct timeval tv;
 
 	if (pcap_fd != -1)
 		return;
@@ -179,28 +173,6 @@ void pcap_init(struct ctx *c)
 	if (!*c->pcap)
 		return;
 
-	if (*c->pcap == 1) {
-		char name[] = PCAP_PREFIX PCAP_ISO8601_STR STR(UINT_MAX)
-			      ".pcap";
-		struct tm *tm;
-
-		if (c->mode == MODE_PASTA)
-			memcpy(name, PCAP_PREFIX_PASTA,
-			       sizeof(PCAP_PREFIX_PASTA));
-
-		gettimeofday(&tv, NULL);
-		tm = localtime(&tv.tv_sec);
-		strftime(name + strlen(PCAP_PREFIX),
-			 sizeof(PCAP_ISO8601_STR) - 1, PCAP_ISO8601_FORMAT, tm);
-
-		snprintf(name + strlen(PCAP_PREFIX) + strlen(PCAP_ISO8601_STR),
-			 sizeof(name) - strlen(PCAP_PREFIX) -
-					strlen(PCAP_ISO8601_STR),
-			 "_%i.pcap", getpid());
-
-		strncpy(c->pcap, name, PATH_MAX);
-	}
-
 	flags |= c->foreground ? O_CLOEXEC : 0;
 	pcap_fd = open(c->pcap, flags, S_IRUSR | S_IWUSR);
 	if (pcap_fd == -1) {
-- 
@@ -31,11 +31,6 @@
 #include "util.h"
 #include "passt.h"
 
-#define PCAP_PREFIX		"/tmp/passt_"
-#define PCAP_PREFIX_PASTA	"/tmp/pasta_"
-#define PCAP_ISO8601_FORMAT	"%FT%H:%M:%SZ"
-#define PCAP_ISO8601_STR	"YYYY-MM-ddTHH:mm:ssZ"
-
 #define PCAP_VERSION_MINOR 4
 
 static int pcap_fd = -1;
@@ -171,7 +166,6 @@ fail:
 void pcap_init(struct ctx *c)
 {
 	int flags = O_WRONLY | O_CREAT | O_TRUNC;
-	struct timeval tv;
 
 	if (pcap_fd != -1)
 		return;
@@ -179,28 +173,6 @@ void pcap_init(struct ctx *c)
 	if (!*c->pcap)
 		return;
 
-	if (*c->pcap == 1) {
-		char name[] = PCAP_PREFIX PCAP_ISO8601_STR STR(UINT_MAX)
-			      ".pcap";
-		struct tm *tm;
-
-		if (c->mode == MODE_PASTA)
-			memcpy(name, PCAP_PREFIX_PASTA,
-			       sizeof(PCAP_PREFIX_PASTA));
-
-		gettimeofday(&tv, NULL);
-		tm = localtime(&tv.tv_sec);
-		strftime(name + strlen(PCAP_PREFIX),
-			 sizeof(PCAP_ISO8601_STR) - 1, PCAP_ISO8601_FORMAT, tm);
-
-		snprintf(name + strlen(PCAP_PREFIX) + strlen(PCAP_ISO8601_STR),
-			 sizeof(name) - strlen(PCAP_PREFIX) -
-					strlen(PCAP_ISO8601_STR),
-			 "_%i.pcap", getpid());
-
-		strncpy(c->pcap, name, PATH_MAX);
-	}
-
 	flags |= c->foreground ? O_CLOEXEC : 0;
 	pcap_fd = open(c->pcap, flags, S_IRUSR | S_IWUSR);
 	if (pcap_fd == -1) {
-- 
2.37.2

[PATCH 2/8] conf: Use "-D none" and "-S none" instead of missing empty option arguments

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 3405 bytes --]

Both the -D (--dns) and -S (--search) options take an optional argument.
If the argument is omitted the option is disabled entirely.  However,
handling the optional argument requires some ugly special case handling if
it's the last option on the command line, and has potential ambiguity with
non-option arguments used with pasta.  It can also make it more confusing
to read command lines.

Simplify the logic here by replacing the non-argument versions with an
explicit "-D none" or "-S none".

Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au>
---
 conf.c  | 18 ++++--------------
 passt.1 |  7 ++++---
 2 files changed, 8 insertions(+), 17 deletions(-)

diff --git a/conf.c b/conf.c
index 4eb9e3d..6770be9 100644
--- a/conf.c
+++ b/conf.c
@@ -1022,8 +1022,8 @@ void conf(struct ctx *c, int argc, char **argv)
 		{"mac-addr",	required_argument,	NULL,		'M' },
 		{"gateway",	required_argument,	NULL,		'g' },
 		{"interface",	required_argument,	NULL,		'i' },
-		{"dns",		optional_argument,	NULL,		'D' },
-		{"search",	optional_argument,	NULL,		'S' },
+		{"dns",		required_argument,	NULL,		'D' },
+		{"search",	required_argument,	NULL,		'S' },
 		{"no-tcp",	no_argument,		&c->no_tcp,	1 },
 		{"no-udp",	no_argument,		&c->no_udp,	1 },
 		{"no-icmp",	no_argument,		&c->no_icmp,	1 },
@@ -1077,16 +1077,6 @@ void conf(struct ctx *c, int argc, char **argv)
 
 		name = getopt_long(argc, argv, optstring, options, NULL);
 
-		if ((name == 'D' || name == 'S') && !optarg &&
-		    optind < argc && *argv[optind] && *argv[optind] != '-') {
-			if (c->mode == MODE_PASTA) {
-				if (conf_ns_opt(c, nsdir, userns, argv[optind]))
-					optarg = argv[optind++];
-			} else {
-				optarg = argv[optind++];
-			}
-		}
-
 		switch (name) {
 		case -1:
 		case 0:
@@ -1403,7 +1393,7 @@ void conf(struct ctx *c, int argc, char **argv)
 				usage(argv[0]);
 			}
 
-			if (!optarg) {
+			if (!strcmp(optarg, "none")) {
 				c->no_dns = 1;
 				break;
 			}
@@ -1430,7 +1420,7 @@ void conf(struct ctx *c, int argc, char **argv)
 				usage(argv[0]);
 			}
 
-			if (!optarg) {
+			if (!strcmp(optarg, "none")) {
 				c->no_dns_search = 1;
 				break;
 			}
diff --git a/passt.1 b/passt.1
index 9bed946..14b01b2 100644
--- a/passt.1
+++ b/passt.1
@@ -171,7 +171,7 @@ version.
 Use \fIaddr\fR (IPv4 or IPv6) for DHCP, DHCPv6, NDP or DNS forwarding, as
 configured (see options \fB--no-dhcp-dns\fR, \fB--dhcp-dns\fR,
 \fB--dns-forward\fR) instead of reading addresses from \fI/etc/resolv.conf\fR.
-This option can be specified multiple times, and a single, empty option disables
+This option can be specified multiple times.  Specifying \fB-D none\fR disables
 usage of DNS addresses altogether.
 
 .TP
@@ -186,8 +186,9 @@ This option can be specified zero to two times (once for IPv4, once for IPv6).
 .BR \-S ", " \-\-search " " \fIlist
 Use space-separated \fIlist\fR for DHCP, DHCPv6, and NDP purposes, instead of
 reading entries from \fI/etc/resolv.conf\fR. See options \fB--no-dhcp-search\fR
-and \fB--dhcp-search\fR. A single, empty option disables the DNS domain search
-list altogether.
+and \fB--dhcp-search\fR. \fB--search none\fR disables the DNS domain search
+list altogether (if you need to search a domain called "none" you can use
+\fB--search none.\fR).
 
 .TP
 .BR \-\-no-dhcp-dns " " \fIaddr
-- 
@@ -171,7 +171,7 @@ version.
 Use \fIaddr\fR (IPv4 or IPv6) for DHCP, DHCPv6, NDP or DNS forwarding, as
 configured (see options \fB--no-dhcp-dns\fR, \fB--dhcp-dns\fR,
 \fB--dns-forward\fR) instead of reading addresses from \fI/etc/resolv.conf\fR.
-This option can be specified multiple times, and a single, empty option disables
+This option can be specified multiple times.  Specifying \fB-D none\fR disables
 usage of DNS addresses altogether.
 
 .TP
@@ -186,8 +186,9 @@ This option can be specified zero to two times (once for IPv4, once for IPv6).
 .BR \-S ", " \-\-search " " \fIlist
 Use space-separated \fIlist\fR for DHCP, DHCPv6, and NDP purposes, instead of
 reading entries from \fI/etc/resolv.conf\fR. See options \fB--no-dhcp-search\fR
-and \fB--dhcp-search\fR. A single, empty option disables the DNS domain search
-list altogether.
+and \fB--dhcp-search\fR. \fB--search none\fR disables the DNS domain search
+list altogether (if you need to search a domain called "none" you can use
+\fB--search none.\fR).
 
 .TP
 .BR \-\-no-dhcp-dns " " \fIaddr
-- 
2.37.2

[PATCH 3/8] Correct manpage for --userns

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 976 bytes --]

The man page states that the --userns option can be given either as a path
or as a name relative to --nsrun-dir.  This is not correct: as the name
suggests --nsrun-dir is (correctly) used only for *netns* resolution, not
*userns* resolution.

Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au>
---
 passt.1 | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/passt.1 b/passt.1
index 14b01b2..78b10b8 100644
--- a/passt.1
+++ b/passt.1
@@ -442,9 +442,8 @@ Default is \fBauto\fR.
 
 .TP
 .BR \-\-userns " " \fIspec
-Target user namespace to join, as path or name (i.e. suffix for --nsrun-dir). If
-PID is given, without this option, the user namespace will be the one of the
-corresponding process.
+Target user namespace to join, as a path. If PID is given, without this option,
+the user namespace will be the one of the corresponding process.
 
 This option requires PID, PATH or NAME to be specified.
 
-- 
@@ -442,9 +442,8 @@ Default is \fBauto\fR.
 
 .TP
 .BR \-\-userns " " \fIspec
-Target user namespace to join, as path or name (i.e. suffix for --nsrun-dir). If
-PID is given, without this option, the user namespace will be the one of the
-corresponding process.
+Target user namespace to join, as a path. If PID is given, without this option,
+the user namespace will be the one of the corresponding process.
 
 This option requires PID, PATH or NAME to be specified.
 
-- 
2.37.2

[PATCH 4/8] Remove --nsrun-dir option

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 4501 bytes --]

pasta can identify a netns as a "name", which is to say a path relative to
(usually) /run/netns, which is the place that ip(8) creates persistent
network namespaces.  Alternatively a full path to a netns can be given.

The --nsrun-dir option allows the user to change the standard path where
netns names are resolved.  However, there's no real point to this, if the
user wants to override the location of the netns, they can just as easily
use the full path to specify the netns.

Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au>
---
 conf.c  | 24 ++++--------------------
 passt.1 |  6 ------
 2 files changed, 4 insertions(+), 26 deletions(-)

diff --git a/conf.c b/conf.c
index 6770be9..e76181c 100644
--- a/conf.c
+++ b/conf.c
@@ -510,14 +510,13 @@ static int conf_ns_check(void *arg)
 /**
  * conf_ns_opt() - Open network, user namespaces descriptors from configuration
  * @c:			Execution context
- * @nsdir:		--nsrun-dir argument, can be an empty string
  * @conf_userns:	--userns argument, can be an empty string
  * @optarg:		PID, path or name of namespace
  *
  * Return: 0 on success, negative error code otherwise
  */
 static int conf_ns_opt(struct ctx *c,
-		       char *nsdir, const char *conf_userns, const char *optarg)
+		       const char *conf_userns, const char *optarg)
 {
 	int ufd = -1, nfd = -1, try, ret, netns_only_reset = c->netns_only;
 	char userns[PATH_MAX] = { 0 }, netns[PATH_MAX];
@@ -557,7 +556,7 @@ static int conf_ns_opt(struct ctx *c,
 				continue;
 		} else if (try == 2) {
 			ret = snprintf(netns, PATH_MAX, "%s/%s",
-				 *nsdir ? nsdir : NETNS_RUN_DIR, optarg);
+				       NETNS_RUN_DIR, optarg);
 			if (ret <= 0 || ret > (int)sizeof(netns))
 				continue;
 		}
@@ -859,8 +858,6 @@ pasta_opts:
 	info(   "  --userns NSPATH 	Target user namespace to join");
 	info(   "  --netns-only		Don't join existing user namespace");
 	info(   "    implied if PATH or NAME are given without --userns");
-	info(   "  --nsrun-dir		Directory for nsfs mountpoints");
-	info(   "    default: " NETNS_RUN_DIR);
 	info(   "  --config-net		Configure tap interface in namespace");
 	info(   "  --ns-mac-addr ADDR	Set MAC address on tap interface");
 
@@ -1040,7 +1037,6 @@ void conf(struct ctx *c, int argc, char **argv)
 		{"udp-ns",	required_argument,	NULL,		'U' },
 		{"userns",	required_argument,	NULL,		2 },
 		{"netns-only",	no_argument,		&c->netns_only,	1 },
-		{"nsrun-dir",	required_argument,	NULL,		3 },
 		{"config-net",	no_argument,		&c->pasta_conf_ns, 1 },
 		{"ns-mac-addr",	required_argument,	NULL,		4 },
 		{"dhcp-dns",	no_argument,		NULL,		5 },
@@ -1054,7 +1050,7 @@ void conf(struct ctx *c, int argc, char **argv)
 		{ 0 },
 	};
 	struct get_bound_ports_ns_arg ns_ports_arg = { .c = c };
-	char nsdir[PATH_MAX] = { 0 }, userns[PATH_MAX] = { 0 };
+	char userns[PATH_MAX] = { 0 };
 	enum conf_port_type tcp_tap = 0, tcp_init = 0;
 	enum conf_port_type udp_tap = 0, udp_init = 0;
 	bool v4_only = false, v6_only = false;
@@ -1093,18 +1089,6 @@ void conf(struct ctx *c, int argc, char **argv)
 				usage(argv[0]);
 			}
 			break;
-		case 3:
-			if (c->mode != MODE_PASTA) {
-				err("--nsrun-dir is for pasta mode only");
-				usage(argv[0]);
-			}
-
-			ret = snprintf(nsdir, sizeof(nsdir), "%s", optarg);
-			if (ret <= 0 || ret >= (int)sizeof(nsdir)) {
-				err("Invalid nsrun-dir: %s", optarg);
-				usage(argv[0]);
-			}
-			break;
 		case 4:
 			if (c->mode != MODE_PASTA) {
 				err("--ns-mac-addr is for pasta mode only");
@@ -1461,7 +1445,7 @@ void conf(struct ctx *c, int argc, char **argv)
 	check_root(c);
 
 	if (c->mode == MODE_PASTA && optind + 1 == argc) {
-		ret = conf_ns_opt(c, nsdir, userns, argv[optind]);
+		ret = conf_ns_opt(c, userns, argv[optind]);
 		if (ret == -ENOENT)
 			err("Namespace %s not found", argv[optind]);
 		if (ret < 0)
diff --git a/passt.1 b/passt.1
index 78b10b8..bbdadc1 100644
--- a/passt.1
+++ b/passt.1
@@ -458,12 +458,6 @@ without \-\-userns.
 If the target network namespace is bound to the filesystem (that is, if PATH or
 NAME are given as target), do not exit once the network namespace is deleted.
 
-.TP
-.BR \-\-nsrun-dir " " \fIpath
-Directory for nsfs mountpoints, used as path prefix for names of namespaces.
-
-The default path is shown with --help.
-
 .TP
 .BR \-\-config-net
 Configure networking in the namespace: set up addresses and routes as configured
-- 
@@ -458,12 +458,6 @@ without \-\-userns.
 If the target network namespace is bound to the filesystem (that is, if PATH or
 NAME are given as target), do not exit once the network namespace is deleted.
 
-.TP
-.BR \-\-nsrun-dir " " \fIpath
-Directory for nsfs mountpoints, used as path prefix for names of namespaces.
-
-The default path is shown with --help.
-
 .TP
 .BR \-\-config-net
 Configure networking in the namespace: set up addresses and routes as configured
-- 
2.37.2

[PATCH 5/8] Move ENOENT error message into conf_ns_opt()

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 941 bytes --]

After calling conf_ns_opt() we check for -ENOENT and print an error
message, but conf_ns_opt() prints messages for other errors itself.  For
consistency move the ENOENT message into conf_ns_opt() as well.

Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au>
---
 conf.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/conf.c b/conf.c
index e76181c..47f3e41 100644
--- a/conf.c
+++ b/conf.c
@@ -602,6 +602,7 @@ static int conf_ns_opt(struct ctx *c,
 
 	c->netns_only = netns_only_reset;
 
+	err("Namespace %s not found", optarg);
 	return -ENOENT;
 }
 
@@ -1446,8 +1447,6 @@ void conf(struct ctx *c, int argc, char **argv)
 
 	if (c->mode == MODE_PASTA && optind + 1 == argc) {
 		ret = conf_ns_opt(c, userns, argv[optind]);
-		if (ret == -ENOENT)
-			err("Namespace %s not found", argv[optind]);
 		if (ret < 0)
 			usage(argv[0]);
 	} else if (c->mode == MODE_PASTA && *userns && optind == argc) {
-- 
@@ -602,6 +602,7 @@ static int conf_ns_opt(struct ctx *c,
 
 	c->netns_only = netns_only_reset;
 
+	err("Namespace %s not found", optarg);
 	return -ENOENT;
 }
 
@@ -1446,8 +1447,6 @@ void conf(struct ctx *c, int argc, char **argv)
 
 	if (c->mode == MODE_PASTA && optind + 1 == argc) {
 		ret = conf_ns_opt(c, userns, argv[optind]);
-		if (ret == -ENOENT)
-			err("Namespace %s not found", argv[optind]);
 		if (ret < 0)
 			usage(argv[0]);
 	} else if (c->mode == MODE_PASTA && *userns && optind == argc) {
-- 
2.37.2

[PATCH 6/8] More deterministic detection of whether argument is a PID, PATH or NAME

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 8229 bytes --]

pasta takes as its only non-option argument either a PID to attach to the
namespaces of, a PATH to a network namespace or a NAME of a network
namespace (relative to /run/netns).  Currently to determine which it is
we try all 3 in that order, and if anything goes wrong we move onto the
next.

This has the potential to cause very confusing failure modes.  e.g. if the
argument is intended to be a network namespace name, but a (non-namespace)
file of the same name exists in the current directory.

Make behaviour more predictable by choosing how to treat the argument based
only on the argument's contents, not anything else on the system:
  - If it's a decimal integer treat it as a PID
  - Otherwise, if it has no '/' characters, treat it as a netns name
    (ip-netns doesn't allow '/' in netns names)
  - Otherwise, treat it as a netns path

If you want to open a persistent netns in the current directory, you can
use './netns'.

This also allows us to split the parsing of the PID|PATH|NAME option from
the actual opening of the namespaces.  In turn that allows us to put the
opening of existing namespaces next to the opening of new namespaces in
pasta_start_ns.  That makes the logical flow easier to follow and will
enable later cleanups.

Caveats:
 - The separation of functions mean we will always generate the basename
   and dirname for the netns_quit system, even when using PID namespaces.
   This is pointless, since the netns_quit system doesn't work for non
   persistent namespaces, but is harmless.

Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au>
---
 conf.c | 172 ++++++++++++++++++++++++++++++---------------------------
 1 file changed, 90 insertions(+), 82 deletions(-)

diff --git a/conf.c b/conf.c
index 47f3e41..10b7b9f 100644
--- a/conf.c
+++ b/conf.c
@@ -489,6 +489,51 @@ out:
 		warn("Couldn't get any nameserver address");
 }
 
+/**
+ * conf_ns_opt() - Parse non-option argument to namespace paths
+ * @userns:	buffer of size PATH_MAX, initially contains --userns
+ *		argument (may be empty), updated with userns path
+ * @netns:	buffer of size PATH_MAX, updated with netns path
+ * @arg:	PID, path or name of network namespace
+ *
+ * Return: 0 on success, negative error code otherwise
+ */
+static int conf_ns_opt(char *userns, char *netns, const char *arg)
+{
+	char *endptr;
+	long pidval;
+	int ret;
+
+	pidval = strtol(arg, &endptr, 10);
+	if (!*endptr) {
+		/* Looks like a pid */
+		if (pidval < 0 || pidval > INT_MAX) {
+			err("Invalid PID %s", arg);
+			return -EINVAL;
+		}
+
+		snprintf(netns, PATH_MAX, "/proc/%ld/ns/net", pidval);
+		if (!*userns)
+			snprintf(userns, PATH_MAX, "/proc/%ld/ns/user", pidval);
+		return 0;
+	}
+
+	if (!strchr(arg, '/')) {
+		/* looks like a netns name */
+		ret = snprintf(netns, PATH_MAX, "%s/%s", NETNS_RUN_DIR, arg);
+	} else {
+		/* otherwise assume it's a netns path */
+		ret = snprintf(netns, PATH_MAX, "%s", arg);
+	}
+
+	if (ret <= 0 || ret > PATH_MAX) {
+		err("Network namespace name/path %s too long");
+		return -E2BIG;
+	}
+
+	return 0;
+}
+
 /**
  * conf_ns_check() - Check if we can enter configured namespaces
  * @arg:	Execution context
@@ -508,102 +553,58 @@ static int conf_ns_check(void *arg)
 }
 
 /**
- * conf_ns_opt() - Open network, user namespaces descriptors from configuration
- * @c:			Execution context
- * @conf_userns:	--userns argument, can be an empty string
- * @optarg:		PID, path or name of namespace
+ * conf_ns_open() - Open network, user namespaces descriptors from configuration
+ * @c:		Execution context
+ * @userns:	--userns argument, can be an empty string
+ * @netns:	network namespace path
  *
  * Return: 0 on success, negative error code otherwise
  */
-static int conf_ns_opt(struct ctx *c,
-		       const char *conf_userns, const char *optarg)
+static int conf_ns_open(struct ctx *c, const char *userns, const char *netns)
 {
-	int ufd = -1, nfd = -1, try, ret, netns_only_reset = c->netns_only;
-	char userns[PATH_MAX] = { 0 }, netns[PATH_MAX];
-	char *endptr;
-	long pid_arg;
-	pid_t pid;
+	int ufd = -1, nfd = -1;
 
-	if (c->netns_only && *conf_userns) {
+	if (c->netns_only && *userns) {
 		err("Both --userns and --netns-only given");
 		return -EINVAL;
 	}
 
-	/* It might be a PID, a netns path, or a netns name */
-	for (try = 0; try < 3; try++) {
-		if (try == 0) {
-			pid_arg = strtol(optarg, &endptr, 10);
-			if (*endptr || pid_arg < 0 || pid_arg > INT_MAX)
-				continue;
-
-			pid = pid_arg;
-
-			if (!*conf_userns && !c->netns_only) {
-				ret = snprintf(userns, PATH_MAX,
-					       "/proc/%i/ns/user", pid);
-				if (ret <= 0 || ret > (int)sizeof(userns))
-					continue;
-			}
-			ret = snprintf(netns, PATH_MAX, "/proc/%i/ns/net", pid);
-			if (ret <= 0 || ret > (int)sizeof(netns))
-				continue;
-		} else if (try == 1) {
-			if (!*conf_userns)
-				c->netns_only = 1;
-
-			ret = snprintf(netns, PATH_MAX, "%s", optarg);
-			if (ret <= 0 || ret > (int)sizeof(userns))
-				continue;
-		} else if (try == 2) {
-			ret = snprintf(netns, PATH_MAX, "%s/%s",
-				       NETNS_RUN_DIR, optarg);
-			if (ret <= 0 || ret > (int)sizeof(netns))
-				continue;
-		}
-
-		if (!c->netns_only) {
-			if (*conf_userns)
-				ufd = open(conf_userns, O_RDONLY | O_CLOEXEC);
-			else if (*userns)
-				ufd = open(userns, O_RDONLY | O_CLOEXEC);
-		}
-
-		nfd = open(netns, O_RDONLY | O_CLOEXEC);
-
-		if (nfd == -1 || (ufd == -1 && !c->netns_only)) {
-			if (nfd >= 0)
-				close(nfd);
-
-			if (ufd >= 0)
-				close(ufd);
+	nfd = open(netns, O_RDONLY | O_CLOEXEC);
+	if (nfd < 0) {
+		err("Couldn't open network namespace %s", netns);
+		return -ENOENT;
+	}
 
-			continue;
+	if (!c->netns_only && *userns) {
+		ufd = open(userns, O_RDONLY | O_CLOEXEC);
+		if (ufd < 0) {
+			close(nfd);
+			err("Couldn't open user namespace %s", userns);
+			return -ENOENT;
 		}
+	}
 
-		c->pasta_netns_fd = nfd;
-		c->pasta_userns_fd = ufd;
-
-		NS_CALL(conf_ns_check, c);
+	c->pasta_netns_fd = nfd;
+	c->pasta_userns_fd = ufd;
+	c->netns_only = !*userns;
 
-		if (c->pasta_netns_fd >= 0) {
-			char buf[PATH_MAX];
+	NS_CALL(conf_ns_check, c);
 
-			if (try == 0 || c->no_netns_quit)
-				return 0;
+	if (c->pasta_netns_fd < 0) {
+		err("Couldn't switch to pasta namespaces");
+		return -ENOENT;
+	}
 
-			strncpy(buf, netns, PATH_MAX);
-			strncpy(c->netns_base, basename(buf), PATH_MAX - 1);
-			strncpy(buf, netns, PATH_MAX);
-			strncpy(c->netns_dir, dirname(buf), PATH_MAX - 1);
+	if (!c->no_netns_quit) {
+		char buf[PATH_MAX];
 
-			return 0;
-		}
+		strncpy(buf, netns, PATH_MAX);
+		strncpy(c->netns_base, basename(buf), PATH_MAX - 1);
+		strncpy(buf, netns, PATH_MAX);
+		strncpy(c->netns_dir, dirname(buf), PATH_MAX - 1);
 	}
 
-	c->netns_only = netns_only_reset;
-
-	err("Namespace %s not found", optarg);
-	return -ENOENT;
+	return 0;
 }
 
 /**
@@ -1051,7 +1052,7 @@ void conf(struct ctx *c, int argc, char **argv)
 		{ 0 },
 	};
 	struct get_bound_ports_ns_arg ns_ports_arg = { .c = c };
-	char userns[PATH_MAX] = { 0 };
+	char userns[PATH_MAX] = { 0 }, netns[PATH_MAX] = { 0 };
 	enum conf_port_type tcp_tap = 0, tcp_init = 0;
 	enum conf_port_type udp_tap = 0, udp_init = 0;
 	bool v4_only = false, v6_only = false;
@@ -1446,7 +1447,7 @@ void conf(struct ctx *c, int argc, char **argv)
 	check_root(c);
 
 	if (c->mode == MODE_PASTA && optind + 1 == argc) {
-		ret = conf_ns_opt(c, userns, argv[optind]);
+		ret = conf_ns_opt(userns, netns, argv[optind]);
 		if (ret < 0)
 			usage(argv[0]);
 	} else if (c->mode == MODE_PASTA && *userns && optind == argc) {
@@ -1459,8 +1460,15 @@ void conf(struct ctx *c, int argc, char **argv)
 	if (c->pasta_conf_ns)
 		c->no_ra = 1;
 
-	if (c->mode == MODE_PASTA && c->pasta_netns_fd == -1)
-		pasta_start_ns(c);
+	if (c->mode == MODE_PASTA) {
+		if (*netns) {
+			ret = conf_ns_open(c, userns, netns);
+			if (ret < 0)
+				usage(argv[0]);
+		} else {
+			pasta_start_ns(c);
+		}
+	}
 
 	if (nl_sock_init(c)) {
 		err("Failed to get netlink socket");
-- 
@@ -489,6 +489,51 @@ out:
 		warn("Couldn't get any nameserver address");
 }
 
+/**
+ * conf_ns_opt() - Parse non-option argument to namespace paths
+ * @userns:	buffer of size PATH_MAX, initially contains --userns
+ *		argument (may be empty), updated with userns path
+ * @netns:	buffer of size PATH_MAX, updated with netns path
+ * @arg:	PID, path or name of network namespace
+ *
+ * Return: 0 on success, negative error code otherwise
+ */
+static int conf_ns_opt(char *userns, char *netns, const char *arg)
+{
+	char *endptr;
+	long pidval;
+	int ret;
+
+	pidval = strtol(arg, &endptr, 10);
+	if (!*endptr) {
+		/* Looks like a pid */
+		if (pidval < 0 || pidval > INT_MAX) {
+			err("Invalid PID %s", arg);
+			return -EINVAL;
+		}
+
+		snprintf(netns, PATH_MAX, "/proc/%ld/ns/net", pidval);
+		if (!*userns)
+			snprintf(userns, PATH_MAX, "/proc/%ld/ns/user", pidval);
+		return 0;
+	}
+
+	if (!strchr(arg, '/')) {
+		/* looks like a netns name */
+		ret = snprintf(netns, PATH_MAX, "%s/%s", NETNS_RUN_DIR, arg);
+	} else {
+		/* otherwise assume it's a netns path */
+		ret = snprintf(netns, PATH_MAX, "%s", arg);
+	}
+
+	if (ret <= 0 || ret > PATH_MAX) {
+		err("Network namespace name/path %s too long");
+		return -E2BIG;
+	}
+
+	return 0;
+}
+
 /**
  * conf_ns_check() - Check if we can enter configured namespaces
  * @arg:	Execution context
@@ -508,102 +553,58 @@ static int conf_ns_check(void *arg)
 }
 
 /**
- * conf_ns_opt() - Open network, user namespaces descriptors from configuration
- * @c:			Execution context
- * @conf_userns:	--userns argument, can be an empty string
- * @optarg:		PID, path or name of namespace
+ * conf_ns_open() - Open network, user namespaces descriptors from configuration
+ * @c:		Execution context
+ * @userns:	--userns argument, can be an empty string
+ * @netns:	network namespace path
  *
  * Return: 0 on success, negative error code otherwise
  */
-static int conf_ns_opt(struct ctx *c,
-		       const char *conf_userns, const char *optarg)
+static int conf_ns_open(struct ctx *c, const char *userns, const char *netns)
 {
-	int ufd = -1, nfd = -1, try, ret, netns_only_reset = c->netns_only;
-	char userns[PATH_MAX] = { 0 }, netns[PATH_MAX];
-	char *endptr;
-	long pid_arg;
-	pid_t pid;
+	int ufd = -1, nfd = -1;
 
-	if (c->netns_only && *conf_userns) {
+	if (c->netns_only && *userns) {
 		err("Both --userns and --netns-only given");
 		return -EINVAL;
 	}
 
-	/* It might be a PID, a netns path, or a netns name */
-	for (try = 0; try < 3; try++) {
-		if (try == 0) {
-			pid_arg = strtol(optarg, &endptr, 10);
-			if (*endptr || pid_arg < 0 || pid_arg > INT_MAX)
-				continue;
-
-			pid = pid_arg;
-
-			if (!*conf_userns && !c->netns_only) {
-				ret = snprintf(userns, PATH_MAX,
-					       "/proc/%i/ns/user", pid);
-				if (ret <= 0 || ret > (int)sizeof(userns))
-					continue;
-			}
-			ret = snprintf(netns, PATH_MAX, "/proc/%i/ns/net", pid);
-			if (ret <= 0 || ret > (int)sizeof(netns))
-				continue;
-		} else if (try == 1) {
-			if (!*conf_userns)
-				c->netns_only = 1;
-
-			ret = snprintf(netns, PATH_MAX, "%s", optarg);
-			if (ret <= 0 || ret > (int)sizeof(userns))
-				continue;
-		} else if (try == 2) {
-			ret = snprintf(netns, PATH_MAX, "%s/%s",
-				       NETNS_RUN_DIR, optarg);
-			if (ret <= 0 || ret > (int)sizeof(netns))
-				continue;
-		}
-
-		if (!c->netns_only) {
-			if (*conf_userns)
-				ufd = open(conf_userns, O_RDONLY | O_CLOEXEC);
-			else if (*userns)
-				ufd = open(userns, O_RDONLY | O_CLOEXEC);
-		}
-
-		nfd = open(netns, O_RDONLY | O_CLOEXEC);
-
-		if (nfd == -1 || (ufd == -1 && !c->netns_only)) {
-			if (nfd >= 0)
-				close(nfd);
-
-			if (ufd >= 0)
-				close(ufd);
+	nfd = open(netns, O_RDONLY | O_CLOEXEC);
+	if (nfd < 0) {
+		err("Couldn't open network namespace %s", netns);
+		return -ENOENT;
+	}
 
-			continue;
+	if (!c->netns_only && *userns) {
+		ufd = open(userns, O_RDONLY | O_CLOEXEC);
+		if (ufd < 0) {
+			close(nfd);
+			err("Couldn't open user namespace %s", userns);
+			return -ENOENT;
 		}
+	}
 
-		c->pasta_netns_fd = nfd;
-		c->pasta_userns_fd = ufd;
-
-		NS_CALL(conf_ns_check, c);
+	c->pasta_netns_fd = nfd;
+	c->pasta_userns_fd = ufd;
+	c->netns_only = !*userns;
 
-		if (c->pasta_netns_fd >= 0) {
-			char buf[PATH_MAX];
+	NS_CALL(conf_ns_check, c);
 
-			if (try == 0 || c->no_netns_quit)
-				return 0;
+	if (c->pasta_netns_fd < 0) {
+		err("Couldn't switch to pasta namespaces");
+		return -ENOENT;
+	}
 
-			strncpy(buf, netns, PATH_MAX);
-			strncpy(c->netns_base, basename(buf), PATH_MAX - 1);
-			strncpy(buf, netns, PATH_MAX);
-			strncpy(c->netns_dir, dirname(buf), PATH_MAX - 1);
+	if (!c->no_netns_quit) {
+		char buf[PATH_MAX];
 
-			return 0;
-		}
+		strncpy(buf, netns, PATH_MAX);
+		strncpy(c->netns_base, basename(buf), PATH_MAX - 1);
+		strncpy(buf, netns, PATH_MAX);
+		strncpy(c->netns_dir, dirname(buf), PATH_MAX - 1);
 	}
 
-	c->netns_only = netns_only_reset;
-
-	err("Namespace %s not found", optarg);
-	return -ENOENT;
+	return 0;
 }
 
 /**
@@ -1051,7 +1052,7 @@ void conf(struct ctx *c, int argc, char **argv)
 		{ 0 },
 	};
 	struct get_bound_ports_ns_arg ns_ports_arg = { .c = c };
-	char userns[PATH_MAX] = { 0 };
+	char userns[PATH_MAX] = { 0 }, netns[PATH_MAX] = { 0 };
 	enum conf_port_type tcp_tap = 0, tcp_init = 0;
 	enum conf_port_type udp_tap = 0, udp_init = 0;
 	bool v4_only = false, v6_only = false;
@@ -1446,7 +1447,7 @@ void conf(struct ctx *c, int argc, char **argv)
 	check_root(c);
 
 	if (c->mode == MODE_PASTA && optind + 1 == argc) {
-		ret = conf_ns_opt(c, userns, argv[optind]);
+		ret = conf_ns_opt(userns, netns, argv[optind]);
 		if (ret < 0)
 			usage(argv[0]);
 	} else if (c->mode == MODE_PASTA && *userns && optind == argc) {
@@ -1459,8 +1460,15 @@ void conf(struct ctx *c, int argc, char **argv)
 	if (c->pasta_conf_ns)
 		c->no_ra = 1;
 
-	if (c->mode == MODE_PASTA && c->pasta_netns_fd == -1)
-		pasta_start_ns(c);
+	if (c->mode == MODE_PASTA) {
+		if (*netns) {
+			ret = conf_ns_open(c, userns, netns);
+			if (ret < 0)
+				usage(argv[0]);
+		} else {
+			pasta_start_ns(c);
+		}
+	}
 
 	if (nl_sock_init(c)) {
 		err("Failed to get netlink socket");
-- 
2.37.2

[PATCH 7/8] Use explicit --netns option rather than multiplexing with PID

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 6896 bytes --]

When attaching to an existing namespace, pasta can take a PID or the name
or path of a network namespace as a non-option parameter.  We disambiguate
based on what the parameter looks like.  Make this more explicit by using
a --netns option for explicitly giving the path or name, and treating a
non-option argument always as a PID.

Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au>
---
 conf.c  | 85 ++++++++++++++++++++++++++++++++++++++++-----------------
 passt.1 | 16 +++++++++--
 2 files changed, 73 insertions(+), 28 deletions(-)

diff --git a/conf.c b/conf.c
index 10b7b9f..2a18124 100644
--- a/conf.c
+++ b/conf.c
@@ -490,19 +490,51 @@ out:
 }
 
 /**
- * conf_ns_opt() - Parse non-option argument to namespace paths
+ * conf_netns() - Parse --netns option
+ * @netns:	buffer of size PATH_MAX, updated with netns path
+ * @arg:	--netns argument
+ *
+ * Return: 0 on success, negative error code otherwise
+ */
+static int conf_netns(char *netns, const char *arg)
+{
+	int ret;
+
+	if (!strchr(arg, '/')) {
+		/* looks like a netns name */
+		ret = snprintf(netns, PATH_MAX, "%s/%s", NETNS_RUN_DIR, arg);
+	} else {
+		/* otherwise assume it's a netns path */
+		ret = snprintf(netns, PATH_MAX, "%s", arg);
+	}
+
+	if (ret <= 0 || ret > PATH_MAX) {
+		err("Network namespace name/path %s too long");
+		return -E2BIG;
+	}
+
+	return 0;
+}
+
+/**
+ * conf_ns_pid() - Parse non-option argument as a PID
  * @userns:	buffer of size PATH_MAX, initially contains --userns
  *		argument (may be empty), updated with userns path
- * @netns:	buffer of size PATH_MAX, updated with netns path
- * @arg:	PID, path or name of network namespace
+ * @netns:	buffer of size PATH_MAX, initial contains --netns
+ *		argument (may be empty), updated with netns path
+ * @arg:	PID of network namespace
  *
  * Return: 0 on success, negative error code otherwise
  */
-static int conf_ns_opt(char *userns, char *netns, const char *arg)
+static int conf_ns_pid(char *userns, char *netns, const char *arg)
 {
 	char *endptr;
 	long pidval;
-	int ret;
+
+	if (*netns) {
+		err("Both --netns and PID given");
+		return -EINVAL;
+	}
 
 	pidval = strtol(arg, &endptr, 10);
 	if (!*endptr) {
@@ -518,20 +550,7 @@ static int conf_ns_opt(char *userns, char *netns, const char *arg)
 		return 0;
 	}
 
-	if (!strchr(arg, '/')) {
-		/* looks like a netns name */
-		ret = snprintf(netns, PATH_MAX, "%s/%s", NETNS_RUN_DIR, arg);
-	} else {
-		/* otherwise assume it's a netns path */
-		ret = snprintf(netns, PATH_MAX, "%s", arg);
-	}
-
-	if (ret <= 0 || ret > PATH_MAX) {
-		err("Network namespace name/path %s too long");
-		return -E2BIG;
-	}
-
-	return 0;
+	return -EINVAL;
 }
 
 /**
@@ -708,10 +727,13 @@ static unsigned int conf_ip6(unsigned int ifi,
 static void usage(const char *name)
 {
 	if (strstr(name, "pasta")) {
-		info("Usage: %s [OPTION]... [PID|PATH|NAME]", name);
+		info("Usage: %s [OPTION]... [COMMAND] [ARGS]...", name);
+		info("       %s [OPTION]... PID", name);
+		info("       %s [OPTION]... --netns [PATH|NAME]", name);
 		info("");
-		info("Without PID|PATH|NAME, run the default shell in a new");
-		info("network and user namespace, and connect it via pasta.");
+		info("Without PID or --netns, run the given command or a");
+		info("default shell in a new network and user namespace, and");
+		info("connect it via pasta.");
 	} else {
 		info("Usage: %s [OPTION]...", name);
 	}
@@ -858,6 +880,7 @@ pasta_opts:
 	info(   "    SPEC is as described above");
 	info(   "    default: auto");
 	info(   "  --userns NSPATH 	Target user namespace to join");
+	info(   "  --netns PATH|NAME	Target network namespace to join");
 	info(   "  --netns-only		Don't join existing user namespace");
 	info(   "    implied if PATH or NAME are given without --userns");
 	info(   "  --config-net		Configure tap interface in namespace");
@@ -1038,6 +1061,7 @@ void conf(struct ctx *c, int argc, char **argv)
 		{"tcp-ns",	required_argument,	NULL,		'T' },
 		{"udp-ns",	required_argument,	NULL,		'U' },
 		{"userns",	required_argument,	NULL,		2 },
+		{"netns",	required_argument,	NULL,		3 },
 		{"netns-only",	no_argument,		&c->netns_only,	1 },
 		{"config-net",	no_argument,		&c->pasta_conf_ns, 1 },
 		{"ns-mac-addr",	required_argument,	NULL,		4 },
@@ -1091,6 +1115,16 @@ void conf(struct ctx *c, int argc, char **argv)
 				usage(argv[0]);
 			}
 			break;
+		case 3:
+			if (c->mode != MODE_PASTA) {
+				err("--netns is for pasta mode only");
+				usage(argv[0]);
+			}
+
+			ret = conf_netns(netns, optarg);
+			if (ret < 0)
+				usage(argv[0]);
+			break;
 		case 4:
 			if (c->mode != MODE_PASTA) {
 				err("--ns-mac-addr is for pasta mode only");
@@ -1447,11 +1481,12 @@ void conf(struct ctx *c, int argc, char **argv)
 	check_root(c);
 
 	if (c->mode == MODE_PASTA && optind + 1 == argc) {
-		ret = conf_ns_opt(userns, netns, argv[optind]);
+		ret = conf_ns_pid(userns, netns, argv[optind]);
 		if (ret < 0)
 			usage(argv[0]);
-	} else if (c->mode == MODE_PASTA && *userns && optind == argc) {
-		err("--userns requires PID, PATH or NAME");
+	} else if (c->mode == MODE_PASTA && *userns
+		   && !*netns && optind == argc) {
+		err("--userns requires --netns or PID");
 		usage(argv[0]);
 	} else if (optind != argc) {
 		usage(argv[0]);
diff --git a/passt.1 b/passt.1
index bbdadc1..4a09ced 100644
--- a/passt.1
+++ b/passt.1
@@ -15,7 +15,10 @@
 [\fIOPTION\fR]...
 .br
 .B pasta
-[\fIOPTION\fR]... [\fIPID\fR|\fIPATH\fR|\fINAME\fR]
+[\fIOPTION\fR]... [\fIPID\fR]
+.br
+.B pasta
+[\fIOPTION\fR]... \fB--netns\fR [\fIPATH\fR|\fINAME\fR]
 
 .SH DESCRIPTION
 
@@ -59,7 +62,7 @@ or with the \fBqrap\fR(1) wrapper.
 equivalent functionality to network namespaces, as the one offered by
 \fBpasst\fR for virtual machines.
 
-If PID, PATH or NAME are given, \fBpasta\fR associates to an existing user and
+If PID or --netns are given, \fBpasta\fR associates to an existing user and
 network namespace. Otherwise, \fBpasta\fR creates a new user and network
 namespace, and spawns an interactive shell within this context. A \fItap\fR
 device within the network namespace is created to provide network connectivity.
@@ -445,7 +448,14 @@ Default is \fBauto\fR.
 Target user namespace to join, as a path. If PID is given, without this option,
 the user namespace will be the one of the corresponding process.
 
-This option requires PID, PATH or NAME to be specified.
+This option requires --netns or a PID to be specified.
+
+.TP
+.BR \-\-netns " " \fIspec
+Target network namespace to join, as a path or a name.  A name is treated as
+with \fBip-netns(8)\fR as a equivalent to a path in \fI/run/netns\fR.
+
+This option can't be specified with a PID.
 
 .TP
 .BR \-\-netns-only
-- 
@@ -15,7 +15,10 @@
 [\fIOPTION\fR]...
 .br
 .B pasta
-[\fIOPTION\fR]... [\fIPID\fR|\fIPATH\fR|\fINAME\fR]
+[\fIOPTION\fR]... [\fIPID\fR]
+.br
+.B pasta
+[\fIOPTION\fR]... \fB--netns\fR [\fIPATH\fR|\fINAME\fR]
 
 .SH DESCRIPTION
 
@@ -59,7 +62,7 @@ or with the \fBqrap\fR(1) wrapper.
 equivalent functionality to network namespaces, as the one offered by
 \fBpasst\fR for virtual machines.
 
-If PID, PATH or NAME are given, \fBpasta\fR associates to an existing user and
+If PID or --netns are given, \fBpasta\fR associates to an existing user and
 network namespace. Otherwise, \fBpasta\fR creates a new user and network
 namespace, and spawns an interactive shell within this context. A \fItap\fR
 device within the network namespace is created to provide network connectivity.
@@ -445,7 +448,14 @@ Default is \fBauto\fR.
 Target user namespace to join, as a path. If PID is given, without this option,
 the user namespace will be the one of the corresponding process.
 
-This option requires PID, PATH or NAME to be specified.
+This option requires --netns or a PID to be specified.
+
+.TP
+.BR \-\-netns " " \fIspec
+Target network namespace to join, as a path or a name.  A name is treated as
+with \fBip-netns(8)\fR as a equivalent to a path in \fI/run/netns\fR.
+
+This option can't be specified with a PID.
 
 .TP
 .BR \-\-netns-only
-- 
2.37.2

[PATCH 8/8] Allow pasta to take a command to execute

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 5941 bytes --]

When not given an existing PID or network namspace to attach to, pasta
spawns a shell.  Most commands which can spawn a shell in an altered
environment can also run other commands in that same environment, which can
be useful in automation.

Allow pasta to do the same thing; it can be given an arbitrary command to
run in the network and user namespace which pasta creates.  If neither a
command nor an existing PID or netns to attach to is given, continue to
spawn a default shell, as before.

Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au>
---
 conf.c  | 27 ++++++++++++++++++---------
 passt.1 | 14 +++++++++-----
 pasta.c | 33 +++++++++++++++++++++++----------
 pasta.h |  2 +-
 4 files changed, 51 insertions(+), 25 deletions(-)

diff --git a/conf.c b/conf.c
index 2a18124..162c2dd 100644
--- a/conf.c
+++ b/conf.c
@@ -550,7 +550,8 @@ static int conf_ns_pid(char *userns, char *netns, const char *arg)
 		return 0;
 	}
 
-	return -EINVAL;
+	/* Not a PID, later code will treat as a command */
+	return 0;
 }
 
 /**
@@ -1480,14 +1481,18 @@ void conf(struct ctx *c, int argc, char **argv)
 
 	check_root(c);
 
-	if (c->mode == MODE_PASTA && optind + 1 == argc) {
-		ret = conf_ns_pid(userns, netns, argv[optind]);
-		if (ret < 0)
+	if (c->mode == MODE_PASTA) {
+		if (*netns && optind != argc) {
+			err("Both --netns and PID or command given");
 			usage(argv[0]);
-	} else if (c->mode == MODE_PASTA && *userns
-		   && !*netns && optind == argc) {
-		err("--userns requires --netns or PID");
-		usage(argv[0]);
+		} else if (optind + 1 == argc) {
+			ret = conf_ns_pid(userns, netns, argv[optind]);
+			if (ret < 0)
+				usage(argv[0]);
+		} else if (*userns && !*netns && optind == argc) {
+			err("--userns requires --netns or PID");
+			usage(argv[0]);
+		}
 	} else if (optind != argc) {
 		usage(argv[0]);
 	}
@@ -1501,7 +1506,11 @@ void conf(struct ctx *c, int argc, char **argv)
 			if (ret < 0)
 				usage(argv[0]);
 		} else {
-			pasta_start_ns(c);
+			if (*userns) {
+				err("Both --userns and command given");
+				usage(argv[0]);
+			}
+			pasta_start_ns(c, argc - optind, argv + optind);
 		}
 	}
 
diff --git a/passt.1 b/passt.1
index 4a09ced..3cc5a9d 100644
--- a/passt.1
+++ b/passt.1
@@ -15,7 +15,10 @@
 [\fIOPTION\fR]...
 .br
 .B pasta
-[\fIOPTION\fR]... [\fIPID\fR]
+[\fIOPTION\fR]... [\fICOMMAND\fR [\fIARG\fR]...]
+.br
+.B pasta
+[\fIOPTION\fR]... \fIPID\fR
 .br
 .B pasta
 [\fIOPTION\fR]... \fB--netns\fR [\fIPATH\fR|\fINAME\fR]
@@ -62,10 +65,11 @@ or with the \fBqrap\fR(1) wrapper.
 equivalent functionality to network namespaces, as the one offered by
 \fBpasst\fR for virtual machines.
 
-If PID or --netns are given, \fBpasta\fR associates to an existing user and
-network namespace. Otherwise, \fBpasta\fR creates a new user and network
-namespace, and spawns an interactive shell within this context. A \fItap\fR
-device within the network namespace is created to provide network connectivity.
+If PID or --netns are given, \fBpasta\fR associates to an existing
+user and network namespace. Otherwise, \fBpasta\fR creates a new user
+and network namespace, and spawns the given command or a default shell
+within this context. A \fItap\fR device within the network namespace
+is created to provide network connectivity.
 
 For local TCP and UDP traffic only, \fBpasta\fR also implements a bypass path
 directly mapping Layer-4 sockets between \fIinit\fR and target namespaces,
diff --git a/pasta.c b/pasta.c
index 830748f..a844af2 100644
--- a/pasta.c
+++ b/pasta.c
@@ -108,6 +108,7 @@ netns:
 struct pasta_setup_ns_arg {
 	struct ctx *c;
 	int euid;
+	char **argv;
 };
 
 /**
@@ -119,7 +120,6 @@ struct pasta_setup_ns_arg {
 static int pasta_setup_ns(void *arg)
 {
 	struct pasta_setup_ns_arg *a = (struct pasta_setup_ns_arg *)arg;
-	char *shell;
 
 	if (!a->c->netns_only) {
 		char buf[BUFSIZ];
@@ -139,29 +139,42 @@ static int pasta_setup_ns(void *arg)
 	FWRITE("/proc/sys/net/ipv4/ping_group_range", "0 0",
 	       "Cannot set ping_group_range, ICMP requests might fail");
 
-	shell = getenv("SHELL") ? getenv("SHELL") : "/bin/sh";
-	if (strstr(shell, "/bash"))
-		execve(shell, ((char *[]) { shell, "-l", NULL }), environ);
-	else
-		execve(shell, ((char *[]) { shell, NULL }), environ);
+	execvp(a->argv[0], a->argv);
 
-	perror("execve");
+	perror("execvp");
 	exit(EXIT_FAILURE);
 }
 
 /**
- * pasta_start_ns() - Fork shell in new namespace if target ns is not given
+ * pasta_start_ns() - Fork command in new namespace if target ns is not given
  * @c:		Execution context
+ * @argc:	Number of arguments for spawned command
+ * @argv:	Command to spawn and arguments
  */
-void pasta_start_ns(struct ctx *c)
+void pasta_start_ns(struct ctx *c, int argc, char *argv[])
 {
-	struct pasta_setup_ns_arg arg = { .c = c, .euid = geteuid() };
+	struct pasta_setup_ns_arg arg = {
+		.c = c,
+		.euid = geteuid(),
+		.argv = argv,
+	};
+	char *shell = getenv("SHELL") ? getenv("SHELL") : "/bin/sh";
+	char *sh_argv[] = { shell, NULL };
+	char *bash_argv[] = { shell, "-l", NULL };
 	char ns_fn_stack[NS_FN_STACK_SIZE];
 
 	c->foreground = 1;
 	if (!c->debug)
 		c->quiet = 1;
 
+	if (argc == 0) {
+		if (strstr(shell, "/bash")) {
+			arg.argv = bash_argv;
+		} else {
+			arg.argv = sh_argv;
+		}
+	}
+
 	pasta_child_pid = clone(pasta_setup_ns,
 				ns_fn_stack + sizeof(ns_fn_stack) / 2,
 				(c->netns_only ? 0 : CLONE_NEWNET) |
diff --git a/pasta.h b/pasta.h
index 8c80006..19b2e54 100644
--- a/pasta.h
+++ b/pasta.h
@@ -6,7 +6,7 @@
 #ifndef PASTA_H
 #define PASTA_H
 
-void pasta_start_ns(struct ctx *c);
+void pasta_start_ns(struct ctx *c, int argc, char *argv[]);
 void pasta_ns_conf(struct ctx *c);
 void pasta_child_handler(int signal);
 int pasta_netns_quit_init(struct ctx *c);
-- 
@@ -6,7 +6,7 @@
 #ifndef PASTA_H
 #define PASTA_H
 
-void pasta_start_ns(struct ctx *c);
+void pasta_start_ns(struct ctx *c, int argc, char *argv[]);
 void pasta_ns_conf(struct ctx *c);
 void pasta_child_handler(int signal);
 int pasta_netns_quit_init(struct ctx *c);
-- 
2.37.2

Re: [PATCH 7/8] Use explicit --netns option rather than multiplexing with PID

[Stefano Brivio]

[-- Attachment #1: Type: text/plain, Size: 1550 bytes --]

On Fri, 26 Aug 2022 14:58:38 +1000
David Gibson <david(a)gibson.dropbear.id.au> wrote:

> [...]
>
> +++ b/passt.1
> @@ -15,7 +15,10 @@
>  [\fIOPTION\fR]...
>  .br
>  .B pasta
> -[\fIOPTION\fR]... [\fIPID\fR|\fIPATH\fR|\fINAME\fR]
> +[\fIOPTION\fR]... [\fIPID\fR]
> +.br
> +.B pasta
> +[\fIOPTION\fR]... \fB--netns\fR [\fIPATH\fR|\fINAME\fR]
>  
>  .SH DESCRIPTION
>  
> @@ -59,7 +62,7 @@ or with the \fBqrap\fR(1) wrapper.
>  equivalent functionality to network namespaces, as the one offered by
>  \fBpasst\fR for virtual machines.
>  
> -If PID, PATH or NAME are given, \fBpasta\fR associates to an existing user and
> +If PID or --netns are given, \fBpasta\fR associates to an existing user and
>  network namespace. Otherwise, \fBpasta\fR creates a new user and network
>  namespace, and spawns an interactive shell within this context. A \fItap\fR
>  device within the network namespace is created to provide network connectivity.
> @@ -445,7 +448,14 @@ Default is \fBauto\fR.
>  Target user namespace to join, as a path. If PID is given, without this option,
>  the user namespace will be the one of the corresponding process.
>  
> -This option requires PID, PATH or NAME to be specified.
> +This option requires --netns or a PID to be specified.
> +
> +.TP
> +.BR \-\-netns " " \fIspec
> +Target network namespace to join, as a path or a name.  A name is treated as
> +with \fBip-netns(8)\fR as a equivalent to a path in \fI/run/netns\fR.

an equivalent (I can fix this up on merge).

-- 
Stefano

Re: [PATCH 8/8] Allow pasta to take a command to execute

[Stefano Brivio]

[-- Attachment #1: Type: text/plain, Size: 2842 bytes --]

On Fri, 26 Aug 2022 14:58:39 +1000
David Gibson <david(a)gibson.dropbear.id.au> wrote:

> When not given an existing PID or network namspace to attach to, pasta
> spawns a shell.  Most commands which can spawn a shell in an altered
> environment can also run other commands in that same environment, which can
> be useful in automation.
> 
> Allow pasta to do the same thing; it can be given an arbitrary command to
> run in the network and user namespace which pasta creates.  If neither a
> command nor an existing PID or netns to attach to is given, continue to
> spawn a default shell, as before.
> 
> Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au>
> ---
>  conf.c  | 27 ++++++++++++++++++---------
>  passt.1 | 14 +++++++++-----
>  pasta.c | 33 +++++++++++++++++++++++----------
>  pasta.h |  2 +-
>  4 files changed, 51 insertions(+), 25 deletions(-)
> 
> diff --git a/conf.c b/conf.c
> index 2a18124..162c2dd 100644
> --- a/conf.c
> +++ b/conf.c
> @@ -550,7 +550,8 @@ static int conf_ns_pid(char *userns, char *netns, const char *arg)
>  		return 0;
>  	}
>  
> -	return -EINVAL;
> +	/* Not a PID, later code will treat as a command */
> +	return 0;
>  }
>  
>  /**
> @@ -1480,14 +1481,18 @@ void conf(struct ctx *c, int argc, char **argv)
>  
>  	check_root(c);
>  
> -	if (c->mode == MODE_PASTA && optind + 1 == argc) {
> -		ret = conf_ns_pid(userns, netns, argv[optind]);
> -		if (ret < 0)
> +	if (c->mode == MODE_PASTA) {
> +		if (*netns && optind != argc) {
> +			err("Both --netns and PID or command given");
>  			usage(argv[0]);
> -	} else if (c->mode == MODE_PASTA && *userns
> -		   && !*netns && optind == argc) {
> -		err("--userns requires --netns or PID");
> -		usage(argv[0]);
> +		} else if (optind + 1 == argc) {
> +			ret = conf_ns_pid(userns, netns, argv[optind]);
> +			if (ret < 0)
> +				usage(argv[0]);
> +		} else if (*userns && !*netns && optind == argc) {
> +			err("--userns requires --netns or PID");
> +			usage(argv[0]);
> +		}
>  	} else if (optind != argc) {
>  		usage(argv[0]);
>  	}
>
> [...]

I haven't really looked into this yet, but I guess we should now
handle getopts return codes a bit differently, because this works:

  $ ./pasta --config-net -- sh -c 'sleep 1; ip li sh'
  1: lo: <LOOPBACK,UP,LOWER_UP> mtu 65536 qdisc noqueue state UNKNOWN mode DEFAULT group default qlen 1000
      link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
  2: enp9s0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 65520 qdisc pfifo_fast state UNKNOWN mode DEFAULT group default qlen 1000
      link/ether aa:3e:39:5f:c6:15 brd ff:ff:ff:ff:ff:ff

while this doesn't:

  $ ./pasta --config-net sh -c 'sleep 1; ip li sh'
  ./pasta: invalid option -- 'c'
  [...]

despite the fact that there's no ambiguity.

-- 
Stefano

Re: [PATCH 7/8] Use explicit --netns option rather than multiplexing with PID

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 1988 bytes --]

On Mon, Aug 29, 2022 at 09:16:50PM +0200, Stefano Brivio wrote:
> On Fri, 26 Aug 2022 14:58:38 +1000
> David Gibson <david(a)gibson.dropbear.id.au> wrote:
> 
> > [...]
> >
> > +++ b/passt.1
> > @@ -15,7 +15,10 @@
> >  [\fIOPTION\fR]...
> >  .br
> >  .B pasta
> > -[\fIOPTION\fR]... [\fIPID\fR|\fIPATH\fR|\fINAME\fR]
> > +[\fIOPTION\fR]... [\fIPID\fR]
> > +.br
> > +.B pasta
> > +[\fIOPTION\fR]... \fB--netns\fR [\fIPATH\fR|\fINAME\fR]
> >  
> >  .SH DESCRIPTION
> >  
> > @@ -59,7 +62,7 @@ or with the \fBqrap\fR(1) wrapper.
> >  equivalent functionality to network namespaces, as the one offered by
> >  \fBpasst\fR for virtual machines.
> >  
> > -If PID, PATH or NAME are given, \fBpasta\fR associates to an existing user and
> > +If PID or --netns are given, \fBpasta\fR associates to an existing user and
> >  network namespace. Otherwise, \fBpasta\fR creates a new user and network
> >  namespace, and spawns an interactive shell within this context. A \fItap\fR
> >  device within the network namespace is created to provide network connectivity.
> > @@ -445,7 +448,14 @@ Default is \fBauto\fR.
> >  Target user namespace to join, as a path. If PID is given, without this option,
> >  the user namespace will be the one of the corresponding process.
> >  
> > -This option requires PID, PATH or NAME to be specified.
> > +This option requires --netns or a PID to be specified.
> > +
> > +.TP
> > +.BR \-\-netns " " \fIspec
> > +Target network namespace to join, as a path or a name.  A name is treated as
> > +with \fBip-netns(8)\fR as a equivalent to a path in \fI/run/netns\fR.
> 
> an equivalent (I can fix this up on merge).

Oops, sorry.  I think I meant to say just "as equivalent", which I
think reads better than "as an equivalent".

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 8/8] Allow pasta to take a command to execute

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 3786 bytes --]

On Mon, Aug 29, 2022 at 09:16:58PM +0200, Stefano Brivio wrote:
> On Fri, 26 Aug 2022 14:58:39 +1000
> David Gibson <david(a)gibson.dropbear.id.au> wrote:
> 
> > When not given an existing PID or network namspace to attach to, pasta
> > spawns a shell.  Most commands which can spawn a shell in an altered
> > environment can also run other commands in that same environment, which can
> > be useful in automation.
> > 
> > Allow pasta to do the same thing; it can be given an arbitrary command to
> > run in the network and user namespace which pasta creates.  If neither a
> > command nor an existing PID or netns to attach to is given, continue to
> > spawn a default shell, as before.
> > 
> > Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au>
> > ---
> >  conf.c  | 27 ++++++++++++++++++---------
> >  passt.1 | 14 +++++++++-----
> >  pasta.c | 33 +++++++++++++++++++++++----------
> >  pasta.h |  2 +-
> >  4 files changed, 51 insertions(+), 25 deletions(-)
> > 
> > diff --git a/conf.c b/conf.c
> > index 2a18124..162c2dd 100644
> > --- a/conf.c
> > +++ b/conf.c
> > @@ -550,7 +550,8 @@ static int conf_ns_pid(char *userns, char *netns, const char *arg)
> >  		return 0;
> >  	}
> >  
> > -	return -EINVAL;
> > +	/* Not a PID, later code will treat as a command */
> > +	return 0;
> >  }
> >  
> >  /**
> > @@ -1480,14 +1481,18 @@ void conf(struct ctx *c, int argc, char **argv)
> >  
> >  	check_root(c);
> >  
> > -	if (c->mode == MODE_PASTA && optind + 1 == argc) {
> > -		ret = conf_ns_pid(userns, netns, argv[optind]);
> > -		if (ret < 0)
> > +	if (c->mode == MODE_PASTA) {
> > +		if (*netns && optind != argc) {
> > +			err("Both --netns and PID or command given");
> >  			usage(argv[0]);
> > -	} else if (c->mode == MODE_PASTA && *userns
> > -		   && !*netns && optind == argc) {
> > -		err("--userns requires --netns or PID");
> > -		usage(argv[0]);
> > +		} else if (optind + 1 == argc) {
> > +			ret = conf_ns_pid(userns, netns, argv[optind]);
> > +			if (ret < 0)
> > +				usage(argv[0]);
> > +		} else if (*userns && !*netns && optind == argc) {
> > +			err("--userns requires --netns or PID");
> > +			usage(argv[0]);
> > +		}
> >  	} else if (optind != argc) {
> >  		usage(argv[0]);
> >  	}
> >
> > [...]
> 
> I haven't really looked into this yet, but I guess we should now
> handle getopts return codes a bit differently, because this works:
> 
>   $ ./pasta --config-net -- sh -c 'sleep 1; ip li sh'
>   1: lo: <LOOPBACK,UP,LOWER_UP> mtu 65536 qdisc noqueue state UNKNOWN mode DEFAULT group default qlen 1000
>       link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
>   2: enp9s0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 65520 qdisc pfifo_fast state UNKNOWN mode DEFAULT group default qlen 1000
>       link/ether aa:3e:39:5f:c6:15 brd ff:ff:ff:ff:ff:ff
> 
> while this doesn't:
> 
>   $ ./pasta --config-net sh -c 'sleep 1; ip li sh'
>   ./pasta: invalid option -- 'c'
>   [...]
> 
> despite the fact that there's no ambiguity.

You mean because pasta itself doesn't have a -c option?  Attempting to
account for that sounds like a bad idea.  Requiring -- when the
command given has options of its own that aren't supposed to go to the
wrapper is pretty common for these sorts of tools.  Basically the
trade-off is that you either need to require that, or you have to
require that all non-option arguments of the wrapper come last (old
style POSIXish command line parsing, as opposed to GNUish
conventions).  The latter is usually more awkward than the former.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 7/8] Use explicit --netns option rather than multiplexing with PID

[Stefano Brivio]

[-- Attachment #1: Type: text/plain, Size: 2038 bytes --]

On Tue, 30 Aug 2022 11:12:50 +1000
David Gibson <david(a)gibson.dropbear.id.au> wrote:

> On Mon, Aug 29, 2022 at 09:16:50PM +0200, Stefano Brivio wrote:
> > On Fri, 26 Aug 2022 14:58:38 +1000
> > David Gibson <david(a)gibson.dropbear.id.au> wrote:
> >   
> > > [...]
> > >
> > > +++ b/passt.1
> > > @@ -15,7 +15,10 @@
> > >  [\fIOPTION\fR]...
> > >  .br
> > >  .B pasta
> > > -[\fIOPTION\fR]... [\fIPID\fR|\fIPATH\fR|\fINAME\fR]
> > > +[\fIOPTION\fR]... [\fIPID\fR]
> > > +.br
> > > +.B pasta
> > > +[\fIOPTION\fR]... \fB--netns\fR [\fIPATH\fR|\fINAME\fR]
> > >  
> > >  .SH DESCRIPTION
> > >  
> > > @@ -59,7 +62,7 @@ or with the \fBqrap\fR(1) wrapper.
> > >  equivalent functionality to network namespaces, as the one offered by
> > >  \fBpasst\fR for virtual machines.
> > >  
> > > -If PID, PATH or NAME are given, \fBpasta\fR associates to an existing user and
> > > +If PID or --netns are given, \fBpasta\fR associates to an existing user and
> > >  network namespace. Otherwise, \fBpasta\fR creates a new user and network
> > >  namespace, and spawns an interactive shell within this context. A \fItap\fR
> > >  device within the network namespace is created to provide network connectivity.
> > > @@ -445,7 +448,14 @@ Default is \fBauto\fR.
> > >  Target user namespace to join, as a path. If PID is given, without this option,
> > >  the user namespace will be the one of the corresponding process.
> > >  
> > > -This option requires PID, PATH or NAME to be specified.
> > > +This option requires --netns or a PID to be specified.
> > > +
> > > +.TP
> > > +.BR \-\-netns " " \fIspec
> > > +Target network namespace to join, as a path or a name.  A name is treated as
> > > +with \fBip-netns(8)\fR as a equivalent to a path in \fI/run/netns\fR.  
> > 
> > an equivalent (I can fix this up on merge).  
> 
> Oops, sorry.  I think I meant to say just "as equivalent", which I
> think reads better than "as an equivalent".

Oh, right, then I'll change it to that.

-- 
Stefano

Re: [PATCH 8/8] Allow pasta to take a command to execute

[Stefano Brivio]

[-- Attachment #1: Type: text/plain, Size: 4397 bytes --]

On Tue, 30 Aug 2022 11:16:05 +1000
David Gibson <david(a)gibson.dropbear.id.au> wrote:

> On Mon, Aug 29, 2022 at 09:16:58PM +0200, Stefano Brivio wrote:
> > On Fri, 26 Aug 2022 14:58:39 +1000
> > David Gibson <david(a)gibson.dropbear.id.au> wrote:
> >   
> > > When not given an existing PID or network namspace to attach to, pasta
> > > spawns a shell.  Most commands which can spawn a shell in an altered
> > > environment can also run other commands in that same environment, which can
> > > be useful in automation.
> > > 
> > > Allow pasta to do the same thing; it can be given an arbitrary command to
> > > run in the network and user namespace which pasta creates.  If neither a
> > > command nor an existing PID or netns to attach to is given, continue to
> > > spawn a default shell, as before.
> > > 
> > > Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au>
> > > ---
> > >  conf.c  | 27 ++++++++++++++++++---------
> > >  passt.1 | 14 +++++++++-----
> > >  pasta.c | 33 +++++++++++++++++++++++----------
> > >  pasta.h |  2 +-
> > >  4 files changed, 51 insertions(+), 25 deletions(-)
> > > 
> > > diff --git a/conf.c b/conf.c
> > > index 2a18124..162c2dd 100644
> > > --- a/conf.c
> > > +++ b/conf.c
> > > @@ -550,7 +550,8 @@ static int conf_ns_pid(char *userns, char *netns, const char *arg)
> > >  		return 0;
> > >  	}
> > >  
> > > -	return -EINVAL;
> > > +	/* Not a PID, later code will treat as a command */
> > > +	return 0;
> > >  }
> > >  
> > >  /**
> > > @@ -1480,14 +1481,18 @@ void conf(struct ctx *c, int argc, char **argv)
> > >  
> > >  	check_root(c);
> > >  
> > > -	if (c->mode == MODE_PASTA && optind + 1 == argc) {
> > > -		ret = conf_ns_pid(userns, netns, argv[optind]);
> > > -		if (ret < 0)
> > > +	if (c->mode == MODE_PASTA) {
> > > +		if (*netns && optind != argc) {
> > > +			err("Both --netns and PID or command given");
> > >  			usage(argv[0]);
> > > -	} else if (c->mode == MODE_PASTA && *userns
> > > -		   && !*netns && optind == argc) {
> > > -		err("--userns requires --netns or PID");
> > > -		usage(argv[0]);
> > > +		} else if (optind + 1 == argc) {
> > > +			ret = conf_ns_pid(userns, netns, argv[optind]);
> > > +			if (ret < 0)
> > > +				usage(argv[0]);
> > > +		} else if (*userns && !*netns && optind == argc) {
> > > +			err("--userns requires --netns or PID");
> > > +			usage(argv[0]);
> > > +		}
> > >  	} else if (optind != argc) {
> > >  		usage(argv[0]);
> > >  	}
> > >
> > > [...]  
> > 
> > I haven't really looked into this yet, but I guess we should now
> > handle getopts return codes a bit differently, because this works:
> > 
> >   $ ./pasta --config-net -- sh -c 'sleep 1; ip li sh'
> >   1: lo: <LOOPBACK,UP,LOWER_UP> mtu 65536 qdisc noqueue state UNKNOWN mode DEFAULT group default qlen 1000
> >       link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
> >   2: enp9s0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 65520 qdisc pfifo_fast state UNKNOWN mode DEFAULT group default qlen 1000
> >       link/ether aa:3e:39:5f:c6:15 brd ff:ff:ff:ff:ff:ff
> > 
> > while this doesn't:
> > 
> >   $ ./pasta --config-net sh -c 'sleep 1; ip li sh'
> >   ./pasta: invalid option -- 'c'
> >   [...]
> > 
> > despite the fact that there's no ambiguity.  
> 
> You mean because pasta itself doesn't have a -c option?

Ah, no, I meant it's after 'sh', which is a non-option argument.
However,

> Attempting to
> account for that sounds like a bad idea.  Requiring -- when the
> command given has options of its own that aren't supposed to go to the
> wrapper is pretty common for these sorts of tools.  Basically the
> trade-off is that you either need to require that, or you have to
> require that all non-option arguments of the wrapper come last (old
> style POSIXish command line parsing, as opposed to GNUish
> conventions).  The latter is usually more awkward than the former.

...right, my assumption was exactly that, but it's probably not a good
one. Let's keep it this way then. I wonder, though, if in the man page:

       pasta [OPTION]... [COMMAND [ARG]...]

we should explicitly mention this, because from this synopsis line it
looks like it's enough to put any command, with any argument, at the
end. Or maybe it's already covered by typical GNUish conventions and
users are used to it.

-- 
Stefano

Re: [PATCH 2/8] conf: Use "-D none" and "-S none" instead of missing empty option arguments

[Stefano Brivio]

[-- Attachment #1: Type: text/plain, Size: 5288 bytes --]

On Fri, 26 Aug 2022 14:58:33 +1000
David Gibson <david(a)gibson.dropbear.id.au> wrote:

> Both the -D (--dns) and -S (--search) options take an optional argument.
> If the argument is omitted the option is disabled entirely.  However,
> handling the optional argument requires some ugly special case handling if
> it's the last option on the command line, and has potential ambiguity with
> non-option arguments used with pasta.  It can also make it more confusing
> to read command lines.
> 
> Simplify the logic here by replacing the non-argument versions with an
> explicit "-D none" or "-S none".
> 
> Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au>
> ---
>  conf.c  | 18 ++++--------------
>  passt.1 |  7 ++++---
>  2 files changed, 8 insertions(+), 17 deletions(-)
> 
> diff --git a/conf.c b/conf.c
> index 4eb9e3d..6770be9 100644
> --- a/conf.c
> +++ b/conf.c
> @@ -1022,8 +1022,8 @@ void conf(struct ctx *c, int argc, char **argv)
>  		{"mac-addr",	required_argument,	NULL,		'M' },
>  		{"gateway",	required_argument,	NULL,		'g' },
>  		{"interface",	required_argument,	NULL,		'i' },
> -		{"dns",		optional_argument,	NULL,		'D' },
> -		{"search",	optional_argument,	NULL,		'S' },
> +		{"dns",		required_argument,	NULL,		'D' },
> +		{"search",	required_argument,	NULL,		'S' },
>  		{"no-tcp",	no_argument,		&c->no_tcp,	1 },
>  		{"no-udp",	no_argument,		&c->no_udp,	1 },
>  		{"no-icmp",	no_argument,		&c->no_icmp,	1 },
> @@ -1077,16 +1077,6 @@ void conf(struct ctx *c, int argc, char **argv)
>  
>  		name = getopt_long(argc, argv, optstring, options, NULL);
>  
> -		if ((name == 'D' || name == 'S') && !optarg &&
> -		    optind < argc && *argv[optind] && *argv[optind] != '-') {
> -			if (c->mode == MODE_PASTA) {
> -				if (conf_ns_opt(c, nsdir, userns, argv[optind]))
> -					optarg = argv[optind++];
> -			} else {
> -				optarg = argv[optind++];
> -			}
> -		}
> -
>  		switch (name) {
>  		case -1:
>  		case 0:
> @@ -1403,7 +1393,7 @@ void conf(struct ctx *c, int argc, char **argv)
>  				usage(argv[0]);
>  			}
>  
> -			if (!optarg) {
> +			if (!strcmp(optarg, "none")) {
>  				c->no_dns = 1;
>  				break;
>  			}
> @@ -1430,7 +1420,7 @@ void conf(struct ctx *c, int argc, char **argv)
>  				usage(argv[0]);
>  			}
>  
> -			if (!optarg) {
> +			if (!strcmp(optarg, "none")) {
>  				c->no_dns_search = 1;
>  				break;
>  			}

For these two hunks, clang-tidy reported something I missed in my
review:

/home/sbrivio/passt/conf.c:1417:9: error: Null pointer passed to 1st parameter expecting 'nonnull' [clang-analyzer-core.NonNullParamChecker,-warnings-as-errors]
                        if (!strcmp(optarg, "none")) {
                             ^      ~~~~~~
[...]

/home/sbrivio/passt/conf.c:1411:8: note: Assuming field 'no_dns' is 0
                        if (c->no_dns ||
                            ^~~~~~~~~
/home/sbrivio/passt/conf.c:1411:8: note: Left side of '||' is false
/home/sbrivio/passt/conf.c:1412:9: note: Assuming 'optarg' is null
                            (!optarg && (dns4 - c->ip4.dns || dns6 - c->ip6.dns))) {
                             ^~~~~~~
/home/sbrivio/passt/conf.c:1412:9: note: Left side of '&&' is true
/home/sbrivio/passt/conf.c:1412:21: note: Left side of '||' is false
                            (!optarg && (dns4 - c->ip4.dns || dns6 - c->ip6.dns))) {

...the validation logic needs to be updated here. I'm taking the
liberty of fixing this up on merge, with this diff on top:

diff --git a/conf.c b/conf.c
index 162c2dd..e6d1c62 100644
--- a/conf.c
+++ b/conf.c
@@ -1408,17 +1408,26 @@ void conf(struct ctx *c, int argc, char **argv)
 			}
 			break;
 		case 'D':
-			if (c->no_dns ||
-			    (!optarg && (dns4 - c->ip4.dns || dns6 - c->ip6.dns))) {
-				err("Empty and non-empty DNS options given");
-				usage(argv[0]);
-			}
-
 			if (!strcmp(optarg, "none")) {
+				if (c->no_dns) {
+					err("Redundant DNS options");
+					usage(argv[0]);
+				}
+
+				if (dns4 - c->ip4.dns || dns6 - c->ip6.dns) {
+					err("Conflicting DNS options");
+					usage(argv[0]);
+				}
+
 				c->no_dns = 1;
 				break;
 			}
 
+			if (c->no_dns) {
+				err("Conflicting DNS options");
+				usage(argv[0]);
+			}
+
 			if (dns4 - &c->ip4.dns[0] < ARRAY_SIZE(c->ip4.dns) &&
 			    inet_pton(AF_INET, optarg, dns4)) {
 				dns4++;
@@ -1435,17 +1444,26 @@ void conf(struct ctx *c, int argc, char **argv)
 			usage(argv[0]);
 			break;
 		case 'S':
-			if (c->no_dns_search ||
-			    (!optarg && dnss != c->dns_search)) {
-				err("Empty and non-empty DNS search given");
-				usage(argv[0]);
-			}
-
 			if (!strcmp(optarg, "none")) {
+				if (c->no_dns_search) {
+					err("Redundant DNS search options");
+					usage(argv[0]);
+				}
+
+				if (dnss != c->dns_search) {
+					err("Conflicting DNS search options");
+					usage(argv[0]);
+				}
+
 				c->no_dns_search = 1;
 				break;
 			}
 
+			if (c->no_dns_search) {
+				err("Conflicting DNS search options");
+				usage(argv[0]);
+			}
+
 			if (dnss - c->dns_search < ARRAY_SIZE(c->dns_search)) {
 				ret = snprintf(dnss->n, sizeof(*c->dns_search),
 					       "%s", optarg);


-- 
@@ -1408,17 +1408,26 @@ void conf(struct ctx *c, int argc, char **argv)
 			}
 			break;
 		case 'D':
-			if (c->no_dns ||
-			    (!optarg && (dns4 - c->ip4.dns || dns6 - c->ip6.dns))) {
-				err("Empty and non-empty DNS options given");
-				usage(argv[0]);
-			}
-
 			if (!strcmp(optarg, "none")) {
+				if (c->no_dns) {
+					err("Redundant DNS options");
+					usage(argv[0]);
+				}
+
+				if (dns4 - c->ip4.dns || dns6 - c->ip6.dns) {
+					err("Conflicting DNS options");
+					usage(argv[0]);
+				}
+
 				c->no_dns = 1;
 				break;
 			}
 
+			if (c->no_dns) {
+				err("Conflicting DNS options");
+				usage(argv[0]);
+			}
+
 			if (dns4 - &c->ip4.dns[0] < ARRAY_SIZE(c->ip4.dns) &&
 			    inet_pton(AF_INET, optarg, dns4)) {
 				dns4++;
@@ -1435,17 +1444,26 @@ void conf(struct ctx *c, int argc, char **argv)
 			usage(argv[0]);
 			break;
 		case 'S':
-			if (c->no_dns_search ||
-			    (!optarg && dnss != c->dns_search)) {
-				err("Empty and non-empty DNS search given");
-				usage(argv[0]);
-			}
-
 			if (!strcmp(optarg, "none")) {
+				if (c->no_dns_search) {
+					err("Redundant DNS search options");
+					usage(argv[0]);
+				}
+
+				if (dnss != c->dns_search) {
+					err("Conflicting DNS search options");
+					usage(argv[0]);
+				}
+
 				c->no_dns_search = 1;
 				break;
 			}
 
+			if (c->no_dns_search) {
+				err("Conflicting DNS search options");
+				usage(argv[0]);
+			}
+
 			if (dnss - c->dns_search < ARRAY_SIZE(c->dns_search)) {
 				ret = snprintf(dnss->n, sizeof(*c->dns_search),
 					       "%s", optarg);


-- 
Stefano

Re: [PATCH 8/8] Allow pasta to take a command to execute

[Stefano Brivio]

[-- Attachment #1: Type: text/plain, Size: 4969 bytes --]

On Tue, 30 Aug 2022 10:26:02 +0200
Stefano Brivio <sbrivio(a)redhat.com> wrote:

> On Tue, 30 Aug 2022 11:16:05 +1000
> David Gibson <david(a)gibson.dropbear.id.au> wrote:
> 
> > On Mon, Aug 29, 2022 at 09:16:58PM +0200, Stefano Brivio wrote:  
> > > On Fri, 26 Aug 2022 14:58:39 +1000
> > > David Gibson <david(a)gibson.dropbear.id.au> wrote:
> > >     
> > > > When not given an existing PID or network namspace to attach to, pasta
> > > > spawns a shell.  Most commands which can spawn a shell in an altered
> > > > environment can also run other commands in that same environment, which can
> > > > be useful in automation.
> > > > 
> > > > Allow pasta to do the same thing; it can be given an arbitrary command to
> > > > run in the network and user namespace which pasta creates.  If neither a
> > > > command nor an existing PID or netns to attach to is given, continue to
> > > > spawn a default shell, as before.
> > > > 
> > > > Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au>
> > > > ---
> > > >  conf.c  | 27 ++++++++++++++++++---------
> > > >  passt.1 | 14 +++++++++-----
> > > >  pasta.c | 33 +++++++++++++++++++++++----------
> > > >  pasta.h |  2 +-
> > > >  4 files changed, 51 insertions(+), 25 deletions(-)
> > > > 
> > > > diff --git a/conf.c b/conf.c
> > > > index 2a18124..162c2dd 100644
> > > > --- a/conf.c
> > > > +++ b/conf.c
> > > > @@ -550,7 +550,8 @@ static int conf_ns_pid(char *userns, char *netns, const char *arg)
> > > >  		return 0;
> > > >  	}
> > > >  
> > > > -	return -EINVAL;
> > > > +	/* Not a PID, later code will treat as a command */
> > > > +	return 0;
> > > >  }
> > > >  
> > > >  /**
> > > > @@ -1480,14 +1481,18 @@ void conf(struct ctx *c, int argc, char **argv)
> > > >  
> > > >  	check_root(c);
> > > >  
> > > > -	if (c->mode == MODE_PASTA && optind + 1 == argc) {
> > > > -		ret = conf_ns_pid(userns, netns, argv[optind]);
> > > > -		if (ret < 0)
> > > > +	if (c->mode == MODE_PASTA) {
> > > > +		if (*netns && optind != argc) {
> > > > +			err("Both --netns and PID or command given");
> > > >  			usage(argv[0]);
> > > > -	} else if (c->mode == MODE_PASTA && *userns
> > > > -		   && !*netns && optind == argc) {
> > > > -		err("--userns requires --netns or PID");
> > > > -		usage(argv[0]);
> > > > +		} else if (optind + 1 == argc) {
> > > > +			ret = conf_ns_pid(userns, netns, argv[optind]);
> > > > +			if (ret < 0)
> > > > +				usage(argv[0]);
> > > > +		} else if (*userns && !*netns && optind == argc) {
> > > > +			err("--userns requires --netns or PID");
> > > > +			usage(argv[0]);
> > > > +		}
> > > >  	} else if (optind != argc) {
> > > >  		usage(argv[0]);
> > > >  	}
> > > >
> > > > [...]    
> > > 
> > > I haven't really looked into this yet, but I guess we should now
> > > handle getopts return codes a bit differently, because this works:
> > > 
> > >   $ ./pasta --config-net -- sh -c 'sleep 1; ip li sh'
> > >   1: lo: <LOOPBACK,UP,LOWER_UP> mtu 65536 qdisc noqueue state UNKNOWN mode DEFAULT group default qlen 1000
> > >       link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
> > >   2: enp9s0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 65520 qdisc pfifo_fast state UNKNOWN mode DEFAULT group default qlen 1000
> > >       link/ether aa:3e:39:5f:c6:15 brd ff:ff:ff:ff:ff:ff
> > > 
> > > while this doesn't:
> > > 
> > >   $ ./pasta --config-net sh -c 'sleep 1; ip li sh'
> > >   ./pasta: invalid option -- 'c'
> > >   [...]
> > > 
> > > despite the fact that there's no ambiguity.    
> > 
> > You mean because pasta itself doesn't have a -c option?  
> 
> Ah, no, I meant it's after 'sh', which is a non-option argument.
> However,
> 
> > Attempting to
> > account for that sounds like a bad idea.  Requiring -- when the
> > command given has options of its own that aren't supposed to go to the
> > wrapper is pretty common for these sorts of tools.  Basically the
> > trade-off is that you either need to require that, or you have to
> > require that all non-option arguments of the wrapper come last (old
> > style POSIXish command line parsing, as opposed to GNUish
> > conventions).  The latter is usually more awkward than the former.  
> 
> ...right, my assumption was exactly that, but it's probably not a good
> one. Let's keep it this way then. I wonder, though, if in the man page:
> 
>        pasta [OPTION]... [COMMAND [ARG]...]
> 
> we should explicitly mention this, because from this synopsis line it
> looks like it's enough to put any command, with any argument, at the
> end. Or maybe it's already covered by typical GNUish conventions and
> users are used to it.

...I couldn't find any example of an explicit mention of "--" in man
pages, so I'm not sure what's the convention for this, if any.

I'd leave the man page as you patched it, if somebody has a better idea
or stumbles upon this, we can improve it later.

-- 
Stefano

Re: [PATCH 0/8] Allow pasta to take a command to spawn instead of shell

[Stefano Brivio]

[-- Attachment #1: Type: text/plain, Size: 1090 bytes --]

On Fri, 26 Aug 2022 14:58:31 +1000
David Gibson <david(a)gibson.dropbear.id.au> wrote:

> When not attaching to an existing network namespace, pasta always
> spawns an interactive shell in a new namespace to attach to.  Most
> commands which can issue a shell in a modified environment can also
> issue other commands as well (e.g. env, strace).  We want to allow
> pasta to do the same.
> 
> Because of the way the non-option argument to pasta is currently
> overloaded, allowing this requires some other changes to the way we
> parse the command line.
> 
> David Gibson (8):
>   conf: Make the argument to --pcap option mandatory
>   conf: Use "-D none" and "-S none" instead of missing empty option
>     arguments
>   Correct manpage for --userns
>   Remove --nsrun-dir option
>   Move ENOENT error message into conf_ns_opt()
>   More deterministic detection of whether argument is a PID, PATH or
>     NAME
>   Use explicit --netns option rather than multiplexing with PID
>   Allow pasta to take a command to execute

Applied now with those small changes, thanks a lot.

-- 
Stefano