[PATCH] chroot: add --before to get UID/GID before chroot

[norihiro]

I have a feature request for chroot.

An option --userspec did not work with a chroot jail created from scratch,
even if it has /etc/passwd and /etc/group and it says:
chroot: invalid user
The option --userspec requires many files to retrieve UID/GID.
I suggest an option --before to retrieve UID/GID before chroot.
With the option --before, UID/GID is retrieved from old root.

I attached a patch to add the option --before.
I split an old function set_additional_groups into 2 functions
get_additional_groups and set_additional_groups.
With --before, get_additional_groups and/or parse_user_spec will
be called before chroot and store UID/GIDs to local variables
in a function main. Without --before, get_additional_groups will 
be called just before set_additional_groups and parse_user_spec
will be called in same order to that of previous.

Thanks.

>From 38c0c5256c21a00de5e1f77a394fcf4af05c10e7 Mon Sep 17 00:00:00 2001
From: Kamae Norihiro <address@hidden>
Date: Wed, 20 Jul 2011 01:40:29 +0900
Subject: [PATCH] chroot: add --before to get UID/GID before chroot

If you use chroot jail which has no account information,
chroot with --userspec or --groups will fail with
error invalid user.
With --before, UID/GID will be retrieved before chroot
to get account informations from old root.
* src/chroot.c: add option --before
* NEWS: mention about the option
* doc/coreutils.texi: describe it
---
 NEWS               |    6 ++++
 doc/coreutils.texi |    7 +++++
 src/chroot.c       |   74 ++++++++++++++++++++++++++++++++++++++++++----------
 3 files changed, 73 insertions(+), 14 deletions(-)

diff --git a/NEWS b/NEWS
index 61e6e63..84cb141 100644
--- a/NEWS
+++ b/NEWS
@@ -41,6 +41,12 @@ GNU coreutils NEWS                                    -*- 
outline -*-
   directly from a shell prompt, where the command is interactive or needs to
   receive signals initiated from the terminal.
 
+  chroot accepts a new --before option. With --userspec and/or --groups and
+  this option,  UID/GID will be determined before chroot().  For example,
+  with a small chroot jail without account information files,  this option
+  enables the option --userspec or --groups to work with an account before
+  chroot call.
+
 ** Improvements
 
   shuf outputs small subsets of large permutations much more efficiently.
diff --git a/doc/coreutils.texi b/doc/coreutils.texi
index 424446c..9bf59fb 100644
--- a/doc/coreutils.texi
+++ b/doc/coreutils.texi
@@ -15004,6 +15004,13 @@ Use this option to specify the supplementary 
@var{groups} to be
 used by the new process.
 The items in the list (names or numeric IDs) must be separated by commas.
 
+@itemx --before
+@opindex --before
+By default, option @option{--userspec} or @option{--groups}
+retrieves UID or GID from @var{newroot}.
+Use this option
+to retrieve UID or GID from old root.
+
 @end table
 
 Here are a few tips to help avoid common problems in using chroot.
diff --git a/src/chroot.c b/src/chroot.c
index 95c227b..e068fbf 100644
--- a/src/chroot.c
+++ b/src/chroot.c
@@ -40,13 +40,15 @@
 enum
 {
   GROUPS = UCHAR_MAX + 1,
-  USERSPEC
+  USERSPEC,
+  BEFORE
 };
 
 static struct option const long_opts[] =
 {
   {"groups", required_argument, NULL, GROUPS},
   {"userspec", required_argument, NULL, USERSPEC},
+  {"before", no_argument, NULL, BEFORE},
   {GETOPT_HELP_OPTION_DECL},
   {GETOPT_VERSION_OPTION_DECL},
   {NULL, 0, NULL, 0}
@@ -58,7 +60,7 @@ static struct option const long_opts[] =
    resulting numbers.  Upon any failure give a diagnostic and return nonzero.
    Otherwise return zero.  */
 static int
-set_additional_groups (char const *groups)
+get_additional_groups (char const *groups, size_t *n_gids_ret, GETGROUPS_T 
**gids_ret)
 {
   GETGROUPS_T *gids = NULL;
   size_t n_gids_allocated = 0;
@@ -93,6 +95,17 @@ set_additional_groups (char const *groups)
       gids[n_gids++] = value;
     }
 
+  *n_gids_ret = n_gids;
+  *gids_ret = gids;
+
+  free (buffer);
+  return ret;
+}
+
+static int
+set_additional_groups (char const *groups, int n_gids, GETGROUPS_T *gids)
+{
+  int ret = 0;
   if (ret == 0 && n_gids == 0)
     {
       error (0, 0, _("invalid group list %s"), quote (groups));
@@ -106,7 +119,6 @@ set_additional_groups (char const *groups)
         error (0, errno, _("failed to set additional groups"));
     }
 
-  free (buffer);
   free (gids);
   return ret;
 }
@@ -132,6 +144,7 @@ Run COMMAND with root directory set to NEWROOT.\n\
       fputs (_("\
   --userspec=USER:GROUP  specify user and group (ID or name) to use\n\
   --groups=G_LIST        specify supplementary groups as g1,g2,..,gN\n\
+  --before               UID or GID is determined before chroot\n\
 "), stdout);
 
       fputs (HELP_OPTION_DESCRIPTION, stdout);
@@ -151,6 +164,11 @@ main (int argc, char **argv)
   int c;
   char const *userspec = NULL;
   char const *groups = NULL;
+  bool before = false;
+  uid_t uid = -1;
+  gid_t gid = -1;
+  int n_gids;
+  GETGROUPS_T *gids = NULL;
 
   initialize_main (&argc, &argv);
   set_program_name (argv[0]);
@@ -173,6 +191,10 @@ main (int argc, char **argv)
           groups = optarg;
           break;
 
+        case BEFORE:
+          before = true;
+          break;
+
         case_GETOPT_HELP_CHAR;
 
         case_GETOPT_VERSION_CHAR (PROGRAM_NAME, AUTHORS);
@@ -188,6 +210,24 @@ main (int argc, char **argv)
       usage (EXIT_CANCELED);
     }
 
+  if (before)
+    {
+      if (userspec)
+        {
+          char *user;
+          char *group;
+          char const *err = parse_user_spec (userspec, &uid, &gid, &user, 
&group);
+
+          if (err)
+            error (EXIT_CANCELED, errno, "%s", err);
+
+          free (user);
+          free (group);
+        }
+      if (groups && get_additional_groups (groups, &n_gids, &gids))
+        exit (EXIT_CANCELED);
+    }
+
   if (chroot (argv[optind]) != 0)
     error (EXIT_CANCELED, errno, _("cannot change root directory to %s"),
            argv[optind]);
@@ -217,19 +257,22 @@ main (int argc, char **argv)
      Diagnose any failures.  If any have failed, exit before execvp.  */
   if (userspec)
     {
-      uid_t uid = -1;
-      gid_t gid = -1;
-      char *user;
-      char *group;
-      char const *err = parse_user_spec (userspec, &uid, &gid, &user, &group);
+      if (!before)
+        {
+          char *user;
+          char *group;
+          char const *err = parse_user_spec (userspec, &uid, &gid, &user, 
&group);
 
-      if (err)
-        error (EXIT_CANCELED, errno, "%s", err);
+          if (err)
+            error (EXIT_CANCELED, errno, "%s", err);
 
-      free (user);
-      free (group);
+          free (user);
+          free (group);
+          if (groups && get_additional_groups (groups, &n_gids, &gids))
+            fail = true;
+        }
 
-      if (groups && set_additional_groups (groups))
+      if (!fail && groups && set_additional_groups (groups, n_gids, gids))
         fail = true;
 
       if (gid != (gid_t) -1 && setgid (gid))
@@ -246,11 +289,14 @@ main (int argc, char **argv)
     }
   else
     {
+      if (!before && groups && get_additional_groups (groups, n_gids, gids))
+        fail = true;
+
       /* Yes, this call is identical to the one above.
          However, when --userspec and --groups groups are used together,
          we don't want to call this function until after parsing USER:GROUP,
          and it must be called before setuid.  */
-      if (groups && set_additional_groups (groups))
+      if (groups && set_additional_groups (groups, n_gids, gids))
         fail = true;
     }
 
-- 
1.7.4.4


-- 
Kamae Norihiro