aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorEelco Dolstra <e.dolstra@tudelft.nl>2006-12-06 20:00:15 +0000
committerEelco Dolstra <e.dolstra@tudelft.nl>2006-12-06 20:00:15 +0000
commit6e5ec1029ad279c1ac69e14730afb4d2d9964b5d (patch)
treed0b031a8c4a40d6fa9c6d77b95b1acc66ea1840c
parent751f6d2157a1b89f2463b68a90f8515deb3f942c (diff)
* Get rid of `build-users'. We'll just take all the members of
`build-users-group'. This makes configuration easier: you can just add users in /etc/group.
-rw-r--r--nix.conf.example52
-rw-r--r--src/libstore/build.cc88
-rw-r--r--src/nix-setuid-helper/main.cc10
3 files changed, 86 insertions, 64 deletions
diff --git a/nix.conf.example b/nix.conf.example
index a75045b14..92e114dc5 100644
--- a/nix.conf.example
+++ b/nix.conf.example
@@ -78,44 +78,44 @@
#build-max-jobs = 1
-### Option `build-users'
+### Option `build-users-group'
#
-# This option contains a list of user names under which Nix can
-# execute builds. In multi-user Nix installations, builds should not
+# This options specifies the Unix group containing the Nix build user
+# accounts. In multi-user Nix installations, builds should not
# be performed by the Nix account since that would allow users to
# arbitrarily modify the Nix store and database by supplying specially
# crafted builders; and they cannot be performed by the calling user
# since that would allow him/her to influence the build result.
#
-# Thus this list should contain a number of `special' user accounts
-# created specifically for Nix, e.g., `nix-builder-1',
-# `nix-builder-2', and so on. The more users the better, since at
-# most a number of builds equal to the number of build users can be
-# running simultaneously.
+# Therefore, if this option is non-empty and specifies a valid group,
+# builds will be performed under the user accounts that are a member
+# of the group specified here (as listed in /etc/group). Those user
+# accounts should not be used for any other purpose!
#
-# If this list is empty, builds will be performed under the Nix
-# account (that is, the uid under which the Nix daemon runs, or that
-# owns the setuid nix-worker program).
+# Nix will never run two builds under the same user account at the
+# same time. This is to prevent an obvious security hole: a malicious
+# user writing a Nix expression that modifies the build result of a
+# legitimate Nix expression being built by another user. Therefore it
+# is good to have as many Nix build user accounts as you can spare.
+# (Remember: uids are cheap.)
#
-# Example:
-# build-users = nix-builder-1 nix-builder-2 nix-builder-3
-#build-users =
-
-
-### Option `build-users-group'
+# The build users should have permission to create files in the Nix
+# store, but not delete them. Therefore, /nix/store should be owned
+# by the Nix account, its group should be the group specified here,
+# and its mode should be 1775.
#
-# If `build-users' is used, then this option specifies the group ID
-# (gid) under which each build is to be performed. This group should
-# have permission to create files in the Nix store, but not delete
-# them. I.e., /nix/store should be owned by the Nix account, its
-# group should be the group specified here, and its mode should be
-# 1775.
+# If the build users group is empty, builds will be performed under
+# the uid of the Nix process (that is, the uid of the caller if
+# $NIX_REMOTE is empty, the uid under which the Nix daemon runs if
+# $NIX_REMOTE is `daemon', or the uid that owns the setuid nix-worker
+# program if $NIX_REMOTE is `slave'). Obviously, this should not be
+# used in multi-user settings with untrusted users.
#
-# The default is `nix'.
+# The default is empty.
#
# Example:
-# build-users-group = nix
-#build-users-group =
+# build-users-group = nix-builders
+build-users-group = nix-builders
### Option `system'
diff --git a/src/libstore/build.cc b/src/libstore/build.cc
index 0ecd8bb10..ec3353cf3 100644
--- a/src/libstore/build.cc
+++ b/src/libstore/build.cc
@@ -341,6 +341,7 @@ private:
AutoCloseFD fdUserLock;
uid_t uid;
+ gid_t gid;
public:
UserLock();
@@ -350,6 +351,7 @@ public:
void release();
uid_t getUID();
+ uid_t getGID();
};
@@ -358,7 +360,7 @@ PathSet UserLock::lockedPaths;
UserLock::UserLock()
{
- uid = 0;
+ uid = gid = 0;
}
@@ -371,21 +373,37 @@ UserLock::~UserLock()
void UserLock::acquire()
{
assert(uid == 0);
-
- Strings buildUsers = querySetting("build-users", Strings());
- if (buildUsers.empty())
- throw Error(
- "cannot build as `root'; you must define "
- "one or more users for building in `build-users' "
- "in the Nix configuration file");
+ string buildUsersGroup = querySetting("build-users-group", "");
+ assert(buildUsersGroup != "");
+
+ /* Get the members of the build-users-group. */
+ struct group * gr = getgrnam(buildUsersGroup.c_str());
+ if (!gr)
+ throw Error(format("the group `%1%' specified in `build-users-group' does not exist")
+ % buildUsersGroup);
+ gid = gr->gr_gid;
+
+ /* Copy the result of getgrnam. */
+ Strings users;
+ for (char * * p = gr->gr_mem; *p; ++p) {
+ debug(format("found build user `%1%'") % *p);
+ users.push_back(*p);
+ }
+
+ if (users.empty())
+ throw Error(format("the build users group `%1%' has no members")
+ % buildUsersGroup);
- for (Strings::iterator i = buildUsers.begin(); i != buildUsers.end(); ++i) {
+ /* Find a user account that isn't currently in use for another
+ build. */
+ for (Strings::iterator i = users.begin(); i != users.end(); ++i) {
debug(format("trying user `%1%'") % *i);
struct passwd * pw = getpwnam(i->c_str());
if (!pw)
- throw Error(format("the user `%1%' listed in `build-users' does not exist") % *i);
+ throw Error(format("the user `%1%' in the group `%2%' does not exist")
+ % *i % buildUsersGroup);
fnUserLock = (format("%1%/userpool/%2%") % nixStateDir % pw->pw_uid).str();
@@ -405,9 +423,9 @@ void UserLock::acquire()
}
}
- throw Error("all build users are currently in use; "
- "consider expanding the `build-users' field "
- "in the Nix configuration file");
+ throw Error(format("all build users are currently in use; "
+ "consider creating additional users and adding them to the `%1%' group")
+ % buildUsersGroup);
}
@@ -428,6 +446,12 @@ uid_t UserLock::getUID()
}
+uid_t UserLock::getGID()
+{
+ return gid;
+}
+
+
static void killUser(uid_t uid)
{
debug(format("killing all processes running under uid `%1%'") % uid);
@@ -1275,12 +1299,12 @@ void DerivationGoal::startBuilder()
}
- /* If `build-users' is not empty, then we have to build as one of
- the users listed in `build-users'. */
- gid_t gidBuildGroup = -1;
- if (querySetting("build-users", Strings()).size() > 0) {
+ /* If `build-users-group' is not empty, then we have to build as
+ one of the members of that group. */
+ if (querySetting("build-users-group", "") != "") {
buildUser.acquire();
assert(buildUser.getUID() != 0);
+ assert(buildUser.getGID() != 0);
/* Make sure that no other processes are executing under this
uid. */
@@ -1290,16 +1314,8 @@ void DerivationGoal::startBuilder()
if (chown(tmpDir.c_str(), buildUser.getUID(), (gid_t) -1) == -1)
throw SysError(format("cannot change ownership of `%1%'") % tmpDir);
- /* What group to execute the builder in? */
- string buildGroup = querySetting("build-users-group", "nix");
- struct group * gr = getgrnam(buildGroup.c_str());
- if (!gr) throw Error(
- format("the group `%1%' specified in `build-users-group' does not exist")
- % buildGroup);
- gidBuildGroup = gr->gr_gid;
-
/* Check that the Nix store has the appropriate permissions,
- i.e., owned by root and mode 1777 (sticky bit on so that
+ i.e., owned by root and mode 1775 (sticky bit on so that
the builder can create its output but not mess with the
outputs of other processes). */
struct stat st;
@@ -1307,11 +1323,11 @@ void DerivationGoal::startBuilder()
throw SysError(format("cannot stat `%1%'") % nixStore);
if (!(st.st_mode & S_ISVTX) ||
((st.st_mode & S_IRWXG) != S_IRWXG) ||
- (st.st_gid != gidBuildGroup))
+ (st.st_gid != buildUser.getGID()))
throw Error(format(
"builder does not have write permission to `%1%'; "
"try `chgrp %1% %2%; chmod 1775 %2%'")
- % buildGroup % nixStore);
+ % buildUser.getGID() % nixStore);
}
@@ -1365,13 +1381,15 @@ void DerivationGoal::startBuilder()
if (setgroups(0, 0) == -1)
throw SysError("cannot clear the set of supplementary groups");
- setgid(gidBuildGroup);
- assert(getgid() == gidBuildGroup);
- assert(getegid() == gidBuildGroup);
-
- setuid(buildUser.getUID());
- assert(getuid() == buildUser.getUID());
- assert(geteuid() == buildUser.getUID());
+ if (setgid(buildUser.getGID()) == -1 ||
+ getgid() != buildUser.getGID() ||
+ getegid() != buildUser.getGID())
+ throw SysError("setgid failed");
+
+ if (setuid(buildUser.getUID()) == -1 ||
+ getuid() != buildUser.getUID() ||
+ geteuid() != buildUser.getUID())
+ throw SysError("setuid failed");
}
/* Execute the program. This should not return. */
diff --git a/src/nix-setuid-helper/main.cc b/src/nix-setuid-helper/main.cc
index 50a059f50..d278d5677 100644
--- a/src/nix-setuid-helper/main.cc
+++ b/src/nix-setuid-helper/main.cc
@@ -40,14 +40,18 @@ static void runBuilder(string userName,
don't want to create that directory here. */
secureChown(pw->pw_uid, gidBuilders, ".");
+
/* Set the real, effective and saved gid. Must be done before
setuid(), otherwise it won't set the real and saved gids. */
+ if (setgroups(0, 0) == -1)
+ throw SysError("cannot clear the set of supplementary groups");
//setgid(gidBuilders);
/* Set the real, effective and saved uid. */
- setuid(pw->pw_uid);
- if (getuid() != pw->pw_uid || geteuid() != pw->pw_uid)
- throw Error("cannot setuid");
+ if (setuid(pw->pw_uid) == -1 ||
+ getuid() != pw->pw_uid ||
+ geteuid() != pw->pw_uid)
+ throw SysError("setuid failed");
/* Execute the program. */
std::vector<const char *> args;