Add a tristate to guard against unexpected circ purpose transitions

2 files changed, 54 insertions, 0 deletions

diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c
index 9732a48c43..ddc11d0e35 100644
--- a/src/or/circuitbuild.c
+++ b/src/or/circuitbuild.c
@@ -1352,6 +1352,24 @@ pathbias_should_count(origin_circuit_t *circ)
           circ->base_.purpose == CIRCUIT_PURPOSE_S_REND_JOINED ||
           (circ->base_.purpose >= CIRCUIT_PURPOSE_C_INTRODUCING &&
            circ->base_.purpose <= CIRCUIT_PURPOSE_C_INTRODUCE_ACKED)) {
+
+    /* Check to see if the shouldcount result has changed due to a
+     * unexpected purpose change that would affect our results.
+     *
+     * The reason we check the path state too here is because for the
+     * cannibalized versions of these purposes, we count them as successful
+     * before their purpose change.
+     */
+    if (circ->pathbias_shouldcount == PATHBIAS_SHOULDCOUNT_COUNTED
+            && circ->path_state != PATH_STATE_ALREADY_COUNTED) {
+      log_info(LD_BUG,
+               "Circuit %d is now being ignored despite being counted "
+               "in the past. Purpose is %s, path state is %s",
+               circ->global_identifier,
+               circuit_purpose_to_string(circ->base_.purpose),
+               pathbias_state_to_string(circ->path_state));
+    }
+    circ->pathbias_shouldcount = PATHBIAS_SHOULDCOUNT_IGNORED;
     return 0;
   }
 
@@ -1374,9 +1392,33 @@ pathbias_should_count(origin_circuit_t *circ)
       }
       tor_fragile_assert();
     }
+
+    /* Check to see if the shouldcount result has changed due to a
+     * unexpected change that would affect our results */
+    if (circ->pathbias_shouldcount == PATHBIAS_SHOULDCOUNT_COUNTED) {
+      log_info(LD_BUG,
+               "One-hop circuit %d is now being ignored despite being counted "
+               "in the past. Purpose is %s, path state is %s",
+               circ->global_identifier,
+               circuit_purpose_to_string(circ->base_.purpose),
+               pathbias_state_to_string(circ->path_state));
+    }
+    circ->pathbias_shouldcount = PATHBIAS_SHOULDCOUNT_IGNORED;
     return 0;
   }
 
+  /* Check to see if the shouldcount result has changed due to a
+   * unexpected purpose change that would affect our results */
+  if (circ->pathbias_shouldcount == PATHBIAS_SHOULDCOUNT_IGNORED) {
+      log_info(LD_BUG,
+              "Circuit %d is now being counted despite being ignored "
+              "in the past. Purpose is %s, path state is %s",
+              circ->global_identifier,
+              circuit_purpose_to_string(circ->base_.purpose),
+              pathbias_state_to_string(circ->path_state));
+  }
+  circ->pathbias_shouldcount = PATHBIAS_SHOULDCOUNT_COUNTED;
+
   return 1;
 }
 
diff --git a/src/or/or.h b/src/or/or.h
index b5ccd0c3c7..3c962e360b 100644
--- a/src/or/or.h
+++ b/src/or/or.h
@@ -2920,6 +2920,18 @@ typedef struct origin_circuit_t {
    */
   ENUM_BF(path_state_t) path_state : 3;
 
+  /**
+   * Tristate variable to guard against pathbias miscounting
+   * due to circuit purpose transitions changing the decision
+   * of pathbias_should_count(). This variable is informational
+   * only. The current results of pathbias_should_count() are
+   * the official decision for pathbias accounting.
+   */
+  uint8_t pathbias_shouldcount;
+#define PATHBIAS_SHOULDCOUNT_UNDECIDED 0
+#define PATHBIAS_SHOULDCOUNT_IGNORED   1
+#define PATHBIAS_SHOULDCOUNT_COUNTED   2
+
   /** For path probing. Store the temporary probe stream ID
    * for response comparison */
   streamid_t pathbias_probe_id;