From 9dc946ba67a5d457ce00bf2853f624c3c7c344aa Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Wed, 26 Feb 2020 10:10:27 -0500 Subject: [PATCH 1/7] Add a config_lines_partition() function to help with LINELIST_V. This function works a little bit like strsep(), to get a chunk of configuration lines with a given header. We can use this to make hidden service config easier to parse. --- src/lib/encoding/confline.c | 29 +++++++++++++++++++++ src/lib/encoding/confline.h | 1 + src/test/test_util.c | 51 +++++++++++++++++++++++++++++++++++++ 3 files changed, 81 insertions(+) diff --git a/src/lib/encoding/confline.c b/src/lib/encoding/confline.c index ff8bacba3c..eb1a4e30f0 100644 --- a/src/lib/encoding/confline.c +++ b/src/lib/encoding/confline.c @@ -253,6 +253,35 @@ config_lines_dup_and_filter(const config_line_t *inp, return result; } +/** + * Given a linelist inp beginning with the key header, find the + * next line with that key, and remove that instance and all following lines + * from the list. Return the lines that were removed. Operate + * case-insensitively. + * + * For example, if the header is "H", and inp contains "H, A, B, H, C, + * H, D", this function will alter inp to contain only "H, A, B", and + * return the elements "H, C, H, D" as a separate list. + **/ +config_line_t * +config_lines_partition(config_line_t *inp, const char *header) +{ + if (BUG(inp == NULL)) + return NULL; + if (BUG(strcasecmp(inp->key, header))) + return NULL; + + /* Advance ptr until it points to the link to the next segment of this + list. */ + config_line_t **ptr = &inp->next; + while (*ptr && strcasecmp((*ptr)->key, header)) { + ptr = &(*ptr)->next; + } + config_line_t *remainder = *ptr; + *ptr = NULL; + return remainder; +} + /** Return true iff a and b contain identical keys and values in identical * order. */ int diff --git a/src/lib/encoding/confline.h b/src/lib/encoding/confline.h index cd343e0e99..ce0d6c6e17 100644 --- a/src/lib/encoding/confline.h +++ b/src/lib/encoding/confline.h @@ -50,6 +50,7 @@ const config_line_t *config_line_find(const config_line_t *lines, const char *key); const config_line_t *config_line_find_case(const config_line_t *lines, const char *key); +config_line_t *config_lines_partition(config_line_t *inp, const char *header); int config_lines_eq(const config_line_t *a, const config_line_t *b); int config_count_key(const config_line_t *a, const char *key); void config_free_lines_(config_line_t *front); diff --git a/src/test/test_util.c b/src/test/test_util.c index fecd279096..234ae06745 100644 --- a/src/test/test_util.c +++ b/src/test/test_util.c @@ -1850,6 +1850,56 @@ test_util_config_line_crlf(void *arg) tor_free(k); tor_free(v); } +static void +test_util_config_line_partition(void *arg) +{ + (void)arg; + config_line_t *lines = NULL, *orig, *rest = NULL; + + config_line_append(&lines, "Header", "X"); + config_line_append(&lines, "Item", "Y"); + config_line_append(&lines, "Thing", "Z"); + + config_line_append(&lines, "HEADER", "X2"); + + config_line_append(&lines, "header", "X3"); + config_line_append(&lines, "Item3", "Foob"); + + /* set up h2 and h3 to point to the places where we hope the headers will + be. */ + config_line_t *h2 = lines->next->next->next; + config_line_t *h3 = h2->next; + tt_str_op(h2->key, OP_EQ, "HEADER"); + tt_str_op(h3->key, OP_EQ, "header"); + + orig = lines; + rest = config_lines_partition(lines, "Header"); + tt_ptr_op(lines, OP_EQ, orig); + tt_ptr_op(rest, OP_EQ, h2); + tt_str_op(lines->next->key, OP_EQ, "Item"); + tt_str_op(lines->next->next->key, OP_EQ, "Thing"); + tt_ptr_op(lines->next->next->next, OP_EQ, NULL); + config_free_lines(lines); + + orig = lines = rest; + rest = config_lines_partition(lines, "Header"); + tt_ptr_op(lines, OP_EQ, orig); + tt_ptr_op(rest, OP_EQ, h3); + tt_ptr_op(lines->next, OP_EQ, NULL); + config_free_lines(lines); + + orig = lines = rest; + rest = config_lines_partition(lines, "Header"); + tt_ptr_op(lines, OP_EQ, orig); + tt_ptr_op(rest, OP_EQ, NULL); + tt_str_op(lines->next->key, OP_EQ, "Item3"); + tt_ptr_op(lines->next->next, OP_EQ, NULL); + + done: + config_free_lines(lines); + config_free_lines(rest); +} + #ifndef DISABLE_PWDB_TESTS static void test_util_expand_filename(void *arg) @@ -6379,6 +6429,7 @@ struct testcase_t util_tests[] = { UTIL_LEGACY(config_line_comment_character), UTIL_LEGACY(config_line_escaped_content), UTIL_LEGACY(config_line_crlf), + UTIL_TEST(config_line_partition, 0), UTIL_TEST_PWDB(expand_filename, 0), UTIL_LEGACY(escape_string_socks), UTIL_LEGACY(string_is_key_value), From 43b578e099caac25544abdbfa4bd2c24539a8b8d Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Wed, 26 Feb 2020 10:12:54 -0500 Subject: [PATCH 2/7] Use config_lines_partition() to parse hs config sections. --- src/feature/hs/hs_config.c | 54 ++++++++++++++++++++------------------ 1 file changed, 29 insertions(+), 25 deletions(-) diff --git a/src/feature/hs/hs_config.c b/src/feature/hs/hs_config.c index 684f7bc975..f38e3655db 100644 --- a/src/feature/hs/hs_config.c +++ b/src/feature/hs/hs_config.c @@ -444,6 +444,12 @@ config_service_v3(const config_line_t *line_, return -1; } +/** + * Header key indicating the start of a new hidden service configuration + * block. + **/ +static const char SECTION_HEADER[] = "HiddenServiceDir"; + /** Configure a service using the given options in line_ and options. This is * called for any service regardless of its version which means that all * directives in this function are generic to any service version. This @@ -456,12 +462,11 @@ config_service_v3(const config_line_t *line_, * * Return 0 on success else -1. */ static int -config_generic_service(const config_line_t *line_, +config_generic_service(const config_line_t *line, const or_options_t *options, hs_service_t *service) { int dir_seen = 0; - const config_line_t *line; hs_service_config_t *config; /* If this is set, we've seen a duplicate of this option. Keep the string * so we can log the directive. */ @@ -471,25 +476,23 @@ config_generic_service(const config_line_t *line_, int have_dir_group_read = 0, have_max_streams = 0; int have_max_streams_close = 0; - tor_assert(line_); + tor_assert(line); tor_assert(options); tor_assert(service); /* Makes thing easier. */ config = &service->config; + if (BUG(strcasecmp(line->key, "HiddenServiceDir"))) + return -1; + /* The first line starts with HiddenServiceDir so we consider what's next is * the configuration of the service. */ - for (line = line_; line ; line = line->next) { + for ( ; line ; line = line->next) { int ok = 0; /* This indicate that we have a new service to configure. */ if (!strcasecmp(line->key, "HiddenServiceDir")) { - /* This function only configures one service at a time so if we've - * already seen one, stop right now. */ - if (dir_seen) { - break; - } /* Ok, we've seen one and we are about to configure it. */ dir_seen = 1; config->directory_path = tor_strdup(line->value); @@ -690,8 +693,8 @@ config_service(const config_line_t *line, const or_options_t *options, int hs_config_service_all(const or_options_t *options, int validate_only) { - int dir_option_seen = 0, ret = -1; - const config_line_t *line; + int ret = -1; + config_line_t *remaining = NULL; smartlist_t *new_service_list = NULL; tor_assert(options); @@ -700,23 +703,24 @@ hs_config_service_all(const or_options_t *options, int validate_only) * validation and staging for >= v3. */ new_service_list = smartlist_new(); - for (line = options->RendConfigLines; line; line = line->next) { - /* Ignore all directives that aren't the start of a service. */ - if (strcasecmp(line->key, "HiddenServiceDir")) { - if (!dir_option_seen) { - log_warn(LD_CONFIG, "%s with no preceding HiddenServiceDir directive", - line->key); - goto err; - } - continue; - } - /* Flag that we've seen a directory directive and we'll use it to make - * sure that the torrc options ordering is actually valid. */ - dir_option_seen = 1; + /* We need to start with a HiddenServiceDir line */ + if (options->RendConfigLines && + strcasecmp(options->RendConfigLines->key, SECTION_HEADER)) { + log_warn(LD_CONFIG, "%s with no preceding %s directive", + options->RendConfigLines->key, SECTION_HEADER); + goto err; + } + + remaining = config_lines_dup(options->RendConfigLines); + while (remaining) { + config_line_t *section = remaining; + remaining = config_lines_partition(section, SECTION_HEADER); /* Try to configure this service now. On success, it will be added to the * list and validated against the service in that same list. */ - if (config_service(line, options, new_service_list) < 0) { + int rv = config_service(section, options, new_service_list); + config_free_lines(section); + if (rv < 0) { goto err; } } From cfaf1bca989f1841acb5360a482e4e5da3ede7dd Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Tue, 21 Jan 2020 10:23:27 -0500 Subject: [PATCH 3/7] Start using confmgt logic to parse HS configuration. This patch doesn't actually use the results of the parsed object to create the service configuration: subsequent patches will do that. This patch just introduces the necessary configuration tables and starts using them to validate the configuration. As of this writing, this patch breaks tests. I'll likely fix that in a rebase later on: the current error messages for failures to parse options are a regression, so I've opened #33640 for that. --- src/feature/hs/.may_include | 1 + src/feature/hs/hs_config.c | 87 ++++++++++++++++++++++++++++++++++- src/feature/hs/hs_config.h | 3 +- src/feature/hs/hs_options.inc | 35 ++++++++++++++ src/feature/hs/hs_opts_st.h | 30 ++++++++++++ src/feature/hs/hs_service.c | 1 + src/feature/hs/include.am | 2 + 7 files changed, 157 insertions(+), 2 deletions(-) create mode 100644 src/feature/hs/hs_options.inc create mode 100644 src/feature/hs/hs_opts_st.h diff --git a/src/feature/hs/.may_include b/src/feature/hs/.may_include index 424c745c12..11c5ffbb14 100644 --- a/src/feature/hs/.may_include +++ b/src/feature/hs/.may_include @@ -1 +1,2 @@ *.h +*.inc diff --git a/src/feature/hs/hs_config.c b/src/feature/hs/hs_config.c index f38e3655db..0f2cbbd588 100644 --- a/src/feature/hs/hs_config.c +++ b/src/feature/hs/hs_config.c @@ -31,8 +31,63 @@ #include "feature/rend/rendclient.h" #include "feature/rend/rendservice.h" #include "lib/encoding/confline.h" +#include "lib/conf/confdecl.h" +#include "lib/confmgt/confmgt.h" + +#include "feature/hs/hs_opts_st.h" #include "app/config/or_options_st.h" +/* Declare the table mapping hs options to hs_opts_t */ +#define CONF_CONTEXT TABLE +#include "feature/hs/hs_options.inc" +#undef CONF_CONTEXT + +/** Magic number for hs_opts_t. */ +#define HS_OPTS_MAGIC 0x6f6e796e + +static const config_format_t hs_opts_fmt = { + .size = sizeof(hs_opts_t), + .magic = { "hs_opts_t", + HS_OPTS_MAGIC, + offsetof(hs_opts_t, magic) }, + .vars = hs_opts_t_vars, +}; + +/** Global configuration manager to handle HS sections*/ +static config_mgr_t *hs_opts_mgr = NULL; + +/** + * Return a configuration manager for the hs_opts_t configuration type. + **/ +static const config_mgr_t * +get_hs_opts_mgr(void) +{ + if (PREDICT_UNLIKELY(hs_opts_mgr == NULL)) { + hs_opts_mgr = config_mgr_new(&hs_opts_fmt); + config_mgr_freeze(hs_opts_mgr); + } + return hs_opts_mgr; +} + +/** + * Allocate, initialize, and return a new hs_opts_t. + **/ +static hs_opts_t * +hs_opts_new(void) +{ + const config_mgr_t *mgr = get_hs_opts_mgr(); + hs_opts_t *r = config_new(mgr); + tor_assert(r); + config_init(mgr, r); + return r; +} + +/** + * Free an hs_opts_t. + **/ +#define hs_opts_free(opts) \ + config_free(get_hs_opts_mgr(), (opts)) + /** Using the given list of services, stage them into our global state. Every * service version are handled. This function can remove entries in the given * service_list. @@ -607,11 +662,13 @@ config_generic_service(const config_line_t *line, * the service to the given list and return 0. On error, nothing is added to * the list and a negative value is returned. */ static int -config_service(const config_line_t *line, const or_options_t *options, +config_service(config_line_t *line, const or_options_t *options, smartlist_t *service_list) { int ret; hs_service_t *service = NULL; + hs_opts_t *hs_opts = NULL; + char *msg = NULL; tor_assert(line); tor_assert(options); @@ -620,6 +677,22 @@ config_service(const config_line_t *line, const or_options_t *options, /* We have a new hidden service. */ service = hs_service_new(options); + /* Try to validate and parse the configuration lines into 'hs_opts' */ + hs_opts = hs_opts_new(); + ret = config_assign(get_hs_opts_mgr(), hs_opts, line, 0, &msg); + if (ret < 0) { + log_warn(LD_REND, "Can't parse configuration for onion service: %s", msg); + goto err; + } + tor_assert_nonfatal(msg == NULL); + validation_status_t vs = config_validate(get_hs_opts_mgr(), NULL, + hs_opts, &msg); + if (vs < 0) { + log_warn(LD_REND, "Bad configuration for onion service: %s", msg); + goto err; + } + tor_assert_nonfatal(msg == NULL); + /* 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) { @@ -679,11 +752,14 @@ config_service(const config_line_t *line, const or_options_t *options, /* Passes, add it to the given list. */ smartlist_add(service_list, service); + hs_opts_free(hs_opts); return 0; err: hs_service_free(service); + hs_opts_free(hs_opts); + tor_free(msg); return -1; } @@ -780,3 +856,12 @@ hs_config_client_auth_all(const or_options_t *options, int validate_only) done: return ret; } + +/** + * Free all resources held by the hs_config.c module. + **/ +void +hs_config_free_all(void) +{ + config_mgr_free(hs_opts_mgr); +} diff --git a/src/feature/hs/hs_config.h b/src/feature/hs/hs_config.h index 5694cf1e9b..c60b4fbb5d 100644 --- a/src/feature/hs/hs_config.h +++ b/src/feature/hs/hs_config.h @@ -30,5 +30,6 @@ int hs_config_service_all(const or_options_t *options, int validate_only); int hs_config_client_auth_all(const or_options_t *options, int validate_only); -#endif /* !defined(TOR_HS_CONFIG_H) */ +void hs_config_free_all(void); +#endif /* !defined(TOR_HS_CONFIG_H) */ diff --git a/src/feature/hs/hs_options.inc b/src/feature/hs/hs_options.inc new file mode 100644 index 0000000000..c3566f4461 --- /dev/null +++ b/src/feature/hs/hs_options.inc @@ -0,0 +1,35 @@ +/* Copyright (c) 2001 Matej Pfajfar. + * Copyright (c) 2001-2004, Roger Dingledine. + * Copyright (c) 2004-2006, Roger Dingledine, Nick Mathewson. + * Copyright (c) 2007-2019, The Tor Project, Inc. */ +/* See LICENSE for licensing information */ + +/** + * @file dirauth_options.inc + * @brief Declare configuration options for a single hidden service. + * + * Note that this options file behaves differently from most, since it + * is not used directly by the options manager. Instead, it is applied to + * a group of hidden service options starting with a HiddenServiceDir and + * extending up to the next HiddenServiceDir. + **/ + +/** Holds configuration for a single hidden service. */ +BEGIN_CONF_STRUCT(hs_opts_t) + +CONF_VAR(HiddenServiceDir, FILENAME, 0, NULL) +CONF_VAR(HiddenServiceDirGroupReadable, BOOL, 0, "0") +CONF_VAR(HiddenServicePort, STRING, 0, NULL) +CONF_VAR(HiddenServiceVersion, POSINT, 0, "3") +CONF_VAR(HiddenServiceAuthorizeClient, STRING, 0, NULL) +CONF_VAR(HiddenServiceAllowUnknownPorts, BOOL, 0, "0") +CONF_VAR(HiddenServiceMaxStreams, POSINT, 0, "0") +CONF_VAR(HiddenServiceMaxStreamsCloseCircuit, BOOL, 0, "0") +CONF_VAR(HiddenServiceNumIntroductionPoints, POSINT, 0, "3") +CONF_VAR(HiddenServiceExportCircuitID, STRING, 0, NULL) +CONF_VAR(HiddenServiceEnableIntroDoSDefense, BOOL, 0, "0") +CONF_VAR(HiddenServiceEnableIntroDoSRatePerSec, POSINT, 0, "25") +CONF_VAR(HiddenServiceEnableIntroDoSBurstPerSec, POSINT, 0, "200") +CONF_VAR(HiddenServiceOnionBalanceInstance, BOOL, 0, "0") + +END_CONF_STRUCT(hs_opts_t) diff --git a/src/feature/hs/hs_opts_st.h b/src/feature/hs/hs_opts_st.h new file mode 100644 index 0000000000..f6ee0f3cfa --- /dev/null +++ b/src/feature/hs/hs_opts_st.h @@ -0,0 +1,30 @@ +/* Copyright (c) 2001 Matej Pfajfar. + * Copyright (c) 2001-2004, Roger Dingledine. + * Copyright (c) 2004-2006, Roger Dingledine, Nick Mathewson. + * Copyright (c) 2007-2020, The Tor Project, Inc. */ +/* See LICENSE for licensing information */ + +/** + * @file dirauth_options_st.h + * @brief Structure hs_opts_t to hold options for a single hidden service. + **/ + +#ifndef TOR_FEATURE_HS_HS_OPTS_ST_H +#define TOR_FEATURE_HS_HS_OPTS_ST_H + +#include "lib/conf/confdecl.h" +#define CONF_CONTEXT STRUCT +#include "feature/hs/hs_options.inc" +#undef CONF_CONTEXT + +/** + * An hs_opts_t holds the parsed options for a single HS configuration + * section. + * + * This name ends with 'opts' instead of 'options' to signal that it is not + * handled directly by the or_options_t configuration manager, but that + * first we partition the "HiddenService*" options by section. + **/ +typedef struct hs_opts_t hs_opts_t; + +#endif diff --git a/src/feature/hs/hs_service.c b/src/feature/hs/hs_service.c index 8b51b770a9..8c30b56b04 100644 --- a/src/feature/hs/hs_service.c +++ b/src/feature/hs/hs_service.c @@ -4112,6 +4112,7 @@ hs_service_free_all(void) { rend_service_free_all(); service_free_all(); + hs_config_free_all(); } #ifdef TOR_UNIT_TESTS diff --git a/src/feature/hs/include.am b/src/feature/hs/include.am index f83907c76b..af1dc65585 100644 --- a/src/feature/hs/include.am +++ b/src/feature/hs/include.am @@ -32,6 +32,8 @@ noinst_HEADERS += \ src/feature/hs/hs_ident.h \ src/feature/hs/hs_intropoint.h \ src/feature/hs/hs_ob.h \ + src/feature/hs/hs_opts_st.h \ + src/feature/hs/hs_options.inc \ src/feature/hs/hs_service.h \ src/feature/hs/hs_stats.h \ src/feature/hs/hsdir_index_st.h From d421050f3af14d04b316e5fad6e76b001f618cb2 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Fri, 28 Feb 2020 08:46:18 -0500 Subject: [PATCH 4/7] Derive hidden service configuration from hs_opts_t. This simplifies our parsing code by about 150 lines, and makes the functions more straightforward. --- src/feature/hs/hs_config.c | 371 +++++++++++---------------------- src/feature/hs/hs_options.inc | 5 +- src/feature/rend/rendservice.c | 216 +++++++++---------- src/feature/rend/rendservice.h | 3 +- 4 files changed, 225 insertions(+), 370 deletions(-) diff --git a/src/feature/hs/hs_config.c b/src/feature/hs/hs_config.c index 0f2cbbd588..4b55ba2af0 100644 --- a/src/feature/hs/hs_config.c +++ b/src/feature/hs/hs_config.c @@ -172,31 +172,18 @@ service_is_duplicate_in_list(const smartlist_t *service_list, return ret; } -/** Helper function: Given an configuration option name, its value, a minimum - * min and a maxium max, parse the value as a uint64_t. On success, ok is set - * to 1 and ret is the parsed value. On error, ok is set to 0 and ret must be - * ignored. This function logs both on error and success. */ -static uint64_t -helper_parse_uint64(const char *opt, const char *value, uint64_t min, - uint64_t max, int *ok) +/** Check whether an integer i is out of bounds (not between low + * and high incusive). If it is, then log a warning about the option + * name, and return true. Otherwise return false. */ +static bool +check_value_oob(int i, const char *name, int low, int high) { - uint64_t ret = 0; - - tor_assert(opt); - tor_assert(value); - tor_assert(ok); - - *ok = 0; - ret = tor_parse_uint64(value, 10, min, max, ok, NULL); - if (!*ok) { - log_warn(LD_CONFIG, "%s must be between %" PRIu64 " and %"PRIu64 - ", not %s.", - opt, min, max, value); - goto err; + if (i < low || i > high) { + log_warn(LD_CONFIG, "%s must be between %d and %d, not %d.", + name, low, high, i); + return true; } - log_info(LD_CONFIG, "%s was parsed to %" PRIu64, opt, ret); - err: - return ret; + return false; } /** Helper function: Given a configuration option and its value, parse the @@ -359,126 +346,71 @@ config_validate_service(const hs_service_config_t *config) return -1; } -/** Configuration funcion for a version 3 service. The line_ must be pointing - * to the directive directly after a HiddenServiceDir. That way, when hitting - * the next HiddenServiceDir line or reaching the end of the list of lines, we - * know that we have to stop looking for more options. The given service +/** Configuration funcion for a version 3 service. The given service * object must be already allocated and passed through * config_generic_service() prior to calling this function. * * Return 0 on success else a negative value. */ static int -config_service_v3(const config_line_t *line_, +config_service_v3(const hs_opts_t *hs_opts, hs_service_config_t *config) { - int have_num_ip = 0; - bool export_circuit_id = false; /* just to detect duplicate options */ - bool dos_enabled = false, dos_rate_per_sec = false; - bool dos_burst_per_sec = false, ob_instance = false; - const char *dup_opt_seen = NULL; - const config_line_t *line; - tor_assert(config); + tor_assert(hs_opts); - for (line = line_; line; line = line->next) { - int ok = 0; - if (!strcasecmp(line->key, "HiddenServiceDir")) { - /* We just hit the next hidden service, stop right now. */ - break; + /* Number of introduction points. */ + if (check_value_oob(hs_opts->HiddenServiceNumIntroductionPoints, + "HiddenServiceNumIntroductionPoints", + NUM_INTRO_POINTS_DEFAULT, + HS_CONFIG_V3_MAX_INTRO_POINTS)) { + goto err; + } + config->num_intro_points = hs_opts->HiddenServiceNumIntroductionPoints; + + /* Circuit ID export setting. */ + if (hs_opts->HiddenServiceExportCircuitID) { + int ok; + config->circuit_id_protocol = + helper_parse_circuit_id_protocol("HiddenServcieExportCircuitID", + hs_opts->HiddenServiceExportCircuitID, + &ok); + if (!ok) { + goto err; } - /* Number of introduction points. */ - if (!strcasecmp(line->key, "HiddenServiceNumIntroductionPoints")) { - config->num_intro_points = - (unsigned int) helper_parse_uint64(line->key, line->value, - NUM_INTRO_POINTS_DEFAULT, - HS_CONFIG_V3_MAX_INTRO_POINTS, - &ok); - if (!ok || have_num_ip) { - if (have_num_ip) - dup_opt_seen = line->key; - goto err; - } - have_num_ip = 1; - continue; - } - if (!strcasecmp(line->key, "HiddenServiceExportCircuitID")) { - config->circuit_id_protocol = - helper_parse_circuit_id_protocol(line->key, line->value, &ok); - if (!ok || export_circuit_id) { - if (export_circuit_id) { - dup_opt_seen = line->key; - } - goto err; - } - export_circuit_id = true; - continue; - } - if (!strcasecmp(line->key, "HiddenServiceEnableIntroDoSDefense")) { - config->has_dos_defense_enabled = - (unsigned int) helper_parse_uint64(line->key, line->value, - HS_CONFIG_V3_DOS_DEFENSE_DEFAULT, - 1, &ok); - if (!ok || dos_enabled) { - if (dos_enabled) { - dup_opt_seen = line->key; - } - goto err; - } - dos_enabled = true; - continue; - } - if (!strcasecmp(line->key, "HiddenServiceEnableIntroDoSRatePerSec")) { - config->intro_dos_rate_per_sec = - (unsigned int) helper_parse_uint64(line->key, line->value, - HS_CONFIG_V3_DOS_DEFENSE_RATE_PER_SEC_MIN, - HS_CONFIG_V3_DOS_DEFENSE_RATE_PER_SEC_MAX, &ok); - if (!ok || dos_rate_per_sec) { - if (dos_rate_per_sec) { - dup_opt_seen = line->key; - } - goto err; - } - dos_rate_per_sec = true; - log_info(LD_REND, "Service INTRO2 DoS defenses rate set to: %" PRIu32, - config->intro_dos_rate_per_sec); - continue; - } - if (!strcasecmp(line->key, "HiddenServiceEnableIntroDoSBurstPerSec")) { - config->intro_dos_burst_per_sec = - (unsigned int) helper_parse_uint64(line->key, line->value, - HS_CONFIG_V3_DOS_DEFENSE_BURST_PER_SEC_MIN, - HS_CONFIG_V3_DOS_DEFENSE_BURST_PER_SEC_MAX, &ok); - if (!ok || dos_burst_per_sec) { - if (dos_burst_per_sec) { - dup_opt_seen = line->key; - } - goto err; - } - dos_burst_per_sec = true; - log_info(LD_REND, "Service INTRO2 DoS defenses burst set to: %" PRIu32, - config->intro_dos_burst_per_sec); - continue; - } - if (!strcasecmp(line->key, "HiddenServiceOnionBalanceInstance")) { - bool enabled = !!helper_parse_uint64(line->key, line->value, - 0, 1, &ok); - if (!ok || ob_instance) { - if (ob_instance) { - dup_opt_seen = line->key; - } - goto err; - } - ob_instance = true; - if (!enabled) { - /* Skip if this is disabled. */ - continue; - } - /* Option is enabled, parse config file. */ - ok = hs_ob_parse_config_file(config); - if (!ok) { - goto err; - } - continue; + } + + /* Is the DoS defense enabled? */ + config->has_dos_defense_enabled = + hs_opts->HiddenServiceEnableIntroDoSDefense; + + /* Rate for DoS defense */ + if (check_value_oob(hs_opts->HiddenServiceEnableIntroDoSRatePerSec, + "HiddenServiceEnableIntroDoSRatePerSec", + HS_CONFIG_V3_DOS_DEFENSE_RATE_PER_SEC_MIN, + HS_CONFIG_V3_DOS_DEFENSE_RATE_PER_SEC_MAX)) { + goto err; + } + config->intro_dos_rate_per_sec = + hs_opts->HiddenServiceEnableIntroDoSRatePerSec; + log_info(LD_REND, "Service INTRO2 DoS defenses rate set to: %" PRIu32, + config->intro_dos_rate_per_sec); + + if (check_value_oob(hs_opts->HiddenServiceEnableIntroDoSBurstPerSec, + "HiddenServiceEnableIntroDoSBurstPerSec", + HS_CONFIG_V3_DOS_DEFENSE_BURST_PER_SEC_MIN, + HS_CONFIG_V3_DOS_DEFENSE_BURST_PER_SEC_MAX)) { + goto err; + } + config->intro_dos_burst_per_sec = + hs_opts->HiddenServiceEnableIntroDoSBurstPerSec; + log_info(LD_REND, "Service INTRO2 DoS defenses burst set to: %" PRIu32, + config->intro_dos_burst_per_sec); + + /* Is this an onionbalance instance? */ + if (hs_opts->HiddenServiceOnionBalanceInstance) { + /* Option is enabled, parse config file. */ + if (! hs_ob_parse_config_file(config)) { + goto err; } } @@ -493,9 +425,6 @@ config_service_v3(const config_line_t *line_, return 0; err: - if (dup_opt_seen) { - log_warn(LD_CONFIG, "Duplicate directive %s.", dup_opt_seen); - } return -1; } @@ -505,7 +434,7 @@ config_service_v3(const config_line_t *line_, **/ static const char SECTION_HEADER[] = "HiddenServiceDir"; -/** Configure a service using the given options in line_ and options. This is +/** Configure a service using the given options in hs_opts and options. This is * called for any service regardless of its version which means that all * directives in this function are generic to any service version. This * function will also check the validity of the service directory path. @@ -517,130 +446,75 @@ static const char SECTION_HEADER[] = "HiddenServiceDir"; * * Return 0 on success else -1. */ static int -config_generic_service(const config_line_t *line, +config_generic_service(const hs_opts_t *hs_opts, const or_options_t *options, hs_service_t *service) { - int dir_seen = 0; hs_service_config_t *config; - /* If this is set, we've seen a duplicate of this option. Keep the string - * so we can log the directive. */ - const char *dup_opt_seen = NULL; - /* These variables will tell us if we ever have duplicate. */ - int have_version = 0, have_allow_unknown_ports = 0; - int have_dir_group_read = 0, have_max_streams = 0; - int have_max_streams_close = 0; - tor_assert(line); + tor_assert(hs_opts); tor_assert(options); tor_assert(service); /* Makes thing easier. */ config = &service->config; - if (BUG(strcasecmp(line->key, "HiddenServiceDir"))) - return -1; + /* Directory where the service's keys are stored. */ + tor_assert(hs_opts->HiddenServiceDir); + config->directory_path = tor_strdup(hs_opts->HiddenServiceDir); + log_info(LD_CONFIG, "HiddenServiceDir=%s. Configuring...", + escaped(config->directory_path)); - /* The first line starts with HiddenServiceDir so we consider what's next is - * the configuration of the service. */ - for ( ; line ; line = line->next) { - int ok = 0; + /* Protocol version for the service. */ + if (hs_opts->HiddenServiceVersion == -1) { + /* No value was set; stay with the default. */ + } else if (check_value_oob(hs_opts->HiddenServiceVersion, + "HiddenServiceVersion", + HS_VERSION_MIN, HS_VERSION_MAX)) { + goto err; + } else { + config->hs_version_explicitly_set = 1; + config->version = hs_opts->HiddenServiceVersion; + } - /* This indicate that we have a new service to configure. */ - if (!strcasecmp(line->key, "HiddenServiceDir")) { - /* Ok, we've seen one and we are about to configure it. */ - dir_seen = 1; - config->directory_path = tor_strdup(line->value); - log_info(LD_CONFIG, "HiddenServiceDir=%s. Configuring...", - escaped(config->directory_path)); - continue; - } - if (BUG(!dir_seen)) { + /* Virtual port. */ + for (const config_line_t *portline = hs_opts->HiddenServicePort; + portline; portline = portline->next) { + char *err_msg = NULL; + /* XXX: Can we rename this? */ + rend_service_port_config_t *portcfg = + rend_service_parse_port_config(portline->value, " ", &err_msg); + if (!portcfg) { + if (err_msg) { + log_warn(LD_CONFIG, "%s", err_msg); + } + tor_free(err_msg); goto err; } - /* Version of the service. */ - if (!strcasecmp(line->key, "HiddenServiceVersion")) { - service->config.version = - (uint32_t) helper_parse_uint64(line->key, line->value, HS_VERSION_MIN, - HS_VERSION_MAX, &ok); - if (!ok || have_version) { - if (have_version) - dup_opt_seen = line->key; - goto err; - } - have_version = service->config.hs_version_explicitly_set = 1; - continue; - } - /* Virtual port. */ - if (!strcasecmp(line->key, "HiddenServicePort")) { - char *err_msg = NULL; - /* XXX: Can we rename this? */ - rend_service_port_config_t *portcfg = - rend_service_parse_port_config(line->value, " ", &err_msg); - if (!portcfg) { - if (err_msg) { - log_warn(LD_CONFIG, "%s", err_msg); - } - tor_free(err_msg); - goto err; - } - tor_assert(!err_msg); - smartlist_add(config->ports, portcfg); - log_info(LD_CONFIG, "HiddenServicePort=%s for %s", - line->value, escaped(config->directory_path)); - continue; - } - /* Do we allow unknown ports. */ - if (!strcasecmp(line->key, "HiddenServiceAllowUnknownPorts")) { - config->allow_unknown_ports = - (unsigned int) helper_parse_uint64(line->key, line->value, 0, 1, &ok); - if (!ok || have_allow_unknown_ports) { - if (have_allow_unknown_ports) - dup_opt_seen = line->key; - goto err; - } - have_allow_unknown_ports = 1; - continue; - } - /* Directory group readable. */ - if (!strcasecmp(line->key, "HiddenServiceDirGroupReadable")) { - config->dir_group_readable = - (unsigned int) helper_parse_uint64(line->key, line->value, 0, 1, &ok); - if (!ok || have_dir_group_read) { - if (have_dir_group_read) - dup_opt_seen = line->key; - goto err; - } - have_dir_group_read = 1; - continue; - } - /* Maximum streams per circuit. */ - if (!strcasecmp(line->key, "HiddenServiceMaxStreams")) { - config->max_streams_per_rdv_circuit = - helper_parse_uint64(line->key, line->value, 0, - HS_CONFIG_MAX_STREAMS_PER_RDV_CIRCUIT, &ok); - if (!ok || have_max_streams) { - if (have_max_streams) - dup_opt_seen = line->key; - goto err; - } - have_max_streams = 1; - continue; - } - /* Maximum amount of streams before we close the circuit. */ - if (!strcasecmp(line->key, "HiddenServiceMaxStreamsCloseCircuit")) { - config->max_streams_close_circuit = - (unsigned int) helper_parse_uint64(line->key, line->value, 0, 1, &ok); - if (!ok || have_max_streams_close) { - if (have_max_streams_close) - dup_opt_seen = line->key; - goto err; - } - have_max_streams_close = 1; - continue; - } + tor_assert(!err_msg); + smartlist_add(config->ports, portcfg); + log_info(LD_CONFIG, "HiddenServicePort=%s for %s", + portline->value, escaped(config->directory_path)); } + /* Do we allow unknown ports? */ + config->allow_unknown_ports = hs_opts->HiddenServiceAllowUnknownPorts; + + /* Directory group readable. */ + config->dir_group_readable = hs_opts->HiddenServiceDirGroupReadable; + + /* Maximum streams per circuit. */ + if (check_value_oob(hs_opts->HiddenServiceMaxStreams, + "HiddenServiceMaxStreams", 0, + HS_CONFIG_MAX_STREAMS_PER_RDV_CIRCUIT)) { + goto err; + } + config->max_streams_per_rdv_circuit = hs_opts->HiddenServiceMaxStreams; + + /* Maximum amount of streams before we close the circuit. */ + config->max_streams_close_circuit = + hs_opts->HiddenServiceMaxStreamsCloseCircuit; + /* Check if we are configured in non anonymous mode meaning every service * becomes a single onion service. */ if (rend_service_non_anonymous_mode_enabled(options)) { @@ -650,9 +524,6 @@ config_generic_service(const config_line_t *line, /* Success */ return 0; err: - if (dup_opt_seen) { - log_warn(LD_CONFIG, "Duplicate directive %s.", dup_opt_seen); - } return -1; } @@ -695,7 +566,7 @@ config_service(config_line_t *line, const or_options_t *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) { + if (config_generic_service(hs_opts, options, service) < 0) { goto err; } @@ -730,10 +601,10 @@ config_service(config_line_t *line, const or_options_t *options, * directory line, the function knows that it has to stop parsing. */ switch (service->config.version) { case HS_VERSION_TWO: - ret = rend_config_service(line->next, options, &service->config); + ret = rend_config_service(hs_opts, options, &service->config); break; case HS_VERSION_THREE: - ret = config_service_v3(line->next, &service->config); + ret = config_service_v3(hs_opts, &service->config); break; default: /* We do validate before if we support the parsed version. */ diff --git a/src/feature/hs/hs_options.inc b/src/feature/hs/hs_options.inc index c3566f4461..1a1444fd05 100644 --- a/src/feature/hs/hs_options.inc +++ b/src/feature/hs/hs_options.inc @@ -19,8 +19,9 @@ BEGIN_CONF_STRUCT(hs_opts_t) CONF_VAR(HiddenServiceDir, FILENAME, 0, NULL) CONF_VAR(HiddenServiceDirGroupReadable, BOOL, 0, "0") -CONF_VAR(HiddenServicePort, STRING, 0, NULL) -CONF_VAR(HiddenServiceVersion, POSINT, 0, "3") +CONF_VAR(HiddenServicePort, LINELIST, 0, NULL) +// "-1" means "auto" here. +CONF_VAR(HiddenServiceVersion, INT, 0, "-1") CONF_VAR(HiddenServiceAuthorizeClient, STRING, 0, NULL) CONF_VAR(HiddenServiceAllowUnknownPorts, BOOL, 0, "0") CONF_VAR(HiddenServiceMaxStreams, POSINT, 0, "0") diff --git a/src/feature/rend/rendservice.c b/src/feature/rend/rendservice.c index 182e935fa1..10a3403166 100644 --- a/src/feature/rend/rendservice.c +++ b/src/feature/rend/rendservice.c @@ -49,6 +49,7 @@ #include "core/or/crypt_path_reference_st.h" #include "core/or/edge_connection_st.h" #include "core/or/extend_info_st.h" +#include "feature/hs/hs_opts_st.h" #include "feature/nodelist/networkstatus_st.h" #include "core/or/origin_circuit_st.h" #include "feature/rend/rend_authorized_client_st.h" @@ -714,22 +715,20 @@ service_config_shadow_copy(rend_service_t *service, config->ports = NULL; } -/* Parse the hidden service configuration starting at line_ using the +/* Parse the hidden service configuration from hs_opts using the * already configured generic service configuration in config. This * function will translate the config object to a rend_service_t and add it to * the temporary list if valid. If validate_only is set, parse, warn * and return as normal but don't actually add the service to the list. */ int -rend_config_service(const config_line_t *line_, +rend_config_service(const hs_opts_t *hs_opts, const or_options_t *options, hs_service_config_t *config) { - const config_line_t *line; rend_service_t *service = NULL; - /* line_ can be NULL which would mean that the service configuration only - * have one line that is the directory directive. */ tor_assert(options); + tor_assert(hs_opts); tor_assert(config); /* Use the staging service list so that we can check then do the pruning @@ -746,126 +745,109 @@ rend_config_service(const config_line_t *line_, * options, we'll copy over the useful data to the rend_service_t object. */ service_config_shadow_copy(service, config); - for (line = line_; line; line = line->next) { - if (!strcasecmp(line->key, "HiddenServiceDir")) { - /* We just hit the next hidden service, stop right now. */ - break; + /* Number of introduction points. */ + if (hs_opts->HiddenServiceNumIntroductionPoints > NUM_INTRO_POINTS_MAX) { + log_warn(LD_CONFIG, "HiddenServiceNumIntroductionPoints must be " + "between 0 and %d, not %d.", + NUM_INTRO_POINTS_MAX, + hs_opts->HiddenServiceNumIntroductionPoints); + goto err; + } + service->n_intro_points_wanted = hs_opts->HiddenServiceNumIntroductionPoints; + log_info(LD_CONFIG, "HiddenServiceNumIntroductionPoints=%d for %s", + service->n_intro_points_wanted, escaped(service->directory)); + + /* Client authorization */ + if (hs_opts->HiddenServiceAuthorizeClient) { + /* Parse auth type and comma-separated list of client names and add a + * rend_authorized_client_t for each client to the service's list + * of authorized clients. */ + smartlist_t *type_names_split, *clients; + const char *authname; + type_names_split = smartlist_new(); + smartlist_split_string(type_names_split, + hs_opts->HiddenServiceAuthorizeClient, " ", 0, 2); + if (smartlist_len(type_names_split) < 1) { + log_warn(LD_BUG, "HiddenServiceAuthorizeClient has no value. This " + "should have been prevented when parsing the " + "configuration."); + smartlist_free(type_names_split); + goto err; } - /* Number of introduction points. */ - if (!strcasecmp(line->key, "HiddenServiceNumIntroductionPoints")) { - int ok = 0; - /* Those are specific defaults for version 2. */ - service->n_intro_points_wanted = - (unsigned int) tor_parse_long(line->value, 10, - 0, NUM_INTRO_POINTS_MAX, &ok, NULL); - if (!ok) { - log_warn(LD_CONFIG, - "HiddenServiceNumIntroductionPoints " - "should be between %d and %d, not %s", - 0, NUM_INTRO_POINTS_MAX, line->value); - goto err; - } - log_info(LD_CONFIG, "HiddenServiceNumIntroductionPoints=%d for %s", - service->n_intro_points_wanted, escaped(service->directory)); - continue; - } - if (!strcasecmp(line->key, "HiddenServiceAuthorizeClient")) { - /* Parse auth type and comma-separated list of client names and add a - * rend_authorized_client_t for each client to the service's list - * of authorized clients. */ - smartlist_t *type_names_split, *clients; - const char *authname; - if (service->auth_type != REND_NO_AUTH) { - log_warn(LD_CONFIG, "Got multiple HiddenServiceAuthorizeClient " - "lines for a single service."); - goto err; - } - type_names_split = smartlist_new(); - smartlist_split_string(type_names_split, line->value, " ", 0, 2); - if (smartlist_len(type_names_split) < 1) { - log_warn(LD_BUG, "HiddenServiceAuthorizeClient has no value. This " - "should have been prevented when parsing the " - "configuration."); - smartlist_free(type_names_split); - goto err; - } - authname = smartlist_get(type_names_split, 0); - if (!strcasecmp(authname, "basic")) { - service->auth_type = REND_BASIC_AUTH; - } else if (!strcasecmp(authname, "stealth")) { - service->auth_type = REND_STEALTH_AUTH; - } else { - log_warn(LD_CONFIG, "HiddenServiceAuthorizeClient contains " - "unrecognized auth-type '%s'. Only 'basic' or 'stealth' " - "are recognized.", - (char *) smartlist_get(type_names_split, 0)); - SMARTLIST_FOREACH(type_names_split, char *, cp, tor_free(cp)); - smartlist_free(type_names_split); - goto err; - } - service->clients = smartlist_new(); - if (smartlist_len(type_names_split) < 2) { - log_warn(LD_CONFIG, "HiddenServiceAuthorizeClient contains " - "auth-type '%s', but no client names.", - service->auth_type == REND_BASIC_AUTH ? "basic" : "stealth"); - SMARTLIST_FOREACH(type_names_split, char *, cp, tor_free(cp)); - smartlist_free(type_names_split); - continue; - } - clients = smartlist_new(); - smartlist_split_string(clients, smartlist_get(type_names_split, 1), - ",", SPLIT_SKIP_SPACE, 0); + authname = smartlist_get(type_names_split, 0); + if (!strcasecmp(authname, "basic")) { + service->auth_type = REND_BASIC_AUTH; + } else if (!strcasecmp(authname, "stealth")) { + service->auth_type = REND_STEALTH_AUTH; + } else { + log_warn(LD_CONFIG, "HiddenServiceAuthorizeClient contains " + "unrecognized auth-type '%s'. Only 'basic' or 'stealth' " + "are recognized.", + (char *) smartlist_get(type_names_split, 0)); SMARTLIST_FOREACH(type_names_split, char *, cp, tor_free(cp)); smartlist_free(type_names_split); - /* Remove duplicate client names. */ - { - int num_clients = smartlist_len(clients); - smartlist_sort_strings(clients); - smartlist_uniq_strings(clients); - if (smartlist_len(clients) < num_clients) { - log_info(LD_CONFIG, "HiddenServiceAuthorizeClient contains %d " - "duplicate client name(s); removing.", - num_clients - smartlist_len(clients)); - } + goto err; + } + service->clients = smartlist_new(); + if (smartlist_len(type_names_split) < 2) { + log_warn(LD_CONFIG, "HiddenServiceAuthorizeClient contains " + "auth-type '%s', but no client names.", + service->auth_type == REND_BASIC_AUTH ? "basic" : "stealth"); + SMARTLIST_FOREACH(type_names_split, char *, cp, tor_free(cp)); + smartlist_free(type_names_split); + goto err; + } + clients = smartlist_new(); + smartlist_split_string(clients, smartlist_get(type_names_split, 1), + ",", SPLIT_SKIP_SPACE, 0); + SMARTLIST_FOREACH(type_names_split, char *, cp, tor_free(cp)); + smartlist_free(type_names_split); + /* Remove duplicate client names. */ + { + int num_clients = smartlist_len(clients); + smartlist_sort_strings(clients); + smartlist_uniq_strings(clients); + if (smartlist_len(clients) < num_clients) { + log_info(LD_CONFIG, "HiddenServiceAuthorizeClient contains %d " + "duplicate client name(s); removing.", + num_clients - smartlist_len(clients)); } - SMARTLIST_FOREACH_BEGIN(clients, const char *, client_name) - { - rend_authorized_client_t *client; - if (!rend_valid_client_name(client_name)) { - log_warn(LD_CONFIG, "HiddenServiceAuthorizeClient contains an " - "illegal client name: '%s'. Names must be " - "between 1 and %d characters and contain " - "only [A-Za-z0-9+_-].", - client_name, REND_CLIENTNAME_MAX_LEN); - SMARTLIST_FOREACH(clients, char *, cp, tor_free(cp)); - smartlist_free(clients); - goto err; - } - client = tor_malloc_zero(sizeof(rend_authorized_client_t)); - client->client_name = tor_strdup(client_name); - smartlist_add(service->clients, client); - log_debug(LD_REND, "Adding client name '%s'", client_name); - } - SMARTLIST_FOREACH_END(client_name); - SMARTLIST_FOREACH(clients, char *, cp, tor_free(cp)); - smartlist_free(clients); - /* Ensure maximum number of clients. */ - if ((service->auth_type == REND_BASIC_AUTH && - smartlist_len(service->clients) > 512) || - (service->auth_type == REND_STEALTH_AUTH && - smartlist_len(service->clients) > 16)) { - log_warn(LD_CONFIG, "HiddenServiceAuthorizeClient contains %d " - "client authorization entries, but only a " - "maximum of %d entries is allowed for " - "authorization type '%s'.", - smartlist_len(service->clients), - service->auth_type == REND_BASIC_AUTH ? 512 : 16, - service->auth_type == REND_BASIC_AUTH ? "basic" : "stealth"); + } + SMARTLIST_FOREACH_BEGIN(clients, const char *, client_name) { + rend_authorized_client_t *client; + if (!rend_valid_client_name(client_name)) { + log_warn(LD_CONFIG, "HiddenServiceAuthorizeClient contains an " + "illegal client name: '%s'. Names must be " + "between 1 and %d characters and contain " + "only [A-Za-z0-9+_-].", + client_name, REND_CLIENTNAME_MAX_LEN); + SMARTLIST_FOREACH(clients, char *, cp, tor_free(cp)); + smartlist_free(clients); goto err; } - continue; + client = tor_malloc_zero(sizeof(rend_authorized_client_t)); + client->client_name = tor_strdup(client_name); + smartlist_add(service->clients, client); + log_debug(LD_REND, "Adding client name '%s'", client_name); + } SMARTLIST_FOREACH_END(client_name); + SMARTLIST_FOREACH(clients, char *, cp, tor_free(cp)); + smartlist_free(clients); + /* Ensure maximum number of clients. */ + if ((service->auth_type == REND_BASIC_AUTH && + smartlist_len(service->clients) > 512) || + (service->auth_type == REND_STEALTH_AUTH && + smartlist_len(service->clients) > 16)) { + log_warn(LD_CONFIG, "HiddenServiceAuthorizeClient contains %d " + "client authorization entries, but only a " + "maximum of %d entries is allowed for " + "authorization type '%s'.", + smartlist_len(service->clients), + service->auth_type == REND_BASIC_AUTH ? 512 : 16, + service->auth_type == REND_BASIC_AUTH ? "basic" : "stealth"); + goto err; } } + /* Validate the service just parsed. */ if (rend_validate_service(rend_service_staging_list, service) < 0) { /* Service is in the staging list so don't try to free it. */ diff --git a/src/feature/rend/rendservice.h b/src/feature/rend/rendservice.h index 8202c4fcd3..012afc0f9f 100644 --- a/src/feature/rend/rendservice.h +++ b/src/feature/rend/rendservice.h @@ -139,7 +139,8 @@ STATIC void rend_service_prune_list_impl_(void); #endif /* defined(RENDSERVICE_PRIVATE) */ int rend_num_services(void); -int rend_config_service(const struct config_line_t *line_, +struct hs_opts_t; +int rend_config_service(const struct hs_opts_t *hs_opts, const or_options_t *options, hs_service_config_t *config); void rend_service_prune_list(void); From 8aacd78e149c8228c1696a0866d09f8cba9ba250 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Thu, 5 Mar 2020 10:28:06 -0500 Subject: [PATCH 5/7] Update expected log messages in tests to new format. --- src/test/test_hs_config.c | 43 +++++++++++++++++++++++---------------- 1 file changed, 25 insertions(+), 18 deletions(-) diff --git a/src/test/test_hs_config.c b/src/test/test_hs_config.c index b2537b746b..dc3b598c34 100644 --- a/src/test/test_hs_config.c +++ b/src/test/test_hs_config.c @@ -62,8 +62,9 @@ test_invalid_service(void *arg) setup_full_capture_of_logs(LOG_WARN); ret = helper_config_service(conf, 1); tt_int_op(ret, OP_EQ, -1); - expect_log_msg_containing("HiddenServiceAllowUnknownPorts must be " - "between 0 and 1, not 2"); + expect_log_msg_containing("Could not parse " + "HiddenServiceAllowUnknownPorts: Unrecognized " + "value 2. Allowed values are 0 and 1."); teardown_capture_of_logs(); } @@ -76,8 +77,9 @@ test_invalid_service(void *arg) setup_full_capture_of_logs(LOG_WARN); ret = helper_config_service(conf, 1); tt_int_op(ret, OP_EQ, -1); - expect_log_msg_containing("HiddenServiceDirGroupReadable must be " - "between 0 and 1, not 2"); + expect_log_msg_containing("Could not parse " + "HiddenServiceDirGroupReadable: " + "Unrecognized value 2."); teardown_capture_of_logs(); } @@ -90,8 +92,9 @@ test_invalid_service(void *arg) setup_full_capture_of_logs(LOG_WARN); ret = helper_config_service(conf, 1); tt_int_op(ret, OP_EQ, -1); - expect_log_msg_containing("HiddenServiceMaxStreamsCloseCircuit must " - "be between 0 and 1, not 2"); + expect_log_msg_containing("Could not parse " + "HiddenServiceMaxStreamsCloseCircuit: " + "Unrecognized value 2"); teardown_capture_of_logs(); } @@ -228,8 +231,8 @@ test_invalid_service_v2(void *arg) 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("HiddenServiceNumIntroductionPoints should " - "be between 0 and 10, not 11"); + expect_log_msg_containing("HiddenServiceNumIntroductionPoints must " + "be between 0 and 10, not 11."); teardown_capture_of_logs(); } @@ -243,8 +246,9 @@ test_invalid_service_v2(void *arg) 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("HiddenServiceNumIntroductionPoints should " - "be between 0 and 10, not -1"); + expect_log_msg_containing("Could not parse " + "HiddenServiceNumIntroductionPoints: " + "Integer -1 is malformed or out of bounds."); teardown_capture_of_logs(); } @@ -532,9 +536,10 @@ test_dos_parameters(void *arg) setup_full_capture_of_logs(LOG_WARN); ret = helper_config_service(conf, 0); tt_int_op(ret, OP_EQ, -1); - expect_log_msg_containing("HiddenServiceEnableIntroDoSRatePerSec must " - "be between 0 and 2147483647, " - "not 137438953472"); + expect_log_msg_containing("Could not parse " + "HiddenServiceEnableIntroDoSRatePerSec: " + "Integer 137438953472 is malformed or out of " + "bounds."); teardown_capture_of_logs(); } @@ -551,9 +556,10 @@ test_dos_parameters(void *arg) setup_full_capture_of_logs(LOG_WARN); ret = helper_config_service(conf, 0); tt_int_op(ret, OP_EQ, -1); - expect_log_msg_containing("HiddenServiceEnableIntroDoSBurstPerSec must " - "be between 0 and 2147483647, " - "not 274877906944"); + expect_log_msg_containing("Could not parse " + "HiddenServiceEnableIntroDoSBurstPerSec: " + "Integer 274877906944 is malformed or out " + "of bounds."); teardown_capture_of_logs(); } @@ -588,8 +594,9 @@ test_dos_parameters(void *arg) setup_full_capture_of_logs(LOG_WARN); ret = helper_config_service(conf, 0); tt_int_op(ret, OP_EQ, -1); - expect_log_msg_containing("HiddenServiceEnableIntroDoSRatePerSec must be " - "between 0 and 2147483647, not -1"); + expect_log_msg_containing("Could not parse " + "HiddenServiceEnableIntroDoSRatePerSec: " + "Integer -1 is malformed or out of bounds."); teardown_capture_of_logs(); } From 84868109d2c99c349d48e29470a22c16ff7fb88e Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Sat, 7 Mar 2020 09:32:54 -0500 Subject: [PATCH 6/7] Use SECTION_HEADER, not "HiddenServiceDir". Add a nonfatal assertion about a branch that should be unreachable. --- src/feature/hs/hs_config.c | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/src/feature/hs/hs_config.c b/src/feature/hs/hs_config.c index 4b55ba2af0..c799eb2086 100644 --- a/src/feature/hs/hs_config.c +++ b/src/feature/hs/hs_config.c @@ -232,6 +232,12 @@ config_learn_service_version(hs_service_t *service) return version; } +/** + * Header key indicating the start of a new hidden service configuration + * block. + **/ +static const char SECTION_HEADER[] = "HiddenServiceDir"; + /** Return true iff the given options starting at line_ for a hidden service * contains at least one invalid option. Each hidden service option don't * apply to all versions so this function can find out. The line_ MUST start @@ -286,8 +292,11 @@ config_has_invalid_options(const config_line_t *line_, for (int i = 0; optlist[i]; i++) { const char *opt = optlist[i]; for (line = line_; line; line = line->next) { - if (!strcasecmp(line->key, "HiddenServiceDir")) { - /* We just hit the next hidden service, stop right now. */ + if (!strcasecmp(line->key, SECTION_HEADER)) { + /* We just hit the next hidden service, stop right now. + * (This shouldn't be possible, now that we have partitioned the list + * into sections.) */ + tor_assert_nonfatal_unreached(); goto end; } if (!strcasecmp(line->key, opt)) { @@ -428,12 +437,6 @@ config_service_v3(const hs_opts_t *hs_opts, return -1; } -/** - * Header key indicating the start of a new hidden service configuration - * block. - **/ -static const char SECTION_HEADER[] = "HiddenServiceDir"; - /** Configure a service using the given options in hs_opts and options. This is * called for any service regardless of its version which means that all * directives in this function are generic to any service version. This @@ -462,8 +465,8 @@ config_generic_service(const hs_opts_t *hs_opts, /* Directory where the service's keys are stored. */ tor_assert(hs_opts->HiddenServiceDir); config->directory_path = tor_strdup(hs_opts->HiddenServiceDir); - log_info(LD_CONFIG, "HiddenServiceDir=%s. Configuring...", - escaped(config->directory_path)); + log_info(LD_CONFIG, "%s=%s. Configuring...", + SECTION_HEADER, escaped(config->directory_path)); /* Protocol version for the service. */ if (hs_opts->HiddenServiceVersion == -1) { From 0dc25a4b66aedc804360c10a6f710d69f0b17bfe Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Sun, 8 Mar 2020 15:49:40 -0400 Subject: [PATCH 7/7] Use a macro to make our hs_opts checking terser. --- src/feature/hs/hs_config.c | 38 ++++++++++++++++++++------------------ 1 file changed, 20 insertions(+), 18 deletions(-) diff --git a/src/feature/hs/hs_config.c b/src/feature/hs/hs_config.c index c799eb2086..0dad8dd6d8 100644 --- a/src/feature/hs/hs_config.c +++ b/src/feature/hs/hs_config.c @@ -186,6 +186,13 @@ check_value_oob(int i, const char *name, int low, int high) return false; } +/** + * Helper: check whether the integer value called name in opts + * is out-of-bounds. + **/ +#define CHECK_OOB(opts, name, low, high) \ + check_value_oob((opts)->name, #name, (low), (high)) + /** Helper function: Given a configuration option and its value, parse the * value as a hs_circuit_id_protocol_t. On success, ok is set to 1 and ret is * the parse value. On error, ok is set to 0 and the "none" @@ -368,10 +375,9 @@ config_service_v3(const hs_opts_t *hs_opts, tor_assert(hs_opts); /* Number of introduction points. */ - if (check_value_oob(hs_opts->HiddenServiceNumIntroductionPoints, - "HiddenServiceNumIntroductionPoints", - NUM_INTRO_POINTS_DEFAULT, - HS_CONFIG_V3_MAX_INTRO_POINTS)) { + if (CHECK_OOB(hs_opts, HiddenServiceNumIntroductionPoints, + NUM_INTRO_POINTS_DEFAULT, + HS_CONFIG_V3_MAX_INTRO_POINTS)) { goto err; } config->num_intro_points = hs_opts->HiddenServiceNumIntroductionPoints; @@ -393,10 +399,9 @@ config_service_v3(const hs_opts_t *hs_opts, hs_opts->HiddenServiceEnableIntroDoSDefense; /* Rate for DoS defense */ - if (check_value_oob(hs_opts->HiddenServiceEnableIntroDoSRatePerSec, - "HiddenServiceEnableIntroDoSRatePerSec", - HS_CONFIG_V3_DOS_DEFENSE_RATE_PER_SEC_MIN, - HS_CONFIG_V3_DOS_DEFENSE_RATE_PER_SEC_MAX)) { + if (CHECK_OOB(hs_opts, HiddenServiceEnableIntroDoSRatePerSec, + HS_CONFIG_V3_DOS_DEFENSE_RATE_PER_SEC_MIN, + HS_CONFIG_V3_DOS_DEFENSE_RATE_PER_SEC_MAX)) { goto err; } config->intro_dos_rate_per_sec = @@ -404,10 +409,9 @@ config_service_v3(const hs_opts_t *hs_opts, log_info(LD_REND, "Service INTRO2 DoS defenses rate set to: %" PRIu32, config->intro_dos_rate_per_sec); - if (check_value_oob(hs_opts->HiddenServiceEnableIntroDoSBurstPerSec, - "HiddenServiceEnableIntroDoSBurstPerSec", - HS_CONFIG_V3_DOS_DEFENSE_BURST_PER_SEC_MIN, - HS_CONFIG_V3_DOS_DEFENSE_BURST_PER_SEC_MAX)) { + if (CHECK_OOB(hs_opts, HiddenServiceEnableIntroDoSBurstPerSec, + HS_CONFIG_V3_DOS_DEFENSE_BURST_PER_SEC_MIN, + HS_CONFIG_V3_DOS_DEFENSE_BURST_PER_SEC_MAX)) { goto err; } config->intro_dos_burst_per_sec = @@ -471,9 +475,8 @@ config_generic_service(const hs_opts_t *hs_opts, /* Protocol version for the service. */ if (hs_opts->HiddenServiceVersion == -1) { /* No value was set; stay with the default. */ - } else if (check_value_oob(hs_opts->HiddenServiceVersion, - "HiddenServiceVersion", - HS_VERSION_MIN, HS_VERSION_MAX)) { + } else if (CHECK_OOB(hs_opts, HiddenServiceVersion, + HS_VERSION_MIN, HS_VERSION_MAX)) { goto err; } else { config->hs_version_explicitly_set = 1; @@ -507,9 +510,8 @@ config_generic_service(const hs_opts_t *hs_opts, config->dir_group_readable = hs_opts->HiddenServiceDirGroupReadable; /* Maximum streams per circuit. */ - if (check_value_oob(hs_opts->HiddenServiceMaxStreams, - "HiddenServiceMaxStreams", 0, - HS_CONFIG_MAX_STREAMS_PER_RDV_CIRCUIT)) { + if (CHECK_OOB(hs_opts, HiddenServiceMaxStreams, + 0, HS_CONFIG_MAX_STREAMS_PER_RDV_CIRCUIT)) { goto err; } config->max_streams_per_rdv_circuit = hs_opts->HiddenServiceMaxStreams;