chroot: with --userspec clear root's supplemental groups

It's dangerous and confusing to leave root's supplemental
groups in place when specifying other users with --userspec.
In the edge case that that is desired one can explicitly
specify --groups.

Also we implicitly set the system defined supplemental groups
for a user.  The existing mechanism where supplemental groups
needed to be explicitly specified is confusing and not general
when the lookup needs to be done within the chroot.

Also we extend the --groups syntax slightly to allow clearing
the set of supplementary groups using --groups=''.

* src/chroot.c (setgroups): On systems without supplemental groups,
clearing then is a noop and so should return success.
(main): Lookup the primary GID with getpwuid() when just a numeric
uid is specified, and also infer the USERNAME from this call,
needed when we're later looking up the supplemental groups for a user.
Support clearing supplemental groups, either implicitly for
unknown users, or explicitly when --groups='' is specified.
* tests/misc/chroot-credentials.sh: Various new test cases
* doc/coreutils.texi (chroot invocation): Adjust for the new behavior.
* NEWS: Mention the change in behavior.

1 files changed, 65 insertions, 25 deletions

diff --git a/tests/misc/chroot-credentials.sh b/tests/misc/chroot-credentials.sh
index 904696d1c..d50704ccc 100755
--- a/tests/misc/chroot-credentials.sh
+++ b/tests/misc/chroot-credentials.sh
@@ -27,6 +27,18 @@ grep '^#define HAVE_SETGROUPS 1' "$CONFIG_HEADER" >/dev/null \
 
 root=$(id -nu 0) || skip_ "Couldn't look up root username"
 
+# verify numeric IDs looked up similarly to names
+NON_ROOT_UID=$(id -u $NON_ROOT_USERNAME)
+NON_ROOT_GID=$(id -g $NON_ROOT_USERNAME)
+
+# "uid:" is supported (unlike chown etc.) since we treat it like "uid"
+chroot --userspec=$NON_ROOT_UID: / true || fail=1
+
+# verify that invalid groups are diagnosed
+for g in ' ' ',' '0trail'; do
+  test "$(chroot --groups="$g" / id -G)" && fail=1
+done
+
 # Verify that root credentials are kept.
 test $(chroot / whoami) = "$root" || fail=1
 test "$(groups)" = "$(chroot / groups)" || fail=1
@@ -37,41 +49,69 @@ whoami_after_chroot=$(
 )
 test "$whoami_after_chroot" != "$root" || 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
+# Verify that when specifying only a group we don't change the
+# list of supplemental groups
+test "$(chroot --userspec=:$NON_ROOT_GROUP / id -G)" = \
+     "$NON_ROOT_GID $(id -G)" || fail=1
+
+if ! test "$HAVE_SETGROUPS"; then
+  Exit $fail
 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
+
+# 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
+
+# Verify that when specifying only the user name we get all their groups
+test "$(chroot --userspec=$NON_ROOT_USERNAME / id -G)" = \
+     "$(id -G $NON_ROOT_USERNAME)" || fail=1
+
+# Ditto with trailing : on the user name.
+test "$(chroot --userspec=$NON_ROOT_USERNAME: / id -G)" = \
+     "$(id -G $NON_ROOT_USERNAME)" || fail=1
+
+# Verify that when specifying only the user and clearing supplemental groups
+# that we only get the primary group
+test "$(chroot --userspec=$NON_ROOT_USERNAME --groups='' / id -G)" = \
+     "$(id -g $NON_ROOT_USERNAME)" || fail=1
+
+# Verify that when specifying only the UID we get all their groups
+test "$(chroot --userspec=$NON_ROOT_UID / id -G)" = \
+     "$(id -G $NON_ROOT_USERNAME)" || fail=1
+
+# Verify that when specifying only the user and clearing supplemental groups
+# that we only get the primary group. Note this variant with prepended '+'
+# results in no lookups in the name database which could be useful depending
+# on your chroot setup.
+test "$(chroot --userspec=+$NON_ROOT_UID:+$NON_ROOT_GID --groups='' / id -G)" =\
+     "$(id -g $NON_ROOT_USERNAME)" || 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
 
-# verify that invalid groups are diagnosed
-for g in ' ' ',' '0trail'; do
-  test "$(chroot --groups="$g" / id -G)" && fail=1
-done
+# verify that arbitrary numeric IDs are supported
+test "$(chroot --userspec=1234:+5678 --groups=' +8765,4321' / id -G)" \
+  || fail=1
 
-if test "$HAVE_SETGROUPS"; then
-  # verify that arbitrary numeric IDs are supported
-  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 cumulative
+test "$(chroot --groups='invalid ignored' --groups='' / id -G)" \
+  || fail=1
 
-  # demonstrate that extraneous commas are supported
-  test "$(chroot --userspec=1234:+5678 --groups=',8765,,4321,' / id -G)" \
-    || fail=1
+if ! id -u +12342; then
+  # Ensure supplemental groups cleared from some arbitrary unknown ID
+  test "$(chroot --userspec=+12342:+5678 / id -G)" = '5678' || fail=1
 
-  # demonstrate that --groups is not cumlative
-  test "$(chroot --groups='invalid ignored' --groups='' / id -G)" \
-    || fail=1
+  # Ensure we fail when we don't know what groups to set for an unknown ID
+  chroot --userspec=+12342 / true && fail=1
 fi
 
 Exit $fail