Re: [PATCH] chroot specify user/group feature

[Giuseppe Scrivano]

Thank you Jim, Pádraig and Andreas for all your suggestions.  I took all
of them in consideration.  I temporary lent some code from setuidgid.c
and the additional groups are allocated dinamically.

I think that in the future the `set_additional_groups' function should
be moved in a separate library, so it can be shared with setuidgid that
at the moment doesn't support group names but only group IDs and this is
what setuidgid will get back.

Cheers,
Giuseppe



>From 0b305c281fc7575b3989b0f80fe452918cf93f62 Mon Sep 17 00:00:00 2001
From: Giuseppe Scrivano <address@hidden>
Date: Fri, 1 May 2009 23:50:11 +0200
Subject: [PATCH] Chroot now accepts the options --userspec and --groups.

* NEWS: Added note about the new chroot features.
* doc/coreutils.texi: Likewise.
* src/chroot.c (main): Added support for --userspec and --groups.
* tests/Makefile.am: Added tests for chroot.
* tests/chroot/credentials: Likewise.
* tests/test-lib.sh: Likewise.
---
 NEWS                     |    5 ++
 doc/coreutils.texi       |   29 +++++++++-
 src/chroot.c             |  133 ++++++++++++++++++++++++++++++++++++++++++++-
 tests/Makefile.am        |    1 +
 tests/chroot/credentials |   43 +++++++++++++++
 tests/test-lib.sh        |    1 +
 6 files changed, 206 insertions(+), 6 deletions(-)
 create mode 100644 tests/chroot/credentials

diff --git a/NEWS b/NEWS
index 31f1b1a..3af06e4 100644
--- a/NEWS
+++ b/NEWS
@@ -7,6 +7,11 @@ GNU coreutils NEWS                                    -*- 
outline -*-
   truncate -s failed to skip all whitespace in the option argument in
   some locales.
 
+** New features
+
+  chroot now accepts the options --userspec and --groups.
+
+
 * Noteworthy changes in release 7.4 (2009-05-07) [stable]
 
 ** Bug fixes
diff --git a/doc/coreutils.texi b/doc/coreutils.texi
index b96fdb2..1963c74 100644
--- a/doc/coreutils.texi
+++ b/doc/coreutils.texi
@@ -14144,7 +14144,7 @@ underlying function is non-privileged due to lack of 
support in MS-Windows.}
 Synopses:
 
 @example
-chroot @var{newroot} address@hidden address@hidden@dots{}]
+chroot @var{option} @var{newroot} address@hidden address@hidden@dots{}]
 chroot @var{option}
 @end example
 
@@ -14157,8 +14157,31 @@ variable or @command{/bin/sh} if not set, invoked with 
the @option{-i} option.
 @var{command} must not be a special built-in utility
 (@pxref{Special built-in utilities}).
 
-The only options are @option{--help} and @option{--version}.  @xref{Common
-options}.  Options must precede operands.
+The program accepts the following options.  Also see @ref{Common options}.
+Options must precede operands.
+
address@hidden @samp
+
address@hidden address@hidden
address@hidden --userspec
+Use the @var{userspec} user and group in the new environment.
address@hidden is given in the form @var{user}:@var{group}, the group
+can be omitted and in this case the original one will be used.
+
address@hidden address@hidden
address@hidden --groups
+Specify an additional set of groups @var{groups} to be used by the
+chroot process.
+
address@hidden table
+
+By default, @command{chroot} retains the user credentials---usually those
+of the super-user---in the new environment.
+You can modify this behavior via the @option{--userspec=USER:GROUP} option,
+to specify the desired user and/or primary group.
+Supplementary groups can be configured using the @option{--groups=group_list}
+option.  If either @option{--userspec} or @option{--groups} is omitted,
+then the original values are kept.
 
 Here are a few tips to help avoid common problems in using chroot.
 To start with a simple example, make @var{command} refer to a statically
diff --git a/src/chroot.c b/src/chroot.c
index 6d3fddf..3e0b30d 100644
--- a/src/chroot.c
+++ b/src/chroot.c
@@ -21,17 +21,89 @@
 #include <getopt.h>
 #include <stdio.h>
 #include <sys/types.h>
+#include <grp.h>
 
 #include "system.h"
 #include "error.h"
 #include "long-options.h"
 #include "quote.h"
+#include "userspec.h"
+#include "xstrtol.h"
+#include "xalloc.h"
 
 /* The official name of this program (e.g., no `g' prefix).  */
 #define PROGRAM_NAME "chroot"
 
 #define AUTHORS proper_name ("Roland McGrath")
 
+#ifndef MAXGID
+#define MAXGID GID_T_MAX
+#endif
+
+enum
+{
+  USERSPEC = UCHAR_MAX + 1,
+  GROUPS
+};
+
+static struct option const long_opts[] =
+{
+  {"userspec", required_argument, NULL, USERSPEC},
+  {"groups", required_argument, NULL, GROUPS},
+  {GETOPT_HELP_OPTION_DECL},
+  {GETOPT_VERSION_OPTION_DECL},
+  {NULL, 0, NULL, 0}
+};
+
+/* Groups is a comma separated list of additional groups.  */
+static int
+set_additional_groups (char const *groups)
+{
+  GETGROUPS_T *gids = NULL;
+  size_t n_gids_allocated = 0;
+  size_t n_gids = 0;
+  char *buffer = xstrdup (groups);
+  char const *tmp;
+  int ret;
+
+  for (tmp = strtok (buffer, ","); tmp; tmp = strtok (NULL, ","))
+    {
+      struct group *g;
+      unsigned long int value;
+
+      if (xstrtoul (tmp, NULL, 10, &value, "") == LONGINT_OK && value <= 
MAXGID)
+        {
+          g = getgrgid (value);
+        }
+      else
+        {
+          g = getgrnam (tmp);
+          if (g != NULL)
+            value = g->gr_gid;
+        }
+
+      if (g == NULL)
+        {
+          error (0, errno, _("cannot find group %s"), tmp);
+          free (buffer);
+          return 1;
+        }
+
+      if (n_gids == n_gids_allocated)
+        gids = x2nrealloc (gids, &n_gids_allocated, sizeof *gids);
+      gids[n_gids++] = value;
+    }
+
+  free (buffer);
+
+  ret = setgroups (n_gids, gids);
+
+  free (gids);
+
+  return ret;
+}
+
+
 void
 usage (int status)
 {
@@ -41,13 +113,20 @@ usage (int status)
   else
     {
       printf (_("\
-Usage: %s NEWROOT [COMMAND [ARG]...]\n\
+Usage: %s [OPTION] NEWROOT [COMMAND [ARG]...]\n\
   or:  %s OPTION\n\
 "), program_name, program_name);
+
       fputs (_("\
 Run COMMAND with root directory set to NEWROOT.\n\
 \n\
 "), stdout);
+
+      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\
+"), stdout);
+
       fputs (HELP_OPTION_DESCRIPTION, stdout);
       fputs (VERSION_OPTION_DESCRIPTION, stdout);
       fputs (_("\
@@ -62,6 +141,10 @@ If no command is given, run ``${SHELL} -i'' (default: 
/bin/sh).\n\
 int
 main (int argc, char **argv)
 {
+  int c;
+  char const *userspec = NULL;
+  char const *groups = NULL;
+
   initialize_main (&argc, &argv);
   set_program_name (argv[0]);
   setlocale (LC_ALL, "");
@@ -73,8 +156,21 @@ main (int argc, char **argv)
 
   parse_long_options (argc, argv, PROGRAM_NAME, PACKAGE_NAME, Version,
                      usage, AUTHORS, (char const *) NULL);
-  if (getopt_long (argc, argv, "+", NULL, NULL) != -1)
-    usage (EXIT_FAILURE);
+
+  while ((c = getopt_long (argc, argv, "+", long_opts, NULL)) != -1)
+    {
+      switch (c)
+        {
+        case USERSPEC:
+          userspec = optarg;
+          break;
+        case GROUPS:
+          groups = optarg;
+          break;
+        default:
+          usage (EXIT_FAILURE);
+        }
+    }
 
   if (argc <= optind)
     {
@@ -105,6 +201,37 @@ main (int argc, char **argv)
       argv += optind + 1;
     }
 
+  if (userspec)
+    {
+      uid_t uid;
+      gid_t gid;
+      char *user;
+      char *group;
+      char const *err = parse_user_spec (userspec, &uid, &gid, &user, &group);
+
+      if (err)
+        error (EXIT_FAILURE, errno, err);
+
+      free (user);
+      free (group);
+
+      if (groups)
+        {
+          if (set_additional_groups (groups))
+            error (0, errno, _("cannot set additional groups"));
+        }
+
+      if (gid && setgid (gid))
+        {
+          error (0, errno, _("cannot setgid"));
+        }
+
+      if (uid && setuid (uid))
+        {
+          error (0, errno, _("cannot setgid"));
+        }
+    }
+
   /* Execute the given command.  */
   execvp (argv[0], argv);
 
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 7fe74c0..a0ed986 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -24,6 +24,7 @@ root_tests =                                  \
   cp/preserve-gid                              \
   cp/special-bits                              \
   cp/cp-mv-enotsup-xattr                       \
+  chroot/credentials                           \
   dd/skip-seek-past-dev                                \
   install/install-C-root                       \
   ls/capability                                        \
diff --git a/tests/chroot/credentials b/tests/chroot/credentials
new file mode 100644
index 0000000..23d66bd
--- /dev/null
+++ b/tests/chroot/credentials
@@ -0,0 +1,43 @@
+#!/bin/sh
+# Verify that the credentials are changed correctly.
+
+# Copyright (C) 2009 Free Software Foundation, Inc.
+
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+
+if test "$VERBOSE" = yes; then
+  set -x
+  chroot --version
+fi
+
+. $srcdir/test-lib.sh
+
+require_root_
+
+fail=0
+
+# Verify that root credentials are kept.
+test $(chroot / whoami) = root || fail=1
+test "$(groups)" = "$(chroot / groups)" || fail=1
+
+# Verify that credentials are changed correctly.
+test "$(chroot --userspec=$NON_ROOT_USERNAME:$NON_ROOT_GROUP / whoami)" != 
root \
+    || fail=1
+
+# Verify that there are no additional groups.
+test "$(chroot --userspec=$NON_ROOT_USERNAME:$NON_ROOT_GROUP --groups= / id 
-nG)"\
+    = $NON_ROOT_GROUP || fail=1
+
+Exit $fail
diff --git a/tests/test-lib.sh b/tests/test-lib.sh
index 17a3538..a765bd6 100644
--- a/tests/test-lib.sh
+++ b/tests/test-lib.sh
@@ -204,6 +204,7 @@ require_root_()
 {
   uid_is_privileged_ || skip_test_ "must be run as root"
   NON_ROOT_USERNAME=${NON_ROOT_USERNAME=nobody}
+  NON_ROOT_GROUP=${NON_ROOT_GROUP=nogroup}
 }
 
 skip_if_root_() { uid_is_privileged_ && skip_test_ "must be run as non-root"; }
-- 
1.6.2.4