[tor-commits] [tor/master] Ticket #11291: patch from "anon":
nickm at torproject.org
nickm at torproject.org
Wed Nov 5 19:19:18 UTC 2014
commit c13db1f6143cf99830dc73dd527898e711e6b704
Author: anonymous <anon at 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
+};
+
More information about the tor-commits
mailing list