Merge remote-tracking branch 'weasel/bug18458'

changes/bug18458

@@ -0,0 +1,9 @@
+  o Minor features:
+    - Since some operating systems do not consider the actual modes on a
+      UNIX domain socket itself, tor does not allow creating such a
+      socket in a directory that is group or world accessible if it is
+      supposed to be private.  Likewise, it will not allow only group
+      accessible sockets in a world accessible directory.
+      However, on some operating systems this is unnecessary, so
+      add a per-socket option called RelaxDirModeCheck.
+      Fixes bug 18458.  Patch by weasel.

doc/tor.1.txt

@@ -321,6 +321,9 @@ GENERAL OPTIONS
     **WorldWritable**;;
         Unix domain sockets only: makes the socket get created as
         world-writable.
+    **RelaxDirModeCheck**;;
+        Unix domain sockets only: Do not insist that the directory
+        that holds the socket be read-restricted.
 
 [[ControlListenAddress]] **ControlListenAddress** __IP__[:__PORT__]::
     Bind the controller listener to this address. If you specify a port, bind

src/common/util.c

@@ -2063,7 +2063,6 @@ check_private_dir(const char *dirname, cpd_check_t check,
 
 #ifndef _WIN32
   int fd;
-  unsigned unwanted_bits = 0;
   const struct passwd *pw = NULL;
   uid_t running_uid;
   gid_t running_gid;
@@ -2200,12 +2199,17 @@ check_private_dir(const char *dirname, cpd_check_t check,
     close(fd);
     return -1;
   }
+  unsigned unwanted_bits = 0;
   if (check & (CPD_GROUP_OK|CPD_GROUP_READ)) {
     unwanted_bits = 0027;
   } else {
     unwanted_bits = 0077;
   }
-  if ((st.st_mode & unwanted_bits) != 0) {
+  unsigned check_bits_filter = ~0;
+  if (check & CPD_RELAX_DIRMODE_CHECK) {
+    check_bits_filter = 0022;
+  }
+  if ((st.st_mode & unwanted_bits & check_bits_filter) != 0) {
     unsigned new_mode;
     if (check & CPD_CHECK_MODE_ONLY) {
       log_warn(LD_FS, "Permissions on directory %s are too permissive.",

src/common/util.h

@@ -357,12 +357,13 @@ file_status_t file_status(const char *filename);
 /** Possible behaviors for check_private_dir() on encountering a nonexistent
  * directory; see that function's documentation for details. */
 typedef unsigned int cpd_check_t;
-#define CPD_NONE 0
-#define CPD_CREATE 1
-#define CPD_CHECK 2
-#define CPD_GROUP_OK 4
-#define CPD_GROUP_READ 8
-#define CPD_CHECK_MODE_ONLY 16
+#define CPD_NONE                 0
+#define CPD_CREATE               (1u << 0)
+#define CPD_CHECK                (1u << 1)
+#define CPD_GROUP_OK             (1u << 2)
+#define CPD_GROUP_READ           (1u << 3)
+#define CPD_CHECK_MODE_ONLY      (1u << 4)
+#define CPD_RELAX_DIRMODE_CHECK  (1u << 5)
 int check_private_dir(const char *dirname, cpd_check_t check,
                       const char *effective_user);
 

src/or/config.c

@@ -6325,7 +6325,8 @@ 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, world_writable = 0, group_writable = 0;
+      prefer_ipv6_automap = 1, world_writable = 0, group_writable = 0, relax_dirmode_check = 0,
+      has_used_unix_socket_only_option = 0;
 
     smartlist_split_string(elts, ports->value, NULL,
                            SPLIT_SKIP_SPACE|SPLIT_IGNORE_BLANK, 0);
@@ -6478,9 +6479,15 @@ parse_port_config(smartlist_t *out,
 
         if (!strcasecmp(elt, "GroupWritable")) {
           group_writable = !no;
+          has_used_unix_socket_only_option = 1;
           continue;
         } else if (!strcasecmp(elt, "WorldWritable")) {
           world_writable = !no;
+          has_used_unix_socket_only_option = 1;
+          continue;
+        } else if (!strcasecmp(elt, "RelaxDirModeCheck")) {
+          relax_dirmode_check = !no;
+          has_used_unix_socket_only_option = 1;
           continue;
         }
 
@@ -6568,9 +6575,9 @@ 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);
+    if ( has_used_unix_socket_only_option && ! unix_socket_path) {
+      log_warn(LD_CONFIG, "You have a %sPort entry with GroupWritable, "
+               "WorldWritable, or RelaxDirModeCheck, but it is not a unix socket.", portname);
       goto err;
     }
 
@@ -6596,6 +6603,7 @@ parse_port_config(smartlist_t *out,
       cfg->type = listener_type;
       cfg->is_world_writable = world_writable;
       cfg->is_group_writable = group_writable;
+      cfg->relax_dirmode_check = relax_dirmode_check;
       cfg->entry_cfg.isolation_flags = isolation;
       cfg->entry_cfg.session_group = sessiongroup;
       cfg->server_cfg.no_advertise = no_advertise;

src/or/connection.c

@@ -1014,6 +1014,10 @@ check_location_for_unix_socket(const or_options_t *options, const char *path,
     flags |= CPD_GROUP_OK;
   }
 
+  if (port->relax_dirmode_check) {
+    flags |= CPD_RELAX_DIRMODE_CHECK;
+  }
+
   if (check_private_dir(p, flags, options->User) < 0) {
     char *escpath, *escdir;
     escpath = esc_for_log(path);

src/or/or.h

@@ -3456,6 +3456,7 @@ typedef struct port_cfg_t {
 
   unsigned is_group_writable : 1;
   unsigned is_world_writable : 1;
+  unsigned relax_dirmode_check : 1;
 
   entry_port_cfg_t entry_cfg;
 

src/test/test_config.c

@@ -4038,6 +4038,13 @@ test_config_parse_port_config__ports__ports_given(void *data)
                           "127.0.0.3", 0, 0);
   tt_int_op(ret, OP_EQ, -1);
 
+  // Test failure if we specify group writable for an IP Port
+  config_free_lines(config_port_invalid); config_port_invalid = NULL;
+  config_port_invalid = mock_config_line("DNSPort", "42 RelaxDirModeCheck");
+  ret = parse_port_config(NULL, config_port_invalid, NULL, "DNS", 0,
+                          "127.0.0.3", 0, 0);
+  tt_int_op(ret, OP_EQ, -1);
+
   // Test success with only a port (this will fail without a default address)
   config_free_lines(config_port_valid); config_port_valid = NULL;
   config_port_valid = mock_config_line("DNSPort", "42");