On 05/16/2014 10:59 PM, Pádraig Brady wrote: Thanks for the detailed tests.
> [[ chroot --user=+5000 / id -G ]] > before: 0 1 2 3 4 6 10 > after: src/chroot: failed to get primary group While the logic behind may be okay, our users will probably be confused with the above change in behavior. I propose to change the error diagnostic to give a hint that the user must specify the group explicitly when using a numeric UID. > diff --git a/doc/coreutils.texi b/doc/coreutils.texi > index 789cd68..bfaae9f 100644 > --- a/doc/coreutils.texi > +++ b/doc/coreutils.texi > @@ -16112,12 +16112,17 @@ 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}. > +If a @var{user} is specified then the supplementary groups > +are set according to the system defined list for that user, > +unless overridden with the @option{--groups} option. > > @item --groups=@var{groups} > @opindex --groups > -Use this option to specify the supplementary @var{groups} to be > +Use this option to override 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. > +Use @samp{--groups=''} to disable the supplementary group lookup Didn't we use s/lookup/look-up/ recently, see v8.22-38-ge972be3? > diff --git a/src/chroot.c b/src/chroot.c > index a2debac..a679658 100644 > --- a/src/chroot.c > +++ b/src/chroot.c > @@ -232,12 +250,36 @@ main (int argc, char **argv) > /* We have to look up users and groups twice. > - First, outside the chroot to load potentially necessary > passwd/group > parsing plugins (e.g. NSS); > - - Second, inside chroot to redo parsing in case IDs are different. > */ > + - Second, inside chroot to redo parsing in case IDs are different. > + Within chroot lookup is the main justification for having > + the --user option supported cy the chroot command itself. */ s/cy/by/ > - if (gids && setgroups (n_gids, gids) != 0) > + if ((uid_set (uid) || groups) && setgroups (n_gids, gids) != 0) > { > - error (0, errno, _("failed to set additional groups")); > + error (0, errno, _("failed to %s supplemental groups"), > + gids ? "set" : "clear"); > fail = true; > } > > free (in_gids); > free (out_gids); > > - if (gid != (gid_t) -1 && setgid (gid)) > + if (gid_set (gid) && setgid (gid)) > { > error (0, errno, _("failed to set group-ID")); > fail = true; > } > > - if (uid != (uid_t) -1 && setuid (uid)) > + if (uid_set (uid) && setuid (uid)) > { > error (0, errno, _("failed to set user-ID")); > fail = true; Is there a reason not to exit immediately? IMHO it looks strange (not to say worrying) to get 3 error messages: $ src/chroot --user=+5000:123 / id -G src/chroot: failed to clear supplemental groups: Operation not permitted src/chroot: failed to set group-ID: Operation not permitted src/chroot: failed to set user-ID: Operation not permitted Otherwise: nice work, thanks! Have a nice day, Berny