r15636@catbus: nickm | 2007-10-10 15:28:12 -0400

Retry consensus and certificate downloads properly.  Do not fail when there are no certificates to download.  Do not download certificates we already have when retrying.


svn:r11841

ChangeLog

@@ -13,6 +13,8 @@ Changes in version 0.2.0.8-alpha - 2007-10-12
     - Caches now download v3 network status documents as needed.
     - Caches now download descriptors listed in their v3 network status
       documents.
+    - All hosts now attempt to download and keep fresh v3 authority
+      certificates, and re-attempt after failures.
 
   o Minor features (router descriptor cache):
     - Store routers in a file called cached-descriptors instead of in

doc/TODO

@@ -85,8 +85,12 @@ Things we'd like to do in 0.2.0.x:
         them
         o Download code
         o Code to schedule downloads
-        - Code to retry failed downloads
+        o Code to retry failed downloads
-        - Code to delay next download while fetching certificates
+        - Code to delay next download while fetching certificates to verify
+          a consensus we already got.
+        - Code to retry consensus download if we got one we already have.
+        - Use if-modified-since on consensus download
+        - Use if-modified-since on certificate download
         o Code to download routers listed in v3 networkstatus consensuses.
         - Enable for non-caches
       - Code to use v3 networkstatus documents once clients are

src/or/directory.c

@@ -1411,7 +1411,7 @@ connection_dir_client_reached_eof(dir_connection_t *conn)
            status_code, escaped(reason), conn->_base.address,
            conn->_base.port);
       tor_free(body); tor_free(headers); tor_free(reason);
-      /* XXXX020NMNM retry. */
+      networkstatus_consensus_download_failed(status_code);
       return -1;
     }
     log_info(LD_DIR,"Received consensus directory (size %d) from server "

src/or/networkstatus.c

@@ -45,12 +45,10 @@ static int networkstatus_list_has_changed = 0;
  * mirrors).  Clients don't use this now. */
 static time_t last_networkstatus_download_attempted = 0;
 
-/** The last time we tried to download a networkstatus, or 0 for "never".  We
- * use this to rate-limit download attempts for directory caches (including
- * mirrors).  Clients don't use this now. */
-static time_t last_consensus_networkstatus_download_attempted = 0;
 /**DOCDOC*/
 static time_t time_to_download_next_consensus = 0;
+/**DOCDOC*/
+static download_status_t consensus_dl_status = { 0, 0};
 
 /** List of strings for nicknames or fingerprints we've already warned about
  * and that are still conflicted. */ /*XXXX020 obsoleted by v3 dirs? */
@@ -644,7 +642,7 @@ routerstatus_get_by_hexdigest(const char *hexdigest)
  * network-statuses.
  */
 static void
-update_networkstatus_cache_downloads(time_t now)
+update_v2_networkstatus_cache_downloads(time_t now)
 {
   int authority = authdir_mode_v2(get_options());
   int interval =
@@ -706,7 +704,7 @@ update_networkstatus_cache_downloads(time_t now)
  * necessary".  See function comments for implementation details.
  */
 static void
-update_networkstatus_client_downloads(time_t now)
+update_v2_networkstatus_client_downloads(time_t now)
 {
   int n_live = 0, n_dirservers, n_running_dirservers, needed = 0;
   int fetch_latest = 0;
@@ -836,15 +834,21 @@ update_consensus_networkstatus_downloads(time_t now)
     return;
   if (authdir_mode_v3(options))
     return;
+  if (!download_status_is_ready(&consensus_dl_status, now, 8))
+    return; /*XXXX020 magic number 8.*/
   if (connection_get_by_type_purpose(CONN_TYPE_DIR,
                                      DIR_PURPOSE_FETCH_CONSENSUS))
     return;
-  /* XXXX020 on failure, delay until next retry. */
 
-  last_consensus_networkstatus_download_attempted = now;/*XXXX020 use this*/
   directory_get_from_dirserver(DIR_PURPOSE_FETCH_CONSENSUS,
                                ROUTER_PURPOSE_GENERAL, NULL, 1);
-  // XXXX020 time_to_download_next_consensus = put it off for a while?
+}
+
+/** DOCDOC */
+void
+networkstatus_consensus_download_failed(int status_code)
+{
+  download_status_failed(&consensus_dl_status, status_code);
 }
 
 /** DOCDOC */
@@ -888,7 +892,8 @@ should_delay_dir_fetches(or_options_t *options)
   return 0;
 }
 
-/** Launch requests for networkstatus documents as appropriate. */
+/** Launch requests for networkstatus documents and authority certificates as
+ * appropriate. */
 void
 update_networkstatus_downloads(time_t now)
 {
@@ -896,10 +901,14 @@ update_networkstatus_downloads(time_t now)
   if (should_delay_dir_fetches(options))
     return;
   if (dirserver_mode(options))
-    update_networkstatus_cache_downloads(now);
+    update_v2_networkstatus_cache_downloads(now);
   else
-    update_networkstatus_client_downloads(now);
+    update_v2_networkstatus_client_downloads(now);
   update_consensus_networkstatus_downloads(now);
+  if (consensus_waiting_for_certs)
+    authority_certs_fetch_missing(consensus_waiting_for_certs, now);
+  else
+    authority_certs_fetch_missing(current_consensus, now);
 }
 
 /** Return the network status with a given identity digest. */
@@ -978,7 +987,7 @@ networkstatus_set_current_consensus(const char *consensus, int from_cache,
                        options->DataDirectory);
           write_str_to_file(filename, consensus, 0);
         }
-        authority_certs_fetch_missing(c);
+        authority_certs_fetch_missing(c, now);
       }
       return 0;
     } else {
@@ -992,7 +1001,7 @@ networkstatus_set_current_consensus(const char *consensus, int from_cache,
 
   /* Are we missing any certificates at all? */
   if (r != 1)
-    authority_certs_fetch_missing(c);
+    authority_certs_fetch_missing(c, now);
 
   if (current_consensus)
     networkstatus_vote_free(current_consensus);

src/or/or.h

@@ -3043,10 +3043,11 @@ int tor_init(int argc, char **argv);
 
 /********************************* networkstatus.c *********************/
 
-/** How old do we allow a network-status to get before removing it
+/** How old do we allow a v2 network-status to get before removing it
  * completely? */
 #define MAX_NETWORKSTATUS_AGE (10*24*60*60)
 
+void networkstatus_consensus_download_failed(int status_code);
 void networkstatus_reset_warnings(void);
 int router_reload_networkstatus(void);
 /* for consensuses. */
@@ -3511,7 +3512,7 @@ authority_cert_t *authority_cert_get_newest_by_id(const char *id_digest);
 authority_cert_t *authority_cert_get_by_sk_digest(const char *sk_digest);
 authority_cert_t *authority_cert_get_by_digests(const char *id_digest,
                                                 const char *sk_digest);
-void authority_certs_fetch_missing(networkstatus_vote_t *status);
+void authority_certs_fetch_missing(networkstatus_vote_t *status, time_t now);
 void routerlist_add_family(smartlist_t *sl, routerinfo_t *router);
 void add_nickname_list_to_smartlist(smartlist_t *sl, const char *list,
                                     int must_be_running);

src/or/routerlist.c

@@ -97,6 +97,7 @@ trusted_dirs_reload_certs(void)
   if (!contents)
     return 0;
   r = trusted_dirs_load_certs_from_string(contents, 1);
+  log_notice(LD_DIR, "Loaded %d certs from cache.", r);
   tor_free(contents);
   return r;
 }
@@ -279,6 +280,9 @@ authority_cert_get_by_digests(const char *id_digest,
   return NULL;
 }
 
+/** How many times will we try to fetch a certificate before giving up? */
+#define MAX_CERT_DL_FAILURES 8
+
 /** Try to download any v3 authority certificates that we may be missing.  If
  * <b>status</b> is provided, try to get all the ones that were used to sign
  * <b>status</b>.  Additionally, try to have a non-expired certificate for
@@ -286,12 +290,11 @@ authority_cert_get_by_digests(const char *id_digest,
  * already have.
  **/
 void
-authority_certs_fetch_missing(networkstatus_vote_t *status)
+authority_certs_fetch_missing(networkstatus_vote_t *status, time_t now)
 {
   digestmap_t *pending = digestmap_new();
   smartlist_t *missing_digests = smartlist_create();
-  char *resource;
+  char *resource = NULL;
-  time_t now = time(NULL);
 
   list_pending_downloads(pending, DIR_PURPOSE_FETCH_CERTIFICATE, "fp/");
   if (status) {
@@ -299,9 +302,15 @@ authority_certs_fetch_missing(networkstatus_vote_t *status)
       {
         trusted_dir_server_t *ds
           = trusteddirserver_get_by_v3_auth_digest(voter->identity_digest);
-        if (ds &&
+        if (!ds)
-            !authority_cert_get_by_digests(voter->identity_digest,
+          continue;
-                                           voter->signing_key_digest))
+        if (authority_cert_get_by_digests(voter->identity_digest,
+                                          voter->signing_key_digest)) {
+          download_status_reset(&ds->cert_dl_status);
+          continue;
+        }
+        if (download_status_is_ready(&ds->cert_dl_status, now,
+                                     MAX_CERT_DL_FAILURES))
           smartlist_add(missing_digests, voter->identity_digest);
       });
   }
@@ -312,18 +321,26 @@ authority_certs_fetch_missing(networkstatus_vote_t *status)
         continue;
       if (smartlist_digest_isin(missing_digests, ds->v3_identity_digest))
         continue;
+      if (!ds->v3_certs)
+        ds->v3_certs = smartlist_create();
       SMARTLIST_FOREACH(ds->v3_certs, authority_cert_t *, cert,
         {
-          if (ftime_definitely_before(cert->expires, now)) {
+          if (!ftime_definitely_after(now, cert->expires)) {
-            /* It's definitely expired. */
+            /* It's not expired, and we weren't looking for something to
+             * verify a consensus with.  Call it done. */
+            download_status_reset(&ds->cert_dl_status);
             found = 1;
             break;
           }
         });
-      smartlist_add(missing_digests, ds->v3_identity_digest);
+      if (!found && download_status_is_ready(&ds->cert_dl_status, now,
+                                             MAX_CERT_DL_FAILURES))
+        smartlist_add(missing_digests, ds->v3_identity_digest);
     });
 
-  {
+  if (!smartlist_len(missing_digests)) {
+    goto done;
+  } else {
     smartlist_t *fps = smartlist_create();
     smartlist_add(fps, tor_strdup("fp/"));
     SMARTLIST_FOREACH(missing_digests, const char *, d, {
@@ -341,12 +358,14 @@ authority_certs_fetch_missing(networkstatus_vote_t *status)
     SMARTLIST_FOREACH(fps, char *, cp, tor_free(cp));
     smartlist_free(fps);
   }
-  log_notice(LD_DIR, "Launching request for %d missing certificates.",
+  log_notice(LD_DIR, "Launching request for %d missing certificates",
-             smartlist_len(missing_digests)); /*XXXX020 downgrade to INFO*/
+             smartlist_len(missing_digests));
-  smartlist_free(missing_digests);
   directory_get_from_dirserver(DIR_PURPOSE_FETCH_CERTIFICATE, 0,
                                resource, 1);
+
+ done:
   tor_free(resource);
+  smartlist_free(missing_digests);
   digestmap_free(pending, NULL);
 }