commit 173ed05d2f7233371dfcb1ef32a4d95f5096c435 Author: Mike Perry <mikeperry-git@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