Make ControlSocketsGroupWritable work with User.

Original message from bug3393:

check_private_dir() to ensure that ControlSocketsGroupWritable is
safe to use. Unfortunately, check_private_dir() only checks against
the currently running user… which can be root until privileges are
dropped to the user and group configured by the User config option.

The attached patch fixes the issue by adding a new effective_user
argument to check_private_dir() and updating the callers. It might
not be the best way to fix the issue, but it did in my tests.

(Code by lunar; changelog by nickm)

6 files changed, 13 insertions, 11 deletions

diff --git a/src/or/config.c b/src/or/config.c
index 44cecf353b..8ab23a3b80 100644
--- a/src/or/config.c
+++ b/src/or/config.c
@@ -1025,7 +1025,8 @@ options_act_reversible(or_options_t *old_options, char **msg)
 
   /* Ensure data directory is private; create if possible. */
   if (check_private_dir(options->DataDirectory,
-                        running_tor ? CPD_CREATE : CPD_CHECK)<0) {
+                        running_tor ? CPD_CREATE : CPD_CHECK,
+                        options->User)<0) {
     tor_asprintf(msg,
               "Couldn't access/create private data directory \"%s\"",
               options->DataDirectory);
@@ -1038,7 +1039,8 @@ options_act_reversible(or_options_t *old_options, char **msg)
     char *fn = tor_malloc(len);
     tor_snprintf(fn, len, "%s"PATH_SEPARATOR"cached-status",
                  options->DataDirectory);
-    if (check_private_dir(fn, running_tor ? CPD_CREATE : CPD_CHECK) < 0) {
+    if (check_private_dir(fn, running_tor ? CPD_CREATE : CPD_CHECK,
+                          options->User) < 0) {
       tor_asprintf(msg,
                 "Couldn't access/create private data directory \"%s\"", fn);
       tor_free(fn);
diff --git a/src/or/connection.c b/src/or/connection.c
index 3f4ca1db4b..a9e3a74ed6 100644
--- a/src/or/connection.c
+++ b/src/or/connection.c
@@ -867,7 +867,7 @@ check_location_for_unix_socket(or_options_t *options, const char *path)
   if (options->ControlSocketsGroupWritable)
     flags |= CPD_GROUP_OK;
 
-  if (check_private_dir(p, flags) < 0) {
+  if (check_private_dir(p, flags, options->User) < 0) {
     char *escpath, *escdir;
     escpath = esc_for_log(path);
     escdir = esc_for_log(p);
diff --git a/src/or/geoip.c b/src/or/geoip.c
index 5bb2410a75..c621ea8183 100644
--- a/src/or/geoip.c
+++ b/src/or/geoip.c
@@ -970,7 +970,7 @@ geoip_dirreq_stats_write(time_t now)
   geoip_remove_old_clients(start_of_dirreq_stats_interval);
 
   statsdir = get_datadir_fname("stats");
-  if (check_private_dir(statsdir, CPD_CREATE) < 0)
+  if (check_private_dir(statsdir, CPD_CREATE, get_options()->User) < 0)
     goto done;
   filename = get_datadir_fname2("stats", "dirreq-stats");
   data_v2 = geoip_get_client_history(GEOIP_CLIENT_NETWORKSTATUS_V2);
@@ -1209,7 +1209,7 @@ geoip_bridge_stats_write(time_t now)
 
   /* Write it to disk. */
   statsdir = get_datadir_fname("stats");
-  if (check_private_dir(statsdir, CPD_CREATE) < 0)
+  if (check_private_dir(statsdir, CPD_CREATE, get_options()->User) < 0)
     goto done;
   filename = get_datadir_fname2("stats", "bridge-stats");
 
@@ -1304,7 +1304,7 @@ geoip_entry_stats_write(time_t now)
   geoip_remove_old_clients(start_of_entry_stats_interval);
 
   statsdir = get_datadir_fname("stats");
-  if (check_private_dir(statsdir, CPD_CREATE) < 0)
+  if (check_private_dir(statsdir, CPD_CREATE, get_options()->User) < 0)
     goto done;
   filename = get_datadir_fname2("stats", "entry-stats");
   data = geoip_get_client_history(GEOIP_CLIENT_CONNECT);
diff --git a/src/or/rendservice.c b/src/or/rendservice.c
index a10e43fead..d9a936471e 100644
--- a/src/or/rendservice.c
+++ b/src/or/rendservice.c
@@ -569,7 +569,7 @@ rend_service_load_keys(void)
              s->directory);
 
     /* Check/create directory */
-    if (check_private_dir(s->directory, CPD_CREATE) < 0)
+    if (check_private_dir(s->directory, CPD_CREATE, get_options()->User) < 0)
       return -1;
 
     /* Load key */
diff --git a/src/or/rephist.c b/src/or/rephist.c
index 54593a06c3..b7341f3c0d 100644
--- a/src/or/rephist.c
+++ b/src/or/rephist.c
@@ -2307,7 +2307,7 @@ rep_hist_exit_stats_write(time_t now)
 
   /* Try to write to disk. */
   statsdir = get_datadir_fname("stats");
-  if (check_private_dir(statsdir, CPD_CREATE) < 0) {
+  if (check_private_dir(statsdir, CPD_CREATE, get_options()->User) < 0) {
     log_warn(LD_HIST, "Unable to create stats/ directory!");
     goto done;
   }
@@ -2497,7 +2497,7 @@ rep_hist_buffer_stats_write(time_t now)
   smartlist_clear(circuits_for_buffer_stats);
   /* write to file */
   statsdir = get_datadir_fname("stats");
-  if (check_private_dir(statsdir, CPD_CREATE) < 0)
+  if (check_private_dir(statsdir, CPD_CREATE, get_options()->User) < 0)
     goto done;
   filename = get_datadir_fname2("stats", "buffer-stats");
   out = start_writing_to_stdio_file(filename, OPEN_FLAGS_APPEND,
diff --git a/src/or/router.c b/src/or/router.c
index 68e29bb4c8..2165e6ea90 100644
--- a/src/or/router.c
+++ b/src/or/router.c
@@ -533,12 +533,12 @@ init_keys(void)
     return 0;
   }
   /* Make sure DataDirectory exists, and is private. */
-  if (check_private_dir(options->DataDirectory, CPD_CREATE)) {
+  if (check_private_dir(options->DataDirectory, CPD_CREATE, options->User)) {
     return -1;
   }
   /* Check the key directory. */
   keydir = get_datadir_fname("keys");
-  if (check_private_dir(keydir, CPD_CREATE)) {
+  if (check_private_dir(keydir, CPD_CREATE, options->User)) {
     tor_free(keydir);
     return -1;
   }