1 files changed, 275 insertions, 228 deletions

diff --git a/src/feature/hs/hs_config.c b/src/feature/hs/hs_config.c
index 79fadf4c0d..f8d71674de 100644
--- a/src/feature/hs/hs_config.c
+++ b/src/feature/hs/hs_config.c
@@ -1,4 +1,4 @@
-/* Copyright (c) 2017-2019, The Tor Project, Inc. */
+/* Copyright (c) 2017-2020, The Tor Project, Inc. */
 /* See LICENSE for licensing information */
 
 /**
@@ -16,25 +16,79 @@
  * options and then put in a staging list. It will stay there until
  * hs_service_load_all_keys() is called. That function is responsible to
  * load/generate the keys for the service in the staging list and if
- * successful, transfert the service to the main global service list where
+ * successful, transferred the service to the main global service list where
  * at that point it is ready to be used.
  *
  * Configuration functions are per-version and there is a main generic one for
  * every option that is common to all version (config_generic_service).
  **/
 
-#define HS_CONFIG_PRIVATE
-
 #include "feature/hs/hs_common.h"
 #include "feature/hs/hs_config.h"
 #include "feature/hs/hs_client.h"
+#include "feature/hs/hs_ob.h"
 #include "feature/hs/hs_service.h"
 #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"
 
-/* Using the given list of services, stage them into our global state. Every
+/* 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.
  *
@@ -70,7 +124,7 @@ stage_services(smartlist_t *service_list)
   hs_service_stage_services(service_list);
 }
 
-/* Validate the given service against all service in the given list. If the
+/** Validate the given service against all service in the given list. If the
  * service is ephemeral, this function ignores it. Services with the same
  * directory path aren't allowed and will return an error. If a duplicate is
  * found, 1 is returned else 0 if none found. */
@@ -118,37 +172,31 @@ 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 <b>i</b> is out of bounds (not between <b>low</b>
+ * and <b>high</b> incusive).  If it is, then log a warning about the option
+ * <b>name</b>, 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) {
-    if (min == max) {
-      log_warn(LD_CONFIG, "%s must be %" PRIu64 ", not %s.", opt, max, value);
+  if (i < low || i > high) {
+    if (low == high) {
+      log_warn(LD_CONFIG, "%s must be %d, not %d.", name, low, i);
     } else {
-      log_warn(LD_CONFIG, "%s must be between %" PRIu64 " and %"PRIu64
-                          ", not %s.",
-               opt, min, max, value);
+      log_warn(LD_CONFIG, "%s must be between %d and %d, not %d.",
+               name, low, high, i);
     }
-    goto err;
+    return true;
   }
-  log_info(LD_CONFIG, "%s was parsed to %" PRIu64, opt, ret);
- err:
-  return ret;
+  return false;
 }
 
+/**
+ * Helper: check whether the integer value called <b>name</b> in <b>opts</b>
+ * 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"
@@ -177,7 +225,7 @@ helper_parse_circuit_id_protocol(const char *key, const char *value, int *ok)
   return ret;
 }
 
-/* Return the service version by trying to learn it from the key on disk if
+/** Return the service version by trying to learn it from the key on disk if
  * any. If nothing is found, the current service configured version is
  * returned. */
 static int
@@ -195,7 +243,13 @@ config_learn_service_version(hs_service_t *service)
   return version;
 }
 
-/* Return true iff the given options starting at line_ for a hidden service
+/**
+ * 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
  * right after the HiddenServiceDir line of this service.
@@ -222,6 +276,10 @@ config_has_invalid_options(const config_line_t *line_,
 
   const char *opts_exclude_v2[] = {
     "HiddenServiceExportCircuitID",
+    "HiddenServiceEnableIntroDoSDefense",
+    "HiddenServiceEnableIntroDoSRatePerSec",
+    "HiddenServiceEnableIntroDoSBurstPerSec",
+    "HiddenServiceOnionBalanceInstance",
     NULL /* End marker. */
   };
 
@@ -245,8 +303,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)) {
@@ -254,6 +315,16 @@ config_has_invalid_options(const config_line_t *line_,
                             "version %" PRIu32 " of service in %s",
                  opt, service->config.version,
                  service->config.directory_path);
+
+        if (!strcasecmp(line->key, "HiddenServiceAuthorizeClient")) {
+          /* Special case this v2 option so that we can offer alternatives.
+           * If more such special cases appear, it would be good to
+           * generalize the exception mechanism here. */
+          log_warn(LD_CONFIG, "For v3 onion service client authorization, "
+                   "please read the 'CLIENT AUTHORIZATION' section in the "
+                   "manual.");
+        }
+
         ret = 1;
         /* Continue the loop so we can find all possible options. */
         continue;
@@ -264,7 +335,7 @@ config_has_invalid_options(const config_line_t *line_,
   return ret;
 }
 
-/* Validate service configuration. This is used when loading the configuration
+/** Validate service configuration. This is used when loading the configuration
  * and once we've setup a service object, it's config object is passed to this
  * function for further validation. This does not validate service key
  * material. Return 0 if valid else -1 if invalid. */
@@ -280,63 +351,83 @@ config_validate_service(const hs_service_config_t *config)
     goto invalid;
   }
 
+  /* DoS validation values. */
+  if (config->has_dos_defense_enabled &&
+      (config->intro_dos_burst_per_sec < config->intro_dos_rate_per_sec)) {
+    log_warn(LD_CONFIG, "Hidden service DoS defenses burst (%" PRIu32 ") can "
+                        "not be smaller than the rate value (%" PRIu32 ").",
+             config->intro_dos_burst_per_sec, config->intro_dos_rate_per_sec);
+    goto invalid;
+  }
+
   /* Valid. */
   return 0;
  invalid:
   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 function 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 */
-  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 (!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;
+  /* Number of introduction 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;
+
+  /* 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;
     }
-    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;
+  }
+
+  /* Is the DoS defense enabled? */
+  config->has_dos_defense_enabled =
+    hs_opts->HiddenServiceEnableIntroDoSDefense;
+
+  /* Rate for DoS defense */
+  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 =
+    hs_opts->HiddenServiceEnableIntroDoSRatePerSec;
+  log_info(LD_REND, "Service INTRO2 DoS defenses rate set to: %" PRIu32,
+           config->intro_dos_rate_per_sec);
+
+  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 =
+    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;
     }
   }
 
@@ -351,13 +442,10 @@ 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;
 }
 
-/* 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.
@@ -369,168 +457,98 @@ 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 hs_opts_t *hs_opts,
                        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. */
-  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;
 
-  /* 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) {
-    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;
+  /* 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, "%s=%s. Configuring...",
+           SECTION_HEADER, escaped(config->directory_path));
+
+  /* Protocol version for the service. */
+  if (hs_opts->HiddenServiceVersion == -1) {
+    /* No value was set; stay with the default. */
+  } else if (CHECK_OOB(hs_opts, HiddenServiceVersion,
+                       HS_VERSION_MIN, HS_VERSION_MAX)) {
+    goto err;
+  } else {
+    config->hs_version_explicitly_set = 1;
+    config->version = hs_opts->HiddenServiceVersion;
+  }
+
+  /* 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);
       }
-      /* 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)) {
+      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_OOB(hs_opts, 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)) {
     config->is_single_onion = 1;
-    /* We will add support for IPv6-only v3 single onion services in a future
-     * Tor version. This won't catch "ReachableAddresses reject *4", but that
-     * option doesn't work anyway. */
-    if (options->ClientUseIPv4 == 0 && config->version == HS_VERSION_THREE) {
-      log_warn(LD_CONFIG, "IPv6-only v3 single onion services are not "
-               "supported. Set HiddenServiceSingleHopMode 0 and "
-               "HiddenServiceNonAnonymousMode 0, or set ClientUseIPv4 1.");
-      goto err;
-    }
   }
 
   /* Success */
   return 0;
  err:
-  if (dup_opt_seen) {
-    log_warn(LD_CONFIG, "Duplicate directive %s.", dup_opt_seen);
-  }
   return -1;
 }
 
-/* Configure a service using the given line and options. This function will
+/** Configure a service using the given line and options. This function will
  * call the corresponding configuration function for a specific service
  * version and validate the service against the other ones. On success, add
  * 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);
@@ -539,9 +557,25 @@ 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) {
+  if (config_generic_service(hs_opts, options, service) < 0) {
     goto err;
   }
 
@@ -576,10 +610,10 @@ config_service(const 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. */
@@ -598,22 +632,25 @@ 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;
 }
 
-/* From a set of <b>options</b>, setup every hidden service found. Return 0 on
+/** From a set of <b>options</b>, setup every hidden service found. Return 0 on
  * success or -1 on failure. If <b>validate_only</b> is set, parse, warn and
  * return as normal, but don't actually change the configured services. */
 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);
@@ -622,23 +659,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;
     }
   }
@@ -674,7 +712,7 @@ hs_config_service_all(const or_options_t *options, int validate_only)
   return ret;
 }
 
-/* From a set of <b>options</b>, setup every client authorization found.
+/** From a set of <b>options</b>, setup every client authorization found.
  * Return 0 on success or -1 on failure. If <b>validate_only</b> is set,
  * parse, warn and return as normal, but don't actually change the
  * configured state. */
@@ -698,3 +736,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);
+}