On Thu, Oct 13, 2022 at 04:18:29AM +0200, Stefano Brivio wrote: > On Tue, 11 Oct 2022 16:40:17 +1100 > David Gibson 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 > > --- > > 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. Removed. > > 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. Done. > > 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. Fixed. > > 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); > -- 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