diff --git a/changes/ticket27992 b/changes/ticket27992 new file mode 100644 index 0000000000..9329a78915 --- /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 ed9efb6fca..4cbfa01a06 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 edab684d7c..e0a334c797 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"), @@ -1515,11 +1515,39 @@ options_switch_id(char **msg_out) return 0; } +/** + * Helper. Given a data directory (datadir) and another directory + * (subdir) with respective group-writable permissions + * datadir_gr and subdir_gr, 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 0af96a0c26..c6c03329bb 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 aaef1d9110..abc1c65481 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);