From mboxrd@z Thu Jan 1 00:00:00 1970 From: ludo@gnu.org (Ludovic =?UTF-8?Q?Court=C3=A8s?=) Subject: bug#18994: [PATCH] Preserve supplementary groups of build users Date: Wed, 01 Jul 2015 11:12:51 +0200 Message-ID: <87vbe4w8a4.fsf__10253.2928729945$1435742125$gmane$org@gnu.org> References: <87ppcxsse0.fsf@gnu.org> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:55518) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZAE5k-00066h-Ib for bug-guix@gnu.org; Wed, 01 Jul 2015 05:14:05 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZAE5j-0001z6-1d for bug-guix@gnu.org; Wed, 01 Jul 2015 05:14:04 -0400 Received: from debbugs.gnu.org ([140.186.70.43]:33754) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZAE5i-0001z2-Uq for bug-guix@gnu.org; Wed, 01 Jul 2015 05:14:02 -0400 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.80) (envelope-from ) id 1ZAE5i-0007N2-LU for bug-guix@gnu.org; Wed, 01 Jul 2015 05:14:02 -0400 Sender: "Debbugs-submit" Resent-Message-ID: In-Reply-To: <87ppcxsse0.fsf@gnu.org> ("Ludovic \=\?utf-8\?Q\?Court\=C3\=A8s\=22'\?\= \=\?utf-8\?Q\?s\?\= message of "Sat, 08 Nov 2014 15:01:43 +0100") List-Id: Bug reports for GNU Guix List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-guix-bounces+gcggb-bug-guix=m.gmane.org@gnu.org Sender: bug-guix-bounces+gcggb-bug-guix=m.gmane.org@gnu.org To: 18994@debbugs.gnu.org, Eelco Dolstra Cc: nix-dev@lists.science.uu.nl --=-=-= Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable ludo@gnu.org (Ludovic Court=C3=A8s) skribis: > Currently, the build environment made by the daemon does not preserve > supplementary groups of the build users. > > Thus, even though the standalone Guix system sets /dev/kvm 660, owned by > root:kvm, and adds the build users to the kvm group, build users are > unable to access it. The following patch is an attempt to address this bug (see ) by preserving the supplementary groups of build users in the build environment. In practice, I would expect that supplementary groups would contain only one or two groups: the build users group, and possibly the =E2=80=9Ckvm=E2= =80=9D group. WDYT? Thanks, Ludo=E2=80=99. --=-=-= Content-Type: text/x-patch Content-Disposition: inline diff --git a/nix/libstore/build.cc b/nix/libstore/build.cc index 85a818b..4644810 100644 --- a/nix/libstore/build.cc +++ b/nix/libstore/build.cc @@ -447,6 +447,7 @@ private: string user; uid_t uid; gid_t gid; + std::vector supplementaryGIDs; public: UserLock(); @@ -460,6 +461,7 @@ public: string getUser() { return user; } uid_t getUID() { return uid; } uid_t getGID() { return gid; } + std::vector getSupplementaryGIDs() { return supplementaryGIDs; } bool enabled() { return uid != 0; } @@ -539,6 +541,18 @@ void UserLock::acquire() throw Error(format("the Nix user should not be a member of `%1%'") % settings.buildUsersGroup); + /* Get the list of supplementary groups of this build user. This + is usually either empty or contains a group such as "kvm". */ + supplementaryGIDs.resize(10); + int ngroups = supplementaryGIDs.size(); + int err = getgrouplist(pw->pw_name, pw->pw_gid, + &supplementaryGIDs.at(0), &ngroups); + if (err == -1) + throw Error(format("failed to get list of supplementary groups for `%1'") + % pw->pw_name); + + supplementaryGIDs.resize(ngroups); + return; } } @@ -2179,8 +2193,11 @@ void DerivationGoal::runChild() if (buildUser.enabled()) { printMsg(lvlChatty, format("switching to user `%1%'") % buildUser.getUser()); - if (setgroups(0, 0) == -1) - throw SysError("cannot clear the set of supplementary groups"); + /* Preserve supplementary groups of the build user, to allow + admins to specify groups such as "kvm". */ + if (setgroups(buildUser.getSupplementaryGIDs().size(), + &buildUser.getSupplementaryGIDs().at(0)) == -1) + throw SysError("cannot set supplementary groups of build user"); if (setgid(buildUser.getGID()) == -1 || getgid() != buildUser.getGID() || --=-=-=--