From 522ecab9b83902de5a3010b50b9532e376cbba4c Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Wed, 3 Oct 2012 17:30:45 -0400 Subject: [PATCH] Drop support for running nix-worker in "slave" mode AFAIK nobody uses this, setuid binaries are evil, and there is no good reason why people can't just run the daemon. --- doc/manual/conf-file.xml | 6 ++-- src/libmain/shared.cc | 58 +----------------------------------- src/libmain/shared.hh | 3 -- src/libstore/remote-store.cc | 58 ++---------------------------------- src/nix-worker/nix-worker.cc | 24 ++------------- tests/remote-store.sh | 6 ---- 6 files changed, 9 insertions(+), 146 deletions(-) diff --git a/doc/manual/conf-file.xml b/doc/manual/conf-file.xml index f56cc27264..2dc260b357 100644 --- a/doc/manual/conf-file.xml +++ b/doc/manual/conf-file.xml @@ -201,10 +201,8 @@ flag, e.g. --option gc-keep-outputs false. 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. + daemon). Obviously, this should not be used in + multi-user settings with untrusted users. diff --git a/src/libmain/shared.cc b/src/libmain/shared.cc index f8149fc608..9e5964acf8 100644 --- a/src/libmain/shared.cc +++ b/src/libmain/shared.cc @@ -125,8 +125,7 @@ static void initAndRun(int argc, char * * argv) /* There is no privacy in the Nix system ;-) At least not for now. In particular, store objects should be readable by - everybody. This prevents nasty surprises when using a shared - store (with the setuid() hack). */ + everybody. */ umask(0022); /* Process the NIX_LOG_TYPE environment variable. */ @@ -218,56 +217,6 @@ static void initAndRun(int argc, char * * argv) } -bool setuidMode = false; - - -static void setuidInit() -{ - /* Don't do anything if this is not a setuid binary. */ - if (getuid() == geteuid() && getgid() == getegid()) return; - - uid_t nixUid = geteuid(); - gid_t nixGid = getegid(); - - setuidCleanup(); - - /* Don't trust the current directory. */ - if (chdir("/") == -1) abort(); - - /* Set the real (and preferably also the save) uid/gid to the - effective uid/gid. This matters mostly when we're not using - build-users (bad!), since some builders (like Perl) complain - when real != effective. - - On systems where setresuid is unavailable, we can't drop the - saved uid/gid. This means that we could go back to the - original real uid (i.e., the uid of the caller). That's not - really a problem, except maybe when we execute a builder and - we're not using build-users. In that case, the builder may be - able to switch to the uid of the caller and possibly do bad - stuff. But note that when not using build-users, the builder - could also modify the Nix executables (say, replace them by a - Trojan horse), so the problem is already there. */ - -#if HAVE_SETRESUID - if (setresuid(nixUid, nixUid, nixUid)) abort(); - if (setresgid(nixGid, nixGid, nixGid)) abort(); -#elif HAVE_SETREUID - /* Note: doesn't set saved uid/gid! */ - fprintf(stderr, "warning: cannot set saved uid\n"); - if (setreuid(nixUid, nixUid)) abort(); - if (setregid(nixGid, nixGid)) abort(); -#else - /* Note: doesn't set real and saved uid/gid! */ - fprintf(stderr, "warning: cannot set real and saved uids\n"); - if (setuid(nixUid)) abort(); - if (setgid(nixGid)) abort(); -#endif - - setuidMode = true; -} - - /* Called when the Boehm GC runs out of memory. */ static void * oomHandler(size_t requested) { @@ -298,11 +247,6 @@ int main(int argc, char * * argv) argvSaved = argv; - /* If we're setuid, then we need to take some security precautions - right away. */ - if (argc == 0) abort(); - setuidInit(); - /* Turn on buffering for cerr. */ #if HAVE_PUBSETBUF std::cerr.rdbuf()->pubsetbuf(buf, sizeof(buf)); diff --git a/src/libmain/shared.hh b/src/libmain/shared.hh index 9e5ec83609..b054b0717f 100644 --- a/src/libmain/shared.hh +++ b/src/libmain/shared.hh @@ -44,9 +44,6 @@ template N getIntArg(const string & opt, /* Show the manual page for the specified program. */ void showManPage(const string & name); -/* Whether we're running setuid. */ -extern bool setuidMode; - extern volatile ::sig_atomic_t blockInt; /* Exit code of the program. */ diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc index 08e409d3f0..16b5db8082 100644 --- a/src/libstore/remote-store.cc +++ b/src/libstore/remote-store.cc @@ -50,16 +50,12 @@ void RemoteStore::openConnection(bool reserveSpace) string remoteMode = getEnv("NIX_REMOTE"); - if (remoteMode == "slave") - /* Fork off a setuid worker to do the privileged work. */ - forkSlave(); - else if (remoteMode == "daemon") + if (remoteMode == "daemon") /* Connect to a daemon that does the privileged work for us. */ - connectToDaemon(); + connectToDaemon(); else - throw Error(format("invalid setting for NIX_REMOTE, `%1%'") - % remoteMode); + throw Error(format("invalid setting for NIX_REMOTE, `%1%'") % remoteMode); from.fd = fdSocket; to.fd = fdSocket; @@ -88,54 +84,6 @@ void RemoteStore::openConnection(bool reserveSpace) } -void RemoteStore::forkSlave() -{ - int sockets[2]; - if (socketpair(AF_UNIX, SOCK_STREAM, 0, sockets) == -1) - throw SysError("cannot create sockets"); - - fdSocket = sockets[0]; - AutoCloseFD fdChild = sockets[1]; - - /* Start the worker. */ - Path worker = getEnv("NIX_WORKER"); - if (worker == "") - worker = settings.nixBinDir + "/nix-worker"; - - child = fork(); - - switch (child) { - - case -1: - throw SysError("unable to fork"); - - case 0: - try { /* child */ - - if (dup2(fdChild, STDOUT_FILENO) == -1) - throw SysError("dupping write side"); - - if (dup2(fdChild, STDIN_FILENO) == -1) - throw SysError("dupping read side"); - - close(fdSocket); - close(fdChild); - - execlp(worker.c_str(), worker.c_str(), "--slave", NULL); - - throw SysError(format("executing `%1%'") % worker); - - } catch (std::exception & e) { - std::cerr << format("child error: %1%\n") % e.what(); - } - quickExit(1); - } - - fdChild.close(); - -} - - void RemoteStore::connectToDaemon() { fdSocket = socket(PF_UNIX, SOCK_STREAM, 0); diff --git a/src/nix-worker/nix-worker.cc b/src/nix-worker/nix-worker.cc index 17ffdb616c..833fc35184 100644 --- a/src/nix-worker/nix-worker.cc +++ b/src/nix-worker/nix-worker.cc @@ -893,33 +893,15 @@ static void daemonLoop() void run(Strings args) { - bool slave = false; bool daemon = false; for (Strings::iterator i = args.begin(); i != args.end(); ) { string arg = *i++; - if (arg == "--slave") slave = true; - if (arg == "--daemon") daemon = true; + if (arg == "--daemon") /* ignored for backwards compatibility */; } - if (slave) { - /* This prevents us from receiving signals from the terminal - when we're running in setuid mode. */ - if (setsid() == -1) - throw SysError(format("creating a new session")); - - processConnection(); - } - - else if (daemon) { - if (setuidMode) - throw Error("daemon cannot be started in setuid mode"); - chdir("/"); - daemonLoop(); - } - - else - throw Error("must be run in either --slave or --daemon mode"); + chdir("/"); + daemonLoop(); } diff --git a/tests/remote-store.sh b/tests/remote-store.sh index ef289ab79a..6a9585cacf 100644 --- a/tests/remote-store.sh +++ b/tests/remote-store.sh @@ -1,11 +1,5 @@ source common.sh -echo '*** testing slave mode ***' -clearStore -clearManifests -NIX_REMOTE_=slave $SHELL ./user-envs.sh - -echo '*** testing daemon mode ***' clearStore clearManifests startDaemon