diff --git a/changes/bug28127 b/changes/bug28127 new file mode 100644 index 0000000000..541128c88e --- /dev/null +++ b/changes/bug28127 @@ -0,0 +1,7 @@ + o Minor bugfixes (onion services): + - Unless we have explicitly set HiddenServiceVersion, detect the onion + service version and then look for invalid options. Previously, we + did the reverse, but that broke existing configs which were pointed + to a v2 hidden service and had options like HiddenServiceAuthorizeClient + set Fixes bug 28127; bugfix on 0.3.5.1-alpha. Patch by Neel Chauhan. + diff --git a/src/feature/hs/hs_config.c b/src/feature/hs/hs_config.c index 93d7403dfb..497e31fbb4 100644 --- a/src/feature/hs/hs_config.c +++ b/src/feature/hs/hs_config.c @@ -419,7 +419,7 @@ config_generic_service(const config_line_t *line_, dup_opt_seen = line->key; goto err; } - have_version = 1; + have_version = service->config.hs_version_explicitly_set = 1; continue; } /* Virtual port. */ @@ -534,18 +534,15 @@ config_service(const config_line_t *line, const or_options_t *options, /* We have a new hidden service. */ service = hs_service_new(options); + /* We'll configure that service as a generic one and then pass it to a * specific function according to the configured version number. */ if (config_generic_service(line, options, service) < 0) { goto err; } + tor_assert(service->config.version <= HS_VERSION_MAX); - /* Before we configure the service on a per-version basis, we'll make - * sure that this set of options for a service are valid that is for - * instance an option only for v2 is not used for v3. */ - if (config_has_invalid_options(line->next, service)) { - goto err; - } + /* Check permission on service directory that was just parsed. And this must * be done regardless of the service version. Do not ask for the directory * to be created, this is done when the keys are loaded because we could be @@ -556,11 +553,19 @@ config_service(const config_line_t *line, const or_options_t *options, 0) < 0) { goto err; } + /* We'll try to learn the service version here by loading the key(s) if - * present. Depending on the key format, we can figure out the service - * version. If we can't find a key, the configuration version will be used - * which has been set previously. */ - service->config.version = config_learn_service_version(service); + * present and we did not set HiddenServiceVersion. Depending on the key + * format, we can figure out the service version. */ + if (!service->config.hs_version_explicitly_set) { + service->config.version = config_learn_service_version(service); + } + + /* We make sure that this set of options for a service are valid that is for + * instance an option only for v2 is not used for v3. */ + if (config_has_invalid_options(line->next, service)) { + goto err; + } /* Different functions are in charge of specific options for a version. We * start just after the service directory line so once we hit another @@ -580,13 +585,16 @@ config_service(const config_line_t *line, const or_options_t *options, if (ret < 0) { goto err; } + /* We'll check if this service can be kept depending on the others * configured previously. */ if (service_is_duplicate_in_list(service_list, service)) { goto err; } + /* Passes, add it to the given list. */ smartlist_add(service_list, service); + return 0; err: diff --git a/src/feature/hs/hs_service.h b/src/feature/hs/hs_service.h index 6fb15b9d37..cc0006f1dc 100644 --- a/src/feature/hs/hs_service.h +++ b/src/feature/hs/hs_service.h @@ -178,6 +178,9 @@ typedef struct hs_service_config_t { * option. */ uint32_t version; + /* Have we explicitly set HiddenServiceVersion? */ + unsigned int hs_version_explicitly_set : 1; + /* List of rend_service_port_config_t */ smartlist_t *ports; diff --git a/src/test/test_hs_config.c b/src/test/test_hs_config.c index 553b96758a..b6ab0c21f9 100644 --- a/src/test/test_hs_config.c +++ b/src/test/test_hs_config.c @@ -366,6 +366,22 @@ test_invalid_service_v3(void *arg) teardown_capture_of_logs(); } + /* v2-specific HiddenServiceAuthorizeClient set. */ + { + const char *conf = + "HiddenServiceDir /tmp/tor-test-hs-RANDOM/hs1\n" + "HiddenServiceVersion 3\n" + "HiddenServiceAuthorizeClient stealth client1\n"; + setup_full_capture_of_logs(LOG_WARN); + ret = helper_config_service(conf, validate_only); + tt_int_op(ret, OP_EQ, -1); + expect_log_msg_containing("Hidden service option " + "HiddenServiceAuthorizeClient is incompatible " + "with version 3 of service in " + "/tmp/tor-test-hs-RANDOM/hs1"); + teardown_capture_of_logs(); + } + done: ; }