[tor-commits] [tor/master] Clean up patch

nickm at torproject.org nickm at torproject.org
Wed Nov 5 19:19:18 UTC 2014


commit 227b65924b557b30855f659360a8547e352c1ec6
Author: David Stainton <dstainton415 at gmail.com>
Date:   Fri Aug 29 05:58:53 2014 +0000

    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(-)

diff --git a/doc/tor.1.txt b/doc/tor.1.txt
index ec5cc14..272eba3 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 6971e4d..0865fe7 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 c98a32c..755ef4b 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 1753722..97b3601 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 9207dba..1544b70 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 83e6a3b..456b548 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 877c789..76fe131 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));





More information about the tor-commits mailing list