Merge branch 'bug20638_029_v2_squashed' into maint-0.2.9

changes/bug20638

@@ -0,0 +1,5 @@
+  o Minor bugfixes (hidden services):
+    - Stop ignoring hidden service key anonymity when first starting tor.
+      Instead, refuse to start tor if any hidden service key has been used in
+      a different hidden service anonymity mode.
+      Fixes bug 20638; bugfix on 17178 in 0.2.9.3-alpha; reported by ahf.

src/or/config.c

@@ -1729,25 +1729,6 @@ options_act(const or_options_t *old_options)
 
   monitor_owning_controller_process(options->OwningControllerProcess);
 
-  /* We must create new keys after we poison the directories, because our
-   * poisoning code checks for existing keys, and refuses to modify their
-   * directories. */
-
-  /* If we use non-anonymous single onion services, make sure we poison any
-     new hidden service directories, so that we never accidentally launch the
-     non-anonymous hidden services thinking they are anonymous. */
-  if (running_tor && rend_service_non_anonymous_mode_enabled(options)) {
-    if (options->RendConfigLines && !num_rend_services()) {
-      log_warn(LD_BUG,"Error: hidden services configured, but not parsed.");
-      return -1;
-    }
-    if (rend_service_poison_new_single_onion_dirs(NULL) < 0) {
-      log_warn(LD_GENERAL,"Failed to mark new hidden services as non-anonymous"
-               ".");
-      return -1;
-    }
-  }
-
   /* reload keys as needed for rendezvous services. */
   if (rend_service_load_all_keys(NULL)<0) {
     log_warn(LD_GENERAL,"Error loading rendezvous service keys");
@@ -2891,21 +2872,6 @@ options_validate_single_onion(or_options_t *options, char **msg)
     options->UseEntryGuards = 0;
   }
 
-  /* Check if existing hidden service keys were created in a different
-   * single onion service mode, and refuse to launch if they
-   * have. We'll poison new keys in options_act() just before we create them.
-   */
-  if (rend_service_list_verify_single_onion_poison(NULL, options) < 0) {
-    log_warn(LD_GENERAL, "We are configured with "
-             "HiddenServiceNonAnonymousMode %d, but one or more hidden "
-             "service keys were created in %s mode. This is not allowed.",
-             rend_service_non_anonymous_mode_enabled(options) ? 1 : 0,
-             rend_service_non_anonymous_mode_enabled(options) ?
-             "an anonymous" : "a non-anonymous"
-             );
-    return -1;
-  }
-
   return 0;
 }
 

src/or/rendservice.c

@@ -75,6 +75,9 @@ static ssize_t rend_service_parse_intro_for_v3(
 static int rend_service_check_private_dir(const or_options_t *options,
                                           const rend_service_t *s,
                                           int create);
+static int rend_service_check_private_dir_impl(const or_options_t *options,
+                                               const rend_service_t *s,
+                                               int create);
 
 /** Represents the mapping from a virtual port of a rendezvous service to
  * a real port on some IP.
@@ -216,15 +219,30 @@ rend_service_free_all(void)
   rend_service_list = NULL;
 }
 
-/** Validate <b>service</b> and add it to rend_service_list if possible.
+/** Validate <b>service</b> and add it to <b>service_list</b>, or to
+ * the global rend_service_list if <b>service_list</b> is NULL.
  * Return 0 on success.  On failure, free <b>service</b> and return -1.
+ * Takes ownership of <b>service</b>.
  */
 static int
-rend_add_service(rend_service_t *service)
+rend_add_service(smartlist_t *service_list, rend_service_t *service)
 {
   int i;
   rend_service_port_config_t *p;
 
+  smartlist_t *s_list;
+  /* If no special service list is provided, then just use the global one. */
+  if (!service_list) {
+    if (BUG(!rend_service_list)) {
+      /* No global HS list, which is a failure. */
+      return -1;
+    }
+
+    s_list = rend_service_list;
+  } else {
+    s_list = service_list;
+  }
+
   service->intro_nodes = smartlist_new();
   service->expiring_nodes = smartlist_new();
 
@@ -246,7 +264,8 @@ rend_add_service(rend_service_t *service)
   }
 
   if (service->auth_type != REND_NO_AUTH &&
-      smartlist_len(service->clients) == 0) {
+      (!service->clients ||
+       smartlist_len(service->clients) == 0)) {
     log_warn(LD_CONFIG, "Hidden service (%s) with client authorization but no "
                         "clients; ignoring.",
              rend_service_escaped_dir(service));
@@ -254,7 +273,7 @@ rend_add_service(rend_service_t *service)
     return -1;
   }
 
-  if (!smartlist_len(service->ports)) {
+  if (!service->ports || !smartlist_len(service->ports)) {
     log_warn(LD_CONFIG, "Hidden service (%s) with no ports configured; "
              "ignoring.",
              rend_service_escaped_dir(service));
@@ -277,8 +296,9 @@ rend_add_service(rend_service_t *service)
      * lock file.  But this is enough to detect a simple mistake that
      * at least one person has actually made.
      */
-    if (service->directory != NULL) { /* Skip dupe for ephemeral services. */
+    if (service->directory != NULL) {
-      SMARTLIST_FOREACH(rend_service_list, rend_service_t*, ptr,
+      /* Skip dupe for ephemeral services. */
+      SMARTLIST_FOREACH(s_list, rend_service_t*, ptr,
                         dupe = dupe ||
                                !strcmp(ptr->directory, service->directory));
       if (dupe) {
@@ -289,7 +309,7 @@ rend_add_service(rend_service_t *service)
         return -1;
       }
     }
-    smartlist_add(rend_service_list, service);
+    smartlist_add(s_list, service);
     log_debug(LD_REND,"Configuring service with directory \"%s\"",
               service->directory);
     for (i = 0; i < smartlist_len(service->ports); ++i) {
@@ -444,6 +464,61 @@ rend_service_port_config_free(rend_service_port_config_t *p)
   tor_free(p);
 }
 
+/* Check the directory for <b>service</b>, and add the service to
+ * <b>service_list</b>, or to the global list if <b>service_list</b> is NULL.
+ * Only add the service to the list if <b>validate_only</b> is false.
+ * If <b>validate_only</b> is true, free the service.
+ * If <b>service</b> is NULL, ignore it, and return 0.
+ * Returns 0 on success, and -1 on failure.
+ * Takes ownership of <b>service</b>, either freeing it, or adding it to the
+ * global service list.
+ */
+STATIC int
+rend_service_check_dir_and_add(smartlist_t *service_list,
+                               const or_options_t *options,
+                               rend_service_t *service,
+                               int validate_only)
+{
+  if (!service) {
+    /* It is ok for a service to be NULL, this means there are no services */
+    return 0;
+  }
+
+  if (rend_service_check_private_dir(options, service, !validate_only)
+      < 0) {
+    rend_service_free(service);
+    return -1;
+  }
+
+  if (validate_only) {
+    rend_service_free(service);
+    return 0;
+  } else {
+    /* Use service_list for unit tests */
+    smartlist_t *s_list = NULL;
+    /* If no special service list is provided, then just use the global one. */
+    if (!service_list) {
+      if (BUG(!rend_service_list)) {
+        /* No global HS list, which is a failure, because we plan on adding to
+         * it */
+        return -1;
+      }
+      s_list = rend_service_list;
+    } else {
+      s_list = service_list;
+    }
+    /* s_list can not be NULL here - if both service_list and rend_service_list
+     * are NULL, and validate_only is false, we exit earlier in the function
+     */
+    if (BUG(!s_list)) {
+      return -1;
+    }
+    /* Ignore service failures until 030 */
+    rend_add_service(s_list, service);
+    return 0;
+  }
+}
+
 /** Set up rend_service_list, based on the values of HiddenServiceDir and
  * HiddenServicePort in <b>options</b>.  Return 0 on success and -1 on
  * failure.  (If <b>validate_only</b> is set, parse, warn and return as
@@ -465,17 +540,13 @@ rend_config_services(const or_options_t *options, int validate_only)
 
   for (line = options->RendConfigLines; line; line = line->next) {
     if (!strcasecmp(line->key, "HiddenServiceDir")) {
-      if (service) { /* register the one we just finished parsing */
+      /* register the service we just finished parsing
-        if (rend_service_check_private_dir(options, service, 0) < 0) {
+       * this code registers every service except the last one parsed,
-          rend_service_free(service);
+       * which is registered below the loop */
+      if (rend_service_check_dir_and_add(NULL, options, service,
+                                         validate_only) < 0) {
           return -1;
       }
-
-        if (validate_only)
-          rend_service_free(service);
-        else
-          rend_add_service(service);
-      }
       service = tor_malloc_zero(sizeof(rend_service_t));
       service->directory = tor_strdup(line->value);
       service->ports = smartlist_new();
@@ -680,19 +751,14 @@ rend_config_services(const or_options_t *options, int validate_only)
       }
     }
   }
-  if (service) {
+  /* register the final service after we have finished parsing all services
-    if (rend_service_check_private_dir(options, service, 0) < 0) {
+   * this code only registers the last service, other services are registered
-      rend_service_free(service);
+   * within the loop. It is ok for this service to be NULL, it is ignored. */
+  if (rend_service_check_dir_and_add(NULL, options, service,
+                                     validate_only) < 0) {
     return -1;
   }
 
-    if (validate_only) {
-      rend_service_free(service);
-    } else {
-      rend_add_service(service);
-    }
-  }
-
   /* If this is a reload and there were hidden services configured before,
    * keep the introduction points that are still needed and close the
    * other ones. */
@@ -847,7 +913,7 @@ rend_service_add_ephemeral(crypto_pk_t *pk,
   }
 
   /* Initialize the service. */
-  if (rend_add_service(s)) {
+  if (rend_add_service(NULL, s)) {
     return RSAE_INTERNAL;
   }
   *service_id_out = tor_strdup(s->service_id);
@@ -996,6 +1062,11 @@ service_is_single_onion_poisoned(const rend_service_t *service)
   char *poison_fname = NULL;
   file_status_t fstatus;
 
+  /* Passing a NULL service is a bug */
+  if (BUG(!service)) {
+    return 0;
+  }
+
   if (!service->directory) {
     return 0;
   }
@@ -1029,58 +1100,64 @@ rend_service_private_key_exists(const rend_service_t *service)
   return private_key_status == FN_FILE;
 }
 
-/** Check the single onion service poison state of all existing hidden service
+/** Check the single onion service poison state of the directory for s:
- * directories:
+ * - If the service is poisoned, and we are in Single Onion Mode,
- * - If each service is poisoned, and we are in Single Onion Mode,
  *   return 0,
- * - If each service is not poisoned, and we are not in Single Onion Mode,
+ * - If the service is not poisoned, and we are not in Single Onion Mode,
  *   return 0,
- * - Otherwise, the poison state is invalid, and a service that was created in
+ * - Otherwise, the poison state is invalid: the service was created in one
- *   one mode is being used in the other, return -1.
+ *   mode, and is being used in the other, return -1.
- * Hidden service directories without keys are not checked for consistency.
+ * Hidden service directories without keys are always considered consistent.
- * When their keys are created, they will be poisoned (if needed).
+ * They will be poisoned after their directory is created (if needed). */
- * If a <b>service_list</b> is provided, treat it
+STATIC int
- * as the list of hidden services (used in unittests). */
+rend_service_verify_single_onion_poison(const rend_service_t* s,
-int
+                                        const or_options_t* options)
-rend_service_list_verify_single_onion_poison(const smartlist_t *service_list,
-                                             const or_options_t *options)
 {
-  const smartlist_t *s_list;
+  /* Passing a NULL service is a bug */
-  /* If no special service list is provided, then just use the global one. */
+  if (BUG(!s)) {
-  if (!service_list) {
+    return -1;
-    if (!rend_service_list) { /* No global HS list. Nothing to see here. */
+  }
+
+  /* Ephemeral services are checked at ADD_ONION time */
+  if (!s->directory) {
     return 0;
   }
 
-    s_list = rend_service_list;
+  /* Services without keys are always ok - their keys will only ever be used
-  } else {
+   * in the current mode */
-    s_list = service_list;
+  if (!rend_service_private_key_exists(s)) {
+    return 0;
   }
 
-  int consistent = 1;
+  /* The key has been used before in a different mode */
-  SMARTLIST_FOREACH_BEGIN(s_list, const rend_service_t *, s) {
   if (service_is_single_onion_poisoned(s) !=
-        rend_service_non_anonymous_mode_enabled(options) &&
+      rend_service_non_anonymous_mode_enabled(options)) {
-        rend_service_private_key_exists(s)) {
+    return -1;
-      consistent = 0;
   }
-  } SMARTLIST_FOREACH_END(s);
 
-  return consistent ? 0 : -1;
+  /* The key exists and is consistent with the current mode */
+  return 0;
 }
 
-/*** Helper for rend_service_poison_new_single_onion_dirs(). Add a file to
+/*** Helper for rend_service_poison_new_single_onion_dir(). Add a file to
- * this hidden service directory that marks it as a single onion service.
+ * the hidden service directory for s that marks it as a single onion service.
- * Tor must be in single onion mode before calling this function.
+ * Tor must be in single onion mode before calling this function, and the
+ * service directory must already have been created.
  * Returns 0 when a directory is successfully poisoned, or if it is already
  * poisoned. Returns -1 on a failure to read the directory or write the poison
  * file, or if there is an existing private key file in the directory. (The
  * service should have been poisoned when the key was created.) */
 static int
-poison_new_single_onion_hidden_service_dir(const rend_service_t *service)
+poison_new_single_onion_hidden_service_dir_impl(const rend_service_t *service,
+                                                const or_options_t* options)
 {
+  /* Passing a NULL service is a bug */
+  if (BUG(!service)) {
+    return -1;
+  }
+
   /* We must only poison directories if we're in Single Onion mode */
-  tor_assert(rend_service_non_anonymous_mode_enabled(get_options()));
+  tor_assert(rend_service_non_anonymous_mode_enabled(options));
 
   int fd;
   int retval = -1;
@@ -1098,8 +1175,8 @@ poison_new_single_onion_hidden_service_dir(const rend_service_t *service)
     return -1;
   }
 
-  /* Make sure the directory exists */
+  /* Make sure the directory was created before calling this function. */
-  if (rend_service_check_private_dir(get_options(), service, 1) < 0)
+  if (BUG(rend_service_check_private_dir_impl(options, service, 0) < 0))
     return -1;
 
   poison_fname = rend_service_sos_poison_path(service);
@@ -1136,44 +1213,29 @@ poison_new_single_onion_hidden_service_dir(const rend_service_t *service)
   return retval;
 }
 
-/** We just got launched in Single Onion Mode. That's a non-anoymous
+/** We just got launched in Single Onion Mode. That's a non-anoymous mode for
- * mode for hidden services; hence we should mark all new hidden service
+ * hidden services. If s is new, we should mark its hidden service
- * directories appropriately so that they are never launched as
+ * directory appropriately so that it is never launched as a location-private
- * location-private hidden services again. (New directories don't have private
+ * hidden service. (New directories don't have private key files.)
- * key files.)
- * If a <b>service_list</b> is provided, treat it as the list of hidden
- * services (used in unittests).
  * Return 0 on success, -1 on fail. */
-int
+STATIC int
-rend_service_poison_new_single_onion_dirs(const smartlist_t *service_list)
+rend_service_poison_new_single_onion_dir(const rend_service_t *s,
+                                         const or_options_t* options)
 {
+  /* Passing a NULL service is a bug */
+  if (BUG(!s)) {
+    return -1;
+  }
+
   /* We must only poison directories if we're in Single Onion mode */
-  tor_assert(rend_service_non_anonymous_mode_enabled(get_options()));
+  tor_assert(rend_service_non_anonymous_mode_enabled(options));
 
-  const smartlist_t *s_list;
-  /* If no special service list is provided, then just use the global one. */
-  if (!service_list) {
-    if (!rend_service_list) { /* No global HS list. Nothing to see here. */
-      return 0;
-    }
-
-    s_list = rend_service_list;
-  } else {
-    s_list = service_list;
-  }
-
-  SMARTLIST_FOREACH_BEGIN(s_list, const rend_service_t *, s) {
   if (!rend_service_private_key_exists(s)) {
-      if (poison_new_single_onion_hidden_service_dir(s) < 0) {
+    if (poison_new_single_onion_hidden_service_dir_impl(s, options)
+        < 0) {
       return -1;
     }
   }
-  } SMARTLIST_FOREACH_END(s);
-
-  /* The keys for these services are linked to the server IP address */
-  log_notice(LD_REND, "The configured onion service directories have been "
-             "used in single onion mode. They can not be used for anonymous "
-             "hidden services.");
 
   return 0;
 }
@@ -1187,10 +1249,12 @@ rend_service_poison_new_single_onion_dirs(const smartlist_t *service_list)
 int
 rend_service_load_all_keys(const smartlist_t *service_list)
 {
-  const smartlist_t *s_list;
+  const smartlist_t *s_list = NULL;
   /* If no special service list is provided, then just use the global one. */
   if (!service_list) {
-    tor_assert(rend_service_list);
+    if (BUG(!rend_service_list)) {
+      return -1;
+    }
     s_list = rend_service_list;
   } else {
     s_list = service_list;
@@ -1257,17 +1321,10 @@ rend_service_derive_key_digests(struct rend_service_t *s)
   return 0;
 }
 
-/** Make sure that the directory for <b>s</b> is private, using the config in
+/* Implements the directory check from rend_service_check_private_dir,
- * <b>options</b>.
+ * without doing the single onion poison checks. */
- * If <b>create</b> is true:
- *  - if the directory exists, change permissions if needed,
- *  - if the directory does not exist, create it with the correct permissions.
- * If <b>create</b> is false:
- *  - if the directory exists, check permissions,
- *  - if the directory does not exist, check if we think we can create it.
- * Return 0 on success, -1 on failure. */
 static int
-rend_service_check_private_dir(const or_options_t *options,
+rend_service_check_private_dir_impl(const or_options_t *options,
                                     const rend_service_t *s,
                                     int create)
 {
@@ -1283,8 +1340,79 @@ rend_service_check_private_dir(const or_options_t *options,
   }
   /* Check/create directory */
   if (check_private_dir(s->directory, check_opts, options->User) < 0) {
+    log_warn(LD_REND, "Checking service directory %s failed.", s->directory);
     return -1;
   }
+
+  return 0;
+}
+
+/** Make sure that the directory for <b>s</b> is private, using the config in
+ * <b>options</b>.
+ * If <b>create</b> is true:
+ *  - if the directory exists, change permissions if needed,
+ *  - if the directory does not exist, create it with the correct permissions.
+ * If <b>create</b> is false:
+ *  - if the directory exists, check permissions,
+ *  - if the directory does not exist, check if we think we can create it.
+ * Return 0 on success, -1 on failure. */
+static int
+rend_service_check_private_dir(const or_options_t *options,
+                               const rend_service_t *s,
+                               int create)
+{
+  /* Passing a NULL service is a bug */
+  if (BUG(!s)) {
+    return -1;
+  }
+
+  /* Check/create directory */
+  if (rend_service_check_private_dir_impl(options, s, create) < 0) {
+    return -1;
+  }
+
+  /* Check if the hidden service key exists, and was created in a different
+   * single onion service mode, and refuse to launch if it has.
+   * This is safe to call even when create is false, as it ignores missing
+   * keys and directories: they are always valid.
+   */
+  if (rend_service_verify_single_onion_poison(s, options) < 0) {
+    /* We can't use s->service_id here, as the key may not have been loaded */
+    log_warn(LD_GENERAL, "We are configured with "
+             "HiddenServiceNonAnonymousMode %d, but the hidden "
+             "service key in directory %s was created in %s mode. "
+             "This is not allowed.",
+             rend_service_non_anonymous_mode_enabled(options) ? 1 : 0,
+             rend_service_escaped_dir(s),
+             rend_service_non_anonymous_mode_enabled(options) ?
+             "an anonymous" : "a non-anonymous"
+             );
+    return -1;
+  }
+
+  /* Poison new single onion directories immediately after they are created,
+   * so that we never accidentally launch non-anonymous hidden services
+   * thinking they are anonymous. Any keys created later will end up with the
+   * correct poisoning state.
+   */
+  if (create && rend_service_non_anonymous_mode_enabled(options)) {
+    static int logged_warning = 0;
+
+    if (rend_service_poison_new_single_onion_dir(s, options) < 0) {
+      log_warn(LD_GENERAL,"Failed to mark new hidden services as non-anonymous"
+               ".");
+      return -1;
+    }
+
+    if (!logged_warning) {
+      /* The keys for these services are linked to the server IP address */
+      log_notice(LD_REND, "The configured onion service directories have been "
+                 "used in single onion mode. They can not be used for "
+                 "anonymous hidden services.");
+      logged_warning = 1;
+    }
+  }
+
   return 0;
 }
 
@@ -1297,7 +1425,9 @@ rend_service_load_keys(rend_service_t *s)
   char *fname = NULL;
   char buf[128];
 
-  if (rend_service_check_private_dir(get_options(), s, 1) < 0)
+  /* Make sure the directory was created and single onion poisoning was
+   * checked before calling this function */
+  if (BUG(rend_service_check_private_dir(get_options(), s, 0) < 0))
     goto err;
 
   /* Load key */

src/or/rendservice.h

@@ -119,7 +119,16 @@ typedef struct rend_service_t {
 
 STATIC void rend_service_free(rend_service_t *service);
 STATIC char *rend_service_sos_poison_path(const rend_service_t *service);
-
+STATIC int rend_service_check_dir_and_add(smartlist_t *service_list,
+                                          const or_options_t *options,
+                                          rend_service_t *service,
+                                          int validate_only);
+STATIC int rend_service_verify_single_onion_poison(
+                                                  const rend_service_t *s,
+                                                  const or_options_t *options);
+STATIC int rend_service_poison_new_single_onion_dir(
+                                                  const rend_service_t *s,
+                                                  const or_options_t* options);
 #endif
 
 int num_rend_services(void);
@@ -165,11 +174,6 @@ void rend_service_port_config_free(rend_service_port_config_t *p);
 
 void rend_authorized_client_free(rend_authorized_client_t *client);
 
-int rend_service_list_verify_single_onion_poison(
-                                              const smartlist_t *service_list,
-                                              const or_options_t *options);
-int rend_service_poison_new_single_onion_dirs(const smartlist_t *service_list);
-
 /** Return value from rend_service_add_ephemeral. */
 typedef enum {
   RSAE_BADAUTH = -5, /**< Invalid auth_type/auth_clients */

src/test/test_hs.c

@@ -542,16 +542,16 @@ test_single_onion_poisoning(void *arg)
   char *dir2 = tor_strdup(get_fname_rnd("test_hs_dir2"));
   smartlist_t *services = smartlist_new();
 
-  /* No services, no problem! */
+  /* No services, no service to verify, no problem! */
   mock_options->HiddenServiceSingleHopMode = 0;
   mock_options->HiddenServiceNonAnonymousMode = 0;
-  ret = rend_service_list_verify_single_onion_poison(services, mock_options);
+  ret = rend_config_services(mock_options, 1);
   tt_assert(ret == 0);
 
   /* Either way, no problem. */
   mock_options->HiddenServiceSingleHopMode = 1;
   mock_options->HiddenServiceNonAnonymousMode = 1;
-  ret = rend_service_list_verify_single_onion_poison(services, mock_options);
+  ret = rend_config_services(mock_options, 1);
   tt_assert(ret == 0);
 
   /* Create the data directory, and, if the correct bit in arg is set,
@@ -571,42 +571,86 @@ test_single_onion_poisoning(void *arg)
 
   service_1->directory = dir1;
   service_2->directory = dir2;
+  /* The services own the directory pointers now */
   dir1 = dir2 = NULL;
-  smartlist_add(services, service_1);
+  /* Add port to service 1 */
-  /* But don't add the second service yet. */
+  service_1->ports = smartlist_new();
+  service_2->ports = smartlist_new();
+  char *err_msg = NULL;
+  rend_service_port_config_t *port1 = rend_service_parse_port_config("80", " ",
+                                                                     &err_msg);
+  tt_assert(port1);
+  tt_assert(!err_msg);
+  smartlist_add(service_1->ports, port1);
 
-  /* Service directories, but no previous keys, no problem! */
+  rend_service_port_config_t *port2 = rend_service_parse_port_config("90", " ",
+                                                                     &err_msg);
+  /* Add port to service 2 */
+  tt_assert(port2);
+  tt_assert(!err_msg);
+  smartlist_add(service_2->ports, port2);
+
+  /* No services, a service to verify, no problem! */
   mock_options->HiddenServiceSingleHopMode = 0;
   mock_options->HiddenServiceNonAnonymousMode = 0;
-  ret = rend_service_list_verify_single_onion_poison(services, mock_options);
+  ret = rend_service_verify_single_onion_poison(service_1, mock_options);
+  tt_assert(ret == 0);
+  ret = rend_service_verify_single_onion_poison(service_2, mock_options);
   tt_assert(ret == 0);
 
   /* Either way, no problem. */
   mock_options->HiddenServiceSingleHopMode = 1;
   mock_options->HiddenServiceNonAnonymousMode = 1;
-  ret = rend_service_list_verify_single_onion_poison(services, mock_options);
+  ret = rend_service_verify_single_onion_poison(service_1, mock_options);
+  tt_assert(ret == 0);
+  ret = rend_service_verify_single_onion_poison(service_2, mock_options);
+  tt_assert(ret == 0);
+
+  /* Add the first service */
+  ret = rend_service_check_dir_and_add(services, mock_options, service_1, 0);
+  tt_assert(ret == 0);
+  /* But don't add the second service yet. */
+
+  /* Service directories, but no previous keys, no problem! */
+  mock_options->HiddenServiceSingleHopMode = 0;
+  mock_options->HiddenServiceNonAnonymousMode = 0;
+  ret = rend_service_verify_single_onion_poison(service_1, mock_options);
+  tt_assert(ret == 0);
+  ret = rend_service_verify_single_onion_poison(service_2, mock_options);
+  tt_assert(ret == 0);
+
+  /* Either way, no problem. */
+  mock_options->HiddenServiceSingleHopMode = 1;
+  mock_options->HiddenServiceNonAnonymousMode = 1;
+  ret = rend_service_verify_single_onion_poison(service_1, mock_options);
+  tt_assert(ret == 0);
+  ret = rend_service_verify_single_onion_poison(service_2, mock_options);
   tt_assert(ret == 0);
 
   /* Poison! Poison! Poison!
    * This can only be done in HiddenServiceSingleHopMode. */
   mock_options->HiddenServiceSingleHopMode = 1;
   mock_options->HiddenServiceNonAnonymousMode = 1;
-  ret = rend_service_poison_new_single_onion_dirs(services);
+  ret = rend_service_poison_new_single_onion_dir(service_1, mock_options);
   tt_assert(ret == 0);
   /* Poisoning twice is a no-op. */
-  ret = rend_service_poison_new_single_onion_dirs(services);
+  ret = rend_service_poison_new_single_onion_dir(service_1, mock_options);
   tt_assert(ret == 0);
 
   /* Poisoned service directories, but no previous keys, no problem! */
   mock_options->HiddenServiceSingleHopMode = 0;
   mock_options->HiddenServiceNonAnonymousMode = 0;
-  ret = rend_service_list_verify_single_onion_poison(services, mock_options);
+  ret = rend_service_verify_single_onion_poison(service_1, mock_options);
+  tt_assert(ret == 0);
+  ret = rend_service_verify_single_onion_poison(service_2, mock_options);
   tt_assert(ret == 0);
 
   /* Either way, no problem. */
   mock_options->HiddenServiceSingleHopMode = 1;
   mock_options->HiddenServiceNonAnonymousMode = 1;
-  ret = rend_service_list_verify_single_onion_poison(services, mock_options);
+  ret = rend_service_verify_single_onion_poison(service_1, mock_options);
+  tt_assert(ret == 0);
+  ret = rend_service_verify_single_onion_poison(service_2, mock_options);
   tt_assert(ret == 0);
 
   /* Now add some keys, and we'll have a problem. */
@@ -616,38 +660,48 @@ test_single_onion_poisoning(void *arg)
   /* Poisoned service directories with previous keys are not allowed. */
   mock_options->HiddenServiceSingleHopMode = 0;
   mock_options->HiddenServiceNonAnonymousMode = 0;
-  ret = rend_service_list_verify_single_onion_poison(services, mock_options);
+  ret = rend_service_verify_single_onion_poison(service_1, mock_options);
   tt_assert(ret < 0);
+  ret = rend_service_verify_single_onion_poison(service_2, mock_options);
+  tt_assert(ret == 0);
 
   /* But they are allowed if we're in non-anonymous mode. */
   mock_options->HiddenServiceSingleHopMode = 1;
   mock_options->HiddenServiceNonAnonymousMode = 1;
-  ret = rend_service_list_verify_single_onion_poison(services, mock_options);
+  ret = rend_service_verify_single_onion_poison(service_1, mock_options);
+  tt_assert(ret == 0);
+  ret = rend_service_verify_single_onion_poison(service_2, mock_options);
   tt_assert(ret == 0);
 
   /* Re-poisoning directories with existing keys is a no-op, because
    * directories with existing keys are ignored. */
   mock_options->HiddenServiceSingleHopMode = 1;
   mock_options->HiddenServiceNonAnonymousMode = 1;
-  ret = rend_service_poison_new_single_onion_dirs(services);
+  ret = rend_service_poison_new_single_onion_dir(service_1, mock_options);
   tt_assert(ret == 0);
   /* And it keeps the poison. */
-  ret = rend_service_list_verify_single_onion_poison(services, mock_options);
+  ret = rend_service_verify_single_onion_poison(service_1, mock_options);
+  tt_assert(ret == 0);
+  ret = rend_service_verify_single_onion_poison(service_2, mock_options);
   tt_assert(ret == 0);
 
   /* Now add the second service: it has no key and no poison file */
-  smartlist_add(services, service_2);
+  ret = rend_service_check_dir_and_add(services, mock_options, service_2, 0);
 
   /* A new service, and an existing poisoned service. Not ok. */
   mock_options->HiddenServiceSingleHopMode = 0;
   mock_options->HiddenServiceNonAnonymousMode = 0;
-  ret = rend_service_list_verify_single_onion_poison(services, mock_options);
+  ret = rend_service_verify_single_onion_poison(service_1, mock_options);
   tt_assert(ret < 0);
+  ret = rend_service_verify_single_onion_poison(service_2, mock_options);
+  tt_assert(ret == 0);
 
   /* But ok to add in non-anonymous mode. */
   mock_options->HiddenServiceSingleHopMode = 1;
   mock_options->HiddenServiceNonAnonymousMode = 1;
-  ret = rend_service_list_verify_single_onion_poison(services, mock_options);
+  ret = rend_service_verify_single_onion_poison(service_1, mock_options);
+  tt_assert(ret == 0);
+  ret = rend_service_verify_single_onion_poison(service_2, mock_options);
   tt_assert(ret == 0);
 
   /* Now remove the poisoning from the first service, and we have the opposite
@@ -661,50 +715,62 @@ test_single_onion_poisoning(void *arg)
    * directories. */
   mock_options->HiddenServiceSingleHopMode = 0;
   mock_options->HiddenServiceNonAnonymousMode = 0;
-  ret = rend_service_list_verify_single_onion_poison(services, mock_options);
+  ret = rend_service_verify_single_onion_poison(service_1, mock_options);
+  tt_assert(ret == 0);
+  ret = rend_service_verify_single_onion_poison(service_2, mock_options);
   tt_assert(ret == 0);
 
   /* But the existing unpoisoned key is not ok in non-anonymous mode, even if
    * there is an empty service. */
   mock_options->HiddenServiceSingleHopMode = 1;
   mock_options->HiddenServiceNonAnonymousMode = 1;
-  ret = rend_service_list_verify_single_onion_poison(services, mock_options);
+  ret = rend_service_verify_single_onion_poison(service_1, mock_options);
   tt_assert(ret < 0);
+  ret = rend_service_verify_single_onion_poison(service_2, mock_options);
+  tt_assert(ret == 0);
 
   /* Poisoning directories with existing keys is a no-op, because directories
    * with existing keys are ignored. But the new directory should poison. */
   mock_options->HiddenServiceSingleHopMode = 1;
   mock_options->HiddenServiceNonAnonymousMode = 1;
-  ret = rend_service_poison_new_single_onion_dirs(services);
+  ret = rend_service_poison_new_single_onion_dir(service_1, mock_options);
+  tt_assert(ret == 0);
+  ret = rend_service_poison_new_single_onion_dir(service_2, mock_options);
   tt_assert(ret == 0);
   /* And the old directory remains unpoisoned. */
-  ret = rend_service_list_verify_single_onion_poison(services, mock_options);
+  ret = rend_service_verify_single_onion_poison(service_1, mock_options);
   tt_assert(ret < 0);
+  ret = rend_service_verify_single_onion_poison(service_2, mock_options);
+  tt_assert(ret == 0);
 
   /* And the new directory should be ignored, because it has no key. */
   mock_options->HiddenServiceSingleHopMode = 0;
   mock_options->HiddenServiceNonAnonymousMode = 0;
-  ret = rend_service_list_verify_single_onion_poison(services, mock_options);
+  ret = rend_service_verify_single_onion_poison(service_1, mock_options);
+  tt_assert(ret == 0);
+  ret = rend_service_verify_single_onion_poison(service_2, mock_options);
   tt_assert(ret == 0);
 
   /* Re-poisoning directories without existing keys is a no-op. */
   mock_options->HiddenServiceSingleHopMode = 1;
   mock_options->HiddenServiceNonAnonymousMode = 1;
-  ret = rend_service_poison_new_single_onion_dirs(services);
+  ret = rend_service_poison_new_single_onion_dir(service_1, mock_options);
+  tt_assert(ret == 0);
+  ret = rend_service_poison_new_single_onion_dir(service_2, mock_options);
   tt_assert(ret == 0);
   /* And the old directory remains unpoisoned. */
-  ret = rend_service_list_verify_single_onion_poison(services, mock_options);
+  ret = rend_service_verify_single_onion_poison(service_1, mock_options);
   tt_assert(ret < 0);
+  ret = rend_service_verify_single_onion_poison(service_2, mock_options);
+  tt_assert(ret == 0);
 
  done:
   /* The test harness deletes the directories at exit */
+  smartlist_free(services);
   rend_service_free(service_1);
   rend_service_free(service_2);
-  smartlist_free(services);
   UNMOCK(get_options);
   tor_free(mock_options->DataDirectory);
-  tor_free(dir1);
-  tor_free(dir2);
 }
 
 struct testcase_t hs_tests[] = {