* Change the ownership of store paths to the Nix account before

  deleting them using the setuid helper.

3 files changed, 104 insertions, 38 deletions

diff --git a/src/libstore/build.cc b/src/libstore/build.cc
index 2b2b54cd1f..2b6e1be145 100644
--- a/src/libstore/build.cc
+++ b/src/libstore/build.cc
@@ -458,7 +458,7 @@ static bool amPrivileged()
 }
 
 
-void killUserWrapped(uid_t uid)
+static void killUserWrapped(uid_t uid)
 {
     if (amPrivileged())
         killUser(uid);
@@ -468,6 +468,58 @@ void killUserWrapped(uid_t uid)
 }
 
 
+static void getOwnership(const Path & path)
+{
+    string program = nixLibexecDir + "/nix-setuid-helper";
+            
+    /* Fork. */
+    Pid pid;
+    pid = fork();
+    switch (pid) {
+
+    case -1:
+        throw SysError("unable to fork");
+
+    case 0: /* child */
+        try {
+            std::vector<const char *> args; /* careful with c_str()!
+                                               */
+            args.push_back(program.c_str());
+            args.push_back("get-ownership");
+            args.push_back(path.c_str());
+            args.push_back(0);
+
+            execve(program.c_str(), (char * *) &args[0], 0);
+            throw SysError(format("executing `%1%'") % program);
+        }
+        catch (std::exception & e) {
+            std::cerr << "error: " << e.what() << std::endl;
+        }
+        quickExit(1);
+    }
+
+    /* Parent. */
+
+    /* Wait for the child to finish. */
+    int status = pid.wait(true);
+    if (!statusOk(status))
+        throw Error(format("program `%1%' %2%")
+            % program % statusToString(status));
+}
+
+
+static void deletePathWrapped(const Path & path)
+{
+    /* When using build users and we're not root, we may not have
+       sufficient permission to delete the path.  So use the setuid
+       helper to change ownership to us. */
+    if (querySetting("build-users-group", "") != ""
+        || !amPrivileged())
+        getOwnership(path);
+    deletePath(path);
+}
+
+
 //////////////////////////////////////////////////////////////////////
 
 
@@ -1170,7 +1222,7 @@ void DerivationGoal::startBuilder()
             throw Error(format("obstructed build: path `%1%' exists") % path);
         if (pathExists(path)) {
             debug(format("removing unregistered path `%1%'") % path);
-            deletePath(path);
+            deletePathWrapped(path);
         }
     }
 
@@ -1619,7 +1671,7 @@ void DerivationGoal::deleteTmpDir(bool force)
 		format("builder for `%1%' failed; keeping build directory `%2%'")
                 % drvPath % tmpDir);
         else
-            deletePath(tmpDir);
+            deletePathWrapped(tmpDir);
         tmpDir = "";
     }
 }
@@ -1833,7 +1885,7 @@ void SubstitutionGoal::tryToRun()
 
     /* Remove the (stale) output path if it exists. */
     if (pathExists(storePath))
-        deletePath(storePath);
+        deletePathWrapped(storePath);
 
     /* Fork the substitute program. */
     pid = fork();
diff --git a/src/libutil/util.cc b/src/libutil/util.cc
index 5907adc807..b152dc8f4b 100644
--- a/src/libutil/util.cc
+++ b/src/libutil/util.cc
@@ -797,7 +797,7 @@ string runProgram(Path program)
     /* Wait for the child to finish. */
     int status = pid.wait(true);
     if (!statusOk(status))
-        throw Error(format("program `%1% %2%")
+        throw Error(format("program `%1%' %2%")
             % program % statusToString(status));
 
     return result;
diff --git a/src/nix-setuid-helper/main.cc b/src/nix-setuid-helper/main.cc
index ea60b28001..e9ffcfd023 100644
--- a/src/nix-setuid-helper/main.cc
+++ b/src/nix-setuid-helper/main.cc
@@ -17,27 +17,42 @@
 using namespace nix;
 
 
-/* Recursively change the ownership of `path' from `uidFrom' to
-   `uidTo' and `gidTo'.  Barf if we encounter a file not owned by
-   `uidFrom'. */
-static void secureChown(uid_t uidFrom, uid_t uidTo, gid_t gidTo,
-    const Path & path)
+/* Recursively change the ownership of `path' to user `uidTo' and
+   group `gidTo'.  `path' must currently be owned by user `uidFrom',
+   or, if `uidFrom' is -1, by group `gidFrom'. */
+static void secureChown(uid_t uidFrom, gid_t gidFrom,
+    uid_t uidTo, gid_t gidTo, const Path & path)
 {
+    format error = format("cannot change ownership of `%1%'") % path;
+    
     struct stat st;
     if (lstat(path.c_str(), &st) == -1)
-        throw SysError(format("statting of `%1%'") % path);
+        /* Important: don't give any detailed error messages here.
+           Otherwise, the Nix account can discover information about
+           the existence of paths that it doesn't normally have access
+           to. */
+        throw Error(error);
+
+    if (uidFrom != -1) {
+        assert(uidFrom != 0);
+        if (st.st_uid != uidFrom)
+            throw Error(error);
+    } else {
+        assert(gidFrom != 0);
+        if (st.st_gid != gidFrom)
+            throw Error(error);
+    }
 
-    if (st.st_uid != uidFrom)
-        throw Error(format("path `%1%' owned by the wrong owner") % path);
+    assert(uidTo != 0 && gidTo != 0);
 
     if (lchown(path.c_str(), uidTo, gidTo) == -1)
-        throw SysError(format("changing ownership of `%1%'") % path);
+        throw Error(error);
 
     if (S_ISDIR(st.st_mode)) {
         Strings names = readDirectory(path);
 	for (Strings::iterator i = names.begin(); i != names.end(); ++i)
             /* !!! recursion; check stack depth */
-	    secureChown(uidFrom, uidTo, gidTo, path + "/" + *i);
+	    secureChown(uidFrom, gidFrom, uidTo, gidTo, path + "/" + *i);
     }
 }
 
@@ -55,8 +70,8 @@ static uid_t nameToUid(const string & userName)
    be a member of `buildUsersGroup'.  The ownership of the current
    directory is changed from the Nix user (uidNix) to the target
    user. */
-static void runBuilder(uid_t uidNix,
-    const string & buildUsersGroup, const string & targetUser,
+static void runBuilder(uid_t uidNix, gid_t gidBuildUsers,
+    const StringSet & buildUsers, const string & targetUser,
     string program, int argc, char * * argv, char * * env)
 {
     uid_t uidTargetUser = nameToUid(targetUser);
@@ -65,29 +80,16 @@ static void runBuilder(uid_t uidNix,
     if (uidTargetUser == 0)
         throw Error("won't setuid to root");
 
-    /* Get the gid and members of buildUsersGroup. */
-    struct group * gr = getgrnam(buildUsersGroup.c_str());
-    if (!gr)
-        throw Error(format("group `%1%' does not exist") % buildUsersGroup);
-    gid_t gidBuildUsers = gr->gr_gid;
-
     /* Verify that the target user is a member of that group. */
-    Strings users;
-    bool found = false;
-    for (char * * p = gr->gr_mem; *p; ++p)
-        if (string(*p) == targetUser) {
-            found = true;
-            break;
-        }
-    if (!found)
-        throw Error(format("user `%1%' is not a member of `%2%'")
-            % targetUser % buildUsersGroup);
+    if (buildUsers.find(targetUser) == buildUsers.end())
+        throw Error(format("user `%1%' is not a member of the build users group")
+            % targetUser);
     
     /* Chown the current directory, *if* it is owned by the Nix
        account.  The idea is that the current directory is the
        temporary build directory in /tmp or somewhere else, and we
        don't want to create that directory here. */
-    secureChown(uidNix, uidTargetUser, gidBuildUsers, ".");
+    secureChown(uidNix, -1, uidTargetUser, gidBuildUsers, ".");
 
     /* Set the real, effective and saved gid.  Must be done before
        setuid(), otherwise it won't set the real and saved gids. */
@@ -171,6 +173,17 @@ static void run(int argc, char * * argv)
         throw Error("you are not allowed to call this program, go away");
     
     
+    /* Get the gid and members of buildUsersGroup. */
+    struct group * gr = getgrnam(buildUsersGroup.c_str());
+    if (!gr)
+        throw Error(format("group `%1%' does not exist") % buildUsersGroup);
+    gid_t gidBuildUsers = gr->gr_gid;
+
+    StringSet buildUsers;
+    for (char * * p = gr->gr_mem; *p; ++p)
+        buildUsers.insert(*p);
+
+    
     /* Perform the desired command. */
     if (argc < 2)
         throw Error("invalid arguments");
@@ -181,19 +194,20 @@ static void run(int argc, char * * argv)
         /* Syntax: nix-setuid-helper run-builder <username> <program>
              <arg0 arg1...> */
         if (argc < 4) throw Error("missing user name / program name");
-        runBuilder(uidNix, buildUsersGroup,
+        runBuilder(uidNix, gidBuildUsers, buildUsers,
             argv[2], argv[3], argc - 4, argv + 4, oldEnviron);
     }
 
-    else if (command == "fix-ownership") {
-        /* Syntax: nix-setuid-helper <fix-ownership> <path> */
+    else if (command == "get-ownership") {
+        /* Syntax: nix-setuid-helper get-ownership <path> */
+        if (argc != 3) throw Error("missing path");
+        secureChown(-1, gidBuildUsers, uidNix, gidBuildUsers, argv[2]);
     }
 
     else throw Error ("invalid command");
 }
 
 
-
 int main(int argc, char * * argv)
 {
     try {