On Thu, Oct 13, 2022 at 04:18:29AM +0200, Stefano Brivio wrote:On Tue, 11 Oct 2022 16:40:17 +1100 David Gibson <david(a)gibson.dropbear.id.au> wrote:Removed.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.Done.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.Fixed.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.-- 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/~dgibsonif (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);