r14659@catbus: nickm | 2007-08-18 14:19:34 -0400

 When we are loading state info from disk, never believe any date in the future.  Doing so can keep us from retrying guards, rotating onion keys, storing bandwidth info, etc.  Fixes bug 434, and others. Backport candidate, once it has been tested.


svn:r11166

5 files changed, 28 insertions, 5 deletions

diff --git a/ChangeLog b/ChangeLog
index 470b3379b5..bcfece2b6e 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -46,6 +46,13 @@ Changes in version 0.2.0.5-alpha - 2007-??-??
       every time we change any piece of our config.
     - Fix a bug with AutomapHostsOnResolve that would always cause the second
       request to fail.  Bug reported by Kate.  Bugfix on 0.2.0.3-alpha.
+    - When loading bandwidth history, do not believe any information in
+      the future.  Fixes bug 434.
+    - When loading entry guard information, do not believe any information
+      in the future.
+    - When we have our clock set far in the future and generate an onion key,
+      then re-set our clock to be correct, we should not stop the onion
+      key from getting rotated.
 
 
 Changes in version 0.2.0.4-alpha - 2007-08-01
diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c
index fd99c0f519..7dcbf71d29 100644
--- a/src/or/circuitbuild.c
+++ b/src/or/circuitbuild.c
@@ -2465,6 +2465,7 @@ entry_guards_parse_state(or_state_t *state, int set, char **msg)
   entry_guard_t *node = NULL;
   smartlist_t *new_entry_guards = smartlist_create();
   config_line_t *line;
+  time_t now = time(NULL);
 
   *msg = NULL;
   for (line = state->EntryGuards; line; line = line->next) {
@@ -2507,6 +2508,11 @@ entry_guards_parse_state(or_state_t *state, int set, char **msg)
                           "Bad time in EntryGuardDownSince/UnlistedSince");
         break;
       }
+      if (when > now) {
+        /* It's a bad idea to believe info in the future: you can wind
+         * up with timeouts that aren't allowed to happen for years. */
+        continue;
+      }
       if (strlen(line->value) >= ISO_TIME_LEN+ISO_TIME_LEN+1) {
         /* ignore failure */
         parse_iso_time(line->value+ISO_TIME_LEN+1, &last_try);
diff --git a/src/or/hibernate.c b/src/or/hibernate.c
index 621e042894..8e3c7332e7 100644
--- a/src/or/hibernate.c
+++ b/src/or/hibernate.c
@@ -696,6 +696,9 @@ read_bandwidth_usage(void)
     goto done;
   }
 
+  /* XXXX020 do something if the interval "starts" far in the future?
+   * or do we alrady handle that. */
+
   n_bytes_read_in_interval = n_read;
   n_bytes_written_in_interval = n_written;
   n_seconds_active_in_interval = n_seconds;
diff --git a/src/or/rephist.c b/src/or/rephist.c
index 1bb8775d8f..3228d24a3f 100644
--- a/src/or/rephist.c
+++ b/src/or/rephist.c
@@ -993,7 +993,8 @@ rep_hist_load_state(or_state_t *state, char **err)
     s_values =  r?state->BWHistoryReadValues:state->BWHistoryWriteValues;
     if (s_values && s_begins >= now - NUM_SECS_BW_SUM_INTERVAL*NUM_TOTALS) {
       start = s_begins - s_interval*(smartlist_len(s_values));
-
+      if (start > now)
+        continue;
       b->cur_obs_time = start;
       b->next_period = start + NUM_SECS_BW_SUM_INTERVAL;
       SMARTLIST_FOREACH(s_values, char *, cp, {
@@ -1002,8 +1003,10 @@ rep_hist_load_state(or_state_t *state, char **err)
           all_ok=0;
           log_notice(LD_GENERAL, "Could not parse '%s' into a number.'", cp);
         }
-        add_obs(b, start, v);
-        start += NUM_SECS_BW_SUM_INTERVAL;
+        if (start < now) {
+          add_obs(b, start, v);
+          start += NUM_SECS_BW_SUM_INTERVAL;
+        }
       });
     }
 
diff --git a/src/or/router.c b/src/or/router.c
index 135c115d1d..2c165a7aa8 100644
--- a/src/or/router.c
+++ b/src/or/router.c
@@ -332,6 +332,7 @@ init_keys(void)
   or_options_t *options = get_options();
   or_state_t *state = get_or_state();
   authority_type_t type;
+  time_t now = time(NULL);
 
   if (!key_lock)
     key_lock = tor_mutex_new();
@@ -389,14 +390,17 @@ init_keys(void)
   prkey = init_key_from_file(keydir, 1, LOG_ERR);
   if (!prkey) return -1;
   set_onion_key(prkey);
-  if (state->LastRotatedOnionKey > 100) { /* allow for some parsing slop. */
+  if (state->LastRotatedOnionKey > 100 && state->LastRotatedOnionKey < now) {
+    /* We allow for some parsing slop, but we don't want to risk accepting
+     * values in the distant future.  If we did, we might never rotate the
+     * onion key. */
     onionkey_set_at = state->LastRotatedOnionKey;
   } else {
     /* We have no LastRotatedOnionKey set; either we just created the key
      * or it's a holdover from 0.1.2.4-alpha-dev or earlier.  In either case,
      * start the clock ticking now so that we will eventually rotate it even
      * if we don't stay up for a full MIN_ONION_KEY_LIFETIME. */
-    state->LastRotatedOnionKey = onionkey_set_at = time(NULL);
+    state->LastRotatedOnionKey = onionkey_set_at = now;
     or_state_mark_dirty(state, options->AvoidDiskWrites ? time(NULL)+3600 : 0);
   }