[tor-commits] [tor/master] Allow {World, Group}Writable on AF_UNIX {Socks, Control}Ports.

nickm at torproject.org nickm at torproject.org
Thu Jul 16 19:39:44 UTC 2015


commit 809517a8634b320ddfced409ac6ea90e8c0b9344
Author: Nick Mathewson <nickm at torproject.org>
Date:   Wed Mar 11 13:26:14 2015 -0400

    Allow {World,Group}Writable on AF_UNIX {Socks,Control}Ports.
    
    Closes ticket 15220
---
 changes/feature15220 |    5 +++++
 doc/tor.1.txt        |   16 ++++++++++++++-
 src/or/config.c      |   55 +++++++++++++++++++++++++++++++++++++-------------
 src/or/connection.c  |   46 +++++++++++++++++++++--------------------
 src/or/or.h          |    3 +++
 5 files changed, 88 insertions(+), 37 deletions(-)

diff --git a/changes/feature15220 b/changes/feature15220
new file mode 100644
index 0000000..6cab36d
--- /dev/null
+++ b/changes/feature15220
@@ -0,0 +1,5 @@
+  o Minor features (client, unix sockets):
+    - Add GroupWritable and WorldWritable options to unix-socket based
+      SocksPort and ControlPort options. These options apply to a single
+      socket, and override {Control,Socks}SocketsGroupWritable. Closes
+      ticket 15220.
diff --git a/doc/tor.1.txt b/doc/tor.1.txt
index e136bd0..b2ee848 100644
--- a/doc/tor.1.txt
+++ b/doc/tor.1.txt
@@ -274,7 +274,7 @@ GENERAL OPTIONS
     all sockets will be set to this limit. Must be a value between 2048 and
     262144, in 1024 byte increments. Default of 8192 is recommended.
 
-[[ControlPort]] **ControlPort** __PORT__|**unix:**__path__|**auto**::
+[[ControlPort]] **ControlPort** __PORT__|**unix:**__path__|**auto** [__flags__]::
     If set, Tor will accept connections on this port and allow those
     connections to control the Tor process using the Tor Control Protocol
     (described in control-spec.txt). Note: unless you also specify one or
@@ -284,6 +284,14 @@ GENERAL OPTIONS
     method is sufficient to authenticate to Tor.) This
     option is required for many Tor controllers; most use the value of 9051.
     Set it to "auto" to have Tor pick a port for you. (Default: 0)
+ +
+    Recognized flags are::
+    **GroupWritable**;;
+        Unix domain sockets only: makes the socket get created as
+        group-writable.
+    **WorldWritable**;;
+        Unix domain sockets only: makes the socket get created as
+        world-writable.
 
 [[ControlListenAddress]] **ControlListenAddress** __IP__[:__PORT__]::
     Bind the controller listener to this address. If you specify a port, bind
@@ -1015,6 +1023,12 @@ The following options are useful only for clients (that is, if
     **CacheIPv6DNS**;;
         Tells the client to remember IPv6 DNS answers we receive from exit
         nodes via this connection.
+    **GroupWritable**;;
+        Unix domain sockets only: makes the socket get created as
+        group-writable.
+    **WorldWritable**;;
+        Unix domain sockets only: makes the socket get created as
+        world-writable.
     **CacheDNS**;;
         Tells the client to remember all DNS answers we receive from exit
         nodes via this connection.
diff --git a/src/or/config.c b/src/or/config.c
index 5ba8c99..db35396 100644
--- a/src/or/config.c
+++ b/src/or/config.c
@@ -5618,7 +5618,7 @@ warn_nonlocal_ext_orports(const smartlist_t *ports, const char *portname)
  * then emit a stronger warning and remove the port from the list.
  */
 static void
-warn_nonlocal_controller_ports(smartlist_t *ports, unsigned forbid)
+warn_nonlocal_controller_ports(smartlist_t *ports, unsigned forbid_nonlocal)
 {
   int warned = 0;
   SMARTLIST_FOREACH_BEGIN(ports, port_cfg_t *, port) {
@@ -5627,7 +5627,7 @@ warn_nonlocal_controller_ports(smartlist_t *ports, unsigned forbid)
     if (port->is_unix_addr)
       continue;
     if (!tor_addr_is_loopback(&port->addr)) {
-      if (forbid) {
+      if (forbid_nonlocal) {
         if (!warned)
           log_warn(LD_CONFIG,
                  "You have a ControlPort set to accept "
@@ -5655,13 +5655,14 @@ warn_nonlocal_controller_ports(smartlist_t *ports, unsigned forbid)
   } SMARTLIST_FOREACH_END(port);
 }
 
-#define CL_PORT_NO_OPTIONS (1u<<0)
+#define CL_PORT_NO_STREAM_OPTIONS (1u<<0)
 #define CL_PORT_WARN_NONLOCAL (1u<<1)
 #define CL_PORT_ALLOW_EXTRA_LISTENADDR (1u<<2)
 #define CL_PORT_SERVER_OPTIONS (1u<<3)
 #define CL_PORT_FORBID_NONLOCAL (1u<<4)
 #define CL_PORT_TAKES_HOSTNAMES (1u<<5)
 #define CL_PORT_IS_UNIXSOCKET (1u<<6)
+#define CL_PORT_DFLT_GROUP_WRITABLE (1u<<7)
 
 #ifdef HAVE_SYS_UN_H
 
@@ -5729,7 +5730,7 @@ config_parse_unix_port(const char *addrport, char **path_out)
  * If no address is specified, default to <b>defaultaddr</b>.  If no
  * FooPort is given, default to defaultport (if 0, there is no default).
  *
- * If CL_PORT_NO_OPTIONS is set in <b>flags</b>, do not allow stream
+ * If CL_PORT_NO_STREAM_OPTIONS is set in <b>flags</b>, do not allow stream
  * isolation options in the FooPort entries.
  *
  * If CL_PORT_WARN_NONLOCAL is set in <b>flags</b>, warn if any of the
@@ -5764,10 +5765,12 @@ parse_port_config(smartlist_t *out,
   int retval = -1;
   const unsigned is_control = (listener_type == CONN_TYPE_CONTROL_LISTENER);
   const unsigned is_ext_orport = (listener_type == CONN_TYPE_EXT_OR_LISTENER);
-  const unsigned allow_no_options = flags & CL_PORT_NO_OPTIONS;
+  const unsigned allow_no_stream_options = flags & CL_PORT_NO_STREAM_OPTIONS;
   const unsigned use_server_options = flags & CL_PORT_SERVER_OPTIONS;
   const unsigned warn_nonlocal = flags & CL_PORT_WARN_NONLOCAL;
   const unsigned forbid_nonlocal = flags & CL_PORT_FORBID_NONLOCAL;
+  const unsigned default_to_group_writable =
+    flags & CL_PORT_DFLT_GROUP_WRITABLE;
   const unsigned allow_spurious_listenaddr =
     flags & CL_PORT_ALLOW_EXTRA_LISTENADDR;
   const unsigned takes_hostnames = flags & CL_PORT_TAKES_HOSTNAMES;
@@ -5891,7 +5894,7 @@ parse_port_config(smartlist_t *out,
       ipv4_traffic = 1, ipv6_traffic = 0, prefer_ipv6 = 0,
       cache_ipv4 = 1, use_cached_ipv4 = 0,
       cache_ipv6 = 0, use_cached_ipv6 = 0,
-      prefer_ipv6_automap = 1;
+      prefer_ipv6_automap = 1, world_writable = 0, group_writable = 0;
 
     smartlist_split_string(elts, ports->value, NULL,
                            SPLIT_SKIP_SPACE|SPLIT_IGNORE_BLANK, 0);
@@ -5900,11 +5903,6 @@ parse_port_config(smartlist_t *out,
       goto err;
     }
 
-    if (allow_no_options && smartlist_len(elts) > 1) {
-      log_warn(LD_CONFIG, "Too many options on %sPort line", portname);
-      goto err;
-    }
-
     /* Now parse the addr/port value */
     addrport = smartlist_get(elts, 0);
 
@@ -5962,6 +5960,9 @@ parse_port_config(smartlist_t *out,
       }
     }
 
+    if (unix_socket_path && default_to_group_writable)
+      group_writable = 1;
+
     /* Now parse the rest of the options, if any. */
     if (use_server_options) {
       /* This is a server port; parse advertising options */
@@ -6018,10 +6019,11 @@ parse_port_config(smartlist_t *out,
         const char *elt_orig = elt;
         if (elt_sl_idx == 0)
           continue; /* Skip addr:port */
+
         if (!strcasecmpstart(elt, "SessionGroup=")) {
           int group = (int)tor_parse_long(elt+strlen("SessionGroup="),
                                           10, 0, INT_MAX, &ok, NULL);
-          if (!ok) {
+          if (!ok || !allow_no_stream_options) {
             log_warn(LD_CONFIG, "Invalid %sPort option '%s'",
                      portname, escaped(elt));
             goto err;
@@ -6040,6 +6042,18 @@ parse_port_config(smartlist_t *out,
           elt += 2;
         }
 
+        if (!strcasecmp(elt, "GroupWritable")) {
+          group_writable = !no;
+        } else if (!strcasecmp(elt, "WorldWritable")) {
+          world_writable = !no;
+        }
+
+        if (allow_no_stream_options) {
+          log_warn(LD_CONFIG, "Unrecognized %sPort option '%s'",
+                   portname, escaped(elt));
+          continue;
+        }
+
         if (takes_hostnames) {
           if (!strcasecmp(elt, "IPv4Traffic")) {
             ipv4_traffic = ! no;
@@ -6115,6 +6129,12 @@ parse_port_config(smartlist_t *out,
       goto err;
     }
 
+    if ( (world_writable || group_writable) && ! unix_socket_path) {
+      log_warn(LD_CONFIG, "You have a %sPort entry with GroupWritable "
+               "or WorldWritable set, but it is not a unix socket.", portname);
+      goto err;
+    }
+
     if (out && port) {
       size_t namelen = unix_socket_path ? strlen(unix_socket_path) : 0;
       port_cfg_t *cfg = port_cfg_new(namelen);
@@ -6128,6 +6148,8 @@ parse_port_config(smartlist_t *out,
         cfg->port = port;
       }
       cfg->type = listener_type;
+      cfg->is_world_writable = world_writable;
+      cfg->is_group_writable = group_writable;
       cfg->entry_cfg.isolation_flags = isolation;
       cfg->entry_cfg.session_group = sessiongroup;
       cfg->server_cfg.no_advertise = no_advertise;
@@ -6214,12 +6236,14 @@ parse_ports(or_options_t *options, int validate_only,
 
   *n_ports_out = 0;
 
+  const unsigned gw_flag = options->SocksSocketsGroupWritable ?
+    CL_PORT_DFLT_GROUP_WRITABLE : 0;
   if (parse_port_config(ports,
              options->SocksPort_lines, options->SocksListenAddress,
              "Socks", CONN_TYPE_AP_LISTENER,
              "127.0.0.1", 9050,
              CL_PORT_WARN_NONLOCAL|CL_PORT_ALLOW_EXTRA_LISTENADDR|
-             CL_PORT_TAKES_HOSTNAMES) < 0) {
+             CL_PORT_TAKES_HOSTNAMES|gw_flag) < 0) {
     *msg = tor_strdup("Invalid SocksPort/SocksListenAddress configuration");
     goto err;
   }
@@ -6248,12 +6272,15 @@ parse_ports(or_options_t *options, int validate_only,
     goto err;
   }
   {
-    unsigned control_port_flags = CL_PORT_NO_OPTIONS | CL_PORT_WARN_NONLOCAL;
+    unsigned control_port_flags = CL_PORT_NO_STREAM_OPTIONS |
+      CL_PORT_WARN_NONLOCAL;
     const int any_passwords = (options->HashedControlPassword ||
                                options->HashedControlSessionPassword ||
                                options->CookieAuthentication);
     if (! any_passwords)
       control_port_flags |= CL_PORT_FORBID_NONLOCAL;
+    if (options->ControlSocketsGroupWritable)
+      control_port_flags |= CL_PORT_DFLT_GROUP_WRITABLE;
 
     if (parse_port_config(ports,
                           options->ControlPort_lines,
diff --git a/src/or/connection.c b/src/or/connection.c
index 7db0238..1fda57b 100644
--- a/src/or/connection.c
+++ b/src/or/connection.c
@@ -972,7 +972,7 @@ unix_socket_purpose_to_string(int purpose)
  * <b>path</b>.  Return 0 if we should go ahead and -1 if we shouldn't. */
 static int
 check_location_for_unix_socket(const or_options_t *options, const char *path,
-                               int purpose)
+                               int purpose, const port_cfg_t *port)
 {
   int r = -1;
   char *p = NULL;
@@ -987,10 +987,13 @@ check_location_for_unix_socket(const or_options_t *options, const char *path,
     goto done;
   }
 
-  if ((purpose == UNIX_SOCKET_PURPOSE_CONTROL_SOCKET &&
-       options->ControlSocketsGroupWritable) ||
-      (purpose == UNIX_SOCKET_PURPOSE_SOCKS_SOCKET &&
-       options->SocksSocketsGroupWritable)) {
+  if (port->is_world_writable) {
+    /* World-writable sockets can go anywhere. */
+    r = 0;
+    goto done;
+  }
+
+  if (port->is_group_writable) {
     flags |= CPD_GROUP_OK;
   }
 
@@ -1004,7 +1007,7 @@ check_location_for_unix_socket(const or_options_t *options, const char *path,
              "who can list a socket can connect to it, so Tor is being "
              "careful.)",
              unix_socket_purpose_to_string(purpose), escpath, escdir,
-             options->ControlSocketsGroupWritable ? " and group" : "");
+             port->is_group_writable ? " and group" : "");
     tor_free(escpath);
     tor_free(escdir);
     goto done;
@@ -1198,7 +1201,7 @@ connection_listener_new(const struct sockaddr *listensockaddr,
     if (check_location_for_unix_socket(options, address,
           (type == CONN_TYPE_CONTROL_LISTENER) ?
            UNIX_SOCKET_PURPOSE_CONTROL_SOCKET :
-           UNIX_SOCKET_PURPOSE_SOCKS_SOCKET) < 0) {
+           UNIX_SOCKET_PURPOSE_SOCKS_SOCKET, port_cfg) < 0) {
         goto err;
     }
 
@@ -1241,24 +1244,23 @@ connection_listener_new(const struct sockaddr *listensockaddr,
     }
 #endif
 
-    if ((type == CONN_TYPE_CONTROL_LISTENER &&
-         options->ControlSocketsGroupWritable) ||
-        (type == CONN_TYPE_AP_LISTENER &&
-         options->SocksSocketsGroupWritable)) {
-      /* We need to use chmod; fchmod doesn't work on sockets on all
-       * platforms. */
-      if (chmod(address, 0660) < 0) {
-        log_warn(LD_FS,"Unable to make %s group-writable.", address);
-        goto err;
+    {
+      unsigned mode;
+      const char *status;
+      if (port_cfg->is_world_writable) {
+        mode = 0666;
+        status = "world-writable";
+      } else if (port_cfg->is_group_writable) {
+        mode = 0660;
+        status = "group-writable";
+      } else {
+        mode = 0600;
+        status = "private";
       }
-    } else if ((type == CONN_TYPE_CONTROL_LISTENER &&
-                !(options->ControlSocketsGroupWritable)) ||
-               (type == CONN_TYPE_AP_LISTENER &&
-                !(options->SocksSocketsGroupWritable))) {
       /* We need to use chmod; fchmod doesn't work on sockets on all
        * platforms. */
-      if (chmod(address, 0600) < 0) {
-        log_warn(LD_FS,"Unable to make %s group-writable.", address);
+      if (chmod(address, mode) < 0) {
+        log_warn(LD_FS,"Unable to make %s %s.", address, status);
         goto err;
       }
     }
diff --git a/src/or/or.h b/src/or/or.h
index 6723f93..3350f3b 100644
--- a/src/or/or.h
+++ b/src/or/or.h
@@ -3322,6 +3322,9 @@ typedef struct port_cfg_t {
   uint8_t type; /**< One of CONN_TYPE_*_LISTENER */
   unsigned is_unix_addr : 1; /**< True iff this is an AF_UNIX address. */
 
+  unsigned is_group_writable : 1;
+  unsigned is_world_writable : 1;
+
   entry_port_cfg_t entry_cfg;
 
   server_port_cfg_t server_cfg;





More information about the tor-commits mailing list