c->uid and c->gid are first set in conf(), and last used in check_root()
itself called from conf().  Therefore these don't need to be fields in the
long lived context structure and can instead be locals in conf().

Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au>
---
 conf.c  |  8 +++++---
 passt.h |  5 -----
 util.c  | 12 ++++++------
 util.h  |  2 +-
 4 files changed, 12 insertions(+), 15 deletions(-)

diff --git a/conf.c b/conf.c
index 2edb4ae..0fe5266 100644
--- a/conf.c
+++ b/conf.c
@@ -1086,6 +1086,8 @@ void conf(struct ctx *c, int argc, char **argv)
 	uint32_t *dns4 = c->ip4.dns;
 	int name, ret, mask, b, i;
 	unsigned int ifi = 0;
+	uid_t uid = 0;
+	gid_t gid = 0;
 
 	if (c->mode == MODE_PASTA)
 		c->no_dhcp_dns = c->no_dhcp_dns_search = 1;
@@ -1208,12 +1210,12 @@ void conf(struct ctx *c, int argc, char **argv)
 			c->trace = c->debug = c->foreground = 1;
 			break;
 		case 12:
-			if (c->uid || c->gid) {
+			if (uid || gid) {
 				err("Multiple --runas options given");
 				usage(argv[0]);
 			}
 
-			if (conf_runas(optarg, &c->uid, &c->gid)) {
+			if (conf_runas(optarg, &uid, &gid)) {
 				err("Invalid --runas option: %s", optarg);
 				usage(argv[0]);
 			}
@@ -1497,7 +1499,7 @@ void conf(struct ctx *c, int argc, char **argv)
 		}
 	} while (name != -1);
 
-	check_root(c);
+	check_root(&uid, &gid);
 
 	if (c->mode == MODE_PASTA) {
 		if (*netns && optind != argc) {
diff --git a/passt.h b/passt.h
index 347e7c1..3035430 100644
--- a/passt.h
+++ b/passt.h
@@ -144,8 +144,6 @@ struct ip6_ctx {
  * @sock_path:		Path for UNIX domain socket
  * @pcap:		Path for packet capture file
  * @pid_file:		Path to PID file, empty string if not configured
- * @uid:		UID we should drop to, if started as root
- * @gid:		GID we should drop to, if started as root
  * @pasta_netns_fd:	File descriptor for network namespace in pasta mode
  * @pasta_userns_fd:	Descriptor for user namespace to join, -1 once joined
  * @netns_only:		In pasta mode, don't join or create a user namespace
@@ -198,9 +196,6 @@ struct ctx {
 	char pcap[PATH_MAX];
 	char pid_file[PATH_MAX];
 
-	uid_t uid;
-	uid_t gid;
-
 	int pasta_netns_fd;
 	int pasta_userns_fd;
 	int netns_only;
diff --git a/util.c b/util.c
index 7e10deb..b2ccb3d 100644
--- a/util.c
+++ b/util.c
@@ -485,7 +485,7 @@ void drop_caps(void)
 /**
  * check_root() - Check if root in init ns, exit if we can't drop to user
  */
-void check_root(struct ctx *c)
+void check_root(uid_t *uid, gid_t *gid)
 {
 	const char root_uid_map[] = "         0          0 4294967295";
 	struct passwd *pw;
@@ -506,7 +506,7 @@ void check_root(struct ctx *c)
 
 	close(fd);
 
-	if (!c->uid) {
+	if (!*uid) {
 		fprintf(stderr, "Don't run as root. Changing to nobody...\n");
 #ifndef GLIBC_NO_STATIC_NSS
 		pw = getpwnam("nobody");
@@ -515,17 +515,17 @@ void check_root(struct ctx *c)
 			exit(EXIT_FAILURE);
 		}
 
-		c->uid = pw->pw_uid;
-		c->gid = pw->pw_gid;
+		*uid = pw->pw_uid;
+		*gid = pw->pw_gid;
 #else
 		(void)pw;
 
 		/* Common value for 'nobody', not really specified */
-		c->uid = c->gid = 65534;
+		*uid = *gid = 65534;
 #endif
 	}
 
-	if (!setgroups(0, NULL) && !setgid(c->gid) && !setuid(c->uid))
+	if (!setgroups(0, NULL) && !setgid(*gid) && !setuid(*uid))
 		return;
 
 	fprintf(stderr, "Can't change user/group, exiting");
diff --git a/util.h b/util.h
index 8297bec..58312fb 100644
--- a/util.h
+++ b/util.h
@@ -234,7 +234,7 @@ char *line_read(char *buf, size_t len, int fd);
 void procfs_scan_listen(struct ctx *c, uint8_t proto, int ip_version, int ns,
 			uint8_t *map, uint8_t *exclude);
 void drop_caps(void);
-void check_root(struct ctx *c);
+void check_root(uid_t *uid, gid_t *gid);
 int ns_enter(const struct ctx *c);
 void write_pidfile(int fd, pid_t pid);
 int __daemon(int pidfile_fd, int devnull_fd);
-- 
2.37.3

Currently the logic to work out what UID and GID we will run as is spread
across conf().  If --runas is specified it's handled in conf_runas(),
otherwise it's handled by check_root(), which depends on initialization of
the uid and gid variables by either conf() itself or conf_runas().

Make this clearer by putting all the UID and GID logic into a single
conf_ugid() function.

Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au>
---
 conf.c | 81 ++++++++++++++++++++++++++++++++++++++++++++++++++++------
 util.c | 50 ------------------------------------
 util.h |  1 -
 3 files changed, 73 insertions(+), 59 deletions(-)

diff --git a/conf.c b/conf.c
index 545f61d..5c293b5 100644
--- a/conf.c
+++ b/conf.c
@@ -1021,6 +1021,70 @@ static int conf_runas(const char *opt, unsigned int *uid, unsigned
int *gid)
 #endif /* !GLIBC_NO_STATIC_NSS */
 }
 
+/**
+ * conf_ugid() - Determine UID and GID to run as
+ * @runas:	--runas option, may be NULL
+ * @uid:	User ID, set on success
+ * @gid:	Group ID, set on success
+ *
+ * Return: 0 on success, negative error code on failure
+ */
+static int conf_ugid(const char *runas, uid_t *uid, gid_t *gid)
+{
+	const char root_uid_map[] = "         0          0 4294967295";
+	struct passwd *pw;
+	char buf[BUFSIZ];
+	int ret;
+	int fd;
+
+	/* If user has specified --runas, that takes precedence */
+	if (runas) {
+		ret = conf_runas(runas, uid, gid);
+		if (ret)
+			err("Invalid --runas option: %s", runas);
+		return ret;
+	}
+
+	/* Otherwise default to current user and group.. */
+	*uid = geteuid();
+	*gid = getegid();
+
+	/* ..as long as it's not root.. */
+	if (*uid)
+		return 0;
+
+	/* ..or at least not root in the init namespace.. */
+	if ((fd = open("/proc/self/uid_map", O_RDONLY | O_CLOEXEC)) < 0)
+		return 0;
+
+	if (read(fd, buf, BUFSIZ) != sizeof(root_uid_map) ||
+	    strncmp(buf, root_uid_map, sizeof(root_uid_map) - 1)) {
+		close(fd);
+		return 0;
+	}
+
+	close(fd);
+
+	/* ..otherwise use nobody:nobody */
+	warn("Don't run as root. Changing to nobody...");
+#ifndef GLIBC_NO_STATIC_NSS
+	pw = getpwnam("nobody");
+	if (!pw) {
+		perror("getpwnam");
+		exit(EXIT_FAILURE);
+	}
+
+	*uid = pw->pw_uid;
+	*gid = pw->pw_gid;
+#else
+	(void)pw;
+
+	/* Common value for 'nobody', not really specified */
+	*uid = *gid = 65534;
+#endif
+	return 0;
+}
+
 /**
  * conf() - Process command-line arguments and set configuration
  * @c:		Execution context
@@ -1085,9 +1149,10 @@ void conf(struct ctx *c, int argc, char **argv)
 	struct fqdn *dnss = c->dns_search;
 	uint32_t *dns4 = c->ip4.dns;
 	int name, ret, mask, b, i;
+	const char *runas = NULL;
 	unsigned int ifi = 0;
-	uid_t uid = 0;
-	gid_t gid = 0;
+	uid_t uid;
+	gid_t gid;
 
 	if (c->mode == MODE_PASTA)
 		c->no_dhcp_dns = c->no_dhcp_dns_search = 1;
@@ -1210,15 +1275,12 @@ void conf(struct ctx *c, int argc, char **argv)
 			c->trace = c->debug = c->foreground = 1;
 			break;
 		case 12:
-			if (uid || gid) {
+			if (runas) {
 				err("Multiple --runas options given");
 				usage(argv[0]);
 			}
 
-			if (conf_runas(optarg, &uid, &gid)) {
-				err("Invalid --runas option: %s", optarg);
-				usage(argv[0]);
-			}
+			runas = optarg;
 			break;
 		case 'd':
 			if (c->debug) {
@@ -1499,7 +1561,10 @@ void conf(struct ctx *c, int argc, char **argv)
 		}
 	} while (name != -1);
 
-	check_root(&uid, &gid);
+	ret = conf_ugid(runas, &uid, &gid);
+	if (ret)
+		usage(argv[0]);
+
 	drop_root(uid, gid);
 
 	if (c->mode == MODE_PASTA) {
diff --git a/util.c b/util.c
index 17595c1..654410f 100644
--- a/util.c
+++ b/util.c
@@ -482,56 +482,6 @@ void drop_caps(void)
 	}
 }
 
-/**
- * check_root() - Check if root in init ns, exit if we can't drop to user
- */
-void check_root(uid_t *uid, gid_t *gid)
-{
-	const char root_uid_map[] = "         0          0 4294967295";
-	struct passwd *pw;
-	char buf[BUFSIZ];
-	int fd;
-
-	if (!*uid)
-		*uid = geteuid();
-
-	if (!*gid)
-		*gid = getegid();
-
-	if (*uid)
-		return;
-
-	if ((fd = open("/proc/self/uid_map", O_RDONLY | O_CLOEXEC)) < 0)
-		return;
-
-	if (read(fd, buf, BUFSIZ) != sizeof(root_uid_map) ||
-	    strncmp(buf, root_uid_map, sizeof(root_uid_map) - 1)) {
-		close(fd);
-		return;
-	}
-
-	close(fd);
-
-	if (!*uid) {
-		fprintf(stderr, "Don't run as root. Changing to nobody...\n");
-#ifndef GLIBC_NO_STATIC_NSS
-		pw = getpwnam("nobody");
-		if (!pw) {
-			perror("getpwnam");
-			exit(EXIT_FAILURE);
-		}
-
-		*uid = pw->pw_uid;
-		*gid = pw->pw_gid;
-#else
-		(void)pw;
-
-		/* Common value for 'nobody', not really specified */
-		*uid = *gid = 65534;
-#endif
-	}
-}
-
 /**
  * drop_root() - Switch to given UID and GID
  */
diff --git a/util.h b/util.h
index e2f686b..9626cb5 100644
--- a/util.h
+++ b/util.h
@@ -234,7 +234,6 @@ char *line_read(char *buf, size_t len, int fd);
 void procfs_scan_listen(struct ctx *c, uint8_t proto, int ip_version, int ns,
 			uint8_t *map, uint8_t *exclude);
 void drop_caps(void);
-void check_root(uid_t *uid, gid_t *gid);
 void drop_root(uid_t uid, gid_t gid);
 int ns_enter(const struct ctx *c);
 void write_pidfile(int fd, pid_t pid);
-- 
2.37.3

passt/pasta contains a number of routines designed to isolate passt from
the rest of the system for security.  These are spread through util.c and
passt.c.  Move them together into a new isolation.c file.

Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au>
---
 Makefile    |   8 +--
 conf.c      |   1 +
 isolation.c | 165 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 isolation.h |  15 +++++
 passt.c     | 113 +----------------------------------
 pasta.c     |   1 +
 util.c      |  49 ----------------
 util.h      |   2 -
 8 files changed, 187 insertions(+), 167 deletions(-)
 create mode 100644 isolation.c
 create mode 100644 isolation.h

diff --git a/Makefile b/Makefile
index 644a541..af3d1ff 100644
--- a/Makefile
+++ b/Makefile
@@ -32,16 +32,16 @@ CFLAGS += -DRLIMIT_STACK_VAL=$(RLIMIT_STACK_VAL)
 CFLAGS += -DARCH=\"$(TARGET_ARCH)\"
 
 PASST_SRCS = arch.c arp.c checksum.c conf.c dhcp.c dhcpv6.c icmp.c igmp.c \
-	lineread.c mld.c ndp.c netlink.c packet.c passt.c pasta.c pcap.c \
-	siphash.c tap.c tcp.c tcp_splice.c udp.c util.c
+	isolation.c lineread.c mld.c ndp.c netlink.c packet.c passt.c pasta.c \
+	pcap.c siphash.c tap.c tcp.c tcp_splice.c udp.c util.c
 QRAP_SRCS = qrap.c
 SRCS = $(PASST_SRCS) $(QRAP_SRCS)
 
 MANPAGES = passt.1 pasta.1 qrap.1
 
 PASST_HEADERS = arch.h arp.h checksum.h conf.h dhcp.h dhcpv6.h icmp.h \
-	lineread.h ndp.h netlink.h packet.h passt.h pasta.h pcap.h \
-	siphash.h tap.h tcp.h tcp_splice.h udp.h util.h
+	isolation.h lineread.h ndp.h netlink.h packet.h passt.h pasta.h \
+	pcap.h siphash.h tap.h tcp.h tcp_splice.h udp.h util.h
 HEADERS = $(PASST_HEADERS)
 
 # On gcc 11.2, with -O2 and -flto, tcp_hash() and siphash_20b(), if inlined,
diff --git a/conf.c b/conf.c
index f1aaa8a..08a2106 100644
--- a/conf.c
+++ b/conf.c
@@ -40,6 +40,7 @@
 #include "tcp.h"
 #include "pasta.h"
 #include "lineread.h"
+#include "isolation.h"
 
 /**
  * get_bound_ports() - Get maps of ports with bound sockets
diff --git a/isolation.c b/isolation.c
new file mode 100644
index 0000000..bc8240f
--- /dev/null
+++ b/isolation.c
@@ -0,0 +1,165 @@
+// SPDX-License-Identifier: AGPL-3.0-or-later
+
+/* PASST - Plug A Simple Socket Transport
+ *  for qemu/UNIX domain socket mode
+ *
+ * PASTA - Pack A Subtle Tap Abstraction
+ *  for network namespace/tap device mode
+ *
+ * isolation.c - Self isolation helpers
+ *
+ * Copyright Red Hat
+ * Author: Stefano Brivio <sbrivio(a)redhat.com>
+ * Author: David Gibson <david(a)gibson.dropbear.id.au>
+ */
+
+#include <errno.h>
+#include <fcntl.h>
+#include <grp.h>
+#include <inttypes.h>
+#include <limits.h>
+#include <pwd.h>
+#include <sched.h>
+#include <stddef.h>
+#include <stdlib.h>
+#include <string.h>
+#include <time.h>
+#include <unistd.h>
+#include <sys/mount.h>
+#include <sys/prctl.h>
+#include <sys/socket.h>
+#include <sys/syscall.h>
+#include <sys/types.h>
+#include <netinet/in.h>
+#include <netinet/if_ether.h>
+
+#include <linux/audit.h>
+#include <linux/capability.h>
+#include <linux/filter.h>
+#include <linux/seccomp.h>
+
+#include "util.h"
+#include "seccomp.h"
+#include "passt.h"
+#include "isolation.h"
+
+/**
+ * drop_caps() - Drop capabilities we might have except for CAP_NET_BIND_SERVICE
+ */
+void drop_caps(void)
+{
+	int i;
+
+	for (i = 0; i < 64; i++) {
+		if (i == CAP_NET_BIND_SERVICE)
+			continue;
+
+		prctl(PR_CAPBSET_DROP, i, 0, 0, 0);
+	}
+}
+
+/**
+ * drop_root() - Switch to given UID and GID
+ */
+void drop_root(uid_t uid, gid_t gid)
+{
+	if (setgroups(0, NULL)) {
+		/* If we don't start with CAP_SETGID, this will EPERM */
+		if (errno != EPERM) {
+			err("Can't drop supplementary groups: %s",
+			    strerror(errno));
+			exit(EXIT_FAILURE);
+		}
+	}
+
+	if (!setgid(gid) && !setuid(uid))
+		return;
+
+	err("Can't change user/group, exiting");
+	exit(EXIT_FAILURE);
+}
+
+/**
+ * sandbox() - Unshare IPC, mount, PID, UTS, and user namespaces, "unmount"
root
+ *
+ * Return: negative error code on failure, zero on success
+ */
+int sandbox(struct ctx *c)
+{
+	int flags = CLONE_NEWIPC | CLONE_NEWNS | CLONE_NEWUTS;
+
+	if (!c->netns_only) {
+		if (c->pasta_userns_fd == -1)
+			flags |= CLONE_NEWUSER;
+		else
+			setns(c->pasta_userns_fd, CLONE_NEWUSER);
+	}
+
+	c->pasta_userns_fd = -1;
+
+	/* If we run in foreground, we have no chance to actually move to a new
+	 * PID namespace. For passt, use CLONE_NEWPID anyway, in case somebody
+	 * ever gets around seccomp profiles -- there's no harm in passing it.
+	 */
+	if (!c->foreground || c->mode == MODE_PASST)
+		flags |= CLONE_NEWPID;
+
+	if (unshare(flags)) {
+		perror("unshare");
+		return -errno;
+	}
+
+	if (mount("", "/", "", MS_UNBINDABLE | MS_REC, NULL)) {
+		perror("mount /");
+		return -errno;
+	}
+
+	if (mount("", TMPDIR, "tmpfs",
+		  MS_NODEV | MS_NOEXEC | MS_NOSUID | MS_RDONLY,
+		  "nr_inodes=2,nr_blocks=0")) {
+		perror("mount tmpfs");
+		return -errno;
+	}
+
+	if (chdir(TMPDIR)) {
+		perror("chdir");
+		return -errno;
+	}
+
+	if (syscall(SYS_pivot_root, ".", ".")) {
+		perror("pivot_root");
+		return -errno;
+	}
+
+	if (umount2(".", MNT_DETACH | UMOUNT_NOFOLLOW)) {
+		perror("umount2");
+		return -errno;
+	}
+
+	drop_caps();	/* Relative to the new user namespace this time. */
+
+	return 0;
+}
+
+/**
+ * seccomp() - Set up seccomp filters depending on mode, won't return on failure
+ * @c:		Execution context
+ */
+void seccomp(const struct ctx *c)
+{
+	struct sock_fprog prog;
+
+	if (c->mode == MODE_PASST) {
+		prog.len = (unsigned short)ARRAY_SIZE(filter_passt);
+		prog.filter = filter_passt;
+	} else {
+		prog.len = (unsigned short)ARRAY_SIZE(filter_pasta);
+		prog.filter = filter_pasta;
+	}
+
+	if (prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0) ||
+	    prctl(PR_SET_SECCOMP, SECCOMP_MODE_FILTER, &prog)) {
+		perror("prctl");
+		exit(EXIT_FAILURE);
+	}
+}
diff --git a/isolation.h b/isolation.h
new file mode 100644
index 0000000..2540a35
--- /dev/null
+++ b/isolation.h
@@ -0,0 +1,15 @@
+/* SPDX-License-Identifier: AGPL-3.0-or-later
+ * Copyright Red Hat
+ * Author: Stefano Brivio &lt;sbrivio(a)redhat.com&gt;
+ * Author: David Gibson &lt;david(a)gibson.dropbear.id.au&gt;
+ */
+
+#ifndef ISOLATION_H
+#define ISOLATION_H
+
+void drop_caps(void);
+void drop_root(uid_t uid, gid_t gid);
+int sandbox(struct ctx *c);
+void seccomp(const struct ctx *c);
+
+#endif /* ISOLATION_H */
diff --git a/passt.c b/passt.c
index bbf53d9..2a8314c 100644
--- a/passt.c
+++ b/passt.c
@@ -19,51 +19,25 @@
  * created in a separate network namespace).
  */
 
-#include <sched.h>
-#include <stdio.h>
 #include <sys/epoll.h>
-#include <sys/socket.h>
-#include <sys/types.h>
-#include <sys/stat.h>
-#include <dirent.h>
 #include <fcntl.h>
 #include <sys/mman.h>
 #include <sys/resource.h>
-#include <sys/uio.h>
-#include <sys/syscall.h>
-#include <sys/wait.h>
-#include <sys/mount.h>
-#include <netinet/ip.h>
-#include <net/ethernet.h>
-#include <libgen.h>
 #include <stdlib.h>
 #include <unistd.h>
-#include <net/if.h>
 #include <netdb.h>
 #include <string.h>
 #include <errno.h>
 #include <time.h>
 #include <syslog.h>
-#include <sys/stat.h>
 #include <sys/prctl.h>
-#include <stddef.h>
-#include <netinet/udp.h>
-#include <netinet/tcp.h>
 #include <netinet/if_ether.h>
 
-#include <linux/seccomp.h>
-#include <linux/audit.h>
-#include <linux/filter.h>
-#include <linux/icmpv6.h>
-
 #include "util.h"
-#include "seccomp.h"
 #include "passt.h"
 #include "dhcp.h"
 #include "dhcpv6.h"
-#include "icmp.h"
-#include "tcp.h"
-#include "udp.h"
+#include "isolation.h"
 #include "pcap.h"
 #include "tap.h"
 #include "conf.h"
@@ -166,91 +140,6 @@ void proto_update_l2_buf(const unsigned char *eth_d, const unsigned
char *eth_s,
 	udp_update_l2_buf(eth_d, eth_s, ip_da);
 }
 
-/**
- * seccomp() - Set up seccomp filters depending on mode, won't return on failure
- * @c:		Execution context
- */
-static void seccomp(const struct ctx *c)
-{
-	struct sock_fprog prog;
-
-	if (c->mode == MODE_PASST) {
-		prog.len = (unsigned short)ARRAY_SIZE(filter_passt);
-		prog.filter = filter_passt;
-	} else {
-		prog.len = (unsigned short)ARRAY_SIZE(filter_pasta);
-		prog.filter = filter_pasta;
-	}
-
-	if (prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0) ||
-	    prctl(PR_SET_SECCOMP, SECCOMP_MODE_FILTER, &prog)) {
-		perror("prctl");
-		exit(EXIT_FAILURE);
-	}
-}
-
-/**
- * sandbox() - Unshare IPC, mount, PID, UTS, and user namespaces, "unmount"
root
- *
- * Return: negative error code on failure, zero on success
- */
-static int sandbox(struct ctx *c)
-{
-	int flags = CLONE_NEWIPC | CLONE_NEWNS | CLONE_NEWUTS;
-
-	if (!c->netns_only) {
-		if (c->pasta_userns_fd == -1)
-			flags |= CLONE_NEWUSER;
-		else
-			setns(c->pasta_userns_fd, CLONE_NEWUSER);
-	}
-
-	c->pasta_userns_fd = -1;
-
-	/* If we run in foreground, we have no chance to actually move to a new
-	 * PID namespace. For passt, use CLONE_NEWPID anyway, in case somebody
-	 * ever gets around seccomp profiles -- there's no harm in passing it.
-	 */
-	if (!c->foreground || c->mode == MODE_PASST)
-		flags |= CLONE_NEWPID;
-
-	if (unshare(flags)) {
-		perror("unshare");
-		return -errno;
-	}
-
-	if (mount("", "/", "", MS_UNBINDABLE | MS_REC, NULL)) {
-		perror("mount /");
-		return -errno;
-	}
-
-	if (mount("", TMPDIR, "tmpfs",
-		  MS_NODEV | MS_NOEXEC | MS_NOSUID | MS_RDONLY,
-		  "nr_inodes=2,nr_blocks=0")) {
-		perror("mount tmpfs");
-		return -errno;
-	}
-
-	if (chdir(TMPDIR)) {
-		perror("chdir");
-		return -errno;
-	}
-
-	if (syscall(SYS_pivot_root, ".", ".")) {
-		perror("pivot_root");
-		return -errno;
-	}
-
-	if (umount2(".", MNT_DETACH | UMOUNT_NOFOLLOW)) {
-		perror("umount2");
-		return -errno;
-	}
-
-	drop_caps();	/* Relative to the new user namespace this time. */
-
-	return 0;
-}
-
 /**
  * exit_handler() - Signal handler for SIGQUIT and SIGTERM
  * @unused:	Unused, handler deals with SIGQUIT and SIGTERM only
diff --git a/pasta.c b/pasta.c
index a844af2..0bdb655 100644
--- a/pasta.c
+++ b/pasta.c
@@ -40,6 +40,7 @@
 
 #include "util.h"
 #include "passt.h"
+#include "isolation.h"
 #include "netlink.h"
 
 /* PID of child, in case we created a namespace */
diff --git a/util.c b/util.c
index 654410f..f709838 100644
--- a/util.c
+++ b/util.c
@@ -13,30 +13,17 @@
  */
 
 #include <sched.h>
-#include <stdio.h>
-#include <stdint.h>
-#include <stddef.h>
 #include <stdlib.h>
 #include <unistd.h>
 #include <arpa/inet.h>
 #include <net/ethernet.h>
-#include <net/if.h>
-#include <netinet/tcp.h>
-#include <netinet/udp.h>
 #include <sys/epoll.h>
-#include <sys/prctl.h>
-#include <sys/types.h>
-#include <sys/stat.h>
 #include <fcntl.h>
 #include <syslog.h>
 #include <stdarg.h>
 #include <string.h>
 #include <time.h>
 #include <errno.h>
-#include <pwd.h>
-#include <grp.h>
-
-#include <linux/capability.h>
 
 #include "util.h"
 #include "passt.h"
@@ -467,42 +454,6 @@ void procfs_scan_listen(struct ctx *c, uint8_t proto, int ip_version,
int ns,
 	}
 }
 
-/**
- * drop_caps() - Drop capabilities we might have except for CAP_NET_BIND_SERVICE
- */
-void drop_caps(void)
-{
-	int i;
-
-	for (i = 0; i < 64; i++) {
-		if (i == CAP_NET_BIND_SERVICE)
-			continue;
-
-		prctl(PR_CAPBSET_DROP, i, 0, 0, 0);
-	}
-}
-
-/**
- * drop_root() - Switch to given UID and GID
- */
-void drop_root(uid_t uid, gid_t gid)
-{
-	if (setgroups(0, NULL)) {
-		/* If we don't start with CAP_SETGID, this will EPERM */
-		if (errno != EPERM) {
-			err("Can't drop supplementary groups: %s",
-			    strerror(errno));
-			exit(EXIT_FAILURE);
-		}
-	}
-
-	if (!setgid(gid) && !setuid(uid))
-		return;
-
-	err("Can't change user/group, exiting");
-	exit(EXIT_FAILURE);
-}
-
 /**
  * ns_enter() - Enter configured user (unless already joined) and network ns
  * @c:		Execution context
diff --git a/util.h b/util.h
index 9626cb5..1003303 100644
--- a/util.h
+++ b/util.h
@@ -233,8 +233,6 @@ int bitmap_isset(const uint8_t *map, int bit);
 char *line_read(char *buf, size_t len, int fd);
 void procfs_scan_listen(struct ctx *c, uint8_t proto, int ip_version, int ns,
 			uint8_t *map, uint8_t *exclude);
-void drop_caps(void);
-void drop_root(uid_t uid, gid_t gid);
 int ns_enter(const struct ctx *c);
 void write_pidfile(int fd, pid_t pid);
 int __daemon(int pidfile_fd, int devnull_fd);
-- 
2.37.3

There are a number of different ways to specify namespaces for pasta to
use.  Some combinations are valid and some are not.  Currently validation
for these is spread across several places: conf_ns_pid() validates PID
options specifically.  Near its callsite in conf() several other checks
are made. Some additional checks are made in conf_ns_open() and finally
theres a check just before the call to pasta_start_ns().

This is quite hard to follow.  Make it easier by putting all the validation
logic together in a new conf_pasta_ns() function, which subsumes
conf_ns_pid().  This reveals that some of the checks were redundant with
each other, so remove those.

For good measure, rename conf_netns() to conf_netns_opt() to make it
clearer its handling just the --netns option specifically, not overall
configuration of the netns.

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

diff --git a/conf.c b/conf.c
index 08a2106..5018794 100644
--- a/conf.c
+++ b/conf.c
@@ -491,13 +491,13 @@ out:
 }
 
 /**
- * conf_netns() - Parse --netns option
+ * conf_netns_opt() - 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)
+static int conf_netns_opt(char *netns, const char *arg)
 {
 	int ret;
 
@@ -518,40 +518,59 @@ static int conf_netns(char *netns, const char *arg)
 }
 
 /**
- * conf_ns_pid() - Parse non-option argument as a PID
+ * conf_pasta_ns() - Validate all pasta namespace options
+ * @netns_only:	Don't use userns, may be updated
  * @userns:	buffer of size PATH_MAX, initially contains --userns
  *		argument (may be empty), updated with userns path
  * @netns:	buffer of size PATH_MAX, initial contains --netns
  *		argument (may be empty), updated with netns path
- * @arg:	PID of network namespace
+ * @optind:	Index of first non-option argument
+ * @argc:	Number of arguments
+ * @argv:	Command line arguments
  *
  * Return: 0 on success, negative error code otherwise
  */
-static int conf_ns_pid(char *userns, char *netns, const char *arg)
+static int conf_pasta_ns(int *netns_only, char *userns, char *netns,
+			 int optind, int argc, char *argv[])
 {
-	char *endptr;
-	long pidval;
+	if (*netns_only && *userns) {
+		err("Both --userns and --netns-only given");
+		return -EINVAL;
+	}
 
-	if (*netns) {
-		err("Both --netns and PID given");
+	if (*netns && optind != argc) {
+		err("Both --netns and PID or command given");
 		return -EINVAL;
 	}
 
-	pidval = strtol(arg, &endptr, 10);
-	if (!*endptr) {
-		/* Looks like a pid */
-		if (pidval < 0 || pidval > INT_MAX) {
-			err("Invalid PID %s", arg);
-			return -EINVAL;
+	if (optind + 1 == argc) {
+		char *endptr;
+		long pidval;
+
+		pidval = strtol(argv[optind], &endptr, 10);
+		if (!*endptr) {
+			/* Looks like a pid */
+			if (pidval < 0 || pidval > INT_MAX) {
+				err("Invalid PID %s", argv[optind]);
+				return -EINVAL;
+			}
+
+			snprintf(netns, PATH_MAX, "/proc/%ld/ns/net", pidval);
+			if (!*userns)
+				snprintf(userns, PATH_MAX, "/proc/%ld/ns/user",
+					 pidval);
 		}
+	}
 
-		snprintf(netns, PATH_MAX, "/proc/%ld/ns/net", pidval);
-		if (!*userns)
-			snprintf(userns, PATH_MAX, "/proc/%ld/ns/user", pidval);
-		return 0;
+	if (*userns && !*netns) {
+		err("--userns requires --netns or PID");
+		return -EINVAL;
 	}
 
-	/* Not a PID, later code will treat as a command */
+	/* Attaching to a netns/PID, with no userns given */
+	if (*netns && !*userns)
+		*netns_only = 1;
+
 	return 0;
 }
 
@@ -585,11 +604,6 @@ static int conf_ns_open(struct ctx *c, const char *userns, const char
*netns)
 {
 	int ufd = -1, nfd = -1;
 
-	if (c->netns_only && *userns) {
-		err("Both --userns and --netns-only given");
-		return -EINVAL;
-	}
-
 	nfd = open(netns, O_RDONLY | O_CLOEXEC);
 	if (nfd < 0) {
 		err("Couldn't open network namespace %s", netns);
@@ -607,7 +621,6 @@ static int conf_ns_open(struct ctx *c, const char *userns, const char
*netns)
 
 	c->pasta_netns_fd = nfd;
 	c->pasta_userns_fd = ufd;
-	c->netns_only = !*userns;
 
 	NS_CALL(conf_ns_check, c);
 
@@ -1194,7 +1207,7 @@ void conf(struct ctx *c, int argc, char **argv)
 				usage(argv[0]);
 			}
 
-			ret = conf_netns(netns, optarg);
+			ret = conf_netns_opt(netns, optarg);
 			if (ret < 0)
 				usage(argv[0]);
 			break;
@@ -1573,17 +1586,9 @@ void conf(struct ctx *c, int argc, char **argv)
 	drop_root(uid, gid);
 
 	if (c->mode == MODE_PASTA) {
-		if (*netns && optind != argc) {
-			err("Both --netns and PID or command given");
-			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");
+		if (conf_pasta_ns(&c->netns_only, userns, netns,
+				  optind, argc, argv) < 0)
 			usage(argv[0]);
-		}
 	} else if (optind != argc) {
 		usage(argv[0]);
 	}
@@ -1597,10 +1602,6 @@ void conf(struct ctx *c, int argc, char **argv)
 			if (ret < 0)
 				usage(argv[0]);
 		} else {
-			if (*userns) {
-				err("Both --userns and command given");
-				usage(argv[0]);
-			}
 			pasta_start_ns(c, argc - optind, argv + optind);
 		}
 	}
-- 
2.37.3

--netns-only is supposed to make pasta use only a network namespace, not
a user namespace.  However, pasta_start_ns() has this backwards, and if
--netns-only is specified it creates a user namespace but *not* a network
namespace.  Correct this.

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

diff --git a/pasta.c b/pasta.c
index 0fd45e4..7eac8e9 100644
--- a/pasta.c
+++ b/pasta.c
@@ -244,8 +244,8 @@ void pasta_start_ns(struct ctx *c, int argc, char *argv[])
 
 	pasta_child_pid = clone(pasta_setup_ns,
 				ns_fn_stack + sizeof(ns_fn_stack) / 2,
-				(c->netns_only ? 0 : CLONE_NEWNET) |
-				CLONE_NEWIPC | CLONE_NEWPID | CLONE_NEWUSER |
+				(c->netns_only ? 0 : CLONE_NEWUSER) |
+				CLONE_NEWIPC | CLONE_NEWPID | CLONE_NEWNET |
 				CLONE_NEWUTS,
 				(void *)&arg);
 
-- 
2.37.3

Currently --userns is only allowed when pasta is attaching to an existing
netns or PID, and is prohibited when creating a new netns by spawning a
command or shell.

With the new handling of userns, this check isn't neccessary.  I'm not sure
if there's any use case for --userns with a spawned command, but it's
strictly more flexible and requires zero extra code, so we might as well.

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

diff --git a/conf.c b/conf.c
index 27d520e..ec191c2 100644
--- a/conf.c
+++ b/conf.c
@@ -561,11 +561,6 @@ static int conf_pasta_ns(int *netns_only, char *userns, char *netns,
 		}
 	}
 
-	if (*userns && !*netns) {
-		err("--userns requires --netns or PID");
-		return -EINVAL;
-	}
-
 	/* Attaching to a netns/PID, with no userns given */
 	if (*netns && !*userns)
 		*netns_only = 1;
-- 
2.37.3

On Thu,  8 Sep 2022 13:58:57 +1000
David Gibson <david(a)gibson.dropbear.id.au> wrote:

...
  Sorry for the resend, but I found a bug that means
this will fail to
 build on some distros / versions.
 
 Our handling of user namespaces is more complex than it needs to be.
 This simplifies the handling by identifying and entering (or creating)
 the correct userns earlier, so that later code doesn't need to deal
 with it any more.
 
 Along the way we make a number of other cleanups to handling of userns
 and setting our user and group.
 
 This is based on my earlier test command dispatch and performance test
 cleanup series.
 
 Changes since v1:
  * Fixed overenthusiastic pruning of #includes when moving the
    self-isolation code which broke compile on some distro versions 
I haven't tested this yet, but it looks great.

I can fix up those small things I reported directly while merging, if
you agree, and apply.

A suggestion on how to update the man page as a result of 10/10 would
help, though.

-- 
Stefano