From 8d4162ff9e940ea9e2f97b07f3030a722695901a Mon Sep 17 00:00:00 2001 From: John Ericson Date: Wed, 12 Aug 2020 15:14:56 +0000 Subject: [PATCH] Separate auth and logic for the daemon Before, processConnection wanted to know a user name and user id, and `nix-daemon --stdio`, when it isn't proxying to an underlying daemon, would just assume "root" and 0. But `nix-daemon --stdio` (no proxying) shouldn't make guesses about who holds the other end of its standard streams. Now processConnection takes an "auth hook", so `nix-daemon` can provide the appropriate policy and daemon.cc doesn't need to know or care what it is. --- src/libstore/build.cc | 3 ++- src/libstore/daemon.cc | 13 ++----------- src/libstore/daemon.hh | 7 +++++-- src/nix-daemon/nix-daemon.cc | 15 +++++++++++++-- tests/remote-store.sh | 3 +++ 5 files changed, 25 insertions(+), 16 deletions(-) diff --git a/src/libstore/build.cc b/src/libstore/build.cc index 76baa1a6e..3fb052f00 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -2920,7 +2920,8 @@ void DerivationGoal::startDaemon() FdSink to(remote.get()); try { daemon::processConnection(store, from, to, - daemon::NotTrusted, daemon::Recursive, "nobody", 65535); + daemon::NotTrusted, daemon::Recursive, + [&](Store & store) { store.createUser("nobody", 65535); }); debug("terminated daemon connection"); } catch (SysError &) { ignoreException(); diff --git a/src/libstore/daemon.cc b/src/libstore/daemon.cc index 5e568fc94..7a6eb99be 100644 --- a/src/libstore/daemon.cc +++ b/src/libstore/daemon.cc @@ -817,8 +817,7 @@ void processConnection( FdSink & to, TrustedFlag trusted, RecursiveFlag recursive, - const std::string & userName, - uid_t userId) + std::function authHook) { auto monitor = !recursive ? std::make_unique(from.fd) : nullptr; @@ -859,15 +858,7 @@ void processConnection( /* If we can't accept clientVersion, then throw an error *here* (not above). */ - -#if 0 - /* Prevent users from doing something very dangerous. */ - if (geteuid() == 0 && - querySetting("build-users-group", "") == "") - throw Error("if you run 'nix-daemon' as root, then you MUST set 'build-users-group'!"); -#endif - - store->createUser(userName, userId); + authHook(*store); tunnelLogger->stopWork(); to.flush(); diff --git a/src/libstore/daemon.hh b/src/libstore/daemon.hh index 266932013..841ace316 100644 --- a/src/libstore/daemon.hh +++ b/src/libstore/daemon.hh @@ -12,7 +12,10 @@ void processConnection( FdSink & to, TrustedFlag trusted, RecursiveFlag recursive, - const std::string & userName, - uid_t userId); + /* Arbitrary hook to check authorization / initialize user data / whatever + after the protocol has been negotiated. The idea is that this function + and everything it calls doesn't know about this stuff, and the + `nix-daemon` handles that instead. */ + std::function authHook); } diff --git a/src/nix-daemon/nix-daemon.cc b/src/nix-daemon/nix-daemon.cc index bcb86cbce..cfa634a44 100644 --- a/src/nix-daemon/nix-daemon.cc +++ b/src/nix-daemon/nix-daemon.cc @@ -239,7 +239,15 @@ static void daemonLoop(char * * argv) // Handle the connection. FdSource from(remote.get()); FdSink to(remote.get()); - processConnection(openUncachedStore(), from, to, trusted, NotRecursive, user, peer.uid); + processConnection(openUncachedStore(), from, to, trusted, NotRecursive, [&](Store & store) { +#if 0 + /* Prevent users from doing something very dangerous. */ + if (geteuid() == 0 && + querySetting("build-users-group", "") == "") + throw Error("if you run 'nix-daemon' as root, then you MUST set 'build-users-group'!"); +#endif + store.createUser(user, peer.uid); + }); exit(0); }, options); @@ -324,7 +332,10 @@ static int _main(int argc, char * * argv) } else { FdSource from(STDIN_FILENO); FdSink to(STDOUT_FILENO); - processConnection(openUncachedStore(), from, to, Trusted, NotRecursive, "root", 0); + /* Auth hook is empty because in this mode we blindly trust the + standard streams. Limitting access to thoses is explicitly + not `nix-daemon`'s responsibility. */ + processConnection(openUncachedStore(), from, to, Trusted, NotRecursive, [&](Store & _){}); } } else { daemonLoop(argv); diff --git a/tests/remote-store.sh b/tests/remote-store.sh index 4cc73465a..3a61946f9 100644 --- a/tests/remote-store.sh +++ b/tests/remote-store.sh @@ -2,6 +2,9 @@ source common.sh clearStore +# Ensure "fake ssh" remote store works just as legacy fake ssh would. +nix --store ssh-ng://localhost?remote-store=$TEST_ROOT/other-store doctor + startDaemon storeCleared=1 NIX_REMOTE_=$NIX_REMOTE $SHELL ./user-envs.sh