Just nits here:

On Tue, 11 Oct 2022 16:40:10 +1100
David Gibson <david(a)gibson.dropbear.id.au> wrote:

...
  When invoked so as to spawn a shell, pasta checks
explicitly for the
 shell being bash and if so, adds a "-l" option to make it a login shell.
 This is not ideal, since this is a bash specific option and requires pasta
 to know about specific shell variants.
 
 There's a general convention for starting a login shell, which is to
 prepend a "-" to argv[0].  Use this approach instead, so we don't need
bash
 specific logic. 
Hah, I didn't know that was the meaning.

...
  Signed-off-by: David Gibson
<david(a)gibson.dropbear.id.au>
 ---
  pasta.c | 32 ++++++++++++++++++++------------
  1 file changed, 20 insertions(+), 12 deletions(-)
 
 diff --git a/pasta.c b/pasta.c
 index 1dd8267..7c3acef 100644
 --- a/pasta.c
 +++ b/pasta.c
 @@ -148,10 +148,12 @@ void pasta_open_ns(struct ctx *c, const char *netns)
  
  /**
   * struct pasta_setup_ns_arg - Argument for pasta_setup_ns()
 + * @exe:	Executable to run
   * @argv:	Command and arguments to run
   */
  struct pasta_setup_ns_arg {
 -	char **argv;
 +	const char *exe;
 +	char *const *argv;
  };
  
  /**
 @@ -162,12 +164,13 @@ struct pasta_setup_ns_arg {
   */
  static int pasta_setup_ns(void *arg)
  {
 -	struct pasta_setup_ns_arg *a = (struct pasta_setup_ns_arg *)arg;
 +	const struct pasta_setup_ns_arg *a
 +		= (const struct pasta_setup_ns_arg *)arg; 
At this point the assignment could be split onto another line.

...
   
  	FWRITE("/proc/sys/net/ipv4/ping_group_range", "0 0",
  	       "Cannot set ping_group_range, ICMP requests might fail");
  
 -	execvp(a->argv[0], a->argv);
 +	execvp(a->exe, a->argv);
  
  	perror("execvp");
  	exit(EXIT_FAILURE);
 @@ -182,26 +185,31 @@ static int pasta_setup_ns(void *arg)
  void pasta_start_ns(struct ctx *c, int argc, char *argv[])
  {
  	struct pasta_setup_ns_arg arg = {
 +		.exe = argv[0],
  		.argv = argv,
  	};
 -	char *shell = getenv("SHELL");
 -	char *sh_argv[] = { shell, NULL };
 -	char *bash_argv[] = { shell, "-l", NULL };
 +	char *sh_argv[] = { NULL, NULL };
  	char ns_fn_stack[NS_FN_STACK_SIZE]; 
If you respin, it would be nice to have the usual ordering here
(sh_argv[] after ns_fn_stack).

...
  +	char sh_arg0[PATH_MAX + 1];
  
  	c->foreground = 1;
  	if (!c->debug)
  		c->quiet = 1;
  
 -	if (!shell)
 -		shell = "/bin/sh";
  
  	if (argc == 0) {
 -		if (strstr(shell, "/bash")) {
 -			arg.argv = bash_argv;
 -		} else {
 -			arg.argv = sh_argv;
 +		arg.exe = getenv("SHELL");
 +		if (!arg.exe)
 +			arg.exe = "/bin/sh";
 +
 +		if ((size_t)snprintf(sh_arg0, sizeof(sh_arg0),
 +				     "-%s", arg.exe) >= sizeof(sh_arg0)) { 
This is completely specified and looks safe, but it also looks more
complicated than it actually is, at a glance.

Maybe a separate length check before snprintf() would make it look
more natural. Not a strong preference though.

...
  +			err("$SHELL is too long (%u bytes)",
 +			    strlen(arg.exe));
 +			exit(EXIT_FAILURE);
  		}
 +		sh_argv[0] = sh_arg0;
 +		arg.argv = sh_argv;
  	}
  
  	pasta_child_pid = clone(pasta_setup_ns, 
-- 
Stefano

On Thu, 13 Oct 2022 19:22:27 +1100
David Gibson <david(a)gibson.dropbear.id.au> wrote:

...
  On Thu, Oct 13, 2022 at 04:16:59AM +0200, Stefano
Brivio wrote:
  Just nits here:
 
 On Tue, 11 Oct 2022 16:40:10 +1100
 David Gibson <david(a)gibson.dropbear.id.au> wrote:
   
  When invoked so as to spawn a shell, pasta checks
explicitly for the
 shell being bash and if so, adds a "-l" option to make it a login shell.
 This is not ideal, since this is a bash specific option and requires pasta
 to know about specific shell variants.
 
 There's a general convention for starting a login shell, which is to
 prepend a "-" to argv[0].  Use this approach instead, so we don't need
bash
 specific logic.   
 
 Hah, I didn't know that was the meaning.
   
  Signed-off-by: David Gibson
<david(a)gibson.dropbear.id.au>
 ---
  pasta.c | 32 ++++++++++++++++++++------------
  1 file changed, 20 insertions(+), 12 deletions(-)
 
 diff --git a/pasta.c b/pasta.c
 index 1dd8267..7c3acef 100644
 --- a/pasta.c
 +++ b/pasta.c
 @@ -148,10 +148,12 @@ void pasta_open_ns(struct ctx *c, const char *netns)
  
  /**
   * struct pasta_setup_ns_arg - Argument for pasta_setup_ns()
 + * @exe:	Executable to run
   * @argv:	Command and arguments to run
   */
  struct pasta_setup_ns_arg {
 -	char **argv;
 +	const char *exe;
 +	char *const *argv;
  };
  
  /**
 @@ -162,12 +164,13 @@ struct pasta_setup_ns_arg {
   */
  static int pasta_setup_ns(void *arg)
  {
 -	struct pasta_setup_ns_arg *a = (struct pasta_setup_ns_arg *)arg;
 +	const struct pasta_setup_ns_arg *a
 +		= (const struct pasta_setup_ns_arg *)arg;   
 
 At this point the assignment could be split onto another line.   
 
 Uh.. I'm not sure what you mean by that. 
const struct pasta_setup_ns_arg *a;

a = (const struct pasta_setup_ns_arg *)arg;

...
  
 
   
  	FWRITE("/proc/sys/net/ipv4/ping_group_range", "0 0",
  	       "Cannot set ping_group_range, ICMP requests might fail");
  
 -	execvp(a->argv[0], a->argv);
 +	execvp(a->exe, a->argv);
  
  	perror("execvp");
  	exit(EXIT_FAILURE);
 @@ -182,26 +185,31 @@ static int pasta_setup_ns(void *arg)
  void pasta_start_ns(struct ctx *c, int argc, char *argv[])
  {
  	struct pasta_setup_ns_arg arg = {
 +		.exe = argv[0],
  		.argv = argv,
  	};
 -	char *shell = getenv("SHELL");
 -	char *sh_argv[] = { shell, NULL };
 -	char *bash_argv[] = { shell, "-l", NULL };
 +	char *sh_argv[] = { NULL, NULL };
  	char ns_fn_stack[NS_FN_STACK_SIZE];   
 
 If you respin, it would be nice to have the usual ordering here
 (sh_argv[] after ns_fn_stack).   
 
 Done.
 
 
  +	char
sh_arg0[PATH_MAX + 1];
  
  	c->foreground = 1;
  	if (!c->debug)
  		c->quiet = 1;
  
 -	if (!shell)
 -		shell = "/bin/sh";
  
  	if (argc == 0) {
 -		if (strstr(shell, "/bash")) {
 -			arg.argv = bash_argv;
 -		} else {
 -			arg.argv = sh_argv;
 +		arg.exe = getenv("SHELL");
 +		if (!arg.exe)
 +			arg.exe = "/bin/sh";
 +
 +		if ((size_t)snprintf(sh_arg0, sizeof(sh_arg0),
 +				     "-%s", arg.exe) >= sizeof(sh_arg0)) {   
 
 This is completely specified and looks safe, but it also looks more
 complicated than it actually is, at a glance.
 
 Maybe a separate length check before snprintf() would make it look
 more natural. Not a strong preference though.   
 
 Uh.. not sure what you mean by that either. 
Ah, sorry.

if ((len = strlen(arg.exe)) + 1 /* - */ + 1 /* \0 */ > sizeof(sh_arg0))
	err("$SHELL is too long (%u bytes)", strlen(arg.exe));

(void)snprintf(sh_arg0, sizeof(sh_arg0), "-%s", arg.exe);

I don't know, it doesn't look so... cramped. Maybe it's just me. If it
doesn't make sense just disregard ;)

-- 
Stefano

On Thu, Oct 13, 2022 at 04:17:05AM +0200, Stefano Brivio wrote:
...
  I think this, in particular, is really a notable
improvement. Same here,
 only nits:
 
 On Tue, 11 Oct 2022 16:40:13 +1100
 David Gibson <david(a)gibson.dropbear.id.au> wrote:
 
  We have a number of steps of self-isolation
scattered across our code.
 Improve function names and add comments to make it clearer what the self
 isolation model is, what the steps do, and why they happen at the points
 they happen.
 
 Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au>
 ---
  isolation.c | 85 +++++++++++++++++++++++++++++++++++++++++++++++++----
  isolation.h |  6 ++--
  passt.c     |  8 ++---
  3 files changed, 86 insertions(+), 13 deletions(-)
 
 diff --git a/isolation.c b/isolation.c
 index 124dea4..10cef05 100644
 --- a/isolation.c
 +++ b/isolation.c
 @@ -12,6 +12,48 @@
   * Author: Stefano Brivio <sbrivio(a)redhat.com>
   * Author: David Gibson <david(a)gibson.dropbear.id.au>
   */
 +/**
 + * DOC: Theory of Operation
 + *
 + * For security the passt/pasta process performs a number of
 + * self-isolations steps, dropping capabilities, setting namespaces
 + * and otherwise minimizing the impact we can have on the system at
 + * large if we were compromised. 
 
 I would try to be consistent with the BrE spelling we used everywhere
 else: minimising, daemonising. 
Ah, sure.  Here in australia we mostly use british spelling, but tend
to use 'izing' rather than 'ising'.  I've updated it.

...
 
  + *
 + * Obviously we can't isolate ourselves from resources before we've
 + * done anything we need to do with those resources, so we have
 + * multiple stages of self-isolation.  In order these are:
 + *
 + * 1. isolate_initial()
 + * ====================
 + *
 + * Executed immediately after startup, drops capabilities we don't
 + * need at any point during execution (or which we gain back when we
 + * need by joining other namespaces).
 + *
 + * 2. isolate_user()
 + * =================
 + *
 + * Executed once we know what user and user namespace we want to
 + * operate in.  Sets our final UID & GID, and enters the correct user
 + * namespace.
 + *
 + * 3. isolate_prefork()
 + * ====================
 + *
 + * Executed after all setup, but before daemonizing (fork()ing into
 + * the background).  Uses mount namespace and pivot_root() to remove
 + * our access to the filesystem(). 
 
 filesystem() is not a function I know about. :) 
Heh, oops.

...
 
  + *
 + * 4. isolate_postfork()
 + * =====================
 + *
 + * Executed immediately after daemonizing, but before entering the
 + * actual packet forwarding phase of operation.  Or, if not
 + * daemonizing, immediately after isolate_prefork().  Uses seccomp()
 + * to restrict ourselves to the handful of syscalls we need during
 + * runtime operation.
 + */
  
  #include <errno.h>
  #include <fcntl.h>
 @@ -47,7 +89,7 @@
  /**
   * drop_caps() - Drop capabilities we might have except for CAP_NET_BIND_SERVICE
   */
 -void drop_caps(void)
 +static void drop_caps(void)
  {
  	int i;
  
 @@ -59,12 +101,31 @@ void drop_caps(void)
  	}
  }
  
 +/**
 + * isolate_initial() - Early, config independent self isolation
 + *
 + * Should:
 + *  - drop unneeded capabilities
 + * Musn't:
 + *  - remove filessytem access (we need to access files during setup)
 + */
 +void isolate_initial(void)
 +{
 +	drop_caps();
 +}
 +
  /**
   * isolate_user() - Switch to final UID/GID and move into userns
   * @uid:	User ID to run as (in original userns)
   * @gid:	Group ID to run as (in original userns)
   * @use_userns:	Whether to join or create a userns
   * @userns:	userns path to enter, may be empty
 + *
 + * Should:
 + *  - set our final UID and GID
 + *  - enter our final user namespace
 + * Mustn't:
 + *  - remove filesystem access (we need that for further setup)
   */
  void isolate_user(uid_t uid, gid_t gid, bool use_userns, const char *userns)
  {
 @@ -134,11 +195,19 @@ void isolate_user(uid_t uid, gid_t gid, bool use_userns, const char
*userns)
  }
  
  /**
 - * sandbox() - Unshare IPC, mount, PID, UTS, and user namespaces, "unmount"
root
 + * isolate_prefork() - Self isolation before daemonizing
 + * @c:		Execution context
 + * 
 
 What does it return? 
I had that below, I've moved it up here.

...
 
  + * Should:
 + *  - Moves us to our own IPC and UTS namespaces
 + *  - Moves us to a mount namespace with only an empty directory
 + *  - Drops unneeded capabilities (in the new user namespace) 
 
 - move us ...
 - drop ... 
Fixed.

...
  now, this will not move us into a new PID namespace,
so it's a bit
 difficult to summarise in a very short form what it does with regard to
 that, and the comment below is already descriptive enough -- unless you
 can think of something. 
Right.  These lists aren't supposed to be totally exhaustive, just
covering the key points.

...
 
  + *
Mustn't:
 + *  - Remove syscalls we need to daemonize 
 
 "remove", "daemonise", for consistency. 
Adjusted.

...
 
    *
   * Return: negative error code on failure, zero on success
   */
 -int sandbox(struct ctx *c)
 +int isolate_prefork(struct ctx *c)
  {
  	int flags = CLONE_NEWIPC | CLONE_NEWNS | CLONE_NEWUTS;
  
 @@ -187,13 +256,19 @@ int sandbox(struct ctx *c)
  }
  
  /**
 - * seccomp() - Set up seccomp filters depending on mode, won't return on failure
 + * isolate_postfork() - Self isolation after daemonizing
   * @c:		Execution context
 + *
 + * Should:
 + *  - disable core dumps
 + *  - limit to a minimal set of syscalls
   */
 -void seccomp(const struct ctx *c)
 +void isolate_postfork(const struct ctx *c)
  {
  	struct sock_fprog prog;
  
 +	prctl(PR_SET_DUMPABLE, 0);
 +
  	if (c->mode == MODE_PASST) {
  		prog.len = (unsigned short)ARRAY_SIZE(filter_passt);
  		prog.filter = filter_passt;
 diff --git a/isolation.h b/isolation.h
 index 2c73df7..70a38f9 100644
 --- a/isolation.h
 +++ b/isolation.h
 @@ -7,9 +7,9 @@
  #ifndef ISOLATION_H
  #define ISOLATION_H
  
 -void drop_caps(void);
 +void isolate_initial(void);
  void isolate_user(uid_t uid, gid_t gid, bool use_userns, const char *userns);
 -int sandbox(struct ctx *c);
 -void seccomp(const struct ctx *c);
 +int isolate_prefork(struct ctx *c);
 +void isolate_postfork(const struct ctx *c);
  
  #endif /* ISOLATION_H */
 diff --git a/passt.c b/passt.c
 index 2c4a986..46f80a0 100644
 --- a/passt.c
 +++ b/passt.c
 @@ -184,7 +184,7 @@ int main(int argc, char **argv)
  
  	arch_avx2_exec(argv);
  
 -	drop_caps();
 +	isolate_initial();
  
  	c.pasta_netns_fd = c.fd_tap = c.fd_tap_listen = -1;
  
 @@ -287,7 +287,7 @@ int main(int argc, char **argv)
  		}
  	}
  
 -	if (sandbox(&c)) {
 +	if (isolate_prefork(&c)) {
  		err("Failed to sandbox process, exiting\n");
  		exit(EXIT_FAILURE);
  	}
 @@ -297,9 +297,7 @@ int main(int argc, char **argv)
  	else
  		write_pidfile(pidfile_fd, getpid());
  
 -	prctl(PR_SET_DUMPABLE, 0);
 -
 -	seccomp(&c);
 +	isolate_postfork(&c);
  
  	timer_init(&c, &now);
   
  
-- 
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

On Thu, Oct 13, 2022 at 02:49:19PM +0200, Stefano Brivio wrote:
...
  Just spotted a typo:
 
 On Tue, 11 Oct 2022 16:40:13 +1100
 David Gibson <david(a)gibson.dropbear.id.au> wrote:
 
  @@ -59,12 +101,31 @@ void drop_caps(void)
  	}
  }
  
 +/**
 + * isolate_initial() - Early, config independent self isolation
 + *
 + * Should:
 + *  - drop unneeded capabilities
 + * Musn't:
 + *  - remove filessytem access (we need to access files during setup) 
 
 filesystem 
Corrected.

-- 
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

On Tue, 11 Oct 2022 16:40:14 +1100
David Gibson <david(a)gibson.dropbear.id.au> wrote:

...
  In a few places we use the FWRITE() macro to open a
file, replace it's
 contents with a given string and close it again.  There's no real
 reason this needs to be a macro rather than just a function though. 
Well, there's a bit of a reason: it gives more descriptive messages in
isolate_user() with the same LoCs.

On the other hand I would also prefer a function here, probably better
than the alternative I'm not sure why this series needs this by the
way.

...
  Turn it into a function 'write_file()' and
make some ancillary
 cleanups while we're there:
   - Add a return code so the caller can handle giving a useful error message
   - Handle the case of short write()s (unlikely, but possible)
   - Add O_TRUNC, to make sure we replace the existing contents entirely
 
 Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au>
 ---
  isolation.c | 17 +++++++++--------
  pasta.c     |  4 ++--
  util.c      | 33 +++++++++++++++++++++++++++++++++
  util.h      | 13 +------------
  4 files changed, 45 insertions(+), 22 deletions(-)
 
 diff --git a/isolation.c b/isolation.c
 index 10cef05..4aa75e6 100644
 --- a/isolation.c
 +++ b/isolation.c
 @@ -129,7 +129,8 @@ void isolate_initial(void)
   */
  void isolate_user(uid_t uid, gid_t gid, bool use_userns, const char *userns)
  {
 -	char nsmap[BUFSIZ];
 +	char uidmap[BUFSIZ];
 +	char gidmap[BUFSIZ];
  
  	/* First set our UID & GID in the original namespace */
  	if (setgroups(0, NULL)) {
 @@ -184,14 +185,14 @@ void isolate_user(uid_t uid, gid_t gid, bool use_userns, const char
*userns)
  	}
  
  	/* Configure user and group mappings */
 -	snprintf(nsmap, BUFSIZ, "0 %u 1", uid);
 -	FWRITE("/proc/self/uid_map", nsmap, "Cannot set uid_map in
namespace");
 +	snprintf(uidmap, BUFSIZ, "0 %u 1", uid);
 +	snprintf(gidmap, BUFSIZ, "0 %u 1", gid);
  
 -	FWRITE("/proc/self/setgroups", "deny",
 -	       "Cannot write to setgroups in namespace");
 -
 -	snprintf(nsmap, BUFSIZ, "0 %u 1", gid);
 -	FWRITE("/proc/self/gid_map", nsmap, "Cannot set gid_map in
namespace");
 +	if (write_file("/proc/self/uid_map", uidmap) ||
 +	    write_file("/proc/self/setgroups", "deny") ||
 +	    write_file("/proc/self/gid_map", gidmap)) {
 +		warn("Couldn't configure user namespace"); 
It's unlikely that we can write to uid_map but not to setgroups. Still,
having separate messages, as we had them, could help investigating some
issues.

...
  +	}
  }
  
  /**
 diff --git a/pasta.c b/pasta.c
 index 4ff322c..0ab2fe4 100644
 --- a/pasta.c
 +++ b/pasta.c
 @@ -167,8 +167,8 @@ static int pasta_setup_ns(void *arg)
  	const struct pasta_setup_ns_arg *a
  		= (const struct pasta_setup_ns_arg *)arg;
  
 -	FWRITE("/proc/sys/net/ipv4/ping_group_range", "0 0",
 -	       "Cannot set ping_group_range, ICMP requests might fail");
 +	if (write_file("/proc/sys/net/ipv4/ping_group_range", "0 0"))
 +		warn("Cannot set ping_group_range, ICMP requests might fail");
  
  	execvp(a->exe, a->argv);
  
 diff --git a/util.c b/util.c
 index 6b86ead..93b93b1 100644
 --- a/util.c
 +++ b/util.c
 @@ -547,3 +547,36 @@ int fls(unsigned long x)
  
  	return y;
  }
 +
 +/**
 + * write_file() - Replace contents of file with a string
 + * @path:	File to write
 + * @buf:	String to write
 + *
 + * Return: 0 on success, -1 on any error
 + */
 +int write_file(const char *path, const char *buf) 
We could also use this for the PID file by optionally taking a file number, but
I haven't tried how it looks like.

...
  +{
 +	int fd = open(path, O_WRONLY | O_TRUNC | O_CLOEXEC);
 +	size_t len = strlen(buf);
 +
 +	if (fd < 0) {
 +		warn("Could not open %s: %s", path, strerror(errno));
 +		return -1;
 +	}
 +
 +	while (len) {
 +		ssize_t rc = write(fd, buf, len);
 +
 +		if (rc < 0) { 
I would change this to <= 0. Not that it matters with write(), but
should we ever change that another function, we run the (absolutely
not critical) risk of forgetting to adjust this and get stuck in a loop
here.

...
  +			warn("Couldn't write to %s: %s",
path, strerror(errno));
 +			break;
 +		}
 +
 +		buf += rc;
 +		len -= rc;
 +	}
 +
 +	close(fd);
 +	return len == 0 ? 0 : -1;
 +}
 diff --git a/util.h b/util.h
 index 0c06e34..f957522 100644
 --- a/util.h
 +++ b/util.h
 @@ -58,18 +58,6 @@ void trace_init(int enable);
  #define TMPDIR		"/tmp"
  #endif
  
 -#define FWRITE(path, buf, str)						\
 -	do {								\
 -		int flags = O_WRONLY | O_CLOEXEC;			\
 -		int fd = open(path, flags);				\
 -									\
 -		if (fd < 0 ||						\
 -		    write(fd, buf, strlen(buf)) != (int)strlen(buf))	\
 -			warn(str);					\
 -		if (fd >= 0)						\
 -			close(fd);					\
 -	} while (0)
 -
  #define V4		0
  #define V6		1
  #define IP_VERSIONS	2
 @@ -215,5 +203,6 @@ int ns_enter(const struct ctx *c);
  void write_pidfile(int fd, pid_t pid);
  int __daemon(int pidfile_fd, int devnull_fd);
  int fls(unsigned long x);
 +int write_file(const char *path, const char *buf);
  
  #endif /* UTIL_H */ 
-- 
Stefano

On Thu, Oct 13, 2022 at 04:18:24AM +0200, Stefano Brivio wrote:
...
  Well, this drop_caps() is pretty much the same as
patch 8/10, so it
 actually did something. :) 
Yes, but not what we wanted :).

...
  On Tue, 11 Oct 2022 16:40:15 +1100
 David Gibson <david(a)gibson.dropbear.id.au> wrote:
 
  The current implementation of drop_caps()
doesn't really work because it
 attempts to drop capabilities from the bounding set.  hat's not the set
 that really matters: the bounding set is about limiting the abilities of
 otherwise things we might later exec() rather than our own capabilities.
 In addition altering the bounding set requires CAP_SETPCAP which we won't
 usually have.
 
 Replace it with a new version which uses setcap(2) to drop capabilities
 from the effective and permitted sets, which is what actually matters for
 most purposes.  For now we leave the inheritable set alone, since we don't
 want to preclude the user from passing inheritable capabilities to the
 command spawed by pasta.
 
 Correctly dropping caps reveals that we actually need CAP_SYS_ADMIN within
 the userns we create/join in pasta mode, so that we can later setns() to
 the netns within it.
 
 Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au>
 ---
  isolation.c | 52 ++++++++++++++++++++++++++++++++++++++++++++--------
  1 file changed, 44 insertions(+), 8 deletions(-)
 
 diff --git a/isolation.c b/isolation.c
 index 4aa75e6..2468f84 100644
 --- a/isolation.c
 +++ b/isolation.c
 @@ -86,18 +86,37 @@
  #include "passt.h"
  #include "isolation.h"
  
 +#define CAP_VERSION	_LINUX_CAPABILITY_VERSION_3
 +#define CAP_WORDS	_LINUX_CAPABILITY_U32S_3
 +
  /**
 - * drop_caps() - Drop capabilities we might have except for CAP_NET_BIND_SERVICE
 + * drop_caps_ep_except() - Drop capabilities from effective & permitted sets
 + * @keep:	Capabilities to keep
   */
 -static void drop_caps(void)
 +static void drop_caps_ep_except(uint64_t keep)
  {
 +	struct __user_cap_header_struct hdr = {
 +		.version = CAP_VERSION,
 +		.pid = 0,
 +	};
 +	struct __user_cap_data_struct data[CAP_WORDS];
  	int i;
  
 -	for (i = 0; i < 64; i++) {
 -		if (i == CAP_NET_BIND_SERVICE)
 -			continue;
 +	if (syscall(SYS_capget, &hdr, data)) {
 +		err("Couldn't get current capabilities: %s", strerror(errno));
 +		exit(EXIT_FAILURE);
 +	}
 +
 +	for (i = 0; i < CAP_WORDS; i++) {
 +		uint32_t mask = keep >> (32 * i);
 +
 +		data[i].effective &= mask;
 +		data[i].permitted &= mask;
 +	}
  
 -		prctl(PR_CAPBSET_DROP, i, 0, 0, 0);
 +	if (syscall(SYS_capset, &hdr, data)) {
 +		err("Couldn't drop capabilities: %s", strerror(errno));
 +		exit(EXIT_FAILURE);
  	}
  }
  
 @@ -111,7 +130,11 @@ static void drop_caps(void)
   */
  void isolate_initial(void)
  {
 -	drop_caps();
 +	/* We want to keep CAP_NET_BIND_SERVICE in the initial
 +	 * namespace if we have it, so that we can forward low ports
 +	 * into the guest/namespace
 +	 */
 +	drop_caps_ep_except((1UL << CAP_NET_BIND_SERVICE)); 
 
 You could use BIT() (util.h) here, 
Ah, yes, done.

...
 
   }
  
  /**
 @@ -211,6 +234,7 @@ void isolate_user(uid_t uid, gid_t gid, bool use_userns, const char
*userns)
  int isolate_prefork(struct ctx *c)
  {
  	int flags = CLONE_NEWIPC | CLONE_NEWNS | CLONE_NEWUTS;
 +	uint64_t ns_caps = 0;
  
  	/* 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
 @@ -251,7 +275,19 @@ int isolate_prefork(struct ctx *c)
  		return -errno;
  	}
  
 -	drop_caps();	/* Relative to the new user namespace this time. */
 +	/* Drop capabilites in our new userns */
 +	if (c->mode == MODE_PASTA) {
 +		/* Keep CAP_SYS_ADMIN, so that we can setns() to the
 +		 * netns when we need to act upon it
 +		 */
 +		ns_caps |= 1UL << CAP_SYS_ADMIN; 
 
 here,
 
  +		/* Keep CAP_NET_BIND_SERVICE, so we can
splice
 +		 * outbound connections to low port numbers
 +		 */
 +		ns_caps |= 1UL << CAP_NET_BIND_SERVICE; 
 
 and here.
 
  +	}
 +
 +	drop_caps_ep_except(ns_caps);
  
  	return 0;
  } 
  
-- 
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

On Thu, 13 Oct 2022 06:01:19 +0200
Stefano Brivio <sbrivio(a)redhat.com> wrote:

...
  On Tue, 11 Oct 2022 16:40:15 +1100
 David Gibson <david(a)gibson.dropbear.id.au> wrote:
 
  @@ -251,7 +275,19 @@ int isolate_prefork(struct
ctx *c)
  		return -errno;
  	}
  
 -	drop_caps();	/* Relative to the new user namespace this time. */
 +	/* Drop capabilites in our new userns */
 +	if (c->mode == MODE_PASTA) {
 +		/* Keep CAP_SYS_ADMIN, so that we can setns() to the
 +		 * netns when we need to act upon it
 +		 */
 +		ns_caps |= 1UL << CAP_SYS_ADMIN;
 +		/* Keep CAP_NET_BIND_SERVICE, so we can splice
 +		 * outbound connections to low port numbers
 +		 */
 +		ns_caps |= 1UL << CAP_NET_BIND_SERVICE;
 +	}
 +
 +	drop_caps_ep_except(ns_caps);   
 
 Hmm, I didn't really look into this yet, but there seems to be an issue
 with filesystem-bound network namespaces now. Running something like:
 
   pasta --config-net --netns
/run/user/1000/netns/netns-6466ff4b-1efc-2b58-685b-cbc12feb9ccc
 
 (from Podman), this happens:
 
 [...]

 [pid 1763223] setns(7, CLONE_NEWNET)    = -1 EPERM (Operation not permitted) 
Ah, "of course". Podman calls us with UID 0 in the user namespace it
just created, so if we drop CAP_SYS_ADMIN in isolate_initial() we can't
join the network namespace, and if we drop CAP_NET_ADMIN we can't
configure it.

So for that case (and only for that, I suppose), we need something like
(tested):

diff --git a/isolation.c b/isolation.c
index 1769180..fee6dbd 100644
--- a/isolation.c
+++ b/isolation.c
@@ -190,7 +190,7 @@ void isolate_initial(void)
         * namespace if we have it, so that we can forward low ports
         * into the guest/namespace
         */
-       drop_caps_ep_except((1UL << CAP_NET_BIND_SERVICE));
+       drop_caps_ep_except(BIT(CAP_SYS_ADMIN) | BIT(CAP_NET_ADMIN));
 }

...which is a bit pointless. Better than *any* capability, but not by
far.

So, if we make this totally independent from configuration, we need
those two capabilities.

We could add a "postconf" stage and cover a tiny bit more of conf.c.

Or we could have a special path in isolate_initial() for the case we
know we're not in the init namespace.

I'm not sure. If you have a specific preference/strong opinion I would
actually be happier. :)

-- 
Stefano

On Thu, Oct 13, 2022 at 06:37:05PM +0200, Stefano Brivio wrote:
...
  On Thu, 13 Oct 2022 15:08:02 +0200
 Stefano Brivio <sbrivio(a)redhat.com> wrote:
 
  On Thu, 13 Oct 2022 06:01:19 +0200
 Stefano Brivio <sbrivio(a)redhat.com> wrote:
 
  On Tue, 11 Oct 2022 16:40:15 +1100
 David Gibson <david(a)gibson.dropbear.id.au> wrote:
   
  @@ -251,7 +275,19 @@ int isolate_prefork(struct
ctx *c)
  		return -errno;
  	}
  
 -	drop_caps();	/* Relative to the new user namespace this time. */
 +	/* Drop capabilites in our new userns */
 +	if (c->mode == MODE_PASTA) {
 +		/* Keep CAP_SYS_ADMIN, so that we can setns() to the
 +		 * netns when we need to act upon it
 +		 */
 +		ns_caps |= 1UL << CAP_SYS_ADMIN;
 +		/* Keep CAP_NET_BIND_SERVICE, so we can splice
 +		 * outbound connections to low port numbers
 +		 */
 +		ns_caps |= 1UL << CAP_NET_BIND_SERVICE;
 +	}
 +
 +	drop_caps_ep_except(ns_caps);     
 
 Hmm, I didn't really look into this yet, but there seems to be an issue
 with filesystem-bound network namespaces now. Running something like:
 
   pasta --config-net --netns
/run/user/1000/netns/netns-6466ff4b-1efc-2b58-685b-cbc12feb9ccc
 
 (from Podman), this happens:
 
 [...]

 [pid 1763223] setns(7, CLONE_NEWNET)    = -1 EPERM (Operation not permitted)   
 
 Ah, "of course". Podman calls us with UID 0 in the user namespace it
 just created, so if we drop CAP_SYS_ADMIN in isolate_initial() we can't
 join the network namespace, and if we drop CAP_NET_ADMIN we can't
 configure it.
 
 So for that case (and only for that, I suppose), we need something like
 (tested):
 
 diff --git a/isolation.c b/isolation.c
 index 1769180..fee6dbd 100644
 --- a/isolation.c
 +++ b/isolation.c
 @@ -190,7 +190,7 @@ void isolate_initial(void)
          * namespace if we have it, so that we can forward low ports
          * into the guest/namespace
          */
 -       drop_caps_ep_except((1UL << CAP_NET_BIND_SERVICE));
 +       drop_caps_ep_except(BIT(CAP_SYS_ADMIN) | BIT(CAP_NET_ADMIN));
  }
 
 ...which is a bit pointless. Better than *any* capability, but not by
 far.
 
 So, if we make this totally independent from configuration, we need
 those two capabilities.
 
 We could add a "postconf" stage and cover a tiny bit more of conf.c.
 
 Or we could have a special path in isolate_initial() for the case we
 know we're not in the init namespace.
 
 I'm not sure. If you have a specific preference/strong opinion I would
 actually be happier. :) 
 
 Further on, if we are started as root, we'll fail to drop to 'nobody'
 or any other user, if we lose CAP_SETUID and CAP_SETGID here. I have
 tested this version of isolate_initial():
 
 	drop_caps_ep_except(BIT(CAP_SYS_ADMIN) | BIT(CAP_NET_ADMIN) |
 			    BIT(CAP_SETGID)    | BIT(CAP_SETUID)    |
 			    BIT(CAP_NET_BIND_SERVICE));
 
 for any use case I can reasonably think of. Yes, it's a lot -- we
 should make it really clear that those are not the capabilities we
 actually use at "run time". 
Ah, right.  I didn't think through the --netns-only case.

I think what we need to do in the short term at least is:

 * Add all those caps and a comment in isolate_initial() (or even just
   remove the drop_caps there)
 * In isolate user drop the caps we can at that point (SETGID and
   SETUID) - whether or not we've joined a new userns
 * Remove the rest of the "runtime" caps in isolate_prefork() like we
   do now.

I'll rework accordingly.

-- 
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

On Tue, 11 Oct 2022 16:40:16 +1100
David Gibson <david(a)gibson.dropbear.id.au> wrote:

...
  We drop our own capabilities, but it's possible
that processes we exec()
 could gain extra privilege via file capabilities.  It shouldn't be possible
 for us to exec() anyway due to seccomp() and our filesystem isolation.  But
 just in case, zero the bounding and inheritable capability sets to prevent
 any such child from gainin privilege.
 
 Note that we do this *after* spawning the pasta shell/command (if any),
 because we do want the user to be able to give that privilege if they want.
 
 Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au>
 ---
  isolation.c | 56 +++++++++++++++++++++++++++++++++++++++++++++++++++++
  1 file changed, 56 insertions(+)
 
 diff --git a/isolation.c b/isolation.c
 index 2468f84..e1a024d 100644
 --- a/isolation.c
 +++ b/isolation.c
 @@ -120,6 +120,61 @@ static void drop_caps_ep_except(uint64_t keep)
  	}
  }
  
 +/**
 + * clamp_caps() - Prevent any children from gaining caps 
"clamp" doesn't sound very specific or clear. caps_drop_inherit_bound()
would actually tell me what the function does, but it's a bit of a
mouthful in comparison. I'm fine with both, really, but if you have a
better idea...

...
  + *
 + * This drops all capabilities from both the inheritable and the
 + * bounding set.  This means that any exec()ed processes can't gain
 + * capabilities, even if they have file capabilities which would grant
 + * them.  We shouldn't ever exec() in any case, but this provides an
 + * additional layer of protection.  Executing this requires
 + * CAP_SETPCAP, which we will have within our userns.
 + *
 + * Note that dropping capabilites from the bounding set limits
 + * exec()ed processes, but does not remove them from the effective or
 + * permitted sets, so it doesn't reduce our own capabilities.
 + */
 +static void clamp_caps(void)
 +{
 +	struct __user_cap_header_struct hdr = {
 +		.version = CAP_VERSION,
 +		.pid = 0,
 +	};
 +	struct __user_cap_data_struct data[CAP_WORDS]; 
For consistency, I'd move this before hdr.

...
  +	int i;
 +
 +	for (i = 0; i < 64; i++) {
 +		/* Some errors can be ignored:
 +		 * - EINVAL, we'll get this for all values in 0..63
 +		 *   that are not actually allocated caps
 +		 * - EPERM, we'll get this if we don't have
 +		 *   CAP_SETPCAP, which can happen if using
 +		 *   --netns-only.  We don't need CAP_SETPCAP for
 +		 *   normal operation, so carry on without it.
 +		 */
 +		if (prctl(PR_CAPBSET_DROP, i, 0, 0, 0) &&
 +		    errno != EINVAL && errno != EPERM) {
 +			err("Couldn't drop cap %i from bounding set: %s",
 +			    i, strerror(errno));
 +			exit(EXIT_FAILURE);
 +		}
 +	}
 +
 +	if (syscall(SYS_capget, &hdr, data)) {
 +		err("Couldn't get current capabilities: %s", strerror(errno));
 +		exit(EXIT_FAILURE);
 +	}
 +
 +	for (i = 0; i < CAP_WORDS; i++)
 +		data[i].inheritable = 0; 
Any specific reason why? Initialisers can have variable sizes to some
extent, but if there's a reason why it can't be done, perhaps that
would warrant a comment here.

...
  +
 +	if (syscall(SYS_capset, &hdr, data)) {
 +		err("Couldn't drop inheritable capabilities: %s",
 +		    strerror(errno));
 +		exit(EXIT_FAILURE);
 +	}
 +}
 +
  /**
   * isolate_initial() - Early, config independent self isolation
   *
 @@ -287,6 +342,7 @@ int isolate_prefork(struct ctx *c)
  		ns_caps |= 1UL << CAP_NET_BIND_SERVICE;
  	}
  
 +	clamp_caps();
  	drop_caps_ep_except(ns_caps);
  
  	return 0; 
-- 
Stefano

On Thu, 13 Oct 2022 20:33:34 +1100
David Gibson <david(a)gibson.dropbear.id.au> wrote:

...
  On Thu, Oct 13, 2022 at 04:17:30AM +0200, Stefano
Brivio wrote:
  On Tue, 11 Oct 2022 16:40:16 +1100
 David Gibson <david(a)gibson.dropbear.id.au> wrote:
   
  We drop our own capabilities, but it's
possible that processes we exec()
 could gain extra privilege via file capabilities.  It shouldn't be possible
 for us to exec() anyway due to seccomp() and our filesystem isolation.  But
 just in case, zero the bounding and inheritable capability sets to prevent
 any such child from gainin privilege.
 
 Note that we do this *after* spawning the pasta shell/command (if any),
 because we do want the user to be able to give that privilege if they want.
 
 Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au>
 ---
  isolation.c | 56 +++++++++++++++++++++++++++++++++++++++++++++++++++++
  1 file changed, 56 insertions(+)
 
 diff --git a/isolation.c b/isolation.c
 index 2468f84..e1a024d 100644
 --- a/isolation.c
 +++ b/isolation.c
 @@ -120,6 +120,61 @@ static void drop_caps_ep_except(uint64_t keep)
  	}
  }
  
 +/**
 + * clamp_caps() - Prevent any children from gaining caps   
 
 "clamp" doesn't sound very specific or clear. caps_drop_inherit_bound()
 would actually tell me what the function does, but it's a bit of a
 mouthful in comparison. I'm fine with both, really, but if you have a
 better idea...   
 
 Yeah, I couldn't think of something that was both brief and specific,
 so I went with brief.
 
 
  + *
 + * This drops all capabilities from both the inheritable and the
 + * bounding set.  This means that any exec()ed processes can't gain
 + * capabilities, even if they have file capabilities which would grant
 + * them.  We shouldn't ever exec() in any case, but this provides an
 + * additional layer of protection.  Executing this requires
 + * CAP_SETPCAP, which we will have within our userns.
 + *
 + * Note that dropping capabilites from the bounding set limits
 + * exec()ed processes, but does not remove them from the effective or
 + * permitted sets, so it doesn't reduce our own capabilities.
 + */
 +static void clamp_caps(void)
 +{
 +	struct __user_cap_header_struct hdr = {
 +		.version = CAP_VERSION,
 +		.pid = 0,
 +	};
 +	struct __user_cap_data_struct data[CAP_WORDS];   
 
 For consistency, I'd move this before hdr.   
 
 Ok.
 
 
  +	int i;
 +
 +	for (i = 0; i < 64; i++) {
 +		/* Some errors can be ignored:
 +		 * - EINVAL, we'll get this for all values in 0..63
 +		 *   that are not actually allocated caps
 +		 * - EPERM, we'll get this if we don't have
 +		 *   CAP_SETPCAP, which can happen if using
 +		 *   --netns-only.  We don't need CAP_SETPCAP for
 +		 *   normal operation, so carry on without it.
 +		 */
 +		if (prctl(PR_CAPBSET_DROP, i, 0, 0, 0) &&
 +		    errno != EINVAL && errno != EPERM) {
 +			err("Couldn't drop cap %i from bounding set: %s",
 +			    i, strerror(errno));
 +			exit(EXIT_FAILURE);
 +		}
 +	}
 +
 +	if (syscall(SYS_capget, &hdr, data)) {
 +		err("Couldn't get current capabilities: %s", strerror(errno));
 +		exit(EXIT_FAILURE);
 +	}
 +
 +	for (i = 0; i < CAP_WORDS; i++)
 +		data[i].inheritable = 0;   
 
 Any specific reason why? Initialisers can have variable sizes to some
 extent, but if there's a reason why it can't be done, perhaps that
 would warrant a comment here.   
 
 Why what?  We're not trying to alter the permitted or effective sets
 here, so we're doing a capget() first, zeroing the inheritable field,
 then setting it back again. 
Oops, never mind, of course, I missed the capget() for a moment.

-- 
Stefano

On Tue, 11 Oct 2022 16:40:17 +1100
David Gibson <david(a)gibson.dropbear.id.au> wrote:

...
  When in passt mode, or pasta mode spawning a command,
we create a userns
 for ourselves.  This is used both to isolate the pasta/passt process itself
 and to run the spawned command, if any.
 
 Since eed17a47 "Handle userns isolation and dropping root at the same time"
 we've handled both cases the same, configuring the UID and GID mappings in
 the new userns to map whichever UID we're running as to root within the
 userns.
 
 This mapping is desirable when spawning a shell or other command, so that
 the user gets a root shell with reasonably clear abilities within the
 userns and netns.  It's not necessarily essential, though.  When not
 spawning a shell, it doesn't really have any purpose: passt itself doesn't
 need to be root and can operate fine with an unmapped user (using some of
 the capabilities we get when entering the userns instead).
 
 Configuring the uid_map can cause problems if passt is running with any
 capabilities in the initial namespace, such as CAP_NET_BIND_SERVICE to
 allow it to forward low ports.  In this case the kernel makes files in
 /proc/pid owned by root rather than the starting user to prevent the user
 from interfering with the operation of the capability-enhanced process.
 This includes uid_map meaning we are not able to write to it.
 
 Whether this behaviour is correct in the kernel is debatable, but in any
 case we might as well avoid problems by only initializing the user mappings
 when we really want them.
 
 Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au>
 ---
  conf.c      |  3 ++-
  isolation.c | 13 -------------
  pasta.c     | 16 ++++++++++++++--
  pasta.h     |  3 ++-
  4 files changed, 18 insertions(+), 17 deletions(-)
 
 diff --git a/conf.c b/conf.c
 index 1537dbf..b7661b6 100644
 --- a/conf.c
 +++ b/conf.c
 @@ -1478,7 +1478,8 @@ void conf(struct ctx *c, int argc, char **argv)
  		if (*netns) {
  			pasta_open_ns(c, netns);
  		} else {
 -			pasta_start_ns(c, argc - optind, argv + optind);
 +			pasta_start_ns(c, uid, gid,
 +				       argc - optind, argv + optind);
  		}
  	}
  
 diff --git a/isolation.c b/isolation.c
 index e1a024d..b94226d 100644
 --- a/isolation.c
 +++ b/isolation.c
 @@ -207,9 +207,6 @@ void isolate_initial(void)
   */
  void isolate_user(uid_t uid, gid_t gid, bool use_userns, const char *userns)
  {
 -	char uidmap[BUFSIZ];
 -	char gidmap[BUFSIZ];
 -
  	/* First set our UID & GID in the original namespace */
  	if (setgroups(0, NULL)) {
  		/* If we don't have CAP_SETGID, this will EPERM */
 @@ -261,16 +258,6 @@ void isolate_user(uid_t uid, gid_t gid, bool use_userns, const char
*userns)
  		err("Couldn't create user namespace: %s", strerror(errno));
  		exit(EXIT_FAILURE);
  	}
 -
 -	/* Configure user and group mappings */
 -	snprintf(uidmap, BUFSIZ, "0 %u 1", uid);
 -	snprintf(gidmap, BUFSIZ, "0 %u 1", gid);
 -
 -	if (write_file("/proc/self/uid_map", uidmap) ||
 -	    write_file("/proc/self/setgroups", "deny") ||
 -	    write_file("/proc/self/gid_map", gidmap)) {
 -		warn("Couldn't configure user namespace");
 -	}
  }
  
  /**
 diff --git a/pasta.c b/pasta.c
 index 0ab2fe4..9666fed 100644
 --- a/pasta.c
 +++ b/pasta.c
 @@ -166,7 +166,6 @@ static int pasta_setup_ns(void *arg)
  {
  	const struct pasta_setup_ns_arg *a
  		= (const struct pasta_setup_ns_arg *)arg;
 - 
Unrelated.

...
   	if
(write_file("/proc/sys/net/ipv4/ping_group_range", "0 0"))
  		warn("Cannot set ping_group_range, ICMP requests might fail");
  
 @@ -179,16 +178,20 @@ static int pasta_setup_ns(void *arg)
  /**
   * pasta_start_ns() - Fork command in new namespace if target ns is not given
   * @c:		Execution context
 + * @uid:	UID we're running as in the init namespace
 + * @gid:	GID we're running as in the init namespace
   * @argc:	Number of arguments for spawned command
   * @argv:	Command to spawn and arguments
   */
 -void pasta_start_ns(struct ctx *c, int argc, char *argv[])
 +void pasta_start_ns(struct ctx *c, uid_t uid, gid_t gid,
 +		    int argc, char *argv[])
  {
  	struct pasta_setup_ns_arg arg = {
  		.exe = argv[0],
  		.argv = argv,
  	};
  	char *sh_argv[] = { NULL, NULL };
 +	char uidmap[BUFSIZ], gidmap[BUFSIZ]; 
Should go at the beginning for the usual semi-silly reason.

...
   	char ns_fn_stack[NS_FN_STACK_SIZE];
  	char sh_arg0[PATH_MAX + 1];
  
 @@ -196,7 +199,16 @@ void pasta_start_ns(struct ctx *c, int argc, char *argv[])
  	if (!c->debug)
  		c->quiet = 1;
  
 +	/* Configure user and group mappings */
 +	snprintf(uidmap, BUFSIZ, "0 %u 1", uid);
 +	snprintf(gidmap, BUFSIZ, "0 %u 1", gid);
  
 +	if (write_file("/proc/self/uid_map", uidmap) ||
 +	    write_file("/proc/self/setgroups", "deny") ||
 +	    write_file("/proc/self/gid_map", gidmap)) {
 +		warn("Couldn't configure user mappings");
 +	}
 +	 
Excess whitespace.

...
   	if (argc == 0) {
  		arg.exe = getenv("SHELL");
  		if (!arg.exe)
 diff --git a/pasta.h b/pasta.h
 index 02df1f6..a8b9893 100644
 --- a/pasta.h
 +++ b/pasta.h
 @@ -7,7 +7,8 @@
  #define PASTA_H
  
  void pasta_open_ns(struct ctx *c, const char *netns);
 -void pasta_start_ns(struct ctx *c, int argc, char *argv[]);
 +void pasta_start_ns(struct ctx *c, uid_t uid, gid_t gid,
 +		    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); 
-- 
Stefano