newuidmap and newgidmap: support passing pid as fd

Closes #635

newuidmap and newgidmap currently take an integner pid as
the first argument, determining the process id on which to
act.  Accept also "fd:N", where N must be an open file
descriptor to the /proc/pid directory for the process to
act upon.  This way, if you

exec 10</proc/99
newuidmap fd:10 100000 0 65536

and pid 99 dies and a new process happens to take pid 99 before
newuidmap happens to do its work, then since newuidmap will use
openat() using fd 10, it won't change the mapping for the new
process.

Example:

// terminal 1:
serge@jerom ~/src/nsexec$ ./nsexec -W -s 0 -S 0 -U
about to unshare with 10000000
Press any key to exec (I am 129176)

// terminal 2:
serge@jerom ~/src/shadow$ exec 10</proc/129176
serge@jerom ~/src/shadow$ sudo chown root src/newuidmap src/newgidmap
serge@jerom ~/src/shadow$ sudo chmod u+s src/newuidmap
serge@jerom ~/src/shadow$ sudo chmod u+s src/newgidmap
serge@jerom ~/src/shadow$ ./src/newuidmap fd:10 0 100000 10
serge@jerom ~/src/shadow$ ./src/newgidmap fd:10 0 100000 10

// Terminal 1:
uid=0(root) gid=0(root) groups=0(root)

Signed-off-by: Serge Hallyn <serge@hallyn.com>

diff --git a/lib/get_pid.c b/lib/get_pid.c
index 10184bf0adcb891813a5b76cc85bcdea71091e16..ab91d1585a09cc347a460a0019e88f27876c0804 100644 (file)
--- a/lib/get_pid.c
+++ b/lib/get_pid.c
@@ -10,6 +10,9 @@
 
 #include "prototypes.h"
 #include "defines.h"
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <fcntl.h>
 
 int get_pid (const char *pidstr, pid_t *pid)
 {
@@ -29,3 +32,51 @@ int get_pid (const char *pidstr, pid_t *pid)
        return 1;
 }
 
+/*
+ * If use passed in fd:4 as an argument, then return the
+ * value '4', the fd to use.
+ */
+int get_pidfd_from_fd(const char *pidfdstr)
+{
+       long long int val;
+       char *endptr;
+
+       errno = 0;
+       val = strtoll (pidfdstr, &endptr, 10);
+       if (   ('\0' == *pidfdstr)
+           || ('\0' != *endptr)
+           || (ERANGE == errno)
+           || (/*@+longintegral@*/val != (pid_t)val)/*@=longintegral@*/) {
+               return 0;
+       }
+
+       return (int)val;
+}
+
+int open_pidfd(const char *pidstr)
+{
+       int proc_dir_fd;
+       int written;
+       char proc_dir_name[32];
+       pid_t target;
+
+       if (get_pid(pidstr, &target) == 0)
+               return -ENOENT;
+
+       /* max string length is 6 + 10 + 1 + 1 = 18, allocate 32 bytes */
+       written = snprintf(proc_dir_name, sizeof(proc_dir_name), "/proc/%u/",
+               target);
+       if ((written <= 0) || ((size_t)written >= sizeof(proc_dir_name))) {
+               fprintf(stderr, "snprintf of proc path failed for %u: %s\n",
+                       target, strerror(errno));
+               return -EINVAL;
+       }
+
+       proc_dir_fd = open(proc_dir_name, O_DIRECTORY);
+       if (proc_dir_fd < 0) {
+               fprintf(stderr, _("Could not open proc directory for target %u: %s\n"),
+                       target, strerror(errno));
+               return -EINVAL;
+       }
+       return proc_dir_fd;
+}

diff --git a/lib/prototypes.h b/lib/prototypes.h
index 400d5b97b8852ee8a9f0fc00c8e22e6016ace3e9..21df6f615c89c5cdb7a53b9cb5e859c8e637f018 100644 (file)
--- a/lib/prototypes.h
+++ b/lib/prototypes.h
@@ -160,6 +160,8 @@ extern int getlong (const char *numstr, /*@out@*/long int *result);
 
 /* get_pid.c */
 extern int get_pid (const char *pidstr, pid_t *pid);
+extern int get_pidfd_from_fd(const char *pidfdstr);
+extern int open_pidfd(const char *pidstr);
 
 /* getrange */
 extern int getrange (const char *range,

diff --git a/man/newgidmap.1.xml b/man/newgidmap.1.xml
index e4ebc69e80f0e68fa6cf6e0df0e6d0bd3f2e4506..9b7683ebae708a903e466a6c93b66a16ada77739 100644 (file)
--- a/man/newgidmap.1.xml
+++ b/man/newgidmap.1.xml
@@ -116,6 +116,17 @@
     <para>
       Note that newgidmap may be used only once for a given process.
     </para>
+    <para>
+      Instead of an integer process id, the first argument may be
+      specified as <replaceable>fd:N</replaceable>, where the integer N
+      is the file descriptor number for the calling process's opened
+      file for <filename>/proc/[pid[</filename>.  In this case,
+      <command>newgidmap</command> will use
+      <refentrytitle>openat</refentrytitle><manvolnum>2</manvolnum>
+      to open the <filename>gid_map</filename> file under that
+      directory, avoiding a TOCTTOU in case the process exits and
+      the pid is immediately reused.
+    </para>
 
   </refsect1>
 

diff --git a/man/newuidmap.1.xml b/man/newuidmap.1.xml
index f5cb5b484c8bb8a74844ef309b9c5ad6c5db4c52..ca917a7725ae51f0c0f741e0defb5c993a4e9ce7 100644 (file)
--- a/man/newuidmap.1.xml
+++ b/man/newuidmap.1.xml
@@ -116,6 +116,17 @@
     <para>
       Note that newuidmap may be used only once for a given process.
     </para>
+    <para>
+      Instead of an integer process id, the first argument may be
+      specified as <replaceable>fd:N</replaceable>, where the integer N
+      is the file descriptor number for the calling process's opened
+      file for <filename>/proc/[pid[</filename>.  In this case,
+      <command>newuidmap</command> will use
+      <refentrytitle>openat</refentrytitle><manvolnum>2</manvolnum>
+      to open the <filename>uid_map</filename> file under that
+      directory, avoiding a TOCTTOU in case the process exits and
+      the pid is immediately reused.
+    </para>
   </refsect1>
 
   <refsect1 id='options'>

diff --git a/src/newgidmap.c b/src/newgidmap.c
index 01d0fe90a85cca11b812661e2e4fecfcbcde7500..d6d29725ca371b685a62298349cf3cfec6d814f7 100644 (file)
--- a/src/newgidmap.c
+++ b/src/newgidmap.c
@@ -69,7 +69,7 @@ static void verify_ranges(struct passwd *pw, int ranges,
 
 static void usage(void)
 {
-       fprintf(stderr, _("usage: %s <pid> <gid> <lowergid> <count> [ <gid> <lowergid> <count> ] ... \n"), Prog);
+       fprintf(stderr, _("usage: %s [<pid|fd:<pidfd>] <gid> <lowergid> <count> [ <gid> <lowergid> <count> ] ... \n"), Prog);
        exit(EXIT_FAILURE);
 }
 
@@ -143,15 +143,12 @@ out:
  */
 int main(int argc, char **argv)
 {
-       char proc_dir_name[32];
        char *target_str;
-       pid_t target;
        int proc_dir_fd;
        int ranges;
        struct map_range *mappings;
        struct stat st;
        struct passwd *pw;
-       int written;
        bool allow_setgroups = false;
 
        Prog = Basename (argv[0]);
@@ -168,25 +165,19 @@ int main(int argc, char **argv)
        /* Find the process that needs its user namespace
         * gid mapping set.
         */
-       target_str = argv[1];
-       if (!get_pid(target_str, &target))
-               usage();
 
-       /* max string length is 6 + 10 + 1 + 1 = 18, allocate 32 bytes */
-       written = snprintf(proc_dir_name, sizeof(proc_dir_name), "/proc/%u/",
-               target);
-       if ((written <= 0) || ((size_t)written >= sizeof(proc_dir_name))) {
-               fprintf(stderr, "%s: snprintf of proc path failed: %s\n",
-                       Prog, strerror(errno));
-       }
-
-       proc_dir_fd = open(proc_dir_name, O_DIRECTORY);
-       if (proc_dir_fd < 0) {
-               fprintf(stderr, _("%s: Could not open proc directory for target %u\n"),
-                       Prog, target);
-               return EXIT_FAILURE;
+       target_str = argv[1];
+       if (strlen(target_str) > 3 && strncmp(target_str, "fd:", 3) == 0) {
+               /* the user passed in a /proc/pid fd for the process */
+               target_str = &target_str[3];
+               proc_dir_fd = get_pidfd_from_fd(target_str);
+               if (proc_dir_fd < 0)
+                       usage();
+       } else {
+               proc_dir_fd = open_pidfd(target_str);
+               if (proc_dir_fd < 0)
+                       usage();
        }
-
        /* Who am i? */
        pw = get_my_pwent ();
        if (NULL == pw) {
@@ -200,8 +191,8 @@ int main(int argc, char **argv)
 
        /* Get the effective uid and effective gid of the target process */
        if (fstat(proc_dir_fd, &st) < 0) {
-               fprintf(stderr, _("%s: Could not stat directory for target %u\n"),
-                       Prog, target);
+               fprintf(stderr, _("%s: Could not stat directory for process\n"),
+                       Prog);
                return EXIT_FAILURE;
        }
 
@@ -213,8 +204,8 @@ int main(int argc, char **argv)
            (!getdef_bool("GRANT_AUX_GROUP_SUBIDS") && (getgid() != pw->pw_gid)) ||
            (pw->pw_uid != st.st_uid) ||
            (getgid() != st.st_gid)) {
-               fprintf(stderr, _( "%s: Target %u is owned by a different user: uid:%lu pw_uid:%lu st_uid:%lu, gid:%lu pw_gid:%lu st_gid:%lu\n" ),
-                       Prog, target,
+               fprintf(stderr, _( "%s: Target process is owned by a different user: uid:%lu pw_uid:%lu st_uid:%lu, gid:%lu pw_gid:%lu st_gid:%lu\n" ),
+                       Prog,
                        (unsigned long int)getuid(), (unsigned long int)pw->pw_uid, (unsigned long int)st.st_uid,
                        (unsigned long int)getgid(), (unsigned long int)pw->pw_gid, (unsigned long int)st.st_gid);
                return EXIT_FAILURE;

diff --git a/src/newuidmap.c b/src/newuidmap.c
index e8798409bc042c36a9feb944997be23c6b559d47..e99655c9282e98708f45f2b4fbcf743d509f7e0e 100644 (file)
--- a/src/newuidmap.c
+++ b/src/newuidmap.c
@@ -64,7 +64,7 @@ static void verify_ranges(struct passwd *pw, int ranges,
 
 static void usage(void)
 {
-       fprintf(stderr, _("usage: %s <pid> <uid> <loweruid> <count> [ <uid> <loweruid> <count> ] ... \n"), Prog);
+       fprintf(stderr, _("usage: %s [<pid>|fd:<pidfd>] <uid> <loweruid> <count> [ <uid> <loweruid> <count> ] ... \n"), Prog);
        exit(EXIT_FAILURE);
 }
 
@@ -73,15 +73,12 @@ static void usage(void)
  */
 int main(int argc, char **argv)
 {
-       char proc_dir_name[32];
        char *target_str;
-       pid_t target;
        int proc_dir_fd;
        int ranges;
        struct map_range *mappings;
        struct stat st;
        struct passwd *pw;
-       int written;
 
        Prog = Basename (argv[0]);
        log_set_progname(Prog);
@@ -94,26 +91,20 @@ int main(int argc, char **argv)
        if (argc < 2)
                usage();
 
+       target_str = argv[1];
        /* Find the process that needs its user namespace
         * uid mapping set.
         */
-       target_str = argv[1];
-       if (!get_pid(target_str, &target))
-               usage();
-
-       /* max string length is 6 + 10 + 1 + 1 = 18, allocate 32 bytes */
-       written = snprintf(proc_dir_name, sizeof(proc_dir_name), "/proc/%u/",
-               target);
-       if ((written <= 0) || ((size_t)written >= sizeof(proc_dir_name))) {
-               fprintf(stderr, "%s: snprintf of proc path failed: %s\n",
-                       Prog, strerror(errno));
-       }
-
-       proc_dir_fd = open(proc_dir_name, O_DIRECTORY);
-       if (proc_dir_fd < 0) {
-               fprintf(stderr, _("%s: Could not open proc directory for target %u\n"),
-                       Prog, target);
-               return EXIT_FAILURE;
+       if (strlen(target_str) > 3 && strncmp(target_str, "fd:", 3) == 0) {
+               /* the user passed in a /proc/pid fd for the process */
+               target_str = &target_str[3];
+               proc_dir_fd = get_pidfd_from_fd(target_str);
+               if (proc_dir_fd < 0)
+                       usage();
+       } else {
+               proc_dir_fd = open_pidfd(target_str);
+               if (proc_dir_fd < 0)
+                       usage();
        }
 
        /* Who am i? */
@@ -129,8 +120,7 @@ int main(int argc, char **argv)
 
        /* Get the effective uid and effective gid of the target process */
        if (fstat(proc_dir_fd, &st) < 0) {
-               fprintf(stderr, _("%s: Could not stat directory for target %u\n"),
-                       Prog, target);
+               fprintf(stderr, _("%s: Could not stat directory for target process\n"), Prog);
                return EXIT_FAILURE;
        }
 
@@ -142,8 +132,8 @@ int main(int argc, char **argv)
            (!getdef_bool("GRANT_AUX_GROUP_SUBIDS") && (getgid() != pw->pw_gid)) ||
            (pw->pw_uid != st.st_uid) ||
            (getgid() != st.st_gid)) {
-               fprintf(stderr, _( "%s: Target process %u is owned by a different user: uid:%lu pw_uid:%lu st_uid:%lu, gid:%lu pw_gid:%lu st_gid:%lu\n" ),
-                       Prog, target,
+               fprintf(stderr, _( "%s: Target process is owned by a different user: uid:%lu pw_uid:%lu st_uid:%lu, gid:%lu pw_gid:%lu st_gid:%lu\n" ),
+                       Prog,
                        (unsigned long int)getuid(), (unsigned long int)pw->pw_uid, (unsigned long int)st.st_uid,
                        (unsigned long int)getgid(), (unsigned long int)pw->pw_gid, (unsigned long int)st.st_gid);
                return EXIT_FAILURE;