Giuseppe Scrivano wrote: > 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.
Thank you. a few nits. Some were exposed by running this: ./configure --enable-gcc-warnings && make && make syntax-check I rewrote parts of the .texinfo documentation and removed a paragraph, since now that the new options are documented, it seemed redundant. I'll merge these 4 change sets into yours and push late today or tomorrow, to give people time for any additional feedback. >From fe77420a0ed029100f413173be79c678cd346064 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyer...@redhat.com> Date: Mon, 25 May 2009 14:55:39 +0200 Subject: [PATCH 1/4] sort things, cpp-indent --- src/chroot.c | 8 ++++---- 1 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/chroot.c b/src/chroot.c index 3e0b30d..7ca9dc4 100644 --- a/src/chroot.c +++ b/src/chroot.c @@ -37,19 +37,19 @@ #define AUTHORS proper_name ("Roland McGrath") #ifndef MAXGID -#define MAXGID GID_T_MAX +# define MAXGID GID_T_MAX #endif enum { - USERSPEC = UCHAR_MAX + 1, - GROUPS + GROUPS = UCHAR_MAX + 1, + USERSPEC }; static struct option const long_opts[] = { - {"userspec", required_argument, NULL, USERSPEC}, {"groups", required_argument, NULL, GROUPS}, + {"userspec", required_argument, NULL, USERSPEC}, {GETOPT_HELP_OPTION_DECL}, {GETOPT_VERSION_OPTION_DECL}, {NULL, 0, NULL, 0} -- 1.6.3.1.241.g24356 >From 5a0ca8823504e1eea922ab83a284f63c31c900a6 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyer...@redhat.com> Date: Mon, 25 May 2009 15:03:36 +0200 Subject: [PATCH 2/4] don't include xalloc.h: system.h already does that --- src/chroot.c | 1 - 1 files changed, 0 insertions(+), 1 deletions(-) diff --git a/src/chroot.c b/src/chroot.c index 7ca9dc4..b801220 100644 --- a/src/chroot.c +++ b/src/chroot.c @@ -29,7 +29,6 @@ #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" -- 1.6.3.1.241.g24356 >From d4e8934ce07683d09ae42dae27868c3681f5986e Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyer...@redhat.com> Date: Mon, 25 May 2009 15:15:49 +0200 Subject: [PATCH 3/4] Avoid new warning, fix diagnostic, remove curly braces cc1: warnings being treated as errors chroot.c: In function 'main': chroot.c:213: error: format not a string literal and \ no format arguments [-Wformat-security] correct copy-paste-o: failed setuid would diagnose setgid failure Remove curly braces around single-stmt blocks. --- src/chroot.c | 17 +++++------------ 1 files changed, 5 insertions(+), 12 deletions(-) diff --git a/src/chroot.c b/src/chroot.c index b801220..788a1fc 100644 --- a/src/chroot.c +++ b/src/chroot.c @@ -209,26 +209,19 @@ main (int argc, char **argv) char const *err = parse_user_spec (userspec, &uid, &gid, &user, &group); if (err) - error (EXIT_FAILURE, errno, err); + error (EXIT_FAILURE, errno, "%s", err); free (user); free (group); - if (groups) - { - if (set_additional_groups (groups)) - error (0, errno, _("cannot set additional groups")); - } + if (groups && set_additional_groups (groups)) + error (0, errno, _("failed to set additional groups")); if (gid && setgid (gid)) - { - error (0, errno, _("cannot setgid")); - } + error (0, errno, _("failed to set group-ID")); if (uid && setuid (uid)) - { - error (0, errno, _("cannot setgid")); - } + error (0, errno, _("failed to set user-ID")); } /* Execute the given command. */ -- 1.6.3.1.241.g24356 >From 6de3363e51770876065f6e70abac847b4bb3bcc6 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyer...@redhat.com> Date: Mon, 25 May 2009 15:46:36 +0200 Subject: [PATCH 4/4] tweak documentation --- doc/coreutils.texi | 22 ++++++++-------------- 1 files changed, 8 insertions(+), 14 deletions(-) diff --git a/doc/coreutils.texi b/doc/coreutils.texi index 3371ba5..97ea830 100644 --- a/doc/coreutils.texi +++ b/doc/coreutils.texi @@ -14162,27 +14162,21 @@ chroot invocation @table @samp -...@itemx --usersp...@var{userspec} +...@itemx --usersp...@var{user}[:@var{group}] @opindex --userspec -Use the @var{userspec} user and group in the new environment. -...@var{userspec} is given in the form @var{user}:@var{group}, the group -can be omitted and in this case the original one will be used. +By default, @var{command} is run with the same credentials +as the invoking process. +Use this option to run it as a different @var{user} and/or with a +different primary @var{group}. @itemx --grou...@var{groups} @opindex --groups -Specify an additional set of groups @var{groups} to be used by the -chroot process. +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. @end 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 linked binary. If you were to use a dynamically linked executable, then -- 1.6.3.1.241.g24356 _______________________________________________ Bug-coreutils mailing list Bug-coreutils@gnu.org http://lists.gnu.org/mailman/listinfo/bug-coreutils