On 03/13/2014 07:37 AM, Bernhard Voelker wrote:
> On 03/13/2014 02:47 AM, Pádraig Brady wrote:
>> Proposed patch is attached.
>
> The patch looks good to me, but I think that whole "out-then-inside"
> thing should be mentioned in the texinfo manual.
>
> Additionally, I think a link to
> @node Disambiguating names and IDs
> would be nice in there (like in '@node chown invocation').
>
> BTW:
> As id(1) is also using the leading "+" as disambiguation,
> I think that chapter should be generalized more, and also referenced
> there.
Done with the attached adjustment.
Also I've now allowed an empty --groups=""
which results in a noop rather than an error.
This seems more natural and also --userspec="" was already allowed.
Also I've added more test cases.
> BTW2:
> A side note for the info pages: it is not documented that --userspec
> also accepts the dot ('.') as separator between the user and the group,
> but this is true for chown(1), too (the other program using
> parse_user_spec for user *and* group parsing, id(1) does not).
There is already a note in "chown invocation" about the support for "."
I think we'll leave it there for now, since it's legacy and
shouldn't be used with the newer `chroot --userspec` option.
thanks!
Pádraig.
diff --git a/doc/coreutils.texi b/doc/coreutils.texi
index 7ba8cd4..7efcb6f 100644
--- a/doc/coreutils.texi
+++ b/doc/coreutils.texi
@@ -221,7 +221,7 @@ Common Options
* Block size:: Block size
* Floating point:: Floating point number representation
* Signal specifications:: Specifying signals
-* Disambiguating names and IDs:: chgrp and chown owner and group syntax
+* Disambiguating names and IDs:: chgrp, chown, chroot, id: user and group syntax
* Random sources:: Sources of random data
* Target directory:: Target directory
* Trailing slashes:: Trailing slashes
@@ -736,7 +736,7 @@ name.
* Block size:: BLOCK_SIZE and --block-size, in some programs.
* Floating point:: Floating point number representation.
* Signal specifications:: Specifying signals using the --signal option.
-* Disambiguating names and IDs:: chgrp and chown owner and group syntax
+* Disambiguating names and IDs:: chgrp, chown, chroot, id: user and group syntax
* Random sources:: --random-source, in some programs.
* Target directory:: Specifying a target directory, in some programs.
* Trailing slashes:: --strip-trailing-slashes, in some programs.
@@ -1135,20 +1135,20 @@ also support at least eight real-time signals called @samp{RTMIN},
@samp{RTMIN+1}, @dots{}, @samp{RTMAX-1}, @samp{RTMAX}.
@node Disambiguating names and IDs
-@section chown and chgrp: Disambiguating user names and IDs
+@section chown, chgrp, chroot, id: Disambiguating user names and IDs
@cindex user names, disambiguating
@cindex user IDs, disambiguating
@cindex group names, disambiguating
@cindex group IDs, disambiguating
@cindex disambiguating group names and IDs
-Since the @var{owner} and @var{group} arguments to @command{chown} and
-@command{chgrp} may be specified as names or numeric IDs, there is an
+Since the @var{user} and @var{group} arguments to these commands
+may be specified as names or numeric IDs, there is an
apparent ambiguity.
What if a user or group @emph{name} is a string of digits?
@footnote{Using a number as a user name is common in some environments.}
Should the command interpret it as a user name or as an ID@?
-POSIX requires that @command{chown} and @command{chgrp}
+POSIX requires that these commands
first attempt to resolve the specified string as a name, and
only once that fails, then try to interpret it as an ID@.
This is troublesome when you want to specify a numeric ID, say 42,
@@ -1157,9 +1157,9 @@ and it must work even in a pathological situation where
Simply invoking @code{chown 42 F}, will set @file{F}s owner ID to
1000---not what you intended.
-GNU @command{chown} and @command{chgrp} provide a way to work around this,
-that at the same time may result in a significant performance improvement
-by eliminating a database look-up.
+GNU @command{chown}, @command{chgrp}, @command{chroot}, and @command{id}
+provide a way to work around this, that at the same time may result in a
+significant performance improvement by eliminating a database look-up.
Simply precede each numeric user ID and/or group ID with a @samp{+},
in order to force its interpretation as an integer:
@@ -1169,8 +1169,7 @@ chgrp +$numeric_group_id another-file
chown +0:+0 /
@end example
-GNU @command{chown} and @command{chgrp}
-skip the name look-up process for each @samp{+}-prefixed string,
+The name look-up process is skipped for each @samp{+}-prefixed string,
because a string containing @samp{+} is never a valid user or group name.
This syntax is accepted on most common Unix systems, but not on Solaris 10.
@@ -14538,8 +14537,9 @@ running it if no user is specified. Synopsis:
id [@var{option}]@dots{} [@var{user}]
@end example
-@var{user} can be either a user ID or a name, with name lookup
+@var{user} can be either a user ID or a name, with name look up
taking precedence unless the ID is specified with a leading @samp{+}.
+@xref{Disambiguating names and IDs}.
@vindex POSIXLY_CORRECT
By default, it prints the real user ID, real group ID, effective user ID
@@ -16109,6 +16109,13 @@ The items in the list (names or numeric IDs) must be separated by commas.
@end table
+The user and group name look up performed by the @option{--userspec}
+and @option{--groups} options, is done both outside and inside
+the chroot, with successful look ups inside the chroot taking precedence.
+If the specified user or group items are intended to represent a numeric ID,
+then a name to ID resolving step is avoided by specifying a leading @samp{+}.
+@xref{Disambiguating names and IDs}.
+
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
diff --git a/src/chroot.c b/src/chroot.c
index 8044e5b..9db5cf8 100644
--- a/src/chroot.c
+++ b/src/chroot.c
@@ -231,7 +231,7 @@ main (int argc, char **argv)
- Second, inside chroot to redo the parsing in case IDs are different. */
if (userspec)
ignore_value (parse_user_spec (userspec, &uid, &gid, NULL, NULL));
- if (groups)
+ if (groups && *groups)
ignore_value (parse_additional_groups (groups, &out_gids, &n_gids, false));
if (chroot (argv[optind]) != 0)
@@ -271,7 +271,7 @@ main (int argc, char **argv)
GETGROUPS_T *gids = out_gids;
GETGROUPS_T *in_gids = NULL;
- if (groups)
+ if (groups && *groups)
{
if (parse_additional_groups (groups, &in_gids, &n_gids, !n_gids) != 0)
{
diff --git a/tests/misc/chroot-credentials.sh b/tests/misc/chroot-credentials.sh
index 56f8ce7..38112e7 100755
--- a/tests/misc/chroot-credentials.sh
+++ b/tests/misc/chroot-credentials.sh
@@ -22,6 +22,9 @@ print_ver_ chroot
require_root_
+grep '^#define HAVE_SETGROUPS 1' "$CONFIG_HEADER" >/dev/null \
+ && HAVE_SETGROUPS=1
+
root=$(id -nu 0) || skip_ "Couldn't lookup root username"
# Verify that root credentials are kept.
@@ -34,26 +37,41 @@ whoami_after_chroot=$(
)
test "$whoami_after_chroot" != "$root" || fail=1
-# Verify that there are no additional groups.
-id_G_after_chroot=$(
- chroot --userspec=$NON_ROOT_USERNAME:$NON_ROOT_GROUP \
- --groups=$NON_ROOT_GROUP / id -G
-)
-test "$id_G_after_chroot" = $NON_ROOT_GROUP || fail=1
+if test "$HAVE_SETGROUPS"; then
+ # Verify that there are no additional groups.
+ id_G_after_chroot=$(
+ chroot --userspec=$NON_ROOT_USERNAME:$NON_ROOT_GROUP \
+ --groups=$NON_ROOT_GROUP / id -G
+ )
+ test "$id_G_after_chroot" = $NON_ROOT_GROUP || fail=1
+fi
# Verify that when specifying only the user name we get the current
# primary group ID.
test "$(chroot --userspec=$NON_ROOT_USERNAME / id -g)" = "$(id -g)" \
- || fail=1
+ || fail=1
# Verify that when specifying only a group we get the current user ID
test "$(chroot --userspec=:$NON_ROOT_GROUP / id -u)" = "$(id -u)" \
- || fail=1
+ || fail=1
+
+# verify that invalid groups are diagnosed
+for g in ' ' ',' '0trail'; do
+ test "$(chroot --groups="$g" / id -G)" && fail=1
+done
-if grep '^#define HAVE_SETGROUPS 1' "$CONFIG_HEADER" >/dev/null; then
+if test "$HAVE_SETGROUPS"; then
# verify that arbitrary numeric IDs are supported
- test "$(chroot --userspec=1234:+5678 --groups=" +8765,4321" / id -G)" \
- || fail=1
+ test "$(chroot --userspec=1234:+5678 --groups=' +8765,4321' / id -G)" \
+ || fail=1
+
+ # demonstrate that extraneous commas are supported
+ test "$(chroot --userspec=1234:+5678 --groups=',8765,,4321,' / id -G)" \
+ || fail=1
+
+ # demonstrate that --groups is not cumlative
+ test "$(chroot --groups='invalid ignored' --groups='' / id -G)" \
+ || fail=1
fi
Exit $fail
