Merge pull request #3923 from obsidiansystems/daemon-auth-cleanup

Separate auth and logic for the daemon

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();

src/libstore/daemon.cc

@@ -817,8 +817,7 @@ void processConnection(
     FdSink & to,
     TrustedFlag trusted,
     RecursiveFlag recursive,
-    const std::string & userName,
+    std::function<void(Store &)> authHook)
-    uid_t userId)
 {
     auto monitor = !recursive ? std::make_unique<MonitorFdHup>(from.fd) : nullptr;
 
@@ -859,15 +858,7 @@ void processConnection(
 
         /* If we can't accept clientVersion, then throw an error
            *here* (not above). */
-
+        authHook(*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(userName, userId);
 
         tunnelLogger->stopWork();
         to.flush();

src/libstore/daemon.hh

@@ -12,7 +12,10 @@ void processConnection(
     FdSink & to,
     TrustedFlag trusted,
     RecursiveFlag recursive,
-    const std::string & userName,
+    /* Arbitrary hook to check authorization / initialize user data / whatever
-    uid_t userId);
+       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<void(Store &)> authHook);
 
 }

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);

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