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 <meyer...@redhat.com> 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 <meyer...@redhat.com> 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 <meyer...@redhat.com> 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 +...@itemx --usersp...@var{spec} +...@opindex --userspec +...@itemx --grou...@var{group_list} +...@opindex --groups @command{chroot} runs a command with a specified root directory. On many systems, only the super-user can do th...@footnote{however, @@ -14144,7 +14148,7 @@ chroot invocation Synopses: @example -chroot @var{newroot} [...@var{command} [...@var{args}]@dots{}] +chroot @var{option} @var{newroot} [...@var{command} [...@var{args}]@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}, -...@option{--userspec} 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. +...@c 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 <meyer...@redhat.com> 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 <meyer...@redhat.com> 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 _______________________________________________ Bug-coreutils mailing list Bug-coreutils@gnu.org http://lists.gnu.org/mailman/listinfo/bug-coreutils