[tor-commits] [tor/master] Make KeyDirectory's GroupReadable behave the same as CacheDirectory's.
nickm at torproject.org
nickm at torproject.org
Thu Nov 21 12:49:28 UTC 2019
commit a30d143228b4211fd24093c244117e07e9409de5
Author: Nick Mathewson <nickm at torproject.org>
Date: Tue Nov 19 11:59:21 2019 -0500
Make KeyDirectory's GroupReadable behave the same as CacheDirectory's.
In #26913 we solved a bug where CacheDirectoryGroupReadable would
override DataDirectoryGroupReadable when the two directories are the
same. We never did the same for KeyDirectory, though, because
that's a rare setting.
Now that I'm testing this code, though, fixing this issue seems
fine. Fixes bug #27992; bugfix on 0.3.3.1-alpha.
---
changes/ticket27992 | 5 ++++
doc/tor.1.txt | 8 +++---
src/app/config/config.c | 65 ++++++++++++++++++++++++++++++++-------------
src/app/config/config.h | 2 ++
src/test/test_options_act.c | 6 +----
5 files changed, 59 insertions(+), 27 deletions(-)
diff --git a/changes/ticket27992 b/changes/ticket27992
new file mode 100644
index 000000000..9329a7891
--- /dev/null
+++ b/changes/ticket27992
@@ -0,0 +1,5 @@
+ o Minor bugfixes (configuration):
+ - When creating a KeyDirectory with the same location as the
+ DataDirectory (not recommended), respect the DataDirectory's
+ group-readable setting if one has not been set for the KeyDirectory.
+ Fixes bug 27992; bugfix on 0.3.3.1-alpha.
diff --git a/doc/tor.1.txt b/doc/tor.1.txt
index ed9efb6fc..4cbfa01a0 100644
--- a/doc/tor.1.txt
+++ b/doc/tor.1.txt
@@ -2589,10 +2589,12 @@ is non-zero):
running.
(Default: the "keys" subdirectory of DataDirectory.)
-[[KeyDirectoryGroupReadable]] **KeyDirectoryGroupReadable** **0**|**1**::
+[[KeyDirectoryGroupReadable]] **KeyDirectoryGroupReadable** **0**|**1**|**auto**::
If this option is set to 0, don't allow the filesystem group to read the
- KeywDirectory. If the option is set to 1, make the KeyDirectory readable
- by the default GID. (Default: 0)
+ KeyDirectory. If the option is set to 1, make the KeyDirectory readable
+ by the default GID. If the option is "auto", then we use the
+ setting for DataDirectoryGroupReadable when the KeyDirectory is the
+ same as the DataDirectory, and 0 otherwise. (Default: auto)
[[RephistTrackTime]] **RephistTrackTime** __N__ **seconds**|**minutes**|**hours**|**days**|**weeks**::
Tells an authority, or other node tracking node reliability and history,
diff --git a/src/app/config/config.c b/src/app/config/config.c
index edab684d7..e0a334c79 100644
--- a/src/app/config/config.c
+++ b/src/app/config/config.c
@@ -540,7 +540,7 @@ static const config_var_t option_vars_[] = {
V(Socks5ProxyUsername, STRING, NULL),
V(Socks5ProxyPassword, STRING, NULL),
VAR_IMMUTABLE("KeyDirectory", FILENAME, KeyDirectory_option, NULL),
- V(KeyDirectoryGroupReadable, BOOL, "0"),
+ V(KeyDirectoryGroupReadable, AUTOBOOL, "auto"),
VAR_D("HSLayer2Nodes", ROUTERSET, HSLayer2Nodes, NULL),
VAR_D("HSLayer3Nodes", ROUTERSET, HSLayer3Nodes, NULL),
V(KeepalivePeriod, INTERVAL, "5 minutes"),
@@ -1516,10 +1516,38 @@ options_switch_id(char **msg_out)
}
/**
+ * Helper. Given a data directory (<b>datadir</b>) and another directory
+ * (<b>subdir</b>) with respective group-writable permissions
+ * <b>datadir_gr</b> and <b>subdir_gr</b>, compute whether the subdir should
+ * be group-writeable.
+ **/
+static int
+compute_group_readable_flag(const char *datadir,
+ const char *subdir,
+ int datadir_gr,
+ int subdir_gr)
+{
+ if (subdir_gr != -1) {
+ /* The user specified a default for "subdir", so we always obey it. */
+ return subdir_gr;
+ }
+
+ /* The user left the subdir_gr option on "auto." */
+ if (0 == strcmp(subdir, datadir)) {
+ /* The directories are the same, so we use the group-readable flag from
+ * the datadirectory */
+ return datadir_gr;
+ } else {
+ /* The directores are different, so we default to "not group-readable" */
+ return 0;
+ }
+}
+
+/**
* Create our DataDirectory, CacheDirectory, and KeyDirectory, and
* set their permissions correctly.
*/
-static int
+STATIC int
options_create_directories(char **msg_out)
{
const or_options_t *options = get_options();
@@ -1536,30 +1564,29 @@ options_create_directories(char **msg_out)
msg_out) < 0) {
return -1;
}
+
+ /* We need to handle the group-readable flag for the cache directory and key
+ * directory specially, since they may be the same as the data directory */
+ const int key_dir_group_readable = compute_group_readable_flag(
+ options->DataDirectory,
+ options->KeyDirectory,
+ options->DataDirectoryGroupReadable,
+ options->KeyDirectoryGroupReadable);
+
if (check_and_create_data_directory(running_tor /* create */,
options->KeyDirectory,
- options->KeyDirectoryGroupReadable,
+ key_dir_group_readable,
options->User,
msg_out) < 0) {
return -1;
}
- /* We need to handle the group-readable flag for the cache directory
- * specially, since the directory defaults to being the same as the
- * DataDirectory. */
- int cache_dir_group_readable;
- if (options->CacheDirectoryGroupReadable != -1) {
- /* If the user specified a value, use their setting */
- cache_dir_group_readable = options->CacheDirectoryGroupReadable;
- } else if (!strcmp(options->CacheDirectory, options->DataDirectory)) {
- /* If the user left the value as "auto", and the cache is the same as the
- * datadirectory, use the datadirectory setting.
- */
- cache_dir_group_readable = options->DataDirectoryGroupReadable;
- } else {
- /* Otherwise, "auto" means "not group readable". */
- cache_dir_group_readable = 0;
- }
+ const int cache_dir_group_readable = compute_group_readable_flag(
+ options->DataDirectory,
+ options->CacheDirectory,
+ options->DataDirectoryGroupReadable,
+ options->CacheDirectoryGroupReadable);
+
if (check_and_create_data_directory(running_tor /* create */,
options->CacheDirectory,
cache_dir_group_readable,
diff --git a/src/app/config/config.h b/src/app/config/config.h
index 0af96a0c2..c6c03329b 100644
--- a/src/app/config/config.h
+++ b/src/app/config/config.h
@@ -303,6 +303,8 @@ STATIC int open_and_add_file_log(const log_severity_list_t *severity,
STATIC int options_init_logs(const or_options_t *old_options,
const or_options_t *options, int validate_only);
+STATIC int options_create_directories(char **msg_out);
+
#ifdef TOR_UNIT_TESTS
int options_validate(const or_options_t *old_options,
or_options_t *options,
diff --git a/src/test/test_options_act.c b/src/test/test_options_act.c
index aaef1d911..abc1c6548 100644
--- a/src/test/test_options_act.c
+++ b/src/test/test_options_act.c
@@ -109,11 +109,7 @@ test_options_act_create_dirs(void *arg)
opts->KeyDirectory = tor_strdup(fn);
opts->DataDirectoryGroupReadable = 1;
opts->CacheDirectoryGroupReadable = -1; /* default. */
-#if 1
- /* Bug 27992: this setting shouldn't be needed, but for now it is, in the
- * unusual case that DataDirectory == KeyDirectory */
- opts->KeyDirectoryGroupReadable = 1;
-#endif
+ opts->KeyDirectoryGroupReadable = -1; /* default */
r = options_create_directories(&msg);
tt_int_op(r, OP_EQ, 0);
tt_ptr_op(msg, OP_EQ, NULL);
More information about the tor-commits
mailing list