bug-coreutils
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] chroot specify user/group feature


From: Jim Meyering
Subject: Re: [PATCH] chroot specify user/group feature
Date: Wed, 20 May 2009 16:32:36 +0200

Giuseppe Scrivano wrote:
> this is an updated version for the previous patch.  I added
> documentation and new tests.
>
> Since I don't use short-named options, there are not conflicts with -u,
> -g and -G used by different chroot implementations.
> In my first version -g has a different meaning than the -g on FreeBSD
> and OpenBSD; now I removed this option and I kept only the long-named
> version --groups.

Hi Giuseppe,

Almost there.
I've made some small changes with the patches below.
Please read through them, then squash into your own patch so you have
a single change-set, then address the FIXME I added, and compare your
code to what's done in the helper program, src/setuidgid.c, since it
performs a very similar task.

Also, when I started to compare, I noticed that you used NGROUPS,
while setuidgid.c allocates space dynamically.  I think we had problems
with early versions using NGROUPS.


>From 39f569d6f502096a5f9988ba8e378695a9b087c6 Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Wed, 20 May 2009 15:21:20 +0200
Subject: [PATCH 1/5] tweak help, const, formatting

---
 src/chroot.c |   23 ++++++++++++++---------
 1 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/src/chroot.c b/src/chroot.c
index ca9ac00..94aebf5 100644
--- a/src/chroot.c
+++ b/src/chroot.c
@@ -26,8 +26,8 @@
 #include "system.h"
 #include "error.h"
 #include "long-options.h"
-#include "userspec.h"
 #include "quote.h"
+#include "userspec.h"
 #include "xstrtol.h"

 #ifndef GID_T_MAX
@@ -44,7 +44,11 @@

 #define AUTHORS proper_name ("Roland McGrath")

-enum { USERSPEC = UCHAR_MAX + 1, GROUPS};
+enum
+  {
+    USERSPEC = UCHAR_MAX + 1,
+    GROUPS
+  };

 static struct option const long_opts[] =
 {
@@ -56,12 +60,13 @@ static struct option const long_opts[] =
 };

 /* Groups is a comma separated list of additional groups.  */
-static int set_additional_groups (const char *groups)
+static int
+set_additional_groups (char const *groups)
 {
   gid_t groups_id [NGROUPS];
   int ngroups = 0;
   char *buffer = xstrdup (groups);
-  const char *tmp;
+  char const *tmp;

   for (tmp = strtok (buffer, ","); tmp; tmp = strtok (NULL, ","))
     {
@@ -114,8 +119,8 @@ Run COMMAND with root directory set to NEWROOT.\n\
 "), stdout);

       fputs (_("\
-  --userspec,  specify the userspec in the form USER:GROUP\n\
-  --groups,  specify the additional groups as g1,g2,..,gn\n\
+  --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);
@@ -133,8 +138,8 @@ int
 main (int argc, char **argv)
 {
   int c;
-  const char *userspec = NULL;
-  const char *groups = NULL;
+  char const *userspec = NULL;
+  char const *groups = NULL;

   initialize_main (&argc, &argv);
   set_program_name (argv[0]);
@@ -197,7 +202,7 @@ main (int argc, char **argv)
       gid_t gid;
       char *user;
       char *group;
-      const char *err = parse_user_spec (userspec, &uid, &gid, &user, &group);
+      char const *err = parse_user_spec (userspec, &uid, &gid, &user, &group);
       if (err)
         {
           perror (err);
--
1.6.3.1.149.gbc70c


>From 1312b100431a9851a954ffa84b234fd371077941 Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Wed, 20 May 2009 15:31:21 +0200
Subject: [PATCH 2/5] size_t ngroups

---
 src/chroot.c |   35 ++++++++++++++++++-----------------
 1 files changed, 18 insertions(+), 17 deletions(-)

diff --git a/src/chroot.c b/src/chroot.c
index 94aebf5..7a61095 100644
--- a/src/chroot.c
+++ b/src/chroot.c
@@ -45,10 +45,10 @@
 #define AUTHORS proper_name ("Roland McGrath")

 enum
-  {
-    USERSPEC = UCHAR_MAX + 1,
-    GROUPS
-  };
+{
+  USERSPEC = UCHAR_MAX + 1,
+  GROUPS
+};

 static struct option const long_opts[] =
 {
@@ -63,8 +63,8 @@ static struct option const long_opts[] =
 static int
 set_additional_groups (char const *groups)
 {
-  gid_t groups_id [NGROUPS];
-  int ngroups = 0;
+  gid_t groups_id[NGROUPS];
+  size_t ngroups = 0;
   char *buffer = xstrdup (groups);
   char const *tmp;

@@ -153,18 +153,19 @@ main (int argc, char **argv)
   parse_long_options (argc, argv, PROGRAM_NAME, PACKAGE_NAME, Version,
                      usage, AUTHORS, (char const *) NULL);

-  while ((c = getopt_long (argc, argv, "+", long_opts, NULL))   != -1)
+  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);
-      }
+      switch (c)
+        {
+        case USERSPEC:
+          userspec = optarg;
+          break;
+        case GROUPS:
+          groups = optarg;
+          break;
+        default:
+          usage (EXIT_FAILURE);
+        }
     }

   if (argc <= optind)
--
1.6.3.1.149.gbc70c


>From b4774ce8725b0ac552658d2fa17b593c0f9335ca Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Wed, 20 May 2009 15:49:22 +0200
Subject: [PATCH 3/5] tweak documentation

---
 doc/coreutils.texi |   25 +++++++++++++++----------
 1 files changed, 15 insertions(+), 10 deletions(-)

diff --git a/doc/coreutils.texi b/doc/coreutils.texi
index e63dbc5..1cb2694 100644
--- a/doc/coreutils.texi
+++ b/doc/coreutils.texi
@@ -14134,6 +14134,10 @@ chroot invocation
 @pindex chroot
 @cindex running a program in a specified root directory
 @cindex root directory, running a program in a specified
address@hidden address@hidden
address@hidden --userspec
address@hidden address@hidden
address@hidden --groups

 @command{chroot} runs a command with a specified root directory.
 On many systems, only the super-user can do address@hidden,
@@ -14144,7 +14148,7 @@ chroot invocation
 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,17 +14161,18 @@ chroot invocation
 @var{command} must not be a special built-in utility
 (@pxref{Special built-in utilities}).

-Options accepted by chroot are @option{--help}, @option{--version},
address@hidden and @option{--groups}.  @xref{Common options}.
+The program accepts the following options.  Also see @ref{Common options}.
 Options must precede operands.

-If not specified differently, @command{chroot} keeps the user
-credentials in the new environment, that usually is the super-user.
-This behaviour can be changed by the @option{--userspec} option.  It
-allows to specify new credentials in the form @var{user:group}.
-Additional groups can be configured using the @option{--groups}
-option.  If any of @option{--userspec} or  @option{--groups} are not
-specified then the original values are kept.
address@hidden FIXME Add a @table of options here
+
+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
--
1.6.3.1.149.gbc70c


>From 78adc503433992525aaa38ae9e95c45ff3fe2add Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Wed, 20 May 2009 15:55:17 +0200
Subject: [PATCH 4/5] Makefile.am: adjust indentation

---
 tests/Makefile.am |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/tests/Makefile.am b/tests/Makefile.am
index c3746e2..a0ed986 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -24,7 +24,7 @@ root_tests =                                  \
   cp/preserve-gid                              \
   cp/special-bits                              \
   cp/cp-mv-enotsup-xattr                       \
-       chroot/credentials \
+  chroot/credentials                           \
   dd/skip-seek-past-dev                                \
   install/install-C-root                       \
   ls/capability                                        \
--
1.6.3.1.149.gbc70c


>From 3090b5a6627a106d395cc0bc31f1640b385ffcfb Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Wed, 20 May 2009 16:05:31 +0200
Subject: [PATCH 5/5] use $(...), rather than `...`

---
 tests/chroot/credentials |   10 ++++++----
 1 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/tests/chroot/credentials b/tests/chroot/credentials
index f3e7a32..fd87a86 100644
--- a/tests/chroot/credentials
+++ b/tests/chroot/credentials
@@ -29,13 +29,15 @@ require_root_
 fail=0

 # Verify that root credentials are kept.
-test `chroot / whoami` == root || fail=1
-test "`groups`" == "`chroot / groups`" || fail=1
+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
+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
+test "$(chroot --userspec=$NON_ROOT_USERNAME:$NON_ROOT_GROUP --groups= / id 
-nG)"\
+    == $NON_ROOT_GROUP || fail=1

 Exit $fail
--
1.6.3.1.149.gbc70c




reply via email to

[Prev in Thread] Current Thread [Next in Thread]