Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feature request: unshare in two steps: setup_cmd as uid=0 and run_cmd as uid=uid #885

Closed
zokrezyl opened this issue Oct 18, 2019 · 11 comments
Labels
TODO We going to think about it ;-)

Comments

@zokrezyl
Copy link

Just found out that bunch of useful new cmd line arguments were added (--map-current-user, --root). Now you can become both root (-r) or current user (-c). Something is however missing, kind of the combination of the two. A usecase I bumped into often recently is following;

  1. create unprivileged CLONE_NEWUSER, combined with at least CLONE_NEWNS with unshare -r
  2. mount some stuff
  3. pivot_root
  4. create nested namespace
  5. map user to original user (-c)

While --root solves part of the problem, often there would be bunch of stuff needed to be done before the nested user namespace with user mapped to current user.

So what is missing and would be very useful to have a flag/argument for unshare that would run some "setup" before the nested user namespace.

unshare -c --setup my_setup_cmd --pid --fork --uts --mount real_cmd

The problem is that even with the new features (-c) I could not use the combination with unshare -r and unshare -c to achieve the desired result.

@zokrezyl zokrezyl changed the title unshare two steps: setup as uid=0 and run_cmd as uid=uid feature request: unshare in two steps: setup_cmd as uid=0 and run_cmd as uid=uid Oct 18, 2019
@zokrezyl
Copy link
Author

Shall I create a PR?

@karelzak
Copy link
Collaborator

If I good understand you want to call two unshare syscalls in one unshare(8) execution and some --setup command between the syscalls, right? Sounds pretty complex for this small low-level tool tools ;-)

Please, read https://lore.kernel.org/util-linux/87ef1fj12w.fsf@xmission.com/T/#t where is our previous upstream discussion about more complex stuff in unshare(8). The goal is keep unshare(8) simple and stupid wrapper around unshare(2) syscall.

Maybe from long point of view we need a new command for more complicated tasks where you can specify individual steps and execute additional commands or mount something. The question is if something like this is really necessary as we already have another projects to maintain container etc.

@zokrezyl
Copy link
Author

Right, two unshare calls in one execution, yes. I do not mind alternative solutions, but atm with existing tools you cannot achieve the very powerful usecase, I mentioned. Depends what you mean when you say "complex". Coding-wise you may admit, would be 6-7 lines of code. Though the tool is already a bit more than a wrapper around unshare kernel call and I kind of agree that it is against Unix principles, but the reasoning is:

-> For what I need to achieve (and again, I believe this is a usecase would be useful for thousands), to have a container in userspace without root privileges, unshare tool does not have that step
-> that steps involvs a call to "unshare", thus a tool like "unshare" would make sense
-> running unshare in the process started by first unshare would make sense, but again unshare tool is missing that feature.

So, what about implementing the basic usecase in the unshare tool then:
-> unshare(CLONE_NEWUSER)
-> write("/proc/$$/uid_map, "1000 0 1"
-> write("/proc/$$/gid_map, "1000 0 1"

and that would solve the problem in an elegant way, by allowing in a shell script to start first script with uid/gid=0 and the second script would "drop" to the original uid.

Or is that feature (use-case) available in the latest versions? Sorry, was working so far only with released versions.

@mat8913
Copy link
Contributor

mat8913 commented Jan 3, 2020

I think simply allowing the user to specify custom uid/gid mappings would solve this nicely. That way unshare(8) maintains its simplicity and one unshare syscall per unshare(8) execution, while this usecase can still be implemented outside of unshare(8) like so:

#!/bin/sh
REAL_UID=$(id -ru)
REAL_GID=$(id -rg)
unshare -r sh -c "setup_cmd; exec unshare --map-user=$REAL_UID --map-group=$REAL_GID run_cmd"

Here's a patch:

From 4594294314efb20507db17a37d79fb311673c887 Mon Sep 17 00:00:00 2001
From: Matthew Harm Bekkema <id@mbekkema.name>
Date: Sat, 4 Jan 2020 09:11:30 +1100
Subject: [PATCH] unshare: allow custom uid/gid mappings in userns

Two new options are added: `--map-user=<uid>` and `--map-group=<gid>`
for custom user and group mappings respectively. These are just
generalizations of the existing `--map-root-user` and
`--map-current-user` options.

As a side effect of this commit, specifying both `--map-root-user` and
`--map-current-user` no longer causes an error. Instead, the last
occurrence takes precedence.

Addresses: https://github.com/karelzak/util-linux/issues/885
Signed-off-by: Matthew Harm Bekkema <id@mbekkema.name>
---
 sys-utils/unshare.1 | 12 +++++++++
 sys-utils/unshare.c | 66 +++++++++++++++++++++------------------------
 2 files changed, 42 insertions(+), 36 deletions(-)

diff --git a/sys-utils/unshare.1 b/sys-utils/unshare.1
index 597e6160e..55b5926b3 100644
--- a/sys-utils/unshare.1
+++ b/sys-utils/unshare.1
@@ -156,6 +156,16 @@ implies creating a new mount namespace since the /proc mount would otherwise
 mess up existing programs on the system.  The new proc filesystem is explicitly
 mounted as private (with MS_PRIVATE|MS_REC).
 .TP
+.BR \-\-map\-user=\fIuid
+Run the program only after the current effective user ID has been mapped to \fIuid\fP.
+If this option is specified multiple times, the last occurrence takes precedence.
+This option implies \fB\-\-user\fR.
+.TP
+.BR \-\-map\-group=\fIgid
+Run the program only after the current effective group ID has been mapped to \fIgid\fP.
+If this option is specified multiple times, the last occurrence takes precedence.
+This option implies \fB\-\-setgroups=deny\fR and \fB\-\-user\fR.
+.TP
 .BR \-r , " \-\-map\-root\-user"
 Run the program only after the current effective user and group IDs have been mapped to
 the superuser UID and GID in the newly created user namespace.  This makes it possible to
@@ -164,11 +174,13 @@ namespaces (such as configuring interfaces in the network namespace or mounting
 the mount namespace) even when run unprivileged.  As a mere convenience feature, it does not support
 more sophisticated use cases, such as mapping multiple ranges of UIDs and GIDs.
 This option implies \fB\-\-setgroups=deny\fR and \fB\-\-user\fR.
+This option is equivalent to \fB\-\-map-user=0 \-\-map-group=0\fR.
 .TP
 .BR \-c , " \-\-map\-current\-user"
 Run the program only after the current effective user and group IDs have been mapped to
 the same UID and GID in the newly created user namespace. This option implies
 \fB\-\-setgroups=deny\fR and \fB\-\-user\fR.
+This option is equivalent to \fB\-\-map-user=$(id -ru) \-\-map-group=$(id -rg)\fR.
 .TP
 .BR "\-\-propagation private" | shared | slave | unchanged
 Recursively set the mount propagation flag in the new mount namespace.  The default
diff --git a/sys-utils/unshare.c b/sys-utils/unshare.c
index 8d33f2273..9e9d5ee02 100644
--- a/sys-utils/unshare.c
+++ b/sys-utils/unshare.c
@@ -76,12 +76,6 @@ enum {
 	SETGROUPS_ALLOW = 1,
 };
 
-enum {
-	MAP_USER_NONE,
-	MAP_USER_ROOT,
-	MAP_USER_CURRENT,
-};
-
 static const char *setgroups_strings[] =
 {
 	[SETGROUPS_DENY] = "deny",
@@ -269,6 +263,8 @@ static void __attribute__((__noreturn__)) usage(void)
 	fputs(_(" -C, --cgroup[=<file>]     unshare cgroup namespace\n"), out);
 	fputs(USAGE_SEPARATOR, out);
 	fputs(_(" -f, --fork                fork before launching <program>\n"), out);
+	fputs(_(" --map-user=<uid>          map current user to uid (implies --user)\n"), out);
+	fputs(_(" --map-group=<gid>         map current group to gid (implies --user)\n"), out);
 	fputs(_(" -r, --map-root-user       map current user to root (implies --user)\n"), out);
 	fputs(_(" -c, --map-current-user    map current user to itself (implies --user)\n"), out);
 	fputs(USAGE_SEPARATOR, out);
@@ -300,6 +296,8 @@ int main(int argc, char *argv[])
 		OPT_SETGROUPS,
 		OPT_KILLCHILD,
 		OPT_KEEPCAPS,
+		OPT_MAPUSER,
+		OPT_MAPGROUP,
 	};
 	static const struct option longopts[] = {
 		{ "help",          no_argument,       NULL, 'h'             },
@@ -316,6 +314,8 @@ int main(int argc, char *argv[])
 		{ "fork",          no_argument,       NULL, 'f'             },
 		{ "kill-child",    optional_argument, NULL, OPT_KILLCHILD   },
 		{ "mount-proc",    optional_argument, NULL, OPT_MOUNTPROC   },
+		{ "map-user",      required_argument, NULL, OPT_MAPUSER     },
+		{ "map-group",     required_argument, NULL, OPT_MAPGROUP    },
 		{ "map-root-user", no_argument,       NULL, 'r'             },
 		{ "map-current-user", no_argument,    NULL, 'c'             },
 		{ "propagation",   required_argument, NULL, OPT_PROPAGATION },
@@ -330,7 +330,9 @@ int main(int argc, char *argv[])
 
 	int setgrpcmd = SETGROUPS_NONE;
 	int unshare_flags = 0;
-	int c, forkit = 0, mapuser = MAP_USER_NONE;
+	int c, forkit = 0;
+	uid_t mapuser = -1;
+	gid_t mapgroup = -1;
 	int kill_child_signo = 0; /* 0 means --kill-child was not used */
 	const char *procmnt = NULL;
 	const char *newroot = NULL;
@@ -393,21 +395,23 @@ int main(int argc, char *argv[])
 			unshare_flags |= CLONE_NEWNS;
 			procmnt = optarg ? optarg : "/proc";
 			break;
+		case OPT_MAPUSER:
+			unshare_flags |= CLONE_NEWUSER;
+			mapuser = strtoul_or_err(optarg, _("failed to parse uid"));
+			break;
+		case OPT_MAPGROUP:
+			unshare_flags |= CLONE_NEWUSER;
+			mapgroup = strtoul_or_err(optarg, _("failed to parse gid"));
+			break;
 		case 'r':
-			if (mapuser == MAP_USER_CURRENT)
-			        errx(EXIT_FAILURE, _("options --map-root-user and "
-					"--map-current-user are mutually exclusive"));
-
 			unshare_flags |= CLONE_NEWUSER;
-			mapuser = MAP_USER_ROOT;
+			mapuser = 0;
+			mapgroup = 0;
 			break;
 		case 'c':
-			if (mapuser == MAP_USER_ROOT)
-			        errx(EXIT_FAILURE, _("options --map-root-user and "
-					"--map-current-user are mutually exclusive"));
-
 			unshare_flags |= CLONE_NEWUSER;
-			mapuser = MAP_USER_CURRENT;
+			mapuser = real_euid;
+			mapgroup = real_egid;
 			break;
 		case OPT_SETGROUPS:
 			setgrpcmd = setgroups_str2id(optarg);
@@ -508,33 +512,23 @@ int main(int argc, char *argv[])
 	if (kill_child_signo != 0 && prctl(PR_SET_PDEATHSIG, kill_child_signo) < 0)
 		err(EXIT_FAILURE, "prctl failed");
 
+        if (mapuser != (uid_t) -1)
+		map_id(_PATH_PROC_UIDMAP, mapuser, real_euid);
+
         /* Since Linux 3.19 unprivileged writing of /proc/self/gid_map
          * has been disabled unless /proc/self/setgroups is written
          * first to permanently disable the ability to call setgroups
          * in that user namespace. */
-        switch (mapuser) {
-        case MAP_USER_ROOT:
+	if (mapgroup != (gid_t) -1) {
 		if (setgrpcmd == SETGROUPS_ALLOW)
 			errx(EXIT_FAILURE, _("options --setgroups=allow and "
-					"--map-root-user are mutually exclusive"));
-
+					"--map-group are mutually exclusive"));
 		setgroups_control(SETGROUPS_DENY);
-		map_id(_PATH_PROC_UIDMAP, 0, real_euid);
-		map_id(_PATH_PROC_GIDMAP, 0, real_egid);
-                break;
-        case MAP_USER_CURRENT:
-		if (setgrpcmd == SETGROUPS_ALLOW)
-			errx(EXIT_FAILURE, _("options --setgroups=allow and "
-					"--map-current-user are mutually exclusive"));
+		map_id(_PATH_PROC_GIDMAP, mapgroup, real_egid);
+	}
 
-		setgroups_control(SETGROUPS_DENY);
-		map_id(_PATH_PROC_UIDMAP, real_euid, real_euid);
-		map_id(_PATH_PROC_GIDMAP, real_egid, real_egid);
-                break;
-        case MAP_USER_NONE:
-	        if (setgrpcmd != SETGROUPS_NONE)
-		        setgroups_control(setgrpcmd);
-        }
+	if (setgrpcmd != SETGROUPS_NONE)
+		setgroups_control(setgrpcmd);
 
 	if ((unshare_flags & CLONE_NEWNS) && propagation)
 		set_propagation(propagation);
-- 
2.20.1

@karelzak
Copy link
Collaborator

karelzak commented Jan 6, 2020

@mat8913 it's too late for the currect release, but I'll think abou tit for the next v2.36. Thanks!

@karelzak karelzak added NEXT-RELEASE TODO We going to think about it ;-) labels Jan 6, 2020
@zokrezyl
Copy link
Author

Thanks @mat8913 for providing the patch! I believe you bumped into the same little limitation in similar use-cases. This will allow us to use unshare for more complex namespace setup in scripts without elevated privileges.

@zokrezyl
Copy link
Author

@karelzak feel free to close the issue, especially if you included into your next release. Please CC me here. Thanks

mat8913 added a commit to mat8913/util-linux that referenced this issue Apr 12, 2020
Two new options are added: `--map-user=<uid>` and `--map-group=<gid>`
for custom user and group mappings respectively. These are just
generalizations of the existing `--map-root-user` and
`--map-current-user` options.

As a side effect of this commit, specifying both `--map-root-user` and
`--map-current-user` no longer causes an error. Instead, the last
occurrence takes precedence.

Addresses: util-linux#885
Signed-off-by: Matthew Harm Bekkema <id@mbekkema.name>
@mat8913
Copy link
Contributor

mat8913 commented Apr 12, 2020

Thanks for reminding me. I made a new PR (#1005) because the current patch has conflicts.

@karelzak
Copy link
Collaborator

OK, closing. Let's continue in @mat8913's PR #1005.

@zokrezyl
Copy link
Author

Thanks for both of you for your time.

I had some further thoughts:
While the solution would cover lot of use-cases, it wont cover some of them and those once would discard the usefulness of the current solution with two invocation. Such a use-case would look like:

-> Run first invocation with at least new mount ns
-> Mount a new root, switch to that root
-> Run second invocation (well, from where?)
-> Run something under that root

So unshare won't be available for the second invocation in such usecases. It feels like: I created o tool for you but I don't give it to you :)

Well, little dirty solution would be to mount 'unshare' under some directory in the new mount ns, but you would need to mount it's compatible libc etc.

Having this in mind, I believe that the original proposal with two unshare syscalls in one process (invocation) is way much more powerful and still implementing .

Obviously I am not insisting, just wanted to let you know about the limitations.

@zokrezyl
Copy link
Author

@karelzak @mat8913 opinion about the above observation?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
TODO We going to think about it ;-)
Projects
None yet
Development

No branches or pull requests

3 participants