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.
This commit is contained in:
Eelco Dolstra 2012-10-03 17:30:45 -04:00
parent 7586095504
commit 522ecab9b8
6 changed files with 9 additions and 146 deletions

View file

@ -201,10 +201,8 @@ flag, e.g. <literal>--option gc-keep-outputs false</literal>.</para>
under the uid of the Nix process (that is, the uid of the caller under the uid of the Nix process (that is, the uid of the caller
if <envar>NIX_REMOTE</envar> is empty, the uid under which the Nix if <envar>NIX_REMOTE</envar> is empty, the uid under which the Nix
daemon runs if <envar>NIX_REMOTE</envar> is daemon runs if <envar>NIX_REMOTE</envar> is
<literal>daemon</literal>, or the uid that owns the setuid <literal>daemon</literal>). Obviously, this should not be used in
<command>nix-worker</command> program if <envar>NIX_REMOTE</envar> multi-user settings with untrusted users.</para>
is <literal>slave</literal>). Obviously, this should not be used
in multi-user settings with untrusted users.</para>
</listitem> </listitem>

View file

@ -125,8 +125,7 @@ static void initAndRun(int argc, char * * argv)
/* There is no privacy in the Nix system ;-) At least not for /* There is no privacy in the Nix system ;-) At least not for
now. In particular, store objects should be readable by now. In particular, store objects should be readable by
everybody. This prevents nasty surprises when using a shared everybody. */
store (with the setuid() hack). */
umask(0022); umask(0022);
/* Process the NIX_LOG_TYPE environment variable. */ /* 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. */ /* Called when the Boehm GC runs out of memory. */
static void * oomHandler(size_t requested) static void * oomHandler(size_t requested)
{ {
@ -298,11 +247,6 @@ int main(int argc, char * * argv)
argvSaved = 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. */ /* Turn on buffering for cerr. */
#if HAVE_PUBSETBUF #if HAVE_PUBSETBUF
std::cerr.rdbuf()->pubsetbuf(buf, sizeof(buf)); std::cerr.rdbuf()->pubsetbuf(buf, sizeof(buf));

View file

@ -44,9 +44,6 @@ template<class N> N getIntArg(const string & opt,
/* Show the manual page for the specified program. */ /* Show the manual page for the specified program. */
void showManPage(const string & name); void showManPage(const string & name);
/* Whether we're running setuid. */
extern bool setuidMode;
extern volatile ::sig_atomic_t blockInt; extern volatile ::sig_atomic_t blockInt;
/* Exit code of the program. */ /* Exit code of the program. */

View file

@ -50,16 +50,12 @@ void RemoteStore::openConnection(bool reserveSpace)
string remoteMode = getEnv("NIX_REMOTE"); string remoteMode = getEnv("NIX_REMOTE");
if (remoteMode == "slave") if (remoteMode == "daemon")
/* Fork off a setuid worker to do the privileged work. */
forkSlave();
else if (remoteMode == "daemon")
/* Connect to a daemon that does the privileged work for /* Connect to a daemon that does the privileged work for
us. */ us. */
connectToDaemon(); connectToDaemon();
else else
throw Error(format("invalid setting for NIX_REMOTE, `%1%'") throw Error(format("invalid setting for NIX_REMOTE, `%1%'") % remoteMode);
% remoteMode);
from.fd = fdSocket; from.fd = fdSocket;
to.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() void RemoteStore::connectToDaemon()
{ {
fdSocket = socket(PF_UNIX, SOCK_STREAM, 0); fdSocket = socket(PF_UNIX, SOCK_STREAM, 0);

View file

@ -893,33 +893,15 @@ static void daemonLoop()
void run(Strings args) void run(Strings args)
{ {
bool slave = false;
bool daemon = false; bool daemon = false;
for (Strings::iterator i = args.begin(); i != args.end(); ) { for (Strings::iterator i = args.begin(); i != args.end(); ) {
string arg = *i++; string arg = *i++;
if (arg == "--slave") slave = true; if (arg == "--daemon") /* ignored for backwards compatibility */;
if (arg == "--daemon") daemon = true;
} }
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("/"); chdir("/");
daemonLoop(); daemonLoop();
}
else
throw Error("must be run in either --slave or --daemon mode");
} }

View file

@ -1,11 +1,5 @@
source common.sh source common.sh
echo '*** testing slave mode ***'
clearStore
clearManifests
NIX_REMOTE_=slave $SHELL ./user-envs.sh
echo '*** testing daemon mode ***'
clearStore clearStore
clearManifests clearManifests
startDaemon startDaemon