Eric Blake wrote: > According to Jim Meyering on 5/26/2009 7:21 AM: >> Merged and pushed. >> Thanks again. > > Would it be worth starting to patch the testsuite to replace 'setuidgid -g > list usr cmd arg' with 'chroot --user usr --groups=list / cmd arg' in > order to give this feature more exposure and reduce our dependence on > uninstalled apps?
Yes. It would be good to get more coverage for installed tools. However, I've just noticed that the new code in chroot.c needs work. Two problems: - set*ID failure did not evoke non-zero exit - "chroot --u=N / id -a" currently uses "gid" uninitialized i.e., when --userspec=USER (without ":GROUP") Same for --u=:1 (i.e, group ID with no user-ID) $ sudo ./chroot --u=0 / id -g 3496015512 Here are patches, for review. I'll add tests tomorrow. >From 63a1039e21fbf127e052f1b4d80176e3a2386d2a Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyer...@redhat.com> Date: Wed, 27 May 2009 22:06:04 +0200 Subject: [PATCH] chroot: set-*-ID failure must provoke nonzero exit before execvp * src/chroot.c (main): Exit upon set-group-ID or set-user-ID failure. --- src/chroot.c | 21 ++++++++++++++++++--- 1 files changed, 18 insertions(+), 3 deletions(-) diff --git a/src/chroot.c b/src/chroot.c index 788a1fc..ab35944 100644 --- a/src/chroot.c +++ b/src/chroot.c @@ -207,6 +207,7 @@ main (int argc, char **argv) char *user; char *group; char const *err = parse_user_spec (userspec, &uid, &gid, &user, &group); + unsigned int fail = 0; if (err) error (EXIT_FAILURE, errno, "%s", err); @@ -214,14 +215,28 @@ main (int argc, char **argv) free (user); free (group); + /* Attempt to set all three: supplementary groups, group ID, user ID. + Diagnose any failures. If any have failed, exit before execvp. */ if (groups && set_additional_groups (groups)) - error (0, errno, _("failed to set additional groups")); + { + error (0, errno, _("failed to set additional groups")); + ++fail; + } if (gid && setgid (gid)) - error (0, errno, _("failed to set group-ID")); + { + error (0, errno, _("failed to set group-ID")); + ++fail; + } if (uid && setuid (uid)) - error (0, errno, _("failed to set user-ID")); + { + error (0, errno, _("failed to set user-ID")); + ++fail; + } + + if (fail) + exit (EXIT_FAILURE); } /* Execute the given command. */ -- 1.6.3.1.268.g94d6d1 >From 6752c02be6bc760e92687e5ae71e93513f5b84da Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyer...@redhat.com> Date: Wed, 27 May 2009 23:06:15 +0200 Subject: [PATCH] chroot: don't set bogus user-ID or group-ID for --u=U or --u=:G * src/chroot.c (main): Initialize both "uid" and "gid". To -1. This also allows one to set the primary group-ID to 0, in case it's not that already. --- src/chroot.c | 8 ++++---- 1 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/chroot.c b/src/chroot.c index ab35944..8bc4e59 100644 --- a/src/chroot.c +++ b/src/chroot.c @@ -202,8 +202,8 @@ main (int argc, char **argv) if (userspec) { - uid_t uid; - gid_t gid; + uid_t uid = -1; + gid_t gid = -1; char *user; char *group; char const *err = parse_user_spec (userspec, &uid, &gid, &user, &group); @@ -223,13 +223,13 @@ main (int argc, char **argv) ++fail; } - if (gid && setgid (gid)) + if (gid != (gid_t) -1 && setgid (gid)) { error (0, errno, _("failed to set group-ID")); ++fail; } - if (uid && setuid (uid)) + if (uid != (uid_t) -1 && setuid (uid)) { error (0, errno, _("failed to set user-ID")); ++fail; -- 1.6.3.1.268.g94d6d1 _______________________________________________ Bug-coreutils mailing list Bug-coreutils@gnu.org http://lists.gnu.org/mailman/listinfo/bug-coreutils