From c13db1f6143cf99830dc73dd527898e711e6b704 Mon Sep 17 00:00:00 2001
From: anonymous <anon@fakeanonemail.org>
Date: Thu, 28 Aug 2014 18:10:21 +0000
Subject: Ticket #11291: patch from "anon":

test-11291-group-redable-hsdirs-wtests-may8.patch
---
 src/common/util.c        |  15 ++++--
 src/common/util.h        |  27 +++++++++-
 src/or/config.c          |   1 +
 src/or/or.h              |   4 ++
 src/or/rendservice.c     |  36 +++++++++++--
 src/test/Makefile.nmake  |   4 +-
 src/test/include.am      |   1 +
 src/test/test.c          |   2 +
 src/test/test_checkdir.c | 130 +++++++++++++++++++++++++++++++++++++++++++++++
 9 files changed, 209 insertions(+), 11 deletions(-)
 create mode 100644 src/test/test_checkdir.c

(limited to 'src')

diff --git a/src/common/util.c b/src/common/util.c
index 16ff8e3a80..6971e4d627 100644
--- a/src/common/util.c
+++ b/src/common/util.c
@@ -1872,6 +1872,9 @@ file_status(const char *fname)
  * <b>check</b>&CPD_CHECK, and we think we can create it, return 0.  Else
  * return -1.  If CPD_GROUP_OK is set, then it's okay if the directory
  * is group-readable, but in all cases we create the directory mode 0700.
+ * If CPD_GROUP_READ is set, existing directory behaves as CPD_GROUP_OK and
+ * if the directory is created it will use mode 0750 with group read permission.
+ * Group read privileges also assume execute permission as norm for directories.
  * If CPD_CHECK_MODE_ONLY is set, then we don't alter the directory permissions
  * if they are too permissive: we just return -1.
  * When effective_user is not NULL, check permissions against the given user
@@ -1910,7 +1913,12 @@ check_private_dir(const char *dirname, cpd_check_t check,
 #if defined (_WIN32)
       r = mkdir(dirname);
 #else
-      r = mkdir(dirname, 0700);
+      if (check & CPD_GROUP_READ) {
+        r = mkdir(dirname, STAT_RWXU|STAT_RGRP|STAT_XGRP);
+      }
+      else {
+        r = mkdir(dirname, STAT_RWXU);
+      }
 #endif
       if (r) {
         log_warn(LD_FS, "Error creating directory %s: %s", dirname,
@@ -1963,7 +1971,8 @@ check_private_dir(const char *dirname, cpd_check_t check,
     tor_free(process_ownername);
     return -1;
   }
-  if ((check & CPD_GROUP_OK) && st.st_gid != running_gid) {
+  if ( (check & (CPD_GROUP_OK|CPD_GROUP_READ))
+       && (st.st_gid != running_gid) ) {
     struct group *gr;
     char *process_groupname = NULL;
     gr = getgrgid(running_gid);
@@ -1978,7 +1987,7 @@ check_private_dir(const char *dirname, cpd_check_t check,
     tor_free(process_groupname);
     return -1;
   }
-  if (check & CPD_GROUP_OK) {
+  if (check & (CPD_GROUP_OK|CPD_GROUP_READ)) {
     mask = 0027;
   } else {
     mask = 0077;
diff --git a/src/common/util.h b/src/common/util.h
index 61bb05f016..c98a32c19c 100644
--- a/src/common/util.h
+++ b/src/common/util.h
@@ -344,9 +344,34 @@ typedef unsigned int cpd_check_t;
 #define CPD_CREATE 1
 #define CPD_CHECK 2
 #define CPD_GROUP_OK 4
-#define CPD_CHECK_MODE_ONLY 8
+#define CPD_GROUP_READ 8
+#define CPD_CHECK_MODE_ONLY 16 
 int check_private_dir(const char *dirname, cpd_check_t check,
                       const char *effective_user);
+
+#if !defined(_WIN32) && !defined (WINCE)
+/** Unix like (non win32) group and world permission constants.
+ * These constants are directly compatible with *nix ABI, e.g. S_IRWXU,*
+ * NOTE: these are distinct from the private directory flags CPD_*
+ *        which are a portable way for storing private data cross platform.
+ */
+#define STAT_RWXU 00700
+#define STAT_RUSR 00400
+#define STAT_WUSR 00200
+#define STAT_XUSR 00100
+
+#define STAT_RWXG 00070
+#define STAT_RGRP 00040
+#define STAT_WGRP 00020
+#define STAT_XGRP 00010
+
+#define STAT_RWXO 00007
+#define STAT_ROTH 00004
+#define STAT_WOTH 00002
+#define STAT_XOTH 00001
+#endif
+ 
+/** File open and create options */
 #define OPEN_FLAGS_REPLACE (O_WRONLY|O_CREAT|O_TRUNC)
 #define OPEN_FLAGS_APPEND (O_WRONLY|O_CREAT|O_APPEND)
 #define OPEN_FLAGS_DONT_REPLACE (O_CREAT|O_EXCL|O_APPEND|O_WRONLY)
diff --git a/src/or/config.c b/src/or/config.c
index 7800ec1908..1753722f95 100644
--- a/src/or/config.c
+++ b/src/or/config.c
@@ -271,6 +271,7 @@ static config_var_t option_vars_[] = {
   V(AccelDir,                    FILENAME, NULL),
   V(HashedControlPassword,       LINELIST, NULL),
   V(HidServDirectoryV2,          BOOL,     "1"),
+  V(HiddenServiceGroupReadable,  BOOL,     "0"),
   VAR("HiddenServiceDir",    LINELIST_S, RendConfigLines,    NULL),
   OBSOLETE("HiddenServiceExcludeNodes"),
   OBSOLETE("HiddenServiceNodes"),
diff --git a/src/or/or.h b/src/or/or.h
index 3683607741..9207dbaa91 100644
--- a/src/or/or.h
+++ b/src/or/or.h
@@ -4223,6 +4223,10 @@ typedef struct {
 
   /** Should we send the timestamps that pre-023 hidden services want? */
   int Support022HiddenServices;
+
+  /** Create the Hidden Service directories and hostname files group readable. */
+  int HiddenServiceGroupReadable;
+
 } or_options_t;
 
 /** Persistent state for an onion router, as saved to disk. */
diff --git a/src/or/rendservice.c b/src/or/rendservice.c
index 749d6fa880..83e6a3b82c 100644
--- a/src/or/rendservice.c
+++ b/src/or/rendservice.c
@@ -368,10 +368,12 @@ rend_config_services(const or_options_t *options, int validate_only)
   for (line = options->RendConfigLines; line; line = line->next) {
     if (!strcasecmp(line->key, "HiddenServiceDir")) {
       if (service) { /* register the one we just finished parsing */
-        if (validate_only)
+        if (validate_only) {
           rend_service_free(service);
-        else
+        }
+        else {
           rend_add_service(service);
+        }
       }
       service = tor_malloc_zero(sizeof(rend_service_t));
       service->directory = tor_strdup(line->value);
@@ -513,10 +515,12 @@ rend_config_services(const or_options_t *options, int validate_only)
     }
   }
   if (service) {
-    if (validate_only)
+    if (validate_only) {
       rend_service_free(service);
-    else
+    }
+    else {
       rend_add_service(service);
+    }
   }
 
   /* If this is a reload and there were hidden services configured before,
@@ -693,10 +697,23 @@ rend_service_load_keys(rend_service_t *s)
 {
   char fname[512];
   char buf[128];
+  cpd_check_t  check_opts = CPD_CREATE;
 
+  if (get_options()->HiddenServiceGroupReadable) {
+    check_opts |= CPD_GROUP_READ;
+  }
   /* Check/create directory */
-  if (check_private_dir(s->directory, CPD_CREATE, get_options()->User) < 0)
+  if (check_private_dir(s->directory, check_opts, get_options()->User) < 0) {
     return -1;
+  }
+#ifndef _WIN32
+  if (get_options()->HiddenServiceGroupReadable) {
+    /** Only new dirs created get new opts, also enforce group read. */
+    if (chmod(s->directory, STAT_RWXU|STAT_RGRP|STAT_XGRP)) {
+      log_warn(LD_FS,"Unable to make %s group-readable.", s->directory);
+    }
+  }
+#endif
 
   /* Load key */
   if (strlcpy(fname,s->directory,sizeof(fname)) >= sizeof(fname) ||
@@ -733,6 +750,15 @@ rend_service_load_keys(rend_service_t *s)
     memwipe(buf, 0, sizeof(buf));
     return -1;
   }
+#ifndef _WIN32
+  if (get_options()->HiddenServiceGroupReadable) {
+    /** Also verify hostname file created with group read. */
+    if (chmod(fname, STAT_RUSR|STAT_WUSR|STAT_RGRP)) {
+      log_warn(LD_FS,"Unable to make hidden hostname file %s group-readable.", fname);
+    }
+  }
+#endif
+
   memwipe(buf, 0, sizeof(buf));
 
   /* If client authorization is configured, load or generate keys. */
diff --git a/src/test/Makefile.nmake b/src/test/Makefile.nmake
index 822431f3b8..f6ee7f3f53 100644
--- a/src/test/Makefile.nmake
+++ b/src/test/Makefile.nmake
@@ -12,8 +12,8 @@ LIBS = ..\..\..\build-alpha\lib\libevent.lib \
  crypt32.lib gdi32.lib user32.lib
 
 TEST_OBJECTS = test.obj test_addr.obj test_containers.obj \
-	test_controller_events.ogj test_crypto.obj test_data.obj test_dir.obj \
-	test_microdesc.obj test_pt.obj test_util.obj test_config.obj \
+	test_controller_events.obj test_crypto.obj test_data.obj test_dir.obj \
+	test_checkdir.obj test_microdesc.obj test_pt.obj test_util.obj test_config.obj \
 	test_cell_formats.obj test_replay.obj test_introduce.obj tinytest.obj \
 	test_hs.obj
 
diff --git a/src/test/include.am b/src/test/include.am
index 77c92f12f8..36d86c74ea 100644
--- a/src/test/include.am
+++ b/src/test/include.am
@@ -28,6 +28,7 @@ src_test_test_SOURCES = \
 	src/test/test_cell_queue.c \
 	src/test/test_data.c \
 	src/test/test_dir.c \
+	src/test/test_checkdir.c \
 	src/test/test_entrynodes.c \
 	src/test/test_extorport.c \
 	src/test/test_introduce.c \
diff --git a/src/test/test.c b/src/test/test.c
index e836160bf4..6d7c6fa64b 100644
--- a/src/test/test.c
+++ b/src/test/test.c
@@ -1302,6 +1302,7 @@ extern struct testcase_t crypto_tests[];
 extern struct testcase_t container_tests[];
 extern struct testcase_t util_tests[];
 extern struct testcase_t dir_tests[];
+extern struct testcase_t checkdir_tests[];
 extern struct testcase_t microdesc_tests[];
 extern struct testcase_t pt_tests[];
 extern struct testcase_t config_tests[];
@@ -1338,6 +1339,7 @@ static struct testgroup_t testgroups[] = {
   { "cellfmt/", cell_format_tests },
   { "cellqueue/", cell_queue_tests },
   { "dir/", dir_tests },
+  { "checkdir/", checkdir_tests },
   { "dir/md/", microdesc_tests },
   { "pt/", pt_tests },
   { "config/", config_tests },
diff --git a/src/test/test_checkdir.c b/src/test/test_checkdir.c
new file mode 100644
index 0000000000..877c789bcc
--- /dev/null
+++ b/src/test/test_checkdir.c
@@ -0,0 +1,130 @@
+/* Copyright (c) 2014, The Tor Project, Inc. */
+/* See LICENSE for licensing information */
+
+#include "orconfig.h"
+#include "or.h"
+#include <dirent.h>
+#include "config.h"
+#include "test.h"
+#include "util.h"
+
+/** Run unit tests for private dir permission enforcement logic. */
+static void
+test_checkdir_perms(void *testdata)
+{
+  or_options_t *options = get_options_mutable();
+  const char *subdir = "test_checkdir";
+  char *testdir;
+  cpd_check_t  cpd_chkopts;
+  cpd_check_t  unix_create_opts;
+  cpd_check_t  unix_verify_optsmask;
+  struct stat st;
+
+  /** setup data directory before tests. */
+  tor_free(options->DataDirectory);
+  options->DataDirectory = tor_strdup(get_fname(subdir));
+  tt_int_op(mkdir(options->DataDirectory, STAT_RWXU), ==, 0);
+
+  /** test: create new dir, no flags. */
+  testdir = get_datadir_fname("checkdir_new_none");
+  cpd_chkopts = CPD_CREATE;
+  unix_verify_optsmask = STAT_RWXO|STAT_RWXG; /* 0077 */
+  tt_int_op(0, ==, check_private_dir(testdir, cpd_chkopts, NULL));
+  tt_int_op(0, ==, stat(testdir, &st));
+  tt_int_op(0, ==, (st.st_mode & unix_verify_optsmask));
+  tor_free(testdir);
+
+  /** test: create new dir, CPD_GROUP_OK option set. */
+  testdir = get_datadir_fname("checkdir_new_groupok");
+  cpd_chkopts = CPD_CREATE|CPD_GROUP_OK;
+  unix_verify_optsmask = STAT_RWXO|STAT_RWXG; /* 0077 */
+  tt_int_op(0, ==, check_private_dir(testdir, cpd_chkopts, NULL));
+  tt_int_op(0, ==, stat(testdir, &st));
+  tt_int_op(0, ==, (st.st_mode & unix_verify_optsmask));
+  tor_free(testdir);
+
+
+  /** test: create new dir, CPD_GROUP_READ option set. */
+  testdir = get_datadir_fname("checkdir_new_groupread");
+  cpd_chkopts = CPD_CREATE|CPD_GROUP_READ;
+  unix_verify_optsmask = STAT_RWXO|STAT_WGRP; /* 0027 */
+  tt_int_op(0, ==, check_private_dir(testdir, cpd_chkopts, NULL));
+  tt_int_op(0, ==, stat(testdir, &st));
+  tt_int_op(0, ==, (st.st_mode & unix_verify_optsmask));
+  tor_free(testdir);
+
+
+  /** test: check existing dir created with defaults,
+            and verify with CPD_CREATE only. */
+  testdir = get_datadir_fname("checkdir_exists_none");
+  cpd_chkopts = CPD_CREATE;
+  unix_create_opts = STAT_RWXU; /* 0700 */
+  unix_verify_optsmask = STAT_RWXO|STAT_RWXG; /* 0077 */
+  tt_int_op(0, ==, mkdir(testdir, unix_create_opts));
+  tt_int_op(0, ==, check_private_dir(testdir, cpd_chkopts, NULL));
+  tt_int_op(0, ==, stat(testdir, &st));
+  tt_int_op(0, ==, (st.st_mode & unix_verify_optsmask));
+  tor_free(testdir);
+
+
+  /** test: check existing dir created with defaults,
+            and verify with CPD_GROUP_OK option set. */
+  testdir = get_datadir_fname("checkdir_exists_groupok");
+  cpd_chkopts = CPD_CREATE;
+  unix_verify_optsmask = STAT_RWXO|STAT_RWXG; /* 0077 */
+  tt_int_op(0, ==, check_private_dir(testdir, cpd_chkopts, NULL));
+  cpd_chkopts = CPD_GROUP_OK;
+  tt_int_op(0, ==, check_private_dir(testdir, cpd_chkopts, NULL));
+  tt_int_op(0, ==, stat(testdir, &st));
+  tt_int_op(0, ==, (st.st_mode & unix_verify_optsmask));
+  tor_free(testdir);
+
+
+  /** test: check existing dir created with defaults,
+            and verify with CPD_GROUP_READ option set. */
+  testdir = get_datadir_fname("checkdir_exists_groupread");
+  cpd_chkopts = CPD_CREATE;
+  unix_verify_optsmask = STAT_RWXO|STAT_WGRP; /* 0027 */
+  tt_int_op(0, ==, check_private_dir(testdir, cpd_chkopts, NULL));
+  cpd_chkopts = CPD_GROUP_READ;
+  tt_int_op(0, ==, check_private_dir(testdir, cpd_chkopts, NULL));
+  tt_int_op(0, ==, stat(testdir, &st));
+  tt_int_op(0, ==, (st.st_mode & unix_verify_optsmask));
+  tor_free(testdir);
+
+
+  /** test: check existing dir created with CPD_GROUP_READ,
+            and verify with CPD_GROUP_OK option set. */
+  testdir = get_datadir_fname("checkdir_existsread_groupok");
+  cpd_chkopts = CPD_CREATE|CPD_GROUP_READ;
+  unix_verify_optsmask = STAT_RWXO|STAT_WGRP; /* 0027 */
+  tt_int_op(0, ==, check_private_dir(testdir, cpd_chkopts, NULL));
+  cpd_chkopts = CPD_GROUP_OK;
+  tt_int_op(0, ==, check_private_dir(testdir, cpd_chkopts, NULL));
+  tt_int_op(0, ==, stat(testdir, &st));
+  tt_int_op(0, ==, (st.st_mode & unix_verify_optsmask));
+  tor_free(testdir);
+
+
+  /** test: check existing dir created with CPD_GROUP_READ,
+            and verify with CPD_GROUP_READ option set. */
+  testdir = get_datadir_fname("checkdir_existsread_groupread");
+  cpd_chkopts = CPD_CREATE|CPD_GROUP_READ;
+  unix_verify_optsmask = STAT_RWXO|STAT_WGRP; /* 0027 */
+  tt_int_op(0, ==, check_private_dir(testdir, cpd_chkopts, NULL));
+  tt_int_op(0, ==, stat(testdir, &st));
+  tt_int_op(0, ==, (st.st_mode & unix_verify_optsmask));
+  tor_free(testdir);
+
+  done:
+  ;
+}
+
+#define CHECKDIR(name,flags)                              \
+  { #name, test_checkdir_##name, (flags), NULL, NULL }
+
+struct testcase_t checkdir_tests[] = {
+  CHECKDIR(perms, 0),
+  END_OF_TESTCASES
+};
+
-- 
cgit v1.2.3-54-g00ecf


From 227b65924b557b30855f659360a8547e352c1ec6 Mon Sep 17 00:00:00 2001
From: David Stainton <dstainton415@gmail.com>
Date: Fri, 29 Aug 2014 05:58:53 +0000
Subject: Clean up patch

Here I clean up anon's patch with a few of nickm's suggestions from comment 12:
https://trac.torproject.org/projects/tor/ticket/11291#comment:12

I did not yet completely implement all his suggestions.
---
 doc/tor.1.txt            |  2 +-
 src/common/util.c        |  7 +++----
 src/common/util.h        | 23 -----------------------
 src/or/config.c          |  2 +-
 src/or/or.h              |  2 +-
 src/or/rendservice.c     | 23 ++++++++++-------------
 src/test/test_checkdir.c | 36 ++++++++++++++++++------------------
 7 files changed, 34 insertions(+), 61 deletions(-)

(limited to 'src')

diff --git a/doc/tor.1.txt b/doc/tor.1.txt
index ec5cc14380..272eba3ef6 100644
--- a/doc/tor.1.txt
+++ b/doc/tor.1.txt
@@ -2078,7 +2078,7 @@ The following options are used to configure a hidden service.
     service descriptors to the directory servers. This information  is also
     uploaded whenever it changes. (Default: 1 hour)
 
-[[HiddenServiceGroupReadable]] **HiddenServiceGroupReadable** **0**|**1**::
+[[HiddenServiceDirGroupReadable]] **HiddenServiceGroupReadable** **0**|**1**::
     If this option is set to 1, allow the filesystem group to read the
     hidden service directory and hostname file. If the option is set to 0,
     only owner is able to read the hidden service directory. (Default: 0)
diff --git a/src/common/util.c b/src/common/util.c
index 6971e4d627..0865fe7c7f 100644
--- a/src/common/util.c
+++ b/src/common/util.c
@@ -1914,10 +1914,9 @@ check_private_dir(const char *dirname, cpd_check_t check,
       r = mkdir(dirname);
 #else
       if (check & CPD_GROUP_READ) {
-        r = mkdir(dirname, STAT_RWXU|STAT_RGRP|STAT_XGRP);
-      }
-      else {
-        r = mkdir(dirname, STAT_RWXU);
+        r = mkdir(dirname, 0750);
+      } else {
+        r = mkdir(dirname, 0700);
       }
 #endif
       if (r) {
diff --git a/src/common/util.h b/src/common/util.h
index c98a32c19c..755ef4b82a 100644
--- a/src/common/util.h
+++ b/src/common/util.h
@@ -349,29 +349,6 @@ typedef unsigned int cpd_check_t;
 int check_private_dir(const char *dirname, cpd_check_t check,
                       const char *effective_user);
 
-#if !defined(_WIN32) && !defined (WINCE)
-/** Unix like (non win32) group and world permission constants.
- * These constants are directly compatible with *nix ABI, e.g. S_IRWXU,*
- * NOTE: these are distinct from the private directory flags CPD_*
- *        which are a portable way for storing private data cross platform.
- */
-#define STAT_RWXU 00700
-#define STAT_RUSR 00400
-#define STAT_WUSR 00200
-#define STAT_XUSR 00100
-
-#define STAT_RWXG 00070
-#define STAT_RGRP 00040
-#define STAT_WGRP 00020
-#define STAT_XGRP 00010
-
-#define STAT_RWXO 00007
-#define STAT_ROTH 00004
-#define STAT_WOTH 00002
-#define STAT_XOTH 00001
-#endif
- 
-/** File open and create options */
 #define OPEN_FLAGS_REPLACE (O_WRONLY|O_CREAT|O_TRUNC)
 #define OPEN_FLAGS_APPEND (O_WRONLY|O_CREAT|O_APPEND)
 #define OPEN_FLAGS_DONT_REPLACE (O_CREAT|O_EXCL|O_APPEND|O_WRONLY)
diff --git a/src/or/config.c b/src/or/config.c
index 1753722f95..97b3601706 100644
--- a/src/or/config.c
+++ b/src/or/config.c
@@ -271,7 +271,7 @@ static config_var_t option_vars_[] = {
   V(AccelDir,                    FILENAME, NULL),
   V(HashedControlPassword,       LINELIST, NULL),
   V(HidServDirectoryV2,          BOOL,     "1"),
-  V(HiddenServiceGroupReadable,  BOOL,     "0"),
+  V(HiddenServiceDirGroupReadable,  BOOL,     "0"),
   VAR("HiddenServiceDir",    LINELIST_S, RendConfigLines,    NULL),
   OBSOLETE("HiddenServiceExcludeNodes"),
   OBSOLETE("HiddenServiceNodes"),
diff --git a/src/or/or.h b/src/or/or.h
index 9207dbaa91..1544b70996 100644
--- a/src/or/or.h
+++ b/src/or/or.h
@@ -4225,7 +4225,7 @@ typedef struct {
   int Support022HiddenServices;
 
   /** Create the Hidden Service directories and hostname files group readable. */
-  int HiddenServiceGroupReadable;
+  int HiddenServiceDirGroupReadable;
 
 } or_options_t;
 
diff --git a/src/or/rendservice.c b/src/or/rendservice.c
index 83e6a3b82c..456b548715 100644
--- a/src/or/rendservice.c
+++ b/src/or/rendservice.c
@@ -368,12 +368,10 @@ rend_config_services(const or_options_t *options, int validate_only)
   for (line = options->RendConfigLines; line; line = line->next) {
     if (!strcasecmp(line->key, "HiddenServiceDir")) {
       if (service) { /* register the one we just finished parsing */
-        if (validate_only) {
+        if (validate_only)
           rend_service_free(service);
-        }
-        else {
+        else
           rend_add_service(service);
-        }
       }
       service = tor_malloc_zero(sizeof(rend_service_t));
       service->directory = tor_strdup(line->value);
@@ -517,8 +515,7 @@ rend_config_services(const or_options_t *options, int validate_only)
   if (service) {
     if (validate_only) {
       rend_service_free(service);
-    }
-    else {
+    } else {
       rend_add_service(service);
     }
   }
@@ -699,7 +696,7 @@ rend_service_load_keys(rend_service_t *s)
   char buf[128];
   cpd_check_t  check_opts = CPD_CREATE;
 
-  if (get_options()->HiddenServiceGroupReadable) {
+  if (get_options()->HiddenServiceDirGroupReadable) {
     check_opts |= CPD_GROUP_READ;
   }
   /* Check/create directory */
@@ -707,9 +704,9 @@ rend_service_load_keys(rend_service_t *s)
     return -1;
   }
 #ifndef _WIN32
-  if (get_options()->HiddenServiceGroupReadable) {
-    /** Only new dirs created get new opts, also enforce group read. */
-    if (chmod(s->directory, STAT_RWXU|STAT_RGRP|STAT_XGRP)) {
+  if (get_options()->HiddenServiceDirGroupReadable) {
+    /* Only new dirs created get new opts, also enforce group read. */
+    if (chmod(s->directory, 0750)) {
       log_warn(LD_FS,"Unable to make %s group-readable.", s->directory);
     }
   }
@@ -751,9 +748,9 @@ rend_service_load_keys(rend_service_t *s)
     return -1;
   }
 #ifndef _WIN32
-  if (get_options()->HiddenServiceGroupReadable) {
-    /** Also verify hostname file created with group read. */
-    if (chmod(fname, STAT_RUSR|STAT_WUSR|STAT_RGRP)) {
+  if (get_options()->HiddenServiceDirGroupReadable) {
+    /* Also verify hostname file created with group read. */
+    if (chmod(fname, 0640)) {
       log_warn(LD_FS,"Unable to make hidden hostname file %s group-readable.", fname);
     }
   }
diff --git a/src/test/test_checkdir.c b/src/test/test_checkdir.c
index 877c789bcc..76fe1315a7 100644
--- a/src/test/test_checkdir.c
+++ b/src/test/test_checkdir.c
@@ -20,46 +20,46 @@ test_checkdir_perms(void *testdata)
   cpd_check_t  unix_verify_optsmask;
   struct stat st;
 
-  /** setup data directory before tests. */
+  /* setup data directory before tests. */
   tor_free(options->DataDirectory);
   options->DataDirectory = tor_strdup(get_fname(subdir));
   tt_int_op(mkdir(options->DataDirectory, STAT_RWXU), ==, 0);
 
-  /** test: create new dir, no flags. */
+  /* test: create new dir, no flags. */
   testdir = get_datadir_fname("checkdir_new_none");
   cpd_chkopts = CPD_CREATE;
-  unix_verify_optsmask = STAT_RWXO|STAT_RWXG; /* 0077 */
+  unix_verify_optsmask = 0077;
   tt_int_op(0, ==, check_private_dir(testdir, cpd_chkopts, NULL));
   tt_int_op(0, ==, stat(testdir, &st));
   tt_int_op(0, ==, (st.st_mode & unix_verify_optsmask));
   tor_free(testdir);
 
-  /** test: create new dir, CPD_GROUP_OK option set. */
+  /* test: create new dir, CPD_GROUP_OK option set. */
   testdir = get_datadir_fname("checkdir_new_groupok");
   cpd_chkopts = CPD_CREATE|CPD_GROUP_OK;
-  unix_verify_optsmask = STAT_RWXO|STAT_RWXG; /* 0077 */
+  unix_verify_optsmask = 0077;
   tt_int_op(0, ==, check_private_dir(testdir, cpd_chkopts, NULL));
   tt_int_op(0, ==, stat(testdir, &st));
   tt_int_op(0, ==, (st.st_mode & unix_verify_optsmask));
   tor_free(testdir);
 
 
-  /** test: create new dir, CPD_GROUP_READ option set. */
+  /* test: create new dir, CPD_GROUP_READ option set. */
   testdir = get_datadir_fname("checkdir_new_groupread");
   cpd_chkopts = CPD_CREATE|CPD_GROUP_READ;
-  unix_verify_optsmask = STAT_RWXO|STAT_WGRP; /* 0027 */
+  unix_verify_optsmask = 0027;
   tt_int_op(0, ==, check_private_dir(testdir, cpd_chkopts, NULL));
   tt_int_op(0, ==, stat(testdir, &st));
   tt_int_op(0, ==, (st.st_mode & unix_verify_optsmask));
   tor_free(testdir);
 
 
-  /** test: check existing dir created with defaults,
+  /* test: check existing dir created with defaults,
             and verify with CPD_CREATE only. */
   testdir = get_datadir_fname("checkdir_exists_none");
   cpd_chkopts = CPD_CREATE;
-  unix_create_opts = STAT_RWXU; /* 0700 */
-  unix_verify_optsmask = STAT_RWXO|STAT_RWXG; /* 0077 */
+  unix_create_opts = 0700;
+  unix_verify_optsmask = 0077;
   tt_int_op(0, ==, mkdir(testdir, unix_create_opts));
   tt_int_op(0, ==, check_private_dir(testdir, cpd_chkopts, NULL));
   tt_int_op(0, ==, stat(testdir, &st));
@@ -67,11 +67,11 @@ test_checkdir_perms(void *testdata)
   tor_free(testdir);
 
 
-  /** test: check existing dir created with defaults,
+  /* test: check existing dir created with defaults,
             and verify with CPD_GROUP_OK option set. */
   testdir = get_datadir_fname("checkdir_exists_groupok");
   cpd_chkopts = CPD_CREATE;
-  unix_verify_optsmask = STAT_RWXO|STAT_RWXG; /* 0077 */
+  unix_verify_optsmask = 0077;
   tt_int_op(0, ==, check_private_dir(testdir, cpd_chkopts, NULL));
   cpd_chkopts = CPD_GROUP_OK;
   tt_int_op(0, ==, check_private_dir(testdir, cpd_chkopts, NULL));
@@ -80,11 +80,11 @@ test_checkdir_perms(void *testdata)
   tor_free(testdir);
 
 
-  /** test: check existing dir created with defaults,
+  /* test: check existing dir created with defaults,
             and verify with CPD_GROUP_READ option set. */
   testdir = get_datadir_fname("checkdir_exists_groupread");
   cpd_chkopts = CPD_CREATE;
-  unix_verify_optsmask = STAT_RWXO|STAT_WGRP; /* 0027 */
+  unix_verify_optsmask = 0027;
   tt_int_op(0, ==, check_private_dir(testdir, cpd_chkopts, NULL));
   cpd_chkopts = CPD_GROUP_READ;
   tt_int_op(0, ==, check_private_dir(testdir, cpd_chkopts, NULL));
@@ -93,11 +93,11 @@ test_checkdir_perms(void *testdata)
   tor_free(testdir);
 
 
-  /** test: check existing dir created with CPD_GROUP_READ,
+  /* test: check existing dir created with CPD_GROUP_READ,
             and verify with CPD_GROUP_OK option set. */
   testdir = get_datadir_fname("checkdir_existsread_groupok");
   cpd_chkopts = CPD_CREATE|CPD_GROUP_READ;
-  unix_verify_optsmask = STAT_RWXO|STAT_WGRP; /* 0027 */
+  unix_verify_optsmask = 0027;
   tt_int_op(0, ==, check_private_dir(testdir, cpd_chkopts, NULL));
   cpd_chkopts = CPD_GROUP_OK;
   tt_int_op(0, ==, check_private_dir(testdir, cpd_chkopts, NULL));
@@ -106,11 +106,11 @@ test_checkdir_perms(void *testdata)
   tor_free(testdir);
 
 
-  /** test: check existing dir created with CPD_GROUP_READ,
+  /* test: check existing dir created with CPD_GROUP_READ,
             and verify with CPD_GROUP_READ option set. */
   testdir = get_datadir_fname("checkdir_existsread_groupread");
   cpd_chkopts = CPD_CREATE|CPD_GROUP_READ;
-  unix_verify_optsmask = STAT_RWXO|STAT_WGRP; /* 0027 */
+  unix_verify_optsmask = 0027;
   tt_int_op(0, ==, check_private_dir(testdir, cpd_chkopts, NULL));
   tt_int_op(0, ==, stat(testdir, &st));
   tt_int_op(0, ==, (st.st_mode & unix_verify_optsmask));
-- 
cgit v1.2.3-54-g00ecf


From 6b9016fe3c4dd814bee07e4439efcb6aca4efc43 Mon Sep 17 00:00:00 2001
From: David Stainton <dstainton415@gmail.com>
Date: Fri, 29 Aug 2014 18:58:56 +0000
Subject: Correct check_private_dir's dir mode

This commit attempts to satisfy nickm's comment on check_private_dir() permissions:
https://trac.torproject.org/projects/tor/ticket/11291#comment:12
"""check_private_dir() ensures that the directory has bits 0700 if CPD_CHECK_MODE_ONLY is not set. Shouldn't it also ensure that the directory has bits 0050 if CPD_CHECK_MODE_ONLY is not set, and CPD_GROUP_READ is set?"""
---
 src/common/util.c | 22 ++++++++++------------
 1 file changed, 10 insertions(+), 12 deletions(-)

(limited to 'src')

diff --git a/src/common/util.c b/src/common/util.c
index 0865fe7c7f..0323264494 100644
--- a/src/common/util.c
+++ b/src/common/util.c
@@ -1888,7 +1888,6 @@ check_private_dir(const char *dirname, cpd_check_t check,
   struct stat st;
   char *f;
 #ifndef _WIN32
-  int mask;
   const struct passwd *pw = NULL;
   uid_t running_uid;
   gid_t running_gid;
@@ -1986,22 +1985,20 @@ check_private_dir(const char *dirname, cpd_check_t check,
     tor_free(process_groupname);
     return -1;
   }
-  if (check & (CPD_GROUP_OK|CPD_GROUP_READ)) {
-    mask = 0027;
-  } else {
-    mask = 0077;
-  }
-  if (st.st_mode & mask) {
-    unsigned new_mode;
-    if (check & CPD_CHECK_MODE_ONLY) {
+  if (check & CPD_CHECK_MODE_ONLY) {
+    if (st.st_mode & 0077) {
       log_warn(LD_FS, "Permissions on directory %s are too permissive.",
                dirname);
       return -1;
     }
+  } else {
     log_warn(LD_FS, "Fixing permissions on directory %s", dirname);
-    new_mode = st.st_mode;
-    new_mode |= 0700; /* Owner should have rwx */
-    new_mode &= ~mask; /* Clear the other bits that we didn't want set...*/
+    unsigned new_mode;
+    if (check & CPD_GROUP_READ) {
+      new_mode = 0750;
+    } else {
+      new_mode = 0700;
+    }
     if (chmod(dirname, new_mode)) {
       log_warn(LD_FS, "Could not chmod directory %s: %s", dirname,
           strerror(errno));
@@ -2010,6 +2007,7 @@ check_private_dir(const char *dirname, cpd_check_t check,
       return 0;
     }
   }
+
 #endif
   return 0;
 }
-- 
cgit v1.2.3-54-g00ecf


From ae18c0812e917ae4d3352ef7e537c7ab8a396f36 Mon Sep 17 00:00:00 2001
From: meejah <meejah@meejah.ca>
Date: Sat, 30 Aug 2014 15:14:17 -0600
Subject: fix two typos

---
 doc/tor.1.txt            | 2 +-
 src/test/test_checkdir.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

(limited to 'src')

diff --git a/doc/tor.1.txt b/doc/tor.1.txt
index 272eba3ef6..bf0f5c95c7 100644
--- a/doc/tor.1.txt
+++ b/doc/tor.1.txt
@@ -2078,7 +2078,7 @@ The following options are used to configure a hidden service.
     service descriptors to the directory servers. This information  is also
     uploaded whenever it changes. (Default: 1 hour)
 
-[[HiddenServiceDirGroupReadable]] **HiddenServiceGroupReadable** **0**|**1**::
+[[HiddenServiceDirGroupReadable]] **HiddenServiceDirGroupReadable** **0**|**1**::
     If this option is set to 1, allow the filesystem group to read the
     hidden service directory and hostname file. If the option is set to 0,
     only owner is able to read the hidden service directory. (Default: 0)
diff --git a/src/test/test_checkdir.c b/src/test/test_checkdir.c
index 76fe1315a7..59c1783978 100644
--- a/src/test/test_checkdir.c
+++ b/src/test/test_checkdir.c
@@ -23,7 +23,7 @@ test_checkdir_perms(void *testdata)
   /* setup data directory before tests. */
   tor_free(options->DataDirectory);
   options->DataDirectory = tor_strdup(get_fname(subdir));
-  tt_int_op(mkdir(options->DataDirectory, STAT_RWXU), ==, 0);
+  tt_int_op(mkdir(options->DataDirectory, 0750), ==, 0);
 
   /* test: create new dir, no flags. */
   testdir = get_datadir_fname("checkdir_new_none");
-- 
cgit v1.2.3-54-g00ecf


From 7caf7e9f2a26dfb425dab761b4b41a38d96db0af Mon Sep 17 00:00:00 2001
From: meejah <meejah@meejah.ca>
Date: Sat, 30 Aug 2014 15:14:51 -0600
Subject: Make HiddenServiceDirGroupReadable per-hidden-service

---
 src/or/config.c      |  2 +-
 src/or/rendservice.c | 17 ++++++++++++++---
 2 files changed, 15 insertions(+), 4 deletions(-)

(limited to 'src')

diff --git a/src/or/config.c b/src/or/config.c
index 97b3601706..847ae162ed 100644
--- a/src/or/config.c
+++ b/src/or/config.c
@@ -271,8 +271,8 @@ static config_var_t option_vars_[] = {
   V(AccelDir,                    FILENAME, NULL),
   V(HashedControlPassword,       LINELIST, NULL),
   V(HidServDirectoryV2,          BOOL,     "1"),
-  V(HiddenServiceDirGroupReadable,  BOOL,     "0"),
   VAR("HiddenServiceDir",    LINELIST_S, RendConfigLines,    NULL),
+  VAR("HiddenServiceDirGroupReadable",  LINELIST_S, RendConfigLines, NULL),
   OBSOLETE("HiddenServiceExcludeNodes"),
   OBSOLETE("HiddenServiceNodes"),
   VAR("HiddenServiceOptions",LINELIST_V, RendConfigLines,    NULL),
diff --git a/src/or/rendservice.c b/src/or/rendservice.c
index 456b548715..a1d572e1ac 100644
--- a/src/or/rendservice.c
+++ b/src/or/rendservice.c
@@ -95,6 +95,7 @@ typedef struct rend_service_port_config_t {
 typedef struct rend_service_t {
   /* Fields specified in config file */
   char *directory; /**< where in the filesystem it stores it */
+  int dir_group_readable; /**< if 1, allow group read permissions on directory */
   smartlist_t *ports; /**< List of rend_service_port_config_t */
   rend_auth_type_t auth_type; /**< Client authorization type or 0 if no client
                                * authorization is performed. */
@@ -359,6 +360,7 @@ rend_config_services(const or_options_t *options, int validate_only)
   rend_service_t *service = NULL;
   rend_service_port_config_t *portcfg;
   smartlist_t *old_service_list = NULL;
+  int ok = 0;
 
   if (!validate_only) {
     old_service_list = rend_service_list;
@@ -393,6 +395,15 @@ rend_config_services(const or_options_t *options, int validate_only)
         return -1;
       }
       smartlist_add(service->ports, portcfg);
+    } else if (!strcasecmp(line->key, "HiddenServiceDirGroupReadable")) {
+        service->dir_group_readable = (int)tor_parse_long(line->value, 10, 0, 1, &ok, NULL);
+        if (!ok) {
+            log_warn(LD_CONFIG, "HiddenServiceDirGroupReadable should be 0 or 1, not %s",
+                     line->value);
+            rend_service_free(service);
+            return -1;
+        }
+        log_info(LD_CONFIG, "HiddenServiceDirGroupReadable=%d for %s", service->dir_group_readable, service->directory);
     } else if (!strcasecmp(line->key, "HiddenServiceAuthorizeClient")) {
       /* Parse auth type and comma-separated list of client names and add a
        * rend_authorized_client_t for each client to the service's list
@@ -696,7 +707,7 @@ rend_service_load_keys(rend_service_t *s)
   char buf[128];
   cpd_check_t  check_opts = CPD_CREATE;
 
-  if (get_options()->HiddenServiceDirGroupReadable) {
+  if (s->dir_group_readable) {
     check_opts |= CPD_GROUP_READ;
   }
   /* Check/create directory */
@@ -704,7 +715,7 @@ rend_service_load_keys(rend_service_t *s)
     return -1;
   }
 #ifndef _WIN32
-  if (get_options()->HiddenServiceDirGroupReadable) {
+  if (s->dir_group_readable) {
     /* Only new dirs created get new opts, also enforce group read. */
     if (chmod(s->directory, 0750)) {
       log_warn(LD_FS,"Unable to make %s group-readable.", s->directory);
@@ -748,7 +759,7 @@ rend_service_load_keys(rend_service_t *s)
     return -1;
   }
 #ifndef _WIN32
-  if (get_options()->HiddenServiceDirGroupReadable) {
+  if (s->dir_group_readable) {
     /* Also verify hostname file created with group read. */
     if (chmod(fname, 0640)) {
       log_warn(LD_FS,"Unable to make hidden hostname file %s group-readable.", fname);
-- 
cgit v1.2.3-54-g00ecf


From 6e4efb559d9921e44c40b99e69619d4fa8b36668 Mon Sep 17 00:00:00 2001
From: David Stainton <dstainton415@gmail.com>
Date: Tue, 2 Sep 2014 17:59:31 +0000
Subject: Fix white space

---
 src/common/util.c        |  9 +++++----
 src/common/util.h        |  2 +-
 src/or/or.h              |  3 ++-
 src/or/rendservice.c     | 22 ++++++++++++++--------
 src/test/test_checkdir.c |  6 ------
 5 files changed, 22 insertions(+), 20 deletions(-)

(limited to 'src')

diff --git a/src/common/util.c b/src/common/util.c
index 0323264494..c5b47b13f8 100644
--- a/src/common/util.c
+++ b/src/common/util.c
@@ -1873,10 +1873,11 @@ file_status(const char *fname)
  * return -1.  If CPD_GROUP_OK is set, then it's okay if the directory
  * is group-readable, but in all cases we create the directory mode 0700.
  * If CPD_GROUP_READ is set, existing directory behaves as CPD_GROUP_OK and
- * if the directory is created it will use mode 0750 with group read permission.
- * Group read privileges also assume execute permission as norm for directories.
- * If CPD_CHECK_MODE_ONLY is set, then we don't alter the directory permissions
- * if they are too permissive: we just return -1.
+ * if the directory is created it will use mode 0750 with group read
+ * permission. Group read privileges also assume execute permission
+ * as norm for directories. If CPD_CHECK_MODE_ONLY is set, then we don't
+ * alter the directory permissions if they are too permissive:
+ * we just return -1.
  * When effective_user is not NULL, check permissions against the given user
  * and its primary group.
  */
diff --git a/src/common/util.h b/src/common/util.h
index 755ef4b82a..30dc22852e 100644
--- a/src/common/util.h
+++ b/src/common/util.h
@@ -345,7 +345,7 @@ typedef unsigned int cpd_check_t;
 #define CPD_CHECK 2
 #define CPD_GROUP_OK 4
 #define CPD_GROUP_READ 8
-#define CPD_CHECK_MODE_ONLY 16 
+#define CPD_CHECK_MODE_ONLY 16
 int check_private_dir(const char *dirname, cpd_check_t check,
                       const char *effective_user);
 
diff --git a/src/or/or.h b/src/or/or.h
index 1544b70996..33a582ba7e 100644
--- a/src/or/or.h
+++ b/src/or/or.h
@@ -4224,7 +4224,8 @@ typedef struct {
   /** Should we send the timestamps that pre-023 hidden services want? */
   int Support022HiddenServices;
 
-  /** Create the Hidden Service directories and hostname files group readable. */
+  /** Create the Hidden Service directories
+      and hostname files group readable. */
   int HiddenServiceDirGroupReadable;
 
 } or_options_t;
diff --git a/src/or/rendservice.c b/src/or/rendservice.c
index a1d572e1ac..75080cbe94 100644
--- a/src/or/rendservice.c
+++ b/src/or/rendservice.c
@@ -95,7 +95,8 @@ typedef struct rend_service_port_config_t {
 typedef struct rend_service_t {
   /* Fields specified in config file */
   char *directory; /**< where in the filesystem it stores it */
-  int dir_group_readable; /**< if 1, allow group read permissions on directory */
+  int dir_group_readable; /**< if 1, allow group read
+                             permissions on directory */
   smartlist_t *ports; /**< List of rend_service_port_config_t */
   rend_auth_type_t auth_type; /**< Client authorization type or 0 if no client
                                * authorization is performed. */
@@ -395,15 +396,20 @@ rend_config_services(const or_options_t *options, int validate_only)
         return -1;
       }
       smartlist_add(service->ports, portcfg);
-    } else if (!strcasecmp(line->key, "HiddenServiceDirGroupReadable")) {
-        service->dir_group_readable = (int)tor_parse_long(line->value, 10, 0, 1, &ok, NULL);
+    } else if (!strcasecmp(line->key,
+                           "HiddenServiceDirGroupReadable")) {
+        service->dir_group_readable = (int)tor_parse_long(line->value,
+                                                          10, 0, 1, &ok, NULL);
         if (!ok) {
-            log_warn(LD_CONFIG, "HiddenServiceDirGroupReadable should be 0 or 1, not %s",
+            log_warn(LD_CONFIG,
+                     "HiddenServiceDirGroupReadable should be 0 or 1, not %s",
                      line->value);
             rend_service_free(service);
             return -1;
         }
-        log_info(LD_CONFIG, "HiddenServiceDirGroupReadable=%d for %s", service->dir_group_readable, service->directory);
+        log_info(LD_CONFIG,
+                 "HiddenServiceDirGroupReadable=%d for %s",
+                 service->dir_group_readable, service->directory);
     } else if (!strcasecmp(line->key, "HiddenServiceAuthorizeClient")) {
       /* Parse auth type and comma-separated list of client names and add a
        * rend_authorized_client_t for each client to the service's list
@@ -761,9 +767,9 @@ rend_service_load_keys(rend_service_t *s)
 #ifndef _WIN32
   if (s->dir_group_readable) {
     /* Also verify hostname file created with group read. */
-    if (chmod(fname, 0640)) {
-      log_warn(LD_FS,"Unable to make hidden hostname file %s group-readable.", fname);
-    }
+    if (chmod(fname, 0640))
+      log_warn(LD_FS,"Unable to make hidden hostname file %s group-readable.",
+               fname);
   }
 #endif
 
diff --git a/src/test/test_checkdir.c b/src/test/test_checkdir.c
index 59c1783978..1580e6271d 100644
--- a/src/test/test_checkdir.c
+++ b/src/test/test_checkdir.c
@@ -43,7 +43,6 @@ test_checkdir_perms(void *testdata)
   tt_int_op(0, ==, (st.st_mode & unix_verify_optsmask));
   tor_free(testdir);
 
-
   /* test: create new dir, CPD_GROUP_READ option set. */
   testdir = get_datadir_fname("checkdir_new_groupread");
   cpd_chkopts = CPD_CREATE|CPD_GROUP_READ;
@@ -53,7 +52,6 @@ test_checkdir_perms(void *testdata)
   tt_int_op(0, ==, (st.st_mode & unix_verify_optsmask));
   tor_free(testdir);
 
-
   /* test: check existing dir created with defaults,
             and verify with CPD_CREATE only. */
   testdir = get_datadir_fname("checkdir_exists_none");
@@ -66,7 +64,6 @@ test_checkdir_perms(void *testdata)
   tt_int_op(0, ==, (st.st_mode & unix_verify_optsmask));
   tor_free(testdir);
 
-
   /* test: check existing dir created with defaults,
             and verify with CPD_GROUP_OK option set. */
   testdir = get_datadir_fname("checkdir_exists_groupok");
@@ -79,7 +76,6 @@ test_checkdir_perms(void *testdata)
   tt_int_op(0, ==, (st.st_mode & unix_verify_optsmask));
   tor_free(testdir);
 
-
   /* test: check existing dir created with defaults,
             and verify with CPD_GROUP_READ option set. */
   testdir = get_datadir_fname("checkdir_exists_groupread");
@@ -92,7 +88,6 @@ test_checkdir_perms(void *testdata)
   tt_int_op(0, ==, (st.st_mode & unix_verify_optsmask));
   tor_free(testdir);
 
-
   /* test: check existing dir created with CPD_GROUP_READ,
             and verify with CPD_GROUP_OK option set. */
   testdir = get_datadir_fname("checkdir_existsread_groupok");
@@ -105,7 +100,6 @@ test_checkdir_perms(void *testdata)
   tt_int_op(0, ==, (st.st_mode & unix_verify_optsmask));
   tor_free(testdir);
 
-
   /* test: check existing dir created with CPD_GROUP_READ,
             and verify with CPD_GROUP_READ option set. */
   testdir = get_datadir_fname("checkdir_existsread_groupread");
-- 
cgit v1.2.3-54-g00ecf


From 7203040835f6b9379ab6c8a730a18409f07bfc53 Mon Sep 17 00:00:00 2001
From: David Stainton <dstainton415@gmail.com>
Date: Tue, 2 Sep 2014 22:46:46 +0000
Subject: Fix regression nickm pointed out

---
 src/common/util.c | 22 +++++++++++++---------
 1 file changed, 13 insertions(+), 9 deletions(-)

(limited to 'src')

diff --git a/src/common/util.c b/src/common/util.c
index c5b47b13f8..791ca136c3 100644
--- a/src/common/util.c
+++ b/src/common/util.c
@@ -1889,6 +1889,8 @@ check_private_dir(const char *dirname, cpd_check_t check,
   struct stat st;
   char *f;
 #ifndef _WIN32
+  int mask = 0;
+  int perm = 0;
   const struct passwd *pw = NULL;
   uid_t running_uid;
   gid_t running_gid;
@@ -1986,29 +1988,31 @@ check_private_dir(const char *dirname, cpd_check_t check,
     tor_free(process_groupname);
     return -1;
   }
-  if (check & CPD_CHECK_MODE_ONLY) {
-    if (st.st_mode & 0077) {
-      log_warn(LD_FS, "Permissions on directory %s are too permissive.",
-               dirname);
-      return -1;
+  if(check & CPD_CHECK_MODE_ONLY) {
+    if(check & CPD_GROUP_OK || check & CPD_GROUP_READ) {
+      if (!st.st_mode & 0027) {
+        log_warn(LD_FS, "Incorrect permissions on directory %s a.", dirname);
+        return -1;
+      }
     }
   } else {
     log_warn(LD_FS, "Fixing permissions on directory %s", dirname);
     unsigned new_mode;
+    new_mode = 0700;
+    if (check & CPD_GROUP_OK) {
+      new_mode = 0700;
+    }
     if (check & CPD_GROUP_READ) {
       new_mode = 0750;
-    } else {
-      new_mode = 0700;
     }
     if (chmod(dirname, new_mode)) {
       log_warn(LD_FS, "Could not chmod directory %s: %s", dirname,
-          strerror(errno));
+               strerror(errno));
       return -1;
     } else {
       return 0;
     }
   }
-
 #endif
   return 0;
 }
-- 
cgit v1.2.3-54-g00ecf


From 59e052b896dfdfd1c185d2f93b4a135de9bdb6ed Mon Sep 17 00:00:00 2001
From: David Stainton <dstainton415@gmail.com>
Date: Wed, 3 Sep 2014 17:22:15 +0000
Subject: Remove HiddenServiceDirGroupReadable from or_options_t

...and also fix whitespace.
---
 src/common/util.c | 4 ++--
 src/or/or.h       | 4 ----
 2 files changed, 2 insertions(+), 6 deletions(-)

(limited to 'src')

diff --git a/src/common/util.c b/src/common/util.c
index 791ca136c3..3f04932112 100644
--- a/src/common/util.c
+++ b/src/common/util.c
@@ -1988,8 +1988,8 @@ check_private_dir(const char *dirname, cpd_check_t check,
     tor_free(process_groupname);
     return -1;
   }
-  if(check & CPD_CHECK_MODE_ONLY) {
-    if(check & CPD_GROUP_OK || check & CPD_GROUP_READ) {
+  if (check & CPD_CHECK_MODE_ONLY) {
+    if (check & CPD_GROUP_OK || check & CPD_GROUP_READ) {
       if (!st.st_mode & 0027) {
         log_warn(LD_FS, "Incorrect permissions on directory %s a.", dirname);
         return -1;
diff --git a/src/or/or.h b/src/or/or.h
index 33a582ba7e..cbdc95efe1 100644
--- a/src/or/or.h
+++ b/src/or/or.h
@@ -4224,10 +4224,6 @@ typedef struct {
   /** Should we send the timestamps that pre-023 hidden services want? */
   int Support022HiddenServices;
 
-  /** Create the Hidden Service directories
-      and hostname files group readable. */
-  int HiddenServiceDirGroupReadable;
-
 } or_options_t;
 
 /** Persistent state for an onion router, as saved to disk. */
-- 
cgit v1.2.3-54-g00ecf


From b59fd2efb61e0b6def3fdbf4b8e359acc852776c Mon Sep 17 00:00:00 2001
From: David Stainton <dstainton415@gmail.com>
Date: Thu, 4 Sep 2014 22:21:30 +0000
Subject: Fix permissions logic

---
 src/common/util.c | 26 ++++++++++++++------------
 1 file changed, 14 insertions(+), 12 deletions(-)

(limited to 'src')

diff --git a/src/common/util.c b/src/common/util.c
index 3f04932112..bf00270df3 100644
--- a/src/common/util.c
+++ b/src/common/util.c
@@ -1988,23 +1988,25 @@ check_private_dir(const char *dirname, cpd_check_t check,
     tor_free(process_groupname);
     return -1;
   }
-  if (check & CPD_CHECK_MODE_ONLY) {
-    if (check & CPD_GROUP_OK || check & CPD_GROUP_READ) {
-      if (!st.st_mode & 0027) {
-        log_warn(LD_FS, "Incorrect permissions on directory %s a.", dirname);
-        return -1;
-      }
-    }
+  if (check & (CPD_GROUP_OK|CPD_GROUP_READ)) {
+    mask = 0027;
   } else {
-    log_warn(LD_FS, "Fixing permissions on directory %s", dirname);
+    mask = 0077;
+  }
+  if (st.st_mode & mask) {
     unsigned new_mode;
-    new_mode = 0700;
-    if (check & CPD_GROUP_OK) {
-      new_mode = 0700;
+    if (check & CPD_CHECK_MODE_ONLY) {
+      log_warn(LD_FS, "Permissions on directory %s are too permissive.",
+               dirname);
+      return -1;
     }
+    log_warn(LD_FS, "Fixing permissions on directory %s", dirname);
+    new_mode = st.st_mode;
+    new_mode |= 0700; /* Owner should have rwx */
     if (check & CPD_GROUP_READ) {
-      new_mode = 0750;
+      new_mode |= 0050; /* Group should have rx */
     }
+    new_mode &= ~mask; /* Clear the other bits that we didn't want set...*/
     if (chmod(dirname, new_mode)) {
       log_warn(LD_FS, "Could not chmod directory %s: %s", dirname,
                strerror(errno));
-- 
cgit v1.2.3-54-g00ecf


From 3d0d49be230a8720ebdadf668b993f8ba2c5b2ca Mon Sep 17 00:00:00 2001
From: meejah <meejah@meejah.ca>
Date: Wed, 15 Oct 2014 02:17:54 -0600
Subject: Additional test for error-case

This error-case was already fixed by previous changes,
this is to cover it in case there's a regression.
---
 src/test/test_checkdir.c | 8 ++++++++
 1 file changed, 8 insertions(+)

(limited to 'src')

diff --git a/src/test/test_checkdir.c b/src/test/test_checkdir.c
index 1580e6271d..7bf735e061 100644
--- a/src/test/test_checkdir.c
+++ b/src/test/test_checkdir.c
@@ -43,6 +43,14 @@ test_checkdir_perms(void *testdata)
   tt_int_op(0, ==, (st.st_mode & unix_verify_optsmask));
   tor_free(testdir);
 
+  /* test: should get an error on existing dir with
+           wrong perms */
+  testdir = get_datadir_fname("checkdir_new_groupok_err");
+  tt_int_op(0, ==, mkdir(testdir, 027));
+  cpd_chkopts = CPD_CHECK_MODE_ONLY|CPD_CREATE|CPD_GROUP_OK;
+  tt_int_op(-1, ==, check_private_dir(testdir, cpd_chkopts, NULL));
+  tor_free(testdir);
+
   /* test: create new dir, CPD_GROUP_READ option set. */
   testdir = get_datadir_fname("checkdir_new_groupread");
   cpd_chkopts = CPD_CREATE|CPD_GROUP_READ;
-- 
cgit v1.2.3-54-g00ecf