[tor-commits] [tor/master] Clarify state transition and related pathbias comments

[nickm at torproject.org]

commit 173ed05d2f7233371dfcb1ef32a4d95f5096c435
Author: Mike Perry <mikeperry-git at fscked.org>
Date:   Wed Jan 30 17:01:10 2013 -0400

    Clarify state transition and related pathbias comments
---
 src/or/circuitbuild.c |   10 +++++-----
 src/or/circuituse.c   |    8 +++++---
 src/or/or.h           |   27 +++++++++++++++++++--------
 3 files changed, 29 insertions(+), 16 deletions(-)

diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c
index e3a9d59..9732a48 100644
--- a/src/or/circuitbuild.c
+++ b/src/or/circuitbuild.c
@@ -1605,13 +1605,13 @@ pathbias_count_use_attempt(origin_circuit_t *circ)
 }
 
 /**
- * Check the circuit's path stat is appropriate and it as successfully
- * used.
+ * Check the circuit's path state is appropriate and mark it as
+ * successfully used. Used for path bias usage accounting.
  *
  * We don't actually increment the guard's counters until
- * pathbias_check_close().
- *
- * Used for path bias usage accounting.
+ * pathbias_check_close(), because the circuit can still transition
+ * back to PATH_STATE_USE_ATTEMPTED if a stream fails later (this
+ * is done so we can probe the circuit for liveness at close).
  */
 void
 pathbias_mark_use_success(origin_circuit_t *circ)
diff --git a/src/or/circuituse.c b/src/or/circuituse.c
index 48a7743..cfd41be 100644
--- a/src/or/circuituse.c
+++ b/src/or/circuituse.c
@@ -1498,15 +1498,17 @@ circuit_launch_by_extend_info(uint8_t purpose,
            purpose == CIRCUIT_PURPOSE_C_INTRODUCING) &&
           circ->path_state == PATH_STATE_BUILD_SUCCEEDED) {
         /* Path bias: Cannibalized rends pre-emptively count as a
-         * successfully used circ. We don't wait until the extend,
-         * because the rend point could be malicious.
+         * successfully built but unused closed circuit. We don't
+         * wait until the extend (or the close) because the rend
+         * point could be malicious.
          *
          * Same deal goes for client side introductions. Clients
          * can be manipulated to connect repeatedly to them
          * (especially web clients).
          *
          * If we decide to probe the initial portion of these circs,
-         * (up to the adversaries final hop), we need to remove this.
+         * (up to the adversary's final hop), we need to remove this,
+         * or somehow mark the circuit with a special path state.
          */
 
         /* This must be called before the purpose change */
diff --git a/src/or/or.h b/src/or/or.h
index df933c3..b5ccd0c 100644
--- a/src/or/or.h
+++ b/src/or/or.h
@@ -2827,8 +2827,18 @@ typedef struct circuit_t {
 
 /**
  * Describes the circuit building process in simplified terms based
- * on the path bias accounting state for a circuit. Created to prevent
- * overcounting due to unknown cases of circuit reuse. See Bug #6475.
+ * on the path bias accounting state for a circuit.
+ *
+ * NOTE: These state values are enumerated in the order for which we
+ * expect circuits to transition through them. If you add states,
+ * you need to preserve this overall ordering. The various pathbias
+ * state transition and accounting functions (pathbias_mark_* and
+ * pathbias_count_*) contain ordinal comparisons to enforce proper
+ * state transitions for corrections.
+ *
+ * This state machine and the associated logic was created to prevent
+ * miscounting due to unknown cases of circuit reuse. See also tickets
+ * #6475 and #7802.
  */
 typedef enum {
     /** This circuit is "new". It has not yet completed a first hop
@@ -2851,10 +2861,8 @@ typedef enum {
     /** Did any SOCKS streams or hidserv introductions actually succeed on
       * this circuit?
       *
-      * Note: If we ever implement end-to-end stream timing through test
-      * stream probes (#5707), we must *not* set this for those probes
-      * (or any other automatic streams) because the adversary could
-      * just tag at a later point.
+      * If any streams detatch/fail from this circuit, the code transitions
+      * the circuit back to PATH_STATE_USE_ATTEMPTED to ensure we probe.
       */
     PATH_STATE_USE_SUCCEEDED = 4,
 
@@ -2905,8 +2913,11 @@ typedef struct origin_circuit_t {
    * cannibalized circuits. */
   unsigned int has_opened : 1;
 
-  /** Kludge to help us prevent the warn in bug #6475 and eventually
-   * debug why we are not seeing first hops in some cases. */
+  /**
+   * Path bias state machine. Used to ensure integrity of our
+   * circuit building and usage accounting. See path_state_t
+   * for more details.
+   */
   ENUM_BF(path_state_t) path_state : 3;
 
   /** For path probing. Store the temporary probe stream ID