r14826@catbus: nickm | 2007-08-29 13:19:55 -0400

 Add a line to the state file for each guard to let us know which version added the guard.  If the line is absent, assume the guard was added by whatever version of Tor last wrote the state file.  Remove guards if the version that added them was using a bad guard selection algorithm.  (Previously, we removed guards if the version that wrote the file was using a bad guard selection algorithm, even if the guards themselves were chosen by a good version.) 


svn:r11298

4 files changed, 147 insertions, 25 deletions

diff --git a/ChangeLog b/ChangeLog
index fdf77a769..a0745e2f5 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,8 +1,14 @@
-Changes in version 0.2.0.6-alpha - 2007-??-??
+Changes in version 0.2.0.7-alpha - 2007-??-??
   o Minor features (security):
     - As a client, do not believe any server that tells us that any address
       maps to an internal address space.
 
+  o Minor features (guard nodes):
+    - Tag every guard node in our state file with the version that we believe
+      added it, or with our own version if we add it.  This way, if a user
+      temporarily runs an old version of Tor and then switches back to a new
+      one, she doesn't automatically lose her guards.
+
   o Minor bugfixes:
     - When generating information telling us how to extend to a given
       router, do not try to include the nickname if it is absent.  Fixes
diff --git a/doc/TODO b/doc/TODO
index 1e38a359d..d25d8fdfb 100644
--- a/doc/TODO
+++ b/doc/TODO
@@ -159,8 +159,9 @@ N     - Design/implement the "local-status" or something like it, from the
      - Or maybe close connections from same IP when we get a lot from one.
      - Or maybe block IPs that connect too many times at once.
     - add an AuthDirBadexit torrc option if we decide we want one.
-    - Add a GuardsSelectedByVersion line to the state file so we know
+    o Add a GuardsSelectedByVersion line to the state file so we know
       not to drop guards we added.
+      o Have it include the date too.
 
   - Testing
 N   - Hack up a client that gives out weird/no certificates, so we can
diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c
index 972d48951..4b14e16f6 100644
--- a/src/or/circuitbuild.c
+++ b/src/or/circuitbuild.c
@@ -25,6 +25,10 @@ extern circuit_t *global_circuitlist;
 typedef struct {
   char nickname[MAX_NICKNAME_LEN+1];
   char identity[DIGEST_LEN];
+  time_t chosen_on_date; /**< Approximately when was this guard added?
+                          * "0" if we don't know. */
+  char *chosen_by_version; /**< What tor version added this guard? NULL
+                            * if we don't know. */
   unsigned int made_contact : 1; /**< 0 if we have never connected to this
                                   * router, 1 if we have. */
   unsigned int can_retry : 1; /**< Should we retry connecting to this entry,
@@ -56,6 +60,7 @@ static int count_acceptable_routers(smartlist_t *routers);
 static int onion_append_hop(crypt_path_t **head_ptr, extend_info_t *choice);
 
 static void entry_guards_changed(void);
+static time_t start_of_month(time_t when);
 
 /** Iterate over values of circ_id, starting from conn-\>next_circ_id,
  * and with the high bit specified by conn-\>circ_id_type, until we get
@@ -2054,6 +2059,8 @@ add_an_entry_guard(routerinfo_t *chosen, int reset_status)
   log_info(LD_CIRC, "Chose '%s' as new entry guard.", router->nickname);
   strlcpy(entry->nickname, router->nickname, sizeof(entry->nickname));
   memcpy(entry->identity, router->cache_info.identity_digest, DIGEST_LEN);
+  entry->chosen_on_date = start_of_month(time(NULL));
+  entry->chosen_by_version = tor_strdup(VERSION);
   if (chosen) /* prepend */
     smartlist_insert(entry_guards, 0, entry);
   else /* append */
@@ -2087,11 +2094,62 @@ pick_entry_guards(void)
  * unlisted, excluded, or otherwise nonusable before we give up on it? */
 #define ENTRY_GUARD_REMOVE_AFTER (30*24*60*60)
 
+/** Release all storage held by <b>e</b>. */
+static void
+entry_guard_free(entry_guard_t *e)
+{
+  tor_assert(e);
+  tor_free(e->chosen_by_version);
+  tor_free(e);
+}
+
+/** DOCDOC */
+static int
+remove_obsolete_entry_guards(void)
+{
+  int changed, i;
+  for (i = 0; i < smartlist_len(entry_guards); ++i) {
+    entry_guard_t *entry = smartlist_get(entry_guards, i);
+    const char *ver = entry->chosen_by_version;
+    const char *msg = NULL;
+    tor_version_t v;
+    int version_is_bad = 0;
+    if (!ver) {
+      msg = "does not say what version of Tor it was selected by";
+      version_is_bad = 1;
+    } else if (tor_version_parse(ver, &v)) {
+      msg = "does not seem to be from any recognized version of Tor";
+      version_is_bad = 1;
+    } else if ((tor_version_as_new_as(ver, "0.1.0.10-alpha") &&
+                !tor_version_as_new_as(ver, "0.1.2.16-dev")) ||
+               (tor_version_as_new_as(ver, "0.2.0.0-alpha") &&
+                !tor_version_as_new_as(ver, "0.2.0.6-alpha"))) {
+      msg = "was selected without regard for guard bandwidth";
+      version_is_bad = 1;
+    }
+    if (version_is_bad) {
+      char dbuf[HEX_DIGEST_LEN+1];
+      tor_assert(msg);
+      base16_encode(dbuf, sizeof(dbuf), entry->identity, DIGEST_LEN);
+      log_notice(LD_CIRC, "Entry guard '%s' (%s) %s. (Version=%s.)  "
+                 "Replacing it.",
+                 entry->nickname, dbuf, msg, ver?escaped(ver):"none");
+      control_event_guard(entry->nickname, entry->identity, "DROPPED");
+      entry_guard_free(entry);
+      smartlist_del_keeporder(entry_guards, i--);
+      log_entry_guards(LOG_INFO);
+      changed = 1;
+    }
+  }
+
+  return changed ? 1 : 0;
+}
+
 /** Remove all entry guards that have been down or unlisted for so
  * long that we don't think they'll come up again. Return 1 if we
  * removed any, or 0 if we did nothing. */
 static int
-remove_dead_entries(void)
+remove_dead_entry_guards(void)
 {
   char dbuf[HEX_DIGEST_LEN+1];
   char tbuf[ISO_TIME_LEN+1];
@@ -2110,7 +2168,7 @@ remove_dead_entries(void)
                "since %s local time; removing.",
                entry->nickname, dbuf, tbuf);
       control_event_guard(entry->nickname, entry->identity, "DROPPED");
-      tor_free(entry);
+      entry_guard_free(entry);
       smartlist_del_keeporder(entry_guards, i);
       log_entry_guards(LOG_INFO);
       changed = 1;
@@ -2155,7 +2213,7 @@ entry_guards_compute_status(void)
                entry_is_live(entry, 0, 1, 0) ? "live" : "not live");
     });
 
-  if (remove_dead_entries())
+  if (remove_dead_entry_guards())
     changed = 1;
 
   if (changed) {
@@ -2454,6 +2512,19 @@ choose_random_entry(cpath_build_state_t *state)
   return r;
 }
 
+/** DOCDOC */
+static time_t
+start_of_month(time_t now)
+{
+  struct tm tm;
+  tor_gmtime_r(&now, &tm);
+  tm.tm_sec = 0;
+  tm.tm_min = 0;
+  tm.tm_hour = 0;
+  tm.tm_mday = 1;
+  return tor_timegm(&tm);
+}
+
 /** Parse <b>state</b> and learn about the entry guards it describes.
  * If <b>set</b> is true, and there are no errors, replace the global
  * entry_list with what we find.
@@ -2467,6 +2538,8 @@ entry_guards_parse_state(or_state_t *state, int set, char **msg)
   smartlist_t *new_entry_guards = smartlist_create();
   config_line_t *line;
   time_t now = time(NULL);
+  const char *state_version = state->TorVersion;
+  digestmap_t *added_by = digestmap_new();
 
   *msg = NULL;
   for (line = state->EntryGuards; line; line = line->next) {
@@ -2496,7 +2569,8 @@ entry_guards_parse_state(or_state_t *state, int set, char **msg)
       smartlist_free(args);
       if (*msg)
         break;
-    } else {
+    } else if (!strcasecmp(line->key, "EntryGuardDownSince") ||
+               !strcasecmp(line->key, "EntryGuardUnlistedSince")) {
       time_t when;
       time_t last_try = 0;
       if (!node) {
@@ -2524,9 +2598,46 @@ entry_guards_parse_state(or_state_t *state, int set, char **msg)
       } else {
         node->bad_since = when;
       }
+    } else if (!strcasecmp(line->key, "EntryGuardAddedBy")) {
+      char d[DIGEST_LEN];
+      /* format is digest version date */
+      if (strlen(line->value) < HEX_DIGEST_LEN+1+1+1+ISO_TIME_LEN) {
+        log_warn(LD_BUG, "EntryGuardAddedBy line is not long enough.");
+        continue;
+      }
+      if (base16_decode(d, sizeof(d), line->value, HEX_DIGEST_LEN)<0 ||
+          line->value[HEX_DIGEST_LEN] != ' ') {
+        log_warn(LD_BUG, "EntryGuardAddedBy line %s does not begin with "
+                 "hex digest", escaped(line->value));
+        continue;
+      }
+      digestmap_set(added_by, d, tor_strdup(line->value+HEX_DIGEST_LEN+1));
+    } else {
+      log_warn(LD_BUG, "Unexpected key %s", line->key);
     }
   }
 
+  SMARTLIST_FOREACH(new_entry_guards, entry_guard_t *, e,
+   {
+     char *sp;
+     char *val = digestmap_get(added_by, e->identity);
+     if (val && (sp = strchr(val, ' '))) {
+       time_t when;
+       *sp++ = '\0';
+       if (parse_iso_time(sp, &when)<0) {
+         log_warn(LD_BUG, "Can't read time %s in EntryGuardAddedBy", sp);
+       } else {
+         e->chosen_by_version = tor_strdup(val);
+         e->chosen_on_date = when;
+       }
+     } else {
+       if (state_version) {
+         e->chosen_by_version = tor_strdup(state_version);
+         e->chosen_on_date = start_of_month(time(NULL));
+       }
+     }
+   });
+
   if (*msg || !set) {
     SMARTLIST_FOREACH(new_entry_guards, entry_guard_t *, e, tor_free(e));
     smartlist_free(new_entry_guards);
@@ -2537,7 +2648,10 @@ entry_guards_parse_state(or_state_t *state, int set, char **msg)
     }
     entry_guards = new_entry_guards;
     entry_guards_dirty = 0;
+    if (remove_obsolete_entry_guards())
+      entry_guards_dirty = 1;
   }
+  digestmap_free(added_by, _tor_free);
   return *msg ? -1 : 0;
 }
 
@@ -2603,6 +2717,22 @@ entry_guards_update_state(or_state_t *state)
         format_iso_time(line->value, e->bad_since);
         next = &(line->next);
       }
+      if (e->chosen_on_date && e->chosen_by_version &&
+          !strchr(e->chosen_by_version, ' ')) {
+        char d[HEX_DIGEST_LEN+1];
+        char t[ISO_TIME_LEN+1];
+        size_t val_len;
+        *next = line = tor_malloc_zero(sizeof(config_line_t));
+        line->key = tor_strdup("EntryGuardAddedBy");
+        val_len = (HEX_DIGEST_LEN+1+strlen(e->chosen_by_version)
+                   +1+ISO_TIME_LEN+1);
+        line->value = tor_malloc(val_len);
+        base16_encode(d, sizeof(d), e->identity, DIGEST_LEN);
+        format_iso_time(t, e->chosen_on_date);
+        tor_snprintf(line->value, val_len, "%s %s %s",
+                     d, e->chosen_by_version, t);
+        next = &(line->next);
+      }
     });
   if (!get_options()->AvoidDiskWrites)
     or_state_mark_dirty(get_or_state(), 0);
@@ -2670,6 +2800,7 @@ getinfo_helper_entry_guards(control_connection_t *conn,
   return 0;
 }
 
+/** DOCDOC */
 typedef struct {
   uint32_t addr;
   uint16_t port;
@@ -2920,7 +3051,8 @@ void
 entry_guards_free_all(void)
 {
   if (entry_guards) {
-    SMARTLIST_FOREACH(entry_guards, entry_guard_t *, e, tor_free(e));
+    SMARTLIST_FOREACH(entry_guards, entry_guard_t *, e,
+                      entry_guard_free(e));
     smartlist_free(entry_guards);
     entry_guards = NULL;
   }
diff --git a/src/or/config.c b/src/or/config.c
index 6285fe4ae..735d21432 100644
--- a/src/or/config.c
+++ b/src/or/config.c
@@ -302,6 +302,7 @@ static config_var_t _state_vars[] = {
   VAR("EntryGuard",              LINELIST_S,  EntryGuards,             NULL),
   VAR("EntryGuardDownSince",     LINELIST_S,  EntryGuards,             NULL),
   VAR("EntryGuardUnlistedSince", LINELIST_S,  EntryGuards,             NULL),
+  VAR("EntryGuardAddedBy",       LINELIST_S,  EntryGuards,             NULL),
   V(EntryGuards,                 LINELIST_V,  NULL),
 
   V(BWHistoryReadEnds,                ISOTIME,  NULL),
@@ -4355,24 +4356,6 @@ or_state_validate(or_state_t *old_state, or_state_t *state,
   if (entry_guards_parse_state(state, 0, msg)<0)
     return -1;
 
-  if (state->EntryGuards && state->TorVersion) {
-    tor_version_t v;
-    if (tor_version_parse(state->TorVersion, &v)) {
-      log_warn(LD_GENERAL, "Can't parse Tor version '%s' from your state "
-               "file. Proceeding anyway.", state->TorVersion);
-    } else { /* take action based on v */
-      if ((tor_version_as_new_as(state->TorVersion, "0.1.1.10-alpha") &&
-           !tor_version_as_new_as(state->TorVersion, "0.1.2.16-dev")) ||
-          (tor_version_as_new_as(state->TorVersion, "0.2.0.0-alpha") &&
-           !tor_version_as_new_as(state->TorVersion, "0.2.0.6-alpha"))) {
-        log_notice(LD_CONFIG, "Detected state file from old version '%s'. "
-                   "Choosing new entry guards for you.",
-                   state->TorVersion);
-        config_free_lines(state->EntryGuards);
-        state->EntryGuards = NULL;
-      }
-    }
-  }
   return 0;
 }