Browse Source

Big bridge bugfixes. Backport candidates.
If you have more than one bridge but don't know their keys,
you would only learn a request for the descriptor of the first one
on your list. (Tor considered launching requests for the others, but
found that it already had a connection on the way for $0000...0000
so it didn't open another.)
If you have more than one bridge but don't know their keys, and the
connection to one of the bridges failed, you would cancel all
pending bridge connections. (After all, they all have the same
digest.)


svn:r15366

Roger Dingledine 16 years ago
parent
commit
dd50ffb1de
5 changed files with 50 additions and 15 deletions
  1. 12 1
      ChangeLog
  2. 3 2
      src/or/circuitbuild.c
  3. 14 5
      src/or/circuituse.c
  4. 19 6
      src/or/connection_edge.c
  5. 2 1
      src/or/or.h

+ 12 - 1
ChangeLog

@@ -1,9 +1,20 @@
-Changes in version 0.2.1.2-alpha - 2008-??-??
+Changes in version 0.2.1.2-alpha - 2008-06-??
   o Major features:
     - New TestingTorNetwork option to allow adjustment of previously constant
       values that, while reasonable, could slow bootstrapping.  Implements
       proposal 135.  Patch from Karsten.
 
+  o Major bugfixes:
+    - If you have more than one bridge but don't know their keys,
+      you would only learn a request for the descriptor of the first one
+      on your list. (Tor considered launching requests for the others, but
+      found that it already had a connection on the way for $0000...0000
+      so it didn't open another.)
+    - If you have more than one bridge but don't know their keys, and the
+      connection to one of the bridges failed, you would cancel all
+      pending bridge connections. (After all, they all have the same
+      digest.)
+
   o Minor features:
     - Allow OpenSSL to use dynamic locks if it wants.
     - When building a consensus do not include routers that are down.

+ 3 - 2
src/or/circuitbuild.c

@@ -414,8 +414,9 @@ circuit_n_conn_done(or_connection_t *or_conn, int status)
   smartlist_t *pending_circs;
   int err_reason = 0;
 
-  log_debug(LD_CIRC,"or_conn to %s, status=%d",
-            or_conn->nickname ? or_conn->nickname : "NULL", status);
+  log_debug(LD_CIRC,"or_conn to %s/%s, status=%d",
+            or_conn->nickname ? or_conn->nickname : "NULL",
+            or_conn->_base.address, status);
 
   pending_circs = smartlist_create();
   circuit_get_all_pending_on_or_conn(pending_circs, or_conn);

+ 14 - 5
src/or/circuituse.c

@@ -95,10 +95,19 @@ circuit_is_acceptable(circuit_t *circ, edge_connection_t *conn,
       tor_assert(conn->chosen_exit_name);
       if (build_state->chosen_exit) {
         char digest[DIGEST_LEN];
-        if (hexdigest_to_digest(conn->chosen_exit_name, digest) < 0 ||
-            memcmp(digest, build_state->chosen_exit->identity_digest,
-                   DIGEST_LEN))
+        if (hexdigest_to_digest(conn->chosen_exit_name, digest) < 0)
+          return 0; /* broken digest, we don't want it */
+        if (memcmp(digest, build_state->chosen_exit->identity_digest,
+                          DIGEST_LEN))
           return 0; /* this is a circuit to somewhere else */
+        if (tor_digest_is_zero(digest)) {
+          /* we don't know the digest; have to compare addr:port */
+          struct in_addr in;
+          if (!tor_inet_aton(conn->socks_request->address, &in) ||
+              build_state->chosen_exit->addr != ntohl(in.s_addr) ||
+              build_state->chosen_exit->port != conn->socks_request->port)
+            return 0;
+        }
       }
     } else {
       if (conn->want_onehop) {
@@ -749,7 +758,7 @@ circuit_build_failed(origin_circuit_t *circ)
     }
     /* if there are any one-hop streams waiting on this circuit, fail
      * them now so they can retry elsewhere. */
-    connection_ap_fail_onehop(circ->_base.n_conn_id_digest);
+    connection_ap_fail_onehop(circ->_base.n_conn_id_digest, circ->build_state);
   }
 
   switch (circ->_base.purpose) {
@@ -1330,7 +1339,7 @@ connection_ap_handshake_attach_circuit(edge_connection_t *conn)
     /* find the circuit that we should use, if there is one. */
     retval = circuit_get_open_circ_or_launch(
         conn, CIRCUIT_PURPOSE_C_GENERAL, &circ);
-    if (retval < 1)
+    if (retval < 1) // XXX021 if we totally fail, this still returns 0 -RD
       return retval;
 
     log_debug(LD_APP|LD_CIRC,

+ 19 - 6
src/or/connection_edge.c

@@ -466,7 +466,8 @@ connection_ap_attach_pending(void)
  * onehop streams to circ->p_streams so they get marked in
  * circuit_mark_for_close like normal p_streams. */
 void
-connection_ap_fail_onehop(const char *failed_digest)
+connection_ap_fail_onehop(const char *failed_digest,
+                          cpath_build_state_t *build_state)
 {
   edge_connection_t *edge_conn;
   char digest[DIGEST_LEN];
@@ -480,12 +481,24 @@ connection_ap_fail_onehop(const char *failed_digest)
     edge_conn = TO_EDGE_CONN(conn);
     if (!edge_conn->want_onehop)
       continue;
-    if (!hexdigest_to_digest(edge_conn->chosen_exit_name, digest) &&
-        !memcmp(digest, failed_digest, DIGEST_LEN)) {
-      log_info(LD_APP, "Closing onehop stream to '%s' because the OR conn "
-                       "just failed.", edge_conn->chosen_exit_name);
-      connection_mark_unattached_ap(edge_conn, END_STREAM_REASON_TIMEOUT);
+    if (hexdigest_to_digest(edge_conn->chosen_exit_name, digest) < 0 ||
+        memcmp(digest, failed_digest, DIGEST_LEN))
+      continue;
+    (void)build_state;
+    if (tor_digest_is_zero(digest)) {
+      /* we don't know the digest; have to compare addr:port */
+      struct in_addr in;
+      if (!build_state || !build_state->chosen_exit ||
+          !edge_conn->socks_request || !edge_conn->socks_request->address ||
+          !tor_inet_aton(edge_conn->socks_request->address, &in) ||
+          build_state->chosen_exit->addr != ntohl(in.s_addr) ||
+          build_state->chosen_exit->port != edge_conn->socks_request->port)
+        continue;
     }
+    log_info(LD_APP, "Closing onehop stream to '%s/%s' because the OR conn "
+                     "just failed.", edge_conn->chosen_exit_name,
+                     edge_conn->socks_request->address);
+    connection_mark_unattached_ap(edge_conn, END_STREAM_REASON_TIMEOUT);
   });
 }
 

+ 2 - 1
src/or/or.h

@@ -2875,7 +2875,8 @@ int connection_edge_is_rendezvous_stream(edge_connection_t *conn);
 int connection_ap_can_use_exit(edge_connection_t *conn, routerinfo_t *exit);
 void connection_ap_expire_beginning(void);
 void connection_ap_attach_pending(void);
-void connection_ap_fail_onehop(const char *failed_digest);
+void connection_ap_fail_onehop(const char *failed_digest,
+                               cpath_build_state_t *build_state);
 void circuit_discard_optional_exit_enclaves(extend_info_t *info);
 int connection_ap_detach_retriable(edge_connection_t *conn,
                                    origin_circuit_t *circ,