[tor-commits] [tor/master] hs: Rescan the main loop event list if the service map changes

[nickm at torproject.org]

commit f7633c1fcaefe72bf97c001bab9062132c919996
Author: David Goulet <dgoulet at torproject.org>
Date:   Fri Apr 27 11:14:12 2018 -0400

    hs: Rescan the main loop event list if the service map changes
    
    Because ADD_ONION/DEL_ONION can modify the global service map (both for v2 and
    v3), we need to rescan the event list so we either enable or disable the HS
    service main loop event.
    
    Fixees #25939
    
    Signed-off-by: David Goulet <dgoulet at torproject.org>
---
 src/or/hs_service.c            | 28 ++++++++++++++++++++++++++++
 src/or/hs_service.h            |  1 +
 src/or/rendservice.c           | 11 +++++++++++
 src/test/test_periodic_event.c | 12 +++++++++---
 4 files changed, 49 insertions(+), 3 deletions(-)

diff --git a/src/or/hs_service.c b/src/or/hs_service.c
index f6c7e3cd8..7af14373d 100644
--- a/src/or/hs_service.c
+++ b/src/or/hs_service.c
@@ -80,6 +80,7 @@ static smartlist_t *hs_service_staging_list;
  *  reupload if needed */
 static int consider_republishing_hs_descriptors = 0;
 
+/* Static declaration. */
 static void set_descriptor_revision_counter(hs_descriptor_t *hs_desc);
 static void move_descriptors(hs_service_t *src, hs_service_t *dst);
 
@@ -152,6 +153,12 @@ register_service(hs_service_ht *map, hs_service_t *service)
   }
   /* Taking ownership of the object at this point. */
   HT_INSERT(hs_service_ht, map, service);
+
+  /* If we just modified the global map, we notify. */
+  if (map == hs_service_map) {
+    hs_service_map_has_changed();
+  }
+
   return 0;
 }
 
@@ -178,6 +185,11 @@ remove_service(hs_service_ht *map, hs_service_t *service)
                      "while removing service %s",
              escaped(service->config.directory_path));
   }
+
+  /* If we just modified the global map, we notify. */
+  if (map == hs_service_map) {
+    hs_service_map_has_changed();
+  }
 }
 
 /* Set the default values for a service configuration object <b>c</b>. */
@@ -916,6 +928,11 @@ register_all_services(void)
   smartlist_clear(hs_service_staging_list);
   service_free_all();
   hs_service_map = new_service_map;
+  /* We've just register services into the new map and now we've replaced the
+   * global map with it so we have to notify that the change happened. When
+   * registering a service, the notify is only triggered if the destination
+   * map is the global map for which in here it was not. */
+  hs_service_map_has_changed();
 }
 
 /* Write the onion address of a given service to the given filename fname_ in
@@ -2936,6 +2953,17 @@ service_add_fnames_to_list(const hs_service_t *service, smartlist_t *list)
 /* Public API */
 /* ========== */
 
+/* This is called everytime the service map (v2 or v3) changes that is if an
+ * element is added or removed. */
+void
+hs_service_map_has_changed(void)
+{
+  /* If we now have services where previously we had not, we need to enable
+   * the HS service main loop event. If we changed to having no services, we
+   * need to disable the event. */
+  rescan_periodic_events(get_options());
+}
+
 /* Upload an encoded descriptor in encoded_desc of the given version. This
  * descriptor is for the service identity_pk and blinded_pk used to setup the
  * directory connection identifier. It is uploaded to the directory hsdir_rs
diff --git a/src/or/hs_service.h b/src/or/hs_service.h
index d163eeef2..2e27d8a89 100644
--- a/src/or/hs_service.h
+++ b/src/or/hs_service.h
@@ -260,6 +260,7 @@ void hs_service_lists_fnames_for_sandbox(smartlist_t *file_list,
 int hs_service_set_conn_addr_port(const origin_circuit_t *circ,
                                   edge_connection_t *conn);
 
+void hs_service_map_has_changed(void);
 void hs_service_dir_info_changed(void);
 void hs_service_run_scheduled_events(time_t now);
 void hs_service_circuit_has_opened(origin_circuit_t *circ);
diff --git a/src/or/rendservice.c b/src/or/rendservice.c
index 1a93c3643..afaeabe5d 100644
--- a/src/or/rendservice.c
+++ b/src/or/rendservice.c
@@ -348,6 +348,13 @@ rend_add_service(smartlist_t *service_list, rend_service_t *service)
   /* The service passed all the checks */
   tor_assert(s_list);
   smartlist_add(s_list, service);
+
+  /* Notify that our global service list has changed only if this new service
+   * went into our global list. If not, when we move service from the staging
+   * list to the new list, a notify is triggered. */
+  if (s_list == rend_service_list) {
+    hs_service_map_has_changed();
+  }
   return 0;
 }
 
@@ -609,6 +616,8 @@ rend_service_prune_list_impl_(void)
     circuit_mark_for_close(TO_CIRCUIT(ocirc), END_CIRC_REASON_FINISHED);
   }
   smartlist_free(surviving_services);
+  /* Notify that our global service list has changed. */
+  hs_service_map_has_changed();
 }
 
 /* Try to prune our main service list using the temporary one that we just
@@ -958,6 +967,8 @@ rend_service_del_ephemeral(const char *service_id)
     }
   } SMARTLIST_FOREACH_END(circ);
   smartlist_remove(rend_service_list, s);
+  /* Notify that we just removed a service from our global list. */
+  hs_service_map_has_changed();
   rend_service_free(s);
 
   log_debug(LD_CONFIG, "Removed ephemeral Onion Service: %s", service_id);
diff --git a/src/test/test_periodic_event.c b/src/test/test_periodic_event.c
index bebbb5e58..9f62cd680 100644
--- a/src/test/test_periodic_event.c
+++ b/src/test/test_periodic_event.c
@@ -69,7 +69,7 @@ test_pe_initialize(void *arg)
 static void
 test_pe_launch(void *arg)
 {
-  hs_service_t service;
+  hs_service_t service, *to_remove = NULL;
   or_options_t *options;
 
   (void) arg;
@@ -152,8 +152,11 @@ test_pe_launch(void *arg)
   options->V3AuthoritativeDir = 1; options->BridgeAuthoritativeDir = 1;
   register_dummy_hidden_service(&service);
   periodic_events_on_new_options(options);
-  /* Remove it now so the hs_free_all() doesn't try to free stack memory. */
-  remove_service(get_hs_service_map(), &service);
+  /* Note down the reference because we need to remove this service from the
+   * global list before the hs_free_all() call so it doesn't try to free
+   * memory on the stack. Furthermore, we can't remove it now else it will
+   * trigger a rescan of the event disabling the HS service event. */
+  to_remove = &service;
 
   for (int i = 0; periodic_events[i].name; ++i) {
     periodic_event_item_t *item = &periodic_events[i];
@@ -161,6 +164,9 @@ test_pe_launch(void *arg)
   }
 
  done:
+  if (to_remove) {
+    remove_service(get_hs_service_map(), to_remove);
+  }
   hs_free_all();
 }