Handle transitions in Automap*, VirtualAddrNetwork correctly

Previously, if they changed in torrc during a SIGHUP, all was well,
since we would just clear all transient entries from the addrmap
thanks to bug 1345.  But if you changed them from the controller, Tor
would leave old mappings in place.

The VirtualAddrNetwork bug has been here since 0.1.1.19-rc; the
AutomapHosts* bug has been here since 0.2.0.1-alpha.

5 files changed, 69 insertions, 1 deletions

diff --git a/changes/bug1345 b/changes/bug1345
index b35e78f5e8..0c9375a35d 100644
--- a/changes/bug1345
+++ b/changes/bug1345
@@ -5,3 +5,9 @@
     - When TrackHostExits is changed from a controller, remove any
       mappings for hosts that should no longer have their exits tracked.
       Bugfix on Tor 0.1.0.1-rc.
+    - When VirtualAddrNetwork option is changed from a controller,
+      remove any mappings for hosts that were automapped to
+      that network.  Bugfix on 0.1.1.19-rc.
+    - When one of the AutomapHosts* options is changed from a
+      controller, remove any mappings for hosts that should no longer be
+      automapped.  Bugfix on 0.2.0.1-alpha.
diff --git a/src/or/config.c b/src/or/config.c
index 147cc66b6b..9c68b6fa59 100644
--- a/src/or/config.c
+++ b/src/or/config.c
@@ -1265,6 +1265,7 @@ options_act(or_options_t *old_options)
   /* Check for transitions that need action. */
   if (old_options) {
     int revise_trackexithosts = 0;
+    int revise_automap_entries = 0;
     if ((options->UseEntryGuards && !old_options->UseEntryGuards) ||
         !routerset_equal(old_options->ExcludeNodes,options->ExcludeNodes) ||
         !routerset_equal(old_options->ExcludeExitNodes,
@@ -1287,6 +1288,20 @@ options_act(or_options_t *old_options)
     if (revise_trackexithosts)
       addressmap_clear_excluded_trackexithosts(options);
 
+    if (old_options->AutomapHostsOnResolve && !options->AutomapHostsOnResolve) {
+      revise_automap_entries = 1;
+    } else if (options->AutomapHostsOnResolve) {
+      if (!smartlist_strings_eq(old_options->AutomapHostsSuffixes,
+                                options->AutomapHostsSuffixes))
+        revise_automap_entries = 1;
+      else if (!opt_streq(old_options->VirtualAddrNetwork,
+                          options->VirtualAddrNetwork))
+        revise_automap_entries = 1;
+    }
+
+    if (revise_automap_entries)
+      addressmap_clear_invalid_automaps(options);
+
 /* How long should we delay counting bridge stats after becoming a bridge?
  * We use this so we don't count people who used our bridge thinking it is
  * a relay. If you change this, don't forget to change the log message
diff --git a/src/or/connection_edge.c b/src/or/connection_edge.c
index 5301471e91..7828f16386 100644
--- a/src/or/connection_edge.c
+++ b/src/or/connection_edge.c
@@ -861,6 +861,49 @@ addressmap_clear_excluded_trackexithosts(or_options_t *options)
   } STRMAP_FOREACH_END;
 }
 
+/** Remove all AUTOMAP mappings from the addressmap for which the
+ * source address no longer matches AutomapHostsSuffixes, which is
+ * no longer allowed by AutomapHostsOnResolve, or for which the
+ * target address is no longer in the virtual network. */
+void
+addressmap_clear_invalid_automaps(or_options_t *options)
+{
+  int clear_all = !options->AutomapHostsOnResolve;
+  const smartlist_t *suffixes = options->AutomapHostsSuffixes;
+
+  if (!addressmap)
+    return;
+
+  if (!suffixes)
+    clear_all = 1; /* This should be impossible, but let's be sure. */
+
+  STRMAP_FOREACH_MODIFY(addressmap, src_address, addressmap_entry_t *, ent) {
+    int remove = clear_all;
+    if (ent->source != ADDRMAPSRC_AUTOMAP)
+      continue; /* not an automap mapping. */
+
+    if (!remove) {
+      int suffix_found = 0;
+      SMARTLIST_FOREACH(suffixes, const char *, suffix, {
+          if (!strcasecmpend(src_address, suffix)) {
+            suffix_found = 1;
+            break;
+          }
+      });
+      if (!suffix_found)
+        remove = 1;
+    }
+
+    if (!remove && ! address_is_in_virtual_range(ent->new_address))
+      remove = 1;
+
+    if (remove) {
+      addressmap_ent_remove(src_address, ent);
+      MAP_DEL_CURRENT(src_address);
+    }
+  } STRMAP_FOREACH_END;
+}
+
 /** Remove all entries from the addressmap that were set via the
  * configuration file or the command line. */
 void
@@ -1372,7 +1415,7 @@ addressmap_register_virtual_address(int type, char *new_address)
   log_info(LD_APP, "Registering map from %s to %s", *addrp, new_address);
   if (vent_needs_to_be_added)
     strmap_set(virtaddress_reversemap, new_address, vent);
-  addressmap_register(*addrp, new_address, 2, ADDRMAPSRC_CONTROLLER);
+  addressmap_register(*addrp, new_address, 2, ADDRMAPSRC_AUTOMAP);
 
 #if 0
   {
diff --git a/src/or/connection_edge.h b/src/or/connection_edge.h
index 70d0dd2713..8ba2fafd08 100644
--- a/src/or/connection_edge.h
+++ b/src/or/connection_edge.h
@@ -62,6 +62,7 @@ int address_is_invalid_destination(const char *address, int client);
 
 void addressmap_init(void);
 void addressmap_clear_excluded_trackexithosts(or_options_t *options);
+void addressmap_clear_invalid_automaps(or_options_t *options);
 void addressmap_clean(time_t now);
 void addressmap_clear_configured(void);
 void addressmap_clear_transient(void);
diff --git a/src/or/or.h b/src/or/or.h
index a73d98ab74..5647691550 100644
--- a/src/or/or.h
+++ b/src/or/or.h
@@ -3150,6 +3150,9 @@ typedef enum setopt_err_t {
 typedef enum {
   /** We're remapping this address because the controller told us to. */
   ADDRMAPSRC_CONTROLLER,
+  /** We're remapping this address because of an AutomapHostsOnResolve
+   * configuration. */
+  ADDRMAPSRC_AUTOMAP,
   /** We're remapping this address because our configuration (via torrc, the
    * command line, or a SETCONF command) told us to. */
   ADDRMAPSRC_TORRC,