[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]

Re: : Export END_CIRC_REASON_* to controler



Thus spake Roger Dingledine (arma@xxxxxxx):

> On Fri, Oct 13, 2006 at 02:57:02AM -0500, Mike Perry wrote:
> > > Probably, we should define more reasons; see some of the comments I
> > > did for the patch in r8672 for other reasons that don't match up with
> > > what the spec seems to say.
> > 
> > Actually, do you want me to do this?
> 
> Hi Mike,
> 
> That would be great. Step one as usual is a spec patch, but I'm
> guessing in this case it would make sense to patch the spec and
> the code in tandem.

Ok, here you are. I think these reasons clarify things quite a bit.

In reference to my post to Paul, the only reasons I currently consider
OK are "REQUESTED", "FINISHED", and "ORIGIN" (which should not happen
any more). As far as I know, there is now no way to spoof these
reasons to the controller. They happen only when Tor or the controller
have decided to close a circuit under normal conditions.

-- 
Mike Perry
Mad Computer Scientist
fscked.org evil labs
Index: src/or/circuitlist.c
===================================================================
--- src/or/circuitlist.c	(revision 8701)
+++ src/or/circuitlist.c	(working copy)
@@ -780,7 +780,7 @@
     if (CIRCUIT_IS_ORIGIN(circ) &&
         !circ->marked_for_close &&
         !circ->timestamp_dirty)
-      circuit_mark_for_close(circ, END_CIRC_REASON_REQUESTED);
+      circuit_mark_for_close(circ, END_CIRC_REASON_FINISHED);
   }
 }
 
Index: src/or/rendservice.c
===================================================================
--- src/or/rendservice.c	(revision 8701)
+++ src/or/rendservice.c	(working copy)
@@ -738,8 +738,7 @@
   if (!service) {
     log_warn(LD_REND, "Unrecognized service ID %s on introduction circuit %d.",
              serviceid, circuit->_base.n_circ_id);
-    /* XXXX Add a no-such-servicer reason? */
-    reason = END_CIRC_REASON_CONNECTFAILED;
+    reason = END_CIRC_REASON_NOSUCHSERVICE;
     goto err;
   }
 
Index: src/or/control.c
===================================================================
--- src/or/control.c	(revision 8701)
+++ src/or/control.c	(working copy)
@@ -2825,6 +2825,16 @@
       return "REASON=OR_IDENTITY";
     case END_CIRC_REASON_OR_CONN_CLOSED:
       return "REASON=OR_CONN_CLOSED";
+    case END_CIRC_REASON_FINISHED:
+      return "REASON=FINISHED";
+    case END_CIRC_REASON_TIMEOUT:
+      return "REASON=TIMEOUT";
+    case END_CIRC_REASON_DESTROYED:
+      return "REASON=DESTROYED";
+    case END_CIRC_REASON_NOPATH:
+      return "REASON=NOPATH";
+    case END_CIRC_REASON_NOSUCHSERVICE:
+      return "REASON=NOSUCHSERVICE";
     default:
       log_warn(LD_BUG, "Unrecognized reason code %d", (int)reason);
       return NULL;
Index: src/or/or.h
===================================================================
--- src/or/or.h	(revision 8701)
+++ src/or/or.h	(working copy)
@@ -505,7 +505,12 @@
 #define END_CIRC_REASON_CONNECTFAILED   6
 #define END_CIRC_REASON_OR_IDENTITY     7
 #define END_CIRC_REASON_OR_CONN_CLOSED  8
-#define _END_CIRC_REASON_MAX            8
+#define END_CIRC_REASON_FINISHED        9
+#define END_CIRC_REASON_TIMEOUT         10
+#define END_CIRC_REASON_DESTROYED       11
+#define END_CIRC_REASON_NOPATH          12
+#define END_CIRC_REASON_NOSUCHSERVICE   13
+#define _END_CIRC_REASON_MAX            13
 
 /** Length of 'y' portion of 'y.onion' URL. */
 #define REND_SERVICE_ID_LEN 16
Index: src/or/command.c
===================================================================
--- src/or/command.c	(revision 8701)
+++ src/or/command.c	(working copy)
@@ -379,18 +379,12 @@
     circuit_set_n_circid_orconn(circ, 0, NULL);
     if (CIRCUIT_IS_ORIGIN(circ)) {
       /* Prevent arbitrary destroys from going unnoticed by controller */
-      /* XXXX Not quite right; what we want is to tell the controller the
-       *      exact reason that we were asked to close, but tell it that we
-       *      closed because we were asked. Anything else is not accurate.
-       *      OR_CONN_CLOSED is certainly wrong, since a destroy doesn't mean
-       *      that the underlying OR connection got closed. -NM */
-#if 0
       if (reason == END_CIRC_AT_ORIGIN ||
           reason == END_CIRC_REASON_NONE ||
+          reason == END_CIRC_REASON_FINISHED ||
           reason == END_CIRC_REASON_REQUESTED) {
-        reason = END_CIRC_REASON_OR_CONN_CLOSED;
+        reason = END_CIRC_REASON_DESTROYED;
       }
-#endif
       circuit_mark_for_close(circ, reason);
     } else {
       char payload[1];
Index: src/or/circuituse.c
===================================================================
--- src/or/circuituse.c	(revision 8701)
+++ src/or/circuituse.c	(working copy)
@@ -265,8 +265,7 @@
                circuit_state_to_string(victim->state), victim->purpose);
 
     circuit_log_path(LOG_INFO,LD_CIRC,TO_ORIGIN_CIRCUIT(victim));
-    /* XXXX Should there be a timeout reason? CONNECTFAILED isn't right. */
-    circuit_mark_for_close(victim, END_CIRC_REASON_CONNECTFAILED);
+    circuit_mark_for_close(victim, END_CIRC_REASON_TIMEOUT);
   }
 }
 
@@ -584,8 +583,7 @@
       log_debug(LD_CIRC, "Closing n_circ_id %d (dirty %d secs ago, purp %d)",
                 circ->n_circ_id, (int)(now - circ->timestamp_dirty),
                 circ->purpose);
-      /* XXXX Should there be a timeout reason? REQUESTED isn't right. */
-      circuit_mark_for_close(circ, END_CIRC_REASON_REQUESTED);
+      circuit_mark_for_close(circ, END_CIRC_REASON_FINISHED);
     } else if (!circ->timestamp_dirty &&
                circ->state == CIRCUIT_STATE_OPEN &&
                circ->purpose == CIRCUIT_PURPOSE_C_GENERAL) {
@@ -593,8 +591,7 @@
         log_debug(LD_CIRC,
                   "Closing circuit that has been unused for %d seconds.",
                   (int)(now - circ->timestamp_created));
-        /* XXXX Should there be a timeout reason? REQUESTED isn't right. */
-        circuit_mark_for_close(circ, END_CIRC_REASON_REQUESTED);
+        circuit_mark_for_close(circ, END_CIRC_REASON_FINISHED);
       }
     }
   }
Index: src/or/circuitbuild.c
===================================================================
--- src/or/circuitbuild.c	(revision 8701)
+++ src/or/circuitbuild.c	(working copy)
@@ -305,8 +305,7 @@
 
   if (onion_pick_cpath_exit(circ, info) < 0 ||
       onion_populate_cpath(circ) < 0) {
-    /* XXX should there be a 'couldn't build a path' reason? */
-    circuit_mark_for_close(TO_CIRCUIT(circ), END_CIRC_REASON_INTERNAL);
+    circuit_mark_for_close(TO_CIRCUIT(circ), END_CIRC_REASON_NOPATH);
     return NULL;
   }
 
Index: src/or/rendmid.c
===================================================================
--- src/or/rendmid.c	(revision 8701)
+++ src/or/rendmid.c	(working copy)
@@ -90,7 +90,7 @@
   while ((c = circuit_get_intro_point(pk_digest))) {
     log_info(LD_REND, "Replacing old circuit for service %s",
              safe_str(serviceid));
-    circuit_mark_for_close(TO_CIRCUIT(c), END_CIRC_REASON_REQUESTED);
+    circuit_mark_for_close(TO_CIRCUIT(c), END_CIRC_REASON_FINISHED);
     /* Now it's marked, and it won't be returned next time. */
   }
 
Index: src/or/rendclient.c
===================================================================
--- src/or/rendclient.c	(revision 8701)
+++ src/or/rendclient.c	(working copy)
@@ -211,7 +211,7 @@
     }
     /* close the circuit: we won't need it anymore. */
     circ->_base.purpose = CIRCUIT_PURPOSE_C_INTRODUCE_ACKED;
-    circuit_mark_for_close(TO_CIRCUIT(circ), END_CIRC_AT_ORIGIN);
+    circuit_mark_for_close(TO_CIRCUIT(circ), END_CIRC_REASON_FINISHED);
   } else {
     /* It's a NAK; the introduction point didn't relay our request. */
     circ->_base.purpose = CIRCUIT_PURPOSE_C_INTRODUCING;
Index: doc/control-spec.txt
===================================================================
--- doc/control-spec.txt	(revision 8701)
+++ doc/control-spec.txt	(working copy)
@@ -765,7 +765,8 @@
 
       Reason = "NONE" / "TORPROTOCOL" / "INTERNAL" / "REQUESTED" /
                "HIBERNATING" / "RESOURCELIMIT" / "CONNECTFAILED" /
-               "OR_IDENTITY" / "OR_CONN_CLOSED"
+               "OR_IDENTITY" / "OR_CONN_CLOSED" / "TIMEOUT" / 
+               "FINISHED" / "DESTROYED" / "NOPATH" / "NOSUCHSERVICE"
 
    The path is provided only when the circuit has been extended at least one
    hop.
@@ -774,7 +775,7 @@
    if extended events are enabled (see 3.19).  Clients MUST accept reasons
    not listed above.
 
-   [XXXX Explain what the reasons mean.]
+   [XXXX Explain what the reasons mean. See tor-spec.txt for now.]
 
 4.1.2. Stream status changed
 
Index: doc/tor-spec.txt
===================================================================
--- doc/tor-spec.txt	(revision 8701)
+++ doc/tor-spec.txt	(working copy)
@@ -563,6 +563,11 @@
                            as expected.)
      8 -- OR_CONN_CLOSED  (The OR connection that was carrying this circuit
                            died.)
+     9 -- FINISHED        (The circuit is old and/or dirty)  
+    10 -- TIMEOUT         (Circuit construction took too long)  
+    11 -- DESTROYED       (The circuit was destroyed w/o client TRUNCATE)
+    12 -- NOPATH          (Internal only: not enough nodes to make circuit)
+    13 -- NOSUCHSERVICE   (Request for unknown hidden service)
 
    [Versions of Tor prior to 0.1.0.11 didn't send reasons; implementations
    MUST accept empty TRUNCATED and DESTROY cells.]