03/05: daemon: '--listen' can be passed several times, can specify TCP endpoints.

[Ludovic Courtès]

civodul pushed a commit to branch master
in repository guix.

commit 1071f781d97509347144754b3248581cf7c6c1d5
Author: Ludovic Courtès <address@hidden>
Date:   Mon Jun 19 17:39:24 2017 +0200

    daemon: '--listen' can be passed several times, can specify TCP endpoints.
    
    * nix/nix-daemon/guix-daemon.cc (DEFAULT_GUIX_PORT): New macro.
    (listen_options): New variable.
    (parse_opt): Push back '--listen' options to LISTEN_OPTIONS.
    (open_unix_domain_socket, open_inet_socket)
    (listening_sockets): New functions.
    (main): Use it.  Pass SOCKETS to 'run'.
    * nix/nix-daemon/nix-daemon.cc (matchUser): Remove.
    (SD_LISTEN_FDS_START): Remove.
    (acceptConnection): New function.
    (daemonLoop): Rewrite to take a vector of file descriptors, to select(2)
    on them, and to call 'acceptConnection'.
    (run): Change to take a vector of file descriptors.
    * tests/guix-daemon.sh: Add test.
---
 doc/guix.texi                 |  55 +++++++-
 nix/nix-daemon/guix-daemon.cc | 152 +++++++++++++++++++++--
 nix/nix-daemon/nix-daemon.cc  | 283 +++++++++++++++++++-----------------------
 tests/guix-daemon.sh          |  12 ++
 4 files changed, 327 insertions(+), 175 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index ee9f80e..729ec08 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -1258,12 +1258,47 @@ Assume @var{system} as the current system type.  By 
default it is the
 architecture/kernel pair found at configure time, such as
 @code{x86_64-linux}.
 
address@hidden address@hidden
-Listen for connections on @var{socket}, the file name of a Unix-domain
-socket.  The default socket is
address@hidden@var{localstatedir}/daemon-socket/socket}.  This option is only
-useful in exceptional circumstances, such as if you need to run several
-daemons on the same machine.
address@hidden address@hidden
+Listen for connections on @var{endpoint}.  @var{endpoint} is interpreted
+as the file name of a Unix-domain socket if it starts with
address@hidden/} (slash sign).  Otherwise, @var{endpoint} is interpreted as a
+host name or host name and port to listen to.  Here are a few examples:
+
address@hidden @code
address@hidden --listen=/gnu/var/daemon
+Listen for connections on the @file{/gnu/var/daemon} Unix-domain socket,
+creating it if needed.
+
address@hidden --listen=localhost
address@hidden daemon, remote access
address@hidden remote access to the daemon
address@hidden daemon, cluster setup
address@hidden clusters, daemon setup
+Listen for TCP connections on the network interface corresponding to
address@hidden, on port 44146.
+
address@hidden --listen=128.0.0.42:1234
+Listen for TCP connections on the network interface corresponding to
address@hidden, on port 1234.
address@hidden table
+
+This option can be repeated multiple times, in which case
address@hidden accepts connections on all the specified
+endpoints.  Users can tell client commands what endpoint to connect to
+by setting the @code{GUIX_DAEMON_SOCKET} environment variable
+(@pxref{The Store, @code{GUIX_DAEMON_SOCKET}}).
+
address@hidden Note
+The daemon protocol is @emph{unauthenticated and unencrypted}.  Using
address@hidden@var{host}} is suitable on local networks, such as
+clusters, where only trusted nodes may connect to the build daemon.  In
+other cases where remote access to the daemon is needed, we recommend
+using Unix-domain sockets along with SSH.
address@hidden quotation
+
+When @code{--listen} is omitted, @command{guix-daemon} listens for
+connections on the Unix-domain socket located at
address@hidden@var{localstatedir}/daemon-socket/socket}.
 @end table
 
 
@@ -3769,6 +3804,10 @@ These are for Unix-domain sockets.
 @file{/var/guix/daemon-socket/socket}.
 
 @item guix
address@hidden daemon, remote access
address@hidden remote access to the daemon
address@hidden daemon, cluster setup
address@hidden clusters, daemon setup
 These URIs denote connections over TCP/IP, without encryption nor
 authentication of the remote host.  The URI must specify the host name
 and optionally a port number (by default port 44146 is used):
@@ -3781,6 +3820,10 @@ This setup is suitable on local networks, such as 
clusters, where only
 trusted nodes may connect to the build daemon at
 @code{master.guix.example.org}.
 
+The @code{--listen} option of @command{guix-daemon} can be used to
+instruct it to listen for TCP connections (@pxref{Invoking guix-daemon,
address@hidden).
+
 @item ssh
 @cindex SSH access to build daemons
 These URIs allow you to connect to a remote daemon over
diff --git a/nix/nix-daemon/guix-daemon.cc b/nix/nix-daemon/guix-daemon.cc
index 0d9c33d..7963358 100644
--- a/nix/nix-daemon/guix-daemon.cc
+++ b/nix/nix-daemon/guix-daemon.cc
@@ -1,5 +1,6 @@
 /* GNU Guix --- Functional package management for GNU
    Copyright (C) 2012, 2013, 2014, 2015, 2016, 2017 Ludovic Courtès 
<address@hidden>
+   Copyright (C) 2006, 2010, 2012, 2014 Eelco Dolstra <address@hidden>
 
    This file is part of GNU Guix.
 
@@ -30,8 +31,12 @@
 #include <unistd.h>
 #include <sys/types.h>
 #include <sys/stat.h>
+#include <sys/socket.h>
+#include <sys/un.h>
+#include <netdb.h>
 #include <strings.h>
 #include <exception>
+#include <iostream>
 
 #include <libintl.h>
 #include <locale.h>
@@ -43,7 +48,7 @@ char **argvSaved;
 using namespace nix;
 
 /* Entry point in `nix-daemon.cc'.  */
-extern void run (Strings args);
+extern void run (const std::vector<int> &);
 
 
 /* Command-line options.  */
@@ -149,6 +154,12 @@ to live outputs") },
   };
 
 
+/* Default port for '--listen' on TCP/IP.  */
+#define DEFAULT_GUIX_PORT "44146"
+
+/* List of '--listen' options.  */
+static std::list<std::string> listen_options;
+
 /* Convert ARG to a Boolean value, or throw an error if it does not denote a
    Boolean.  */
 static bool
@@ -217,15 +228,7 @@ parse_opt (int key, char *arg, struct argp_state *state)
       settings.keepLog = false;
       break;
     case GUIX_OPT_LISTEN:
-      try
-       {
-         settings.nixDaemonSocketFile = canonPath (arg);
-       }
-      catch (std::exception &e)
-       {
-         fprintf (stderr, _("error: %s\n"), e.what ());
-         exit (EXIT_FAILURE);
-       }
+      listen_options.push_back (arg);
       break;
     case GUIX_OPT_SUBSTITUTE_URLS:
       settings.set ("substitute-urls", arg);
@@ -276,13 +279,134 @@ static const struct argp argp =
     guix_textdomain
   };
 
+
+static int
+open_unix_domain_socket (const char *file)
+{
+  /* Create and bind to a Unix domain socket. */
+  AutoCloseFD fdSocket = socket (PF_UNIX, SOCK_STREAM, 0);
+  if (fdSocket == -1)
+    throw SysError (_("cannot create Unix domain socket"));
+
+  createDirs (dirOf (file));
+
+  /* Urgh, sockaddr_un allows path names of only 108 characters.
+     So chdir to the socket directory so that we can pass a
+     relative path name. */
+  if (chdir (dirOf (file).c_str ()) == -1)
+    throw SysError (_("cannot change current directory"));
+  Path fileRel = "./" + baseNameOf (file);
+
+  struct sockaddr_un addr;
+  addr.sun_family = AF_UNIX;
+  if (fileRel.size () >= sizeof (addr.sun_path))
+    throw Error (format (_("socket file name '%1%' is too long")) % fileRel);
+  strcpy (addr.sun_path, fileRel.c_str ());
+
+  unlink (file);
+
+  /* Make sure that the socket is created with 0666 permission
+     (everybody can connect --- provided they have access to the
+     directory containing the socket). */
+  mode_t oldMode = umask (0111);
+  int res = bind (fdSocket, (struct sockaddr *) &addr, sizeof addr);
+  umask (oldMode);
+  if (res == -1)
+    throw SysError (format (_("cannot bind to socket '%1%'")) % file);
+
+  if (chdir ("/") == -1) /* back to the root */
+    throw SysError (_("cannot change current directory"));
+
+  if (listen (fdSocket, 5) == -1)
+    throw SysError (format (_("cannot listen on socket '%1%'")) % file);
+
+  return fdSocket.borrow ();
+}
+
+/* Return a listening socket for ADDRESS, which has the given LENGTH.  */
+static int
+open_inet_socket (const struct sockaddr *address, socklen_t length)
+{
+  AutoCloseFD fd = socket (address->sa_family, SOCK_STREAM, 0);
+  if (fd == -1)
+    throw SysError (_("cannot create TCP socket"));
+
+  int res = bind (fd, address, length);
+  if (res == -1)
+    throw SysError (_("cannot bind TCP socket"));
+
+  if (listen (fd, 5) == -1)
+    throw SysError (format (_("cannot listen on TCP socket")));
+
+  return fd.borrow ();
+}
+
+/* Return a list of file descriptors of listening sockets.  */
+static std::vector<int>
+listening_sockets (const std::list<std::string> &options)
+{
+  std::vector<int> result;
+
+  if (options.empty ())
+    {
+      /* Open the default Unix-domain socket.  */
+      auto fd = open_unix_domain_socket (settings.nixDaemonSocketFile.c_str 
());
+      result.push_back (fd);
+      return result;
+    }
+
+  /* Open the user-specified sockets.  */
+  for (const std::string& option: options)
+    {
+      if (option[0] == '/')
+       {
+         /* Assume OPTION is the file name of a Unix-domain socket.  */
+         settings.nixDaemonSocketFile = canonPath (option);
+         int fd =
+           open_unix_domain_socket (settings.nixDaemonSocketFile.c_str ());
+         result.push_back (fd);
+       }
+      else
+       {
+         /* Assume OPTIONS has the form "HOST" or "HOST:PORT".  */
+         auto colon = option.find_last_of (":");
+         auto host = colon == std::string::npos
+           ? option : option.substr (0, colon);
+         auto port = colon == std::string::npos
+           ? DEFAULT_GUIX_PORT
+           : option.substr (colon + 1, option.size () - colon - 1);
+
+         struct addrinfo *res, hints;
+
+         memset (&hints, '\0', sizeof hints);
+         hints.ai_socktype = SOCK_STREAM;
+         hints.ai_flags = AI_NUMERICSERV | AI_ADDRCONFIG;
+
+         int err = getaddrinfo (host.c_str(), port.c_str (),
+                                &hints, &res);
+
+         if (err != 0)
+           throw Error(format ("failed to look up '%1%': %2%")
+                       % option % gai_strerror (err));
+
+         printMsg (lvlDebug, format ("listening on '%1%', port '%2%'")
+                   % host % port);
+
+         /* XXX: Pick the first result, RES.  */
+         result.push_back (open_inet_socket (res->ai_addr,
+                                             res->ai_addrlen));
+
+         freeaddrinfo (res);
+       }
+    }
+
+  return result;
+}
 
 
 int
 main (int argc, char *argv[])
 {
-  static const Strings nothing;
-
   setlocale (LC_ALL, "");
   bindtextdomain (guix_textdomain, LOCALEDIR);
   textdomain (guix_textdomain);
@@ -359,6 +483,8 @@ main (int argc, char *argv[])
 
       argp_parse (&argp, argc, argv, 0, 0, 0);
 
+      auto sockets = listening_sockets (listen_options);
+
       /* Effect all the changes made via 'settings.set'.  */
       settings.update ();
 
@@ -402,7 +528,7 @@ using `--build-users-group' is highly recommended\n"));
       printMsg (lvlDebug,
                format ("listening on `%1%'") % settings.nixDaemonSocketFile);
 
-      run (nothing);
+      run (sockets);
     }
   catch (std::exception &e)
     {
diff --git a/nix/nix-daemon/nix-daemon.cc b/nix/nix-daemon/nix-daemon.cc
index 79580ff..3d8e909 100644
--- a/nix/nix-daemon/nix-daemon.cc
+++ b/nix/nix-daemon/nix-daemon.cc
@@ -18,6 +18,7 @@
 #include <sys/stat.h>
 #include <sys/socket.h>
 #include <sys/un.h>
+#include <arpa/inet.h>
 #include <fcntl.h>
 #include <errno.h>
 #include <pwd.h>
@@ -809,151 +810,87 @@ static void setSigChldAction(bool autoReap)
 }
 
 
-bool matchUser(const string & user, const string & group, const Strings & 
users)
+/* Accept a connection on FDSOCKET and fork a server process to process the
+   new connection.  */
+static void acceptConnection(int fdSocket)
 {
-    if (find(users.begin(), users.end(), "*") != users.end())
-        return true;
-
-    if (find(users.begin(), users.end(), user) != users.end())
-        return true;
-
-    for (auto & i : users)
-        if (string(i, 0, 1) == "@") {
-            if (group == string(i, 1)) return true;
-            struct group * gr = getgrnam(i.c_str() + 1);
-            if (!gr) continue;
-            for (char * * mem = gr->gr_mem; *mem; mem++)
-                if (user == string(*mem)) return true;
-        }
-
-    return false;
-}
-
-
-#define SD_LISTEN_FDS_START 3
-
-
-static void daemonLoop()
-{
-    if (chdir("/") == -1)
-        throw SysError("cannot change current directory");
-
-    /* Get rid of children automatically; don't let them become
-       zombies. */
-    setSigChldAction(true);
-
-    AutoCloseFD fdSocket;
-
-    /* Handle socket-based activation by systemd. */
-    if (getEnv("LISTEN_FDS") != "") {
-        if (getEnv("LISTEN_PID") != std::to_string(getpid()) || 
getEnv("LISTEN_FDS") != "1")
-            throw Error("unexpected systemd environment variables");
-        fdSocket = SD_LISTEN_FDS_START;
-    }
-
-    /* Otherwise, create and bind to a Unix domain socket. */
-    else {
-
-        /* Create and bind to a Unix domain socket. */
-        fdSocket = socket(PF_UNIX, SOCK_STREAM, 0);
-        if (fdSocket == -1)
-            throw SysError("cannot create Unix domain socket");
-
-        string socketPath = settings.nixDaemonSocketFile;
-
-        createDirs(dirOf(socketPath));
-
-        /* Urgh, sockaddr_un allows path names of only 108 characters.
-           So chdir to the socket directory so that we can pass a
-           relative path name. */
-        if (chdir(dirOf(socketPath).c_str()) == -1)
-            throw SysError("cannot change current directory");
-        Path socketPathRel = "./" + baseNameOf(socketPath);
-
-        struct sockaddr_un addr;
-        addr.sun_family = AF_UNIX;
-        if (socketPathRel.size() >= sizeof(addr.sun_path))
-            throw Error(format("socket path `%1%' is too long") % 
socketPathRel);
-        strcpy(addr.sun_path, socketPathRel.c_str());
-
-        unlink(socketPath.c_str());
-
-        /* Make sure that the socket is created with 0666 permission
-           (everybody can connect --- provided they have access to the
-           directory containing the socket). */
-        mode_t oldMode = umask(0111);
-        int res = bind(fdSocket, (struct sockaddr *) &addr, sizeof(addr));
-        umask(oldMode);
-        if (res == -1)
-            throw SysError(format("cannot bind to socket `%1%'") % socketPath);
-
-        if (chdir("/") == -1) /* back to the root */
-            throw SysError("cannot change current directory");
-
-        if (listen(fdSocket, 5) == -1)
-            throw SysError(format("cannot listen on socket `%1%'") % 
socketPath);
-    }
-
-    closeOnExec(fdSocket);
-
-    /* Loop accepting connections. */
-    while (1) {
-
-        try {
-            /* Important: the server process *cannot* open the SQLite
-               database, because it doesn't like forks very much. */
-            assert(!store);
-
-            /* Accept a connection. */
-            struct sockaddr_un remoteAddr;
-            socklen_t remoteAddrLen = sizeof(remoteAddr);
-
-            AutoCloseFD remote = accept(fdSocket,
-                (struct sockaddr *) &remoteAddr, &remoteAddrLen);
-            checkInterrupt();
-            if (remote == -1) {
-                if (errno == EINTR)
-                    continue;
-                else
-                    throw SysError("accepting connection");
-            }
-
-            closeOnExec(remote);
-
-            bool trusted = false;
-            pid_t clientPid = -1;
+    uid_t clientUid = (uid_t) -1;
+    gid_t clientGid = (gid_t) -1;
 
+    try {
+       /* Important: the server process *cannot* open the SQLite
+          database, because it doesn't like forks very much. */
+       assert(!store);
+
+       /* Accept a connection. */
+       struct sockaddr_storage remoteAddr;
+       socklen_t remoteAddrLen = sizeof(remoteAddr);
+
+    try_again:
+       AutoCloseFD remote = accept(fdSocket,
+                                   (struct sockaddr *) &remoteAddr, 
&remoteAddrLen);
+       checkInterrupt();
+       if (remote == -1) {
+           if (errno == EINTR)
+               goto try_again;
+           else
+               throw SysError("accepting connection");
+       }
+
+       closeOnExec(remote);
+
+       pid_t clientPid = -1;
+       bool trusted = false;
+
+       /* Get the identity of the caller, if possible. */
+       if (remoteAddr.ss_family == AF_UNIX) {
 #if defined(SO_PEERCRED)
-            /* Get the identity of the caller, if possible. */
-            ucred cred;
-            socklen_t credLen = sizeof(cred);
-            if (getsockopt(remote, SOL_SOCKET, SO_PEERCRED, &cred, &credLen) 
== -1)
-                throw SysError("getting peer credentials");
+           ucred cred;
+           socklen_t credLen = sizeof(cred);
+           if (getsockopt(remote, SOL_SOCKET, SO_PEERCRED,
+                          &cred, &credLen) == -1)
+               throw SysError("getting peer credentials");
 
-            clientPid = cred.pid;
+           clientPid = cred.pid;
+           clientUid = cred.uid;
+           clientGid = cred.gid;
+           trusted = clientUid == 0;
 
             struct passwd * pw = getpwuid(cred.uid);
             string user = pw ? pw->pw_name : std::to_string(cred.uid);
 
-            struct group * gr = getgrgid(cred.gid);
-            string group = gr ? gr->gr_name : std::to_string(cred.gid);
-
-            Strings trustedUsers = settings.get("trusted-users", 
Strings({"root"}));
-            Strings allowedUsers = settings.get("allowed-users", 
Strings({"*"}));
-
-            if (matchUser(user, group, trustedUsers))
-                trusted = true;
-
-            if (!trusted && !matchUser(user, group, allowedUsers))
-                throw Error(format("user `%1%' is not allowed to connect to 
the Nix daemon") % user);
-
-            printMsg(lvlInfo, format((string) "accepted connection from pid 
%1%, user %2%"
-                    + (trusted ? " (trusted)" : "")) % clientPid % user);
+           printMsg(lvlInfo,
+                    format((string) "accepted connection from pid %1%, user 
%2%")
+                    % clientPid % user);
 #endif
-
-            /* Fork a child to handle the connection. */
-            startProcess([&]() {
-                fdSocket.close();
+       } else {
+           char address_str[128];
+           const char *result;
+
+           if (remoteAddr.ss_family == AF_INET) {
+               struct sockaddr_in *addr = (struct sockaddr_in *) &remoteAddr;
+               struct in_addr inaddr = { addr->sin_addr };
+               result = inet_ntop(AF_INET, &inaddr,
+                                  address_str, sizeof address_str);
+           } else if (remoteAddr.ss_family == AF_INET6) {
+               struct sockaddr_in6 *addr = (struct sockaddr_in6 *) &remoteAddr;
+               struct in6_addr inaddr = { addr->sin6_addr };
+               result = inet_ntop(AF_INET6, &inaddr,
+                                  address_str, sizeof address_str);
+           } else {
+               result = NULL;
+           }
+
+           if (result != NULL) {
+               printMsg(lvlInfo,
+                        format("accepted connection from %1%")
+                        % address_str);
+           }
+       }
+
+       /* Fork a child to handle the connection. */
+       startProcess([&]() {
+                close(fdSocket);
 
                 /* Background the daemon. */
                 if (setsid() == -1)
@@ -968,17 +905,11 @@ static void daemonLoop()
                     strncpy(argvSaved[1], processName.c_str(), 
strlen(argvSaved[1]));
                 }
 
-#if defined(SO_PEERCRED)
                 /* Store the client's user and group for this connection. This
                    has to be done in the forked process since it is per
-                   connection. */
-                settings.clientUid = cred.uid;
-                settings.clientGid = cred.gid;
-#else
-                /* Setting these to -1 means: do not change */
-                settings.clientUid = (uid_t) -1;
-                settings.clientGid = (gid_t) -1;
-#endif
+                   connection.  Setting these to -1 means: do not change.  */
+                settings.clientUid = clientUid;
+               settings.clientGid = clientGid;
 
                 /* Handle the connection. */
                 from.fd = remote;
@@ -988,23 +919,63 @@ static void daemonLoop()
                 exit(0);
             }, false, "unexpected Nix daemon error: ", true);
 
-        } catch (Interrupted & e) {
-            throw;
-        } catch (Error & e) {
-            printMsg(lvlError, format("error processing connection: %1%") % 
e.msg());
-        }
+    } catch (Interrupted & e) {
+       throw;
+    } catch (Error & e) {
+       printMsg(lvlError, format("error processing connection: %1%") % 
e.msg());
     }
 }
 
-
-void run(Strings args)
+static void daemonLoop(const std::vector<int>& sockets)
 {
-    for (Strings::iterator i = args.begin(); i != args.end(); ) {
-        string arg = *i++;
-        if (arg == "--daemon") /* ignored for backwards compatibility */;
+    if (chdir("/") == -1)
+        throw SysError("cannot change current directory");
+
+    /* Get rid of children automatically; don't let them become
+       zombies. */
+    setSigChldAction(true);
+
+    /* Mark sockets as close-on-exec.  */
+    for(int fd: sockets) {
+       closeOnExec(fd);
     }
 
-    daemonLoop();
+    /* Prepare the FD set corresponding to SOCKETS.  */
+    auto initializeFDSet = [&](fd_set *set) {
+       FD_ZERO(set);
+       for (int fd: sockets) {
+           FD_SET(fd, set);
+       }
+    };
+
+    /* Loop accepting connections. */
+    while (1) {
+       fd_set readfds;
+
+       initializeFDSet(&readfds);
+       int count =
+           select(*std::max_element(sockets.begin(), sockets.end()) + 1,
+                  &readfds, NULL, NULL,
+                  NULL);
+       if (count < 0) {
+           int err = errno;
+           if (err == EINTR)
+               continue;
+           throw SysError(format("select error: %1%") % strerror(err));
+       }
+
+       for (unsigned int i = 0; i < sockets.size(); i++) {
+           if (FD_ISSET(sockets[i], &readfds)) {
+               acceptConnection(sockets[i]);
+           }
+       }
+    }
+}
+
+
+void run(const std::vector<int>& sockets)
+{
+    daemonLoop(sockets);
 }
 
 
diff --git a/tests/guix-daemon.sh b/tests/guix-daemon.sh
index 9186ffd..7212e3e 100644
--- a/tests/guix-daemon.sh
+++ b/tests/guix-daemon.sh
@@ -81,6 +81,18 @@ guile -c "
 
 kill "$daemon_pid"
 
+# Pass several '--listen' options, and make sure they are all honored.
+guix-daemon --disable-chroot --listen="$socket" --listen="$socket-second" \
+           --listen="localhost" --listen="localhost:9876" &
+daemon_pid=$!
+
+for uri in "$socket" "$socket-second" \
+                    "guix://localhost" "guix://localhost:9876"
+do
+    GUIX_DAEMON_SOCKET="$uri" guix build guile-bootstrap
+done
+
+kill "$daemon_pid"
 
 # Check the failed build cache.