[tor-commits] [tor/master] Allow any valid 'end' cell to mean a circuit was used successfully.

[nickm at torproject.org]

commit 686fc222593fd46ec82d62f0fa62ca02900c1014
Author: Mike Perry <mikeperry-git at fscked.org>
Date:   Sat Dec 8 16:37:22 2012 -0800

    Allow any valid 'end' cell to mean a circuit was used successfully.
    
    Also improve some log messages.
---
 src/or/circuitbuild.c    |    9 ++++++---
 src/or/connection_edge.c |   17 +++++++++++------
 src/or/relay.c           |    8 ++++++++
 3 files changed, 25 insertions(+), 9 deletions(-)

diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c
index 9b1236f..c3a5827 100644
--- a/src/or/circuitbuild.c
+++ b/src/or/circuitbuild.c
@@ -1411,16 +1411,18 @@ pathbias_check_close(origin_circuit_t *ocirc, int reason)
       // state.. Can we use that? Does optimistic data change this?
 
       log_info(LD_CIRC,
-            "Circuit closed without successful use for reason %d. "
+            "Circuit %d closed without successful use for reason %d. "
             "Circuit purpose %d currently %s.",
+            ocirc->global_identifier,
             reason, circ->purpose, circuit_state_to_string(circ->state));
       pathbias_count_unusable(ocirc);
     } else {
       if (reason & END_CIRC_REASON_FLAG_REMOTE) {
         /* Unused remote circ close reasons all could be bias */
         log_info(LD_CIRC,
-            "Circuit remote-closed without successful use for reason %d. "
+            "Circuit %d remote-closed without successful use for reason %d. "
             "Circuit purpose %d currently %s.",
+            ocirc->global_identifier,
             reason, circ->purpose, circuit_state_to_string(circ->state));
         pathbias_count_collapse(ocirc);
       } else if ((reason & ~END_CIRC_REASON_FLAG_REMOTE)
@@ -1432,8 +1434,9 @@ pathbias_check_close(origin_circuit_t *ocirc, int reason)
         /* FIXME: Only count bias if the network is live?
          * What about clock jumps/suspends? */
         log_info(LD_CIRC,
-            "Circuit's channel closed without successful use for reason %d, "
+            "Circuit %d's channel closed without successful use for reason %d, "
             "channel reason %d. Circuit purpose %d currently %s.",
+            ocirc->global_identifier,
             reason, circ->n_chan->reason_for_closing,
             circ->purpose, circuit_state_to_string(circ->state));
         pathbias_count_collapse(ocirc);
diff --git a/src/or/connection_edge.c b/src/or/connection_edge.c
index 79bb54c..ca6060c 100644
--- a/src/or/connection_edge.c
+++ b/src/or/connection_edge.c
@@ -2178,14 +2178,19 @@ connection_ap_handshake_socks_reply(entry_connection_t *conn, char *reply,
 
   /* Flag this stream's circuit as having completed a stream successfully
    * (for path bias) */
-  if (status == SOCKS5_SUCCEEDED) {
+  if (status == SOCKS5_SUCCEEDED ||
+      endreason == END_STREAM_REASON_RESOLVEFAILED ||
+      endreason == END_STREAM_REASON_CONNECTREFUSED ||
+      endreason == END_STREAM_REASON_CONNRESET ||
+      endreason == END_STREAM_REASON_NOROUTE ||
+      endreason == END_STREAM_REASON_RESOURCELIMIT) {
     if(!conn->edge_.on_circuit ||
        !CIRCUIT_IS_ORIGIN(conn->edge_.on_circuit)) {
-      // XXX: Weird. We hit this a lot, and yet have no unusable_circs.
-      // Maybe during addrmaps/resolves?
-      log_warn(LD_BUG,
-               "(Harmless.) No origin circuit for successful SOCKS stream. "
-               "Reason: %d", endreason);
+      // DNS remaps can trigger this. So can failed hidden service
+      // lookups.
+      log_info(LD_BUG,
+               "(Harmless.) No origin circuit for successful SOCKS stream %ld. "
+               "Reason: %d", ENTRY_TO_CONN(conn)->global_identifier, endreason);
     } else {
       TO_ORIGIN_CIRCUIT(conn->edge_.on_circuit)->path_state
           = PATH_STATE_USE_SUCCEEDED;
diff --git a/src/or/relay.c b/src/or/relay.c
index 7f49299..1638dae 100644
--- a/src/or/relay.c
+++ b/src/or/relay.c
@@ -693,6 +693,14 @@ connection_ap_process_end_not_open(
   edge_connection_t *edge_conn = ENTRY_TO_EDGE_CONN(conn);
   (void) layer_hint; /* unused */
 
+  if (rh->length > 0) {
+    /* Path bias: If we get a valid reason code from the exit, 
+     * it wasn't due to tagging */
+    // XXX: This relies on recognized+digest being strong enough not
+    // to be spoofable.. Is that a valid assumption?
+    circ->path_state = PATH_STATE_USE_SUCCEEDED;
+  }
+
   if (rh->length > 0 && edge_reason_is_retriable(reason) &&
       /* avoid retry if rend */
       !connection_edge_is_rendezvous_stream(edge_conn)) {