commit c13db1f6143cf99830dc73dd527898e711e6b704 Author: anonymous anon@fakeanonemail.org Date: Thu Aug 28 18:10:21 2014 +0000
Ticket #11291: patch from "anon":
test-11291-group-redable-hsdirs-wtests-may8.patch --- doc/tor.1.txt | 5 ++ 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 ++++++++++++++++++++++++++++++++++++++++++++++ 10 files changed, 214 insertions(+), 11 deletions(-)
diff --git a/doc/tor.1.txt b/doc/tor.1.txt index ce42a9b..ec5cc14 100644 --- a/doc/tor.1.txt +++ b/doc/tor.1.txt @@ -2078,6 +2078,11 @@ 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**:: + 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) + TESTING NETWORK OPTIONS -----------------------
diff --git a/src/common/util.c b/src/common/util.c index 16ff8e3..6971e4d 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 61bb05f..c98a32c 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 7800ec1..1753722 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 3683607..9207dba 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 749d6fa..83e6a3b 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 822431f..f6ee7f3 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 77c92f1..36d86c7 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 e836160..6d7c6fa 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 0000000..877c789 --- /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 +}; +