karsten's patch for bug 767.

svn:r16808

ChangeLog

@@ -3,6 +3,14 @@ Changes in version 0.2.1.6-alpha - 2008-09-xx
     - Fix a bug when parsing ports in tor_addr_port_parse() that caused
       Tor to fail to start if you had it configured to use a bridge
       relay. Fixes bug 809. Bugfix on 0.2.1.5-alpha.
+    - When extending a circuit to a hidden service directory to upload a
+      rendezvous descriptor using a BEGIN_DIR cell, almost 1/6 of all
+      requests failed, because the router descriptor has not been downloaded
+      before. In these cases, do not attempt to upload the rendezvous
+      descriptor, but wait until the router descriptor is downloaded and
+      retry. Likewise, do not attempt to fetch a rendezvous from a hidden
+      service directory for which the router descriptor has not been
+      downloaded, yet. Fixes bug 767. Bugfix on 0.2.0.10-alpha.
 
   o Minor bugfixes:
     - Fix compile on OpenBSD 4.4-current. Bugfix on 0.2.1.5-alpha.

src/or/circuituse.c

@@ -1321,6 +1321,11 @@ connection_ap_handshake_attach_circuit(edge_connection_t *conn)
       routerinfo_t *router = router_get_by_nickname(conn->chosen_exit_name, 1);
       int opt = conn->_base.chosen_exit_optional;
       if (!router && !want_onehop) {
+        /* We ran into this warning when trying to extend a circuit to a
+         * hidden service directory for which we didn't have a router
+         * descriptor. See flyspray task 767 for more details. We should
+         * keep this in mind when deciding to use BEGIN_DIR cells for other
+         * directory requests as well. -KL*/
         log_fn(opt ? LOG_INFO : LOG_WARN, LD_APP,
                "Requested exit point '%s' is not known. %s.",
                conn->chosen_exit_name, opt ? "Trying others" : "Closing");

src/or/main.c

@@ -1126,8 +1126,10 @@ run_scheduled_events(time_t now)
   circuit_close_all_marked();
 
   /** 7. And upload service descriptors if necessary. */
-  if (has_completed_circuit && !we_are_hibernating())
+  if (has_completed_circuit && !we_are_hibernating()) {
+    rend_consider_descriptor_republication();
     rend_consider_services_upload(now);
+  }
 
   /** 8. and blow away any connections that need to die. have to do this now,
    * because if we marked a conn for close and left its socket -1, then

src/or/or.h

@@ -3898,6 +3898,13 @@ typedef struct rend_service_descriptor_t {
   /** List of the service's introduction points.  Elements are removed if
    * introduction attempts fail. */
   smartlist_t *intro_nodes;
+  /** Has descriptor been uploaded to all hidden service directories? */
+  int all_uploads_performed;
+  /** List of hidden service directories to which an upload request for
+   * this descriptor could be sent. Smartlist exists only when at least one
+   * of the previous upload requests failed (otherwise it's not important
+   * to know which uploads succeeded and which not). */
+  smartlist_t *successful_uploads;
 } rend_service_descriptor_t;
 
 int rend_cmp_service_ids(const char *one, const char *two);
@@ -3961,6 +3968,8 @@ int rend_service_load_keys(void);
 void rend_services_init(void);
 void rend_services_introduce(void);
 void rend_consider_services_upload(time_t now);
+void rend_hsdir_routers_changed(void);
+void rend_consider_descriptor_republication(void);
 
 void rend_service_intro_has_opened(origin_circuit_t *circuit);
 int rend_service_intro_established(origin_circuit_t *circuit,

src/or/rendclient.c

@@ -354,12 +354,13 @@ directory_get_from_hs_dir(const char *desc_id, const char *query)
                 desc_id, DIGEST_LEN);
 
   /* Only select those hidden service directories to which we did not send
-   * a request recently. */
+   * a request recently and for which we have a router descriptor here. */
   directory_clean_last_hid_serv_requests(); /* Clean request history first. */
 
   SMARTLIST_FOREACH(responsible_dirs, routerstatus_t *, dir, {
     if (lookup_last_hid_serv_request(dir, desc_id_base32, 0, 0) +
-        REND_HID_SERV_DIR_REQUERY_PERIOD >= now)
+            REND_HID_SERV_DIR_REQUERY_PERIOD >= now ||
+        !router_get_by_digest(dir->identity_digest))
       SMARTLIST_DEL_CURRENT(responsible_dirs, dir);
   });
 

src/or/rendcommon.c

@@ -32,6 +32,10 @@ rend_service_descriptor_free(rend_service_descriptor_t *desc)
       rend_intro_point_free(intro););
     smartlist_free(desc->intro_nodes);
   }
+  if (desc->successful_uploads) {
+    SMARTLIST_FOREACH(desc->successful_uploads, char *, c, tor_free(c););
+    smartlist_free(desc->successful_uploads);
+  }
   tor_free(desc);
 }
 

src/or/rendservice.c

@@ -1358,11 +1358,13 @@ find_intro_circuit(rend_intro_point_t *intro, const char *pk_digest,
  * <b>service_id</b> and <b>seconds_valid</b> are only passed for logging
  * purposes. */
 static void
-directory_post_to_hs_dir(smartlist_t *descs, const char *service_id,
+directory_post_to_hs_dir(rend_service_descriptor_t *renddesc,
+                         smartlist_t *descs, const char *service_id,
                          int seconds_valid)
 {
-  int i, j;
+  int i, j, failed_upload = 0;
   smartlist_t *responsible_dirs = smartlist_create();
+  smartlist_t *successful_uploads = smartlist_create();
   routerstatus_t *hs_dir;
   for (i = 0; i < smartlist_len(descs); i++) {
     rend_encoded_v2_service_descriptor_t *desc = smartlist_get(descs, i);
@@ -1372,11 +1374,24 @@ directory_post_to_hs_dir(smartlist_t *descs, const char *service_id,
       log_warn(LD_REND, "Could not determine the responsible hidden service "
                         "directories to post descriptors to.");
       smartlist_free(responsible_dirs);
+      smartlist_free(successful_uploads);
       return;
     }
     for (j = 0; j < smartlist_len(responsible_dirs); j++) {
       char desc_id_base32[REND_DESC_ID_V2_LEN_BASE32 + 1];
       hs_dir = smartlist_get(responsible_dirs, j);
+      if (smartlist_digest_isin(renddesc->successful_uploads,
+                                hs_dir->identity_digest))
+        /* Don't upload descriptor if we succeeded in doing so last time. */
+        continue;
+      if (!router_get_by_digest(hs_dir->identity_digest)) {
+        log_info(LD_REND, "Not sending publish request for v2 descriptor to "
+                          "hidden service directory '%s'; we don't have its "
+                          "router descriptor. Queueing for later upload.",
+                 hs_dir->nickname);
+        failed_upload = -1;
+        continue;
+      }
       /* Send publish request. */
       directory_initiate_command_routerstatus(hs_dir,
                                               DIR_PURPOSE_UPLOAD_RENDDESC_V2,
@@ -1394,10 +1409,33 @@ directory_post_to_hs_dir(smartlist_t *descs, const char *service_id,
                seconds_valid,
                hs_dir->nickname,
                hs_dir->dir_port);
+      /* Remember successful upload to this router for next time. */
+      if (!smartlist_digest_isin(successful_uploads, hs_dir->identity_digest))
+        smartlist_add(successful_uploads, hs_dir->identity_digest);
     }
     smartlist_clear(responsible_dirs);
   }
+  if (!failed_upload) {
+    if (renddesc->successful_uploads) {
+      SMARTLIST_FOREACH(renddesc->successful_uploads, char *, c, tor_free(c););
+      smartlist_free(renddesc->successful_uploads);
+    }
+    renddesc->all_uploads_performed = -1;
+  } else {
+    /* Remember which routers worked this time, so that we don't upload the
+     * descriptor to them again. */
+    if (!renddesc->successful_uploads)
+      renddesc->successful_uploads = smartlist_create();
+    SMARTLIST_FOREACH(successful_uploads, char *, c, {
+      if (!smartlist_digest_isin(renddesc->successful_uploads, c)) {
+        char *hsdir_id = tor_malloc_zero(DIGEST_LEN);
+        memcpy(hsdir_id, c, DIGEST_LEN);
+        smartlist_add(renddesc->successful_uploads, hsdir_id);
+      }
+    });
+  }
   smartlist_free(responsible_dirs);
+  smartlist_free(successful_uploads);
 }
 
 /** Encode and sign up-to-date v0 and/or v2 service descriptors for
@@ -1412,9 +1450,6 @@ upload_service_descriptor(rend_service_t *service)
   char serviceid[REND_SERVICE_ID_LEN_BASE32+1];
   int uploaded = 0;
 
-  /* Update the descriptor. */
-  rend_service_update_descriptor(service);
-
   rendpostperiod = get_options()->RendPostPeriod;
 
   /* Upload unversioned (v0) descriptor? */
@@ -1491,7 +1526,8 @@ upload_service_descriptor(rend_service_t *service)
         rend_get_service_id(service->desc->pk, serviceid);
         log_info(LD_REND, "Sending publish request for hidden service %s",
                      serviceid);
-        directory_post_to_hs_dir(descs, serviceid, seconds_valid);
+        directory_post_to_hs_dir(service->desc, descs, serviceid,
+                                 seconds_valid);
         /* Free memory for descriptors. */
         for (i = 0; i < smartlist_len(descs); i++)
           rend_encoded_v2_service_descriptor_free(smartlist_get(descs, i));
@@ -1519,7 +1555,8 @@ upload_service_descriptor(rend_service_t *service)
             smartlist_free(client_cookies);
             return;
           }
-          directory_post_to_hs_dir(descs, serviceid, seconds_valid);
+          directory_post_to_hs_dir(service->desc, descs, serviceid,
+                                   seconds_valid);
           /* Free memory for descriptors. */
           for (i = 0; i < smartlist_len(descs); i++)
             rend_encoded_v2_service_descriptor_free(smartlist_get(descs, i));
@@ -1695,6 +1732,47 @@ rend_consider_services_upload(time_t now)
       /* if it's time, or if the directory servers have a wrong service
        * descriptor and ours has been stable for 30 seconds, upload a
        * new one of each format. */
+      rend_service_update_descriptor(service);
+      upload_service_descriptor(service);
+    }
+  }
+}
+
+/** True if the list of available router descriptors might have changed so
+ * that we should have a look whether we can republish previously failed
+ * rendezvous service descriptors. */
+static int consider_republishing_rend_descriptors = 1;
+
+/** Called when our internal view of the directory has changed, so that we
+ * might have router descriptors of hidden service directories available that
+ * we did not have before. */
+void
+rend_hsdir_routers_changed(void)
+{
+  consider_republishing_rend_descriptors = 1;
+}
+
+/** Consider republication of rendezvous service descriptors that failed
+ * previously, but without regenerating descriptor contents.
+ */
+void
+rend_consider_descriptor_republication(void)
+{
+  int i;
+  rend_service_t *service;
+
+  if (!consider_republishing_rend_descriptors)
+    return;
+  consider_republishing_rend_descriptors = 0;
+
+  if (!get_options()->PublishHidServDescriptors)
+    return;
+
+  for (i=0; i < smartlist_len(rend_service_list); ++i) {
+    service = smartlist_get(rend_service_list, i);
+    if (service->desc && !service->desc->all_uploads_performed) {
+      /* If we failed in uploading a descriptor last time, try again *without*
+       * updating the descriptor's contents. */
       upload_service_descriptor(service);
     }
   }

src/or/routerlist.c

@@ -4254,6 +4254,7 @@ void
 router_dir_info_changed(void)
 {
   need_to_update_have_min_dir_info = 1;
+  rend_hsdir_routers_changed();
 }
 
 /** Return a string describing what we're missing before we have enough