Merge branch 'maint-0.2.3' into release-0.2.3

33 files changed, 660 insertions, 174 deletions

diff --git a/changes/bug6244_part_c b/changes/bug6244_part_c
new file mode 100644
index 0000000000..dea6e7b69e
--- /dev/null
+++ b/changes/bug6244_part_c
@@ -0,0 +1,6 @@
+  o Major bugfixes (controller):
+    - Make wildcarded addresses (that is, ones beginning with *.) work when
+      provided via the controller's MapAddress command.  Previously, they
+      were accepted, but we never actually noticed that they were wildcards.
+      Fix for bug 6244; bugfix on 0.2.3.9-alpha.
+
diff --git a/changes/bug6251 b/changes/bug6251
new file mode 100644
index 0000000000..c782a93e49
--- /dev/null
+++ b/changes/bug6251
@@ -0,0 +1,6 @@
+  o Minor bugfixes:
+    - Downgrade "set buildtimeout to low value" messages to INFO
+      severity; they were never an actual problem, there was never
+      anything reasonable to do about them, and they tended to spam
+      logs from time to time. Fix for bug 6251; bugfix on
+      0.2.2.2-alpha.
\ No newline at end of file
diff --git a/changes/bug6252_again b/changes/bug6252_again
new file mode 100644
index 0000000000..f7fd00cb38
--- /dev/null
+++ b/changes/bug6252_again
@@ -0,0 +1,11 @@
+  o Security fixes:
+    - Tear down the circuit if we get an unexpected SENDME cell. Clients
+      could use this trick to make their circuits receive cells faster
+      than our flow control would have allowed, or to gum up the network,
+      or possibly to do targeted memory denial-of-service attacks on
+      entry nodes. Fixes bug 6252. Bugfix on the 54th commit on Tor --
+      from July 2002, before the release of Tor 0.0.0.  We had committed
+      this patch previously, but we had to revert it because of bug 6271.
+      Now that 6271 is fixed, this appears to work.
+
+
diff --git a/changes/bug6379 b/changes/bug6379
new file mode 100644
index 0000000000..1f2b6941cd
--- /dev/null
+++ b/changes/bug6379
@@ -0,0 +1,6 @@
+  o Minor bugfixes:
+    - Fix build warnings from --enable-openbsd-malloc with gcc warnings
+      enabled. Fixes bug 6379.
+    - Fix 64-bit warnings from --enable-openbsd-malloc. Fixes bug 6379.
+      Bugfix on 0.2.0.20-rc.
+
diff --git a/changes/bug6404 b/changes/bug6404
new file mode 100644
index 0000000000..948f00b92e
--- /dev/null
+++ b/changes/bug6404
@@ -0,0 +1,16 @@
+  o Minor bugfixes:
+
+    - Remove the maximum length of microdescriptor we are willing to
+      generate. Occasionally this is needed for routers
+      with complex policies or family declarations. Partial fix for
+      bug 6404; fix on 0.2.2.6-alpha.
+
+    - Authorities no longer include any router in their
+      microdescriptor consensuses for which they couldn't generate or
+      agree on a microdescriptor.  Partial fix for bug 6404; fix on
+      0.2.2.6-alpha.
+
+    - Move log message when unable to find a microdesc in a
+      routerstatus entry to parse time. Previously we'd spam this
+      warning every time we tried to figure out which microdescriptors
+      to download. Partial fix for bug 6404; fix on 0.2.3.18-rc.
diff --git a/changes/bug6423 b/changes/bug6423
new file mode 100644
index 0000000000..2ea4f1410d
--- /dev/null
+++ b/changes/bug6423
@@ -0,0 +1,3 @@
+ o Minor features:
+   - Consider new, removed or changed IPv6 OR ports a non cosmetic
+     change.
diff --git a/changes/bug6472 b/changes/bug6472
new file mode 100644
index 0000000000..dcd42ebe68
--- /dev/null
+++ b/changes/bug6472
@@ -0,0 +1,4 @@
+  o Minor bugfixes:
+    - Avoid a pair of double-free and use-after-mark bugs that can
+      occur with certain timings in canceled and re-received DNS
+      requests. Fix for bug 6472; bugfix on 0.0.7rc1.
diff --git a/changes/bug6475 b/changes/bug6475
new file mode 100644
index 0000000000..67bab99622
--- /dev/null
+++ b/changes/bug6475
@@ -0,0 +1,6 @@
+  o Minor bugfixes:
+     - Add internal circuit construction state to protect against
+       the noisy warn message "Unexpectedly high circuit_successes".
+       Also add some additional rate-limited notice messages to help
+       determine the root cause of the warn. Fixes bug 6475.
+       Bugfix against 0.2.3.17-beta.
diff --git a/changes/bug6507 b/changes/bug6507
new file mode 100644
index 0000000000..89940cbf7b
--- /dev/null
+++ b/changes/bug6507
@@ -0,0 +1,15 @@
+  o Major bugfixes:
+    - Detect 'ORPort 0' as meaning, uniformly, that we're not running
+      as a server. Previously, some of our code would treat the
+      presence of any ORPort line as meaning that we should act like a
+      server, even though our new listener code would correctly not
+      open any ORPorts for ORPort 0. Similar bugs in other Port
+      options are also fixed. Fixes bug 6507; bugfix on 0.2.3.3-alpha.
+
+  o Minor features:
+
+    - Detect and reject attempts to specify both 'FooPort' and
+      'FooPort 0' in the same configuration domain.  (It's still okay
+      to have a FooPort in your configuration file,and use 'FooPort 0'
+      on the command line to disable it.) Fixes another case of
+      bug6507; bugfix on 0.2.3.3-alpha.
diff --git a/changes/bug6514 b/changes/bug6514
new file mode 100644
index 0000000000..84633bd279
--- /dev/null
+++ b/changes/bug6514
@@ -0,0 +1,5 @@
+  o Minor bugfixes:
+    - Add a (probably redundant) memory clear between iterations of
+      the router status voting loop, to prevent future coding errors
+      where data might leak between iterations of the loop. Resolves
+      ticket 6514.
diff --git a/changes/bug6690 b/changes/bug6690
new file mode 100644
index 0000000000..99d42976ed
--- /dev/null
+++ b/changes/bug6690
@@ -0,0 +1,7 @@
+  o Major bugfixes (security):
+    - Do not crash when comparing an address with port value 0 to an
+      address policy. This bug could have been used to cause a remote
+      assertion failure by or against directory authorities, or to
+      allow some applications to crash clients. Fixes bug 6690; bugfix
+      on 0.2.1.10-alpha.
+
diff --git a/changes/bug6710 b/changes/bug6710
new file mode 100644
index 0000000000..2c89346114
--- /dev/null
+++ b/changes/bug6710
@@ -0,0 +1,6 @@
+  o Major bugfixes (security):
+    - Reject any attempt to extend to an internal address. Without
+      this fix, a router could be used to probe addresses on an
+      internal network to see whether they were accepting
+      connections. Fix for bug 6710; bugfix on 0.0.8pre1.
+
diff --git a/changes/bug6732 b/changes/bug6732
new file mode 100644
index 0000000000..7a744e014a
--- /dev/null
+++ b/changes/bug6732
@@ -0,0 +1,3 @@
+  o Documentation:
+    - Add missing documentation for consensus and microdesc files. Fix for
+      bug 6732.
diff --git a/changes/disable_pathbias_messages b/changes/disable_pathbias_messages
new file mode 100644
index 0000000000..3bc996347b
--- /dev/null
+++ b/changes/disable_pathbias_messages
@@ -0,0 +1,3 @@
+  o Disabeled features
+    - Downgrade path-bias warning messages to INFO. We'll try to get them
+      working better in 0.2.4. Fixes bug 6475; bugfix on 0.2.3.17-beta.
diff --git a/configure.in b/configure.in
index c85930948f..a66ae21ba1 100644
--- a/configure.in
+++ b/configure.in
@@ -300,6 +300,7 @@ AC_CHECK_FUNCS(
         gmtime_r \
         inet_aton \
         ioctl \
+        issetugid \
         localtime_r \
         lround \
         memmem \
diff --git a/doc/tor.1.txt b/doc/tor.1.txt
index e7ba8485c0..62259b7268 100644
--- a/doc/tor.1.txt
+++ b/doc/tor.1.txt
@@ -1470,6 +1470,11 @@ is non-zero):
     its extra-info documents that it uploads to the directory authorities.
     (Default: 1)
 
+**ExtendAllowPrivateAddresses** **0**|**1**::
+    When this option is enabled, Tor routers allow EXTEND request to
+    localhost, RFC1918 addresses, and so on. This can create security issues;
+    you should probably leave it off. (Default: 0)
+
 DIRECTORY SERVER OPTIONS
 ------------------------
 
@@ -1795,6 +1800,7 @@ The following options are used for running a testing Tor network.
        ClientRejectInternalAddresses 0
        CountPrivateBandwidth 1
        ExitPolicyRejectPrivate 0
+       ExtendAllowPrivateAddresses 1
        V3AuthVotingInterval 5 minutes
        V3AuthVoteDelay 20 seconds
        V3AuthDistDelay 20 seconds
@@ -1876,7 +1882,10 @@ FILES
 __DataDirectory__**/cached-status/**::
     The most recently downloaded network status document for each authority.
     Each file holds one such document; the filenames are the hexadecimal
-    identity key fingerprints of the directory authorities.
+    identity key fingerprints of the directory authorities.  Mostly obsolete.
+
+__DataDirectory__**/cached-consensus** and/or **cached-microdesc-consensus**::
+    The most recent consensus network status document we've downloaded.
 
 __DataDirectory__**/cached-descriptors** and **cached-descriptors.new**::
     These files hold downloaded router statuses. Some routers may appear more
@@ -1885,6 +1894,12 @@ __DataDirectory__**/cached-descriptors** and **cached-descriptors.new**::
     a given router. The ".new" file is an append-only journal; when it gets
     too large, all entries are merged into a new cached-descriptors file.
 
+__DataDirectory__**/cached-microdescs** and **cached-microdescs.new**::
+    These files hold downloaded microdescriptors.  Lines beginning with
+    @-signs are annotations that contain more information about a given
+    router. The ".new" file is an append-only journal; when it gets too
+    large, all entries are merged into a new cached-microdescs file.
+
 __DataDirectory__**/cached-routers** and **cached-routers.new**::
     Obsolete versions of cached-descriptors and cached-descriptors.new. When
     Tor can't find the newer files, it looks here instead.
diff --git a/src/common/OpenBSD_malloc_Linux.c b/src/common/OpenBSD_malloc_Linux.c
index 92ca9c0066..da82729811 100644
--- a/src/common/OpenBSD_malloc_Linux.c
+++ b/src/common/OpenBSD_malloc_Linux.c
@@ -14,6 +14,10 @@
  * ----------------------------------------------------------------------------
  */
 
+/* We use this macro to remove some code that we don't actually want,
+ * rather than to fix its warnings. */
+#define BUILDING_FOR_TOR
+
 /*
  * Defining MALLOC_EXTRA_SANITY will enable extra checks which are
  * related to internal conditions and consistency in malloc.c. This has
@@ -79,6 +83,7 @@ static size_t g_alignment = 0;
 
 extern int __libc_enable_secure;
 
+#ifndef HAVE_ISSETUGID
 static int issetugid(void)
 {
 	if (__libc_enable_secure) return 1;
@@ -86,8 +91,10 @@ static int issetugid(void)
 	if (getgid() != getegid()) return 1;
 	return 0;
 }
+#endif
 
 #define PGSHIFT 12
+#undef MADV_FREE
 #define MADV_FREE MADV_DONTNEED
 #include <pthread.h>
 static pthread_mutex_t gen_mutex = PTHREAD_MUTEX_INITIALIZER;
@@ -443,6 +450,7 @@ extern char	*__progname;
 static void
 wrterror(const char *p)
 {
+#ifndef BUILDING_FOR_TOR
 	const char		*q = " error: ";
 	struct iovec	iov[5];
 
@@ -457,7 +465,9 @@ wrterror(const char *p)
 	iov[4].iov_base = (char*)"\n";
 	iov[4].iov_len = 1;
 	writev(STDERR_FILENO, iov, 5);
-
+#else
+        (void)p;
+#endif
 	suicide = 1;
 #ifdef MALLOC_STATS
 	if (malloc_stats)
@@ -471,14 +481,17 @@ wrterror(const char *p)
 static void
 wrtwarning(const char *p)
 {
+#ifndef BUILDING_FOR_TOR
 	const char		*q = " warning: ";
 	struct iovec	iov[5];
+#endif
 
 	if (malloc_abort)
 		wrterror(p);
 	else if (malloc_silent)
 		return;
 
+#ifndef BUILDING_FOR_TOR
 	iov[0].iov_base = __progname;
 	iov[0].iov_len = strlen(__progname);
 	iov[1].iov_base = (char*)malloc_func;
@@ -489,8 +502,11 @@ wrtwarning(const char *p)
 	iov[3].iov_len = strlen(p);
 	iov[4].iov_base = (char*)"\n";
 	iov[4].iov_len = 1;
-	
-	writev(STDERR_FILENO, iov, 5);
+
+	(void) writev(STDERR_FILENO, iov, 5);
+#else
+        (void)p;
+#endif
 }
 
 #ifdef MALLOC_STATS
@@ -665,7 +681,7 @@ malloc_init(void)
 	for (i = 0; i < 3; i++) {
 		switch (i) {
 		case 0:
-			j = readlink("/etc/malloc.conf", b, sizeof b - 1);
+			j = (int) readlink("/etc/malloc.conf", b, sizeof b - 1);
 			if (j <= 0)
 				continue;
 			b[j] = '\0';
@@ -1145,9 +1161,10 @@ malloc_bytes(size_t size)
 	if (size == 0)
 		j = 0;
 	else {
+		size_t ii;
 		j = 1;
-		i = size - 1;
-		while (i >>= 1)
+		ii = size - 1;
+		while (ii >>= 1)
 			j++;
 	}
 
@@ -1971,6 +1988,7 @@ calloc(size_t num, size_t size)
 	return(p);
 }
 
+#ifndef BUILDING_FOR_TOR
 static int ispowerof2 (size_t a) {
 	size_t b;
 	for (b = 1ULL << (sizeof(size_t)*NBBY - 1); b > 1; b >>= 1)
@@ -1978,7 +1996,9 @@ static int ispowerof2 (size_t a) {
 		return 1;
 	return 0;
 }
+#endif
 
+#ifndef BUILDING_FOR_TOR
 int posix_memalign(void **memptr, size_t alignment, size_t size)
 {
 	void *r;
@@ -2015,18 +2035,20 @@ void *valloc(size_t size)
 	posix_memalign(&r, malloc_pagesize, size);
 	return r;
 }
+#endif
 
 size_t malloc_good_size(size_t size)
 {
 	if (size == 0) {
 		return 1;
 	} else if (size <= malloc_maxsize) {
-		int i, j;
+		int j;
+		size_t ii;
 		/* round up to the nearest power of 2, with same approach
 		 * as malloc_bytes() uses. */
 		j = 1;
-		i = size - 1;
-		while (i >>= 1)
+		ii = size - 1;
+		while (ii >>= 1)
 			j++;
 		return ((size_t)1) << j;
 	} else {
diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c
index b82cce9881..e5576018a6 100644
--- a/src/or/circuitbuild.c
+++ b/src/or/circuitbuild.c
@@ -135,6 +135,9 @@ static entry_guard_t *entry_guard_get_by_id_digest(const char *digest);
 
 static void bridge_free(bridge_info_t *bridge);
 
+static int entry_guard_inc_first_hop_count(entry_guard_t *guard);
+static void pathbias_count_success(origin_circuit_t *circ);
+
 /**
  * This function decides if CBT learning should be disabled. It returns
  * true if one or more of the following four conditions are met:
@@ -1624,7 +1627,7 @@ circuit_build_times_set_timeout(circuit_build_times_t *cbt)
     return;
 
   if (cbt->timeout_ms < circuit_build_times_min_timeout()) {
-    log_warn(LD_CIRC, "Set buildtimeout to low value %fms. Setting to %dms",
+    log_info(LD_CIRC, "Set buildtimeout to low value %fms. Setting to %dms",
              cbt->timeout_ms, circuit_build_times_min_timeout());
     cbt->timeout_ms = circuit_build_times_min_timeout();
     if (cbt->close_ms < cbt->timeout_ms) {
@@ -2285,28 +2288,11 @@ circuit_send_next_onion_skin(origin_circuit_t *circ)
       }
       log_info(LD_CIRC,"circuit built!");
       circuit_reset_failure_count(0);
-      /* Don't count cannibalized or onehop circs for path bias */
+
       if (circ->build_state->onehop_tunnel || circ->has_opened) {
         control_event_bootstrap(BOOTSTRAP_STATUS_REQUESTING_STATUS, 0);
-      } else {
-        entry_guard_t *guard =
-          entry_guard_get_by_id_digest(circ->_base.n_conn->identity_digest);
-
-        if (guard) {
-          guard->circuit_successes++;
-
-          log_info(LD_PROTOCOL, "Got success count %u/%u for guard %s=%s",
-                   guard->circuit_successes, guard->first_hops,
-                   guard->nickname, hex_str(guard->identity, DIGEST_LEN));
-
-          if (guard->first_hops < guard->circuit_successes) {
-            log_warn(LD_BUG, "Unexpectedly high circuit_successes (%u/%u) "
-                     "for guard %s",
-                     guard->circuit_successes, guard->first_hops,
-                     guard->nickname);
-          }
-        }
       }
+
       if (!can_complete_circuit && !circ->build_state->onehop_tunnel) {
         const or_options_t *options = get_options();
         can_complete_circuit=1;
@@ -2322,6 +2308,8 @@ circuit_send_next_onion_skin(origin_circuit_t *circ)
           consider_testing_reachability(1, 1);
         }
       }
+
+      pathbias_count_success(circ);
       circuit_rep_hist_note_result(circ);
       circuit_has_opened(circ); /* do other actions as necessary */
 
@@ -2444,6 +2432,13 @@ circuit_extend(cell_t *cell, circuit_t *circ)
     return -1;
   }
 
+  if (tor_addr_is_internal(&n_addr, 0) &&
+      !get_options()->ExtendAllowPrivateAddresses) {
+    log_fn(LOG_PROTOCOL_WARN, LD_PROTOCOL,
+           "Client asked me to extend to a private address");
+    return -1;
+  }
+
   /* Check if they asked us for 0000..0000. We support using
    * an empty fingerprint for the first hop (e.g. for a bridge relay),
    * but we don't want to let people send us extend cells for empty
@@ -2621,6 +2616,194 @@ pathbias_get_scale_factor(const or_options_t *options)
                                 DFLT_PATH_BIAS_SCALE_THRESHOLD, 1, INT32_MAX);
 }
 
+static const char *
+pathbias_state_to_string(path_state_t state)
+{
+  switch (state) {
+    case PATH_STATE_NEW_CIRC:
+      return "new";
+    case PATH_STATE_DID_FIRST_HOP:
+      return "first hop";
+    case PATH_STATE_SUCCEEDED:
+      return "succeeded";
+  }
+
+  return "unknown";
+}
+
+/**
+ * Check our circuit state to see if this is a successful first hop.
+ * If so, record it in the current guard's path bias first_hop count.
+ *
+ * Also check for several potential error cases for bug #6475.
+ */
+static int
+pathbias_count_first_hop(origin_circuit_t *circ)
+{
+#define FIRST_HOP_NOTICE_INTERVAL (600)
+  static ratelim_t first_hop_notice_limit =
+    RATELIM_INIT(FIRST_HOP_NOTICE_INTERVAL);
+  char *rate_msg = NULL;
+
+  /* Completely ignore one hop circuits */
+  if (circ->build_state->onehop_tunnel) {
+    tor_assert(circ->build_state->desired_path_len == 1);
+    return 0;
+  }
+
+  if (circ->cpath->state == CPATH_STATE_AWAITING_KEYS) {
+    /* Help track down the real cause of bug #6475: */
+    if (circ->has_opened && circ->path_state != PATH_STATE_DID_FIRST_HOP) {
+      if ((rate_msg = rate_limit_log(&first_hop_notice_limit,
+                                     approx_time()))) {
+        log_info(LD_BUG,
+                "Opened circuit is in strange path state %s. "
+                "Circuit is a %s currently %s. %s",
+                pathbias_state_to_string(circ->path_state),
+                circuit_purpose_to_string(circ->_base.purpose),
+                circuit_state_to_string(circ->_base.state),
+                rate_msg);
+      }
+    }
+
+    /* Don't count cannibalized circs for path bias */
+    if (!circ->has_opened) {
+      entry_guard_t *guard;
+
+      guard = entry_guard_get_by_id_digest(
+              circ->_base.n_conn->identity_digest);
+      if (guard) {
+        if (circ->path_state == PATH_STATE_NEW_CIRC) {
+          circ->path_state = PATH_STATE_DID_FIRST_HOP;
+
+          if (entry_guard_inc_first_hop_count(guard) < 0) {
+            /* Bogus guard; we already warned. */
+            return -END_CIRC_REASON_TORPROTOCOL;
+          }
+        } else {
+          if ((rate_msg = rate_limit_log(&first_hop_notice_limit,
+                  approx_time()))) {
+            log_info(LD_BUG,
+                   "Unopened circuit has strange path state %s. "
+                   "Circuit is a %s currently %s. %s",
+                   pathbias_state_to_string(circ->path_state),
+                   circuit_purpose_to_string(circ->_base.purpose),
+                   circuit_state_to_string(circ->_base.state),
+                   rate_msg);
+          }
+        }
+      } else {
+        if ((rate_msg = rate_limit_log(&first_hop_notice_limit,
+                approx_time()))) {
+          log_info(LD_BUG,
+              "Unopened circuit has no known guard. "
+              "Circuit is a %s currently %s. %s",
+              circuit_purpose_to_string(circ->_base.purpose),
+              circuit_state_to_string(circ->_base.state),
+              rate_msg);
+        }
+      }
+    }
+  } else {
+    /* Help track down the real cause of bug #6475: */
+    if (circ->path_state == PATH_STATE_NEW_CIRC) {
+      if ((rate_msg = rate_limit_log(&first_hop_notice_limit,
+                approx_time()))) {
+        log_info(LD_BUG,
+            "A %s circuit is in cpath state %d (opened: %d). "
+            "Circuit is a %s currently %s. %s",
+            pathbias_state_to_string(circ->path_state),
+            circ->cpath->state, circ->has_opened,
+            circuit_purpose_to_string(circ->_base.purpose),
+            circuit_state_to_string(circ->_base.state),
+            rate_msg);
+      }
+    }
+  }
+
+  return 0;
+}
+
+/**
+ * Check our circuit state to see if this is a successful circuit
+ * completion. If so, record it in the current guard's path bias
+ * success count.
+ *
+ * Also check for several potential error cases for bug #6475.
+ */
+static void
+pathbias_count_success(origin_circuit_t *circ)
+{
+#define SUCCESS_NOTICE_INTERVAL (600)
+  static ratelim_t success_notice_limit =
+    RATELIM_INIT(SUCCESS_NOTICE_INTERVAL);
+  char *rate_msg = NULL;
+
+  /* Ignore one hop circuits */
+  if (circ->build_state->onehop_tunnel) {
+    tor_assert(circ->build_state->desired_path_len == 1);
+    return;
+  }
+
+  /* Don't count cannibalized/reused circs for path bias */
+  if (!circ->has_opened) {
+    entry_guard_t *guard =
+      entry_guard_get_by_id_digest(circ->_base.n_conn->identity_digest);
+
+    if (guard) {
+      if (circ->path_state == PATH_STATE_DID_FIRST_HOP) {
+        circ->path_state = PATH_STATE_SUCCEEDED;
+        guard->circuit_successes++;
+
+        log_info(LD_PROTOCOL, "Got success count %u/%u for guard %s=%s",
+                 guard->circuit_successes, guard->first_hops,
+                 guard->nickname, hex_str(guard->identity, DIGEST_LEN));
+      } else {
+        if ((rate_msg = rate_limit_log(&success_notice_limit,
+                approx_time()))) {
+          log_info(LD_BUG,
+              "Succeeded circuit is in strange path state %s. "
+              "Circuit is a %s currently %s. %s",
+              pathbias_state_to_string(circ->path_state),
+              circuit_purpose_to_string(circ->_base.purpose),
+              circuit_state_to_string(circ->_base.state),
+              rate_msg);
+        }
+      }
+
+      if (guard->first_hops < guard->circuit_successes) {
+        log_info(LD_BUG, "Unexpectedly high circuit_successes (%u/%u) "
+                 "for guard %s=%s",
+                 guard->circuit_successes, guard->first_hops,
+                 guard->nickname, hex_str(guard->identity, DIGEST_LEN));
+      }
+    } else {
+      if ((rate_msg = rate_limit_log(&success_notice_limit,
+              approx_time()))) {
+        log_info(LD_BUG,
+            "Completed circuit has no known guard. "
+            "Circuit is a %s currently %s. %s",
+            circuit_purpose_to_string(circ->_base.purpose),
+            circuit_state_to_string(circ->_base.state),
+            rate_msg);
+      }
+    }
+  } else {
+    if (circ->path_state != PATH_STATE_SUCCEEDED) {
+      if ((rate_msg = rate_limit_log(&success_notice_limit,
+              approx_time()))) {
+        log_info(LD_BUG,
+            "Opened circuit is in strange path state %s. "
+            "Circuit is a %s currently %s. %s",
+            pathbias_state_to_string(circ->path_state),
+            circuit_purpose_to_string(circ->_base.purpose),
+            circuit_state_to_string(circ->_base.state),
+            rate_msg);
+      }
+    }
+  }
+}
+
 /** Increment the number of times we successfully extended a circuit to
  * 'guard', first checking if the failure rate is high enough that we should
  * eliminate the guard.  Return -1 if the guard looks no good; return 0 if the
@@ -2639,7 +2822,7 @@ entry_guard_inc_first_hop_count(entry_guard_t *guard)
     if (guard->circuit_successes/((double)guard->first_hops)
         < pathbias_get_disable_rate(options)) {
 
-      log_warn(LD_PROTOCOL,
+      log_info(LD_PROTOCOL,
                "Extremely low circuit success rate %u/%u for guard %s=%s. "
                "This might indicate an attack, or a bug.",
                guard->circuit_successes, guard->first_hops, guard->nickname,
@@ -2652,7 +2835,7 @@ entry_guard_inc_first_hop_count(entry_guard_t *guard)
                < pathbias_get_notice_rate(options)
                && !guard->path_bias_notice) {
       guard->path_bias_notice = 1;
-      log_notice(LD_PROTOCOL,
+      log_info(LD_PROTOCOL,
                  "Low circuit success rate %u/%u for guard %s=%s.",
                  guard->circuit_successes, guard->first_hops, guard->nickname,
                  hex_str(guard->identity, DIGEST_LEN));
@@ -2666,8 +2849,9 @@ entry_guard_inc_first_hop_count(entry_guard_t *guard)
     guard->circuit_successes /= scale_factor;
   }
   guard->first_hops++;
-  log_info(LD_PROTOCOL, "Got success count %u/%u for guard %s",
-           guard->circuit_successes, guard->first_hops, guard->nickname);
+  log_info(LD_PROTOCOL, "Got success count %u/%u for guard %s=%s",
+           guard->circuit_successes, guard->first_hops, guard->nickname,
+           hex_str(guard->identity, DIGEST_LEN));
   return 0;
 }
 
@@ -2687,22 +2871,13 @@ circuit_finish_handshake(origin_circuit_t *circ, uint8_t reply_type,
 {
   char keys[CPATH_KEY_MATERIAL_LEN];
   crypt_path_t *hop;
+  int rv;
+
+  if ((rv = pathbias_count_first_hop(circ)) < 0)
+    return rv;
 
   if (circ->cpath->state == CPATH_STATE_AWAITING_KEYS) {
     hop = circ->cpath;
-    /* Don't count cannibalized or onehop circs for path bias */
-    if (!circ->has_opened && !circ->build_state->onehop_tunnel) {
-      entry_guard_t *guard;
-
-      guard = entry_guard_get_by_id_digest(
-              circ->_base.n_conn->identity_digest);
-      if (guard) {
-        if (entry_guard_inc_first_hop_count(guard) < 0) {
-          /* Bogus guard; we already warned. */
-          return -END_CIRC_REASON_TORPROTOCOL;
-        }
-      }
-    }
   } else {
     hop = onion_next_hop_in_cpath(circ->cpath);<