mirrors/coreutils
0
0
Fork 0
mirror of https://github.com/coreutils/coreutils.git synced 2024-12-18 06:18:25 +08:00
Code Issues Packages Projects Releases Wiki Activity

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

Browse Source 
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.
This commit is contained in:
Pádraig Brady 2014-05-16 09:50:24 +01:00
parent 99960eeab9
commit ce0c08b52d
4 changed files with 177 additions and 40 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
NEWS
View File

@ -85,6 +85,9 @@ GNU coreutils NEWS -*- outline -*-
chroot with an argument of "/" no longer implicitly changes the current
directory to "/", allowing changing only user credentials for a command.
chroot --userspec will now unset supplemental groups associated with root,
and instead use the supplemental groups of the specified user.
ls with none of LS_COLORS or COLORTERM environment variables set,
will now honor an empty or unknown TERM environment variable,
and not output colors even with --colors=always.

7
doc/coreutils.texi
View File

@ -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 look-up
implicit in the @option{--userspec} option.
@end table

117
src/chroot.c
View File

@ -20,11 +20,13 @@
#include <getopt.h>
#include <stdio.h>
#include <sys/types.h>
#include <pwd.h>
#include <grp.h>
#include "system.h"
#include "error.h"
#include "ignore-value.h"
#include "mgetgroups.h"
#include "quote.h"
#include "userspec.h"
#include "xstrtol.h"
@ -38,6 +40,11 @@
# define MAXGID GID_T_MAX
#endif
static inline bool uid_unset (uid_t uid) { return uid == (uid_t) -1; }
static inline bool gid_unset (gid_t gid) { return gid == (gid_t) -1; }
#define uid_set(x) (!uid_unset (x))
#define gid_set(x) (!gid_unset (x))
enum
{
GROUPS = UCHAR_MAX + 1,
@ -56,10 +63,20 @@ static struct option const long_opts[] =
#if ! HAVE_SETGROUPS
/* At least Interix lacks supplemental group support. */
static int
setgroups (size_t size _GL_UNUSED, gid_t const *list _GL_UNUSED)
setgroups (size_t size, gid_t const *list _GL_UNUSED)
{
errno = ENOTSUP;
return -1;
if (size == 0)
{
/* Return success when clearing supplemental groups
as ! HAVE_SETGROUPS should only be the case on
platforms that don't support supplemental groups. */
return 0;
}
else
{
errno = ENOTSUP;
return -1;
}
}
#endif
@ -180,7 +197,8 @@ main (int argc, char **argv)
int c;
/* Input user and groups spec. */
char const *userspec = NULL;
char *userspec = NULL;
char const *username = NULL;
char const *groups = NULL;
/* Parsed user and group IDs. */
@ -203,8 +221,16 @@ main (int argc, char **argv)
switch (c)
{
case USERSPEC:
userspec = optarg;
break;
{
userspec = optarg;
/* Treat 'user:' just like 'user'
as we lookup the primary group by default
(and support doing so for UIDs as well as names. */
size_t userlen = strlen (userspec);
if (userlen && userspec[userlen - 1] == ':')
userspec[userlen - 1] = '\0';
break;
}
case GROUPS:
groups = optarg;
@ -232,12 +258,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 by the chroot command itself. */
if (userspec)
ignore_value (parse_user_spec (userspec, &uid, &gid, NULL, NULL));
/* If no gid is supplied or looked up, do so now.
Also lookup the username for use with getgroups. */
if (uid_set (uid) && (! groups || gid_unset (gid)))
{
const struct passwd *pwd;
if ((pwd = getpwuid (uid)))
{
if (gid_unset (gid))
gid = pwd->pw_gid;
username = pwd->pw_name;
}
}
if (groups && *groups)
ignore_value (parse_additional_groups (groups, &out_gids, &n_gids,
false));
#if HAVE_SETGROUPS
else if (! groups && gid_set (gid) && username)
{
int ngroups = xgetgroups (username, gid, &out_gids);
if (0 < ngroups)
n_gids = ngroups;
}
#endif
if (chroot (argv[optind]) != 0)
error (EXIT_CANCELED, errno, _("cannot change root directory to %s"),
@ -271,40 +321,79 @@ main (int argc, char **argv)
{
char const *err = parse_user_spec (userspec, &uid, &gid, NULL, NULL);
if (err && uid == -1 && gid == -1)
if (err && uid_unset (uid) && gid_unset (gid))
error (EXIT_CANCELED, errno, "%s", err);
}
/* If no gid is supplied or looked up, do so now.
Also lookup the username for use with getgroups. */
if (uid_set (uid) && (! groups || gid_unset (gid)))
{
const struct passwd *pwd;
if ((pwd = getpwuid (uid)))
{
if (gid_unset (gid))
gid = pwd->pw_gid;
username = pwd->pw_name;
}
else if (gid_unset (gid))
{
error (EXIT_CANCELED, errno,
_("no group specified for unknown uid: %d"), (int) uid);
}
}
GETGROUPS_T *gids = out_gids;
GETGROUPS_T *in_gids = NULL;
if (groups && *groups)
{
if (parse_additional_groups (groups, &in_gids, &n_gids, !n_gids) != 0)
{
/* If look-up outside the chroot worked, then go with those gids. */
if (! n_gids)
fail = true;
/* else look-up outside the chroot worked, then go with those. */
}
else
gids = in_gids;
}
if (gids && setgroups (n_gids, gids) != 0)
#if HAVE_SETGROUPS
else if (! groups && gid_set (gid) && username)
{
error (0, errno, _("failed to set additional groups"));
int ngroups = xgetgroups (username, gid, &in_gids);
if (ngroups <= 0)
{
if (! n_gids)
{
fail = true;
error (0, errno, _("failed to get supplemental groups"));
}
/* else look-up outside the chroot worked, then go with those. */
}
else
{
n_gids = ngroups;
gids = in_gids;
}
}
#endif
if ((uid_set (uid) || groups) && setgroups (n_gids, gids) != 0)
{
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;

90
tests/misc/chroot-credentials.sh
View File

@ -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 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 --groups is not cumlative
test "$(chroot --groups='invalid ignored' --groups='' / 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
# 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