[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]

Patch to authenticate by uid/gid on ControlSocket



Hi all,

The attached patch allows tor (0.2.0.34) to automatically authenticate
the client of a Unix-domain control socket, by asking the kernel for the
uid and gid of the remote user.  There are still some unresolved issues,
but feedback would be appreciated.

Unauthenticated connections on Unix-domain sockets are now disabled,
even if no authentication is configured; however, tor will always accept
control connections if the client has the same uid and gid as the server
(regardless of authentication settings).  The new config file parameters
"ControlAllowUsers" and "ControlAllowGroups" can be used to allow other
users.  Each takes a comma-separated list of user/group names; for
groups, any member of a group (as determined by /etc/group) is accepted.
This is similar to the AllowUsers/AllowGroups settings in openssh.

The problem I've run into is that the listener socket permissions are
affected by the umask, and Linux won't let users without write
permission connect (apparently other systems ignore the permissions).
The obvious solution of fchmod(fd, 0777) has no effect; other possible
solutions are:
 - Use chmod; this would be vulnerable to a race condition if another
   user could delete and replace the socket before the chmod, but it
   should be okay if there's a warning in the documentation.
 - Change the umask temporarily; this would affect all threads, so it's
   only an option if filesystem operations are restricted to one thread.

Any suggestions?

So far this patch has only been tested on Linux, but it seems to work
fine after manually fixing the socket permissions.  Testing on other
Unix-like systems would be helpful, if anyone's interested (run
autoreconf before building it).

-- Michael
diff --git a/configure.in b/configure.in
index e733ad9..d7c0a25 100644
--- a/configure.in
+++ b/configure.in
@@ -18,7 +18,7 @@ fi
 
 # Not a no-op; we want to make sure that CPPFLAGS is set before we use
 # the += operator on it in src/or/Makefile.am
-CPPFLAGS="$CPPFLAGS -I../common"
+CPPFLAGS="$CPPFLAGS -I\${top_srcdir}/src/common"
 
 AC_ARG_ENABLE(debug,
  AS_HELP_STRING(--enable-debug, compile with debugging info),
@@ -183,7 +183,7 @@ dnl -------------------------------------------------------------------
 dnl Check for functions before libevent, since libevent-1.2 apparently
 dnl exports strlcpy without defining it in a header.
 
-AC_CHECK_FUNCS(gettimeofday ftime socketpair uname inet_aton strptime getrlimit strlcat strlcpy strtoull ftello getaddrinfo localtime_r gmtime_r memmem strtok_r inet_pton inet_ntop)
+AC_CHECK_FUNCS(gettimeofday ftime socketpair uname inet_aton strptime getrlimit strlcat strlcpy strtoull ftello getaddrinfo localtime_r gmtime_r memmem strtok_r inet_pton inet_ntop getpeereid)
 
 using_custom_malloc=no
 if test x$enable_openbsd_malloc = xyes ; then
diff --git a/src/common/compat.c b/src/common/compat.c
index 681d43b..beaafb6 100644
--- a/src/common/compat.c
+++ b/src/common/compat.c
@@ -1141,6 +1141,22 @@ switch_id(const char *user)
 #endif
 }
 
+#ifdef NEED_GETPEEREID
+int
+tor_getpeereid(int sockfd, uid_t *euid, gid_t *egid)
+{
+  struct ucred cred;
+  socklen_t credlen = sizeof(cred);
+  if (getsockopt(sockfd, SOL_SOCKET, SO_PEERCRED, &cred, &credlen) < 0)
+    return -1;
+
+  tor_assert(credlen == sizeof(cred));
+  *euid = cred.uid;
+  *egid = cred.gid;
+  return 0;
+}
+#endif
+
 #ifdef HAVE_PWD_H
 /** Allocate and return a string containing the home directory for the
  * user <b>username</b>. Only works on posix-like systems. */
diff --git a/src/common/compat.h b/src/common/compat.h
index 2a8cba3..fac4773 100644
--- a/src/common/compat.h
+++ b/src/common/compat.h
@@ -420,6 +420,19 @@ int network_init(void);
 
 int tor_addr_lookup(const char *name, uint16_t family, tor_addr_t *addr_out);
 
+#ifdef HAVE_GETPEEREID
+#define HAVE_COMPAT_GETPEEREID
+#define tor_getpeereid getpeereid
+#elif defined(SO_PEERCRED)
+#define HAVE_COMPAT_GETPEEREID
+#define NEED_GETPEEREID
+int tor_getpeereid(int s, uid_t *euid, gid_t *egid);
+#endif
+
+#if defined(HAVE_COMPAT_GETPEEREID) && defined(HAVE_PWD_H)
+#define AF_UNIX_UID_AUTH
+#endif
+
 /* For stupid historical reasons, windows sockets have an independent
  * set of errnos, and an independent way to get them.  Also, you can't
  * always believe WSAEWOULDBLOCK.  Use the macros below to compare
diff --git a/src/or/config.c b/src/or/config.c
index 4267cb1..a7a5159 100644
--- a/src/or/config.c
+++ b/src/or/config.c
@@ -168,6 +168,8 @@ static config_var_t _option_vars[] = {
   V(ControlListenAddress,        LINELIST, NULL),
   V(ControlPort,                 UINT,     "0"),
   V(ControlSocket,               LINELIST, NULL),
+  V(ControlAllowUsers,           CSV, NULL),
+  V(ControlAllowGroups,          CSV, NULL),
   V(CookieAuthentication,        BOOL,     "0"),
   V(CookieAuthFileGroupReadable, BOOL,     "0"),
   V(CookieAuthFile,              STRING,   NULL),
@@ -382,6 +384,16 @@ static config_var_description_t options_description[] = {
     "(localhost only) on this port, and allow those connections to control "
     "the Tor process using the Tor Control Protocol (described in"
     "control-spec.txt).", },
+  { "ControlSocket", "If set, Tor will accept connections on a Unix domain "
+    "socket with the given path, and allow those connections to control "
+    "the Tor process using the Tor Control Protocol (described in"
+    "control-spec.txt).  Unix only.", },
+  { "ControlAllowUsers", "If set, Tor will treat control socket connections "
+    "from any of the specified users as authenticated.  "
+    "This works on sockets defined with ControlSocket (not ControlPort).", },
+  { "ControlAllowGroups", "If set, Tor will treat control socket connections "
+    "from a user in any of the specified groups as authenticated.  "
+    "This works on sockets defined with ControlSocket (not ControlPort).", },
   { "CookieAuthentication", "If this option is set to 1, don't allow any "
     "connections to the control port except when the connecting process "
     "can read a file that Tor creates in its data directory." },
diff --git a/src/or/connection.c b/src/or/connection.c
index 697cbcf..90aa39c 100644
--- a/src/or/connection.c
+++ b/src/or/connection.c
@@ -835,6 +835,7 @@ connection_create_listener(struct sockaddr *listensockaddr, int type,
                        strerror(errno));
       goto err;
     }
+
     s = tor_open_socket(AF_UNIX, SOCK_STREAM, 0);
     if (s < 0) {
       log_warn(LD_NET,"Socket creation failed: %s.", strerror(errno));
@@ -853,6 +854,14 @@ connection_create_listener(struct sockaddr *listensockaddr, int type,
       tor_close_socket(s);
       goto err;
     }
+#ifdef AF_UNIX_UID_AUTH
+    /*FIXME: this has no effect on Linux */
+    if (fchmod(s, 0777) < 0) {
+      log_warn(LD_NET,"Could not set permissions on %s: %s.", address,
+               tor_socket_strerror(tor_socket_errno(s)));
+      goto err;
+    }
+#endif
 #endif /* HAVE_SYS_UN_H */
   } else {
       log_err(LD_BUG,"Got unexpected address family %d.",
@@ -1085,8 +1094,7 @@ connection_init_accepted_conn(connection_t *conn, uint8_t listener_type)
       conn->state = DIR_CONN_STATE_SERVER_COMMAND_WAIT;
       break;
     case CONN_TYPE_CONTROL:
-      conn->state = CONTROL_CONN_STATE_NEEDAUTH;
-      break;
+      return connection_control_init_accepted_conn(TO_CONTROL_CONN(conn));
   }
   return 0;
 }
diff --git a/src/or/control.c b/src/or/control.c
index 8aa5c6c..1ecebc8 100644
--- a/src/or/control.c
+++ b/src/or/control.c
@@ -13,6 +13,7 @@ const char control_c_id[] =
 
 #define CONTROL_PRIVATE
 
+#define _BSD_SOURCE  /* for getgrouplist, getpwnam_r, etc. */
 #include "or.h"
 
 /** Yield true iff <b>s</b> is the state of a control_connection_t that has
@@ -1004,6 +1005,12 @@ handle_control_authenticate(control_connection_t *conn, uint32_t len,
   int bad_cookie=0, bad_password=0;
   smartlist_t *sl = NULL;
 
+  if (conn->_base.state == CONTROL_CONN_STATE_OPEN) {
+    /* the connection is already authenticated, so don't check again */
+    send_control_done(conn);
+    return 0;
+  }
+
   if (TOR_ISXDIGIT(body[0])) {
     cp = body;
     while (TOR_ISXDIGIT(*cp))
@@ -1034,13 +1041,6 @@ handle_control_authenticate(control_connection_t *conn, uint32_t len,
     used_quoted_string = 1;
   }
 
-  if (!options->CookieAuthentication && !options->HashedControlPassword &&
-      !options->HashedControlSessionPassword) {
-    /* if Tor doesn't demand any stronger authentication, then
-     * the controller can get in with anything. */
-    goto ok;
-  }
-
   if (options->CookieAuthentication) {
     int also_password = options->HashedControlPassword != NULL ||
       options->HashedControlSessionPassword != NULL;
@@ -2642,6 +2642,179 @@ connection_control_reached_eof(control_connection_t *conn)
   return 0;
 }
 
+/** Called to initialise a new control socket. */
+int
+connection_control_init_accepted_conn(control_connection_t *conn)
+{
+  int auth = 0;
+  or_options_t *options = get_options();
+  conn->_base.state = CONTROL_CONN_STATE_NEEDAUTH;
+
+  if (!options->CookieAuthentication &&
+      !options->HashedControlPassword &&
+      !options->HashedControlSessionPassword &&
+#ifdef AF_UNIX_UID_AUTH
+      conn->_base.socket_family != AF_UNIX &&
+#endif
+      !options->ControlAllowUsers &&
+      !options->ControlAllowGroups) {
+    /* if Tor doesn't demand any stronger authentication, then
+     * the controller can get in with anything. */
+    auth = 1;
+  }
+
+#ifdef AF_UNIX_UID_AUTH
+  if (!auth && conn->_base.socket_family == AF_UNIX) {
+    int sock = conn->_base.s;
+    uid_t sockuid;
+    gid_t sockgid;
+    struct passwd _pwd;
+    char *pwdbuf;
+    size_t pwdbufsize;
+
+    if (tor_getpeereid(sock, &sockuid, &sockgid) < 0) {
+      log_warn(LD_CONTROL, "Couldn't authenticate Unix-domain control "
+                           "socket: %s", strerror(errno));
+      goto skip_unixauth;
+    }
+
+    if (sockuid == getuid()) {
+      /* Always let a user access their own server. */
+      log_info(LD_CONTROL, "Authenticated control connection (%d) "
+                           "via own UID (%d)",
+               sock, sockuid);
+      auth = 1;
+      goto skip_unixauth;
+    }
+
+    pwdbufsize = sysconf(_SC_GETPW_R_SIZE_MAX);
+    if (pwdbufsize == -1)
+      pwdbufsize = 16384;
+    pwdbuf = tor_malloc(pwdbufsize);
+
+    if (options->ControlAllowUsers) {
+      SMARTLIST_FOREACH(options->ControlAllowUsers, const char *, name,
+      {
+        struct passwd *pwd;
+        errno = getpwnam_r(name, &_pwd, pwdbuf, pwdbufsize, &pwd);
+        if (!pwd) {
+          if (errno)
+            log_warn(LD_CONTROL, "Couldn't get info for username \"%s\" "
+                                 "in ControlAllowUsers: %s",
+                     name, strerror(errno));
+          else
+            log_warn(LD_CONTROL, "Invalid username \"%s\" in "
+                                 "ControlAllowUsers", name);
+        } else {
+          if (sockuid == pwd->pw_uid) {
+            log_info(LD_CONTROL, "Authenticated control connection (%d) via "
+                                 "ControlAllowUsers (username \"%s\")",
+                     sock, name);
+            auth = 1;
+            break;
+          }
+        }
+      });
+    }
+
+    if (!auth && options->ControlAllowGroups) {
+      struct passwd *sockpwd;
+      gid_t *sockgroups = NULL;
+      const int maxgroups = 32;
+      int ngroups = 0;
+
+      struct group _grp;
+      char *grpbuf;
+      size_t grpbufsize;
+
+      /* Find the user's groups. */
+
+      sockgroups = tor_malloc(sizeof(gid_t) * maxgroups);
+
+      errno = getpwuid_r(sockuid, &_pwd, pwdbuf, pwdbufsize, &sockpwd);
+      if (sockpwd) {
+        int rv, oldn = ngroups;
+
+        ngroups = maxgroups;
+        rv = getgrouplist(sockpwd->pw_name, sockpwd->pw_gid,
+                          sockgroups, &ngroups);
+        if (rv < 0 && ngroups > oldn) {
+          /* retry with a bigger buffer */
+          sockgroups = tor_realloc(sockgroups, sizeof(gid_t) * ngroups);
+          rv = getgrouplist(sockpwd->pw_name, sockpwd->pw_gid,
+                            sockgroups, &ngroups);
+        }
+
+        if (rv < 0) {
+          /* the getgrouplist man page doesn't say whether it sets errno */
+          log_warn(LD_CONTROL, "Couldn't get group list for connecting user "
+                               "\"%s\" to check ControlAllowGroups",
+                   sockpwd->pw_name);
+          ngroups = 0;
+        }
+      } else {  /* getpwuid failed */
+        if (errno)
+          log_warn(LD_CONTROL, "Couldn't get user info for connecting UID %d "
+                               "to check ControlAllowGroups: %s",
+                   sockuid, strerror(errno));
+        /* else, it's a UID with no username */
+
+        /* we can still check the gid returned by getpeereid */
+        ngroups = 0;
+      }
+
+      /* Check whether the user is in an acceptable group. */
+
+      grpbufsize = sysconf(_SC_GETGR_R_SIZE_MAX);
+      if (grpbufsize == -1)
+        grpbufsize = 16384;
+      grpbuf = tor_malloc(grpbufsize);
+
+      SMARTLIST_FOREACH(options->ControlAllowGroups, const char *, name,
+      {
+        struct group *grp;
+        errno = getgrnam_r(name, &_grp, grpbuf, grpbufsize, &grp);
+        if (!grp) {
+          if (errno)
+            log_warn(LD_CONTROL, "Couldn't get info for group \"%s\" in "
+                                 "ControlAllowGroups: %s",
+                     name, strerror(errno));
+          else
+            log_warn(LD_CONTROL, "Invalid group name \"%s\" in "
+                                 "ControlAllowGroups", name);
+        } else {
+          int i;
+          auth = (sockgid == grp->gr_gid ||
+                  (sockpwd && sockpwd->pw_gid == grp->gr_gid));
+          for (i = 0; i < ngroups && !auth; ++i) {
+            if (sockgroups[i] == grp->gr_gid) auth = 1;
+          }
+
+          if (auth) {
+            /* log the numeric UID since pwd may be null */
+            log_info(LD_CONTROL, "Authenticated control connection (%d) via "
+                                 "ControlAllowGroups (group \"%s\", UID %d)",
+                     sock, grp->gr_name, sockuid);
+            break;
+          }
+        }
+      });
+
+      tor_free(sockgroups);
+      tor_free(grpbuf);
+    }
+
+    tor_free(pwdbuf);
+skip_unixauth: ;
+  }
+#endif
+
+  if (auth)
+    conn->_base.state = CONTROL_CONN_STATE_OPEN;
+
+  return 0;
+}
+
 /** Return true iff <b>cmd</b> is allowable (or at least forgivable) at this
  * stage of the protocol. */
 static int
diff --git a/src/or/or.h b/src/or/or.h
index 9e926b4..1188fb1 100644
--- a/src/or/or.h
+++ b/src/or/or.h
@@ -64,6 +64,12 @@
 #ifdef HAVE_TIME_H
 #include <time.h>
 #endif
+#ifdef HAVE_PWD_H
+#include <pwd.h>
+#endif
+#ifdef HAVE_GRP_H
+#include <grp.h>
+#endif
 
 #ifdef MS_WINDOWS
 #include <io.h>
@@ -2060,6 +2066,10 @@ typedef struct {
   int ControlPort; /**< Port to listen on for control connections. */
   config_line_t *ControlSocket; /**< List of Unix Domain Sockets to listen on
                                  * for control connections. */
+  smartlist_t *ControlAllowUsers; /**< List of usernames that should have
+                                   * access to the control socket. */
+  smartlist_t *ControlAllowGroups; /**< List of group names that should have
+                                    * access to the control socket. */
   int DirPort; /**< Port to listen on for directory connections. */
   int DNSPort; /**< Port to listen on for DNS requests. */
   int AssumeReachable; /**< Whether to publish our descriptor regardless. */
@@ -2956,6 +2966,7 @@ void control_adjust_event_log_severity(void);
 
 int connection_control_finished_flushing(control_connection_t *conn);
 int connection_control_reached_eof(control_connection_t *conn);
+int connection_control_init_accepted_conn(control_connection_t *conn);
 int connection_control_process_inbuf(control_connection_t *conn);
 
 #define EVENT_AUTHDIR_NEWDESCS 0x000D

Attachment: signature.asc
Description: Digital signature